fix: write only defined bytes into v1 tree and queue accounts - #2392
Conversation
The v1 concurrent Merkle tree changelog and the v1 hash set queues copied
undefined bytes into account data, so account bytes depended on runtime
stack contents. Those bytes can hold VM pointers, which makes account
state diverge when the virtual_address_space_adjustments feature changes
the address layout.
Concurrent Merkle tree changelog:
- CyclicBoundedVec::push copies a ChangelogEntry with ptr::write,
including the undefined value bytes of None nodes and the repr(C)
padding between path and index. Route all pushes through
push_changelog_entry, which zeroes the slot and then writes only the
index and the Some nodes.
Hash set buckets:
- Writing None or Some(HashSetCell { sequence_number: None, .. }) left the
unused enum payload bytes undefined. Write buckets through a repr(C)
RawHashSetCell that defines all 48 bytes.
- Reject zero-copy buffers that are missing the reserved 8-byte gap
before the buckets instead of reading past the end.
Pin the deployed layouts with compile-time assertions: changelog node tag
values, entry sizes and index offsets for heights 22/26/32/40, the hash
set bucket layout and niche tag values, and the v1 indexed changelog
entry. Items used only by these assertions live inside the const blocks,
because older rustc versions (platform-tools) report them as dead code.
Types, account sizes and on-chain layout are unchanged.
Add tests that prefill account buffers with marker bytes and check that
every written byte is defined.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: Lightprotocol/light-protocol/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (8)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe concurrent Merkle tree, hash set, and indexed Merkle tree add account-layout assertions and deterministic byte-write checks. Tests inspect serialized changelog entries and hash-set buckets, including buffer-size validation. ChangesDefined account bytes
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to This change makes v1 tree and queue account bytes deterministic without changing account sizes or on-chain layout. The new layout assertions and tests check the written bytes, and no remaining issue blocks merging. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 Clippy (1.98.1)Clippy execution failed Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
The v1 concurrent Merkle tree changelog and the v1 hash set queues copied undefined bytes into account data. Those bytes came from the stack and can hold VM pointers, so account state diverges when the
virtual_address_space_adjustmentsfeature changes the address layout.This PR contains only the
program-libspart of #2389, squashed into one commit and rebased onmain.Changes
push_changelog_entry, which zeroes the slot and then writes only the index and theSomenodes. Before,CyclicBoundedVec::pushcopied the undefined value bytes ofNonenodes and therepr(C)padding beforeindex.repr(C)RawHashSetCellthat defines all 48 bytes. Before, writingNoneorSome(HashSetCell { sequence_number: None, .. })left the unused enum payload bytes undefined.Types, account sizes and on-chain layout are unchanged.
HashSet::size_in_accountreturns the same value as before, since the old formula always added 8 bytes.CI fix compared to #2389
#2389 failed in
just programs build: the platform-tools rustc reportsMARKED_BUCKET_TAGandbucket_tagas dead code under-D warnings, because older rustc versions don't count uses insideconst _: () = { .. }blocks. Both now live inside the assertion block. Reproduced and verified with rustc 1.84.1.Test plan
cargo test -p light-concurrent-merkle-tree -p light-hash-set -p light-indexed-merkle-tree(Rust 1.91,RUSTFLAGS="-D warnings")cargo clippyon the three crates with--all-targets -- -D warningscargo +1.84.1 rustc --lib -- -D warningson the three crates (reproduces the fix: write only defined bytes into concurrent Merkle tree changelog #2389 failure before the fix)RUSTFLAGS="-D warnings" cargo build-sbf --features 'test, migrate-state'inprograms/account-compression🤖 Generated with Claude Code
Summary by CodeRabbit