From 9f5c613acc079dd338b37c5bcda7721192a76c44 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Mon, 17 Nov 2025 17:16:26 +0400 Subject: [PATCH 01/29] feat: parallelize task cleanup feat: add metrics for task preparation & alt preparation --- .../intent_execution_engine.rs | 10 +-- .../src/intent_executor/mod.rs | 62 +++++++++---------- .../intent_executor/single_stage_executor.rs | 2 +- .../src/intent_executor/two_stage_executor.rs | 2 +- .../src/tasks/args_task.rs | 12 ++++ .../src/tasks/buffer_task.rs | 9 +++ magicblock-committor-service/src/tasks/mod.rs | 3 +- .../delivery_preparator.rs | 41 +++++++++--- magicblock-metrics/src/metrics/mod.rs | 51 +++++++++++++++ 9 files changed, 140 insertions(+), 52 deletions(-) diff --git a/magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs b/magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs index 6fce85908..f1bc8f7fa 100644 --- a/magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs +++ b/magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs @@ -294,7 +294,7 @@ where intent: &ScheduledBaseIntentWrapper, result: &IntentExecutorResult, ) { - const EXECUTION_TIME_THRESHOLD: f64 = 2.0; + const EXECUTION_TIME_THRESHOLD: f64 = 5.0; let intent_execution_secs = execution_time.as_secs_f64(); metrics::observe_committor_intent_execution_time_histogram( @@ -309,12 +309,8 @@ where // Loki alerts if intent_execution_secs >= EXECUTION_TIME_THRESHOLD { info!( - "Intent took too long to execute: {}s. {}", - intent_execution_secs, - result - .as_ref() - .map(|_| "succeeded".to_string()) - .unwrap_or_else(|err| format!("{err:?}")) + "Intent took too long to execute: {}s. {:?}", + intent_execution_secs, result ); } else { trace!("Seconds took to execute intent: {}", intent_execution_secs); diff --git a/magicblock-committor-service/src/intent_executor/mod.rs b/magicblock-committor-service/src/intent_executor/mod.rs index fa85cfee1..464d8cdf6 100644 --- a/magicblock-committor-service/src/intent_executor/mod.rs +++ b/magicblock-committor-service/src/intent_executor/mod.rs @@ -102,7 +102,7 @@ pub struct IntentExecutorImpl { impl IntentExecutorImpl where - T: TransactionPreparator, + T: TransactionPreparator + Clone, F: TaskInfoFetcher, { pub fn new( @@ -253,7 +253,6 @@ where } /// Starting execution from single stage - // TODO(edwin): introduce recursion stop value in case of some bug? pub async fn single_stage_execution_flow( &self, base_intent: ScheduledBaseIntent, @@ -264,21 +263,7 @@ where let res = SingleStageExecutor::new(self) .execute(base_intent, transaction_strategy, &mut junk, persister) .await; - - // Cleanup after intent - // Note: in some cases it maybe critical to execute cleanup synchronously - // Example: if commit nonces were invalid during execution - // next intent could use wrongly initiated buffers by current intent - let cleanup_futs = junk.iter().map(|to_cleanup| { - self.transaction_preparator.cleanup_for_strategy( - &self.authority, - &to_cleanup.optimized_tasks, - &to_cleanup.lookup_tables_keys, - ) - }); - if let Err(err) = try_join_all(cleanup_futs).await { - error!("Failed to cleanup after intent: {}", err); - } + self.spawn_cleanup_task(junk); res } @@ -300,21 +285,7 @@ where persister, ) .await; - - // Cleanup after intent - // Note: in some cases it maybe critical to execute cleanup synchronously - // Example: if commit nonces were invalid during execution - // next intent could use wrongly initiated buffers by current intent - let cleanup_futs = junk.iter().map(|to_cleanup| { - self.transaction_preparator.cleanup_for_strategy( - &self.authority, - &to_cleanup.optimized_tasks, - &to_cleanup.lookup_tables_keys, - ) - }); - if let Err(err) = try_join_all(cleanup_futs).await { - error!("Failed to cleanup after intent: {}", err); - } + self.spawn_cleanup_task(junk); res } @@ -678,6 +649,31 @@ where .await } + /// Cleanup after intent + /// Note: in some cases it maybe critical to execute cleanup synchronously + /// Example: if commit nonces were invalid during execution + /// next intent could use wrongly initiated buffers by current intent + /// We assume that this case is highly unlikely since it would mean: + /// user redelegates amd reaches current commit id faster than we execute transactions below + fn spawn_cleanup_task(&self, junk: Vec) { + let authority = self.authority.insecure_clone(); + let transaction_preparator = self.transaction_preparator.clone(); + let cleanup_fut = async move { + let cleanup_futs = junk.iter().map(|to_cleanup| { + transaction_preparator.cleanup_for_strategy( + &authority, + &to_cleanup.optimized_tasks, + &to_cleanup.lookup_tables_keys, + ) + }); + + if let Err(err) = try_join_all(cleanup_futs).await { + error!("Failed to cleanup after intent: {}", err); + } + }; + tokio::spawn(cleanup_fut); + } + async fn intent_metrics( rpc_client: MagicblockRpcClient, execution_outcome: ExecutionOutput, @@ -741,7 +737,7 @@ where #[async_trait] impl IntentExecutor for IntentExecutorImpl where - T: TransactionPreparator, + T: TransactionPreparator + Clone, C: TaskInfoFetcher, { /// Executes Message on Base layer 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 ad42448c6..4a752edb9 100644 --- a/magicblock-committor-service/src/intent_executor/single_stage_executor.rs +++ b/magicblock-committor-service/src/intent_executor/single_stage_executor.rs @@ -24,7 +24,7 @@ pub struct SingleStageExecutor<'a, T, F> { impl<'a, T, F> SingleStageExecutor<'a, T, F> where - T: TransactionPreparator, + T: TransactionPreparator + Clone, F: TaskInfoFetcher, { pub fn new(executor: &'a IntentExecutorImpl) -> Self { 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 227a67221..64751fef2 100644 --- a/magicblock-committor-service/src/intent_executor/two_stage_executor.rs +++ b/magicblock-committor-service/src/intent_executor/two_stage_executor.rs @@ -23,7 +23,7 @@ pub struct TwoStageExecutor<'a, T, F> { impl<'a, T, F> TwoStageExecutor<'a, T, F> where - T: TransactionPreparator, + T: TransactionPreparator + Clone, F: TaskInfoFetcher, { pub fn new(executor: &'a IntentExecutorImpl) -> Self { diff --git a/magicblock-committor-service/src/tasks/args_task.rs b/magicblock-committor-service/src/tasks/args_task.rs index 301db3b63..fd41a0db4 100644 --- a/magicblock-committor-service/src/tasks/args_task.rs +++ b/magicblock-committor-service/src/tasks/args_task.rs @@ -1,4 +1,5 @@ use dlp::args::{CallHandlerArgs, CommitStateArgs}; +use magicblock_metrics::metrics::LabelValue; use solana_pubkey::Pubkey; use solana_sdk::instruction::{AccountMeta, Instruction}; @@ -165,3 +166,14 @@ impl BaseTask for ArgsTask { commit_task.commit_id = commit_id; } } + +impl LabelValue for ArgsTask { + fn value(&self) -> &str { + match self.task_type { + ArgsTaskType::Commit(_) => "args_commit", + ArgsTaskType::BaseAction(_) => "args_action", + ArgsTaskType::Finalize(_) => "args_finalize", + ArgsTaskType::Undelegate(_) => "args_undelegate", + } + } +} diff --git a/magicblock-committor-service/src/tasks/buffer_task.rs b/magicblock-committor-service/src/tasks/buffer_task.rs index dbb23a9b5..853c0dcd9 100644 --- a/magicblock-committor-service/src/tasks/buffer_task.rs +++ b/magicblock-committor-service/src/tasks/buffer_task.rs @@ -1,5 +1,6 @@ use dlp::args::CommitStateFromBufferArgs; use magicblock_committor_program::Chunks; +use magicblock_metrics::metrics::LabelValue; use solana_pubkey::Pubkey; use solana_sdk::instruction::Instruction; @@ -140,3 +141,11 @@ impl BaseTask for BufferTask { self.preparation_state = Self::preparation_required(&self.task_type) } } + +impl LabelValue for BufferTask { + fn value(&self) -> &str { + match self.task_type { + BufferTaskType::Commit(_) => "buffer_commit", + } + } +} diff --git a/magicblock-committor-service/src/tasks/mod.rs b/magicblock-committor-service/src/tasks/mod.rs index a31e52c4a..3841cab6a 100644 --- a/magicblock-committor-service/src/tasks/mod.rs +++ b/magicblock-committor-service/src/tasks/mod.rs @@ -10,6 +10,7 @@ use magicblock_committor_program::{ }, pdas, ChangesetChunks, Chunks, }; +use magicblock_metrics::metrics::LabelValue; use magicblock_program::magic_scheduled_base_intent::{ BaseAction, CommittedAccount, }; @@ -50,7 +51,7 @@ pub enum TaskStrategy { } /// A trait representing a task that can be executed on Base layer -pub trait BaseTask: Send + Sync + DynClone { +pub trait BaseTask: Send + Sync + DynClone + LabelValue { /// Gets all pubkeys that involved in Task's instruction fn involved_accounts(&self, validator: &Pubkey) -> Vec { self.instruction(validator) diff --git a/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs b/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs index 13001df0e..94da08628 100644 --- a/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs +++ b/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs @@ -6,6 +6,7 @@ use log::{error, info}; use magicblock_committor_program::{ instruction_chunks::chunk_realloc_ixs, Chunks, }; +use magicblock_metrics::metrics; use magicblock_rpc_client::{ utils::{ decide_rpc_error_flow, map_magicblock_client_error, @@ -66,13 +67,30 @@ impl DeliveryPreparator { persister: &Option

, ) -> DeliveryPreparatorResult> { let preparation_futures = - strategy.optimized_tasks.iter_mut().map(|task| { - self.prepare_task_handling_errors(authority, task, persister) + strategy.optimized_tasks.iter_mut().map(|task| async move { + let timer = + metrics::observe_committor_intent_task_preparation_time( + task.as_ref(), + ); + let res = self + .prepare_task_handling_errors(authority, task, persister) + .await; + timer.stop_and_record(); + + res }); let task_preparations = join_all(preparation_futures); - let alts_preparations = - self.prepare_lookup_tables(authority, &strategy.lookup_tables_keys); + let alts_preparations = async { + let timer = + metrics::observe_committor_intent_alt_preparation_time(); + let res = self + .prepare_lookup_tables(authority, &strategy.lookup_tables_keys) + .await; + timer.stop_and_record(); + + res + }; let (res1, res2) = join(task_preparations, alts_preparations).await; res1.into_iter() @@ -215,11 +233,16 @@ impl DeliveryPreparator { }) .collect::>(); - // Initialization & reallocs - for instructions in preparation_instructions { - self.send_ixs_with_retry(&instructions, authority, 5) - .await?; - } + // Initialization + self.send_ixs_with_retry(&preparation_instructions[0], authority, 5) + .await?; + + // Reallocs can be performed in parallel + let preparation_futs = + preparation_instructions.iter().skip(1).map(|instructions| { + self.send_ixs_with_retry(instructions, authority, 5) + }); + try_join_all(preparation_futs).await?; Ok(()) } diff --git a/magicblock-metrics/src/metrics/mod.rs b/magicblock-metrics/src/metrics/mod.rs index 96171bcdd..c1555ca71 100644 --- a/magicblock-metrics/src/metrics/mod.rs +++ b/magicblock-metrics/src/metrics/mod.rs @@ -192,6 +192,10 @@ lazy_static::lazy_static! { // ----------------- // CommittorService // ----------------- + static ref COMMITTOR_INTENTS_COUNT: IntCounter = IntCounter::new( + "committor_intents_count", "Total number of scheduled" + ).unwrap(); + static ref COMMITTOR_INTENTS_BACKLOG_COUNT: IntGauge = IntGauge::new( "committor_intent_backlog_count", "Number of intents in backlog", ).unwrap(); @@ -219,6 +223,28 @@ lazy_static::lazy_static! { static ref COMMITTOR_INTENT_CU_USAGE: IntGauge = IntGauge::new( "committor_intent_cu_usage", "Compute units used for Intent" ).unwrap(); + + + static ref COMMITTOR_INTENT_TASK_PREPARATION_TIME: HistogramVec = HistogramVec::new( + HistogramOpts::new( + "committor_intent_task_preparation_time", + "Committor 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", + "Committor in seconds spent on ALTs preparation" + ) + .buckets( + vec![1.0, 3.0, 5.0, 10.0, 15.0, 17.0, 20.0] + ), + ).unwrap(); } pub(crate) fn register() { @@ -252,11 +278,14 @@ pub(crate) fn register() { register!(MONITORED_ACCOUNTS_GAUGE); register!(SUBSCRIPTIONS_COUNT_GAUGE); register!(EVICTED_ACCOUNTS_COUNT); + register!(COMMITTOR_INTENTS_COUNT); register!(COMMITTOR_INTENTS_BACKLOG_COUNT); register!(COMMITTOR_FAILED_INTENTS_COUNT); register!(COMMITTOR_EXECUTORS_BUSY_COUNT); register!(COMMITTOR_INTENT_EXECUTION_TIME_HISTOGRAM); register!(COMMITTOR_INTENT_CU_USAGE); + register!(COMMITTOR_INTENT_TASK_PREPARATION_TIME); + register!(COMMITTOR_INTENT_ALT_PREPARATION_TIME); register!(ENSURE_ACCOUNTS_TIME); register!(RPC_REQUEST_HANDLING_TIME); register!(TRANSACTION_PROCESSING_TIME); @@ -363,6 +392,14 @@ pub fn inc_evicted_accounts_count() { EVICTED_ACCOUNTS_COUNT.inc(); } +pub fn inc_committor_intents_count() { + COMMITTOR_INTENTS_COUNT.inc() +} + +pub fn inc_committor_intents_count_by(by: u64) { + COMMITTOR_INTENTS_COUNT.inc_by(by) +} + pub fn set_committor_intents_backlog_count(value: i64) { COMMITTOR_INTENTS_BACKLOG_COUNT.set(value) } @@ -393,3 +430,17 @@ pub fn observe_committor_intent_execution_time_histogram( pub fn set_commmittor_intent_cu_usage(value: i64) { COMMITTOR_INTENT_CU_USAGE.set(value) } + +pub fn observe_committor_intent_task_preparation_time< + L: LabelValue + ?Sized, +>( + task_type: &L, +) -> HistogramTimer { + COMMITTOR_INTENT_TASK_PREPARATION_TIME + .with_label_values(&[task_type.value()]) + .start_timer() +} + +pub fn observe_committor_intent_alt_preparation_time() -> HistogramTimer { + COMMITTOR_INTENT_ALT_PREPARATION_TIME.start_timer() +} From 20194914afcd3003d690eff1627f8fcb8deec1a4 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Tue, 18 Nov 2025 10:46:42 +0400 Subject: [PATCH 02/29] fix: comments for metrics --- magicblock-metrics/src/metrics/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/magicblock-metrics/src/metrics/mod.rs b/magicblock-metrics/src/metrics/mod.rs index c1555ca71..6dcddcefc 100644 --- a/magicblock-metrics/src/metrics/mod.rs +++ b/magicblock-metrics/src/metrics/mod.rs @@ -193,7 +193,7 @@ lazy_static::lazy_static! { // CommittorService // ----------------- static ref COMMITTOR_INTENTS_COUNT: IntCounter = IntCounter::new( - "committor_intents_count", "Total number of scheduled" + "committor_intents_count", "Total number of scheduled committor intents" ).unwrap(); static ref COMMITTOR_INTENTS_BACKLOG_COUNT: IntGauge = IntGauge::new( @@ -228,7 +228,7 @@ lazy_static::lazy_static! { static ref COMMITTOR_INTENT_TASK_PREPARATION_TIME: HistogramVec = HistogramVec::new( HistogramOpts::new( "committor_intent_task_preparation_time", - "Committor in seconds spent on task preparation" + "Time in seconds spent on task preparation" ) .buckets( vec![0.1, 1.0, 2.0, 3.0, 5.0] @@ -239,7 +239,7 @@ lazy_static::lazy_static! { static ref COMMITTOR_INTENT_ALT_PREPARATION_TIME: Histogram = Histogram::with_opts( HistogramOpts::new( "committor_intent_alt_preparation_time", - "Committor in seconds spent on ALTs preparation" + "Time in seconds spent on ALTs preparation" ) .buckets( vec![1.0, 3.0, 5.0, 10.0, 15.0, 17.0, 20.0] From 02029893f36b4db079ef728c494b7fb0c919315f Mon Sep 17 00:00:00 2001 From: taco-paco Date: Tue, 18 Nov 2025 11:49:11 +0400 Subject: [PATCH 03/29] feat: add undelegation patching --- .../src/intent_executor/error.rs | 66 ++++++++++++------- .../src/intent_executor/mod.rs | 32 ++++++++- .../intent_executor/single_stage_executor.rs | 7 ++ .../src/intent_executor/two_stage_executor.rs | 15 ++++- 4 files changed, 95 insertions(+), 25 deletions(-) diff --git a/magicblock-committor-service/src/intent_executor/error.rs b/magicblock-committor-service/src/intent_executor/error.rs index 62199ac56..ea306bf87 100644 --- a/magicblock-committor-service/src/intent_executor/error.rs +++ b/magicblock-committor-service/src/intent_executor/error.rs @@ -40,6 +40,8 @@ pub enum IntentExecutorError { EmptyIntentError, #[error("User supplied actions are ill-formed: {0}. {:?}", .1)] ActionsError(#[source] TransactionError, Option), + #[error("Invalid undelegation: {0}. {:?}", .1)] + UndelegationError(#[source] TransactionError, Option), #[error("Accounts committed with an invalid Commit id: {0}. {:?}", .1)] CommitIDError(#[source] TransactionError, Option), #[error("Max instruction trace length exceeded: {0}. {:?}", .1)] @@ -94,6 +96,10 @@ impl IntentExecutorError { err, signature, ) => IntentExecutorError::CommitIDError(err, signature), + TransactionStrategyExecutionError::UndelegationError( + err, + signature, + ) => IntentExecutorError::UndelegationError(err, signature), TransactionStrategyExecutionError::InternalError(err) => { converter(err) } @@ -117,6 +123,8 @@ impl metrics::LabelValue for IntentExecutorError { pub enum TransactionStrategyExecutionError { #[error("User supplied actions are ill-formed: {0}. {:?}", .1)] ActionsError(#[source] TransactionError, Option), + #[error("Invalid undelegation: {0}. {:?}", .1)] + UndelegationError(#[source] TransactionError, Option), #[error("Accounts committed with an invalid Commit id: {0}. {:?}", .1)] CommitIDError(#[source] TransactionError, Option), #[error("Max instruction trace length exceeded: {0}. {:?}", .1)] @@ -146,14 +154,6 @@ impl TransactionStrategyExecutionError { dlp::error::DlpError::NonceOutOfOrder as u32; match err { - // Filter CommitIdError by custom error code - transaction_err @ TransactionError::InstructionError( - _, - InstructionError::Custom(NONCE_OUT_OF_ORDER), - ) => Ok(TransactionStrategyExecutionError::CommitIDError( - transaction_err, - signature, - )), // Some tx may use too much CPIs and we can handle it in certain cases transaction_err @ TransactionError::InstructionError( _, @@ -162,25 +162,45 @@ impl TransactionStrategyExecutionError { transaction_err, signature, )), - // Filter ActionError, we can attempt recovery by stripping away actions - transaction_err @ TransactionError::InstructionError(index, _) => { + // Filter CommitIdError by custom error code + TransactionError::InstructionError(index, instruction_err) => { + let tx_err_helper = |instruction_err| -> TransactionError { + TransactionError::InstructionError(index, instruction_err) + }; let Some(action_index) = index.checked_sub(OFFSET) else { - return Err(transaction_err); + return Err(tx_err_helper(instruction_err)); + }; + + let Some(task_type) = tasks + .get(action_index as usize) + .map(|task| task.task_type()) + else { + return Err(tx_err_helper(instruction_err)); }; - // If index corresponds to an Action -> ActionsError; otherwise -> InternalError. - if matches!( - tasks - .get(action_index as usize) - .map(|task| task.task_type()), - Some(TaskType::Action) - ) { - Ok(TransactionStrategyExecutionError::ActionsError( - transaction_err, + match (task_type, instruction_err) { + ( + TaskType::Commit, + InstructionError::Custom(NONCE_OUT_OF_ORDER), + ) => Ok(TransactionStrategyExecutionError::CommitIDError( + tx_err_helper(InstructionError::Custom( + NONCE_OUT_OF_ORDER, + )), signature, - )) - } else { - Err(transaction_err) + )), + (TaskType::Action, instruction_err) => { + Ok(TransactionStrategyExecutionError::ActionsError( + tx_err_helper(instruction_err), + signature, + )) + } + (TaskType::Undelegate, instruction_err) => Ok( + TransactionStrategyExecutionError::UndelegationError( + tx_err_helper(instruction_err), + signature, + ), + ), + (_, instruction_err) => Err(tx_err_helper(instruction_err)), } } // This means transaction failed to other reasons that we don't handle - propagate diff --git a/magicblock-committor-service/src/intent_executor/mod.rs b/magicblock-committor-service/src/intent_executor/mod.rs index fa85cfee1..7a5c0cf68 100644 --- a/magicblock-committor-service/src/intent_executor/mod.rs +++ b/magicblock-committor-service/src/intent_executor/mod.rs @@ -445,6 +445,35 @@ where (commit_strategy, finalize_strategy, to_cleanup) } + /// Handles actions error, stripping away actions + /// Returns [`TransactionStrategy`] to be cleaned up + fn handle_undelegation_error( + &self, + strategy: &mut TransactionStrategy, + ) -> TransactionStrategy { + let position = strategy + .optimized_tasks + .iter() + .position(|el| el.task_type() == TaskType::Undelegate); + + if let Some(position) = position { + // Remove everything after undelegation including post undelegation actions + let removed_task = + strategy.optimized_tasks.drain(position..).collect(); + let old_alts = + strategy.dummy_revaluate_alts(&self.authority.pubkey()); + TransactionStrategy { + optimized_tasks: removed_task, + lookup_tables_keys: old_alts, + } + } else { + TransactionStrategy { + optimized_tasks: vec![], + lookup_tables_keys: vec![], + } + } + } + /// Shared helper for sending transactions async fn send_prepared_message( &self, @@ -520,7 +549,8 @@ where } Err(IntentExecutorError::CommitIDError(_, _)) | Err(IntentExecutorError::ActionsError(_, _)) - | Err(IntentExecutorError::CpiLimitError(_, _)) => None, + | Err(IntentExecutorError::CpiLimitError(_, _)) + | Err(IntentExecutorError::UndelegationError(_, _)) => None, Err(IntentExecutorError::EmptyIntentError) | Err(IntentExecutorError::FailedToFitError) | Err(IntentExecutorError::TaskBuilderError(_)) 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 ad42448c6..991051ae9 100644 --- a/magicblock-committor-service/src/intent_executor/single_stage_executor.rs +++ b/magicblock-committor-service/src/intent_executor/single_stage_executor.rs @@ -175,6 +175,13 @@ where .await?; Ok(ControlFlow::Continue(to_cleanup)) } + 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 + let to_cleanup = + self.handle_undelegation_error(transaction_strategy); + Ok(ControlFlow::Continue(to_cleanup)) + } TransactionStrategyExecutionError::CpiLimitError(_, _) => { // Can't be handled in scope of single stage execution // We signal flow break 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 227a67221..25f6626aa 100644 --- a/magicblock-committor-service/src/intent_executor/two_stage_executor.rs +++ b/magicblock-committor-service/src/intent_executor/two_stage_executor.rs @@ -174,7 +174,13 @@ where TransactionStrategyExecutionError::ActionsError(_, _) => { // Unexpected in Two Stage commit // That would mean that Two Stage executes Standalone commit - error!("Unexpected error in Two stage commit flow: {}", err); + error!("Unexpected error in two stage commit flow: {}", err); + Ok(ControlFlow::Break(())) + } + TransactionStrategyExecutionError::UndelegationError(_, _) => { + // Unexpected in Two Stage commit + // That would mean that Two Stage executes undelegation in commit phase + error!("Unexpected error in two stage commit flow: {}", err); Ok(ControlFlow::Break(())) } TransactionStrategyExecutionError::CpiLimitError(_, _) => { @@ -213,6 +219,13 @@ where let to_cleanup = self.handle_actions_error(finalize_strategy); Ok(ControlFlow::Continue(to_cleanup)) } + 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 + let to_cleanup = + self.handle_undelegation_error(finalize_strategy); + Ok(ControlFlow::Continue(to_cleanup)) + } TransactionStrategyExecutionError::CpiLimitError(_, _) => { // Can't be handled warn!("Finalization tasks exceeded CpiLimitError: {}", err); From 1a094ecd827b3544f9dc8c62a4bde968b1b57d28 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Tue, 18 Nov 2025 13:13:50 +0400 Subject: [PATCH 04/29] feat: add tests --- .../src/intent_executor/mod.rs | 2 + .../programs/flexi-counter/src/processor.rs | 11 +- .../programs/flexi-counter/src/state.rs | 2 + .../tests/test_intent_executor.rs | 117 ++++++++++++++++-- .../tests/test_ix_commit_local.rs | 12 +- .../tests/utils/instructions.rs | 4 +- .../tests/utils/transactions.rs | 3 +- 7 files changed, 133 insertions(+), 18 deletions(-) diff --git a/magicblock-committor-service/src/intent_executor/mod.rs b/magicblock-committor-service/src/intent_executor/mod.rs index 7a5c0cf68..6c61cd8cc 100644 --- a/magicblock-committor-service/src/intent_executor/mod.rs +++ b/magicblock-committor-service/src/intent_executor/mod.rs @@ -788,6 +788,8 @@ where let result = self.execute_inner(base_intent, &persister).await; if let Some(pubkeys) = pubkeys { // Reset TaskInfoFetcher, as cache could become invalid + // NOTE: if undelegation was removed - we still reset + // We assume its safe since all consecutive commits will fail if result.is_err() || is_undelegate { self.task_info_fetcher.reset(ResetType::Specific(&pubkeys)); } diff --git a/test-integration/programs/flexi-counter/src/processor.rs b/test-integration/programs/flexi-counter/src/processor.rs index a81021d9a..eab3b8f70 100644 --- a/test-integration/programs/flexi-counter/src/processor.rs +++ b/test-integration/programs/flexi-counter/src/processor.rs @@ -37,7 +37,7 @@ use crate::{ }, schedule_intent::process_create_intent, }, - state::FlexiCounter, + state::{FlexiCounter, FAIL_UNDELEGATION_CODE, FAIL_UNDELEGATION_LABEL}, utils::assert_keys_equal, }; @@ -387,6 +387,15 @@ fn process_undelegate_request( ProgramError::InvalidArgument })?; + let counter = { + let data = delegated_account.data.borrow(); + FlexiCounter::deserialize(&mut data.as_ref())? + }; + + if counter.label == FAIL_UNDELEGATION_LABEL { + return Err(ProgramError::Custom(FAIL_UNDELEGATION_CODE)); + } + undelegate_account( delegated_account, &crate::id(), diff --git a/test-integration/programs/flexi-counter/src/state.rs b/test-integration/programs/flexi-counter/src/state.rs index 02b286a31..2eb9a5255 100644 --- a/test-integration/programs/flexi-counter/src/state.rs +++ b/test-integration/programs/flexi-counter/src/state.rs @@ -9,6 +9,8 @@ pub struct FlexiCounter { } const FLEXI_COUNTER_SEED: &[u8] = b"flexi_counter"; +pub const FAIL_UNDELEGATION_LABEL: &str = "undelegate_fail"; +pub const FAIL_UNDELEGATION_CODE: u32 = 122; impl FlexiCounter { pub fn new(label: String) -> Self { 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 8e904e902..5d4991168 100644 --- a/test-integration/test-committor-service/tests/test_intent_executor.rs +++ b/test-integration/test-committor-service/tests/test_intent_executor.rs @@ -35,7 +35,8 @@ use magicblock_program::{ }; use magicblock_table_mania::TableMania; use program_flexi_counter::{ - instruction::FlexiCounterInstruction, state::FlexiCounter, + instruction::FlexiCounterInstruction, + state::{FlexiCounter, FAIL_UNDELEGATION_LABEL}, }; use solana_account::Account; use solana_pubkey::Pubkey; @@ -117,7 +118,7 @@ async fn test_commit_id_error_parsing() { task_info_fetcher, pre_test_tablemania_state: _, } = TestEnv::setup().await; - let (counter_auth, account) = setup_counter(COUNTER_SIZE).await; + let (counter_auth, account) = setup_counter(COUNTER_SIZE, None).await; let intent = create_intent( vec![CommittedAccount { pubkey: FlexiCounter::pda(&counter_auth.pubkey()).0, @@ -157,6 +158,55 @@ async fn test_commit_id_error_parsing() { assert!(err.to_string().contains(EXPECTED_ERR_MSG)); } +#[tokio::test] +async fn test_undelegation_error_parsing() { + const COUNTER_SIZE: u64 = 70; + const EXPECTED_ERR_MSG: &str = "Invalid undelegation: Error processing Instruction 4: Failed to serialize or deserialize account data"; + + let TestEnv { + fixture, + intent_executor, + task_info_fetcher, + pre_test_tablemania_state: _, + } = TestEnv::setup().await; + + // Create counter that will force undelegation to fail + let (counter_auth, account) = + setup_counter(COUNTER_SIZE, Some(FAIL_UNDELEGATION_LABEL.to_string())) + .await; + let intent = create_intent( + vec![CommittedAccount { + pubkey: FlexiCounter::pda(&counter_auth.pubkey()).0, + account, + }], + true, + ); + + let mut transaction_strategy = single_flow_transaction_strategy( + &fixture.authority.pubkey(), + &task_info_fetcher, + &intent, + ) + .await; + let execution_result = intent_executor + .prepare_and_execute_strategy( + &mut transaction_strategy, + &None::, + ) + .await; + assert!(execution_result.is_ok(), "Preparation is expected to pass!"); + + // Verify that we got CommitIdError + let execution_result = execution_result.unwrap(); + assert!(execution_result.is_err()); + let err = execution_result.unwrap_err(); + assert!(matches!( + err, + TransactionStrategyExecutionError::UndelegationError(_, _) + )); + assert!(err.to_string().contains(EXPECTED_ERR_MSG)); +} + #[tokio::test] async fn test_action_error_parsing() { const COUNTER_SIZE: u64 = 70; @@ -169,7 +219,7 @@ async fn test_action_error_parsing() { pre_test_tablemania_state: _, } = TestEnv::setup().await; - let (counter_auth, account) = setup_counter(COUNTER_SIZE).await; + let (counter_auth, account) = setup_counter(COUNTER_SIZE, None).await; setup_payer_with_keypair(&counter_auth, fixture.rpc_client.get_inner()) .await; @@ -230,7 +280,7 @@ async fn test_cpi_limits_error_parsing() { } = TestEnv::setup().await; let counters = (0..COUNTER_NUM).map(|_| async { - let (counter_auth, account) = setup_counter(COUNTER_SIZE).await; + let (counter_auth, account) = setup_counter(COUNTER_SIZE, None).await; setup_payer_with_keypair(&counter_auth, fixture.rpc_client.get_inner()) .await; @@ -287,7 +337,8 @@ async fn test_commit_id_error_recovery() { let counter_auth = Keypair::new(); let (pubkey, mut account) = - init_and_delegate_account_on_chain(&counter_auth, COUNTER_SIZE).await; + init_and_delegate_account_on_chain(&counter_auth, COUNTER_SIZE, None) + .await; account.owner = program_flexi_counter::id(); let committed_account = CommittedAccount { pubkey, account }; @@ -326,6 +377,46 @@ async fn test_commit_id_error_recovery() { .await; } +#[tokio::test] +async fn test_undelegation_error_recovery() { + const COUNTER_SIZE: u64 = 70; + + let TestEnv { + fixture, + intent_executor, + task_info_fetcher: _, + pre_test_tablemania_state, + } = TestEnv::setup().await; + + let counter_auth = Keypair::new(); + let (pubkey, mut account) = init_and_delegate_account_on_chain( + &counter_auth, + COUNTER_SIZE, + Some(FAIL_UNDELEGATION_LABEL.to_string()), + ) + .await; + + account.owner = program_flexi_counter::id(); + let committed_account = CommittedAccount { pubkey, account }; + let intent = create_intent(vec![committed_account.clone()], true); + + // Execute intent + let res = intent_executor + .execute(intent, None::) + .await; + assert!(res.is_ok()); + assert!(matches!(res.unwrap(), ExecutionOutput::SingleStage(_))); + + verify( + &fixture.table_mania, + fixture.rpc_client.get_inner(), + &HashMap::new(), + &pre_test_tablemania_state, + &[committed_account], + ) + .await; +} + #[tokio::test] async fn test_action_error_recovery() { const COUNTER_SIZE: u64 = 100; @@ -339,7 +430,7 @@ async fn test_action_error_recovery() { let payer = setup_payer(fixture.rpc_client.get_inner()).await; let (counter_pubkey, mut account) = - init_and_delegate_account_on_chain(&payer, COUNTER_SIZE).await; + init_and_delegate_account_on_chain(&payer, COUNTER_SIZE, None).await; account.owner = program_flexi_counter::id(); let committed_account = CommittedAccount { @@ -389,7 +480,7 @@ async fn test_commit_id_and_action_errors_recovery() { let payer = setup_payer(fixture.rpc_client.get_inner()).await; let (counter_pubkey, mut account) = - init_and_delegate_account_on_chain(&payer, COUNTER_SIZE).await; + init_and_delegate_account_on_chain(&payer, COUNTER_SIZE, None).await; account.owner = program_flexi_counter::id(); let committed_account = CommittedAccount { @@ -446,7 +537,7 @@ async fn test_cpi_limits_error_recovery() { } = TestEnv::setup().await; let counters = (0..COUNTER_NUM).map(|_| async { - let (counter_auth, account) = setup_counter(COUNTER_SIZE).await; + let (counter_auth, account) = setup_counter(COUNTER_SIZE, None).await; setup_payer_with_keypair(&counter_auth, fixture.rpc_client.get_inner()) .await; @@ -533,7 +624,7 @@ async fn test_commit_id_actions_cpi_limit_errors_recovery() { // Prepare multiple counters; each needs an escrow (payer) to be able to execute base actions. // We also craft unique on-chain data so we can verify post-commit state exactly. let counters = (0..COUNTER_NUM).map(|_| async { - let (counter_auth, account) = setup_counter(COUNTER_SIZE).await; + let (counter_auth, account) = setup_counter(COUNTER_SIZE, None).await; setup_payer_with_keypair(&counter_auth, fixture.rpc_client.get_inner()) .await; (counter_auth, account) @@ -728,10 +819,14 @@ async fn setup_payer_with_keypair( ); } -async fn setup_counter(counter_bytes: u64) -> (Keypair, Account) { +async fn setup_counter( + counter_bytes: u64, + label: Option, +) -> (Keypair, Account) { let counter_auth = Keypair::new(); let (_, mut account) = - init_and_delegate_account_on_chain(&counter_auth, counter_bytes).await; + init_and_delegate_account_on_chain(&counter_auth, counter_bytes, label) + .await; account.owner = program_flexi_counter::id(); (counter_auth, account) 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 4281d3149..8e84567bc 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 @@ -112,7 +112,8 @@ async fn commit_single_account( let counter_auth = Keypair::new(); let (pubkey, mut account) = - init_and_delegate_account_on_chain(&counter_auth, bytes as u64).await; + init_and_delegate_account_on_chain(&counter_auth, bytes as u64, None) + .await; account.owner = program_flexi_counter::id(); account.data = vec![101_u8; bytes]; @@ -393,9 +394,12 @@ async fn create_bundles( let bytes = *bytes; join_set.spawn(async move { let counter_auth = Keypair::new(); - let (pda, mut pda_acc) = - init_and_delegate_account_on_chain(&counter_auth, bytes as u64) - .await; + let (pda, mut pda_acc) = init_and_delegate_account_on_chain( + &counter_auth, + bytes as u64, + None, + ) + .await; pda_acc.owner = program_flexi_counter::id(); pda_acc.data = vec![0u8; bytes]; diff --git a/test-integration/test-committor-service/tests/utils/instructions.rs b/test-integration/test-committor-service/tests/utils/instructions.rs index 4e90c0b7d..9b7e3ffbd 100644 --- a/test-integration/test-committor-service/tests/utils/instructions.rs +++ b/test-integration/test-committor-service/tests/utils/instructions.rs @@ -20,12 +20,14 @@ pub struct InitAccountAndDelegateIxs { pub fn init_account_and_delegate_ixs( payer: Pubkey, bytes: u64, + label: Option, ) -> InitAccountAndDelegateIxs { const MAX_ALLOC: u64 = magicblock_committor_program::consts::MAX_ACCOUNT_ALLOC_PER_INSTRUCTION_SIZE as u64; use program_flexi_counter::{instruction::*, state::*}; - let init_counter_ix = create_init_ix(payer, "COUNTER".to_string()); + let init_counter_ix = + create_init_ix(payer, label.unwrap_or("COUNTER".to_string())); let rent_exempt = Rent::default().minimum_balance(bytes as usize); let num_reallocs = bytes.div_ceil(MAX_ALLOC); diff --git a/test-integration/test-committor-service/tests/utils/transactions.rs b/test-integration/test-committor-service/tests/utils/transactions.rs index 82440d93c..851be7e39 100644 --- a/test-integration/test-committor-service/tests/utils/transactions.rs +++ b/test-integration/test-committor-service/tests/utils/transactions.rs @@ -127,6 +127,7 @@ pub async fn tx_logs_contain( pub async fn init_and_delegate_account_on_chain( counter_auth: &Keypair, bytes: u64, + label: Option, ) -> (Pubkey, Account) { let rpc_client = RpcClient::new("http://localhost:7799".to_string()); @@ -142,7 +143,7 @@ pub async fn init_and_delegate_account_on_chain( delegate: delegate_ix, pda, rent_excempt, - } = init_account_and_delegate_ixs(counter_auth.pubkey(), bytes); + } = init_account_and_delegate_ixs(counter_auth.pubkey(), bytes, label); let latest_block_hash = rpc_client.get_latest_blockhash().await.unwrap(); // 1. Init account From 541c1a544e10eb1d5fcda4db3a1341d7d0a5d457 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Tue, 18 Nov 2025 13:21:26 +0400 Subject: [PATCH 05/29] fix: some comments --- magicblock-committor-service/src/intent_executor/error.rs | 2 +- .../test-committor-service/tests/test_intent_executor.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/magicblock-committor-service/src/intent_executor/error.rs b/magicblock-committor-service/src/intent_executor/error.rs index ea306bf87..e17eda365 100644 --- a/magicblock-committor-service/src/intent_executor/error.rs +++ b/magicblock-committor-service/src/intent_executor/error.rs @@ -162,7 +162,7 @@ impl TransactionStrategyExecutionError { transaction_err, signature, )), - // Filter CommitIdError by custom error code + // Map per-task InstructionError into CommitID / Actions / Undelegation errors when possible TransactionError::InstructionError(index, instruction_err) => { let tx_err_helper = |instruction_err| -> TransactionError { TransactionError::InstructionError(index, instruction_err) 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 5d4991168..d9c2f4efe 100644 --- a/test-integration/test-committor-service/tests/test_intent_executor.rs +++ b/test-integration/test-committor-service/tests/test_intent_executor.rs @@ -196,7 +196,7 @@ async fn test_undelegation_error_parsing() { .await; assert!(execution_result.is_ok(), "Preparation is expected to pass!"); - // Verify that we got CommitIdError + // Verify that we got UndelegationError let execution_result = execution_result.unwrap(); assert!(execution_result.is_err()); let err = execution_result.unwrap_err(); From 2684f8da956d12d0e1adec6ed8bb26169d0277e9 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Tue, 18 Nov 2025 13:33:08 +0400 Subject: [PATCH 06/29] fix: wait a little bit for async cleanup --- .../test-committor-service/tests/test_intent_executor.rs | 9 +++++++++ 1 file changed, 9 insertions(+) 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 d9c2f4efe..c58ebfabf 100644 --- a/test-integration/test-committor-service/tests/test_intent_executor.rs +++ b/test-integration/test-committor-service/tests/test_intent_executor.rs @@ -367,6 +367,8 @@ async fn test_commit_id_error_recovery() { ) }) .collect(); + + tokio::time::sleep(Duration::from_secs(1)).await; verify( &fixture.table_mania, fixture.rpc_client.get_inner(), @@ -407,6 +409,7 @@ async fn test_undelegation_error_recovery() { assert!(res.is_ok()); assert!(matches!(res.unwrap(), ExecutionOutput::SingleStage(_))); + tokio::time::sleep(Duration::from_secs(1)).await; verify( &fixture.table_mania, fixture.rpc_client.get_inner(), @@ -460,6 +463,8 @@ async fn test_action_error_recovery() { &[committed_account], ) .await; + + tokio::time::sleep(Duration::from_secs(1)).await; verify_table_mania_released( &fixture.table_mania, &pre_test_tablemania_state, @@ -517,6 +522,8 @@ async fn test_commit_id_and_action_errors_recovery() { &[committed_account], ) .await; + + tokio::time::sleep(Duration::from_secs(1)).await; verify_table_mania_released( &fixture.table_mania, &pre_test_tablemania_state, @@ -589,6 +596,7 @@ async fn test_cpi_limits_error_recovery() { } )); + tokio::time::sleep(Duration::from_secs(1)).await; let commit_ids_by_pk: HashMap<_, _> = committed_accounts .iter() .map(|el| { @@ -701,6 +709,7 @@ async fn test_commit_id_actions_cpi_limit_errors_recovery() { } )); + tokio::time::sleep(Duration::from_secs(1)).await; let commit_ids_by_pk: HashMap<_, _> = committed_accounts .iter() .map(|el| { From 254652a2a4c2b5724c39f42d21e5bdd87418812e Mon Sep 17 00:00:00 2001 From: taco-paco Date: Tue, 18 Nov 2025 13:48:20 +0400 Subject: [PATCH 07/29] fix: wait a little bit for async cleanup # Conflicts: # test-integration/test-committor-service/tests/test_intent_executor.rs --- .../test-committor-service/tests/test_intent_executor.rs | 8 ++++++++ 1 file changed, 8 insertions(+) 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 8e904e902..cd301c764 100644 --- a/test-integration/test-committor-service/tests/test_intent_executor.rs +++ b/test-integration/test-committor-service/tests/test_intent_executor.rs @@ -316,6 +316,8 @@ async fn test_commit_id_error_recovery() { ) }) .collect(); + + tokio::time::sleep(Duration::from_secs(1)).await; verify( &fixture.table_mania, fixture.rpc_client.get_inner(), @@ -369,6 +371,8 @@ async fn test_action_error_recovery() { &[committed_account], ) .await; + + tokio::time::sleep(Duration::from_secs(1)).await; verify_table_mania_released( &fixture.table_mania, &pre_test_tablemania_state, @@ -426,6 +430,8 @@ async fn test_commit_id_and_action_errors_recovery() { &[committed_account], ) .await; + + tokio::time::sleep(Duration::from_secs(1)).await; verify_table_mania_released( &fixture.table_mania, &pre_test_tablemania_state, @@ -498,6 +504,7 @@ async fn test_cpi_limits_error_recovery() { } )); + tokio::time::sleep(Duration::from_secs(1)).await; let commit_ids_by_pk: HashMap<_, _> = committed_accounts .iter() .map(|el| { @@ -610,6 +617,7 @@ async fn test_commit_id_actions_cpi_limit_errors_recovery() { } )); + tokio::time::sleep(Duration::from_secs(1)).await; let commit_ids_by_pk: HashMap<_, _> = committed_accounts .iter() .map(|el| { From 470fceafa1f054b5c85ef15bafebec34277c5be2 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Thu, 20 Nov 2025 10:58:16 +0400 Subject: [PATCH 08/29] refactor: Executors store reports and junk --- .../intent_execution_engine.rs | 2 +- .../src/intent_executor/mod.rs | 93 +++++++++--- .../intent_executor/single_stage_executor.rs | 133 ++++++++---------- .../src/intent_executor/two_stage_executor.rs | 99 ++++++++----- 4 files changed, 193 insertions(+), 134 deletions(-) diff --git a/magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs b/magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs index f1bc8f7fa..212111cd0 100644 --- a/magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs +++ b/magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs @@ -238,7 +238,7 @@ where /// Wrapper on [`IntentExecutor`] that handles its results and drops execution permit async fn execute( - executor: E, + mut executor: E, persister: Option

, intent: ScheduledBaseIntentWrapper, inner_scheduler: Arc>, diff --git a/magicblock-committor-service/src/intent_executor/mod.rs b/magicblock-committor-service/src/intent_executor/mod.rs index 1d3cc95d6..695ce48fa 100644 --- a/magicblock-committor-service/src/intent_executor/mod.rs +++ b/magicblock-committor-service/src/intent_executor/mod.rs @@ -87,7 +87,7 @@ pub trait IntentExecutor: Send + Sync + 'static { /// Executes Message on Base layer /// Returns `ExecutionOutput` or an `Error` async fn execute( - &self, + &mut self, base_intent: ScheduledBaseIntent, persister: Option

, ) -> IntentExecutorResult; @@ -98,6 +98,11 @@ pub struct IntentExecutorImpl { rpc_client: MagicblockRpcClient, transaction_preparator: T, task_info_fetcher: Arc, + + /// Junk that needs to be cleaned up + pub junk: Vec, + /// Errors we patched trying to recover intent + pub patched_errors: Vec, } impl IntentExecutorImpl @@ -116,6 +121,8 @@ where rpc_client, transaction_preparator, task_info_fetcher, + junk: vec![], + patched_errors: vec![], } } @@ -154,7 +161,7 @@ where } async fn execute_inner( - &self, + &mut self, base_intent: ScheduledBaseIntent, persister: &Option

, ) -> IntentExecutorResult { @@ -254,38 +261,84 @@ where /// Starting execution from single stage pub async fn single_stage_execution_flow( - &self, + &mut self, base_intent: ScheduledBaseIntent, transaction_strategy: TransactionStrategy, persister: &Option

, ) -> IntentExecutorResult { - let mut junk = Vec::new(); - let res = SingleStageExecutor::new(self) - .execute(base_intent, transaction_strategy, &mut junk, persister) + let mut single_stage_executor = + SingleStageExecutor::new(self, transaction_strategy); + + let committed_pubkeys = base_intent.get_committed_pubkeys(); + let res = single_stage_executor + .execute( + committed_pubkeys.as_ref().map(|el| el.as_slice()), + persister, + ) .await; - self.spawn_cleanup_task(junk); - res + // After execution is complete record junk & patched errors + self.junk.append(&mut single_stage_executor.junk); + self.patched_errors + .append(&mut single_stage_executor.patched_errors); + + let execution_err = match res { + Ok(value) => return Ok(value), + Err(err) => err, + }; + + if !(execution_err.is_cpi_limit_error() && committed_pubkeys.is_some()) + { + // Can't recover this - return Err + return Err(execution_err); + } + + // With actions, we can't predict num of CPIs + // If we get here we will try to switch from Single stage to Two Stage commit + // Note that this not necessarily will pass at the end due to the same reason + // SAFETY: is_some() checked prior + let committed_pubkeys = committed_pubkeys.unwrap(); + let (commit_strategy, finalize_strategy, cleanup) = self + .handle_cpi_limit_error(single_stage_executor.transaction_strategy); + self.junk.push(cleanup); + self.patched_errors.push(execution_err); + + self.two_stage_execution_flow( + &committed_pubkeys, + commit_strategy, + finalize_strategy, + persister, + ) + .await } pub async fn two_stage_execution_flow( - &self, + &mut self, committed_pubkeys: &[Pubkey], commit_strategy: TransactionStrategy, finalize_strategy: TransactionStrategy, persister: &Option

, ) -> IntentExecutorResult { - let mut junk = Vec::new(); - let res = TwoStageExecutor::new(self) - .execute( - committed_pubkeys, - commit_strategy, - finalize_strategy, - &mut junk, - persister, - ) + let mut two_stage_executor = + TwoStageExecutor::new(self, commit_strategy, finalize_strategy); + let res = two_stage_executor + .execute(committed_pubkeys, persister) .await; - self.spawn_cleanup_task(junk); + + // Dissasemble executor to take all the goodies + let TwoStageExecutor { + inner: _, + commit_strategy, + finalize_strategy, + junk, + patched_errors, + } = two_stage_executor; + // After execution ended - everything becomes junk + // need to dispose it + self.junk.extend(junk.into_iter()); + self.junk.push(commit_strategy); + self.junk.push(finalize_strategy); + self.patched_errors.extend(patched_errors.into_iter()); res } @@ -773,7 +826,7 @@ where /// Executes Message on Base layer /// Returns `ExecutionOutput` or an `Error` async fn execute( - &self, + &mut self, base_intent: ScheduledBaseIntent, persister: Option

, ) -> IntentExecutorResult { 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 d893705cf..f7600a053 100644 --- a/magicblock-committor-service/src/intent_executor/single_stage_executor.rs +++ b/magicblock-committor-service/src/intent_executor/single_stage_executor.rs @@ -1,7 +1,7 @@ use std::ops::{ControlFlow, Deref}; -use log::{error, info}; -use magicblock_program::magic_scheduled_base_intent::ScheduledBaseIntent; +use log::error; +use solana_pubkey::Pubkey; use crate::{ intent_executor::{ @@ -19,7 +19,12 @@ use crate::{ pub struct SingleStageExecutor<'a, T, F> { inner: &'a IntentExecutorImpl, - // TODO: add strategy here? + pub transaction_strategy: TransactionStrategy, + + /// Junk that needs to be cleaned up + pub junk: Vec, + /// Errors we patched trying to recover intent + pub patched_errors: Vec, } impl<'a, T, F> SingleStageExecutor<'a, T, F> @@ -27,27 +32,34 @@ where T: TransactionPreparator + Clone, F: TaskInfoFetcher, { - pub fn new(executor: &'a IntentExecutorImpl) -> Self { - Self { inner: executor } + pub fn new( + executor: &'a IntentExecutorImpl, + transaction_strategy: TransactionStrategy, + ) -> Self { + Self { + inner: executor, + transaction_strategy, + junk: vec![], + patched_errors: vec![], + } } pub async fn execute( - &self, - base_intent: ScheduledBaseIntent, - mut transaction_strategy: TransactionStrategy, - junk: &mut Vec, + &mut self, + committed_pubkeys: Option<&[Pubkey]>, persister: &Option

, ) -> IntentExecutorResult { const RECURSION_CEILING: u8 = 10; let mut i = 0; - let (execution_err, last_transaction_strategy) = loop { + let execution_err = loop { i += 1; // Prepare & execute message let execution_result = self + .inner .prepare_and_execute_strategy( - &mut transaction_strategy, + &mut self.transaction_strategy, persister, ) .await @@ -56,85 +68,51 @@ where let execution_err = match execution_result { // break with result, strategy that was executed at this point has to be returned for cleanup Ok(value) => { - junk.push(transaction_strategy); return Ok(ExecutionOutput::SingleStage(value)); } Err(err) => err, }; // Attempt patching - let flow = self - .patch_strategy( - &execution_err, - &mut transaction_strategy, - &base_intent, - ) - .await?; + let flow = Self::patch_strategy( + self.inner, + &execution_err, + &mut self.transaction_strategy, + committed_pubkeys, + ) + .await?; let cleanup = match flow { - ControlFlow::Continue(cleanup) => { - info!( - "Patched intent: {}. patched error: {:?}", - base_intent.id, execution_err - ); - cleanup - } + ControlFlow::Continue(cleanup) => cleanup, ControlFlow::Break(()) => { - error!("Could not patch failed intent: {}", base_intent.id); - break (execution_err, transaction_strategy); + break execution_err; } }; + self.junk.push(cleanup); if i >= RECURSION_CEILING { error!( "CRITICAL! Recursion ceiling reached in intent execution." ); - break (execution_err, cleanup); + break execution_err; } else { - junk.push(cleanup); + self.patched_errors.push(execution_err); } }; // Special case - let committed_pubkeys = base_intent.get_committed_pubkeys(); - if i < RECURSION_CEILING - && matches!( - execution_err, - TransactionStrategyExecutionError::CpiLimitError(_, _) - ) - && committed_pubkeys.is_some() - { - // With actions, we can't predict num of CPIs - // If we get here we will try to switch from Single stage to Two Stage commit - // Note that this not necessarily will pass at the end due to the same reason - - // SAFETY: is_some() checked prior - let committed_pubkeys = committed_pubkeys.unwrap(); - let (commit_strategy, finalize_strategy, cleanup) = - self.handle_cpi_limit_error(last_transaction_strategy); - junk.push(cleanup); - self.two_stage_execution_flow( - &committed_pubkeys, - commit_strategy, - finalize_strategy, - persister, - ) - .await - } else { - junk.push(last_transaction_strategy); - let err = IntentExecutorError::from_strategy_execution_error( - execution_err, - |internal_err| { - let signature = internal_err.signature(); - IntentExecutorError::FailedToFinalizeError { - err: internal_err, - commit_signature: signature, - finalize_signature: signature, - } - }, - ); - - Err(err) - } + let err = IntentExecutorError::from_strategy_execution_error( + execution_err, + |internal_err| { + let signature = internal_err.signature(); + IntentExecutorError::FailedToFinalizeError { + err: internal_err, + commit_signature: signature, + finalize_signature: signature, + } + }, + ); + + Err(err) } /// Patch the current `transaction_strategy` in response to a recoverable @@ -145,13 +123,12 @@ where /// - `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( - &self, + inner: &IntentExecutorImpl, err: &TransactionStrategyExecutionError, transaction_strategy: &mut TransactionStrategy, - base_intent: &ScheduledBaseIntent, + committed_pubkeys: Option<&[Pubkey]>, ) -> IntentExecutorResult> { - let Some(committed_pubkeys) = base_intent.get_committed_pubkeys() - else { + let Some(committed_pubkeys) = committed_pubkeys else { // No patching is applicable if intent doesn't commit accounts return Ok(ControlFlow::Break(())); }; @@ -161,15 +138,15 @@ where // Here we patch strategy for it to be retried in next iteration // & we also record data that has to be cleaned up after patch let to_cleanup = - self.handle_actions_error(transaction_strategy); + inner.handle_actions_error(transaction_strategy); Ok(ControlFlow::Continue(to_cleanup)) } TransactionStrategyExecutionError::CommitIDError(_, _) => { // Here we patch strategy for it to be retried in next iteration // & we also record data that has to be cleaned up after patch - let to_cleanup = self + let to_cleanup = inner .handle_commit_id_error( - &committed_pubkeys, + committed_pubkeys, transaction_strategy, ) .await?; @@ -179,7 +156,7 @@ where // Here we patch strategy for it to be retried in next iteration // & we also record data that has to be cleaned up after patch let to_cleanup = - self.handle_undelegation_error(transaction_strategy); + inner.handle_undelegation_error(transaction_strategy); Ok(ControlFlow::Continue(to_cleanup)) } TransactionStrategyExecutionError::CpiLimitError(_, _) => { 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 11e110810..0f81f6481 100644 --- a/magicblock-committor-service/src/intent_executor/two_stage_executor.rs +++ b/magicblock-committor-service/src/intent_executor/two_stage_executor.rs @@ -17,8 +17,18 @@ use crate::{ transaction_preparator::TransactionPreparator, }; +// TODO(edwin): Could be splitted into 2 States +// TwoStageExecutor & TwoStageExecutorCommit pub struct TwoStageExecutor<'a, T, F> { inner: &'a IntentExecutorImpl, + // TODO(edwin): don't forget to cleanup this at the end + pub commit_strategy: TransactionStrategy, + pub finalize_strategy: TransactionStrategy, + + /// Junk that needs to be cleaned up + pub junk: Vec, + /// Errors we patched trying to recover intent + pub patched_errors: Vec, } impl<'a, T, F> TwoStageExecutor<'a, T, F> @@ -26,62 +36,74 @@ where T: TransactionPreparator + Clone, F: TaskInfoFetcher, { - pub fn new(executor: &'a IntentExecutorImpl) -> Self { - Self { inner: executor } + pub fn new( + executor: &'a IntentExecutorImpl, + commit_strategy: TransactionStrategy, + finalize_strategy: TransactionStrategy, + ) -> Self { + Self { + inner: executor, + commit_strategy, + finalize_strategy, + + junk: vec![], + patched_errors: vec![], + } } pub async fn execute( - &self, + &mut self, committed_pubkeys: &[Pubkey], - mut commit_strategy: TransactionStrategy, - mut finalize_strategy: TransactionStrategy, - junk: &mut Vec, persister: &Option

, ) -> IntentExecutorResult { const RECURSION_CEILING: u8 = 10; let mut i = 0; - let (commit_result, last_commit_strategy) = loop { + let commit_result = loop { i += 1; // Prepare & execute message let execution_result = self - .prepare_and_execute_strategy(&mut commit_strategy, persister) + .inner + .prepare_and_execute_strategy( + &mut self.commit_strategy, + persister, + ) .await .map_err(IntentExecutorError::FailedCommitPreparationError)?; let execution_err = match execution_result { - Ok(value) => break (Ok(value), commit_strategy), + Ok(value) => break Ok(value), Err(err) => err, }; - let flow = self - .patch_commit_strategy( - &execution_err, - &mut commit_strategy, - committed_pubkeys, - ) - .await?; + let flow = Self::patch_commit_strategy( + self.inner, + &execution_err, + &mut self.commit_strategy, + committed_pubkeys, + ) + .await?; let cleanup = match flow { ControlFlow::Continue(value) => { info!("Patched intent, error was: {:?}", execution_err); value } ControlFlow::Break(()) => { - break (Err(execution_err), commit_strategy) + break Err(execution_err); } }; + self.junk.push(cleanup); if i >= RECURSION_CEILING { error!( "CRITICAL! Recursion ceiling reached in intent execution." ); - break (Err(execution_err), cleanup); + break Err(execution_err); } else { - junk.push(cleanup); + self.patched_errors.push(execution_err); } }; - junk.push(last_commit_strategy); let commit_signature = commit_result.map_err(|err| { IntentExecutorError::from_strategy_execution_error( err, @@ -96,41 +118,48 @@ where })?; i = 0; - let (finalize_result, last_finalize_strategy) = loop { + let finalize_result = loop { i += 1; // Prepare & execute message let execution_result = self - .prepare_and_execute_strategy(&mut finalize_strategy, persister) + .inner + .prepare_and_execute_strategy( + &mut self.finalize_strategy, + persister, + ) .await .map_err(IntentExecutorError::FailedFinalizePreparationError)?; let execution_err = match execution_result { - Ok(value) => break (Ok(value), finalize_strategy), + Ok(value) => break Ok(value), Err(err) => err, }; - let flow = self - .patch_finalize_strategy(&execution_err, &mut finalize_strategy) - .await?; + let flow = Self::patch_finalize_strategy( + self.inner, + &execution_err, + &mut self.finalize_strategy, + ) + .await?; let cleanup = match flow { ControlFlow::Continue(cleanup) => cleanup, ControlFlow::Break(()) => { - break (Err(execution_err), finalize_strategy) + break Err(execution_err); } }; + self.junk.push(cleanup); if i >= RECURSION_CEILING { error!( "CRITICAL! Recursion ceiling reached in intent execution." ); - break (Err(execution_err), cleanup); + break Err(execution_err); } else { - junk.push(cleanup); + self.patched_errors.push(execution_err); } }; - junk.push(last_finalize_strategy); let finalize_signature = finalize_result.map_err(|err| { IntentExecutorError::from_strategy_execution_error( err, @@ -159,14 +188,14 @@ where /// - `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( - &self, + inner: &IntentExecutorImpl, err: &TransactionStrategyExecutionError, commit_strategy: &mut TransactionStrategy, committed_pubkeys: &[Pubkey], ) -> IntentExecutorResult> { match err { TransactionStrategyExecutionError::CommitIDError(_, _) => { - let to_cleanup = self + let to_cleanup = inner .handle_commit_id_error(committed_pubkeys, commit_strategy) .await?; Ok(ControlFlow::Continue(to_cleanup)) @@ -203,7 +232,7 @@ where /// - `Continue(to_cleanup)` when a retry should be attempted with cleanup metadata, or /// - `Break(())` when this stage cannot be recovered. pub async fn patch_finalize_strategy( - &self, + inner: &IntentExecutorImpl, err: &TransactionStrategyExecutionError, finalize_strategy: &mut TransactionStrategy, ) -> IntentExecutorResult> { @@ -216,14 +245,14 @@ where TransactionStrategyExecutionError::ActionsError(_, _) => { // Here we patch strategy for it to be retried in next iteration // & we also record data that has to be cleaned up after patch - let to_cleanup = self.handle_actions_error(finalize_strategy); + let to_cleanup = inner.handle_actions_error(finalize_strategy); Ok(ControlFlow::Continue(to_cleanup)) } 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 let to_cleanup = - self.handle_undelegation_error(finalize_strategy); + inner.handle_undelegation_error(finalize_strategy); Ok(ControlFlow::Continue(to_cleanup)) } TransactionStrategyExecutionError::CpiLimitError(_, _) => { From f4d9eaebc9d928bdbb9320c099f393a058d53cf0 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Thu, 20 Nov 2025 16:06:23 +0400 Subject: [PATCH 09/29] fix: compilation --- .../src/scheduled_commits_processor.rs | 6 +- .../intent_execution_engine.rs | 61 ++++--- .../src/intent_executor/error.rs | 92 ++++++----- .../src/intent_executor/mod.rs | 152 +++++++++++------- .../intent_executor/single_stage_executor.rs | 22 +-- .../src/intent_executor/two_stage_executor.rs | 57 +++---- .../delivery_preparator.rs | 3 +- .../src/transaction_preparator/mod.rs | 6 +- 8 files changed, 230 insertions(+), 169 deletions(-) diff --git a/magicblock-accounts/src/scheduled_commits_processor.rs b/magicblock-accounts/src/scheduled_commits_processor.rs index 83a03c064..940790dc1 100644 --- a/magicblock-accounts/src/scheduled_commits_processor.rs +++ b/magicblock-accounts/src/scheduled_commits_processor.rs @@ -214,7 +214,9 @@ impl ScheduledCommitsProcessorImpl { let (intent_id, trigger_type) = execution_result .as_ref() .map(|output| (output.id, output.trigger_type)) - .unwrap_or_else(|(id, trigger_type, _)| (*id, *trigger_type)); + .unwrap_or_else(|(id, trigger_type, _, _)| { + (*id, *trigger_type) + }); // Here we handle on OnChain triggered intent // TODO: should be removed once crank supported @@ -251,7 +253,7 @@ impl ScheduledCommitsProcessorImpl { ) .await; } - Err((_, _, err)) => { + Err((_, _, _, err)) => { match err.as_ref() { &magicblock_committor_service::intent_executor::error::IntentExecutorError::EmptyIntentError => { warn!("Empty intent was scheduled!"); diff --git a/magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs b/magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs index 212111cd0..4eed20ac6 100644 --- a/magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs +++ b/magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs @@ -21,7 +21,10 @@ use crate::{ IntentExecutionManagerError, }, intent_executor::{ - error::{IntentExecutorError, IntentExecutorResult}, + error::{ + IntentExecutorError, IntentExecutorResult, + TransactionStrategyExecutionError, + }, intent_executor_factory::IntentExecutorFactory, ExecutionOutput, IntentExecutor, }, @@ -33,14 +36,22 @@ const SEMAPHORE_CLOSED_MSG: &str = "Executors semaphore closed!"; /// Max number of executors that can send messages in parallel to Base layer const MAX_EXECUTORS: u8 = 50; +pub type PatchedErrors = Vec; + #[derive(Clone, Debug)] pub struct ExecutionOutputWrapper { pub id: u64, pub output: ExecutionOutput, + pub patched_errors: Arc, pub trigger_type: TriggerType, } -pub type BroadcastedError = (u64, TriggerType, Arc); +pub type BroadcastedError = ( + u64, + TriggerType, + Arc, + Arc, +); pub type BroadcastedIntentExecutionResult = IntentExecutorResult; @@ -246,28 +257,30 @@ where result_sender: broadcast::Sender, ) { let instant = Instant::now(); - let result = executor - .execute(intent.inner.clone(), persister) - .await - .inspect_err(|err| { - error!( - "Failed to execute BaseIntent. id: {}. {}", - intent.id, err - ) - }); + let result = executor.execute(intent.inner.clone(), persister).await; + + let _ = result.inner.as_ref().inspect_err(|err| { + error!("Failed to execute BaseIntent. id: {}. {}", intent.id, err) + }); // Metrics - Self::execution_metrics(instant.elapsed(), &intent, &result); + Self::execution_metrics(instant.elapsed(), &intent, &result.inner); - let result = result - .map(|output| ExecutionOutputWrapper { + let patched_errors = Arc::new(result.patched_errors); + let result = match result.inner { + Ok(output) => Ok(ExecutionOutputWrapper { id: intent.id, trigger_type: intent.trigger_type, + patched_errors, output, - }) - .map_err(|err| { - (intent.inner.id, intent.trigger_type, Arc::new(err)) - }); + }), + Err(err) => Err(( + intent.inner.id, + intent.trigger_type, + patched_errors, + Arc::new(err), + )), + }; // Broadcast result to subscribers if let Err(err) = result_sender.send(result) { @@ -284,6 +297,18 @@ where .complete(&intent.inner) .expect("Valid completion of previously scheduled message"); + tokio::spawn(async move { + // Cleanup after intent + // Note: in some cases it maybe critical to execute cleanup synchronously + // Example: if commit nonces were invalid during execution + // next intent could use wrongly initiated buffers by current intent + // We assume that this case is highly unlikely since it would mean: + // user redelegates amd reaches current commit id faster than we execute transactions below + if let Err(err) = executor.cleanup().await { + error!("Failed to cleanup after intent: {}", err); + } + }); + // Free worker drop(execution_permit); } diff --git a/magicblock-committor-service/src/intent_executor/error.rs b/magicblock-committor-service/src/intent_executor/error.rs index e17eda365..268663694 100644 --- a/magicblock-committor-service/src/intent_executor/error.rs +++ b/magicblock-committor-service/src/intent_executor/error.rs @@ -38,14 +38,6 @@ impl InternalError { pub enum IntentExecutorError { #[error("EmptyIntentError")] EmptyIntentError, - #[error("User supplied actions are ill-formed: {0}. {:?}", .1)] - ActionsError(#[source] TransactionError, Option), - #[error("Invalid undelegation: {0}. {:?}", .1)] - UndelegationError(#[source] TransactionError, Option), - #[error("Accounts committed with an invalid Commit id: {0}. {:?}", .1)] - CommitIDError(#[source] TransactionError, Option), - #[error("Max instruction trace length exceeded: {0}. {:?}", .1)] - CpiLimitError(#[source] TransactionError, Option), #[error("Failed to fit in single TX")] FailedToFitError, #[error("SignerError: {0}")] @@ -56,13 +48,13 @@ pub enum IntentExecutorError { #[error("FailedToCommitError: {err}")] FailedToCommitError { #[source] - err: InternalError, + err: TransactionStrategyExecutionError, signature: Option, }, #[error("FailedToFinalizeError: {err}")] FailedToFinalizeError { #[source] - err: InternalError, + err: TransactionStrategyExecutionError, commit_signature: Option, finalize_signature: Option, }, @@ -73,36 +65,25 @@ pub enum IntentExecutorError { } impl IntentExecutorError { - pub fn is_cpi_limit_error(&self) -> bool { - matches!(self, IntentExecutorError::CpiLimitError(_, _)) + pub fn from_commmit_execution_error( + error: TransactionStrategyExecutionError, + ) -> IntentExecutorError { + let signature = error.signature(); + IntentExecutorError::FailedToCommitError { + err: error, + signature, + } } - pub fn from_strategy_execution_error( + pub fn from_finalize_execution_error( error: TransactionStrategyExecutionError, - converter: F, - ) -> IntentExecutorError - where - F: FnOnce(InternalError) -> IntentExecutorError, - { - match error { - TransactionStrategyExecutionError::ActionsError(err, signature) => { - IntentExecutorError::ActionsError(err, signature) - } - TransactionStrategyExecutionError::CpiLimitError( - err, - signature, - ) => IntentExecutorError::CpiLimitError(err, signature), - TransactionStrategyExecutionError::CommitIDError( - err, - signature, - ) => IntentExecutorError::CommitIDError(err, signature), - TransactionStrategyExecutionError::UndelegationError( - err, - signature, - ) => IntentExecutorError::UndelegationError(err, signature), - TransactionStrategyExecutionError::InternalError(err) => { - converter(err) - } + commit_signature: Option, + ) -> IntentExecutorError { + let finalize_signature = error.signature(); + IntentExecutorError::FailedToFinalizeError { + err: error, + commit_signature, + finalize_signature, } } } @@ -110,9 +91,14 @@ impl IntentExecutorError { impl metrics::LabelValue for IntentExecutorError { fn value(&self) -> &str { match self { - IntentExecutorError::ActionsError(_, _) => "actions_failed", - IntentExecutorError::CpiLimitError(_, _) => "cpi_limit_failed", - IntentExecutorError::CommitIDError(_, _) => "commit_nonce_failed", + IntentExecutorError::FailedToCommitError { err, signature: _ } => { + err.value() + } + IntentExecutorError::FailedToFinalizeError { + err, + commit_signature: _, + finalize_signature: _, + } => err.value(), _ => "failed", } } @@ -140,6 +126,20 @@ impl From for TransactionStrategyExecutionError { } impl TransactionStrategyExecutionError { + pub fn is_cpi_limit_error(&self) -> bool { + matches!(self, Self::CommitIDError(_, _)) + } + + pub fn signature(&self) -> Option { + match self { + Self::InternalError(err) => err.signature(), + Self::CommitIDError(_, signature) + | Self::ActionsError(_, signature) + | Self::UndelegationError(_, signature) + | Self::CpiLimitError(_, signature) => *signature, + } + } + /// Convert [`TransactionError`] into known errors that can be handled /// Otherwise return original [`TransactionError`] /// [`TransactionStrategyExecutionError`] @@ -215,6 +215,18 @@ impl TransactionStrategyExecutionError { } } +impl metrics::LabelValue for TransactionStrategyExecutionError { + fn value(&self) -> &str { + match self { + Self::ActionsError(_, _) => "actions_failed", + Self::CpiLimitError(_, _) => "cpi_limit_failed", + Self::CommitIDError(_, _) => "commit_nonce_failed", + Self::UndelegationError(_, _) => "undelegation_failed", + _ => "failed", + } + } +} + pub(crate) struct IntentTransactionErrorMapper<'a> { pub tasks: &'a [Box], } diff --git a/magicblock-committor-service/src/intent_executor/mod.rs b/magicblock-committor-service/src/intent_executor/mod.rs index 695ce48fa..5500b443c 100644 --- a/magicblock-committor-service/src/intent_executor/mod.rs +++ b/magicblock-committor-service/src/intent_executor/mod.rs @@ -4,11 +4,11 @@ pub mod single_stage_executor; pub mod task_info_fetcher; pub mod two_stage_executor; -use std::{ops::ControlFlow, sync::Arc, time::Duration}; +use std::{mem, ops::ControlFlow, sync::Arc, time::Duration}; use async_trait::async_trait; use futures_util::future::try_join_all; -use log::{error, trace, warn}; +use log::{trace, warn}; use magicblock_metrics::metrics; use magicblock_program::{ magic_scheduled_base_intent::ScheduledBaseIntent, @@ -51,6 +51,7 @@ use crate::{ BaseTask, TaskType, }, transaction_preparator::{ + delivery_preparator::BufferExecutionError, error::TransactionPreparatorError, TransactionPreparator, }, utils::persist_status_update_by_message_set, @@ -82,6 +83,13 @@ impl metrics::LabelValue for ExecutionOutput { } } +pub struct IntentExecutionResult { + /// Final result of Intent Execution + pub inner: IntentExecutorResult, + /// Errors patched along the way + pub patched_errors: Vec, +} + #[async_trait] pub trait IntentExecutor: Send + Sync + 'static { /// Executes Message on Base layer @@ -90,7 +98,10 @@ pub trait IntentExecutor: Send + Sync + 'static { &mut self, base_intent: ScheduledBaseIntent, persister: Option

, - ) -> IntentExecutorResult; + ) -> IntentExecutionResult; + + /// Cleans up after intent + async fn cleanup(self) -> Result<(), BufferExecutionError>; } pub struct IntentExecutorImpl { @@ -107,7 +118,7 @@ pub struct IntentExecutorImpl { impl IntentExecutorImpl where - T: TransactionPreparator + Clone, + T: TransactionPreparator, F: TaskInfoFetcher, { pub fn new( @@ -278,28 +289,39 @@ where .await; // After execution is complete record junk & patched errors - self.junk.append(&mut single_stage_executor.junk); - self.patched_errors - .append(&mut single_stage_executor.patched_errors); + let SingleStageExecutor { + inner: _, + transaction_strategy, + junk, + patched_errors, + } = single_stage_executor; + self.junk.extend(junk.into_iter()); + self.patched_errors.extend(patched_errors.into_iter()); + // Here we continue only IF the error is CpiLimitError + // We can recover that Error by splitting execution + // in 2 stages - commit & finalize + // Otherwise we return error let execution_err = match res { - Ok(value) => return Ok(value), - Err(err) => err, + Err(IntentExecutorError::FailedToFinalizeError { + err: + err @ TransactionStrategyExecutionError::CpiLimitError(_, _), + commit_signature: _, + finalize_signature: _, + }) if committed_pubkeys.is_some() => err, + res => { + self.junk.push(transaction_strategy); + return res; + } }; - if !(execution_err.is_cpi_limit_error() && committed_pubkeys.is_some()) - { - // Can't recover this - return Err - return Err(execution_err); - } - // With actions, we can't predict num of CPIs // If we get here we will try to switch from Single stage to Two Stage commit // Note that this not necessarily will pass at the end due to the same reason // SAFETY: is_some() checked prior let committed_pubkeys = committed_pubkeys.unwrap(); - let (commit_strategy, finalize_strategy, cleanup) = self - .handle_cpi_limit_error(single_stage_executor.transaction_strategy); + let (commit_strategy, finalize_strategy, cleanup) = + self.handle_cpi_limit_error(transaction_strategy); self.junk.push(cleanup); self.patched_errors.push(execution_err); @@ -319,13 +341,40 @@ where finalize_strategy: TransactionStrategy, persister: &Option

, ) -> IntentExecutorResult { + // TODO: implement custom drop where we flush junk Into Exector? let mut two_stage_executor = TwoStageExecutor::new(self, commit_strategy, finalize_strategy); - let res = two_stage_executor - .execute(committed_pubkeys, persister) + + // Execute commit stage + let commit_signature = two_stage_executor + .commit(committed_pubkeys, persister) + .await; + let commit_signature = match commit_signature { + Ok(signature) => signature, + Err(err) => { + let TwoStageExecutor { + inner: _, + finalize_strategy: _, + commit_strategy, + junk, + patched_errors, + } = two_stage_executor; + self.junk.extend(junk.into_iter()); + self.junk.push(commit_strategy); + self.patched_errors.extend(patched_errors.into_iter()); + + return Err(err); + } + }; + + // Execute finalize stage + let finalize_result = two_stage_executor + .finalize(commit_signature, persister) .await; // Dissasemble executor to take all the goodies + // After execution ended - everything becomes junk + // need to dispose it let TwoStageExecutor { inner: _, commit_strategy, @@ -333,14 +382,16 @@ where junk, patched_errors, } = two_stage_executor; - // After execution ended - everything becomes junk - // need to dispose it self.junk.extend(junk.into_iter()); self.junk.push(commit_strategy); self.junk.push(finalize_strategy); self.patched_errors.extend(patched_errors.into_iter()); - res + let finalize_signature = finalize_result?; + Ok(ExecutionOutput::TwoStage { + commit_signature, + finalize_signature, + }) } /// Handles out of sync commit id error, fixes current strategy @@ -571,10 +622,6 @@ where return; } - Err(IntentExecutorError::CommitIDError(_, _)) - | Err(IntentExecutorError::ActionsError(_, _)) - | Err(IntentExecutorError::CpiLimitError(_, _)) - | Err(IntentExecutorError::UndelegationError(_, _)) => None, Err(IntentExecutorError::EmptyIntentError) | Err(IntentExecutorError::FailedToFitError) | Err(IntentExecutorError::TaskBuilderError(_)) @@ -732,31 +779,6 @@ where .await } - /// Cleanup after intent - /// Note: in some cases it maybe critical to execute cleanup synchronously - /// Example: if commit nonces were invalid during execution - /// next intent could use wrongly initiated buffers by current intent - /// We assume that this case is highly unlikely since it would mean: - /// user redelegates amd reaches current commit id faster than we execute transactions below - fn spawn_cleanup_task(&self, junk: Vec) { - let authority = self.authority.insecure_clone(); - let transaction_preparator = self.transaction_preparator.clone(); - let cleanup_fut = async move { - let cleanup_futs = junk.iter().map(|to_cleanup| { - transaction_preparator.cleanup_for_strategy( - &authority, - &to_cleanup.optimized_tasks, - &to_cleanup.lookup_tables_keys, - ) - }); - - if let Err(err) = try_join_all(cleanup_futs).await { - error!("Failed to cleanup after intent: {}", err); - } - }; - tokio::spawn(cleanup_fut); - } - async fn intent_metrics( rpc_client: MagicblockRpcClient, execution_outcome: ExecutionOutput, @@ -820,7 +842,7 @@ where #[async_trait] impl IntentExecutor for IntentExecutorImpl where - T: TransactionPreparator + Clone, + T: TransactionPreparator, C: TaskInfoFetcher, { /// Executes Message on Base layer @@ -829,7 +851,7 @@ where &mut self, base_intent: ScheduledBaseIntent, persister: Option

, - ) -> IntentExecutorResult { + ) -> IntentExecutionResult { let message_id = base_intent.id; let is_undelegate = base_intent.is_undelegate(); let pubkeys = base_intent.get_committed_pubkeys(); @@ -846,12 +868,32 @@ where // Write result of intent into Persister Self::persist_result(&persister, &result, message_id, &pubkeys); } - result.inspect(|output| { + + // Gather metrics in separate task + let result = result.inspect(|output| { tokio::spawn(Self::intent_metrics( self.rpc_client.clone(), *output, )); - }) + }); + + IntentExecutionResult { + inner: result, + patched_errors: mem::take(&mut self.patched_errors), + } + } + + /// Cleanup after intent using junk + async fn cleanup(mut self) -> Result<(), BufferExecutionError> { + let cleanup_futs = self.junk.iter().map(|to_cleanup| { + self.transaction_preparator.cleanup_for_strategy( + &self.authority, + &to_cleanup.optimized_tasks, + &to_cleanup.lookup_tables_keys, + ) + }); + + try_join_all(cleanup_futs).await.map(|_| ()) } } 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 f7600a053..624b702e2 100644 --- a/magicblock-committor-service/src/intent_executor/single_stage_executor.rs +++ b/magicblock-committor-service/src/intent_executor/single_stage_executor.rs @@ -18,7 +18,7 @@ use crate::{ }; pub struct SingleStageExecutor<'a, T, F> { - inner: &'a IntentExecutorImpl, + pub(in crate::intent_executor) inner: &'a IntentExecutorImpl, pub transaction_strategy: TransactionStrategy, /// Junk that needs to be cleaned up @@ -29,7 +29,7 @@ pub struct SingleStageExecutor<'a, T, F> { impl<'a, T, F> SingleStageExecutor<'a, T, F> where - T: TransactionPreparator + Clone, + T: TransactionPreparator, F: TaskInfoFetcher, { pub fn new( @@ -64,6 +64,7 @@ where ) .await .map_err(IntentExecutorError::FailedFinalizePreparationError)?; + // Process error: Ok - return, Err - handle further let execution_err = match execution_result { // break with result, strategy that was executed at this point has to be returned for cleanup @@ -99,20 +100,11 @@ where } }; - // Special case - let err = IntentExecutorError::from_strategy_execution_error( + Err(IntentExecutorError::from_finalize_execution_error( execution_err, - |internal_err| { - let signature = internal_err.signature(); - IntentExecutorError::FailedToFinalizeError { - err: internal_err, - commit_signature: signature, - finalize_signature: signature, - } - }, - ); - - Err(err) + // TODO(edwin): shall one stage have same signature for commit & finalize + None, + )) } /// Patch the current `transaction_strategy` in response to a recoverable 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 0f81f6481..5295237c0 100644 --- a/magicblock-committor-service/src/intent_executor/two_stage_executor.rs +++ b/magicblock-committor-service/src/intent_executor/two_stage_executor.rs @@ -2,6 +2,7 @@ use std::ops::{ControlFlow, Deref}; use log::{error, info, warn}; use solana_pubkey::Pubkey; +use solana_sdk::signature::Signature; use crate::{ intent_executor::{ @@ -10,7 +11,7 @@ use crate::{ TransactionStrategyExecutionError, }, task_info_fetcher::TaskInfoFetcher, - ExecutionOutput, IntentExecutorImpl, + IntentExecutorImpl, }, persist::IntentPersister, tasks::task_strategist::TransactionStrategy, @@ -20,8 +21,7 @@ use crate::{ // TODO(edwin): Could be splitted into 2 States // TwoStageExecutor & TwoStageExecutorCommit pub struct TwoStageExecutor<'a, T, F> { - inner: &'a IntentExecutorImpl, - // TODO(edwin): don't forget to cleanup this at the end + pub(in crate::intent_executor) inner: &'a IntentExecutorImpl, pub commit_strategy: TransactionStrategy, pub finalize_strategy: TransactionStrategy, @@ -33,9 +33,11 @@ pub struct TwoStageExecutor<'a, T, F> { impl<'a, T, F> TwoStageExecutor<'a, T, F> where - T: TransactionPreparator + Clone, + T: TransactionPreparator, F: TaskInfoFetcher, { + const RECURSION_CEILING: u8 = 10; + pub fn new( executor: &'a IntentExecutorImpl, commit_strategy: TransactionStrategy, @@ -51,13 +53,11 @@ where } } - pub async fn execute( + pub async fn commit( &mut self, committed_pubkeys: &[Pubkey], persister: &Option

, - ) -> IntentExecutorResult { - const RECURSION_CEILING: u8 = 10; - + ) -> IntentExecutorResult { let mut i = 0; let commit_result = loop { i += 1; @@ -94,7 +94,7 @@ where }; self.junk.push(cleanup); - if i >= RECURSION_CEILING { + if i >= Self::RECURSION_CEILING { error!( "CRITICAL! Recursion ceiling reached in intent execution." ); @@ -105,19 +105,18 @@ where }; let commit_signature = commit_result.map_err(|err| { - IntentExecutorError::from_strategy_execution_error( - err, - |internal_err| { - let signature = internal_err.signature(); - IntentExecutorError::FailedToCommitError { - err: internal_err, - signature, - } - }, - ) + IntentExecutorError::from_commmit_execution_error(err) })?; - i = 0; + Ok(commit_signature) + } + + pub async fn finalize( + &mut self, + commit_signature: Signature, + persister: &Option

, + ) -> IntentExecutorResult { + let mut i = 0; let finalize_result = loop { i += 1; @@ -150,7 +149,7 @@ where }; self.junk.push(cleanup); - if i >= RECURSION_CEILING { + if i >= Self::RECURSION_CEILING { error!( "CRITICAL! Recursion ceiling reached in intent execution." ); @@ -161,23 +160,13 @@ where }; let finalize_signature = finalize_result.map_err(|err| { - IntentExecutorError::from_strategy_execution_error( + IntentExecutorError::from_finalize_execution_error( err, - |internal_err| { - let finalize_signature = internal_err.signature(); - IntentExecutorError::FailedToFinalizeError { - err: internal_err, - commit_signature: Some(commit_signature), - finalize_signature, - } - }, + Some(commit_signature), ) })?; - Ok(ExecutionOutput::TwoStage { - commit_signature, - finalize_signature, - }) + Ok(finalize_signature) } /// Patches Commit stage `transaction_strategy` in response to a recoverable diff --git a/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs b/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs index 94da08628..037d95bf4 100644 --- a/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs +++ b/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs @@ -439,7 +439,7 @@ impl DeliveryPreparator { authority: &Keypair, tasks: &[Box], lookup_table_keys: &[Pubkey], - ) -> DeliveryPreparatorResult<(), InternalError> { + ) -> DeliveryPreparatorResult<(), BufferExecutionError> { self.table_mania .release_pubkeys(&HashSet::from_iter( lookup_table_keys.iter().cloned(), @@ -490,7 +490,6 @@ impl DeliveryPreparator { join_all(close_futs) .await .into_iter() - .map(|res| res.map_err(InternalError::from)) .inspect(|res| { if let Err(err) = res { error!("Failed to cleanup buffers: {}", err); diff --git a/magicblock-committor-service/src/transaction_preparator/mod.rs b/magicblock-committor-service/src/transaction_preparator/mod.rs index 47c795797..f0364797f 100644 --- a/magicblock-committor-service/src/transaction_preparator/mod.rs +++ b/magicblock-committor-service/src/transaction_preparator/mod.rs @@ -11,7 +11,7 @@ use crate::{ }, transaction_preparator::{ delivery_preparator::{ - DeliveryPreparator, DeliveryPreparatorResult, InternalError, + BufferExecutionError, DeliveryPreparator, DeliveryPreparatorResult, }, error::PreparatorResult, }, @@ -38,7 +38,7 @@ pub trait TransactionPreparator: Send + Sync + 'static { authority: &Keypair, tasks: &[Box], lookup_table_keys: &[Pubkey], - ) -> DeliveryPreparatorResult<(), InternalError>; + ) -> DeliveryPreparatorResult<(), BufferExecutionError>; } /// [`TransactionPreparatorImpl`] first version of preparator @@ -114,7 +114,7 @@ impl TransactionPreparator for TransactionPreparatorImpl { authority: &Keypair, tasks: &[Box], lookup_table_keys: &[Pubkey], - ) -> DeliveryPreparatorResult<(), InternalError> { + ) -> DeliveryPreparatorResult<(), BufferExecutionError> { self.delivery_preparator .cleanup(authority, tasks, lookup_table_keys) .await From 95e0fb0f10b6f4485e9851262b1af0af732408ca Mon Sep 17 00:00:00 2001 From: taco-paco Date: Fri, 21 Nov 2025 10:56:42 +0400 Subject: [PATCH 10/29] fix: tests --- .../intent_execution_engine.rs | 57 ++++++--- .../src/stubs/changeset_committor_stub.rs | 1 + .../programs/flexi-counter/src/processor.rs | 18 +-- .../tests/test_intent_executor.rs | 114 +++++++++++++++--- 4 files changed, 150 insertions(+), 40 deletions(-) diff --git a/magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs b/magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs index 4eed20ac6..1f65f3974 100644 --- a/magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs +++ b/magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs @@ -367,6 +367,7 @@ mod tests { use magicblock_program::magic_scheduled_base_intent::ScheduledBaseIntent; use solana_pubkey::{pubkey, Pubkey}; use solana_sdk::{signature::Signature, signer::SignerError}; + use solana_sdk::transaction::TransactionError; use tokio::{sync::mpsc, time::sleep}; use super::*; @@ -376,15 +377,14 @@ mod tests { intent_scheduler::create_test_intent, }, intent_executor::{ - error::{ - IntentExecutorError as ExecutorError, IntentExecutorResult, - InternalError, - }, + error::{IntentExecutorError as ExecutorError, InternalError}, task_info_fetcher::{ ResetType, TaskInfoFetcher, TaskInfoFetcherResult, }, + IntentExecutionResult, }, persist::IntentPersisterImpl, + transaction_preparator::delivery_preparator::BufferExecutionError, }; type MockIntentExecutionEngine = IntentExecutionEngine< @@ -476,14 +476,18 @@ mod tests { // Verify the failure was properly reported let result = result_receiver.recv().await.unwrap(); - let Err((id, trigger_type, err)) = result else { + let Err((id, trigger_type, patched_errors, err)) = result else { panic!(); }; assert_eq!(id, 1); assert_eq!(trigger_type, TriggerType::OffChain); + assert_eq!( + patched_errors[0].to_string(), + "User supplied actions are ill-formed: Attempt to debit an account but found no record of a prior credit.. None" + ); assert_eq!( err.to_string(), - "FailedToCommitError: SignerError: custom error: oops" + "FailedToCommitError: InternalError: SignerError: custom error: oops" ); } @@ -767,33 +771,50 @@ mod tests { #[async_trait] impl IntentExecutor for MockIntentExecutor { async fn execute( - &self, + &mut self, _base_intent: ScheduledBaseIntent, _persister: Option

, - ) -> IntentExecutorResult { + ) -> IntentExecutionResult { self.on_task_started(); // Simulate some work sleep(Duration::from_millis(50)).await; let result = if self.should_fail { - Err(ExecutorError::FailedToCommitError { - err: InternalError::SignerError(SignerError::Custom( - "oops".to_string(), - )), - signature: None, - }) + IntentExecutionResult { + inner: Err(ExecutorError::FailedToCommitError { + err: TransactionStrategyExecutionError::InternalError( + InternalError::SignerError(SignerError::Custom( + "oops".to_string(), + )), + ), + signature: None, + }), + patched_errors: vec![ + TransactionStrategyExecutionError::ActionsError( + TransactionError::AccountNotFound, + None + ) + ], + } } else { - Ok(ExecutionOutput::TwoStage { - commit_signature: Signature::default(), - finalize_signature: Signature::default(), - }) + IntentExecutionResult { + inner: Ok(ExecutionOutput::TwoStage { + commit_signature: Signature::default(), + finalize_signature: Signature::default(), + }), + patched_errors: vec![], + } }; self.on_task_finished(); result } + + async fn cleanup(self) -> Result<(), BufferExecutionError> { + Ok(()) + } } #[derive(Clone)] diff --git a/magicblock-committor-service/src/stubs/changeset_committor_stub.rs b/magicblock-committor-service/src/stubs/changeset_committor_stub.rs index 6893b5cd9..6fba2c112 100644 --- a/magicblock-committor-service/src/stubs/changeset_committor_stub.rs +++ b/magicblock-committor-service/src/stubs/changeset_committor_stub.rs @@ -206,6 +206,7 @@ impl BaseIntentCommittorExt for ChangesetCommittorStub { commit_signature: Signature::new_unique(), finalize_signature: Signature::new_unique(), }, + patched_errors: Arc::new(vec![]), trigger_type: TriggerType::OnChain, }) }) diff --git a/test-integration/programs/flexi-counter/src/processor.rs b/test-integration/programs/flexi-counter/src/processor.rs index eab3b8f70..62b5e9b0c 100644 --- a/test-integration/programs/flexi-counter/src/processor.rs +++ b/test-integration/programs/flexi-counter/src/processor.rs @@ -387,6 +387,15 @@ fn process_undelegate_request( ProgramError::InvalidArgument })?; + undelegate_account( + delegated_account, + &crate::id(), + buffer, + payer, + system_program, + account_seeds, + )?; + let counter = { let data = delegated_account.data.borrow(); FlexiCounter::deserialize(&mut data.as_ref())? @@ -396,14 +405,7 @@ fn process_undelegate_request( return Err(ProgramError::Custom(FAIL_UNDELEGATION_CODE)); } - undelegate_account( - delegated_account, - &crate::id(), - buffer, - payer, - system_program, - account_seeds, - )?; + Ok(()) } 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 c58ebfabf..49fadb61d 100644 --- a/test-integration/test-committor-service/tests/test_intent_executor.rs +++ b/test-integration/test-committor-service/tests/test_intent_executor.rs @@ -1,5 +1,6 @@ use std::{ collections::HashMap, + mem, sync::{ atomic::{AtomicU64, Ordering}, Arc, @@ -16,7 +17,8 @@ use magicblock_committor_service::{ intent_executor::{ error::TransactionStrategyExecutionError, task_info_fetcher::{CacheTaskInfoFetcher, TaskInfoFetcher}, - ExecutionOutput, IntentExecutor, IntentExecutorImpl, + ExecutionOutput, IntentExecutionResult, IntentExecutor, + IntentExecutorImpl, }, persist::IntentPersisterImpl, tasks::{ @@ -161,7 +163,7 @@ async fn test_commit_id_error_parsing() { #[tokio::test] async fn test_undelegation_error_parsing() { const COUNTER_SIZE: u64 = 70; - const EXPECTED_ERR_MSG: &str = "Invalid undelegation: Error processing Instruction 4: Failed to serialize or deserialize account data"; + const EXPECTED_ERR_MSG: &str = "Invalid undelegation: Error processing Instruction 4: custom program error: 0x7a"; let TestEnv { fixture, @@ -330,7 +332,7 @@ async fn test_commit_id_error_recovery() { let TestEnv { fixture, - intent_executor, + mut intent_executor, task_info_fetcher, pre_test_tablemania_state, } = TestEnv::setup().await; @@ -355,9 +357,22 @@ async fn test_commit_id_error_recovery() { let res = intent_executor .execute(intent, None::) .await; + let IntentExecutionResult { + inner: res, + patched_errors, + } = res; assert!(res.is_ok()); assert!(matches!(res.unwrap(), ExecutionOutput::SingleStage(_))); + assert_eq!(patched_errors.len(), 1, "Only 1 patch expected"); + let commit_id_error = patched_errors.into_iter().next().unwrap(); + assert!(matches!( + commit_id_error, + TransactionStrategyExecutionError::CommitIDError(_, _) + )); + + // Cleanup succeeds + assert!(intent_executor.cleanup().await.is_ok()); let commit_ids_by_pk: HashMap<_, _> = [&committed_account] .iter() .map(|el| { @@ -368,7 +383,6 @@ async fn test_commit_id_error_recovery() { }) .collect(); - tokio::time::sleep(Duration::from_secs(1)).await; verify( &fixture.table_mania, fixture.rpc_client.get_inner(), @@ -385,7 +399,7 @@ async fn test_undelegation_error_recovery() { let TestEnv { fixture, - intent_executor, + mut intent_executor, task_info_fetcher: _, pre_test_tablemania_state, } = TestEnv::setup().await; @@ -406,10 +420,24 @@ async fn test_undelegation_error_recovery() { let res = intent_executor .execute(intent, None::) .await; + let IntentExecutionResult { + inner: res, + patched_errors, + } = res; + assert!(res.is_ok()); assert!(matches!(res.unwrap(), ExecutionOutput::SingleStage(_))); + assert_eq!(patched_errors.len(), 1, "Only 1 patch expected"); - tokio::time::sleep(Duration::from_secs(1)).await; + // Assert errors patched + let undelegation_error = patched_errors.into_iter().next().unwrap(); + assert!(matches!( + undelegation_error, + TransactionStrategyExecutionError::UndelegationError(_, _) + )); + + // Cleanup succeeds + assert!(intent_executor.cleanup().await.is_ok()); verify( &fixture.table_mania, fixture.rpc_client.get_inner(), @@ -426,7 +454,7 @@ async fn test_action_error_recovery() { let TestEnv { fixture, - intent_executor, + mut intent_executor, task_info_fetcher: _, pre_test_tablemania_state, } = TestEnv::setup().await; @@ -455,8 +483,21 @@ async fn test_action_error_recovery() { let res = intent_executor .execute(scheduled_intent, None::) .await; + let IntentExecutionResult { + inner: res, + patched_errors, + } = res; + assert!(res.is_ok()); assert!(matches!(res.unwrap(), ExecutionOutput::SingleStage(_))); + assert_eq!(patched_errors.len(), 1, "Only 1 patch expected"); + + // Assert errors patched + let action_error = patched_errors.into_iter().next().unwrap(); + assert!(matches!( + action_error, + TransactionStrategyExecutionError::ActionsError(_, _) + )); verify_committed_accounts_state( fixture.rpc_client.get_inner(), @@ -478,7 +519,7 @@ async fn test_commit_id_and_action_errors_recovery() { let TestEnv { fixture, - intent_executor, + mut intent_executor, task_info_fetcher, pre_test_tablemania_state, } = TestEnv::setup().await; @@ -511,11 +552,34 @@ async fn test_commit_id_and_action_errors_recovery() { }); let scheduled_intent = create_scheduled_intent(base_intent); + // Execute intent let res = intent_executor .execute(scheduled_intent, None::) .await; + let IntentExecutionResult { + inner: res, + patched_errors, + } = res; + assert!(res.is_ok()); assert!(matches!(res.unwrap(), ExecutionOutput::SingleStage(_))); + assert_eq!(patched_errors.len(), 2, "Only 2 patches expected"); + + // Assert errors patched + let mut iter = patched_errors.into_iter(); + let commit_id_error = iter.next().unwrap(); + let actions_error = iter.next().unwrap(); + assert!(matches!( + commit_id_error, + TransactionStrategyExecutionError::CommitIDError(_, _) + )); + assert!(matches!( + actions_error, + TransactionStrategyExecutionError::ActionsError(_, _) + )); + + // Cleanup succeeds + assert!(intent_executor.cleanup().await.is_ok()); verify_committed_accounts_state( fixture.rpc_client.get_inner(), @@ -538,7 +602,7 @@ async fn test_cpi_limits_error_recovery() { let TestEnv { fixture, - intent_executor, + mut intent_executor, task_info_fetcher, pre_test_tablemania_state, } = TestEnv::setup().await; @@ -566,7 +630,7 @@ async fn test_cpi_limits_error_recovery() { account.data = to_vec(&data).unwrap(); CommittedAccount { pubkey: FlexiCounter::pda(&counter.pubkey()).0, - account: account.clone(), + account, } }) .collect(); @@ -596,7 +660,8 @@ async fn test_cpi_limits_error_recovery() { } )); - tokio::time::sleep(Duration::from_secs(1)).await; + // Cleanup after intent + assert!(intent_executor.cleanup().await.is_ok()); let commit_ids_by_pk: HashMap<_, _> = committed_accounts .iter() .map(|el| { @@ -606,6 +671,7 @@ async fn test_cpi_limits_error_recovery() { ) }) .collect(); + verify( &fixture.table_mania, fixture.rpc_client.get_inner(), @@ -624,7 +690,7 @@ async fn test_commit_id_actions_cpi_limit_errors_recovery() { let TestEnv { fixture, - intent_executor, + mut intent_executor, task_info_fetcher, pre_test_tablemania_state, } = TestEnv::setup().await; @@ -695,12 +761,13 @@ async fn test_commit_id_actions_cpi_limit_errors_recovery() { ) .await; - println!("{:?}", res); + let patched_errors = mem::take(&mut intent_executor.patched_errors); // We expect recovery to succeed by splitting into two stages (commit, then finalize) assert!( res.is_ok(), "Expected recovery from CommitID, Actions, and CpiLimit errors" ); + assert_eq!(patched_errors.len(), 3, "Only 3 patches expected"); assert!(matches!( res.unwrap(), ExecutionOutput::TwoStage { @@ -709,7 +776,26 @@ async fn test_commit_id_actions_cpi_limit_errors_recovery() { } )); - tokio::time::sleep(Duration::from_secs(1)).await; + let mut iter = patched_errors.into_iter(); + let commit_id_error = iter.next().unwrap(); + let cpi_limit_err = iter.next().unwrap(); + let action_error = iter.next().unwrap(); + + assert!(matches!( + commit_id_error, + TransactionStrategyExecutionError::CommitIDError(_, _) + )); + assert!(matches!( + cpi_limit_err, + TransactionStrategyExecutionError::CpiLimitError(_, _) + )); + assert!(matches!( + action_error, + TransactionStrategyExecutionError::ActionsError(_, _) + )); + + // Cleanup after intent + assert!(intent_executor.cleanup().await.is_ok()); let commit_ids_by_pk: HashMap<_, _> = committed_accounts .iter() .map(|el| { From 196861727b8908168204bf9d616e848e0df777f2 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Fri, 21 Nov 2025 10:59:45 +0400 Subject: [PATCH 11/29] fix: tests --- .../programs/flexi-counter/src/processor.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/test-integration/programs/flexi-counter/src/processor.rs b/test-integration/programs/flexi-counter/src/processor.rs index eab3b8f70..febde3a05 100644 --- a/test-integration/programs/flexi-counter/src/processor.rs +++ b/test-integration/programs/flexi-counter/src/processor.rs @@ -387,6 +387,15 @@ fn process_undelegate_request( ProgramError::InvalidArgument })?; + undelegate_account( + delegated_account, + &crate::id(), + buffer, + payer, + system_program, + account_seeds, + )?; + let counter = { let data = delegated_account.data.borrow(); FlexiCounter::deserialize(&mut data.as_ref())? @@ -396,14 +405,6 @@ fn process_undelegate_request( return Err(ProgramError::Custom(FAIL_UNDELEGATION_CODE)); } - undelegate_account( - delegated_account, - &crate::id(), - buffer, - payer, - system_program, - account_seeds, - )?; Ok(()) } From d02cc05a22bff5d3d289d856100cb5676ea25b71 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Wed, 19 Nov 2025 12:48:00 +0400 Subject: [PATCH 12/29] feat: added error message field into SentCommit feat: trigger SentCommit on intent failure and failed retry --- .../src/scheduled_commits_processor.rs | 107 +++++++----------- .../src/intent_executor/error.rs | 27 +++++ .../src/intent_executor/task_info_fetcher.rs | 11 ++ .../src/tasks/task_builder.rs | 10 ++ .../delivery_preparator.rs | 28 ++++- .../src/transaction_preparator/error.rs | 20 +++- .../process_scheduled_commit_sent.rs | 12 ++ 7 files changed, 141 insertions(+), 74 deletions(-) diff --git a/magicblock-accounts/src/scheduled_commits_processor.rs b/magicblock-accounts/src/scheduled_commits_processor.rs index 83a03c064..cdd79c3d0 100644 --- a/magicblock-accounts/src/scheduled_commits_processor.rs +++ b/magicblock-accounts/src/scheduled_commits_processor.rs @@ -16,9 +16,7 @@ use magicblock_chainlink::{ Chainlink, }; use magicblock_committor_service::{ - intent_execution_manager::{ - BroadcastedIntentExecutionResult, ExecutionOutputWrapper, - }, + intent_execution_manager::BroadcastedIntentExecutionResult, intent_executor::ExecutionOutput, types::{ScheduledBaseIntentWrapper, TriggerType}, BaseIntentCommittor, CommittorService, @@ -241,79 +239,56 @@ impl ScheduledCommitsProcessorImpl { continue; }; - match execution_result { - Ok(value) => { - Self::process_intent_result( - intent_id, - &internal_transaction_scheduler, - value, - intent_meta, - ) - .await; - } - Err((_, _, err)) => { - match err.as_ref() { - &magicblock_committor_service::intent_executor::error::IntentExecutorError::EmptyIntentError => { - warn!("Empty intent was scheduled!"); - Self::process_empty_intent( - intent_id, - &internal_transaction_scheduler, - intent_meta - ).await; - } - _ => { - error!( - "Failed to commit in slot: {}, blockhash: {}. {:?}", - intent_meta.slot, intent_meta.blockhash, err - ); - } - } - } - } + Self::process_intent_result( + intent_id, + &internal_transaction_scheduler, + execution_result, + intent_meta, + ) + .await; } } async fn process_intent_result( intent_id: u64, internal_transaction_scheduler: &TransactionSchedulerHandle, - execution_outcome: ExecutionOutputWrapper, + result: BroadcastedIntentExecutionResult, mut intent_meta: ScheduledBaseIntentMeta, ) { - let chain_signatures = match execution_outcome.output { - ExecutionOutput::SingleStage(signature) => vec![signature], - ExecutionOutput::TwoStage { - commit_signature, - finalize_signature, - } => vec![commit_signature, finalize_signature], - }; - let intent_sent_transaction = - std::mem::take(&mut intent_meta.intent_sent_transaction); - let sent_commit = - Self::build_sent_commit(intent_id, chain_signatures, intent_meta); - register_scheduled_commit_sent(sent_commit); - match internal_transaction_scheduler - .execute(intent_sent_transaction) - .await - { - Ok(signature) => debug!( - "Signaled sent commit with internal signature: {:?}", - signature - ), - Err(err) => { - error!("Failed to signal sent commit via transaction: {}", err); + let error_message = result + .as_ref() + .err() + .map(|(_, _, err)| format!("{:?}", err)); + let chain_signatures = match result { + Ok(execution_outcome) => match execution_outcome.output { + ExecutionOutput::SingleStage(signature) => vec![signature], + ExecutionOutput::TwoStage { + commit_signature, + finalize_signature, + } => vec![commit_signature, finalize_signature], + }, + Err((_, _, err)) => { + error!( + "Failed to commit in slot: {}, blockhash: {}. {:?}", + intent_meta.slot, intent_meta.blockhash, err + ); + err.signatures() + .map(|(commit, finalize)| { + finalize + .map(|finalize| vec![commit, finalize]) + .unwrap_or(vec![commit]) + }) + .unwrap_or(vec![]) } - } - } - - async fn process_empty_intent( - intent_id: u64, - internal_transaction_scheduler: &TransactionSchedulerHandle, - mut intent_meta: ScheduledBaseIntentMeta, - ) { + }; let intent_sent_transaction = std::mem::take(&mut intent_meta.intent_sent_transaction); - let sent_commit = - Self::build_sent_commit(intent_id, vec![], intent_meta); + let sent_commit = Self::build_sent_commit( + intent_id, + chain_signatures, + intent_meta, + error_message, + ); register_scheduled_commit_sent(sent_commit); match internal_transaction_scheduler .execute(intent_sent_transaction) @@ -333,6 +308,7 @@ impl ScheduledCommitsProcessorImpl { intent_id: u64, chain_signatures: Vec, intent_meta: ScheduledBaseIntentMeta, + error_message: Option, ) -> SentCommit { SentCommit { message_id: intent_id, @@ -343,6 +319,7 @@ impl ScheduledCommitsProcessorImpl { included_pubkeys: intent_meta.included_pubkeys, excluded_pubkeys: intent_meta.excluded_pubkeys, requested_undelegation: intent_meta.requested_undelegation, + error_message, } } } diff --git a/magicblock-committor-service/src/intent_executor/error.rs b/magicblock-committor-service/src/intent_executor/error.rs index e17eda365..f89a3fbe1 100644 --- a/magicblock-committor-service/src/intent_executor/error.rs +++ b/magicblock-committor-service/src/intent_executor/error.rs @@ -105,6 +105,33 @@ impl IntentExecutorError { } } } + + pub fn signatures(&self) -> Option<(Signature, Option)> { + match self { + IntentExecutorError::CpiLimitError(_, signature) + | IntentExecutorError::ActionsError(_, signature) + | IntentExecutorError::CommitIDError(_, signature) + | IntentExecutorError::UndelegationError(_, signature) + | IntentExecutorError::FailedToCommitError { signature, err: _ } => { + signature.map(|el| (el, None)) + } + IntentExecutorError::FailedCommitPreparationError(err) + | IntentExecutorError::FailedFinalizePreparationError(err) => { + err.signature().map(|el| (el, None)) + } + IntentExecutorError::TaskBuilderError(err) => { + err.signature().map(|el| (el, None)) + } + IntentExecutorError::FailedToFinalizeError { + err: _, + commit_signature, + finalize_signature, + } => commit_signature.map(|el| (el, *finalize_signature)), + IntentExecutorError::EmptyIntentError + | IntentExecutorError::FailedToFitError + | IntentExecutorError::SignerError(_) => None, + } + } } impl metrics::LabelValue for IntentExecutorError { diff --git a/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs b/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs index d76f97c86..15e607c71 100644 --- a/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs +++ b/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs @@ -10,6 +10,7 @@ use log::{error, warn}; use lru::LruCache; use magicblock_rpc_client::{MagicBlockRpcClientError, MagicblockRpcClient}; use solana_pubkey::Pubkey; +use solana_sdk::signature::Signature; const NUM_FETCH_RETRIES: NonZeroUsize = unsafe { NonZeroUsize::new_unchecked(5) }; @@ -274,4 +275,14 @@ pub enum TaskInfoFetcherError { MagicBlockRpcClientError(#[from] MagicBlockRpcClientError), } +impl TaskInfoFetcherError { + pub fn signature(&self) -> Option { + match self { + Self::MetadataNotFoundError(_) => None, + Self::InvalidAccountDataError(_) => None, + Self::MagicBlockRpcClientError(err) => err.signature(), + } + } +} + pub type TaskInfoFetcherResult = Result; diff --git a/magicblock-committor-service/src/tasks/task_builder.rs b/magicblock-committor-service/src/tasks/task_builder.rs index 36c7315eb..076c98b8f 100644 --- a/magicblock-committor-service/src/tasks/task_builder.rs +++ b/magicblock-committor-service/src/tasks/task_builder.rs @@ -7,6 +7,7 @@ use magicblock_program::magic_scheduled_base_intent::{ UndelegateType, }; use solana_pubkey::Pubkey; +use solana_sdk::signature::Signature; use crate::{ intent_executor::task_info_fetcher::{ @@ -206,4 +207,13 @@ pub enum TaskBuilderError { FinalizedTasksBuildError(#[source] TaskInfoFetcherError), } +impl TaskBuilderError { + pub fn signature(&self) -> Option { + match self { + Self::CommitTasksBuildError(err) => err.signature(), + Self::FinalizedTasksBuildError(err) => err.signature(), + } + } +} + pub type TaskBuilderResult = Result; diff --git a/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs b/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs index 13001df0e..e74dce2cf 100644 --- a/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs +++ b/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs @@ -77,8 +77,9 @@ impl DeliveryPreparator { let (res1, res2) = join(task_preparations, alts_preparations).await; res1.into_iter() .collect::, _>>() - .map_err(Error::FailedToPrepareBufferAccounts)?; - let lookup_tables = res2.map_err(Error::FailedToCreateALTError)?; + .map_err(DeliveryPreparatorError::FailedToPrepareBufferAccounts)?; + let lookup_tables = + res2.map_err(DeliveryPreparatorError::FailedToCreateALTError)?; Ok(lookup_tables) } @@ -524,12 +525,31 @@ pub enum InternalError { BaseTaskError(#[from] BaseTaskError), } +impl InternalError { + pub fn signature(&self) -> Option { + match self { + Self::MagicBlockRpcClientError(err) => err.signature(), + _ => None, + } + } +} + #[derive(thiserror::Error, Debug)] -pub enum Error { +pub enum DeliveryPreparatorError { #[error("FailedToPrepareBufferAccounts: {0}")] FailedToPrepareBufferAccounts(#[source] InternalError), #[error("FailedToCreateALTError: {0}")] FailedToCreateALTError(#[source] InternalError), } -pub type DeliveryPreparatorResult = Result; +impl DeliveryPreparatorError { + pub fn signature(&self) -> Option { + match self { + Self::FailedToCreateALTError(err) + | Self::FailedToPrepareBufferAccounts(err) => err.signature(), + } + } +} + +pub type DeliveryPreparatorResult = + Result; diff --git a/magicblock-committor-service/src/transaction_preparator/error.rs b/magicblock-committor-service/src/transaction_preparator/error.rs index 72ceb7380..75772daca 100644 --- a/magicblock-committor-service/src/transaction_preparator/error.rs +++ b/magicblock-committor-service/src/transaction_preparator/error.rs @@ -1,7 +1,10 @@ -use solana_sdk::signer::SignerError; +use solana_sdk::{signature::Signature, signer::SignerError}; use thiserror::Error; -use crate::tasks::task_strategist::TaskStrategistError; +use crate::{ + tasks::task_strategist::TaskStrategistError, + transaction_preparator::delivery_preparator::DeliveryPreparatorError, +}; #[derive(Error, Debug)] pub enum TransactionPreparatorError { @@ -10,9 +13,16 @@ pub enum TransactionPreparatorError { #[error("SignerError: {0}")] SignerError(#[from] SignerError), #[error("DeliveryPreparationError: {0}")] - DeliveryPreparationError( - #[from] crate::transaction_preparator::delivery_preparator::Error, - ), + DeliveryPreparationError(#[from] DeliveryPreparatorError), +} + +impl TransactionPreparatorError { + pub fn signature(&self) -> Option { + match self { + Self::DeliveryPreparationError(err) => err.signature(), + _ => None, + } + } } impl From for TransactionPreparatorError { diff --git a/programs/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rs b/programs/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rs index 7bb293d8a..02712422a 100644 --- a/programs/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rs +++ b/programs/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rs @@ -26,6 +26,7 @@ pub struct SentCommit { pub included_pubkeys: Vec, pub excluded_pubkeys: Vec, pub requested_undelegation: bool, + pub error_message: Option, } /// This is a printable version of the SentCommit struct. @@ -40,6 +41,7 @@ struct SentCommitPrintable { included_pubkeys: String, excluded_pubkeys: String, requested_undelegation: bool, + error_message: Option, } impl From for SentCommitPrintable { @@ -67,6 +69,7 @@ impl From for SentCommitPrintable { .collect::>() .join(", "), requested_undelegation: commit.requested_undelegation, + error_message: commit.error_message, } } } @@ -210,6 +213,14 @@ pub fn process_scheduled_commit_sent( ic_msg!(invoke_context, "ScheduledCommitSent requested undelegation",); } + if let Some(error_message) = commit.error_message { + ic_msg!( + invoke_context, + "ScheduledCommitSent error message: {}", + error_message + ); + } + Ok(()) } @@ -245,6 +256,7 @@ mod tests { included_pubkeys: vec![acc], excluded_pubkeys: Default::default(), requested_undelegation: false, + error_message: None, } } From 98aa18bc80affbcda0666d3432aced45c50ad736 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Fri, 21 Nov 2025 11:15:00 +0400 Subject: [PATCH 13/29] fix: compilation --- magicblock-committor-service/src/intent_executor/error.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/magicblock-committor-service/src/intent_executor/error.rs b/magicblock-committor-service/src/intent_executor/error.rs index 680e471a1..dcf387503 100644 --- a/magicblock-committor-service/src/intent_executor/error.rs +++ b/magicblock-committor-service/src/intent_executor/error.rs @@ -89,11 +89,7 @@ impl IntentExecutorError { pub fn signatures(&self) -> Option<(Signature, Option)> { match self { - IntentExecutorError::CpiLimitError(_, signature) - | IntentExecutorError::ActionsError(_, signature) - | IntentExecutorError::CommitIDError(_, signature) - | IntentExecutorError::UndelegationError(_, signature) - | IntentExecutorError::FailedToCommitError { signature, err: _ } => { + IntentExecutorError::FailedToCommitError { signature, err: _ } => { signature.map(|el| (el, None)) } IntentExecutorError::FailedCommitPreparationError(err) From 21743caba5a67ef0dc76038905691f4d6df0a39e Mon Sep 17 00:00:00 2001 From: taco-paco Date: Fri, 21 Nov 2025 12:22:39 +0400 Subject: [PATCH 14/29] feat: introduced patched errors trace in SentCommit transaction feat: SentCommit fails if underlying intent failed to execute on Base fix: compilation --- .../src/scheduled_commits_processor.rs | 81 +++++++++---------- .../src/intent_execution_manager.rs | 4 +- .../intent_execution_engine.rs | 80 +++++++++--------- .../src/service_ext.rs | 7 +- .../process_scheduled_commit_sent.rs | 21 ++++- 5 files changed, 102 insertions(+), 91 deletions(-) diff --git a/magicblock-accounts/src/scheduled_commits_processor.rs b/magicblock-accounts/src/scheduled_commits_processor.rs index fba6ac664..2f8e3a54e 100644 --- a/magicblock-accounts/src/scheduled_commits_processor.rs +++ b/magicblock-accounts/src/scheduled_commits_processor.rs @@ -28,9 +28,7 @@ use magicblock_program::{ magic_scheduled_base_intent::ScheduledBaseIntent, register_scheduled_commit_sent, SentCommit, TransactionScheduler, }; -use solana_sdk::{ - hash::Hash, pubkey::Pubkey, signature::Signature, transaction::Transaction, -}; +use solana_sdk::{hash::Hash, pubkey::Pubkey, transaction::Transaction}; use tokio::{ sync::{broadcast, oneshot}, task, @@ -209,13 +207,8 @@ impl ScheduledCommitsProcessorImpl { } }; - let (intent_id, trigger_type) = execution_result - .as_ref() - .map(|output| (output.id, output.trigger_type)) - .unwrap_or_else(|(id, trigger_type, _, _)| { - (*id, *trigger_type) - }); - + let intent_id = execution_result.id; + let trigger_type = execution_result.trigger_type; // Here we handle on OnChain triggered intent // TODO: should be removed once crank supported if matches!(trigger_type, TriggerType::OffChain) { @@ -257,40 +250,10 @@ impl ScheduledCommitsProcessorImpl { result: BroadcastedIntentExecutionResult, mut intent_meta: ScheduledBaseIntentMeta, ) { - let error_message = result - .as_ref() - .err() - .map(|(_, _, _, err)| format!("{:?}", err)); - let chain_signatures = match result { - Ok(execution_outcome) => match execution_outcome.output { - ExecutionOutput::SingleStage(signature) => vec![signature], - ExecutionOutput::TwoStage { - commit_signature, - finalize_signature, - } => vec![commit_signature, finalize_signature], - }, - Err((_, _, _, err)) => { - error!( - "Failed to commit in slot: {}, blockhash: {}. {:?}", - intent_meta.slot, intent_meta.blockhash, err - ); - err.signatures() - .map(|(commit, finalize)| { - finalize - .map(|finalize| vec![commit, finalize]) - .unwrap_or(vec![commit]) - }) - .unwrap_or(vec![]) - } - }; let intent_sent_transaction = std::mem::take(&mut intent_meta.intent_sent_transaction); - let sent_commit = Self::build_sent_commit( - intent_id, - chain_signatures, - intent_meta, - error_message, - ); + let sent_commit = + Self::build_sent_commit(intent_id, intent_meta, result); register_scheduled_commit_sent(sent_commit); match internal_transaction_scheduler .execute(intent_sent_transaction) @@ -308,10 +271,39 @@ impl ScheduledCommitsProcessorImpl { fn build_sent_commit( intent_id: u64, - chain_signatures: Vec, intent_meta: ScheduledBaseIntentMeta, - error_message: Option, + result: BroadcastedIntentExecutionResult, ) -> SentCommit { + let error_message = + result.as_ref().err().map(|err| format!("{:?}", err)); + let chain_signatures = match result.inner { + Ok(value) => match value { + ExecutionOutput::SingleStage(signature) => vec![signature], + ExecutionOutput::TwoStage { + commit_signature, + finalize_signature, + } => vec![commit_signature, finalize_signature], + }, + Err(err) => { + error!( + "Failed to commit in slot: {}, blockhash: {}. {:?}", + intent_meta.slot, intent_meta.blockhash, err + ); + err.signatures() + .map(|(commit, finalize)| { + finalize + .map(|finalize| vec![commit, finalize]) + .unwrap_or(vec![commit]) + }) + .unwrap_or(vec![]) + } + }; + let patched_errors = result + .patched_errors + .iter() + .map(|err| err.to_string()) + .collect(); + SentCommit { message_id: intent_id, slot: intent_meta.slot, @@ -322,6 +314,7 @@ impl ScheduledCommitsProcessorImpl { excluded_pubkeys: intent_meta.excluded_pubkeys, requested_undelegation: intent_meta.requested_undelegation, error_message, + patched_errors, } } } diff --git a/magicblock-committor-service/src/intent_execution_manager.rs b/magicblock-committor-service/src/intent_execution_manager.rs index 57b5f377a..5c85354ed 100644 --- a/magicblock-committor-service/src/intent_execution_manager.rs +++ b/magicblock-committor-service/src/intent_execution_manager.rs @@ -4,9 +4,7 @@ pub mod intent_scheduler; use std::sync::Arc; -pub use intent_execution_engine::{ - BroadcastedIntentExecutionResult, ExecutionOutputWrapper, -}; +pub use intent_execution_engine::BroadcastedIntentExecutionResult; use magicblock_rpc_client::MagicblockRpcClient; use magicblock_table_mania::TableMania; use tokio::sync::{broadcast, mpsc, mpsc::error::TrySendError}; diff --git a/magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs b/magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs index 1f65f3974..07f926a67 100644 --- a/magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs +++ b/magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs @@ -1,4 +1,5 @@ use std::{ + ops::Deref, sync::{Arc, Mutex}, time::{Duration, Instant}, }; @@ -26,7 +27,7 @@ use crate::{ TransactionStrategyExecutionError, }, intent_executor_factory::IntentExecutorFactory, - ExecutionOutput, IntentExecutor, + ExecutionOutput, IntentExecutionResult, IntentExecutor, }, persist::IntentPersister, types::{ScheduledBaseIntentWrapper, TriggerType}, @@ -39,22 +40,37 @@ const MAX_EXECUTORS: u8 = 50; pub type PatchedErrors = Vec; #[derive(Clone, Debug)] -pub struct ExecutionOutputWrapper { +pub struct BroadcastedIntentExecutionResult { + pub inner: Result>, pub id: u64, - pub output: ExecutionOutput, - pub patched_errors: Arc, pub trigger_type: TriggerType, + pub patched_errors: Arc, } -pub type BroadcastedError = ( - u64, - TriggerType, - Arc, - Arc, -); +impl BroadcastedIntentExecutionResult { + fn new( + id: u64, + trigger_type: TriggerType, + execution_result: IntentExecutionResult, + ) -> Self { + let inner = execution_result.inner.map_err(Arc::new); + let patched_errors = execution_result.patched_errors.into(); + Self { + id, + trigger_type, + patched_errors, + inner, + } + } +} -pub type BroadcastedIntentExecutionResult = - IntentExecutorResult; +impl Deref for BroadcastedIntentExecutionResult { + type Target = Result>; + + fn deref(&self) -> &Self::Target { + &self.inner + } +} /// Struct that exposes only `subscribe` method of `broadcast::Sender` for better isolation pub struct ResultSubscriber( @@ -257,33 +273,23 @@ where result_sender: broadcast::Sender, ) { let instant = Instant::now(); - let result = executor.execute(intent.inner.clone(), persister).await; + // Execute an Intent + let result = executor.execute(intent.inner.clone(), persister).await; let _ = result.inner.as_ref().inspect_err(|err| { error!("Failed to execute BaseIntent. id: {}. {}", intent.id, err) }); - // Metrics + // Record metrics after execution Self::execution_metrics(instant.elapsed(), &intent, &result.inner); - let patched_errors = Arc::new(result.patched_errors); - let result = match result.inner { - Ok(output) => Ok(ExecutionOutputWrapper { - id: intent.id, - trigger_type: intent.trigger_type, - patched_errors, - output, - }), - Err(err) => Err(( - intent.inner.id, - intent.trigger_type, - patched_errors, - Arc::new(err), - )), - }; - // Broadcast result to subscribers - if let Err(err) = result_sender.send(result) { + let broadcasted_result = BroadcastedIntentExecutionResult::new( + intent.id, + intent.trigger_type, + result, + ); + if let Err(err) = result_sender.send(broadcasted_result) { warn!("No result listeners of intent execution: {}", err); } @@ -366,8 +372,10 @@ mod tests { use async_trait::async_trait; use magicblock_program::magic_scheduled_base_intent::ScheduledBaseIntent; use solana_pubkey::{pubkey, Pubkey}; - use solana_sdk::{signature::Signature, signer::SignerError}; - use solana_sdk::transaction::TransactionError; + use solana_sdk::{ + signature::Signature, signer::SignerError, + transaction::TransactionError, + }; use tokio::{sync::mpsc, time::sleep}; use super::*; @@ -543,7 +551,7 @@ mod tests { let result = result_receiver.recv().await.unwrap(); assert!(result.is_ok()); // Tasks are blocking so will complete sequentially - assert_eq!(result.unwrap().id, completed as u64); + assert_eq!(result.id, completed as u64); completed += 1; } @@ -793,8 +801,8 @@ mod tests { patched_errors: vec![ TransactionStrategyExecutionError::ActionsError( TransactionError::AccountNotFound, - None - ) + None, + ), ], } } else { diff --git a/magicblock-committor-service/src/service_ext.rs b/magicblock-committor-service/src/service_ext.rs index f9329f85c..c0e344ce2 100644 --- a/magicblock-committor-service/src/service_ext.rs +++ b/magicblock-committor-service/src/service_ext.rs @@ -91,15 +91,10 @@ impl CommittorServiceExt { } }; - let id = match &execution_result { - Ok(value) => value.id, - Err(err) => err.0, - }; - let sender = if let Some(sender) = pending_message .lock() .expect(POISONED_MUTEX_MSG) - .remove(&id) + .remove(&execution_result.id) { sender } else { diff --git a/programs/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rs b/programs/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rs index 02712422a..9216800d6 100644 --- a/programs/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rs +++ b/programs/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rs @@ -16,6 +16,9 @@ use crate::{ utils::accounts::get_instruction_pubkey_with_idx, validator, }; +/// Code emmitted if Intent failed to execute on base chain +const INTENT_FAILED_CODE: u32 = 0x7461636F; + #[derive(Default, Debug, Clone)] pub struct SentCommit { pub message_id: u64, @@ -27,6 +30,7 @@ pub struct SentCommit { pub excluded_pubkeys: Vec, pub requested_undelegation: bool, pub error_message: Option, + pub patched_errors: Vec, } /// This is a printable version of the SentCommit struct. @@ -42,6 +46,7 @@ struct SentCommitPrintable { excluded_pubkeys: String, requested_undelegation: bool, error_message: Option, + patched_errors: Vec, } impl From for SentCommitPrintable { @@ -70,6 +75,7 @@ impl From for SentCommitPrintable { .join(", "), requested_undelegation: commit.requested_undelegation, error_message: commit.error_message, + patched_errors: commit.patched_errors, } } } @@ -213,15 +219,26 @@ pub fn process_scheduled_commit_sent( ic_msg!(invoke_context, "ScheduledCommitSent requested undelegation",); } + for (idx, error) in commit.patched_errors.iter().enumerate() { + ic_msg!( + invoke_context, + "ScheduledCommitSent patched error[{}]: {}", + idx, + error + ); + } + if let Some(error_message) = commit.error_message { ic_msg!( invoke_context, "ScheduledCommitSent error message: {}", error_message ); - } - Ok(()) + Err(InstructionError::Custom(INTENT_FAILED_CODE)) + } else { + Ok(()) + } } #[cfg(test)] From 6909e0e0c59e134bc415b016cead1df6fca126cb Mon Sep 17 00:00:00 2001 From: taco-paco Date: Fri, 21 Nov 2025 12:23:29 +0400 Subject: [PATCH 15/29] fix: typo --- .../src/schedule_transactions/process_scheduled_commit_sent.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rs b/programs/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rs index 9216800d6..205faf509 100644 --- a/programs/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rs +++ b/programs/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rs @@ -16,7 +16,7 @@ use crate::{ utils::accounts::get_instruction_pubkey_with_idx, validator, }; -/// Code emmitted if Intent failed to execute on base chain +/// Code emitted if Intent failed to execute on base chain const INTENT_FAILED_CODE: u32 = 0x7461636F; #[derive(Default, Debug, Clone)] From b319f3d44d8d69f11118f7d08bcd7d7a1759d7ec Mon Sep 17 00:00:00 2001 From: taco-paco Date: Fri, 21 Nov 2025 13:41:15 +0400 Subject: [PATCH 16/29] fix: tests --- .../test-committor-service/tests/test_intent_executor.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 c58ebfabf..ccc3dfc4c 100644 --- a/test-integration/test-committor-service/tests/test_intent_executor.rs +++ b/test-integration/test-committor-service/tests/test_intent_executor.rs @@ -161,7 +161,7 @@ async fn test_commit_id_error_parsing() { #[tokio::test] async fn test_undelegation_error_parsing() { const COUNTER_SIZE: u64 = 70; - const EXPECTED_ERR_MSG: &str = "Invalid undelegation: Error processing Instruction 4: Failed to serialize or deserialize account data"; + const EXPECTED_ERR_MSG: &str = "Invalid undelegation: Error processing Instruction 4: custom program error: 0x7a."; let TestEnv { fixture, @@ -200,6 +200,7 @@ async fn test_undelegation_error_parsing() { let execution_result = execution_result.unwrap(); assert!(execution_result.is_err()); let err = execution_result.unwrap_err(); + println!("{}", err.to_string()); assert!(matches!( err, TransactionStrategyExecutionError::UndelegationError(_, _) From 438c527943509174cf0280540e0afc232e23e002 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Fri, 21 Nov 2025 14:08:23 +0400 Subject: [PATCH 17/29] fix: tests --- .../programs/flexi-counter/src/processor.rs | 12 ++++++------ .../tests/test_intent_executor.rs | 1 - 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/test-integration/programs/flexi-counter/src/processor.rs b/test-integration/programs/flexi-counter/src/processor.rs index febde3a05..aa6b9f993 100644 --- a/test-integration/programs/flexi-counter/src/processor.rs +++ b/test-integration/programs/flexi-counter/src/processor.rs @@ -396,15 +396,15 @@ fn process_undelegate_request( account_seeds, )?; - let counter = { + { let data = delegated_account.data.borrow(); - FlexiCounter::deserialize(&mut data.as_ref())? + if let Ok(counter) = FlexiCounter::deserialize(&mut data.as_ref()) { + if counter.label == FAIL_UNDELEGATION_LABEL { + return Err(ProgramError::Custom(FAIL_UNDELEGATION_CODE)); + } + } }; - if counter.label == FAIL_UNDELEGATION_LABEL { - return Err(ProgramError::Custom(FAIL_UNDELEGATION_CODE)); - } - Ok(()) } 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 ccc3dfc4c..5d78507d3 100644 --- a/test-integration/test-committor-service/tests/test_intent_executor.rs +++ b/test-integration/test-committor-service/tests/test_intent_executor.rs @@ -200,7 +200,6 @@ async fn test_undelegation_error_parsing() { let execution_result = execution_result.unwrap(); assert!(execution_result.is_err()); let err = execution_result.unwrap_err(); - println!("{}", err.to_string()); assert!(matches!( err, TransactionStrategyExecutionError::UndelegationError(_, _) From e4f241d7400fa55c277ca471856877c6eaf8faf4 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Wed, 19 Nov 2025 12:48:00 +0400 Subject: [PATCH 18/29] feat: added error message field into SentCommit feat: trigger SentCommit on intent failure and failed retry --- .../src/scheduled_commits_processor.rs | 107 +++++++----------- .../src/intent_executor/error.rs | 27 +++++ .../src/intent_executor/task_info_fetcher.rs | 11 ++ .../src/tasks/task_builder.rs | 10 ++ .../delivery_preparator.rs | 28 ++++- .../src/transaction_preparator/error.rs | 20 +++- .../process_scheduled_commit_sent.rs | 12 ++ 7 files changed, 141 insertions(+), 74 deletions(-) diff --git a/magicblock-accounts/src/scheduled_commits_processor.rs b/magicblock-accounts/src/scheduled_commits_processor.rs index 83a03c064..cdd79c3d0 100644 --- a/magicblock-accounts/src/scheduled_commits_processor.rs +++ b/magicblock-accounts/src/scheduled_commits_processor.rs @@ -16,9 +16,7 @@ use magicblock_chainlink::{ Chainlink, }; use magicblock_committor_service::{ - intent_execution_manager::{ - BroadcastedIntentExecutionResult, ExecutionOutputWrapper, - }, + intent_execution_manager::BroadcastedIntentExecutionResult, intent_executor::ExecutionOutput, types::{ScheduledBaseIntentWrapper, TriggerType}, BaseIntentCommittor, CommittorService, @@ -241,79 +239,56 @@ impl ScheduledCommitsProcessorImpl { continue; }; - match execution_result { - Ok(value) => { - Self::process_intent_result( - intent_id, - &internal_transaction_scheduler, - value, - intent_meta, - ) - .await; - } - Err((_, _, err)) => { - match err.as_ref() { - &magicblock_committor_service::intent_executor::error::IntentExecutorError::EmptyIntentError => { - warn!("Empty intent was scheduled!"); - Self::process_empty_intent( - intent_id, - &internal_transaction_scheduler, - intent_meta - ).await; - } - _ => { - error!( - "Failed to commit in slot: {}, blockhash: {}. {:?}", - intent_meta.slot, intent_meta.blockhash, err - ); - } - } - } - } + Self::process_intent_result( + intent_id, + &internal_transaction_scheduler, + execution_result, + intent_meta, + ) + .await; } } async fn process_intent_result( intent_id: u64, internal_transaction_scheduler: &TransactionSchedulerHandle, - execution_outcome: ExecutionOutputWrapper, + result: BroadcastedIntentExecutionResult, mut intent_meta: ScheduledBaseIntentMeta, ) { - let chain_signatures = match execution_outcome.output { - ExecutionOutput::SingleStage(signature) => vec![signature], - ExecutionOutput::TwoStage { - commit_signature, - finalize_signature, - } => vec![commit_signature, finalize_signature], - }; - let intent_sent_transaction = - std::mem::take(&mut intent_meta.intent_sent_transaction); - let sent_commit = - Self::build_sent_commit(intent_id, chain_signatures, intent_meta); - register_scheduled_commit_sent(sent_commit); - match internal_transaction_scheduler - .execute(intent_sent_transaction) - .await - { - Ok(signature) => debug!( - "Signaled sent commit with internal signature: {:?}", - signature - ), - Err(err) => { - error!("Failed to signal sent commit via transaction: {}", err); + let error_message = result + .as_ref() + .err() + .map(|(_, _, err)| format!("{:?}", err)); + let chain_signatures = match result { + Ok(execution_outcome) => match execution_outcome.output { + ExecutionOutput::SingleStage(signature) => vec![signature], + ExecutionOutput::TwoStage { + commit_signature, + finalize_signature, + } => vec![commit_signature, finalize_signature], + }, + Err((_, _, err)) => { + error!( + "Failed to commit in slot: {}, blockhash: {}. {:?}", + intent_meta.slot, intent_meta.blockhash, err + ); + err.signatures() + .map(|(commit, finalize)| { + finalize + .map(|finalize| vec![commit, finalize]) + .unwrap_or(vec![commit]) + }) + .unwrap_or(vec![]) } - } - } - - async fn process_empty_intent( - intent_id: u64, - internal_transaction_scheduler: &TransactionSchedulerHandle, - mut intent_meta: ScheduledBaseIntentMeta, - ) { + }; let intent_sent_transaction = std::mem::take(&mut intent_meta.intent_sent_transaction); - let sent_commit = - Self::build_sent_commit(intent_id, vec![], intent_meta); + let sent_commit = Self::build_sent_commit( + intent_id, + chain_signatures, + intent_meta, + error_message, + ); register_scheduled_commit_sent(sent_commit); match internal_transaction_scheduler .execute(intent_sent_transaction) @@ -333,6 +308,7 @@ impl ScheduledCommitsProcessorImpl { intent_id: u64, chain_signatures: Vec, intent_meta: ScheduledBaseIntentMeta, + error_message: Option, ) -> SentCommit { SentCommit { message_id: intent_id, @@ -343,6 +319,7 @@ impl ScheduledCommitsProcessorImpl { included_pubkeys: intent_meta.included_pubkeys, excluded_pubkeys: intent_meta.excluded_pubkeys, requested_undelegation: intent_meta.requested_undelegation, + error_message, } } } diff --git a/magicblock-committor-service/src/intent_executor/error.rs b/magicblock-committor-service/src/intent_executor/error.rs index e17eda365..f89a3fbe1 100644 --- a/magicblock-committor-service/src/intent_executor/error.rs +++ b/magicblock-committor-service/src/intent_executor/error.rs @@ -105,6 +105,33 @@ impl IntentExecutorError { } } } + + pub fn signatures(&self) -> Option<(Signature, Option)> { + match self { + IntentExecutorError::CpiLimitError(_, signature) + | IntentExecutorError::ActionsError(_, signature) + | IntentExecutorError::CommitIDError(_, signature) + | IntentExecutorError::UndelegationError(_, signature) + | IntentExecutorError::FailedToCommitError { signature, err: _ } => { + signature.map(|el| (el, None)) + } + IntentExecutorError::FailedCommitPreparationError(err) + | IntentExecutorError::FailedFinalizePreparationError(err) => { + err.signature().map(|el| (el, None)) + } + IntentExecutorError::TaskBuilderError(err) => { + err.signature().map(|el| (el, None)) + } + IntentExecutorError::FailedToFinalizeError { + err: _, + commit_signature, + finalize_signature, + } => commit_signature.map(|el| (el, *finalize_signature)), + IntentExecutorError::EmptyIntentError + | IntentExecutorError::FailedToFitError + | IntentExecutorError::SignerError(_) => None, + } + } } impl metrics::LabelValue for IntentExecutorError { diff --git a/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs b/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs index d76f97c86..15e607c71 100644 --- a/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs +++ b/magicblock-committor-service/src/intent_executor/task_info_fetcher.rs @@ -10,6 +10,7 @@ use log::{error, warn}; use lru::LruCache; use magicblock_rpc_client::{MagicBlockRpcClientError, MagicblockRpcClient}; use solana_pubkey::Pubkey; +use solana_sdk::signature::Signature; const NUM_FETCH_RETRIES: NonZeroUsize = unsafe { NonZeroUsize::new_unchecked(5) }; @@ -274,4 +275,14 @@ pub enum TaskInfoFetcherError { MagicBlockRpcClientError(#[from] MagicBlockRpcClientError), } +impl TaskInfoFetcherError { + pub fn signature(&self) -> Option { + match self { + Self::MetadataNotFoundError(_) => None, + Self::InvalidAccountDataError(_) => None, + Self::MagicBlockRpcClientError(err) => err.signature(), + } + } +} + pub type TaskInfoFetcherResult = Result; diff --git a/magicblock-committor-service/src/tasks/task_builder.rs b/magicblock-committor-service/src/tasks/task_builder.rs index 36c7315eb..076c98b8f 100644 --- a/magicblock-committor-service/src/tasks/task_builder.rs +++ b/magicblock-committor-service/src/tasks/task_builder.rs @@ -7,6 +7,7 @@ use magicblock_program::magic_scheduled_base_intent::{ UndelegateType, }; use solana_pubkey::Pubkey; +use solana_sdk::signature::Signature; use crate::{ intent_executor::task_info_fetcher::{ @@ -206,4 +207,13 @@ pub enum TaskBuilderError { FinalizedTasksBuildError(#[source] TaskInfoFetcherError), } +impl TaskBuilderError { + pub fn signature(&self) -> Option { + match self { + Self::CommitTasksBuildError(err) => err.signature(), + Self::FinalizedTasksBuildError(err) => err.signature(), + } + } +} + pub type TaskBuilderResult = Result; diff --git a/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs b/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs index 13001df0e..e74dce2cf 100644 --- a/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs +++ b/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs @@ -77,8 +77,9 @@ impl DeliveryPreparator { let (res1, res2) = join(task_preparations, alts_preparations).await; res1.into_iter() .collect::, _>>() - .map_err(Error::FailedToPrepareBufferAccounts)?; - let lookup_tables = res2.map_err(Error::FailedToCreateALTError)?; + .map_err(DeliveryPreparatorError::FailedToPrepareBufferAccounts)?; + let lookup_tables = + res2.map_err(DeliveryPreparatorError::FailedToCreateALTError)?; Ok(lookup_tables) } @@ -524,12 +525,31 @@ pub enum InternalError { BaseTaskError(#[from] BaseTaskError), } +impl InternalError { + pub fn signature(&self) -> Option { + match self { + Self::MagicBlockRpcClientError(err) => err.signature(), + _ => None, + } + } +} + #[derive(thiserror::Error, Debug)] -pub enum Error { +pub enum DeliveryPreparatorError { #[error("FailedToPrepareBufferAccounts: {0}")] FailedToPrepareBufferAccounts(#[source] InternalError), #[error("FailedToCreateALTError: {0}")] FailedToCreateALTError(#[source] InternalError), } -pub type DeliveryPreparatorResult = Result; +impl DeliveryPreparatorError { + pub fn signature(&self) -> Option { + match self { + Self::FailedToCreateALTError(err) + | Self::FailedToPrepareBufferAccounts(err) => err.signature(), + } + } +} + +pub type DeliveryPreparatorResult = + Result; diff --git a/magicblock-committor-service/src/transaction_preparator/error.rs b/magicblock-committor-service/src/transaction_preparator/error.rs index 72ceb7380..75772daca 100644 --- a/magicblock-committor-service/src/transaction_preparator/error.rs +++ b/magicblock-committor-service/src/transaction_preparator/error.rs @@ -1,7 +1,10 @@ -use solana_sdk::signer::SignerError; +use solana_sdk::{signature::Signature, signer::SignerError}; use thiserror::Error; -use crate::tasks::task_strategist::TaskStrategistError; +use crate::{ + tasks::task_strategist::TaskStrategistError, + transaction_preparator::delivery_preparator::DeliveryPreparatorError, +}; #[derive(Error, Debug)] pub enum TransactionPreparatorError { @@ -10,9 +13,16 @@ pub enum TransactionPreparatorError { #[error("SignerError: {0}")] SignerError(#[from] SignerError), #[error("DeliveryPreparationError: {0}")] - DeliveryPreparationError( - #[from] crate::transaction_preparator::delivery_preparator::Error, - ), + DeliveryPreparationError(#[from] DeliveryPreparatorError), +} + +impl TransactionPreparatorError { + pub fn signature(&self) -> Option { + match self { + Self::DeliveryPreparationError(err) => err.signature(), + _ => None, + } + } } impl From for TransactionPreparatorError { diff --git a/programs/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rs b/programs/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rs index 7bb293d8a..02712422a 100644 --- a/programs/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rs +++ b/programs/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rs @@ -26,6 +26,7 @@ pub struct SentCommit { pub included_pubkeys: Vec, pub excluded_pubkeys: Vec, pub requested_undelegation: bool, + pub error_message: Option, } /// This is a printable version of the SentCommit struct. @@ -40,6 +41,7 @@ struct SentCommitPrintable { included_pubkeys: String, excluded_pubkeys: String, requested_undelegation: bool, + error_message: Option, } impl From for SentCommitPrintable { @@ -67,6 +69,7 @@ impl From for SentCommitPrintable { .collect::>() .join(", "), requested_undelegation: commit.requested_undelegation, + error_message: commit.error_message, } } } @@ -210,6 +213,14 @@ pub fn process_scheduled_commit_sent( ic_msg!(invoke_context, "ScheduledCommitSent requested undelegation",); } + if let Some(error_message) = commit.error_message { + ic_msg!( + invoke_context, + "ScheduledCommitSent error message: {}", + error_message + ); + } + Ok(()) } @@ -245,6 +256,7 @@ mod tests { included_pubkeys: vec![acc], excluded_pubkeys: Default::default(), requested_undelegation: false, + error_message: None, } } From 3c3b39837a08f9d173b9a5417dc1d7fc0217724b Mon Sep 17 00:00:00 2001 From: taco-paco Date: Fri, 21 Nov 2025 14:37:33 +0400 Subject: [PATCH 19/29] refactor: add extra logging + fix coderabbit --- magicblock-accounts/src/scheduled_commits_processor.rs | 5 ++++- magicblock-committor-service/src/intent_executor/error.rs | 4 ++-- .../src/intent_executor/two_stage_executor.rs | 4 ++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/magicblock-accounts/src/scheduled_commits_processor.rs b/magicblock-accounts/src/scheduled_commits_processor.rs index 2f8e3a54e..4e1476152 100644 --- a/magicblock-accounts/src/scheduled_commits_processor.rs +++ b/magicblock-accounts/src/scheduled_commits_processor.rs @@ -301,7 +301,10 @@ impl ScheduledCommitsProcessorImpl { let patched_errors = result .patched_errors .iter() - .map(|err| err.to_string()) + .map(|err| { + info!("Patched intent, error was: {}", err); + err.to_string() + }) .collect(); SentCommit { diff --git a/magicblock-committor-service/src/intent_executor/error.rs b/magicblock-committor-service/src/intent_executor/error.rs index dcf387503..f47249598 100644 --- a/magicblock-committor-service/src/intent_executor/error.rs +++ b/magicblock-committor-service/src/intent_executor/error.rs @@ -65,7 +65,7 @@ pub enum IntentExecutorError { } impl IntentExecutorError { - pub fn from_commmit_execution_error( + pub fn from_commit_execution_error( error: TransactionStrategyExecutionError, ) -> IntentExecutorError { let signature = error.signature(); @@ -150,7 +150,7 @@ impl From for TransactionStrategyExecutionError { impl TransactionStrategyExecutionError { pub fn is_cpi_limit_error(&self) -> bool { - matches!(self, Self::CommitIDError(_, _)) + matches!(self, Self::CpiLimitError(_, _)) } pub fn signature(&self) -> Option { 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 5295237c0..cf919a28e 100644 --- a/magicblock-committor-service/src/intent_executor/two_stage_executor.rs +++ b/magicblock-committor-service/src/intent_executor/two_stage_executor.rs @@ -105,7 +105,7 @@ where }; let commit_signature = commit_result.map_err(|err| { - IntentExecutorError::from_commmit_execution_error(err) + IntentExecutorError::from_commit_execution_error(err) })?; Ok(commit_signature) @@ -228,7 +228,7 @@ where match err { TransactionStrategyExecutionError::CommitIDError(_, _) => { // Unexpected error in Two Stage commit - error!("Unexpected error in Two stage commit flow: {}", err); + error!("Unexpected error in two stage commit flow: {}", err); Ok(ControlFlow::Break(())) } TransactionStrategyExecutionError::ActionsError(_, _) => { From e01bc794d04a444ae27eaaed833ff01cfe132f21 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Fri, 21 Nov 2025 18:09:39 +0400 Subject: [PATCH 20/29] refactor: executors --- .../intent_execution_engine.rs | 23 +- .../src/intent_executor/mod.rs | 64 +----- .../intent_executor/single_stage_executor.rs | 45 ++-- .../src/intent_executor/two_stage_executor.rs | 200 +++++++++++------- .../src/stubs/changeset_committor_stub.rs | 22 +- .../process_scheduled_commit_sent.rs | 1 + 6 files changed, 163 insertions(+), 192 deletions(-) diff --git a/magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs b/magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs index 07f926a67..8a77ebdaf 100644 --- a/magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs +++ b/magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs @@ -440,8 +440,7 @@ mod tests { // Verify the message was processed let result = result_receiver.recv().await.unwrap(); assert!(result.is_ok()); - let output = result.unwrap(); - assert_eq!(output.id, 1); + assert_eq!(result.id, 1); } #[tokio::test] @@ -461,12 +460,12 @@ mod tests { // First message should be processed immediately let result1 = result_receiver.recv().await.unwrap(); assert!(result1.is_ok()); - assert_eq!(result1.unwrap().id, 1); + assert_eq!(result1.id, 1); // Second message should be processed after first completes let result2 = result_receiver.recv().await.unwrap(); assert!(result2.is_ok()); - assert_eq!(result2.unwrap().id, 2); + assert_eq!(result2.id, 2); } #[tokio::test] @@ -484,17 +483,15 @@ mod tests { // Verify the failure was properly reported let result = result_receiver.recv().await.unwrap(); - let Err((id, trigger_type, patched_errors, err)) = result else { - panic!(); - }; - assert_eq!(id, 1); - assert_eq!(trigger_type, TriggerType::OffChain); + assert!(result.inner.is_err()); + assert_eq!(result.id, 1); + assert_eq!(result.trigger_type, TriggerType::OffChain); assert_eq!( - patched_errors[0].to_string(), + result.patched_errors[0].to_string(), "User supplied actions are ill-formed: Attempt to debit an account but found no record of a prior credit.. None" ); assert_eq!( - err.to_string(), + result.inner.unwrap_err().to_string(), "FailedToCommitError: InternalError: SignerError: custom error: oops" ); } @@ -517,7 +514,7 @@ mod tests { // Verify the message from DB was processed let result = result_receiver.recv().await.unwrap(); assert!(result.is_ok()); - assert_eq!(result.unwrap().id, 1); + assert_eq!(result.id, 1); } /// Tests multiple blocking messages being sent at the same time @@ -619,7 +616,7 @@ mod tests { assert!(result.is_ok()); // Message has to be present in set - let id = result.unwrap().id; + let id = result.id; assert!(received_ids.remove(&id)); completed += 1; diff --git a/magicblock-committor-service/src/intent_executor/mod.rs b/magicblock-committor-service/src/intent_executor/mod.rs index 5500b443c..522b4073b 100644 --- a/magicblock-committor-service/src/intent_executor/mod.rs +++ b/magicblock-committor-service/src/intent_executor/mod.rs @@ -282,26 +282,14 @@ where let committed_pubkeys = base_intent.get_committed_pubkeys(); let res = single_stage_executor - .execute( - committed_pubkeys.as_ref().map(|el| el.as_slice()), - persister, - ) + .execute(committed_pubkeys.as_deref(), persister) .await; - // After execution is complete record junk & patched errors - let SingleStageExecutor { - inner: _, - transaction_strategy, - junk, - patched_errors, - } = single_stage_executor; - self.junk.extend(junk.into_iter()); - self.patched_errors.extend(patched_errors.into_iter()); - // Here we continue only IF the error is CpiLimitError // We can recover that Error by splitting execution // in 2 stages - commit & finalize // Otherwise we return error + let transaction_strategy = single_stage_executor.transaction_strategy; let execution_err = match res { Err(IntentExecutorError::FailedToFinalizeError { err: @@ -341,56 +329,20 @@ where finalize_strategy: TransactionStrategy, persister: &Option

, ) -> IntentExecutorResult { - // TODO: implement custom drop where we flush junk Into Exector? - let mut two_stage_executor = + let two_stage_executor = TwoStageExecutor::new(self, commit_strategy, finalize_strategy); // Execute commit stage - let commit_signature = two_stage_executor + let committed_stage = two_stage_executor .commit(committed_pubkeys, persister) - .await; - let commit_signature = match commit_signature { - Ok(signature) => signature, - Err(err) => { - let TwoStageExecutor { - inner: _, - finalize_strategy: _, - commit_strategy, - junk, - patched_errors, - } = two_stage_executor; - self.junk.extend(junk.into_iter()); - self.junk.push(commit_strategy); - self.patched_errors.extend(patched_errors.into_iter()); - - return Err(err); - } - }; + .await?; // Execute finalize stage - let finalize_result = two_stage_executor - .finalize(commit_signature, persister) - .await; + let finalized_stage = committed_stage.finalize(persister).await?; - // Dissasemble executor to take all the goodies - // After execution ended - everything becomes junk - // need to dispose it - let TwoStageExecutor { - inner: _, - commit_strategy, - finalize_strategy, - junk, - patched_errors, - } = two_stage_executor; - self.junk.extend(junk.into_iter()); - self.junk.push(commit_strategy); - self.junk.push(finalize_strategy); - self.patched_errors.extend(patched_errors.into_iter()); - - let finalize_signature = finalize_result?; Ok(ExecutionOutput::TwoStage { - commit_signature, - finalize_signature, + commit_signature: finalized_stage.state.commit_signature, + finalize_signature: finalized_stage.state.finalize_signature, }) } 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 624b702e2..44edac30f 100644 --- a/magicblock-committor-service/src/intent_executor/single_stage_executor.rs +++ b/magicblock-committor-service/src/intent_executor/single_stage_executor.rs @@ -1,4 +1,4 @@ -use std::ops::{ControlFlow, Deref}; +use std::ops::ControlFlow; use log::error; use solana_pubkey::Pubkey; @@ -18,13 +18,8 @@ use crate::{ }; pub struct SingleStageExecutor<'a, T, F> { - pub(in crate::intent_executor) inner: &'a IntentExecutorImpl, + pub(in crate::intent_executor) inner: &'a mut IntentExecutorImpl, pub transaction_strategy: TransactionStrategy, - - /// Junk that needs to be cleaned up - pub junk: Vec, - /// Errors we patched trying to recover intent - pub patched_errors: Vec, } impl<'a, T, F> SingleStageExecutor<'a, T, F> @@ -33,14 +28,12 @@ where F: TaskInfoFetcher, { pub fn new( - executor: &'a IntentExecutorImpl, + executor: &'a mut IntentExecutorImpl, transaction_strategy: TransactionStrategy, ) -> Self { Self { inner: executor, transaction_strategy, - junk: vec![], - patched_errors: vec![], } } @@ -52,7 +45,7 @@ where const RECURSION_CEILING: u8 = 10; let mut i = 0; - let execution_err = loop { + let result = loop { i += 1; // Prepare & execute message @@ -69,7 +62,7 @@ where let execution_err = match execution_result { // break with result, strategy that was executed at this point has to be returned for cleanup Ok(value) => { - return Ok(ExecutionOutput::SingleStage(value)); + break Ok(ExecutionOutput::SingleStage(value)); } Err(err) => err, }; @@ -85,26 +78,28 @@ where let cleanup = match flow { ControlFlow::Continue(cleanup) => cleanup, ControlFlow::Break(()) => { - break execution_err; + break Err(execution_err); } }; - self.junk.push(cleanup); + self.inner.junk.push(cleanup); if i >= RECURSION_CEILING { error!( "CRITICAL! Recursion ceiling reached in intent execution." ); - break execution_err; + break Err(execution_err); } else { - self.patched_errors.push(execution_err); + self.inner.patched_errors.push(execution_err); } }; - Err(IntentExecutorError::from_finalize_execution_error( - execution_err, - // TODO(edwin): shall one stage have same signature for commit & finalize - None, - )) + result.map_err(|err| { + IntentExecutorError::from_finalize_execution_error( + err, + // TODO(edwin): shall one stage have same signature for commit & finalize + None, + ) + }) } /// Patch the current `transaction_strategy` in response to a recoverable @@ -163,11 +158,3 @@ where } } } - -impl<'a, T, F> Deref for SingleStageExecutor<'a, T, F> { - type Target = IntentExecutorImpl; - - fn deref(&self) -> &'a Self::Target { - self.inner - } -} 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 cf919a28e..85546a68e 100644 --- a/magicblock-committor-service/src/intent_executor/two_stage_executor.rs +++ b/magicblock-committor-service/src/intent_executor/two_stage_executor.rs @@ -1,4 +1,4 @@ -use std::ops::{ControlFlow, Deref}; +use std::ops::ControlFlow; use log::{error, info, warn}; use solana_pubkey::Pubkey; @@ -11,6 +11,7 @@ use crate::{ TransactionStrategyExecutionError, }, task_info_fetcher::TaskInfoFetcher, + two_stage_executor::sealed::Sealed, IntentExecutorImpl, }, persist::IntentPersister, @@ -18,20 +19,33 @@ use crate::{ transaction_preparator::TransactionPreparator, }; -// TODO(edwin): Could be splitted into 2 States -// TwoStageExecutor & TwoStageExecutorCommit -pub struct TwoStageExecutor<'a, T, F> { - pub(in crate::intent_executor) inner: &'a IntentExecutorImpl, +pub struct Initialized { + /// Commit stage strategy pub commit_strategy: TransactionStrategy, + /// Finalize stage strategy pub finalize_strategy: TransactionStrategy, +} + +pub struct Committed { + /// Signature of commit stage + pub commit_signature: Signature, + /// Finalize stage strategy + pub finalize_strategy: TransactionStrategy, +} - /// Junk that needs to be cleaned up - pub junk: Vec, - /// Errors we patched trying to recover intent - pub patched_errors: Vec, +pub struct Finalized { + /// Signature of commit stage + pub commit_signature: Signature, + /// Signature of finalize stage + pub finalize_signature: Signature, } -impl<'a, T, F> TwoStageExecutor<'a, T, F> +pub struct TwoStageExecutor<'a, T, F, S: Sealed> { + pub(in crate::intent_executor) inner: &'a mut IntentExecutorImpl, + pub state: S, +} + +impl<'a, T, F> TwoStageExecutor<'a, T, F, Initialized> where T: TransactionPreparator, F: TaskInfoFetcher, @@ -39,25 +53,24 @@ where const RECURSION_CEILING: u8 = 10; pub fn new( - executor: &'a IntentExecutorImpl, + executor: &'a mut IntentExecutorImpl, commit_strategy: TransactionStrategy, finalize_strategy: TransactionStrategy, ) -> Self { Self { inner: executor, - commit_strategy, - finalize_strategy, - - junk: vec![], - patched_errors: vec![], + state: Initialized { + commit_strategy, + finalize_strategy, + }, } } pub async fn commit( - &mut self, + mut self, committed_pubkeys: &[Pubkey], persister: &Option

, - ) -> IntentExecutorResult { + ) -> IntentExecutorResult> { let mut i = 0; let commit_result = loop { i += 1; @@ -66,7 +79,7 @@ where let execution_result = self .inner .prepare_and_execute_strategy( - &mut self.commit_strategy, + &mut self.state.commit_strategy, persister, ) .await @@ -79,7 +92,7 @@ where let flow = Self::patch_commit_strategy( self.inner, &execution_err, - &mut self.commit_strategy, + &mut self.state.commit_strategy, committed_pubkeys, ) .await?; @@ -92,7 +105,7 @@ where break Err(execution_err); } }; - self.junk.push(cleanup); + self.inner.junk.push(cleanup); if i >= Self::RECURSION_CEILING { error!( @@ -100,22 +113,82 @@ where ); break Err(execution_err); } else { - self.patched_errors.push(execution_err); + self.inner.patched_errors.push(execution_err); } }; + // Even if failed - dump commit into junk + self.inner.junk.push(self.state.commit_strategy); let commit_signature = commit_result.map_err(|err| { IntentExecutorError::from_commit_execution_error(err) })?; - Ok(commit_signature) + let state = Committed { + commit_signature, + finalize_strategy: self.state.finalize_strategy, + }; + Ok(TwoStageExecutor { + inner: self.inner, + state, + }) } + /// Patches Commit stage `transaction_strategy` in response to a recoverable + /// [`TransactionStrategyExecutionError`], optionally preparing cleanup data + /// to be applied after a retry. + /// + /// [`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( + inner: &IntentExecutorImpl, + err: &TransactionStrategyExecutionError, + commit_strategy: &mut TransactionStrategy, + committed_pubkeys: &[Pubkey], + ) -> IntentExecutorResult> { + match err { + TransactionStrategyExecutionError::CommitIDError(_, _) => { + let to_cleanup = inner + .handle_commit_id_error(committed_pubkeys, commit_strategy) + .await?; + Ok(ControlFlow::Continue(to_cleanup)) + } + TransactionStrategyExecutionError::ActionsError(_, _) => { + // Unexpected in Two Stage commit + // That would mean that Two Stage executes Standalone commit + error!("Unexpected error in two stage commit flow: {}", err); + Ok(ControlFlow::Break(())) + } + TransactionStrategyExecutionError::UndelegationError(_, _) => { + // Unexpected in Two Stage commit + // That would mean that Two Stage executes undelegation in commit phase + error!("Unexpected error in two stage commit flow: {}", err); + Ok(ControlFlow::Break(())) + } + TransactionStrategyExecutionError::CpiLimitError(_, _) => { + // Can't be handled + error!("Commit tasks exceeded CpiLimitError: {}", err); + Ok(ControlFlow::Break(())) + } + TransactionStrategyExecutionError::InternalError(_) => { + // Can't be handled + Ok(ControlFlow::Break(())) + } + } + } +} + +impl<'a, T, F> TwoStageExecutor<'a, T, F, Committed> +where + T: TransactionPreparator, + F: TaskInfoFetcher, +{ + const RECURSION_CEILING: u8 = 10; + pub async fn finalize( - &mut self, - commit_signature: Signature, + mut self, persister: &Option

, - ) -> IntentExecutorResult { + ) -> IntentExecutorResult> { let mut i = 0; let finalize_result = loop { i += 1; @@ -124,7 +197,7 @@ where let execution_result = self .inner .prepare_and_execute_strategy( - &mut self.finalize_strategy, + &mut self.state.finalize_strategy, persister, ) .await @@ -137,7 +210,7 @@ where let flow = Self::patch_finalize_strategy( self.inner, &execution_err, - &mut self.finalize_strategy, + &mut self.state.finalize_strategy, ) .await?; @@ -147,7 +220,7 @@ where break Err(execution_err); } }; - self.junk.push(cleanup); + self.inner.junk.push(cleanup); if i >= Self::RECURSION_CEILING { error!( @@ -155,62 +228,27 @@ where ); break Err(execution_err); } else { - self.patched_errors.push(execution_err); + self.inner.patched_errors.push(execution_err); } }; + // Even if failed - dump commit into junk + self.inner.junk.push(self.state.finalize_strategy); let finalize_signature = finalize_result.map_err(|err| { IntentExecutorError::from_finalize_execution_error( err, - Some(commit_signature), + Some(self.state.commit_signature), ) })?; - Ok(finalize_signature) - } - - /// Patches Commit stage `transaction_strategy` in response to a recoverable - /// [`TransactionStrategyExecutionError`], optionally preparing cleanup data - /// to be applied after a retry. - /// - /// [`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( - inner: &IntentExecutorImpl, - err: &TransactionStrategyExecutionError, - commit_strategy: &mut TransactionStrategy, - committed_pubkeys: &[Pubkey], - ) -> IntentExecutorResult> { - match err { - TransactionStrategyExecutionError::CommitIDError(_, _) => { - let to_cleanup = inner - .handle_commit_id_error(committed_pubkeys, commit_strategy) - .await?; - Ok(ControlFlow::Continue(to_cleanup)) - } - TransactionStrategyExecutionError::ActionsError(_, _) => { - // Unexpected in Two Stage commit - // That would mean that Two Stage executes Standalone commit - error!("Unexpected error in two stage commit flow: {}", err); - Ok(ControlFlow::Break(())) - } - TransactionStrategyExecutionError::UndelegationError(_, _) => { - // Unexpected in Two Stage commit - // That would mean that Two Stage executes undelegation in commit phase - error!("Unexpected error in two stage commit flow: {}", err); - Ok(ControlFlow::Break(())) - } - TransactionStrategyExecutionError::CpiLimitError(_, _) => { - // Can't be handled - error!("Commit tasks exceeded CpiLimitError: {}", err); - Ok(ControlFlow::Break(())) - } - TransactionStrategyExecutionError::InternalError(_) => { - // Can't be handled - Ok(ControlFlow::Break(())) - } - } + let finalized = Finalized { + commit_signature: self.state.commit_signature, + finalize_signature, + }; + Ok(TwoStageExecutor { + inner: self.inner, + state: finalized, + }) } /// Patches Finalize stage `transaction_strategy` in response to a recoverable @@ -257,10 +295,10 @@ where } } -impl<'a, T, F> Deref for TwoStageExecutor<'a, T, F> { - type Target = IntentExecutorImpl; +mod sealed { + pub trait Sealed {} - fn deref(&self) -> &'a Self::Target { - self.inner - } + impl Sealed for super::Initialized {} + impl Sealed for super::Committed {} + impl Sealed for super::Finalized {} } diff --git a/magicblock-committor-service/src/stubs/changeset_committor_stub.rs b/magicblock-committor-service/src/stubs/changeset_committor_stub.rs index 6fba2c112..47be7d491 100644 --- a/magicblock-committor-service/src/stubs/changeset_committor_stub.rs +++ b/magicblock-committor-service/src/stubs/changeset_committor_stub.rs @@ -17,9 +17,7 @@ use tokio_util::sync::{CancellationToken, WaitForCancellationFutureOwned}; use crate::{ error::CommittorServiceResult, - intent_execution_manager::{ - BroadcastedIntentExecutionResult, ExecutionOutputWrapper, - }, + intent_execution_manager::BroadcastedIntentExecutionResult, intent_executor::ExecutionOutput, persist::{CommitStatusRow, IntentPersisterImpl, MessageSignatures}, service_ext::{BaseIntentCommitorExtResult, BaseIntentCommittorExt}, @@ -199,16 +197,14 @@ impl BaseIntentCommittorExt for ChangesetCommittorStub { self.schedule_base_intent(base_intents.clone()).await??; let res = base_intents .into_iter() - .map(|message| { - Ok(ExecutionOutputWrapper { - id: message.inner.id, - output: ExecutionOutput::TwoStage { - commit_signature: Signature::new_unique(), - finalize_signature: Signature::new_unique(), - }, - patched_errors: Arc::new(vec![]), - trigger_type: TriggerType::OnChain, - }) + .map(|message| BroadcastedIntentExecutionResult { + id: message.inner.id, + inner: Ok(ExecutionOutput::TwoStage { + commit_signature: Signature::new_unique(), + finalize_signature: Signature::new_unique(), + }), + patched_errors: Arc::new(vec![]), + trigger_type: TriggerType::OnChain, }) .collect::>(); diff --git a/programs/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rs b/programs/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rs index 205faf509..f0369d741 100644 --- a/programs/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rs +++ b/programs/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rs @@ -274,6 +274,7 @@ mod tests { excluded_pubkeys: Default::default(), requested_undelegation: false, error_message: None, + patched_errors: vec![], } } From cd79fa92a1ae932572dc4f7213fd0035dec5b540 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Fri, 21 Nov 2025 21:18:02 +0400 Subject: [PATCH 21/29] refactor --- .../src/intent_executor/mod.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/magicblock-committor-service/src/intent_executor/mod.rs b/magicblock-committor-service/src/intent_executor/mod.rs index 522b4073b..e5b1e8ba5 100644 --- a/magicblock-committor-service/src/intent_executor/mod.rs +++ b/magicblock-committor-service/src/intent_executor/mod.rs @@ -329,16 +329,12 @@ where finalize_strategy: TransactionStrategy, persister: &Option

, ) -> IntentExecutorResult { - let two_stage_executor = - TwoStageExecutor::new(self, commit_strategy, finalize_strategy); - - // Execute commit stage - let committed_stage = two_stage_executor - .commit(committed_pubkeys, persister) - .await?; - - // Execute finalize stage - let finalized_stage = committed_stage.finalize(persister).await?; + let finalized_stage = + TwoStageExecutor::new(self, commit_strategy, finalize_strategy) + .commit(committed_pubkeys, persister) + .await? + .finalize(persister) + .await?; Ok(ExecutionOutput::TwoStage { commit_signature: finalized_stage.state.commit_signature, From d1f9a74a7a26dc999932e4188307d9a8ce6e2666 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Fri, 21 Nov 2025 21:38:08 +0400 Subject: [PATCH 22/29] fix: tests --- .../tests/test_ix_commit_local.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) 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 8e84567bc..01b0b232e 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 @@ -3,7 +3,7 @@ use std::{ sync::Arc, time::{Duration, Instant}, }; - +use borsh::to_vec; use log::*; use magicblock_committor_service::{ config::ChainConfig, @@ -27,6 +27,7 @@ use solana_sdk::{ }; use test_kit::init_logger; use tokio::task::JoinSet; +use program_flexi_counter::state::FlexiCounter; use utils::transactions::tx_logs_contain; use crate::utils::{ @@ -115,7 +116,15 @@ async fn commit_single_account( init_and_delegate_account_on_chain(&counter_auth, bytes as u64, None) .await; account.owner = program_flexi_counter::id(); - account.data = vec![101_u8; bytes]; + + let counter = FlexiCounter { + label: "Counter".to_string(), + updates: 0, + count: 101 + }; + let mut data = to_vec(&counter).unwrap(); + data.resize(bytes, 0); + account.data = data; let account = CommittedAccount { pubkey, account }; let base_intent = if undelegate { @@ -507,8 +516,7 @@ async fn ix_commit_local( .await .unwrap() .into_iter() - .collect::, _>>() - .expect("Some commits failed"); + .collect::>(); // Assert that all completed assert_eq!(execution_outputs.len(), base_intents.len()); @@ -519,7 +527,7 @@ async fn ix_commit_local( for (execution_output, base_intent) in execution_outputs.into_iter().zip(base_intents.into_iter()) { - let execution_output = execution_output.output; + let execution_output = execution_output.inner.unwrap(); let (commit_signature, finalize_signature) = match execution_output { ExecutionOutput::SingleStage(signature) => (signature, signature), ExecutionOutput::TwoStage { From 9461a487467ecf5c47dd2030801decaf10d2cfa5 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Fri, 21 Nov 2025 21:38:42 +0400 Subject: [PATCH 23/29] fix: fmt --- .../test-committor-service/tests/test_ix_commit_local.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 01b0b232e..3b1032a78 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 @@ -3,6 +3,7 @@ use std::{ sync::Arc, time::{Duration, Instant}, }; + use borsh::to_vec; use log::*; use magicblock_committor_service::{ @@ -18,6 +19,7 @@ use magicblock_program::magic_scheduled_base_intent::{ ScheduledBaseIntent, UndelegateType, }; use magicblock_rpc_client::MagicblockRpcClient; +use program_flexi_counter::state::FlexiCounter; use solana_account::{Account, ReadableAccount}; use solana_pubkey::Pubkey; use solana_rpc_client::nonblocking::rpc_client::RpcClient; @@ -27,7 +29,6 @@ use solana_sdk::{ }; use test_kit::init_logger; use tokio::task::JoinSet; -use program_flexi_counter::state::FlexiCounter; use utils::transactions::tx_logs_contain; use crate::utils::{ @@ -120,7 +121,7 @@ async fn commit_single_account( let counter = FlexiCounter { label: "Counter".to_string(), updates: 0, - count: 101 + count: 101, }; let mut data = to_vec(&counter).unwrap(); data.resize(bytes, 0); From 927a357eb4b8080059389866f99f4e9334d60c75 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Fri, 21 Nov 2025 21:40:32 +0400 Subject: [PATCH 24/29] refactor: naming --- .../tests/test_ix_commit_local.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) 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 3b1032a78..b24efce2a 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 @@ -525,11 +525,11 @@ async fn ix_commit_local( let rpc_client = RpcClient::new("http://localhost:7799".to_string()); let mut strategies = ExpectedStrategies::new(); - for (execution_output, base_intent) in + for (execution_result, base_intent) in execution_outputs.into_iter().zip(base_intents.into_iter()) { - let execution_output = execution_output.inner.unwrap(); - let (commit_signature, finalize_signature) = match execution_output { + let output = execution_result.inner.unwrap(); + let (commit_signature, finalize_signature) = match output { ExecutionOutput::SingleStage(signature) => (signature, signature), ExecutionOutput::TwoStage { commit_signature, @@ -537,6 +537,11 @@ async fn ix_commit_local( } => (commit_signature, finalize_signature), }; + assert_eq!( + execution_result.patched_errors.len(), + 0, + "No errors expected to be patched" + ); assert!( tx_logs_contain(&rpc_client, &commit_signature, "CommitState") .await From 60c6e459e3a007190ffa0403b0d96fd0fc06660b Mon Sep 17 00:00:00 2001 From: taco-paco Date: Tue, 25 Nov 2025 13:28:01 +0700 Subject: [PATCH 25/29] refactor: remove unnecessary impls --- magicblock-committor-service/src/intent_executor/mod.rs | 2 +- .../src/transaction_preparator/delivery_preparator.rs | 1 - magicblock-committor-service/src/transaction_preparator/mod.rs | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/magicblock-committor-service/src/intent_executor/mod.rs b/magicblock-committor-service/src/intent_executor/mod.rs index c893af176..e5b1e8ba5 100644 --- a/magicblock-committor-service/src/intent_executor/mod.rs +++ b/magicblock-committor-service/src/intent_executor/mod.rs @@ -790,7 +790,7 @@ where #[async_trait] impl IntentExecutor for IntentExecutorImpl where - T: TransactionPreparator + Clone, + T: TransactionPreparator, C: TaskInfoFetcher, { /// Executes Message on Base layer diff --git a/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs b/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs index 49407e425..a7ca327fa 100644 --- a/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs +++ b/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs @@ -39,7 +39,6 @@ use crate::{ ComputeBudgetConfig, }; -#[derive(Clone)] pub struct DeliveryPreparator { rpc_client: MagicblockRpcClient, table_mania: TableMania, diff --git a/magicblock-committor-service/src/transaction_preparator/mod.rs b/magicblock-committor-service/src/transaction_preparator/mod.rs index f0364797f..daf9f476a 100644 --- a/magicblock-committor-service/src/transaction_preparator/mod.rs +++ b/magicblock-committor-service/src/transaction_preparator/mod.rs @@ -44,7 +44,6 @@ pub trait TransactionPreparator: Send + Sync + 'static { /// [`TransactionPreparatorImpl`] first version of preparator /// It omits future commit_bundle/finalize_bundle logic /// It creates TXs using current per account commit/finalize -#[derive(Clone)] pub struct TransactionPreparatorImpl { delivery_preparator: DeliveryPreparator, compute_budget_config: ComputeBudgetConfig, From 922ab1a4922d40fc0b1128e796f848a1729eeae4 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Tue, 25 Nov 2025 13:42:13 +0700 Subject: [PATCH 26/29] refactor: address PR comments --- .../src/intent_executor/mod.rs | 2 +- .../delivery_preparator.rs | 19 +++++++++++++++++++ .../process_scheduled_commit_sent.rs | 3 ++- 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/magicblock-committor-service/src/intent_executor/mod.rs b/magicblock-committor-service/src/intent_executor/mod.rs index e5b1e8ba5..f6c15bd56 100644 --- a/magicblock-committor-service/src/intent_executor/mod.rs +++ b/magicblock-committor-service/src/intent_executor/mod.rs @@ -468,7 +468,7 @@ where (commit_strategy, finalize_strategy, to_cleanup) } - /// Handles actions error, stripping away actions + /// Handles undelegation error, stripping away actions /// Returns [`TransactionStrategy`] to be cleaned up fn handle_undelegation_error( &self, diff --git a/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs b/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs index a7ca327fa..703b9f870 100644 --- a/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs +++ b/magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs @@ -509,6 +509,15 @@ pub enum TransactionSendError { MagicBlockRpcClientError(#[from] MagicBlockRpcClientError), } +impl TransactionSendError { + pub fn signature(&self) -> Option { + match self { + Self::MagicBlockRpcClientError(err) => err.signature(), + _ => None, + } + } +} + #[derive(thiserror::Error, Debug)] pub enum BufferExecutionError { #[error("AccountAlreadyInitializedError: {0}")] @@ -520,6 +529,15 @@ pub enum BufferExecutionError { TransactionSendError(#[from] TransactionSendError), } +impl BufferExecutionError { + pub fn signature(&self) -> Option { + match self { + Self::AccountAlreadyInitializedError(_, signature) => *signature, + Self::TransactionSendError(err) => err.signature(), + } + } +} + impl From for BufferExecutionError { fn from(value: MagicBlockRpcClientError) -> Self { Self::TransactionSendError( @@ -550,6 +568,7 @@ impl InternalError { pub fn signature(&self) -> Option { match self { Self::MagicBlockRpcClientError(err) => err.signature(), + Self::BufferExecutionError(err) => err.signature(), _ => None, } } diff --git a/programs/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rs b/programs/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rs index f0369d741..43172ed8f 100644 --- a/programs/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rs +++ b/programs/magicblock/src/schedule_transactions/process_scheduled_commit_sent.rs @@ -16,7 +16,8 @@ use crate::{ utils::accounts::get_instruction_pubkey_with_idx, validator, }; -/// Code emitted if Intent failed to execute on base chain +/// Error code returned when an intent execution failed. +/// This indicates the intent could not be successfully executed despite patching attempts. const INTENT_FAILED_CODE: u32 = 0x7461636F; #[derive(Default, Debug, Clone)] From b0409ef01c0e15b1b08d52b89d10d28c85614804 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Tue, 25 Nov 2025 13:45:32 +0700 Subject: [PATCH 27/29] refactor: log cleanup --- magicblock-accounts/src/scheduled_commits_processor.rs | 2 +- .../src/intent_executor/two_stage_executor.rs | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/magicblock-accounts/src/scheduled_commits_processor.rs b/magicblock-accounts/src/scheduled_commits_processor.rs index 4f70d7abb..da5a81a14 100644 --- a/magicblock-accounts/src/scheduled_commits_processor.rs +++ b/magicblock-accounts/src/scheduled_commits_processor.rs @@ -296,7 +296,7 @@ impl ScheduledCommitsProcessorImpl { .patched_errors .iter() .map(|err| { - info!("Patched intent, error was: {}", err); + info!("Patched intent: {}. error was: {}", intent_id, err); err.to_string() }) .collect(); 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 85546a68e..4c3e744cb 100644 --- a/magicblock-committor-service/src/intent_executor/two_stage_executor.rs +++ b/magicblock-committor-service/src/intent_executor/two_stage_executor.rs @@ -97,10 +97,7 @@ where ) .await?; let cleanup = match flow { - ControlFlow::Continue(value) => { - info!("Patched intent, error was: {:?}", execution_err); - value - } + ControlFlow::Continue(value) => value, ControlFlow::Break(()) => { break Err(execution_err); } From 703c94e91d918b2e46b2dc676b559baa7427ce20 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Tue, 25 Nov 2025 14:24:52 +0700 Subject: [PATCH 28/29] refactor: fix log --- .../src/intent_executor/two_stage_executor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 4c3e744cb..314a462e4 100644 --- a/magicblock-committor-service/src/intent_executor/two_stage_executor.rs +++ b/magicblock-committor-service/src/intent_executor/two_stage_executor.rs @@ -263,7 +263,7 @@ where match err { TransactionStrategyExecutionError::CommitIDError(_, _) => { // Unexpected error in Two Stage commit - error!("Unexpected error in two stage commit flow: {}", err); + error!("Unexpected error in two stage finalize flow: {}", err); Ok(ControlFlow::Break(())) } TransactionStrategyExecutionError::ActionsError(_, _) => { From 090c47c2a171548158d0ca4958f9c4d0b6f00354 Mon Sep 17 00:00:00 2001 From: taco-paco Date: Tue, 25 Nov 2025 20:04:34 +0700 Subject: [PATCH 29/29] refactor: comments fixes --- .../src/intent_executor/two_stage_executor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 bee138e8a..b1c6e496b 100644 --- a/magicblock-committor-service/src/intent_executor/two_stage_executor.rs +++ b/magicblock-committor-service/src/intent_executor/two_stage_executor.rs @@ -229,7 +229,7 @@ where } }; - // Even if failed - dump commit into junk + // Even if failed - dump finalize into junk self.inner.junk.push(self.state.finalize_strategy); let finalize_signature = finalize_result.map_err(|err| { IntentExecutorError::from_finalize_execution_error(