From a98ac2ca0784009b168f70fafef54147de7be51f Mon Sep 17 00:00:00 2001 From: nol4lej Date: Wed, 21 Oct 2026 22:20:36 -0300 Subject: [PATCH 1/3] =?UTF-8?q?fix(shielded-pool):=20a=20replaced=20root?= =?UTF-8?q?=20stays=20spendable=20for=20the=20retention=20window=20from=20?= =?UTF-8?q?its=20replacement,=20not=20its=20creation=20=E2=80=94=20the=20n?= =?UTF-8?q?ext=20insert=20no=20longer=20refuses=20spends=20proven=20agains?= =?UTF-8?q?t=20an=20old=20active=20root?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- frame/shielded-pool/CHANGELOG.md | 8 ++++ frame/shielded-pool/src/merkle/mod.rs | 45 +++++++++++++++++++++++ frame/shielded-pool/src/merkle/service.rs | 17 +++++++++ 3 files changed, 70 insertions(+) diff --git a/frame/shielded-pool/CHANGELOG.md b/frame/shielded-pool/CHANGELOG.md index 94c57b8d..155a86e8 100644 --- a/frame/shielded-pool/CHANGELOG.md +++ b/frame/shielded-pool/CHANGELOG.md @@ -4,6 +4,14 @@ All notable changes to `pallet-shielded-pool` will be documented in this file. ## [Unreleased] +### Fixed + +- A root replaced by a leaf insert stays spendable for `RootRetentionBlocks` + from its replacement, not from its creation. The active root never expires, + so a wallet may prove against one older than the window; the next insert + used to refuse that spend at once with `UnknownMerkleRoot`. Costs at most one + extra historic-root slot per block. + ## [0.25.1] - 2026-10-21 ### Changed diff --git a/frame/shielded-pool/src/merkle/mod.rs b/frame/shielded-pool/src/merkle/mod.rs index beedd8e8..b14bf90f 100644 --- a/frame/shielded-pool/src/merkle/mod.rs +++ b/frame/shielded-pool/src/merkle/mod.rs @@ -1018,6 +1018,51 @@ mod tests { }); } + /// An active root older than the window, replaced by an insert, stays + /// spendable for a full window from its replacement: a wallet that proved + /// against it is not refused by the next insert. + #[test] + fn a_replaced_root_counts_its_window_from_its_replacement() { + new_test_ext().execute_with(|| { + let start = System::block_number(); + MerkleTreeService::insert_leaf::(Commitment::new([0x11u8; 32])).unwrap(); + let old = MerkleRepository::get_poseidon_root::(); + + // Active for longer than a window, then replaced. + let replaced_at = start + retention() * 3; + System::set_block_number(replaced_at); + MerkleTreeService::insert_leaf::(Commitment::new([0x22u8; 32])).unwrap(); + assert_ne!(MerkleRepository::get_poseidon_root::(), old); + assert!( + MerkleTreeService::is_known_root::(&old), + "replaced root refused right after the insert" + ); + + System::set_block_number(replaced_at + retention()); + assert!(MerkleTreeService::is_known_root::(&old)); + System::set_block_number(replaced_at + retention() + 1); + assert!(!MerkleTreeService::is_known_root::(&old)); + }); + } + + /// Inserts in the same block refresh the outgoing root once: the roots made + /// within the block already carry a full window. + #[test] + fn replacing_a_root_from_the_same_block_queues_nothing_extra() { + new_test_ext().execute_with(|| { + System::set_block_number(System::block_number() + 1); + let before = MerkleRepository::historic_roots_queued::(); + for i in 1..=5u8 { + MerkleTreeService::insert_leaf::(Commitment::new([i; 32])).unwrap(); + } + // The genesis root, refreshed once, plus one slot per new root. + assert_eq!( + MerkleRepository::historic_roots_queued::(), + before + 1 + 5 + ); + }); + } + /// A root stays spendable for the whole retention window and stops being /// accepted once it has elapsed. #[test] diff --git a/frame/shielded-pool/src/merkle/service.rs b/frame/shielded-pool/src/merkle/service.rs index fa981802..131425ca 100644 --- a/frame/shielded-pool/src/merkle/service.rs +++ b/frame/shielded-pool/src/merkle/service.rs @@ -83,6 +83,7 @@ impl MerkleTreeService { MerkleRepository::set_tree_size::(index.saturating_add(1)); PoolStatsRepository::increment_commitments_inserted::(); MerkleRepository::set_frontier::(frontier); + Self::retire_active_root::(old_poseidon_root); MerkleRepository::set_poseidon_root::(new_poseidon_root); Self::add_poseidon_historic_root::(new_poseidon_root); @@ -124,6 +125,22 @@ impl MerkleTreeService { // ── Historic roots ────────────────────────────────────────────────────── + /// Keep the root being replaced spendable for a full window from now. + /// + /// The active root never expires, so a wallet may prove against one created + /// long ago; its window must count from when it stops being active, or the + /// next insert would refuse that spend at once. Only a root created in an + /// earlier block needs this, so it costs at most one slot per block. + fn retire_active_root(root: Hash) { + let expires_at = + frame_system::Pallet::::block_number().saturating_add(T::RootRetentionBlocks::get()); + let fresh = MerkleRepository::get_historic_root_expiry::(&root) + .is_some_and(|expiry| expiry >= expires_at); + if !fresh { + Self::add_poseidon_historic_root::(root); + } + } + /// Record a new root, dropping the ones whose retention window has passed. /// /// The window is measured in blocks (`RootRetentionBlocks`); the queue only From 43767efe67903ad1e2b06378a9d96048b9509136 Mon Sep 17 00:00:00 2001 From: nol4lej Date: Wed, 21 Oct 2026 22:21:44 -0300 Subject: [PATCH 2/3] =?UTF-8?q?fix(shielded-pool):=20tag=20each=20note=20a?= =?UTF-8?q?=20spend=20creates=20(transfer=20outputs,=20unshield=20change)?= =?UTF-8?q?=20in=20the=20pool,=20so=20two=20spends=20creating=20the=20same?= =?UTF-8?q?=20note=20are=20mutually=20exclusive=20/=20an=20invalid=20proof?= =?UTF-8?q?=20costs=20the=20sending=20peer=20too=20little=20reputation=20t?= =?UTF-8?q?o=20get=20it=20banned=20=E2=80=94=20validators=20should=20take?= =?UTF-8?q?=20transactions=20only=20from=20their=20sentries?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- frame/shielded-pool/CHANGELOG.md | 3 + .../src/validate_unsigned/mod.rs | 11 ++- .../src/validate_unsigned/tests.rs | 98 +++++++++++++++++-- .../src/validate_unsigned/transfer.rs | 8 +- .../src/validate_unsigned/unshield.rs | 14 +-- 5 files changed, 119 insertions(+), 15 deletions(-) diff --git a/frame/shielded-pool/CHANGELOG.md b/frame/shielded-pool/CHANGELOG.md index 155a86e8..93435706 100644 --- a/frame/shielded-pool/CHANGELOG.md +++ b/frame/shielded-pool/CHANGELOG.md @@ -11,6 +11,9 @@ All notable changes to `pallet-shielded-pool` will be documented in this file. so a wallet may prove against one older than the window; the next insert used to refuse that spend at once with `UnknownMerkleRoot`. Costs at most one extra historic-root slot per block. +- Pool admission tags each note a spend creates (transfer outputs, unshield + change) as well as each note it spends: two spends creating the same note + are mutually exclusive in the pool, since only one can execute. ## [0.25.1] - 2026-10-21 diff --git a/frame/shielded-pool/src/validate_unsigned/mod.rs b/frame/shielded-pool/src/validate_unsigned/mod.rs index 56a1b40c..1ed57eb9 100644 --- a/frame/shielded-pool/src/validate_unsigned/mod.rs +++ b/frame/shielded-pool/src/validate_unsigned/mod.rs @@ -5,7 +5,9 @@ //! would share the original's nullifier tag, outrank it on priority, replace it //! and then fail in the block — free, repeatable censorship — and junk spends //! would fill blocks for nothing. The price is node CPU per admission and -//! revalidation; a peer that sends invalid proofs loses reputation for it. +//! revalidation, paid by every node a spend reaches. 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`). //! Inside a block (`pre_dispatch`) only the cheap checks run: the dispatchable //! verifies the proof, and its weight covers one pairing. //! @@ -42,5 +44,12 @@ pub(crate) const TX_LONGEVITY: u64 = 64; /// chain: one note, one entry. pub(crate) const SPEND_TAG_PREFIX: &str = "ShieldedPoolSpend"; +/// Marks a tag as a note a spend creates, apart from the nullifiers it spends. +/// +/// A commitment is inserted once, so two spends creating the same note are as +/// exclusive as two spending the same one: only one can execute. Tagged as +/// `(OUTPUT_TAG, commitment)`, which never encodes like a bare nullifier. +pub(crate) const OUTPUT_TAG: &[u8] = b"output"; + #[cfg(test)] mod tests; diff --git a/frame/shielded-pool/src/validate_unsigned/tests.rs b/frame/shielded-pool/src/validate_unsigned/tests.rs index f9ca72c8..07b6b9cb 100644 --- a/frame/shielded-pool/src/validate_unsigned/tests.rs +++ b/frame/shielded-pool/src/validate_unsigned/tests.rs @@ -19,7 +19,8 @@ fn full_memo() -> EncryptedMemo { /// A well-formed transfer with the given public values: two canonical /// commitments with a full memo each, and a dummy second input when only one -/// nullifier is given. +/// nullifier is given. The outputs follow from the input notes, in any order, +/// so unrelated spends never create the same note. fn transfer_request( merkle_root: &Hash, nullifiers: &BoundedVec>, @@ -30,10 +31,12 @@ fn transfer_request( while nullifiers.len() < 2 { nullifiers.try_push(Nullifier::new([0u8; 32])).unwrap(); } + let inputs = nullifiers.iter().fold(0u8, |x, n| x ^ n.0[0]); let commitment = |i: u8| { let mut bytes = [0u8; 32]; bytes[0] = i + 1; bytes[1] = 0xC0; + bytes[2] = inputs; Commitment::new(bytes) }; TransferRequest { @@ -676,15 +679,16 @@ fn attack_each_nullifier_contributes_an_independent_tag() { new_test_ext().execute_with(|| { MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + // Plus one tag per output note, two in every transfer. let one = validate_private_transfer(&KNOWN_ROOT, &nullifiers_of(&[0x91]), &0u128, 1).unwrap(); - assert_eq!(one.provides.len(), 1, "one real input → one tag"); + assert_eq!(one.provides.len(), 1 + 2, "one real input → one tag"); let two = validate_private_transfer(&KNOWN_ROOT, &nullifiers_of(&[0x92, 0x93]), &0u128, 1) .unwrap(); assert_eq!( two.provides.len(), - 2, + 2 + 2, "two real inputs → two independent tags" ); @@ -693,7 +697,11 @@ fn attack_each_nullifier_contributes_an_independent_tag() { padded.try_push(nullifier(0x94)).unwrap(); padded.try_push(Nullifier::new([0u8; 32])).unwrap(); let p = validate_private_transfer(&KNOWN_ROOT, &padded, &0u128, 1).unwrap(); - assert_eq!(p.provides.len(), 1, "the dummy must not contribute a tag"); + assert_eq!( + p.provides.len(), + 1 + 2, + "the dummy must not contribute a tag" + ); }); } @@ -715,13 +723,91 @@ fn attack_transfer_and_unshield_of_the_same_note_are_mutually_exclusive() { validate_private_transfer(&KNOWN_ROOT, &nullifiers_of(&[0x77]), &10u128, 1).unwrap(); let unshield = validate_unshield(&KNOWN_ROOT, &n, &0u32, &100u128, &10u128, 1).unwrap(); - assert_eq!( - transfer.provides, unshield.provides, + assert!( + unshield + .provides + .iter() + .all(|t| transfer.provides.contains(t)), "one note must back one pool entry, whichever operation spends it" ); }); } +// ── one note created, one pool entry ───────────────────────────────────────── +// +// A commitment is inserted once, so two spends creating the same note cannot +// both execute. Without a tag per output both would sit in the pool, propagate +// and be revalidated, and the loser would fail in the block for free. + +/// Two transfers spending different notes into the same output note are +/// mutually exclusive in the pool. +#[test] +fn transfers_creating_the_same_note_are_mutually_exclusive() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + let first = transfer_request(&KNOWN_ROOT, &nullifiers_of(&[0xA1]), 0, 1); + let mut second = transfer_request(&KNOWN_ROOT, &nullifiers_of(&[0xA2]), 0, 1); + second.commitments[1] = first.commitments[0]; + + let a = super::validate_private_transfer(Some(&proof()), &first).unwrap(); + let b = super::validate_private_transfer(Some(&proof()), &second).unwrap(); + let shared: Vec<_> = a + .provides + .iter() + .filter(|t| b.provides.contains(t)) + .collect(); + assert_eq!(shared.len(), 1, "exactly the shared output note's tag"); + }); +} + +/// A partial unshield's change note collides with a transfer creating the same +/// note; a total unshield creates none and tags its nullifier alone. +#[test] +fn an_unshield_change_note_is_tagged_like_a_transfer_output() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + PoolBalanceRepository::set_asset_balance::(0, 1_000); + let transfer = transfer_request(&KNOWN_ROOT, &nullifiers_of(&[0xB1]), 0, 1); + let total = unshield_request(&KNOWN_ROOT, &nullifier(0xB2), 0, 100, 0, 1); + let partial = UnshieldRequest { + change_commitment: transfer.commitments[0].0, + change_memo: full_memo(), + ..total.clone() + }; + + let t = super::validate_private_transfer(Some(&proof()), &transfer).unwrap(); + let total = super::validate_unshield(Some(&proof()), &total).unwrap(); + let partial = super::validate_unshield(Some(&proof()), &partial).unwrap(); + assert_eq!(total.provides.len(), 1); + assert_eq!(partial.provides.len(), 2); + assert!(partial.provides.iter().any(|tag| t.provides.contains(tag))); + assert!(!total.provides.iter().any(|tag| t.provides.contains(tag))); + }); +} + +/// An output tag never equals a nullifier tag, even for the same 32 bytes: a +/// note created by one spend must not block an unrelated spend of a note. +#[test] +fn an_output_tag_never_collides_with_a_nullifier_tag() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + PoolBalanceRepository::set_asset_balance::(0, 1_000); + let transfer = transfer_request(&KNOWN_ROOT, &nullifiers_of(&[0xC1]), 0, 1); + let unshield = unshield_request( + &KNOWN_ROOT, + &Nullifier::new(transfer.commitments[0].0), + 0, + 100, + 0, + 1, + ); + + let t = super::validate_private_transfer(Some(&proof()), &transfer).unwrap(); + let u = super::validate_unshield(Some(&proof()), &unshield).unwrap(); + assert!(!u.provides.iter().any(|tag| t.provides.contains(tag))); + }); +} + // ── Solvency arithmetic ────────────────────────────────────────────────────── // // `amount + fee` is attacker-chosen and summed before the balance compare. diff --git a/frame/shielded-pool/src/validate_unsigned/transfer.rs b/frame/shielded-pool/src/validate_unsigned/transfer.rs index 11b99146..93655239 100644 --- a/frame/shielded-pool/src/validate_unsigned/transfer.rs +++ b/frame/shielded-pool/src/validate_unsigned/transfer.rs @@ -52,8 +52,7 @@ pub fn validate_private_transfer( // ── 3. Proof ───────────────────────────────────────────────────────────── // Without it, a copy of a pending spend with swapped memos or a higher fee // would share the nullifier tags, outrank the original on priority, replace - // it, and then fail in the block — free, repeatable censorship. An invalid - // proof from a peer also costs that peer reputation. + // it, and then fail in the block — free, repeatable censorship. if let Some(proof) = proof { let valid = ensure_valid_proof::(|| { T::ZkVerifier::verify_transfer_proof(proof, &req.statement(), Some(req.circuit_version)) @@ -91,6 +90,11 @@ pub fn validate_private_transfer( for nullifier in req.real_nullifiers() { builder = builder.and_provides(nullifier); } + // Likewise one per output note: two spends creating the same note cannot + // both execute. + for commitment in req.commitments.iter() { + builder = builder.and_provides((super::OUTPUT_TAG, commitment)); + } builder.build() } diff --git a/frame/shielded-pool/src/validate_unsigned/unshield.rs b/frame/shielded-pool/src/validate_unsigned/unshield.rs index 36bf24b3..eebeb02d 100644 --- a/frame/shielded-pool/src/validate_unsigned/unshield.rs +++ b/frame/shielded-pool/src/validate_unsigned/unshield.rs @@ -51,8 +51,7 @@ pub fn validate_unshield( // ── 3. Proof ───────────────────────────────────────────────────────────── // Without it, a copy of a pending spend with swapped memos or a higher fee // would share the nullifier tag, outrank the original on priority, replace - // it, and then fail in the block — free, repeatable censorship. An invalid - // proof from a peer also costs that peer reputation. + // it, and then fail in the block — free, repeatable censorship. if let Some(proof) = proof { let valid = req.statement().is_ok_and(|statement| { ensure_valid_proof::(|| { @@ -65,17 +64,20 @@ pub fn validate_unshield( } } - // ── 4. Pool tag: the nullifier alone (one note, one pool entry) ────────── + // ── 4. Pool tags: the nullifier, and the change note if any ────────────── // // Every copy of a spend is mutually exclusive in the pool, whatever else it // changes; with the proof checked, only a valid one gets in. Mirrors // `transfer.rs`. - ValidTransaction::with_tag_prefix(super::SPEND_TAG_PREFIX) + let mut builder = ValidTransaction::with_tag_prefix(super::SPEND_TAG_PREFIX) .priority(req.fee.saturated_into()) .longevity(TX_LONGEVITY) .and_provides(req.nullifier) - .propagate(true) - .build() + .propagate(true); + if req.has_change() { + builder = builder.and_provides((super::OUTPUT_TAG, req.change_commitment)); + } + builder.build() } /// The pool rejection for a failed unshield check. From 8e1ddd5be81b9cafd01b0b7a183effc8433db508 Mon Sep 17 00:00:00 2001 From: nol4lej Date: Wed, 21 Oct 2026 22:23:34 -0300 Subject: [PATCH 3/3] =?UTF-8?q?chore(shielded-pool):=20release=20pallet-sh?= =?UTF-8?q?ielded-pool=200.25.2=20=E2=80=94=20replaced=20roots=20keep=20a?= =?UTF-8?q?=20full=20retention=20window,=20pool=20tags=20for=20created=20n?= =?UTF-8?q?otes=20(spec=2020)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Cargo.lock | 2 +- frame/shielded-pool/CHANGELOG.md | 2 ++ frame/shielded-pool/Cargo.toml | 2 +- template/runtime/RUNTIME_VERSIONS.md | 15 ++++++++++++++- 4 files changed, 18 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b127c06d..9e51f05b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -12358,7 +12358,7 @@ dependencies = [ [[package]] name = "pallet-shielded-pool" -version = "0.25.1" +version = "0.25.2" dependencies = [ "ark-bn254", "ark-ff 0.5.0", diff --git a/frame/shielded-pool/CHANGELOG.md b/frame/shielded-pool/CHANGELOG.md index 93435706..bac8d94e 100644 --- a/frame/shielded-pool/CHANGELOG.md +++ b/frame/shielded-pool/CHANGELOG.md @@ -4,6 +4,8 @@ All notable changes to `pallet-shielded-pool` will be documented in this file. ## [Unreleased] +## [0.25.2] - 2026-10-21 + ### Fixed - A root replaced by a leaf insert stays spendable for `RootRetentionBlocks` diff --git a/frame/shielded-pool/Cargo.toml b/frame/shielded-pool/Cargo.toml index 1edc8f1b..5cbca738 100644 --- a/frame/shielded-pool/Cargo.toml +++ b/frame/shielded-pool/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pallet-shielded-pool" -version = "0.25.1" +version = "0.25.2" description = "Shielded pool pallet for private transactions using ZK proofs" authors = { workspace = true } license = "GPL-3.0-or-later" diff --git a/template/runtime/RUNTIME_VERSIONS.md b/template/runtime/RUNTIME_VERSIONS.md index 25e2d9b8..50481d40 100644 --- a/template/runtime/RUNTIME_VERSIONS.md +++ b/template/runtime/RUNTIME_VERSIONS.md @@ -31,8 +31,11 @@ raw `Ed25519` (`0x00`) never verifies, and Solana wallets and Substrate extensio sign through the new `Ed25519Text` (`0x03`) and `Ed25519WrappedText` (`0x04`). Circuit keys: unsound ones are recorded on chain and never verify again, a removed version keeps its key, and a key without phase 2 is refused. +Shielded pool: a replaced root stays spendable for a full window, and spends +creating the same note are exclusive in the pool. `transaction_version` is unchanged. Ships with `pallet-evm-precompile-balances` -0.2.0, `pallet-zk-verifier` 0.19.0 and `orbinum-zk-verifier` 3.4.0. +0.2.0, `pallet-zk-verifier` 0.19.0, `orbinum-zk-verifier` 3.4.0 and +`pallet-shielded-pool` 0.25.2. - **Balances precompile (`0x…0802`)** refuses static calls, delegated calls (DELEGATECALL, CALLCODE) and calls carrying value. Under DELEGATECALL the @@ -127,6 +130,16 @@ removed version keeps its key, and a key without phase 2 is refused. - **Weights.** Every proof verification adds one read (the denied-key check); `deny_version` is estimated from `retire_version`, and registration and removal add the `RemovedVkHashes` read/write, until re-benchmarked. +- **Replaced roots keep a full window.** A leaf insert records the root it + replaces with `RootRetentionBlocks` from now, not from when that root was + created. The active root never expires, so a wallet may prove against one + older than the window, and the next insert refused that spend at once + (`UnknownMerkleRoot`, pool code 1). At most one extra historic-root slot per + block, not yet in the benchmarked weights. +- **Created notes tagged in the pool.** A transfer's outputs and an unshield's + change note each `provide` a pool tag, besides the spent nullifiers: two + spends creating the same note can no longer both sit in the pool and + propagate when only one can execute. ### spec 19 — tx 7 — 2026-10-13 (`v0.5.0`)