Skip to content

Commit

Permalink
chore: make artificially shortened executions count as successes (#3204)
Browse files Browse the repository at this point in the history
Currently, successful validations are reported as reverts, which is
super confusing.

Why this is safe:
- `TracerExecutionStopReason::Finished` is only emitted on success,
Errors always emit Abort instead
- Some of the removed code was just dead code. `self.result` was checked
twice.

I'd rather fix the strange behaviour than emulate it to make ShadowVm
line up.
  • Loading branch information
joonazan authored Oct 31, 2024
1 parent 9ad1f0d commit f1328c0
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,10 @@ impl<S: WriteStorage, H: HistoryMode> Tracer for DefaultExecutionTracer<S, H> {
);

match hook {
VmHook::TxHasEnded => self.tx_has_been_processed = true,
VmHook::TxHasEnded if matches!(self.execution_mode, VmExecutionMode::OneTx) => {
self.result_tracer.tx_finished_in_one_tx_mode = true;
self.tx_has_been_processed = true;
}
VmHook::NoValidationEntered => self.in_account_validation = false,
VmHook::AccountValidationEntered => self.in_account_validation = true,
VmHook::FinalBatchInfo => self.final_batch_info_requested = true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ pub(crate) struct ResultTracer<S> {
far_call_tracker: FarCallTracker,
subversion: MultiVMSubversion,

pub(crate) tx_finished_in_one_tx_mode: bool,

_phantom: PhantomData<S>,
}

Expand All @@ -115,6 +117,7 @@ impl<S> ResultTracer<S> {
execution_mode,
far_call_tracker: Default::default(),
subversion,
tx_finished_in_one_tx_mode: false,
_phantom: PhantomData,
}
}
Expand Down Expand Up @@ -297,7 +300,7 @@ impl<S: WriteStorage> ResultTracer<S> {

let has_failed =
tx_has_failed(state, bootloader_state.current_tx() as u32, self.subversion);
if has_failed {
if self.tx_finished_in_one_tx_mode && has_failed {
self.result = Some(Result::Error {
error_reason: VmRevertReason::General {
msg: "Transaction reverted with empty reason. Possibly out of gas"
Expand All @@ -306,9 +309,9 @@ impl<S: WriteStorage> ResultTracer<S> {
},
});
} else {
self.result = Some(self.result.clone().unwrap_or(Result::Success {
self.result = Some(Result::Success {
return_data: vec![],
}));
});
}
}
}
Expand Down

0 comments on commit f1328c0

Please sign in to comment.