magicblock-program modification to migrate from ScheduledCommit to ScheduledAction #370

taco-paco · 2025-05-20T09:15:47Z

This PR replaces ScheduledCommit with ScheduledAction in magicblock-program but not in validator. Replacement in validator will occur in follow up PR to split effort into smaller pieces. Right now ScheduledAction is converted to ScheduledCommit in validator, so it uses an old flow

With new ephemeral-sdk user can create something like:

let builder = MagicInstructionBuilder {
    payer: ctx.accounts.payer.to_account_info(),
    magic_context: ctx.accounts.magic_context.to_account_info(),
    magic_program: ctx.accounts.magic_program.to_account_info(),
    magic_action: MagicAction::Commit(
        CommitType::WithHandler {
            commited_accounts: vec![ctx.accounts.pda1.to_account_info(), ctx.accounts.pda2.to_account_info()]
            call_handlers: vec![CallHandler {
                accounts: vec![ctx.accounts.actor_pda.to_account_info(), ctx.accounts.destination_to_account_info(), system_program]
                args: HandlerArgs {
                    data,
                    escrow_index: 1
                },
                destination_program: ctx.accounts.destination_program.to_account_info()
            }]
        }
    )
}

builder.build_and_invoke()?

Now our SDK will create MagicActionArgs from this information, so for example CallHandlerArgs will look like:

#[derive(Clone, Serialize, Deserialize, Debug, PartialEq, Eq)]
pub struct CallHandlerArgs {
    pub args: HandlerArgs,
    pub destination_program: u8, // index of the account
    pub accounts: Vec<u8>,       // indices of account
}

And once we get this in magicblock-program we are able to reconstruct data from indices, see MagicAction::try_from_args.

greptile-apps

_{23 file(s) reviewed, 33 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

magicblock-mutator/src/transactions.rs

magicblock-accounts/src/remote_scheduled_commits_processor.rs

greptile-apps · 2025-05-20T09:16:41Z

magicblock-accounts/src/remote_scheduled_commits_processor.rs

    fn scheduled_commits_len(&self) -> usize {
-        self.transaction_scheduler.scheduled_commits_len()
+        self.transaction_scheduler.scheduled_actions_len()
    }

    fn clear_scheduled_commits(&self) {
-        self.transaction_scheduler.clear_scheduled_commits();
+        self.transaction_scheduler.clear_scheduled_actions();
    }


style: Method names have been updated but the trait interface still uses 'commits' terminology. This inconsistency could cause confusion.

programs/magicblock/src/errors.rs

programs/magicblock/src/utils/instruction_utils.rs

bmuddha

Looks good overall, left a few questions and nits

bmuddha · 2025-05-21T11:37:10Z

programs/magicblock/src/magic_context.rs

-    pub commit_sent_transaction: Transaction,
-    pub request_undelegation: bool,
-}
+// Q: can user initiate actions on arbitrary accounts?


Please rephrase the whole QA, currently it's not clear and incoherent.

bmuddha · 2025-05-21T11:40:03Z

programs/magicblock/src/magic_context.rs

+    pub request_undelegation: bool,
+}
+
+impl From<ScheduledCommit> for ScheduledAction {


Does this particular transition from commit to action (which is handled by delegation program) break anything. I.e. once the new delegation program is deployed, old validator code stops working until upgraded?

Or does the DLP also maintain full backward compatibility, sorry for silly questions, I haven't seen the relevant changes to DLP.

bmuddha · 2025-05-21T11:46:25Z

programs/magicblock/src/magic_schedule_action.rs

+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+pub enum MagicAction {
+    /// Actions without commitment or undelegation
+    CallHandler(Vec<CallHandler>),


Are there any checks on how much data we can cram into this? As far as I understand this is further encoded into solana transaction which does have size limitation.

bmuddha · 2025-05-21T11:51:47Z

programs/magicblock/src/mutate_accounts/process_mutate_accounts.rs

@@ -309,7 +307,9 @@ mod tests {
            data: Some(vec![1, 2, 3, 4, 5]),
            rent_epoch: Some(88),
        };
-        let ix = modify_accounts_instruction(vec![modification.clone()]);
+        let ix = InstructionUtils::modify_accounts_instruction(vec![


nit: but please turn this InstructionUtils to mod, it's so unrusty to use empty types for free functions, I know it's a matter of taste and yada-yada, but this is just a rust convention.

bmuddha · 2025-05-21T12:00:05Z

programs/magicblock/src/utils/instruction_utils.rs

+    Pubkey,
+};
+
+pub struct InstructionUtils;


please turn this into module

GabrielePicco

Overall LGTM! Left a question

GabrielePicco · 2025-05-26T18:10:45Z

programs/magicblock/src/magic_schedule_action.rs

+
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+pub struct Handler {
+    pub escrow_index: u8,


Q: should the handler rely on a specific hardcoded index (e.g. 255) always used for the signing/handler or are there use-cases where multiple indexes are useful?
We could otherwise slightly simplify the code

…rted + some conversions

…it. Calling schedule action will throw an error for now

taco-paco requested review from thlorenz, GabrielePicco, bmuddha and Dodecahedr0x May 20, 2025 09:15

greptile-apps bot reviewed May 20, 2025

View reviewed changes

bmuddha approved these changes May 21, 2025

View reviewed changes

GabrielePicco approved these changes May 26, 2025

View reviewed changes

taco-paco force-pushed the feat/base-layer-ix/magicblock-program branch from 83e7ba8 to 882edcb Compare June 5, 2025 04:04

taco-paco added 11 commits June 5, 2025 13:24

feat: added construction of ScheduleAction

b7b6e4b

refactor: changed file structure

0cfda9c

feat: replace ScheduledCommit -> SceduledAction in MagicContext

f9f0307

refactor: namings + some logging

a2a372b

refactor: separate file for accepted commits

c0cf401

feat: error on process_schedule_action call until feature fully suppo…

9775d7d

…rted + some conversions

feat: until supported converting ScheduledAction to old ScheduledComm…

597435e

…it. Calling schedule action will throw an error for now

fix: tests + bug

a955a12

refactor: renamings

39c4fd4

fix: bug

1abe69c

fix: rebase

0073370

taco-paco force-pushed the feat/base-layer-ix/magicblock-program branch from 882edcb to 0073370 Compare June 5, 2025 04:27

magicblock-program modification to migrate from ScheduledCommit to ScheduledAction #370

Are you sure you want to change the base?

magicblock-program modification to migrate from ScheduledCommit to ScheduledAction #370

Uh oh!

Conversation

taco-paco commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

greptile-apps bot May 20, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bmuddha left a comment

Choose a reason for hiding this comment

Uh oh!

bmuddha May 21, 2025

Choose a reason for hiding this comment

Uh oh!

bmuddha May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bmuddha May 21, 2025

Choose a reason for hiding this comment

Uh oh!

bmuddha May 21, 2025

Choose a reason for hiding this comment

Uh oh!

bmuddha May 21, 2025

Choose a reason for hiding this comment

Uh oh!

bmuddha May 21, 2025

Choose a reason for hiding this comment

Uh oh!

GabrielePicco left a comment

Choose a reason for hiding this comment

Uh oh!

GabrielePicco May 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

taco-paco commented May 20, 2025 •

edited

Loading

bmuddha May 21, 2025 •

edited

Loading