Skip to content

Commit

Permalink
fix(api): Propagate fallback errors in traces (#3469)
Browse files Browse the repository at this point in the history
(hopefully) fixes #3394
See the issue for the context.

Looks like some old transactions do not have error stored for out-of-gas
errors. However, we know that they're failed, and we know that based on
[the error field from the transactions
table](https://github.com/matter-labs/zksync-era/blob/main/core/lib/dal/src/models/storage_transaction.rs#L361).

This PR adds a "fallback" error -- if no other error is detected in the
call trace, we use the error from the sequencer.

Granted, I do not have access to the mainnet DB, so not sure what error
messages are inside, but if I am to guess, it will be more or less
adequate. 😅

⚠️ To make things spicier, I have no way to test this functionality (per
@dutterbutter report in the issue, it does not reproduce with the
current protocol version), so it's kind of "hope it helps" fix. Please
be careful during the review.
  • Loading branch information
popzxc authored Jan 14, 2025
1 parent c44f7a7 commit 84e3e31
Show file tree
Hide file tree
Showing 9 changed files with 88 additions and 45 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

This file was deleted.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions core/lib/dal/src/blocks_web3_dal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,8 @@ impl BlocksWeb3Dal<'_, '_> {
SELECT
transactions.hash AS tx_hash,
transactions.index_in_block AS tx_index_in_block,
call_trace
call_trace,
transactions.error AS tx_error
FROM
call_traces
INNER JOIN transactions ON tx_hash = transactions.hash
Expand All @@ -583,14 +584,15 @@ impl BlocksWeb3Dal<'_, '_> {
.fetch_all(self.storage)
.await?
.into_iter()
.map(|call_trace| {
.map(|mut call_trace| {
let tx_hash = H256::from_slice(&call_trace.tx_hash);
let index = call_trace.tx_index_in_block.unwrap_or_default() as usize;
let meta = CallTraceMeta {
index_in_block: index,
tx_hash,
block_number: block_number.0,
block_hash,
internal_error: call_trace.tx_error.take(),
};
(call_trace.into_call(protocol_version), meta)
})
Expand Down
1 change: 1 addition & 0 deletions core/lib/dal/src/models/storage_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,7 @@ pub(crate) struct CallTrace {
pub call_trace: Vec<u8>,
pub tx_hash: Vec<u8>,
pub tx_index_in_block: Option<i32>,
pub tx_error: Option<String>,
}

impl CallTrace {
Expand Down
7 changes: 5 additions & 2 deletions core/lib/dal/src/transactions_dal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2233,9 +2233,11 @@ impl TransactionsDal<'_, '_> {
Ok(sqlx::query!(
r#"
SELECT
call_trace
call_trace,
transactions.error AS tx_error
FROM
call_traces
INNER JOIN transactions ON tx_hash = transactions.hash
WHERE
tx_hash = $1
"#,
Expand All @@ -2245,14 +2247,15 @@ impl TransactionsDal<'_, '_> {
.with_arg("tx_hash", &tx_hash)
.fetch_optional(self.storage)
.await?
.map(|call_trace| {
.map(|mut call_trace| {
(
parse_call_trace(&call_trace.call_trace, protocol_version),
CallTraceMeta {
index_in_block: row.index_in_block.unwrap_or_default() as usize,
tx_hash,
block_number: row.miniblock_number as u32,
block_hash: H256::from_slice(&row.miniblocks_hash),
internal_error: call_trace.tx_error.take(),
},
)
}))
Expand Down
7 changes: 7 additions & 0 deletions core/lib/types/src/debug_flat_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,11 @@ pub struct CallTraceMeta {
pub tx_hash: H256,
pub block_number: u32,
pub block_hash: H256,
/// Error message associated with the transaction in the sequencer database.
/// Can be used to identify a failed transaction if error information is not
/// recorded otherwise (e.g. out-of-gas errors in early protocol versions).
///
/// Should be seen as a fallback value (e.g. if the trace doesn't contain the error
/// or revert reason).
pub internal_error: Option<String>,
}
46 changes: 32 additions & 14 deletions core/node/api_server/src/web3/namespaces/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,14 @@ impl DebugNamespace {

pub(crate) fn map_call(
call: Call,
meta: CallTraceMeta,
mut meta: CallTraceMeta,
tracer_option: TracerConfig,
) -> CallTracerResult {
match tracer_option.tracer {
SupportedTracers::CallTracer => CallTracerResult::CallTrace(Self::map_default_call(
call,
tracer_option.tracer_config.only_top_call,
meta.internal_error,
)),
SupportedTracers::FlatCallTracer => {
let mut calls = vec![];
Expand All @@ -47,19 +48,24 @@ impl DebugNamespace {
&mut calls,
&mut traces,
tracer_option.tracer_config.only_top_call,
&meta,
&mut meta,
);
CallTracerResult::FlatCallTrace(calls)
}
}
}
pub(crate) fn map_default_call(call: Call, only_top_call: bool) -> DebugCall {
pub(crate) fn map_default_call(
call: Call,
only_top_call: bool,
internal_error: Option<String>,
) -> DebugCall {
let calls = if only_top_call {
vec![]
} else {
// We don't need to propagate the internal error to the nested calls.
call.calls
.into_iter()
.map(|call| Self::map_default_call(call, false))
.map(|call| Self::map_default_call(call, false, None))
.collect()
};
let debug_type = match call.r#type {
Expand All @@ -76,7 +82,7 @@ impl DebugNamespace {
value: call.value,
output: web3::Bytes::from(call.output),
input: web3::Bytes::from(call.input),
error: call.error,
error: call.error.or(internal_error),
revert_reason: call.revert_reason,
calls,
}
Expand All @@ -87,7 +93,7 @@ impl DebugNamespace {
calls: &mut Vec<DebugCallFlat>,
trace_address: &mut Vec<usize>,
only_top_call: bool,
meta: &CallTraceMeta,
meta: &mut CallTraceMeta,
) {
let subtraces = call.calls.len();
let debug_type = match call.r#type {
Expand All @@ -96,16 +102,24 @@ impl DebugNamespace {
CallType::NearCall => unreachable!("We have to filter our near calls before"),
};

let (result, error) = match (call.revert_reason, call.error) {
(Some(revert_reason), _) => {
// We only want to set the internal error for topmost call, so we take it.
let internal_error = meta.internal_error.take();

let (result, error) = match (call.revert_reason, call.error, internal_error) {
(Some(revert_reason), _, _) => {
// If revert_reason exists, it takes priority over VM error
(None, Some(revert_reason))
}
(None, Some(vm_error)) => {
(None, Some(vm_error), _) => {
// If no revert_reason but VM error exists
(None, Some(vm_error))
}
(None, None) => (
(None, None, Some(internal_error)) => {
// No VM error, but there is an error in the sequencer DB.
// Only to be set as a topmost error.
(None, Some(internal_error))
}
(None, None, None) => (
Some(CallResult {
output: web3::Bytes::from(call.output),
gas_used: U256::from(call.gas_used),
Expand Down Expand Up @@ -175,23 +189,27 @@ impl DebugNamespace {
SupportedTracers::CallTracer => CallTracerBlockResult::CallTrace(
call_traces
.into_iter()
.map(|(call, _)| ResultDebugCall {
result: Self::map_default_call(call, options.tracer_config.only_top_call),
.map(|(call, meta)| ResultDebugCall {
result: Self::map_default_call(
call,
options.tracer_config.only_top_call,
meta.internal_error,
),
})
.collect(),
),
SupportedTracers::FlatCallTracer => {
let res = call_traces
.into_iter()
.map(|(call, meta)| {
.map(|(call, mut meta)| {
let mut traces = vec![meta.index_in_block];
let mut flat_calls = vec![];
Self::flatten_call(
call,
&mut flat_calls,
&mut traces,
options.tracer_config.only_top_call,
&meta,
&mut meta,
);
ResultDebugCallFlat {
tx_hash: meta.tx_hash,
Expand Down
4 changes: 2 additions & 2 deletions core/node/api_server/src/web3/tests/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ impl HttpTest for TraceBlockTest {
let expected_calls: Vec<_> = tx_result
.call_traces
.iter()
.map(|call| DebugNamespace::map_default_call(call.clone(), false))
.map(|call| DebugNamespace::map_default_call(call.clone(), false, None))
.collect();
assert_eq!(result.calls, expected_calls);
}
Expand Down Expand Up @@ -216,7 +216,7 @@ impl HttpTest for TraceTransactionTest {
let expected_calls: Vec<_> = tx_results[0]
.call_traces
.iter()
.map(|call| DebugNamespace::map_default_call(call.clone(), false))
.map(|call| DebugNamespace::map_default_call(call.clone(), false, None))
.collect();

let result = client
Expand Down

0 comments on commit 84e3e31

Please sign in to comment.