Skip to content

Commit 84e3e31

Browse files
authored
fix(api): Propagate fallback errors in traces (matter-labs#3469)
(hopefully) fixes matter-labs#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.
1 parent c44f7a7 commit 84e3e31

9 files changed

+88
-45
lines changed

core/lib/dal/.sqlx/query-7235e50f9ce4b5c4f6f8325117eaccc7108538405743fe1ad71451d0f1842561.json renamed to core/lib/dal/.sqlx/query-34cb5e326f02cca0dac3483a64d21e30a2a643f7909b7b6803a9708357f8ecbe.json

Lines changed: 9 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

core/lib/dal/.sqlx/query-87f27295de500591f01ed76731df2aed7049c3f44a6d25556967ea867e0caf25.json

Lines changed: 0 additions & 22 deletions
This file was deleted.

core/lib/dal/.sqlx/query-fd68afd6eb8890f01fe646339951d1184afcb08d2bdf310a3fd3fb5b47d4d947.json

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

core/lib/dal/src/blocks_web3_dal.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,8 @@ impl BlocksWeb3Dal<'_, '_> {
567567
SELECT
568568
transactions.hash AS tx_hash,
569569
transactions.index_in_block AS tx_index_in_block,
570-
call_trace
570+
call_trace,
571+
transactions.error AS tx_error
571572
FROM
572573
call_traces
573574
INNER JOIN transactions ON tx_hash = transactions.hash
@@ -583,14 +584,15 @@ impl BlocksWeb3Dal<'_, '_> {
583584
.fetch_all(self.storage)
584585
.await?
585586
.into_iter()
586-
.map(|call_trace| {
587+
.map(|mut call_trace| {
587588
let tx_hash = H256::from_slice(&call_trace.tx_hash);
588589
let index = call_trace.tx_index_in_block.unwrap_or_default() as usize;
589590
let meta = CallTraceMeta {
590591
index_in_block: index,
591592
tx_hash,
592593
block_number: block_number.0,
593594
block_hash,
595+
internal_error: call_trace.tx_error.take(),
594596
};
595597
(call_trace.into_call(protocol_version), meta)
596598
})

core/lib/dal/src/models/storage_transaction.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -591,6 +591,7 @@ pub(crate) struct CallTrace {
591591
pub call_trace: Vec<u8>,
592592
pub tx_hash: Vec<u8>,
593593
pub tx_index_in_block: Option<i32>,
594+
pub tx_error: Option<String>,
594595
}
595596

596597
impl CallTrace {

core/lib/dal/src/transactions_dal.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2233,9 +2233,11 @@ impl TransactionsDal<'_, '_> {
22332233
Ok(sqlx::query!(
22342234
r#"
22352235
SELECT
2236-
call_trace
2236+
call_trace,
2237+
transactions.error AS tx_error
22372238
FROM
22382239
call_traces
2240+
INNER JOIN transactions ON tx_hash = transactions.hash
22392241
WHERE
22402242
tx_hash = $1
22412243
"#,
@@ -2245,14 +2247,15 @@ impl TransactionsDal<'_, '_> {
22452247
.with_arg("tx_hash", &tx_hash)
22462248
.fetch_optional(self.storage)
22472249
.await?
2248-
.map(|call_trace| {
2250+
.map(|mut call_trace| {
22492251
(
22502252
parse_call_trace(&call_trace.call_trace, protocol_version),
22512253
CallTraceMeta {
22522254
index_in_block: row.index_in_block.unwrap_or_default() as usize,
22532255
tx_hash,
22542256
block_number: row.miniblock_number as u32,
22552257
block_hash: H256::from_slice(&row.miniblocks_hash),
2258+
internal_error: call_trace.tx_error.take(),
22562259
},
22572260
)
22582261
}))

core/lib/types/src/debug_flat_call.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,4 +49,11 @@ pub struct CallTraceMeta {
4949
pub tx_hash: H256,
5050
pub block_number: u32,
5151
pub block_hash: H256,
52+
/// Error message associated with the transaction in the sequencer database.
53+
/// Can be used to identify a failed transaction if error information is not
54+
/// recorded otherwise (e.g. out-of-gas errors in early protocol versions).
55+
///
56+
/// Should be seen as a fallback value (e.g. if the trace doesn't contain the error
57+
/// or revert reason).
58+
pub internal_error: Option<String>,
5259
}

core/node/api_server/src/web3/namespaces/debug.rs

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,14 @@ impl DebugNamespace {
3131

3232
pub(crate) fn map_call(
3333
call: Call,
34-
meta: CallTraceMeta,
34+
mut meta: CallTraceMeta,
3535
tracer_option: TracerConfig,
3636
) -> CallTracerResult {
3737
match tracer_option.tracer {
3838
SupportedTracers::CallTracer => CallTracerResult::CallTrace(Self::map_default_call(
3939
call,
4040
tracer_option.tracer_config.only_top_call,
41+
meta.internal_error,
4142
)),
4243
SupportedTracers::FlatCallTracer => {
4344
let mut calls = vec![];
@@ -47,19 +48,24 @@ impl DebugNamespace {
4748
&mut calls,
4849
&mut traces,
4950
tracer_option.tracer_config.only_top_call,
50-
&meta,
51+
&mut meta,
5152
);
5253
CallTracerResult::FlatCallTrace(calls)
5354
}
5455
}
5556
}
56-
pub(crate) fn map_default_call(call: Call, only_top_call: bool) -> DebugCall {
57+
pub(crate) fn map_default_call(
58+
call: Call,
59+
only_top_call: bool,
60+
internal_error: Option<String>,
61+
) -> DebugCall {
5762
let calls = if only_top_call {
5863
vec![]
5964
} else {
65+
// We don't need to propagate the internal error to the nested calls.
6066
call.calls
6167
.into_iter()
62-
.map(|call| Self::map_default_call(call, false))
68+
.map(|call| Self::map_default_call(call, false, None))
6369
.collect()
6470
};
6571
let debug_type = match call.r#type {
@@ -76,7 +82,7 @@ impl DebugNamespace {
7682
value: call.value,
7783
output: web3::Bytes::from(call.output),
7884
input: web3::Bytes::from(call.input),
79-
error: call.error,
85+
error: call.error.or(internal_error),
8086
revert_reason: call.revert_reason,
8187
calls,
8288
}
@@ -87,7 +93,7 @@ impl DebugNamespace {
8793
calls: &mut Vec<DebugCallFlat>,
8894
trace_address: &mut Vec<usize>,
8995
only_top_call: bool,
90-
meta: &CallTraceMeta,
96+
meta: &mut CallTraceMeta,
9197
) {
9298
let subtraces = call.calls.len();
9399
let debug_type = match call.r#type {
@@ -96,16 +102,24 @@ impl DebugNamespace {
96102
CallType::NearCall => unreachable!("We have to filter our near calls before"),
97103
};
98104

99-
let (result, error) = match (call.revert_reason, call.error) {
100-
(Some(revert_reason), _) => {
105+
// We only want to set the internal error for topmost call, so we take it.
106+
let internal_error = meta.internal_error.take();
107+
108+
let (result, error) = match (call.revert_reason, call.error, internal_error) {
109+
(Some(revert_reason), _, _) => {
101110
// If revert_reason exists, it takes priority over VM error
102111
(None, Some(revert_reason))
103112
}
104-
(None, Some(vm_error)) => {
113+
(None, Some(vm_error), _) => {
105114
// If no revert_reason but VM error exists
106115
(None, Some(vm_error))
107116
}
108-
(None, None) => (
117+
(None, None, Some(internal_error)) => {
118+
// No VM error, but there is an error in the sequencer DB.
119+
// Only to be set as a topmost error.
120+
(None, Some(internal_error))
121+
}
122+
(None, None, None) => (
109123
Some(CallResult {
110124
output: web3::Bytes::from(call.output),
111125
gas_used: U256::from(call.gas_used),
@@ -175,23 +189,27 @@ impl DebugNamespace {
175189
SupportedTracers::CallTracer => CallTracerBlockResult::CallTrace(
176190
call_traces
177191
.into_iter()
178-
.map(|(call, _)| ResultDebugCall {
179-
result: Self::map_default_call(call, options.tracer_config.only_top_call),
192+
.map(|(call, meta)| ResultDebugCall {
193+
result: Self::map_default_call(
194+
call,
195+
options.tracer_config.only_top_call,
196+
meta.internal_error,
197+
),
180198
})
181199
.collect(),
182200
),
183201
SupportedTracers::FlatCallTracer => {
184202
let res = call_traces
185203
.into_iter()
186-
.map(|(call, meta)| {
204+
.map(|(call, mut meta)| {
187205
let mut traces = vec![meta.index_in_block];
188206
let mut flat_calls = vec![];
189207
Self::flatten_call(
190208
call,
191209
&mut flat_calls,
192210
&mut traces,
193211
options.tracer_config.only_top_call,
194-
&meta,
212+
&mut meta,
195213
);
196214
ResultDebugCallFlat {
197215
tx_hash: meta.tx_hash,

core/node/api_server/src/web3/tests/debug.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ impl HttpTest for TraceBlockTest {
7373
let expected_calls: Vec<_> = tx_result
7474
.call_traces
7575
.iter()
76-
.map(|call| DebugNamespace::map_default_call(call.clone(), false))
76+
.map(|call| DebugNamespace::map_default_call(call.clone(), false, None))
7777
.collect();
7878
assert_eq!(result.calls, expected_calls);
7979
}
@@ -216,7 +216,7 @@ impl HttpTest for TraceTransactionTest {
216216
let expected_calls: Vec<_> = tx_results[0]
217217
.call_traces
218218
.iter()
219-
.map(|call| DebugNamespace::map_default_call(call.clone(), false))
219+
.map(|call| DebugNamespace::map_default_call(call.clone(), false, None))
220220
.collect();
221221

222222
let result = client

0 commit comments

Comments
 (0)