fix(shielded-pool): bound the zero-hash ladder and the tree depth shift

frame/shielded-pool/CHANGELOG.md

@@ -30,6 +30,34 @@ All notable changes to `pallet-shielded-pool` will be documented in this file.
   tree is untouched: still 20 point reads and zero hashes.
 
 ### Fixed
+- **`zero_hash_at_level` is iterative.** It was defined recursively, one stack
+  frame per level, and `get_zero_hash_cached` falls through to it for any level
+  past its 21-entry table. `usize` is 32 bits under Wasm, so a caller passing a
+  large level would exhaust the runtime's fixed 1 MB stack — and a stack
+  overflow there aborts the process rather than raising a catchable panic.
+
+  Measured on a 1 MiB thread: a recursion of this shape returns at 10,000 frames
+  and aborts at 20,000. The loop returns at 5,000 with the same stack and would
+  at any depth, since it uses a constant number of frames.
+
+  Unreachable today, and not just by configuration: every `level` at every call
+  site comes from a `0..depth` loop bound by `DEFAULT_TREE_DEPTH`, so no external
+  input selects one. Latent because the ladder is `pub` and the bound lives in
+  the callers rather than in the function.
+
+  The digests are unchanged, which is the part that matters: these hashes stand
+  in for empty subtrees inside every Merkle path the chain has served, so a
+  divergence at any level would invalidate proofs against notes already on
+  chain. A test compares the loop against a local recursive reference across
+  levels 0-24, including past the cache boundary.
+
+- **`IncrementalMerkleTree` rejects depths of 32 or more at compile time.**
+  `capacity()` computes `1u32 << DEPTH`, which is undefined past 31: debug
+  builds panic, release builds wrap to 1 and the tree reports itself full after
+  a single leaf. The struct is `pub` and generic over depth, so the bound
+  belonged on the type rather than only in the runtime's `integrity_test`. A
+  const assertion now fails the build instead.
+
 - `hash_pair_poseidon` clamps its output copy instead of slicing `&bytes[..32]`
   raw. BN254 `Fr` always yields 32 bytes, so the clamp never binds today — but
   this runs on the block-import path, where slicing past the end panics the node

frame/shielded-pool/src/merkle/hashing.rs

@@ -7,13 +7,20 @@
 use alloc::boxed::Box;
 use ark_ff::BigInteger;
 
-/// Default hash for empty nodes at each level.
+/// Digest of an empty subtree rooted at `level`.
+///
+/// Iterative rather than recursive. The recursion this replaces spent one stack
+/// frame per level, and `get_zero_hash_cached` falls through to here for any
+/// level past its 21-entry table — with `usize` being 32 bits under Wasm, a
+/// caller passing a large level would exhaust the runtime's fixed 1 MB stack.
+/// A stack overflow there takes the node down rather than failing a call, while
+/// a loop just runs long: slow is recoverable, overflowing is not.
 pub fn zero_hash_at_level(level: usize) -> [u8; 32] {
-	if level == 0 {
-		return [0u8; 32];
+	let mut current = [0u8; 32];
+	for _ in 0..level {
+		current = hash_pair(&current, &current);
 	}
-	let prev = zero_hash_at_level(level - 1);
-	hash_pair(&prev, &prev)
+	current
 }
 
 /// Cached zero hashes for Poseidon (lazy-initialized, thread-safe).

frame/shielded-pool/src/merkle/mod.rs

@@ -105,6 +105,65 @@ mod tests {
 		}
 	}
 
+	/// The ladder used to be defined recursively. Turning it into a loop must not
+	/// move a single digest: these hashes stand in for empty subtrees inside
+	/// every Merkle path the chain has ever served, so a divergence at any level
+	/// would invalidate proofs against notes already on chain.
+	///
+	/// Checked against a local recursive reference rather than against stored
+	/// values, so the property survives a change to the hash function itself.
+	#[test]
+	fn iterative_zero_hash_matches_the_recursive_definition() {
+		fn recursive(level: usize) -> Hash {
+			if level == 0 {
+				return [0u8; 32];
+			}
+			let prev = recursive(level - 1);
+			hash_pair(&prev, &prev)
+		}
+
+		// Past 20 as well: that is where `get_zero_hash_cached` stops using its
+		// table and falls through to the function being changed here.
+		for level in 0..=24usize {
+			assert_eq!(
+				zero_hash_at_level(level),
+				recursive(level),
+				"zero hash diverged at level {level}"
+			);
+		}
+	}
+
+	/// The cache and the function must agree across the boundary of the table,
+	/// not just inside it — level 21 and up is the fall-through path.
+	#[test]
+	fn cached_and_computed_agree_past_the_cache_boundary() {
+		for level in 19..=23usize {
+			assert_eq!(
+				get_zero_hash_cached(level),
+				zero_hash_at_level(level),
+				"cache and computation disagree at level {level}"
+			);
+		}
+	}
+
+	/// A level far past anything the tree uses must return rather than exhaust
+	/// the stack. The recursion this replaces spent one frame per level against
+	/// the runtime's fixed 1 MB Wasm stack, where an overflow downs the node
+	/// instead of failing the call.
+	#[test]
+	fn a_large_level_returns_instead_of_overflowing_the_stack() {
+		// Measured on a 1 MiB thread, matching the runtime's Wasm stack: a bare
+		// recursion of this shape returns at 10_000 frames and aborts the process
+		// at 20_000 — not a catchable panic, the whole runtime goes. The loop
+		// returns at 5_000 with the same stack, and would at any depth: it uses a
+		// constant number of frames.
+		//
+		// Kept at 5_000 rather than higher because each level is a Poseidon hash
+		// and the property being shown is "returns at all", not "returns fast".
+		let deep = zero_hash_at_level(5_000);
+		assert_ne!(deep, [0u8; 32]);
+	}
+
 	// ── IncrementalMerkleTree ────────────────────────────────────────────────
 
 	#[test]
@@ -125,6 +184,27 @@ mod tests {
 		assert_eq!(IncrementalMerkleTree::<2>::new().capacity(), 4);
 	}
 
+	/// `capacity()` shifts into a `u32`, so depth 31 is the last one that holds.
+	/// Past it the shift is undefined — release builds wrap to 1 and the tree
+	/// declares itself full after a single leaf — which is why a const assertion
+	/// on the type rejects those depths at compile time. That case cannot be
+	/// tested at runtime: it does not build. This pins the boundary that does.
+	#[test]
+	fn capacity_holds_at_the_deepest_supported_tree() {
+		assert_eq!(IncrementalMerkleTree::<31>::new().capacity(), 1u32 << 31);
+		assert_eq!(IncrementalMerkleTree::<20>::new().capacity(), 1_048_576);
+	}
+
+	/// Production depth: the value `integrity_test` pins and every client
+	/// derives `tree_id` from.
+	#[test]
+	fn production_depth_capacity_is_two_to_the_twenty() {
+		assert_eq!(
+			IncrementalMerkleTree::<{ crate::types::DEFAULT_TREE_DEPTH }>::new().capacity(),
+			1_048_576
+		);
+	}
+
 	#[test]
 	fn tree_insert_returns_sequential_indices() {
 		let mut tree = IncrementalMerkleTree::<4>::new();

frame/shielded-pool/src/merkle/tree.rs

@@ -23,6 +23,19 @@ impl<const DEPTH: usize> Default for IncrementalMerkleTree<DEPTH> {
 }
 
 impl<const DEPTH: usize> IncrementalMerkleTree<DEPTH> {
+	/// `capacity()` shifts into a `u32`, so a depth of 32 or more is undefined:
+	/// debug builds panic, release builds wrap to 1 and the tree reports itself
+	/// full after a single leaf.
+	///
+	/// The bound belongs on the type rather than in the runtime config. The
+	/// pallet's `integrity_test` pins `MaxTreeDepth` to 20, but this struct is
+	/// `pub` and generic, so nothing stopped a downstream caller from picking
+	/// its own depth. Instantiating past the limit now fails to compile.
+	const _DEPTH_FITS_IN_U32: () = assert!(
+		DEPTH < 32,
+		"IncrementalMerkleTree DEPTH must be below 32: capacity() shifts into a u32"
+	);
+
 	pub fn new() -> Self {
 		let root = Self::compute_empty_root();
 		Self {
@@ -45,6 +58,7 @@ impl<const DEPTH: usize> IncrementalMerkleTree<DEPTH> {
 	}
 
 	pub fn capacity(&self) -> u32 {
+		let () = Self::_DEPTH_FITS_IN_U32;
 		1u32 << DEPTH
 	}
 	pub fn is_full(&self) -> bool {

ts-tests/node/merkle-adversarial.test.cjs

@@ -0,0 +1,174 @@
+// Adversarial probe of the Merkle surfaces, against a live node.
+//
+// The other Merkle test is confirmatory: it checks the zero-hash ladder did not
+// move. This one tries to break the node instead.
+//
+// What is being attacked: `zero_hash_at_level` used to recurse one stack frame
+// per level, and the runtime's Wasm stack is a fixed 1 MB. A stack overflow
+// there aborts the whole runtime rather than failing one call, so any input
+// that reaches the ladder with a large level was a node-kill primitive. The
+// function is now a loop, and `IncrementalMerkleTree` rejects depths past 31 at
+// compile time.
+//
+// Every case below is expected to be REJECTED, not to succeed. The failure this
+// is looking for is the node dying, hanging, or stopping block production —
+// which is why block height is checked after every batch.
+//
+//   ./target/release/orbinum-node --dev --tmp --rpc-port 9955 --sealing=instant
+//   node ts-tests/node/merkle-adversarial.test.cjs
+const { ApiPromise, WsProvider } = require('@polkadot/api');
+const { Keyring } = require('@polkadot/keyring');
+
+const ok = [], bad = [];
+const check = (n, c, x = '') => { (c ? ok : bad).push(n); console.log(`${c ? 'PASS' : 'FAIL'}  ${n}${x ? ` — ${x}` : ''}`); };
+const sect = (t) => console.log(`\n── ${t} ──`);
+
+const RPC_TIMEOUT_MS = 15_000;
+
+/** Send an RPC and classify the outcome. A timeout is the interesting failure. */
+async function probe(provider, method, params) {
+  const started = Date.now();
+  try {
+    const result = await Promise.race([
+      provider.send(method, params),
+      new Promise((_, rej) => setTimeout(() => rej(new Error('TIMEOUT')), RPC_TIMEOUT_MS)),
+    ]);
+    return { outcome: 'ok', ms: Date.now() - started, result };
+  } catch (e) {
+    if (e.message === 'TIMEOUT') return { outcome: 'timeout', ms: Date.now() - started };
+    return { outcome: 'error', ms: Date.now() - started, message: e.message };
+  }
+}
+
+const memo = () => '0x' + 'ab'.repeat(180);
+
+function submit(tx, signer, nonce) {
+  return new Promise((resolve, reject) => {
+    const timer = setTimeout(() => reject(new Error('tx timed out')), 30_000);
+    tx.signAndSend(signer, { nonce }, ({ status, dispatchError }) => {
+      if (dispatchError) { clearTimeout(timer); return reject(new Error(dispatchError.toString())); }
+      if (status.isInBlock) { clearTimeout(timer); resolve(); }
+    }).catch(reject);
+  });
+}
+
+(async () => {
+  const provider = new WsProvider('ws://127.0.0.1:9955');
+  const api = await ApiPromise.create({ provider, noInitWarn: true });
+  const alice = new Keyring({ type: 'sr25519' }).addFromUri('//Alice');
+  let nonce = (await api.rpc.system.accountNextIndex(alice.address)).toNumber();
+
+  const height = async () => (await api.rpc.chain.getHeader()).number.toNumber();
+  const stillAlive = async (label) => {
+    const before = await height();
+    await submit(api.tx.system.remark('0x00'), alice, nonce++);
+    const after = await height();
+    check(`node still producing blocks after ${label}`, after > before, `${before} -> ${after}`);
+  };
+
+  // Give the tree a leaf so the proof paths have something to resolve.
+  const RUN = require('crypto').randomBytes(4).toString('hex');
+  await submit(
+    api.tx.shieldedPool.shield(0, 10n ** 18n, '0x' + RUN + 'aa'.repeat(28), memo()),
+    alice, nonce++);
+
+  sect('Extreme leaf indices');
+
+  // The ladder is indexed by tree level, not by leaf index — but leaf_index is
+  // the only Merkle input a stranger controls, and it feeds index arithmetic
+  // that derives levels. u32::MAX is the largest value the codec will carry.
+  const extremes = [
+    ['u32::MAX', 4294967295],
+    ['u32::MAX - 1', 4294967294],
+    ['2^31 (sign boundary)', 2147483648],
+    ['2^20 (one tree)', 1048576],
+    ['2^20 - 1', 1048575],
+  ];
+
+  for (const [label, idx] of extremes) {
+    const r = await probe(provider, 'privacy_getMerkleProof', [idx]);
+    // Rejection is the correct answer: these are past tree_size.
+    check(`leaf_index ${label} is refused, not hung`, r.outcome === 'error',
+      `${r.outcome}${r.ms > 1000 ? ` in ${r.ms}ms` : ''}`);
+  }
+
+  await stillAlive('extreme leaf indices');
+
+  sect('Malformed commitments');
+
+  // getMerkleProofByCommitment parses a hex string, so it takes attacker bytes
+  // of attacker-chosen length.
+  const malformed = [
+    ['empty', '0x'],
+    ['one byte', '0xff'],
+    ['31 bytes', '0x' + 'ff'.repeat(31)],
+    ['33 bytes', '0x' + 'ff'.repeat(33)],
+    ['1 KiB', '0x' + 'ff'.repeat(1024)],
+    ['64 KiB', '0x' + 'ff'.repeat(65536)],
+    ['no 0x prefix', 'ff'.repeat(32)],
+    ['not hex', '0xzzzz'],
+  ];
+
+  for (const [label, c] of malformed) {
+    const r = await probe(provider, 'privacy_getMerkleProofByCommitment', [c]);
+    check(`commitment ${label} is refused, not hung`, r.outcome === 'error',
+      `${r.outcome}${r.ms > 1000 ? ` in ${r.ms}ms` : ''}`);
+  }
+
+  await stillAlive('malformed commitments');
+
+  sect('Repeated pressure');
+
+  // A single rejection is cheap. The concern is whether repeated rejections
+  // accumulate — a leak, a runtime instance that never resets, a queue that
+  // grows. Fire a burst and check the node is unchanged.
+  const burst = [];
+  for (let i = 0; i < 50; i++) {
+    burst.push(probe(provider, 'privacy_getMerkleProof', [4294967295 - i]));
+  }
+  const results = await Promise.all(burst);
+  const hung = results.filter((r) => r.outcome === 'timeout').length;
+  const slowest = Math.max(...results.map((r) => r.ms));
+  check('50 rejected proof requests, none hung', hung === 0, `slowest ${slowest}ms`);
+
+  await stillAlive('a 50-request burst');
+
+  sect('Storage query surfaces');
+
+  // merkleNodes is a three-key map; the level key is a u8, so 255 is the
+  // largest a caller can name. Before the rewrite, a level past the 21-entry
+  // cache fell through to the recursive ladder.
+  for (const level of [20, 21, 31, 32, 100, 255]) {
+    try {
+      const v = await api.query.shieldedPool.merkleNodes(0, level, 0);
+      check(`merkleNodes at level ${level} answers`, v.isNone || v.isSome,
+        v.isSome ? 'stored' : 'empty');
+    } catch (e) {
+      check(`merkleNodes at level ${level} answers`, false, e.message.slice(0, 60));
+    }
+  }
+
+  await stillAlive('high-level node queries');
+
+  sect('Runtime API surface');
+
+  // get_root_for_leaf takes a raw leaf index straight into tree arithmetic.
+  for (const [label, idx] of extremes) {
+    try {
+      const r = await api.call.shieldedPoolRuntimeApi.getRootForLeaf(idx);
+      check(`getRootForLeaf ${label} answers without trapping`, r.isNone || r.isSome,
+        r.isSome ? 'root' : 'none');
+    } catch (e) {
+      // A clean error is fine. A trap would have downed the runtime instance.
+      check(`getRootForLeaf ${label} answers without trapping`,
+        !/unreachable|trap/i.test(e.message), e.message.slice(0, 60));
+    }
+  }
+
+  await stillAlive('runtime API probing');
+
+  await api.disconnect();
+  console.log(`\n${ok.length} passed, ${bad.length} failed`);
+  if (bad.length) console.log('Failed: ' + bad.join(', '));
+  process.exit(bad.length ? 1 : 0);
+})().catch((e) => { console.error('ERROR:', e.message); process.exit(1); });