Fix/block based root window

frame/evm/precompile/shielded-pool/src/mock.rs

@@ -120,6 +120,7 @@ parameter_types! {
 	pub const ShieldedPoolPalletId: PalletId = PalletId(*b"shldpool");
 	pub const MaxTreeDepth: u32 = 20;
 	pub const MaxHistoricRoots: u32 = 100;
+	pub const RootRetentionBlocks: u64 = 128;
 	pub const MaxLeavesPerTree: u32 = 8;
 	pub const MinShieldAmount: u128 = 100;
 }
@@ -245,6 +246,7 @@ impl pallet_shielded_pool::Config for Test {
 	type PalletId = ShieldedPoolPalletId;
 	type MaxTreeDepth = MaxTreeDepth;
 	type MaxHistoricRoots = MaxHistoricRoots;
+	type RootRetentionBlocks = RootRetentionBlocks;
 	type MaxLeavesPerTree = MaxLeavesPerTree;
 	type MinShieldAmount = MinShieldAmount;
 	type WeightInfo = ();

frame/shielded-pool/CHANGELOG.md

@@ -2,6 +2,101 @@
 
 All notable changes to `pallet-shielded-pool` will be documented in this file.
 
+## [0.14.0] - 2026-08-05
+
+### Changed
+- **Historic-root window is now measured in blocks, not in inserts.** The window
+  was bounded by `MaxHistoricRoots` (100) counted in *leaf insertions*, while an
+  unsigned transaction stays valid in the pool for `TX_LONGEVITY` (64) *blocks*.
+  A `private_transfer` inserts up to 2 commitments, so ~50 transfers rotated the
+  whole window: under load an honest spend passed `validate_unsigned`, gossiped
+  across the network, and only then reverted with `UnknownMerkleRoot`, with its
+  nullifier still unspent. No attacker was required — ordinary throughput did it,
+  and an attacker could force it cheaply with valid transfers.
+- New Config constant **`RootRetentionBlocks`** (production 300 blocks, ~30 min at
+  6s) sets the window. `integrity_test` asserts it exceeds `TX_LONGEVITY`, so a
+  runtime that misaligns the two units refuses to build.
+- **`MaxHistoricRoots` changed meaning**: it is no longer the window but a safety
+  cap on queue length (production 16384, sized for ~54 sustained inserts/block).
+  Reaching it logs a warning rather than silently shortening the window.
+- `HistoricPoseidonRoots` value type `bool` → `BlockNumberFor<T>` (the block at
+  which a root stops being accepted), `OptionQuery`.
+- `HistoricRootsOrder` (a single `BoundedVec`) replaced by a slot-indexed queue:
+  `HistoricRootsQueue: StorageMap<u64, (Hash, expiry)>` plus `HistoricRootsHead`
+  and `HistoricRootsTail`. A `StorageValue` would be read and rewritten in full on
+  every leaf insert — hundreds of KiB on the hottest path once the window holds
+  thousands of roots. The map touches one entry plus the few it prunes.
+- Pruning is lazy and capped at `MAX_ROOTS_PRUNED_PER_INSERT` (4) per insert, so
+  one extrinsic never pays for a backlog it did not create. Leftovers drain on
+  subsequent inserts; a stale queue slot is harmless because spendability is
+  decided by the expiry stored per root, never by queue membership.
+- **Weights re-benchmarked against the v3 layout.** The recorded storage access
+  now covers `HistoricRootsQueue` / `HistoricRootsHead` / `HistoricRootsTail` and
+  drops the `HistoricRootsOrder` entry, which no longer exists. The measurement
+  exercises a queue with expired entries, so the per-insert pruning is charged
+  rather than free: `shield` 17r/34w, `unshield` 16r/8w, and per-`n` components
+  on `shield_batch` (2r/6w) and `private_transfer` (3r/6w).
+- **Pool-rejection code for `amount + fee` overflow moved from `Custom(2)` to
+  `Custom(4)`.** Code 2 previously meant two different things depending on which
+  validator rejected the transaction — "all inputs are dummy" in
+  `private_transfer`, "amount overflow" in `unshield` — so a `Custom error: 2` in
+  a node log was ambiguous. The other codes keep their values. Codes are part of
+  the observable interface, so a test now pins them.
+- `is_known_root` now accepts the **active root unconditionally**. Expiries are
+  only refreshed by a leaf insert and the pallet has no hooks, so on a chain idle
+  for a full window the current root — the one every wallet proves against —
+  would expire and wedge the pool: `private_transfer` and `unshield` both need a
+  known root, and only a funded `shield` could mint a new one.
+
+### Added
+- `migrations::v3::MigrateToV3` (`STORAGE_VERSION` 2 → 3). Both value types
+  changed, so v2 entries cannot be decoded by the new definitions and are
+  rewritten here. Every existing root is kept and granted a full window from the
+  upgrade block rather than dropped: a root on chain backs proofs wallets may be
+  about to submit. The old map is enumerated by **raw key** rather than
+  `translate`, which does not delete an entry whose value fails to decode — it
+  logs, skips, and leaves the bytes in place, which for a 1-byte `bool` under a
+  4-byte slot would leave an unreadable, unprunable residue. Map and queue are
+  written in the same loop so the two can never diverge.
+
+### Fixed
+- Honest spends no longer revert with `UnknownMerkleRoot` after passing pool
+  admission: a root now outlives every transaction admitted against it.
+- Map and queue can no longer diverge, which previously left roots reachable by
+  neither the pruner nor an eviction path — permanently spendable and impossible
+  to remove.
+- The cap path no longer drops a root whose window has not elapsed; a live root
+  is re-queued instead of being forgotten.
+- Reaching the queue cap logs a warning instead of using `defensive!`, which
+  expands to `debug_assert!(false)` and would halt a validator running a
+  debug-assertions build on a reachable operational state.
+
+### Refactored
+- The four largest modules were split into directories, one file per
+  responsibility. No behaviour change: every type and function keeps its path
+  through re-exports, so no caller outside the pallet was touched.
+  - `merkle.rs` (1642 lines) → `merkle/` — `hashing`, `tree`, `batch`, `service`.
+    The split isolates the only storage-touching layer (`service`) from the pure
+    maths; `batch` now states in its own docs that it is O(n) and used off-chain
+    only, which the single file left implicit.
+  - `storage.rs` (658 lines) → `storage/` — one module per repository: `asset`,
+    `commitment`, `merkle`, `nullifier`, `stats`, `balance`.
+  - `validate_unsigned.rs` (683 lines) → `validate_unsigned/` — `transfer`,
+    `unshield`, and a new `codes` module holding the named rejection codes that
+    were previously bare literals duplicated across both validators.
+  - `types.rs` (719 lines) → `types/` — `ids`, `note`, `merkle`, `memo`, `asset`.
+
+### Verification
+298 pallet tests, 65 precompile tests, and 78 end-to-end checks against a running
+dev node covering `shield_batch`, same-block bursts, idle chains, the steady-state
+plateau (299 → 301 slots after +30 inserts), orphan detection in both directions,
+and a spend against a root the chain has churned past.
+
+### Notes
+- `spec_version` moves 6 → 7 in this release: the storage layout changes and a
+  migration is back in the tuple. `transaction_version` stays — no call
+  signature changed.
+
 ## [0.13.0] - 2026-08-04
 
 ### Removed

frame/shielded-pool/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "pallet-shielded-pool"
-version = "0.13.0"
+version = "0.14.0"
 description = "Shielded pool pallet for private transactions using ZK proofs"
 authors = ["Orbinum Team"]
 license = "GPL-3.0-or-later"

frame/shielded-pool/src/benchmarking.rs

@@ -23,7 +23,7 @@ use alloc::vec;
 mod benchmarks {
 	use super::*;
 	use crate::FrameEncryptedMemo;
-	use crate::pallet::{Assets, HistoricPoseidonRoots, NextAssetId, PoolBalancePerAsset};
+	use crate::pallet::{Assets, NextAssetId, PoolBalancePerAsset};
 	use pallet_relayer::RelayerInterface;
 	use sp_core::H160;
 	use sp_std::vec::Vec;
@@ -107,7 +107,7 @@ mod benchmarks {
 		let merkle_root = [1u8; 32];
 
 		// Setup valid root in storage
-		HistoricPoseidonRoots::<T>::insert(merkle_root, true);
+		crate::storage::MerkleRepository::add_historic_poseidon_root::<T>(merkle_root);
 
 		let proof: BoundedVec<u8, ConstU32<512>> = vec![0u8; 128].try_into().unwrap();
 
@@ -154,7 +154,7 @@ mod benchmarks {
 		let amount: BalanceOf<T> = T::MinShieldAmount::get() * 10u32.into();
 
 		// Setup valid state: root and pool balance
-		HistoricPoseidonRoots::<T>::insert(merkle_root, true);
+		crate::storage::MerkleRepository::add_historic_poseidon_root::<T>(merkle_root);
 		PoolBalancePerAsset::<T>::insert(asset_id, amount * 2u32.into());
 		// Fund pool account too for actual transfer
 		let _ = <T::Currency as Currency<T::AccountId>>::make_free_balance_be(

frame/shielded-pool/src/genesis.rs

@@ -2,13 +2,13 @@
 
 use crate::{
 	pallet::{
-		Assets, Config, HistoricPoseidonRoots, HistoricRootsOrder, MerkleTreeFrontier, NextAssetId,
-		PoseidonRoot,
+		Assets, Config, HistoricPoseidonRoots, HistoricRootsHead, HistoricRootsQueue,
+		HistoricRootsTail, MerkleTreeFrontier, NextAssetId, PoseidonRoot,
 	},
 	types::AssetMetadata,
 	types::Hash,
 };
-use frame_support::{pallet_prelude::*, traits::Get};
+use frame_support::traits::Get;
 use sp_runtime::traits::AccountIdConversion;
 
 /// Helper function to initialize genesis state
@@ -22,13 +22,13 @@ pub fn initialize_genesis<T: Config>(initial_root: Hash) {
 	// starts from the correct baseline.
 	MerkleTreeFrontier::<T>::put([[0u8; 32]; 20]);
 
-	// Add genesis root to historic roots
-	HistoricPoseidonRoots::<T>::insert(initial_root, true);
-
-	// Initialize the order list with the genesis root
-	let mut order = BoundedVec::new();
-	let _ = order.try_push(initial_root);
-	HistoricRootsOrder::<T>::put(order);
+	// Seed the historic-root window with the genesis root, expiring one full
+	// retention window from block zero like any other root.
+	let expires_at = T::RootRetentionBlocks::get();
+	HistoricPoseidonRoots::<T>::insert(initial_root, expires_at);
+	HistoricRootsQueue::<T>::insert(0u64, (initial_root, expires_at));
+	HistoricRootsHead::<T>::put(1u64);
+	HistoricRootsTail::<T>::put(0u64);
 
 	// Register native asset (asset_id = 0) at genesis
 	let native_asset = AssetMetadata {
@@ -83,7 +83,7 @@ mod tests {
 		new_test_ext().execute_with(|| {
 			let root = PoseidonRoot::<Test>::get();
 			assert!(
-				HistoricPoseidonRoots::<Test>::get(root),
+				HistoricPoseidonRoots::<Test>::get(root).is_some(),
 				"Genesis root must be in historic roots"
 			);
 		});
@@ -104,7 +104,7 @@ mod tests {
 			let custom_root = [0xCDu8; 32];
 			super::initialize_genesis::<Test>(custom_root);
 			assert!(
-				HistoricPoseidonRoots::<Test>::get(custom_root),
+				HistoricPoseidonRoots::<Test>::get(custom_root).is_some(),
 				"Custom root must be in historic roots"
 			);
 		});

frame/shielded-pool/src/lib.rs

@@ -65,6 +65,7 @@ mod benchmarking;
 pub mod genesis;
 pub mod helpers;
 pub mod merkle;
+pub mod migrations;
 pub mod operations;
 pub mod storage;
 pub mod types;
@@ -103,11 +104,12 @@ pub mod pallet {
 
 	/// Storage version history:
 	/// - v1: `MerkleNodes` (internal Merkle tree nodes), backfilled from `MerkleLeaves`.
+	///   Migration removed once every live chain reached v2 — see git history.
 	/// - v2: multi-tree forest — `SealedTreeRoots` / `SealedRootIndex` (start empty).
-	///
-	/// The v1/v2 migration code was removed once every live chain reached v2; a new
-	/// chain starts here via genesis. See git history if an old chain ever needs it.
-	pub const STORAGE_VERSION: StorageVersion = StorageVersion::new(2);
+	///   Migration removed once every live chain reached v2 — see git history.
+	/// - v3: historic-root window re-anchored from insert counts to block numbers
+	///   (`migrations::v3::MigrateToV3`); both historic-root items carry an expiry.
+	pub const STORAGE_VERSION: StorageVersion = StorageVersion::new(3);
 
 	#[pallet::pallet]
 	#[pallet::storage_version(STORAGE_VERSION)]
@@ -143,10 +145,24 @@ pub mod pallet {
 		#[pallet::constant]
 		type MaxLeavesPerTree: Get<u32>;
 
-		/// Maximum number of historic roots to keep
+		/// Safety cap on the historic-root queue: the most roots kept at once,
+		/// and the most a single insert may prune. This bounds worst-case work
+		/// and storage — the retention *window* is [`Self::RootRetentionBlocks`].
+		/// Size it above the roots produced in one window, or it becomes the
+		/// binding constraint and the window silently shortens.
 		#[pallet::constant]
 		type MaxHistoricRoots: Get<u32>;
 
+		/// How long a historic root stays spendable, in blocks.
+		///
+		/// Must exceed the mempool longevity of an unsigned transaction
+		/// (`TX_LONGEVITY`), otherwise a transaction can be admitted against a
+		/// root that expires before it is included — it would propagate, reach a
+		/// block, and only then revert with `UnknownMerkleRoot`. Enforced by
+		/// `integrity_test`.
+		#[pallet::constant]
+		type RootRetentionBlocks: Get<BlockNumberFor<Self>>;
+
 		/// Minimum amount that can be shielded
 		#[pallet::constant]
 		type MinShieldAmount: Get<BalanceOf<Self>>;
@@ -229,15 +245,44 @@ pub mod pallet {
 	#[pallet::storage]
 	pub type SealedRootIndex<T> = StorageMap<_, Blake2_128Concat, Hash, u32, OptionQuery>;
 
-	/// Historic Poseidon Merkle roots (for proving against recent states)
+	/// Historic Poseidon Merkle roots (for proving against recent states),
+	/// mapped to the block at which each stops being accepted.
+	///
+	/// Expiry is measured in **blocks**, not in insertions, so the window
+	/// always outlives the mempool longevity a transaction was admitted with.
+	/// Counting insertions instead made the window rotate faster than
+	/// transactions expire under load, so honest spends reverted with
+	/// `UnknownMerkleRoot` after propagating.
+	#[pallet::storage]
+	pub type HistoricPoseidonRoots<T: Config> =
+		StorageMap<_, Blake2_128Concat, Hash, BlockNumberFor<T>, OptionQuery>;
+
+	/// Expiry queue for historic roots: monotonic slot -> `(root, expires_at)`.
+	///
+	/// A map rather than one vector on purpose. The window has to hold a full
+	/// `RootRetentionBlocks` worth of roots — thousands under load — and a
+	/// `StorageValue` would be read and rewritten in full on every single leaf
+	/// insert, turning a hot path into hundreds of KiB of I/O. Keyed by slot,
+	/// each insert touches exactly one entry plus the few it prunes.
+	///
+	/// Slots are handed out by [`HistoricRootsHead`] and consumed from
+	/// [`HistoricRootsTail`], so the queue drains in insertion order, which is
+	/// also expiry order (every insert stores `now + retention` with a
+	/// non-decreasing `now`).
+	#[pallet::storage]
+	pub type HistoricRootsQueue<T: Config> =
+		StorageMap<_, Twox64Concat, u64, (Hash, BlockNumberFor<T>), OptionQuery>;
+
+	/// Next slot to write in [`HistoricRootsQueue`]. Monotonic; never reset.
 	#[pallet::storage]
-	pub type HistoricPoseidonRoots<T> = StorageMap<_, Blake2_128Concat, Hash, bool, ValueQuery>;
+	pub type HistoricRootsHead<T> = StorageValue<_, u64, ValueQuery>;
 
-	/// Order of historic roots (FIFO queue for pruning)
-	/// Stores roots in insertion order, oldest first
+	/// Oldest slot still queued in [`HistoricRootsQueue`]. Monotonic; never reset.
+	///
+	/// `head - tail` is the number of live entries, bounded in practice by the
+	/// retention window and hard-capped by `MaxHistoricRoots`.
 	#[pallet::storage]
-	pub type HistoricRootsOrder<T: Config> =
-		StorageValue<_, BoundedVec<Hash, T::MaxHistoricRoots>, ValueQuery>;
+	pub type HistoricRootsTail<T> = StorageValue<_, u64, ValueQuery>;
 
 	/// Encrypted memos for commitments
 	///
@@ -340,8 +385,20 @@ pub mod pallet {
 
 			assert!(
 				T::MaxHistoricRoots::get() > 0,
-				"MaxHistoricRoots must be non-zero, otherwise the historic-root map \
-				 grows unbounded and the root window never evicts"
+				"MaxHistoricRoots must be non-zero, otherwise no root can ever be stored"
+			);
+
+			// The retention window must outlive the mempool longevity an unsigned
+			// transaction is admitted with, or a spend can pass validation, get
+			// gossiped, and only revert once included — the failure SP-20 fixed.
+			let retention: u64 =
+				sp_runtime::traits::UniqueSaturatedInto::<u64>::unique_saturated_into(
+					T::RootRetentionBlocks::get(),
+				);
+			assert!(
+				retention > crate::validate_unsigned::TX_LONGEVITY,
+				"RootRetentionBlocks must exceed TX_LONGEVITY, otherwise a root can \
+				 expire while a transaction admitted against it is still valid in the pool"
 			);
 
 			let cap = T::MaxLeavesPerTree::get();