feat: txmgr return ErrReverted if confirmed tx reverts

Currently the txmgr treats confirmation of a transaction as successful
if it achieves the proper confirmation depth, however it fails to
acknowledge the possibility that the transaction reverts. Thus to an
external caller the transaction silently succeeds.

This commit now inspects the receipt and returns an ErrReverted failure
in the event that receipt.Status is not 1. This allows higher level
applications to add logging/telemetry or take action on these events.

.changeset/lucky-cameras-grin.md

+---
+'@eth-optimism/batch-submitter-service': patch
+'@eth-optimism/teleportr': patch
+---
+
+Count reverted transactions in failed_submissions

go/bss-core/drivers/clear_pending_tx_test.go

@@ -231,6 +231,7 @@ func TestClearPendingTxClearingTxConfirms(t *testing.T) {
 			return &types.Receipt{
 				TxHash:      txHash,
 				BlockNumber: big.NewInt(int64(testBlockNumber)),
+				Status:      types.ReceiptStatusSuccessful,
 			}, nil
 		},
 	})
@@ -296,6 +297,7 @@ func TestClearPendingTxMultipleConfs(t *testing.T) {
 			return &types.Receipt{
 				TxHash:      txHash,
 				BlockNumber: big.NewInt(int64(testBlockNumber)),
+				Status:      types.ReceiptStatusSuccessful,
 			}, nil
 		},
 	}, numConfs)

go/bss-core/service.go

@@ -215,6 +215,18 @@ func (s *Service) eventLoop() {
 			receipt, err := s.txMgr.Send(
 				s.ctx, updateGasPrice, s.cfg.Driver.SendTransaction,
 			)
+
+			// Record the confirmation time and gas used if we receive a
+			// receipt, as this indicates the transaction confirmed. We record
+			// these metrics here as the transaction may have reverted, and will
+			// abort below.
+			if receipt != nil {
+				batchConfirmationTime := time.Since(batchConfirmationStart) /
+					time.Millisecond
+				s.metrics.BatchConfirmationTimeMs().Set(float64(batchConfirmationTime))
+				s.metrics.SubmissionGasUsedWei().Set(float64(receipt.GasUsed))
+			}
+
 			if err != nil {
 				log.Error(name+" unable to publish batch tx",
 					"err", err)
@@ -225,11 +237,7 @@ func (s *Service) eventLoop() {
 			// The transaction was successfully submitted.
 			log.Info(name+" batch tx successfully published",
 				"tx_hash", receipt.TxHash)
-			batchConfirmationTime := time.Since(batchConfirmationStart) /
-				time.Millisecond
-			s.metrics.BatchConfirmationTimeMs().Set(float64(batchConfirmationTime))
 			s.metrics.BatchesSubmitted().Inc()
-			s.metrics.SubmissionGasUsedWei().Set(float64(receipt.GasUsed))
 			s.metrics.SubmissionTimestamp().Set(float64(time.Now().UnixNano() / 1e6))
 
 		case err := <-s.ctx.Done():

go/bss-core/txmgr/txmgr.go

@@ -2,6 +2,7 @@ package txmgr
 
 import (
 	"context"
+	"errors"
 	"math/big"
 	"strings"
 	"sync"
@@ -12,6 +13,9 @@ import (
 	"github.com/ethereum/go-ethereum/log"
 )
 
+// ErrReverted signals that a mined transaction reverted.
+var ErrReverted = errors.New("transaction reverted")
+
 // UpdateGasPriceSendTxFunc defines a function signature for publishing a
 // desired tx with a specific gas price. Implementations of this signature
 // should also return promptly when the context is canceled.
@@ -225,6 +229,9 @@ func (m *SimpleTxManager) Send(
 
 		// The transaction has confirmed.
 		case receipt := <-receiptChan:
+			if receipt.Status == types.ReceiptStatusFailed {
+				return receipt, ErrReverted
+			}
 			return receipt, nil
 		}
 	}
@@ -288,7 +295,10 @@ func waitMined(
 			// tipHeight. The equation is rewritten in this form to avoid
 			// underflows.
 			if txHeight+numConfirmations <= tipHeight+1 {
-				log.Info("Transaction confirmed", "txHash", txHash)
+				reverted := receipt.Status == types.ReceiptStatusFailed
+				log.Info("Transaction confirmed",
+					"txHash", txHash,
+					"reverted", reverted)
 				return receipt, nil
 			}
 

go/bss-core/txmgr/txmgr_test.go

@@ -98,6 +98,7 @@ func (g *gasPricer) sample() (*big.Int, *big.Int) {
 type minedTxInfo struct {
 	gasFeeCap   *big.Int
 	blockNumber uint64
+	reverted    bool
 }
 
 // mockBackend implements txmgr.ReceiptSource that tracks mined transactions
@@ -123,6 +124,20 @@ func newMockBackend() *mockBackend {
 // TransactionReceipt with a matching txHash will result in a non-nil receipt.
 // If a nil txHash is supplied this has the effect of mining an empty block.
 func (b *mockBackend) mine(txHash *common.Hash, gasFeeCap *big.Int) {
+	b.mineWithStatus(txHash, gasFeeCap, false)
+}
+
+// mineWithStatus records a (txHash, gasFeeCap) pair as confirmed, but also
+// includes the option to specify whether or not the transaction reverted.
+// Subsequent calls to TransactionReceipt with a matching txHash will result in
+// a non-nil receipt. If a nil txHash is supplied this has the effect of mining
+// an empty block.
+func (b *mockBackend) mineWithStatus(
+	txHash *common.Hash,
+	gasFeeCap *big.Int,
+	revert bool,
+) {
+
 	b.mu.Lock()
 	defer b.mu.Unlock()
 
@@ -131,6 +146,7 @@ func (b *mockBackend) mine(txHash *common.Hash, gasFeeCap *big.Int) {
 		b.minedTxs[*txHash] = minedTxInfo{
 			gasFeeCap:   gasFeeCap,
 			blockNumber: b.blockHeight,
+			reverted:    revert,
 		}
 	}
 }
@@ -160,12 +176,18 @@ func (b *mockBackend) TransactionReceipt(
 		return nil, nil
 	}
 
+	var status = types.ReceiptStatusSuccessful
+	if txInfo.reverted {
+		status = types.ReceiptStatusFailed
+	}
+
 	// Return the gas fee cap for the transaction in the GasUsed field so that
 	// we can assert the proper tx confirmed in our tests.
 	return &types.Receipt{
 		TxHash:      txHash,
 		GasUsed:     txInfo.gasFeeCap.Uint64(),
 		BlockNumber: big.NewInt(int64(txInfo.blockNumber)),
+		Status:      status,
 	}, nil
 }
 
@@ -201,6 +223,39 @@ func TestTxMgrConfirmAtMinGasPrice(t *testing.T) {
 	require.Equal(t, gasPricer.expGasFeeCap().Uint64(), receipt.GasUsed)
 }
 
+// TestTxMgrFailsForRevertedTxn asserts that Send returns ErrReverted if the
+// confirmed transaction reverts during execution, and returns the resulting
+// receipt.
+func TestTxMgrFailsForRevertedTxn(t *testing.T) {
+	t.Parallel()
+
+	h := newTestHarness()
+
+	gasPricer := newGasPricer(1)
+
+	updateGasPrice := func(ctx context.Context) (*types.Transaction, error) {
+		gasTipCap, gasFeeCap := gasPricer.sample()
+		return types.NewTx(&types.DynamicFeeTx{
+			GasTipCap: gasTipCap,
+			GasFeeCap: gasFeeCap,
+		}), nil
+	}
+
+	sendTx := func(ctx context.Context, tx *types.Transaction) error {
+		if gasPricer.shouldMine(tx.GasFeeCap()) {
+			txHash := tx.Hash()
+			h.backend.mineWithStatus(&txHash, tx.GasFeeCap(), true)
+		}
+		return nil
+	}
+
+	ctx := context.Background()
+	receipt, err := h.mgr.Send(ctx, updateGasPrice, sendTx)
+	require.Equal(t, txmgr.ErrReverted, err)
+	require.NotNil(t, receipt)
+	require.Equal(t, gasPricer.expGasFeeCap().Uint64(), receipt.GasUsed)
+}
+
 // TestTxMgrNeverConfirmCancel asserts that a Send can be canceled even if no
 // transaction is mined. This is done to ensure the the tx mgr can properly
 // abort on shutdown, even if a txn is in the process of being published.
@@ -519,6 +574,7 @@ func (b *failingBackend) TransactionReceipt(
 	return &types.Receipt{
 		TxHash:      txHash,
 		BlockNumber: big.NewInt(1),
+		Status:      types.ReceiptStatusSuccessful,
 	}, nil
 }