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.

Sign up for GitHub

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

Jump to bottom

fix: check simulation call trace for reversion #1861

Merged
merged 4 commits into from
Apr 18, 2023
Merged

fix: check simulation call trace for reversion #1861

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
28461ed
fix: check simulation call trace for reversion
iamacook Apr 13, 2023
6274755
fix: adjust error message for call trace
iamacook Apr 13, 2023
114b1a4
fix: error message priority
iamacook Apr 13, 2023
d82f1e6
fix: generalise error
iamacook Apr 13, 2023
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
28 changes: 24 additions & 4 deletions src/components/tx/TxSimulation/SimulationResult.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@ type SimulationResultProps = {
onClose: () => void
}

const getCallTraceErrors = (simulation?: TenderlySimulation) => {
if (!simulation) {
return []
}

return simulation.transaction.call_trace.filter((call) => call.error)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we go for call errors instead of the ExecutionFailure event as written in the comment?

We could iterate over simulation.transaction.logs and filter by log.name === 'ExecutionFailure' && log.raw.address === safeAddress emitted from safeAddress.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For future reference: after discussing this, differing Safe versions emit different events (<1.1.0 emits ExecutionFailed) so the logs aren't consistent.

As such, we've adjusted the code to only extract the call traces if the transaction simulation "succeeds".

}

export const SimulationResult = ({
simulationRequestStatus,
simulation,
Expand All @@ -30,8 +38,13 @@ export const SimulationResult = ({
return null
}

const isSuccess = simulation?.simulation.status

// Safe can emit failure event even though Tenderly simulation succeeds
const isCallTraceError = isSuccess && getCallTraceErrors(simulation).length > 0

// Error
if (requestError || !simulation?.simulation.status) {
if (requestError || !isSuccess || isCallTraceError) {
return (
<Alert severity="error" onClose={onClose} className={css.result}>
<AlertTitle color="error">
Expand All @@ -44,9 +57,16 @@ export const SimulationResult = ({
</Typography>
) : (
<Typography>
The transaction failed during the simulation throwing error <b>{simulation?.transaction.error_message}</b>{' '}
in the contract at <b>{simulation?.transaction.error_info?.address}</b>. Full simulation report is available{' '}
<ExternalLink href={simulationLink}>on Tenderly</ExternalLink>.
{isCallTraceError ? (
<>The transaction failed during the simulation.</>
) : (
<>
The transaction failed during the simulation throwing error{' '}
<b>{simulation?.transaction.error_message}</b> in the contract at{' '}
<b>{simulation?.transaction.error_info?.address}</b>.
</>
)}{' '}
Full simulation report is available <ExternalLink href={simulationLink}>on Tenderly</ExternalLink>.
</Typography>
)}
</Alert>
Expand Down
5 changes: 5 additions & 0 deletions src/components/tx/TxSimulation/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,11 @@ interface Transaction {
timestamp: Date
method: string
decoded_input: null
// Note: manually added (partial keys of `call_trace`)
call_trace: Array<{
error?: string
input: string
}>
}

interface TransactionInfo {
Expand Down