Merge pull request #2338 from cfromknecht/bss-core-txmgr-error-revert

feat: txmgr return ErrReverted if confirmed tx reverts

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