Skip to content

Commit

Permalink
fix(rpc): workaround opaque blockifier errors
Browse files Browse the repository at this point in the history
Some blockifier errors are opaque and hide useful information.
  • Loading branch information
Mirko-von-Leipzig committed Dec 4, 2023
1 parent c1e599b commit 09c11cd
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 17 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ More expansive patch notes and explanations may be found in the specific [pathfi
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## Unreleased

### Fixed

- Execution errors are opaque and don't always include the root cause.

## [0.10.0] - 2023-11-29

### Added
Expand Down
78 changes: 78 additions & 0 deletions crates/executor/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,3 +101,81 @@ impl From<anyhow::Error> for TransactionExecutionError {
Self::Internal(value)
}
}

impl TransactionExecutionError {
pub fn new(transaction_index: usize, error: BlockifierTransactionExecutionError) -> Self {
Self::ExecutionError {
transaction_index,
error: match &error {
// Some variants don't propagate their child's error so we do this manually until it is
// fixed in the blockifier. We have a test to ensure we don't miss fix.
BlockifierTransactionExecutionError::ContractConstructorExecutionFailed(x) => {
format!("{error} {x}")
}
BlockifierTransactionExecutionError::ExecutionError(x) => format!("{error} {x}"),
BlockifierTransactionExecutionError::ValidateTransactionError(x) => {
format!("{error} {x}")
}
other => other.to_string(),
},
}
}
}

#[cfg(test)]
mod tests {
use super::*;

mod transaction_errors_are_mapped_correctly {
//! Some variants in the blockifier are opaque and omit the inner error's data. We've patched this manually
//! and this tests ensures we don't accidentally stutter once the blockifier fixes this.
use super::*;
use blockifier::execution::errors::EntryPointExecutionError;

#[test]
fn contract_constructor_execution_failed() {
let child = EntryPointExecutionError::RecursionDepthExceeded;
let expected = format!("Contract constructor execution has failed. {child}");

let err =
BlockifierTransactionExecutionError::ContractConstructorExecutionFailed(child);
let err = TransactionExecutionError::new(0, err);
let err = match err {
TransactionExecutionError::ExecutionError { error, .. } => error,
_ => unreachable!("unexpected variant"),
};

assert_eq!(err, expected);
}

#[test]
fn execution_error() {
let child = EntryPointExecutionError::RecursionDepthExceeded;
let expected = format!("Transaction execution has failed. {child}");

let err = BlockifierTransactionExecutionError::ExecutionError(child);
let err = TransactionExecutionError::new(0, err);
let err = match err {
TransactionExecutionError::ExecutionError { error, .. } => error,
_ => unreachable!("unexpected variant"),
};

assert_eq!(err, expected);
}

#[test]
fn validate_transaction_error() {
let child = EntryPointExecutionError::RecursionDepthExceeded;
let expected = format!("Transaction validation has failed. {child}");

let err = BlockifierTransactionExecutionError::ValidateTransactionError(child);
let err = TransactionExecutionError::new(0, err);
let err = match err {
TransactionExecutionError::ExecutionError { error, .. } => error,
_ => unreachable!("unexpected variant"),
};

assert_eq!(err, expected);
}
}
}
5 changes: 1 addition & 4 deletions crates/executor/src/estimate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,7 @@ pub fn estimate(
}
Err(error) => {
tracing::debug!(%error, %transaction_idx, "Transaction estimation failed");
return Err(TransactionExecutionError::ExecutionError {
transaction_index: transaction_idx,
error: error.to_string(),
});
return Err(TransactionExecutionError::new(transaction_idx, error));
}
}
}
Expand Down
11 changes: 3 additions & 8 deletions crates/executor/src/simulate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,7 @@ pub fn simulate(
}
Err(error) => {
tracing::debug!(%error, %transaction_idx, "Transaction simulation failed");
return Err(TransactionExecutionError::ExecutionError {
transaction_index: transaction_idx,
error: error.to_string(),
});
return Err(TransactionExecutionError::new(transaction_idx, error));
}
}
}
Expand Down Expand Up @@ -295,7 +292,7 @@ fn to_trace(
let maybe_function_invocation = execution_info.execute_call_info.map(Into::into);
let fee_transfer_invocation = execution_info.fee_transfer_call_info.map(Into::into);

let trace = match transaction_type {
match transaction_type {
TransactionType::Declare => TransactionTrace::Declare(DeclareTransactionTrace {
validate_invocation,
fee_transfer_invocation,
Expand Down Expand Up @@ -323,7 +320,5 @@ fn to_trace(
function_invocation: maybe_function_invocation,
state_diff,
}),
};

trace
}
}
6 changes: 1 addition & 5 deletions crates/executor/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,11 +178,7 @@ impl From<blockifier::execution::call_info::CallInfo> for FunctionInvocation {
fn from(call_info: blockifier::execution::call_info::CallInfo) -> Self {
let messages = ordered_l2_to_l1_messages(&call_info);

let internal_calls = call_info
.inner_calls
.into_iter()
.map(Into::into)
.collect();
let internal_calls = call_info.inner_calls.into_iter().map(Into::into).collect();

let events = call_info
.execution
Expand Down

0 comments on commit 09c11cd

Please sign in to comment.