Skip to content

Commit 609158f

Browse files
authored
Handle uncleaned buffers + generalized tx send retries (#597)
### Error handling & parsing Old `DeliveryPreparator::send_ixs_with_retry` wasn't parsing any errors and retrying blindly for specified `max_retries` times. This led to Intents taking more time retrying errors it couldn't recover from. Now we adapt error parsing from IntentExecutor. and not to duplicate quite cumbersome error dispatches of `MagicBlockRpcClientError` a generalized version that could be used from both `IntentExecutor` and `DeliveryPreparator` is introduced. ### Handling of buffer program AccountAlreadyInitialized error See error explained in #590. We handle it by cleaning and then reainitializing buffer. Due structure of `magicblock-committer-program` reusing existing buffer is only possible if initialized buffer length is equal to current account state, which isn't guaranteed at all but could be a future optimization <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a public utils module with a pluggable async retry framework (send-with-retries), error mappers, and RPC error decision logic. * Delivery preparator and intent executor now surface richer send outcomes and mappers, exposing new public error types and APIs that can carry optional signatures. * **Bug Fixes** * Improved error propagation and messages to include RPC context and optional signatures across execution and retry flows. * Centralized retry/error-mapping for more consistent prepare/delivery behavior. * **Tests** * Added/updated integration tests for cleanup, re-prepare, already-initialized, and adjusted error assertions. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 3e940c9 commit 609158f

File tree

10 files changed

+827
-282
lines changed

10 files changed

+827
-282
lines changed

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

Lines changed: 64 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
use log::error;
2-
use magicblock_rpc_client::MagicBlockRpcClientError;
2+
use magicblock_rpc_client::{
3+
utils::TransactionErrorMapper, MagicBlockRpcClientError,
4+
};
35
use solana_sdk::{
46
instruction::InstructionError,
57
signature::{Signature, SignerError},
@@ -35,12 +37,12 @@ impl InternalError {
3537
pub enum IntentExecutorError {
3638
#[error("EmptyIntentError")]
3739
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),
40+
#[error("User supplied actions are ill-formed: {0}. {:?}", .1)]
41+
ActionsError(#[source] TransactionError, Option<Signature>),
42+
#[error("Accounts committed with an invalid Commit id: {0}. {:?}", .1)]
43+
CommitIDError(#[source] TransactionError, Option<Signature>),
44+
#[error("Max instruction trace length exceeded: {0}. {:?}", .1)]
45+
CpiLimitError(#[source] TransactionError, Option<Signature>),
4446
#[error("Failed to fit in single TX")]
4547
FailedToFitError,
4648
#[error("SignerError: {0}")]
@@ -69,7 +71,7 @@ pub enum IntentExecutorError {
6971

7072
impl IntentExecutorError {
7173
pub fn is_cpi_limit_error(&self) -> bool {
72-
matches!(self, IntentExecutorError::CpiLimitError(_))
74+
matches!(self, IntentExecutorError::CpiLimitError(_, _))
7375
}
7476

7577
pub fn from_strategy_execution_error<F>(
@@ -80,15 +82,17 @@ impl IntentExecutorError {
8082
F: FnOnce(InternalError) -> IntentExecutorError,
8183
{
8284
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)
85+
TransactionStrategyExecutionError::ActionsError(err, signature) => {
86+
IntentExecutorError::ActionsError(err, signature)
9187
}
88+
TransactionStrategyExecutionError::CpiLimitError(
89+
err,
90+
signature,
91+
) => IntentExecutorError::CpiLimitError(err, signature),
92+
TransactionStrategyExecutionError::CommitIDError(
93+
err,
94+
signature,
95+
) => IntentExecutorError::CommitIDError(err, signature),
9296
TransactionStrategyExecutionError::InternalError(err) => {
9397
converter(err)
9498
}
@@ -99,24 +103,31 @@ impl IntentExecutorError {
99103
/// Those are the errors that may occur during Commit/Finalize stages on Base layer
100104
#[derive(thiserror::Error, Debug)]
101105
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),
106+
#[error("User supplied actions are ill-formed: {0}. {:?}", .1)]
107+
ActionsError(#[source] TransactionError, Option<Signature>),
108+
#[error("Accounts committed with an invalid Commit id: {0}. {:?}", .1)]
109+
CommitIDError(#[source] TransactionError, Option<Signature>),
110+
#[error("Max instruction trace length exceeded: {0}. {:?}", .1)]
111+
CpiLimitError(#[source] TransactionError, Option<Signature>),
108112
#[error("InternalError: {0}")]
109113
InternalError(#[from] InternalError),
110114
}
111115

116+
impl From<MagicBlockRpcClientError> for TransactionStrategyExecutionError {
117+
fn from(value: MagicBlockRpcClientError) -> Self {
118+
Self::InternalError(InternalError::MagicBlockRpcClientError(value))
119+
}
120+
}
121+
112122
impl TransactionStrategyExecutionError {
113123
/// Convert [`TransactionError`] into known errors that can be handled
124+
/// Otherwise return original [`TransactionError`]
114125
/// [`TransactionStrategyExecutionError`]
115-
pub fn from_transaction_error(
126+
pub fn try_from_transaction_error(
116127
err: TransactionError,
128+
signature: Option<Signature>,
117129
tasks: &[Box<dyn BaseTask>],
118-
map: impl FnOnce(TransactionError) -> MagicBlockRpcClientError,
119-
) -> Self {
130+
) -> Result<Self, TransactionError> {
120131
// There's always 2 budget instructions in front
121132
const OFFSET: u8 = 2;
122133
const NONCE_OUT_OF_ORDER: u32 =
@@ -127,24 +138,22 @@ impl TransactionStrategyExecutionError {
127138
transaction_err @ TransactionError::InstructionError(
128139
_,
129140
InstructionError::Custom(NONCE_OUT_OF_ORDER),
130-
) => TransactionStrategyExecutionError::CommitIDError(
141+
) => Ok(TransactionStrategyExecutionError::CommitIDError(
131142
transaction_err,
132-
),
143+
signature,
144+
)),
133145
// Some tx may use too much CPIs and we can handle it in certain cases
134146
transaction_err @ TransactionError::InstructionError(
135147
_,
136148
InstructionError::MaxInstructionTraceLengthExceeded,
137-
) => TransactionStrategyExecutionError::CpiLimitError(
149+
) => Ok(TransactionStrategyExecutionError::CpiLimitError(
138150
transaction_err,
139-
),
151+
signature,
152+
)),
140153
// Filter ActionError, we can attempt recovery by stripping away actions
141154
transaction_err @ TransactionError::InstructionError(index, _) => {
142155
let Some(action_index) = index.checked_sub(OFFSET) else {
143-
return TransactionStrategyExecutionError::InternalError(
144-
InternalError::MagicBlockRpcClientError(map(
145-
transaction_err,
146-
)),
147-
);
156+
return Err(transaction_err);
148157
};
149158

150159
// If index corresponds to an Action -> ActionsError; otherwise -> InternalError.
@@ -154,15 +163,12 @@ impl TransactionStrategyExecutionError {
154163
.map(|task| task.task_type()),
155164
Some(TaskType::Action)
156165
) {
157-
TransactionStrategyExecutionError::ActionsError(
166+
Ok(TransactionStrategyExecutionError::ActionsError(
158167
transaction_err,
159-
)
168+
signature,
169+
))
160170
} else {
161-
TransactionStrategyExecutionError::InternalError(
162-
InternalError::MagicBlockRpcClientError(map(
163-
transaction_err,
164-
)),
165-
)
171+
Err(transaction_err)
166172
}
167173
}
168174
// This means transaction failed to other reasons that we don't handle - propagate
@@ -171,14 +177,28 @@ impl TransactionStrategyExecutionError {
171177
"Message execution failed and we can not handle it: {}",
172178
err
173179
);
174-
TransactionStrategyExecutionError::InternalError(
175-
InternalError::MagicBlockRpcClientError(map(err)),
176-
)
180+
Err(err)
177181
}
178182
}
179183
}
180184
}
181185

186+
pub(crate) struct IntentTransactionErrorMapper<'a> {
187+
pub tasks: &'a [Box<dyn BaseTask>],
188+
}
189+
impl TransactionErrorMapper for IntentTransactionErrorMapper<'_> {
190+
type ExecutionError = TransactionStrategyExecutionError;
191+
fn try_map(
192+
&self,
193+
error: TransactionError,
194+
signature: Option<Signature>,
195+
) -> Result<Self::ExecutionError, TransactionError> {
196+
TransactionStrategyExecutionError::try_from_transaction_error(
197+
error, signature, self.tasks,
198+
)
199+
}
200+
}
201+
182202
impl From<TaskStrategistError> for IntentExecutorError {
183203
fn from(value: TaskStrategistError) -> Self {
184204
match value {

0 commit comments

Comments
 (0)