From fed0694ca12b369760431c8744ccde1f5e552f2b Mon Sep 17 00:00:00 2001 From: TaprootFreak Date: Mon, 4 May 2026 09:04:17 +0200 Subject: [PATCH] fix(smt): bind insert result before debug_assert to keep side effect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `debug_assert_eq!`/`debug_assert_ne!` expand to `if cfg!(debug_assertions) { assert!(...) }`. In release builds `cfg!(debug_assertions)` is `false`, so the entire expression is eliminated and its argument is never evaluated — including any side effects. `SparseMerkleTree::insert` wrapped two mutating `HashMap::insert` calls inside `debug_assert!` macros. In release this meant `leaf_values.insert(...)` and the root-node `nodes.insert(...)` were never executed: the tree state was never updated, `tree.root()` always returned `ZERO_HASH`, and any code that compared the post-insert root against an externally computed root (e.g. via `NonInclusionProof::insert`) would diverge. Reproduction on master: cargo test -p zkcoins-program --lib merkle::sparse_merkle_tree # 10/10 pass cargo test --release -p zkcoins-program --lib merkle::sparse_merkle_tree # 8/10 fail, including test_verify_and_insert and # test_verify_and_insert_sibling Fix: bind the `HashMap::insert` result to a local first, then run the assertion on the local. The mutation now always runs; the assertion remains debug-only as originally intended. --- program/src/merkle/sparse_merkle_tree.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/program/src/merkle/sparse_merkle_tree.rs b/program/src/merkle/sparse_merkle_tree.rs index a4afe3c..b7fdd2f 100644 --- a/program/src/merkle/sparse_merkle_tree.rs +++ b/program/src/merkle/sparse_merkle_tree.rs @@ -269,8 +269,11 @@ impl SparseMerkleTree { }; } - // Store the leaf to get it with the key - debug_assert_eq!(self.leaf_values.insert(key, leaf), None); + // Store the leaf to get it with the key. + // Bind the insert result first: `debug_assert!` is a no-op in release + // and would otherwise drop the side effect entirely. + let prev_leaf = self.leaf_values.insert(key, leaf); + debug_assert_eq!(prev_leaf, None); // Hash leaf with key let leaf_hash = hash_concat(&leaf, &key); @@ -295,8 +298,9 @@ impl SparseMerkleTree { }; } } - // Update the merkle root - debug_assert_ne!(self.nodes.insert((0, [0; 32]), current_hash), Some(current_hash)); + // Update the merkle root. Same caveat as above: bind first, assert second. + let prev_root = self.nodes.insert((0, [0; 32]), current_hash); + debug_assert_ne!(prev_root, Some(current_hash)); Ok(()) }