Merged
Conversation
… check - Add CannotZeroCompleteRootHistory error type - Update zero_out_roots to return Result instead of panicking - Fix bounds check to use >= instead of == for better safety
- Remove unused assignment to latest_root_0 in test - Fix needless late initialization of last_batch0_root - Replace find().is_none() with !any() for better performance - Address all clippy warnings in batched merkle tree tests
Contributor
WalkthroughAdded an error variant to prevent zeroing the entire root history, changed root-zeroing to return Result and added safety checks, propagated the error across callers, split monolithic e2e tests into modular e2e_tests, added a test dev-dependency, and adjusted a few test loop counts. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Updater as Update flow
participant Zeroer as zero_out_roots
participant Roots as RootHistory
Updater->>Zeroer: zero_out_roots(sequence_number, first_safe_root_index)
Zeroer->>Roots: inspect root_history.len() and first_safe_root_index
alt Would zero all roots
Roots-->>Zeroer: detect unsafe range
Zeroer-->>Updater: Err(CannotZeroCompleteRootHistory)
else Safe to zero
Zeroer->>Roots: for i in 0..num_remaining_roots -> zero entry at first_safe_root_index + i
Roots-->>Zeroer: Ok
Zeroer-->>Updater: Ok(())
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
The test was incorrectly using get(index).unwrap().1 which expects a tuple, but should use get_by_key(key) which returns the value directly.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
program-tests/batched-merkle-tree-test/tests/e2e_tests/e2e.rs (1)
99-142: Log volume may swamp CI output.
println!fires on every iteration and branch of this 2.2k-loop, so the test emits tens of thousands of lines. That slows runs and buries real failures. Please consider downgrading to gated tracing (e.g.,tracing::debug!) or trimming the prints once the scenario is stable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
program-libs/batched-merkle-tree/src/errors.rs(2 hunks)program-libs/batched-merkle-tree/src/merkle_tree.rs(19 hunks)program-tests/batched-merkle-tree-test/Cargo.toml(1 hunks)program-tests/batched-merkle-tree-test/tests/e2e.rs(1 hunks)program-tests/batched-merkle-tree-test/tests/e2e_tests/address.rs(1 hunks)program-tests/batched-merkle-tree-test/tests/e2e_tests/e2e.rs(1 hunks)program-tests/batched-merkle-tree-test/tests/e2e_tests/mod.rs(1 hunks)program-tests/batched-merkle-tree-test/tests/e2e_tests/shared.rs(1 hunks)program-tests/batched-merkle-tree-test/tests/e2e_tests/simulate_txs.rs(1 hunks)program-tests/batched-merkle-tree-test/tests/e2e_tests/state.rs(1 hunks)program-tests/batched-merkle-tree-test/tests/merkle_tree.rs(0 hunks)
💤 Files with no reviewable changes (1)
- program-tests/batched-merkle-tree-test/tests/merkle_tree.rs
🧰 Additional context used
🧠 Learnings (17)
📚 Learning: 2025-10-16T06:33:19.426Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: program-libs/compressible/CLAUDE.md:0-0
Timestamp: 2025-10-16T06:33:19.426Z
Learning: Applies to program-libs/compressible/src/error.rs : Define error types with numeric codes in the 19xxx range and propagate hasher errors in the 7xxx range; include ProgramError conversions (Anchor, Pinocchio, Solana) in src/error.rs
Applied to files:
program-libs/batched-merkle-tree/src/errors.rsprogram-libs/batched-merkle-tree/src/merkle_tree.rs
📚 Learning: 2025-10-11T21:59:25.222Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: program-libs/account-checks/CLAUDE.md:0-0
Timestamp: 2025-10-11T21:59:25.222Z
Learning: Applies to program-libs/account-checks/src/error.rs : Maintain stable mapping of AccountError to ProgramError, including Pinocchio code mapping (1–11), in error.rs
Applied to files:
program-libs/batched-merkle-tree/src/errors.rsprogram-libs/batched-merkle-tree/src/merkle_tree.rs
📚 Learning: 2025-10-11T21:59:25.222Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: program-libs/account-checks/CLAUDE.md:0-0
Timestamp: 2025-10-11T21:59:25.222Z
Learning: Applies to program-libs/account-checks/src/**/*.rs : Return AccountError variants (codes 12006–12021) and rely on automatic ProgramError conversions; avoid returning raw ProgramError directly
Applied to files:
program-libs/batched-merkle-tree/src/errors.rs
📚 Learning: 2025-10-16T06:33:55.362Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: programs/compressed-token/program/CLAUDE.md:0-0
Timestamp: 2025-10-16T06:33:55.362Z
Learning: Applies to programs/compressed-token/program/programs/compressed-token/anchor/src/lib.rs : Define all program-specific error codes in anchor_compressed_token::ErrorCode in programs/compressed-token/anchor/src/lib.rs; return errors as ProgramError::Custom(error_code as u32)
Applied to files:
program-libs/batched-merkle-tree/src/errors.rs
📚 Learning: 2025-10-11T21:59:25.222Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: program-libs/account-checks/CLAUDE.md:0-0
Timestamp: 2025-10-11T21:59:25.222Z
Learning: Applies to program-libs/account-checks/src/account_iterator.rs : Use AccountIterator for sequential account retrieval to get precise file:line:column error locations; avoid manual index handling
Applied to files:
program-libs/batched-merkle-tree/src/errors.rsprogram-libs/batched-merkle-tree/src/merkle_tree.rs
📚 Learning: 2025-10-11T21:59:25.222Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: program-libs/account-checks/CLAUDE.md:0-0
Timestamp: 2025-10-11T21:59:25.222Z
Learning: Applies to program-libs/account-checks/src/checks.rs : Expose and maintain account validation helpers (check_owner, check_program, check_mut/non_mut, check_signer, check_discriminator, set_discriminator, check_pda_seeds, check_account_balance_is_rent_exempt, account_info_init) in checks.rs
Applied to files:
program-libs/batched-merkle-tree/src/errors.rsprogram-tests/batched-merkle-tree-test/tests/e2e_tests/address.rsprogram-tests/batched-merkle-tree-test/tests/e2e_tests/shared.rs
📚 Learning: 2025-10-15T03:46:26.767Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: programs/registry/CLAUDE.md:0-0
Timestamp: 2025-10-15T03:46:26.767Z
Learning: Applies to programs/registry/src/account_compression_cpi/mod.rs : Export each new operation module by adding pub mod <operation>; and re-export with pub use <operation>::*.
Applied to files:
program-tests/batched-merkle-tree-test/tests/e2e_tests/mod.rsprogram-tests/batched-merkle-tree-test/tests/e2e.rsprogram-tests/batched-merkle-tree-test/tests/e2e_tests/shared.rs
📚 Learning: 2025-10-11T21:59:25.222Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: program-libs/account-checks/CLAUDE.md:0-0
Timestamp: 2025-10-11T21:59:25.222Z
Learning: Applies to program-libs/account-checks/src/account_info/{solana.rs,pinocchio.rs,test_account_info.rs} : Gate SDK-specific implementations with #[cfg(feature = "solana"|"pinocchio"|"test-only")]
Applied to files:
program-tests/batched-merkle-tree-test/tests/e2e_tests/mod.rsprogram-tests/batched-merkle-tree-test/Cargo.tomlprogram-tests/batched-merkle-tree-test/tests/e2e_tests/shared.rs
📚 Learning: 2025-10-15T03:46:43.242Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: sdk-tests/sdk-token-test/CLAUDE.md:0-0
Timestamp: 2025-10-15T03:46:43.242Z
Learning: Applies to sdk-tests/sdk-token-test/**/tests/**/*.rs : Every test should only contain functional integration tests
Applied to files:
program-tests/batched-merkle-tree-test/tests/e2e.rs
📚 Learning: 2025-10-15T03:46:43.242Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: sdk-tests/sdk-token-test/CLAUDE.md:0-0
Timestamp: 2025-10-15T03:46:43.242Z
Learning: Run tests with: cargo test-sbf -p sdk-token-test --test <test-file-name>
Applied to files:
program-tests/batched-merkle-tree-test/tests/e2e.rs
📚 Learning: 2025-10-11T21:59:25.222Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: program-libs/account-checks/CLAUDE.md:0-0
Timestamp: 2025-10-11T21:59:25.222Z
Learning: Applies to program-libs/account-checks/**/Cargo.toml : Define features solana, pinocchio, and test-only in Cargo.toml; default build should enable none
Applied to files:
program-tests/batched-merkle-tree-test/Cargo.tomlprogram-tests/batched-merkle-tree-test/tests/e2e_tests/shared.rs
📚 Learning: 2025-08-14T00:36:53.191Z
Learnt from: ananas-block
Repo: Lightprotocol/light-protocol PR: 1909
File: program-libs/zero-copy/src/init_mut.rs:241-249
Timestamp: 2025-08-14T00:36:53.191Z
Learning: In the light-protocol zero-copy crate, performance is prioritized over safety checks for edge cases like Vec lengths exceeding u32::MAX, even when there might be wire format inconsistencies.
Applied to files:
program-tests/batched-merkle-tree-test/Cargo.toml
📚 Learning: 2025-10-16T06:33:19.426Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: program-libs/compressible/CLAUDE.md:0-0
Timestamp: 2025-10-16T06:33:19.426Z
Learning: Applies to program-libs/compressible/src/config.rs : Ensure serialization compatibility across Anchor, Pinocchio, and Borsh for core account types used by dependent programs
Applied to files:
program-libs/batched-merkle-tree/src/merkle_tree.rsprogram-tests/batched-merkle-tree-test/tests/e2e_tests/shared.rs
📚 Learning: 2025-10-15T03:46:26.767Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: programs/registry/CLAUDE.md:0-0
Timestamp: 2025-10-15T03:46:26.767Z
Learning: Applies to programs/registry/src/lib.rs : Load accounts according to type before check_forester: batched via BatchedMerkleTreeAccount::type_from_account_info(); regular via ctx.accounts.account.load()?.metadata; use custom deserialization when required.
Applied to files:
program-libs/batched-merkle-tree/src/merkle_tree.rsprogram-tests/batched-merkle-tree-test/tests/e2e_tests/address.rsprogram-tests/batched-merkle-tree-test/tests/e2e_tests/shared.rs
📚 Learning: 2025-10-15T03:46:26.767Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: programs/registry/CLAUDE.md:0-0
Timestamp: 2025-10-15T03:46:26.767Z
Learning: Applies to programs/registry/src/lib.rs : Instruction handlers must compute work_units by operation type: batch operations use account.queue_batches.batch_size; single operations use DEFAULT_WORK_V1; custom operations compute based on complexity.
Applied to files:
program-libs/batched-merkle-tree/src/merkle_tree.rs
📚 Learning: 2025-10-11T21:59:25.222Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: program-libs/account-checks/CLAUDE.md:0-0
Timestamp: 2025-10-11T21:59:25.222Z
Learning: Applies to program-libs/account-checks/src/account_info/test_account_info.rs : Use the mock AccountInfo implementation under the test-only feature for unit tests
Applied to files:
program-tests/batched-merkle-tree-test/tests/e2e_tests/simulate_txs.rsprogram-tests/batched-merkle-tree-test/tests/e2e_tests/shared.rs
📚 Learning: 2025-10-11T21:59:25.222Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: program-libs/account-checks/CLAUDE.md:0-0
Timestamp: 2025-10-11T21:59:25.222Z
Learning: Applies to program-libs/account-checks/src/account_info/{solana.rs,pinocchio.rs,test_account_info.rs} : Provide SDK-specific AccountInfoTrait implementations in account_info/{solana.rs,pinocchio.rs,test_account_info.rs}
Applied to files:
program-tests/batched-merkle-tree-test/tests/e2e_tests/shared.rs
🧬 Code graph analysis (5)
program-tests/batched-merkle-tree-test/tests/e2e_tests/state.rs (7)
program-libs/batched-merkle-tree/src/initialize_state_tree.rs (2)
init_batched_state_merkle_tree_accounts(117-200)get_state_merkle_tree_account_size_from_params(353-363)program-libs/batched-merkle-tree/src/merkle_tree.rs (5)
BatchedMerkleTreeAccount(1099-1099)BatchedMerkleTreeAccount(1109-1109)BatchedMerkleTreeAccount(1119-1119)pubkey(939-941)state_from_bytes(133-139)program-libs/batched-merkle-tree/src/queue.rs (3)
get_output_queue_account_size_from_params(591-608)BatchedQueueAccount(662-662)output_from_bytes(181-186)program-libs/hasher/src/hash_chain.rs (1)
create_hash_chain_from_slice(23-32)prover/client/src/prover.rs (1)
spawn_prover(17-51)program-tests/batched-merkle-tree-test/tests/e2e_tests/shared.rs (6)
rng(683-683)get_rnd_bytes(682-686)assert_output_queue_insert(985-1072)get_random_leaf(754-761)assert_nullifier_queue_insert(763-795)perform_input_update(688-752)program-libs/batched-merkle-tree/src/batch.rs (2)
get_state(122-124)bloom_filter_is_zeroed(126-128)
program-tests/batched-merkle-tree-test/tests/e2e_tests/e2e.rs (5)
program-libs/batched-merkle-tree/src/initialize_state_tree.rs (1)
init_batched_state_merkle_tree_accounts(117-200)program-libs/batched-merkle-tree/src/merkle_tree.rs (1)
get_merkle_tree_account_size_default(956-959)program-libs/batched-merkle-tree/src/queue.rs (3)
get_output_queue_account_size_default(574-589)BatchedQueueAccount(662-662)output_from_bytes(181-186)program-libs/hasher/src/hash_chain.rs (1)
create_hash_chain_from_slice(23-32)program-tests/batched-merkle-tree-test/tests/e2e_tests/shared.rs (6)
rng(683-683)get_rnd_bytes(682-686)assert_output_queue_insert(985-1072)get_random_leaf(754-761)assert_nullifier_queue_insert(763-795)perform_input_update(688-752)
program-tests/batched-merkle-tree-test/tests/e2e_tests/address.rs (5)
program-libs/batched-merkle-tree/src/initialize_address_tree.rs (2)
get_address_merkle_tree_account_size_from_params(172-182)init_batched_address_merkle_tree_account(83-129)program-libs/batched-merkle-tree/src/merkle_tree.rs (5)
BatchedMerkleTreeAccount(1099-1099)BatchedMerkleTreeAccount(1109-1109)BatchedMerkleTreeAccount(1119-1119)pubkey(939-941)address_from_bytes(173-178)prover/client/src/prover.rs (1)
spawn_prover(17-51)program-tests/batched-merkle-tree-test/tests/e2e_tests/shared.rs (4)
rng(683-683)get_rnd_bytes(682-686)assert_input_queue_insert(800-979)perform_address_update(23-113)program-libs/batched-merkle-tree/src/batch.rs (2)
bloom_filter_is_zeroed(126-128)get_state(122-124)
program-tests/batched-merkle-tree-test/tests/e2e_tests/simulate_txs.rs (6)
program-libs/batched-merkle-tree/src/initialize_state_tree.rs (1)
init_batched_state_merkle_tree_accounts(117-200)program-libs/batched-merkle-tree/src/merkle_tree.rs (7)
assert_batch_append_event_event(1031-1058)assert_nullify_event(1003-1029)get_merkle_tree_account_size_default(956-959)BatchedMerkleTreeAccount(1099-1099)BatchedMerkleTreeAccount(1109-1109)BatchedMerkleTreeAccount(1119-1119)pubkey(939-941)program-libs/batched-merkle-tree/src/queue.rs (3)
get_output_queue_account_size_default(574-589)BatchedQueueAccount(662-662)output_from_bytes(181-186)program-libs/hasher/src/hash_chain.rs (1)
create_hash_chain_from_slice(23-32)prover/client/src/prover.rs (1)
spawn_prover(17-51)program-tests/batched-merkle-tree-test/tests/e2e_tests/shared.rs (6)
rng(683-683)get_rnd_bytes(682-686)get_random_leaf(754-761)assert_nullifier_queue_insert(763-795)assert_output_queue_insert(985-1072)assert_merkle_tree_update(115-528)
program-tests/batched-merkle-tree-test/tests/e2e_tests/shared.rs (4)
program-libs/batched-merkle-tree/src/merkle_tree.rs (8)
assert_batch_adress_event(1060-1085)BatchedMerkleTreeAccount(1099-1099)BatchedMerkleTreeAccount(1109-1109)BatchedMerkleTreeAccount(1119-1119)pubkey(939-941)address_from_bytes(173-178)get_metadata(882-884)state_from_bytes(133-139)js/stateless.js/src/state/types.ts (1)
CompressedProof(373-386)program-libs/array-map/src/lib.rs (1)
find(69-71)program-libs/batched-merkle-tree/src/batch.rs (2)
get_num_inserted_elements(258-260)get_state(122-124)
🪛 GitHub Actions: lint
program-libs/batched-merkle-tree/src/merkle_tree.rs
[warning] 1174-1174: Diff in merkle_tree.rs:1174: assertion state change detected (formatting/diff in test).
[warning] 1203-1203: Diff in merkle_tree.rs:1203: assertion state change detected (formatting/diff in test).
[warning] 1305-1305: Diff in merkle_tree.rs:1305: assertion state change detected (formatting/diff in test).
[warning] 1413-1413: Diff in merkle_tree.rs:1413: assertion state change detected (formatting/diff in test).
[warning] 1468-1468: Diff in merkle_tree.rs:1468: assertion state change detected (formatting/diff in test).
🪛 GitHub Actions: rust
program-libs/batched-merkle-tree/src/merkle_tree.rs
[error] 1506-1506: unused variable: pre_roots
🪛 GitHub Check: lint
program-libs/batched-merkle-tree/src/merkle_tree.rs
[warning] 1174-1174:
Diff in /home/runner/work/light-protocol/light-protocol/program-libs/batched-merkle-tree/src/merkle_tree.rs
[warning] 1305-1305:
Diff in /home/runner/work/light-protocol/light-protocol/program-libs/batched-merkle-tree/src/merkle_tree.rs
🪛 GitHub Check: Test batched-merkle-tree-simulate
program-libs/batched-merkle-tree/src/merkle_tree.rs
[failure] 1506-1506:
unused variable: pre_roots
🔇 Additional comments (4)
program-tests/batched-merkle-tree-test/Cargo.toml (1)
25-25: Dependency addition lines up with new ArrayMap usage.The e2e helpers rely on
ArrayMap, so pulling inlight-array-mapas a workspace dev-dependency keeps this test crate self-contained.program-tests/batched-merkle-tree-test/tests/e2e.rs (1)
1-2: Module alias wires in the new e2e suite cleanly.Using
#[path]keeps the integration test crate tidy while exposing the reorganized helpers without deep relative paths.program-libs/batched-merkle-tree/src/errors.rs (1)
54-76: Error catalog stays numerically stable.Threading
CannotZeroCompleteRootHistoryand assigning code 14313 preserves the contiguous mapping, so downstream ProgramError conversions remain deterministic.program-tests/batched-merkle-tree-test/tests/e2e_tests/mod.rs (1)
1-4: Public module surface mirrors the new layout.Re-exporting
address,shared,simulate_txs, andstatekeeps the test suite discoverable from the top-level module.
ananas-block
force-pushed
the
jorrit/batched-merkle-tree-fix-clean
branch
2 times, most recently
from
f3fb931 to
8676539
Compare
ananas-block
changed the title
ananas-block
changed the title
ananas-block
force-pushed
the
jorrit/batched-merkle-tree-fix-clean
branch
from
8676539 to
d457876
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (11)
program-tests/registry-test/tests/tests.rs (2)
1883-1905: Document the rationale for the updated iteration count.The loop count was changed from 4 to 6, apparently to accommodate the new merkle tree zeroing behavior. Consider adding a comment explaining why 6 iterations are required, especially in relation to the safe root constraints introduced in the zeroing fix. If possible, derive this value from
tree_paramsrather than using a magic number.Example:
+ // Perform 6 updates to ensure sufficient safe roots are maintained + // after the zeroing operation (changed from 4 due to PR #2045 zeroing fix) for _ in 0..6 {
1931-1953: Document the rationale for the reduced iteration count.The loop count was decreased from 5 to 3, complementing the increase at line 1883. This maintains the total iteration count at 9 but redistributes the work. Consider adding a comment explaining this distribution, particularly how it relates to the new zeroing constraints that require at least one safe root to remain.
Example:
+ // Perform 3 updates (reduced from 5 to accommodate safe root requirements) for _ in 0..3 {program-tests/batched-merkle-tree-test/tests/e2e_tests/address.rs (2)
1-1: Consider removing or narrowing the scope ofunused_assignmentssuppression.While acceptable in test code, this file-wide directive may hide legitimate issues. Consider applying
#[allow(unused_assignments)]only to specific functions where needed.
143-143: Remove commented-out code.The commented line should be removed to maintain code clarity.
Apply this diff:
if tx == params.input_queue_batch_size as usize * 2 - 1 { // Error when the value is already inserted into the other batch. assert_eq!(result.unwrap_err(), BatchedMerkleTreeError::BatchNotReady); } else if tx >= params.input_queue_batch_size as usize - 1 - // || tx == params.input_queue_batch_size as usize {program-tests/batched-merkle-tree-test/tests/e2e_tests/state.rs (3)
1-1: Consider removing or narrowing the scope ofunused_assignmentssuppression.While acceptable in test code, this file-wide directive may hide legitimate issues. Consider applying
#[allow(unused_assignments)]only to specific functions where needed.
264-283: Clean up commented-out assertion code.The duplicate insertion test calls
unwrap_err()but doesn't verify the specific error type. Either uncomment and fix the assertion, or remove the commented code and document why the specific error type isn't checked.Consider uncommenting and fixing:
let result = merkle_tree_account.insert_nullifier_into_queue( &leaf.to_vec().try_into().unwrap(), leaf_index as u64, &tx_hash, ¤t_slot, ); - result.unwrap_err(); - // assert_eq!( - // result.unwrap_err(), - // BatchedMerkleTreeError::BatchInsertFailed.into() - // ); + assert!(result.is_err(), "Duplicate insertion should fail");
284-304: Clean up commented-out assertion code.Similar to the previous test block, this validates error behavior but doesn't check specific error types.
Apply similar cleanup as the previous suggestion.
program-tests/batched-merkle-tree-test/tests/e2e_tests/shared.rs (4)
1-1: Consider removing or narrowing the scope ofunused_assignmentssuppression.This file-wide directive may hide legitimate issues in test utility functions that could affect multiple test cases.
115-523: Consider refactoring this complex function into smaller helpers.This 408-line function handles multiple responsibilities:
- Root history updates
- Zeroing logic for input queues
- Zeroing logic for output queues
- Batch state transitions
- Extensive validation assertions
While the logic appears correct, the function would be more maintainable if split into focused helpers like:
validate_root_zeroing()validate_input_queue_update()validate_output_queue_update()track_batch_roots()
525-675: Consider extracting common validation logic.This function shares substantial logic with
assert_merkle_tree_update(lines 115-523), particularly the root zeroing validation. While some specialization is needed for address vs state trees, common validation patterns could be extracted into shared helpers to reduce duplication and improve maintainability.
795-974: Comprehensive input queue validation with room for improvement.This function thoroughly validates input queue insertions including:
- Bloom filter containment in correct batch
- Non-containment in other batches
- Hash chain updates
- Batch state transitions
- Zeroing behavior
The validation is sound but the 179-line function could benefit from extracting sub-validations into focused helpers for improved maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
program-libs/batched-merkle-tree/src/merkle_tree.rs(19 hunks)program-tests/batched-merkle-tree-test/tests/e2e_tests/address.rs(1 hunks)program-tests/batched-merkle-tree-test/tests/e2e_tests/shared.rs(1 hunks)program-tests/batched-merkle-tree-test/tests/e2e_tests/state.rs(1 hunks)program-tests/registry-test/tests/tests.rs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- program-libs/batched-merkle-tree/src/merkle_tree.rs
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: program-libs/compressible/CLAUDE.md:0-0
Timestamp: 2025-10-16T06:33:19.426Z
Learning: Applies to program-libs/compressible/src/error.rs : Define error types with numeric codes in the 19xxx range and propagate hasher errors in the 7xxx range; include ProgramError conversions (Anchor, Pinocchio, Solana) in src/error.rs
📚 Learning: 2025-10-11T21:59:25.222Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: program-libs/account-checks/CLAUDE.md:0-0
Timestamp: 2025-10-11T21:59:25.222Z
Learning: Applies to program-libs/account-checks/src/checks.rs : Expose and maintain account validation helpers (check_owner, check_program, check_mut/non_mut, check_signer, check_discriminator, set_discriminator, check_pda_seeds, check_account_balance_is_rent_exempt, account_info_init) in checks.rs
Applied to files:
program-tests/batched-merkle-tree-test/tests/e2e_tests/address.rsprogram-tests/batched-merkle-tree-test/tests/e2e_tests/shared.rs
📚 Learning: 2025-10-15T03:46:26.767Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: programs/registry/CLAUDE.md:0-0
Timestamp: 2025-10-15T03:46:26.767Z
Learning: Applies to programs/registry/src/lib.rs : Load accounts according to type before check_forester: batched via BatchedMerkleTreeAccount::type_from_account_info(); regular via ctx.accounts.account.load()?.metadata; use custom deserialization when required.
Applied to files:
program-tests/batched-merkle-tree-test/tests/e2e_tests/address.rsprogram-tests/batched-merkle-tree-test/tests/e2e_tests/shared.rs
📚 Learning: 2025-10-15T03:46:26.767Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: programs/registry/CLAUDE.md:0-0
Timestamp: 2025-10-15T03:46:26.767Z
Learning: Applies to programs/registry/src/account_compression_cpi/mod.rs : Export each new operation module by adding pub mod <operation>; and re-export with pub use <operation>::*.
Applied to files:
program-tests/batched-merkle-tree-test/tests/e2e_tests/shared.rs
📚 Learning: 2025-10-11T21:59:25.222Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: program-libs/account-checks/CLAUDE.md:0-0
Timestamp: 2025-10-11T21:59:25.222Z
Learning: Applies to program-libs/account-checks/src/account_info/test_account_info.rs : Use the mock AccountInfo implementation under the test-only feature for unit tests
Applied to files:
program-tests/batched-merkle-tree-test/tests/e2e_tests/shared.rs
📚 Learning: 2025-10-11T21:59:25.222Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: program-libs/account-checks/CLAUDE.md:0-0
Timestamp: 2025-10-11T21:59:25.222Z
Learning: Applies to program-libs/account-checks/src/account_info/{solana.rs,pinocchio.rs,test_account_info.rs} : Gate SDK-specific implementations with #[cfg(feature = "solana"|"pinocchio"|"test-only")]
Applied to files:
program-tests/batched-merkle-tree-test/tests/e2e_tests/shared.rs
📚 Learning: 2025-10-16T06:33:19.426Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: program-libs/compressible/CLAUDE.md:0-0
Timestamp: 2025-10-16T06:33:19.426Z
Learning: Applies to program-libs/compressible/src/config.rs : Ensure serialization compatibility across Anchor, Pinocchio, and Borsh for core account types used by dependent programs
Applied to files:
program-tests/batched-merkle-tree-test/tests/e2e_tests/shared.rs
🧬 Code graph analysis (3)
program-tests/batched-merkle-tree-test/tests/e2e_tests/state.rs (7)
program-libs/batched-merkle-tree/src/initialize_state_tree.rs (2)
init_batched_state_merkle_tree_accounts(117-200)get_state_merkle_tree_account_size_from_params(353-363)program-libs/batched-merkle-tree/src/merkle_tree.rs (5)
BatchedMerkleTreeAccount(1099-1099)BatchedMerkleTreeAccount(1109-1109)BatchedMerkleTreeAccount(1119-1119)pubkey(939-941)state_from_bytes(133-139)program-libs/batched-merkle-tree/src/queue.rs (2)
get_output_queue_account_size_from_params(591-608)BatchedQueueAccount(662-662)program-libs/hasher/src/hash_chain.rs (1)
create_hash_chain_from_slice(23-32)prover/client/src/prover.rs (1)
spawn_prover(17-51)program-tests/batched-merkle-tree-test/tests/e2e_tests/shared.rs (6)
rng(678-678)get_rnd_bytes(677-681)assert_output_queue_insert(980-1067)get_random_leaf(749-756)assert_nullifier_queue_insert(758-790)perform_input_update(683-747)program-libs/batched-merkle-tree/src/batch.rs (2)
get_state(122-124)bloom_filter_is_zeroed(126-128)
program-tests/batched-merkle-tree-test/tests/e2e_tests/address.rs (5)
program-libs/batched-merkle-tree/src/initialize_address_tree.rs (2)
get_address_merkle_tree_account_size_from_params(172-182)init_batched_address_merkle_tree_account(83-129)program-libs/batched-merkle-tree/src/merkle_tree.rs (5)
BatchedMerkleTreeAccount(1099-1099)BatchedMerkleTreeAccount(1109-1109)BatchedMerkleTreeAccount(1119-1119)pubkey(939-941)address_from_bytes(173-178)prover/client/src/prover.rs (1)
spawn_prover(17-51)program-tests/batched-merkle-tree-test/tests/e2e_tests/shared.rs (4)
rng(678-678)get_rnd_bytes(677-681)assert_input_queue_insert(795-974)perform_address_update(23-113)program-libs/batched-merkle-tree/src/batch.rs (2)
bloom_filter_is_zeroed(126-128)get_state(122-124)
program-tests/batched-merkle-tree-test/tests/e2e_tests/shared.rs (4)
program-libs/batched-merkle-tree/src/merkle_tree.rs (8)
assert_batch_adress_event(1060-1085)BatchedMerkleTreeAccount(1099-1099)BatchedMerkleTreeAccount(1109-1109)BatchedMerkleTreeAccount(1119-1119)pubkey(939-941)address_from_bytes(173-178)get_metadata(882-884)state_from_bytes(133-139)program-libs/batched-merkle-tree/src/queue.rs (1)
BatchedQueueAccount(662-662)program-libs/batched-merkle-tree/src/queue_batch_metadata.rs (1)
get_previous_batch(72-74)program-libs/batched-merkle-tree/src/batch.rs (2)
get_num_inserted_elements(258-260)get_state(122-124)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: programs (system-cpi-test, ["cargo-test-sbf -p system-cpi-test", "cargo test -p light-system-prog...
- GitHub Check: programs (system-cpi-test-v2-functional-account-infos, ["cargo-test-sbf -p system-cpi-v2-test -- ...
- GitHub Check: programs (compressed-token-batched-tree, ["cargo-test-sbf -p compressed-token-test -- test_transf...
- GitHub Check: programs (system-cpi-test-v2-functional-read-only, ["cargo-test-sbf -p system-cpi-v2-test -- func...
- GitHub Check: programs (compressed-token-and-e2e, ["cargo-test-sbf -p compressed-token-test --test v1", "cargo-...
- GitHub Check: programs (account-compression-and-registry, ["cargo-test-sbf -p account-compression-test", "cargo...
- GitHub Check: programs (light-system-program-address, ["cargo-test-sbf -p system-test -- test_with_address", "c...
- GitHub Check: system-programs (native, ["cargo-test-sbf -p sdk-native-test", "cargo-test-sbf -p sdk-v1-native-t...
- GitHub Check: system-programs (anchor & pinocchio, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -p sdk...
- GitHub Check: system-programs (sdk-libs, light-sdk-macros light-sdk light-program-test light-client light-compr...
- GitHub Check: system-programs (token test, ["cargo-test-sbf -p sdk-token-test"])
- GitHub Check: Forester e2e test
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: Test program-libs-slow
- GitHub Check: Test program-libs-fast
- GitHub Check: lint
- GitHub Check: cli-v2
- GitHub Check: cli-v1
- GitHub Check: stateless-js-v2
- GitHub Check: stateless-js-v1
🔇 Additional comments (5)
program-tests/batched-merkle-tree-test/tests/e2e_tests/address.rs (2)
203-221: Excellent validation of unsafe root zeroing behavior.This assertion properly validates the core fix in this PR - ensuring all unsafe roots from a completed batch are zeroed from the root history when the next batch becomes half-full.
258-274: LGTM - Root history validation logic is sound.This loop correctly validates that:
- The root at the expected index matches the tracked update root
- The preceding index is zeroed
- All other indices up to this point are zeroed
This ensures the zeroing behavior works as intended.
program-tests/batched-merkle-tree-test/tests/e2e_tests/state.rs (1)
344-367: Excellent validation of unsafe root zeroing for state trees.This correctly validates that after batch 0 is fully inserted and batch 1 becomes half-full, all unsafe roots from batch 0 are removed from the root history. This is the key fix being tested in this PR.
program-tests/batched-merkle-tree-test/tests/e2e_tests/shared.rs (2)
191-221: Core unsafe root zeroing logic is well-implemented.This section correctly implements the expected behavior:
- Identifies the last unsafe root and first safe root indices
- Calculates the number of roots to zero based on sequence numbers
- Zeros out old unsafe roots in a circular buffer manner
The debug logging is valuable for test troubleshooting.
222-264: Comprehensive validation of root zeroing correctness.This validation logic effectively ensures:
- All unsafe roots from the completed batch are zeroed
- The correct number of safe roots remain non-zero
- Tracked roots are cleared for batch index reuse
The assertions provide strong guarantees about the zeroing behavior's correctness.
ananas-block
commented
Nov 10, 2025
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
program-libs/batched-merkle-tree/src/merkle_tree.rs (1)
750-782: Approve the fix, but update the outdated comment.The changes correctly implement the fix described in the PR objectives:
- ✅ Loop changed from
1..num_remaining_rootsto0..num_remaining_rootsto zero all unsafe roots- ✅ Validation added to prevent zeroing the complete root history
- ✅ Function signature changed to return Result for proper error handling
However, the comment on lines 768-769 is now outdated and contradicts the implementation.
Apply this diff to update the comment:
// 2.2. Zero out roots oldest to first safe root. - // Skip one iteration we don't need to zero out - // the first safe root. for _ in 0..num_remaining_roots {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
program-libs/batched-merkle-tree/src/merkle_tree.rs(18 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-10-11T21:59:25.222Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: program-libs/account-checks/CLAUDE.md:0-0
Timestamp: 2025-10-11T21:59:25.222Z
Learning: Applies to program-libs/account-checks/src/error.rs : Maintain stable mapping of AccountError to ProgramError, including Pinocchio code mapping (1–11), in error.rs
Applied to files:
program-libs/batched-merkle-tree/src/merkle_tree.rs
📚 Learning: 2025-10-15T03:46:26.767Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: programs/registry/CLAUDE.md:0-0
Timestamp: 2025-10-15T03:46:26.767Z
Learning: Applies to programs/registry/src/lib.rs : Load accounts according to type before check_forester: batched via BatchedMerkleTreeAccount::type_from_account_info(); regular via ctx.accounts.account.load()?.metadata; use custom deserialization when required.
Applied to files:
program-libs/batched-merkle-tree/src/merkle_tree.rs
📚 Learning: 2025-10-11T21:59:25.222Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: program-libs/account-checks/CLAUDE.md:0-0
Timestamp: 2025-10-11T21:59:25.222Z
Learning: Applies to program-libs/account-checks/src/account_iterator.rs : Use AccountIterator for sequential account retrieval to get precise file:line:column error locations; avoid manual index handling
Applied to files:
program-libs/batched-merkle-tree/src/merkle_tree.rs
📚 Learning: 2025-10-15T03:46:26.767Z
Learnt from: CR
Repo: Lightprotocol/light-protocol PR: 0
File: programs/registry/CLAUDE.md:0-0
Timestamp: 2025-10-15T03:46:26.767Z
Learning: Applies to programs/registry/src/lib.rs : Instruction handlers must compute work_units by operation type: batch operations use account.queue_batches.batch_size; single operations use DEFAULT_WORK_V1; custom operations compute based on complexity.
Applied to files:
program-libs/batched-merkle-tree/src/merkle_tree.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (21)
- GitHub Check: lint
- GitHub Check: stateless-js-v2
- GitHub Check: Forester e2e test
- GitHub Check: programs (light-system-program-address, ["cargo-test-sbf -p system-test -- test_with_address", "c...
- GitHub Check: cli-v1
- GitHub Check: programs (system-cpi-test, ["cargo-test-sbf -p system-cpi-test", "cargo test -p light-system-prog...
- GitHub Check: programs (light-system-program-compression, ["cargo-test-sbf -p system-test -- test_with_compress...
- GitHub Check: programs (system-cpi-test-v2-functional-read-only, ["cargo-test-sbf -p system-cpi-v2-test -- func...
- GitHub Check: cli-v2
- GitHub Check: programs (system-cpi-test-v2-functional-account-infos, ["cargo-test-sbf -p system-cpi-v2-test -- ...
- GitHub Check: programs (compressed-token-batched-tree, ["cargo-test-sbf -p compressed-token-test -- test_transf...
- GitHub Check: programs (compressed-token-and-e2e, ["cargo-test-sbf -p compressed-token-test --test v1", "cargo-...
- GitHub Check: programs (account-compression-and-registry, ["cargo-test-sbf -p account-compression-test", "cargo...
- GitHub Check: system-programs (token test, ["cargo-test-sbf -p sdk-token-test"])
- GitHub Check: system-programs (sdk-libs, light-sdk-macros light-sdk light-program-test light-client light-compr...
- GitHub Check: system-programs (native, ["cargo-test-sbf -p sdk-native-test", "cargo-test-sbf -p sdk-v1-native-t...
- GitHub Check: system-programs (anchor & pinocchio, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -p sdk...
- GitHub Check: stateless-js-v1
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: Test program-libs-fast
- GitHub Check: Test program-libs-slow
🔇 Additional comments (2)
program-libs/batched-merkle-tree/src/merkle_tree.rs (2)
807-864: LGTM! The wraparound fix and safety checks are correctly implemented.The changes properly address the past review comment about wraparound:
- Line 845 uses modulo to wrap
first_safe_root_indexwithin the cyclic buffer bounds- Lines 826-829 add a pre-flight check to prevent calling
zero_out_rootswhen it would fail- Line 835-836 gate the zeroing operation on
!no_insert_since_last_batch_rootThe logic correctly ensures that:
- We never attempt to zero out the complete root history (which would delete the tree state)
- The
first_safe_root_indexis properly wrapped for cyclic buffer operations- The validation in
zero_out_rootsacts as a defensive backstop for edge cases
1141-1573: Comprehensive test coverage for the fix.The test updates thoroughly verify the fix:
- ✅ New test case 4.1 (lines 1239-1250) verifies the safety check prevents zeroing when no safe roots exist
- ✅ Assertions at lines 1302 and 1512-1515 verify that all unsafe roots are now zeroed (including
last_batch0_rootandlast_batch0_root_update2), confirming the fix addresses the bug where the first unsafe root was previously left un-zeroed- ✅ Test case 9 (lines 1421-1517) comprehensively tests the complex scenario with overlapping batches and root updates
The test assertions directly validate that the loop change from
1..num_remaining_rootsto0..num_remaining_rootscorrectly zeros all unsafe roots as intended.
SwenSchaeferjohann
approved these changes
Nov 11, 2025
ananas-block
added a commit
that referenced
this pull request
Nov 17, 2025
This reverts commit 435e679.
ananas-block
added a commit
that referenced
this pull request
Nov 18, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue:
batch.root_indexmarks the last unsafe root not the first safe rootFix:
1..num_remaining_roots {->0..num_remaining_roots {Changes:
fn should_zero_bloom_filter, refactors conditional logic into a separate function for easier formal verificationbatched-merkle-tree-tests/merkle_tree.rsinto individual files in a directorySummary by CodeRabbit
Bug Fixes
Tests
Chores