txmgr: Include the revert reason in the error when estimateGas fails (#10829)

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 {

op-service/errutil/errors.go

+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
+	}
+}

op-service/errutil/errors_test.go

+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"
+}

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
 	}