feat: split up SubmitBatchTx driver method

Rather than having a combined update the gas price and publish the tx,
we split this this into two methods: UpdateGasPrice and SendTransaction.
This will allow the txmgr to obtain the txHash before publishing the
tx.

go/batch-submitter/drivers/proposer/driver.go

@@ -227,10 +227,11 @@ func (d *Driver) CraftBatchTx(
 	}
 }
 
-// SubmitBatchTx using the passed transaction as a template, signs and
-// publishes the transaction unmodified apart from sampling the current gas
-// price. The final transaction is returned to the caller.
-func (d *Driver) SubmitBatchTx(
+// UpdateGasPrice signs an otherwise identical txn to the one provided but with
+// updated gas prices sampled from the existing network conditions.
+//
+// NOTE: Thie method SHOULD NOT publish the resulting transaction.
+func (d *Driver) UpdateGasPrice(
 	ctx context.Context,
 	tx *types.Transaction,
 ) (*types.Transaction, error) {
@@ -243,6 +244,7 @@ func (d *Driver) SubmitBatchTx(
 	}
 	opts.Context = ctx
 	opts.Nonce = new(big.Int).SetUint64(tx.Nonce())
+	opts.NoSend = true
 
 	finalTx, err := d.rawSccContract.RawTransact(opts, tx.Data())
 	switch {
@@ -265,3 +267,12 @@ func (d *Driver) SubmitBatchTx(
 		return nil, err
 	}
 }
+
+// SendTransaction injects a signed transaction into the pending pool for
+// execution.
+func (d *Driver) SendTransaction(
+	ctx context.Context,
+	tx *types.Transaction,
+) error {
+	return d.cfg.L1Client.SendTransaction(ctx, tx)
+}

go/batch-submitter/drivers/sequencer/driver.go

@@ -257,10 +257,11 @@ func (d *Driver) CraftBatchTx(
 	}
 }
 
-// SubmitBatchTx using the passed transaction as a template, signs and publishes
-// the transaction unmodified apart from sampling the current gas price. The
-// final transaction is returned to the caller.
-func (d *Driver) SubmitBatchTx(
+// UpdateGasPrice signs an otherwise identical txn to the one provided but with
+// updated gas prices sampled from the existing network conditions.
+//
+// NOTE: Thie method SHOULD NOT publish the resulting transaction.
+func (d *Driver) UpdateGasPrice(
 	ctx context.Context,
 	tx *types.Transaction,
 ) (*types.Transaction, error) {
@@ -273,6 +274,7 @@ func (d *Driver) SubmitBatchTx(
 	}
 	opts.Context = ctx
 	opts.Nonce = new(big.Int).SetUint64(tx.Nonce())
+	opts.NoSend = true
 
 	finalTx, err := d.rawCtcContract.RawTransact(opts, tx.Data())
 	switch {
@@ -295,3 +297,12 @@ func (d *Driver) SubmitBatchTx(
 		return nil, err
 	}
 }
+
+// SendTransaction injects a signed transaction into the pending pool for
+// execution.
+func (d *Driver) SendTransaction(
+	ctx context.Context,
+	tx *types.Transaction,
+) error {
+	return d.cfg.L1Client.SendTransaction(ctx, tx)
+}

go/bss-core/drivers/clear_pending_tx.go

@@ -48,7 +48,7 @@ func ClearPendingTx(
 	// Construct the clearing transaction submission clousure that will attempt
 	// to send the a clearing transaction transaction at the given nonce and gas
 	// price.
-	sendTx := func(
+	updateGasPrice := func(
 		ctx context.Context,
 	) (*types.Transaction, error) {
 		log.Info(name+" clearing pending tx", "nonce", nonce)
@@ -61,11 +61,16 @@ func ClearPendingTx(
 				"err", err)
 			return nil, err
 		}
-		txHash := signedTx.Hash()
-		gasTipCap := signedTx.GasTipCap()
-		gasFeeCap := signedTx.GasFeeCap()
 
-		err = l1Client.SendTransaction(ctx, signedTx)
+		return signedTx, nil
+	}
+
+	sendTx := func(ctx context.Context, tx *types.Transaction) error {
+		txHash := tx.Hash()
+		gasTipCap := tx.GasTipCap()
+		gasFeeCap := tx.GasFeeCap()
+
+		err := l1Client.SendTransaction(ctx, tx)
 		switch {
 
 		// Clearing transaction successfully confirmed.
@@ -74,7 +79,7 @@ func ClearPendingTx(
 				"gasTipCap", gasTipCap, "gasFeeCap", gasFeeCap,
 				"txHash", txHash)
 
-			return signedTx, nil
+			return nil
 
 		// Getting a nonce too low error implies that a previous transaction in
 		// the mempool has confirmed and we should abort trying to publish at
@@ -83,7 +88,7 @@ func ClearPendingTx(
 			log.Info(name + " transaction from previous restart confirmed, " +
 				"aborting mempool clearing")
 			cancel()
-			return nil, context.Canceled
+			return context.Canceled
 
 		// An unexpected error occurred. This also handles the case where the
 		// clearing transaction has not yet bested the gas price a prior
@@ -94,11 +99,11 @@ func ClearPendingTx(
 			log.Error(name+" unable to submit clearing tx",
 				"nonce", nonce, "gasTipCap", gasTipCap, "gasFeeCap", gasFeeCap,
 				"txHash", txHash, "err", err)
-			return nil, err
+			return err
 		}
 	}
 
-	receipt, err := txMgr.Send(ctx, sendTx)
+	receipt, err := txMgr.Send(ctx, updateGasPrice, sendTx)
 	switch {
 
 	// If the current context is canceled, a prior transaction in the mempool

go/bss-core/service.go

@@ -54,13 +54,18 @@ type Driver interface {
 		start, end, nonce *big.Int,
 	) (*types.Transaction, error)
 
-	// SubmitBatchTx using the passed transaction as a template, signs and
-	// publishes the transaction unmodified apart from sampling the current gas
-	// price. The final transaction is returned to the caller.
-	SubmitBatchTx(
+	// UpdateGasPrice signs an otherwise identical txn to the one provided but
+	// with updated gas prices sampled from the existing network conditions.
+	//
+	// NOTE: Thie method SHOULD NOT publish the resulting transaction.
+	UpdateGasPrice(
 		ctx context.Context,
 		tx *types.Transaction,
 	) (*types.Transaction, error)
+
+	// SendTransaction injects a signed transaction into the pending pool for
+	// execution.
+	SendTransaction(ctx context.Context, tx *types.Transaction) error
 }
 
 type ServiceConfig struct {
@@ -193,30 +198,19 @@ func (s *Service) eventLoop() {
 
 			// Construct the transaction submission clousure that will attempt
 			// to send the next transaction at the given nonce and gas price.
-			sendTx := func(ctx context.Context) (*types.Transaction, error) {
-				log.Info(name+" attempting batch tx", "start", start,
+			updateGasPrice := func(ctx context.Context) (*types.Transaction, error) {
+				log.Info(name+" updating batch tx gas price", "start", start,
 					"end", end, "nonce", nonce)
 
-				tx, err := s.cfg.Driver.SubmitBatchTx(ctx, tx)
-				if err != nil {
-					return nil, err
-				}
-
-				log.Info(
-					name+" submitted batch tx",
-					"start", start,
-					"end", end,
-					"nonce", nonce,
-					"tx_hash", tx.Hash(),
-				)
-
-				return tx, nil
+				return s.cfg.Driver.UpdateGasPrice(ctx, tx)
 			}
 
 			// Wait until one of our submitted transactions confirms. If no
 			// receipt is received it's likely our gas price was too low.
 			batchConfirmationStart := time.Now()
-			receipt, err := s.txMgr.Send(s.ctx, sendTx)
+			receipt, err := s.txMgr.Send(
+				s.ctx, updateGasPrice, s.cfg.Driver.SendTransaction,
+			)
 			if err != nil {
 				log.Error(name+" unable to publish batch tx",
 					"err", err)

go/bss-core/txmgr/txmgr.go

@@ -13,10 +13,12 @@ import (
 	"github.com/ethereum/go-ethereum/log"
 )
 
-// SendTxFunc 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.
-type SendTxFunc = func(ctx context.Context) (*types.Transaction, error)
+// 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.
+type UpdateGasPriceFunc = func(ctx context.Context) (*types.Transaction, error)
+
+type SendTransactionFunc = func(ctx context.Context, tx *types.Transaction) error
 
 // Config houses parameters for altering the behavior of a SimpleTxManager.
 type Config struct {
@@ -48,7 +50,11 @@ type TxManager interface {
 	// prices). The method may be canceled using the passed context.
 	//
 	// NOTE: Send should be called by AT MOST one caller at a time.
-	Send(ctx context.Context, sendTx SendTxFunc) (*types.Receipt, error)
+	Send(
+		ctx context.Context,
+		updateGasPrice UpdateGasPriceFunc,
+		sendTxn SendTransactionFunc,
+	) (*types.Receipt, error)
 }
 
 // ReceiptSource is a minimal function signature used to detect the confirmation
@@ -96,7 +102,10 @@ func NewSimpleTxManager(
 //
 // NOTE: Send should be called by AT MOST one caller at a time.
 func (m *SimpleTxManager) Send(
-	ctx context.Context, sendTx SendTxFunc) (*types.Receipt, error) {
+	ctx context.Context,
+	updateGasPrice UpdateGasPriceFunc,
+	sendTx SendTransactionFunc,
+) (*types.Receipt, error) {
 
 	name := m.name
 
@@ -119,8 +128,25 @@ func (m *SimpleTxManager) Send(
 	sendTxAsync := func() {
 		defer wg.Done()
 
+		tx, err := updateGasPrice(ctxc)
+		if err != nil {
+			if err == context.Canceled ||
+				strings.Contains(err.Error(), "context canceled") {
+				return
+			}
+			log.Error(name+" unable to update txn gas price", "err", err)
+			return
+		}
+
+		txHash := tx.Hash()
+		nonce := tx.Nonce()
+		gasTipCap := tx.GasTipCap()
+		gasFeeCap := tx.GasFeeCap()
+		log.Info(name+" publishing transaction", "txHash", txHash,
+			"nonce", nonce, "gasTipCap", gasTipCap, "gasFeeCap", gasFeeCap)
+
 		// Sign and publish transaction with current gas price.
-		tx, err := sendTx(ctxc)
+		err = sendTx(ctxc, tx)
 		if err != nil {
 			if err == context.Canceled ||
 				strings.Contains(err.Error(), "context canceled") {
@@ -134,11 +160,8 @@ func (m *SimpleTxManager) Send(
 			return
 		}
 
-		txHash := tx.Hash()
-		gasTipCap := tx.GasTipCap()
-		gasFeeCap := tx.GasFeeCap()
 		log.Info(name+" transaction published successfully", "hash", txHash,
-			"gasTipCap", gasTipCap, "gasFeeCap", gasFeeCap)
+			"nonce", nonce, "gasTipCap", gasTipCap, "gasFeeCap", gasFeeCap)
 
 		// Wait for the transaction to be mined, reporting the receipt
 		// back to the main event loop if found.
@@ -148,7 +171,8 @@ func (m *SimpleTxManager) Send(
 		)
 		if err != nil {
 			log.Debug(name+" send tx failed", "hash", txHash,
-				"gasTipCap", gasTipCap, "gasFeeCap", gasFeeCap, "err", err)
+				"nonce", nonce, "gasTipCap", gasTipCap, "gasFeeCap", gasFeeCap,
+				"err", err)
 		}
 		if receipt != nil {
 			// Use non-blocking select to ensure function can exit
@@ -156,7 +180,8 @@ func (m *SimpleTxManager) Send(
 			select {
 			case receiptChan <- receipt:
 				log.Trace(name+" send tx succeeded", "hash", txHash,
-					"gasTipCap", gasTipCap, "gasFeeCap", gasFeeCap)
+					"nonce", nonce, "gasTipCap", gasTipCap,
+					"gasFeeCap", gasFeeCap)
 			default:
 			}
 		}

go/bss-core/txmgr/txmgr_test.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/ethereum-optimism/optimism/go/bss-core/txmgr"
 	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/core"
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/stretchr/testify/require"
 )
@@ -71,6 +72,10 @@ func (g *gasPricer) expGasFeeCap() *big.Int {
 	return gasFeeCap
 }
 
+func (g *gasPricer) shouldMine(gasFeeCap *big.Int) bool {
+	return g.expGasFeeCap().Cmp(gasFeeCap) == 0
+}
+
 func (g *gasPricer) feesForEpoch(epoch int64) (*big.Int, *big.Int) {
 	epochBaseFee := new(big.Int).Mul(g.baseBaseFee, big.NewInt(epoch))
 	epochGasTipCap := new(big.Int).Mul(g.baseGasTipFee, big.NewInt(epoch))
@@ -79,15 +84,14 @@ func (g *gasPricer) feesForEpoch(epoch int64) (*big.Int, *big.Int) {
 	return epochGasTipCap, epochGasFeeCap
 }
 
-func (g *gasPricer) sample() (*big.Int, *big.Int, bool) {
+func (g *gasPricer) sample() (*big.Int, *big.Int) {
 	g.mu.Lock()
 	defer g.mu.Unlock()
 
 	g.epoch++
 	epochGasTipCap, epochGasFeeCap := g.feesForEpoch(g.epoch)
-	shouldMine := g.epoch == g.mineAtEpoch
 
-	return epochGasTipCap, epochGasFeeCap, shouldMine
+	return epochGasTipCap, epochGasFeeCap
 }
 
 type minedTxInfo struct {
@@ -171,23 +175,29 @@ func TestTxMgrConfirmAtMinGasPrice(t *testing.T) {
 
 	h := newTestHarness()
 
-	gasFeeCap := big.NewInt(5)
-	sendTxFunc := func(
-		ctx context.Context,
-	) (*types.Transaction, error) {
-		tx := types.NewTx(&types.DynamicFeeTx{
+	gasPricer := newGasPricer(1)
+
+	updateGasPrice := func(ctx context.Context) (*types.Transaction, error) {
+		gasTipCap, gasFeeCap := gasPricer.sample()
+		return types.NewTx(&types.DynamicFeeTx{
+			GasTipCap: gasTipCap,
 			GasFeeCap: gasFeeCap,
-		})
-		txHash := tx.Hash()
-		h.backend.mine(&txHash, gasFeeCap)
-		return tx, nil
+		}), nil
+	}
+
+	sendTx := func(ctx context.Context, tx *types.Transaction) error {
+		if gasPricer.shouldMine(tx.GasFeeCap()) {
+			txHash := tx.Hash()
+			h.backend.mine(&txHash, tx.GasFeeCap())
+		}
+		return nil
 	}
 
 	ctx := context.Background()
-	receipt, err := h.mgr.Send(ctx, sendTxFunc)
+	receipt, err := h.mgr.Send(ctx, updateGasPrice, sendTx)
 	require.Nil(t, err)
 	require.NotNil(t, receipt)
-	require.Equal(t, gasFeeCap.Uint64(), receipt.GasUsed)
+	require.Equal(t, gasPricer.expGasFeeCap().Uint64(), receipt.GasUsed)
 }
 
 // TestTxMgrNeverConfirmCancel asserts that a Send can be canceled even if no
@@ -198,19 +208,23 @@ func TestTxMgrNeverConfirmCancel(t *testing.T) {
 
 	h := newTestHarness()
 
-	sendTxFunc := func(
-		ctx context.Context,
-	) (*types.Transaction, error) {
-		// Don't publish tx to backend, simulating never being mined.
+	updateGasPrice := func(ctx context.Context) (*types.Transaction, error) {
+		gasTipCap, gasFeeCap := h.gasPricer.sample()
 		return types.NewTx(&types.DynamicFeeTx{
-			GasFeeCap: big.NewInt(5),
+			GasTipCap: gasTipCap,
+			GasFeeCap: gasFeeCap,
 		}), nil
 	}
 
+	sendTx := func(ctx context.Context, tx *types.Transaction) error {
+		// Don't publish tx to backend, simulating never being mined.
+		return nil
+	}
+
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 	defer cancel()
 
-	receipt, err := h.mgr.Send(ctx, sendTxFunc)
+	receipt, err := h.mgr.Send(ctx, updateGasPrice, sendTx)
 	require.Equal(t, err, context.DeadlineExceeded)
 	require.Nil(t, receipt)
 }
@@ -222,23 +236,24 @@ func TestTxMgrConfirmsAtHigherGasPrice(t *testing.T) {
 
 	h := newTestHarness()
 
-	sendTxFunc := func(
-		ctx context.Context,
-	) (*types.Transaction, error) {
-		gasTipCap, gasFeeCap, shouldMine := h.gasPricer.sample()
-		tx := types.NewTx(&types.DynamicFeeTx{
+	updateGasPrice := func(ctx context.Context) (*types.Transaction, error) {
+		gasTipCap, gasFeeCap := h.gasPricer.sample()
+		return types.NewTx(&types.DynamicFeeTx{
 			GasTipCap: gasTipCap,
 			GasFeeCap: gasFeeCap,
-		})
-		if shouldMine {
+		}), nil
+	}
+
+	sendTx := func(ctx context.Context, tx *types.Transaction) error {
+		if h.gasPricer.shouldMine(tx.GasFeeCap()) {
 			txHash := tx.Hash()
-			h.backend.mine(&txHash, gasFeeCap)
+			h.backend.mine(&txHash, tx.GasFeeCap())
 		}
-		return tx, nil
+		return nil
 	}
 
 	ctx := context.Background()
-	receipt, err := h.mgr.Send(ctx, sendTxFunc)
+	receipt, err := h.mgr.Send(ctx, updateGasPrice, sendTx)
 	require.Nil(t, err)
 	require.NotNil(t, receipt)
 	require.Equal(t, h.gasPricer.expGasFeeCap().Uint64(), receipt.GasUsed)
@@ -255,16 +270,22 @@ func TestTxMgrBlocksOnFailingRpcCalls(t *testing.T) {
 
 	h := newTestHarness()
 
-	sendTxFunc := func(
-		ctx context.Context,
-	) (*types.Transaction, error) {
-		return nil, errRpcFailure
+	updateGasPrice := func(ctx context.Context) (*types.Transaction, error) {
+		gasTipCap, gasFeeCap := h.gasPricer.sample()
+		return types.NewTx(&types.DynamicFeeTx{
+			GasTipCap: gasTipCap,
+			GasFeeCap: gasFeeCap,
+		}), nil
+	}
+
+	sendTx := func(ctx context.Context, tx *types.Transaction) error {
+		return errRpcFailure
 	}
 
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 	defer cancel()
 
-	receipt, err := h.mgr.Send(ctx, sendTxFunc)
+	receipt, err := h.mgr.Send(ctx, updateGasPrice, sendTx)
 	require.Equal(t, err, context.DeadlineExceeded)
 	require.Nil(t, receipt)
 }
@@ -277,27 +298,27 @@ func TestTxMgrOnlyOnePublicationSucceeds(t *testing.T) {
 
 	h := newTestHarness()
 
-	sendTxFunc := func(
-		ctx context.Context,
-	) (*types.Transaction, error) {
-		gasTipCap, gasFeeCap, shouldMine := h.gasPricer.sample()
+	updateGasPrice := func(ctx context.Context) (*types.Transaction, error) {
+		gasTipCap, gasFeeCap := h.gasPricer.sample()
+		return types.NewTx(&types.DynamicFeeTx{
+			GasTipCap: gasTipCap,
+			GasFeeCap: gasFeeCap,
+		}), nil
+	}
 
+	sendTx := func(ctx context.Context, tx *types.Transaction) error {
 		// Fail all but the final attempt.
-		if !shouldMine {
-			return nil, errRpcFailure
+		if !h.gasPricer.shouldMine(tx.GasFeeCap()) {
+			return errRpcFailure
 		}
 
-		tx := types.NewTx(&types.DynamicFeeTx{
-			GasTipCap: gasTipCap,
-			GasFeeCap: gasFeeCap,
-		})
 		txHash := tx.Hash()
-		h.backend.mine(&txHash, gasFeeCap)
-		return tx, nil
+		h.backend.mine(&txHash, tx.GasFeeCap())
+		return nil
 	}
 
 	ctx := context.Background()
-	receipt, err := h.mgr.Send(ctx, sendTxFunc)
+	receipt, err := h.mgr.Send(ctx, updateGasPrice, sendTx)
 	require.Nil(t, err)
 
 	require.NotNil(t, receipt)
@@ -312,22 +333,23 @@ func TestTxMgrConfirmsMinGasPriceAfterBumping(t *testing.T) {
 
 	h := newTestHarness()
 
-	sendTxFunc := func(
-		ctx context.Context,
-	) (*types.Transaction, error) {
-		gasTipCap, gasFeeCap, shouldMine := h.gasPricer.sample()
-		tx := types.NewTx(&types.DynamicFeeTx{
+	updateGasPrice := func(ctx context.Context) (*types.Transaction, error) {
+		gasTipCap, gasFeeCap := h.gasPricer.sample()
+		return types.NewTx(&types.DynamicFeeTx{
 			GasTipCap: gasTipCap,
 			GasFeeCap: gasFeeCap,
-		})
+		}), nil
+	}
+
+	sendTx := func(ctx context.Context, tx *types.Transaction) error {
 		// Delay mining the tx with the min gas price.
-		if shouldMine {
+		if h.gasPricer.shouldMine(tx.GasFeeCap()) {
 			time.AfterFunc(5*time.Second, func() {
 				txHash := tx.Hash()
-				h.backend.mine(&txHash, gasFeeCap)
+				h.backend.mine(&txHash, tx.GasFeeCap())
 			})
 		}
-		return tx, nil
+		return nil
 	}
 
 	ctx := context.Background()