diff --git a/magicblock-api/src/magic_sys_adapter.rs b/magicblock-api/src/magic_sys_adapter.rs index 270cd0b20..0b690c9d8 100644 --- a/magicblock-api/src/magic_sys_adapter.rs +++ b/magicblock-api/src/magic_sys_adapter.rs @@ -3,9 +3,14 @@ use std::{collections::HashMap, sync::Arc, time::Duration}; use magicblock_committor_service::{ committor_processor::CommittorProcessor, intent_executor::task_info_fetcher::TaskInfoFetcherResult, + tasks::intent_size_validator::IntentSizeValidator, +}; +use magicblock_core::{ + intent::{types::CommittedAccount, MagicIntentBundle}, + traits::MagicSys, }; -use magicblock_core::{intent::CommittedAccount, traits::MagicSys}; use magicblock_metrics::metrics; +use magicblock_program::magic_sys::INTENT_TOO_LARGE_ERR; use solana_instruction::error::InstructionError; use solana_pubkey::Pubkey; use tracing::error; @@ -98,4 +103,15 @@ impl MagicSys for MagicSysAdapter { }) .map_err(|_| InstructionError::Custom(Self::FETCH_ERR)) } + + fn validate_intent_size( + &self, + intent: &MagicIntentBundle, + ) -> Result<(), InstructionError> { + if IntentSizeValidator::fits(intent) { + Ok(()) + } else { + Err(InstructionError::Custom(INTENT_TOO_LARGE_ERR)) + } + } } diff --git a/magicblock-committor-service/src/intent_execution_manager/intent_scheduler.rs b/magicblock-committor-service/src/intent_execution_manager/intent_scheduler.rs index 580acebf6..099a01b81 100644 --- a/magicblock-committor-service/src/intent_execution_manager/intent_scheduler.rs +++ b/magicblock-committor-service/src/intent_execution_manager/intent_scheduler.rs @@ -613,7 +613,7 @@ mod complex_blocking_test { #[cfg(test)] mod edge_cases_test { - use magicblock_program::magic_scheduled_base_intent::MagicIntentBundle; + use magicblock_core::intent::MagicIntentBundle; use super::*; use crate::test_utils; @@ -637,7 +637,7 @@ mod edge_cases_test { #[cfg(test)] mod complete_error_test { - use magicblock_core::intent::CommittedAccount; + use magicblock_core::intent::types::CommittedAccount; use solana_account::Account; use solana_pubkey::pubkey; @@ -854,11 +854,11 @@ pub(crate) fn create_test_intent( pubkeys: &[Pubkey], is_undelegate: bool, ) -> ScheduledIntentBundle { - use magicblock_core::intent::CommittedAccount; - use magicblock_program::magic_scheduled_base_intent::{ - CommitAndUndelegate, CommitType, MagicIntentBundle, - ScheduledIntentBundle, UndelegateType, + use magicblock_core::intent::{ + types::CommittedAccount, CommitAndUndelegate, CommitType, + MagicIntentBundle, UndelegateType, }; + use magicblock_program::magic_scheduled_base_intent::ScheduledIntentBundle; use solana_account::Account; use solana_hash::Hash; use solana_transaction::Transaction; @@ -904,11 +904,11 @@ pub(crate) fn create_test_intent_bundle( commit_pubkeys: &[Pubkey], commit_and_undelegate_pubkeys: &[Pubkey], ) -> ScheduledIntentBundle { - use magicblock_core::intent::CommittedAccount; - use magicblock_program::magic_scheduled_base_intent::{ - CommitAndUndelegate, CommitType, MagicIntentBundle, - ScheduledIntentBundle, UndelegateType, + use magicblock_core::intent::{ + types::CommittedAccount, CommitAndUndelegate, CommitType, + MagicIntentBundle, UndelegateType, }; + use magicblock_program::magic_scheduled_base_intent::ScheduledIntentBundle; use solana_account::Account; use solana_hash::Hash; use solana_transaction::Transaction; diff --git a/magicblock-committor-service/src/intent_executor/two_stage_executor.rs b/magicblock-committor-service/src/intent_executor/two_stage_executor.rs index 196ae72be..6ba0cad8c 100644 --- a/magicblock-committor-service/src/intent_executor/two_stage_executor.rs +++ b/magicblock-committor-service/src/intent_executor/two_stage_executor.rs @@ -255,12 +255,16 @@ where | TransactionStrategyExecutionError::LoadedAccountsDataSizeExceeded( _, _, - ) - | TransactionStrategyExecutionError::TransactionTooLargeError(_) => { + ) => { // Can't be handled error!(error = ?err, "Commit tasks exceeded execution limit"); Ok(ControlFlow::Break(())) } + TransactionStrategyExecutionError::TransactionTooLargeError(_) => { + // Can't be handled but also shouldn't occur + error!(strategy = ?self.state.commit_strategy, error = ?err, "Commit tasks do not fit in tx"); + Ok(ControlFlow::Break(())) + } TransactionStrategyExecutionError::InternalError(_) => { // Can't be handled Ok(ControlFlow::Break(())) @@ -496,12 +500,16 @@ where Ok(ControlFlow::Continue(to_cleanup)) } TransactionStrategyExecutionError::CpiLimitError(_, _) - | TransactionStrategyExecutionError::LoadedAccountsDataSizeExceeded(_, _) - | TransactionStrategyExecutionError::TransactionTooLargeError(_) => { + | TransactionStrategyExecutionError::LoadedAccountsDataSizeExceeded(_, _) => { // Can't be handled warn!(error = ?err, "Finalization tasks exceeded execution limit"); Ok(ControlFlow::Break(())) } + TransactionStrategyExecutionError::TransactionTooLargeError(_) => { + // Can't be handled but also shouldn't occur + error!(strategy = ?self.state.finalize_strategy, error = ?err, "Finalization tasks do not fit in tx"); + Ok(ControlFlow::Break(())) + } TransactionStrategyExecutionError::InternalError(_) => { // Can't be handled Ok(ControlFlow::Break(())) diff --git a/magicblock-committor-service/src/persist/commit_persister.rs b/magicblock-committor-service/src/persist/commit_persister.rs index 375dd7f59..4c641863f 100644 --- a/magicblock-committor-service/src/persist/commit_persister.rs +++ b/magicblock-committor-service/src/persist/commit_persister.rs @@ -4,11 +4,11 @@ use std::{ sync::{Arc, Mutex}, }; -use magicblock_core::intent::CommittedAccount; -use magicblock_program::magic_scheduled_base_intent::{ - CommitAndUndelegate, CommitType as IntentCommitType, MagicIntentBundle, - ScheduledIntentBundle, UndelegateType, +use magicblock_core::intent::{ + types::CommittedAccount, CommitAndUndelegate, + CommitType as IntentCommitType, MagicIntentBundle, UndelegateType, }; +use magicblock_program::magic_scheduled_base_intent::ScheduledIntentBundle; use solana_account::Account; use solana_pubkey::Pubkey; use solana_transaction::Transaction; @@ -603,9 +603,9 @@ fn committed_account_from_row( #[cfg(test)] mod tests { - use magicblock_core::intent::CommittedAccount; - use magicblock_program::magic_scheduled_base_intent::{ - CommitAndUndelegate, CommitType, MagicIntentBundle, UndelegateType, + use magicblock_core::intent::{ + types::CommittedAccount, CommitAndUndelegate, CommitType, + MagicIntentBundle, UndelegateType, }; use solana_account::Account; use solana_hash::Hash; diff --git a/magicblock-committor-service/src/tasks/commit_finalize_task.rs b/magicblock-committor-service/src/tasks/commit_finalize_task.rs index b189f0c1f..c7255dd53 100644 --- a/magicblock-committor-service/src/tasks/commit_finalize_task.rs +++ b/magicblock-committor-service/src/tasks/commit_finalize_task.rs @@ -4,7 +4,7 @@ use dlp_api::{ instruction_builder::{commit_diff_size_budget, commit_size_budget}, AccountSizeClass, }; -use magicblock_core::intent::CommittedAccount; +use magicblock_core::intent::types::CommittedAccount; use solana_account::{Account, ReadableAccount}; use solana_instruction::Instruction; use solana_pubkey::Pubkey; diff --git a/magicblock-committor-service/src/tasks/commit_task.rs b/magicblock-committor-service/src/tasks/commit_task.rs index 439999827..8eacb670b 100644 --- a/magicblock-committor-service/src/tasks/commit_task.rs +++ b/magicblock-committor-service/src/tasks/commit_task.rs @@ -3,7 +3,7 @@ use dlp_api::{ diff::compute_diff, AccountSizeClass, }; -use magicblock_core::intent::CommittedAccount; +use magicblock_core::intent::types::CommittedAccount; use solana_account::{Account, ReadableAccount}; use solana_instruction::Instruction; use solana_pubkey::Pubkey; diff --git a/magicblock-committor-service/src/tasks/intent_size_validator.rs b/magicblock-committor-service/src/tasks/intent_size_validator.rs new file mode 100644 index 000000000..d48167c95 --- /dev/null +++ b/magicblock-committor-service/src/tasks/intent_size_validator.rs @@ -0,0 +1,311 @@ +use magicblock_core::intent::{ + types::CommittedAccount, CommitType, MagicIntentBundle, UndelegateType, +}; +use solana_keypair::Keypair; +use solana_pubkey::Pubkey; +use solana_signer::Signer; + +use crate::{ + tasks::{ + commit_task::CommitDelivery, + task_strategist::TaskStrategist, + utils::{ + create_action_tasks, create_commit_finalize_task, + create_commit_task, TransactionUtils, + }, + BaseTask, BaseTaskImpl, FinalizeTask, UndelegateTask, + }, + transactions::{serialized_transaction_size, MAX_TRANSACTION_WIRE_SIZE}, +}; + +/// Checks whether an intent could ever fit on the base layer, so intents +/// that can never succeed can be refused up front instead of failing later +/// during execution +/// +/// The check assumes the smallest possible task representation (buffer-mode +/// commits for oversized accounts, full address-lookup-table coverage, +/// 2-stage execution) -- if it doesn't fit even then, no amount of +/// optimization at execution time will make it fit. Unlike +/// [`TaskBuilderImpl`], this never fetches anything (no commit ids, no +/// base-layer state): it only needs to know whether a fit is *possible*, +/// not build the real tasks that will actually be executed. +/// NOTE: this assumes ALTs are used optimally +/// where number of ALTs is - pubkeys.div_ceil(256). It is impossible to know in advance +/// how many ALTs will be used. +pub struct IntentSizeValidator; + +impl IntentSizeValidator { + /// Returns `true` if `intent`'s commit and finalize transactions could + /// both fit within [`MAX_TRANSACTION_WIRE_SIZE`]. If this returns + /// `false`, the intent can never succeed and should be refused. + pub fn fits(intent: &MagicIntentBundle) -> bool { + Self::tasks_fit(&Self::commit_tasks(intent)) + && Self::tasks_fit(&Self::finalize_tasks(intent)) + } + + /// Builds the commit-stage tasks used for the size estimate: real + /// standalone/base actions, and the smallest-by-default + /// `Commit`/`CommitFinalize` task for each committed account. + fn commit_tasks(intent: &MagicIntentBundle) -> Vec { + let mut tasks: Vec = + create_action_tasks(&intent.standalone_actions).collect(); + + if let Some(ref commit) = intent.commit { + tasks.extend(Self::commit_type_tasks(commit)); + } + if let Some(ref cau) = intent.commit_and_undelegate { + tasks.extend(Self::commit_type_tasks(&cau.commit_action)); + } + if let Some(ref commit_finalize) = intent.commit_finalize { + tasks.extend(Self::commit_finalize_type_tasks(commit_finalize)); + } + if let Some(ref cfau) = intent.commit_finalize_and_undelegate { + tasks.extend(Self::commit_finalize_type_tasks(&cfau.commit_action)); + } + + tasks + } + + /// Builds the finalize-stage tasks used for the size estimate, mirroring + /// [`crate::tasks::task_builder::TasksBuilder::finalize_tasks`] but + /// without fetching rent reimbursements (a placeholder pubkey is used + /// instead, since it doesn't affect instruction size). + fn finalize_tasks(intent: &MagicIntentBundle) -> Vec { + fn finalize_task(account: &CommittedAccount) -> BaseTaskImpl { + FinalizeTask { + delegated_account: account.pubkey, + } + .into() + } + + fn undelegate_task(account: &CommittedAccount) -> BaseTaskImpl { + UndelegateTask { + delegated_account: account.pubkey, + owner_program: account.account.owner, + // Real reimbursement pubkey is unknown here; doesn't affect size. + rent_reimbursement: Pubkey::default(), + } + .into() + } + + fn commit_type_finalize_tasks( + commit_type: &CommitType, + ) -> Vec { + let mut tasks: Vec = commit_type + .get_committed_accounts() + .iter() + .map(finalize_task) + .collect(); + if let CommitType::WithBaseActions { base_actions, .. } = + commit_type + { + tasks.extend(create_action_tasks(base_actions)); + } + tasks + } + + let mut tasks = Vec::new(); + + if let Some(ref commit) = intent.commit { + tasks.extend(commit_type_finalize_tasks(commit)); + } + + if let Some(ref cau) = intent.commit_and_undelegate { + tasks.extend(commit_type_finalize_tasks(&cau.commit_action)); + tasks.extend( + cau.commit_action + .get_committed_accounts() + .iter() + .map(undelegate_task), + ); + if let UndelegateType::WithBaseActions(actions) = + &cau.undelegate_action + { + tasks.extend(create_action_tasks(actions)); + } + } + + // `commit_finalize` needs no separate finalize step: commit and + // finalize already happen together in a single `CommitFinalizeTask`. + if let Some(ref cfau) = intent.commit_finalize_and_undelegate { + tasks.extend( + cfau.commit_action + .get_committed_accounts() + .iter() + .map(undelegate_task), + ); + if let UndelegateType::WithBaseActions(actions) = + &cfau.undelegate_action + { + tasks.extend(create_action_tasks(actions)); + } + } + + tasks + } + + /// Builds the smallest-by-default `CommitTask` for `account`. Reuses + /// [`create_commit_task`]'s real `COMMIT_STATE_SIZE_THRESHOLD` check by + /// passing a clone of the account's own data as a stand-in base account + /// -- large enough accounts land on `DiffInArgs`, which is then + /// immediately escalated to buffer mode since the real diff size is + /// unknowable ahead of time and a buffer instruction only ever + /// references the buffer PDA, never account data. + /// + /// `allow_undelegation` is always a placeholder: it's a fixed-size flag + /// in the instruction args and never changes instruction size, so which + /// value we pass here doesn't matter. What actually differs between a + /// commit and a commit-and-undelegate is the extra `UndelegateTask` + /// built in [`Self::finalize_tasks`]. + fn commit_task(account: &CommittedAccount) -> BaseTaskImpl { + let mut task = create_commit_task( + 0, + false, + account.clone(), + Some(account.account.clone()), + ); + if matches!(task.delivery_details, CommitDelivery::DiffInArgs { .. }) { + task.try_optimize_tx_size(); + } + task.into() + } + + /// Same as [`Self::commit_task`] but for `CommitFinalizeTask`. + fn commit_finalize_task(account: &CommittedAccount) -> BaseTaskImpl { + let mut task = create_commit_finalize_task( + 0, + false, + account.clone(), + Some(account.account.clone()), + ); + if matches!(task.delivery, CommitDelivery::DiffInArgs { .. }) { + task.try_optimize_tx_size(); + } + task.into() + } + + fn commit_type_tasks(commit_type: &CommitType) -> Vec { + let mut tasks: Vec = commit_type + .get_committed_accounts() + .iter() + .map(Self::commit_task) + .collect(); + if let CommitType::WithBaseActions { base_actions, .. } = commit_type { + tasks.extend(create_action_tasks(base_actions)); + } + tasks + } + + fn commit_finalize_type_tasks( + commit_type: &CommitType, + ) -> Vec { + let mut tasks: Vec = commit_type + .get_committed_accounts() + .iter() + .map(Self::commit_finalize_task) + .collect(); + if let CommitType::WithBaseActions { base_actions, .. } = commit_type { + tasks.extend(create_action_tasks(base_actions)); + } + tasks + } + + /// Returns `true` if `tasks`, assembled into a single transaction with + /// full address-lookup-table coverage, fits within + /// [`MAX_TRANSACTION_WIRE_SIZE`]. + fn tasks_fit(tasks: &[BaseTaskImpl]) -> bool { + let placeholder = Keypair::new(); + let lookup_table_keys = TaskStrategist::collect_lookup_table_keys( + &placeholder.pubkey(), + tasks, + ); + let lookup_tables = + TransactionUtils::dummy_lookup_table(&lookup_table_keys); + + TransactionUtils::assemble_tasks_tx( + &placeholder, + tasks, + 0, + &lookup_tables, + ) + .map(|tx| serialized_transaction_size(&tx) <= MAX_TRANSACTION_WIRE_SIZE) + .unwrap_or(false) + } +} + +#[cfg(test)] +mod tests { + use magicblock_core::intent::{BaseAction, ProgramArgs}; + use solana_account::Account; + + use super::*; + + fn make_committed_account(data_len: usize) -> CommittedAccount { + CommittedAccount { + pubkey: Pubkey::new_unique(), + account: Account { + lamports: 1_000, + data: vec![0; data_len], + owner: Pubkey::new_unique(), + executable: false, + rent_epoch: 0, + }, + remote_slot: 0, + } + } + + fn make_base_action(data_len: usize) -> BaseAction { + BaseAction { + compute_units: 10_000, + destination_program: Pubkey::new_unique(), + source_program: None, + escrow_authority: Pubkey::new_unique(), + data_per_program: ProgramArgs { + escrow_index: 0, + data: vec![0; data_len], + }, + account_metas_per_program: vec![], + callback: None, + } + } + + #[test] + fn test_empty_intent_fits() { + assert!(IntentSizeValidator::fits(&MagicIntentBundle::default())); + } + + #[test] + fn test_small_commit_fits() { + let intent = MagicIntentBundle { + commit: Some(CommitType::Standalone(vec![make_committed_account( + 10, + )])), + ..Default::default() + }; + assert!(IntentSizeValidator::fits(&intent)); + } + + #[test] + fn test_large_commit_forced_to_buffer_fits() { + // Well above COMMIT_STATE_SIZE_THRESHOLD -- would never fit inline, + // but must be escalated to buffer mode by the validator. + let intent = MagicIntentBundle { + commit: Some(CommitType::Standalone(vec![make_committed_account( + 50_000, + )])), + ..Default::default() + }; + assert!(IntentSizeValidator::fits(&intent)); + } + + #[test] + fn test_oversized_standalone_action_does_not_fit() { + // BaseAction data is used as-is (never optimized away), so a + // payload bigger than the whole transaction wire size can never fit. + let intent = MagicIntentBundle { + standalone_actions: vec![make_base_action(2_000)], + ..Default::default() + }; + assert!(!IntentSizeValidator::fits(&intent)); + } +} diff --git a/magicblock-committor-service/src/tasks/mod.rs b/magicblock-committor-service/src/tasks/mod.rs index 31b8186eb..c23106516 100644 --- a/magicblock-committor-service/src/tasks/mod.rs +++ b/magicblock-committor-service/src/tasks/mod.rs @@ -1,13 +1,13 @@ use dlp_api::{args::CallHandlerArgs, AccountSizeClass}; -use magicblock_core::intent::BaseActionCallback; +use magicblock_core::intent::{BaseAction, BaseActionCallback}; use magicblock_metrics::metrics::LabelValue; -use magicblock_program::magic_scheduled_base_intent::BaseAction; use solana_instruction::{AccountMeta, Instruction}; use solana_pubkey::Pubkey; pub mod commit_finalize_task; pub mod commit_stage_task; pub mod commit_task; +pub mod intent_size_validator; pub mod task_builder; pub mod task_strategist; pub mod utils; @@ -378,10 +378,8 @@ impl From for BaseActionTask { #[cfg(test)] mod serialization_safety_test { - use magicblock_core::intent::CommittedAccount; - use magicblock_program::{ - args::ShortAccountMeta, magic_scheduled_base_intent::ProgramArgs, - }; + use magicblock_core::intent::{types::CommittedAccount, ProgramArgs}; + use magicblock_program::args::ShortAccountMeta; use solana_account::Account; use crate::{ diff --git a/magicblock-committor-service/src/tasks/task_builder.rs b/magicblock-committor-service/src/tasks/task_builder.rs index 515e925f5..a15805251 100644 --- a/magicblock-committor-service/src/tasks/task_builder.rs +++ b/magicblock-committor-service/src/tasks/task_builder.rs @@ -2,11 +2,10 @@ use std::{collections::HashMap, sync::Arc}; use async_trait::async_trait; use futures_util::future::try_join_all; -use magicblock_core::intent::CommittedAccount; -use magicblock_program::magic_scheduled_base_intent::{ - BaseAction, CommitAndUndelegate, CommitType, ScheduledIntentBundle, - UndelegateType, +use magicblock_core::intent::{ + types::CommittedAccount, CommitAndUndelegate, CommitType, UndelegateType, }; +use magicblock_program::magic_scheduled_base_intent::ScheduledIntentBundle; use solana_account::Account; use solana_pubkey::Pubkey; use solana_signature::Signature; @@ -18,9 +17,11 @@ use crate::{ }, persist::IntentPersister, tasks::{ - commit_task::{CommitDelivery, CommitTask}, - BaseActionTask, BaseActionTaskV1, BaseActionTaskV2, BaseTaskImpl, - CommitFinalizeTask, FinalizeTask, UndelegateTask, + utils::{ + create_action_tasks, create_commit_finalize_task, + create_commit_task, COMMIT_STATE_SIZE_THRESHOLD, + }, + BaseTaskImpl, FinalizeTask, UndelegateTask, }, }; @@ -51,59 +52,7 @@ pub struct CommitStageTaskInfo { /// Task builder pub struct TaskBuilderImpl; -// Accounts larger than COMMIT_STATE_SIZE_THRESHOLD use CommitDiff to -// reduce instruction size. Below this threshold, the commit is sent -// as CommitState. The value (256) is chosen because it is sufficient -// for small accounts, which typically could hold up to 8 u32 fields or -// 4 u64 fields. These integers are expected to be on the hot path -// and updated continuously. -pub const COMMIT_STATE_SIZE_THRESHOLD: usize = 256; - impl TaskBuilderImpl { - pub fn create_commit_task( - commit_id: u64, - allow_undelegation: bool, - account: CommittedAccount, - base_account: Option, - ) -> CommitTask { - let base_account = - if account.account.data.len() > COMMIT_STATE_SIZE_THRESHOLD { - base_account - } else { - None - }; - - let delivery_details = if let Some(base_account) = base_account { - CommitDelivery::DiffInArgs { base_account } - } else { - CommitDelivery::StateInArgs - }; - - CommitTask { - commit_id, - allow_undelegation, - committed_account: account, - delivery_details, - } - } - - fn create_action_tasks<'a>( - actions: &'a [BaseAction], - ) -> impl Iterator + 'a { - actions.iter().map(|action| { - let task = match action.source_program { - Some(source_program) => BaseActionTask::V2(BaseActionTaskV2 { - action: action.clone(), - source_program, - }), - None => BaseActionTask::V1(BaseActionTaskV1 { - action: action.clone(), - }), - }; - task.into() - }) - } - async fn fetch_commit_nonces( task_info_fetcher: &Arc, accounts: &[CommittedAccount], @@ -183,33 +132,6 @@ impl TaskBuilderImpl { base_accounts, }) } - - pub fn create_commit_finalize_task( - commit_id: u64, - allow_undelegation: bool, - account: CommittedAccount, - base_account: Option, - ) -> CommitFinalizeTask { - let base_account = - if account.account.data.len() > COMMIT_STATE_SIZE_THRESHOLD { - base_account - } else { - None - }; - - let delivery_details = if let Some(base_account) = base_account { - CommitDelivery::DiffInArgs { base_account } - } else { - CommitDelivery::StateInArgs - }; - - CommitFinalizeTask { - commit_id, - allow_undelegation, - committed_account: account, - delivery: delivery_details, - } - } } #[async_trait] @@ -222,7 +144,7 @@ impl TasksBuilder for TaskBuilderImpl { ) -> TaskBuilderResult> { let mut tasks = Vec::new(); // Add standalone actions first - tasks.extend(Self::create_action_tasks( + tasks.extend(create_action_tasks( intent_bundle.standalone_actions().as_slice(), )); @@ -322,9 +244,7 @@ impl TasksBuilder for TaskBuilderImpl { .iter() .map(finalize_task) .collect::>(); - tasks.extend(TaskBuilderImpl::create_action_tasks( - base_actions, - )); + tasks.extend(create_action_tasks(base_actions)); tasks } } @@ -361,7 +281,7 @@ impl TasksBuilder for TaskBuilderImpl { if let UndelegateType::WithBaseActions(actions) = &commit_and_undelegate.undelegate_action { - tasks.extend(TaskBuilderImpl::create_action_tasks(actions)); + tasks.extend(create_action_tasks(actions)); } Ok(tasks) } @@ -406,13 +326,7 @@ impl<'a> CommitBuilder<'a> { let nonce = take_commit_nonce(self.commit_nonces, account.pubkey); let base = self.base_accounts.remove(&account.pubkey); - TaskBuilderImpl::create_commit_task( - nonce, - false, - account.clone(), - base, - ) - .into() + create_commit_task(nonce, false, account.clone(), base).into() }) .collect() } @@ -432,13 +346,7 @@ impl<'a> CommitAndUndelegateBuilder<'a> { let nonce = take_commit_nonce(self.commit_nonces, account.pubkey); let base = self.base_accounts.remove(&account.pubkey); - TaskBuilderImpl::create_commit_task( - nonce, - true, - account.clone(), - base, - ) - .into() + create_commit_task(nonce, true, account.clone(), base).into() }) .collect() } @@ -458,20 +366,15 @@ impl<'a> CommitFinalizeBuilder<'a> { let nonce = take_commit_nonce(self.commit_nonces, account.pubkey); let base = self.base_accounts.remove(&account.pubkey); - TaskBuilderImpl::create_commit_finalize_task( - nonce, - false, - account.clone(), - base, - ) - .into() + create_commit_finalize_task(nonce, false, account.clone(), base) + .into() }) .collect(); if let CommitType::WithBaseActions { ref base_actions, .. } = commit_type { - tasks.extend(TaskBuilderImpl::create_action_tasks(base_actions)); + tasks.extend(create_action_tasks(base_actions)); } tasks } @@ -491,20 +394,15 @@ impl<'a> CommitFinalizeAndUndelegateBuilder<'a> { let nonce = take_commit_nonce(self.commit_nonces, account.pubkey); let base = self.base_accounts.remove(&account.pubkey); - TaskBuilderImpl::create_commit_finalize_task( - nonce, - true, - account.clone(), - base, - ) - .into() + create_commit_finalize_task(nonce, true, account.clone(), base) + .into() }) .collect(); if let CommitType::WithBaseActions { ref base_actions, .. } = commit_type { - tasks.extend(TaskBuilderImpl::create_action_tasks(base_actions)); + tasks.extend(create_action_tasks(base_actions)); } tasks } diff --git a/magicblock-committor-service/src/tasks/task_strategist.rs b/magicblock-committor-service/src/tasks/task_strategist.rs index b55cbd7dc..6e7a239dc 100644 --- a/magicblock-committor-service/src/tasks/task_strategist.rs +++ b/magicblock-committor-service/src/tasks/task_strategist.rs @@ -15,7 +15,7 @@ use crate::{ transactions::{serialized_transaction_size, MAX_TRANSACTION_WIRE_SIZE}, }; -#[derive(Default)] +#[derive(Default, Debug)] pub struct TransactionStrategy { pub optimized_tasks: Vec, pub lookup_tables_keys: Vec, @@ -472,9 +472,8 @@ pub type TaskStrategistResult = Result; mod tests { use std::{collections::HashMap, sync::Arc}; - use magicblock_core::intent::CommittedAccount; - use magicblock_program::magic_scheduled_base_intent::{ - BaseAction, ProgramArgs, + use magicblock_core::intent::{ + types::CommittedAccount, BaseAction, ProgramArgs, }; use solana_account::Account; use solana_pubkey::Pubkey; @@ -488,9 +487,8 @@ mod tests { persist::IntentPersisterImpl, tasks::{ commit_task::CommitTask, - task_builder::{ - TaskBuilderImpl, TasksBuilder, COMMIT_STATE_SIZE_THRESHOLD, - }, + task_builder::{TaskBuilderImpl, TasksBuilder}, + utils::{create_commit_task, COMMIT_STATE_SIZE_THRESHOLD}, BaseActionTask, BaseActionTaskV1, FinalizeTask, TaskStrategy, UndelegateTask, }, @@ -551,12 +549,7 @@ mod tests { }; if diff_len == 0 { - TaskBuilderImpl::create_commit_task( - commit_id, - false, - committed_account, - None, - ) + create_commit_task(commit_id, false, committed_account, None) } else { let base_account = { let mut acc = committed_account.account.clone(); @@ -566,7 +559,7 @@ mod tests { } acc }; - TaskBuilderImpl::create_commit_task( + create_commit_task( commit_id, false, committed_account, diff --git a/magicblock-committor-service/src/tasks/utils.rs b/magicblock-committor-service/src/tasks/utils.rs index 6b5f3e4cf..a37040c05 100644 --- a/magicblock-committor-service/src/tasks/utils.rs +++ b/magicblock-committor-service/src/tasks/utils.rs @@ -1,6 +1,8 @@ use std::collections::HashSet; use dlp_api::DLP_PROGRAM_DATA_SIZE_CLASS; +use magicblock_core::intent::{types::CommittedAccount, BaseAction}; +use solana_account::Account; use solana_compute_budget_interface::ComputeBudgetInstruction; use solana_hash::Hash; use solana_instruction::Instruction; @@ -13,9 +15,104 @@ use solana_signer::Signer; use solana_transaction::versioned::VersionedTransaction; use crate::tasks::{ - task_strategist::TaskStrategistResult, BaseTask, BaseTaskImpl, + commit_finalize_task::CommitFinalizeTask, + commit_task::{CommitDelivery, CommitTask}, + task_strategist::TaskStrategistResult, + BaseActionTask, BaseActionTaskV1, BaseActionTaskV2, BaseTask, BaseTaskImpl, }; +// Accounts larger than COMMIT_STATE_SIZE_THRESHOLD use CommitDiff to +// reduce instruction size. Below this threshold, the commit is sent +// as CommitState. The value (256) is chosen because it is sufficient +// for small accounts, which typically could hold up to 8 u32 fields or +// 4 u64 fields. These integers are expected to be on the hot path +// and updated continuously. +pub const COMMIT_STATE_SIZE_THRESHOLD: usize = 256; + +/// Builds a [`BaseTaskImpl`] for each `action`, used by both +/// [`crate::tasks::task_builder::TaskBuilderImpl`] (real task construction) +/// and [`crate::tasks::intent_size_validator::IntentSizeValidator`] (size +/// estimation) -- actions carry no unknowns that need fetching, so both +/// build them identically. +pub fn create_action_tasks( + actions: &[BaseAction], +) -> impl Iterator + '_ { + actions.iter().map(|action| { + let task = match action.source_program { + Some(source_program) => BaseActionTask::V2(BaseActionTaskV2 { + action: action.clone(), + source_program, + }), + None => BaseActionTask::V1(BaseActionTaskV1 { + action: action.clone(), + }), + }; + task.into() + }) +} + +/// Decides how a commit's data should be delivered based on account size: +/// accounts larger than `COMMIT_STATE_SIZE_THRESHOLD` diff against +/// `base_account` (when available), everything else is sent as full state. +/// Shared by [`create_commit_task`] and [`create_commit_finalize_task`] so +/// the two never drift apart. +fn commit_delivery( + account: &CommittedAccount, + base_account: Option, +) -> CommitDelivery { + let base_account = + if account.account.data.len() > COMMIT_STATE_SIZE_THRESHOLD { + base_account + } else { + None + }; + + if let Some(base_account) = base_account { + CommitDelivery::DiffInArgs { base_account } + } else { + CommitDelivery::StateInArgs + } +} + +/// Builds a [`CommitTask`] for `account`, used by both +/// [`crate::tasks::task_builder::TaskBuilderImpl`] (real task construction, +/// passing the real base-layer account state to diff against) and +/// [`crate::tasks::intent_size_validator::IntentSizeValidator`] (size +/// estimation, passing a stand-in base account purely to exercise this same +/// `COMMIT_STATE_SIZE_THRESHOLD` check). +pub fn create_commit_task( + commit_id: u64, + allow_undelegation: bool, + account: CommittedAccount, + base_account: Option, +) -> CommitTask { + let delivery_details = commit_delivery(&account, base_account); + + CommitTask { + commit_id, + allow_undelegation, + committed_account: account, + delivery_details, + } +} + +/// Same as [`create_commit_task`] but for [`CommitFinalizeTask`]. +pub fn create_commit_finalize_task( + commit_id: u64, + allow_undelegation: bool, + account: CommittedAccount, + base_account: Option, +) -> CommitFinalizeTask { + let delivery_details = commit_delivery(&account, base_account); + + CommitFinalizeTask { + commit_id, + allow_undelegation, + committed_account: account, + delivery: delivery_details, + } +} + pub struct TransactionUtils; impl TransactionUtils { const STANDALONE_ACTION_NOOP_PROGRAM_ID: Pubkey = diff --git a/magicblock-core/Cargo.toml b/magicblock-core/Cargo.toml index fe621634b..e1cd89dff 100644 --- a/magicblock-core/Cargo.toml +++ b/magicblock-core/Cargo.toml @@ -17,7 +17,7 @@ bytes = { workspace = true } serde = { workspace = true, features = ["derive"] } solana-account = { workspace = true } -solana-account-decoder = { workspace = true } +solana-account-decoder = { workspace = true, features = ["agave-unstable-api"] } solana-clock = { workspace = true } solana-hash = { workspace = true } solana-message = { workspace = true } @@ -27,7 +27,7 @@ solana-signature = { workspace = true } solana-transaction = { workspace = true, features = ["blake3", "verify"] } solana-transaction-context = { workspace = true } solana-transaction-error = { workspace = true } -solana-transaction-status-client-types = { workspace = true } +solana-transaction-status-client-types = { workspace = true, features = ["agave-unstable-api"] } magicblock-magic-program-api = { workspace = true } spl-token = { workspace = true } spl-token-2022 = { workspace = true } diff --git a/magicblock-core/src/intent/mod.rs b/magicblock-core/src/intent/mod.rs new file mode 100644 index 000000000..486379013 --- /dev/null +++ b/magicblock-core/src/intent/mod.rs @@ -0,0 +1,864 @@ +use std::collections::HashMap; + +use magicblock_magic_program_api::args::{ActionArgs, ShortAccountMeta}; +use serde::{Deserialize, Serialize}; +use solana_program::instruction::InstructionError; +use solana_pubkey::Pubkey; +use types::CommittedAccount; +pub mod types; + +/// Commits that are covered by User's dlp PDAs +pub const ACTUAL_COMMIT_LIMIT: u64 = 25; +/// Fixed fee per commit. +/// https://github.com/magicblock-labs/delegation-program/blob/main/src/consts.rs#L11 +pub const COMMIT_FEE_LAMPORTS: u64 = 100_000; +/// Price per compute unit for a BaseAction executed on Solana base chain, +/// denominated in micro-lamports per CU (mirrors Solana's priority fee model). +pub const COMPUTE_UNIT_PRICE_MICRO_LAMPORTS: u64 = 50_000; +pub const MISSING_COMMIT_NONCE_ERR: u32 = 0xA000_0001; + +// BaseIntent user wants to send to base layer +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub enum MagicBaseIntent { + /// Actions without commitment or undelegation + BaseActions(Vec), + Commit(CommitType), + CommitAndUndelegate(CommitAndUndelegate), + CommitFinalize(CommitType), + CommitFinalizeAndUndelegate(CommitAndUndelegate), +} + +// Bundle of BaseIntents +#[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct MagicIntentBundle { + pub commit: Option, + pub commit_and_undelegate: Option, + pub commit_finalize: Option, + pub commit_finalize_and_undelegate: Option, + pub standalone_actions: Vec, +} + +impl From for MagicIntentBundle { + fn from(value: MagicBaseIntent) -> Self { + let mut this = Self::default(); + match value { + MagicBaseIntent::BaseActions(value) => { + this.standalone_actions.extend(value) + } + MagicBaseIntent::Commit(value) => this.commit = Some(value), + MagicBaseIntent::CommitAndUndelegate(value) => { + this.commit_and_undelegate = Some(value) + } + MagicBaseIntent::CommitFinalize(value) => { + this.commit_finalize = Some(value) + } + MagicBaseIntent::CommitFinalizeAndUndelegate(value) => { + this.commit_finalize_and_undelegate = Some(value) + } + } + + this + } +} + +impl MagicIntentBundle { + pub fn calculate_fee( + &self, + commit_nonces: &HashMap, + ) -> Result { + let mut fee = 0; + if let Some(ref commit) = self.commit { + fee += commit.calculate_fee(commit_nonces)?; + } + if let Some(ref cau) = self.commit_and_undelegate { + fee += cau.calculate_fee(commit_nonces)?; + } + if let Some(ref commit_finalize) = self.commit_finalize { + fee += commit_finalize.calculate_fee(commit_nonces)?; + } + if let Some(ref cfau) = self.commit_finalize_and_undelegate { + fee += cfau.calculate_fee(commit_nonces)?; + } + fee += calculate_actions_fee(&self.standalone_actions); + Ok(fee) + } + + pub fn has_undelegate_intent(&self) -> bool { + self.commit_and_undelegate.is_some() + || self.commit_finalize_and_undelegate.is_some() + } + + pub fn has_committed_accounts(&self) -> bool { + let has_commit_intent_accounts = self + .get_commit_intent_accounts() + .map(|el| !el.is_empty()) + .unwrap_or(false); + let has_undelegate_intent_accounts = self + .get_undelegate_intent_accounts() + .map(|el| !el.is_empty()) + .unwrap_or(false); + let has_commit_finalize_intent_accounts = self + .get_commit_finalize_intent_accounts() + .map(|el| !el.is_empty()) + .unwrap_or(false); + let has_commit_finalize_and_undelegate_intent_accounts = self + .get_commit_finalize_and_undelegate_intent_accounts() + .map(|el| !el.is_empty()) + .unwrap_or(false); + + has_commit_intent_accounts + || has_undelegate_intent_accounts + || has_commit_finalize_intent_accounts + || has_commit_finalize_and_undelegate_intent_accounts + } + + /// Returns `[CommitAndUndelegate]` intent's accounts + pub fn get_undelegate_intent_accounts( + &self, + ) -> Option<&Vec> { + Some( + self.commit_and_undelegate + .as_ref()? + .get_committed_accounts(), + ) + } + + /// Returns `Commit` intent's accounts + pub fn get_commit_intent_accounts(&self) -> Option<&Vec> { + Some(self.commit.as_ref()?.get_committed_accounts()) + } + + /// Returns `[CommitAndUndelegate]` intent's accounts + pub fn get_commit_finalize_and_undelegate_intent_accounts( + &self, + ) -> Option<&Vec> { + Some( + self.commit_finalize_and_undelegate + .as_ref()? + .get_committed_accounts(), + ) + } + + /// Returns `Commit` intent's accounts + pub fn get_commit_finalize_intent_accounts( + &self, + ) -> Option<&Vec> { + Some(self.commit_finalize.as_ref()?.get_committed_accounts()) + } + + /// Returns `Commit` intent's accounts + pub fn get_commit_intent_accounts_mut( + &mut self, + ) -> Option<&mut Vec> { + Some(self.commit.as_mut()?.get_committed_accounts_mut()) + } + + /// Returns all the accounts that will be committed, + /// including the ones that will be undelegated as well + pub fn get_all_committed_accounts(&self) -> Vec { + let committed = self.get_commit_intent_accounts(); + let undelegated = self.get_undelegate_intent_accounts(); + let commit_finalize = self.get_commit_finalize_intent_accounts(); + let commit_finalize_and_undelegate = + self.get_commit_finalize_and_undelegate_intent_accounts(); + + [ + committed, + undelegated, + commit_finalize, + commit_finalize_and_undelegate, + ] + .into_iter() + .flatten() + .flatten() + .cloned() + .collect() + } + + pub fn get_all_committed_pubkeys(&self) -> Vec { + [ + self.get_commit_intent_pubkeys(), + self.get_undelegate_intent_pubkeys(), + self.get_commit_finalize_intent_pubkeys(), + self.get_commit_finalize_and_undelegate_intent_pubkeys(), + ] + .into_iter() + .flatten() + .flatten() + .collect() + } + + pub fn get_commit_intent_pubkeys(&self) -> Option> { + self.commit + .as_ref() + .map(|value| value.get_committed_pubkeys()) + } + + pub fn get_undelegate_intent_pubkeys(&self) -> Option> { + self.commit_and_undelegate + .as_ref() + .map(|value| value.get_committed_pubkeys()) + } + + pub fn get_commit_finalize_intent_pubkeys(&self) -> Option> { + self.commit_finalize + .as_ref() + .map(|value| value.get_committed_pubkeys()) + } + + pub fn get_commit_finalize_and_undelegate_intent_pubkeys( + &self, + ) -> Option> { + self.commit_finalize_and_undelegate + .as_ref() + .map(|value| value.get_committed_pubkeys()) + } + + pub fn is_empty(&self) -> bool { + let no_committed = + self.commit.as_ref().map(|el| el.is_empty()).unwrap_or(true); + + let no_committed_and_undelegated = self + .commit_and_undelegate + .as_ref() + .map(|el| el.is_empty()) + .unwrap_or(true); + + let no_commit_finalize = self + .commit_finalize + .as_ref() + .map(|el| el.is_empty()) + .unwrap_or(true); + + let no_commit_finalize_and_undelegate = self + .commit_finalize_and_undelegate + .as_ref() + .map(|el| el.is_empty()) + .unwrap_or(true); + + let no_actions = self.standalone_actions.is_empty(); + + no_committed + && no_committed_and_undelegated + && no_commit_finalize + && no_commit_finalize_and_undelegate + && no_actions + } + + pub fn has_callbacks(&self) -> bool { + let x = self + .commit + .as_ref() + .map(|el| el.has_callbacks()) + .unwrap_or(false); + let y = self + .commit_and_undelegate + .as_ref() + .map(|el| el.has_callbacks()) + .unwrap_or(false); + let cf = self + .commit_finalize + .as_ref() + .map(|el| el.has_callbacks()) + .unwrap_or(false); + let cfau = self + .commit_finalize_and_undelegate + .as_ref() + .map(|el| el.has_callbacks()) + .unwrap_or(false); + let z = self + .standalone_actions + .iter() + .any(|el| el.callback.is_some()); + + x || y || cf || cfau || z + } + + pub fn get_action_mut(&mut self, index: usize) -> Option<&mut BaseAction> { + let mut offset = 0usize; + + if let Some(commit) = self.commit.as_mut() { + let count = commit.action_count(); + if index < offset + count { + return commit.get_action_mut(index - offset); + } + offset += count; + } + + if let Some(cau) = self.commit_and_undelegate.as_mut() { + let count = cau.action_count(); + if index < offset + count { + return cau.get_action_mut(index - offset); + } + offset += count; + } + + if let Some(commit_finalize) = self.commit_finalize.as_mut() { + let count = commit_finalize.action_count(); + if index < offset + count { + return commit_finalize.get_action_mut(index - offset); + } + offset += count; + } + + if let Some(cfau) = self.commit_finalize_and_undelegate.as_mut() { + let count = cfau.action_count(); + if index < offset + count { + return cfau.get_action_mut(index - offset); + } + offset += count; + } + + self.standalone_actions.get_mut(index.checked_sub(offset)?) + } +} + +impl MagicBaseIntent { + pub fn is_undelegate(&self) -> bool { + match &self { + MagicBaseIntent::BaseActions(_) => false, + MagicBaseIntent::Commit(_) => false, + MagicBaseIntent::CommitAndUndelegate(_) => true, + MagicBaseIntent::CommitFinalize(_) => false, + MagicBaseIntent::CommitFinalizeAndUndelegate(_) => true, + } + } + + pub fn is_commit_finalize(&self) -> bool { + match &self { + MagicBaseIntent::BaseActions(_) => false, + MagicBaseIntent::Commit(_) => false, + MagicBaseIntent::CommitAndUndelegate(_) => false, + MagicBaseIntent::CommitFinalize(_) => true, + MagicBaseIntent::CommitFinalizeAndUndelegate(_) => true, + } + } + + pub fn get_committed_accounts(&self) -> Option<&Vec> { + match self { + MagicBaseIntent::BaseActions(_) => None, + MagicBaseIntent::Commit(t) => Some(t.get_committed_accounts()), + MagicBaseIntent::CommitAndUndelegate(t) => { + Some(t.get_committed_accounts()) + } + MagicBaseIntent::CommitFinalize(t) => { + Some(t.get_committed_accounts()) + } + MagicBaseIntent::CommitFinalizeAndUndelegate(t) => { + Some(t.get_committed_accounts()) + } + } + } + + pub fn get_committed_accounts_mut( + &mut self, + ) -> Option<&mut Vec> { + match self { + MagicBaseIntent::BaseActions(_) => None, + MagicBaseIntent::Commit(t) => Some(t.get_committed_accounts_mut()), + MagicBaseIntent::CommitAndUndelegate(t) => { + Some(t.get_committed_accounts_mut()) + } + MagicBaseIntent::CommitFinalize(t) => { + Some(t.get_committed_accounts_mut()) + } + MagicBaseIntent::CommitFinalizeAndUndelegate(t) => { + Some(t.get_committed_accounts_mut()) + } + } + } + + pub fn get_committed_pubkeys(&self) -> Option> { + self.get_committed_accounts().map(|accounts| { + accounts.iter().map(|account| account.pubkey).collect() + }) + } + + pub fn is_empty(&self) -> bool { + match self { + MagicBaseIntent::BaseActions(actions) => actions.is_empty(), + MagicBaseIntent::Commit(t) => t.is_empty(), + MagicBaseIntent::CommitAndUndelegate(t) => t.is_empty(), + MagicBaseIntent::CommitFinalize(t) => t.is_empty(), + MagicBaseIntent::CommitFinalizeAndUndelegate(t) => t.is_empty(), + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct CommitAndUndelegate { + pub commit_action: CommitType, + pub undelegate_action: UndelegateType, +} + +impl CommitAndUndelegate { + pub fn calculate_fee( + &self, + commit_nonces: &HashMap, + ) -> Result { + let mut fee = 0; + fee += self.commit_action.calculate_fee(commit_nonces)?; + fee += self.undelegate_action.calculate_fee(commit_nonces)?; + Ok(fee) + } + + pub fn get_committed_accounts(&self) -> &Vec { + self.commit_action.get_committed_accounts() + } + + pub fn get_committed_accounts_mut(&mut self) -> &mut Vec { + self.commit_action.get_committed_accounts_mut() + } + + pub fn get_committed_pubkeys(&self) -> Vec { + self.commit_action.get_committed_pubkeys() + } + + pub fn is_empty(&self) -> bool { + self.commit_action.is_empty() + } + + pub fn has_callbacks(&self) -> bool { + let x = self.commit_action.has_callbacks(); + let y = if let UndelegateType::WithBaseActions(actions) = + &self.undelegate_action + { + actions.iter().any(|el| el.callback.is_some()) + } else { + false + }; + + x || y + } + + pub fn action_count(&self) -> usize { + self.commit_action.action_count() + + self.undelegate_action.action_count() + } + + pub fn get_action_mut(&mut self, index: usize) -> Option<&mut BaseAction> { + let commit_count = self.commit_action.action_count(); + if index < commit_count { + return self.commit_action.get_action_mut(index); + } + self.undelegate_action.get_action_mut(index - commit_count) + } +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ProgramArgs { + pub escrow_index: u8, + pub data: Vec, +} + +impl From for ProgramArgs { + fn from(value: ActionArgs) -> Self { + Self { + escrow_index: value.escrow_index, + data: value.data, + } + } +} + +impl From<&ActionArgs> for ProgramArgs { + fn from(value: &ActionArgs) -> Self { + value.clone().into() + } +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct BaseAction { + pub compute_units: u32, + pub destination_program: Pubkey, + pub source_program: Option, + pub escrow_authority: Pubkey, + pub data_per_program: ProgramArgs, + pub account_metas_per_program: Vec, + pub callback: Option, +} + +/// A callback that is execution with result of BaseAction +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct BaseActionCallback { + pub destination_program: Pubkey, + pub discriminator: Vec, + pub payload: Vec, + pub compute_units: u32, + pub account_metas_per_program: Vec, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub enum CommitType { + /// Regular commit without actions + Standalone(Vec), // accounts to commit + /// Commits accounts and runs actions + WithBaseActions { + committed_accounts: Vec, + base_actions: Vec, + }, +} + +impl CommitType { + /// Calculate fee commits + pub fn calculate_fee( + &self, + commit_nonces: &HashMap, + ) -> Result { + let mut fee = 0; + match self { + CommitType::Standalone(ref committed_accounts) => { + fee += calculate_commit_fee(committed_accounts, commit_nonces)?; + } + CommitType::WithBaseActions { + committed_accounts, + base_actions, + } => { + fee += calculate_commit_fee(committed_accounts, commit_nonces)?; + fee += calculate_actions_fee(base_actions); + } + } + + Ok(fee) + } + + pub fn get_committed_accounts(&self) -> &Vec { + match self { + Self::Standalone(committed_accounts) => committed_accounts, + Self::WithBaseActions { + committed_accounts, .. + } => committed_accounts, + } + } + + pub fn get_committed_accounts_mut(&mut self) -> &mut Vec { + match self { + Self::Standalone(committed_accounts) => committed_accounts, + Self::WithBaseActions { + committed_accounts, .. + } => committed_accounts, + } + } + + pub fn get_committed_pubkeys(&self) -> Vec { + self.get_committed_accounts() + .iter() + .map(|account| account.pubkey) + .collect() + } + + pub fn is_empty(&self) -> bool { + match self { + Self::Standalone(committed_accounts) => { + committed_accounts.is_empty() + } + Self::WithBaseActions { + committed_accounts, .. + } => committed_accounts.is_empty(), + } + } + + pub fn has_callbacks(&self) -> bool { + if let Self::WithBaseActions { + committed_accounts: _, + base_actions, + } = self + { + base_actions.iter().any(|el| el.callback.is_some()) + } else { + false + } + } + + pub fn action_count(&self) -> usize { + match self { + Self::Standalone(_) => 0, + Self::WithBaseActions { base_actions, .. } => base_actions.len(), + } + } + + pub fn get_action_mut(&mut self, index: usize) -> Option<&mut BaseAction> { + if let Self::WithBaseActions { base_actions, .. } = self { + base_actions.get_mut(index) + } else { + None + } + } +} + +/// No CommitedAccounts since it is only used with CommitAction. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub enum UndelegateType { + Standalone, + WithBaseActions(Vec), +} + +impl UndelegateType { + pub fn action_count(&self) -> usize { + match self { + Self::Standalone => 0, + Self::WithBaseActions(actions) => actions.len(), + } + } + + pub fn get_action_mut(&mut self, index: usize) -> Option<&mut BaseAction> { + if let Self::WithBaseActions(actions) = self { + actions.get_mut(index) + } else { + None + } + } + + pub fn calculate_fee( + &self, + _commit_nonces: &HashMap, + ) -> Result { + match self { + UndelegateType::Standalone => Ok(0), + UndelegateType::WithBaseActions(actions) => { + Ok(calculate_actions_fee(actions)) + } + } + } +} + +pub fn calculate_commit_fee( + accounts: &[CommittedAccount], + commit_nonces: &HashMap, +) -> Result { + accounts.iter().try_fold(0u64, |fee, account| { + if let Some(nonce) = commit_nonces.get(&account.pubkey) { + if nonce >= &ACTUAL_COMMIT_LIMIT { + Ok(fee + COMMIT_FEE_LAMPORTS) + } else { + Ok(fee) + } + } else { + Err(InstructionError::Custom(MISSING_COMMIT_NONCE_ERR)) + } + }) +} + +fn calculate_actions_fee(actions: &[BaseAction]) -> 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, + ) + }); + micro_lamports.div_ceil(MICRO_LAMPORTS_PER_LAMPORT) +} + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use solana_program::instruction::InstructionError; + use solana_pubkey::Pubkey; + + use crate::intent::{ + calculate_actions_fee, calculate_commit_fee, types::CommittedAccount, + BaseAction, BaseActionCallback, CommitAndUndelegate, CommitType, + MagicIntentBundle, ProgramArgs, UndelegateType, ACTUAL_COMMIT_LIMIT, + COMMIT_FEE_LAMPORTS, MISSING_COMMIT_NONCE_ERR, + }; + + fn make_committed_account(pubkey: Pubkey) -> CommittedAccount { + CommittedAccount { + pubkey, + account: solana_account::Account::default(), + remote_slot: 0, + } + } + + fn make_base_action(compute_units: u32) -> BaseAction { + BaseAction { + compute_units, + destination_program: Pubkey::new_unique(), + source_program: None, + escrow_authority: Pubkey::new_unique(), + data_per_program: ProgramArgs { + escrow_index: 0, + data: vec![], + }, + account_metas_per_program: vec![], + callback: None, + } + } + + // ---- calculate_commit_fee ---- + + #[test] + fn test_commit_fee_at_limit_is_zero() { + let pk = Pubkey::new_unique(); + // nonce is commits done so far; nonce+1 is the next commit number. + // ACTUAL_COMMIT_LIMIT - 1 means the next commit is exactly at the limit → free. + let nonces = HashMap::from([(pk, ACTUAL_COMMIT_LIMIT - 1)]); + let fee = calculate_commit_fee(&[make_committed_account(pk)], &nonces) + .unwrap(); + assert_eq!(fee, 0); + } + + #[test] + fn test_commit_fee_above_limit_charges_per_account() { + let pk1 = Pubkey::new_unique(); + let pk2 = Pubkey::new_unique(); + let nonces = HashMap::from([ + (pk1, ACTUAL_COMMIT_LIMIT + 1), + (pk2, ACTUAL_COMMIT_LIMIT + 1), + ]); + let fee = calculate_commit_fee( + &[make_committed_account(pk1), make_committed_account(pk2)], + &nonces, + ) + .unwrap(); + assert_eq!(fee, COMMIT_FEE_LAMPORTS * 2); + } + + #[test] + fn test_commit_fee_mixed_accounts() { + let pk_below = Pubkey::new_unique(); + let pk_above = Pubkey::new_unique(); + let nonces = HashMap::from([ + (pk_below, ACTUAL_COMMIT_LIMIT - 1), // next commit is exactly at limit → free + (pk_above, ACTUAL_COMMIT_LIMIT), // next commit exceeds limit → charged + ]); + let fee = calculate_commit_fee( + &[ + make_committed_account(pk_below), + make_committed_account(pk_above), + ], + &nonces, + ) + .unwrap(); + assert_eq!(fee, COMMIT_FEE_LAMPORTS); + } + + #[test] + fn test_commit_fee_missing_nonce_errors() { + let pk = Pubkey::new_unique(); + let err = calculate_commit_fee( + &[make_committed_account(pk)], + &HashMap::new(), + ) + .unwrap_err(); + assert_eq!(err, InstructionError::Custom(MISSING_COMMIT_NONCE_ERR)); + } + + /// two actions of 200_000 CUs each → 20_000 lamports + #[test] + fn test_actions_fee_multiple_actions() { + assert_eq!( + calculate_actions_fee(&[ + make_base_action(200_000), + make_base_action(200_000) + ]), + 20_000 + ); + } + + // ---- MagicIntentBundle::calculate_fee / has_callbacks / get_action_mut + // must include commit_finalize and commit_finalize_and_undelegate ---- + + #[test] + fn test_calculate_fee_includes_commit_finalize_variants() { + let pk = Pubkey::new_unique(); + let nonces = HashMap::from([(pk, ACTUAL_COMMIT_LIMIT)]); // over limit -> charged + let expected = COMMIT_FEE_LAMPORTS + 10_000; // + one 200_000 CU action + + let commit_finalize_bundle = MagicIntentBundle { + commit_finalize: Some(CommitType::WithBaseActions { + committed_accounts: vec![make_committed_account(pk)], + base_actions: vec![make_base_action(200_000)], + }), + ..Default::default() + }; + assert_eq!( + commit_finalize_bundle.calculate_fee(&nonces).unwrap(), + expected + ); + + let commit_finalize_and_undelegate_bundle = MagicIntentBundle { + commit_finalize_and_undelegate: Some(CommitAndUndelegate { + commit_action: CommitType::Standalone(vec![ + make_committed_account(pk), + ]), + undelegate_action: UndelegateType::WithBaseActions(vec![ + make_base_action(200_000), + ]), + }), + ..Default::default() + }; + assert_eq!( + commit_finalize_and_undelegate_bundle + .calculate_fee(&nonces) + .unwrap(), + expected + ); + } + + #[test] + fn test_has_callbacks_detects_commit_finalize_variants() { + fn make_base_action_with_callback() -> BaseAction { + let mut action = make_base_action(0); + action.callback = Some(BaseActionCallback { + destination_program: Pubkey::new_unique(), + discriminator: vec![], + payload: vec![], + compute_units: 0, + account_metas_per_program: vec![], + }); + action + } + + let commit_finalize_bundle = MagicIntentBundle { + commit_finalize: Some(CommitType::WithBaseActions { + committed_accounts: vec![], + base_actions: vec![make_base_action_with_callback()], + }), + ..Default::default() + }; + assert!(commit_finalize_bundle.has_callbacks()); + + let commit_finalize_and_undelegate_bundle = MagicIntentBundle { + commit_finalize_and_undelegate: Some(CommitAndUndelegate { + commit_action: CommitType::Standalone(vec![]), + undelegate_action: UndelegateType::WithBaseActions(vec![ + make_base_action_with_callback(), + ]), + }), + ..Default::default() + }; + assert!(commit_finalize_and_undelegate_bundle.has_callbacks()); + + // No callbacks anywhere -> false, so the above aren't vacuously true. + let no_callbacks_bundle = MagicIntentBundle { + commit_finalize: Some(CommitType::WithBaseActions { + committed_accounts: vec![], + base_actions: vec![make_base_action(0)], + }), + ..Default::default() + }; + assert!(!no_callbacks_bundle.has_callbacks()); + } + + #[test] + fn test_get_action_mut_indexes_commit_finalize_variants() { + let mut bundle = MagicIntentBundle { + commit: Some(CommitType::WithBaseActions { + committed_accounts: vec![], + base_actions: vec![make_base_action(111)], + }), + commit_finalize: Some(CommitType::WithBaseActions { + committed_accounts: vec![], + base_actions: vec![make_base_action(222)], + }), + standalone_actions: vec![make_base_action(333)], + ..Default::default() + }; + + assert_eq!(bundle.get_action_mut(0).unwrap().compute_units, 111); + assert_eq!(bundle.get_action_mut(1).unwrap().compute_units, 222); + assert_eq!(bundle.get_action_mut(2).unwrap().compute_units, 333); + assert!(bundle.get_action_mut(3).is_none()); + } +} diff --git a/magicblock-core/src/intent.rs b/magicblock-core/src/intent/types.rs similarity index 79% rename from magicblock-core/src/intent.rs rename to magicblock-core/src/intent/types.rs index d66b0b1fd..7f9a3e622 100644 --- a/magicblock-core/src/intent.rs +++ b/magicblock-core/src/intent/types.rs @@ -1,7 +1,6 @@ -use magicblock_magic_program_api::args::ShortAccountMeta; use serde::{Deserialize, Serialize}; use solana_account::{Account, AccountSharedData}; -use solana_pubkey::Pubkey; +use solana_message::Address as Pubkey; use crate::token_programs::try_remap_ata_to_eata; @@ -55,13 +54,3 @@ impl CommittedAccount { } } } - -/// A callback that is execution with result of BaseAction -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct BaseActionCallback { - pub destination_program: Pubkey, - pub discriminator: Vec, - pub payload: Vec, - pub compute_units: u32, - pub account_metas_per_program: Vec, -} diff --git a/magicblock-core/src/traits.rs b/magicblock-core/src/traits.rs index 7ccc27bae..435d8f2ae 100644 --- a/magicblock-core/src/traits.rs +++ b/magicblock-core/src/traits.rs @@ -8,7 +8,7 @@ use solana_signature::Signature; use solana_transaction_error::TransactionError; use crate::{ - intent::{BaseActionCallback, CommittedAccount}, + intent::{types::CommittedAccount, BaseActionCallback, MagicIntentBundle}, Slot, }; @@ -19,6 +19,17 @@ pub trait MagicSys: Sync + Send + 'static { &self, commits: &[CommittedAccount], ) -> Result, InstructionError>; + + /// Returns `Ok(())` if `intent` could ever fit on the base layer, even + /// under the smallest possible task representation (buffer-mode commits, + /// address lookup tables, 2-stage execution). If it can never fit no + /// matter how it gets optimized at execution time, returns `Err` so the + /// intent can be refused up front instead of failing later during + /// commit/finalize. + fn validate_intent_size( + &self, + intent: &MagicIntentBundle, + ) -> Result<(), InstructionError>; } /// Provides read access to the latest confirmed block's metadata. diff --git a/magicblock-metrics/src/metrics/mod.rs b/magicblock-metrics/src/metrics/mod.rs index dfccf1239..fc4c6b7a1 100644 --- a/magicblock-metrics/src/metrics/mod.rs +++ b/magicblock-metrics/src/metrics/mod.rs @@ -382,6 +382,43 @@ lazy_static::lazy_static! { "committor_intent_cu_usage_gauge", "Compute units used for Intent" ).unwrap(); + static ref COMMITTOR_INTENT_TASK_PREPARATION_TIME: HistogramVec = HistogramVec::new( + HistogramOpts::new( + "committor_intent_task_preparation_time", + "Time in seconds spent on task preparation" + ) + .buckets( + vec![0.1, 1.0, 2.0, 3.0, 5.0] + ), + &["task_type"], + ).unwrap(); + + static ref COMMITTOR_INTENT_ALT_PREPARATION_TIME: Histogram = Histogram::with_opts( + HistogramOpts::new( + "committor_intent_alt_preparation_time", + "Time in seconds spent on ALTs preparation" + ) + .buckets( + vec![1.0, 3.0, 5.0, 10.0, 15.0, 17.0, 20.0] + ), + ).unwrap(); + + static ref COMMITTOR_INTENT_ALT_COUNT: Histogram = Histogram::with_opts( + HistogramOpts::new( + "committor_intent_alt_count", + "Number of address lookup tables used per intent transaction (only recorded when ALTs are present)" + ) + .buckets(vec![0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 10.0]) + ).unwrap(); + + static ref COMMITTOR_FETCH_COMMIT_NONCES_WAIT_TIME: Histogram = Histogram::with_opts( + HistogramOpts::new( + "committor_fetch_commit_nonces_wait_time_second", + "Time in seconds spent waiting for fetch_current_commit_nonces response" + ) + .buckets(vec![0.1, 0.5, 1.0, 2.0, 5.0, 10.0, 20.0, 30.0]), + ).unwrap(); + // GetMultiplAccount investigation static ref REMOTE_ACCOUNT_PROVIDER_A_COUNT: IntCounter = IntCounter::new( "remote_account_provider_a_count", "Get mupltiple account count" @@ -429,44 +466,6 @@ lazy_static::lazy_static! { "Number of signatures included in batched getSignatureStatuses requests" ).unwrap(); - - static ref COMMITTOR_INTENT_TASK_PREPARATION_TIME: HistogramVec = HistogramVec::new( - HistogramOpts::new( - "committor_intent_task_preparation_time", - "Time in seconds spent on task preparation" - ) - .buckets( - vec![0.1, 1.0, 2.0, 3.0, 5.0] - ), - &["task_type"], - ).unwrap(); - - static ref COMMITTOR_INTENT_ALT_PREPARATION_TIME: Histogram = Histogram::with_opts( - HistogramOpts::new( - "committor_intent_alt_preparation_time", - "Time in seconds spent on ALTs preparation" - ) - .buckets( - vec![1.0, 3.0, 5.0, 10.0, 15.0, 17.0, 20.0] - ), - ).unwrap(); - - static ref COMMITTOR_INTENT_ALT_COUNT: Histogram = Histogram::with_opts( - HistogramOpts::new( - "committor_intent_alt_count", - "Number of address lookup tables used per intent transaction (only recorded when ALTs are present)" - ) - .buckets(vec![0.0, 1.0, 2.0, 3.0, 4.0, 5.0, 10.0]) - ).unwrap(); - - static ref COMMITTOR_FETCH_COMMIT_NONCES_WAIT_TIME: Histogram = Histogram::with_opts( - HistogramOpts::new( - "committor_fetch_commit_nonces_wait_time_second", - "Time in seconds spent waiting for fetch_current_commit_nonces response" - ) - .buckets(vec![0.1, 0.5, 1.0, 2.0, 5.0, 10.0, 20.0, 30.0]), - ).unwrap(); - // ----------------- // Pubsub Clients // ----------------- diff --git a/programs/magicblock/src/magic_scheduled_base_intent.rs b/programs/magicblock/src/magic_scheduled_base_intent.rs index 88050bf62..a7299a41b 100644 --- a/programs/magicblock/src/magic_scheduled_base_intent.rs +++ b/programs/magicblock/src/magic_scheduled_base_intent.rs @@ -1,16 +1,21 @@ use std::collections::{HashMap, HashSet}; +pub use magicblock_core::intent::{ + calculate_commit_fee, BaseAction, CommitAndUndelegate, CommitType, + MagicBaseIntent, MagicIntentBundle, ProgramArgs, UndelegateType, + ACTUAL_COMMIT_LIMIT, COMMIT_FEE_LAMPORTS, + COMPUTE_UNIT_PRICE_MICRO_LAMPORTS, +}; use magicblock_core::{ - intent::{BaseActionCallback, CommittedAccount}, + intent::types::CommittedAccount, token_programs::{ EATA_PROGRAM_ID, TOKEN_2022_PROGRAM_ID, TOKEN_PROGRAM_ID, }, Slot, }; use magicblock_magic_program_api::args::{ - ActionArgs, BaseActionArgs, CommitAndUndelegateArgs, CommitTypeArgs, - MagicBaseIntentArgs, MagicIntentBundleArgs, ShortAccountMeta, - UndelegateTypeArgs, + BaseActionArgs, CommitAndUndelegateArgs, CommitTypeArgs, + MagicBaseIntentArgs, MagicIntentBundleArgs, UndelegateTypeArgs, }; use serde::{Deserialize, Serialize}; use solana_account::ReadableAccount; @@ -25,7 +30,7 @@ use solana_transaction::Transaction; use crate::{ instruction_utils::InstructionUtils, - magic_sys::MISSING_COMMIT_NONCE_ERR, + magic_sys::validate_intent_size, utils::accounts::{ get_instruction_account_with_idx, get_instruction_pubkey_with_idx, get_writable_with_idx, InstructionAccount, @@ -33,15 +38,6 @@ use crate::{ validator::effective_validator_authority_id, }; -/// Commits that are covered by User's dlp PDAs -pub const ACTUAL_COMMIT_LIMIT: u64 = 25; -/// Fixed fee per commit. -/// https://github.com/magicblock-labs/delegation-program/blob/main/src/consts.rs#L11 -pub const COMMIT_FEE_LAMPORTS: u64 = 100_000; -/// Price per compute unit for a BaseAction executed on Solana base chain, -/// denominated in micro-lamports per CU (mirrors Solana's priority fee model). -pub const COMPUTE_UNIT_PRICE_MICRO_LAMPORTS: u64 = 50_000; - /// Context necessary for construction of Schedule Action pub struct ConstructionContext<'a, 'ic, 'ix_data> { parent_program_id: Option, @@ -199,56 +195,26 @@ impl ScheduledIntentBundle { } } -// BaseIntent user wants to send to base layer -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub enum MagicBaseIntent { - /// Actions without commitment or undelegation - BaseActions(Vec), - Commit(CommitType), - CommitAndUndelegate(CommitAndUndelegate), - CommitFinalize(CommitType), - CommitFinalizeAndUndelegate(CommitAndUndelegate), -} - -// Bundle of BaseIntents -#[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct MagicIntentBundle { - pub commit: Option, - pub commit_and_undelegate: Option, - pub commit_finalize: Option, - pub commit_finalize_and_undelegate: Option, - pub standalone_actions: Vec, -} - -impl From for MagicIntentBundle { - fn from(value: MagicBaseIntent) -> Self { - let mut this = Self::default(); - match value { - MagicBaseIntent::BaseActions(value) => { - this.standalone_actions.extend(value) - } - MagicBaseIntent::Commit(value) => this.commit = Some(value), - MagicBaseIntent::CommitAndUndelegate(value) => { - this.commit_and_undelegate = Some(value) - } - MagicBaseIntent::CommitFinalize(value) => { - this.commit_finalize = Some(value) - } - MagicBaseIntent::CommitFinalizeAndUndelegate(value) => { - this.commit_finalize_and_undelegate = Some(value) - } - } - - this - } +/// Constructs a type from its wire [`Args`] representation. +/// +/// The corresponding data types live in [`magicblock_core::intent::types`] +/// since they need to be shared with `MagicSys` implementors, but the +/// construction logic stays here since it depends on [`ConstructionContext`] +/// (`InvokeContext`, `ic_msg!`, etc.) which only exists on-chain. A local +/// trait lets us `impl` construction for those foreign types. +pub trait TryFromArgs: Sized { + fn try_from_args( + args: A, + context: &ConstructionContext<'_, '_, '_>, + ) -> Result; } -impl MagicIntentBundle { - pub fn try_from_args( +impl TryFromArgs for MagicIntentBundle { + fn try_from_args( args: MagicIntentBundleArgs, context: &ConstructionContext<'_, '_, '_>, ) -> Result { - Self::validate(&args, context)?; + validate_magic_intent_bundle_args(&args, context)?; let commit = args .commit @@ -283,317 +249,105 @@ impl MagicIntentBundle { commit_finalize_and_undelegate, standalone_actions: actions, }; - this.post_validation(context)?; - - Ok(this) - } - - /// Cross intent validation: - /// 1. Set of committed accounts shall not overlap with - /// set of undelegated accounts - /// 2. None for now :) - fn validate( - args: &MagicIntentBundleArgs, - context: &ConstructionContext<'_, '_, '_>, - ) -> Result<(), InstructionError> { - let committed_set: Option> = - args.commit.as_ref().map(|el| { - el.committed_accounts_indices().iter().copied().collect() - }); - let Some(committed_set) = committed_set else { - return Ok(()); - }; - - args.commit_and_undelegate - .as_ref() - .map(|el| { - let has_cross_reference = el - .committed_accounts_indices() - .iter() - .any(|ind| committed_set.contains(ind)); - if has_cross_reference { - ic_msg!( - context.invoke_context, - "ScheduleCommit ERR: duplicate committed account across bundle", - ); - Err(InstructionError::InvalidInstructionData) - } else { - Ok(()) - } - }) - .unwrap_or(Ok(())) - } - - /// Post cross intent validation: - /// 1. Validates that all committed accounts across the entire intent bundle - /// are globally unique by pubkey. - fn post_validation( - &self, - context: &ConstructionContext<'_, '_, '_>, - ) -> Result<(), InstructionError> { - if self.is_empty() { + post_validate_magic_intent_bundle(&this, context)?; + validate_intent_size(&this).inspect_err(|_| { ic_msg!( context.invoke_context, - "ScheduleCommit ERR: intent bundle must not be empty.", + "ScheduleCommit ERR: intent is too large to ever fit into transaction", ); - return Err(InstructionError::InvalidInstructionData); - } - - let mut seen = HashSet::::new(); - let mut check = - |accounts: &Vec| -> Result<(), InstructionError> { - for el in accounts { - if !seen.insert(el.pubkey) { - ic_msg!( - context.invoke_context, - "ScheduleCommit ERR: duplicate committed account pubkey across bundle: {}", - el.pubkey - ); - return Err(InstructionError::InvalidInstructionData); - } - } - Ok(()) - }; + })?; - if let Some(commit) = &self.commit { - check(commit.get_committed_accounts())?; - } - if let Some(cau) = &self.commit_and_undelegate { - check(cau.get_committed_accounts())?; - } - if let Some(commit_finalize) = &self.commit_finalize { - check(commit_finalize.get_committed_accounts())?; - } - if let Some(commit_finalize_and_undelegate) = - &self.commit_finalize_and_undelegate - { - check(commit_finalize_and_undelegate.get_committed_accounts())?; - } - - Ok(()) - } - - pub fn calculate_fee( - &self, - commit_nonces: &HashMap, - ) -> Result { - let mut fee = 0; - if let Some(ref commit) = self.commit { - fee += commit.calculate_fee(commit_nonces)?; - } - if let Some(ref cau) = self.commit_and_undelegate { - fee += cau.calculate_fee(commit_nonces)?; - } - fee += calculate_actions_fee(&self.standalone_actions); - Ok(fee) - } - - pub fn has_undelegate_intent(&self) -> bool { - self.commit_and_undelegate.is_some() - || self.commit_finalize_and_undelegate.is_some() - } - - pub fn has_committed_accounts(&self) -> bool { - let has_commit_intent_accounts = self - .get_commit_intent_accounts() - .map(|el| !el.is_empty()) - .unwrap_or(false); - let has_undelegate_intent_accounts = self - .get_undelegate_intent_accounts() - .map(|el| !el.is_empty()) - .unwrap_or(false); - let has_commit_finalize_intent_accounts = self - .get_commit_finalize_intent_accounts() - .map(|el| !el.is_empty()) - .unwrap_or(false); - let has_commit_finalize_and_undelegate_intent_accounts = self - .get_commit_finalize_and_undelegate_intent_accounts() - .map(|el| !el.is_empty()) - .unwrap_or(false); - - has_commit_intent_accounts - || has_undelegate_intent_accounts - || has_commit_finalize_intent_accounts - || has_commit_finalize_and_undelegate_intent_accounts - } - - /// Returns `[CommitAndUndelegate]` intent's accounts - pub fn get_undelegate_intent_accounts( - &self, - ) -> Option<&Vec> { - Some( - self.commit_and_undelegate - .as_ref()? - .get_committed_accounts(), - ) - } - - /// Returns `Commit` intent's accounts - pub fn get_commit_intent_accounts(&self) -> Option<&Vec> { - Some(self.commit.as_ref()?.get_committed_accounts()) - } - - /// Returns `[CommitAndUndelegate]` intent's accounts - pub fn get_commit_finalize_and_undelegate_intent_accounts( - &self, - ) -> Option<&Vec> { - Some( - self.commit_finalize_and_undelegate - .as_ref()? - .get_committed_accounts(), - ) - } - - /// Returns `Commit` intent's accounts - pub fn get_commit_finalize_intent_accounts( - &self, - ) -> Option<&Vec> { - Some(self.commit_finalize.as_ref()?.get_committed_accounts()) - } - - /// Returns `Commit` intent's accounts - pub fn get_commit_intent_accounts_mut( - &mut self, - ) -> Option<&mut Vec> { - Some(self.commit.as_mut()?.get_committed_accounts_mut()) - } - - /// Returns all the accounts that will be committed, - /// including the ones that will be undelegated as well - pub fn get_all_committed_accounts(&self) -> Vec { - let committed = self.get_commit_intent_accounts(); - let undelegated = self.get_undelegate_intent_accounts(); - let commit_finalize = self.get_commit_finalize_intent_accounts(); - let commit_finalize_and_undelegate = - self.get_commit_finalize_and_undelegate_intent_accounts(); - - [ - committed, - undelegated, - commit_finalize, - commit_finalize_and_undelegate, - ] - .into_iter() - .flatten() - .flatten() - .cloned() - .collect() + Ok(this) } +} - pub fn get_all_committed_pubkeys(&self) -> Vec { - [ - self.get_commit_intent_pubkeys(), - self.get_undelegate_intent_pubkeys(), - self.get_commit_finalize_intent_pubkeys(), - self.get_commit_finalize_and_undelegate_intent_pubkeys(), - ] - .into_iter() - .flatten() - .flatten() - .collect() - } +/// Cross intent validation: +/// 1. Set of committed accounts shall not overlap with +/// set of undelegated accounts +/// 2. None for now :) +fn validate_magic_intent_bundle_args( + args: &MagicIntentBundleArgs, + context: &ConstructionContext<'_, '_, '_>, +) -> Result<(), InstructionError> { + let committed_set: Option> = args + .commit + .as_ref() + .map(|el| el.committed_accounts_indices().iter().copied().collect()); + let Some(committed_set) = committed_set else { + return Ok(()); + }; + + args.commit_and_undelegate + .as_ref() + .map(|el| { + let has_cross_reference = el + .committed_accounts_indices() + .iter() + .any(|ind| committed_set.contains(ind)); + if has_cross_reference { + ic_msg!( + context.invoke_context, + "ScheduleCommit ERR: duplicate committed account across bundle", + ); + Err(InstructionError::InvalidInstructionData) + } else { + Ok(()) + } + }) + .unwrap_or(Ok(())) +} - pub fn get_commit_intent_pubkeys(&self) -> Option> { - self.commit - .as_ref() - .map(|value| value.get_committed_pubkeys()) +/// Post cross intent validation: +/// 1. Validates that all committed accounts across the entire intent bundle +/// are globally unique by pubkey. +fn post_validate_magic_intent_bundle( + bundle: &MagicIntentBundle, + context: &ConstructionContext<'_, '_, '_>, +) -> Result<(), InstructionError> { + if bundle.is_empty() { + ic_msg!( + context.invoke_context, + "ScheduleCommit ERR: intent bundle must not be empty.", + ); + return Err(InstructionError::InvalidInstructionData); } - pub fn get_undelegate_intent_pubkeys(&self) -> Option> { - self.commit_and_undelegate - .as_ref() - .map(|value| value.get_committed_pubkeys()) - } + let mut seen = HashSet::::new(); + let mut check = + |accounts: &Vec| -> Result<(), InstructionError> { + for el in accounts { + if !seen.insert(el.pubkey) { + ic_msg!( + context.invoke_context, + "ScheduleCommit ERR: duplicate committed account pubkey across bundle: {}", + el.pubkey + ); + return Err(InstructionError::InvalidInstructionData); + } + } + Ok(()) + }; - pub fn get_commit_finalize_intent_pubkeys(&self) -> Option> { - self.commit_finalize - .as_ref() - .map(|value| value.get_committed_pubkeys()) + if let Some(commit) = &bundle.commit { + check(commit.get_committed_accounts())?; } - - pub fn get_commit_finalize_and_undelegate_intent_pubkeys( - &self, - ) -> Option> { - self.commit_finalize_and_undelegate - .as_ref() - .map(|value| value.get_committed_pubkeys()) + if let Some(cau) = &bundle.commit_and_undelegate { + check(cau.get_committed_accounts())?; } - - pub fn is_empty(&self) -> bool { - let no_committed = - self.commit.as_ref().map(|el| el.is_empty()).unwrap_or(true); - - let no_committed_and_undelegated = self - .commit_and_undelegate - .as_ref() - .map(|el| el.is_empty()) - .unwrap_or(true); - - let no_commit_finalize = self - .commit_finalize - .as_ref() - .map(|el| el.is_empty()) - .unwrap_or(true); - - let no_commit_finalize_and_undelegate = self - .commit_finalize_and_undelegate - .as_ref() - .map(|el| el.is_empty()) - .unwrap_or(true); - - let no_actions = self.standalone_actions.is_empty(); - - no_committed - && no_committed_and_undelegated - && no_commit_finalize - && no_commit_finalize_and_undelegate - && no_actions + if let Some(commit_finalize) = &bundle.commit_finalize { + check(commit_finalize.get_committed_accounts())?; } - - pub fn has_callbacks(&self) -> bool { - let x = self - .commit - .as_ref() - .map(|el| el.has_callbacks()) - .unwrap_or(false); - let y = self - .commit_and_undelegate - .as_ref() - .map(|el| el.has_callbacks()) - .unwrap_or(false); - let z = self - .standalone_actions - .iter() - .any(|el| el.callback.is_some()); - - x || y || z + if let Some(commit_finalize_and_undelegate) = + &bundle.commit_finalize_and_undelegate + { + check(commit_finalize_and_undelegate.get_committed_accounts())?; } - pub fn get_action_mut(&mut self, index: usize) -> Option<&mut BaseAction> { - let mut offset = 0usize; - - if let Some(commit) = self.commit.as_mut() { - let count = commit.action_count(); - if index < offset + count { - return commit.get_action_mut(index - offset); - } - offset += count; - } - - if let Some(cau) = self.commit_and_undelegate.as_mut() { - let count = cau.action_count(); - if index < offset + count { - return cau.get_action_mut(index - offset); - } - offset += count; - } - - self.standalone_actions.get_mut(index.checked_sub(offset)?) - } + Ok(()) } -impl MagicBaseIntent { - pub fn try_from_args( +impl TryFromArgs for MagicBaseIntent { + fn try_from_args( args: MagicBaseIntentArgs, context: &ConstructionContext<'_, '_, '_>, ) -> Result { @@ -627,91 +381,18 @@ impl MagicBaseIntent { } } } - - pub fn is_undelegate(&self) -> bool { - match &self { - MagicBaseIntent::BaseActions(_) => false, - MagicBaseIntent::Commit(_) => false, - MagicBaseIntent::CommitAndUndelegate(_) => true, - MagicBaseIntent::CommitFinalize(_) => false, - MagicBaseIntent::CommitFinalizeAndUndelegate(_) => true, - } - } - - pub fn is_commit_finalize(&self) -> bool { - match &self { - MagicBaseIntent::BaseActions(_) => false, - MagicBaseIntent::Commit(_) => false, - MagicBaseIntent::CommitAndUndelegate(_) => false, - MagicBaseIntent::CommitFinalize(_) => true, - MagicBaseIntent::CommitFinalizeAndUndelegate(_) => true, - } - } - - pub fn get_committed_accounts(&self) -> Option<&Vec> { - match self { - MagicBaseIntent::BaseActions(_) => None, - MagicBaseIntent::Commit(t) => Some(t.get_committed_accounts()), - MagicBaseIntent::CommitAndUndelegate(t) => { - Some(t.get_committed_accounts()) - } - MagicBaseIntent::CommitFinalize(t) => { - Some(t.get_committed_accounts()) - } - MagicBaseIntent::CommitFinalizeAndUndelegate(t) => { - Some(t.get_committed_accounts()) - } - } - } - - pub fn get_committed_accounts_mut( - &mut self, - ) -> Option<&mut Vec> { - match self { - MagicBaseIntent::BaseActions(_) => None, - MagicBaseIntent::Commit(t) => Some(t.get_committed_accounts_mut()), - MagicBaseIntent::CommitAndUndelegate(t) => { - Some(t.get_committed_accounts_mut()) - } - MagicBaseIntent::CommitFinalize(t) => { - Some(t.get_committed_accounts_mut()) - } - MagicBaseIntent::CommitFinalizeAndUndelegate(t) => { - Some(t.get_committed_accounts_mut()) - } - } - } - - pub fn get_committed_pubkeys(&self) -> Option> { - self.get_committed_accounts().map(|accounts| { - accounts.iter().map(|account| account.pubkey).collect() - }) - } - - pub fn is_empty(&self) -> bool { - match self { - MagicBaseIntent::BaseActions(actions) => actions.is_empty(), - MagicBaseIntent::Commit(t) => t.is_empty(), - MagicBaseIntent::CommitAndUndelegate(t) => t.is_empty(), - MagicBaseIntent::CommitFinalize(t) => t.is_empty(), - MagicBaseIntent::CommitFinalizeAndUndelegate(t) => t.is_empty(), - } - } -} - -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct CommitAndUndelegate { - pub commit_action: CommitType, - pub undelegate_action: UndelegateType, } -impl CommitAndUndelegate { - pub fn try_from_args( +impl TryFromArgs for CommitAndUndelegate { + fn try_from_args( args: CommitAndUndelegateArgs, context: &ConstructionContext<'_, '_, '_>, ) -> Result { let account_indices = args.commit_type.committed_accounts_indices(); - Self::validate(account_indices.as_slice(), context)?; + validate_commit_and_undelegate_accounts( + account_indices.as_slice(), + context, + )?; let commit_action = CommitType::try_from_args(args.commit_type, context)?; @@ -723,122 +404,38 @@ impl CommitAndUndelegate { undelegate_action, }) } +} - pub fn validate( - account_indices: &[u8], - context: &ConstructionContext<'_, '_, '_>, - ) -> Result<(), InstructionError> { - account_indices.iter().copied().try_for_each(|idx| { - let is_writable = - get_writable_with_idx(context.transaction_context(), idx as u16)?; - let delegated = get_instruction_account_with_idx( +fn validate_commit_and_undelegate_accounts( + account_indices: &[u8], + context: &ConstructionContext<'_, '_, '_>, +) -> Result<(), InstructionError> { + account_indices.iter().copied().try_for_each(|idx| { + let is_writable = + get_writable_with_idx(context.transaction_context(), idx as u16)?; + let delegated = get_instruction_account_with_idx( + context.transaction_context(), + idx as u16, + )?; + if is_writable && delegated.borrow()?.delegated() { + Ok(()) + } else { + let pubkey = get_instruction_pubkey_with_idx( context.transaction_context(), idx as u16, )?; - if is_writable && delegated.borrow()?.delegated() { - Ok(()) - } else { - let pubkey = get_instruction_pubkey_with_idx( - context.transaction_context(), - idx as u16, - )?; - ic_msg!( - context.invoke_context, - "ScheduleCommit ERR: account {} is required to be writable and delegated in order to be undelegated", - pubkey - ); - Err(InstructionError::ReadonlyDataModified) - } - }) - } - - pub fn calculate_fee( - &self, - commit_nonces: &HashMap, - ) -> Result { - let mut fee = 0; - fee += self.commit_action.calculate_fee(commit_nonces)?; - fee += self.undelegate_action.calculate_fee(commit_nonces)?; - Ok(fee) - } - - pub fn get_committed_accounts(&self) -> &Vec { - self.commit_action.get_committed_accounts() - } - - pub fn get_committed_accounts_mut(&mut self) -> &mut Vec { - self.commit_action.get_committed_accounts_mut() - } - - pub fn get_committed_pubkeys(&self) -> Vec { - self.commit_action.get_committed_pubkeys() - } - - pub fn is_empty(&self) -> bool { - self.commit_action.is_empty() - } - - pub fn has_callbacks(&self) -> bool { - let x = self.commit_action.has_callbacks(); - let y = if let UndelegateType::WithBaseActions(actions) = - &self.undelegate_action - { - actions.iter().any(|el| el.callback.is_some()) - } else { - false - }; - - x || y - } - - pub fn action_count(&self) -> usize { - self.commit_action.action_count() - + self.undelegate_action.action_count() - } - - pub fn get_action_mut(&mut self, index: usize) -> Option<&mut BaseAction> { - let commit_count = self.commit_action.action_count(); - if index < commit_count { - return self.commit_action.get_action_mut(index); - } - self.undelegate_action.get_action_mut(index - commit_count) - } -} - -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct ProgramArgs { - pub escrow_index: u8, - pub data: Vec, -} - -impl From for ProgramArgs { - fn from(value: ActionArgs) -> Self { - Self { - escrow_index: value.escrow_index, - data: value.data, + ic_msg!( + context.invoke_context, + "ScheduleCommit ERR: account {} is required to be writable and delegated in order to be undelegated", + pubkey + ); + Err(InstructionError::ReadonlyDataModified) } - } -} - -impl From<&ActionArgs> for ProgramArgs { - fn from(value: &ActionArgs) -> Self { - value.clone().into() - } -} - -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct BaseAction { - pub compute_units: u32, - pub destination_program: Pubkey, - pub source_program: Option, - pub escrow_authority: Pubkey, - pub data_per_program: ProgramArgs, - pub account_metas_per_program: Vec, - pub callback: Option, + }) } -impl BaseAction { - pub fn try_from_args( +impl TryFromArgs for BaseAction { + fn try_from_args( args: BaseActionArgs, context: &ConstructionContext<'_, '_, '_>, ) -> Result { @@ -892,103 +489,95 @@ impl BaseAction { } } -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub enum CommitType { - /// Regular commit without actions - Standalone(Vec), // accounts to commit - /// Commits accounts and runs actions - WithBaseActions { - committed_accounts: Vec, - base_actions: Vec, - }, -} - -impl CommitType { - fn validate_accounts( - accounts: &[CommitAccountRef<'_, '_>], - context: &ConstructionContext<'_, '_, '_>, - ) -> Result<(), InstructionError> { - accounts.iter().try_for_each(|(pubkey, account)| { - if account.to_account_shared_data()?.confined() { - ic_msg!( - context.invoke_context, - "ScheduleCommit ERR: account {} is confined and cannot be committed", - pubkey - ); - return Err(InstructionError::InvalidAccountData); - } +fn validate_commit_type_accounts( + accounts: &[CommitAccountRef<'_, '_>], + context: &ConstructionContext<'_, '_, '_>, +) -> Result<(), InstructionError> { + accounts.iter().try_for_each(|(pubkey, account)| { + if account.to_account_shared_data()?.confined() { + ic_msg!( + context.invoke_context, + "ScheduleCommit ERR: account {} is confined and cannot be committed", + pubkey + ); + return Err(InstructionError::InvalidAccountData); + } - // Prevent ephemeral accounts from being committed to base chain - if account.to_account_shared_data()?.ephemeral() { - ic_msg!( - context.invoke_context, - "ScheduleCommit ERR: account {} is ephemeral and cannot be committed to base chain", - pubkey - ); - return Err(InstructionError::InvalidAccountData); - } + // Prevent ephemeral accounts from being committed to base chain + if account.to_account_shared_data()?.ephemeral() { + ic_msg!( + context.invoke_context, + "ScheduleCommit ERR: account {} is ephemeral and cannot be committed to base chain", + pubkey + ); + return Err(InstructionError::InvalidAccountData); + } - if !account.borrow()?.delegated() { - ic_msg!( - context.invoke_context, - "ScheduleCommit ERR: account {} is required to be delegated to the current validator, in order to be committed", - pubkey - ); - return Err(InstructionError::IllegalOwner) - } + if !account.borrow()?.delegated() { + ic_msg!( + context.invoke_context, + "ScheduleCommit ERR: account {} is required to be delegated to the current validator, in order to be committed", + pubkey + ); + return Err(InstructionError::IllegalOwner) + } - // Validate committed account was scheduled by valid authority - let owner = *account.borrow()?.owner(); - validate_commit_schedule_permissions( - &context.invoke_context, - &owner, - pubkey, - context.parent_program_id.as_ref(), - context.signers, - ) - }) - } + // Validate committed account was scheduled by valid authority + let owner = *account.borrow()?.owner(); + validate_commit_schedule_permissions( + &context.invoke_context, + &owner, + pubkey, + context.parent_program_id.as_ref(), + context.signers, + ) + }) +} - // I delegated an account, now the owner is delegation program - // parent_program_id != Some(&acc_owner) should fail. or any modification on ER - // ER perceives owner as old one, hence for ER those are valid txs - // On commit_and_undelegate and commit we will set owner to DLP, for latter temporarily - // The owner shall be real owner on chain - // So first: - // 1. Validate - // 2. Fetch current account states - pub(crate) fn extract_commit_accounts<'a, 'ix_data>( - account_indices: &[u8], - transaction_context: &'a TransactionContext<'ix_data>, - ) -> Result>, InstructionError> { - account_indices - .iter() - .map(|i| { - let account = get_instruction_account_with_idx( - transaction_context, - *i as u16, - )?; - let pubkey = *get_instruction_pubkey_with_idx( - transaction_context, - *i as u16, - )?; +// I delegated an account, now the owner is delegation program +// parent_program_id != Some(&acc_owner) should fail. or any modification on ER +// ER perceives owner as old one, hence for ER those are valid txs +// On commit_and_undelegate and commit we will set owner to DLP, for latter temporarily +// The owner shall be real owner on chain +// So first: +// 1. Validate +// 2. Fetch current account states +pub(crate) fn extract_commit_accounts<'a, 'ix_data>( + account_indices: &[u8], + transaction_context: &'a TransactionContext<'ix_data>, +) -> Result>, InstructionError> { + account_indices + .iter() + .map(|i| { + let account = get_instruction_account_with_idx( + transaction_context, + *i as u16, + )?; + let pubkey = *get_instruction_pubkey_with_idx( + transaction_context, + *i as u16, + )?; - Ok((pubkey, account)) - }) - .collect::>() - } + Ok((pubkey, account)) + }) + .collect::>() +} - pub fn try_from_args( +impl TryFromArgs for CommitType { + fn try_from_args( args: CommitTypeArgs, context: &ConstructionContext<'_, '_, '_>, ) -> Result { match args { CommitTypeArgs::Standalone(accounts) => { - let committed_accounts_ref = Self::extract_commit_accounts( + let committed_accounts_ref = extract_commit_accounts( &accounts, context.transaction_context(), )?; - Self::validate_accounts(&committed_accounts_ref, context)?; + validate_commit_type_accounts( + &committed_accounts_ref, + context, + )?; let committed_accounts = committed_accounts_ref .into_iter() .map(|(pubkey, account)| { @@ -1007,11 +596,14 @@ impl CommitType { committed_accounts, base_actions, } => { - let committed_accounts_ref = Self::extract_commit_accounts( + let committed_accounts_ref = extract_commit_accounts( &committed_accounts, context.transaction_context(), )?; - Self::validate_accounts(&committed_accounts_ref, context)?; + validate_commit_type_accounts( + &committed_accounts_ref, + context, + )?; let base_actions = base_actions .into_iter() @@ -1036,117 +628,10 @@ impl CommitType { } } } - - /// Calculate fee commits - pub fn calculate_fee( - &self, - commit_nonces: &HashMap, - ) -> Result { - let mut fee = 0; - match self { - CommitType::Standalone(ref committed_accounts) => { - fee += calculate_commit_fee(committed_accounts, commit_nonces)?; - } - CommitType::WithBaseActions { - committed_accounts, - base_actions, - } => { - fee += calculate_commit_fee(committed_accounts, commit_nonces)?; - fee += calculate_actions_fee(base_actions); - } - } - - Ok(fee) - } - - pub fn get_committed_accounts(&self) -> &Vec { - match self { - Self::Standalone(committed_accounts) => committed_accounts, - Self::WithBaseActions { - committed_accounts, .. - } => committed_accounts, - } - } - - pub fn get_committed_accounts_mut(&mut self) -> &mut Vec { - match self { - Self::Standalone(committed_accounts) => committed_accounts, - Self::WithBaseActions { - committed_accounts, .. - } => committed_accounts, - } - } - - pub fn get_committed_pubkeys(&self) -> Vec { - self.get_committed_accounts() - .iter() - .map(|account| account.pubkey) - .collect() - } - - pub fn is_empty(&self) -> bool { - match self { - Self::Standalone(committed_accounts) => { - committed_accounts.is_empty() - } - Self::WithBaseActions { - committed_accounts, .. - } => committed_accounts.is_empty(), - } - } - - pub fn has_callbacks(&self) -> bool { - if let Self::WithBaseActions { - committed_accounts: _, - base_actions, - } = self - { - base_actions.iter().any(|el| el.callback.is_some()) - } else { - false - } - } - - pub fn action_count(&self) -> usize { - match self { - Self::Standalone(_) => 0, - Self::WithBaseActions { base_actions, .. } => base_actions.len(), - } - } - - pub fn get_action_mut(&mut self, index: usize) -> Option<&mut BaseAction> { - if let Self::WithBaseActions { base_actions, .. } = self { - base_actions.get_mut(index) - } else { - None - } - } -} - -/// No CommitedAccounts since it is only used with CommitAction. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub enum UndelegateType { - Standalone, - WithBaseActions(Vec), } -impl UndelegateType { - pub fn action_count(&self) -> usize { - match self { - Self::Standalone => 0, - Self::WithBaseActions(actions) => actions.len(), - } - } - - pub fn get_action_mut(&mut self, index: usize) -> Option<&mut BaseAction> { - if let Self::WithBaseActions(actions) = self { - actions.get_mut(index) - } else { - None - } - } - - pub fn try_from_args( +impl TryFromArgs for UndelegateType { + fn try_from_args( args: UndelegateTypeArgs, context: &ConstructionContext<'_, '_, '_>, ) -> Result { @@ -1163,18 +648,6 @@ impl UndelegateType { } } } - - pub fn calculate_fee( - &self, - _commit_nonces: &HashMap, - ) -> Result { - match self { - UndelegateType::Standalone => Ok(0), - UndelegateType::WithBaseActions(actions) => { - Ok(calculate_actions_fee(actions)) - } - } - } } /// Validate that a committee account has a permission to be committed. @@ -1226,44 +699,15 @@ pub(crate) fn validate_commit_schedule_permissions( } } -pub(crate) fn calculate_commit_fee( - accounts: &[CommittedAccount], - commit_nonces: &HashMap, -) -> Result { - accounts.iter().try_fold(0u64, |fee, account| { - if let Some(nonce) = commit_nonces.get(&account.pubkey) { - if nonce >= &ACTUAL_COMMIT_LIMIT { - Ok(fee + COMMIT_FEE_LAMPORTS) - } else { - Ok(fee) - } - } else { - Err(InstructionError::Custom(MISSING_COMMIT_NONCE_ERR)) - } - }) -} - -fn calculate_actions_fee(actions: &[BaseAction]) -> 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, - ) - }); - micro_lamports.div_ceil(MICRO_LAMPORTS_PER_LAMPORT) -} - #[cfg(test)] mod tests { use std::collections::HashMap; - use magicblock_core::intent::CommittedAccount; + use magicblock_core::intent::types::CommittedAccount; use solana_account::Account; - use solana_instruction::error::InstructionError; use solana_pubkey::Pubkey; use super::*; - use crate::magic_sys::MISSING_COMMIT_NONCE_ERR; fn make_committed_account(pubkey: Pubkey) -> CommittedAccount { CommittedAccount { @@ -1288,77 +732,6 @@ mod tests { } } - // ---- calculate_commit_fee ---- - - #[test] - fn test_commit_fee_at_limit_is_zero() { - let pk = Pubkey::new_unique(); - // nonce is commits done so far; nonce+1 is the next commit number. - // ACTUAL_COMMIT_LIMIT - 1 means the next commit is exactly at the limit → free. - let nonces = HashMap::from([(pk, ACTUAL_COMMIT_LIMIT - 1)]); - let fee = calculate_commit_fee(&[make_committed_account(pk)], &nonces) - .unwrap(); - assert_eq!(fee, 0); - } - - #[test] - fn test_commit_fee_above_limit_charges_per_account() { - let pk1 = Pubkey::new_unique(); - let pk2 = Pubkey::new_unique(); - let nonces = HashMap::from([ - (pk1, ACTUAL_COMMIT_LIMIT + 1), - (pk2, ACTUAL_COMMIT_LIMIT + 1), - ]); - let fee = calculate_commit_fee( - &[make_committed_account(pk1), make_committed_account(pk2)], - &nonces, - ) - .unwrap(); - assert_eq!(fee, COMMIT_FEE_LAMPORTS * 2); - } - - #[test] - fn test_commit_fee_mixed_accounts() { - let pk_below = Pubkey::new_unique(); - let pk_above = Pubkey::new_unique(); - let nonces = HashMap::from([ - (pk_below, ACTUAL_COMMIT_LIMIT - 1), // next commit is exactly at limit → free - (pk_above, ACTUAL_COMMIT_LIMIT), // next commit exceeds limit → charged - ]); - let fee = calculate_commit_fee( - &[ - make_committed_account(pk_below), - make_committed_account(pk_above), - ], - &nonces, - ) - .unwrap(); - assert_eq!(fee, COMMIT_FEE_LAMPORTS); - } - - #[test] - fn test_commit_fee_missing_nonce_errors() { - let pk = Pubkey::new_unique(); - let err = calculate_commit_fee( - &[make_committed_account(pk)], - &HashMap::new(), - ) - .unwrap_err(); - assert_eq!(err, InstructionError::Custom(MISSING_COMMIT_NONCE_ERR)); - } - - /// two actions of 200_000 CUs each → 20_000 lamports - #[test] - fn test_actions_fee_multiple_actions() { - assert_eq!( - calculate_actions_fee(&[ - make_base_action(200_000), - make_base_action(200_000) - ]), - 20_000 - ); - } - // ---- ScheduledIntentBundle::calculate_fee ---- #[test] diff --git a/programs/magicblock/src/magic_sys.rs b/programs/magicblock/src/magic_sys.rs index 0fda01653..9bfbb2d3c 100644 --- a/programs/magicblock/src/magic_sys.rs +++ b/programs/magicblock/src/magic_sys.rs @@ -4,7 +4,10 @@ use std::{ }; use lazy_static::lazy_static; -use magicblock_core::{intent::CommittedAccount, traits::MagicSys}; +use magicblock_core::{ + intent::{types::CommittedAccount, MagicIntentBundle}, + traits::MagicSys, +}; use solana_instruction::error::InstructionError; use solana_pubkey::Pubkey; @@ -15,6 +18,9 @@ pub const COMMIT_LIMIT: u64 = 10; /// account that has reached [`COMMIT_LIMIT`]. pub const COMMIT_LIMIT_ERR: u32 = 0xA000_0000; pub(crate) const MISSING_COMMIT_NONCE_ERR: u32 = 0xA000_0001; +/// [`InstructionError::Custom`] code returned when an intent could never fit +/// on the base layer, no matter how it gets optimized at execution time. +pub const INTENT_TOO_LARGE_ERR: u32 = 0xA000_0002; lazy_static! { static ref MAGIC_SYS: RwLock>> = RwLock::new(None); @@ -39,3 +45,14 @@ pub(crate) fn fetch_current_commit_nonces( .ok_or(InstructionError::UninitializedAccount)? .fetch_current_commit_nonces(commits) } + +pub(crate) fn validate_intent_size( + intent: &MagicIntentBundle, +) -> Result<(), InstructionError> { + MAGIC_SYS + .read() + .expect(MAGIC_SYS_POISONED_MSG) + .as_ref() + .ok_or(InstructionError::UninitializedAccount)? + .validate_intent_size(intent) +} diff --git a/programs/magicblock/src/schedule_transactions/mod.rs b/programs/magicblock/src/schedule_transactions/mod.rs index b16b66a5a..a7a479fd2 100644 --- a/programs/magicblock/src/schedule_transactions/mod.rs +++ b/programs/magicblock/src/schedule_transactions/mod.rs @@ -11,7 +11,7 @@ pub(crate) mod transaction_scheduler; use std::sync::Arc; -use magicblock_core::intent::CommittedAccount; +use magicblock_core::intent::types::CommittedAccount; use magicblock_magic_program_api::{ pda::CALLBACK_SIGNER, MAGIC_CONTEXT_PUBKEY, }; diff --git a/programs/magicblock/src/schedule_transactions/process_schedule_cloned_undelegation.rs b/programs/magicblock/src/schedule_transactions/process_schedule_cloned_undelegation.rs index d1d6bfc8e..4f3421da2 100644 --- a/programs/magicblock/src/schedule_transactions/process_schedule_cloned_undelegation.rs +++ b/programs/magicblock/src/schedule_transactions/process_schedule_cloned_undelegation.rs @@ -1,6 +1,9 @@ use std::collections::HashSet; -use magicblock_core::intent::CommittedAccount; +use magicblock_core::intent::{ + types::CommittedAccount, CommitAndUndelegate, CommitType, MagicBaseIntent, + UndelegateType, +}; use magicblock_magic_program_api::{ instruction::MagicBlockInstruction, MAGIC_CONTEXT_PUBKEY, }; @@ -12,10 +15,7 @@ use solana_pubkey::Pubkey; use crate::{ clone_account::remove_pending_clone, - magic_scheduled_base_intent::{ - CommitAndUndelegate, CommitType, MagicBaseIntent, - ScheduledIntentBundle, UndelegateType, - }, + magic_scheduled_base_intent::ScheduledIntentBundle, schedule_transactions::get_clock, utils::{ account_actions::mark_account_as_undelegated, diff --git a/programs/magicblock/src/schedule_transactions/process_schedule_commit.rs b/programs/magicblock/src/schedule_transactions/process_schedule_commit.rs index a4a0f641e..eabbba66f 100644 --- a/programs/magicblock/src/schedule_transactions/process_schedule_commit.rs +++ b/programs/magicblock/src/schedule_transactions/process_schedule_commit.rs @@ -1,6 +1,9 @@ use std::collections::HashSet; -use magicblock_core::intent::CommittedAccount; +use magicblock_core::intent::{ + calculate_commit_fee, types::CommittedAccount, CommitAndUndelegate, + CommitType, MagicBaseIntent, UndelegateType, +}; // no direct token remap helpers needed here; handled in CommittedAccount builder use solana_account::{ReadableAccount, WritableAccount}; use solana_instruction::error::InstructionError; @@ -10,11 +13,9 @@ use solana_pubkey::Pubkey; use crate::{ magic_scheduled_base_intent::{ - calculate_commit_fee, validate_commit_schedule_permissions, - CommitAndUndelegate, CommitType, MagicBaseIntent, - ScheduledIntentBundle, UndelegateType, + validate_commit_schedule_permissions, ScheduledIntentBundle, }, - magic_sys::fetch_current_commit_nonces, + magic_sys::{fetch_current_commit_nonces, validate_intent_size}, schedule_transactions::{self, check_commit_limits, try_get_fee_vault}, utils::{ account_actions::{ @@ -306,6 +307,12 @@ pub(crate) fn process_schedule_commit( )) } .into(); + validate_intent_size(&base_intent).inspect_err(|_| { + ic_msg!( + invoke_context, + "ScheduleCommit ERR: intent is too large to ever fit into transaction", + ); + })?; let scheduled_base_intent = ScheduledIntentBundle { id: intent_id, diff --git a/programs/magicblock/src/schedule_transactions/process_schedule_commit_tests.rs b/programs/magicblock/src/schedule_transactions/process_schedule_commit_tests.rs index eeb724135..19dc8c865 100644 --- a/programs/magicblock/src/schedule_transactions/process_schedule_commit_tests.rs +++ b/programs/magicblock/src/schedule_transactions/process_schedule_commit_tests.rs @@ -1,6 +1,7 @@ use std::collections::HashMap; use assert_matches::assert_matches; +use magicblock_core::intent::{ACTUAL_COMMIT_LIMIT, COMMIT_FEE_LAMPORTS}; use magicblock_magic_program_api::{ args::{ ActionArgs, BaseActionArgs, MagicIntentBundleArgs, ShortAccountMeta, @@ -21,9 +22,7 @@ use solana_signer::Signer; use crate::{ magic_context::MagicContext, - magic_scheduled_base_intent::{ - ScheduledIntentBundle, ACTUAL_COMMIT_LIMIT, COMMIT_FEE_LAMPORTS, - }, + magic_scheduled_base_intent::ScheduledIntentBundle, magic_sys::COMMIT_LIMIT, schedule_transactions::{ magic_fee_vault_pubkey, transaction_scheduler::TransactionScheduler, diff --git a/programs/magicblock/src/schedule_transactions/process_schedule_intent_bundle.rs b/programs/magicblock/src/schedule_transactions/process_schedule_intent_bundle.rs index 5210e36f6..5b580a68a 100644 --- a/programs/magicblock/src/schedule_transactions/process_schedule_intent_bundle.rs +++ b/programs/magicblock/src/schedule_transactions/process_schedule_intent_bundle.rs @@ -9,7 +9,7 @@ use solana_pubkey::Pubkey; use crate::{ magic_scheduled_base_intent::{ - CommitType, ConstructionContext, ScheduledIntentBundle, + extract_commit_accounts, ConstructionContext, ScheduledIntentBundle, }, magic_sys::fetch_current_commit_nonces, schedule_transactions::{ @@ -99,7 +99,7 @@ pub(crate) fn process_schedule_intent_bundle( .flatten() .map(|el| el.committed_accounts_indices()) .try_fold(vec![], |mut acc, indices| { - acc.extend(CommitType::extract_commit_accounts( + acc.extend(extract_commit_accounts( indices, construction_context.transaction_context(), )?); diff --git a/programs/magicblock/src/test_utils/mod.rs b/programs/magicblock/src/test_utils/mod.rs index 7a7eda903..e46ddc20c 100644 --- a/programs/magicblock/src/test_utils/mod.rs +++ b/programs/magicblock/src/test_utils/mod.rs @@ -7,7 +7,10 @@ use std::{ }, }; -use magicblock_core::{intent::CommittedAccount, traits::MagicSys}; +use magicblock_core::{ + intent::{types::CommittedAccount, MagicIntentBundle}, + traits::MagicSys, +}; use magicblock_magic_program_api::{id, EPHEMERAL_VAULT_PUBKEY}; use solana_account::AccountSharedData; use solana_instruction::{error::InstructionError, AccountMeta}; @@ -160,4 +163,11 @@ impl MagicSys for MagicSysStub { .collect(), } } + + fn validate_intent_size( + &self, + _intent: &MagicIntentBundle, + ) -> Result<(), InstructionError> { + Ok(()) + } } diff --git a/test-integration/programs/schedulecommit-security/src/lib.rs b/test-integration/programs/schedulecommit-security/src/lib.rs index 2cd34e495..85f54d060 100644 --- a/test-integration/programs/schedulecommit-security/src/lib.rs +++ b/test-integration/programs/schedulecommit-security/src/lib.rs @@ -85,7 +85,7 @@ pub fn process_instruction<'a>( commit_payer: false, has_magic_vault: false, }, - ScheduleCommitType::Commit, + ScheduleCommitType::CommitFinalize, ), } } @@ -126,7 +126,7 @@ fn process_sibling_schedule_cpis( None, players, &pdas, - ScheduleCommitType::Commit, + ScheduleCommitType::CommitFinalize, ); let mut account_infos = account_infos .clone() diff --git a/test-integration/programs/schedulecommit/src/api.rs b/test-integration/programs/schedulecommit/src/api.rs index 0dfe0931b..c8d9ed81e 100644 --- a/test-integration/programs/schedulecommit/src/api.rs +++ b/test-integration/programs/schedulecommit/src/api.rs @@ -329,7 +329,7 @@ pub fn schedule_commit_with_payer_cpi_instruction( players, committees, true, - ScheduleCommitType::Commit, + ScheduleCommitType::CommitFinalize, ) } @@ -349,7 +349,7 @@ pub fn schedule_commit_and_undelegate_cpi_instruction( players, committees, false, - ScheduleCommitType::CommitAndUndelegate, + ScheduleCommitType::CommitFinalizeAndUndelegate, ) } diff --git a/test-integration/programs/schedulecommit/src/lib.rs b/test-integration/programs/schedulecommit/src/lib.rs index 414219861..9376c74b9 100644 --- a/test-integration/programs/schedulecommit/src/lib.rs +++ b/test-integration/programs/schedulecommit/src/lib.rs @@ -208,9 +208,12 @@ pub enum ScheduleCommitInstruction { pub enum ScheduleCommitType { Commit, CommitAndUndelegate, + CommitFinalize, + CommitFinalizeAndUndelegate, } impl ScheduleCommitType { + #[inline(never)] fn invoke_commit<'a, 'info>( self, payer: &'a AccountInfo<'info>, @@ -220,14 +223,32 @@ impl ScheduleCommitType { magic_fee_vault: Option<&'a AccountInfo<'info>>, ) -> ProgramResult { match self { - ScheduleCommitType::Commit => invoke_schedule_commit( + ScheduleCommitType::Commit => invoke_schedule_via_builder( payer, accounts, magic_context, magic_program, magic_fee_vault, + false, ), ScheduleCommitType::CommitAndUndelegate => { + invoke_schedule_via_builder( + payer, + accounts, + magic_context, + magic_program, + magic_fee_vault, + true, + ) + } + ScheduleCommitType::CommitFinalize => invoke_schedule_commit( + payer, + accounts, + magic_context, + magic_program, + magic_fee_vault, + ), + ScheduleCommitType::CommitFinalizeAndUndelegate => { commit_and_undelegate_accounts( payer, accounts, @@ -240,6 +261,35 @@ impl ScheduleCommitType { } } +#[inline(never)] +fn invoke_schedule_via_builder<'a, 'info>( + payer: &'a AccountInfo<'info>, + accounts: Vec<&'a AccountInfo<'info>>, + magic_context: &'a AccountInfo<'info>, + magic_program: &'a AccountInfo<'info>, + magic_fee_vault: Option<&'a AccountInfo<'info>>, + undelegate: bool, +) -> ProgramResult { + let builder = MagicIntentBundleBuilder::new( + payer.clone(), + magic_context.clone(), + magic_program.clone(), + ); + let builder = if let Some(vault) = magic_fee_vault { + builder.magic_fee_vault(vault.clone()) + } else { + builder + }; + let accounts_owned: Vec<_> = accounts.into_iter().cloned().collect(); + if undelegate { + builder + .commit_and_undelegate(&accounts_owned) + .build_and_invoke() + } else { + builder.commit(&accounts_owned).build_and_invoke() + } +} + fn invoke_schedule_commit<'a, 'info>( payer: &'a AccountInfo<'info>, accounts: Vec<&'a AccountInfo<'info>>, diff --git a/test-integration/schedulecommit/client/src/schedule_commit_context.rs b/test-integration/schedulecommit/client/src/schedule_commit_context.rs index 1154fb2bd..0616e19df 100644 --- a/test-integration/schedulecommit/client/src/schedule_commit_context.rs +++ b/test-integration/schedulecommit/client/src/schedule_commit_context.rs @@ -181,28 +181,26 @@ impl ScheduleCommitTestContext { }) } - // ----------------- - // Schedule Commit specific Transactions - // ----------------- - pub fn init_committees(&self) -> Result { + pub fn init_committees_chunk( + &self, + committees: &[(Keypair, Pubkey)], + ) -> Result { let mut ixs = vec![ ComputeBudgetInstruction::set_compute_unit_limit(1_400_000), ComputeBudgetInstruction::set_compute_unit_price(10_000), ]; match self.user_seed { UserSeeds::MagicScheduleCommit => { - ixs.extend(self.committees.iter().map( - |(player, committee)| { - init_account_instruction( - self.payer_chain.pubkey(), - player.pubkey(), - *committee, - ) - }, - )); + ixs.extend(committees.iter().map(|(player, committee)| { + init_account_instruction( + self.payer_chain.pubkey(), + player.pubkey(), + *committee, + ) + })); } UserSeeds::OrderBook => { - ixs.extend(self.committees.iter().map( + ixs.extend(committees.iter().map( |(book_manager, committee)| { init_order_book_instruction( self.payer_chain.pubkey(), @@ -229,8 +227,7 @@ impl ScheduleCommitTestContext { } }; - let mut signers = self - .committees + let mut signers = committees .iter() .map(|(payer, _)| payer) .collect::>(); @@ -262,6 +259,16 @@ impl ScheduleCommitTestContext { Ok(sig) } + pub fn init_committees(&self) -> Result> { + /// Needed to fit in tx size + const COMMITTEES_INIT_CHUNK_SIZE: usize = 7; + + self.committees + .chunks(COMMITTEES_INIT_CHUNK_SIZE) + .map(|committees| self.init_committees_chunk(committees)) + .collect() + } + pub fn escrow_lamports_for_payer(&self) -> Result { let ixs = init_payer_escrow(self.payer_ephem.pubkey()); @@ -284,9 +291,12 @@ impl ScheduleCommitTestContext { .with_context(|| "Failed to escrow fund for payer") } - pub fn delegate_committees(&self) -> Result { + fn delegate_committees_chunk( + &self, + committees: &[(Keypair, Pubkey)], + ) -> Result { let mut ixs = vec![]; - for (player, _) in &self.committees { + for (player, _) in committees { let ix = delegate_account_cpi_instruction( self.payer_chain.pubkey(), self.ephem_validator_identity, @@ -324,6 +334,17 @@ impl ScheduleCommitTestContext { Ok(sig) } + pub fn delegate_committees(&self) -> Result> { + // Delegate instructions carry more accounts (and a larger args + // payload) than an init instruction, so fewer fit per transaction. + const COMMITTEES_DELEGATE_CHUNK_SIZE: usize = 4; + + self.committees + .chunks(COMMITTEES_DELEGATE_CHUNK_SIZE) + .map(|committees| self.delegate_committees_chunk(committees)) + .collect() + } + // ----------------- // Integration Test Context Fields // ----------------- diff --git a/test-integration/schedulecommit/test-scenarios/tests/01_commits.rs b/test-integration/schedulecommit/test-scenarios/tests/01_commits.rs index 15f7b931f..7fcf8bcd8 100644 --- a/test-integration/schedulecommit/test-scenarios/tests/01_commits.rs +++ b/test-integration/schedulecommit/test-scenarios/tests/01_commits.rs @@ -70,7 +70,7 @@ fn test_committing_one_account() { .map(|(player, _)| player.pubkey()) .collect::>(), &committees.iter().map(|(_, pda)| *pda).collect::>(), - program_schedulecommit::ScheduleCommitType::Commit, + program_schedulecommit::ScheduleCommitType::CommitFinalize, ); let ephem_blockhash = ephem_client.get_latest_blockhash().unwrap(); @@ -126,7 +126,7 @@ fn test_committing_two_accounts() { .map(|(player, _)| player.pubkey()) .collect::>(), &committees.iter().map(|(_, pda)| *pda).collect::>(), - program_schedulecommit::ScheduleCommitType::Commit, + program_schedulecommit::ScheduleCommitType::CommitFinalize, ); let ephem_blockhash = ephem_client.get_latest_blockhash().unwrap(); @@ -335,9 +335,9 @@ fn schedule_commit_cpi_illegal_owner( let ix = ScheduleCommitInstruction::ScheduleCommitCpi( cpi_args, if is_undelegate { - ScheduleCommitType::CommitAndUndelegate + ScheduleCommitType::CommitFinalizeAndUndelegate } else { - ScheduleCommitType::Commit + ScheduleCommitType::CommitFinalize }, ); Instruction::new_with_borsh(program_id, &ix, account_metas) diff --git a/test-integration/schedulecommit/test-scenarios/tests/02_commit_and_undelegate.rs b/test-integration/schedulecommit/test-scenarios/tests/02_commit_and_undelegate.rs index dbf3e8515..e0c32a038 100644 --- a/test-integration/schedulecommit/test-scenarios/tests/02_commit_and_undelegate.rs +++ b/test-integration/schedulecommit/test-scenarios/tests/02_commit_and_undelegate.rs @@ -302,13 +302,15 @@ fn test_committing_and_undelegating_one_account() { #[test] fn test_commit_huge_order_book_account() { - run_test_for_commit_huge_order_book_account(ScheduleCommitType::Commit); + run_test_for_commit_huge_order_book_account( + ScheduleCommitType::CommitFinalize, + ); } #[test] fn test_commit_and_undelegate_huge_order_book_account() { run_test_for_commit_huge_order_book_account( - ScheduleCommitType::CommitAndUndelegate, + ScheduleCommitType::CommitFinalizeAndUndelegate, ); } @@ -383,6 +385,12 @@ fn run_test_for_commit_huge_order_book_account( ScheduleCommitType::CommitAndUndelegate => { assert_one_committee_account_was_undelegated_on_chain(&ctx); } + ScheduleCommitType::CommitFinalize => { + assert_one_committee_account_was_not_undelegated_on_chain(&ctx); + } + ScheduleCommitType::CommitFinalizeAndUndelegate => { + assert_one_committee_account_was_undelegated_on_chain(&ctx); + } } }); } diff --git a/test-integration/schedulecommit/test-scenarios/tests/03_commit_limit.rs b/test-integration/schedulecommit/test-scenarios/tests/03_commit_limit.rs index d1eaf858e..803feab32 100644 --- a/test-integration/schedulecommit/test-scenarios/tests/03_commit_limit.rs +++ b/test-integration/schedulecommit/test-scenarios/tests/03_commit_limit.rs @@ -1,10 +1,8 @@ use std::sync::OnceLock; use integration_test_tools::run_test; -use magicblock_program::{ - magic_scheduled_base_intent::{ACTUAL_COMMIT_LIMIT, COMMIT_FEE_LAMPORTS}, - magic_sys::{COMMIT_LIMIT, COMMIT_LIMIT_ERR}, -}; +use magicblock_core::intent::{ACTUAL_COMMIT_LIMIT, COMMIT_FEE_LAMPORTS}; +use magicblock_program::magic_sys::{COMMIT_LIMIT, COMMIT_LIMIT_ERR}; use program_schedulecommit::{ api::{ init_order_book_instruction, @@ -74,7 +72,7 @@ fn get_prepared() -> &'static ScheduleCommitTestContext { None, &players, &pdas, - ScheduleCommitType::Commit, + ScheduleCommitType::CommitFinalize, ); let blockhash = ephem_client.get_latest_blockhash().unwrap(); let tx = Transaction::new_signed_with_payer( @@ -133,7 +131,7 @@ fn test_schedule_commit_fails_at_commit_limit() { None, &[committee.0.pubkey()], &[committee.1], - ScheduleCommitType::Commit, + ScheduleCommitType::CommitFinalize, ); let blockhash = ephem_client.get_latest_blockhash().unwrap(); let tx = Transaction::new_signed_with_payer( diff --git a/test-integration/schedulecommit/test-scenarios/tests/04_intent_size_limit.rs b/test-integration/schedulecommit/test-scenarios/tests/04_intent_size_limit.rs new file mode 100644 index 000000000..703f2acea --- /dev/null +++ b/test-integration/schedulecommit/test-scenarios/tests/04_intent_size_limit.rs @@ -0,0 +1,139 @@ +#![allow(clippy::result_large_err)] + +use integration_test_tools::run_test; +use magicblock_program::magic_sys::INTENT_TOO_LARGE_ERR; +use program_schedulecommit::{ + api::{schedule_commit_cpi_instruction, UserSeeds}, + ScheduleCommitType, +}; +use schedulecommit_client::{verify, ScheduleCommitTestContextFields}; +use solana_rpc_client::rpc_client::SerializableTransaction; +use solana_rpc_client_api::config::RpcSendTransactionConfig; +use solana_sdk::{ + instruction::InstructionError, signature::Signer, transaction::Transaction, +}; +use test_kit::init_logger; +use tracing::*; +use utils::{ + assert_is_instruction_error, extract_transaction_error, + get_context_with_delegated_committees, +}; + +mod utils; + +/// Small enough that the commit and finalize transactions fit even without +/// any size optimization. +const FEW_ACCOUNTS: usize = 3; +/// CommitAndUndelegate won't with more than 11 accs +const TOO_MANY_ACCOUNTS: usize = 12; + +fn schedule_commit_ix( + payer: &solana_sdk::signature::Keypair, + committees: &[( + solana_sdk::signature::Keypair, + solana_sdk::pubkey::Pubkey, + )], +) -> solana_sdk::instruction::Instruction { + schedule_commit_cpi_instruction( + payer.pubkey(), + magicblock_magic_program_api::id(), + magicblock_magic_program_api::MAGIC_CONTEXT_PUBKEY, + None, + &committees + .iter() + .map(|(player, _)| player.pubkey()) + .collect::>(), + &committees.iter().map(|(_, pda)| *pda).collect::>(), + ScheduleCommitType::CommitAndUndelegate, + ) +} + +#[test] +fn test_schedule_commit_with_few_accounts_fits() { + run_test!({ + let ctx = get_context_with_delegated_committees( + FEW_ACCOUNTS, + UserSeeds::MagicScheduleCommit, + ); + + let ScheduleCommitTestContextFields { + payer_chain: payer, + committees, + commitment, + ephem_client, + .. + } = ctx.fields(); + + let ix = schedule_commit_ix(payer, committees); + let blockhash = ephem_client.get_latest_blockhash().unwrap(); + let tx = Transaction::new_signed_with_payer( + &[ix], + Some(&payer.pubkey()), + &[payer], + blockhash, + ); + + let sig = tx.get_signature(); + let res = ephem_client + .send_and_confirm_transaction_with_spinner_and_config( + &tx, + *commitment, + RpcSendTransactionConfig { + skip_preflight: true, + ..Default::default() + }, + ); + info!("{} '{:?}'", sig, res); + assert!( + res.is_ok(), + "Expected schedule commit to succeed: {:?}", + res + ); + + verify::fetch_and_verify_commit_result_from_logs(&ctx, *sig); + }); +} + +#[test] +fn test_schedule_commit_with_too_many_accounts_is_refused() { + run_test!({ + let ctx = get_context_with_delegated_committees( + TOO_MANY_ACCOUNTS, + UserSeeds::MagicScheduleCommit, + ); + + let ScheduleCommitTestContextFields { + payer_chain: payer, + committees, + commitment, + ephem_client, + .. + } = ctx.fields(); + + let ix = schedule_commit_ix(payer, committees); + let blockhash = ephem_client.get_latest_blockhash().unwrap(); + let tx = Transaction::new_signed_with_payer( + &[ix], + Some(&payer.pubkey()), + &[payer], + blockhash, + ); + + let res = ephem_client + .send_and_confirm_transaction_with_spinner_and_config( + &tx, + *commitment, + RpcSendTransactionConfig { + skip_preflight: true, + ..Default::default() + }, + ); + + let (tx_result_err, tx_err) = extract_transaction_error(res); + assert_is_instruction_error( + tx_err.unwrap(), + &tx_result_err, + InstructionError::Custom(INTENT_TOO_LARGE_ERR), + ); + }); +} diff --git a/test-integration/schedulecommit/test-scenarios/tests/utils/mod.rs b/test-integration/schedulecommit/test-scenarios/tests/utils/mod.rs index 28e688ade..520fcc407 100644 --- a/test-integration/schedulecommit/test-scenarios/tests/utils/mod.rs +++ b/test-integration/schedulecommit/test-scenarios/tests/utils/mod.rs @@ -25,15 +25,15 @@ pub fn get_context_with_delegated_committees( .unwrap(); println!("get_context_with_delegated_committees inside"); - let txhash = ctx.init_committees().unwrap(); - println!("txhash (init_committees): {}", txhash); - - ctx.dump_chain_logs(txhash); - - let txhash = ctx.delegate_committees().unwrap(); - println!("txhash (delegate_committees): {}", txhash); + for txhash in ctx.init_committees().unwrap() { + println!("txhash (init_committees): {}", txhash); + ctx.dump_chain_logs(txhash); + } - ctx.dump_chain_logs(txhash); + for txhash in ctx.delegate_committees().unwrap() { + println!("txhash (delegate_committees): {}", txhash); + ctx.dump_chain_logs(txhash); + } ctx } diff --git a/test-integration/schedulecommit/test-security/tests/01_invocations.rs b/test-integration/schedulecommit/test-security/tests/01_invocations.rs index cb47ade1b..71338735d 100644 --- a/test-integration/schedulecommit/test-security/tests/01_invocations.rs +++ b/test-integration/schedulecommit/test-security/tests/01_invocations.rs @@ -310,7 +310,7 @@ fn test_schedule_commit_via_direct_and_from_other_program_indirect_cpi_including None, players, pdas, - ScheduleCommitType::Commit, + ScheduleCommitType::CommitFinalize, ); let nested_cpi_ix = diff --git a/test-integration/test-committor-service/tests/common.rs b/test-integration/test-committor-service/tests/common.rs index db64b1d8b..ce8fd8885 100644 --- a/test-integration/test-committor-service/tests/common.rs +++ b/test-integration/test-committor-service/tests/common.rs @@ -22,7 +22,7 @@ use magicblock_committor_service::{ ComputeBudgetConfig, DEFAULT_ACTIONS_TIMEOUT, }; use magicblock_core::{ - intent::{BaseActionCallback, CommittedAccount}, + intent::{types::CommittedAccount, BaseActionCallback}, traits::{ActionResult, ActionsCallbackScheduler, CallbackScheduleError}, }; use magicblock_rpc_client::MagicblockRpcClient; diff --git a/test-integration/test-committor-service/tests/test_intent_executor.rs b/test-integration/test-committor-service/tests/test_intent_executor.rs index 26acbc58a..8c0dbba3c 100644 --- a/test-integration/test-committor-service/tests/test_intent_executor.rs +++ b/test-integration/test-committor-service/tests/test_intent_executor.rs @@ -36,15 +36,15 @@ use magicblock_committor_service::{ DEFAULT_ACTIONS_TIMEOUT, }; use magicblock_core::{ - intent::{BaseActionCallback, CommittedAccount}, + intent::{ + types::CommittedAccount, BaseAction, BaseActionCallback, + CommitAndUndelegate, CommitType, MagicBaseIntent, MagicIntentBundle, + ProgramArgs, UndelegateType, + }, traits::ActionError, }; use magicblock_program::{ - args::ShortAccountMeta, - magic_scheduled_base_intent::{ - BaseAction, CommitAndUndelegate, CommitType, MagicBaseIntent, - MagicIntentBundle, ProgramArgs, ScheduledIntentBundle, UndelegateType, - }, + args::ShortAccountMeta, magic_scheduled_base_intent::ScheduledIntentBundle, validator::validator_authority_id, }; use magicblock_rpc_client::MagicBlockSendTransactionConfig; diff --git a/test-integration/test-committor-service/tests/test_ix_commit_local.rs b/test-integration/test-committor-service/tests/test_ix_commit_local.rs index 72ae07121..224d68b6c 100644 --- a/test-integration/test-committor-service/tests/test_ix_commit_local.rs +++ b/test-integration/test-committor-service/tests/test_ix_commit_local.rs @@ -8,11 +8,11 @@ use magicblock_committor_service::{ persist::CommitStrategy, ComputeBudgetConfig, }; -use magicblock_core::intent::CommittedAccount; -use magicblock_program::magic_scheduled_base_intent::{ - CommitAndUndelegate, CommitType, MagicBaseIntent, MagicIntentBundle, - ScheduledIntentBundle, UndelegateType, +use magicblock_core::intent::{ + types::CommittedAccount, CommitAndUndelegate, CommitType, MagicBaseIntent, + MagicIntentBundle, UndelegateType, }; +use magicblock_program::magic_scheduled_base_intent::ScheduledIntentBundle; use program_flexi_counter::state::FlexiCounter; use solana_account::{Account, ReadableAccount}; use solana_commitment_config::CommitmentConfig; diff --git a/test-integration/test-committor-service/tests/test_transaction_preparator.rs b/test-integration/test-committor-service/tests/test_transaction_preparator.rs index 341385363..55b274134 100644 --- a/test-integration/test-committor-service/tests/test_transaction_preparator.rs +++ b/test-integration/test-committor-service/tests/test_transaction_preparator.rs @@ -5,16 +5,14 @@ use magicblock_committor_service::{ tasks::{ commit_stage_task::CleanupTask, task_strategist::{TaskStrategist, TransactionStrategy}, - utils::TransactionUtils, + utils::{create_commit_task, TransactionUtils}, BaseActionTask, BaseActionTaskV1, BaseTaskImpl, FinalizeTask, - TaskBuilderImpl, UndelegateTask, + UndelegateTask, }, transaction_preparator::TransactionPreparator, }; -use magicblock_program::{ - args::ShortAccountMeta, - magic_scheduled_base_intent::{BaseAction, ProgramArgs}, -}; +use magicblock_core::intent::{BaseAction, ProgramArgs}; +use magicblock_program::args::ShortAccountMeta; use solana_pubkey::Pubkey; use solana_sdk::signer::Signer; use solana_sdk_ids::system_program; @@ -36,13 +34,7 @@ async fn test_prepare_commit_tx_with_single_account() { let committed_account = create_committed_account(&account_data); let tasks: Vec = vec![ - TaskBuilderImpl::create_commit_task( - 1, - true, - committed_account.clone(), - None, - ) - .into(), + create_commit_task(1, true, committed_account.clone(), None).into(), FinalizeTask { delegated_account: committed_account.pubkey, } @@ -99,13 +91,7 @@ async fn test_prepare_commit_tx_with_multiple_accounts() { // Create test data let tasks: Vec = vec![ // account 1 - TaskBuilderImpl::create_commit_task( - 1, - true, - committed_account1.clone(), - None, - ) - .into(), + create_commit_task(1, true, committed_account1.clone(), None).into(), // account 2 buffer_commit_task.into(), // finalize account 1 diff --git a/test-integration/test-task-scheduler/tests/test_schedule_magic_cpi_crank.rs b/test-integration/test-task-scheduler/tests/test_schedule_magic_cpi_crank.rs index 554f49e3a..713233ba1 100644 --- a/test-integration/test-task-scheduler/tests/test_schedule_magic_cpi_crank.rs +++ b/test-integration/test-task-scheduler/tests/test_schedule_magic_cpi_crank.rs @@ -94,7 +94,7 @@ fn test_crank_can_execute_program_that_cpis_into_magic() { None, &[player.pubkey()], &[committee], - ScheduleCommitType::Commit, + ScheduleCommitType::CommitFinalize, ); crank_ix.accounts[0].is_writable = false;