refactor: outbox intent program - #1430
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe change introduces a dedicated outbox intent program with its own program ID, instruction enum, builtin entrypoint, PDA derivation, account ownership, and CPI flows. Scheduled commit, acceptance, creation, closure, and execution-stage operations are routed through the new program and ephemeral system program. Module exports, validators, readers, test utilities, unit tests, integration result tracking, and Solana dependency revisions are updated accordingly. Suggested reviewers: Mergeability Score: ⚪ Minimal · up to This refactor has no supplied evidence of a concrete behavior, dependency, deployment, or runtime problem; no actionable merge-blocking risk remains after normal checks and review. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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-magic-program-api/src/instruction.rs`:
- Around line 62-64: Update the public account-layout documentation near the
scheduling commit instructions to describe five fixed accounts in order:
validator, outbox program, magic context, vault, and ephemeral system program.
Change the documented PDA starting index from 4 to 5 so it matches the
processor’s expected layout.
In `@programs/magicblock/src/outbox_intent/process_create_outbox_intent.rs`:
- Around line 44-50: In the create-outbox-intent flow, validate that the PDA
account data length matches data.len() before calling copy_from_slice. Return
the appropriate instruction error on mismatch, and retain the existing copy
behavior only when lengths match to prevent a validator panic.
🪄 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: aedfbb5c-cd71-4e59-b8a1-97c0a4bdee3e
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.locktest-integration/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (24)
Cargo.tomlmagicblock-accounts-db/src/reset.rsmagicblock-chainlink/src/chainlink/blacklisted_accounts.rsmagicblock-committor-service/src/outbox/outbox_intent_bundles_reader.rsmagicblock-core/src/intent/outbox.rsmagicblock-magic-program-api/src/instruction.rsmagicblock-magic-program-api/src/lib.rsmagicblock-processor/src/builtins.rsprograms/magicblock/src/intent_bundles/mod.rsprograms/magicblock/src/intent_bundles/process_accept_scheduled_commits.rsprograms/magicblock/src/intent_bundles/schedule/mod.rsprograms/magicblock/src/intent_bundles/schedule/process_schedule_commit_tests.rsprograms/magicblock/src/lib.rsprograms/magicblock/src/magicblock_processor.rsprograms/magicblock/src/outbox_intent/mod.rsprograms/magicblock/src/outbox_intent/outbox_intent_bundles.rsprograms/magicblock/src/outbox_intent/process_create_outbox_intent.rsprograms/magicblock/src/outbox_intent/process_scheduled_commit_sent.rsprograms/magicblock/src/outbox_intent/process_set_intent_execution_stage.rsprograms/magicblock/src/test_utils/mod.rsprograms/magicblock/src/utils/instruction_utils.rstest-integration/Cargo.tomltest-integration/test-schedule-intent/tests/test_schedule_intents.rstest-integration/test-tools/src/scheduled_commits.rs
…program # Conflicts: # Cargo.toml # test-integration/Cargo.toml
…program # Conflicts: # Cargo.lock # Cargo.toml # magicblock-magic-program-api/src/instruction.rs # programs/magicblock/src/intent_bundles/schedule/mod.rs # programs/magicblock/src/magicblock_processor.rs # programs/magicblock/src/outbox_intent/mod.rs # programs/magicblock/src/outbox_intent/process_close_outbox_intent.rs # programs/magicblock/src/outbox_intent/process_scheduled_commit_sent.rs # test-integration/Cargo.lock # test-integration/Cargo.toml
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 `@magicblock-magic-program-api/src/instruction.rs`:
- Around line 431-438: Update the account documentation for CloseOutboxIntent so
account index 1 is identified as the Ephemeral System Program and uses
EPHEMERAL_SYSTEM_PROGRAM_ID, matching process_close_outbox_intent::validate and
InstructionUtils::close_outbox_intent_instruction; leave the remaining account
entries unchanged.
🪄 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 Plus
Run ID: ec5627af-108b-482f-b0e9-5c99d8fb4f5b
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.locktest-integration/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
Cargo.tomlmagicblock-magic-program-api/src/instruction.rsprograms/magicblock/src/magicblock_processor.rsprograms/magicblock/src/outbox_intent/mod.rsprograms/magicblock/src/outbox_intent/outbox_intent_bundles.rsprograms/magicblock/src/outbox_intent/process_close_outbox_intent.rsprograms/magicblock/src/outbox_intent/process_scheduled_commit_sent.rsprograms/magicblock/src/utils/instruction_utils.rstest-integration/Cargo.toml
redsuite: PR vs feat/mimd-0025/mainSingle-run diff on shared runners — indicative only; statistical verdicts come from Bencher thresholds. |
|
Bencher Report| Project | magicblock-labs |
| Branch | feat/mimd-0025/outbox-intent-program |
| Testbed | blacksmith-8vcpu-ubuntu-2404 |
⚠️ WARNING: Truncated view!The full continuous benchmarking report exceeds the maximum length allowed on this platform.
🐰 View full continuous benchmarking report in Bencher
⚠️ WARNING: No Threshold found!Without a Threshold, no Alerts will ever be generated.
| @@ -73,17 +74,8 @@ pub enum MagicBlockInstruction { | |||
| /// - **4..n** `[WRITE]` Outbox intent PDAs, one per accepted intent, seeds: `["outbox-intent", intent_id.to_le_bytes()]` | |||
| AcceptScheduleCommits, | |||
There was a problem hiding this comment.
I think AcceptScheduleCommits should also be moved to Outbox program.
It's at the boundary. So I guess it can be in either program, but since it's practically an internal ix, we can move it too. That way eventually the magic-program will have public facing instructions.
There was a problem hiding this comment.
The problem is that you can't modify MagicContext from outbox-program without CPIing into magic-program. For me this make outbox have a control of magic program which I think should be the other way around, hence the current implementation
|
|
||
| let outbox_candidates_iter = self.accounts_db.get_program_accounts( | ||
| &magicblock_program::ID, | ||
| &magicblock_program::OUTBOX_INTENT_PROGRAM_ID, |
There was a problem hiding this comment.
The path magicblock_program::OUTBOX_INTENT_PROGRAM_ID looks weird. That feels like outbox program is a child program of the magic program.
Why dont we physically separate outbox program into its own crate? It'll be better in the long run, with clean boundaries. The paths will also look sensible.
There was a problem hiding this comment.
I agree, I created a similiar #1431 for that some time ago. But I considered refactoring out of scope this PR. Especially when reviewers ask for a more atomic changes.
This can be done in another PR.
…program # Conflicts: # Cargo.lock # Cargo.toml # magicblock-accounts-db/src/reset.rs # magicblock-chainlink/src/chainlink/blacklisted_accounts.rs # magicblock-committor-service/src/outbox/outbox_intent_bundles_reader.rs # magicblock-processor/src/builtins.rs # programs/magicblock/src/intent_bundles/process_accept_scheduled_commits.rs # programs/magicblock/src/intent_bundles/schedule/mod.rs # programs/magicblock/src/intent_bundles/schedule/process_schedule_commit_tests.rs # programs/magicblock/src/lib.rs # programs/magicblock/src/magicblock_processor.rs # programs/magicblock/src/outbox_intent/process_close_outbox_intent.rs # programs/magicblock/src/outbox_intent/process_scheduled_commit_sent.rs # programs/magicblock/src/test_utils/mod.rs # programs/magicblock/src/utils/instruction_utils.rs # test-integration/Cargo.lock # test-integration/Cargo.toml
…program # Conflicts: # programs/magicblock/src/outbox_intent/process_close_outbox_intent.rs # programs/magicblock/src/outbox_intent/process_scheduled_commit_sent.rs
3 participants
Summary
Extract Outbox intent logic into separate program. As new patching functionality and maybe more will come this will allow us to keep magic-program itself smaller from outbox related things
Breaking Changes
Test Plan
Summary by CodeRabbit
New Features
Bug Fixes
Tests