feat(state): canonical fingerprint commitment for native module state (SRC-20 + NFT)#776
Merged
Merged
Conversation
Native-module state (SRC-20 ContractRegistry + NFT NftRegistry) was absent from the STATE-FP fingerprint, so two validators could diverge purely in token/NFT state and still print identical [STATE-FP] lines (CodeRabbit #1 on #775). Close the debug-visibility gap consistently for both native modules with canonical, HashMap-order-independent hashing. - NftCollection/NftRegistry: canonical_hash() — fixed field order, all maps folded with keys sorted (read-only, no new mutable surface). - SRC20Contract/ContractRegistry: canonical_hash() — same approach. - block_executor: extract compute_state_fingerprint() (testable, no env gate) and fold both registries' canonical_hash into the combined fp. - nft apply path: validate to/from/spender/operator addresses via is_spendable/is_valid_sentrix_address before mutating (CodeRabbit #3), mirroring SRC-20 M-02. Scope: this is fingerprint (debug) commitment only. Consensus-enforced trie/state_root commitment of native-module state is the heavier fork-gated + migrated + soaked follow-up; canonical serialization here is its foundation. No state_root/trie/MDBX change, no fork gate, no consensus impact. Event-staging refactor (CodeRabbit #2, shared with SRC-20) stays deferred — fixing NFT alone would diverge from the SRC-20 precedent. Tests: sentrix-nft canonical_hash (order-independence, state sensitivity, owner distinction, empty stability); sentrix-core fingerprint tracks SRC-20 + NFT changes, replay-deterministic, order-independent, distinct states differ; invalid-address rejection at the apply boundary.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Warning Review limit reached
More reviews will be available in 16 minutes and 35 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
satyakwok
added a commit
that referenced
this pull request
Jun 3, 2026
Patch coverage on #776 flagged 15 uncovered lines in collection.rs canonical_hash — the branches the existing tests didn't hit: bounded max_supply (Some arm), token uri_hash/metadata_hash (Some arm), and the operator-approvals loop. - lib.rs: black-box test covering Some(max_supply), non-empty token URI, single-token approval, and operator-for-all (toggle moves the hash). - collection.rs: white-box test for uri_hash/metadata_hash — these integrity hashes have no public setter yet, so the Some arm is only reachable by setting the fields directly on a live token. canonical_hash is now fully covered in both collection.rs and registry.rs.
github-actions Bot
pushed a commit
that referenced
this pull request
Jun 3, 2026
Patch coverage on #776 flagged 15 uncovered lines in collection.rs canonical_hash — the branches the existing tests didn't hit: bounded max_supply (Some arm), token uri_hash/metadata_hash (Some arm), and the operator-approvals loop. - lib.rs: black-box test covering Some(max_supply), non-empty token URI, single-token approval, and operator-for-all (toggle moves the hash). - collection.rs: white-box test for uri_hash/metadata_hash — these integrity hashes have no public setter yet, so the Some arm is only reachable by setting the fields directly on a live token. canonical_hash is now fully covered in both collection.rs and registry.rs.
This was referenced Jun 3, 2026
satyakwok
added a commit
that referenced
this pull request
Jun 3, 2026
…ted) (#779) * feat(state): commit native module state into trie/state_root (fork-gated) Native-module state (SRC-20 ContractRegistry + NFT NftRegistry) was committed only to the STATE-FP fingerprint (#776), not the consensus state_root — so a divergent registry was not rejected by peers. Commit both registries' canonical hashes into the state trie behind a new fork gate, mirroring the proven SIP-6 STATE_IN_TRIE pattern. - new gate NATIVE_STATE_IN_TRIE_HEIGHT, default u64::MAX on BOTH nets. Deliberately NOT reusing STATE_IN_TRIE_HEIGHT (already active on testnet at 6,026,000) — reusing it would retroactively fork testnet's state_root. - two fixed trie keys: sentrix/v1/native_src20_registry + sentrix/v1/native_nft_registry; value = registry canonical_hash (sorted, HashMap-order-independent — from #776). Always-insert post-fork (empty registry has a stable hash), same semantics as total_minted/epoch. - update_trie_for_block captures both hashes before the trie mut-borrow (Phase 2f), independent of the SIP-6 gate. Pre-fork: state_root is bit-identical to today (native state stays off-trie). Post-fork: SRC-20 / NFT state changes move state_root. Commitment model: the trie stores the registry canonical HASH, not the full registry — divergence is detected (peers reject the block); recovery is via peer resync (the blob remains the data source). Activation requires halt-all + state-root-alignment pre-flight + simul-start so every validator commits identical native state at the activation block (STATE_IN_TRIE playbook). NFT_TOKENOP_HEIGHT stays disabled — the NFT registry commits as empty until it is enabled. Tests: pre-fork preserved; post-fork SRC-20 + NFT changes move state_root; replay-deterministic; different supply/owner differ; deploy-order independent; empty commitment stable; fork-gate default-disabled on both nets + activates at pinned height. * test(trie): cover native registry key builders codecov/patch on #779 flagged 10 uncovered lines (0%) in sentrix-trie/src/address.rs: the two new native_*_registry_key builders. They're exercised by sentrix-core's integration tests, but codecov measures sentrix-trie in its own package run where its unit tests didn't call them. Add a unit test asserting both keys are deterministic, mutually distinct, and distinct from total_minted/epoch/account keys.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft to prevent auto-merge — do not merge; for review only.
Closes the native-module state-commitment gap for both SRC-20
ContractRegistryand NFTNftRegistry, and fixes 2 of the 3 CodeRabbit findings from #775.Problem
SENTRIX_STATE_FINGERPRINT(STATE-FP) hashed only accounts + total_minted + EVM code/storage. Native-module state (SRC-20 contracts, NFT registry) was invisible → two validators could diverge purely in token/NFT state and print identical[STATE-FP]lines (CodeRabbit #1).Change — canonical fingerprint commitment
NftCollection/NftRegistry::canonical_hash()— fixed field order; all maps (tokens, balances, token_approvals, operator_approvals, collections) folded with keys sorted → HashMap-order-independent. Read-only; no new mutable surface on the domain crate.SRC20Contract/ContractRegistry::canonical_hash()— same approach (balances + allowances sorted).compute_state_fingerprint()— extracted fromemit_state_fingerprint(testable, no env gate); foldscontracts.canonical_hash()+nft_registry.canonical_hash()into the combined fingerprint.to/from/spender/operatorviais_spendable_/is_valid_sentrix_addressbefore mutating, mirroring SRC-20 M-02.Design decision (explicit)
Requirements allow "state_root OR fingerprint." I took the fingerprint arm:
Deferred (documented)
Tests
canonical_hashorder-independence, state sensitivity (deploy/mint/approve/transfer/burn), owner distinction, empty stability.Commands
cargo test -p sentrix-nft✅ (43 + 2 doctests)cargo test --workspace✅ (66 suites, 0 failures)cargo fmt --check✅ ·cargo clippy --workspace --all-targets -- -D warnings✅cargo llvm-cov -p sentrix-nft✅ 92.13% linesNo deploy. No RPC/explorer/wallet/marketplace/bridge/image-bytes. No state_root/trie/MDBX change, no fork gate.