refactor: remove crates moved to magicblock engine - #1394
Closed
bmuddha wants to merge 1 commit into
Closed
Conversation
Contributor
|
Important Review skippedToo many files! This PR contains 280 files, which is 180 over the limit of 100. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (8)
📒 Files selected for processing (633)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe change migrates account, cloning, scheduling, callback, undelegation, and task execution paths to Suggested reviewers: ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
bmuddha
force-pushed
the
crate-removal
branch
2 times, most recently
from
July 7, 2026 08:05
e368004 to
94086c1
Compare
![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
🤖 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 `@Cargo.toml`:
- Around line 164-167: The workspace is missing the shared rocksdb dependency
entry, while magicblock-ledger still references rocksdb through the workspace.
Update the root Cargo.toml to either restore rocksdb under
[workspace.dependencies] or change the affected crate’s dependency declaration
to an explicit versioned dependency, and verify the crates using rocksdb still
resolve through the intended symbol.
🪄 Autofix (Beta)
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: ASSERTIVE
Plan: Pro
Run ID: a5aa6499-d803-4b3b-80c6-f6f1ac9d80f6
📒 Files selected for processing (76)
Cargo.tomlmagicblock-account-cloner/Cargo.tomlmagicblock-account-cloner/README.mdmagicblock-account-cloner/src/lib.rsmagicblock-account-cloner/src/util.rsmagicblock-accounts-db/Cargo.tomlmagicblock-accounts-db/README.mdmagicblock-accounts-db/src/error.rsmagicblock-accounts-db/src/index.rsmagicblock-accounts-db/src/index/iterator.rsmagicblock-accounts-db/src/index/table.rsmagicblock-accounts-db/src/index/tests.rsmagicblock-accounts-db/src/index/utils.rsmagicblock-accounts-db/src/lib.rsmagicblock-accounts-db/src/reset.rsmagicblock-accounts-db/src/snapshot.rsmagicblock-accounts-db/src/storage.rsmagicblock-accounts-db/src/tests.rsmagicblock-accounts-db/src/traits.rsmagicblock-accounts/Cargo.tomlmagicblock-accounts/README.mdmagicblock-accounts/src/config.rsmagicblock-accounts/src/errors.rsmagicblock-accounts/src/lib.rsmagicblock-accounts/src/traits.rsmagicblock-processor/Cargo.tomlmagicblock-processor/README.mdmagicblock-processor/src/builtins.rsmagicblock-processor/src/executor/callback.rsmagicblock-processor/src/executor/mod.rsmagicblock-processor/src/executor/processing.rsmagicblock-processor/src/lib.rsmagicblock-processor/src/loader.rsmagicblock-processor/src/scheduler/coordinator.rsmagicblock-processor/src/scheduler/locks.rsmagicblock-processor/src/scheduler/mod.rsmagicblock-processor/src/scheduler/state.rsmagicblock-processor/src/scheduler/tests.rsmagicblock-processor/tests/ephemeral_accounts.rsmagicblock-processor/tests/execution.rsmagicblock-processor/tests/fees.rsmagicblock-processor/tests/post_delegation_actions.rsmagicblock-processor/tests/replay.rsmagicblock-processor/tests/replica_ordering.rsmagicblock-processor/tests/scheduling.rsmagicblock-processor/tests/security.rsmagicblock-processor/tests/simulation.rsstorage-proto/Cargo.tomlstorage-proto/README.mdstorage-proto/build.rsstorage-proto/proto/confirmed_block.protostorage-proto/proto/entries.protostorage-proto/proto/transaction_by_addr.protostorage-proto/src/convert.rsstorage-proto/src/lib.rstest-kit/Cargo.tomltest-kit/src/lib.rstest-kit/src/macros.rstools/genx/Cargo.tomltools/genx/README.mdtools/genx/src/main.rstools/genx/src/test_validator.rstools/keypair-base58/Cargo.tomltools/keypair-base58/README.mdtools/keypair-base58/src/main.rstools/ledger-stats/Cargo.tomltools/ledger-stats/README.mdtools/ledger-stats/src/account.rstools/ledger-stats/src/accounts.rstools/ledger-stats/src/blockhash.rstools/ledger-stats/src/counts.rstools/ledger-stats/src/lib.rstools/ledger-stats/src/main.rstools/ledger-stats/src/transaction_details.rstools/ledger-stats/src/transaction_logs.rstools/ledger-stats/src/utils.rs
💤 Files with no reviewable changes (75)
- magicblock-accounts-db/README.md
- tools/genx/README.md
- magicblock-processor/Cargo.toml
- storage-proto/src/lib.rs
- tools/genx/Cargo.toml
- magicblock-accounts/Cargo.toml
- magicblock-account-cloner/Cargo.toml
- test-kit/Cargo.toml
- tools/ledger-stats/src/main.rs
- magicblock-accounts-db/src/tests.rs
- magicblock-processor/tests/fees.rs
- magicblock-processor/tests/replay.rs
- magicblock-processor/src/builtins.rs
- magicblock-processor/src/scheduler/tests.rs
- magicblock-accounts/src/lib.rs
- tools/ledger-stats/src/counts.rs
- magicblock-accounts/src/traits.rs
- tools/ledger-stats/README.md
- tools/ledger-stats/src/lib.rs
- magicblock-processor/src/executor/mod.rs
- test-kit/src/macros.rs
- magicblock-processor/src/scheduler/state.rs
- tools/ledger-stats/src/transaction_details.rs
- tools/ledger-stats/src/transaction_logs.rs
- magicblock-accounts/README.md
- tools/ledger-stats/src/account.rs
- magicblock-accounts-db/src/reset.rs
- storage-proto/README.md
- tools/genx/src/main.rs
- storage-proto/Cargo.toml
- tools/keypair-base58/Cargo.toml
- tools/ledger-stats/src/blockhash.rs
- storage-proto/proto/entries.proto
- test-kit/src/lib.rs
- magicblock-account-cloner/src/util.rs
- storage-proto/proto/transaction_by_addr.proto
- magicblock-accounts/src/errors.rs
- tools/ledger-stats/src/utils.rs
- magicblock-processor/src/scheduler/coordinator.rs
- magicblock-processor/src/executor/callback.rs
- magicblock-accounts-db/src/index/tests.rs
- magicblock-processor/tests/replica_ordering.rs
- magicblock-account-cloner/README.md
- tools/genx/src/test_validator.rs
- tools/ledger-stats/src/accounts.rs
- tools/keypair-base58/src/main.rs
- magicblock-accounts-db/src/traits.rs
- magicblock-processor/README.md
- magicblock-processor/tests/security.rs
- magicblock-account-cloner/src/lib.rs
- magicblock-accounts-db/Cargo.toml
- magicblock-accounts-db/src/snapshot.rs
- magicblock-accounts-db/src/index/utils.rs
- magicblock-processor/tests/scheduling.rs
- magicblock-processor/src/loader.rs
- magicblock-processor/src/lib.rs
- magicblock-processor/tests/simulation.rs
- magicblock-accounts-db/src/index.rs
- magicblock-processor/tests/post_delegation_actions.rs
- tools/ledger-stats/Cargo.toml
- storage-proto/build.rs
- magicblock-processor/src/scheduler/mod.rs
- magicblock-processor/tests/ephemeral_accounts.rs
- magicblock-accounts-db/src/storage.rs
- magicblock-accounts-db/src/lib.rs
- magicblock-accounts-db/src/error.rs
- magicblock-processor/src/scheduler/locks.rs
- magicblock-accounts-db/src/index/table.rs
- storage-proto/proto/confirmed_block.proto
- tools/keypair-base58/README.md
- magicblock-processor/src/executor/processing.rs
- storage-proto/src/convert.rs
- magicblock-accounts/src/config.rs
- magicblock-processor/tests/execution.rs
- magicblock-accounts-db/src/index/iterator.rs
bmuddha
force-pushed
the
crate-removal
branch
2 times, most recently
from
July 17, 2026 20:01
d34b5bf to
62e9bc0
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
magicblock-chainlink/src/testing/cloner_stub.rs (1)
193-206: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winKeep the stub’s undelegation behavior aligned with
ChainlinkCloner.The stub records the request and succeeds, while production returns
UndelegationSchedulingUnavailable. Tests using this stub cannot detect the current production failure. Model the same outcome until scheduling is implemented, then model the restored behavior in both.🤖 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 `@magicblock-chainlink/src/testing/cloner_stub.rs` around lines 193 - 206, Update the cloner stub method containing the undelegation handling to return UndelegationSchedulingUnavailable whenever request.needs_undelegation is true, matching ChainlinkCloner instead of recording the request and succeeding. Preserve the existing successful path for requests that do not require undelegation, and keep both implementations aligned when scheduling is later implemented.programs/magicblock/src/magic_scheduled_base_intent.rs (1)
504-521: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove redundant account mode validation.
The checks for "confined" and "ephemeral" accounts have both been migrated to
AccountMode::Ephemeral, resulting in two identical validation conditions executed back-to-back. The first check will always catch it, rendering the second block dead code and unnecessarily duplicating theto_account_shared_data()?conversion.♻️ Proposed refactor
- if account.to_account_shared_data()?.is(AccountMode::Ephemeral) { - ic_msg!( - context.invoke_context, - "ScheduleCommit ERR: account {} is confined and cannot be committed", - pubkey - ); - return Err(InstructionError::InvalidAccountData); - } - // Prevent ephemeral accounts from being committed to base chain if account.to_account_shared_data()?.is(AccountMode::Ephemeral) { ic_msg!(🤖 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 `@programs/magicblock/src/magic_scheduled_base_intent.rs` around lines 504 - 521, Remove the second redundant AccountMode::Ephemeral validation block, including its duplicate log message and error return. Keep the first check in the surrounding schedule-commit flow as the sole validation using account.to_account_shared_data().programs/magicblock/src/schedule_transactions/process_schedule_commit.rs (1)
152-198: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winCache
AccountSharedDatato avoid redundant cloning and borrows.Calling
acc.to_account_shared_data()?invokes a clone operation onAccountSharedData. Currently, this is called up to three times per committee account inside this loop (along with an additionalacc.borrow()?.owner()call), which consumes unnecessary Compute Units and memory on a hot execution path.You can consolidate these by performing the clone once at the start of the loop block and reusing it.
⚡ Proposed refactor
- // Prevent accounts that exist only inside the ER from being committed - // to base chain. This covers what used to be two separate checks, for - // ephemeral and for confined accounts, which are now the same mode. - if acc.to_account_shared_data()?.is(AccountMode::Ephemeral) { + let account = acc.to_account_shared_data()?; + + // Prevent accounts that exist only inside the ER from being committed + // to base chain. This covers what used to be two separate checks, for + // ephemeral and for confined accounts, which are now the same mode. + if account.is(AccountMode::Ephemeral) { ic_msg!( invoke_context, "ScheduleCommit ERR: account {} is ephemeral and cannot be committed to base chain", acc_pubkey ); return Err(InstructionError::InvalidAccountData); } { - let is_delegated = acc.to_account_shared_data()?.is(AccountMode::Delegated); + let is_delegated = account.is(AccountMode::Delegated); if opts.request_undelegation { // Must be writable and delegated to avoid double-undelegation let is_writable = get_writable_with_idx(transaction_context, idx as u16)?; if !is_writable || !is_delegated { ic_msg!( invoke_context, "ScheduleCommit ERR: account {} is required to be writable and delegated in order to be undelegated", acc_pubkey ); return Err(InstructionError::ReadonlyDataModified); } } else if !is_delegated { ic_msg!( invoke_context, "ScheduleCommit ERR: account {} is required to be delegated to the current validator, in order to be committed", acc_pubkey ); return Err(InstructionError::IllegalOwner); } // Validate committed account was scheduled by valid authority - let acc_owner = *acc.borrow()?.owner(); + let acc_owner = *account.owner(); validate_commit_schedule_permissions( &invoke_context, &acc_owner, acc_pubkey, parent_program_id, &signers, )?; - let account = acc.to_account_shared_data()?;🤖 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 `@programs/magicblock/src/schedule_transactions/process_schedule_commit.rs` around lines 152 - 198, Cache the result of acc.to_account_shared_data()? once at the start of the committee-account processing block, then reuse it for the AccountMode checks and as the committed account value. Replace the separate acc.borrow()?.owner() access with the cached AccountSharedData’s owner, while preserving the existing validation and error behavior.magicblock-chainlink/src/chainlink/mod.rs (2)
310-319: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winConnect
InnerChainlinkto the FetchCloner’s notification channel.
FetchCloner::newcreates one sender, while this constructor creates another. Consequently,subscribe_undelegation_requests()listens on a channel that the suppliedfetch_clonernever publishes to. Require a shared sender or expose subscription throughFetchCloner.🤖 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 `@magicblock-chainlink/src/chainlink/mod.rs` around lines 310 - 319, Update Chainlink::try_new and the related initialization flow so the supplied FetchCloner and InnerChainlink share the same undelegation-request broadcast sender; do not create an independent channel in try_new. Reuse the sender created by FetchCloner::new, or expose FetchCloner’s subscription/sender through its API, and ensure subscribe_undelegation_requests listens to that shared channel.
672-680: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not classify
Transientaccounts as delegated on ER.Modes are exclusive, but the owner fallback marks every DLP-owned account as delegated. A
Transientaccount commonly remains DLP-owned, so this reports undelegating state asAccountStatusOnEr::Delegated. Classify strictly by mode, with explicit handling forTransientif callers need that distinction.Proposed fix
- Some(account) => { - if account.is(AccountMode::Delegated) - || account.owner().eq(&dlp_api::id()) - { + Some(account) => { + if account.is(AccountMode::Delegated) { AccountStatusOnEr::Delegated } else { AccountStatusOnEr::NotDelegated }🤖 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 `@magicblock-chainlink/src/chainlink/mod.rs` around lines 672 - 680, Update the account classification match around accounts_bank.get_account and AccountStatusOnEr so delegation is determined strictly from AccountMode::Delegated; remove the owner().eq(&dlp_api::id()) fallback, and add explicit AccountMode::Transient handling using the appropriate non-delegated status if callers distinguish it. Preserve Missing handling and avoid classifying DLP-owned transient accounts as Delegated.
🤖 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 `@magicblock-aperture/src/requests/http/mod.rs`:
- Around line 123-126: Wire the Engine’s active role into all coordination-gated
paths: in magicblock-aperture/src/requests/http/mod.rs lines 123-126, update
needs_onchain_interactions to allow primary-only RPCs only when the Engine is
primary; in magicblock-aperture/src/server/http/dispatch.rs lines 241-244, make
/health/primary return 503 unless the Engine is primary; and in
magicblock-api/src/magic_validator.rs lines 1111-1135, run task execution only
while primary and handle role transitions safely.
In `@magicblock-chainlink/src/accounts_bank.rs`:
- Around line 10-17: Update the AccountsBank::get_account contract and the
Engine implementation to return Result<Option<AccountSharedData>, _> instead of
collapsing lookup errors into None. Propagate the error from
self.accounts().get(pubkey) while retaining None only for genuinely missing
accounts, and update all trait callers to handle the Result.
In `@magicblock-chainlink/src/cloner/engine_cloner.rs`:
- Around line 57-64: The undelegation-required clone path is inconsistent:
production rejects it while the test stub succeeds. In
magicblock-chainlink/src/cloner/engine_cloner.rs lines 57-64, implement
Engine-backed undelegation scheduling before releasing the clone; in
magicblock-chainlink/src/testing/cloner_stub.rs lines 193-206, mirror the
production rejection behavior until scheduling is restored, then update tests to
cover the restored path.
- Around line 84-89: Update the LoaderV4Status::Retracted branch in the cloner
flow to delete program_id through the Engine before returning. Propagate any
deletion failure instead of unconditionally returning Ok(()), while preserving
the existing debug logging.
In `@magicblock-committor-service/src/service/intent_client.rs`:
- Around line 146-149: Update the flow around register_scheduled_commit_sent and
self.execute so the scheduled-commit notification is not left in the global
registry when execution fails: either register it only after successful
execution or explicitly remove/restore the entry on error. Preserve the existing
notification behavior on successful execution and ensure the caller’s
ScheduledBaseIntentMeta state remains consistent.
- Around line 115-123: Update the MagicContext lookup in the intent client to
avoid expect-based panicking and preserve account-store failures. Map lookup
errors into the service’s dedicated missing-account error, explicitly handle an
absent account with the same error, and propagate the result from the enclosing
method instead of terminating the validator.
In `@magicblock-task-scheduler/src/service.rs`:
- Around line 223-242: The Lagged branch in the service_messages receive loop
currently skips permanently lost task requests. Update this handling to recover
by replaying the missing messages from a persistent/replayable outbox or by
stopping and resynchronizing from an authoritative source, rather than merely
logging and continuing; preserve the existing Closed behavior and normal
TaskRequest deserialization flow.
---
Outside diff comments:
In `@magicblock-chainlink/src/chainlink/mod.rs`:
- Around line 310-319: Update Chainlink::try_new and the related initialization
flow so the supplied FetchCloner and InnerChainlink share the same
undelegation-request broadcast sender; do not create an independent channel in
try_new. Reuse the sender created by FetchCloner::new, or expose FetchCloner’s
subscription/sender through its API, and ensure subscribe_undelegation_requests
listens to that shared channel.
- Around line 672-680: Update the account classification match around
accounts_bank.get_account and AccountStatusOnEr so delegation is determined
strictly from AccountMode::Delegated; remove the owner().eq(&dlp_api::id())
fallback, and add explicit AccountMode::Transient handling using the appropriate
non-delegated status if callers distinguish it. Preserve Missing handling and
avoid classifying DLP-owned transient accounts as Delegated.
In `@magicblock-chainlink/src/testing/cloner_stub.rs`:
- Around line 193-206: Update the cloner stub method containing the undelegation
handling to return UndelegationSchedulingUnavailable whenever
request.needs_undelegation is true, matching ChainlinkCloner instead of
recording the request and succeeding. Preserve the existing successful path for
requests that do not require undelegation, and keep both implementations aligned
when scheduling is later implemented.
In `@programs/magicblock/src/magic_scheduled_base_intent.rs`:
- Around line 504-521: Remove the second redundant AccountMode::Ephemeral
validation block, including its duplicate log message and error return. Keep the
first check in the surrounding schedule-commit flow as the sole validation using
account.to_account_shared_data().
In `@programs/magicblock/src/schedule_transactions/process_schedule_commit.rs`:
- Around line 152-198: Cache the result of acc.to_account_shared_data()? once at
the start of the committee-account processing block, then reuse it for the
AccountMode checks and as the committed account value. Replace the separate
acc.borrow()?.owner() access with the cached AccountSharedData’s owner, while
preserving the existing validation and error behavior.
🪄 Autofix (Beta)
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: ASSERTIVE
Plan: Pro
Run ID: 9ea8b2bb-ccc7-4051-8bae-9b3b86ea40ee
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (196)
.gitignoreCargo.tomlmagicblock-account-cloner/Cargo.tomlmagicblock-account-cloner/README.mdmagicblock-account-cloner/src/lib.rsmagicblock-account-cloner/src/util.rsmagicblock-accounts-db/Cargo.tomlmagicblock-accounts-db/README.mdmagicblock-accounts-db/src/error.rsmagicblock-accounts-db/src/index.rsmagicblock-accounts-db/src/index/iterator.rsmagicblock-accounts-db/src/index/table.rsmagicblock-accounts-db/src/index/tests.rsmagicblock-accounts-db/src/index/utils.rsmagicblock-accounts-db/src/lib.rsmagicblock-accounts-db/src/reset.rsmagicblock-accounts-db/src/snapshot.rsmagicblock-accounts-db/src/storage.rsmagicblock-accounts-db/src/tests.rsmagicblock-accounts-db/src/traits.rsmagicblock-accounts/Cargo.tomlmagicblock-accounts/README.mdmagicblock-accounts/src/config.rsmagicblock-accounts/src/errors.rsmagicblock-accounts/src/lib.rsmagicblock-accounts/src/traits.rsmagicblock-aperture/Cargo.tomlmagicblock-aperture/src/requests/http/mod.rsmagicblock-aperture/src/server/http/dispatch.rsmagicblock-aperture/tests/transaction_primary_mode.rsmagicblock-api/Cargo.tomlmagicblock-api/src/magic_validator.rsmagicblock-chainlink/Cargo.tomlmagicblock-chainlink/src/accounts_bank.rsmagicblock-chainlink/src/chainlink/fetch_cloner/ata_projection.rsmagicblock-chainlink/src/chainlink/fetch_cloner/delegation.rsmagicblock-chainlink/src/chainlink/fetch_cloner/mod.rsmagicblock-chainlink/src/chainlink/fetch_cloner/pending_operation.rsmagicblock-chainlink/src/chainlink/fetch_cloner/pipeline.rsmagicblock-chainlink/src/chainlink/fetch_cloner/program_loader.rsmagicblock-chainlink/src/chainlink/fetch_cloner/tests.rsmagicblock-chainlink/src/chainlink/mod.rsmagicblock-chainlink/src/cloner/engine_cloner.rsmagicblock-chainlink/src/cloner/errors.rsmagicblock-chainlink/src/cloner/mod.rsmagicblock-chainlink/src/remote_account_provider/program_account.rsmagicblock-chainlink/src/remote_account_provider/pubsub_connection_pool.rsmagicblock-chainlink/src/remote_account_provider/remote_account.rsmagicblock-chainlink/src/submux/debounce_state.rsmagicblock-chainlink/src/testing/accounts.rsmagicblock-chainlink/src/testing/cloner_stub.rsmagicblock-chainlink/src/testing/mod.rsmagicblock-chainlink/tests/01_ensure-accounts.rsmagicblock-chainlink/tests/utils/accounts.rsmagicblock-committor-service/Cargo.tomlmagicblock-committor-service/src/committor_processor.rsmagicblock-committor-service/src/intent_execution_manager.rsmagicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rsmagicblock-committor-service/src/intent_execution_manager/intent_scheduler.rsmagicblock-committor-service/src/intent_executor/intent_execution_client.rsmagicblock-committor-service/src/intent_executor/intent_executor_factory.rsmagicblock-committor-service/src/intent_executor/mod.rsmagicblock-committor-service/src/persist/commit_persister.rsmagicblock-committor-service/src/service.rsmagicblock-committor-service/src/service/intent_client.rsmagicblock-committor-service/src/tasks/task_builder.rsmagicblock-committor-service/src/test_utils.rsmagicblock-core/src/coordination_mode.rsmagicblock-core/src/intent/mod.rsmagicblock-core/src/intent/types.rsmagicblock-core/src/lib.rsmagicblock-core/src/link.rsmagicblock-core/src/link/accounts.rsmagicblock-core/src/link/blocks.rsmagicblock-core/src/link/transactions.rsmagicblock-core/src/replication.rsmagicblock-core/src/tls.rsmagicblock-core/src/token_programs.rsmagicblock-core/src/traits.rsmagicblock-ledger/Cargo.tomlmagicblock-ledger/src/blockstore_processor/mod.rsmagicblock-ledger/src/database/rocks_db.rsmagicblock-ledger/src/ledger_truncator.rsmagicblock-ledger/src/lib.rsmagicblock-ledger/src/store/api.rsmagicblock-ledger/tests/common.rsmagicblock-ledger/tests/get_block.rsmagicblock-ledger/tests/test_ledger_truncator.rsmagicblock-magic-program-api/Cargo.tomlmagicblock-magic-program-api/src/args.rsmagicblock-processor/Cargo.tomlmagicblock-processor/README.mdmagicblock-processor/src/builtins.rsmagicblock-processor/src/executor/callback.rsmagicblock-processor/src/executor/mod.rsmagicblock-processor/src/executor/processing.rsmagicblock-processor/src/lib.rsmagicblock-processor/src/loader.rsmagicblock-processor/src/scheduler/coordinator.rsmagicblock-processor/src/scheduler/locks.rsmagicblock-processor/src/scheduler/mod.rsmagicblock-processor/src/scheduler/state.rsmagicblock-processor/src/scheduler/tests.rsmagicblock-processor/tests/ephemeral_accounts.rsmagicblock-processor/tests/execution.rsmagicblock-processor/tests/fees.rsmagicblock-processor/tests/post_delegation_actions.rsmagicblock-processor/tests/replay.rsmagicblock-processor/tests/replica_ordering.rsmagicblock-processor/tests/scheduling.rsmagicblock-processor/tests/security.rsmagicblock-processor/tests/simulation.rsmagicblock-replicator/Cargo.tomlmagicblock-replicator/README.mdmagicblock-replicator/src/error.rsmagicblock-replicator/src/lib.rsmagicblock-replicator/src/nats/broker.rsmagicblock-replicator/src/nats/consumer.rsmagicblock-replicator/src/nats/lock_watcher.rsmagicblock-replicator/src/nats/mod.rsmagicblock-replicator/src/nats/producer.rsmagicblock-replicator/src/nats/snapshot.rsmagicblock-replicator/src/service/context.rsmagicblock-replicator/src/service/mod.rsmagicblock-replicator/src/service/primary.rsmagicblock-replicator/src/service/replica.rsmagicblock-replicator/src/tests.rsmagicblock-replicator/src/watcher.rsmagicblock-services/Cargo.tomlmagicblock-services/src/actions_callback_service.rsmagicblock-services/src/undelegation_request_service.rsmagicblock-task-scheduler/Cargo.tomlmagicblock-task-scheduler/src/errors.rsmagicblock-task-scheduler/src/service.rsprograms/magicblock/Cargo.tomlprograms/magicblock/src/clone_account/common.rsprograms/magicblock/src/clone_account/mod.rsprograms/magicblock/src/clone_account/process_cleanup.rsprograms/magicblock/src/clone_account/process_clone.rsprograms/magicblock/src/clone_account/process_clone_continue.rsprograms/magicblock/src/clone_account/process_clone_init.rsprograms/magicblock/src/clone_account/process_evict.rsprograms/magicblock/src/clone_account/process_finalize_buffer.rsprograms/magicblock/src/clone_account/process_finalize_v1_buffer.rsprograms/magicblock/src/clone_account/process_post_delegation_actions.rsprograms/magicblock/src/clone_account/process_set_authority.rsprograms/magicblock/src/clone_account/tests.rsprograms/magicblock/src/ephemeral_accounts/mod.rsprograms/magicblock/src/ephemeral_accounts/process_close.rsprograms/magicblock/src/ephemeral_accounts/process_create.rsprograms/magicblock/src/ephemeral_accounts/process_resize.rsprograms/magicblock/src/ephemeral_accounts/validation.rsprograms/magicblock/src/errors.rsprograms/magicblock/src/lib.rsprograms/magicblock/src/magic_scheduled_base_intent.rsprograms/magicblock/src/magicblock_processor.rsprograms/magicblock/src/mutate_accounts/mod.rsprograms/magicblock/src/mutate_accounts/process_mutate_accounts.rsprograms/magicblock/src/schedule_task/mod.rsprograms/magicblock/src/schedule_task/process_cancel_task.rsprograms/magicblock/src/schedule_task/process_execute_task.rsprograms/magicblock/src/schedule_task/process_schedule_task.rsprograms/magicblock/src/schedule_transactions/mod.rsprograms/magicblock/src/schedule_transactions/process_accept_scheduled_commits.rsprograms/magicblock/src/schedule_transactions/process_execute_callback.rsprograms/magicblock/src/schedule_transactions/process_schedule_cloned_undelegation.rsprograms/magicblock/src/schedule_transactions/process_schedule_commit.rsprograms/magicblock/src/schedule_transactions/process_schedule_commit_tests.rsprograms/magicblock/src/schedule_transactions/process_schedule_intent_bundle.rsprograms/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rsprograms/magicblock/src/test_utils/mod.rsprograms/magicblock/src/utils/account_actions.rsprograms/magicblock/src/utils/accounts.rsprograms/magicblock/src/utils/instruction_context_frames.rsprograms/magicblock/src/utils/instruction_sysvar.rsprograms/magicblock/src/utils/instruction_utils.rsprograms/magicblock/src/utils/mod.rsprograms/magicblock/src/utils/validation.rsprograms/magicblock/src/validator.rsrust-toolchain.tomlstorage-proto/Cargo.tomlstorage-proto/build.rsstorage-proto/src/convert.rstest-kit/Cargo.tomltest-kit/src/lib.rstest-kit/src/macros.rstools/genx/Cargo.tomltools/genx/README.mdtools/genx/src/main.rstools/genx/src/test_validator.rstools/keypair-base58/Cargo.tomltools/keypair-base58/README.mdtools/keypair-base58/src/main.rstools/ledger-stats/Cargo.tomltools/ledger-stats/README.mdtools/ledger-stats/src/account.rs
💤 Files with no reviewable changes (61)
- magicblock-accounts-db/README.md
- magicblock-accounts/Cargo.toml
- magicblock-accounts/README.md
- magicblock-ledger/tests/test_ledger_truncator.rs
- magicblock-account-cloner/Cargo.toml
- magicblock-processor/Cargo.toml
- magicblock-processor/tests/execution.rs
- magicblock-accounts-db/src/reset.rs
- magicblock-ledger/tests/get_block.rs
- .gitignore
- magicblock-accounts-db/Cargo.toml
- magicblock-processor/tests/fees.rs
- magicblock-accounts-db/src/error.rs
- magicblock-account-cloner/src/lib.rs
- magicblock-accounts-db/src/traits.rs
- magicblock-processor/src/builtins.rs
- programs/magicblock/src/utils/mod.rs
- magicblock-accounts-db/src/tests.rs
- magicblock-core/src/coordination_mode.rs
- magicblock-accounts-db/src/index/utils.rs
- magicblock-accounts-db/src/index/tests.rs
- magicblock-processor/src/scheduler/state.rs
- magicblock-accounts-db/src/index/iterator.rs
- magicblock-ledger/src/blockstore_processor/mod.rs
- magicblock-account-cloner/README.md
- magicblock-accounts/src/traits.rs
- magicblock-account-cloner/src/util.rs
- magicblock-core/src/link/blocks.rs
- magicblock-processor/README.md
- magicblock-processor/tests/ephemeral_accounts.rs
- magicblock-accounts-db/src/storage.rs
- magicblock-processor/tests/replay.rs
- magicblock-accounts/src/config.rs
- magicblock-processor/src/executor/mod.rs
- magicblock-core/src/tls.rs
- magicblock-processor/src/scheduler/locks.rs
- magicblock-processor/src/executor/callback.rs
- magicblock-ledger/tests/common.rs
- magicblock-processor/tests/replica_ordering.rs
- programs/magicblock/src/schedule_transactions/process_schedule_intent_bundle.rs
- magicblock-aperture/tests/transaction_primary_mode.rs
- magicblock-processor/src/lib.rs
- magicblock-core/src/link/accounts.rs
- magicblock-processor/src/scheduler/mod.rs
- magicblock-ledger/src/ledger_truncator.rs
- magicblock-processor/src/loader.rs
- magicblock-accounts/src/errors.rs
- magicblock-accounts-db/src/index/table.rs
- magicblock-ledger/src/database/rocks_db.rs
- magicblock-processor/src/executor/processing.rs
- magicblock-core/src/link.rs
- magicblock-accounts/src/lib.rs
- magicblock-core/src/link/transactions.rs
- magicblock-processor/tests/post_delegation_actions.rs
- magicblock-processor/src/scheduler/coordinator.rs
- magicblock-accounts-db/src/snapshot.rs
- magicblock-core/src/replication.rs
- programs/magicblock/src/lib.rs
- magicblock-accounts-db/src/lib.rs
- magicblock-accounts-db/src/index.rs
- magicblock-processor/src/scheduler/tests.rs
Comment on lines
+10
to
+17
| pub trait AccountsBank: Send + Sync + 'static { | ||
| fn get_account(&self, pubkey: &Pubkey) -> Option<AccountSharedData>; | ||
| } | ||
|
|
||
| impl AccountsBank for Engine { | ||
| fn get_account(&self, pubkey: &Pubkey) -> Option<AccountSharedData> { | ||
| self.accounts().get(pubkey).ok().flatten() | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Preserve Engine lookup failures instead of reporting missing accounts.
Line 16 converts every Engine read error into None. Downstream code treats None as an absent account and may fetch, clone, or evict based on a storage failure. Make the trait return a Result<Option<AccountSharedData>, _> and propagate the failure.
🤖 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 `@magicblock-chainlink/src/accounts_bank.rs` around lines 10 - 17, Update the
AccountsBank::get_account contract and the Engine implementation to return
Result<Option<AccountSharedData>, _> instead of collapsing lookup errors into
None. Propagate the error from self.accounts().get(pubkey) while retaining None
only for genuinely missing accounts, and update all trait callers to handle the
Result.
Comment on lines
+84
to
+89
| // A program retracted on chain is not deployed there, so it is not | ||
| // materialized here either. | ||
| if matches!(program.loader_status, LoaderV4Status::Retracted) { | ||
| debug!(program_id = %program_id, "Program is retracted on chain"); | ||
| return Ok(()); | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Delete previously cloned programs when they become retracted.
Returning Ok(()) leaves any existing local executable live after its on-chain program becomes retracted. Delete program_id through the Engine and propagate deletion failures.
Proposed fix
if matches!(program.loader_status, LoaderV4Status::Retracted) {
debug!(program_id = %program_id, "Program is retracted on chain");
- return Ok(());
+ return self.evict_account(program_id).await;
}📝 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.
Suggested change
| // A program retracted on chain is not deployed there, so it is not | |
| // materialized here either. | |
| if matches!(program.loader_status, LoaderV4Status::Retracted) { | |
| debug!(program_id = %program_id, "Program is retracted on chain"); | |
| return Ok(()); | |
| } | |
| // A program retracted on chain is not deployed there, so it is not | |
| // materialized here either. | |
| if matches!(program.loader_status, LoaderV4Status::Retracted) { | |
| debug!(program_id = %program_id, "Program is retracted on chain"); | |
| return self.evict_account(program_id).await; | |
| } |
🤖 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 `@magicblock-chainlink/src/cloner/engine_cloner.rs` around lines 84 - 89,
Update the LoaderV4Status::Retracted branch in the cloner flow to delete
program_id through the Engine before returning. Propagate any deletion failure
instead of unconditionally returning Ok(()), while preserving the existing debug
logging.
Comment on lines
+115
to
123
| let magic_context_acc = self | ||
| .engine | ||
| .accounts() | ||
| .get(&MAGIC_CONTEXT_PUBKEY) | ||
| .ok() | ||
| .flatten() | ||
| .expect( | ||
| "Validator found to be running without MagicContext account!", | ||
| ); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Propagate the MagicContext lookup failure instead of panicking.
.ok() also erases the underlying account-store error. Map that error and return a dedicated missing-account error rather than terminating the validator.
As per path instructions, “Treat any usage of .unwrap() or .expect() in production Rust code as a MAJOR issue.”
🤖 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 `@magicblock-committor-service/src/service/intent_client.rs` around lines 115 -
123, Update the MagicContext lookup in the intent client to avoid expect-based
panicking and preserve account-store failures. Map lookup errors into the
service’s dedicated missing-account error, explicitly handle an absent account
with the same error, and propagate the result from the enclosing method instead
of terminating the validator.
Source: Path instructions
Comment on lines
146
to
149
| let sent_commit = build_sent_commit(meta, &result); | ||
| register_scheduled_commit_sent(sent_commit); | ||
| let txn = with_encoded(tx).inspect_err(|err| { | ||
| // Unreachable case, all intent transactions are smaller than 64KB by construction | ||
| error!(error = ?err, "Failed to bincode intent transaction"); | ||
| })?; | ||
| self.transaction_scheduler | ||
| .execute(txn) | ||
| self.execute(message) | ||
| .await |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Roll back or retain the notification when execution fails.
register_scheduled_commit_sent mutates the global registry before the fallible Engine execution. On failure, the entry remains while the caller has already removed ScheduledBaseIntentMeta, so the notification is neither cleaned up nor retried until external recovery.
🤖 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 `@magicblock-committor-service/src/service/intent_client.rs` around lines 146 -
149, Update the flow around register_scheduled_commit_sent and self.execute so
the scheduled-commit notification is not left in the global registry when
execution fails: either register it only after successful execution or
explicitly remove/restore the entry on error. Preserve the existing notification
behavior on successful execution and ensure the caller’s ScheduledBaseIntentMeta
state remains consistent.
Comment on lines
+223
to
+242
| message = self.service_messages.recv() => { | ||
| let encoded = match message { | ||
| Ok(encoded) => encoded, | ||
| // Slow consumers drop the oldest messages; a lagged task | ||
| // request would be lost, so surface it rather than hide it. | ||
| Err(broadcast::error::RecvError::Lagged(skipped)) => { | ||
| warn!("Task scheduler lagged, {skipped} service messages skipped"); | ||
| continue; | ||
| } | ||
| Err(broadcast::error::RecvError::Closed) => { | ||
| info!("Service message stream closed, stopping task scheduler"); | ||
| break; | ||
| } | ||
| }; | ||
| // The stream carries every service message, not only task | ||
| // requests; anything that is not a `TaskRequest` is ignored. | ||
| let Ok(task) = wincode::deserialize::<TaskRequest>(&encoded) | ||
| else { | ||
| continue; | ||
| }; |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Recover lagged task requests instead of dropping them.
A broadcast lag permanently discards committed schedule/cancel messages, yet this branch only logs and continues. Use a replayable sequence/persistent outbox or stop and resynchronize from an authoritative source when Lagged occurs.
🤖 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 `@magicblock-task-scheduler/src/service.rs` around lines 223 - 242, The Lagged
branch in the service_messages receive loop currently skips permanently lost
task requests. Update this handling to recover by replaying the missing messages
from a persistent/replayable outbox or by stopping and resynchronizing from an
authoritative source, rather than merely logging and continuing; preserve the
existing Closed behavior and normal TaskRequest deserialization flow.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
programs/magicblock/src/magic_scheduled_base_intent.rs (1)
504-521: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRemove the duplicated ephemeral-mode check.
Line 504 already rejects
AccountMode::Ephemeral, so the identical check at Line 514 is unreachable and its accurate diagnostic is never emitted. Keep only the second block.Proposed fix
- if account.to_account_shared_data()?.is(AccountMode::Ephemeral) { - ic_msg!( - context.invoke_context, - "ScheduleCommit ERR: account {} is confined and cannot be committed", - pubkey - ); - return Err(InstructionError::InvalidAccountData); - } - // Prevent ephemeral accounts from being committed to base chain if account.to_account_shared_data()?.is(AccountMode::Ephemeral) {🤖 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 `@programs/magicblock/src/magic_scheduled_base_intent.rs` around lines 504 - 521, Remove the first duplicate AccountMode::Ephemeral validation block in the schedule commit flow, including its “confined” diagnostic and error return. Keep the later check and its “ephemeral and cannot be committed to base chain” diagnostic unchanged.magicblock-aperture/src/server/http/dispatch.rs (1)
222-225: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRestore primary-state gating for
/health/primary.This endpoint now returns HTTP 200 for replicas and nodes not yet promoted to primary. Primary-aware load balancers can consequently route writes to an ineligible node. Consult the Engine’s current role/readiness state and return
503 Service Unavailableunless primary.🤖 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 `@magicblock-aperture/src/server/http/dispatch.rs` around lines 222 - 225, Update the `/health/primary` branch in the HTTP dispatch logic to consult the Engine’s current role/readiness state and return `503 Service Unavailable` for replicas or nodes not yet promoted to primary. Preserve the existing successful response and access-control headers only when the node is primary.magicblock-api/src/magic_validator.rs (1)
746-757: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftGate unregistration on the Engine’s active role.
Static
ReplicationMode::Primarydoes not prove this node is currently primary after a runtime role transition. A standby shutting down can therefore unregister the active validator’s on-chain record. Wire the active Engine role into this check.This extends the previously reported missing Engine role-state wiring.
🤖 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 `@magicblock-api/src/magic_validator.rs` around lines 746 - 757, Update start_unregister_validator_on_chain to require the Engine’s current active role to be Primary in addition to the existing replication-mode and lifecycle checks. Use the runtime role-state source already exposed by the Engine rather than relying on config.validator.replication_mode, so a standby after a role transition cannot unregister the active validator’s on-chain record.
♻️ Duplicate comments (2)
magicblock-committor-service/src/service/intent_client.rs (2)
115-123: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winPropagate the MagicContext lookup failure instead of panicking.
.ok()also erases the underlying account-store error. Map that error and return a dedicated missing-account error rather than terminating the validator.As per path instructions, "Treat any usage of
.unwrap()or.expect()in production Rust code as a MAJOR issue."🤖 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 `@magicblock-committor-service/src/service/intent_client.rs` around lines 115 - 123, Replace the panic in the MagicContext lookup within the intent client flow with propagated error handling: preserve account-store failures by mapping them into the service’s dedicated missing-account error, and return that error instead of using `.ok().flatten().expect(...)`. Remove the `.expect()` and ensure the surrounding function’s error return type carries both lookup and missing-account cases.Source: Path instructions
146-154: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftRoll back or retain the notification when execution fails.
register_scheduled_commit_sentmutates the global registry before the fallible Engine execution. On failure, the entry remains while the caller has already removedScheduledBaseIntentMeta, so the notification is neither cleaned up nor retried until external recovery.🤖 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 `@magicblock-committor-service/src/service/intent_client.rs` around lines 146 - 154, Update the flow around register_scheduled_commit_sent and self.execute so a failed execution does not leave an orphaned global notification: either register the sent commit only after self.execute succeeds, or explicitly remove/restore the registration in the inspect_err failure path. Preserve the existing success logging and error propagation behavior.
🤖 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 `@Cargo.toml`:
- Around line 56-60: Update the project setup and CI workflows to ensure the
sibling ../engine checkout is provisioned before Cargo resolves the path
dependencies magicblock-engine, magicblock-engine-nucleus, and ledger. Apply
this consistently to all affected workflow/local setup paths, or replace those
cross-repository path dependencies with available non-path dependencies while
preserving the existing crate usage.
In `@magicblock-committor-service/Cargo.toml`:
- Around line 21-23: Correct the comment typos in Cargo.toml by changing “TDOO”
to “TODO” and “behinf” to “behind,” without modifying the comments’ meaning.
In `@magicblock-task-scheduler/src/service.rs`:
- Around line 440-452: Update submit_crank’s transaction error mapping so
compose/await/commit failures retain distinct error categories instead of all
becoming TransactionExecution. Ensure is_retryable_task_execution_error marks
only transient engine or RPC failures as retryable, while deterministic program
or instruction failures remain permanent.
- Around line 213-217: Update the crank batch task around send_crank_batch and
the corresponding flow near line 431 to drain spawned workers with join_next
instead of join_all. Handle each JoinError individually so one worker panic does
not abort processing, and ensure crank_tx.send(...) is always called with the
batch result regardless of individual worker failures.
---
Outside diff comments:
In `@magicblock-aperture/src/server/http/dispatch.rs`:
- Around line 222-225: Update the `/health/primary` branch in the HTTP dispatch
logic to consult the Engine’s current role/readiness state and return `503
Service Unavailable` for replicas or nodes not yet promoted to primary. Preserve
the existing successful response and access-control headers only when the node
is primary.
In `@magicblock-api/src/magic_validator.rs`:
- Around line 746-757: Update start_unregister_validator_on_chain to require the
Engine’s current active role to be Primary in addition to the existing
replication-mode and lifecycle checks. Use the runtime role-state source already
exposed by the Engine rather than relying on config.validator.replication_mode,
so a standby after a role transition cannot unregister the active validator’s
on-chain record.
In `@programs/magicblock/src/magic_scheduled_base_intent.rs`:
- Around line 504-521: Remove the first duplicate AccountMode::Ephemeral
validation block in the schedule commit flow, including its “confined”
diagnostic and error return. Keep the later check and its “ephemeral and cannot
be committed to base chain” diagnostic unchanged.
---
Duplicate comments:
In `@magicblock-committor-service/src/service/intent_client.rs`:
- Around line 115-123: Replace the panic in the MagicContext lookup within the
intent client flow with propagated error handling: preserve account-store
failures by mapping them into the service’s dedicated missing-account error, and
return that error instead of using `.ok().flatten().expect(...)`. Remove the
`.expect()` and ensure the surrounding function’s error return type carries both
lookup and missing-account cases.
- Around line 146-154: Update the flow around register_scheduled_commit_sent and
self.execute so a failed execution does not leave an orphaned global
notification: either register the sent commit only after self.execute succeeds,
or explicitly remove/restore the registration in the inspect_err failure path.
Preserve the existing success logging and error propagation behavior.
🪄 Autofix (Beta)
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: ASSERTIVE
Plan: Pro
Run ID: 5e761615-5a67-4199-85f8-863d691ef3d6
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (214)
.gitignoreCargo.tomlmagicblock-account-cloner/Cargo.tomlmagicblock-account-cloner/README.mdmagicblock-account-cloner/src/lib.rsmagicblock-account-cloner/src/util.rsmagicblock-accounts-db/Cargo.tomlmagicblock-accounts-db/README.mdmagicblock-accounts-db/src/error.rsmagicblock-accounts-db/src/index.rsmagicblock-accounts-db/src/index/iterator.rsmagicblock-accounts-db/src/index/table.rsmagicblock-accounts-db/src/index/tests.rsmagicblock-accounts-db/src/index/utils.rsmagicblock-accounts-db/src/lib.rsmagicblock-accounts-db/src/reset.rsmagicblock-accounts-db/src/snapshot.rsmagicblock-accounts-db/src/storage.rsmagicblock-accounts-db/src/tests.rsmagicblock-accounts-db/src/traits.rsmagicblock-accounts/Cargo.tomlmagicblock-accounts/README.mdmagicblock-accounts/src/config.rsmagicblock-accounts/src/errors.rsmagicblock-accounts/src/lib.rsmagicblock-accounts/src/traits.rsmagicblock-aperture/Cargo.tomlmagicblock-aperture/src/encoder.rsmagicblock-aperture/src/processor.rsmagicblock-aperture/src/requests/http/mod.rsmagicblock-aperture/src/server/http/dispatch.rsmagicblock-aperture/src/server/http/mod.rsmagicblock-aperture/src/state/blocks.rsmagicblock-aperture/src/state/cache.rsmagicblock-aperture/src/state/mod.rsmagicblock-aperture/src/state/signatures.rsmagicblock-aperture/src/state/subscriptions.rsmagicblock-aperture/src/state/transactions.rsmagicblock-aperture/src/utils.rsmagicblock-aperture/tests/transaction_primary_mode.rsmagicblock-api/Cargo.tomlmagicblock-api/src/magic_validator.rsmagicblock-chainlink/Cargo.tomlmagicblock-chainlink/src/accounts_bank.rsmagicblock-chainlink/src/chainlink/fetch_cloner/ata_projection.rsmagicblock-chainlink/src/chainlink/fetch_cloner/delegation.rsmagicblock-chainlink/src/chainlink/fetch_cloner/mod.rsmagicblock-chainlink/src/chainlink/fetch_cloner/pending_operation.rsmagicblock-chainlink/src/chainlink/fetch_cloner/pipeline.rsmagicblock-chainlink/src/chainlink/fetch_cloner/program_loader.rsmagicblock-chainlink/src/chainlink/fetch_cloner/tests.rsmagicblock-chainlink/src/chainlink/mod.rsmagicblock-chainlink/src/cloner/engine_cloner.rsmagicblock-chainlink/src/cloner/errors.rsmagicblock-chainlink/src/cloner/mod.rsmagicblock-chainlink/src/remote_account_provider/program_account.rsmagicblock-chainlink/src/remote_account_provider/pubsub_connection_pool.rsmagicblock-chainlink/src/remote_account_provider/remote_account.rsmagicblock-chainlink/src/submux/debounce_state.rsmagicblock-chainlink/src/testing/accounts.rsmagicblock-chainlink/src/testing/cloner_stub.rsmagicblock-chainlink/src/testing/mod.rsmagicblock-chainlink/tests/01_ensure-accounts.rsmagicblock-chainlink/tests/utils/accounts.rsmagicblock-committor-service/Cargo.tomlmagicblock-committor-service/src/committor_processor.rsmagicblock-committor-service/src/intent_execution_manager.rsmagicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rsmagicblock-committor-service/src/intent_execution_manager/intent_scheduler.rsmagicblock-committor-service/src/intent_executor/intent_execution_client.rsmagicblock-committor-service/src/intent_executor/intent_executor_factory.rsmagicblock-committor-service/src/intent_executor/mod.rsmagicblock-committor-service/src/persist/commit_persister.rsmagicblock-committor-service/src/service.rsmagicblock-committor-service/src/service/intent_client.rsmagicblock-committor-service/src/tasks/task_builder.rsmagicblock-committor-service/src/test_utils.rsmagicblock-core/src/coordination_mode.rsmagicblock-core/src/intent/mod.rsmagicblock-core/src/intent/types.rsmagicblock-core/src/lib.rsmagicblock-core/src/link.rsmagicblock-core/src/link/accounts.rsmagicblock-core/src/link/blocks.rsmagicblock-core/src/link/transactions.rsmagicblock-core/src/replication.rsmagicblock-core/src/tls.rsmagicblock-core/src/token_programs.rsmagicblock-core/src/traits.rsmagicblock-ledger/Cargo.tomlmagicblock-ledger/src/blockstore_processor/mod.rsmagicblock-ledger/src/database/rocks_db.rsmagicblock-ledger/src/ledger_truncator.rsmagicblock-ledger/src/lib.rsmagicblock-ledger/src/store/api.rsmagicblock-ledger/tests/common.rsmagicblock-ledger/tests/get_block.rsmagicblock-ledger/tests/test_ledger_truncator.rsmagicblock-magic-program-api/Cargo.tomlmagicblock-magic-program-api/src/args.rsmagicblock-processor/Cargo.tomlmagicblock-processor/README.mdmagicblock-processor/src/builtins.rsmagicblock-processor/src/executor/callback.rsmagicblock-processor/src/executor/mod.rsmagicblock-processor/src/executor/processing.rsmagicblock-processor/src/lib.rsmagicblock-processor/src/loader.rsmagicblock-processor/src/scheduler/coordinator.rsmagicblock-processor/src/scheduler/locks.rsmagicblock-processor/src/scheduler/mod.rsmagicblock-processor/src/scheduler/state.rsmagicblock-processor/src/scheduler/tests.rsmagicblock-processor/tests/ephemeral_accounts.rsmagicblock-processor/tests/execution.rsmagicblock-processor/tests/fees.rsmagicblock-processor/tests/post_delegation_actions.rsmagicblock-processor/tests/replay.rsmagicblock-processor/tests/replica_ordering.rsmagicblock-processor/tests/scheduling.rsmagicblock-processor/tests/security.rsmagicblock-processor/tests/simulation.rsmagicblock-replicator/Cargo.tomlmagicblock-replicator/README.mdmagicblock-replicator/src/error.rsmagicblock-replicator/src/lib.rsmagicblock-replicator/src/nats/broker.rsmagicblock-replicator/src/nats/consumer.rsmagicblock-replicator/src/nats/lock_watcher.rsmagicblock-replicator/src/nats/mod.rsmagicblock-replicator/src/nats/producer.rsmagicblock-replicator/src/nats/snapshot.rsmagicblock-replicator/src/service/context.rsmagicblock-replicator/src/service/mod.rsmagicblock-replicator/src/service/primary.rsmagicblock-replicator/src/service/replica.rsmagicblock-replicator/src/tests.rsmagicblock-replicator/src/watcher.rsmagicblock-services/Cargo.tomlmagicblock-services/src/actions_callback_service.rsmagicblock-services/src/undelegation_request_service.rsmagicblock-task-scheduler/Cargo.tomlmagicblock-task-scheduler/src/errors.rsmagicblock-task-scheduler/src/service.rsprograms/magicblock/Cargo.tomlprograms/magicblock/src/clone_account/common.rsprograms/magicblock/src/clone_account/mod.rsprograms/magicblock/src/clone_account/process_cleanup.rsprograms/magicblock/src/clone_account/process_clone.rsprograms/magicblock/src/clone_account/process_clone_continue.rsprograms/magicblock/src/clone_account/process_clone_init.rsprograms/magicblock/src/clone_account/process_evict.rsprograms/magicblock/src/clone_account/process_finalize_buffer.rsprograms/magicblock/src/clone_account/process_finalize_v1_buffer.rsprograms/magicblock/src/clone_account/process_post_delegation_actions.rsprograms/magicblock/src/clone_account/process_set_authority.rsprograms/magicblock/src/clone_account/tests.rsprograms/magicblock/src/ephemeral_accounts/mod.rsprograms/magicblock/src/ephemeral_accounts/process_close.rsprograms/magicblock/src/ephemeral_accounts/process_create.rsprograms/magicblock/src/ephemeral_accounts/process_resize.rsprograms/magicblock/src/ephemeral_accounts/validation.rsprograms/magicblock/src/errors.rsprograms/magicblock/src/lib.rsprograms/magicblock/src/magic_scheduled_base_intent.rsprograms/magicblock/src/magicblock_processor.rsprograms/magicblock/src/mutate_accounts/mod.rsprograms/magicblock/src/mutate_accounts/process_mutate_accounts.rsprograms/magicblock/src/schedule_task/mod.rsprograms/magicblock/src/schedule_task/process_cancel_task.rsprograms/magicblock/src/schedule_task/process_execute_task.rsprograms/magicblock/src/schedule_task/process_schedule_task.rsprograms/magicblock/src/schedule_transactions/mod.rsprograms/magicblock/src/schedule_transactions/process_accept_scheduled_commits.rsprograms/magicblock/src/schedule_transactions/process_execute_callback.rsprograms/magicblock/src/schedule_transactions/process_schedule_cloned_undelegation.rsprograms/magicblock/src/schedule_transactions/process_schedule_commit.rsprograms/magicblock/src/schedule_transactions/process_schedule_commit_tests.rsprograms/magicblock/src/schedule_transactions/process_schedule_intent_bundle.rsprograms/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rsprograms/magicblock/src/test_utils/mod.rsprograms/magicblock/src/utils/account_actions.rsprograms/magicblock/src/utils/accounts.rsprograms/magicblock/src/utils/instruction_context_frames.rsprograms/magicblock/src/utils/instruction_sysvar.rsprograms/magicblock/src/utils/instruction_utils.rsprograms/magicblock/src/utils/mod.rsprograms/magicblock/src/utils/validation.rsprograms/magicblock/src/validator.rsrust-toolchain.tomlstorage-proto/Cargo.tomlstorage-proto/build.rsstorage-proto/src/convert.rstest-kit/Cargo.tomltest-kit/src/lib.rstest-kit/src/macros.rstools/genx/Cargo.tomltools/genx/README.mdtools/genx/src/main.rstools/genx/src/test_validator.rstools/keypair-base58/Cargo.tomltools/keypair-base58/README.mdtools/keypair-base58/src/main.rstools/ledger-stats/Cargo.tomltools/ledger-stats/README.mdtools/ledger-stats/src/account.rstools/ledger-stats/src/accounts.rstools/ledger-stats/src/blockhash.rstools/ledger-stats/src/counts.rstools/ledger-stats/src/lib.rstools/ledger-stats/src/main.rstools/ledger-stats/src/transaction_details.rstools/ledger-stats/src/transaction_logs.rstools/ledger-stats/src/utils.rs
💤 Files with no reviewable changes (63)
- magicblock-accounts/README.md
- magicblock-account-cloner/Cargo.toml
- magicblock-accounts/src/traits.rs
- magicblock-accounts-db/README.md
- magicblock-accounts-db/Cargo.toml
- magicblock-processor/Cargo.toml
- magicblock-ledger/tests/common.rs
- magicblock-accounts/src/lib.rs
- magicblock-account-cloner/README.md
- programs/magicblock/src/schedule_transactions/process_schedule_intent_bundle.rs
- magicblock-accounts/Cargo.toml
- magicblock-processor/src/lib.rs
- magicblock-processor/src/scheduler/state.rs
- magicblock-accounts-db/src/traits.rs
- magicblock-processor/src/builtins.rs
- programs/magicblock/src/utils/mod.rs
- magicblock-core/src/link.rs
- magicblock-aperture/tests/transaction_primary_mode.rs
- magicblock-processor/src/executor/callback.rs
- magicblock-core/src/link/blocks.rs
- magicblock-aperture/src/state/signatures.rs
- magicblock-core/src/tls.rs
- magicblock-processor/tests/ephemeral_accounts.rs
- magicblock-accounts/src/errors.rs
- magicblock-accounts-db/src/index/iterator.rs
- magicblock-ledger/tests/test_ledger_truncator.rs
- magicblock-accounts-db/src/index/tests.rs
- magicblock-processor/src/scheduler/mod.rs
- magicblock-ledger/src/blockstore_processor/mod.rs
- magicblock-processor/src/scheduler/tests.rs
- magicblock-aperture/src/state/cache.rs
- magicblock-aperture/src/state/transactions.rs
- magicblock-accounts/src/config.rs
- magicblock-accounts-db/src/error.rs
- magicblock-accounts-db/src/storage.rs
- magicblock-account-cloner/src/lib.rs
- magicblock-committor-service/src/intent_execution_manager/intent_scheduler.rs
- magicblock-processor/src/loader.rs
- magicblock-ledger/src/database/rocks_db.rs
- magicblock-ledger/tests/get_block.rs
- magicblock-processor/src/executor/mod.rs
- magicblock-committor-service/src/persist/commit_persister.rs
- magicblock-account-cloner/src/util.rs
- magicblock-accounts-db/src/index/utils.rs
- magicblock-aperture/src/state/blocks.rs
- magicblock-processor/src/scheduler/coordinator.rs
- magicblock-accounts-db/src/snapshot.rs
- magicblock-processor/README.md
- .gitignore
- magicblock-processor/src/scheduler/locks.rs
- programs/magicblock/src/lib.rs
- magicblock-aperture/src/processor.rs
- magicblock-processor/tests/execution.rs
- magicblock-accounts-db/src/index.rs
- magicblock-core/src/link/accounts.rs
- magicblock-accounts-db/src/tests.rs
- magicblock-ledger/src/ledger_truncator.rs
- magicblock-processor/src/executor/processing.rs
- magicblock-accounts-db/src/index/table.rs
- magicblock-accounts-db/src/lib.rs
- magicblock-core/src/coordination_mode.rs
- magicblock-core/src/link/transactions.rs
- magicblock-accounts-db/src/reset.rs
Comment on lines
213
to
217
| tokio::spawn(async move { | ||
| let result = | ||
| Self::send_crank_batch(rpc_client, &block, tx_counter, &batch).await; | ||
| Self::send_crank_batch(&engine, tx_counter, &batch).await; | ||
| let _ = crank_tx.send((batch, result)); | ||
| }); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
version=$(awk '
$0 == "name = \"tokio\"" { found=1 }
found && /^version = / { gsub(/[" ]/, "", $3); print $3; exit }
' Cargo.lock)
curl -Ls "https://docs.rs/tokio/${version}/tokio/task/struct.JoinSet.html" |
grep -i -C2 'join_all.*panic\|panics'Repository: magicblock-labs/magicblock-validator
Length of output: 4742
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant section of the file with line numbers.
sed -n '180,250p' magicblock-task-scheduler/src/service.rs | cat -n
echo
echo "----"
echo "Search for JoinSet usage in the file."
rg -n "JoinSet|join_all|join_next|spawn\\(" magicblock-task-scheduler/src/service.rsRepository: magicblock-labs/magicblock-validator
Length of output: 4402
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '180,250p' magicblock-task-scheduler/src/service.rs | cat -n
echo
echo "----"
rg -n "JoinSet|join_all|join_next|spawn\\(" magicblock-task-scheduler/src/service.rsRepository: magicblock-labs/magicblock-validator
Length of output: 4364
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '395,440p' magicblock-task-scheduler/src/service.rs | cat -nRepository: magicblock-labs/magicblock-validator
Length of output: 2182
Handle crank task joins individually join_all panics on a child JoinError, so a single worker panic can abort the detached batch task before crank_tx.send(...) runs. Drain with join_next and always emit a batch result; same issue at line 431.
🤖 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 `@magicblock-task-scheduler/src/service.rs` around lines 213 - 217, Update the
crank batch task around send_crank_batch and the corresponding flow near line
431 to drain spawned workers with join_next instead of join_all. Handle each
JoinError individually so one worker panic does not abort processing, and ensure
crank_tx.send(...) is always called with the batch result regardless of
individual worker failures.
bmuddha
force-pushed
the
crate-removal
branch
6 times, most recently
from
July 20, 2026 14:37
824fc98 to
53b12b1
Compare
Collaborator
Author
|
superseeded by #1452 |
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
1 participant
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.
Summary
Breaking Changes
Test Plan
Summary by CodeRabbit
New Features
Breaking Changes
Compatibility