From cc92e8f87764bc40a9dfdf59c3b2f8da2250017f Mon Sep 17 00:00:00 2001 From: Adrian Sutton Date: Fri, 14 Jun 2024 10:08:01 -0400 Subject: [PATCH] txmgr: Include the revert reason in the error when estimateGas fails --- op-e2e/e2eutils/transactions/send.go | 14 +++--------- op-service/errutil/errors.go | 22 +++++++++++++++++++ op-service/errutil/errors_test.go | 32 ++++++++++++++++++++++++++++ op-service/txmgr/txmgr.go | 3 ++- 4 files changed, 59 insertions(+), 12 deletions(-) create mode 100644 op-service/errutil/errors.go create mode 100644 op-service/errutil/errors_test.go diff --git a/op-e2e/e2eutils/transactions/send.go b/op-e2e/e2eutils/transactions/send.go index eb229e3fb4f2..d154a9751598 100644 --- a/op-e2e/e2eutils/transactions/send.go +++ b/op-e2e/e2eutils/transactions/send.go @@ -3,12 +3,12 @@ package transactions import ( "context" "crypto/ecdsa" - "errors" "fmt" "math/big" "testing" "github.com/ethereum-optimism/optimism/op-e2e/e2eutils/wait" + "github.com/ethereum-optimism/optimism/op-service/errutil" "github.com/ethereum-optimism/optimism/op-service/txmgr" "github.com/ethereum/go-ethereum" "github.com/ethereum/go-ethereum/core/types" @@ -20,10 +20,6 @@ import ( type SendTxOpt func(cfg *sendTxCfg) -type ErrWithData interface { - ErrorData() interface{} -} - type sendTxCfg struct { receiptStatus uint64 } @@ -83,11 +79,7 @@ func SendTx(ctx context.Context, client *ethclient.Client, candidate txmgr.TxCan } gas, err := client.EstimateGas(ctx, msg) if err != nil { - var errWithData ErrWithData - if errors.As(err, &errWithData) { - return nil, nil, fmt.Errorf("failed to estimate gas. errdata: %v err: %w", errWithData.ErrorData(), err) - } - return nil, nil, fmt.Errorf("failed to estimate gas: %w", err) + return nil, nil, fmt.Errorf("failed to estimate gas: %w", errutil.TryAddRevertReason(err)) } tx := types.MustSignNewTx(privKey, types.LatestSignerForChainID(chainID), &types.DynamicFeeTx{ @@ -102,7 +94,7 @@ func SendTx(ctx context.Context, client *ethclient.Client, candidate txmgr.TxCan }) err = client.SendTransaction(ctx, tx) if err != nil { - return nil, nil, fmt.Errorf("failed to send transaction: %w", err) + return nil, nil, fmt.Errorf("failed to send transaction: %w", errutil.TryAddRevertReason(err)) } receipt, err := wait.ForReceipt(ctx, client, tx.Hash(), cfg.receiptStatus) if err != nil { diff --git a/op-service/errutil/errors.go b/op-service/errutil/errors.go new file mode 100644 index 000000000000..f04ffe205ba1 --- /dev/null +++ b/op-service/errutil/errors.go @@ -0,0 +1,22 @@ +package errutil + +import ( + "errors" + "fmt" +) + +type errWithData interface { + ErrorData() interface{} +} + +// TryAddRevertReason attempts to extract the revert reason from geth RPC client errors and adds it to the error message. +// This is most useful when attempting to execute gas, as if the transaction reverts this will then show the reason. +func TryAddRevertReason(err error) error { + var errData errWithData + ok := errors.As(err, &errData) + if ok { + return fmt.Errorf("%w, reason: %v", err, errData.ErrorData()) + } else { + return err + } +} diff --git a/op-service/errutil/errors_test.go b/op-service/errutil/errors_test.go new file mode 100644 index 000000000000..77f2d1de8113 --- /dev/null +++ b/op-service/errutil/errors_test.go @@ -0,0 +1,32 @@ +package errutil + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestTryAddRevertReason(t *testing.T) { + t.Run("AddsReason", func(t *testing.T) { + err := stubError{} + result := TryAddRevertReason(err) + require.Contains(t, result.Error(), "kaboom") + }) + + t.Run("ReturnOriginalWhenNoErrorDataMethod", func(t *testing.T) { + err := errors.New("boom") + result := TryAddRevertReason(err) + require.Same(t, err, result) + }) +} + +type stubError struct{} + +func (s stubError) Error() string { + return "where's the" +} + +func (s stubError) ErrorData() interface{} { + return "kaboom" +} diff --git a/op-service/txmgr/txmgr.go b/op-service/txmgr/txmgr.go index 015d6edc767f..2e5a87d073da 100644 --- a/op-service/txmgr/txmgr.go +++ b/op-service/txmgr/txmgr.go @@ -10,6 +10,7 @@ import ( "sync/atomic" "time" + "github.com/ethereum-optimism/optimism/op-service/errutil" "github.com/ethereum/go-ethereum" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/consensus/misc/eip4844" @@ -272,7 +273,7 @@ func (m *SimpleTxManager) craftTx(ctx context.Context, candidate TxCandidate) (* Value: candidate.Value, }) if err != nil { - return nil, fmt.Errorf("failed to estimate gas: %w", err) + return nil, fmt.Errorf("failed to estimate gas: %w", errutil.TryAddRevertReason(err)) } gasLimit = gas }