-
Notifications
You must be signed in to change notification settings - Fork 25
fix: reset program cache upon error #696
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This fix introduces a retry logic into SVM program cache replenishing logic, where upon first error, the cache is reset and replenish is re-attempted
📝 WalkthroughWalkthroughThis pull request updates the solana-svm dependency to a newer git revision (86c2cb0f) in both the workspace dependencies and patch entries within Cargo.toml. Additionally, the TransactionExecutor::new initialization in magicblock-processor/src/executor/mod.rs now sets the limit_to_load_programs configuration flag to true as part of the TransactionProcessingConfig setup. Possibly related PRs
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 |
Manual Deploy AvailableYou can trigger a manual deploy of this PR branch to testnet: Alternative: Comment
Comment updated automatically when the PR is synchronized. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
Cargo.toml(2 hunks)magicblock-processor/src/executor/mod.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: Dodecahedr0x
Repo: magicblock-labs/magicblock-validator PR: 639
File: Cargo.toml:58-58
Timestamp: 2025-11-24T14:21:00.996Z
Learning: In the magicblock-validator codebase, magicblock-api/Cargo.toml intentionally uses borsh = "1.5.3" (instead of the workspace version 0.10.4) because it needs to deserialize types from the magic-domain-program external dependency, which requires borsh 1.5.x compatibility. This is an intentional exception for interoperability with the magic domain program.
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 589
File: magicblock-processor/src/scheduler/locks.rs:64-102
Timestamp: 2025-11-13T09:38:43.804Z
Learning: In magicblock-processor's TransactionScheduler (scheduler/mod.rs line 59), the executor count is clamped to MAX_SVM_EXECUTORS (63) at initialization time, and executor IDs are assigned sequentially from 0 to count-1. This architectural guarantee ensures that executor IDs used in the bitmask-based AccountLock (scheduler/locks.rs) will always be within valid bounds for bit shifting operations, making runtime bounds checks unnecessary.
Learnt from: thlorenz
Repo: magicblock-labs/magicblock-validator PR: 587
File: test-manual/Cargo.toml:0-0
Timestamp: 2025-10-26T16:54:39.084Z
Learning: In the magicblock-validator repository, use git branch references (not commit hashes or tags) for the helius-laserstream dependency to allow automatic updates when the branch is pushed to.
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 675
File: Cargo.toml:145-145
Timestamp: 2025-11-24T08:45:11.164Z
Learning: The reqwest dependency version 0.11 is intentionally used in the magicblock-validator project to maintain compatibility with the Solana client dependencies.
📚 Learning: 2025-11-24T14:21:00.996Z
Learnt from: Dodecahedr0x
Repo: magicblock-labs/magicblock-validator PR: 639
File: Cargo.toml:58-58
Timestamp: 2025-11-24T14:21:00.996Z
Learning: In the magicblock-validator codebase, magicblock-api/Cargo.toml intentionally uses borsh = "1.5.3" (instead of the workspace version 0.10.4) because it needs to deserialize types from the magic-domain-program external dependency, which requires borsh 1.5.x compatibility. This is an intentional exception for interoperability with the magic domain program.
Applied to files:
Cargo.toml
📚 Learning: 2025-11-24T08:45:11.164Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 675
File: Cargo.toml:145-145
Timestamp: 2025-11-24T08:45:11.164Z
Learning: The reqwest dependency version 0.11 is intentionally used in the magicblock-validator project to maintain compatibility with the Solana client dependencies.
Applied to files:
Cargo.toml
📚 Learning: 2025-10-26T16:54:39.084Z
Learnt from: thlorenz
Repo: magicblock-labs/magicblock-validator PR: 587
File: test-manual/Cargo.toml:0-0
Timestamp: 2025-10-26T16:54:39.084Z
Learning: In the magicblock-validator repository, use git branch references (not commit hashes or tags) for the helius-laserstream dependency to allow automatic updates when the branch is pushed to.
Applied to files:
Cargo.toml
📚 Learning: 2025-11-13T09:38:43.804Z
Learnt from: bmuddha
Repo: magicblock-labs/magicblock-validator PR: 589
File: magicblock-processor/src/scheduler/locks.rs:64-102
Timestamp: 2025-11-13T09:38:43.804Z
Learning: In magicblock-processor's TransactionScheduler (scheduler/mod.rs line 59), the executor count is clamped to MAX_SVM_EXECUTORS (63) at initialization time, and executor IDs are assigned sequentially from 0 to count-1. This architectural guarantee ensures that executor IDs used in the bitmask-based AccountLock (scheduler/locks.rs) will always be within valid bounds for bit shifting operations, making runtime bounds checks unnecessary.
Applied to files:
magicblock-processor/src/executor/mod.rs
📚 Learning: 2025-11-21T10:22:07.520Z
Learnt from: taco-paco
Repo: magicblock-labs/magicblock-validator PR: 661
File: magicblock-committor-service/src/intent_executor/single_stage_executor.rs:20-28
Timestamp: 2025-11-21T10:22:07.520Z
Learning: In magicblock-committor-service's SingleStageExecutor and TwoStageExecutor (single_stage_executor.rs and two_stage_executor.rs), the fields transaction_strategy, junk, and patched_errors are intentionally public because these executors are designed to be used independently outside of the IntentExecutor scope, and callers need access to these execution reports for cleanup and error handling.
Applied to files:
magicblock-processor/src/executor/mod.rs
🔇 Additional comments (2)
Cargo.toml (2)
222-225: Workspace solana-svm rev bump looks consistent; please confirm new commit/feature compatibilityThe workspace solana-svm now pins to
86c2cb0fand keeps the existingdev-context-only-utilsfeature flag. This matches the pattern of other git-pinned internal Solana crates and is consistent with the patch entry below, so the manifest wiring looks good. Please just double-check that commit86c2cb0findeed contains the newTransactionProcessingConfig::limit_to_load_programsfield and that thedev-context-only-utilsfeature remains unchanged in that revision.
231-233: Patched solana-svm entry correctly aligned with workspace dep; keep these revs in lockstepThe crates-io patch for
solana-svmis updated to the same repo and rev (86c2cb0f), which avoids mixed SVM versions between direct and transitive users. This looks correct; just ensure that future bumps always update both the[workspace.dependencies.solana-svm]and this[patch.crates-io]entry together.
GabrielePicco
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
thlorenz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but most changes are actually inside this commit magicblock-labs/magicblock-svm@86c2cb0 and the related PR has lots of unaddressed coderabbits.
If this is urgent we can merge + deploy for, but please LMK when the other PR is ready for review as well.
This reverts commit ad8e9bb.
* master: fix: don't unbork for same delegation slot as remote slot (#702) feat: use simplified configuration management (#685) Report Intent's patched errors (#667) fix: replace cargo-expand with syn-based verification (#611) fix: make db thread safe (#680) fix: reset program cache upon error (#696) chore: use smaller runners (#695) feat: cranked commits (#656) fix: refresh stuck undelegating accounts that are closed on-chain (#691) Fix: abort truncation if validator exited (#689)
This fix introduces a retry logic into SVM program cache replenishing logic, where upon first error,
the cache is reset and replenish is re-attempted
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.