Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
4f5ee62 to
e964aa5
Compare
5507317 to
cc10928
Compare
b2f67ae to
c472d84
Compare
4e77f6c to
dfef7ee
Compare
02e0c7b to
6460a11
Compare
81e789d to
8d9b940
Compare
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Warning
|
| Layer / File(s) | Summary |
|---|---|
v42 calculator program Cargo.toml, programs/v42-calculator-program/... |
Added the calculator SBF crate, entrypoint dispatch, postfix evaluator, nested self-CPI, account and clock operations, transfer processing, checked arithmetic, and stable custom errors. |
Keeper packaging and testkit integration Cargo.toml, keeper/Cargo.toml, keeper/build.rs, keeper/README.md, keeper/src/testkit.rs |
Added workspace integration, optional v42 SBF builds, testkit constructors, transaction and account helpers, snapshot helpers, corruption helpers, and Keeper documentation. |
Keeper startup and durable state keeper/src/error.rs, keeper/src/builder.rs, keeper/src/lib.rs, keeper/src/metrics.rs |
Added Keeper construction, startup account seeding, accounts database validation, snapshot restoration and archival, synchronization, reset handling, typed errors, and Prometheus metrics. |
Caches and subscriptions keeper/src/cache.rs, keeper/src/subscriptions.rs |
Added account-load coordination, expiring caches, block tracking, keyed subscriptions, broadcast channels, periodic cleanup, and shutdown handling. |
Execution coordination API keeper/src/accessor.rs, keeper/src/util.rs, keeper/src/lib.rs |
Added account, transaction, block, and superblock accessors. Added ledger request handling, execution commit conversion, state transition persistence, replay, finalization, and synchronization operations. |
Keeper behavioral validation keeper/src/tests/* |
Added tests for cache behavior, concurrent account loading, startup seeding, snapshot recovery, subscription lifecycle, transaction deduplication, and cached transaction status. |
Estimated code review effort: 5 (Critical) | ~120 minutes
Sequence Diagram(s)
sequenceDiagram
participant Runtime
participant Keeper
participant AccountsDB
participant Ledger
participant Subscribers
Runtime->>Keeper: append and execute transaction
Keeper->>AccountsDB: load and update account state
Keeper->>Ledger: persist transaction and execution event
Ledger-->>Keeper: return commit result
Keeper->>Subscribers: publish status, logs, and account updates
Subscribers-->>Runtime: deliver notifications
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly identifies the primary change: adding the keeper crate as a state orchestrator. |
| Description check | ✅ Passed | The description explains the keeper crate, its coordination role, recovery behavior, APIs, and planned integration. |
| Linked Issues check | ✅ Passed | The changes satisfy issue #12 by adding the keeper API and coordinating accountsdb, ledger, caches, execution results, and slot state. |
| Out of Scope Changes check | ✅ Passed | The changes support the keeper crate, including its testkit and calculator test program, with no clearly unrelated code. |
| Docstring Coverage | ✅ Passed | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. |
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
keeper
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 @coderabbitai help to get the list of available commands.
|
@CodeRabbit review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (1)
keeper/src/lib.rs (1)
213-217: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueA failed thread spawn leaves the temporary archive file behind.
dstis created at Line 215 before the thread spawn. Ifthread::Builder::spawnreturns an error at Line 217, the.tmpfile stays in the superblock directory. The nextfinalize_superblocktruncates it, so this is not a correctness failure, but the stale file remains after a terminal spawn failure.Remove
tmpon the spawn error path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@keeper/src/lib.rs` around lines 213 - 217, Update the thread creation flow around thread::Builder::new().name("snapshot-archiver") so a failed spawn removes the temporary archive at tmp before propagating the spawn error. Preserve the existing successful spawn behavior and error propagation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@keeper/src/accessor.rs`:
- Around line 214-215: Make the ledger appender transition at
accessor.rs:214-215 recoverable with the accountsdb mutation by changing the
Event::Execution flow around commit_state_transitions to use a durable
pending-transition or rollback/replay protocol, and publish the event only after
both stores commit. In accessor.rs:309-319, defer block-cache publication and
block subscriptions until update_sysvars and set_slot succeed, using the same
recovery protocol; both sites are in the accessor transition flow and must
preserve consistent ledger and account state after failures.
In `@keeper/src/builder.rs`:
- Around line 270-275: Update the restored-store failure branch in the
validation match to return a corruption-specific error such as
AccountsDBError::Corruption instead of SnapshotError::Missing, while preserving
the existing backup and logging behavior.
- Around line 256-286: In the accountsdb method, explicitly drop the AccountsDB
instance after saving the backup and before calling self.unarchive(ledger),
ensuring the LMDB environment and mapped storage are closed before unpacking the
restored snapshot. Preserve the existing restore and error-handling flow.
- Around line 196-207: Update the SlotHashes construction in the surrounding
builder flow to initialize it from the retained blocks returned by
handle.recv_timeout().await??, rather than seeding it with [Default::default();
SLOTHASH_ENTRIES]. Add each retained block’s slot and hash to the resulting
SlotHashes and preserve the existing account creation and last_block updates.
- Around line 161-170: The program-account construction loop in the builder must
account for the loader-v4 state layout: prepend the required LoaderV4State
header before each ELF (or switch to a loader matching the raw ELF layout), and
compute rent from the complete account data size. Preserve the loader_v4 owner
and executable configuration while ensuring the seeded data begins with valid
loader-v4 state.
In `@keeper/src/cache.rs`:
- Around line 40-48: Enforce the documented 256-slot minimum for the cache
capacity used by Cache::new and the lru_capacity configuration path: reject
values below 256 with a startup error before constructing HashCache, or
consistently update the configuration documentation and default to a lower
minimum. Preserve valid capacities and ensure the behavior matches the chosen
documented contract.
In `@keeper/src/lib.rs`:
- Around line 180-188: Update the final-shutdown logic in sync so every ledger
reader receives its own shutdown signal; do not rely on sending multiple
ReadRequest::Shutdown messages through the shared MPMC channel, since one reader
may consume more than one. Use the existing per-reader or broadcast mechanism if
available, and preserve the subsequent superblocks and accounts database
synchronization.
In `@keeper/src/metrics.rs`:
- Around line 69-72: Update the rustdoc for the metrics `init` function to
describe only one-time metrics registration via
`METRICS.get_or_init(Default::default)`. Remove the claim that gauges are seeded
from current caches, leaving the implementation unchanged.
- Around line 79-82: Update account_cache_eviction() to decrement
m.account_cache_entries when an account cache entry is evicted, while preserving
its existing eviction-counter increment. Keep account_cache_insert() increasing
the same gauge so ACCOUNT_CACHE_ENTRIES reflects current occupancy.
In `@keeper/src/subscriptions.rs`:
- Around line 43-50: Correct the rustdoc comments on the subscription fields:
end the `programs` description with a period instead of a semicolon, and update
the `blocks` description to state that it broadcasts newly committed `Block`
values rather than slots. Leave the field types and other comments unchanged.
- Around line 82-88: Update Subscriptions::send so sending and conditional
removal occur under the same bucket lock, preventing subscribe from inserting
between the send and removal. Prefer remove_if_sync with a predicate that sends
the cloned value and removes the channel when sending fails or oneshot is true;
otherwise use the existing send_sync/read_sync path while preserving atomic
removal semantics.
In `@keeper/src/testkit.rs`:
- Around line 3-5: Update the module-level rustdoc describing the Keeper test
configuration to state the values actually used by keeper_builder: 100 ms
blocktime and superblock 4, preserving the existing description of the other
parameters.
---
Nitpick comments:
In `@keeper/src/lib.rs`:
- Around line 213-217: Update the thread creation flow around
thread::Builder::new().name("snapshot-archiver") so a failed spawn removes the
temporary archive at tmp before propagating the spawn error. Preserve the
existing successful spawn behavior and error propagation.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9274987e-68ef-495d-9594-3228059455ce
📒 Files selected for processing (23)
Cargo.tomlkeeper/Cargo.tomlkeeper/README.mdkeeper/build.rskeeper/src/accessor.rskeeper/src/builder.rskeeper/src/cache.rskeeper/src/error.rskeeper/src/lib.rskeeper/src/metrics.rskeeper/src/subscriptions.rskeeper/src/testkit.rskeeper/src/tests/caches.rskeeper/src/tests/mod.rskeeper/src/tests/recovery.rskeeper/src/tests/subscriptions.rskeeper/src/util.rsprograms/v42-calculator-program/Cargo.tomlprograms/v42-calculator-program/README.mdprograms/v42-calculator-program/src/calculator.rsprograms/v42-calculator-program/src/error.rsprograms/v42-calculator-program/src/lib.rsprograms/v42-calculator-program/src/transfer.rs
| for (&program, elf) in &self.programs { | ||
| let lamports = self.rent.minimum_balance(elf.len()); | ||
| let account = AccountBuilder::default() | ||
| .lamports(lamports) | ||
| .mode(AccountMode::System) | ||
| .owner(solana_sdk_ids::loader_v4::ID) | ||
| .executable(true) | ||
| .data(elf.clone()); | ||
| accounts.push((program, account.build())); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🌐 Web query:
Solana loader_v4 program account data layout LoaderV4State header before ELF
💡 Result:
In the Solana loader_v4 program, the account data layout is structured with a header followed immediately by the raw program data (the ELF) [1][2]. The header is defined by the LoaderV4State struct, which occupies the first 48 bytes (0x30) of the account data [1][2]. The raw program data begins immediately at the offset defined by LoaderV4State::program_data_offset, which is 0x30 [1][2]. The LoaderV4State structure is defined as follows: - slot (u64): The slot in which the program was last deployed, retracted, or initialized (offset 0x00) [1][2]. - authority_address_or_next_version (Pubkey): The address of the signer that can send program management instructions when the status is not finalized; otherwise, it acts as a forwarding address to the next version of the finalized program (offset 0x08) [1][2]. - status (LoaderV4Status): The deployment status of the program (offset 0x28) [1][2]. The raw ELF data follows this header starting at offset 0x30 [1][2][3]. In practice, this is implemented by allocating an account size equal to the size of the header (48 bytes) plus the length of the ELF, and then copying the ELF bytes into the slice starting at the 0x30 offset [3].
Citations:
- 1: https://docs.rs/crate/solana-program/2.1.16/source/src/loader_v4.rs
- 2: https://docs.rs/hbros-solana-program/latest/src/hbros_solana_program/loader_v4.rs.html
- 3: https://github.com/anza-xyz/agave/blob/31aca18feb8385be3a1ca5d5e1b57f55c9cafe8b/programs/loader-v4/src/lib.rs
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate builder.rs and related files =="
fd -a 'builder\.rs$' . | sed 's#^\./##'
echo
echo "== git status =="
git status --short
echo
echo "== relevant imports and program builder implementation =="
if [ -f keeper/src/builder.rs ]; then
wc -l keeper/src/builder.rs
sed -n '1,240p' keeper/src/builder.rs
fi
echo
echo "== search for AccountBuilder / programs / loader_v4 =="
rg -n "AccountBuilder|programs|loader_v4|LoaderV4|bpf_loader_upgradeable|ProgramBuilder|upgradeable" -S .Repository: magicblock-labs/magicblock-engine
Length of output: 27618
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== account loader loader-specific validation =="
sed -n '500,580p' solana/svm/src/account_loader.rs
sed -n '1280,1370p' solana/svm/src/account_loader.rs
sed -n '156,210p' solana/program-runtime/src/serialization.rs
sed -n '540,590p' solana/program-runtime/src/invoke_context.rs
echo
echo "== loader loader_v4 implementation if present =="
fd -a 'lib\.rs$|loader|-loader|loader_v4\.rs' . | sed 's#^\./##' | head -120
rg -n "struct LoaderV4State|enum LoaderV4Status|LoaderV4State|program_data_offset|process_instruction|deserialize_data|bpf_loader_upgradeable|loader_v4" solana programs keeper -S | head --lines=240
echo
echo "== exact loader_v4 source files =="
git ls-files | rg '(^|/)loader[_-]v4|programs/loader|bpf_loader_upgradeable|loader_v4\.rs|program_runtime/src/loader' || trueRepository: magicblock-labs/magicblock-engine
Length of output: 20780
🌐 Web query:
Solana loader_v4 program loader code LoaderV4State deserialize account data
💡 Result:
The Loader v4 program on Solana handles program management for the v2 program runtime [1][2]. The LoaderV4State struct represents the on-chain state of an account managed by this loader [1][3]. To deserialize LoaderV4State from an account's data buffer in Rust, you can use a pattern that interprets the initial bytes of the account data as the struct [4][5]. The program provides a helper method, program_data_offset, which returns the size of the LoaderV4State structure, allowing you to separate the state metadata from the raw program ELF data that follows [1][3]. Example implementation for deserializing the state: pub fn get_state(data: &[u8]) -> Result<&LoaderV4State, InstructionError> { unsafe { let data = data.get(0..LoaderV4State::program_data_offset).ok_or(InstructionError::AccountDataTooSmall)?.try_into.unwrap; Ok(std::mem::transmute::< &[u8; LoaderV4State::program_data_offset], &LoaderV4State, >(data)) } } The LoaderV4State struct itself is defined with the repr(C) attribute to ensure memory layout compatibility [1][3]: #[repr(C)] #[derive(Debug, PartialEq, Eq, Clone, Copy)] pub struct LoaderV4State { pub slot: u64, pub authority_address_or_next_version: Pubkey, pub status: LoaderV4Status, // Raw program data follows this header } When working with these accounts, the data buffer consists of the fixed-size LoaderV4State header followed by the variable-length program ELF data [1][3]. The program_data_offset function is essentially a wrapper around std::mem::size_of:: [1][3].
Citations:
- 1: https://docs.rs/crate/solana-program/2.1.16/source/src/loader_v4.rs
- 2: https://docs.rs/solana-program/1.18.26/solana_program/loader_v4/index.html
- 3: https://docs.rs/hbros-solana-program/latest/src/hbros_solana_program/loader_v4.rs.html
- 4: https://github.com/anza-xyz/agave/blob/31aca18feb8385be3a1ca5d5e1b57f55c9cafe8b/programs/loader-v4/src/lib.rs
- 5: https://crates.io/crates/solana-loader-v4-program/4.0.0-beta.6/code
🌐 Web query:
Solana loader_v4 source program_data_offset deserialize account data loader_v4.rs
💡 Result:
In the Solana Loader V4 architecture, the LoaderV4State struct defines the layout of the account's header data [1][2]. The program's executable data (ELF) is stored in the same account, immediately following this header [1]. The program_data_offset() function is used to determine the exact byte offset where the program data begins within the account [1]. This offset is calculated as the size of the LoaderV4State structure itself [1]. In practice, this is 48 bytes (0x30 in hexadecimal) [1]. To deserialize the account data, the system retrieves the account's raw data slice and interprets the first program_data_offset() bytes as the LoaderV4State header [3]. The remaining bytes in the slice constitute the actual program executable [4][5]. The following logic demonstrates how this is typically handled: 1. Retrieve the program account's data [4][5]. 2. Validate that the account data length is at least LoaderV4State::program_data_offset() [3]. 3. Treat the slice from index 0 to program_data_offset() as the LoaderV4State header [3]. 4. The program ELF data is then accessed by slicing the account data from LoaderV4State::program_data_offset() to the end [4][5]. This structure allows the loader to maintain metadata (such as the program slot, authority, and status) at the beginning of the account, while keeping the variable-length program data separate and directly accessible [1][6].
Citations:
- 1: https://docs.rs/hbros-solana-program/latest/src/hbros_solana_program/loader_v4.rs.html
- 2: https://docs.rs/solana-program/1.18.26/solana_program/loader_v4/index.html
- 3: https://github.com/anza-xyz/agave/blob/31aca18feb8385be3a1ca5d5e1b57f55c9cafe8b/programs/loader-v4/src/lib.rs
- 4: https://github.com/solana-labs/solana/blob/master/svm/src/transaction_processor.rs
- 5: https://github.com/longcipher/hpsvm/blob/dc084c5b/crates/hpsvm/src/accounts_db.rs
- 6: https://github.com/solana-foundation/solana-improvement-documents/pull/167/files
🌐 Web query:
loader_v4 get_state AccountDataTooSmall LoaderV4State account data loader_v4.rs process_instruction
💡 Result:
In the context of the Solana loader_v4 program, the AccountDataTooSmall error is a standard InstructionError that occurs when an instruction attempts to operate on account data that is insufficient in size for the requested operation [1][2]. Technical Context The loader_v4 program manages the deployment and lifecycle of programs on Solana [3][4]. Within its implementation, specifically in loader_v4.rs (or lib.rs in the Agave repository), the get_state and get_state_mut functions attempt to cast the raw account data into a LoaderV4State struct [1][2]. This process involves slicing the account data at a specific offset defined by LoaderV4State::program_data_offset(). If the provided account data is smaller than this required offset, the function returns an InstructionError::AccountDataTooSmall [1]. Similarly, during data writing operations, the loader attempts to write data to the account at a specific destination offset [1]. If the remaining capacity of the account data is smaller than the size of the data being written (resulting in an out-of-bounds write), the loader explicitly returns InstructionError::AccountDataTooSmall to prevent memory corruption [1]. Common Practical Causes While the error is a programmatic safety check within loader_v4, developers most frequently encounter it during program deployment or upgrades (e.g., via Anchor or the Solana CLI) [5][6][7]. The error typically indicates that the existing program account does not have enough allocated space to hold the new, larger program binary [5][7]. Resolution To resolve this, the program account size must be increased. In modern Solana versions (1.18+), this can be handled using the Solana CLI without needing to redeploy to a new address [5][6][7]: solana program extend <PROGRAM_ID> <NUMBER_OF_BYTES> This command increases the allocated size of the specified program account, providing the necessary space for the deployment to succeed [5][7].
Citations:
- 1: https://github.com/anza-xyz/agave/blob/31aca18feb8385be3a1ca5d5e1b57f55c9cafe8b/programs/loader-v4/src/lib.rs
- 2: https://crates.io/crates/solana-loader-v4-program/4.0.0-beta.6/code
- 3: https://docs.rs/solana-sdk/1.18.26/solana_sdk/loader_v4/index.html
- 4: https://docs.rs/solana-program/1.18.26/solana_program/loader_v4/index.html
- 5: https://solana.stackexchange.com/questions/15016/got-error-account-data-too-small-for-instruction-when-run-anchor-deploy
- 6: https://stackoverflow.com/questions/71267943/solana-deploy-account-data-too-small-for-instruction
- 7: RPC response error -32002: Transaction simulation failed: Error processing Instruction 0: account data too small for instruction otter-sec/anchor#2928
🌐 Web query:
Solana svm account_loader load_program_accounts loader_v4 Deserialize LoaderV4State AccountDataTooSmall
💡 Result:
In the Solana VM (SVM), the AccountDataTooSmall error occurring during load_program_accounts for a Loader V4 program indicates that the account data provided is insufficient to contain the required LoaderV4State structure [1]. The LoaderV4State is a fixed-size header that precedes the actual program (ELF) data within the account's data buffer [2]. The system attempts to deserialize this header by taking a slice of the account data corresponding to LoaderV4State::program_data_offset() [1][2]. If the account's data length is less than this offset, the operation fails with InstructionError::AccountDataTooSmall [1]. Key technical points regarding this error include: 1. Fixed Header Size: Loader V4 accounts must have at least enough space to accommodate the LoaderV4State struct, which includes fields such as the slot, authority address, and status [2]. The program_data_offset() function returns the exact size of this header [2]. 2. Validation Logic: During the program loading process, the SVM checks if the account data size is at least as large as the defined offset [1]. If the data is truncated or improperly initialized, the get_state or get_state_mut functions—which perform the memory mapping or slicing—will return InstructionError::AccountDataTooSmall when the slice operation fails [1]. 3. Context: This error typically occurs in the program_loader or Loader trait implementation within the SVM when preparing a program for execution [3][4]. It indicates that the account associated with the program is either corrupted, uninitialized, or has been truncated below the minimum size required by the Loader V4 interface [1][5]. To resolve this, ensure that the program account is properly initialized with a data length sufficient to hold both the LoaderV4State header and the binary program data [2]. If interacting with the account via raw bytes, verify that the data slice contains at least std::mem::size_of::<LoaderV4State>() bytes before attempting to interpret it as a Loader V4 state [1][2].
Citations:
- 1: https://github.com/anza-xyz/agave/blob/31aca18feb8385be3a1ca5d5e1b57f55c9cafe8b/programs/loader-v4/src/lib.rs
- 2: https://docs.rs/hbros-solana-program/latest/src/hbros_solana_program/loader_v4.rs.html
- 3: SVM: add
program_loadermodule anza-xyz/agave#1017 - 4: SVM:
Loadertrait anza-xyz/agave#1739 - 5: https://github.com/longcipher/hpsvm/blob/dc084c5b/crates/hpsvm/src/accounts_db.rs
🌐 Web query:
Solana svm program_loader load_program_accounts loader_v4 AccountDataTooSmall
💡 Result:
In the context of the Solana SVM (Solana Virtual Machine), encountering an AccountDataTooSmall error within the loader_v4 program during the load_program_accounts process indicates that the account data provided to the loader is insufficient to hold the required LoaderV4State structure [1][2]. The loader_v4 program expects program accounts to contain specific metadata at the beginning of their data buffer [3]. The get_state function within the loader_v4 codebase attempts to read this fixed-size header [1][2]: 1. The function retrieves the slice of account data corresponding to the size of the LoaderV4State header (LoaderV4State::program_data_offset) [1][2]. 2. If the account's data buffer is smaller than this required offset, the slice operation fails, and the program returns the InstructionError::AccountDataTooSmall error [1][2]. This error typically occurs when: - An account is initialized or truncated to a size that does not accommodate the mandatory LoaderV4State header [1][3]. - A program account is improperly configured or corrupted, lacking the expected structure required by the V4 loader to verify its deployment status and slot information [4][5]. To resolve this, ensure that the program account is properly initialized and allocated with enough space to hold both the LoaderV4State header and the actual program code (if applicable) using the loader's defined instructions, such as Truncate [3]. In modern Solana development, this loader is specifically intended for managing programs via the program-v4 subcommand [6].
Citations:
- 1: https://github.com/anza-xyz/agave/blob/31aca18feb8385be3a1ca5d5e1b57f55c9cafe8b/programs/loader-v4/src/lib.rs
- 2: firedancer-io/agave@c319064
- 3: Feature - Loader built-in program v4 solana-labs/solana#30464
- 4: https://github.com/solana-labs/solana/blob/master/svm/src/transaction_processor.rs
- 5: https://github.com/anza-xyz/agave/blob/f205d0e729e05f48ebba4e4bb47859714ff390f2/svm/src/transaction_processor.rs
- 6: https://solana.stackexchange.com/questions/21382/new-anchor-solana-version-unable-to-deploy-contracts-solved-sharing-s
Seed loader-v4 accounts with the required state header.
keeper/src/builder.rs stores each self.programs ELF byte-for-byte in an account owned by loader_v4::ID, but loader-v4 program data must start with LoaderV4State, and minimum_balance(elf.len()) under-allocates by at least that header size. Use a loader whose data layout matches what you are seeding, or prepend/space the required loader-v4 state before the ELF.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@keeper/src/builder.rs` around lines 161 - 170, The program-account
construction loop in the builder must account for the loader-v4 state layout:
prepend the required LoaderV4State header before each ELF (or switch to a loader
matching the raw ELF layout), and compute rent from the complete account data
size. Preserve the loader_v4 owner and executable configuration while ensuring
the seeded data begins with valid loader-v4 state.
| let range = slot.saturating_sub(SLOTHASH_ENTRIES as u64)..slot + 1; | ||
| let (payload, handle) = RequestPayload::new(range); | ||
| ledger.reader.send(ReadRequest::BlockRange(payload))?; | ||
|
|
||
| let mut hashes = SlotHashes::new(&[Default::default(); SLOTHASH_ENTRIES]); | ||
| for block in handle.recv_timeout().await?? { | ||
| hashes.add(block.slot, block.hash); | ||
| last_block.replace(block); | ||
| } | ||
| let acc = self.account(&hashes, &sysvar::ID)?; | ||
| accounts.push((SlotHashes::id(), acc.build())); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect BlockRange handling and ordering guarantees in the ledger crate.
fd -t f -e rs . ledger | xargs rg -n -C 6 'BlockRange'Repository: magicblock-labs/magicblock-engine
Length of output: 8014
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files touching SlotHash and builder:"
fd -t f -e rs 'builder|slot.*hash|sysvar|reader' . | sed -n '1,120p'
echo
echo "Relevant builder.rs excerpt:"
sed -n '150,225p' keeper/src/builder.rs
echo
echo "Search SlotHashes definitions/usages:"
rg -n -C 4 'struct SlotHashes|impl SlotHashes|SlotHashes::new|get_sysvar' keeper ledger .Repository: magicblock-labs/magicblock-engine
Length of output: 17260
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
for p in list(Path('.').rglob('*.rs'))[:200]:
s = p.read_text(errors='ignore')
if 'struct SlotHashes' in s or 'impl SlotHashes' in s or 'SlotHashes::new' in s:
print(f'--- {p} ---')
for i,line in enumerate(s.splitlines(),1):
if 'SlotHashes' in line or 'struct SlotHashes' in line or 'impl SlotHashes' in line:
start=max(1,i-8); end=min(s.count('\n')+1,i+28)
print('\n'.join(f'{j+1}: {l}' for j,l in enumerate(s.splitlines()[start-1:end-1])))
PYRepository: magicblock-labs/magicblock-engine
Length of output: 26495
🌐 Web query:
Solana SlotHashes new add slot_hashes first get source code
💡 Result:
In the Solana codebase, the SlotHashes structure is used to maintain a history of recent slot hashes [1][2]. The add function is responsible for inserting a new slot and its corresponding hash into the structure [2][3][4]. The implementation of the add method typically follows this logic: 1. It performs a binary search within the internal list to check if the slot already exists [2][3]. 2. If the slot is found, it updates the existing entry with the new hash [2][3][5]. 3. If the slot is not found, it inserts the new (slot, hash) tuple at the correct index to maintain the order [2][3][5]. 4. It then truncates the list to a maximum allowed size (MAX_ENTRIES, which is 512) to ensure the sysvar remains within size limits [2][3][5]. The following is a representation of the add method found in the source code: pub fn add(&mut self, slot: u64, hash: Hash) { match self.binary_search_by(|(probe, _)| slot.cmp(probe)) { Ok(index) => (self.0)[index] = (slot, hash), Err(index) => (self.0).insert(index, (slot, hash)), } (self.0).truncate(MAX_ENTRIES); } This structure is a sysvar that stores the most recent hashes of a slot's parent banks [1][6]. Because this sysvar account can be large, standard on-chain processing methods may return an unsupported error [7][8][9]; however, it remains accessible for off-chain reading via RPC or specific optimized structures like PodSlotHashes [10][7].
Citations:
- 1: https://docs.rs/solana-sysvar/latest/src/solana_sysvar/slot_hashes.rs.html
- 2: https://docs.rs/crate/solana-program/1.9.7/source/src/slot_hashes.rs
- 3: https://docs.rs/solana-slot-hashes/latest/src/solana_slot_hashes/lib.rs.html
- 4: https://docs.rs/solana-program-ed25519-dalek-bump/latest/src/solana_program_ed25519_dalek_bump/slot_hashes.rs.html
- 5: https://docs.rs/crate/solana-program/1.7.6/source/src/slot_hashes.rs
- 6: https://github.com/solana-labs/solana/blob/94ab0eb49f1bce18d0a157dfe7a2bb1fb39dbe2c/docs/src/developing/runtime-facilities/sysvars.md
- 7: https://docs.rs/solana-program/latest/solana_program/sysvar/slot_hashes/index.html
- 8: https://docs.rs/solana-sysvar/latest/solana_sysvar/slot_hashes/index.html
- 9: https://docs.rs/solana-sdk/latest/solana_sdk/sysvar/slot_hashes/index.html
- 10: https://docs.rs/solana-program/latest/solana_program/sysvar/slot_hashes/struct.PodSlotHashes.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "ReadRange readers:"
sed -n '1,220p' ledger/src/reader.rs
echo
echo "Range queries / read tests:"
rg -n -C 8 'block_of|read\\(|Range<Slot>|slots\\(' ledger/src/tests/integration.rs keeper/srcRepository: magicblock-labs/magicblock-engine
Length of output: 8769
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Remaining reader.rs blocks implementation:"
sed -n '220,420p' ledger/src/reader.rs
echo
echo "Block query tests:"
rg -n -C 8 'block_of|read\(|Range<Slot>|slots\(' ledger/src/tests/integration.rs keeper/srcRepository: magicblock-labs/magicblock-engine
Length of output: 28989
Build SlotHashes from retained blocks only.
SlotHashes::new(&[Default::default(); SLOTHASH_ENTRIES]) seeds the sysvar with duplicate slot-0 entries before adding returned blocks. Build the sysvar from the retained blocks only so get_sysvar does not expose hash entries for slots that were never produced.
🐛 Proposed fix
- let mut hashes = SlotHashes::new(&[Default::default(); SLOTHASH_ENTRIES]);
+ let mut hashes = SlotHashes::default();
for block in handle.recv_timeout().await?? {
hashes.add(block.slot, block.hash);
last_block.replace(block);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let range = slot.saturating_sub(SLOTHASH_ENTRIES as u64)..slot + 1; | |
| let (payload, handle) = RequestPayload::new(range); | |
| ledger.reader.send(ReadRequest::BlockRange(payload))?; | |
| let mut hashes = SlotHashes::new(&[Default::default(); SLOTHASH_ENTRIES]); | |
| for block in handle.recv_timeout().await?? { | |
| hashes.add(block.slot, block.hash); | |
| last_block.replace(block); | |
| } | |
| let acc = self.account(&hashes, &sysvar::ID)?; | |
| accounts.push((SlotHashes::id(), acc.build())); | |
| } | |
| let range = slot.saturating_sub(SLOTHASH_ENTRIES as u64)..slot + 1; | |
| let (payload, handle) = RequestPayload::new(range); | |
| ledger.reader.send(ReadRequest::BlockRange(payload))?; | |
| let mut hashes = SlotHashes::default(); | |
| for block in handle.recv_timeout().await?? { | |
| hashes.add(block.slot, block.hash); | |
| last_block.replace(block); | |
| } | |
| let acc = self.account(&hashes, &sysvar::ID)?; | |
| accounts.push((SlotHashes::id(), acc.build())); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@keeper/src/builder.rs` around lines 196 - 207, Update the SlotHashes
construction in the surrounding builder flow to initialize it from the retained
blocks returned by handle.recv_timeout().await??, rather than seeding it with
[Default::default(); SLOTHASH_ENTRIES]. Add each retained block’s slot and hash
to the resulting SlotHashes and preserve the existing account creation and
last_block updates.
| fn accountsdb(&self, ledger: &LedgerHandle) -> Result<AccountsDB> { | ||
| let mut backup = None; | ||
| loop { | ||
| let accountsdb = AccountsDB::new(&self.accountsdb.directory)?; | ||
| // Seal N opens ledger head N+1, so accountsdb is current at head-1. | ||
| let expected = ledger.head().saturating_sub(1); | ||
| let restored = backup.is_some(); | ||
| let lagging = accountsdb.superblock() < expected; | ||
| match accountsdb.validate() { | ||
| Ok(()) if restored || !lagging => { | ||
| info!(lagging, "accountsdb validation succeeded"); | ||
| backup.map(fs::remove_dir_all).transpose()?; | ||
| return Ok(accountsdb); | ||
| } | ||
| validation @ (Err(AccountsDBError::Corruption) | Ok(())) => { | ||
| if restored { | ||
| error!(?validation, "restored accountsdb is corrupt"); | ||
| accountsdb.backup(BackupOp::Restore)?; | ||
| return Err(SnapshotError::Missing.into()); | ||
| } | ||
| warn!(?validation, lagging, "state inconsistency detected"); | ||
| backup.replace(accountsdb.backup(BackupOp::Save)?); | ||
| if let Err(error) = self.unarchive(ledger) { | ||
| accountsdb.backup(BackupOp::Restore)?; | ||
| return Err(error); | ||
| } | ||
| } | ||
| Err(other) => return Err(other.into()), | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether AccountsDB::new opens persistent handles/mmaps and how backup/restore work.
fd -t f -e rs . accountsdb | xargs rg -n -C 8 'fn new\(|fn backup\(|mmap|Mmap|File::open|OpenOptions'Repository: magicblock-labs/magicblock-engine
Length of output: 28689
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
fd -t f 'builder.rs|snapshot.rs|store/mod.rs|mmap.rs|index.rs' .
echo "== keeper/src/builder.rs outline =="
ast-grep outline keeper/src/builder.rs --view expanded || true
echo "== keeper/src/builder.rs relevant =="
sed -n '220,310p' keeper/src/builder.rs
echo "== accountsdb/src/snapshot.rs full =="
cat -n accountsdb/src/snapshot.rs
echo "== accountdb drop/drop-related =="
rg -n -C 4 'drop\(|Drop|impl Drop|accountsdb\(|persisted\.flush|flush\(' accountsdb/src builder 2>/dev/null || true
echo "== rustfmt compatibility =="
command -v rustfmtRepository: magicblock-labs/magicblock-engine
Length of output: 22229
Drop the AccountsDB instance before unpacking the restored snapshot.
AccountsDB::new opens the persisted LMDB environment and mapped storage for the active database tree. self.unarchive(ledger) unpacks into that same directory before accountsdb goes out of scope, so the restore can race against the still-open store and leave the recovered tree corrupted. Drop after saving the backup and before tar.unpack, or reopen the database after restore.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@keeper/src/builder.rs` around lines 256 - 286, In the accountsdb method,
explicitly drop the AccountsDB instance after saving the backup and before
calling self.unarchive(ledger), ensuring the LMDB environment and mapped storage
are closed before unpacking the restored snapshot. Preserve the existing restore
and error-handling flow.
| /// Registers keeper metrics once and seeds gauges from current caches. | ||
| pub(crate) fn init() { | ||
| METRICS.get_or_init(Default::default); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
The init doc comment does not match the code.
The comment states that init "seeds gauges from current caches". The body only calls METRICS.get_or_init(Default::default), which registers collectors with zero values. No cache is read.
Update the comment to describe registration only.
📝 Proposed fix
-/// Registers keeper metrics once and seeds gauges from current caches.
+/// Registers keeper metrics once in the default Prometheus registry.
pub(crate) fn init() {
METRICS.get_or_init(Default::default);
}As per path instructions: "Check docs and rustdoc for factual consistency with the code. Flag only real mismatches, broken examples, stale comments, or important omissions."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Registers keeper metrics once and seeds gauges from current caches. | |
| pub(crate) fn init() { | |
| METRICS.get_or_init(Default::default); | |
| } | |
| /// Registers keeper metrics once in the default Prometheus registry. | |
| pub(crate) fn init() { | |
| METRICS.get_or_init(Default::default); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@keeper/src/metrics.rs` around lines 69 - 72, Update the rustdoc for the
metrics `init` function to describe only one-time metrics registration via
`METRICS.get_or_init(Default::default)`. Remove the claim that gauges are seeded
from current caches, leaving the implementation unchanged.
Source: Path instructions
| /// Records an account cache insertion that increases current occupancy. | ||
| pub(crate) fn account_cache_insert() { | ||
| metric::with_metrics(&METRICS, |m| m.account_cache_entries.inc()); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check all account cache mutation sites and their metric calls.
rg -n -C 6 'account_cache_insert|account_cache_eviction|AccountCache' keeper/srcRepository: magicblock-labs/magicblock-engine
Length of output: 15622
🏁 Script executed:
#!/bin/bash
#!/bin/bash
set -euo pipefail
echo "== keeper/src/metrics.rs relevant section =="
sed -n '60,115p' keeper/src/metrics.rs | cat -n
sed -n '115,145p' keeper/src/metrics.rs | cat -n
echo
echo "== keeper/src/cache.rs relevant sections =="
sed -n '1,180p' keeper/src/cache.rs | cat -n
echo
echo "== hash-cache dependency version/source hints =="
rg -n 'hash-cache' Cargo.toml Cargo.lock keeper/Cargo.toml
if [ -f Cargo.lock ]; then
nl -ba Cargo.lock | sed -n '/hash-cache/,+80p'
fi
echo
echo "== all occurrences of account_cache_entries =="
rg -n 'account_cache_entries|ACCOUNT_CACHE_ENTRIES' keeper
echo
echo "== structural check: AccountCache methods using reservations removal and evictions metric =="
python3 - <<'PY'
from pathlib import Path
p=Path('keeper/src/cache.rs')
s=p.read_text()
checks = {
'reserve_sync_insert_calls_account_cache_insert': 'metrics::account_cache_insert()' in s,
'eviction_put_sync_calls_account_cache_eviction': 'metrics::account_cache_eviction();' in s,
'reservations_remove_sync_no_metric': 'reservations.remove_sync' in s and 'metrics::account_cache' not in s[s.find('reservations.remove_sync')-200:s.find('reservations.remove_sync')+300],
'promote_has_metric_call': 'pub(crate) fn promote' in s and 'metrics::account_cache' not in s[s.find('pub(crate) fn promote'):s.find('pub(crate) fn reserve')],
}
for name, ok in checks.items():
print(f"{name}: {ok}")
print("-- locations --")
for needle in ['metrics::account_cache_insert()', 'metrics::account_cache_eviction();', 'reservations.remove_sync', 'pub(crate) fn promote']:
idx=s.find(needle)
print(f"{needle}: {idx+1 if idx>=0 else 'not found'}")
PYRepository: magicblock-labs/magicblock-engine
Length of output: 11456
Track account cache evictions by decreasing the entry gauge.
account_cache_insert() only increases account_cache_entries; account_cache_eviction() only increments the eviction counter. Since ACCOUNT_CACHE_ENTRIES reports current load entries, decrement the gauge when metrics::account_cache_eviction() runs, or keep setting it from lru.len() like the signature/block hash entry gauges.
🐛 Proposed fix
/// Records one account cache eviction.
pub(crate) fn account_cache_eviction() {
- metric::with_metrics(&METRICS, |m| m.account_cache_evictions.inc());
+ metric::with_metrics(&METRICS, |m| {
+ m.account_cache_evictions.inc();
+ m.account_cache_entries.dec();
+ });
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@keeper/src/metrics.rs` around lines 79 - 82, Update account_cache_eviction()
to decrement m.account_cache_entries when an account cache entry is evicted,
while preserving its existing eviction-counter increment. Keep
account_cache_insert() increasing the same gauge so ACCOUNT_CACHE_ENTRIES
reflects current occupancy.
| /// Program account updates keyed by owner pubkey; | ||
| pub(crate) programs: Subscribers<Pubkey, AccountEntry>, | ||
| /// Signature status updates keyed by transaction signature. | ||
| pub(crate) signatures: Subscribers<Signature, TransactionStatus>, | ||
| /// Log broadcasts keyed by mentioned program or account pubkey. | ||
| pub(crate) logs: Subscribers<Pubkey, Arc<TransactionLogs>>, | ||
| /// Broadcast channel for newly committed slots. | ||
| pub(crate) blocks: Sender<Block>, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix two doc defects on the subscription fields.
Line 43 ends with a semicolon instead of a period. Line 49 describes the channel as carrying slots, but the item type is Block.
📝 Proposed doc fix
- /// Program account updates keyed by owner pubkey;
+ /// Program account updates keyed by owner pubkey.
pub(crate) programs: Subscribers<Pubkey, AccountEntry>,
@@
- /// Broadcast channel for newly committed slots.
+ /// Broadcast channel for newly committed blocks.
pub(crate) blocks: Sender<Block>,As per path instructions: "Check docs and rustdoc for factual consistency with the code" and flag "Typos in identifiers, comments, or user-facing strings."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Program account updates keyed by owner pubkey; | |
| pub(crate) programs: Subscribers<Pubkey, AccountEntry>, | |
| /// Signature status updates keyed by transaction signature. | |
| pub(crate) signatures: Subscribers<Signature, TransactionStatus>, | |
| /// Log broadcasts keyed by mentioned program or account pubkey. | |
| pub(crate) logs: Subscribers<Pubkey, Arc<TransactionLogs>>, | |
| /// Broadcast channel for newly committed slots. | |
| pub(crate) blocks: Sender<Block>, | |
| /// Program account updates keyed by owner pubkey. | |
| pub(crate) programs: Subscribers<Pubkey, AccountEntry>, | |
| /// Signature status updates keyed by transaction signature. | |
| pub(crate) signatures: Subscribers<Signature, TransactionStatus>, | |
| /// Log broadcasts keyed by mentioned program or account pubkey. | |
| pub(crate) logs: Subscribers<Pubkey, Arc<TransactionLogs>>, | |
| /// Broadcast channel for newly committed blocks. | |
| pub(crate) blocks: Sender<Block>, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@keeper/src/subscriptions.rs` around lines 43 - 50, Correct the rustdoc
comments on the subscription fields: end the `programs` description with a
period instead of a semicolon, and update the `blocks` description to state that
it broadcasts newly committed `Block` values rather than slots. Leave the field
types and other comments unchanged.
Source: Path instructions
| pub(crate) fn send(&self, key: &K, value: &V, oneshot: bool) { | ||
| let sender = |_: &K, tx: &Sender<V>| tx.send(value.clone()).is_ok(); | ||
| let success = self.read_sync(key, sender).unwrap_or(true); | ||
| if !success || oneshot { | ||
| self.remove_sync(key); | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Determine the scc version and check for existing remove_if_sync usage in the workspace.
set -euo pipefail
fd -H -t f '^Cargo.toml$' --exec rg -n -C1 '\bscc\b' {} \;
rg -n -C2 'remove_if_sync|remove_if\b'Repository: magicblock-labs/magicblock-engine
Length of output: 589
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
fd -H -t f 'subscriptions.rs|Cargo.toml|Cargo.lock' .
echo
echo "subscriptions outline:"
ast-grep outline keeper/src/subscriptions.rs --view expanded || true
echo
echo "subscriptions relevant lines:"
sed -n '1,140p' keeper/src/subscriptions.rs
echo
echo "Cargo workspace scc entries:"
fd -H -t f '^Cargo.toml$' . -x sh -c 'echo "--- $1"; rg -n -C2 "^\s*\[workspace\.dependencies\]|scc\s*=" "$1" || true' sh {}Repository: magicblock-labs/magicblock-engine
Length of output: 9226
🌐 Web query:
scc 3.8.4 HashMap remove_if_sync signature locking guarantee
💡 Result:
In the scc crate (version 3.8.4), the method signature for remove_if_sync on HashMap is [1][2]: pub fn remove_if_sync<Q, F: FnOnce(&mut V) -> bool>( &self, key: &Q, condition: F,) -> Option<(K, V)> where Q: Equivalent + Hash +?Sized, Regarding locking guarantees, scc::HashMap is designed as a lock-free structure at the container level, utilizing a lock-free stack of entry bucket arrays [3][4][5]. Individual buckets within this structure are protected by read-write locks [3][4][5]. When remove_if_sync is called: 1. The method identifies the specific bucket containing the target key [2]. 2. It acquires a synchronous write lock on that specific bucket [2]. 3. The condition function is executed while the bucket is locked, ensuring that the check and the potential removal are performed atomically with respect to other operations on that same bucket [2]. 4. There are no container-level locks, so this locking is localized to the bucket [5][6], minimizing contention. Because this method holds the bucket-level lock during the check and removal process, it provides an atomic "check-and-remove" guarantee for that bucket [2]. You should avoid holding any other scc entries or locks from the same hash map in the calling thread to prevent potential deadlocks [6].
Citations:
- 1: https://docs.rs/scc/latest/scc/hash_map/struct.HashMap.html
- 2: https://docs.rs/scc/latest/src/scc/hash_map.rs.html
- 3: https://docs.rs/crate/scc/latest/source/README.md
- 4: https://lib.rs/crates/scc
- 5: https://docs.rs/crate/scc/latest
- 6: https://dev.acter.global/api/main/rust/scc/hash_map/struct.HashMap.html
🌐 Web query:
runtimes.rs scc 3.8.4 HashMap send_or_insert remove_if_sync
💡 Result:
In the scc crate (version 3.8.4), the HashMap provides several methods for synchronous and asynchronous operations [1][2]. Regarding the specific methods mentioned: remove_if_sync The HashMap struct includes a public method named remove_if_sync, which removes a key-value pair if the key exists and a provided condition is met [1][3]. Its signature is: pub fn remove_if_sync<Q, F: FnOnce(&mut V) -> bool>(&self, key: &Q, condition: F) -> Option<(K, V)> where Q: Equivalent + Hash +?Sized send_or_insert There is no method named send_or_insert in the scc::HashMap API [1][4]. It is likely you are referring to the entry API, specifically the or_insert or or_insert_with methods, which are used to insert a value if the key does not already exist [5]. These are typically accessed via the entry_sync or entry_async methods on the HashMap [1][6]. runtimes.rs The file name runtimes.rs is a common convention in Rust projects for defining runtime-related configurations or abstractions [7][8][9]. It is not a specific component of the scc crate [4][2]. In summary, scc::HashMap supports both blocking (sync) and non-blocking (async) operations for insertion, removal, and updates [1][4], but the specific method name send_or_insert does not exist in the crate's public API [1][5].
Citations:
- 1: https://docs.rs/scc/latest/scc/hash_map/struct.HashMap.html
- 2: https://crates.io/crates/scc
- 3: https://docs.rs/scc/latest/src/scc/hash_map.rs.html
- 4: https://docs.rs/crate/scc/latest/source/README.md
- 5: https://docs.rs/scc/latest/scc/hash_map/enum.Entry.html?search=
- 6: https://docs.rs/crate/scc/3.4.16
- 7: https://docs.rs/tetcore-subxt/latest/src/tetcore_subxt/runtimes.rs.html
- 8: https://hex.pm/packages/tyrex/0.3.0/files/native/tyrex/src/runtimes.rs
- 9: https://hex.pm/packages/deno_rider/0.2.1/files/native/deno_rider/src/runtimes.rs
🌐 Web query:
docs.rs scc 3.8.4 HashMap read_sync FnMut signature
💡 Result:
In scc version 3.8.4 (and other recent versions), the read_sync method for scc::HashMap has the following signature: [1] pub fn read_sync<Q, R, F: FnOnce(&K, &V) -> R>( &self, key: &Q, reader: F) -> Option where Q: Equivalent + Hash +?Sized, K: Borrow, { // Implementation details omitted } The method signature requires the following: [1] - &self: A reference to the HashMap instance. - key: &Q: A reference to the key used for lookup, where Q must implement the Equivalent trait for the map's key type K and the Hash trait. - reader: F: A FnOnce closure that takes references to the found key (&K) and value (&V) and returns a result of type R. - Returns: An Option, which is Some(R) if the key is found and the closure executes, or None if the key does not exist. This method acquires a shared lock on the bucket containing the key to ensure safe, synchronous access to the value. [2][3][4]
Citations:
- 1: https://docs.rs/scc/latest/scc/hash_map/struct.HashMap.html
- 2: https://docs.rs/crate/scc/latest
- 3: https://docs.rs/crate/scc/latest/source/README.md
- 4: https://docs.rs/scc
🌐 Web query:
docs.rs scc 3.8.4 HashMap read_sync release lock before returning
💡 Result:
In the scc crate (including version 3.8.4), the read_sync method for HashMap does not require the user to manually manage the release of the bucket lock [1][2]. The read_sync method takes a closure (reader) as an argument [1][3]. It automatically acquires a shared lock on the bucket containing the entry before executing the provided closure and ensures the lock is released after the closure returns [1][4]. By providing the closure, you encapsulate the read operation, allowing the library to handle the lock lifecycle safely and automatically [1][2]. If you require a completely lock-free read operation that does not use bucket locks, you should use the peek or peek_with methods instead, as these are designed to be entirely lock-free [5][6][7].
Citations:
- 1: https://docs.rs/scc/latest/scc/hash_map/struct.HashMap.html
- 2: https://docs.rs/scc
- 3: https://docs.rs/scc/latest/scc/hash_map/struct.Reserve.html
- 4: https://docs.rs/crate/scc/latest/source/README.md
- 5: https://docs.rs/crate/scc/latest
- 6: https://docs.rs/crate/scc/3.6.10
- 7: https://docs.rs/crate/scc/3.6.12
Make send remove the channel atomically.
send drops the bucket lock after read_sync returns, so subscribe can join the existing Sender before remove_sync deletes it. The new subscriber then never sees values from the next send, which creates a different channel. Use remove_if_sync with the predicate under the bucket lock, or keep send_sync/read_sync for the send path if remove_if_sync does not support the value type and condition shape.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@keeper/src/subscriptions.rs` around lines 82 - 88, Update Subscriptions::send
so sending and conditional removal occur under the same bucket lock, preventing
subscribe from inserting between the send and removal. Prefer remove_if_sync
with a predicate that sends the cloned value and removes the channel when
sending fails or oneshot is true; otherwise use the existing send_sync/read_sync
path while preserving atomic removal semantics.
| //! Builds a real [`Keeper`] over throwaway directories with the canonical test | ||
| //! parameters (retention disabled, 400 ms blocktime, superblock 16), and exposes | ||
| //! the loadable v42 calculator program guaranteed by `build.rs`. The low-level, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the testkit configuration values.
The rustdoc states 400 ms and superblock 16. keeper_builder uses 100 ms, and SUPERBLOCK is 4. Update the rustdoc to match the configured values.
As per path instructions, check docs and rustdoc for factual consistency with code.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@keeper/src/testkit.rs` around lines 3 - 5, Update the module-level rustdoc
describing the Keeper test configuration to state the values actually used by
keeper_builder: 100 ms blocktime and superblock 4, preserving the existing
description of the other parameters.
Source: Path instructions
1 participant
What changed
Added the
keepercrate as the orchestration layer overaccountsdb,ledger, caches, subscriptions, sysvar seeding, and account snapshot archival.Why
Runtime code needs one consistency boundary that keeps account state, ledger records, cache state, and subscription updates aligned around execution and slot progress.
Closes #12.
Impact
Keeper,KeeperBuilder, storage directory parameters, and keeper-level errors.Reviewer notes
finalizeseals the next superblock and archives the matching account snapshot. Restore keeps a backup before unpacking archived state so failed recovery can roll back.Follow-up
Transaction processor and runtime integration can build on the keeper API.