feat: derive crank signer from task id#1188
Conversation
📝 WalkthroughWalkthroughThis PR makes the crank signer PDA derived from the task authority: it adds authority: Pubkey to MagicBlockInstruction::ExecuteCrank, replaces the fixed CRANK_SIGNER constant with pub fn crank_signer_pda(authority: &Pubkey), updates instruction builders to accept authority, threads authority through entrypoints, process_execute_crank, validate_cranks_instructions, the scheduler/service, and updates tests to derive and use per-authority crank signer PDAs. Assessment against linked issues
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
programs/magicblock/src/schedule_task/process_execute_task.rs (1)
80-86: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winConsider including task_id in error messages for better debugging.
The error messages for crank signer validation don't mention which task_id caused the failure. Including it would help with debugging multi-task scenarios.
💡 Proposed improvement to error message
if crank_signer_pubkey != &crank_signer { ic_msg!( invoke_context, - "ExecuteCrank ERR: crank signer pubkey {} is not the expected Crank signer", - crank_signer_pubkey + "ExecuteCrank ERR: crank signer pubkey {} is not the expected Crank signer for task_id {}", + crank_signer_pubkey, + task_id ); return Err(InstructionError::InvalidSeeds); }🤖 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_task/process_execute_task.rs` around lines 80 - 86, The error log when validating the crank signer (in the block comparing crank_signer_pubkey and crank_signer) omits the task identifier; update the ic_msg! call that logs the mismatch (the invocation using invoke_context and the check returning InstructionError::InvalidSeeds) to include the task_id value so the message prints which task failed validation (use the existing task_id variable in the formatted message) before returning the error.
🤖 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 `@programs/magicblock/src/schedule_task/mod.rs`:
- Around line 22-27: The loop currently recomputes crank_signer_pda(task_id) for
every account; reuse the previously computed crank_signer variable instead. In
the nested loop over instructions and instruction.accounts, replace checks that
call crank_signer_pda(task_id) (e.g.,
account.pubkey.ne(&crank_signer_pda(task_id))) with comparisons against the
existing crank_signer binding so the PDA is derived once per function call
rather than per-account.
---
Outside diff comments:
In `@programs/magicblock/src/schedule_task/process_execute_task.rs`:
- Around line 80-86: The error log when validating the crank signer (in the
block comparing crank_signer_pubkey and crank_signer) omits the task identifier;
update the ic_msg! call that logs the mismatch (the invocation using
invoke_context and the check returning InstructionError::InvalidSeeds) to
include the task_id value so the message prints which task failed validation
(use the existing task_id variable in the formatted message) before returning
the error.
🪄 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: 1dd947cc-7b90-4353-a2b4-d9fa10a0983c
📒 Files selected for processing (10)
magicblock-magic-program-api/src/instruction.rsmagicblock-magic-program-api/src/pda.rsmagicblock-task-scheduler/src/service.rsprograms/magicblock/src/magicblock_processor.rsprograms/magicblock/src/schedule_task/mod.rsprograms/magicblock/src/schedule_task/process_execute_task.rsprograms/magicblock/src/schedule_task/process_schedule_task.rsprograms/magicblock/src/utils/instruction_utils.rstest-integration/test-task-scheduler/tests/test_schedule_magic_cpi_crank.rstest-integration/test-task-scheduler/tests/test_use_crank_signer.rs
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 313-316: The ExecuteCrank enum variant currently uses the shared
authority Pubkey which is not unique per task; change the variant to include a
task-unique seed (e.g., task_id: [u8; N] or TaskPubkey) instead of authority,
update the PDA helper that derives the crank signer PDA to use this task-unique
seed (not authority), and propagate that change through the
scheduler/validation/execution plumbing (validation, scheduling, and execution
functions) so all places that derive or check the crank signer PDA use the new
task-unique identifier (reference ExecuteCrank, authority, the crank signer PDA
helper, and the scheduler/validation/execution functions) to ensure per-task
isolation.
🪄 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: eee08cb6-5553-4eb6-a7e1-4682cdbddf51
📒 Files selected for processing (10)
magicblock-magic-program-api/src/instruction.rsmagicblock-magic-program-api/src/pda.rsmagicblock-task-scheduler/src/service.rsprograms/magicblock/src/magicblock_processor.rsprograms/magicblock/src/schedule_task/mod.rsprograms/magicblock/src/schedule_task/process_execute_task.rsprograms/magicblock/src/schedule_task/process_schedule_task.rsprograms/magicblock/src/utils/instruction_utils.rstest-integration/test-task-scheduler/tests/test_schedule_magic_cpi_crank.rstest-integration/test-task-scheduler/tests/test_use_crank_signer.rs
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-task-scheduler/src/service.rs`:
- Around line 240-242: The call to process_transaction currently uses
task.authority as the PDA seed, which allows multiple tasks from the same
authority to share a crank signer; update process_transaction(...) and the
ExecuteCrank construction to use the task-unique seed (the task ID / the same
unique identifier used in your PDA derivation) instead of task.authority, and
thread that task-unique identifier through any code paths that build
ExecuteCrank or derive the crank signer (including the other sites where
task.authority is passed into ExecuteCrank); ensure the signature generation and
PDA derivation use the same task-unique value so each task gets a distinct crank
signer.
🪄 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: 882172d1-4b39-41d3-8357-b948909948f0
📒 Files selected for processing (1)
magicblock-task-scheduler/src/service.rs
| let sig = self | ||
| .process_transaction(task.authority, task.instructions.clone()) | ||
| .await?; |
There was a problem hiding this comment.
Use a task-unique seed here instead of task.authority.
Line 241 and Line 437 still thread task.authority into ExecuteCrank. A single authority can own multiple task IDs in this service, so those tasks would continue to share the same crank signer PDA. That leaves per-task crank authorization bypassable for tasks under the same authority, which does not satisfy the task-ID-based isolation this PR is intended to enforce. Please thread the unique task identifier used by the PDA derivation through this path instead.
Also applies to: 425-437
🤖 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 240 - 242, The call to
process_transaction currently uses task.authority as the PDA seed, which allows
multiple tasks from the same authority to share a crank signer; update
process_transaction(...) and the ExecuteCrank construction to use the
task-unique seed (the task ID / the same unique identifier used in your PDA
derivation) instead of task.authority, and thread that task-unique identifier
through any code paths that build ExecuteCrank or derive the crank signer
(including the other sites where task.authority is passed into ExecuteCrank);
ensure the signature generation and PDA derivation use the same task-unique
value so each task gets a distinct crank signer.
1 participant
Closes #1187
Summary
Derives the
crank_signerfrom the task ID. Users can restrict the execution of a task to a specified crank.Breaking Changes
Summary by CodeRabbit