Skip to content

Commit 36fb2d6

Browse files
taco-pacothlorenzDodecahedr0xBretasArthur1GabrielePicco
authored
Handling of intent execution failures (#564)
### Error Types and Handling #### ActionError Occurs when one or more actions in the intent fail. Depending on configuration, we strip failing actions and continue with the commit phase. A future feature will allow users to mark actions as mandatory, in which case we will return ActionError immediately. #### CommitIdError Happens when another process (retry script) commits using the same commit ID, invalidating ours. The system now resets the TaskInfoFetcher and retries the execution automatically. #### CpiLimitError Indicates that a transaction exceeded Solana’s CPI (cross-program invocation) limits. Recovery flow: Single-stage → Two-stage: switch strategy and retry using the same buffers and ALTs. Two-stage with mandatory actions: strip away actions and retry. If it still fails, return CpiLimitError. This should not normally happen and implies a contract-level validation issue. #### Other Errors Will retry only IO errors <!-- greptile_comment --> <h2>Greptile Overview</h2> Updated On: 2025-10-07 17:46:54 UTC <h3>Summary</h3> This PR introduces comprehensive error handling and recovery mechanisms for intent execution failures in the MagicBlock validator system. The changes implement sophisticated retry logic for three main error types: `ActionError` (when individual actions fail), `CommitIdError` (when commit IDs become invalidated by concurrent processes), and `CpiLimitError` (when transactions exceed Solana's cross-program invocation limits). The implementation refactors the task system from enum-based to modular design with separate `ArgsTask` and `BufferTask` modules, introduces a `PreparationState` lifecycle management system, and adds single-stage and two-stage execution strategies. The error recovery flows include automatic retry mechanisms with exponential backoff, strategy switching (single-stage to two-stage for CPI limit errors), action stripping for failed operations, and cache invalidation with fresh commit ID generation for race conditions. Key architectural changes include: - New error classification system that maps Solana transaction errors to specific recovery strategies - Task state management with preparation lifecycle tracking (`NotNeeded`, `Required`, `Cleanup`) - Resource cleanup mechanisms for buffer accounts and lookup table keys - Retry logic with configurable ceilings and backoff strategies - Visitor pattern implementation for task processing and metadata extraction The changes integrate with the existing committor service architecture while adding resilience against various blockchain-specific failure modes. The system can now handle concurrent commit operations, recover from resource constraint violations, and gracefully degrade by stripping problematic actions while maintaining transaction integrity. <h3>Important Files Changed</h3> <details><summary>Changed Files</summary> | Filename | Score | Overview | |----------|-------|----------| | `Cargo.toml` | 5/5 | Adds `solana-transaction-error` dependency for enhanced transaction error handling | | `magicblock-rpc-client/Cargo.toml` | 5/5 | Adds `solana-transaction-error` dependency to support TransactionResult type | | `test-integration/test-committor-service/Cargo.toml` | 4/5 | Adds futures crate for async testing of new error handling scenarios | | `magicblock-committor-program/bin/magicblock_committor_program.so` | 3/5 | Updated compiled binary reflecting source code changes for error handling | | `test-integration/test-committor-service/tests/common.rs` | 4/5 | Refactors test infrastructure with new transaction preparator naming and flexible keypair setup | | `test-integration/programs/flexi-counter/src/processor/call_handler.rs` | 4/5 | Improves memory management and error handling in counter state updates | | `magicblock-rpc-client/src/lib.rs` | 4/5 | Refactors transaction status polling into separate methods and adds error handling utilities | | `test-integration/programs/flexi-counter/src/lib.rs` | 5/5 | Makes args module public for external access to action data structures | | `magicblock-committor-service/src/intent_executor/task_info_fetcher.rs` | 4/5 | Adds reset functionality for cache invalidation during CommitIdError recovery | | `magicblock-committor-service/src/tasks/task_visitors/mod.rs` | 5/5 | Adds utility visitor module for commit metadata extraction | | `magicblock-committor-service/src/tasks/task_strategist.rs` | 4/5 | Adds ALT recalculation method and refactors task construction for error recovery | | `test-integration/test-committor-service/tests/utils/transactions.rs` | 3/5 | Adds test utilities for account setup with retry logic and error handling | | `test-integration/test-committor-service/tests/test_intent_executor.rs` | 4/5 | Comprehensive integration tests for all three error types and recovery mechanisms | | `test-integration/test-committor-service/tests/test_ix_commit_local.rs` | 5/5 | Refactors to use centralized utility functions from utils modules | | `magicblock-committor-service/src/intent_executor/intent_executor_factory.rs` | 5/5 | Simple renaming from TransactionPreparatorV1 to TransactionPreparatorImpl | | `magicblock-committor-service/src/tasks/task_visitors/utility_visitor.rs` | 4/5 | New visitor for extracting commit metadata with asymmetric task handling | | `magicblock-committor-service/src/tasks/mod.rs` | 4/5 | Major refactoring to modular task system with preparation state management | | `magicblock-committor-service/src/tasks/args_task.rs` | 4/5 | New ArgsTask implementation with optimization and reset capabilities | | `magicblock-committor-service/src/tasks/visitor.rs` | 5/5 | Updates import statements to use direct module paths | | `test-integration/test-committor-service/tests/test_delivery_preparator.rs` | 4/5 | Updates tests for new task wrapper types and preparation state management | | `test-integration/test-committor-service/tests/utils/mod.rs` | 4/5 | Adds thread-safe validator authority initialization function | | `magicblock-committor-service/src/tasks/task_visitors/persistor_visitor.rs` | 5/5 | Updates visitor pattern to handle restructured task types | | `magicblock-committor-service/src/intent_executor/two_stage_executor.rs` | 1/5 | Implements two-stage execution with critical parameter naming typo | | `magicblock-committor-service/src/tasks/buffer_task.rs` | 4/5 | New buffer-based task strategy with preparation state and commit ID reset | | `test-integration/test-committor-service/tests/test_transaction_preparator.rs` | 4/5 | Updates task creation API and adds preparation state verification | | `magicblock-committor-service/src/intent_executor/single_stage_executor.rs` | 4/5 | Implements single-stage execution with retry logic and error recovery | | `magicblock-committor-service/src/intent_executor/mod.rs` | 3/5 | Complex error handling implementation with potential recursion and edge case issues | | `magicblock-committor-service/src/tasks/task_builder.rs` | 4/5 | Refactors task builder to use new ArgsTask wrapper structure | | `magicblock-committor-service/src/transaction_preparator/delivery_preparator.rs` | 4/5 | Enhanced preparation with lifecycle management and comprehensive cleanup | | `magicblock-committor-service/src/transaction_preparator/mod.rs` | 4/5 | Adds cleanup method and refactors preparator interface for error handling | | `magicblock-committor-service/src/intent_executor/error.rs` | 4/5 | Comprehensive error classification system with complex transaction error mapping | </details> <h3>Confidence score: 3/5</h3> - This PR introduces significant complexity with sophisticated error handling but has potential critical issues that need addressing - Score reflects concerns about compilation errors, complex error mapping logic, and potential infinite recursion risks noted in code comments - Pay close attention to `magicblock-committor-service/src/intent_executor/two_stage_executor.rs` which contains critical parameter naming typos that will prevent compilation <h3>Sequence Diagram</h3> ```mermaid sequenceDiagram participant User participant IntentExecutor participant TaskBuilder participant TaskStrategist participant TransactionPreparator participant DeliveryPreparator participant TaskInfoFetcher participant RpcClient User->>IntentExecutor: execute(base_intent, persister) IntentExecutor->>TaskBuilder: commit_tasks(task_info_fetcher, base_intent, persister) TaskBuilder->>TaskInfoFetcher: fetch_next_commit_ids(pubkeys) TaskInfoFetcher->>RpcClient: get_multiple_accounts(pda_accounts) RpcClient-->>TaskInfoFetcher: accounts_data TaskInfoFetcher-->>TaskBuilder: commit_ids TaskBuilder-->>IntentExecutor: commit_tasks IntentExecutor->>TaskBuilder: finalize_tasks(task_info_fetcher, base_intent) TaskBuilder->>TaskInfoFetcher: fetch_rent_reimbursements(pubkeys) TaskInfoFetcher->>RpcClient: get_multiple_accounts(pda_accounts) RpcClient-->>TaskInfoFetcher: accounts_data TaskInfoFetcher-->>TaskBuilder: rent_reimbursements TaskBuilder-->>IntentExecutor: finalize_tasks IntentExecutor->>TaskStrategist: build_strategy(tasks, authority, persister) TaskStrategist-->>IntentExecutor: transaction_strategy alt Single Stage Flow IntentExecutor->>IntentExecutor: single_stage_execution_flow(base_intent, strategy, junk, persister) loop Retry with error recovery IntentExecutor->>TransactionPreparator: prepare_for_strategy(authority, strategy, persister) TransactionPreparator->>DeliveryPreparator: prepare_for_delivery(authority, strategy, persister) DeliveryPreparator->>RpcClient: send preparation transactions RpcClient-->>DeliveryPreparator: signatures DeliveryPreparator-->>TransactionPreparator: lookup_tables TransactionPreparator-->>IntentExecutor: prepared_message IntentExecutor->>RpcClient: send_prepared_message(prepared_message) RpcClient-->>IntentExecutor: transaction_result alt Error occurs IntentExecutor->>IntentExecutor: patch_strategy(error, strategy, base_intent) alt CommitIDError IntentExecutor->>TaskInfoFetcher: reset(ResetType::Specific) IntentExecutor->>TaskInfoFetcher: fetch_next_commit_ids(pubkeys) TaskInfoFetcher-->>IntentExecutor: updated_commit_ids IntentExecutor->>IntentExecutor: reset_commit_id(tasks) end alt ActionsError IntentExecutor->>IntentExecutor: handle_actions_error(strategy) end alt CpiLimitError IntentExecutor->>IntentExecutor: switch to two stage execution end end end else Two Stage Flow IntentExecutor->>IntentExecutor: two_stage_execution_flow(pubkeys, commit_strategy, finalize_strategy, junk, persister) Note over IntentExecutor: Similar pattern for commit and finalize stages end IntentExecutor->>DeliveryPreparator: cleanup(authority, tasks, lookup_table_keys) DeliveryPreparator->>RpcClient: send cleanup transactions RpcClient-->>DeliveryPreparator: cleanup_result IntentExecutor-->>User: ExecutionOutput ``` <!-- greptile_other_comments_section --> <!-- /greptile_comment --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - New Features - Introduced single-stage and two-stage intent execution with automatic fallback and parallel cleanup. - Enhanced error handling with clearer categories (commit ID, actions, CPI limit) and recovery paths. - Added task preparation/cleanup workflow with new task types for argument-based and buffer-based execution. - Improved RPC status waiting and transaction outcome helpers. - Refactor - Replaced legacy builders/preparators with updated implementations across the execution pipeline. - Documentation - Converted and clarified inline comments to Rustdoc in task info fetcher. - Tests - Added comprehensive integration tests covering error parsing, recovery flows, and cleanup behaviors. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Thorsten Lorenz <[email protected]> Co-authored-by: Dodecahedr0x <[email protected]> Co-authored-by: Arthur Bretas <[email protected]> Co-authored-by: Dodecahedr0x <[email protected]> Co-authored-by: Gabriele Picco <[email protected]>
1 parent ccee397 commit 36fb2d6

File tree

34 files changed

+3301
-878
lines changed

34 files changed

+3301
-878
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,7 @@ solana-svm-transaction = { version = "2.2" }
198198
solana-system-program = { version = "2.2" }
199199
solana-timings = "2.2"
200200
solana-transaction-status = { version = "2.2" }
201+
solana-transaction-error = "2.2"
201202
solana-transaction-status-client-types = "2.2"
202203
spl-token = "=7.0"
203204
spl-token-2022 = "=6.0"
Binary file not shown.

magicblock-committor-service/src/intent_executor/error.rs

Lines changed: 133 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,15 @@
1+
use log::error;
12
use magicblock_rpc_client::MagicBlockRpcClientError;
2-
use solana_sdk::signature::{Signature, SignerError};
3+
use solana_sdk::{
4+
instruction::InstructionError,
5+
signature::{Signature, SignerError},
6+
transaction::TransactionError,
7+
};
38

49
use crate::{
510
tasks::{
611
task_builder::TaskBuilderError, task_strategist::TaskStrategistError,
12+
BaseTask, TaskType,
713
},
814
transaction_preparator::error::TransactionPreparatorError,
915
};
@@ -16,10 +22,25 @@ pub enum InternalError {
1622
MagicBlockRpcClientError(#[from] MagicBlockRpcClientError),
1723
}
1824

25+
impl InternalError {
26+
pub fn signature(&self) -> Option<Signature> {
27+
match self {
28+
Self::SignerError(_) => None,
29+
Self::MagicBlockRpcClientError(err) => err.signature(),
30+
}
31+
}
32+
}
33+
1934
#[derive(thiserror::Error, Debug)]
2035
pub enum IntentExecutorError {
2136
#[error("EmptyIntentError")]
2237
EmptyIntentError,
38+
#[error("User supplied actions are ill-formed: {0}")]
39+
ActionsError(#[source] TransactionError),
40+
#[error("Accounts committed with an invalid Commit id: {0}")]
41+
CommitIDError(#[source] TransactionError),
42+
#[error("Max instruction trace length exceeded: {0}")]
43+
CpiLimitError(#[source] TransactionError),
2344
#[error("Failed to fit in single TX")]
2445
FailedToFitError,
2546
#[error("SignerError: {0}")]
@@ -46,6 +67,117 @@ pub enum IntentExecutorError {
4667
FailedFinalizePreparationError(#[source] TransactionPreparatorError),
4768
}
4869

70+
impl IntentExecutorError {
71+
pub fn is_cpi_limit_error(&self) -> bool {
72+
matches!(self, IntentExecutorError::CpiLimitError(_))
73+
}
74+
75+
pub fn from_strategy_execution_error<F>(
76+
error: TransactionStrategyExecutionError,
77+
converter: F,
78+
) -> IntentExecutorError
79+
where
80+
F: FnOnce(InternalError) -> IntentExecutorError,
81+
{
82+
match error {
83+
TransactionStrategyExecutionError::ActionsError(err) => {
84+
IntentExecutorError::ActionsError(err)
85+
}
86+
TransactionStrategyExecutionError::CpiLimitError(err) => {
87+
IntentExecutorError::CpiLimitError(err)
88+
}
89+
TransactionStrategyExecutionError::CommitIDError(err) => {
90+
IntentExecutorError::CommitIDError(err)
91+
}
92+
TransactionStrategyExecutionError::InternalError(err) => {
93+
converter(err)
94+
}
95+
}
96+
}
97+
}
98+
99+
/// Those are the errors that may occur during Commit/Finalize stages on Base layer
100+
#[derive(thiserror::Error, Debug)]
101+
pub enum TransactionStrategyExecutionError {
102+
#[error("User supplied actions are ill-formed: {0}")]
103+
ActionsError(#[source] TransactionError),
104+
#[error("Accounts committed with an invalid Commit id: {0}")]
105+
CommitIDError(#[source] TransactionError),
106+
#[error("Max instruction trace length exceeded: {0}")]
107+
CpiLimitError(#[source] TransactionError),
108+
#[error("InternalError: {0}")]
109+
InternalError(#[from] InternalError),
110+
}
111+
112+
impl TransactionStrategyExecutionError {
113+
/// Convert [`TransactionError`] into known errors that can be handled
114+
/// [`TransactionStrategyExecutionError`]
115+
pub fn from_transaction_error(
116+
err: TransactionError,
117+
tasks: &[Box<dyn BaseTask>],
118+
map: impl FnOnce(TransactionError) -> MagicBlockRpcClientError,
119+
) -> Self {
120+
// There's always 2 budget instructions in front
121+
const OFFSET: u8 = 2;
122+
const OUTDATED_SLOT: u32 = dlp::error::DlpError::OutdatedSlot as u32;
123+
124+
match err {
125+
// Filter CommitIdError by custom error code
126+
transaction_err @ TransactionError::InstructionError(
127+
_,
128+
InstructionError::Custom(OUTDATED_SLOT),
129+
) => TransactionStrategyExecutionError::CommitIDError(
130+
transaction_err,
131+
),
132+
// Some tx may use too much CPIs and we can handle it in certain cases
133+
transaction_err @ TransactionError::InstructionError(
134+
_,
135+
InstructionError::MaxInstructionTraceLengthExceeded,
136+
) => TransactionStrategyExecutionError::CpiLimitError(
137+
transaction_err,
138+
),
139+
// Filter ActionError, we can attempt recovery by stripping away actions
140+
transaction_err @ TransactionError::InstructionError(index, _) => {
141+
let Some(action_index) = index.checked_sub(OFFSET) else {
142+
return TransactionStrategyExecutionError::InternalError(
143+
InternalError::MagicBlockRpcClientError(map(
144+
transaction_err,
145+
)),
146+
);
147+
};
148+
149+
// If index corresponds to an Action -> ActionsError; otherwise -> InternalError.
150+
if matches!(
151+
tasks
152+
.get(action_index as usize)
153+
.map(|task| task.task_type()),
154+
Some(TaskType::Action)
155+
) {
156+
TransactionStrategyExecutionError::ActionsError(
157+
transaction_err,
158+
)
159+
} else {
160+
TransactionStrategyExecutionError::InternalError(
161+
InternalError::MagicBlockRpcClientError(map(
162+
transaction_err,
163+
)),
164+
)
165+
}
166+
}
167+
// This means transaction failed to other reasons that we don't handle - propagate
168+
err => {
169+
error!(
170+
"Message execution failed and we can not handle it: {}",
171+
err
172+
);
173+
TransactionStrategyExecutionError::InternalError(
174+
InternalError::MagicBlockRpcClientError(map(err)),
175+
)
176+
}
177+
}
178+
}
179+
}
180+
49181
impl From<TaskStrategistError> for IntentExecutorError {
50182
fn from(value: TaskStrategistError) -> Self {
51183
match value {

magicblock-committor-service/src/intent_executor/intent_executor_factory.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use crate::{
88
task_info_fetcher::CacheTaskInfoFetcher, IntentExecutor,
99
IntentExecutorImpl,
1010
},
11-
transaction_preparator::TransactionPreparatorV1,
11+
transaction_preparator::TransactionPreparatorImpl,
1212
ComputeBudgetConfig,
1313
};
1414

@@ -28,15 +28,15 @@ pub struct IntentExecutorFactoryImpl {
2828

2929
impl IntentExecutorFactory for IntentExecutorFactoryImpl {
3030
type Executor =
31-
IntentExecutorImpl<TransactionPreparatorV1, CacheTaskInfoFetcher>;
31+
IntentExecutorImpl<TransactionPreparatorImpl, CacheTaskInfoFetcher>;
3232

3333
fn create_instance(&self) -> Self::Executor {
34-
let transaction_preparator = TransactionPreparatorV1::new(
34+
let transaction_preparator = TransactionPreparatorImpl::new(
3535
self.rpc_client.clone(),
3636
self.table_mania.clone(),
3737
self.compute_budget_config.clone(),
3838
);
39-
IntentExecutorImpl::<TransactionPreparatorV1, CacheTaskInfoFetcher>::new(
39+
IntentExecutorImpl::<TransactionPreparatorImpl, CacheTaskInfoFetcher>::new(
4040
self.rpc_client.clone(),
4141
transaction_preparator,
4242
self.commit_id_tracker.clone(),

0 commit comments

Comments
 (0)