From aad73d7b995358f239d40566da20d67f74da5b74 Mon Sep 17 00:00:00 2001 From: Sarfaraz Nawaz Date: Tue, 10 Nov 2026 06:00:34 +0530 Subject: [PATCH] fix: capture post-fee balances when scheduling commits Scheduling previously copied committed accounts before deducting the commit and action fees. When the payer was also committed, the queued balance and resulting settlement instruction still included the fee. The bundled path also looked up the fee vault after changing account ownership for undelegation, when the payer was no longer delegated. Prepare and validate the accounts and actions, reserve commit IDs once, and calculate the complete fee before constructing settlement snapshots. Share the existing fee calculations through helpers accepting commit IDs and action compute units, retaining the sponsored allowance and rounding within each action group. Charge the payer before copying account state in ScheduleCommit, ScheduleCommitAndUndelegate, ScheduleIntentBundle, and ScheduleBaseIntent. Capture both payer debits and fee-vault credits before marking accounts as undelegating, so snapshots retain the original owner. Preserve account permissions, duplicate handling, ATA remapping, and transaction limits. Extend existing tests for payer/account aliasing, separate payers, fee-vault snapshots, free commits, all commit variants, mixed action fees, and undelegation. Extend the Engine test to verify fee transfers, reservations, ownership, and queued intents roll back on insufficient funds or a later instruction failure. Validation: cargo fmt --check; cargo check --tests and targeted Clippy with warnings denied for magicblock-program and magicblock-core; scheduling regressions, Engine rollback coverage, and core fee tests. Used cargo test because nextest is unavailable. Fixes #1908. --- core/src/intent/mod.rs | 21 +- .../magic_scheduled_base_intent.rs | 179 ++++++++--- .../schedule/process_schedule_commit.rs | 73 +++-- .../schedule/process_schedule_commit_tests.rs | 292 +++++++++++++++--- .../process_schedule_intent_bundle.rs | 60 ++-- 5 files changed, 476 insertions(+), 149 deletions(-) diff --git a/core/src/intent/mod.rs b/core/src/intent/mod.rs index 78d7cddee..59fb04b5e 100644 --- a/core/src/intent/mod.rs +++ b/core/src/intent/mod.rs @@ -518,18 +518,29 @@ impl UndelegateType { } pub fn calculate_commit_fee(accounts: &[CommittedAccount]) -> Result { - accounts.iter().try_fold(0u64, |fee, account| { - if account.commit_id == 0 { + calculate_commit_fee_for_ids(accounts.iter().map(|account| account.commit_id)) +} + +/// Calculate the fee before copying the accounts into settlement snapshots. +pub fn calculate_commit_fee_for_ids( + commit_ids: impl IntoIterator, +) -> Result { + commit_ids.into_iter().try_fold(0u64, |fee, commit_id| { + if commit_id == 0 { return Err(InstructionError::Custom(MISSING_COMMIT_NONCE_ERR)); } - Ok(fee + if account.commit_id > ACTUAL_COMMIT_LIMIT { COMMIT_FEE_LAMPORTS } else { 0 }) + Ok(fee + if commit_id > ACTUAL_COMMIT_LIMIT { COMMIT_FEE_LAMPORTS } else { 0 }) }) } fn calculate_actions_fee(actions: &[BaseAction]) -> u64 { + calculate_actions_fee_for_units(actions.iter().map(|action| action.compute_units)) +} + +pub fn calculate_actions_fee_for_units(compute_units: impl IntoIterator) -> u64 { const MICRO_LAMPORTS_PER_LAMPORT: u64 = 1_000_000; - let micro_lamports = actions.iter().fold(0u64, |acc, action| { - acc.saturating_add(action.compute_units as u64 * COMPUTE_UNIT_PRICE_MICRO_LAMPORTS) + let micro_lamports = compute_units.into_iter().fold(0u64, |acc, units| { + acc.saturating_add(units as u64 * COMPUTE_UNIT_PRICE_MICRO_LAMPORTS) }); micro_lamports.div_ceil(MICRO_LAMPORTS_PER_LAMPORT) } diff --git a/programs/magic-program/src/intent_bundles/magic_scheduled_base_intent.rs b/programs/magic-program/src/intent_bundles/magic_scheduled_base_intent.rs index c1f4a9553..348c19549 100644 --- a/programs/magic-program/src/intent_bundles/magic_scheduled_base_intent.rs +++ b/programs/magic-program/src/intent_bundles/magic_scheduled_base_intent.rs @@ -7,12 +7,15 @@ use magic_program_api::args::{ pub use mbv_core::intent::{ ACTUAL_COMMIT_LIMIT, BaseAction, COMMIT_FEE_LAMPORTS, COMPUTE_UNIT_PRICE_MICRO_LAMPORTS, CommitAndUndelegate, CommitType, MagicBaseIntent, MagicIntentBundle, ProgramArgs, - UndelegateType, calculate_commit_fee, + UndelegateType, calculate_actions_fee_for_units, calculate_commit_fee, + calculate_commit_fee_for_ids, }; use mbv_core::{ Slot, intent::types::CommittedAccount, - token_programs::{EATA_PROGRAM_ID, TOKEN_2022_PROGRAM_ID, TOKEN_PROGRAM_ID}, + token_programs::{ + EATA_PROGRAM_ID, TOKEN_2022_PROGRAM_ID, TOKEN_PROGRAM_ID, try_remap_ata_to_eata, + }, }; use serde::{Deserialize, Serialize}; use solana_account::{AccountMode, ReadableAccount}; @@ -79,6 +82,91 @@ impl<'a, 'ic, 'ix_data> ConstructionContext<'a, 'ic, 'ix_data> { type CommitAccountRef<'a, 'ix_data> = (Pubkey, InstructionAccount<'a, 'ix_data>); +/// Validate and reserve IDs without copying account state. The caller charges +/// this fee before constructing the bundle's settlement snapshots. +pub(crate) fn prepare_intent_bundle( + args: &MagicIntentBundleArgs, + context: &ConstructionContext<'_, '_, '_>, +) -> Result { + validate_magic_intent_bundle_args(args, context)?; + let mut accounts_to_commit = Vec::new(); + let mut seen = HashSet::new(); + let mut fee = 0; + + for (commit, undelegate) in [ + args.commit.as_ref().map(|commit| (commit, false)), + args.commit_and_undelegate.as_ref().map(|intent| (&intent.commit_type, true)), + args.commit_finalize.as_ref().map(|commit| (commit, false)), + args.commit_finalize_and_undelegate + .as_ref() + .map(|intent| (&intent.commit_type, true)), + ] + .into_iter() + .flatten() + { + let indices = commit.committed_accounts_indices(); + if undelegate { + validate_commit_and_undelegate_accounts(indices, context)?; + } + let accounts = extract_commit_accounts(indices, context.transaction_context())?; + validate_commit_type_accounts(&accounts, context)?; + for (pubkey, account) in &accounts { + let account = account.borrow()?; + let restored_pubkey = + try_remap_ata_to_eata(pubkey, &account).map_or(*pubkey, |(pubkey, _)| pubkey); + if !seen.insert(restored_pubkey) { + ic_msg!( + context.invoke_context, + "ScheduleCommit ERR: duplicate committed account pubkey across bundle: {}", + restored_pubkey + ); + return Err(InstructionError::InvalidInstructionData); + } + } + accounts_to_commit.extend(accounts); + if let CommitTypeArgs::WithBaseActions { base_actions, .. } = commit { + fee += validate_actions_and_calculate_fee(base_actions, context)?; + } + } + for intent in [ + args.commit_and_undelegate.as_ref(), + args.commit_finalize_and_undelegate.as_ref(), + ] + .into_iter() + .flatten() + { + if let UndelegateTypeArgs::WithBaseActions { base_actions } = &intent.undelegate_type { + fee += validate_actions_and_calculate_fee(base_actions, context)?; + } + } + fee += validate_actions_and_calculate_fee(&args.standalone_actions, context)?; + if accounts_to_commit.is_empty() && args.standalone_actions.is_empty() { + ic_msg!( + context.invoke_context, + "ScheduleCommit ERR: intent bundle must not be empty." + ); + return Err(InstructionError::InvalidInstructionData); + } + + let commit_ids = accounts_to_commit + .iter() + .map(|(_, account)| reserve_commit_id(account)) + .collect::, _>>()?; + Ok(fee + calculate_commit_fee_for_ids(commit_ids)?) +} + +fn validate_actions_and_calculate_fee( + actions: &[BaseActionArgs], + context: &ConstructionContext<'_, '_, '_>, +) -> Result { + for action in actions { + validate_base_action_args(action, context)?; + } + Ok(calculate_actions_fee_for_units( + actions.iter().map(|action| action.compute_units), + )) +} + /// Scheduled action to be executed on base layer #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, SchemaRead, SchemaWrite)] pub struct ScheduledIntentBundle { @@ -93,6 +181,8 @@ pub struct ScheduledIntentBundle { } impl ScheduledIntentBundle { + /// Capture settlement state after `prepare_intent_bundle` reserves the IDs + /// and the scheduling handler charges the fee, before changing ownership. pub fn try_new( args: MagicIntentBundleArgs, intent_id: u64, @@ -427,43 +517,7 @@ impl TryFromArgs for BaseAction { args: BaseActionArgs, context: &mut ConstructionContext<'_, '_, '_>, ) -> Result { - // Since action on Base layer performed on behalf of some escrow - // We need to ensure that action was authorized by legit owner - let authority_pubkey = *get_instruction_pubkey_with_idx( - context.transaction_context(), - args.escrow_authority as u16, - )?; - if !context.signers.contains(&authority_pubkey) { - ic_msg!( - context.invoke_context, - &format!( - "BaseAction: authority pubkey must sign transaction: {}", - authority_pubkey - ) - ); - - return Err(InstructionError::MissingRequiredSignature); - } - - let Some(parent_program_id) = context.parent_program_id else { - ic_msg!( - context.invoke_context, - "BaseAction: actions can only be scheduled via CPI", - ); - return Err(InstructionError::UnsupportedProgramId); - }; - - let source_program = if context.secure { - Some(parent_program_id) - } else if args.destination_program == parent_program_id { - None - } else { - ic_msg!( - context.invoke_context, - "BaseAction: v1 can act only within same program", - ); - return Err(InstructionError::UnsupportedProgramId); - }; + let (authority_pubkey, source_program) = validate_base_action_args(&args, context)?; Ok(BaseAction { id: context.next_action_id(), @@ -478,6 +532,51 @@ impl TryFromArgs for BaseAction { } } +fn validate_base_action_args( + args: &BaseActionArgs, + context: &ConstructionContext<'_, '_, '_>, +) -> Result<(Pubkey, Option), InstructionError> { + // Since action on Base layer performed on behalf of some escrow + // We need to ensure that action was authorized by legit owner + let authority_pubkey = *get_instruction_pubkey_with_idx( + context.transaction_context(), + args.escrow_authority as u16, + )?; + if !context.signers.contains(&authority_pubkey) { + ic_msg!( + context.invoke_context, + &format!( + "BaseAction: authority pubkey must sign transaction: {}", + authority_pubkey + ) + ); + + return Err(InstructionError::MissingRequiredSignature); + } + + let Some(parent_program_id) = context.parent_program_id else { + ic_msg!( + context.invoke_context, + "BaseAction: actions can only be scheduled via CPI", + ); + return Err(InstructionError::UnsupportedProgramId); + }; + + let source_program = if context.secure { + Some(parent_program_id) + } else if args.destination_program == parent_program_id { + None + } else { + ic_msg!( + context.invoke_context, + "BaseAction: v1 can act only within same program", + ); + return Err(InstructionError::UnsupportedProgramId); + }; + + Ok((authority_pubkey, source_program)) +} + fn validate_commit_type_accounts( accounts: &[CommitAccountRef<'_, '_>], context: &ConstructionContext<'_, '_, '_>, @@ -560,7 +659,6 @@ impl TryFromArgs for CommitType { let committed_accounts = committed_accounts_ref .into_iter() .map(|(pubkey, account)| { - reserve_commit_id(&account)?; let account = account.borrow()?; Ok(CommittedAccount::from_account_shared(pubkey, &account)) }) @@ -576,7 +674,6 @@ impl TryFromArgs for CommitType { let committed_accounts = committed_accounts_ref .into_iter() .map(|(pubkey, account)| { - reserve_commit_id(&account)?; let account = account.borrow()?; Ok(CommittedAccount::from_account_shared(pubkey, &account)) }) diff --git a/programs/magic-program/src/intent_bundles/schedule/process_schedule_commit.rs b/programs/magic-program/src/intent_bundles/schedule/process_schedule_commit.rs index a4500cf2e..fc1b1bc45 100644 --- a/programs/magic-program/src/intent_bundles/schedule/process_schedule_commit.rs +++ b/programs/magic-program/src/intent_bundles/schedule/process_schedule_commit.rs @@ -1,9 +1,10 @@ use std::collections::HashSet; use mbv_core::intent::{ - CommitAndUndelegate, CommitType, MagicBaseIntent, UndelegateType, calculate_commit_fee, + CommitAndUndelegate, CommitType, MagicBaseIntent, UndelegateType, calculate_commit_fee_for_ids, outbox::outbox_intent_pda, types::CommittedAccount, }; +use mbv_core::token_programs::try_remap_ata_to_eata; use solana_account::{AccountMode, ReadableAccount, WritableAccount}; use solana_account_info::MAX_PERMITTED_DATA_INCREASE; use solana_instruction::error::InstructionError; @@ -89,7 +90,7 @@ pub(crate) fn process_schedule_commit( // NOTE: we don't require PDAs to be signers as in our case verifying that the // program owning the PDAs invoked us directly via CPI is sufficient // Thus we can be `invoke`d unsigned and no seeds need to be provided - let mut committed_accounts: Vec = Vec::new(); + let mut accounts_to_commit = Vec::new(); let mut seen_committed_pubkeys: HashSet = HashSet::new(); for idx in committees_start..ix_accs_len { let acc_pubkey = get_instruction_pubkey_with_idx(transaction_context, idx as u16)?; @@ -149,55 +150,48 @@ pub(crate) fn process_schedule_commit( )?; let account = acc.borrow()?; - let mut committed = CommittedAccount::from_account_shared(*acc_pubkey, &account); + let committed_pubkey = try_remap_ata_to_eata(acc_pubkey, &account) + .map_or(*acc_pubkey, |(pubkey, _)| pubkey); drop(account); - if &committed.pubkey != acc_pubkey { + if &committed_pubkey != acc_pubkey { ic_msg!( invoke_context, "ScheduleCommit: remapping ATA {} -> eATA {}", acc_pubkey, - committed.pubkey + committed_pubkey ); } // Backwards-compat: merge duplicate accounts (by final restored pubkey). // Keep the first occurrence and ignore subsequent duplicates. - if !(seen_committed_pubkeys.insert(committed.pubkey)) { + if !seen_committed_pubkeys.insert(committed_pubkey) { continue; } - committed.commit_id = reserve_commit_id(&acc)?; - committed_accounts.push(committed); + accounts_to_commit.push((*acc_pubkey, acc)); } + } - if opts.request_undelegation { - // If the account is scheduled to be undelegated then we need to lock it - // immediately in order to prevent the following actions: - // - writes to the account - // - scheduling further commits for this account - // - // Setting the owner will prevent both, since in both cases the _actual_ - // owner program needs to sign for the account which is not possible at - // that point - // NOTE: this owner change only takes effect if the transaction which - // includes this instruction succeeds. - // - // We also set the undelegating flag on the account in order to detect - // undelegations for which we miss updates - mark_account_as_undelegated(invoke_context, &acc)?; - ic_msg!( - invoke_context, - "ScheduleCommit: Marking account {} as undelegating", - acc_pubkey - ); - } + let commit_ids = accounts_to_commit + .iter() + .map(|(_, account)| reserve_commit_id(account)) + .collect::, _>>()?; + if let Some(ref fee_vault) = magic_fee_vault { + let fee = calculate_commit_fee_for_ids(commit_ids)?; + charge_delegated_payer(&payer_account, fee_vault, fee)?; } - if let Some(fee_vault) = magic_fee_vault { - let fee = calculate_commit_fee(&committed_accounts)?; - charge_delegated_payer(&payer_account, &fee_vault, fee)?; - } else if !opts.request_undelegation { + // The payer or fee vault may also be committed. Copy their post-fee state, + // before undelegation changes their owner and account mode. + let committed_accounts = accounts_to_commit + .iter() + .map(|(pubkey, account)| { + let account = account.borrow()?; + Ok(CommittedAccount::from_account_shared(*pubkey, &account)) + }) + .collect::, InstructionError>>()?; + if magic_fee_vault.is_none() && !opts.request_undelegation { // We validate commit nonces only for plain commits. // If accounts are undelegated we don't want to fail. check_commit_limits(&committed_accounts, invoke_context)?; @@ -240,6 +234,19 @@ pub(crate) fn process_schedule_commit( ); })?; + if opts.request_undelegation { + for (pubkey, account) in &accounts_to_commit { + // Lock the local account only after capturing its original owner. + // The transaction rolls this back if a later instruction fails. + mark_account_as_undelegated(invoke_context, account)?; + ic_msg!( + invoke_context, + "ScheduleCommit: Marking account {} as undelegating", + pubkey + ); + } + } + let scheduled_base_intent = ScheduledIntentBundle { intent_id, slot: clock.slot, diff --git a/programs/magic-program/src/intent_bundles/schedule/process_schedule_commit_tests.rs b/programs/magic-program/src/intent_bundles/schedule/process_schedule_commit_tests.rs index 31fc5be5f..aab9437be 100644 --- a/programs/magic-program/src/intent_bundles/schedule/process_schedule_commit_tests.rs +++ b/programs/magic-program/src/intent_bundles/schedule/process_schedule_commit_tests.rs @@ -204,17 +204,53 @@ mod callback_source_tests { #[test] #[serial_test::serial] fn test_callback_destination_bound_to_recorded_source() { + use magic_program_api::args::{ + CommitAndUndelegateArgs, CommitTypeArgs, UndelegateTypeArgs, + }; + let source = Pubkey::new_unique(); let other = Pubkey::new_unique(); for destination in [source, other] { let payer = Keypair::new(); let (mut accounts, mut transaction_accounts, fee_vault) = prepare_schedule_accounts(&payer, true, false, Some(true)); - let schedule = schedule_action_only_bundle_instruction( - &payer.pubkey(), - Pubkey::new_unique(), - other, - fee_vault, + let payer_account = accounts.remove(&payer.pubkey()).unwrap(); + accounts.insert( + payer.pubkey(), + AccountBuilder::from(payer_account) + .lamports(3_000_000) + .commit_seq(ACTUAL_COMMIT_LIMIT as u32) + .build(), + ); + let committee = Pubkey::new_unique(); + accounts.insert( + committee, + AccountBuilder::from(AccountSharedData::new(1_000_000, 0, &source)) + .mode(AccountMode::Delegated) + .build(), + ); + let mut args = action_only_bundle_args(Pubkey::new_unique(), other); + let mut small_action = args.standalone_actions[0].clone(); + small_action.compute_units = 1; + args.commit_finalize = Some(CommitTypeArgs::WithBaseActions { + committed_accounts: vec![0], + base_actions: vec![small_action.clone(), small_action.clone()], + }); + args.commit_and_undelegate = Some(CommitAndUndelegateArgs { + commit_type: CommitTypeArgs::Standalone(vec![3]), + undelegate_type: UndelegateTypeArgs::WithBaseActions { + base_actions: vec![small_action], + }, + }); + let schedule = Instruction::new_with_wincode( + crate::id(), + &MagicBlockInstruction::ScheduleIntentBundle(args), + vec![ + AccountMeta::new(payer.pubkey(), true), + AccountMeta::new(MAGIC_CONTEXT_PUBKEY, false), + AccountMeta::new(fee_vault.unwrap(), false), + AccountMeta::new(committee, false), + ], ); let callback = add_action_callback_instruction(&payer.pubkey(), fee_vault, destination); let mut outer_accounts = schedule.accounts.clone(); @@ -248,6 +284,17 @@ mod callback_source_tests { ); let account = find_magic_context_account(&result).unwrap(); let mut context = MagicContext::deserialize(account.data()).unwrap(); + let scheduled = &context.scheduled_base_intents[0]; + // The 100k-CU action costs 5,000; each group of tiny actions rounds + // up to one lamport. The snapshot includes all scheduling fees. + let expected_fee = COMMIT_FEE_LAMPORTS + 5_000 + 2; + assert_eq!(scheduled.calculate_fee().unwrap(), expected_fee); + let payer_snapshot = scheduled + .get_all_committed_accounts() + .into_iter() + .find(|account| account.pubkey == payer.pubkey()) + .unwrap(); + assert_eq!(payer_snapshot.account.lamports, 3_000_000 - expected_fee); let action = context.scheduled_base_intents[0].intent_bundle.get_action_mut(0).unwrap(); assert_eq!(action.source_program, Some(source)); assert_eq!(action.destination_program, other); @@ -1576,45 +1623,111 @@ mod tests { #[test] #[serial] - fn test_schedule_commit_delegated_payer_charges_fee_vault() { - let payer = Keypair::from_seed(b"delegated_payer_charges_fee_vault").unwrap(); - let program = Pubkey::new_unique(); - let committee = Pubkey::new_unique(); - - let nonce = ACTUAL_COMMIT_LIMIT; - let (mut account_data, mut transaction_accounts) = prepare_delegated_payer_transaction( - &payer, - program, - &[committee], - StubNonces::Global(nonce), - ); - - let ix = InstructionUtils::schedule_commit_with_delegated_payer_instruction( - &payer.pubkey(), - vec![committee], - ); - extend_transaction_accounts_from_ix(&ix, &mut account_data, &mut transaction_accounts); - - let accounts = process_instruction( - ix.data.as_slice(), - transaction_accounts, - ix.accounts, - Ok(()), - ); - - // Fee vault must have received exactly COMMIT_FEE_LAMPORTS - accounts - .iter() - .find(|a| a.lamports() == COMMIT_FEE_LAMPORTS) - .expect("fee vault should have COMMIT_FEE_LAMPORTS"); + fn scheduling_captures_post_fee_balances() { + use magic_program_api::args::{ + CommitAndUndelegateArgs, CommitTypeArgs, MagicBaseIntentArgs, UndelegateTypeArgs, + }; - // Payer must have been debited - accounts - .iter() - .find(|a| { - a.lamports() == 1_000_000 - COMMIT_FEE_LAMPORTS && a.is(AccountMode::Delegated) - }) - .expect("payer should have been debited"); + let payer = Keypair::new(); + let other = Pubkey::new_unique(); + for undelegate in [false, true] { + let commit = CommitTypeArgs::Standalone(vec![3, 4]); + let intents = if undelegate { + let commit = CommitAndUndelegateArgs { + commit_type: commit, + undelegate_type: UndelegateTypeArgs::Standalone, + }; + [ + MagicBaseIntentArgs::CommitAndUndelegate(commit.clone()), + MagicBaseIntentArgs::CommitFinalizeAndUndelegate(commit), + ] + } else { + [ + MagicBaseIntentArgs::Commit(commit.clone()), + MagicBaseIntentArgs::CommitFinalize(commit), + ] + }; + let instructions = [ + if undelegate { + MagicBlockInstruction::ScheduleCommitAndUndelegate + } else { + MagicBlockInstruction::ScheduleCommit + }, + MagicBlockInstruction::ScheduleBaseIntent(intents[0].clone()), + MagicBlockInstruction::ScheduleBaseIntent(intents[1].clone()), + MagicBlockInstruction::ScheduleIntentBundle(intents[0].clone().into()), + MagicBlockInstruction::ScheduleIntentBundle(intents[1].clone().into()), + ]; + for instruction in instructions { + for committee in [payer.pubkey(), other] { + for nonce in [ACTUAL_COMMIT_LIMIT - 1, ACTUAL_COMMIT_LIMIT] { + let fee = + if nonce == ACTUAL_COMMIT_LIMIT { COMMIT_FEE_LAMPORTS } else { 0 }; + let (mut account_data, mut transaction_accounts) = + prepare_delegated_payer_transaction( + &payer, + system_program::id(), + &[committee], + StubNonces::PerAccount(HashMap::from([(committee, nonce)])), + ); + account_data.get_mut(&payer.pubkey()).unwrap().set_lamports(3_000_000); + let fee_vault = magic_fee_vault_pubkey(); + let ix = Instruction::new_with_wincode( + crate::id(), + &instruction, + vec![ + AccountMeta::new(payer.pubkey(), true), + AccountMeta::new(MAGIC_CONTEXT_PUBKEY, false), + AccountMeta::new(fee_vault, false), + AccountMeta::new(committee, true), + AccountMeta::new(fee_vault, true), + ], + ); + extend_transaction_accounts_from_ix( + &ix, + &mut account_data, + &mut transaction_accounts, + ); + let keys: Vec<_> = + transaction_accounts.iter().map(|(key, _)| *key).collect(); + let accounts = process_instruction( + &ix.data, + transaction_accounts, + ix.accounts, + Ok(()), + ); + let account = |key| &accounts[keys.iter().position(|k| *k == key).unwrap()]; + assert_eq!(account(payer.pubkey()).lamports(), 3_000_000 - fee); + assert_eq!(account(fee_vault).lamports(), fee); + let context = + MagicContext::deserialize(account(MAGIC_CONTEXT_PUBKEY).data()) + .unwrap(); + let committed = + context.scheduled_base_intents[0].get_all_committed_accounts(); + assert_eq!(committed.len(), 2); + for snapshot in &committed { + assert_eq!( + snapshot.account.lamports, + account(snapshot.pubkey).lamports() + ); + assert_eq!(snapshot.account.owner, system_program::id()); + let expected_id = + if snapshot.pubkey == committee { nonce + 1 } else { 1 }; + assert_eq!(snapshot.commit_id, expected_id); + assert_eq!( + u64::from(account(snapshot.pubkey).commit_seq()), + expected_id + ); + assert!(account(snapshot.pubkey).is(if undelegate { + AccountMode::Undelegating + } else { + AccountMode::Delegated + })); + } + } + } + } + } } #[test] @@ -1758,8 +1871,9 @@ async fn commit_reservations_are_atomic_and_reused_after_rollback() { use crate::{magicblock_processor::Entrypoint, utils::instruction_utils::InstructionUtils}; use engine::testkit::{Pacing, TestEngine}; use keeper::testkit::{Dirs, keeper_builder}; - use magic_program_api::args::CommitTypeArgs; + use magic_program_api::args::{CommitAndUndelegateArgs, CommitTypeArgs, UndelegateTypeArgs}; use solana_program_runtime::solana_sbpf::program::BuiltinFunctionDefinition; + use solana_transaction::Transaction; let dirs = Dirs::default(); let mut builder = keeper_builder(&dirs); @@ -1784,6 +1898,26 @@ async fn commit_reservations_are_atomic_and_reused_after_rollback() { .commit_seq(u32::MAX) .build(), ); + let payer = Keypair::new(); + let poor_payer = Keypair::new(); + let fee_vault = Pubkey::find_program_address( + &[b"magic-fee-vault", builder.authority.pubkey().as_ref()], + &DELEGATION_PROGRAM_ID, + ) + .0; + for (key, lamports, commit_seq) in [ + (payer.pubkey(), 3_000_000, ACTUAL_COMMIT_LIMIT as u32), + (poor_payer.pubkey(), 900_000, ACTUAL_COMMIT_LIMIT as u32), + (fee_vault, 1_000_000, 0), + ] { + builder.possibly_missing_accounts.insert( + key, + AccountBuilder::from(AccountSharedData::new(lamports, 0, &system_program::id())) + .mode(AccountMode::Delegated) + .commit_seq(commit_seq) + .build(), + ); + } let engine = TestEngine::from_builder(dirs, builder, Pacing::External).await; let mut schedule = InstructionUtils::schedule_commit_instruction( &engine.authority(), @@ -1806,6 +1940,76 @@ async fn commit_reservations_are_atomic_and_reused_after_rollback() { data: vec![255], }; let initial_context = engine.get_account(MAGIC_CONTEXT_PUBKEY).unwrap().data().to_vec(); + for payer in [&payer, &poor_payer] { + let initial_payer = engine.get_account(payer.pubkey()).unwrap(); + for undelegate in [false, true] { + let commit = CommitTypeArgs::Standalone(vec![0]); + let args = if undelegate { + MagicIntentBundleArgs { + commit_finalize_and_undelegate: Some(CommitAndUndelegateArgs { + commit_type: commit, + undelegate_type: UndelegateTypeArgs::Standalone, + }), + ..Default::default() + } + } else { + MagicIntentBundleArgs { + commit_finalize: Some(commit), + ..Default::default() + } + }; + for instruction in [ + if undelegate { + MagicBlockInstruction::ScheduleCommitAndUndelegate + } else { + MagicBlockInstruction::ScheduleCommit + }, + MagicBlockInstruction::ScheduleIntentBundle(args), + ] { + let ix = Instruction::new_with_wincode( + crate::id(), + &instruction, + vec![ + AccountMeta::new(payer.pubkey(), true), + AccountMeta::new(MAGIC_CONTEXT_PUBKEY, false), + AccountMeta::new(fee_vault, false), + AccountMeta::new(payer.pubkey(), true), + ], + ); + let transaction = Transaction::new_signed_with_payer( + &[ix, fail.clone()], + Some(&engine.authority()), + &[engine.signer(), payer], + engine.blockhash(), + ); + let result = engine.execute(transaction).await; + if payer.pubkey() == poor_payer.pubkey() { + assert_matches!( + result, + Err(solana_transaction::TransactionError::InstructionError( + 0, + InstructionError::InsufficientFunds + )) + ); + } else { + assert_matches!( + result, + Err(solana_transaction::TransactionError::InstructionError(1, _)) + ); + } + let account = engine.get_account(payer.pubkey()).unwrap(); + assert_eq!(account.lamports(), initial_payer.lamports()); + assert_eq!(account.commit_seq(), initial_payer.commit_seq()); + assert_eq!(account.owner(), initial_payer.owner()); + assert!(account.is(AccountMode::Delegated)); + assert_eq!(engine.get_account(fee_vault).unwrap().lamports(), 1_000_000); + assert_eq!( + engine.get_account(MAGIC_CONTEXT_PUBKEY).unwrap().data(), + initial_context + ); + } + } + } assert!(engine.execute(&[schedule.clone(), bundle.clone(), fail]).await.is_err()); assert_eq!(engine.get_account(committee).unwrap().commit_seq(), 3); assert_eq!( diff --git a/programs/magic-program/src/intent_bundles/schedule/process_schedule_intent_bundle.rs b/programs/magic-program/src/intent_bundles/schedule/process_schedule_intent_bundle.rs index 9755d43ce..6c3d729e5 100644 --- a/programs/magic-program/src/intent_bundles/schedule/process_schedule_intent_bundle.rs +++ b/programs/magic-program/src/intent_bundles/schedule/process_schedule_intent_bundle.rs @@ -11,7 +11,7 @@ use solana_pubkey::Pubkey; use crate::{ MagicContext, magic_scheduled_base_intent::{ - ConstructionContext, ScheduledIntentBundle, extract_commit_accounts, + ConstructionContext, ScheduledIntentBundle, extract_commit_accounts, prepare_intent_bundle, }, schedule_transactions::{ MAGIC_CONTEXT_IDX, PAYER_IDX, check_commit_limits, check_magic_context_id, get_clock, @@ -77,6 +77,25 @@ pub(crate) fn process_schedule_intent_bundle( let mut construction_context = ConstructionContext::new(parent_program_id, &signers, invoke_context, secure); + // Determine and charge the fee while the payer is still delegated. + // Snapshots below must include the payer debit and fee-vault credit. + let has_fee_vault = { + let transaction_context = construction_context.transaction_context(); + let magic_fee_vault = try_get_fee_vault( + transaction_context, + construction_context.invoke_context, + PAYER_IDX, + MAGIC_CONTEXT_IDX + 1, + )? + .0; + let fee = prepare_intent_bundle(&args, &construction_context)?; + if let Some(ref fee_vault) = magic_fee_vault { + let payer = get_instruction_account_with_idx(transaction_context, PAYER_IDX)?; + charge_delegated_payer(&payer, fee_vault, fee)?; + } + magic_fee_vault.is_some() + }; + // Collect all undelegated account indices. let undelegated_account_indices: Vec = [ args.commit_and_undelegate.as_ref(), @@ -97,6 +116,20 @@ pub(crate) fn process_schedule_intent_bundle( &mut construction_context, )?; + if !has_fee_vault { + // Both commit variants use the sponsored allowance. Undelegation remains + // available after the allowance is exhausted. + for commit_accounts in [ + scheduled_intent.get_commit_intent_accounts(), + scheduled_intent.get_commit_finalize_intent_accounts(), + ] + .into_iter() + .flatten() + { + check_commit_limits(commit_accounts, construction_context.invoke_context)?; + } + } + // Change owner to dlp and set undelegating flag. // Once account is undelegated we need to make it immutable in our validator. let transaction_context = construction_context.transaction_context(); @@ -119,31 +152,6 @@ pub(crate) fn process_schedule_intent_bundle( } let transaction_context = &*invoke_context.transaction_context; - let payer_account = get_instruction_account_with_idx(transaction_context, PAYER_IDX)?; - let magic_fee_vault = try_get_fee_vault( - transaction_context, - invoke_context, - PAYER_IDX, - MAGIC_CONTEXT_IDX + 1, - )? - .0; - if let Some(magic_fee_vault) = magic_fee_vault { - let fee = scheduled_intent.calculate_fee()?; - charge_delegated_payer(&payer_account, &magic_fee_vault, fee)?; - } else { - // Both commit variants use the sponsored allowance. Undelegation remains - // available after the allowance is exhausted. - for commit_accounts in [ - scheduled_intent.get_commit_intent_accounts(), - scheduled_intent.get_commit_finalize_intent_accounts(), - ] - .into_iter() - .flatten() - { - check_commit_limits(commit_accounts, invoke_context)?; - } - } - context.add_scheduled_action(scheduled_intent); let context_acc = get_instruction_account_with_idx(transaction_context, MAGIC_CONTEXT_IDX)?; context.write_to(context_acc.borrow_mut()?.data_as_mut_slice())?;