feat: allow for accountsdb checksum computation

[bmuddha]

Summary

Adds method to compute the checksum of current state of accountsdb.
The checksum computation is performed after acquisition of global lock.

Compatibility

• No breaking changes

Testing

• 2 new tests are added

Checklist

• closes feat: add checksum computation to AccountsDb snapshots #957

Summary by CodeRabbit

• New Features

  • Added a checksum to verify account database integrity and consistency.

• Improvements

  • Added a hashing library and updated dependencies to support deterministic checksums.
  • Refined storage size reporting to reflect actual used data rather than allocated capacity.
  • Updated dependency revisions for related components.

• Tests

  • Added tests to ensure checksum determinism across instances and to detect state changes.

[bmuddha]

• feat: initial attempt at replicator integration #1049
• feat: add sliding block history to executor #1047
• feat: add replicator service with NATS JetStream #1007
• feat(accountsdb): implement external snapshot insertion #1031
• feat: dual mode transaction scheduler #1004
• feat: forward original bincoded transaction #991
• feat: allow for accountsdb checksum computation #990 👈 (View in Graphite)
• feat: modify account cloning to use transactions #974
• master

This stack of pull requests is managed by Graphite. Learn more about stacking.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Adds AccountsDb::checksum() returning a deterministic u64 by acquiring the write lock, iterating all active accounts in LMDB key-sorted order, and hashing pubkey bytes plus serialized account data using xxHash3 (twox-hash). Adds twox-hash dependency and unit tests verifying determinism and detection of state changes. Changes storage.size_bytes() to return the active data segment length rather than allocated capacity. Updates solana-account revision in workspace and test-integration Cargo.toml files.

Assessment against linked issues

Objective	Addressed	Explanation
New method on AccountsDb to compute state checksum ([#957])	✅
Scans all active accounts and hashes them cumulatively ([#957])	✅
Returns single hash value (u64) ([#957])	✅
Unit tests for correctness and determinism ([#957])	✅
Integrates with existing snapshot flow ([#957])	❌	Checksum() added but no snapshot integration points/hooks were implemented.

Out-of-scope changes

Code Change	Explanation
storage.size_bytes() now returns active_segment().len() (magicblock-accounts-db/src/storage.rs)	Changes reported semantics from allocated capacity to used payload; not required by checksum objective.
solana-account revision bumped from 6eae52b to 3ff1b2ea (Cargo.toml, test-integration/Cargo.toml)	Dependency revision updates are unrelated to implementing checksum() and its unit tests.

Suggested reviewers

• thlorenz
• GabrielePicco

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch bmuddha/accountsdb/checksum

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@magicblock-accounts-db/src/lib.rs`:
- Around line 386-397: Change checksum() to return AccountsDbResult<u64> (not
u64), propagate any LMDB/index errors instead of swallowing them, and surface
failures from iter_all() rather than treating an empty iterator as success;
specifically update checksum() to call the underlying index/get_all_accounts
path in a fallible way and return Err(...) on any index/LMDB error. While
iterating accounts inside checksum(), do not silently skip Owned accounts: when
acc.as_borrowed() is None, emit a tracing::error! (including the pubkey) and
return an Err variant indicating an unexpected Owned account (so callers see the
failure). Keep the existing hashing logic (xxhash3_64::Hasher) for Borrowed
accounts only, but ensure all error paths map to AccountsDbResult and are
returned to the caller.

In `@magicblock-accounts-db/src/storage.rs`:
- Around line 374-377: The current size_bytes() change breaks reload() because
it uses active_segment().len() (used bytes) to grow the snapshot file, shrinking
capacity after rollback; update reload() to ensure the file is grown to the full
mmap extent instead of calling size_bytes(): call ensure_file_size(...) with the
full configured size (compute from the configured capacity_blocks × block_size +
METADATA_STORAGE_SIZE or use the stored mmap extent/initial file size) so
map_file recomputes capacity_blocks from the original full allocation; keep
size_bytes() semantics if it must represent used bytes, but do not use it in
reload(); also adjust or add a test (e.g. test_db_size_after_rollback) to
exercise an actual rollback (target_slot < current slot) to verify capacity is
preserved.

In `@magicblock-accounts-db/src/tests.rs`:
- Around line 513-521: The second assertion is comparing env.checksum() to
original_checksum even though accounts[5] was already mutated, so save the
intermediate checksum immediately after inserting accounts[5] (e.g., let
intermediate_checksum = env.checksum()) and then modify
accounts[10].1.set_lamports(...) and env.insert_account(&accounts[10].0,
&accounts[10].1).unwrap(); finally assert_ne!(env.checksum(),
intermediate_checksum, "checksum must detect lamport change") to ensure the
lamport mutation on accounts[10] is the change being tested.

[coderabbitai]

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@magicblock-accounts-db/src/lib.rs`:
- Around line 386-397: Change checksum() to return AccountsDbResult<u64> and
propagate LMDB/iteration errors instead of silently returning an empty checksum:
call iter_all() and if it returns Err (originating from
self.index.get_all_accounts()) return that error wrapped with context; when
iterating, treat acc.as_borrowed() returning None as an error (do not continue
silently) and return a descriptive AccountsDbError indicating a failed account
borrow/unsupported Owned variant (include pubkey info). Keep using
xxhash3_64::Hasher but only finish and Ok(hasher.finish()) on success; update
callers to handle the AccountsDbResult<u64>.

In `@magicblock-accounts-db/src/storage.rs`:
- Around line 374-377: size_bytes() now returns only the active-used portion
(cursor*block_size + METADATA_STORAGE_SIZE) which causes reload() (which calls
ensure_file_size and then map_file that computes capacity_blocks = (file_size -
METADATA_STORAGE_SIZE) / block_size) to see a reduced file_size after rollbacks
and incorrectly shrink capacity; fix reload() so it uses the actual on-disk file
length (e.g., via file metadata or the raw segment file length) when calling
ensure_file_size/map_file instead of self.size_bytes(), leaving size_bytes()
semantics unchanged; update references in reload(), ensure_file_size/map_file
invocation, and any logic around capacity_blocks/METADATA_STORAGE_SIZE to read
the real file size rather than active_segment()/size_bytes().

In `@magicblock-accounts-db/src/tests.rs`:
- Around line 518-527: The test's second assertion doesn't isolate the lamport
change because env.checksum() already diverged after mutating and committing
accounts[5]; capture the checksum immediately after the accounts[5] commit
(e.g., let checksum_after_commit = env.checksum()) and then mutate accounts[10]
via accounts[10].1.set_lamports(...) and env.insert_account(...), then
assert_ne!(env.checksum(), checksum_after_commit) to ensure the assertion
specifically tests the lamports change; reference env.checksum,
accounts[10].1.set_lamports, env.insert_account and
original_checksum/accounts[5] to locate the relevant lines.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@magicblock-accounts-db/src/storage.rs`:
- Around line 374-376: The method size_bytes() currently returns
active_segment().len() (the occupied/used space) which is misleading; rename
size_bytes() to used_bytes() (or another clear name) and update all call sites
to use the new name, keeping the implementation returning
self.active_segment().len() as u64; search for references to size_bytes(),
active_segment(), and any docs/comments in the Storage struct to update names
and wording to reflect "used bytes" semantics.

In `@magicblock-accounts-db/src/tests.rs`:
- Around line 477-485: The test currently inserts the same 50 accounts into db1
and db2 in identical order; change it so insertion order differs between the two
DBs to catch order-sensitive iteration bugs by first producing the set of
keys/accounts (using Pubkey::new_unique and AccountSharedData::new as currently
done), then insert them into db1 in one order and into db2 in a different order
(e.g., reverse the vector or shuffle using a deterministic RNG) before calling
db1.insert_account and db2.insert_account, so the same logical dataset is
present but insertion order differs.

---

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3ed3c71 and 1d4ffb8.

⛔ Files ignored due to path filters (2)

• Cargo.lock is excluded by !**/*.lock
• test-integration/Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (6)

• Cargo.toml
• magicblock-accounts-db/Cargo.toml
• magicblock-accounts-db/src/lib.rs
• magicblock-accounts-db/src/storage.rs
• magicblock-accounts-db/src/tests.rs
• test-integration/Cargo.toml

[thlorenz]

LGTM

[bmuddha]

Merge activity

• Mar 9, 8:19 AM UTC: @bmuddha merged this pull request with Graphite.