diff --git a/.changeset/lucky-cameras-grin.md b/.changeset/lucky-cameras-grin.md new file mode 100644 index 0000000000000..bda5239e292ac --- /dev/null +++ b/.changeset/lucky-cameras-grin.md @@ -0,0 +1,6 @@ +--- +'@eth-optimism/batch-submitter-service': patch +'@eth-optimism/teleportr': patch +--- + +Count reverted transactions in failed_submissions diff --git a/go/bss-core/drivers/clear_pending_tx_test.go b/go/bss-core/drivers/clear_pending_tx_test.go index 45bb31050ef92..985b9c4af8b1c 100644 --- a/go/bss-core/drivers/clear_pending_tx_test.go +++ b/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) diff --git a/go/bss-core/service.go b/go/bss-core/service.go index 2a4de9e8d0a49..6439faa13417b 100644 --- a/go/bss-core/service.go +++ b/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(): diff --git a/go/bss-core/txmgr/txmgr.go b/go/bss-core/txmgr/txmgr.go index ca206da1926a6..8ca41f62aaa33 100644 --- a/go/bss-core/txmgr/txmgr.go +++ b/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 } diff --git a/go/bss-core/txmgr/txmgr_test.go b/go/bss-core/txmgr/txmgr_test.go index 62d8808c281e6..32437a2cb584e 100644 --- a/go/bss-core/txmgr/txmgr_test.go +++ b/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 }