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
| /// and for each CPIs into the outbox intent program's `CreateOutboxIntent` | ||
| /// to create the corresponding outbox intent PDA with `status = Accepted`. | ||
| /// This is the second part of scheduling a commit. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Update the documented account layout for the new CPI targets.
The processor now expects five fixed accounts—validator, outbox program, magic context, vault, and ephemeral system program—with PDAs starting at index 5. The nearby public contract still says four fixed accounts and documents PDAs at 4..n, which will produce malformed client instructions.
🤖 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-magic-program-api/src/instruction.rs` around lines 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.
| let transaction_context = &*invoke_context.transaction_context; | ||
| let pda_acc = | ||
| get_instruction_account_with_idx(transaction_context, PDA_IDX)?; | ||
| pda_acc | ||
| .borrow_mut()? | ||
| .data_as_mut_slice() | ||
| .copy_from_slice(&data); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Prevent potential validator crash on slice length mismatch.
In built-in programs, unhandled panics can crash the validator node. The copy_from_slice method will panic if pda_acc's data slice length differs from data.len(). Although create_ephemeral_account_cpi requests the correct length, defensively checking the length prevents any possibility of a crash if the account state behaves unexpectedly.
🛡️ Proposed defensive fix
let transaction_context = &*invoke_context.transaction_context;
let pda_acc =
get_instruction_account_with_idx(transaction_context, PDA_IDX)?;
- pda_acc
- .borrow_mut()?
- .data_as_mut_slice()
- .copy_from_slice(&data);
+
+ let mut pda_borrow = pda_acc.borrow_mut()?;
+ let pda_slice = pda_borrow.data_as_mut_slice();
+ if pda_slice.len() != data.len() {
+ ic_msg!(
+ invoke_context,
+ "CreateOutboxIntent ERR: PDA account data length mismatch"
+ );
+ return Err(InstructionError::InvalidAccountData);
+ }
+ pda_slice.copy_from_slice(&data);📝 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 transaction_context = &*invoke_context.transaction_context; | |
| let pda_acc = | |
| get_instruction_account_with_idx(transaction_context, PDA_IDX)?; | |
| pda_acc | |
| .borrow_mut()? | |
| .data_as_mut_slice() | |
| .copy_from_slice(&data); | |
| let transaction_context = &*invoke_context.transaction_context; | |
| let pda_acc = | |
| get_instruction_account_with_idx(transaction_context, PDA_IDX)?; | |
| let mut pda_borrow = pda_acc.borrow_mut()?; | |
| let pda_slice = pda_borrow.data_as_mut_slice(); | |
| if pda_slice.len() != data.len() { | |
| ic_msg!( | |
| invoke_context, | |
| "CreateOutboxIntent ERR: PDA account data length mismatch" | |
| ); | |
| return Err(InstructionError::InvalidAccountData); | |
| } | |
| pda_slice.copy_from_slice(&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/outbox_intent/process_create_outbox_intent.rs` around
lines 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.
…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
| /// Closes the outbox intent PDA on successful execution of the intent. | ||
| /// | ||
| /// # Account references | ||
| /// - **0.** `[WRITE, SIGNER]` Validator Authority (receives rent refund from close) | ||
| /// - **1.** `[]` Magic Program | ||
| /// - **2.** `[WRITE]` Ephemeral Vault | ||
| /// - **3.** `[WRITE]` Outbox intent PDA to close, seeds: `["outbox-intent", intent_id.to_le_bytes()]` | ||
| CloseOutboxIntent(u64), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Correct the CloseOutboxIntent account contract.
Line 435 documents Magic Program at account index 1. process_close_outbox_intent::validate and InstructionUtils::close_outbox_intent_instruction require EPHEMERAL_SYSTEM_PROGRAM_ID there. Clients that follow this documentation will receive IncorrectProgramId.
Proposed fix
- /// - **1.** `[]` Magic Program
+ /// - **1.** `[]` Ephemeral System Program (CPI target)📝 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.
| /// Closes the outbox intent PDA on successful execution of the intent. | |
| /// | |
| /// # Account references | |
| /// - **0.** `[WRITE, SIGNER]` Validator Authority (receives rent refund from close) | |
| /// - **1.** `[]` Magic Program | |
| /// - **2.** `[WRITE]` Ephemeral Vault | |
| /// - **3.** `[WRITE]` Outbox intent PDA to close, seeds: `["outbox-intent", intent_id.to_le_bytes()]` | |
| CloseOutboxIntent(u64), | |
| /// Closes the outbox intent PDA on successful execution of the intent. | |
| /// | |
| /// # Account references | |
| /// - **0.** `[WRITE, SIGNER]` Validator Authority (receives rent refund from close) | |
| /// - **1.** `[]` Ephemeral System Program (CPI target) | |
| /// - **2.** `[WRITE]` Ephemeral Vault | |
| /// - **3.** `[WRITE]` Outbox intent PDA to close, seeds: `["outbox-intent", intent_id.to_le_bytes()]` | |
| CloseOutboxIntent(u64), |
🤖 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-magic-program-api/src/instruction.rs` around lines 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.
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.
2 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