diff --git a/magicblock-committor-service/src/committor_processor.rs b/magicblock-committor-service/src/committor_processor.rs index 549e5fe90..5fcf0e4ce 100644 --- a/magicblock-committor-service/src/committor_processor.rs +++ b/magicblock-committor-service/src/committor_processor.rs @@ -31,8 +31,7 @@ use crate::{ }, }, persist::{ - CommitStatusRow, IntentPersister, IntentPersisterImpl, - MessageSignatures, RecoveredIntent, + CommitStatusRow, IntentPersister, IntentPersisterImpl, RecoveredIntent, }, }; const POISONED_MUTEX_MSG: &str = @@ -143,17 +142,6 @@ impl CommittorProcessor { Ok(commit_statuses) } - pub fn get_commit_signature( - &self, - commit_id: u64, - pubkey: Pubkey, - ) -> CommittorServiceResult> { - let signatures = self - .persister - .get_signatures_by_commit(commit_id, &pubkey)?; - Ok(signatures) - } - fn recovery_min_created_at() -> u64 { SystemTime::now() .duration_since(UNIX_EPOCH) diff --git a/magicblock-committor-service/src/intent_executor/error.rs b/magicblock-committor-service/src/intent_executor/error.rs index e5661cce4..c431c0a91 100644 --- a/magicblock-committor-service/src/intent_executor/error.rs +++ b/magicblock-committor-service/src/intent_executor/error.rs @@ -208,8 +208,6 @@ pub enum TransactionStrategyExecutionError { #[source] TransactionError, Option, ), - #[error("Unfinalized account error: {0}, {1:?}")] - UnfinalizedAccountError(#[source] TransactionError, Option), #[error("Transaction too large to send over the wire: {0}")] TransactionTooLargeError(#[source] InternalError), #[error("InternalError: {0}")] @@ -245,32 +243,6 @@ impl TransactionStrategyExecutionError { || matches!(self, Self::TransactionTooLargeError(_)) } - pub fn task_index(&self, task_instruction_offset: u8) -> Option { - match self { - Self::CommitIDError( - TransactionError::InstructionError(index, _), - _, - ) - | Self::ActionsError( - TransactionError::InstructionError(index, _), - _, - ) - | Self::UndelegationError( - TransactionError::InstructionError(index, _), - _, - ) - | Self::UnfinalizedAccountError( - TransactionError::InstructionError(index, _), - _, - ) - | Self::CpiLimitError( - TransactionError::InstructionError(index, _), - _, - ) => index.checked_sub(task_instruction_offset), - _ => None, - } - } - pub fn signature(&self) -> Option { match self { Self::InternalError(err) => err.signature(), @@ -278,7 +250,6 @@ impl TransactionStrategyExecutionError { Self::CommitIDError(_, signature) | Self::ActionsError(_, signature) | Self::UndelegationError(_, signature) - | Self::UnfinalizedAccountError(_, signature) | Self::CpiLimitError(_, signature) | Self::LoadedAccountsDataSizeExceeded(_, signature) => *signature, } @@ -296,16 +267,6 @@ impl TransactionStrategyExecutionError { // Commit Nonce order error const NONCE_OUT_OF_ORDER: u32 = dlp_api::error::DlpError::NonceOutOfOrder as u32; - // Errors when commit state already exists - const COMMIT_STATE_INVALID_ACCOUNT_OWNER: u32 = - dlp_api::error::DlpError::CommitStateInvalidAccountOwner as u32; - const COMMIT_STATE_ALREADY_INITIALIZED: u32 = - dlp_api::error::DlpError::CommitStateAlreadyInitialized as u32; - const COMMIT_RECORD_INVALID_ACCOUNT_OWNER: u32 = - dlp_api::error::DlpError::CommitRecordInvalidAccountOwner as u32; - const COMMIT_RECORD_ALREADY_INITIALIZED: u32 = - dlp_api::error::DlpError::CommitRecordAlreadyInitialized as u32; - match err { // Some tx may use too much CPIs and we can handle it in certain cases transaction_err @ TransactionError::InstructionError( @@ -338,8 +299,7 @@ impl TransactionStrategyExecutionError { match (task, instruction_err) { ( - BaseTaskImpl::Commit(_) - | BaseTaskImpl::CommitFinalize(_), + BaseTaskImpl::CommitFinalize(_), instruction_err, ) => match instruction_err { InstructionError::Custom(NONCE_OUT_OF_ORDER) => Ok( @@ -350,25 +310,6 @@ impl TransactionStrategyExecutionError { signature, ), ), - // Only legacy commits use the temporary state/record - // PDAs that a separate Finalize can recover. - instruction_err @ (InstructionError::Custom( - COMMIT_STATE_INVALID_ACCOUNT_OWNER, - ) - | InstructionError::Custom( - COMMIT_STATE_ALREADY_INITIALIZED, - ) - | InstructionError::Custom( - COMMIT_RECORD_INVALID_ACCOUNT_OWNER, - ) - | InstructionError::Custom( - COMMIT_RECORD_ALREADY_INITIALIZED, - )) if matches!(task, BaseTaskImpl::Commit(_)) => { - Ok(TransactionStrategyExecutionError::UnfinalizedAccountError( - tx_err_helper(instruction_err), - signature - )) - } err => Err(tx_err_helper(err)), }, (BaseTaskImpl::BaseAction(_), instruction_err) => { @@ -383,7 +324,6 @@ impl TransactionStrategyExecutionError { signature, ), ), - (_, instruction_err) => Err(tx_err_helper(instruction_err)), } } // This means transaction failed to other reasons that we don't handle - propagate @@ -405,7 +345,6 @@ impl metrics::LabelValue for TransactionStrategyExecutionError { } Self::CommitIDError(_, _) => "commit_nonce_failed", Self::UndelegationError(_, _) => "undelegation_failed", - Self::UnfinalizedAccountError(_, _) => "unfinalized_account_failed", Self::TransactionTooLargeError(_) => "transaction_too_large", _ => "failed", } @@ -482,7 +421,7 @@ mod tests { const TX_TOO_LARGE_SOLANA: &str = "base64 encoded too large"; #[test] - fn combined_commits_do_not_trigger_legacy_finalization() { + fn combined_commit_errors_use_message_instruction_offset() { use dlp_api::error::DlpError; use magicblock_core::intent::types::CommittedAccount; use solana_account::Account; @@ -491,7 +430,7 @@ mod tests { use solana_transaction_error::TransactionError; use crate::tasks::utils::{ - create_commit_finalize_task, create_commit_task, TransactionUtils, + create_commit_finalize_task, TransactionUtils, }; let account = CommittedAccount { @@ -499,30 +438,21 @@ mod tests { account: Account::default(), remote_slot: 0, }; - let legacy = create_commit_task(1, false, account.clone(), None).into(); let combined = create_commit_finalize_task(1, false, account, None).into(); - let pending_errors = [ - DlpError::CommitStateInvalidAccountOwner, - DlpError::CommitStateAlreadyInitialized, - DlpError::CommitRecordInvalidAccountOwner, - DlpError::CommitRecordAlreadyInitialized, - ]; for offset in [0, TransactionUtils::COMPUTE_BUDGET_INSTRUCTION_COUNT] { - for pending_error in &pending_errors { - let error = TransactionError::InstructionError( + let unrelated_error = TransactionError::InstructionError( + offset, + InstructionError::InvalidAccountData, + ); + let result = + TransactionStrategyExecutionError::try_from_transaction_error( + unrelated_error.clone(), + None, + std::slice::from_ref(&combined), offset, - InstructionError::Custom(*pending_error as u32), - ); - let legacy_result = TransactionStrategyExecutionError::try_from_transaction_error( - error.clone(), None, std::slice::from_ref(&legacy), offset, ); - assert!(matches!(legacy_result, Ok(TransactionStrategyExecutionError::UnfinalizedAccountError(_, _)))); - let combined_result = TransactionStrategyExecutionError::try_from_transaction_error( - error.clone(), None, std::slice::from_ref(&combined), offset, - ); - assert_eq!(combined_result.unwrap_err(), error); - } + assert_eq!(result.unwrap_err(), unrelated_error); let nonce_error = TransactionError::InstructionError( offset, InstructionError::Custom(DlpError::NonceOutOfOrder as u32), diff --git a/magicblock-committor-service/src/intent_executor/mod.rs b/magicblock-committor-service/src/intent_executor/mod.rs index cf2120fbf..98eafac7f 100644 --- a/magicblock-committor-service/src/intent_executor/mod.rs +++ b/magicblock-committor-service/src/intent_executor/mod.rs @@ -45,7 +45,7 @@ use crate::{ }, persist::{CommitStatus, CommitStatusSignatures, IntentPersister}, tasks::{ - task_builder::{TaskBuilderImpl, TasksBuilder}, + task_builder::TaskBuilderImpl, task_strategist::{ StrategyExecutionMode, TaskStrategist, TransactionStrategy, }, @@ -443,10 +443,9 @@ where ) .await?; - let finalized_stage = finalize_executor.done(finalize_signature); Ok(ExecutionOutput::TwoStage { - commit_signature: finalized_stage.commit_signature, - finalize_signature: finalized_stage.finalize_signature, + commit_signature, + finalize_signature, }) } @@ -654,7 +653,6 @@ where /// alias its signature. Such intents must carry a per-intent uniqueness noop. fn requires_uniqueness_nonce(commit_tasks: &[BaseTaskImpl]) -> bool { commit_tasks.iter().any(|task| match task { - BaseTaskImpl::Commit(task) => task.commit_id <= 1, BaseTaskImpl::CommitFinalize(task) => task.commit_id <= 1, _ => false, }) @@ -666,10 +664,10 @@ mod tests { use solana_account::Account; use super::*; - use crate::tasks::{utils::create_commit_task, FinalizeTask}; + use crate::tasks::{utils::create_commit_finalize_task, UndelegateTask}; fn commit_task(commit_id: u64) -> BaseTaskImpl { - create_commit_task( + create_commit_finalize_task( commit_id, false, CommittedAccount { @@ -684,14 +682,17 @@ mod tests { #[test] fn test_requires_uniqueness_nonce_on_first_commit_only() { - let finalize = BaseTaskImpl::Finalize(FinalizeTask { + let undelegate = BaseTaskImpl::Undelegate(UndelegateTask { delegated_account: Pubkey::new_unique(), + owner_program: Pubkey::new_unique(), + rent_reimbursement: Pubkey::new_unique(), + include_undelegation_request: false, }); assert!(requires_uniqueness_nonce(&[commit_task(1)])); assert!(requires_uniqueness_nonce(&[commit_task(5), commit_task(1)])); assert!(!requires_uniqueness_nonce(&[commit_task(2)])); - assert!(!requires_uniqueness_nonce(&[finalize])); + assert!(!requires_uniqueness_nonce(&[undelegate])); assert!(!requires_uniqueness_nonce(&[])); } } diff --git a/magicblock-committor-service/src/intent_executor/single_stage_executor.rs b/magicblock-committor-service/src/intent_executor/single_stage_executor.rs index dc9c7ae7b..634945045 100644 --- a/magicblock-committor-service/src/intent_executor/single_stage_executor.rs +++ b/magicblock-committor-service/src/intent_executor/single_stage_executor.rs @@ -21,8 +21,8 @@ use crate::{ }, IntentExecutionReport, }, - persist::{IntentPersister, IntentPersisterImpl}, - tasks::{task_strategist::TransactionStrategy, BaseTaskImpl, FinalizeTask}, + persist::IntentPersister, + tasks::{task_strategist::TransactionStrategy, BaseTaskImpl}, transaction_preparator::TransactionPreparator, }; @@ -105,11 +105,7 @@ where // Attempt patching let flow = self - .patch_strategy( - &execution_err, - committed_pubkeys, - transaction_preparator, - ) + .patch_strategy(&execution_err, committed_pubkeys) .await?; let cleanup = match flow { ControlFlow::Continue(cleanup) => cleanup, @@ -170,12 +166,7 @@ where self.transaction_strategy .optimized_tasks .iter() - .rposition(|task| { - matches!( - task, - BaseTaskImpl::Commit(_) | BaseTaskImpl::CommitFinalize(_) - ) - }) + .rposition(|task| matches!(task, BaseTaskImpl::CommitFinalize(_))) .is_some_and(|index| { index + 1 < self.transaction_strategy.optimized_tasks.len() }) @@ -188,11 +179,10 @@ where /// [`TransactionStrategyExecutionError`], returning either: /// - `Continue(to_cleanup)` when a retry should be attempted with cleanup metadata, or /// - `Break(())` when this stage cannot be recovered here. - pub async fn patch_strategy( + pub async fn patch_strategy( &mut self, err: &TransactionStrategyExecutionError, committed_pubkeys: &[Pubkey], - transaction_preparator: &T, ) -> IntentExecutorResult> { if committed_pubkeys.is_empty() { // No patching is applicable if intent doesn't commit accounts @@ -227,44 +217,6 @@ where .await?; Ok(ControlFlow::Continue(to_cleanup)) } - err - @ TransactionStrategyExecutionError::UnfinalizedAccountError( - _, - signature, - ) => { - let optimized_tasks = - self.transaction_strategy.optimized_tasks.as_slice(); - let task_index = err.task_index( - self.transaction_strategy.task_instruction_offset(), - ); - if let Some(delegated_account) = task_index - .and_then(|index| optimized_tasks.get(index as usize)) - .and_then(|task| match task { - BaseTaskImpl::Commit(task) => { - Some(task.committed_account.pubkey) - } - BaseTaskImpl::CommitFinalize(task) => { - Some(task.committed_account.pubkey) - } - _ => None, - }) - { - self.handle_unfinalized_account_error( - signature, - delegated_account, - transaction_preparator, - ) - .await - } else { - error!( - task_index = ?task_index, - optimized_tasks_len = optimized_tasks.len(), - error = ?err, - "RPC returned unexpected task index" - ); - Ok(ControlFlow::Break(())) - } - } TransactionStrategyExecutionError::UndelegationError(_, _) => { // Here we patch strategy for it to be retried in next iteration // & we also record data that has to be cleaned up after patch @@ -287,40 +239,4 @@ where } } } - - /// Handles unfinalized account error - /// Sends a separate tx to finalize account and then continues execution - async fn handle_unfinalized_account_error( - &self, - failed_signature: &Option, - delegated_account: Pubkey, - transaction_preparator: &T, - ) -> IntentExecutorResult> { - let finalize_task: BaseTaskImpl = - FinalizeTask { delegated_account }.into(); - prepare_and_execute_strategy( - &self.intent_client, - &self.authority, - transaction_preparator, - &mut TransactionStrategy { - optimized_tasks: vec![finalize_task], - lookup_tables_keys: vec![], - uniqueness_nonce: None, - }, - &None::, - ) - .await - .map_err(IntentExecutorError::FailedFinalizePreparationError)? - .map_err(|err| IntentExecutorError::FailedToFinalizeError { - err, - commit_signature: None, - finalize_signature: *failed_signature, - })?; - - Ok(ControlFlow::Continue(TransactionStrategy { - optimized_tasks: vec![], - lookup_tables_keys: vec![], - uniqueness_nonce: None, - })) - } } 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 a3737920b..b6db607dd 100644 --- a/magicblock-committor-service/src/intent_executor/two_stage_executor.rs +++ b/magicblock-committor-service/src/intent_executor/two_stage_executor.rs @@ -22,8 +22,8 @@ use crate::{ }, IntentExecutionReport, }, - persist::{IntentPersister, IntentPersisterImpl}, - tasks::{task_strategist::TransactionStrategy, BaseTaskImpl, FinalizeTask}, + persist::IntentPersister, + tasks::task_strategist::TransactionStrategy, transaction_preparator::TransactionPreparator, }; @@ -45,13 +45,6 @@ pub struct Committed { current_attempt: u8, } -pub struct Finalized { - /// Signature of commit stage - pub commit_signature: Signature, - /// Signature of finalize stage - pub finalize_signature: Signature, -} - pub struct TwoStageExecutor<'a, A, S: Sealed> { state: S, intent_id: u64, @@ -140,7 +133,6 @@ where .patch_commit_strategy( &execution_err, task_info_fetcher, - transaction_preparator, committed_pubkeys, ) .await @@ -186,15 +178,13 @@ where /// [`TransactionStrategyExecutionError`], returning either: /// - `Continue(to_cleanup)` when a retry should be attempted with cleanup metadata, or /// - `Break(())` when this stage cannot be recovered. - pub async fn patch_commit_strategy( + pub async fn patch_commit_strategy( &mut self, err: &TransactionStrategyExecutionError, task_info_fetcher: &CacheTaskInfoFetcher, - transaction_preparator: &T, committed_pubkeys: &[Pubkey], ) -> IntentExecutorResult> where - T: TransactionPreparator, F: TaskInfoFetcher, { match err { @@ -216,44 +206,6 @@ where } Ok(ControlFlow::Continue(to_cleanup)) } - err - @ TransactionStrategyExecutionError::UnfinalizedAccountError( - _, - signature, - ) => { - let optimized_tasks = - self.state.commit_strategy.optimized_tasks.as_slice(); - let task_index = err.task_index( - self.state.commit_strategy.task_instruction_offset(), - ); - if let Some(delegated_account) = task_index - .and_then(|index| optimized_tasks.get(index as usize)) - .and_then(|task| match task { - BaseTaskImpl::Commit(task) => { - Some(task.committed_account.pubkey) - } - BaseTaskImpl::CommitFinalize(task) => { - Some(task.committed_account.pubkey) - } - _ => None, - }) - { - self.handle_unfinalized_account_error( - signature, - delegated_account, - transaction_preparator, - ) - .await - } else { - error!( - task_index = ?task_index, - optimized_tasks_len = optimized_tasks.len(), - error = ?err, - "RPC returned unexpected task index" - ); - Ok(ControlFlow::Break(())) - } - } TransactionStrategyExecutionError::ActionsError(err, signature) => { // Intent bundles allow for actions to be in commit stage let action_error = Err(ActionError::ActionsError(err.clone(), *signature)); @@ -294,41 +246,6 @@ where } } - /// Handles unfinalized account error - /// Sends a separate tx to finalize account and then continues execution - async fn handle_unfinalized_account_error( - &self, - failed_signature: &Option, - delegated_account: Pubkey, - transaction_preparator: &T, - ) -> IntentExecutorResult> { - let finalize_task: BaseTaskImpl = - FinalizeTask { delegated_account }.into(); - prepare_and_execute_strategy( - &self.intent_client, - &self.authority, - transaction_preparator, - &mut TransactionStrategy { - optimized_tasks: vec![finalize_task], - lookup_tables_keys: vec![], - uniqueness_nonce: None, - }, - &None::, - ) - .await - .map_err(IntentExecutorError::FailedCommitPreparationError)? - .map_err(|err| IntentExecutorError::FailedToCommitError { - err, - signature: *failed_signature, - })?; - - Ok(ControlFlow::Continue(TransactionStrategy { - optimized_tasks: vec![], - lookup_tables_keys: vec![], - uniqueness_nonce: None, - })) - } - /// Surrenders both strategies to the execution report so the engine /// cleanup releases partially prepared resources (ALT reservations, /// buffer accounts) of failed attempts @@ -532,11 +449,7 @@ where err: &TransactionStrategyExecutionError, ) -> IntentExecutorResult> { match err { - TransactionStrategyExecutionError::CommitIDError(_, _) - | TransactionStrategyExecutionError::UnfinalizedAccountError( - _, - _, - ) => { + TransactionStrategyExecutionError::CommitIDError(_, _) => { // Unexpected error in Two Stage commit error!(error = ?err, "Unexpected error in two stage finalize flow"); Ok(ControlFlow::Break(())) @@ -579,14 +492,6 @@ where } } } - - /// Transitions to next executor state - pub fn done(self, finalize_signature: Signature) -> Finalized { - Finalized { - commit_signature: self.state.commit_signature, - finalize_signature, - } - } } mod sealed { @@ -594,5 +499,4 @@ mod sealed { impl Sealed for super::Initialized {} impl Sealed for super::Committed {} - impl Sealed for super::Finalized {} } diff --git a/magicblock-committor-service/src/intent_executor/utils.rs b/magicblock-committor-service/src/intent_executor/utils.rs index 04b955810..38482399a 100644 --- a/magicblock-committor-service/src/intent_executor/utils.rs +++ b/magicblock-committor-service/src/intent_executor/utils.rs @@ -75,10 +75,6 @@ pub(in crate::intent_executor) async fn handle_commit_id_error< .optimized_tasks .iter() .filter_map(|task| match task { - BaseTaskImpl::Commit(task) => Some(( - task.committed_account.pubkey, - task.committed_account.remote_slot, - )), BaseTaskImpl::CommitFinalize(task) => Some(( task.committed_account.pubkey, task.committed_account.remote_slot, @@ -111,36 +107,19 @@ pub(in crate::intent_executor) async fn handle_commit_id_error< // Broken tasks are prepared incorrectly so they have to be cleaned up let mut to_cleanup = Vec::new(); for task in &mut strategy.optimized_tasks { - match task { - BaseTaskImpl::Commit(task) => { - let Some(commit_id) = - commit_ids.get(&task.committed_account.pubkey) - else { - continue; - }; - if commit_id == &task.commit_id { - continue; - } - - // Handle invalid tasks - to_cleanup.push(BaseTaskImpl::Commit(task.clone())); - task.reset_commit_id(*commit_id); + if let BaseTaskImpl::CommitFinalize(task) = task { + let Some(commit_id) = + commit_ids.get(&task.committed_account.pubkey) + else { + continue; + }; + if commit_id == &task.commit_id { + continue; } - BaseTaskImpl::CommitFinalize(task) => { - let Some(commit_id) = - commit_ids.get(&task.committed_account.pubkey) - else { - continue; - }; - if commit_id == &task.commit_id { - continue; - } - // Handle invalid tasks - to_cleanup.push(BaseTaskImpl::CommitFinalize(task.clone())); - task.reset_commit_id(*commit_id); - } - _ => {} + // Handle invalid tasks + to_cleanup.push(BaseTaskImpl::CommitFinalize(task.clone())); + task.reset_commit_id(*commit_id); } } @@ -173,12 +152,10 @@ pub(in crate::intent_executor) fn handle_cpi_limit_error( // We encountered error "Max instruction trace length exceeded" // All the tasks a prepared to be executed at this point // We attempt Two stages commit flow, need to split tasks up - let last_commit_ind = strategy.optimized_tasks.iter().rposition(|el| { - matches!( - el, - BaseTaskImpl::Commit(_) | BaseTaskImpl::CommitFinalize(_) - ) - }); + let last_commit_ind = strategy + .optimized_tasks + .iter() + .rposition(|el| matches!(el, BaseTaskImpl::CommitFinalize(_))); let (mut commit_stage_tasks, mut finalize_stage_tasks) = (vec![], vec![]); for (i, el) in strategy.optimized_tasks.into_iter().enumerate() { if Some(i) <= last_commit_ind { @@ -460,7 +437,7 @@ mod tests { intent_executor::task_info_fetcher::{ AccountSnapshot, TaskInfoFetcherResult, }, - tasks::utils::create_commit_task, + tasks::utils::create_commit_finalize_task, }; /// Reports commit id 1 except for one unchanged account at commit id 5. @@ -531,7 +508,7 @@ mod tests { let unchanged_pubkey = Pubkey::new_unique(); // Stale cache produced commit id 5; on chain the account was // re-delegated, so the correct commit id is 1. - let stale_task = create_commit_task( + let stale_task = create_commit_finalize_task( 5, false, CommittedAccount { @@ -544,7 +521,7 @@ mod tests { let mut strategy = TransactionStrategy { optimized_tasks: vec![ stale_task.into(), - create_commit_task( + create_commit_finalize_task( 5, false, CommittedAccount { diff --git a/magicblock-committor-service/src/persist/error.rs b/magicblock-committor-service/src/persist/error.rs index 49e0df299..8e7e00ab9 100644 --- a/magicblock-committor-service/src/persist/error.rs +++ b/magicblock-committor-service/src/persist/error.rs @@ -25,17 +25,6 @@ pub enum CommitPersistError { #[error("Invalid Commit Strategy: '{0}' ({0:?})")] InvalidCommitStrategy(String), - #[error( - "Commit Status update requires status with bundle id: '{0}' ({0:?})" - )] - CommitStatusUpdateRequiresStatusWithBundleId(String), - - #[error("Commit Status needs bundle id: '{0}' ({0:?})")] - CommitStatusNeedsBundleId(String), - #[error("Commit Status needs signatures: '{0}' ({0:?})")] CommitStatusNeedsSignatures(String), - - #[error("Commit Status needs commit strategy: '{0}' ({0:?})")] - CommitStatusNeedsStrategy(String), } diff --git a/magicblock-committor-service/src/persist/types/commit_status.rs b/magicblock-committor-service/src/persist/types/commit_status.rs index 9942dd296..7be69f40f 100644 --- a/magicblock-committor-service/src/persist/types/commit_status.rs +++ b/magicblock-committor-service/src/persist/types/commit_status.rs @@ -148,14 +148,4 @@ impl CommitStatus { _ => None, } } - - /// The commit fully succeeded and no retry is necessary. - pub fn is_complete(&self) -> bool { - use CommitStatus::*; - matches!(self, Succeeded(_)) - } - - pub fn all_completed(stages: &[Self]) -> bool { - stages.iter().all(Self::is_complete) - } } diff --git a/magicblock-committor-service/src/persist/types/commit_strategy.rs b/magicblock-committor-service/src/persist/types/commit_strategy.rs index 3bf007333..e05005578 100644 --- a/magicblock-committor-service/src/persist/types/commit_strategy.rs +++ b/magicblock-committor-service/src/persist/types/commit_strategy.rs @@ -36,16 +36,6 @@ impl CommitStrategy { DiffBufferWithLookupTable => "DiffBufferWithLookupTable", } } - - pub fn uses_lookup(&self) -> bool { - matches!( - self, - CommitStrategy::StateArgsWithLookupTable - | CommitStrategy::StateBufferWithLookupTable - | CommitStrategy::DiffArgsWithLookupTable - | CommitStrategy::DiffBufferWithLookupTable - ) - } } impl TryFrom<&str> for CommitStrategy { diff --git a/magicblock-committor-service/src/tasks/commit_delivery.rs b/magicblock-committor-service/src/tasks/commit_delivery.rs new file mode 100644 index 000000000..eec7d9687 --- /dev/null +++ b/magicblock-committor-service/src/tasks/commit_delivery.rs @@ -0,0 +1,21 @@ +use solana_account::Account; + +/// Describes how commit data is delivered to the base layer. +/// +/// Small accounts send data directly in instruction args. +/// Large accounts use an on-chain buffer to avoid transaction size limits. +/// When a base account is available, a diff is computed to reduce payload size. +#[derive(Clone, Debug)] +pub enum CommitDelivery { + StateInArgs, + StateInBuffer { + prepared: bool, + }, + DiffInArgs { + base_account: Account, + }, + DiffInBuffer { + base_account: Account, + prepared: bool, + }, +} diff --git a/magicblock-committor-service/src/tasks/commit_finalize_task.rs b/magicblock-committor-service/src/tasks/commit_finalize_task.rs index 9cf60b44e..8687479b9 100644 --- a/magicblock-committor-service/src/tasks/commit_finalize_task.rs +++ b/magicblock-committor-service/src/tasks/commit_finalize_task.rs @@ -11,7 +11,7 @@ use solana_account::{Account, ReadableAccount}; use solana_instruction::Instruction; use solana_pubkey::Pubkey; -use crate::tasks::{commit_task::CommitDelivery, BaseTask, BaseTaskImpl}; +use crate::tasks::{commit_delivery::CommitDelivery, BaseTask, BaseTaskImpl}; /// A task that commits a delegated account's state to the base layer and finalizes it in the same /// instruction. @@ -116,10 +116,6 @@ impl CommitFinalizeTask { } impl BaseTask for CommitFinalizeTask { - fn program_id(&self) -> Pubkey { - dlp_api::id() - } - fn instruction(&self, validator: &Pubkey) -> Instruction { match &self.delivery { CommitDelivery::StateInArgs => { @@ -166,7 +162,7 @@ impl BaseTask for CommitFinalizeTask { fn accounts_size_budget(&self) -> u32 { match &self.delivery { - CommitDelivery::StateInArgs => { + CommitDelivery::StateInArgs | CommitDelivery::DiffInArgs { .. } => { commit_finalize_size_budget(AccountSizeClass::Dynamic( self.committed_account.account.data.len() as u32, )) @@ -175,11 +171,6 @@ impl BaseTask for CommitFinalizeTask { | CommitDelivery::DiffInBuffer { .. } => { commit_finalize_from_buffer_size_budget(AccountSizeClass::Huge) } - CommitDelivery::DiffInArgs { .. } => { - commit_finalize_size_budget(AccountSizeClass::Dynamic( - self.committed_account.account.data.len() as u32, - )) - } } } } diff --git a/magicblock-committor-service/src/tasks/commit_stage_task.rs b/magicblock-committor-service/src/tasks/commit_stage_task.rs index 65da85987..74f49bb34 100644 --- a/magicblock-committor-service/src/tasks/commit_stage_task.rs +++ b/magicblock-committor-service/src/tasks/commit_stage_task.rs @@ -16,8 +16,8 @@ use solana_instruction::Instruction; use crate::{ consts::MAX_WRITE_CHUNK_SIZE, tasks::{ + commit_delivery::CommitDelivery, commit_finalize_task::CommitFinalizeTask, - commit_task::{CommitDelivery, CommitTask}, }, }; @@ -31,47 +31,6 @@ pub struct PreparationTask<'a> { } impl<'a> PreparationTask<'a> { - pub fn from_commit(task: &'a mut CommitTask) -> Option { - match &mut task.delivery_details { - CommitDelivery::StateInArgs | CommitDelivery::DiffInArgs { .. } => { - None - } - CommitDelivery::StateInBuffer { prepared } => { - let buffer_data = task.committed_account.account.data.clone(); - let chunks = Chunks::from_data_length( - buffer_data.len(), - MAX_WRITE_CHUNK_SIZE, - ); - Some(Self { - commit_id: task.commit_id, - pubkey: task.committed_account.pubkey, - buffer_data, - chunks, - prepared, - }) - } - CommitDelivery::DiffInBuffer { - base_account, - prepared, - } => { - let diff = compute_diff( - base_account.data.as_ref(), - &task.committed_account.account.data, - ) - .to_vec(); - let chunks = - Chunks::from_data_length(diff.len(), MAX_WRITE_CHUNK_SIZE); - Some(Self { - commit_id: task.commit_id, - pubkey: task.committed_account.pubkey, - buffer_data: diff, - chunks, - prepared, - }) - } - } - } - pub fn from_commit_finalize( task: &'a mut CommitFinalizeTask, ) -> Option { @@ -139,11 +98,6 @@ impl<'a> PreparationTask<'a> { instruction } - /// Returns compute units required for realloc instruction - pub fn init_compute_units(&self) -> u32 { - 12_000 - } - /// Returns realloc instruction required for Buffer preparation #[allow(clippy::let_and_return)] pub fn realloc_instructions(&self, authority: &Pubkey) -> Vec { @@ -159,11 +113,6 @@ impl<'a> PreparationTask<'a> { realloc_instructions } - /// Returns compute units required for realloc instruction - pub fn realloc_compute_units(&self) -> u32 { - 6_000 - } - /// Returns realloc instruction required for Buffer preparation #[allow(clippy::let_and_return)] pub fn write_instructions(&self, authority: &Pubkey) -> Vec { @@ -185,33 +134,6 @@ impl<'a> PreparationTask<'a> { write_instructions } - pub fn write_compute_units(&self, bytes_count: usize) -> u32 { - const PER_BYTE: u32 = 3; - - u32::try_from(bytes_count) - .ok() - .and_then(|bytes_count| bytes_count.checked_mul(PER_BYTE)) - .unwrap_or(u32::MAX) - } - - pub fn chunks_pda(&self, authority: &Pubkey) -> Pubkey { - pdas::chunks_pda( - authority, - &self.pubkey, - self.commit_id.to_le_bytes().as_slice(), - ) - .0 - } - - pub fn buffer_pda(&self, authority: &Pubkey) -> Pubkey { - pdas::buffer_pda( - authority, - &self.pubkey, - self.commit_id.to_le_bytes().as_slice(), - ) - .0 - } - pub fn cleanup_task(&self) -> CleanupTask { CleanupTask { pubkey: self.pubkey, @@ -231,19 +153,6 @@ pub struct CleanupTask { } impl CleanupTask { - pub fn from_commit(task: &CommitTask) -> Option { - match &task.delivery_details { - CommitDelivery::StateInBuffer { prepared: true } - | CommitDelivery::DiffInBuffer { prepared: true, .. } => { - Some(Self { - commit_id: task.commit_id, - pubkey: task.committed_account.pubkey, - }) - } - _ => None, - } - } - pub fn from_commit_finalize(task: &CommitFinalizeTask) -> Option { match &task.delivery { CommitDelivery::StateInBuffer { prepared: true } diff --git a/magicblock-committor-service/src/tasks/commit_task.rs b/magicblock-committor-service/src/tasks/commit_task.rs deleted file mode 100644 index cdbd7c3f7..000000000 --- a/magicblock-committor-service/src/tasks/commit_task.rs +++ /dev/null @@ -1,233 +0,0 @@ -use dlp_api::{ - args::{CommitDiffArgs, CommitStateArgs, CommitStateFromBufferArgs}, - diff::compute_diff, - AccountSizeClass, -}; -use magicblock_core::intent::types::CommittedAccount; -use solana_account::{Account, ReadableAccount}; -use solana_instruction::Instruction; -use solana_pubkey::Pubkey; - -use crate::tasks::{BaseTask, BaseTaskImpl}; - -/// Describes how commit data is delivered to the base layer. -/// -/// Small accounts send data directly in instruction args. -/// Large accounts use an on-chain buffer to avoid transaction size limits. -/// When a base account is available, a diff is computed to reduce payload size. -#[derive(Clone, Debug)] -pub enum CommitDelivery { - StateInArgs, - StateInBuffer { - prepared: bool, - }, - DiffInArgs { - base_account: Account, - }, - DiffInBuffer { - base_account: Account, - prepared: bool, - }, -} - -/// A task that commits a delegated account's state to the base layer. -/// -/// The delivery strategy ([`CommitDelivery`]) determines how the data reaches -/// the chain (inline args vs buffer, full state vs diff). -#[derive(Clone, Debug)] -pub struct CommitTask { - pub commit_id: u64, - pub allow_undelegation: bool, - pub committed_account: CommittedAccount, - pub delivery_details: CommitDelivery, -} - -impl CommitTask { - #[inline(always)] - fn commit_state_ix(&self, validator: &Pubkey) -> Instruction { - let args = CommitStateArgs { - nonce: self.commit_id, - lamports: self.committed_account.account.lamports, - data: self.committed_account.account.data.clone(), - allow_undelegation: self.allow_undelegation, - }; - dlp_api::instruction_builder::commit_state( - *validator, - self.committed_account.pubkey, - self.committed_account.account.owner, - args, - ) - } - - #[inline(always)] - fn commit_state_from_buffer_ix(&self, validator: &Pubkey) -> Instruction { - let (commit_buffer_pubkey, _) = - magicblock_committor_program::pdas::buffer_pda( - validator, - &self.committed_account.pubkey, - &self.commit_id.to_le_bytes(), - ); - dlp_api::instruction_builder::commit_state_from_buffer( - *validator, - self.committed_account.pubkey, - self.committed_account.account.owner, - commit_buffer_pubkey, - CommitStateFromBufferArgs { - nonce: self.commit_id, - lamports: self.committed_account.account.lamports, - allow_undelegation: self.allow_undelegation, - }, - ) - } - - #[inline(always)] - fn commit_diff_ix( - &self, - validator: &Pubkey, - base_account: &Account, - ) -> Instruction { - let args = CommitDiffArgs { - nonce: self.commit_id, - lamports: self.committed_account.account.lamports, - diff: compute_diff( - base_account.data(), - self.committed_account.account.data(), - ) - .to_vec(), - allow_undelegation: self.allow_undelegation, - }; - - dlp_api::instruction_builder::commit_diff( - *validator, - self.committed_account.pubkey, - self.committed_account.account.owner, - args, - ) - } - - #[inline(always)] - fn commit_diff_from_buffer_ix(&self, validator: &Pubkey) -> Instruction { - let (commit_buffer_pubkey, _) = - magicblock_committor_program::pdas::buffer_pda( - validator, - &self.committed_account.pubkey, - &self.commit_id.to_le_bytes(), - ); - dlp_api::instruction_builder::commit_diff_from_buffer( - *validator, - self.committed_account.pubkey, - self.committed_account.account.owner, - commit_buffer_pubkey, - CommitStateFromBufferArgs { - nonce: self.commit_id, - lamports: self.committed_account.account.lamports, - allow_undelegation: self.allow_undelegation, - }, - ) - } - - pub fn is_buffer(&self) -> bool { - matches!( - self.delivery_details, - CommitDelivery::StateInBuffer { .. } - | CommitDelivery::DiffInBuffer { .. } - ) - } - - pub fn reset_commit_id(&mut self, commit_id: u64) { - self.commit_id = commit_id; - match &mut self.delivery_details { - CommitDelivery::StateInBuffer { prepared } - | CommitDelivery::DiffInBuffer { prepared, .. } => { - *prepared = false - } - _ => {} - }; - } -} - -impl BaseTask for CommitTask { - fn program_id(&self) -> Pubkey { - dlp_api::id() - } - - fn instruction(&self, validator: &Pubkey) -> Instruction { - match &self.delivery_details { - CommitDelivery::StateInArgs => self.commit_state_ix(validator), - CommitDelivery::StateInBuffer { .. } => { - self.commit_state_from_buffer_ix(validator) - } - CommitDelivery::DiffInArgs { base_account } => { - self.commit_diff_ix(validator, base_account) - } - CommitDelivery::DiffInBuffer { .. } => { - self.commit_diff_from_buffer_ix(validator) - } - } - } - - fn try_optimize_tx_size(&mut self) -> bool { - let details = std::mem::replace( - &mut self.delivery_details, - CommitDelivery::StateInArgs, - ); - match details { - CommitDelivery::StateInArgs => { - self.delivery_details = - CommitDelivery::StateInBuffer { prepared: false }; - true - } - CommitDelivery::DiffInArgs { base_account } => { - self.delivery_details = CommitDelivery::DiffInBuffer { - base_account, - prepared: false, - }; - true - } - other @ (CommitDelivery::StateInBuffer { .. } - | CommitDelivery::DiffInBuffer { .. }) => { - self.delivery_details = other; - false - } - } - } - - fn compute_units(&self) -> u32 { - u32::try_from(self.committed_account.account.data.len()) - .unwrap_or(u32::MAX) - .saturating_mul(32) - .saturating_add(72_000) - .max(120_000) - } - - fn accounts_size_budget(&self) -> u32 { - match &self.delivery_details { - CommitDelivery::StateInArgs => { - dlp_api::instruction_builder::commit_size_budget( - AccountSizeClass::Dynamic( - self.committed_account.account.data.len() as u32, - ), - ) - } - CommitDelivery::StateInBuffer { .. } - | CommitDelivery::DiffInBuffer { .. } => { - dlp_api::instruction_builder::commit_size_budget( - AccountSizeClass::Huge, - ) - } - CommitDelivery::DiffInArgs { .. } => { - dlp_api::instruction_builder::commit_diff_size_budget( - AccountSizeClass::Dynamic( - self.committed_account.account.data.len() as u32, - ), - ) - } - } - } -} - -impl From for BaseTaskImpl { - fn from(value: CommitTask) -> Self { - Self::Commit(value) - } -} diff --git a/magicblock-committor-service/src/tasks/intent_size_validator.rs b/magicblock-committor-service/src/tasks/intent_size_validator.rs index 601712192..d50893918 100644 --- a/magicblock-committor-service/src/tasks/intent_size_validator.rs +++ b/magicblock-committor-service/src/tasks/intent_size_validator.rs @@ -9,7 +9,7 @@ use solana_signer::Signer; use crate::{ tasks::{ - commit_task::CommitDelivery, + commit_delivery::CommitDelivery, task_strategist::TaskStrategist, utils::{ create_action_tasks, create_commit_finalize_task, TransactionUtils, @@ -25,7 +25,7 @@ use crate::{ /// Estimates whether an intent's commit and finalize stages fit the supported /// transaction formats without fetching base-layer state or commit metadata. /// -/// Unlike [`crate::tasks::task_builder::TasksBuilder`], admission cannot compute +/// Unlike [`crate::tasks::task_builder::TaskBuilderImpl`], admission cannot compute /// the actual account diff. It estimates larger accounts using buffers, reserves /// distinct keys for unknown rent payers, and includes a uniqueness noop in each /// stage. Each stage is checked against v1, then v0 with full ALT coverage. @@ -78,7 +78,7 @@ impl IntentSizeValidator { } /// Builds the finalize-stage tasks used for the size estimate, mirroring - /// [`crate::tasks::task_builder::TasksBuilder::finalize_tasks`] but + /// [`crate::tasks::task_builder::TaskBuilderImpl::finalize_tasks`] but /// without fetching rent payers. [`Self::tasks_fit`] assigns distinct /// placeholder keys before compiling these tasks, accounting for each /// unknown payer's contribution to the transaction size and account count. @@ -160,20 +160,6 @@ impl IntentSizeValidator { /// 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_finalize_task( - 0, - false, - account.clone(), - Some(account.account.clone()), - ); - if matches!(task.delivery, 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, @@ -194,7 +180,7 @@ impl IntentSizeValidator { commit_type .get_committed_accounts() .iter() - .map(Self::commit_task) + .map(Self::commit_finalize_task) .collect() } @@ -409,12 +395,6 @@ mod tests { finalizes.len(), usize::from(!combined) + usize::from(undelegate) ); - assert!(commits.iter().chain(&finalizes).all( - |task| !matches!( - task, - BaseTaskImpl::Commit(_) | BaseTaskImpl::Finalize(_) - ) - )); assert!(IntentSizeValidator::fits(&intent)); } } @@ -544,7 +524,7 @@ mod tests { Some(0), ); known_keys.push(authority.pubkey()); - known_keys.extend(original.iter().map(BaseTask::program_id)); + known_keys.push(dlp_api::id()); assert!(payers.iter().all(|payer| !known_keys.contains(payer))); assert_eq!(wire_size(&estimated), wire_size(&actual)); diff --git a/magicblock-committor-service/src/tasks/mod.rs b/magicblock-committor-service/src/tasks/mod.rs index 5a4be8689..4cfe539d7 100644 --- a/magicblock-committor-service/src/tasks/mod.rs +++ b/magicblock-committor-service/src/tasks/mod.rs @@ -4,9 +4,9 @@ use magicblock_metrics::metrics::LabelValue; use solana_instruction::{AccountMeta, Instruction}; use solana_pubkey::Pubkey; +pub mod commit_delivery; 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; @@ -14,44 +14,19 @@ pub mod utils; pub use task_builder::TaskBuilderImpl; -use crate::tasks::{ - commit_finalize_task::CommitFinalizeTask, commit_task::CommitTask, -}; - -#[derive(Clone, Copy, PartialEq, Eq, Debug)] -pub enum TaskType { - Commit, - CommitFinalize, - Finalize, - Undelegate, - Action, -} - -#[derive(Copy, Clone, PartialEq, Eq, Debug)] -pub enum TaskStrategy { - Args, - Buffer, -} +use crate::tasks::commit_finalize_task::CommitFinalizeTask; #[derive(Clone, Debug)] pub enum BaseTaskImpl { - Commit(CommitTask), CommitFinalize(CommitFinalizeTask), - Finalize(FinalizeTask), Undelegate(UndelegateTask), BaseAction(BaseActionTask), } impl BaseTask for BaseTaskImpl { - fn program_id(&self) -> Pubkey { - dlp_api::id() - } - fn instruction(&self, validator: &Pubkey) -> Instruction { match self { - Self::Commit(value) => value.instruction(validator), Self::CommitFinalize(value) => value.instruction(validator), - Self::Finalize(value) => value.instruction(validator), Self::Undelegate(value) => value.instruction(validator), Self::BaseAction(value) => value.instruction(validator), } @@ -59,7 +34,6 @@ impl BaseTask for BaseTaskImpl { fn try_optimize_tx_size(&mut self) -> bool { match self { - Self::Commit(value) => value.try_optimize_tx_size(), Self::CommitFinalize(value) => value.try_optimize_tx_size(), _ => false, } @@ -67,24 +41,16 @@ impl BaseTask for BaseTaskImpl { fn compute_units(&self) -> u32 { match self { - Self::Commit(value) => value.compute_units(), Self::CommitFinalize(value) => value.compute_units(), Self::BaseAction(value) => value.compute_units(), - Self::Finalize(_) => 120_000, Self::Undelegate(_) => 120_000, } } fn accounts_size_budget(&self) -> u32 { match self { - Self::Commit(value) => value.accounts_size_budget(), Self::CommitFinalize(value) => value.accounts_size_budget(), Self::BaseAction(value) => value.accounts_size_budget(), - Self::Finalize(_) => { - dlp_api::instruction_builder::finalize_size_budget( - AccountSizeClass::Huge, - ) - } Self::Undelegate(value) => { if value.include_undelegation_request { dlp_api::instruction_builder::undelegate_with_request_size_budget( @@ -100,28 +66,9 @@ impl BaseTask for BaseTaskImpl { } } -impl BaseTaskImpl { - pub fn strategy(&self) -> TaskStrategy { - match self { - Self::Commit(task) if task.is_buffer() => TaskStrategy::Buffer, - Self::CommitFinalize(task) if task.is_buffer() => { - TaskStrategy::Buffer - } - _ => TaskStrategy::Args, - } - } -} - impl LabelValue for BaseTaskImpl { fn value(&self) -> &str { match self { - Self::Commit(task) => { - if task.is_buffer() { - "buffer_commit" - } else { - "args_commit" - } - } Self::CommitFinalize(task) => { if task.is_buffer() { "buffer_commit_finalize" @@ -129,7 +76,6 @@ impl LabelValue for BaseTaskImpl { "args_commit_finalize" } } - Self::Finalize(_) => "args_finalize", Self::Undelegate(_) => "args_undelegate", Self::BaseAction(BaseActionTask::V1(_)) => "args_action", Self::BaseAction(BaseActionTask::V2(_)) => "args_action_v2", @@ -148,9 +94,6 @@ pub trait BaseTask: Send + Sync + Clone { .collect() } - /// Gets target program for task execution - fn program_id(&self) -> Pubkey; - /// Gets instruction for task execution fn instruction(&self, validator: &Pubkey) -> Instruction; @@ -202,26 +145,6 @@ impl From for BaseTaskImpl { } } -#[derive(Clone, Debug)] -pub struct FinalizeTask { - pub delegated_account: Pubkey, -} - -impl FinalizeTask { - pub fn instruction(&self, validator: &Pubkey) -> Instruction { - dlp_api::instruction_builder::finalize( - *validator, - self.delegated_account, - ) - } -} - -impl From for BaseTaskImpl { - fn from(value: FinalizeTask) -> Self { - Self::Finalize(value) - } -} - #[derive(Clone, Debug)] pub enum BaseActionTask { V1(BaseActionTaskV1), @@ -294,37 +217,16 @@ pub struct BaseActionTaskV1 { impl BaseActionTaskV1 { pub fn instruction(&self, validator: &Pubkey) -> Instruction { let action = &self.action; - let account_metas = action - .account_metas_per_program - .iter() - .map(|short_meta| AccountMeta { - pubkey: short_meta.pubkey, - is_writable: short_meta.is_writable, - is_signer: false, - }) - .collect(); - #[allow(deprecated)] dlp_api::instruction_builder::call_handler( *validator, action.destination_program, action.escrow_authority, - account_metas, - CallHandlerArgs { - data: action.data_per_program.data.clone(), - escrow_index: action.data_per_program.escrow_index, - }, + Self::account_metas_static(action), + Self::call_handler_args_static(action), ) } - pub fn account_metas(&self) -> Vec { - BaseActionTaskV1::account_metas_static(&self.action) - } - - pub fn call_handler_args(&self) -> CallHandlerArgs { - BaseActionTaskV1::call_handler_args_static(&self.action) - } - fn account_metas_static(action: &BaseAction) -> Vec { action .account_metas_per_program @@ -392,126 +294,14 @@ impl From for BaseActionTask { } #[cfg(test)] -mod serialization_safety_test { - +mod tests { use dlp_api::{ discriminator::DlpDiscriminator, pda::undelegation_request_pda_from_delegated_account, }; - use magicblock_core::intent::{types::CommittedAccount, ProgramArgs}; - use magicblock_program::args::ShortAccountMeta; - use solana_account::Account; + use solana_pubkey::Pubkey; - use crate::{ - tasks::{ - commit_stage_task::PreparationTask, - commit_task::{CommitDelivery, CommitTask}, - *, - }, - test_utils, - }; - - fn setup() { - test_utils::init_test_logger(); - } - - fn make_commit_task( - commit_id: u64, - allow_undelegation: bool, - data: Vec, - lamports: u64, - ) -> CommitTask { - CommitTask { - commit_id, - allow_undelegation, - committed_account: CommittedAccount { - pubkey: Pubkey::new_unique(), - account: Account { - lamports, - data, - owner: Pubkey::new_unique(), - executable: false, - rent_epoch: 0, - }, - remote_slot: Default::default(), - }, - delivery_details: CommitDelivery::StateInArgs, - } - } - - #[test] - fn test_args_task_instruction_serialization() { - setup(); - let validator = Pubkey::new_unique(); - - // Test Commit variant (StateInArgs) - let commit_task: BaseTaskImpl = - make_commit_task(123, true, vec![1, 2, 3], 1000).into(); - assert_serializable(&commit_task.instruction(&validator)); - - // Test Finalize variant - let finalize_task: BaseTaskImpl = FinalizeTask { - delegated_account: Pubkey::new_unique(), - } - .into(); - assert_serializable(&finalize_task.instruction(&validator)); - - // Test Undelegate variant - let undelegate_task: BaseTaskImpl = UndelegateTask { - delegated_account: Pubkey::new_unique(), - owner_program: Pubkey::new_unique(), - rent_reimbursement: Pubkey::new_unique(), - include_undelegation_request: false, - } - .into(); - assert_serializable(&undelegate_task.instruction(&validator)); - - // Test BaseAction V1 variant - let base_action: BaseTaskImpl = BaseActionTask::V1(BaseActionTaskV1 { - action: BaseAction { - id: 0, - destination_program: Pubkey::new_unique(), - source_program: None, - escrow_authority: Pubkey::new_unique(), - account_metas_per_program: vec![ShortAccountMeta { - pubkey: Pubkey::new_unique(), - is_writable: true, - }], - data_per_program: ProgramArgs { - data: vec![4, 5, 6], - escrow_index: 1, - }, - compute_units: 10_000, - callback: None, - }, - }) - .into(); - assert_serializable(&base_action.instruction(&validator)); - - // Test BaseAction V2 variant - let base_action_v2: BaseTaskImpl = - BaseActionTask::V2(BaseActionTaskV2 { - action: BaseAction { - id: 0, - destination_program: Pubkey::new_unique(), - source_program: Some(Pubkey::new_unique()), - escrow_authority: Pubkey::new_unique(), - account_metas_per_program: vec![ShortAccountMeta { - pubkey: Pubkey::new_unique(), - is_writable: true, - }], - data_per_program: ProgramArgs { - data: vec![7, 8, 9], - escrow_index: 2, - }, - compute_units: 15_000, - callback: None, - }, - source_program: Pubkey::new_unique(), - }) - .into(); - assert_serializable(&base_action_v2.instruction(&validator)); - } + use super::UndelegateTask; #[test] fn test_undelegate_task_uses_request_account_when_included() { @@ -535,57 +325,6 @@ mod serialization_safety_test { assert!(ix.accounts[12].is_writable); assert!(!ix.accounts[12].is_signer); } - - fn make_buffer_commit_task( - commit_id: u64, - allow_undelegation: bool, - data: Vec, - lamports: u64, - ) -> CommitTask { - let task = - make_commit_task(commit_id, allow_undelegation, data, lamports); - CommitTask { - delivery_details: CommitDelivery::StateInBuffer { prepared: false }, - ..task - } - } - - #[test] - fn test_buffer_task_instruction_serialization() { - let validator = Pubkey::new_unique(); - - let commit_task = - make_buffer_commit_task(456, false, vec![7, 8, 9], 2000); - assert!(commit_task.is_buffer()); - assert_serializable(&commit_task.instruction(&validator)); - } - - #[test] - fn test_preparation_instructions_serialization() { - let authority = Pubkey::new_unique(); - - let mut commit_task = - make_buffer_commit_task(789, true, vec![0; 1024], 3000); - - let Some(preparation_task) = - PreparationTask::from_commit(&mut commit_task) - else { - panic!("invalid preparation state on creation!"); - }; - assert_serializable(&preparation_task.init_instruction(&authority)); - for ix in preparation_task.realloc_instructions(&authority) { - assert_serializable(&ix); - } - for ix in preparation_task.write_instructions(&authority) { - assert_serializable(&ix); - } - } - - fn assert_serializable(ix: &Instruction) { - bincode::serialize(ix).unwrap_or_else(|e| { - panic!("Failed to serialize instruction {:?}: {}", ix, e) - }); - } } #[test] diff --git a/magicblock-committor-service/src/tasks/task_builder.rs b/magicblock-committor-service/src/tasks/task_builder.rs index bca2f961a..cb68d1600 100644 --- a/magicblock-committor-service/src/tasks/task_builder.rs +++ b/magicblock-committor-service/src/tasks/task_builder.rs @@ -1,6 +1,5 @@ use std::{collections::HashMap, sync::Arc}; -use async_trait::async_trait; use dlp_api::state::{DelegationMetadata, UndelegationRequester}; use magicblock_core::intent::{ types::CommittedAccount, CommitAndUndelegate, CommitType, UndelegateType, @@ -25,22 +24,6 @@ use crate::{ }, }; -#[async_trait] -pub trait TasksBuilder { - // Creates tasks for commit stage - async fn commit_tasks( - commit_id_fetcher: &Arc, - base_intent: &ScheduledIntentBundle, - persister: &Option

, - ) -> TaskBuilderResult>; - - // Create tasks for finalize stage - async fn finalize_tasks( - info_fetcher: &Arc, - base_intent: &ScheduledIntentBundle, - ) -> TaskBuilderResult>; -} - /// Necessary info for commit stage task creation pub struct CommitStageTaskInfo { /// commit nonce for a given address @@ -148,12 +131,9 @@ impl TaskBuilderImpl { base_accounts, }) } -} -#[async_trait] -impl TasksBuilder for TaskBuilderImpl { /// Returns [`BaseTaskImpl`]s for Commit stage - async fn commit_tasks( + pub async fn commit_tasks( task_info_fetcher: &Arc, intent_bundle: &ScheduledIntentBundle, persister: &Option

, @@ -177,51 +157,55 @@ impl TasksBuilder for TaskBuilderImpl { // Create tasks per intent type if let Some(ref value) = intent_bundle.intent_bundle.commit { - tasks.extend( - CommitBuilder { - commit_nonces: &mut commit_nonces, - base_accounts: &mut base_accounts, - } - .build(value), - ); + tasks.extend(build_commit_tasks( + value, + false, + &mut commit_nonces, + &mut base_accounts, + )); } if let Some(ref value) = intent_bundle.intent_bundle.commit_finalize { - tasks.extend( - CommitFinalizeBuilder { - commit_nonces: &mut commit_nonces, - base_accounts: &mut base_accounts, - } - .build(value), - ); + tasks.extend(build_commit_tasks( + value, + false, + &mut commit_nonces, + &mut base_accounts, + )); + if let CommitType::WithBaseActions { base_actions, .. } = value { + tasks.extend(create_action_tasks(base_actions)); + } } if let Some(ref value) = intent_bundle.intent_bundle.commit_and_undelegate { - tasks.extend( - CommitAndUndelegateBuilder { - commit_nonces: &mut commit_nonces, - base_accounts: &mut base_accounts, - } - .build(&value.commit_action), - ); + tasks.extend(build_commit_tasks( + &value.commit_action, + true, + &mut commit_nonces, + &mut base_accounts, + )); } if let Some(ref value) = intent_bundle.intent_bundle.commit_finalize_and_undelegate { - tasks.extend( - CommitFinalizeAndUndelegateBuilder { - commit_nonces: &mut commit_nonces, - base_accounts: &mut base_accounts, - } - .build(&value.commit_action), - ); + tasks.extend(build_commit_tasks( + &value.commit_action, + true, + &mut commit_nonces, + &mut base_accounts, + )); + if let CommitType::WithBaseActions { base_actions, .. } = + &value.commit_action + { + tasks.extend(create_action_tasks(base_actions)); + } } Ok(tasks) } - /// Returns [`Task`]s for Finalize stage - async fn finalize_tasks( + /// Builds actions and undelegations to run after the combined commits. + pub async fn finalize_tasks( info_fetcher: &Arc, intent_bundle: &ScheduledIntentBundle, ) -> TaskBuilderResult> { @@ -320,102 +304,27 @@ impl TasksBuilder for TaskBuilderImpl { } } -struct CommitBuilder<'a> { - commit_nonces: &'a mut HashMap, - base_accounts: &'a mut HashMap, -} - -impl<'a> CommitBuilder<'a> { - fn build(&mut self, commit_type: &CommitType) -> Vec { - commit_type - .get_committed_accounts() - .iter() - .map(|account| { - let nonce = - take_commit_nonce(self.commit_nonces, account.pubkey); - let base = self.base_accounts.remove(&account.pubkey); - create_commit_finalize_task(nonce, false, account.clone(), base) - .into() - }) - .collect() - } -} - -struct CommitAndUndelegateBuilder<'a> { - commit_nonces: &'a mut HashMap, - base_accounts: &'a mut HashMap, -} - -impl<'a> CommitAndUndelegateBuilder<'a> { - fn build(&mut self, commit_type: &CommitType) -> Vec { - commit_type - .get_committed_accounts() - .iter() - .map(|account| { - let nonce = - take_commit_nonce(self.commit_nonces, account.pubkey); - let base = self.base_accounts.remove(&account.pubkey); - create_commit_finalize_task(nonce, true, account.clone(), base) - .into() - }) - .collect() - } -} - -struct CommitFinalizeBuilder<'a> { - commit_nonces: &'a mut HashMap, - base_accounts: &'a mut HashMap, -} - -impl<'a> CommitFinalizeBuilder<'a> { - fn build(&mut self, commit_type: &CommitType) -> Vec { - let mut tasks: Vec = commit_type - .get_committed_accounts() - .iter() - .map(|account| { - let nonce = - take_commit_nonce(self.commit_nonces, account.pubkey); - let base = self.base_accounts.remove(&account.pubkey); - create_commit_finalize_task(nonce, false, account.clone(), base) - .into() - }) - .collect(); - if let CommitType::WithBaseActions { - ref base_actions, .. - } = commit_type - { - tasks.extend(create_action_tasks(base_actions)); - } - tasks - } -} - -struct CommitFinalizeAndUndelegateBuilder<'a> { - commit_nonces: &'a mut HashMap, - base_accounts: &'a mut HashMap, -} - -impl<'a> CommitFinalizeAndUndelegateBuilder<'a> { - fn build(&mut self, commit_type: &CommitType) -> Vec { - let mut tasks: Vec = commit_type - .get_committed_accounts() - .iter() - .map(|account| { - let nonce = - take_commit_nonce(self.commit_nonces, account.pubkey); - let base = self.base_accounts.remove(&account.pubkey); - create_commit_finalize_task(nonce, true, account.clone(), base) - .into() - }) - .collect(); - if let CommitType::WithBaseActions { - ref base_actions, .. - } = commit_type - { - tasks.extend(create_action_tasks(base_actions)); - } - tasks - } +fn build_commit_tasks( + commit_type: &CommitType, + allow_undelegation: bool, + commit_nonces: &mut HashMap, + base_accounts: &mut HashMap, +) -> Vec { + commit_type + .get_committed_accounts() + .iter() + .map(|account| { + let nonce = take_commit_nonce(commit_nonces, account.pubkey); + let base = base_accounts.remove(&account.pubkey); + create_commit_finalize_task( + nonce, + allow_undelegation, + account.clone(), + base, + ) + .into() + }) + .collect() } fn take_commit_nonce( diff --git a/magicblock-committor-service/src/tasks/task_strategist.rs b/magicblock-committor-service/src/tasks/task_strategist.rs index c6c043a69..270ce034b 100644 --- a/magicblock-committor-service/src/tasks/task_strategist.rs +++ b/magicblock-committor-service/src/tasks/task_strategist.rs @@ -9,8 +9,8 @@ use tracing::error; use crate::{ persist::{CommitStrategy, IntentPersister}, tasks::{ - commit_task::CommitDelivery, utils::TransactionUtils, BaseActionTask, - BaseTask, BaseTaskImpl, + commit_delivery::CommitDelivery, utils::TransactionUtils, + BaseActionTask, BaseTask, BaseTaskImpl, }, transactions::{ serialized_transaction_size, MAX_TRANSACTION_V1_WIRE_SIZE, @@ -95,19 +95,6 @@ impl TransactionStrategy { .any(BaseActionTask::has_callback) } - /// Task transactions use v0 with ALTs and v1 without them; see - /// [`TaskStrategist::build_strategy`]. V0 prepends compute-budget - /// instructions, while v1 stores budgets in its configuration. - pub(crate) fn task_instruction_offset(&self) -> u8 { - if self.uses_alts() { - // In our design, ALTS implies transaction v0 which in turn implies - // compute-budget instructions are passed explicitly - TransactionUtils::COMPUTE_BUDGET_INSTRUCTION_COUNT - } else { - 0 - } - } - pub fn uses_alts(&self) -> bool { !self.lookup_tables_keys.is_empty() } @@ -453,13 +440,6 @@ impl TaskStrategist { for task in tasks { let (commit_id, pubkey, commit_strategy) = match task { - BaseTaskImpl::Commit(commit_task) => ( - commit_task.commit_id, - commit_task.committed_account.pubkey, - commit_strategy_from_delivery( - &commit_task.delivery_details, - ), - ), BaseTaskImpl::CommitFinalize(commit_finalize_task) => ( commit_finalize_task.commit_id, commit_finalize_task.committed_account.pubkey, @@ -595,11 +575,10 @@ mod tests { }, persist::IntentPersisterImpl, tasks::{ - commit_task::CommitTask, - task_builder::{TaskBuilderImpl, TasksBuilder}, - utils::{create_commit_task, COMMIT_STATE_SIZE_THRESHOLD}, - BaseActionTask, BaseActionTaskV1, FinalizeTask, TaskStrategy, - UndelegateTask, + commit_finalize_task::CommitFinalizeTask, + task_builder::TaskBuilderImpl, + utils::{create_commit_finalize_task, COMMIT_STATE_SIZE_THRESHOLD}, + BaseActionTask, BaseActionTaskV1, UndelegateTask, }, test_utils, }; @@ -669,7 +648,7 @@ mod tests { commit_id: u64, data_size: usize, diff_len: usize, - ) -> CommitTask { + ) -> CommitFinalizeTask { let committed_account = CommittedAccount { pubkey: Pubkey::new_unique(), account: Account { @@ -681,7 +660,12 @@ mod tests { }; if diff_len == 0 { - create_commit_task(commit_id, false, committed_account, None) + create_commit_finalize_task( + commit_id, + false, + committed_account, + None, + ) } else { let base_account = { let mut acc = committed_account.account.clone(); @@ -691,7 +675,7 @@ mod tests { } acc }; - create_commit_task( + create_commit_finalize_task( commit_id, false, committed_account, @@ -720,11 +704,11 @@ mod tests { .into() } - // Helper to create a finalize task - fn create_test_finalize_task() -> FinalizeTask { - FinalizeTask { - delegated_account: Pubkey::new_unique(), - } + fn commit_delivery(task: &BaseTaskImpl) -> &CommitDelivery { + let BaseTaskImpl::CommitFinalize(task) = task else { + panic!("expected a combined commit"); + }; + &task.delivery } // Helper to create an undelegate task @@ -773,26 +757,31 @@ mod tests { assert_eq!(strategy.optimized_tasks.len(), 1); assert!(matches!( - strategy.optimized_tasks[0].strategy(), - TaskStrategy::Buffer + commit_delivery(&strategy.optimized_tasks[0]), + CommitDelivery::StateInBuffer { .. } )); } #[test] - fn test_build_strategy_optimizes_to_buffer_u16_exceeded() { + fn test_build_strategy_buffers_payload_above_u16_limit() { let validator = Pubkey::new_unique(); let task = create_test_commit_task(1, 66_000, 0); // Large task let tasks = vec![task.into()]; - let result = TaskStrategist::build_strategy( + let strategy = TaskStrategist::build_strategy( tasks, &validator, &None::, None, - ); + ) + .expect("Buffer delivery must fit even when inline data exceeds u16"); - assert!(matches!(result, Err(TaskStrategistError::FailedToFitError))); + assert!(matches!( + commit_delivery(&strategy.optimized_tasks[0]), + CommitDelivery::StateInBuffer { .. } + )); + assert!(strategy.lookup_tables_keys.is_empty()); } #[test] @@ -812,7 +801,10 @@ mod tests { .expect("Should build strategy with buffer optimization"); assert_eq!(strategy.optimized_tasks.len(), 1); - assert_eq!(strategy.optimized_tasks[0].strategy(), TaskStrategy::Args); + assert!(matches!( + commit_delivery(&strategy.optimized_tasks[0]), + CommitDelivery::DiffInArgs { .. } + )); } #[test] @@ -833,7 +825,10 @@ mod tests { .expect("Should build strategy with buffer optimization"); assert_eq!(strategy.optimized_tasks.len(), 1); - assert_eq!(strategy.optimized_tasks[0].strategy(), TaskStrategy::Args); + assert!(matches!( + commit_delivery(&strategy.optimized_tasks[0]), + CommitDelivery::DiffInArgs { .. } + )); } #[test] @@ -852,10 +847,10 @@ mod tests { .expect("Should build strategy with buffer optimization"); assert_eq!(strategy.optimized_tasks.len(), 1); - assert_eq!( - strategy.optimized_tasks[0].strategy(), - TaskStrategy::Buffer - ); + assert!(matches!( + commit_delivery(&strategy.optimized_tasks[0]), + CommitDelivery::DiffInBuffer { .. } + )); } #[test] @@ -880,26 +875,41 @@ mod tests { ) .expect("Should build strategy with buffer optimization"); - for optimized_task in strategy.optimized_tasks { - assert!(matches!(optimized_task.strategy(), TaskStrategy::Buffer)); + for optimized_task in &strategy.optimized_tasks { + assert!(matches!( + commit_delivery(optimized_task), + CommitDelivery::StateInBuffer { .. } + )); } assert!(strategy.lookup_tables_keys.is_empty()); } #[test] fn test_build_strategy_with_lookup_tables_when_needed() { - // Also max number of committed accounts fit with ALTs! + // Eleven combined commits fit within the compute budget. Extra action + // accounts push their buffered form beyond V1's static-key limit. const NUM_COMMITS: u64 = 11; let validator = Pubkey::new_unique(); - let tasks = (0..NUM_COMMITS) + let mut tasks: Vec = (0..NUM_COMMITS) .map(|i| { // Large task let task = create_test_commit_task(i, 1000, 0); task.into() }) .collect(); + let mut action = create_test_base_action_task(0); + let BaseActionTask::V1(action_task) = &mut action else { + panic!("expected a V1 action"); + }; + action_task.action.account_metas_per_program = (0..20) + .map(|_| ShortAccountMeta { + pubkey: Pubkey::new_unique(), + is_writable: false, + }) + .collect(); + tasks.push(action.into()); let strategy = TaskStrategist::build_strategy( tasks, @@ -909,8 +919,12 @@ mod tests { ) .expect("Should build strategy with buffer optimization"); - for optimized_task in strategy.optimized_tasks { - assert!(matches!(optimized_task.strategy(), TaskStrategy::Buffer)); + for optimized_task in &strategy.optimized_tasks[..NUM_COMMITS as usize] + { + assert!(matches!( + commit_delivery(optimized_task), + CommitDelivery::StateInBuffer { .. } + )); } assert!(!strategy.lookup_tables_keys.is_empty()); } @@ -981,10 +995,10 @@ mod tests { .expect("should fall back to v0 + ALTs"); assert!(!strategy.lookup_tables_keys.is_empty()); - assert!(strategy - .optimized_tasks - .iter() - .all(|task| task.strategy() == TaskStrategy::Args)); + assert!(matches!( + strategy.optimized_tasks.as_slice(), + [BaseTaskImpl::BaseAction(_)] + )); } #[test] @@ -1063,8 +1077,14 @@ mod tests { MAX_TRANSACTION_V1_WIRE_SIZE, ); // The larger task should have been optimized first - assert!(matches!(tasks[0].strategy(), TaskStrategy::Args)); - assert!(matches!(tasks[1].strategy(), TaskStrategy::Buffer)); + assert!(matches!( + commit_delivery(&tasks[0]), + CommitDelivery::StateInArgs + )); + assert!(matches!( + commit_delivery(&tasks[1]), + CommitDelivery::StateInBuffer { .. } + )); } #[test] @@ -1072,7 +1092,7 @@ mod tests { let validator = Pubkey::new_unique(); let tasks: Vec = vec![ create_test_commit_task(1, 5000, 0).into(), - create_test_finalize_task().into(), + create_test_undelegate_task().into(), create_test_base_action_task(500).into(), create_test_undelegate_task().into(), ]; @@ -1087,21 +1107,15 @@ mod tests { assert_eq!(strategy.optimized_tasks.len(), 4); - let strategies: Vec = strategy - .optimized_tasks - .iter() - .map(|t| t.strategy()) - .collect(); - - assert_eq!( - strategies, - vec![ - TaskStrategy::Buffer, // Commit task optimized - TaskStrategy::Args, // Finalize stays - TaskStrategy::Args, // BaseAction stays - TaskStrategy::Args, // Undelegate stays - ] - ); + assert!(matches!( + strategy.optimized_tasks.as_slice(), + [ + BaseTaskImpl::CommitFinalize(task), + BaseTaskImpl::Undelegate(_), + BaseTaskImpl::BaseAction(_), + BaseTaskImpl::Undelegate(_), + ] if matches!(task.delivery, CommitDelivery::StateInBuffer { .. }) + )); assert!(strategy.lookup_tables_keys.is_empty()); } @@ -1359,49 +1373,42 @@ mod tests { } #[tokio::test] - async fn test_build_two_stage_mode_when_task_count_exceeds_single_stage_limit( - ) { - let mut intent = create_test_intent(0, &[], false); - intent.intent_bundle.standalone_actions = (0..23) - .map(|_| BaseAction { - id: 0, - destination_program: Pubkey::new_unique(), - source_program: None, - escrow_authority: Pubkey::new_unique(), - account_metas_per_program: vec![], - data_per_program: ProgramArgs { - data: vec![], - escrow_index: 0, - }, - compute_units: 30_000, - callback: None, - }) - .collect(); - + async fn test_build_two_stage_mode_when_combined_compute_budget_exceeded() { + let pubkeys: [Pubkey; 6] = + std::array::from_fn(|_| Pubkey::new_unique()); + let intent = create_test_intent(0, &pubkeys, true); let info_fetcher = Arc::new(MockInfoFetcher::default()); - let commit_task = TaskBuilderImpl::commit_tasks( + let commit_tasks = TaskBuilderImpl::commit_tasks( &info_fetcher, &intent, &None::, ) .await .unwrap(); - let finalize_task = + let finalize_tasks = TaskBuilderImpl::finalize_tasks(&info_fetcher, &intent) .await .unwrap(); + // Six commits and six undelegations need 1.44M CU together; + // each group fits separately with 720K CU. let execution_mode = TaskStrategist::build_execution_strategy( - commit_task, - finalize_task, + commit_tasks, + finalize_tasks, &Pubkey::new_unique(), &None::, None, ) - .expect("Execution mode created"); - - let StrategyExecutionMode::TwoStage { .. } = execution_mode else { - panic!("Unexpected execution mode"); + .expect("Both transactions fit separately"); + + let StrategyExecutionMode::TwoStage { + commit_stage, + finalize_stage, + } = execution_mode + else { + panic!("expected two transactions"); }; + assert_eq!(commit_stage.optimized_tasks.len(), 6); + assert_eq!(finalize_stage.optimized_tasks.len(), 6); } } diff --git a/magicblock-committor-service/src/tasks/utils.rs b/magicblock-committor-service/src/tasks/utils.rs index 4bd4cb054..83e537408 100644 --- a/magicblock-committor-service/src/tasks/utils.rs +++ b/magicblock-committor-service/src/tasks/utils.rs @@ -17,21 +17,16 @@ use solana_transaction::versioned::VersionedTransaction; use crate::{ tasks::{ + commit_delivery::CommitDelivery, commit_finalize_task::CommitFinalizeTask, - commit_task::{CommitDelivery, CommitTask}, - task_strategist::TaskStrategistResult, - BaseActionTask, BaseActionTaskV1, BaseActionTaskV2, BaseTask, - BaseTaskImpl, + task_strategist::TaskStrategistResult, BaseActionTask, + BaseActionTaskV1, BaseActionTaskV2, BaseTask, BaseTaskImpl, }, transactions::v1, }; -// 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. +// Small accounts send full state in CommitFinalize. Above this threshold, +// compute a diff when a base account is available to reduce the payload. pub const COMMIT_STATE_SIZE_THRESHOLD: usize = 256; /// Builds a [`BaseTaskImpl`] for each `action`, used by both @@ -59,8 +54,6 @@ pub fn create_action_tasks( /// 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, @@ -79,25 +72,7 @@ fn commit_delivery( } } -/// Builds a legacy [`CommitTask`] for `account`. Intent execution and admission -/// use [`create_commit_finalize_task`]; this helper remains for legacy tasks. -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`]. +/// Builds a combined commit and finalization task for `account`. pub fn create_commit_finalize_task( commit_id: u64, allow_undelegation: bool, @@ -165,21 +140,6 @@ impl TransactionUtils { .collect() } - pub fn assemble_tasks_tx( - authority: &Keypair, - tasks: &[BaseTaskImpl], - compute_unit_price: u64, - lookup_tables: &[AddressLookupTableAccount], - ) -> TaskStrategistResult { - Self::assemble_tasks_tx_with_uniqueness_nonce( - authority, - tasks, - compute_unit_price, - lookup_tables, - None, - ) - } - pub fn assemble_tasks_tx_with_uniqueness_nonce( authority: &Keypair, tasks: &[BaseTaskImpl], @@ -342,27 +302,19 @@ impl TransactionUtils { let total_budget: u32 = tasks.iter().map(|task| task.accounts_size_budget()).sum(); - let dlp_task_count: u32 = tasks - .iter() - .filter(|task| task.program_id() == dlp_api::id()) - .count() as u32; - - if dlp_task_count > 0 { - let dlp_program_budget = DLP_PROGRAM_DATA_SIZE_CLASS.size_budget(); - let deduction = dlp_task_count - .saturating_sub(1) - .saturating_mul(dlp_program_budget); - // The API's 350 KiB estimate is smaller than the deployed DLP. - // Reserve at least 1 MiB for its program data, counted only once. - let program_headroom = AccountSizeClass::Huge - .size_budget() - .saturating_sub(dlp_program_budget); - total_budget - .saturating_sub(deduction) - .saturating_add(program_headroom) - } else { - total_budget - } + // All tasks target DLP, so count its program data only once. + let dlp_program_budget = DLP_PROGRAM_DATA_SIZE_CLASS.size_budget(); + let deduction = (tasks.len() as u32) + .saturating_sub(1) + .saturating_mul(dlp_program_budget); + // The API's 350 KiB estimate is smaller than the deployed DLP. + // Reserve at least 1 MiB for its program data, counted only once. + let program_headroom = AccountSizeClass::Huge + .size_budget() + .saturating_sub(dlp_program_budget); + total_budget + .saturating_sub(deduction) + .saturating_add(program_headroom) } fn tasks_accounts_size_budget_with_uniqueness_nonce( diff --git a/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs b/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs index 4e0140c34..d4471b8b4 100644 --- a/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs +++ b/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs @@ -110,9 +110,6 @@ impl DeliveryPreparator { uniqueness_nonce: Option, ) -> DeliveryPreparatorResult<(), InternalError> { let preparation_task = match task { - BaseTaskImpl::Commit(commit_task) => { - PreparationTask::from_commit(commit_task) - } BaseTaskImpl::CommitFinalize(commit_finalize_task) => { PreparationTask::from_commit_finalize(commit_finalize_task) } @@ -195,9 +192,6 @@ impl DeliveryPreparator { // Preparation failed due to buffer existing - cleanup and retry let preparation_task = match task { - BaseTaskImpl::Commit(commit_task) => { - PreparationTask::from_commit(commit_task) - } BaseTaskImpl::CommitFinalize(commit_finalize_task) => { PreparationTask::from_commit_finalize(commit_finalize_task) } diff --git a/magicblock-committor-service/src/transaction_preparator/mod.rs b/magicblock-committor-service/src/transaction_preparator/mod.rs index 4a3f5906d..6241928b0 100644 --- a/magicblock-committor-service/src/transaction_preparator/mod.rs +++ b/magicblock-committor-service/src/transaction_preparator/mod.rs @@ -147,9 +147,6 @@ impl TransactionPreparator for TransactionPreparatorImpl { .optimized_tasks .iter() .filter_map(|task| match task { - BaseTaskImpl::Commit(commit_task) => { - CleanupTask::from_commit(commit_task) - } BaseTaskImpl::CommitFinalize(commit_finalize_task) => { CleanupTask::from_commit_finalize(commit_finalize_task) } diff --git a/test-integration/test-committor-service/tests/common.rs b/test-integration/test-committor-service/tests/common.rs index 3141fa3fa..7a1a91d9f 100644 --- a/test-integration/test-committor-service/tests/common.rs +++ b/test-integration/test-committor-service/tests/common.rs @@ -1,26 +1,17 @@ -use std::{ - collections::HashMap, - sync::{ - atomic::{AtomicU64, Ordering}, - Arc, Mutex, - }, +use std::sync::{ + atomic::{AtomicU64, Ordering}, + Arc, Mutex, }; -use async_trait::async_trait; -use dlp_api::state::{DelegationMetadata, UndelegationRequester}; use magicblock_committor_service::{ - intent_executor::{ - task_info_fetcher::{ - AccountSnapshot, CacheTaskInfoFetcher, TaskInfoFetcher, - TaskInfoFetcherError, TaskInfoFetcherResult, - }, - IntentExecutorImpl, + tasks::{ + commit_delivery::CommitDelivery, + commit_finalize_task::CommitFinalizeTask, }, - tasks::commit_task::{CommitDelivery, CommitTask}, transaction_preparator::{ delivery_preparator::DeliveryPreparator, TransactionPreparatorImpl, }, - ComputeBudgetConfig, DEFAULT_ACTIONS_TIMEOUT, + ComputeBudgetConfig, }; use magicblock_core::{ intent::{types::CommittedAccount, BaseActionCallback}, @@ -131,34 +122,6 @@ impl TestFixture { self.compute_budget_config.clone(), ) } - - #[allow(dead_code)] - pub fn create_intent_executor( - &self, - ) -> IntentExecutorImpl< - TransactionPreparatorImpl, - MockTaskInfoFetcher, - MockActionsCallbackExecutor, - > { - let transaction_preparator = self.create_transaction_preparator(); - - IntentExecutorImpl::new( - self.rpc_client.clone(), - transaction_preparator, - self.create_task_info_fetcher(), - MockActionsCallbackExecutor::default(), - DEFAULT_ACTIONS_TIMEOUT, - ) - } - - #[allow(dead_code)] - pub fn create_task_info_fetcher( - &self, - ) -> Arc> { - Arc::new(CacheTaskInfoFetcher::new(MockTaskInfoFetcher( - self.rpc_client.clone(), - ))) - } } type CallbackCalls = Vec<(Vec, ActionResult)>; @@ -191,68 +154,6 @@ impl ActionsCallbackScheduler for MockActionsCallbackExecutor { } } -pub struct MockTaskInfoFetcher(MagicblockRpcClient); - -#[async_trait] -impl TaskInfoFetcher for MockTaskInfoFetcher { - async fn fetch_next_commit_nonces( - &self, - accounts: &[AccountSnapshot], - _: u64, - ) -> TaskInfoFetcherResult> { - Ok(accounts.iter().map(|(pubkey, _)| (*pubkey, 0)).collect()) - } - - async fn fetch_current_commit_nonces( - &self, - accounts: &[AccountSnapshot], - _: u64, - ) -> TaskInfoFetcherResult> { - Ok(accounts.iter().map(|(pubkey, _)| (*pubkey, 0)).collect()) - } - - async fn fetch_delegation_metadata( - &self, - accounts: &[AccountSnapshot], - _: u64, - ) -> TaskInfoFetcherResult> { - Ok(accounts - .iter() - .map(|(pubkey, _)| { - ( - *pubkey, - DelegationMetadata { - last_commit_id: 0, - undelegation_requester: UndelegationRequester::None, - seeds: vec![], - rent_payer: *pubkey, - }, - ) - }) - .collect()) - } - - async fn get_base_accounts( - &self, - pubkeys: &[Pubkey], - _: u64, - ) -> TaskInfoFetcherResult> { - self.0 - .get_multiple_accounts(pubkeys, None) - .await - .map_err(|err| { - TaskInfoFetcherError::MagicBlockRpcClientError(Box::new(err)) - }) - .map(|accounts| { - pubkeys - .iter() - .zip(accounts) - .filter_map(|(key, value)| value.map(|value| (*key, value))) - .collect() - }) - } -} - #[allow(dead_code)] pub fn generate_random_bytes(length: usize) -> Vec { use rand::Rng; @@ -262,9 +163,9 @@ pub fn generate_random_bytes(length: usize) -> Vec { } #[allow(dead_code)] -pub fn create_commit_task(data: &[u8]) -> CommitTask { +pub fn create_commit_finalize_task(data: &[u8]) -> CommitFinalizeTask { static COMMIT_ID: AtomicU64 = AtomicU64::new(0); - CommitTask { + CommitFinalizeTask { commit_id: COMMIT_ID.fetch_add(1, Ordering::Relaxed), allow_undelegation: false, committed_account: CommittedAccount { @@ -278,15 +179,15 @@ pub fn create_commit_task(data: &[u8]) -> CommitTask { }, remote_slot: Default::default(), }, - delivery_details: CommitDelivery::StateInArgs, + delivery: CommitDelivery::StateInArgs, } } #[allow(dead_code)] -pub fn create_buffer_commit_task(data: &[u8]) -> CommitTask { - let task = create_commit_task(data); - CommitTask { - delivery_details: CommitDelivery::StateInBuffer { prepared: false }, +pub fn create_buffer_commit_finalize_task(data: &[u8]) -> CommitFinalizeTask { + let task = create_commit_finalize_task(data); + CommitFinalizeTask { + delivery: CommitDelivery::StateInBuffer { prepared: false }, ..task } } diff --git a/test-integration/test-committor-service/tests/test_delivery_preparator.rs b/test-integration/test-committor-service/tests/test_delivery_preparator.rs index 2a5417b53..ea1341a4d 100644 --- a/test-integration/test-committor-service/tests/test_delivery_preparator.rs +++ b/test-integration/test-committor-service/tests/test_delivery_preparator.rs @@ -3,8 +3,8 @@ use magicblock_committor_program::Chunks; use magicblock_committor_service::{ persist::IntentPersisterImpl, tasks::{ + commit_delivery::CommitDelivery, commit_stage_task::{CleanupTask, PreparationTask}, - commit_task::CommitDelivery, task_strategist::{TaskStrategist, TransactionStrategy}, BaseTaskImpl, }, @@ -12,8 +12,8 @@ use magicblock_committor_service::{ use solana_sdk::signer::Signer; use crate::common::{ - create_buffer_commit_task, create_commit_task, generate_random_bytes, - TestFixture, + create_buffer_commit_finalize_task, create_commit_finalize_task, + generate_random_bytes, TestFixture, }; mod common; @@ -25,7 +25,7 @@ async fn test_prepare_10kb_buffer() { let data = generate_random_bytes(10 * 1024); let mut strategy = TransactionStrategy { - optimized_tasks: vec![create_buffer_commit_task(&data).into()], + optimized_tasks: vec![create_buffer_commit_finalize_task(&data).into()], lookup_tables_keys: vec![], uniqueness_nonce: None, }; @@ -42,11 +42,13 @@ async fn test_prepare_10kb_buffer() { assert!(result.is_ok(), "Preparation failed: {:?}", result.err()); // Verify the buffer account was created and initialized - let BaseTaskImpl::Commit(ref commit_task) = strategy.optimized_tasks[0] + let BaseTaskImpl::CommitFinalize(ref commit_task) = + strategy.optimized_tasks[0] else { panic!("unexpected task type"); }; - let Some(cleanup_task) = CleanupTask::from_commit(commit_task) else { + let Some(cleanup_task) = CleanupTask::from_commit_finalize(commit_task) + else { panic!("unexpected CommitStage"); }; @@ -91,7 +93,7 @@ async fn test_prepare_multiple_buffers() { ]; let buffer_tasks: Vec = datas .iter() - .map(|data| create_buffer_commit_task(data).into()) + .map(|data| create_buffer_commit_finalize_task(data).into()) .collect(); let mut strategy = TransactionStrategy { optimized_tasks: buffer_tasks, @@ -115,8 +117,8 @@ async fn test_prepare_multiple_buffers() { .optimized_tasks .iter() .filter_map(|el| match el { - BaseTaskImpl::Commit(commit_task) => { - CleanupTask::from_commit(commit_task) + BaseTaskImpl::CommitFinalize(commit_task) => { + CleanupTask::from_commit_finalize(commit_task) } _ => None, }) @@ -168,7 +170,7 @@ async fn test_lookup_tables() { ]; let tasks: Vec = datas .iter() - .map(|data| create_commit_task(data).into()) + .map(|data| create_commit_finalize_task(data).into()) .collect(); let lookup_tables_keys = TaskStrategist::collect_lookup_table_keys( @@ -211,7 +213,7 @@ async fn test_already_initialized_error_handled() { let preparator = fixture.create_delivery_preparator(); let data = generate_random_bytes(10 * 1024); - let mut commit_task = create_buffer_commit_task(&data); + let mut commit_task = create_buffer_commit_finalize_task(&data); let mut strategy = TransactionStrategy { optimized_tasks: vec![commit_task.clone().into()], lookup_tables_keys: vec![], @@ -229,10 +231,11 @@ async fn test_already_initialized_error_handled() { assert!(result.is_ok(), "Preparation failed: {:?}", result.err()); // Verify the buffer account was created and initialized - let BaseTaskImpl::Commit(ref ct) = strategy.optimized_tasks[0] else { + let BaseTaskImpl::CommitFinalize(ref ct) = strategy.optimized_tasks[0] + else { panic!("unexpected task type"); }; - let Some(cleanup_task) = CleanupTask::from_commit(ct) else { + let Some(cleanup_task) = CleanupTask::from_commit_finalize(ct) else { panic!("unexpected CommitStage"); }; // Check buffer account exists @@ -251,8 +254,7 @@ async fn test_already_initialized_error_handled() { commit_task.committed_account.account.data.len() - 2, ); commit_task.committed_account.account.data = data.clone(); - commit_task.delivery_details = - CommitDelivery::StateInBuffer { prepared: false }; + commit_task.delivery = CommitDelivery::StateInBuffer { prepared: false }; let mut strategy = TransactionStrategy { optimized_tasks: vec![commit_task.into()], lookup_tables_keys: vec![], @@ -270,10 +272,11 @@ async fn test_already_initialized_error_handled() { assert!(result.is_ok(), "Preparation failed: {:?}", result.err()); // Verify the buffer account was created and initialized - let BaseTaskImpl::Commit(ref ct) = strategy.optimized_tasks[0] else { + let BaseTaskImpl::CommitFinalize(ref ct) = strategy.optimized_tasks[0] + else { panic!("unexpected task type"); }; - let Some(cleanup_task) = CleanupTask::from_commit(ct) else { + let Some(cleanup_task) = CleanupTask::from_commit_finalize(ct) else { panic!("unexpected CommitStage"); }; @@ -351,9 +354,10 @@ async fn test_reprepare_closed_buffer_with_distinct_intent_nonce() { let preparator = fixture.create_delivery_preparator(); let data = generate_random_bytes(112); - let mut commit_task = create_buffer_commit_task(&data); + let mut commit_task = create_buffer_commit_finalize_task(&data); commit_task.reset_commit_id(1); - let Some(preparation_task) = PreparationTask::from_commit(&mut commit_task) + let Some(preparation_task) = + PreparationTask::from_commit_finalize(&mut commit_task) else { panic!("expected preparation stage"); }; @@ -429,10 +433,12 @@ async fn test_reprepare_closed_buffer_with_distinct_intent_nonce() { .expect("second buffer preparation"); let second_cleanup = match &second_strategy.optimized_tasks[0] { - BaseTaskImpl::Commit(task) => match CleanupTask::from_commit(task) { - Some(cleanup) => cleanup.clone(), - _ => panic!("expected cleanup stage"), - }, + BaseTaskImpl::CommitFinalize(task) => { + match CleanupTask::from_commit_finalize(task) { + Some(cleanup) => cleanup.clone(), + _ => panic!("expected cleanup stage"), + } + } _ => panic!("expected commit task"), }; let buffer = fixture @@ -490,9 +496,9 @@ async fn test_prepare_cleanup_and_reprepare_mixed_tasks() { let buf_b_data = generate_random_bytes(64 * 1024 + 3); // Keep these around to modify data later (same commit IDs, different data) - let mut commit_args = create_commit_task(&args_data); - let mut commit_a = create_buffer_commit_task(&buf_a_data); - let mut commit_b = create_buffer_commit_task(&buf_b_data); + let mut commit_args = create_commit_finalize_task(&args_data); + let mut commit_a = create_buffer_commit_finalize_task(&buf_a_data); + let mut commit_b = create_buffer_commit_finalize_task(&buf_b_data); let mut strategy = TransactionStrategy { optimized_tasks: vec![ @@ -521,7 +527,9 @@ async fn test_prepare_cleanup_and_reprepare_mixed_tasks() { .optimized_tasks .iter() .filter_map(|t| match t { - BaseTaskImpl::Commit(ct) => CleanupTask::from_commit(ct), + BaseTaskImpl::CommitFinalize(ct) => { + CleanupTask::from_commit_finalize(ct) + } _ => None, }) .collect(); @@ -588,10 +596,8 @@ async fn test_prepare_cleanup_and_reprepare_mixed_tasks() { } // Rebuild buffer stages with mutated data - commit_a.delivery_details = - CommitDelivery::StateInBuffer { prepared: false }; - commit_b.delivery_details = - CommitDelivery::StateInBuffer { prepared: false }; + commit_a.delivery = CommitDelivery::StateInBuffer { prepared: false }; + commit_b.delivery = CommitDelivery::StateInBuffer { prepared: false }; // --- Step 4: re-prepare with the same logical tasks (same commit IDs, mutated data) --- let mut strategy2 = TransactionStrategy { @@ -622,7 +628,9 @@ async fn test_prepare_cleanup_and_reprepare_mixed_tasks() { .optimized_tasks .iter() .filter_map(|t| match t { - BaseTaskImpl::Commit(ct) => CleanupTask::from_commit(ct), + BaseTaskImpl::CommitFinalize(ct) => { + CleanupTask::from_commit_finalize(ct) + } _ => None, }) .collect(); 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 44f2d4ba4..1f5146582 100644 --- a/test-integration/test-committor-service/tests/test_intent_executor.rs +++ b/test-integration/test-committor-service/tests/test_intent_executor.rs @@ -26,7 +26,7 @@ use magicblock_committor_service::{ }, persist::IntentPersisterImpl, tasks::{ - task_builder::{TaskBuilderError, TaskBuilderImpl, TasksBuilder}, + task_builder::{TaskBuilderError, TaskBuilderImpl}, task_strategist::{ TaskStrategist, TaskStrategistError, TransactionStrategy, }, 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 bb9b445ad..cbfb1e481 100644 --- a/test-integration/test-committor-service/tests/test_transaction_preparator.rs +++ b/test-integration/test-committor-service/tests/test_transaction_preparator.rs @@ -5,9 +5,8 @@ use magicblock_committor_service::{ tasks::{ commit_stage_task::CleanupTask, task_strategist::{TaskStrategist, TransactionStrategy}, - utils::create_commit_task, - BaseActionTask, BaseActionTaskV1, BaseTaskImpl, FinalizeTask, - UndelegateTask, + utils::create_commit_finalize_task, + BaseActionTask, BaseActionTaskV1, BaseTaskImpl, UndelegateTask, }, transaction_preparator::TransactionPreparator, transactions::PreparedMessage, @@ -19,8 +18,8 @@ use solana_sdk::signer::Signer; use solana_sdk_ids::system_program; use crate::common::{ - create_buffer_commit_task, create_committed_account, generate_random_bytes, - TestFixture, + create_buffer_commit_finalize_task, create_committed_account, + generate_random_bytes, TestFixture, }; mod common; @@ -34,13 +33,13 @@ async fn test_prepare_commit_tx_with_single_account() { let account_data = vec![1, 2, 3, 4, 5]; let committed_account = create_committed_account(&account_data); - let tasks: Vec = vec![ - create_commit_task(1, true, committed_account.clone(), None).into(), - FinalizeTask { - delegated_account: committed_account.pubkey, - } - .into(), - ]; + let tasks: Vec = vec![create_commit_finalize_task( + 1, + true, + committed_account.clone(), + None, + ) + .into()]; let mut tx_strategy = TransactionStrategy { optimized_tasks: tasks, lookup_tables_keys: vec![], @@ -72,24 +71,16 @@ async fn test_prepare_commit_tx_with_multiple_accounts() { let account2_data = generate_random_bytes(12); let committed_account2 = create_committed_account(&account2_data); - let mut buffer_commit_task = create_buffer_commit_task(&account2_data); + let mut buffer_commit_task = + create_buffer_commit_finalize_task(&account2_data); buffer_commit_task.committed_account.pubkey = committed_account2.pubkey; // Create test data let tasks: Vec = vec![ // account 1 - create_commit_task(1, true, committed_account1.clone(), None).into(), + create_commit_finalize_task(1, true, committed_account1.clone(), None) + .into(), // account 2 buffer_commit_task.into(), - // finalize account 1 - FinalizeTask { - delegated_account: committed_account1.pubkey, - } - .into(), - // finalize account 2 - FinalizeTask { - delegated_account: committed_account2.pubkey, - } - .into(), ]; let mut tx_strategy = TransactionStrategy { optimized_tasks: tasks, @@ -109,10 +100,11 @@ async fn test_prepare_commit_tx_with_multiple_accounts() { for task in &tx_strategy.optimized_tasks { let commit_task = match task { - BaseTaskImpl::Commit(ct) => ct, + BaseTaskImpl::CommitFinalize(ct) => ct, _ => continue, }; - let Some(cleanup_task) = CleanupTask::from_commit(commit_task) else { + let Some(cleanup_task) = CleanupTask::from_commit_finalize(commit_task) + else { continue; }; let chunks_pda = cleanup_task.chunks_pda(&fixture.authority.pubkey()); @@ -153,16 +145,11 @@ async fn test_prepare_commit_tx_with_base_actions() { }; let mut buffer_commit_task = - create_buffer_commit_task(&committed_account.account.data); + create_buffer_commit_finalize_task(&committed_account.account.data); buffer_commit_task.committed_account.pubkey = committed_account.pubkey; let tasks: Vec = vec![ // commit account buffer_commit_task.into(), - // finalize account - FinalizeTask { - delegated_account: committed_account.pubkey, - } - .into(), // BaseAction BaseActionTask::V1(BaseActionTaskV1 { action: base_action, @@ -190,10 +177,11 @@ async fn test_prepare_commit_tx_with_base_actions() { // Now we verify that buffers were created for task in &tx_strategy.optimized_tasks { let commit_task = match task { - BaseTaskImpl::Commit(ct) => ct, + BaseTaskImpl::CommitFinalize(ct) => ct, _ => continue, }; - let Some(cleanup_task) = CleanupTask::from_commit(commit_task) else { + let Some(cleanup_task) = CleanupTask::from_commit_finalize(commit_task) + else { continue; }; let chunks_pda = cleanup_task.chunks_pda(&fixture.authority.pubkey()); @@ -211,18 +199,13 @@ async fn test_prepare_commit_tx_with_base_actions() { } #[tokio::test] -async fn test_prepare_finalize_tx_with_undelegate_with_atls() { +async fn test_prepare_undelegate_tx_with_alts() { let fixture = TestFixture::new().await; let preparator = fixture.create_transaction_preparator(); // Create test data let committed_account = create_committed_account(&[1, 2, 3]); let tasks: Vec = vec![ - // finalize account - FinalizeTask { - delegated_account: committed_account.pubkey, - } - .into(), // Undelegate UndelegateTask { delegated_account: committed_account.pubkey, diff --git a/test-integration/test-committor-service/tests/utils/transactions.rs b/test-integration/test-committor-service/tests/utils/transactions.rs index f7e50d9f7..e1a777939 100644 --- a/test-integration/test-committor-service/tests/utils/transactions.rs +++ b/test-integration/test-committor-service/tests/utils/transactions.rs @@ -171,30 +171,6 @@ async fn airdrop_and_confirm( ); } -#[allow(dead_code)] -pub async fn tx_logs_contain( - rpc_client: &RpcClient, - signature: &Signature, - needle: &str, -) -> bool { - fetch_tx_logs(rpc_client, signature) - .await - .iter() - .any(|log| { - // Lots of existing tests pass "CommitState" as needle argument to this function, but since now CommitTask - // could invoke CommitState or CommitDiff depending on the size of the account, we also look for "CommitDiff" - // in the logs when needle == CommitState. It's easier to make this little adjustment here than computing - // the decision and passing either CommitState or CommitDiff from the tests themselves. - if needle == "CommitState" { - log.contains(needle) - || log.contains("CommitDiff") - || log.contains("CommitFinalize") - } else { - log.contains(needle) - } - }) -} - /// This needs to be run for each test that required a new counter to be delegated #[allow(dead_code)] pub async fn init_and_delegate_account_on_chain(