Merge pull request #5246 from ethereum-optimism/jg/proposer_craft_tx_in_tx_manager

txgmr,op-batcher,op-proposer: Migrate transaction creation to the txmgr

op-batcher/batcher/driver.go

@@ -314,7 +314,7 @@ func (l *BatchSubmitter) loop() {
 				}
 
 				// Record TX Status
-				if receipt, err := l.SendTransaction(l.ctx, txdata.Bytes()); err != nil {
+				if receipt, err := l.sendTransaction(l.ctx, txdata.Bytes()); err != nil {
 					l.recordFailedTx(txdata.ID(), err)
 				} else {
 					l.recordConfirmedTx(txdata.ID(), receipt)
@@ -343,31 +343,25 @@ const networkTimeout = 2 * time.Second // How long a single network request can
 // along with op-proposer changes to include the updated tx manager
 const txManagerTimeout = 2 * time.Minute // How long the tx manager can take to send a transaction.
 
-// SendTransaction creates & submits a transaction to the batch inbox address with the given `data`.
+// sendTransaction creates & submits a transaction to the batch inbox address with the given `data`.
 // It currently uses the underlying `txmgr` to handle transaction sending & price management.
 // This is a blocking method. It should not be called concurrently.
-func (l *BatchSubmitter) SendTransaction(ctx context.Context, data []byte) (*types.Receipt, error) {
+func (l *BatchSubmitter) sendTransaction(ctx context.Context, data []byte) (*types.Receipt, error) {
 	// Do the gas estimation offline. A value of 0 will cause the [txmgr] to estimate the gas limit.
 	intrinsicGas, err := core.IntrinsicGas(data, nil, false, true, true, false)
 	if err != nil {
 		return nil, fmt.Errorf("failed to calculate intrinsic gas: %w", err)
 	}
 
-	// Create the transaction
-	tx, err := l.txMgr.CraftTx(ctx, txmgr.TxCandidate{
+	// Send the transaction through the txmgr
+	ctx, cancel := context.WithTimeout(ctx, txManagerTimeout)
+	defer cancel()
+	if receipt, err := l.txMgr.Send(ctx, txmgr.TxCandidate{
 		To:       l.Rollup.BatchInboxAddress,
 		TxData:   data,
 		From:     l.From,
 		GasLimit: intrinsicGas,
-	})
-	if err != nil {
-		return nil, fmt.Errorf("failed to create tx: %w", err)
-	}
-
-	// Send the transaction through the txmgr
-	ctx, cancel := context.WithTimeout(ctx, txManagerTimeout)
-	defer cancel()
-	if receipt, err := l.txMgr.Send(ctx, tx); err != nil {
+	}); err != nil {
 		l.log.Warn("unable to publish tx", "err", err, "data_size", len(data))
 		return nil, err
 	} else {

op-batcher/batcher/driver_test.go

@@ -73,10 +73,9 @@ func TestBatchSubmitter_SendTransaction(t *testing.T) {
 		GasUsed:           gas,
 	}
 
-	txMgr.On("CraftTx", mock.Anything, candidate).Return(tx, nil)
-	txMgr.On("Send", mock.Anything, tx).Return(&expectedReceipt, nil)
+	txMgr.On("Send", mock.Anything, candidate).Return(&expectedReceipt, nil)
 
-	receipt, err := bs.SendTransaction(context.Background(), tx.Data())
+	receipt, err := bs.sendTransaction(context.Background(), tx.Data())
 	require.NoError(t, err)
 	require.Equal(t, receipt, &expectedReceipt)
 }

op-e2e/actions/l2_proposer.go

@@ -6,11 +6,13 @@ import (
 	"math/big"
 	"time"
 
+	"github.com/ethereum/go-ethereum"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/ethereum/go-ethereum/ethclient"
 	"github.com/ethereum/go-ethereum/log"
+	"github.com/ethereum/go-ethereum/params"
 	"github.com/stretchr/testify/require"
 
 	"github.com/ethereum-optimism/optimism/op-node/sources"
@@ -27,11 +29,14 @@ type ProposerCfg struct {
 }
 
 type L2Proposer struct {
-	log     log.Logger
-	l1      *ethclient.Client
-	driver  *proposer.L2OutputSubmitter
-	address common.Address
-	lastTx  common.Hash
+	log          log.Logger
+	l1           *ethclient.Client
+	driver       *proposer.L2OutputSubmitter
+	address      common.Address
+	privKey      *ecdsa.PrivateKey
+	signer       opcrypto.SignerFn
+	contractAddr common.Address
+	lastTx       common.Hash
 }
 
 func NewL2Proposer(t Testing, log log.Logger, cfg *ProposerCfg, l1 *ethclient.Client, rollupCl *sources.RollupClient) *L2Proposer {
@@ -66,13 +71,56 @@ func NewL2Proposer(t Testing, log log.Logger, cfg *ProposerCfg, l1 *ethclient.Cl
 	require.NoError(t, err)
 
 	return &L2Proposer{
-		log:     log,
-		l1:      l1,
-		driver:  dr,
-		address: crypto.PubkeyToAddress(cfg.ProposerKey.PublicKey),
+		log:          log,
+		l1:           l1,
+		driver:       dr,
+		address:      crypto.PubkeyToAddress(cfg.ProposerKey.PublicKey),
+		privKey:      cfg.ProposerKey,
+		signer:       proposerCfg.TxManagerConfig.Signer,
+		contractAddr: cfg.OutputOracleAddr,
 	}
 }
 
+// sendTx reimplements creating & sending transactions because we need to do the final send as async in
+// the action tests while we do it synchronously in the real system.
+func (p *L2Proposer) sendTx(t Testing, data []byte) {
+	gasTipCap := big.NewInt(2 * params.GWei)
+	pendingHeader, err := p.l1.HeaderByNumber(t.Ctx(), big.NewInt(-1))
+	require.NoError(t, err, "need l1 pending header for gas price estimation")
+	gasFeeCap := new(big.Int).Add(gasTipCap, new(big.Int).Mul(pendingHeader.BaseFee, big.NewInt(2)))
+	chainID, err := p.l1.ChainID(t.Ctx())
+	require.NoError(t, err)
+	nonce, err := p.l1.NonceAt(t.Ctx(), p.address, nil)
+	require.NoError(t, err)
+
+	gasLimit, err := p.l1.EstimateGas(t.Ctx(), ethereum.CallMsg{
+		From:      p.address,
+		To:        &p.contractAddr,
+		GasFeeCap: gasFeeCap,
+		GasTipCap: gasTipCap,
+		Data:      data,
+	})
+	require.NoError(t, err)
+
+	rawTx := &types.DynamicFeeTx{
+		Nonce:     nonce,
+		To:        &p.contractAddr,
+		Data:      data,
+		GasFeeCap: gasFeeCap,
+		GasTipCap: gasTipCap,
+		Gas:       gasLimit,
+		ChainID:   chainID,
+	}
+
+	tx, err := types.SignNewTx(p.privKey, types.LatestSignerForChainID(chainID), rawTx)
+	require.NoError(t, err, "need to sign tx")
+
+	err = p.l1.SendTransaction(t.Ctx(), tx)
+	require.NoError(t, err, "need to send tx")
+
+	p.lastTx = tx.Hash()
+}
+
 func (p *L2Proposer) CanPropose(t Testing) bool {
 	_, shouldPropose, err := p.driver.FetchNextOutputInfo(t.Ctx())
 	require.NoError(t, err)
@@ -86,15 +134,12 @@ func (p *L2Proposer) ActMakeProposalTx(t Testing) {
 	}
 	require.NoError(t, err)
 
-	tx, err := p.driver.CreateProposalTx(t.Ctx(), output)
+	txData, err := p.driver.ProposeL2OutputTxData(output)
 	require.NoError(t, err)
 
 	// Note: Use L1 instead of the output submitter's transaction manager because
 	// this is non-blocking while the txmgr is blocking & deadlocks the tests
-	err = p.l1.SendTransaction(t.Ctx(), tx)
-	require.NoError(t, err)
-
-	p.lastTx = tx.Hash()
+	p.sendTx(t, txData)
 }
 
 func (p *L2Proposer) LastProposalTx() common.Hash {

op-node/testutils/random.go

@@ -245,3 +245,23 @@ func RandomBlockPrependTxs(rng *rand.Rand, txCount int, ptxs ...*types.Transacti
 	}
 	return block, receipts
 }
+
+func RandomOutputResponse(rng *rand.Rand) *eth.OutputResponse {
+	return &eth.OutputResponse{
+		Version:               eth.Bytes32(RandomHash(rng)),
+		OutputRoot:            eth.Bytes32(RandomHash(rng)),
+		BlockRef:              RandomL2BlockRef(rng),
+		WithdrawalStorageRoot: RandomHash(rng),
+		StateRoot:             RandomHash(rng),
+		Status: &eth.SyncStatus{
+			CurrentL1:          RandomBlockRef(rng),
+			CurrentL1Finalized: RandomBlockRef(rng),
+			HeadL1:             RandomBlockRef(rng),
+			SafeL1:             RandomBlockRef(rng),
+			FinalizedL1:        RandomBlockRef(rng),
+			UnsafeL2:           RandomL2BlockRef(rng),
+			SafeL2:             RandomL2BlockRef(rng),
+			FinalizedL2:        RandomL2BlockRef(rng),
+		},
+	}
+}

op-proposer/proposer/abi_test.go

+package proposer
+
+import (
+	"math/big"
+	"math/rand"
+	"testing"
+
+	"github.com/ethereum-optimism/optimism/op-bindings/bindings"
+	"github.com/ethereum-optimism/optimism/op-node/testutils"
+	"github.com/ethereum/go-ethereum/accounts/abi/bind"
+	"github.com/ethereum/go-ethereum/accounts/abi/bind/backends"
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/core"
+	"github.com/ethereum/go-ethereum/crypto"
+	"github.com/ethereum/go-ethereum/params"
+	"github.com/stretchr/testify/require"
+)
+
+// setupL2OutputOracle deploys the L2 Output Oracle contract to a simulated backend
+func setupL2OutputOracle() (common.Address, *bind.TransactOpts, *backends.SimulatedBackend, *bindings.L2OutputOracle, error) {
+	privateKey, err := crypto.GenerateKey()
+	from := crypto.PubkeyToAddress(privateKey.PublicKey)
+	if err != nil {
+		return common.Address{}, nil, nil, nil, err
+	}
+	opts, err := bind.NewKeyedTransactorWithChainID(privateKey, big.NewInt(1337))
+	if err != nil {
+		return common.Address{}, nil, nil, nil, err
+	}
+	backend := backends.NewSimulatedBackend(core.GenesisAlloc{from: {Balance: big.NewInt(params.Ether)}}, 50_000_000)
+	_, _, contract, err := bindings.DeployL2OutputOracle(
+		opts,
+		backend,
+		big.NewInt(10),
+		big.NewInt(2),
+		big.NewInt(0),
+		big.NewInt(0),
+		from,
+		common.Address{0xdd},
+		big.NewInt(100))
+	if err != nil {
+		return common.Address{}, nil, nil, nil, err
+	}
+	return from, opts, backend, contract, nil
+}
+
+// TestManualABIPacking ensure that the manual ABI packing is the same as going through the bound contract.
+// We don't use the contract to transact because it does not fit our transaction management scheme, but
+// we want to make sure that we don't incorrectly create the transaction data.
+func TestManualABIPacking(t *testing.T) {
+	_, opts, _, contract, err := setupL2OutputOracle()
+	require.NoError(t, err)
+	rng := rand.New(rand.NewSource(1234))
+
+	abi, err := bindings.L2OutputOracleMetaData.GetAbi()
+	require.NoError(t, err)
+
+	output := testutils.RandomOutputResponse(rng)
+
+	txData, err := proposeL2OutputTxData(abi, output)
+	require.NoError(t, err)
+
+	// set a gas limit to disable gas estimation. The invariantes that the L2OO tries to uphold
+	// are not maintained in this test.
+	opts.GasLimit = 100_000
+	tx, err := contract.ProposeL2Output(
+		opts,
+		output.OutputRoot,
+		new(big.Int).SetUint64(output.BlockRef.Number),
+		output.Status.CurrentL1.Hash,
+		new(big.Int).SetUint64(output.Status.CurrentL1.Number))
+	require.NoError(t, err)
+
+	require.Equal(t, txData, tx.Data())
+}

op-proposer/proposer/l2_output_submitter.go

@@ -8,7 +8,6 @@ import (
 	_ "net/http/pprof"
 	"os"
 	"os/signal"
-	"strings"
 	"sync"
 	"syscall"
 	"time"
@@ -16,7 +15,6 @@ import (
 	"github.com/ethereum/go-ethereum/accounts/abi"
 	"github.com/ethereum/go-ethereum/accounts/abi/bind"
 	"github.com/ethereum/go-ethereum/common"
-	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/ethclient"
 	"github.com/ethereum/go-ethereum/log"
 	"github.com/urfave/cli"
@@ -124,13 +122,14 @@ type L2OutputSubmitter struct {
 	ctx    context.Context
 	cancel context.CancelFunc
 
-	// L1Client is used to submit transactions to
+	// l1Client is retained to make it easier to start the metrics balance check
 	l1Client *ethclient.Client
 	// RollupClient is used to retrieve output roots from
 	rollupClient *sources.RollupClient
 
-	l2ooContract    *bindings.L2OutputOracle
-	rawL2ooContract *bind.BoundContract
+	l2ooContract     *bindings.L2OutputOracleCaller
+	l2ooContractAddr common.Address
+	l2ooABI          *abi.ABI
 
 	// AllowNonFinalized enables the proposal of safe, but non-finalized L2 blocks.
 	// The L1 block-hash embedded in the proposal TX is checked and should ensure the proposal
@@ -139,8 +138,6 @@ type L2OutputSubmitter struct {
 	allowNonFinalized bool
 	// From is the address to send transactions from
 	from common.Address
-	// SignerFn is the function used to sign transactions
-	signerFn opcrypto.SignerFn
 	// How frequently to poll L2 for new finalized outputs
 	pollInterval time.Duration
 }
@@ -211,25 +208,26 @@ func NewL2OutputSubmitter(cfg Config, l log.Logger, m metrics.Metricer) (*L2Outp
 	signer := cfg.SignerFnFactory(chainID)
 	cfg.TxManagerConfig.Signer = signer
 
-	l2ooContract, err := bindings.NewL2OutputOracle(cfg.L2OutputOracleAddr, cfg.L1Client)
+	l2ooContract, err := bindings.NewL2OutputOracleCaller(cfg.L2OutputOracleAddr, cfg.L1Client)
 	if err != nil {
 		cancel()
 		return nil, err
 	}
 
-	version, err := l2ooContract.Version(&bind.CallOpts{})
+	cCtx, cCancel = context.WithTimeout(ctx, defaultDialTimeout)
+	defer cCancel()
+	version, err := l2ooContract.Version(&bind.CallOpts{Context: cCtx})
 	if err != nil {
 		cancel()
 		return nil, err
 	}
 	log.Info("Connected to L2OutputOracle", "address", cfg.L2OutputOracleAddr, "version", version)
 
-	parsed, err := abi.JSON(strings.NewReader(bindings.L2OutputOracleMetaData.ABI))
+	parsed, err := bindings.L2OutputOracleMetaData.GetAbi()
 	if err != nil {
 		cancel()
 		return nil, err
 	}
-	rawL2ooContract := bind.NewBoundContract(cfg.L2OutputOracleAddr, parsed, cfg.L1Client, cfg.L1Client, cfg.L1Client)
 
 	return &L2OutputSubmitter{
 		txMgr:  txmgr.NewSimpleTxManager("proposer", l, cfg.TxManagerConfig, cfg.L1Client),
@@ -242,12 +240,12 @@ func NewL2OutputSubmitter(cfg Config, l log.Logger, m metrics.Metricer) (*L2Outp
 		l1Client:     cfg.L1Client,
 		rollupClient: cfg.RollupClient,
 
-		l2ooContract:    l2ooContract,
-		rawL2ooContract: rawL2ooContract,
+		l2ooContract:     l2ooContract,
+		l2ooContractAddr: cfg.L2OutputOracleAddr,
+		l2ooABI:          parsed,
 
 		allowNonFinalized: cfg.AllowNonFinalized,
 		from:              cfg.From,
-		signerFn:          signer,
 		pollInterval:      cfg.PollInterval,
 	}, nil
 }
@@ -264,29 +262,14 @@ func (l *L2OutputSubmitter) Stop() {
 	l.wg.Wait()
 }
 
-// UpdateGasPrice signs an otherwise identical txn to the one provided but with
-// updated gas prices sampled from the existing network conditions.
-//
-// NOTE: This method SHOULD NOT publish the resulting transaction.
-func (l *L2OutputSubmitter) UpdateGasPrice(ctx context.Context, tx *types.Transaction) (*types.Transaction, error) {
-	opts := &bind.TransactOpts{
-		From: l.from,
-		Signer: func(addr common.Address, tx *types.Transaction) (*types.Transaction, error) {
-			return l.signerFn(ctx, addr, tx)
-		},
-		Context: ctx,
-		Nonce:   new(big.Int).SetUint64(tx.Nonce()),
-		NoSend:  true,
-	}
-	return l.rawL2ooContract.RawTransact(opts, tx.Data())
-}
-
 // FetchNextOutputInfo gets the block number of the next proposal.
 // It returns: the next block number, if the proposal should be made, error
 func (l *L2OutputSubmitter) FetchNextOutputInfo(ctx context.Context) (*eth.OutputResponse, bool, error) {
+	cCtx, cancel := context.WithTimeout(ctx, defaultDialTimeout)
+	defer cancel()
 	callOpts := &bind.CallOpts{
 		From:    l.from,
-		Context: ctx,
+		Context: cCtx,
 	}
 	nextCheckpointBlock, err := l.l2ooContract.NextBlockNumber(callOpts)
 	if err != nil {
@@ -294,7 +277,9 @@ func (l *L2OutputSubmitter) FetchNextOutputInfo(ctx context.Context) (*eth.Outpu
 		return nil, false, err
 	}
 	// Fetch the current L2 heads
-	status, err := l.rollupClient.SyncStatus(ctx)
+	cCtx, cancel = context.WithTimeout(ctx, defaultDialTimeout)
+	defer cancel()
+	status, err := l.rollupClient.SyncStatus(cCtx)
 	if err != nil {
 		l.log.Error("proposer unable to get sync status", "err", err)
 		return nil, false, err
@@ -312,17 +297,23 @@ func (l *L2OutputSubmitter) FetchNextOutputInfo(ctx context.Context) (*eth.Outpu
 		return nil, false, nil
 	}
 
-	output, err := l.rollupClient.OutputAtBlock(ctx, nextCheckpointBlock.Uint64())
+	return l.fetchOuput(ctx, nextCheckpointBlock)
+}
+
+func (l *L2OutputSubmitter) fetchOuput(ctx context.Context, block *big.Int) (*eth.OutputResponse, bool, error) {
+	ctx, cancel := context.WithTimeout(ctx, defaultDialTimeout)
+	defer cancel()
+	output, err := l.rollupClient.OutputAtBlock(ctx, block.Uint64())
 	if err != nil {
-		l.log.Error("failed to fetch output at block %d: %w", nextCheckpointBlock, err)
+		l.log.Error("failed to fetch output at block %d: %w", block, err)
 		return nil, false, err
 	}
 	if output.Version != supportedL2OutputVersion {
 		l.log.Error("unsupported l2 output version: %s", output.Version)
 		return nil, false, errors.New("unsupported l2 output version")
 	}
-	if output.BlockRef.Number != nextCheckpointBlock.Uint64() { // sanity check, e.g. in case of bad RPC caching
-		l.log.Error("invalid blockNumber: next blockNumber is %v, blockNumber of block is %v", nextCheckpointBlock, output.BlockRef.Number)
+	if output.BlockRef.Number != block.Uint64() { // sanity check, e.g. in case of bad RPC caching
+		l.log.Error("invalid blockNumber: next blockNumber is %v, blockNumber of block is %v", block, output.BlockRef.Number)
 		return nil, false, errors.New("invalid blockNumber")
 	}
 
@@ -338,54 +329,36 @@ func (l *L2OutputSubmitter) FetchNextOutputInfo(ctx context.Context) (*eth.Outpu
 	return output, true, nil
 }
 
-// CreateProposalTx transforms an output response into a signed output transaction.
-// It does not send the transaction to the transaction pool.
-func (l *L2OutputSubmitter) CreateProposalTx(ctx context.Context, output *eth.OutputResponse) (*types.Transaction, error) {
-	nonce, err := l.l1Client.NonceAt(ctx, l.from, nil)
-	if err != nil {
-		l.log.Error("Failed to get nonce", "err", err, "from", l.from)
-		return nil, err
-	}
-
-	opts := &bind.TransactOpts{
-		From: l.from,
-		Signer: func(addr common.Address, tx *types.Transaction) (*types.Transaction, error) {
-			return l.signerFn(ctx, addr, tx)
-		},
-		Context: ctx,
-		Nonce:   new(big.Int).SetUint64(nonce),
-		NoSend:  true,
-	}
+// ProposeL2OutputTxData creates the transaction data for the ProposeL2Output function
+func (l *L2OutputSubmitter) ProposeL2OutputTxData(output *eth.OutputResponse) ([]byte, error) {
+	return proposeL2OutputTxData(l.l2ooABI, output)
+}
 
-	tx, err := l.l2ooContract.ProposeL2Output(
-		opts,
+// proposeL2OutputTxData creates the transaction data for the ProposeL2Output function
+func proposeL2OutputTxData(abi *abi.ABI, output *eth.OutputResponse) ([]byte, error) {
+	return abi.Pack(
+		"proposeL2Output",
 		output.OutputRoot,
 		new(big.Int).SetUint64(output.BlockRef.Number),
 		output.Status.CurrentL1.Hash,
 		new(big.Int).SetUint64(output.Status.CurrentL1.Number))
-	if err != nil {
-		l.log.Error("failed to create the ProposeL2Output transaction", "err", err)
-		return nil, err
-	}
-	return tx, nil
 }
 
-// SendTransaction sends a transaction through the transaction manager which handles automatic
-// price bumping.
-// It also hardcodes a timeout of 100s.
-func (l *L2OutputSubmitter) SendTransaction(ctx context.Context, tx *types.Transaction) error {
-	// Wait until one of our submitted transactions confirms. If no
-	// receipt is received it's likely our gas price was too low.
-	cCtx, cancel := context.WithTimeout(ctx, 100*time.Second)
-	defer cancel()
-	l.log.Info("Sending transaction", "tx_hash", tx.Hash())
-	receipt, err := l.txMgr.Send(cCtx, tx)
+// sendTransaction creates & sends transactions through the underlying transaction manager.
+func (l *L2OutputSubmitter) sendTransaction(ctx context.Context, output *eth.OutputResponse) error {
+	data, err := l.ProposeL2OutputTxData(output)
+	if err != nil {
+		return err
+	}
+	receipt, err := l.txMgr.Send(ctx, txmgr.TxCandidate{
+		TxData:   data,
+		To:       l.l2ooContractAddr,
+		GasLimit: 0,
+		From:     l.from,
+	})
 	if err != nil {
-		l.log.Error("proposer unable to publish tx", "err", err)
 		return err
 	}
-
-	// The transaction was successfully submitted
 	l.log.Info("proposer tx successfully published", "tx_hash", receipt.TxHash)
 	return nil
 }
@@ -401,9 +374,7 @@ func (l *L2OutputSubmitter) loop() {
 	for {
 		select {
 		case <-ticker.C:
-			cCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
-			output, shouldPropose, err := l.FetchNextOutputInfo(cCtx)
-			cancel()
+			output, shouldPropose, err := l.FetchNextOutputInfo(ctx)
 			if err != nil {
 				break
 			}
@@ -411,15 +382,8 @@ func (l *L2OutputSubmitter) loop() {
 				break
 			}
 
-			cCtx, cancel = context.WithTimeout(ctx, 30*time.Second)
-			tx, err := l.CreateProposalTx(cCtx, output)
-			cancel()
-			if err != nil {
-				l.log.Error("Failed to create proposal transaction", "err", err)
-				break
-			}
-			cCtx, cancel = context.WithTimeout(ctx, 10*time.Minute)
-			if err := l.SendTransaction(cCtx, tx); err != nil {
+			cCtx, cancel := context.WithTimeout(ctx, 10*time.Minute)
+			if err := l.sendTransaction(cCtx, output); err != nil {
 				l.log.Error("Failed to send proposal transaction", "err", err)
 				cancel()
 				break

op-service/txmgr/mocks/TxManager.go

-// Code generated by mockery v2.22.1. DO NOT EDIT.
+// Code generated by mockery v2.23.1. DO NOT EDIT.
 
 package mocks
 
@@ -16,20 +16,20 @@ type TxManager struct {
 	mock.Mock
 }
 
-// CraftTx provides a mock function with given fields: ctx, candidate
-func (_m *TxManager) CraftTx(ctx context.Context, candidate txmgr.TxCandidate) (*types.Transaction, error) {
+// Send provides a mock function with given fields: ctx, candidate
+func (_m *TxManager) Send(ctx context.Context, candidate txmgr.TxCandidate) (*types.Receipt, error) {
 	ret := _m.Called(ctx, candidate)
 
-	var r0 *types.Transaction
+	var r0 *types.Receipt
 	var r1 error
-	if rf, ok := ret.Get(0).(func(context.Context, txmgr.TxCandidate) (*types.Transaction, error)); ok {
+	if rf, ok := ret.Get(0).(func(context.Context, txmgr.TxCandidate) (*types.Receipt, error)); ok {
 		return rf(ctx, candidate)
 	}
-	if rf, ok := ret.Get(0).(func(context.Context, txmgr.TxCandidate) *types.Transaction); ok {
+	if rf, ok := ret.Get(0).(func(context.Context, txmgr.TxCandidate) *types.Receipt); ok {
 		r0 = rf(ctx, candidate)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(*types.Transaction)
+			r0 = ret.Get(0).(*types.Receipt)
 		}
 	}
 
@@ -42,32 +42,6 @@ func (_m *TxManager) CraftTx(ctx context.Context, candidate txmgr.TxCandidate) (
 	return r0, r1
 }
 
-// Send provides a mock function with given fields: ctx, tx
-func (_m *TxManager) Send(ctx context.Context, tx *types.Transaction) (*types.Receipt, error) {
-	ret := _m.Called(ctx, tx)
-
-	var r0 *types.Receipt
-	var r1 error
-	if rf, ok := ret.Get(0).(func(context.Context, *types.Transaction) (*types.Receipt, error)); ok {
-		return rf(ctx, tx)
-	}
-	if rf, ok := ret.Get(0).(func(context.Context, *types.Transaction) *types.Receipt); ok {
-		r0 = rf(ctx, tx)
-	} else {
-		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(*types.Receipt)
-		}
-	}
-
-	if rf, ok := ret.Get(1).(func(context.Context, *types.Transaction) error); ok {
-		r1 = rf(ctx, tx)
-	} else {
-		r1 = ret.Error(1)
-	}
-
-	return r0, r1
-}
-
 type mockConstructorTestingTNewTxManager interface {
 	mock.TestingT
 	Cleanup(func())

op-service/txmgr/txmgr.go

@@ -74,18 +74,13 @@ type Config struct {
 //
 //go:generate mockery --name TxManager --output ./mocks
 type TxManager interface {
-	// Send is used to publish a transaction with incrementally higher gas
-	// prices until the transaction eventually confirms. This method blocks
-	// until an invocation of sendTx returns (called with differing gas
-	// prices). The method may be canceled using the passed context.
-	//
-	// The initial transaction MUST be signed & ready to submit.
+	// Send is used to create & send a transaction. It will handle increasing
+	// the gas price & ensuring that the transaction remains in the transaction pool.
+	// It can be stopped by cancelling the provided context; however, the transaction
+	// may be included on L1 even if the context is cancelled.
 	//
 	// NOTE: Send should be called by AT MOST one caller at a time.
-	Send(ctx context.Context, tx *types.Transaction) (*types.Receipt, error)
-
-	// CraftTx is used to craft a transaction using a [TxCandidate].
-	CraftTx(ctx context.Context, candidate TxCandidate) (*types.Transaction, error)
+	Send(ctx context.Context, candidate TxCandidate) (*types.Receipt, error)
 }
 
 // ETHBackend is the set of methods that the transaction manager uses to resubmit gas & determine
@@ -166,12 +161,12 @@ func (m *SimpleTxManager) calcGasTipAndFeeCap(ctx context.Context) (gasTipCap *b
 	return gasTipCap, gasFeeCap, nil
 }
 
-// CraftTx creates the signed transaction to the batchInboxAddress.
+// craftTx creates the signed transaction
 // It queries L1 for the current fee market conditions as well as for the nonce.
 // NOTE: This method SHOULD NOT publish the resulting transaction.
 // NOTE: If the [TxCandidate.GasLimit] is non-zero, it will be used as the transaction's gas.
 // NOTE: Otherwise, the [SimpleTxManager] will query the specified backend for an estimate.
-func (m *SimpleTxManager) CraftTx(ctx context.Context, candidate TxCandidate) (*types.Transaction, error) {
+func (m *SimpleTxManager) craftTx(ctx context.Context, candidate TxCandidate) (*types.Transaction, error) {
 	gasTipCap, gasFeeCap, err := m.calcGasTipAndFeeCap(ctx)
 	if err != nil {
 		return nil, err
@@ -337,8 +332,16 @@ func NewSimpleTxManager(name string, l log.Logger, cfg Config, backend ETHBacken
 // but retain the gas used, the nonce, and the data.
 //
 // NOTE: Send should be called by AT MOST one caller at a time.
-func (m *SimpleTxManager) Send(ctx context.Context, tx *types.Transaction) (*types.Receipt, error) {
+func (m *SimpleTxManager) Send(ctx context.Context, candidate TxCandidate) (*types.Receipt, error) {
+	tx, err := m.craftTx(ctx, candidate)
+	if err != nil {
+		m.l.Error("Failed to create the transaction", "err", err)
+		return nil, err
+	}
+	return m.send(ctx, tx)
+}
 
+func (m *SimpleTxManager) send(ctx context.Context, tx *types.Transaction) (*types.Receipt, error) {
 	// Initialize a wait group to track any spawned goroutines, and ensure
 	// we properly clean up any dangling resources this method generates.
 	// We assert that this is the case thoroughly in our unit tests.

op-service/txmgr/txmgr_test.go

@@ -261,7 +261,7 @@ func TestTxMgrConfirmAtMinGasPrice(t *testing.T) {
 
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	defer cancel()
-	receipt, err := h.mgr.Send(ctx, tx)
+	receipt, err := h.mgr.send(ctx, tx)
 	require.Nil(t, err)
 	require.NotNil(t, receipt)
 	require.Equal(t, gasPricer.expGasFeeCap().Uint64(), receipt.GasUsed)
@@ -289,7 +289,7 @@ func TestTxMgrNeverConfirmCancel(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	defer cancel()
 
-	receipt, err := h.mgr.Send(ctx, tx)
+	receipt, err := h.mgr.send(ctx, tx)
 	require.Equal(t, err, context.DeadlineExceeded)
 	require.Nil(t, receipt)
 }
@@ -318,7 +318,7 @@ func TestTxMgrConfirmsAtHigherGasPrice(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	defer cancel()
 
-	receipt, err := h.mgr.Send(ctx, tx)
+	receipt, err := h.mgr.send(ctx, tx)
 	require.Nil(t, err)
 	require.NotNil(t, receipt)
 	require.Equal(t, h.gasPricer.expGasFeeCap().Uint64(), receipt.GasUsed)
@@ -349,7 +349,7 @@ func TestTxMgrBlocksOnFailingRpcCalls(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	defer cancel()
 
-	receipt, err := h.mgr.Send(ctx, tx)
+	receipt, err := h.mgr.send(ctx, tx)
 	require.Equal(t, err, context.DeadlineExceeded)
 	require.Nil(t, receipt)
 }
@@ -362,7 +362,7 @@ func TestTxMgr_CraftTx(t *testing.T) {
 
 	// Craft the transaction.
 	gasTipCap, gasFeeCap := h.gasPricer.feesForEpoch(h.gasPricer.epoch + 1)
-	tx, err := h.mgr.CraftTx(context.Background(), candidate)
+	tx, err := h.mgr.craftTx(context.Background(), candidate)
 	require.Nil(t, err)
 	require.NotNil(t, tx)
 
@@ -391,7 +391,7 @@ func TestTxMgr_EstimateGas(t *testing.T) {
 	gasEstimate := h.gasPricer.baseBaseFee.Uint64()
 
 	// Craft the transaction.
-	tx, err := h.mgr.CraftTx(context.Background(), candidate)
+	tx, err := h.mgr.craftTx(context.Background(), candidate)
 	require.Nil(t, err)
 	require.NotNil(t, tx)
 
@@ -427,7 +427,7 @@ func TestTxMgrOnlyOnePublicationSucceeds(t *testing.T) {
 
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	defer cancel()
-	receipt, err := h.mgr.Send(ctx, tx)
+	receipt, err := h.mgr.send(ctx, tx)
 	require.Nil(t, err)
 
 	require.NotNil(t, receipt)
@@ -462,7 +462,7 @@ func TestTxMgrConfirmsMinGasPriceAfterBumping(t *testing.T) {
 
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	defer cancel()
-	receipt, err := h.mgr.Send(ctx, tx)
+	receipt, err := h.mgr.send(ctx, tx)
 	require.Nil(t, err)
 	require.NotNil(t, receipt)
 	require.Equal(t, h.gasPricer.expGasFeeCap().Uint64(), receipt.GasUsed)
@@ -507,7 +507,7 @@ func TestTxMgrDoesntAbortNonceTooLowAfterMiningTx(t *testing.T) {
 
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	defer cancel()
-	receipt, err := h.mgr.Send(ctx, tx)
+	receipt, err := h.mgr.send(ctx, tx)
 	require.Nil(t, err)
 	require.NotNil(t, receipt)
 	require.Equal(t, h.gasPricer.expGasFeeCap().Uint64(), receipt.GasUsed)