diff --git a/op-batcher/batcher/driver.go b/op-batcher/batcher/driver.go index 0b5e58a6ccc..fc3fe5aab30 100644 --- a/op-batcher/batcher/driver.go +++ b/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 { diff --git a/op-batcher/batcher/driver_test.go b/op-batcher/batcher/driver_test.go index c5e8cc01cb3..6f0878da110 100644 --- a/op-batcher/batcher/driver_test.go +++ b/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) } diff --git a/op-e2e/actions/l2_proposer.go b/op-e2e/actions/l2_proposer.go index b076ac1b769..194357895a4 100644 --- a/op-e2e/actions/l2_proposer.go +++ b/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 { diff --git a/op-node/testutils/random.go b/op-node/testutils/random.go index bf18771cea7..12b5dfe9ab3 100644 --- a/op-node/testutils/random.go +++ b/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 ð.OutputResponse{ + Version: eth.Bytes32(RandomHash(rng)), + OutputRoot: eth.Bytes32(RandomHash(rng)), + BlockRef: RandomL2BlockRef(rng), + WithdrawalStorageRoot: RandomHash(rng), + StateRoot: RandomHash(rng), + Status: ð.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), + }, + } +} diff --git a/op-proposer/proposer/abi_test.go b/op-proposer/proposer/abi_test.go new file mode 100644 index 00000000000..75b7d499503 --- /dev/null +++ b/op-proposer/proposer/abi_test.go @@ -0,0 +1,75 @@ +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()) +} diff --git a/op-proposer/proposer/l2_output_submitter.go b/op-proposer/proposer/l2_output_submitter.go index d08f5658bbd..0a8c7f595e1 100644 --- a/op-proposer/proposer/l2_output_submitter.go +++ b/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 diff --git a/op-service/txmgr/mocks/TxManager.go b/op-service/txmgr/mocks/TxManager.go index 377220b1210..b67f7919e7e 100644 --- a/op-service/txmgr/mocks/TxManager.go +++ b/op-service/txmgr/mocks/TxManager.go @@ -1,4 +1,4 @@ -// 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()) diff --git a/op-service/txmgr/txmgr.go b/op-service/txmgr/txmgr.go index e7717a5d436..afa5a4047c8 100644 --- a/op-service/txmgr/txmgr.go +++ b/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. diff --git a/op-service/txmgr/txmgr_test.go b/op-service/txmgr/txmgr_test.go index a414c3784f2..7dd8c935f52 100644 --- a/op-service/txmgr/txmgr_test.go +++ b/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)