Skip to content

Commit

Permalink
feat: eth: return revert data on failed gas estimation (#10298)
Browse files Browse the repository at this point in the history
Unfortunately, we need to execute the message twice to get this (unless
we want to change some APIs). But it's unlikely to be a performance
issue and will definitely help people debug failures.
  • Loading branch information
Stebalien authored Feb 17, 2023
1 parent d589443 commit 30615a4
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 9 deletions.
21 changes: 15 additions & 6 deletions itests/fevm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -901,14 +901,23 @@ func TestFEVMErrorParsing(t *testing.T) {
"failCustom()": customError,
} {
sig := sig
expected := expected
expected := fmt.Sprintf("exit 33, revert reason: %s, vm error", expected)
t.Run(sig, func(t *testing.T) {
entryPoint := kit.CalcFuncSignature(sig)
_, err := e.EthCall(ctx, ethtypes.EthCall{
To: &contractAddrEth,
Data: entryPoint,
}, "latest")
require.ErrorContains(t, err, fmt.Sprintf("exit 33, revert reason: %s, vm error", expected))
t.Run("EthCall", func(t *testing.T) {
_, err := e.EthCall(ctx, ethtypes.EthCall{
To: &contractAddrEth,
Data: entryPoint,
}, "latest")
require.ErrorContains(t, err, expected)
})
t.Run("EthEstimateGas", func(t *testing.T) {
_, err := e.EthEstimateGas(ctx, ethtypes.EthCall{
To: &contractAddrEth,
Data: entryPoint,
})
require.ErrorContains(t, err, expected)
})
})
}
}
16 changes: 13 additions & 3 deletions node/impl/full/eth.go
Original file line number Diff line number Diff line change
Expand Up @@ -888,12 +888,22 @@ func (a *EthModule) EthEstimateGas(ctx context.Context, tx ethtypes.EthCall) (et
msg.GasLimit = 0

ts := a.Chain.GetHeaviestTipSet()
msg, err = a.GasAPI.GasEstimateMessageGas(ctx, msg, nil, ts.Key())
if err != nil {
gassedMsg, err := a.GasAPI.GasEstimateMessageGas(ctx, msg, nil, ts.Key())
if err != nil {
// On failure, GasEstimateMessageGas doesn't actually return the invocation result,
// it just returns an error. That means we can't get the revert reason.
//
// So we re-execute the message with EthCall (well, applyMessage which contains the
// guts of EthCall). This will give us an ethereum specific error with revert
// information.
msg.GasLimit = build.BlockGasLimit
if _, err2 := a.applyMessage(ctx, msg, ts.Key()); err2 != nil {
err = err2
}
return ethtypes.EthUint64(0), xerrors.Errorf("failed to estimate gas: %w", err)
}

expectedGas, err := ethGasSearch(ctx, a.Chain, a.Stmgr, a.Mpool, msg, ts)
expectedGas, err := ethGasSearch(ctx, a.Chain, a.Stmgr, a.Mpool, gassedMsg, ts)
if err != nil {
log.Errorw("expected gas", "err", err)
}
Expand Down

0 comments on commit 30615a4

Please sign in to comment.