Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(api): Propagate fallback errors in traces #3469

Merged
merged 1 commit into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

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
Loading