Skip to content

Fix/shielded pool hardening - #176

Merged
nol4lej merged 3 commits into
mainfrom
fix/shielded-pool-hardening
Oct 22, 2026
Merged

nol4lej merged 3 commits into
mainfrom
fix/shielded-pool-hardening

Conversation

@nol4lej

@nol4lej nol4lej commented Oct 22, 2026

Copy link
Copy Markdown
Member

Summary

Two shielded-pool fixes for spend admission and execution, shipped in spec 20 with pallet-shielded-pool 0.25.2.

  • A replaced root stays spendable for a full retention window. The active root never expires, so a wallet can prove against a root older than RootRetentionBlocks. Its window used to count from when the root was created, so the next leaf insert, from anyone's shield, made it unknown at once. A spend proven a moment earlier was then refused with UnknownMerkleRoot (pool code 1). Now the root is recorded again when it is replaced, with now + RootRetentionBlocks.
  • Notes a spend creates are tagged in the pool. Pool tags covered only the nullifiers being spent. Two spends of different notes creating the same output note were both admitted and propagated, though only one can execute. Each transfer output and each non-zero unshield change commitment now provides a tag, so such spends are mutually exclusive in the pool, as the chain makes them.

Changes

  • merkle/service.rs: insert_leaf calls retire_active_root on the outgoing root. It is re-queued only when its expiry is shorter than a full window from now, which happens only for a root created in an earlier block. Cost: at most one extra historic-root slot per block.
  • validate_unsigned/{transfer,unshield}.rs: one tag per created note, (OUTPUT_TAG, commitment). The tag never encodes like a bare nullifier tag, so a note one spend creates cannot block an unrelated spend whose nullifier has the same bytes.
  • validate_unsigned/mod.rs: the doc comment no longer suggests that peer reputation limits invalid-proof spam. An invalid proof costs the sending peer too little reputation to get it banned, so validators should take transactions only from their sentries (--reserved-only).
  • pallet-shielded-pool 0.25.2, CHANGELOG, and RUNTIME_VERSIONS.md (spec 20).

Testing

  • Unit tests: 389 pass. New tests:
    • a_replaced_root_counts_its_window_from_its_replacement
    • replacing_a_root_from_the_same_block_queues_nothing_extra
    • transfers_creating_the_same_note_are_mutually_exclusive
    • an_unshield_change_note_is_tagged_like_a_transfer_output
    • an_output_tag_never_collides_with_a_nullifier_tag
  • Mutation checks:
    • Removing the root refresh fails the root tests.
    • Removing the output tags fails three admission tests.
    • Tagging outputs without the marker fails the collision test.
  • Dev node with real Groth16 proofs, each case against the previous binary as a control:
Scenario Before After
Spends proven against an active root older than 300 blocks, root then replaced by a shield: land; one still lands ~296 blocks after the replacement; refused after 300 5/10 10/10
Same scenario on a 3-node network: validator --reserved-only behind a sentry, spends sent through a public node — 10/10
Two transfers into the same output note; partial unshield and transfer creating the same note; unrelated spends unaffected 11/15 15/15
Same on the 3-node network, plus the two spends sent to two different nodes at once (validator pool must hold one) 13/19 19/19
Validator --reserved-only behind a sentry: peers with the sentry alone, refuses an outside node, receives no invalid-proof spend from public nodes, legit spends still land — 8/8

Notes

  • Weights: the extra historic-root slot is not yet in the benchmarked weights. It is at most one read and a few writes per block, and is left for the pending re-benchmark.
  • Deployment: orbinum/node-deploy documents validators behind sentries (--reserved-only) and the RPC node's sentry role.

… window from its replacement, not its creation — the next insert no longer refuses spends proven against an old active root
…unshield change) in the pool, so two spends creating the same note are mutually exclusive / an invalid proof costs the sending peer too little reputation to get it banned — validators should take transactions only from their sentries
…roots keep a full retention window, pool tags for created notes (spec 20)
@nol4lej
nol4lej merged commit 0cc9189 into main Oct 22, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant