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())?;