From 1802156660ca72a27c3cbede26c1158d8debf6a7 Mon Sep 17 00:00:00 2001 From: seolaoh Date: Mon, 24 Apr 2023 21:39:43 +0900 Subject: [PATCH] feat(batcher): apply op-batcher v1.0.3 to Kroma --- components/batcher/batch_submitter.go | 66 +- components/batcher/batcher.go | 162 +++-- components/batcher/channel_builder.go | 32 +- components/batcher/channel_builder_test.go | 202 +++--- components/batcher/channel_manager.go | 244 ++++--- components/batcher/channel_manager_test.go | 249 +++++-- components/batcher/cmd/main.go | 5 +- components/batcher/config.go | 115 +--- components/batcher/flags/flags.go | 54 +- components/batcher/metrics/metrics.go | 22 +- components/batcher/metrics/noop.go | 6 +- components/batcher/rpc/api.go | 6 +- components/node/rollup/derive/channel_out.go | 8 +- utils/monitoring/monitoring.go | 2 +- utils/service/crypto/signature.go | 2 +- utils/service/rpc/server.go | 4 +- utils/service/txmgr/cli.go | 268 ++++++++ utils/service/txmgr/metrics/noop.go | 12 + utils/service/txmgr/metrics/tx_metrics.go | 115 ++++ utils/service/txmgr/mocks/TxManager.go | 77 +++ utils/service/txmgr/price_bump_test.go | 91 +++ utils/service/txmgr/send_state.go | 66 +- utils/service/txmgr/send_state_test.go | 37 +- utils/service/txmgr/txmgr.go | 644 +++++++++++-------- utils/service/txmgr/txmgr_test.go | 443 +++++++------ utils/utils.go | 67 -- 26 files changed, 1869 insertions(+), 1130 deletions(-) create mode 100644 utils/service/txmgr/cli.go create mode 100644 utils/service/txmgr/metrics/noop.go create mode 100644 utils/service/txmgr/metrics/tx_metrics.go create mode 100644 utils/service/txmgr/mocks/TxManager.go create mode 100644 utils/service/txmgr/price_bump_test.go diff --git a/components/batcher/batch_submitter.go b/components/batcher/batch_submitter.go index 1bb64efcd2..c6ba159b25 100644 --- a/components/batcher/batch_submitter.go +++ b/components/batcher/batch_submitter.go @@ -6,30 +6,19 @@ import ( "fmt" "math/big" _ "net/http/pprof" - "time" - "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/log" "github.com/kroma-network/kroma/components/batcher/metrics" "github.com/kroma-network/kroma/components/node/eth" - "github.com/kroma-network/kroma/utils" ) -const networkTimeout = 2 * time.Second // How long a single network request can take. TODO: put in a config somewhere - // BatchSubmitter encapsulates a service responsible for submitting L2 tx // batches to L1 for availability. type BatchSubmitter struct { Config // directly embed the config - done chan struct{} - log log.Logger - - ctx context.Context - cancel context.CancelFunc - // lastStoredBlock is the last block loaded into `state`. If it is empty it should be set to the l2 safe head. lastStoredBlock eth.BlockID lastL1Tip eth.L1BlockRef @@ -40,19 +29,10 @@ type BatchSubmitter struct { // NewBatchSubmitter initializes the BatchSubmitter, gathering any resources // that will be needed during operation. func NewBatchSubmitter(cfg Config, l log.Logger, m metrics.Metricer) (*BatchSubmitter, error) { - ctx, cancel := context.WithCancel(context.Background()) - return &BatchSubmitter{ Config: cfg, - done: make(chan struct{}), - log: l, - // TODO: this context only exists because the event loop doesn't reach done - // if the tx manager is blocking forever due to e.g. insufficient balance. - ctx: ctx, - cancel: cancel, state: NewChannelManager(l, m, cfg.Channel), }, nil - } // LoadBlocksIntoState loads all blocks since the previous stored block @@ -86,7 +66,7 @@ func (b *BatchSubmitter) LoadBlocksIntoState(ctx context.Context) { // loadBlockIntoState fetches & stores a single block into `state`. It returns the block it loaded. func (b *BatchSubmitter) loadBlockIntoState(ctx context.Context, blockNumber uint64) (eth.BlockID, error) { - ctx, cancel := context.WithTimeout(ctx, networkTimeout) + ctx, cancel := context.WithTimeout(ctx, b.NetworkTimeout) block, err := b.L2Client.BlockByNumber(ctx, new(big.Int).SetUint64(blockNumber)) cancel() if err != nil { @@ -100,12 +80,12 @@ func (b *BatchSubmitter) loadBlockIntoState(ctx context.Context, blockNumber uin return id, nil } -// calculateL2BlockRangeToStore determines the range (start,end] that should be loaded into the local state. +// calculateL2BlockRangeToStore determines the range (start,end) that should be loaded into the local state. // It also takes care of initializing some local state (i.e. will modify b.lastStoredBlock in certain conditions) func (b *BatchSubmitter) calculateL2BlockRangeToStore(ctx context.Context) (eth.BlockID, eth.BlockID, error) { - childCtx, cancel := context.WithTimeout(ctx, networkTimeout) + ctx, cancel := context.WithTimeout(ctx, b.NetworkTimeout) defer cancel() - syncStatus, err := b.RollupClient.SyncStatus(childCtx) + syncStatus, err := b.RollupClient.SyncStatus(ctx) // Ensure that we have the sync status if err != nil { return eth.BlockID{}, eth.BlockID{}, fmt.Errorf("failed to get sync status: %w", err) @@ -132,42 +112,6 @@ func (b *BatchSubmitter) calculateL2BlockRangeToStore(ctx context.Context) (eth. return b.lastStoredBlock, syncStatus.UnsafeL2.ID(), nil } -func (b *BatchSubmitter) CreateSubmitTx(data []byte) (*types.Transaction, error) { - ctx, cancel := context.WithTimeout(b.ctx, networkTimeout) - nonce, err := b.L1Client.NonceAt(ctx, b.From, nil) - cancel() - if err != nil { - return nil, fmt.Errorf("failed to get nonce: %w", err) - } - - ctx, cancel = context.WithTimeout(b.ctx, networkTimeout) - gasTipCap, gasFeeCap, err := utils.CalcGasTipAndFeeCap(ctx, b.L1Client) - cancel() - if err != nil { - return nil, fmt.Errorf("failed to calculate gasTipCap and gasFeeCap: %w", err) - } - - rawTx := &types.DynamicFeeTx{ - ChainID: b.Rollup.L1ChainID, - Nonce: nonce, - To: &b.Rollup.BatchInboxAddress, - GasTipCap: gasTipCap, - GasFeeCap: gasFeeCap, - Data: data, - } - - gas, err := core.IntrinsicGas(rawTx.Data, nil, false, true, true, false) - if err != nil { - return nil, fmt.Errorf("failed to calculate intrinsic gas: %w", err) - } - rawTx.Gas = gas - - ctx, cancel = context.WithTimeout(b.ctx, networkTimeout) - defer cancel() - tx := types.NewTx(rawTx) - return b.TxManagerConfig.Signer(ctx, b.From, tx) -} - func (b *BatchSubmitter) recordL1Tip(l1tip eth.L1BlockRef) { if b.lastL1Tip == l1tip { return @@ -190,7 +134,7 @@ func (b *BatchSubmitter) recordConfirmedTx(id txID, receipt *types.Receipt) { // l1Tip gets the current L1 tip as a L1BlockRef. The passed context is assumed // to be a lifetime context, so it is internally wrapped with a network timeout. func (b *BatchSubmitter) l1Tip(ctx context.Context) (eth.L1BlockRef, error) { - tctx, cancel := context.WithTimeout(ctx, networkTimeout) + tctx, cancel := context.WithTimeout(ctx, b.NetworkTimeout) defer cancel() head, err := b.L1Client.HeaderByNumber(tctx, nil) if err != nil { diff --git a/components/batcher/batcher.go b/components/batcher/batcher.go index 3ab7f3fd38..0cbf97efd5 100644 --- a/components/batcher/batcher.go +++ b/components/batcher/batcher.go @@ -7,6 +7,7 @@ import ( "sync" "time" + "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/log" "github.com/urfave/cli" @@ -19,7 +20,7 @@ import ( "github.com/kroma-network/kroma/utils/service/txmgr" ) -// Main is the entrypoint into the Batch Submitter. +// Main is the entrypoint into the Batcher. func Main(version string, cliCtx *cli.Context) error { cliCfg := NewCLIConfig(cliCtx) if err := cliCfg.Check(); err != nil { @@ -28,10 +29,11 @@ func Main(version string, cliCtx *cli.Context) error { l := klog.NewLogger(cliCfg.LogConfig) m := metrics.NewMetrics("default") - l.Info("Initializing Batch Submitter") + l.Info("Initializing Batcher") batcherCfg, err := NewBatcherConfig(cliCfg, l, m) if err != nil { + l.Error("Unable to create batcher config", "err", err) return err } @@ -39,12 +41,16 @@ func Main(version string, cliCtx *cli.Context) error { defer cancel() monitoring.MaybeStartPprof(ctx, cliCfg.PprofConfig, l) - monitoring.MaybeStartMetrics(ctx, cliCfg.MetricsConfig, l, batcherCfg.L1Client, batcherCfg.From) + monitoring.MaybeStartMetrics(ctx, cliCfg.MetricsConfig, l, batcherCfg.L1Client, batcherCfg.TxManager.From()) server, err := monitoring.StartRPC(cliCfg.RPCConfig.ToServiceCLIConfig(), version, krpc.WithLogger(l)) if err != nil { return err } - defer server.Stop() + defer func() { + if err = server.Stop(); err != nil { + l.Error("Error shutting down http server: %w", err) + } + }() m.RecordInfo(version) m.RecordUp() @@ -56,19 +62,20 @@ func Main(version string, cliCtx *cli.Context) error { batcher.Start() <-utils.WaitInterrupt() - batcher.Stop() + batcher.Stop(context.Background()) return nil } type Batcher struct { - ctx context.Context - cancel context.CancelFunc + shutdownCtx context.Context + cancelShutdownCtx context.CancelFunc + killCtx context.Context + cancelKillCtx context.CancelFunc cfg Config l log.Logger batchSubmitter *BatchSubmitter - txMgr txmgr.TxManager wg sync.WaitGroup } @@ -79,43 +86,67 @@ func NewBatcher(parentCtx context.Context, cfg Config, l log.Logger, m metrics.M return nil, err } - ctx, cancel := context.WithCancel(parentCtx) - batchSubmitter, err := NewBatchSubmitter(cfg, l, m) if err != nil { - cancel() return nil, fmt.Errorf("failed to init batch submitter: %w", err) } - balance, err := cfg.L1Client.BalanceAt(ctx, cfg.From, nil) + balance, err := cfg.L1Client.BalanceAt(parentCtx, cfg.TxManager.From(), nil) if err != nil { - cancel() return nil, err } - l.Info("creating batcher", "batcher_addr", cfg.From, "batcher_bal", balance) + l.Info("creating batcher", "batcher_addr", cfg.TxManager.From(), "batcher_bal", balance) return &Batcher{ - ctx: ctx, - cancel: cancel, cfg: cfg, l: l, batchSubmitter: batchSubmitter, - txMgr: txmgr.NewSimpleTxManager("batcher", l, cfg.TxManagerConfig, cfg.L1Client), }, nil } func (b *Batcher) Start() { - b.l.Info("starting Batch Submitter") + b.l.Info("starting Batcher") + + b.shutdownCtx, b.cancelShutdownCtx = context.WithCancel(context.Background()) + b.killCtx, b.cancelKillCtx = context.WithCancel(context.Background()) + b.wg.Add(1) go b.loop() + + b.l.Info("Batcher started") } -func (b *Batcher) Stop() { - b.cancel() +func (b *Batcher) Stop(ctx context.Context) { + b.l.Info("stopping Batcher") + + // go routine will call cancelKillCtx() if the passed in ctx is ever Done + cancelKill := b.cancelKillCtx + wrapped, cancel := context.WithCancel(ctx) + defer cancel() + go func() { + <-wrapped.Done() + cancelKill() + }() + + b.cancelShutdownCtx() b.wg.Wait() + b.cancelKillCtx() + + b.l.Info("Batcher stopped") } +// The following things occur: +// New L2 block (reorg or not) +// L1 transaction is confirmed +// +// What the batcher does: +// Ensure that channels are created & submitted as frames for an L2 range +// +// Error conditions: +// Submitted batch, but it is not valid +// Missed L2 block somehow. + func (b *Batcher) loop() { defer b.wg.Done() @@ -125,34 +156,42 @@ func (b *Batcher) loop() { for { select { case <-ticker.C: - if err := b.submitBatch(); err != nil { + b.batchSubmitter.LoadBlocksIntoState(b.shutdownCtx) + if err := b.submitBatch(b.killCtx); err != nil { + b.l.Error("failed to submit batch channel frame", "err", err) + } + case <-b.shutdownCtx.Done(): + if err := b.submitBatch(b.killCtx); err != nil { b.l.Error("failed to submit batch channel frame", "err", err) } - case <-b.ctx.Done(): return } } } -// The following things occur: -// New L2 block (reorg or not) -// L1 transaction is confirmed -// -// What the batcher does: -// Ensure that channels are created & submitted as frames for an L2 range -// -// Error conditions: -// Submitted batch, but it is not valid -// Missed L2 block somehow. - -func (b *Batcher) submitBatch() error { - b.batchSubmitter.LoadBlocksIntoState(b.ctx) - -blockLoop: +// submitBatch loops through the block data loaded into `state` and +// submits the associated data to the L1 in the form of channel frames. +func (b *Batcher) submitBatch(ctx context.Context) error { for { - l1tip, err := b.batchSubmitter.l1Tip(b.ctx) + // Attempt to gracefully terminate the current channel, ensuring that no new frames will be + // produced. Any remaining frames must still be published to the L1 to prevent stalling. + select { + case <-ctx.Done(): + err := b.batchSubmitter.state.Close() + if err != nil { + b.l.Error("failed to close the channel manager", "err", err) + } + case <-b.shutdownCtx.Done(): + err := b.batchSubmitter.state.Close() + if err != nil { + b.l.Error("failed to close the channel manager", "err", err) + } + default: + } + + l1tip, err := b.batchSubmitter.l1Tip(ctx) if err != nil { - b.l.Error("Failed to query L1 tip", "error", err) + b.l.Error("failed to query L1 tip", "err", err) break } b.batchSubmitter.recordL1Tip(l1tip) @@ -167,45 +206,34 @@ blockLoop: break } - tx, err := b.batchSubmitter.CreateSubmitTx(txdata.Bytes()) - if err != nil { - // record it as a failed TX to resubmit the transaction. - b.batchSubmitter.recordFailedTx(txdata.ID(), err) - return fmt.Errorf("failed to create batch submit transaction: %w", err) - } - b.l.Info("creating batch submit tx", "to", tx.To, "from", b.cfg.From) // Record TX Status - receipt, err := b.SendTransaction(b.ctx, tx) + receipt, err := b.sendTransaction(ctx, txdata.Bytes()) if err != nil { b.batchSubmitter.recordFailedTx(txdata.ID(), err) - return fmt.Errorf("failed to send batch transaction: %w", err) + return fmt.Errorf("failed to send batch submit transaction: %w", err) } b.batchSubmitter.recordConfirmedTx(txdata.ID(), receipt) - - // hack to exit this loop. Proper fix is to do request another send tx or parallel tx sending - // from the channel manager rather than sending the channel in a loop. This stalls b/c if the - // context is cancelled while sending, it will never fully clearing the pending txns. - select { - case <-b.ctx.Done(): - break blockLoop - default: - } } return nil } -// SendTransaction sends a transaction through the transaction manager which handles automatic -// price bumping, and returns transaction receipt. -// It also hardcodes a timeout of 100s. -func (b *Batcher) SendTransaction(ctx context.Context, tx *types.Transaction) (*types.Receipt, 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() +// 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 (b *Batcher) 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) + } - b.l.Info("batcher sending transaction", "tx", tx.Hash()) - receipt, err := b.txMgr.Send(cCtx, tx) + // Send the transaction through the txmgr + receipt, err := b.cfg.TxManager.Send(ctx, txmgr.TxCandidate{ + To: &b.batchSubmitter.Rollup.BatchInboxAddress, + TxData: data, + GasLimit: intrinsicGas, + }) if err != nil { b.l.Error("batcher unable to publish tx", "err", err) return nil, err diff --git a/components/batcher/channel_builder.go b/components/batcher/channel_builder.go index 2756a29f41..505f3441e6 100644 --- a/components/batcher/channel_builder.go +++ b/components/batcher/channel_builder.go @@ -13,14 +13,13 @@ import ( ) var ( - ErrZeroMaxFrameSize = errors.New("max frame size cannot be zero") - ErrSmallMaxFrameSize = errors.New("max frame size cannot be less than 23") ErrInvalidChannelTimeout = errors.New("channel timeout is less than the safety margin") ErrInputTargetReached = errors.New("target amount of input data reached") ErrMaxFrameIndex = errors.New("max frame index reached (uint16)") ErrMaxDurationReached = errors.New("max channel duration reached") ErrChannelTimeoutClose = errors.New("close to channel timeout") ErrProposerWindowClose = errors.New("close to proposer window timeout") + ErrTerminated = errors.New("channel terminated") ) type ChannelFullError struct { @@ -84,15 +83,15 @@ func (cc *ChannelConfig) Check() error { // will infinitely loop when trying to create frames in the // [channelBuilder.OutputFrames] function. if cc.MaxFrameSize == 0 { - return ErrZeroMaxFrameSize + return errors.New("max frame size cannot be zero") } - // If the [MaxFrameSize] is set to < 23, the channel out - // will underflow the maxSize variable in the [derive.ChannelOut]. + // If the [MaxFrameSize] is less than [FrameV0OverHeadSize], the channel + // out will underflow the maxSize variable in the [derive.ChannelOut]. // Since it is of type uint64, it will wrap around to a very large // number, making the frame size extremely large. - if cc.MaxFrameSize < 23 { - return ErrSmallMaxFrameSize + if cc.MaxFrameSize < derive.FrameV0OverHeadSize { + return fmt.Errorf("max frame size %d is less than the minimum 23", cc.MaxFrameSize) } return nil @@ -310,16 +309,17 @@ func (c *channelBuilder) IsFull() bool { // FullErr returns the reason why the channel is full. If not full yet, it // returns nil. // -// It returns a ChannelFullError wrapping one of six possible reasons for the -// channel being full: +// It returns a ChannelFullError wrapping one of the following possible reasons +// for the channel being full: // - ErrInputTargetReached if the target amount of input data has been reached, // - derive.MaxRLPBytesPerChannel if the general maximum amount of input data // would have been exceeded by the latest AddBlock call, // - ErrMaxFrameIndex if the maximum number of frames has been generated // (uint16), -// - ErrMaxDurationReached if the max channel duration got reached. -// - ErrChannelTimeoutClose if the consensus channel timeout got too close. -// - ErrProposerWindowClose if the end of the proposer window got too close. +// - ErrMaxDurationReached if the max channel duration got reached, +// - ErrChannelTimeoutClose if the consensus channel timeout got too close, +// - ErrProposerWindowClose if the end of the proposer window got too close, +// - ErrTerminated if the channel was explicitly terminated. func (c *channelBuilder) FullErr() error { return c.fullErr } @@ -405,6 +405,14 @@ func (c *channelBuilder) outputFrame() error { return err // possibly io.EOF (last frame) } +// Close immediately marks the channel as full with an ErrTerminated +// if the channel is not already full. +func (c *channelBuilder) Close() { + if !c.IsFull() { + c.setFullErr(ErrTerminated) + } +} + // HasFrame returns whether there's any available frame. If true, it can be // popped using NextFrame(). // diff --git a/components/batcher/channel_builder_test.go b/components/batcher/channel_builder_test.go index 3c6827701a..9d48cd398a 100644 --- a/components/batcher/channel_builder_test.go +++ b/components/batcher/channel_builder_test.go @@ -3,6 +3,7 @@ package batcher import ( "bytes" "errors" + "fmt" "math" "math/big" "math/rand" @@ -31,27 +32,79 @@ var defaultTestChannelConfig = ChannelConfig{ ApproxComprRatio: 0.4, } -// TestConfigValidation tests the validation of the [ChannelConfig] struct. -func TestConfigValidation(t *testing.T) { - // Construct a valid config. - validChannelConfig := defaultTestChannelConfig - require.NoError(t, validChannelConfig.Check()) - - // Set the config to have a zero max frame size. - validChannelConfig.MaxFrameSize = 0 - require.ErrorIs(t, validChannelConfig.Check(), ErrZeroMaxFrameSize) - - // Set the config to have a max frame size less than 23. - validChannelConfig.MaxFrameSize = 22 - require.ErrorIs(t, validChannelConfig.Check(), ErrSmallMaxFrameSize) - - // Reset the config and test the Timeout error. - // NOTE: We should be fuzzing these values with the constraint that - // SubSafetyMargin > ChannelTimeout to ensure validation. - validChannelConfig = defaultTestChannelConfig - validChannelConfig.ChannelTimeout = 0 - validChannelConfig.SubSafetyMargin = 1 - require.ErrorIs(t, validChannelConfig.Check(), ErrInvalidChannelTimeout) +// TestChannelConfig_Check tests the [ChannelConfig] [Check] function. +func TestChannelConfig_Check(t *testing.T) { + type test struct { + input ChannelConfig + assertion func(error) + } + + // Construct test cases that test the boundary conditions + zeroChannelConfig := defaultTestChannelConfig + zeroChannelConfig.MaxFrameSize = 0 + timeoutChannelConfig := defaultTestChannelConfig + timeoutChannelConfig.ChannelTimeout = 0 + timeoutChannelConfig.SubSafetyMargin = 1 + tests := []test{ + { + input: defaultTestChannelConfig, + assertion: func(output error) { + require.NoError(t, output) + }, + }, + { + input: timeoutChannelConfig, + assertion: func(output error) { + require.ErrorIs(t, output, ErrInvalidChannelTimeout) + }, + }, + { + input: zeroChannelConfig, + assertion: func(output error) { + require.EqualError(t, output, "max frame size cannot be zero") + }, + }, + } + for i := 1; i < derive.FrameV0OverHeadSize; i++ { + smallChannelConfig := defaultTestChannelConfig + smallChannelConfig.MaxFrameSize = uint64(i) + expectedErr := fmt.Sprintf("max frame size %d is less than the minimum 23", i) + tests = append(tests, test{ + input: smallChannelConfig, + assertion: func(output error) { + require.EqualError(t, output, expectedErr) + }, + }) + } + + // Run the table tests + for _, test := range tests { + test.assertion(test.input.Check()) + } +} + +// FuzzChannelConfig_CheckTimeout tests the [ChannelConfig] [Check] function +// with fuzzing to make sure that a [ErrInvalidChannelTimeout] is thrown when +// the [ChannelTimeout] is less than the [SubSafetyMargin]. +func FuzzChannelConfig_CheckTimeout(f *testing.F) { + for i := range [10]int{} { + f.Add(uint64(i+1), uint64(i)) + } + f.Fuzz(func(t *testing.T, channelTimeout uint64, subSafetyMargin uint64) { + // We only test where [ChannelTimeout] is less than the [SubSafetyMargin] + // So we cannot have [ChannelTimeout] be [math.MaxUint64] + if channelTimeout == math.MaxUint64 { + channelTimeout = math.MaxUint64 - 1 + } + if subSafetyMargin <= channelTimeout { + subSafetyMargin = channelTimeout + 1 + } + + channelConfig := defaultTestChannelConfig + channelConfig.ChannelTimeout = channelTimeout + channelConfig.SubSafetyMargin = subSafetyMargin + require.ErrorIs(t, channelConfig.Check(), ErrInvalidChannelTimeout) + }) } // addMiniBlock adds a minimal valid L2 block to the channel builder using the @@ -131,10 +184,10 @@ func FuzzDurationTimeoutZeroMaxChannelDuration(f *testing.F) { }) } -// FuzzDurationZero ensures that when whenever the MaxChannelDuration +// FuzzChannelBuilder_DurationZero ensures that when whenever the MaxChannelDuration // is not set to 0, the channel builder will always have a duration timeout // as long as the channel builder's timeout is set to 0. -func FuzzDurationZero(f *testing.F) { +func FuzzChannelBuilder_DurationZero(f *testing.F) { for i := range [10]int{} { f.Add(uint64(i), uint64(i)) } @@ -196,7 +249,7 @@ func FuzzDurationTimeoutMaxChannelDuration(f *testing.F) { // FuzzChannelCloseTimeout ensures that the channel builder has a [ErrChannelTimeoutClose] // as long as the timeout constraint is met and the builder's timeout is greater than -// the calculated timeout +// the calculated timeout. func FuzzChannelCloseTimeout(f *testing.F) { // Set multiple seeds in case fuzzing isn't explicitly used for i := range [10]int{} { @@ -251,7 +304,7 @@ func FuzzChannelZeroCloseTimeout(f *testing.F) { // FuzzProposerWindowClose ensures that the channel builder has a [ErrProposerWindowClose] // as long as the timeout constraint is met and the builder's timeout is greater than -// the calculated timeout +// the calculated timeout. func FuzzProposerWindowClose(f *testing.F) { // Set multiple seeds in case fuzzing isn't explicitly used for i := range [10]int{} { @@ -312,8 +365,8 @@ func FuzzProposerWindowZeroTimeoutClose(f *testing.F) { }) } -// TestBuilderNextFrame tests calling NextFrame on a ChannelBuilder with only one frame -func TestBuilderNextFrame(t *testing.T) { +// TestChannelBuilder_NextFrame tests calling NextFrame on a channelBuilder with only one frame. +func TestChannelBuilder_NextFrame(t *testing.T) { channelConfig := defaultTestChannelConfig // Create a new channel builder @@ -352,8 +405,8 @@ func TestBuilderNextFrame(t *testing.T) { require.PanicsWithValue(t, "no next frame", func() { cb.NextFrame() }) } -// TestBuilderInvalidFrameId tests that a panic is thrown when a frame is pushed with an invalid frame id -func TestBuilderWrongFramePanic(t *testing.T) { +// TestChannelBuilder_OutputWrongFramePanic tests that a panic is thrown when a frame is pushed with an invalid frame id. +func TestChannelBuilder_OutputWrongFramePanic(t *testing.T) { channelConfig := defaultTestChannelConfig // Construct a channel builder @@ -382,15 +435,14 @@ func TestBuilderWrongFramePanic(t *testing.T) { }) } -// TestOutputFrames tests the OutputFrames function -func TestOutputFrames(t *testing.T) { +// TestChannelBuilder_OutputFramesWorks tests the [channelBuilder] OutputFrames is successful. +func TestChannelBuilder_OutputFramesWorks(t *testing.T) { channelConfig := defaultTestChannelConfig - channelConfig.MaxFrameSize = 2 + channelConfig.MaxFrameSize = 24 // Construct the channel builder cb, err := newChannelBuilder(channelConfig) require.NoError(t, err) - require.False(t, cb.IsFull()) require.Equal(t, 0, cb.NumFrames()) @@ -399,40 +451,34 @@ func TestOutputFrames(t *testing.T) { require.NoError(t, cb.OutputFrames()) // There should be no ready bytes yet - readyBytes := cb.co.ReadyBytes() - require.Equal(t, 0, readyBytes) + require.Equal(t, 0, cb.co.ReadyBytes()) // Let's add a block - err = addMiniBlock(cb) - require.NoError(t, err) + require.NoError(t, addMiniBlock(cb)) + require.NoError(t, cb.co.Flush()) // Check how many ready bytes - readyBytes = cb.co.ReadyBytes() - require.Equal(t, 2, readyBytes) - + // There should be more than the max frame size ready + require.Greater(t, uint64(cb.co.ReadyBytes()), channelConfig.MaxFrameSize) require.Equal(t, 0, cb.NumFrames()) // The channel should not be full // but we want to output the frames for testing anyways - isFull := cb.IsFull() - require.False(t, isFull) - - // Since we manually set the max frame size to 2, - // we should be able to compress the two frames now - err = cb.OutputFrames() - require.NoError(t, err) + require.False(t, cb.IsFull()) - // There should be one frame in the channel builder now - require.Equal(t, 1, cb.NumFrames()) + // We should be able to output the frames + require.NoError(t, cb.OutputFrames()) - // There should no longer be any ready bytes - readyBytes = cb.co.ReadyBytes() - require.Equal(t, 0, readyBytes) + // There should be many frames in the channel builder now + require.Greater(t, cb.NumFrames(), 1) + for _, frame := range cb.frames { + require.Len(t, frame.data, int(channelConfig.MaxFrameSize)) + } } -// TestMaxRLPBytesPerChannel tests the [channelBuilder.OutputFrames] +// TestChannelBuilder_MaxRLPBytesPerChannel tests the [channelBuilder.OutputFrames] // function errors when the max RLP bytes per channel is reached. -func TestMaxRLPBytesPerChannel(t *testing.T) { +func TestChannelBuilder_MaxRLPBytesPerChannel(t *testing.T) { t.Parallel() channelConfig := defaultTestChannelConfig channelConfig.MaxFrameSize = derive.MaxRLPBytesPerChannel * 2 @@ -448,13 +494,13 @@ func TestMaxRLPBytesPerChannel(t *testing.T) { require.ErrorIs(t, err, derive.ErrTooManyRLPBytes) } -// TestOutputFramesMaxFrameIndex tests the [channelBuilder.OutputFrames] +// TestChannelBuilder_OutputFramesMaxFrameIndex tests the [channelBuilder.OutputFrames] // function errors when the max frame index is reached. -func TestOutputFramesMaxFrameIndex(t *testing.T) { +func TestChannelBuilder_OutputFramesMaxFrameIndex(t *testing.T) { channelConfig := defaultTestChannelConfig - channelConfig.MaxFrameSize = 1 + channelConfig.MaxFrameSize = 24 channelConfig.TargetNumFrames = math.MaxInt - channelConfig.TargetFrameSize = 1 + channelConfig.TargetFrameSize = 24 channelConfig.ApproxComprRatio = 0 // Continuously add blocks until the max frame index is reached @@ -476,6 +522,7 @@ func TestOutputFramesMaxFrameIndex(t *testing.T) { Number: big.NewInt(0), }, txs, nil, nil, trie.NewStackTrie(nil)) _, err = cb.AddBlock(a) + require.NoError(t, cb.co.Flush()) if cb.IsFull() { fullErr := cb.FullErr() require.ErrorIs(t, fullErr, ErrMaxFrameIndex) @@ -488,17 +535,15 @@ func TestOutputFramesMaxFrameIndex(t *testing.T) { } } -// TestBuilderAddBlock tests the AddBlock function -func TestBuilderAddBlock(t *testing.T) { +// TestChannelBuilder_AddBlock tests the AddBlock function +func TestChannelBuilder_AddBlock(t *testing.T) { channelConfig := defaultTestChannelConfig // Lower the max frame size so that we can batch - channelConfig.MaxFrameSize = 2 + channelConfig.MaxFrameSize = 30 // Configure the Input Threshold params so we observe a full channel - // In reality, we only need the input bytes (74) below to be greater than - // or equal to the input threshold (3 * 2) / 1 = 6 - channelConfig.TargetFrameSize = 3 + channelConfig.TargetFrameSize = 30 channelConfig.TargetNumFrames = 2 channelConfig.ApproxComprRatio = 1 @@ -507,8 +552,8 @@ func TestBuilderAddBlock(t *testing.T) { require.NoError(t, err) // Add a nonsense block to the channel builder - err = addMiniBlock(cb) - require.NoError(t, err) + require.NoError(t, addMiniBlock(cb)) + require.NoError(t, cb.co.Flush()) // Check the fields reset in the AddBlock function require.Equal(t, 74, cb.co.InputBytes()) @@ -518,23 +563,22 @@ func TestBuilderAddBlock(t *testing.T) { // Since the channel output is full, the next call to AddBlock // should return the channel out full error - err = addMiniBlock(cb) - require.ErrorIs(t, err, ErrInputTargetReached) + require.ErrorIs(t, addMiniBlock(cb), ErrInputTargetReached) } -// TestBuilderReset tests the Reset function -func TestBuilderReset(t *testing.T) { +// TestChannelBuilder_Reset tests the [Reset] function +func TestChannelBuilder_Reset(t *testing.T) { channelConfig := defaultTestChannelConfig // Lower the max frame size so that we can batch - channelConfig.MaxFrameSize = 2 + channelConfig.MaxFrameSize = 24 cb, err := newChannelBuilder(channelConfig) require.NoError(t, err) // Add a nonsense block to the channel builder - err = addMiniBlock(cb) - require.NoError(t, err) + require.NoError(t, addMiniBlock(cb)) + require.NoError(t, cb.co.Flush()) // Check the fields reset in the Reset function require.Equal(t, 1, len(cb.blocks)) @@ -545,22 +589,20 @@ func TestBuilderReset(t *testing.T) { require.NoError(t, cb.fullErr) // Output frames so we can set the channel builder frames - err = cb.OutputFrames() - require.NoError(t, err) + require.NoError(t, cb.OutputFrames()) // Add another block to increment the block count - err = addMiniBlock(cb) - require.NoError(t, err) + require.NoError(t, addMiniBlock(cb)) + require.NoError(t, cb.co.Flush()) // Check the fields reset in the Reset function require.Equal(t, 2, len(cb.blocks)) - require.Equal(t, 1, len(cb.frames)) + require.Greater(t, len(cb.frames), 1) require.Equal(t, timeout, cb.timeout) require.NoError(t, cb.fullErr) // Reset the channel builder - err = cb.Reset() - require.NoError(t, err) + require.NoError(t, cb.Reset()) // Check the fields reset in the Reset function require.Equal(t, 0, len(cb.blocks)) diff --git a/components/batcher/channel_manager.go b/components/batcher/channel_manager.go index 7d44fb5b99..5d27beec11 100644 --- a/components/batcher/channel_manager.go +++ b/components/batcher/channel_manager.go @@ -42,6 +42,9 @@ type channelManager struct { pendingTransactions map[txID]txData // Set of confirmed txID -> inclusion block. For determining if the channel is timed out confirmedTransactions map[txID]eth.BlockID + + // if set to true, prevents production of any new channel frames + closed bool } func NewChannelManager(log log.Logger, metr metrics.Metricer, cfg ChannelConfig) *channelManager { @@ -57,86 +60,91 @@ func NewChannelManager(log log.Logger, metr metrics.Metricer, cfg ChannelConfig) // Clear clears the entire state of the channel manager. // It is intended to be used after an L2 reorg. -func (s *channelManager) Clear() { - s.log.Trace("clearing channel manager state") - s.blocks = s.blocks[:0] - s.tip = common.Hash{} - s.clearPendingChannel() +func (c *channelManager) Clear() { + c.log.Trace("clearing channel manager state") + c.blocks = c.blocks[:0] + c.tip = common.Hash{} + c.closed = false + c.clearPendingChannel() } // TxFailed records a transaction as failed. It will attempt to resubmit the data // in the failed transaction. -func (s *channelManager) TxFailed(id txID) { - if data, ok := s.pendingTransactions[id]; ok { - s.log.Trace("marked transaction as failed", "id", id) +func (c *channelManager) TxFailed(id txID) { + if data, ok := c.pendingTransactions[id]; ok { + c.log.Trace("marked transaction as failed", "id", id) // Note: when the batcher is changed to send multiple frames per tx, // this needs to be changed to iterate over all frames of the tx data // and re-queue them. - s.pendingChannel.PushFrame(data.Frame()) - delete(s.pendingTransactions, id) + c.pendingChannel.PushFrame(data.Frame()) + delete(c.pendingTransactions, id) } else { - s.log.Warn("unknown transaction marked as failed", "id", id) + c.log.Warn("unknown transaction marked as failed", "id", id) } - s.metr.RecordBatchTxFailed() + c.metr.RecordBatchTxFailed() + if c.closed && len(c.confirmedTransactions) == 0 && len(c.pendingTransactions) == 0 { + c.log.Info("Channel has no submitted transactions, clearing for shutdown", "chID", c.pendingChannel.ID()) + c.clearPendingChannel() + } } // TxConfirmed marks a transaction as confirmed on L1. Unfortunately even if all frames in // a channel have been marked as confirmed on L1 the channel may be invalid & need to be // resubmitted. // This function may reset the pending channel if the pending channel has timed out. -func (s *channelManager) TxConfirmed(id txID, inclusionBlock eth.BlockID) { - s.metr.RecordBatchTxSubmitted() - s.log.Debug("marked transaction as confirmed", "id", id, "block", inclusionBlock) - if _, ok := s.pendingTransactions[id]; !ok { - s.log.Warn("unknown transaction marked as confirmed", "id", id, "block", inclusionBlock) +func (c *channelManager) TxConfirmed(id txID, inclusionBlock eth.BlockID) { + c.metr.RecordBatchTxSubmitted() + c.log.Debug("marked transaction as confirmed", "id", id, "block", inclusionBlock) + if _, ok := c.pendingTransactions[id]; !ok { + c.log.Warn("unknown transaction marked as confirmed", "id", id, "block", inclusionBlock) // TODO: This can occur if we clear the channel while there are still pending transactions // We need to keep track of stale transactions instead return } - delete(s.pendingTransactions, id) - s.confirmedTransactions[id] = inclusionBlock - s.pendingChannel.FramePublished(inclusionBlock.Number) + delete(c.pendingTransactions, id) + c.confirmedTransactions[id] = inclusionBlock + c.pendingChannel.FramePublished(inclusionBlock.Number) // If this channel timed out, put the pending blocks back into the local saved blocks // and then reset this state so it can try to build a new channel. - if s.pendingChannelIsTimedOut() { - s.metr.RecordChannelTimedOut(s.pendingChannel.ID()) - s.log.Warn("Channel timed out", "id", s.pendingChannel.ID()) - s.blocks = append(s.pendingChannel.Blocks(), s.blocks...) - s.clearPendingChannel() + if c.pendingChannelIsTimedOut() { + c.metr.RecordChannelTimedOut(c.pendingChannel.ID()) + c.log.Warn("Channel timed out", "id", c.pendingChannel.ID()) + c.blocks = append(c.pendingChannel.Blocks(), c.blocks...) + c.clearPendingChannel() } // If we are done with this channel, record that. - if s.pendingChannelIsFullySubmitted() { - s.metr.RecordChannelFullySubmitted(s.pendingChannel.ID()) - s.log.Info("Channel is fully submitted", "id", s.pendingChannel.ID()) - s.clearPendingChannel() + if c.pendingChannelIsFullySubmitted() { + c.metr.RecordChannelFullySubmitted(c.pendingChannel.ID()) + c.log.Info("Channel is fully submitted", "id", c.pendingChannel.ID()) + c.clearPendingChannel() } } // clearPendingChannel resets all pending state back to an initialized but empty state. // TODO: Create separate "pending" state -func (s *channelManager) clearPendingChannel() { - s.pendingChannel = nil - s.pendingTransactions = make(map[txID]txData) - s.confirmedTransactions = make(map[txID]eth.BlockID) +func (c *channelManager) clearPendingChannel() { + c.pendingChannel = nil + c.pendingTransactions = make(map[txID]txData) + c.confirmedTransactions = make(map[txID]eth.BlockID) } // pendingChannelIsTimedOut returns true if submitted channel has timed out. // A channel has timed out if the difference in L1 Inclusion blocks between // the first & last included block is greater than or equal to the channel timeout. -func (s *channelManager) pendingChannelIsTimedOut() bool { - if s.pendingChannel == nil { +func (c *channelManager) pendingChannelIsTimedOut() bool { + if c.pendingChannel == nil { return false // no channel to be timed out } // No confirmed transactions => not timed out - if len(s.confirmedTransactions) == 0 { + if len(c.confirmedTransactions) == 0 { return false } // If there are confirmed transactions, find the first + last confirmed block numbers min := uint64(math.MaxUint64) max := uint64(0) - for _, inclusionBlock := range s.confirmedTransactions { + for _, inclusionBlock := range c.confirmedTransactions { if inclusionBlock.Number < min { min = inclusionBlock.Number } @@ -144,30 +152,30 @@ func (s *channelManager) pendingChannelIsTimedOut() bool { max = inclusionBlock.Number } } - return max-min >= s.cfg.ChannelTimeout + return max-min >= c.cfg.ChannelTimeout } // pendingChannelIsFullySubmitted returns true if the channel has been fully submitted. -func (s *channelManager) pendingChannelIsFullySubmitted() bool { - if s.pendingChannel == nil { +func (c *channelManager) pendingChannelIsFullySubmitted() bool { + if c.pendingChannel == nil { return false // todo: can decide either way here. Nonsensical answer though } - return s.pendingChannel.IsFull() && len(s.pendingTransactions)+s.pendingChannel.NumFrames() == 0 + return c.pendingChannel.IsFull() && len(c.pendingTransactions)+c.pendingChannel.NumFrames() == 0 } -// nextTxData pops off s.datas & handles updating the internal state -func (s *channelManager) nextTxData() (txData, error) { - if s.pendingChannel == nil || !s.pendingChannel.HasFrame() { - s.log.Trace("no next tx data") +// nextTxData pops off c.datas & handles updating the internal state +func (c *channelManager) nextTxData() (txData, error) { + if c.pendingChannel == nil || !c.pendingChannel.HasFrame() { + c.log.Trace("no next tx data") return txData{}, io.EOF // TODO: not enough data error instead } - frame := s.pendingChannel.NextFrame() + frame := c.pendingChannel.NextFrame() txdata := txData{frame} id := txdata.ID() - s.log.Trace("returning next tx data", "id", id) - s.pendingTransactions[id] = txdata + c.log.Trace("returning next tx data", "id", id) + c.pendingTransactions[id] = txdata return txdata, nil } @@ -176,81 +184,81 @@ func (s *channelManager) nextTxData() (txData, error) { // It currently only uses one frame per transaction. If the pending channel is // full, it only returns the remaining frames of this channel until it got // successfully fully sent to L1. It returns io.EOF if there's no pending frame. -func (s *channelManager) TxData(l1Head eth.BlockID) (txData, error) { - dataPending := s.pendingChannel != nil && s.pendingChannel.HasFrame() - s.log.Debug("Requested tx data", "l1Head", l1Head, "data_pending", dataPending, "blocks_pending", len(s.blocks)) +func (c *channelManager) TxData(l1Head eth.BlockID) (txData, error) { + dataPending := c.pendingChannel != nil && c.pendingChannel.HasFrame() + c.log.Debug("Requested tx data", "l1Head", l1Head, "data_pending", dataPending, "blocks_pending", len(c.blocks)) - // Short circuit if there is a pending frame. - if dataPending { - return s.nextTxData() + // Short circuit if there is a pending frame or the channel manager is closed. + if dataPending || c.closed { + return c.nextTxData() } // No pending frame, so we have to add new blocks to the channel // If we have no saved blocks, we will not be able to create valid frames - if len(s.blocks) == 0 { + if len(c.blocks) == 0 { return txData{}, io.EOF } - if err := s.ensurePendingChannel(l1Head); err != nil { + if err := c.ensurePendingChannel(l1Head); err != nil { return txData{}, err } - if err := s.processBlocks(); err != nil { + if err := c.processBlocks(); err != nil { return txData{}, err } // Register current L1 head only after all pending blocks have been // processed. Even if a timeout will be triggered now, it is better to have // all pending blocks be included in this channel for submission. - s.registerL1Block(l1Head) + c.registerL1Block(l1Head) - if err := s.outputFrames(); err != nil { + if err := c.outputFrames(); err != nil { return txData{}, err } - return s.nextTxData() + return c.nextTxData() } -func (s *channelManager) ensurePendingChannel(l1Head eth.BlockID) error { - if s.pendingChannel != nil { +func (c *channelManager) ensurePendingChannel(l1Head eth.BlockID) error { + if c.pendingChannel != nil { return nil } - cb, err := newChannelBuilder(s.cfg) + cb, err := newChannelBuilder(c.cfg) if err != nil { return fmt.Errorf("creating new channel: %w", err) } - s.pendingChannel = cb - s.log.Info("Created channel", + c.pendingChannel = cb + c.log.Info("Created channel", "id", cb.ID(), "l1Head", l1Head, - "blocks_pending", len(s.blocks)) - s.metr.RecordChannelOpened(cb.ID(), len(s.blocks)) + "blocks_pending", len(c.blocks)) + c.metr.RecordChannelOpened(cb.ID(), len(c.blocks)) return nil } // registerL1Block registers the given block at the pending channel. -func (s *channelManager) registerL1Block(l1Head eth.BlockID) { - s.pendingChannel.RegisterL1Block(l1Head.Number) - s.log.Debug("new L1-block registered at channel builder", +func (c *channelManager) registerL1Block(l1Head eth.BlockID) { + c.pendingChannel.RegisterL1Block(l1Head.Number) + c.log.Debug("new L1-block registered at channel builder", "l1Head", l1Head, - "channel_full", s.pendingChannel.IsFull(), - "full_reason", s.pendingChannel.FullErr(), + "channel_full", c.pendingChannel.IsFull(), + "full_reason", c.pendingChannel.FullErr(), ) } // processBlocks adds blocks from the blocks queue to the pending channel until // either the queue got exhausted or the channel is full. -func (s *channelManager) processBlocks() error { +func (c *channelManager) processBlocks() error { var ( blocksAdded int _chFullErr *ChannelFullError // throw away, just for type checking latestL2ref eth.L2BlockRef ) - for i, block := range s.blocks { - l1info, err := s.pendingChannel.AddBlock(block) + for i, block := range c.blocks { + l1info, err := c.pendingChannel.AddBlock(block) if errors.As(err, &_chFullErr) { // current block didn't get added because channel is already full break @@ -260,63 +268,63 @@ func (s *channelManager) processBlocks() error { blocksAdded += 1 latestL2ref = l2BlockRefFromBlockAndL1Info(block, l1info) // current block got added but channel is now full - if s.pendingChannel.IsFull() { + if c.pendingChannel.IsFull() { break } } - if blocksAdded == len(s.blocks) { + if blocksAdded == len(c.blocks) { // all blocks processed, reuse slice - s.blocks = s.blocks[:0] + c.blocks = c.blocks[:0] } else { // remove processed blocks - s.blocks = s.blocks[blocksAdded:] + c.blocks = c.blocks[blocksAdded:] } - s.metr.RecordL2BlocksAdded(latestL2ref, + c.metr.RecordL2BlocksAdded(latestL2ref, blocksAdded, - len(s.blocks), - s.pendingChannel.InputBytes(), - s.pendingChannel.ReadyBytes()) - s.log.Debug("Added blocks to channel", + len(c.blocks), + c.pendingChannel.InputBytes(), + c.pendingChannel.ReadyBytes()) + c.log.Debug("Added blocks to channel", "blocks_added", blocksAdded, - "blocks_pending", len(s.blocks), - "channel_full", s.pendingChannel.IsFull(), - "input_bytes", s.pendingChannel.InputBytes(), - "ready_bytes", s.pendingChannel.ReadyBytes(), + "blocks_pending", len(c.blocks), + "channel_full", c.pendingChannel.IsFull(), + "input_bytes", c.pendingChannel.InputBytes(), + "ready_bytes", c.pendingChannel.ReadyBytes(), ) return nil } -func (s *channelManager) outputFrames() error { - if err := s.pendingChannel.OutputFrames(); err != nil { +func (c *channelManager) outputFrames() error { + if err := c.pendingChannel.OutputFrames(); err != nil { return fmt.Errorf("creating frames with channel builder: %w", err) } - if !s.pendingChannel.IsFull() { + if !c.pendingChannel.IsFull() { return nil } - inBytes, outBytes := s.pendingChannel.InputBytes(), s.pendingChannel.OutputBytes() - s.metr.RecordChannelClosed( - s.pendingChannel.ID(), - len(s.blocks), - s.pendingChannel.NumFrames(), + inBytes, outBytes := c.pendingChannel.InputBytes(), c.pendingChannel.OutputBytes() + c.metr.RecordChannelClosed( + c.pendingChannel.ID(), + len(c.blocks), + c.pendingChannel.NumFrames(), inBytes, outBytes, - s.pendingChannel.FullErr(), + c.pendingChannel.FullErr(), ) var comprRatio float64 if inBytes > 0 { comprRatio = float64(outBytes) / float64(inBytes) } - s.log.Info("Channel closed", - "id", s.pendingChannel.ID(), - "blocks_pending", len(s.blocks), - "num_frames", s.pendingChannel.NumFrames(), + c.log.Info("Channel closed", + "id", c.pendingChannel.ID(), + "blocks_pending", len(c.blocks), + "num_frames", c.pendingChannel.NumFrames(), "input_bytes", inBytes, "output_bytes", outBytes, - "full_reason", s.pendingChannel.FullErr(), + "full_reason", c.pendingChannel.FullErr(), "compr_ratio", comprRatio, ) return nil @@ -325,12 +333,12 @@ func (s *channelManager) outputFrames() error { // AddL2Block adds an L2 block to the internal blocks queue. It returns ErrReorg // if the block does not extend the last block loaded into the state. If no // blocks were added yet, the parent hash check is skipped. -func (s *channelManager) AddL2Block(block *types.Block) error { - if s.tip != (common.Hash{}) && s.tip != block.ParentHash() { +func (c *channelManager) AddL2Block(block *types.Block) error { + if c.tip != (common.Hash{}) && c.tip != block.ParentHash() { return ErrReorg } - s.blocks = append(s.blocks, block) - s.tip = block.Hash() + c.blocks = append(c.blocks, block) + c.tip = block.Hash() return nil } @@ -345,3 +353,27 @@ func l2BlockRefFromBlockAndL1Info(block *types.Block, l1info derive.L1BlockInfo) SequenceNumber: l1info.SequenceNumber, } } + +// Close closes the current pending channel, if one exists, outputs any remaining frames, +// and prevents the creation of any new channels. +// Any outputted frames still need to be published. +func (c *channelManager) Close() error { + if c.closed { + return nil + } + + c.closed = true + + // Any pending state can be proactively cleared if there are no submitted transactions + if len(c.confirmedTransactions) == 0 && len(c.pendingTransactions) == 0 { + c.clearPendingChannel() + } + + if c.pendingChannel == nil { + return nil + } + + c.pendingChannel.Close() + + return c.outputFrames() +} diff --git a/components/batcher/channel_manager_test.go b/components/batcher/channel_manager_test.go index f8e6dfbb66..dc845c91f1 100644 --- a/components/batcher/channel_manager_test.go +++ b/components/batcher/channel_manager_test.go @@ -33,8 +33,7 @@ func TestPendingChannelTimeout(t *testing.T) { require.False(t, timeout) // Set the pending channel - err := m.ensurePendingChannel(eth.BlockID{}) - require.NoError(t, err) + require.NoError(t, m.ensurePendingChannel(eth.BlockID{})) // There are no confirmed transactions so // the pending channel cannot be timed out @@ -86,14 +85,10 @@ func TestChannelManagerReturnsErrReorg(t *testing.T) { ParentHash: common.Hash{0xff}, }, nil, nil, nil, nil) - err := m.AddL2Block(a) - require.NoError(t, err) - err = m.AddL2Block(b) - require.NoError(t, err) - err = m.AddL2Block(c) - require.NoError(t, err) - err = m.AddL2Block(x) - require.ErrorIs(t, err, ErrReorg) + require.NoError(t, m.AddL2Block(a)) + require.NoError(t, m.AddL2Block(b)) + require.NoError(t, m.AddL2Block(c)) + require.ErrorIs(t, m.AddL2Block(x), ErrReorg) require.Equal(t, []*types.Block{a, b, c}, m.blocks) } @@ -112,16 +107,14 @@ func TestChannelManagerReturnsErrReorgWhenDrained(t *testing.T) { a := newMiniL2Block(0) x := newMiniL2BlockWithNumberParent(0, big.NewInt(1), common.Hash{0xff}) - err := m.AddL2Block(a) - require.NoError(t, err) + require.NoError(t, m.AddL2Block(a)) - _, err = m.TxData(eth.BlockID{}) + _, err := m.TxData(eth.BlockID{}) require.NoError(t, err) _, err = m.TxData(eth.BlockID{}) require.ErrorIs(t, err, io.EOF) - err = m.AddL2Block(x) - require.ErrorIs(t, err, ErrReorg) + require.ErrorIs(t, m.AddL2Block(x), ErrReorg) } // TestChannelManagerNextTxData checks the nextTxData function. @@ -137,8 +130,7 @@ func TestChannelManagerNextTxData(t *testing.T) { // Set the pending channel // The nextTxData function should still return EOF // since the pending channel has no frames - err = m.ensurePendingChannel(eth.BlockID{}) - require.NoError(t, err) + require.NoError(t, m.ensurePendingChannel(eth.BlockID{})) returnedTxData, err = m.nextTxData() require.ErrorIs(t, err, io.EOF) require.Equal(t, txData{}, returnedTxData) @@ -165,8 +157,10 @@ func TestChannelManagerNextTxData(t *testing.T) { require.Equal(t, expectedTxData, m.pendingTransactions[expectedChannelID]) } -// TestClearChannelManager tests clearing the channel manager. -func TestClearChannelManager(t *testing.T) { +// TestChannelManager_Clear tests clearing the channel manager. +func TestChannelManager_Clear(t *testing.T) { + require := require.New(t) + // Create a channel manager log := testlog.Logger(t, log.LvlCrit) rng := rand.New(rand.NewSource(time.Now().UnixNano())) @@ -177,15 +171,17 @@ func TestClearChannelManager(t *testing.T) { ChannelTimeout: 10, // Have to set the max frame size here otherwise the channel builder would not // be able to output any frames - MaxFrameSize: 1, + MaxFrameSize: 24, + TargetFrameSize: 24, + ApproxComprRatio: 1.0, }) // Channel Manager state should be empty by default - require.Empty(t, m.blocks) - require.Equal(t, common.Hash{}, m.tip) - require.Nil(t, m.pendingChannel) - require.Empty(t, m.pendingTransactions) - require.Empty(t, m.confirmedTransactions) + require.Empty(m.blocks) + require.Equal(common.Hash{}, m.tip) + require.Nil(m.pendingChannel) + require.Empty(m.pendingTransactions) + require.Empty(m.confirmedTransactions) // Add a block to the channel manager a, _ := derivetest.RandomL2Block(rng, 4) @@ -194,28 +190,25 @@ func TestClearChannelManager(t *testing.T) { Hash: a.Hash(), Number: a.NumberU64(), } - err := m.AddL2Block(a) - require.NoError(t, err) + require.NoError(m.AddL2Block(a)) // Make sure there is a channel builder - err = m.ensurePendingChannel(l1BlockID) - require.NoError(t, err) - require.NotNil(t, m.pendingChannel) - require.Equal(t, 0, len(m.confirmedTransactions)) + require.NoError(m.ensurePendingChannel(l1BlockID)) + require.NotNil(m.pendingChannel) + require.Len(m.confirmedTransactions, 0) // Process the blocks // We should have a pending channel with 1 frame // and no more blocks since processBlocks consumes // the list - err = m.processBlocks() - require.NoError(t, err) - err = m.pendingChannel.OutputFrames() - require.NoError(t, err) - _, err = m.nextTxData() - require.NoError(t, err) - require.Equal(t, 0, len(m.blocks)) - require.Equal(t, newL1Tip, m.tip) - require.Equal(t, 1, len(m.pendingTransactions)) + require.NoError(m.processBlocks()) + require.NoError(m.pendingChannel.co.Flush()) + require.NoError(m.pendingChannel.OutputFrames()) + _, err := m.nextTxData() + require.NoError(err) + require.Len(m.blocks, 0) + require.Equal(newL1Tip, m.tip) + require.Len(m.pendingTransactions, 1) // Add a new block so we can test clearing // the channel manager with a full state @@ -223,20 +216,19 @@ func TestClearChannelManager(t *testing.T) { Number: big.NewInt(1), ParentHash: a.Hash(), }, nil, nil, nil, nil) - err = m.AddL2Block(b) - require.NoError(t, err) - require.Equal(t, 1, len(m.blocks)) - require.Equal(t, b.Hash(), m.tip) + require.NoError(m.AddL2Block(b)) + require.Len(m.blocks, 1) + require.Equal(b.Hash(), m.tip) // Clear the channel manager m.Clear() // Check that the entire channel manager state cleared - require.Empty(t, m.blocks) - require.Equal(t, common.Hash{}, m.tip) - require.Nil(t, m.pendingChannel) - require.Empty(t, m.pendingTransactions) - require.Empty(t, m.confirmedTransactions) + require.Empty(m.blocks) + require.Equal(common.Hash{}, m.tip) + require.Nil(m.pendingChannel) + require.Empty(m.pendingTransactions) + require.Empty(m.confirmedTransactions) } // TestChannelManagerTxConfirmed checks the [ChannelManager.TxConfirmed] function. @@ -252,8 +244,7 @@ func TestChannelManagerTxConfirmed(t *testing.T) { // Let's add a valid pending transaction to the channel manager // So we can demonstrate that TxConfirmed's correctness - err := m.ensurePendingChannel(eth.BlockID{}) - require.NoError(t, err) + require.NoError(t, m.ensurePendingChannel(eth.BlockID{})) channelID := m.pendingChannel.ID() frame := frameData{ data: []byte{}, @@ -271,7 +262,7 @@ func TestChannelManagerTxConfirmed(t *testing.T) { require.Equal(t, expectedTxData, returnedTxData) require.Equal(t, 0, m.pendingChannel.NumFrames()) require.Equal(t, expectedTxData, m.pendingTransactions[expectedChannelID]) - require.Equal(t, 1, len(m.pendingTransactions)) + require.Len(t, m.pendingTransactions, 1) // An unknown pending transaction should not be marked as confirmed // and should not be removed from the pending transactions map @@ -282,14 +273,14 @@ func TestChannelManagerTxConfirmed(t *testing.T) { blockID := eth.BlockID{Number: 0, Hash: common.Hash{0x69}} m.TxConfirmed(unknownTxID, blockID) require.Empty(t, m.confirmedTransactions) - require.Equal(t, 1, len(m.pendingTransactions)) + require.Len(t, m.pendingTransactions, 1) // Now let's mark the pending transaction as confirmed // and check that it is removed from the pending transactions map // and added to the confirmed transactions map m.TxConfirmed(expectedChannelID, blockID) require.Empty(t, m.pendingTransactions) - require.Equal(t, 1, len(m.confirmedTransactions)) + require.Len(t, m.confirmedTransactions, 1) require.Equal(t, blockID, m.confirmedTransactions[expectedChannelID]) } @@ -301,8 +292,7 @@ func TestChannelManagerTxFailed(t *testing.T) { // Let's add a valid pending transaction to the channel // manager so we can demonstrate correctness - err := m.ensurePendingChannel(eth.BlockID{}) - require.NoError(t, err) + require.NoError(t, m.ensurePendingChannel(eth.BlockID{})) channelID := m.pendingChannel.ID() frame := frameData{ data: []byte{}, @@ -320,7 +310,7 @@ func TestChannelManagerTxFailed(t *testing.T) { require.Equal(t, expectedTxData, returnedTxData) require.Equal(t, 0, m.pendingChannel.NumFrames()) require.Equal(t, expectedTxData, m.pendingTransactions[expectedChannelID]) - require.Equal(t, 1, len(m.pendingTransactions)) + require.Len(t, m.pendingTransactions, 1) // Trying to mark an unknown pending transaction as failed // shouldn't modify state @@ -349,8 +339,7 @@ func TestChannelManager_TxResend(t *testing.T) { a, _ := derivetest.RandomL2Block(rng, 4) - err := m.AddL2Block(a) - require.NoError(err) + require.NoError(m.AddL2Block(a)) txdata0, err := m.TxData(eth.BlockID{}) require.NoError(err) @@ -375,3 +364,145 @@ func TestChannelManager_TxResend(t *testing.T) { require.NoError(err) require.Len(fs, 1) } + +// TestChannelManagerCloseBeforeFirstUse ensures that the channel manager +// will not produce any frames if closed immediately. +func TestChannelManagerCloseBeforeFirstUse(t *testing.T) { + require := require.New(t) + rng := rand.New(rand.NewSource(time.Now().UnixNano())) + log := testlog.Logger(t, log.LvlCrit) + m := NewChannelManager(log, metrics.NoopMetrics, + ChannelConfig{ + TargetFrameSize: 0, + MaxFrameSize: 100, + ApproxComprRatio: 1.0, + ChannelTimeout: 1000, + }) + + a, _ := derivetest.RandomL2Block(rng, 4) + + m.Close() + + err := m.AddL2Block(a) + require.NoError(err, "Failed to add L2 block") + + _, err = m.TxData(eth.BlockID{}) + require.ErrorIs(err, io.EOF, "Expected closed channel manager to contain no tx data") +} + +// TestChannelManagerCloseNoPendingChannel ensures that the channel manager +// can gracefully close with no pending channels, and will not emit any new +// channel frames. +func TestChannelManagerCloseNoPendingChannel(t *testing.T) { + require := require.New(t) + log := testlog.Logger(t, log.LvlCrit) + m := NewChannelManager(log, metrics.NoopMetrics, + ChannelConfig{ + TargetFrameSize: 0, + MaxFrameSize: 100, + ApproxComprRatio: 1.0, + ChannelTimeout: 1000, + }) + a := newMiniL2Block(0) + b := newMiniL2BlockWithNumberParent(0, big.NewInt(1), a.Hash()) + + err := m.AddL2Block(a) + require.NoError(err, "Failed to add L2 block") + + txdata, err := m.TxData(eth.BlockID{}) + require.NoError(err, "Expected channel manager to return valid tx data") + + m.TxConfirmed(txdata.ID(), eth.BlockID{}) + + _, err = m.TxData(eth.BlockID{}) + require.ErrorIs(err, io.EOF, "Expected channel manager to EOF") + + m.Close() + + err = m.AddL2Block(b) + require.NoError(err, "Failed to add L2 block") + + _, err = m.TxData(eth.BlockID{}) + require.ErrorIs(err, io.EOF, "Expected closed channel manager to return no new tx data") +} + +// TestChannelManagerCloseNoPendingChannel ensures that the channel manager +// can gracefully close with a pending channel, and will not produce any +// new channel frames after this point. +func TestChannelManagerClosePendingChannel(t *testing.T) { + require := require.New(t) + log := testlog.Logger(t, log.LvlCrit) + m := NewChannelManager(log, metrics.NoopMetrics, + ChannelConfig{ + TargetNumFrames: 100, + TargetFrameSize: 1000, + MaxFrameSize: 1000, + ApproxComprRatio: 1.0, + ChannelTimeout: 1000, + }) + + a := newMiniL2Block(50_000) + b := newMiniL2BlockWithNumberParent(10, big.NewInt(1), a.Hash()) + + err := m.AddL2Block(a) + require.NoError(err, "Failed to add L2 block") + + txdata, err := m.TxData(eth.BlockID{}) + require.NoError(err, "Expected channel manager to produce valid tx data") + + m.TxConfirmed(txdata.ID(), eth.BlockID{}) + + m.Close() + + txdata, err = m.TxData(eth.BlockID{}) + require.NoError(err, "Expected channel manager to produce tx data from remaining L2 block data") + + m.TxConfirmed(txdata.ID(), eth.BlockID{}) + + _, err = m.TxData(eth.BlockID{}) + require.ErrorIs(err, io.EOF, "Expected channel manager to have no more tx data") + + err = m.AddL2Block(b) + require.NoError(err, "Failed to add L2 block") + + _, err = m.TxData(eth.BlockID{}) + require.ErrorIs(err, io.EOF, "Expected closed channel manager to produce no more tx data") +} + +// TestChannelManagerCloseAllTxsFailed ensures that the channel manager +// can gracefully close after producing transaction frames if none of these +// have successfully landed on chain. +func TestChannelManagerCloseAllTxsFailed(t *testing.T) { + require := require.New(t) + log := testlog.Logger(t, log.LvlCrit) + m := NewChannelManager(log, metrics.NoopMetrics, + ChannelConfig{ + TargetNumFrames: 100, + TargetFrameSize: 1000, + MaxFrameSize: 1000, + ApproxComprRatio: 1.0, + ChannelTimeout: 1000, + }) + + a := newMiniL2Block(50_000) + + err := m.AddL2Block(a) + require.NoError(err, "Failed to add L2 block") + + txdata, err := m.TxData(eth.BlockID{}) + require.NoError(err, "Expected channel manager to produce valid tx data") + + m.TxFailed(txdata.ID()) + + // Show that this data will continue to be emitted as long as the transaction + // fails and the channel manager is not closed + txdata, err = m.TxData(eth.BlockID{}) + require.NoError(err, "Expected channel manager to re-attempt the failed transaction") + + m.TxFailed(txdata.ID()) + + m.Close() + + _, err = m.TxData(eth.BlockID{}) + require.ErrorIs(err, io.EOF, "Expected closed channel manager to produce no more tx data") +} diff --git a/components/batcher/cmd/main.go b/components/batcher/cmd/main.go index 057986d110..baeec94ffc 100644 --- a/components/batcher/cmd/main.go +++ b/components/batcher/cmd/main.go @@ -24,9 +24,8 @@ func main() { app.Flags = flags.Flags app.Version = fmt.Sprintf("%s-%s", Version, Meta) app.Name = "kroma-batcher" - app.Usage = "Batch Submitter Service" - app.Description = "Service for generating and submitting L2 tx batches " + - "to L1" + app.Usage = "Batcher Service" + app.Description = "Service for generating and submitting L2 tx batches to L1" app.Action = curryMain(Version) err := app.Run(os.Args) diff --git a/components/batcher/config.go b/components/batcher/config.go index 11d8e0d06f..937f3b7014 100644 --- a/components/batcher/config.go +++ b/components/batcher/config.go @@ -5,7 +5,6 @@ import ( "fmt" "time" - "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/ethclient" "github.com/ethereum/go-ethereum/log" "github.com/urfave/cli" @@ -16,12 +15,10 @@ import ( "github.com/kroma-network/kroma/components/node/rollup" "github.com/kroma-network/kroma/components/node/sources" "github.com/kroma-network/kroma/utils" - kcrypto "github.com/kroma-network/kroma/utils/service/crypto" klog "github.com/kroma-network/kroma/utils/service/log" kmetrics "github.com/kroma-network/kroma/utils/service/metrics" kpprof "github.com/kroma-network/kroma/utils/service/pprof" "github.com/kroma-network/kroma/utils/service/txmgr" - ksigner "github.com/kroma-network/kroma/utils/signer/client" ) type Config struct { @@ -30,13 +27,12 @@ type Config struct { L1Client *ethclient.Client L2Client *ethclient.Client RollupClient *sources.RollupClient + TxManager txmgr.TxManager - PollInterval time.Duration - From common.Address - - TxManagerConfig txmgr.Config + NetworkTimeout time.Duration + PollInterval time.Duration - // RollupConfig is queried at startup + // Rollup config is queried at startup Rollup *rollup.Config // Channel builder parameters @@ -55,8 +51,6 @@ func (c *Config) Check() error { } type CLIConfig struct { - /* Required Params */ - // L1EthRpc is the HTTP provider URL for L1. L1EthRpc string @@ -70,8 +64,6 @@ type CLIConfig struct { // channel open. This allows to more eagerly send batcher transactions // during times of low L2 transaction volume. Note that the effective // L1-block distance between batcher transactions is then MaxChannelDuration - // + NumConfirmations because the batcher waits for NumConfirmations blocks - // after sending a batcher tx and only then starts a new channel. // // If 0, duration checks are disabled. MaxChannelDuration uint64 @@ -85,34 +77,6 @@ type CLIConfig struct { // and creating a new batch. PollInterval time.Duration - // NumConfirmations is the number of confirmations which we will wait after - // appending new batches. - NumConfirmations uint64 - - // SafeAbortNonceTooLowCount is the number of ErrNonceTooLowObservations - // required to give up on a tx at a particular nonce without receiving - // confirmation. - SafeAbortNonceTooLowCount uint64 - - // ResubmissionTimeout is time we will wait before resubmitting a - // transaction. - ResubmissionTimeout time.Duration - - // Mnemonic is the HD seed used to derive the wallet private keys for - // the batcher. - Mnemonic string - - // HDPath is the derivation path used to obtain the private key for - // the batcher. - HDPath string - - // PrivateKey is the private key used for the batcher. - PrivateKey string - - RPCConfig rpc.CLIConfig - - /* Optional Params */ - // MaxL1TxSize is the maximum size of a batch tx submitted to L1. MaxL1TxSize uint64 @@ -126,14 +90,11 @@ type CLIConfig struct { // compression algorithm. ApproxComprRatio float64 - LogConfig klog.CLIConfig - + TxMgrConfig txmgr.CLIConfig + RPCConfig rpc.CLIConfig + LogConfig klog.CLIConfig MetricsConfig kmetrics.CLIConfig - - PprofConfig kpprof.CLIConfig - - // SignerConfig contains the client config for signer service - SignerConfig ksigner.CLIConfig + PprofConfig kpprof.CLIConfig } func (c CLIConfig) Check() error { @@ -149,7 +110,7 @@ func (c CLIConfig) Check() error { if err := c.PprofConfig.Check(); err != nil { return err } - if err := c.SignerConfig.Check(); err != nil { + if err := c.TxMgrConfig.Check(); err != nil { return err } return nil @@ -158,42 +119,32 @@ func (c CLIConfig) Check() error { // NewCLIConfig parses the CLIConfig from the provided flags or environment variables. func NewCLIConfig(ctx *cli.Context) CLIConfig { return CLIConfig{ - /* Required Flags */ - L1EthRpc: ctx.GlobalString(flags.L1EthRpcFlag.Name), - L2EthRpc: ctx.GlobalString(flags.L2EthRpcFlag.Name), - RollupRpc: ctx.GlobalString(flags.RollupRpcFlag.Name), - SubSafetyMargin: ctx.GlobalUint64(flags.SubSafetyMarginFlag.Name), - PollInterval: ctx.GlobalDuration(flags.PollIntervalFlag.Name), - NumConfirmations: ctx.GlobalUint64(flags.NumConfirmationsFlag.Name), - SafeAbortNonceTooLowCount: ctx.GlobalUint64(flags.SafeAbortNonceTooLowCountFlag.Name), - ResubmissionTimeout: ctx.GlobalDuration(flags.ResubmissionTimeoutFlag.Name), - - /* Optional Flags */ + // Required Flags + L1EthRpc: ctx.GlobalString(flags.L1EthRpcFlag.Name), + L2EthRpc: ctx.GlobalString(flags.L2EthRpcFlag.Name), + RollupRpc: ctx.GlobalString(flags.RollupRpcFlag.Name), + SubSafetyMargin: ctx.GlobalUint64(flags.SubSafetyMarginFlag.Name), + PollInterval: ctx.GlobalDuration(flags.PollIntervalFlag.Name), + + // Optional Flags MaxChannelDuration: ctx.GlobalUint64(flags.MaxChannelDurationFlag.Name), MaxL1TxSize: ctx.GlobalUint64(flags.MaxL1TxSizeBytesFlag.Name), TargetL1TxSize: ctx.GlobalUint64(flags.TargetL1TxSizeBytesFlag.Name), TargetNumFrames: ctx.GlobalInt(flags.TargetNumFramesFlag.Name), ApproxComprRatio: ctx.GlobalFloat64(flags.ApproxComprRatioFlag.Name), - Mnemonic: ctx.GlobalString(flags.MnemonicFlag.Name), - HDPath: ctx.GlobalString(flags.HDPathFlag.Name), - PrivateKey: ctx.GlobalString(flags.PrivateKeyFlag.Name), + TxMgrConfig: txmgr.ReadCLIConfig(ctx), RPCConfig: rpc.ReadCLIConfig(ctx), LogConfig: klog.ReadCLIConfig(ctx), MetricsConfig: kmetrics.ReadCLIConfig(ctx), PprofConfig: kpprof.ReadCLIConfig(ctx), - SignerConfig: ksigner.ReadCLIConfig(ctx), } } // NewBatcherConfig creates a batcher config with given the CLIConfig func NewBatcherConfig(cfg CLIConfig, l log.Logger, m metrics.Metricer) (*Config, error) { - signer, fromAddress, err := kcrypto.SignerFactoryFromConfig(l, cfg.PrivateKey, cfg.Mnemonic, cfg.HDPath, cfg.SignerConfig) - if err != nil { - return nil, err - } + ctx := context.Background() // Connect to L1 and L2 providers. Perform these last since they are the most expensive. - ctx := context.Background() l1Client, err := utils.DialEthClientWithTimeout(ctx, cfg.L1EthRpc) if err != nil { return nil, err @@ -214,25 +165,21 @@ func NewBatcherConfig(cfg CLIConfig, l log.Logger, m metrics.Metricer) (*Config, return nil, fmt.Errorf("querying rollup config: %w", err) } - txMgrCfg := txmgr.Config{ - ResubmissionTimeout: cfg.ResubmissionTimeout, - ReceiptQueryInterval: time.Second, - NumConfirmations: cfg.NumConfirmations, - SafeAbortNonceTooLowCount: cfg.SafeAbortNonceTooLowCount, - From: fromAddress, - Signer: signer(rcfg.L1ChainID), + txManager, err := txmgr.NewSimpleTxManager("batcher", l, m, cfg.TxMgrConfig) + if err != nil { + return nil, err } return &Config{ - log: l, - metr: m, - L1Client: l1Client, - L2Client: l2Client, - RollupClient: rollupClient, - PollInterval: cfg.PollInterval, - TxManagerConfig: txMgrCfg, - From: fromAddress, - Rollup: rcfg, + log: l, + metr: m, + L1Client: l1Client, + L2Client: l2Client, + RollupClient: rollupClient, + PollInterval: cfg.PollInterval, + NetworkTimeout: cfg.TxMgrConfig.NetworkTimeout, + TxManager: txManager, + Rollup: rcfg, Channel: ChannelConfig{ ProposerWindowSize: rcfg.ProposerWindowSize, ChannelTimeout: rcfg.ChannelTimeout, diff --git a/components/batcher/flags/flags.go b/components/batcher/flags/flags.go index 2cef69c1cc..6785f88737 100644 --- a/components/batcher/flags/flags.go +++ b/components/batcher/flags/flags.go @@ -9,13 +9,13 @@ import ( kmetrics "github.com/kroma-network/kroma/utils/service/metrics" kpprof "github.com/kroma-network/kroma/utils/service/pprof" krpc "github.com/kroma-network/kroma/utils/service/rpc" - ksigner "github.com/kroma-network/kroma/utils/signer/client" + "github.com/kroma-network/kroma/utils/service/txmgr" ) const envVarPrefix = "BATCHER" var ( - /* Required flags */ + // Required flags L1EthRpcFlag = cli.StringFlag{ Name: "l1-eth-rpc", @@ -50,30 +50,8 @@ var ( Required: true, EnvVar: kservice.PrefixEnvVar(envVarPrefix, "POLL_INTERVAL"), } - NumConfirmationsFlag = cli.Uint64Flag{ - Name: "num-confirmations", - Usage: "Number of confirmations which we will wait after " + - "appending a new batch", - Required: true, - EnvVar: kservice.PrefixEnvVar(envVarPrefix, "NUM_CONFIRMATIONS"), - } - SafeAbortNonceTooLowCountFlag = cli.Uint64Flag{ - Name: "safe-abort-nonce-too-low-count", - Usage: "Number of ErrNonceTooLow observations required to " + - "give up on a tx at a particular nonce without receiving " + - "confirmation", - Required: true, - EnvVar: kservice.PrefixEnvVar(envVarPrefix, "SAFE_ABORT_NONCE_TOO_LOW_COUNT"), - } - ResubmissionTimeoutFlag = cli.DurationFlag{ - Name: "resubmission-timeout", - Usage: "Duration we will wait before resubmitting a " + - "transaction to L1", - Required: true, - EnvVar: kservice.PrefixEnvVar(envVarPrefix, "RESUBMISSION_TIMEOUT"), - } - /* Optional flags */ + // Optional flags MaxChannelDurationFlag = cli.Uint64Flag{ Name: "max-channel-duration", @@ -105,22 +83,8 @@ var ( Value: 1.0, EnvVar: kservice.PrefixEnvVar(envVarPrefix, "APPROX_COMPR_RATIO"), } - MnemonicFlag = cli.StringFlag{ - Name: "mnemonic", - Usage: "The mnemonic used to derive the wallets for the batcher", - EnvVar: kservice.PrefixEnvVar(envVarPrefix, "MNEMONIC"), - } - HDPathFlag = cli.StringFlag{ - Name: "hd-path", - Usage: "The HD path used to derive the batcher from the " + - "mnemonic. The mnemonic flag must also be set.", - EnvVar: kservice.PrefixEnvVar(envVarPrefix, "HD_PATH"), - } - PrivateKeyFlag = cli.StringFlag{ - Name: "private-key", - Usage: "The private key to use with the batcher. Must not be used with mnemonic.", - EnvVar: kservice.PrefixEnvVar(envVarPrefix, "PRIVATE_KEY"), - } + // Legacy Flags + HDPathFlag = txmgr.BatcherHDPathFlag ) var requiredFlags = []cli.Flag{ @@ -129,9 +93,6 @@ var requiredFlags = []cli.Flag{ RollupRpcFlag, SubSafetyMarginFlag, PollIntervalFlag, - NumConfirmationsFlag, - SafeAbortNonceTooLowCountFlag, - ResubmissionTimeoutFlag, } var optionalFlags = []cli.Flag{ @@ -140,9 +101,6 @@ var optionalFlags = []cli.Flag{ TargetL1TxSizeBytesFlag, TargetNumFramesFlag, ApproxComprRatioFlag, - MnemonicFlag, - HDPathFlag, - PrivateKeyFlag, } func init() { @@ -151,8 +109,8 @@ func init() { optionalFlags = append(optionalFlags, klog.CLIFlags(envVarPrefix)...) optionalFlags = append(optionalFlags, kmetrics.CLIFlags(envVarPrefix)...) optionalFlags = append(optionalFlags, kpprof.CLIFlags(envVarPrefix)...) - optionalFlags = append(optionalFlags, ksigner.CLIFlags(envVarPrefix)...) optionalFlags = append(optionalFlags, rpc.CLIFlags(envVarPrefix)...) + optionalFlags = append(optionalFlags, txmgr.CLIFlags(envVarPrefix)...) Flags = append(requiredFlags, optionalFlags...) } diff --git a/components/batcher/metrics/metrics.go b/components/batcher/metrics/metrics.go index 9708f67db3..eda56c00e9 100644 --- a/components/batcher/metrics/metrics.go +++ b/components/batcher/metrics/metrics.go @@ -1,16 +1,12 @@ package metrics import ( - "context" - - "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/ethclient" - "github.com/ethereum/go-ethereum/log" "github.com/prometheus/client_golang/prometheus" "github.com/kroma-network/kroma/components/node/eth" "github.com/kroma-network/kroma/components/node/rollup/derive" kmetrics "github.com/kroma-network/kroma/utils/service/metrics" + txmetrics "github.com/kroma-network/kroma/utils/service/txmgr/metrics" ) const Namespace = "kroma_batcher" @@ -22,6 +18,9 @@ type Metricer interface { // Records all L1 and L2 block events kmetrics.RefMetricer + // Record Tx metrics + txmetrics.TxMetricer + RecordLatestL1Block(l1ref eth.L1BlockRef) RecordL2BlocksLoaded(l2ref eth.L2BlockRef) RecordChannelOpened(id derive.ChannelID, numPendingBlocks int) @@ -43,6 +42,7 @@ type Metrics struct { factory kmetrics.Factory kmetrics.RefMetrics + txmetrics.TxMetrics Info prometheus.GaugeVec Up prometheus.Gauge @@ -80,6 +80,7 @@ func NewMetrics(procName string) *Metrics { factory: factory, RefMetrics: kmetrics.MakeRefMetrics(ns, factory), + TxMetrics: txmetrics.MakeTxMetrics(ns, factory), Info: *factory.NewGaugeVec(prometheus.GaugeOpts{ Namespace: ns, @@ -143,19 +144,10 @@ func NewMetrics(procName string) *Metrics { } } -func (m *Metrics) Serve(ctx context.Context, host string, port int) error { - return kmetrics.ListenAndServe(ctx, m.registry, host, port) -} - func (m *Metrics) Document() []kmetrics.DocumentedMetric { return m.factory.Document() } -func (m *Metrics) StartBalanceMetrics(ctx context.Context, - l log.Logger, client *ethclient.Client, account common.Address) { - kmetrics.LaunchBalanceMetrics(ctx, l, m.registry, m.ns, client, account) -} - // RecordInfo sets a pseudo-metric that contains versioning and // config info for the kroma-batcher. func (m *Metrics) RecordInfo(version string) { @@ -185,7 +177,7 @@ func (m *Metrics) RecordLatestL1Block(l1ref eth.L1BlockRef) { m.RecordL1Ref("latest", l1ref) } -// RecordL2BlockLoaded should be called when a new L2 block was loaded into the +// RecordL2BlocksLoaded should be called when a new L2 block was loaded into the // channel manager (but not processed yet). func (m *Metrics) RecordL2BlocksLoaded(l2ref eth.L2BlockRef) { m.RecordL2Ref(StageLoaded, l2ref) diff --git a/components/batcher/metrics/noop.go b/components/batcher/metrics/noop.go index 451455393c..38ddf30b84 100644 --- a/components/batcher/metrics/noop.go +++ b/components/batcher/metrics/noop.go @@ -4,9 +4,13 @@ import ( "github.com/kroma-network/kroma/components/node/eth" "github.com/kroma-network/kroma/components/node/rollup/derive" kmetrics "github.com/kroma-network/kroma/utils/service/metrics" + txmetrics "github.com/kroma-network/kroma/utils/service/txmgr/metrics" ) -type noopMetrics struct{ kmetrics.NoopRefMetrics } +type noopMetrics struct { + kmetrics.NoopRefMetrics + txmetrics.NoopTxMetrics +} var NoopMetrics Metricer = new(noopMetrics) diff --git a/components/batcher/rpc/api.go b/components/batcher/rpc/api.go index 29db8669bc..a1c4d5b2e8 100644 --- a/components/batcher/rpc/api.go +++ b/components/batcher/rpc/api.go @@ -6,7 +6,7 @@ import ( type batcherClient interface { Start() error - Stop() error + Stop(ctx context.Context) error } type adminAPI struct { @@ -23,6 +23,6 @@ func (a *adminAPI) StartBatcher(_ context.Context) error { return a.b.Start() } -func (a *adminAPI) StopBatcher(_ context.Context) error { - return a.b.Stop() +func (a *adminAPI) StopBatcher(ctx context.Context) error { + return a.b.Stop(ctx) } diff --git a/components/node/rollup/derive/channel_out.go b/components/node/rollup/derive/channel_out.go index 67732df1d0..f23eb4fbde 100644 --- a/components/node/rollup/derive/channel_out.go +++ b/components/node/rollup/derive/channel_out.go @@ -15,9 +15,11 @@ import ( "github.com/kroma-network/kroma/components/node/rollup" ) -var ErrMaxFrameSizeTooSmall = errors.New("maxSize is too small to fit the fixed frame overhead") -var ErrNotDepositTx = errors.New("first transaction in block is not a deposit tx") -var ErrTooManyRLPBytes = errors.New("batch would cause RLP bytes to go over limit") +var ( + ErrMaxFrameSizeTooSmall = errors.New("maxSize is too small to fit the fixed frame overhead") + ErrNotDepositTx = errors.New("first transaction in block is not a deposit tx") + ErrTooManyRLPBytes = errors.New("batch would cause RLP bytes to go over limit") +) // FrameV0OverHeadSize is the absolute minimum size of a frame. // This is the fixed overhead frame size, calculated as specified diff --git a/utils/monitoring/monitoring.go b/utils/monitoring/monitoring.go index 92cf2e14b9..ccdc76fdc6 100644 --- a/utils/monitoring/monitoring.go +++ b/utils/monitoring/monitoring.go @@ -32,7 +32,7 @@ func MaybeStartMetrics(ctx context.Context, cfg metrics.CLIConfig, l log.Logger, l.Info("starting metrics server", "addr", cfg.ListenAddr, "port", cfg.ListenPort) go func() { if err := metrics.ListenAndServe(ctx, registry, cfg.ListenAddr, cfg.ListenPort); err != nil { - l.Error("failed to start metrics server", err) + l.Error("failed to start metrics server", "err", err) } }() diff --git a/utils/service/crypto/signature.go b/utils/service/crypto/signature.go index ec3499c387..87985c2784 100644 --- a/utils/service/crypto/signature.go +++ b/utils/service/crypto/signature.go @@ -43,7 +43,7 @@ type SignerFactory func(chainID *big.Int) SignerFn // SignerFactoryFromConfig considers three ways that signers are created & then creates single factory from those config options. // It can either take a remote signer (via ksigner.CLIConfig) or it can be provided either a mnemonic + derivation path or a private key. -// It prefers the remote signer, then the mnemonic or private key (only one of which can be provided). +// It prefers the remote signer, to the mnemonic or private key (only one of which can be provided). func SignerFactoryFromConfig(l log.Logger, privateKey, mnemonic, hdPath string, signerConfig ksigner.CLIConfig) (SignerFactory, common.Address, error) { var signer SignerFactory var fromAddress common.Address diff --git a/utils/service/rpc/server.go b/utils/service/rpc/server.go index 8a3441e63d..0429fd14ad 100644 --- a/utils/service/rpc/server.go +++ b/utils/service/rpc/server.go @@ -203,10 +203,10 @@ func (b *Server) Start() error { } } -func (b *Server) Stop() { +func (b *Server) Stop() error { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - _ = b.httpServer.Shutdown(ctx) + return b.httpServer.Shutdown(ctx) } type HealthzResponse struct { diff --git a/utils/service/txmgr/cli.go b/utils/service/txmgr/cli.go new file mode 100644 index 0000000000..37adc63029 --- /dev/null +++ b/utils/service/txmgr/cli.go @@ -0,0 +1,268 @@ +package txmgr + +import ( + "context" + "errors" + "fmt" + "math/big" + "time" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/ethclient" + "github.com/ethereum/go-ethereum/log" + "github.com/urfave/cli" + + kservice "github.com/kroma-network/kroma/utils/service" + kcrypto "github.com/kroma-network/kroma/utils/service/crypto" + "github.com/kroma-network/kroma/utils/signer/client" +) + +const ( + // Duplicated L1 RPC flag + L1RPCFlagName = "l1-eth-rpc" + // Key Management Flags (also have signer client flags) + MnemonicFlagName = "mnemonic" + HDPathFlagName = "hd-path" + PrivateKeyFlagName = "private-key" + // TxMgr Flags (new + legacy + some shared flags) + NumConfirmationsFlagName = "num-confirmations" + SafeAbortNonceTooLowCountFlagName = "safe-abort-nonce-too-low-count" + ResubmissionTimeoutFlagName = "resubmission-timeout" + NetworkTimeoutFlagName = "network-timeout" + TxSendTimeoutFlagName = "txmgr.send-timeout" + TxNotInMempoolTimeoutFlagName = "txmgr.not-in-mempool-timeout" + ReceiptQueryIntervalFlagName = "txmgr.receipt-query-interval" +) + +var ( + BatcherHDPathFlag = cli.StringFlag{ + Name: "batcher-hd-path", + Usage: "DEPRECATED: The HD path used to derive the batcher wallet from the " + + "mnemonic. The mnemonic flag must also be set.", + EnvVar: "BATCHER_HD_PATH", + } + ValidatorHDPathFlag = cli.StringFlag{ + Name: "validator-hd-path", + Usage: "DEPRECATED: The HD path used to derive the validator wallet from the " + + "mnemonic. The mnemonic flag must also be set.", + EnvVar: "VALIDATOR_HD_PATH", + } +) + +func CLIFlags(envPrefix string) []cli.Flag { + return append([]cli.Flag{ + cli.StringFlag{ + Name: MnemonicFlagName, + Usage: "The mnemonic used to derive the wallets for either the service", + EnvVar: kservice.PrefixEnvVar(envPrefix, "MNEMONIC"), + }, + cli.StringFlag{ + Name: HDPathFlagName, + Usage: "The HD path used to derive the proposer wallet from the mnemonic. The mnemonic flag must also be set.", + EnvVar: kservice.PrefixEnvVar(envPrefix, "HD_PATH"), + }, + BatcherHDPathFlag, + ValidatorHDPathFlag, + cli.StringFlag{ + Name: "private-key", + Usage: "The private key to use with the service. Must not be used with mnemonic.", + EnvVar: kservice.PrefixEnvVar(envPrefix, "PRIVATE_KEY"), + }, + cli.Uint64Flag{ + Name: NumConfirmationsFlagName, + Usage: "Number of confirmations which we will wait after sending a transaction", + Value: 10, + EnvVar: kservice.PrefixEnvVar(envPrefix, "NUM_CONFIRMATIONS"), + }, + cli.Uint64Flag{ + Name: SafeAbortNonceTooLowCountFlagName, + Usage: "Number of ErrNonceTooLow observations required to give up on a tx at a particular nonce without receiving confirmation", + Value: 3, + EnvVar: kservice.PrefixEnvVar(envPrefix, "SAFE_ABORT_NONCE_TOO_LOW_COUNT"), + }, + cli.DurationFlag{ + Name: ResubmissionTimeoutFlagName, + Usage: "Duration we will wait before resubmitting a transaction to L1", + Value: 48 * time.Second, + EnvVar: kservice.PrefixEnvVar(envPrefix, "RESUBMISSION_TIMEOUT"), + }, + cli.DurationFlag{ + Name: NetworkTimeoutFlagName, + Usage: "Timeout for all network operations", + Value: 2 * time.Second, + EnvVar: kservice.PrefixEnvVar(envPrefix, "NETWORK_TIMEOUT"), + }, + cli.DurationFlag{ + Name: TxSendTimeoutFlagName, + Usage: "Timeout for sending transactions. If 0 it is disabled.", + Value: 0, + EnvVar: kservice.PrefixEnvVar(envPrefix, "TXMGR_TX_SEND_TIMEOUT"), + }, + cli.DurationFlag{ + Name: TxNotInMempoolTimeoutFlagName, + Usage: "Timeout for aborting a tx send if the tx does not make it to the mempool.", + Value: 2 * time.Minute, + EnvVar: kservice.PrefixEnvVar(envPrefix, "TXMGR_TX_NOT_IN_MEMPOOL_TIMEOUT"), + }, + cli.DurationFlag{ + Name: ReceiptQueryIntervalFlagName, + Usage: "Frequency to poll for receipts", + Value: 12 * time.Second, + EnvVar: kservice.PrefixEnvVar(envPrefix, "TXMGR_RECEIPT_QUERY_INTERVAL"), + }, + }, client.CLIFlags(envPrefix)...) +} + +type CLIConfig struct { + L1RPCURL string + Mnemonic string + HDPath string + BatcherHDPath string + ValidatorHDPath string + PrivateKey string + SignerCLIConfig client.CLIConfig + NumConfirmations uint64 + SafeAbortNonceTooLowCount uint64 + ResubmissionTimeout time.Duration + ReceiptQueryInterval time.Duration + NetworkTimeout time.Duration + TxSendTimeout time.Duration + TxNotInMempoolTimeout time.Duration +} + +func (m CLIConfig) Check() error { + if m.L1RPCURL == "" { + return errors.New("must provide a L1 RPC url") + } + if m.NumConfirmations == 0 { + return errors.New("NumConfirmations must not be 0") + } + if m.NetworkTimeout == 0 { + return errors.New("must provide NetworkTimeout") + } + if m.ResubmissionTimeout == 0 { + return errors.New("must provide ResubmissionTimeout") + } + if m.ReceiptQueryInterval == 0 { + return errors.New("must provide ReceiptQueryInterval") + } + if m.TxNotInMempoolTimeout == 0 { + return errors.New("must provide TxNotInMempoolTimeout") + } + if m.SafeAbortNonceTooLowCount == 0 { + return errors.New("SafeAbortNonceTooLowCount must not be 0") + } + if err := m.SignerCLIConfig.Check(); err != nil { + return err + } + return nil +} + +func ReadCLIConfig(ctx *cli.Context) CLIConfig { + return CLIConfig{ + L1RPCURL: ctx.GlobalString(L1RPCFlagName), + Mnemonic: ctx.GlobalString(MnemonicFlagName), + HDPath: ctx.GlobalString(HDPathFlagName), + BatcherHDPath: ctx.GlobalString(BatcherHDPathFlag.Name), + ValidatorHDPath: ctx.GlobalString(ValidatorHDPathFlag.Name), + PrivateKey: ctx.GlobalString(PrivateKeyFlagName), + SignerCLIConfig: client.ReadCLIConfig(ctx), + NumConfirmations: ctx.GlobalUint64(NumConfirmationsFlagName), + SafeAbortNonceTooLowCount: ctx.GlobalUint64(SafeAbortNonceTooLowCountFlagName), + ResubmissionTimeout: ctx.GlobalDuration(ResubmissionTimeoutFlagName), + ReceiptQueryInterval: ctx.GlobalDuration(ReceiptQueryIntervalFlagName), + NetworkTimeout: ctx.GlobalDuration(NetworkTimeoutFlagName), + TxSendTimeout: ctx.GlobalDuration(TxSendTimeoutFlagName), + TxNotInMempoolTimeout: ctx.GlobalDuration(TxNotInMempoolTimeoutFlagName), + } +} + +func NewConfig(cfg CLIConfig, l log.Logger) (Config, error) { + if err := cfg.Check(); err != nil { + return Config{}, fmt.Errorf("invalid config: %w", err) + } + + ctx, cancel := context.WithTimeout(context.Background(), cfg.NetworkTimeout) + defer cancel() + l1, err := ethclient.DialContext(ctx, cfg.L1RPCURL) + if err != nil { + return Config{}, fmt.Errorf("could not dial eth client: %w", err) + } + + ctx, cancel = context.WithTimeout(context.Background(), cfg.NetworkTimeout) + defer cancel() + chainID, err := l1.ChainID(ctx) + if err != nil { + return Config{}, fmt.Errorf("could not dial fetch L1 chain ID: %w", err) + } + + // Allow backwards compatible ways of specifying the HD path + hdPath := cfg.HDPath + if hdPath == "" && cfg.BatcherHDPath != "" { + hdPath = cfg.BatcherHDPath + } else if hdPath == "" && cfg.ValidatorHDPath != "" { + hdPath = cfg.ValidatorHDPath + } + + signerFactory, from, err := kcrypto.SignerFactoryFromConfig(l, cfg.PrivateKey, cfg.Mnemonic, hdPath, cfg.SignerCLIConfig) + if err != nil { + return Config{}, fmt.Errorf("could not init signer: %w", err) + } + + return Config{ + Backend: l1, + ResubmissionTimeout: cfg.ResubmissionTimeout, + ChainID: chainID, + TxSendTimeout: cfg.TxSendTimeout, + TxNotInMempoolTimeout: cfg.TxNotInMempoolTimeout, + NetworkTimeout: cfg.NetworkTimeout, + ReceiptQueryInterval: cfg.ReceiptQueryInterval, + NumConfirmations: cfg.NumConfirmations, + SafeAbortNonceTooLowCount: cfg.SafeAbortNonceTooLowCount, + Signer: signerFactory(chainID), + From: from, + }, nil +} + +// Config houses parameters for altering the behavior of a SimpleTxManager. +type Config struct { + Backend ETHBackend + // ResubmissionTimeout is the interval at which, if no previously + // published transaction has been mined, the new tx with a bumped gas + // price will be published. Only one publication at MaxGasPrice will be + // attempted. + ResubmissionTimeout time.Duration + + // ChainID is the chain ID of the L1 chain. + ChainID *big.Int + + // TxSendTimeout is how long to wait for sending a transaction. + // By default it is unbounded. If set, this is recommended to be at least 20 minutes. + TxSendTimeout time.Duration + + // TxNotInMempoolTimeout is how long to wait before aborting a transaction send if the transaction does not + // make it to the mempool. If the tx is in the mempool, TxSendTimeout is used instead. + TxNotInMempoolTimeout time.Duration + + // NetworkTimeout is the allowed duration for a single network request. + // This is intended to be used for network requests that can be replayed. + NetworkTimeout time.Duration + + // RequireQueryInterval is the interval at which the tx manager will + // query the backend to check for confirmations after a tx at a + // specific gas price has been published. + ReceiptQueryInterval time.Duration + + // NumConfirmations specifies how many blocks are need to consider a + // transaction confirmed. + NumConfirmations uint64 + + // SafeAbortNonceTooLowCount specifies how many ErrNonceTooLow observations + // are required to give up on a tx at a particular nonce without receiving + // confirmation. + SafeAbortNonceTooLowCount uint64 + + // Signer is used to sign transactions when the gas price is increased. + Signer kcrypto.SignerFn + From common.Address +} diff --git a/utils/service/txmgr/metrics/noop.go b/utils/service/txmgr/metrics/noop.go new file mode 100644 index 0000000000..ee77357579 --- /dev/null +++ b/utils/service/txmgr/metrics/noop.go @@ -0,0 +1,12 @@ +package metrics + +import "github.com/ethereum/go-ethereum/core/types" + +type NoopTxMetrics struct{} + +func (*NoopTxMetrics) RecordNonce(uint64) {} +func (*NoopTxMetrics) RecordGasBumpCount(int) {} +func (*NoopTxMetrics) RecordTxConfirmationLatency(int64) {} +func (*NoopTxMetrics) TxConfirmed(*types.Receipt) {} +func (*NoopTxMetrics) TxPublished(string) {} +func (*NoopTxMetrics) RPCError() {} diff --git a/utils/service/txmgr/metrics/tx_metrics.go b/utils/service/txmgr/metrics/tx_metrics.go new file mode 100644 index 0000000000..76ea3ab20c --- /dev/null +++ b/utils/service/txmgr/metrics/tx_metrics.go @@ -0,0 +1,115 @@ +package metrics + +import ( + "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/params" + "github.com/prometheus/client_golang/prometheus" + + "github.com/kroma-network/kroma/utils/service/metrics" +) + +type TxMetricer interface { + RecordGasBumpCount(int) + RecordTxConfirmationLatency(int64) + RecordNonce(uint64) + TxConfirmed(*types.Receipt) + TxPublished(string) + RPCError() +} + +type TxMetrics struct { + TxL1GasFee prometheus.Gauge + TxGasBump prometheus.Gauge + LatencyConfirmedTx prometheus.Gauge + currentNonce prometheus.Gauge + txPublishError *prometheus.CounterVec + publishEvent metrics.Event + confirmEvent metrics.EventVec + rpcError prometheus.Counter +} + +func receiptStatusString(receipt *types.Receipt) string { + switch receipt.Status { + case types.ReceiptStatusSuccessful: + return "success" + case types.ReceiptStatusFailed: + return "failed" + default: + return "unknown_status" + } +} + +var _ TxMetricer = (*TxMetrics)(nil) + +func MakeTxMetrics(ns string, factory metrics.Factory) TxMetrics { + return TxMetrics{ + TxL1GasFee: factory.NewGauge(prometheus.GaugeOpts{ + Namespace: ns, + Name: "tx_fee_gwei", + Help: "L1 gas fee for transactions in GWEI", + Subsystem: "txmgr", + }), + TxGasBump: factory.NewGauge(prometheus.GaugeOpts{ + Namespace: ns, + Name: "tx_gas_bump", + Help: "Number of times a transaction gas needed to be bumped before it got included", + Subsystem: "txmgr", + }), + LatencyConfirmedTx: factory.NewGauge(prometheus.GaugeOpts{ + Namespace: ns, + Name: "tx_confirmed_latency_ms", + Help: "Latency of a confirmed transaction in milliseconds", + Subsystem: "txmgr", + }), + currentNonce: factory.NewGauge(prometheus.GaugeOpts{ + Namespace: ns, + Name: "current_nonce", + Help: "Current nonce of the from address", + Subsystem: "txmgr", + }), + txPublishError: factory.NewCounterVec(prometheus.CounterOpts{ + Namespace: ns, + Name: "tx_publish_error_count", + Help: "Count of publish errors. Labels are sanitized error strings", + Subsystem: "txmgr", + }, []string{"error"}), + confirmEvent: metrics.NewEventVec(factory, ns, "confirm", "tx confirm", []string{"status"}), + publishEvent: metrics.NewEvent(factory, ns, "publish", "tx publish"), + rpcError: factory.NewCounter(prometheus.CounterOpts{ + Namespace: ns, + Name: "rpc_error_count", + Help: "Temporary: Count of RPC errors (like timeouts) that have occurred", + Subsystem: "txmgr", + }), + } +} + +func (t *TxMetrics) RecordNonce(nonce uint64) { + t.currentNonce.Set(float64(nonce)) +} + +// TxConfirmed records lots of information about the confirmed transaction +func (t *TxMetrics) TxConfirmed(receipt *types.Receipt) { + t.confirmEvent.Record(receiptStatusString(receipt)) + t.TxL1GasFee.Set(float64(receipt.EffectiveGasPrice.Uint64() * receipt.GasUsed / params.GWei)) +} + +func (t *TxMetrics) RecordGasBumpCount(times int) { + t.TxGasBump.Set(float64(times)) +} + +func (t *TxMetrics) RecordTxConfirmationLatency(latency int64) { + t.LatencyConfirmedTx.Set(float64(latency)) +} + +func (t *TxMetrics) TxPublished(errString string) { + if errString != "" { + t.txPublishError.WithLabelValues(errString).Inc() + } else { + t.publishEvent.Record() + } +} + +func (t *TxMetrics) RPCError() { + t.rpcError.Inc() +} diff --git a/utils/service/txmgr/mocks/TxManager.go b/utils/service/txmgr/mocks/TxManager.go new file mode 100644 index 0000000000..034ddaa51f --- /dev/null +++ b/utils/service/txmgr/mocks/TxManager.go @@ -0,0 +1,77 @@ +// Code generated by mockery v2.26.0. DO NOT EDIT. + +package mocks + +import ( + context "context" + + common "github.com/ethereum/go-ethereum/common" + + mock "github.com/stretchr/testify/mock" + + txmgr "github.com/kroma-network/kroma/utils/service/txmgr" + + types "github.com/ethereum/go-ethereum/core/types" +) + +// TxManager is an autogenerated mock type for the TxManager type +type TxManager struct { + mock.Mock +} + +// From provides a mock function with given fields: +func (_m *TxManager) From() common.Address { + ret := _m.Called() + + var r0 common.Address + if rf, ok := ret.Get(0).(func() common.Address); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(common.Address) + } + } + + return r0 +} + +// 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.Receipt + var r1 error + 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.Receipt); ok { + r0 = rf(ctx, candidate) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*types.Receipt) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, txmgr.TxCandidate) error); ok { + r1 = rf(ctx, candidate) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +type mockConstructorTestingTNewTxManager interface { + mock.TestingT + Cleanup(func()) +} + +// NewTxManager creates a new instance of TxManager. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +func NewTxManager(t mockConstructorTestingTNewTxManager) *TxManager { + mock := &TxManager{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/utils/service/txmgr/price_bump_test.go b/utils/service/txmgr/price_bump_test.go new file mode 100644 index 0000000000..57b00cc653 --- /dev/null +++ b/utils/service/txmgr/price_bump_test.go @@ -0,0 +1,91 @@ +package txmgr + +import ( + "fmt" + "math/big" + "testing" + + "github.com/ethereum/go-ethereum/log" + "github.com/stretchr/testify/require" + + "github.com/kroma-network/kroma/components/node/testlog" +) + +type priceBumpTest struct { + prevGasTip int64 + prevBasefee int64 + newGasTip int64 + newBasefee int64 + expectedTip int64 + expectedFC int64 +} + +func (tc *priceBumpTest) run(t *testing.T) { + prevFC := calcGasFeeCap(big.NewInt(tc.prevBasefee), big.NewInt(tc.prevGasTip)) + lgr := testlog.Logger(t, log.LvlCrit) + + tip, fc := updateFees(big.NewInt(tc.prevGasTip), prevFC, big.NewInt(tc.newGasTip), big.NewInt(tc.newBasefee), lgr) + + require.Equal(t, tc.expectedTip, tip.Int64(), "tip must be as expected") + require.Equal(t, tc.expectedFC, fc.Int64(), "fee cap must be as expected") +} + +func TestUpdateFees(t *testing.T) { + tests := []priceBumpTest{ + { + prevGasTip: 100, prevBasefee: 1000, + newGasTip: 90, newBasefee: 900, + expectedTip: 100, expectedFC: 2100, + }, + { + prevGasTip: 100, prevBasefee: 1000, + newGasTip: 101, newBasefee: 1000, + expectedTip: 115, expectedFC: 2415, + }, + { + prevGasTip: 100, prevBasefee: 1000, + newGasTip: 100, newBasefee: 1001, + expectedTip: 115, expectedFC: 2415, + }, + { + prevGasTip: 100, prevBasefee: 1000, + newGasTip: 101, newBasefee: 900, + expectedTip: 115, expectedFC: 2415, + }, + { + prevGasTip: 100, prevBasefee: 1000, + newGasTip: 90, newBasefee: 1010, + expectedTip: 115, expectedFC: 2415, + }, + { + prevGasTip: 100, prevBasefee: 1000, + newGasTip: 101, newBasefee: 2000, + expectedTip: 115, expectedFC: 4115, + }, + { + prevGasTip: 100, prevBasefee: 1000, + newGasTip: 120, newBasefee: 900, + expectedTip: 120, expectedFC: 2415, + }, + { + prevGasTip: 100, prevBasefee: 1000, + newGasTip: 120, newBasefee: 1100, + expectedTip: 120, expectedFC: 2415, + }, + { + prevGasTip: 100, prevBasefee: 1000, + newGasTip: 120, newBasefee: 1140, + expectedTip: 120, expectedFC: 2415, + }, + { + prevGasTip: 100, prevBasefee: 1000, + newGasTip: 120, newBasefee: 1200, + expectedTip: 120, expectedFC: 2520, + }, + } + for i, test := range tests { + i := i + test := test + t.Run(fmt.Sprint(i), test.run) + } +} diff --git a/utils/service/txmgr/send_state.go b/utils/service/txmgr/send_state.go index 2f34cbb330..f6d9c20273 100644 --- a/utils/service/txmgr/send_state.go +++ b/utils/service/txmgr/send_state.go @@ -3,6 +3,7 @@ package txmgr import ( "strings" "sync" + "time" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core" @@ -12,48 +13,53 @@ import ( // this context, a txn may correspond to multiple different txn hashes due to // varying gas prices, though we treat them all as the same logical txn. This // struct is primarily used to determine whether or not the txmgr should abort a -// given txn and retry with a higher nonce. +// given txn. type SendState struct { - minedTxs map[common.Hash]struct{} - nonceTooLowCount uint64 - mu sync.RWMutex + minedTxs map[common.Hash]struct{} + mu sync.RWMutex + now func() time.Time - safeAbortNonceTooLowCount uint64 + // Config + nonceTooLowCount uint64 + txInMempoolDeadline time.Time // deadline to abort at if no transactions are in the mempool + + // Counts of the different types of errors + successFullPublishCount uint64 // nil error => tx made it to the mempool + safeAbortNonceTooLowCount uint64 // nonce too low error } -// NewSendState parameterizes a new SendState from the passed -// safeAbortNonceTooLowCount. -func NewSendState(safeAbortNonceTooLowCount uint64) *SendState { +// NewSendStateWithNow creates a new send state with the provided clock. +func NewSendStateWithNow(safeAbortNonceTooLowCount uint64, unableToSendTimeout time.Duration, now func() time.Time) *SendState { if safeAbortNonceTooLowCount == 0 { panic("txmgr: safeAbortNonceTooLowCount cannot be zero") } return &SendState{ minedTxs: make(map[common.Hash]struct{}), - nonceTooLowCount: 0, safeAbortNonceTooLowCount: safeAbortNonceTooLowCount, + txInMempoolDeadline: now().Add(unableToSendTimeout), + now: now, } } +// NewSendState creates a new send state +func NewSendState(safeAbortNonceTooLowCount uint64, unableToSendTimeout time.Duration) *SendState { + return NewSendStateWithNow(safeAbortNonceTooLowCount, unableToSendTimeout, time.Now) +} + // ProcessSendError should be invoked with the error returned for each // publication. It is safe to call this method with nil or arbitrary errors. -// Currently it only acts on errors containing the ErrNonceTooLow message. func (s *SendState) ProcessSendError(err error) { - // Nothing to do. - if err == nil { - return - } - - // Only concerned with ErrNonceTooLow. - if !strings.Contains(err.Error(), core.ErrNonceTooLow.Error()) { - return - } - s.mu.Lock() defer s.mu.Unlock() - // Record this nonce too low observation. - s.nonceTooLowCount++ + // Record the type of error + switch { + case err == nil: + s.successFullPublishCount++ + case strings.Contains(err.Error(), core.ErrNonceTooLow.Error()): + s.nonceTooLowCount++ + } } // TxMined records that the txn with txnHash has been mined and is await @@ -85,8 +91,9 @@ func (s *SendState) TxNotMined(txHash common.Hash) { } // ShouldAbortImmediately returns true if the txmgr should give up on trying a -// given txn with the target nonce. For now, this only happens if we see an -// extended period of getting ErrNonceTooLow without having a txn mined. +// given txn with the target nonce. +// This occurs when the set of errors recorded indicates that no further progress can be made +// on this transaction. func (s *SendState) ShouldAbortImmediately() bool { s.mu.RLock() defer s.mu.RUnlock() @@ -96,9 +103,14 @@ func (s *SendState) ShouldAbortImmediately() bool { return false } - // Only abort if we've observed enough ErrNonceTooLow to meet our safe abort - // threshold. - return s.nonceTooLowCount >= s.safeAbortNonceTooLowCount + // If we have exceeded the nonce too low count, abort + if s.nonceTooLowCount >= s.safeAbortNonceTooLowCount || + // If we have not published a transaction in the allotted time, abort + (s.successFullPublishCount == 0 && s.now().After(s.txInMempoolDeadline)) { + return true + } + + return false } // IsWaitingForConfirmation returns true if we have at least one confirmation on diff --git a/utils/service/txmgr/send_state_test.go b/utils/service/txmgr/send_state_test.go index 707a6bb199..d374917c08 100644 --- a/utils/service/txmgr/send_state_test.go +++ b/utils/service/txmgr/send_state_test.go @@ -3,6 +3,7 @@ package txmgr_test import ( "errors" "testing" + "time" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core" @@ -11,14 +12,16 @@ import ( "github.com/kroma-network/kroma/utils/service/txmgr" ) -const testSafeAbortNonceTooLowCount = 3 +var testHash = common.HexToHash("0x01") -var ( - testHash = common.HexToHash("0x01") -) +const testSafeAbortNonceTooLowCount = 3 func newSendState() *txmgr.SendState { - return txmgr.NewSendState(testSafeAbortNonceTooLowCount) + return newSendStateWithTimeout(time.Hour, time.Now) +} + +func newSendStateWithTimeout(t time.Duration, now func() time.Time) *txmgr.SendState { + return txmgr.NewSendStateWithNow(testSafeAbortNonceTooLowCount, t, now) } func processNSendErrors(sendState *txmgr.SendState, err error, n int) { @@ -160,3 +163,27 @@ func TestSendStateIsNotWaitingForConfirmationAfterTxUnmined(t *testing.T) { sendState.TxNotMined(testHash) require.False(t, sendState.IsWaitingForConfirmation()) } + +func stepClock(step time.Duration) func() time.Time { + i := 0 + return func() time.Time { + var start time.Time + i += 1 + return start.Add(time.Duration(i) * step) + } +} + +// TestSendStateTimeoutAbort ensure that this will abort if it passes the tx pool timeout +// when no successful transactions have been recorded +func TestSendStateTimeoutAbort(t *testing.T) { + sendState := newSendStateWithTimeout(10*time.Millisecond, stepClock(20*time.Millisecond)) + require.True(t, sendState.ShouldAbortImmediately(), "Should abort after timing out") +} + +// TestSendStateNoTimeoutAbortIfPublishedTx ensure that this will not abort if there is +// a successful transaction send. +func TestSendStateNoTimeoutAbortIfPublishedTx(t *testing.T) { + sendState := newSendStateWithTimeout(10*time.Millisecond, stepClock(20*time.Millisecond)) + sendState.ProcessSendError(nil) + require.False(t, sendState.ShouldAbortImmediately(), "Should not abort if published transaction successfully") +} diff --git a/utils/service/txmgr/txmgr.go b/utils/service/txmgr/txmgr.go index 91e937f81d..fabd35096d 100644 --- a/utils/service/txmgr/txmgr.go +++ b/utils/service/txmgr/txmgr.go @@ -3,16 +3,20 @@ package txmgr import ( "context" "errors" + "fmt" "math/big" + "strings" "sync" "time" + "github.com/ethereum/go-ethereum" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/core/txpool" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/log" - kcrypto "github.com/kroma-network/kroma/utils/service/crypto" + "github.com/kroma-network/kroma/utils/service/txmgr/metrics" ) // Geth defaults the priceBump to 10 @@ -20,55 +24,27 @@ import ( const priceBump int64 = 15 // new = old * (100 + priceBump) / 100 -var priceBumpPercent = big.NewInt(100 + priceBump) -var oneHundred = big.NewInt(100) - -// 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 { - // ResubmissionTimeout is the interval at which, if no previously - // published transaction has been mined, the new tx with a bumped gas - // price will be published. Only one publication at MaxGasPrice will be - // attempted. - ResubmissionTimeout time.Duration - - // RequireQueryInterval is the interval at which the tx manager will - // query the backend to check for confirmations after a tx at a - // specific gas price has been published. - ReceiptQueryInterval time.Duration - - // NumConfirmations specifies how many blocks are need to consider a - // transaction confirmed. - NumConfirmations uint64 - - // SafeAbortNonceTooLowCount specifies how many ErrNonceTooLow observations - // are required to give up on a tx at a particular nonce without receiving - // confirmation. - SafeAbortNonceTooLowCount uint64 - - // Signer is used to sign transactions when the gas price is increased. - Signer kcrypto.SignerFn - From common.Address -} +var ( + priceBumpPercent = big.NewInt(100 + priceBump) + oneHundred = big.NewInt(100) +) // TxManager is an interface that allows callers to reliably publish txs, // bumping the gas price if needed, and obtain the receipt of the resulting tx. +// +//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) + Send(ctx context.Context, candidate TxCandidate) (*types.Receipt, error) + + // From returns the sending address associated with the instance of the transaction manager. + // It is static for a single instance of a TxManager. + From() common.Address } // ETHBackend is the set of methods that the transaction manager uses to resubmit gas & determine @@ -89,120 +65,58 @@ type ETHBackend interface { // TODO(CLI-3318): Maybe need a generic interface to support different RPC providers HeaderByNumber(ctx context.Context, number *big.Int) (*types.Header, error) SuggestGasTipCap(ctx context.Context) (*big.Int, error) + // NonceAt returns the account nonce of the given account. + // The block number can be nil, in which case the nonce is taken from the latest known block. + NonceAt(ctx context.Context, account common.Address, blockNumber *big.Int) (uint64, error) + // PendingNonceAt returns the pending nonce. + PendingNonceAt(ctx context.Context, account common.Address) (uint64, error) + // EstimateGas returns an estimate of the amount of gas needed to execute the given + // transaction against the current pending block. + EstimateGas(ctx context.Context, msg ethereum.CallMsg) (uint64, error) } // SimpleTxManager is a implementation of TxManager that performs linear fee // bumping of a tx until it confirms. type SimpleTxManager struct { - Config // embed the config directly - name string + cfg Config + name string + chainID *big.Int backend ETHBackend l log.Logger -} - -// IncreaseGasPrice takes the previous transaction & potentially clones then signs it with a higher tip. -// If the tip + basefee suggested by the network are not greater than the previous values, the same transaction -// will be returned. If they are greater, this function will ensure that they are at least greater by 15% than -// the previous transaction's value to ensure that the price bump is large enough. -// -// We do not re-estimate the amount of gas used because for some stateful transactions (like output proposals) the -// act of including the transaction renders the repeat of the transaction invalid. -func (m *SimpleTxManager) IncreaseGasPrice(ctx context.Context, tx *types.Transaction) (*types.Transaction, error) { - ctx, cancel := context.WithTimeout(ctx, 5*time.Second) - defer cancel() - - var gasTipCap, gasFeeCap *big.Int - - if tip, err := m.backend.SuggestGasTipCap(ctx); err != nil { - return nil, err - } else if tip == nil { - return nil, errors.New("the suggested tip was nil") - } else { - gasTipCap = tip - } - - // Return the same transaction if we don't update any fields. - // We do this because ethereum signatures are not deterministic and therefore the transaction hash will change - // when we re-sign the tx. We don't want to do that because we want to see ErrAlreadyKnown instead of ErrReplacementUnderpriced - var reusedTip, reusedFeeCap bool - - // new = old * (100 + priceBump) / 100 - // Enforce a min priceBump on the tip. Do this before the feeCap is calculated - thresholdTip := new(big.Int).Mul(priceBumpPercent, tx.GasTipCap()) - thresholdTip = thresholdTip.Div(thresholdTip, oneHundred) - if tx.GasTipCapIntCmp(gasTipCap) >= 0 { - m.l.Debug("Reusing the previous tip", "previous", tx.GasTipCap(), "suggested", gasTipCap) - gasTipCap = tx.GasTipCap() - reusedTip = true - } else if thresholdTip.Cmp(gasTipCap) > 0 { - m.l.Debug("Overriding the tip to enforce a price bump", "previous", tx.GasTipCap(), "suggested", gasTipCap, "new", thresholdTip) - gasTipCap = thresholdTip - } - - if head, err := m.backend.HeaderByNumber(ctx, nil); err != nil { - return nil, err - } else if head.BaseFee == nil { - return nil, errors.New("txmgr does not support pre-london blocks that do not have a basefee") - } else { - // CalcGasFeeCap ensure that the fee cap is large enough for the tip. - gasFeeCap = CalcGasFeeCap(head.BaseFee, gasTipCap) - } - - // new = old * (100 + priceBump) / 100 - // Enforce a min priceBump on the feeCap - thresholdFeeCap := new(big.Int).Mul(priceBumpPercent, tx.GasFeeCap()) - thresholdFeeCap = thresholdFeeCap.Div(thresholdFeeCap, oneHundred) - if tx.GasFeeCapIntCmp(gasFeeCap) >= 0 { - if reusedTip { - m.l.Debug("Reusing the previous fee cap", "previous", tx.GasFeeCap(), "suggested", gasFeeCap) - gasFeeCap = tx.GasFeeCap() - reusedFeeCap = true - } else { - m.l.Debug("Overriding the fee cap to enforce a price bump because we increased the tip", "previous", tx.GasFeeCap(), "suggested", gasFeeCap, "new", thresholdFeeCap) - gasFeeCap = thresholdFeeCap - } - } else if thresholdFeeCap.Cmp(gasFeeCap) > 0 { - if reusedTip { - // TODO (CLI-3620): Increase the basefee then recompute the feecap - m.l.Warn("Overriding the fee cap to enforce a price bump without increasing the tip. Will likely result in ErrReplacementUnderpriced", - "previous", tx.GasFeeCap(), "suggested", gasFeeCap, "new", thresholdFeeCap) - } else { - m.l.Debug("Overriding the fee cap to enforce a price bump", "previous", tx.GasFeeCap(), "suggested", gasFeeCap, "new", thresholdFeeCap) - } - gasFeeCap = thresholdFeeCap - } - - if reusedTip && reusedFeeCap { - return tx, nil - } - - rawTx := &types.DynamicFeeTx{ - ChainID: tx.ChainId(), - Nonce: tx.Nonce(), - GasTipCap: gasTipCap, - GasFeeCap: gasFeeCap, - Gas: tx.Gas(), - To: tx.To(), - Value: tx.Value(), - Data: tx.Data(), - AccessList: tx.AccessList(), - } - return m.Signer(ctx, m.From, types.NewTx(rawTx)) + metr metrics.TxMetricer } // NewSimpleTxManager initializes a new SimpleTxManager with the passed Config. -func NewSimpleTxManager(name string, l log.Logger, cfg Config, backend ETHBackend) *SimpleTxManager { - if cfg.NumConfirmations == 0 { - panic("txmgr: NumConfirmations cannot be zero") +func NewSimpleTxManager(name string, l log.Logger, m metrics.TxMetricer, cfg CLIConfig) (*SimpleTxManager, error) { + conf, err := NewConfig(cfg, l) + if err != nil { + return nil, err } return &SimpleTxManager{ + chainID: conf.ChainID, name: name, - Config: cfg, - backend: backend, + cfg: conf, + backend: conf.Backend, l: l.New("service", name), - } + metr: m, + }, nil +} + +func (m *SimpleTxManager) From() common.Address { + return m.cfg.From +} + +// TxCandidate is a transaction candidate that can be submitted to ask the +// [TxManager] to construct a transaction with gas price bounds. +type TxCandidate struct { + // TxData is the transaction data to be used in the constructed tx. + TxData []byte + // To is the recipient of the constructed tx. Nil means contract creation. + To *common.Address + // GasLimit is the gas limit to be used in the constructed tx. + GasLimit uint64 } // Send is used to publish a transaction with incrementally higher gas prices @@ -210,194 +124,378 @@ func NewSimpleTxManager(name string, l log.Logger, cfg Config, backend ETHBacken // invocation of sendTx returns (called with differing gas prices). The method // may be canceled using the passed context. // -// The initially supplied transaction must be signed, have gas estimation done, and have a reasonable gas fee. -// When the transaction is resubmitted the tx manager will re-sign the transaction at a different gas pricing -// but retain the gas used, the nonce, and the data. +// The transaction manager handles all signing. If and only if the gas limit is 0, the +// transaction manager will do a gas estimation. // // 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) { + if m.cfg.TxSendTimeout != 0 { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, m.cfg.TxSendTimeout) + defer cancel() + } + tx, err := m.craftTx(ctx, candidate) + if err != nil { + return nil, fmt.Errorf("failed to create the tx: %w", err) + } + return m.send(ctx, tx) +} - // 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. - var wg sync.WaitGroup - defer wg.Wait() +// 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) { + gasTipCap, basefee, err := m.suggestGasPriceCaps(ctx) + if err != nil { + m.metr.RPCError() + return nil, fmt.Errorf("failed to get gas price info: %w", err) + } + gasFeeCap := calcGasFeeCap(basefee, gasTipCap) - // Initialize a subcontext for the goroutines spawned in this process. - // The defer to cancel is done here (in reverse order of Wait) so that - // the goroutines can exit before blocking on the wait group. - ctx, cancel := context.WithCancel(ctx) + // Fetch the sender's nonce from the latest known block (nil `blockNumber`) + childCtx, cancel := context.WithTimeout(ctx, m.cfg.NetworkTimeout) defer cancel() + nonce, err := m.backend.NonceAt(childCtx, m.cfg.From, nil) + if err != nil { + m.metr.RPCError() + return nil, fmt.Errorf("failed to get nonce: %w", err) + } + m.metr.RecordNonce(nonce) - sendState := NewSendState(m.SafeAbortNonceTooLowCount) - - // Create a closure that will block on submitting the tx in the - // background, returning the first successfully mined receipt back to - // the main event loop via receiptChan. - receiptChan := make(chan *types.Receipt, 1) - sendTxAsync := func(tx *types.Transaction) { - defer wg.Done() + rawTx := &types.DynamicFeeTx{ + ChainID: m.chainID, + Nonce: nonce, + To: candidate.To, + GasTipCap: gasTipCap, + GasFeeCap: gasFeeCap, + Data: candidate.TxData, + } - txHash := tx.Hash() - nonce := tx.Nonce() - gasTipCap := tx.GasTipCap() - gasFeeCap := tx.GasFeeCap() - log := m.l.New("txHash", txHash, "nonce", nonce, "gasTipCap", gasTipCap, "gasFeeCap", gasFeeCap) - log.Info("publishing transaction") + m.l.Info("creating tx", "to", rawTx.To, "from", m.cfg.From) - err := m.backend.SendTransaction(ctx, tx) - sendState.ProcessSendError(err) + // If the gas limit is set, we can use that as the gas + if candidate.GasLimit != 0 { + rawTx.Gas = candidate.GasLimit + } else { + gas, err := m.backend.EstimateGas(ctx, ethereum.CallMsg{ + From: m.cfg.From, + To: candidate.To, + GasFeeCap: gasFeeCap, + GasTipCap: gasTipCap, + Data: rawTx.Data, + }) if err != nil { - if errors.Is(err, context.Canceled) { - return - } - if errors.Is(err, txpool.ErrAlreadyKnown) { - log.Info("resubmitted already known transaction") - return - } - log.Error("unable to publish transaction", "err", err) - if sendState.ShouldAbortImmediately() { - log.Warn("Aborting transaction submission") - cancel() - } - // TODO(conner): add retry? - return + return nil, fmt.Errorf("failed to estimate gas: %w", err) } + rawTx.Gas = gas + } - log.Info("transaction published successfully") + ctx, cancel = context.WithTimeout(ctx, m.cfg.NetworkTimeout) + defer cancel() + return m.cfg.Signer(ctx, m.cfg.From, types.NewTx(rawTx)) +} - // Wait for the transaction to be mined, reporting the receipt - // back to the main event loop if found. - receipt, err := m.waitMined(ctx, tx, sendState) - if err != nil { - log.Debug("send tx failed", "err", err) - } - if receipt != nil { - // Use non-blocking select to ensure function can exit - // if more than one receipt is discovered. - select { - case receiptChan <- receipt: - log.Trace("send tx succeeded") - default: - } - } +// send submits the same transaction several times with increasing gas prices as necessary. +// It waits for the transaction to be confirmed on chain. +func (m *SimpleTxManager) send(ctx context.Context, tx *types.Transaction) (*types.Receipt, error) { + var wg sync.WaitGroup + defer wg.Wait() + ctx, cancel := context.WithCancel(ctx) + defer cancel() + + sendState := NewSendState(m.cfg.SafeAbortNonceTooLowCount, m.cfg.TxNotInMempoolTimeout) + receiptChan := make(chan *types.Receipt, 1) + sendTxAsync := func(tx *types.Transaction) { + defer wg.Done() + m.publishAndWaitForTx(ctx, tx, sendState, receiptChan) } - // Submit and wait for the receipt at our first gas price in the - // background, before entering the event loop and waiting out the - // resubmission timeout. + // Immediately publish a transaction before starting the resumbission loop wg.Add(1) go sendTxAsync(tx) - ticker := time.NewTicker(m.ResubmissionTimeout) + ticker := time.NewTicker(m.cfg.ResubmissionTimeout) defer ticker.Stop() + bumpCounter := 0 for { select { - - // Whenever a resubmission timeout has elapsed, bump the gas - // price and publish a new transaction. case <-ticker.C: - // Avoid republishing if we are waiting for confirmation on an - // existing tx. This is primarily an optimization to reduce the - // number of API calls we make, but also reduces the chances of - // getting a false positive reading for ShouldAbortImmediately. + // Don't resubmit a transaction if it has been mined, but we are waiting for the conf depth. if sendState.IsWaitingForConfirmation() { continue } - - // Increase the gas price & submit the new transaction - newTx, err := m.IncreaseGasPrice(ctx, tx) - if err != nil { - m.l.Error("Failed to increase the gas price for the tx", "err", err) - // Don't `continue` here so we resubmit the transaction with the same gas price. - } else { - // Save the tx so we know it's gas price. - tx = newTx + // If we see lots of unrecoverable errors (and no pending transactions) abort sending the transaction. + if sendState.ShouldAbortImmediately() { + m.l.Warn("Aborting transaction submission") + return nil, errors.New("aborted transaction sending") } + // Increase the gas price & submit the new transaction + tx = m.increaseGasPrice(ctx, tx) wg.Add(1) + bumpCounter += 1 go sendTxAsync(tx) - // The passed context has been canceled, i.e. in the event of a - // shutdown. case <-ctx.Done(): return nil, ctx.Err() - // The transaction has confirmed. case receipt := <-receiptChan: + m.metr.RecordGasBumpCount(bumpCounter) + m.metr.TxConfirmed(receipt) return receipt, nil } } } -// waitMined implements the core functionality of WaitMined, with the option to -// pass in a SendState to record whether or not the transaction is mined. -func (m *SimpleTxManager) waitMined(ctx context.Context, tx *types.Transaction, sendState *SendState) (*types.Receipt, error) { - queryTicker := time.NewTicker(m.ReceiptQueryInterval) - defer queryTicker.Stop() +// publishAndWaitForTx publishes the transaction to the transaction pool and then waits for it with [waitMined]. +// It should be called in a new go-routine. It will send the receipt to receiptChan in a non-blocking way if a receipt is found +// for the transaction. +func (m *SimpleTxManager) publishAndWaitForTx(ctx context.Context, tx *types.Transaction, sendState *SendState, receiptChan chan *types.Receipt) { + log := m.l.New("hash", tx.Hash(), "nonce", tx.Nonce(), "gasTipCap", tx.GasTipCap(), "gasFeeCap", tx.GasFeeCap()) + log.Info("publishing transaction") - txHash := tx.Hash() + cCtx, cancel := context.WithTimeout(ctx, m.cfg.NetworkTimeout) + defer cancel() + t := time.Now() + err := m.backend.SendTransaction(cCtx, tx) + sendState.ProcessSendError(err) - for { - receipt, err := m.backend.TransactionReceipt(ctx, txHash) + // Properly log & exit if there is an error + if err != nil { switch { - case receipt != nil: - if sendState != nil { - sendState.TxMined(txHash) - } - - txHeight := receipt.BlockNumber.Uint64() - tipHeight, err := m.backend.BlockNumber(ctx) - if err != nil { - m.l.Error("Unable to fetch block number", "err", err) - break - } - - m.l.Debug("Transaction mined, checking confirmations", "txHash", txHash, "txHeight", txHeight, - "tipHeight", tipHeight, "numConfirmations", m.NumConfirmations) - - // The transaction is considered confirmed when - // txHeight+numConfirmations-1 <= tipHeight. Note that the -1 is - // needed to account for the fact that confirmations have an - // inherent off-by-one, i.e. when using 1 confirmation the - // transaction should be confirmed when txHeight is equal to - // tipHeight. The equation is rewritten in this form to avoid - // underflows. - if txHeight+m.NumConfirmations <= tipHeight+1 { - m.l.Info("Transaction confirmed", "txHash", txHash) - return receipt, nil - } - - // Safe to subtract since we know the LHS above is greater. - confsRemaining := (txHeight + m.NumConfirmations) - (tipHeight + 1) - m.l.Debug("Transaction not yet confirmed", "txHash", txHash, "confsRemaining", confsRemaining) - - case err != nil: - m.l.Trace("Receipt retrieval failed", "hash", txHash, "err", err) - + case errStringMatch(err, core.ErrNonceTooLow): + log.Warn("nonce too low", "err", err) + m.metr.TxPublished("nonce_to_low") + case errStringMatch(err, context.Canceled): + m.metr.RPCError() + log.Warn("transaction send cancelled", "err", err) + m.metr.TxPublished("context_cancelled") + case errStringMatch(err, txpool.ErrAlreadyKnown): + log.Warn("resubmitted already known transaction", "err", err) + m.metr.TxPublished("tx_already_known") + case errStringMatch(err, txpool.ErrReplaceUnderpriced): + log.Warn("transaction replacement is underpriced", "err", err) + m.metr.TxPublished("tx_replacement_underpriced") + case errStringMatch(err, txpool.ErrUnderpriced): + log.Warn("transaction is underpriced", "err", err) + m.metr.TxPublished("tx_underpriced") default: - if sendState != nil { - sendState.TxNotMined(txHash) - } - m.l.Trace("Transaction not yet mined", "hash", txHash) + m.metr.RPCError() + log.Error("unable to publish transaction", "err", err) + m.metr.TxPublished("unknown_error") } + return + } + m.metr.TxPublished("") + + log.Info("Transaction successfully published") + // Poll for the transaction to be ready & then send the result to receiptChan + receipt, err := m.waitMined(ctx, tx, sendState) + if err != nil { + log.Warn("Transaction receipt not found", "err", err) + return + } + select { + case receiptChan <- receipt: + m.metr.RecordTxConfirmationLatency(time.Since(t).Milliseconds()) + default: + } +} +// waitMined waits for the transaction to be mined or for the context to be cancelled. +func (m *SimpleTxManager) waitMined(ctx context.Context, tx *types.Transaction, sendState *SendState) (*types.Receipt, error) { + txHash := tx.Hash() + queryTicker := time.NewTicker(m.cfg.ReceiptQueryInterval) + defer queryTicker.Stop() + for { select { case <-ctx.Done(): - m.l.Warn("context cancelled in waitMined") return nil, ctx.Err() case <-queryTicker.C: + if receipt := m.queryReceipt(ctx, txHash, sendState); receipt != nil { + return receipt, nil + } } } } -// CalcGasFeeCap deterministically computes the recommended gas fee cap given +// queryReceipt queries for the receipt and returns the receipt if it has passed the confirmation depth +func (m *SimpleTxManager) queryReceipt(ctx context.Context, txHash common.Hash, sendState *SendState) *types.Receipt { + ctx, cancel := context.WithTimeout(ctx, m.cfg.NetworkTimeout) + defer cancel() + receipt, err := m.backend.TransactionReceipt(ctx, txHash) + if errors.Is(err, ethereum.NotFound) { + sendState.TxNotMined(txHash) + m.l.Trace("Transaction not yet mined", "hash", txHash) + return nil + } else if err != nil { + m.metr.RPCError() + m.l.Info("Receipt retrieval failed", "hash", txHash, "err", err) + return nil + } else if receipt == nil { + m.metr.RPCError() + m.l.Warn("Receipt and error are both nil", "hash", txHash) + return nil + } + + // Receipt is confirmed to be valid from this point on + sendState.TxMined(txHash) + + txHeight := receipt.BlockNumber.Uint64() + tipHeight, err := m.backend.BlockNumber(ctx) + if err != nil { + m.l.Error("Unable to fetch block number", "err", err) + return nil + } + + m.l.Debug("Transaction mined, checking confirmations", "hash", txHash, "txHeight", txHeight, + "tipHeight", tipHeight, "numConfirmations", m.cfg.NumConfirmations) + + // The transaction is considered confirmed when + // txHeight+numConfirmations-1 <= tipHeight. Note that the -1 is + // needed to account for the fact that confirmations have an + // inherent off-by-one, i.e. when using 1 confirmation the + // transaction should be confirmed when txHeight is equal to + // tipHeight. The equation is rewritten in this form to avoid + // underflows. + if txHeight+m.cfg.NumConfirmations <= tipHeight+1 { + m.l.Info("Transaction confirmed", "hash", txHash) + return receipt + } + + // Safe to subtract since we know the LHS above is greater. + confsRemaining := (txHeight + m.cfg.NumConfirmations) - (tipHeight + 1) + m.l.Debug("Transaction not yet confirmed", "hash", txHash, "confsRemaining", confsRemaining) + return nil +} + +// increaseGasPrice takes the previous transaction & potentially clones then signs it with a higher tip. +// If the tip + basefee suggested by the network are not greater than the previous values, the same transaction +// will be returned. If they are greater, this function will ensure that they are at least greater by 15% than +// the previous transaction's value to ensure that the price bump is large enough. +// +// We do not re-estimate the amount of gas used because for some stateful transactions (like output proposals) the +// act of including the transaction renders the repeat of the transaction invalid. +// +// If it encounters an error with creating the new transaction, it will return the old transaction. +func (m *SimpleTxManager) increaseGasPrice(ctx context.Context, tx *types.Transaction) *types.Transaction { + tip, basefee, err := m.suggestGasPriceCaps(ctx) + if err != nil { + m.l.Warn("failed to get suggested gas tip and basefee", "err", err) + return tx + } + gasTipCap, gasFeeCap := updateFees(tx.GasTipCap(), tx.GasFeeCap(), tip, basefee, m.l) + + if tx.GasTipCapIntCmp(gasTipCap) == 0 && tx.GasFeeCapIntCmp(gasFeeCap) == 0 { + return tx + } + + rawTx := &types.DynamicFeeTx{ + ChainID: tx.ChainId(), + Nonce: tx.Nonce(), + GasTipCap: gasTipCap, + GasFeeCap: gasFeeCap, + Gas: tx.Gas(), + To: tx.To(), + Value: tx.Value(), + Data: tx.Data(), + AccessList: tx.AccessList(), + } + ctx, cancel := context.WithTimeout(ctx, m.cfg.NetworkTimeout) + defer cancel() + newTx, err := m.cfg.Signer(ctx, m.cfg.From, types.NewTx(rawTx)) + if err != nil { + m.l.Warn("failed to sign new transaction", "err", err) + return tx + } + return newTx +} + +// suggestGasPriceCaps suggests what the new tip & new basefee should be based on the current L1 conditions +func (m *SimpleTxManager) suggestGasPriceCaps(ctx context.Context) (*big.Int, *big.Int, error) { + cCtx, cancel := context.WithTimeout(ctx, m.cfg.NetworkTimeout) + defer cancel() + tip, err := m.backend.SuggestGasTipCap(cCtx) + if err != nil { + m.metr.RPCError() + return nil, nil, fmt.Errorf("failed to fetch the suggested gas tip cap: %w", err) + } else if tip == nil { + return nil, nil, errors.New("the suggested tip was nil") + } + cCtx, cancel = context.WithTimeout(ctx, m.cfg.NetworkTimeout) + defer cancel() + head, err := m.backend.HeaderByNumber(cCtx, nil) + if err != nil { + m.metr.RPCError() + return nil, nil, fmt.Errorf("failed to fetch the suggested basefee: %w", err) + } else if head.BaseFee == nil { + return nil, nil, errors.New("txmgr does not support pre-london blocks that do not have a basefee") + } + return tip, head.BaseFee, nil +} + +// calcThresholdValue returns x * priceBumpPercent / 100 +func calcThresholdValue(x *big.Int) *big.Int { + threshold := new(big.Int).Mul(priceBumpPercent, x) + threshold = threshold.Div(threshold, oneHundred) + return threshold +} + +// updateFees takes the old tip/basefee & the new tip/basefee and then suggests +// a gasTipCap and gasFeeCap that satisfies geth's required fee bumps +// Geth: FC and Tip must be bumped if any increase +func updateFees(oldTip, oldFeeCap, newTip, newBaseFee *big.Int, lgr log.Logger) (*big.Int, *big.Int) { + newFeeCap := calcGasFeeCap(newBaseFee, newTip) + lgr = lgr.New("old_tip", oldTip, "old_feecap", oldFeeCap, "new_tip", newTip, "new_feecap", newFeeCap) + // If the new prices are less than the old price, reuse the old prices + if oldTip.Cmp(newTip) >= 0 && oldFeeCap.Cmp(newFeeCap) >= 0 { + lgr.Debug("Reusing old tip and feecap") + return oldTip, oldFeeCap + } + // Determine if we need to increase the suggested values + thresholdTip := calcThresholdValue(oldTip) + thresholdFeeCap := calcThresholdValue(oldFeeCap) + if newTip.Cmp(thresholdTip) >= 0 && newFeeCap.Cmp(thresholdFeeCap) >= 0 { + lgr.Debug("Using new tip and feecap") + return newTip, newFeeCap + } else if newTip.Cmp(thresholdTip) >= 0 && newFeeCap.Cmp(thresholdFeeCap) < 0 { + // Tip has gone up, but basefee is flat or down. + // TODO(CLI-3714): Do we need to recalculate the FC here? + lgr.Debug("Using new tip and threshold feecap") + return newTip, thresholdFeeCap + } else if newTip.Cmp(thresholdTip) < 0 && newFeeCap.Cmp(thresholdFeeCap) >= 0 { + // Basefee has gone up, but the tip hasn't. Recalculate the feecap because if the tip went up a lot + // not enough of the feecap may be dedicated to paying the basefee. + lgr.Debug("Using threshold tip and recalculated feecap") + return thresholdTip, calcGasFeeCap(newBaseFee, thresholdTip) + + } else { + // TODO(CLI-3713): Should we skip the bump in this case? + lgr.Debug("Using threshold tip and threshold feecap") + return thresholdTip, thresholdFeeCap + } +} + +// calcGasFeeCap deterministically computes the recommended gas fee cap given // the base fee and gasTipCap. The resulting gasFeeCap is equal to: // // gasTipCap + 2*baseFee. -func CalcGasFeeCap(baseFee, gasTipCap *big.Int) *big.Int { +func calcGasFeeCap(baseFee, gasTipCap *big.Int) *big.Int { return new(big.Int).Add( gasTipCap, new(big.Int).Mul(baseFee, big.NewInt(2)), ) } + +// errStringMatch returns true if err.Error() is a substring in target.Error() or if both are nil. +// It can accept nil errors without issue. +func errStringMatch(err, target error) bool { + if err == nil && target == nil { + return true + } else if err == nil || target == nil { + return false + } + return strings.Contains(err.Error(), target.Error()) +} diff --git a/utils/service/txmgr/txmgr_test.go b/utils/service/txmgr/txmgr_test.go index 829df2fa42..a427120560 100644 --- a/utils/service/txmgr/txmgr_test.go +++ b/utils/service/txmgr/txmgr_test.go @@ -3,24 +3,29 @@ package txmgr import ( "context" "errors" + "fmt" "math/big" - "math/rand" "sync" "testing" "time" + "github.com/ethereum/go-ethereum" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/core/types" - "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/log" "github.com/stretchr/testify/require" "github.com/kroma-network/kroma/components/node/testlog" - "github.com/kroma-network/kroma/components/node/testutils" - kcrypto "github.com/kroma-network/kroma/utils/service/crypto" + "github.com/kroma-network/kroma/utils/service/txmgr/metrics" ) +type sendTransactionFunc func(ctx context.Context, tx *types.Transaction) error + +func testSendState() *SendState { + return NewSendState(100, time.Hour) +} + // testHarness houses the necessary resources to test the SimpleTxManager. type testHarness struct { cfg Config @@ -34,7 +39,15 @@ type testHarness struct { func newTestHarnessWithConfig(t *testing.T, cfg Config) *testHarness { g := newGasPricer(3) backend := newMockBackend(g) - mgr := NewSimpleTxManager("TEST", testlog.Logger(t, log.LvlCrit), cfg, backend) + cfg.Backend = backend + mgr := &SimpleTxManager{ + chainID: cfg.ChainID, + name: "TEST", + cfg: cfg, + backend: cfg.Backend, + l: testlog.Logger(t, log.LvlCrit), + metr: &metrics.NoopTxMetrics{}, + } return &testHarness{ cfg: cfg, @@ -50,12 +63,23 @@ func newTestHarness(t *testing.T) *testHarness { return newTestHarnessWithConfig(t, configWithNumConfs(1)) } +// createTxCandidate creates a mock [TxCandidate]. +func (h testHarness) createTxCandidate() TxCandidate { + inbox := common.HexToAddress("0x42000000000000000000000000000000000000ff") + return TxCandidate{ + To: &inbox, + TxData: []byte{0x00, 0x01, 0x02}, + GasLimit: uint64(1337), + } +} + func configWithNumConfs(numConfirmations uint64) Config { return Config{ ResubmissionTimeout: time.Second, ReceiptQueryInterval: 50 * time.Millisecond, NumConfirmations: numConfirmations, SafeAbortNonceTooLowCount: 3, + TxNotInMempoolTimeout: 1 * time.Hour, Signer: func(ctx context.Context, from common.Address, tx *types.Transaction) (*types.Transaction, error) { return tx, nil }, @@ -91,7 +115,7 @@ func (g *gasPricer) shouldMine(gasFeeCap *big.Int) bool { 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)) - epochGasFeeCap := CalcGasFeeCap(epochBaseFee, epochGasTipCap) + epochGasFeeCap := calcGasFeeCap(epochBaseFee, epochGasTipCap) return epochGasTipCap, epochGasFeeCap } @@ -123,7 +147,7 @@ type mockBackend struct { mu sync.RWMutex g *gasPricer - send SendTransactionFunc + send sendTransactionFunc // blockHeight tracks the current height of the chain. blockHeight uint64 @@ -140,8 +164,8 @@ func newMockBackend(g *gasPricer) *mockBackend { } } -// setTxSender sets the implementation for the SendTransactionFunction -func (b *mockBackend) setTxSender(s SendTransactionFunc) { +// setTxSender sets the implementation for the sendTransactionFunction +func (b *mockBackend) setTxSender(s sendTransactionFunc) { b.send = s } @@ -175,6 +199,10 @@ func (b *mockBackend) HeaderByNumber(ctx context.Context, number *big.Int) (*typ }, nil } +func (b *mockBackend) EstimateGas(ctx context.Context, msg ethereum.CallMsg) (uint64, error) { + return b.g.basefee().Uint64(), nil +} + func (b *mockBackend) SuggestGasTipCap(ctx context.Context) (*big.Int, error) { tip, _ := b.g.sample() return tip, nil @@ -185,7 +213,18 @@ func (b *mockBackend) SendTransaction(ctx context.Context, tx *types.Transaction panic("set sender function was not set") } return b.send(ctx, tx) +} + +func (b *mockBackend) NonceAt(ctx context.Context, account common.Address, blockNumber *big.Int) (uint64, error) { + return 0, nil +} + +func (b *mockBackend) PendingNonceAt(ctx context.Context, account common.Address) (uint64, error) { + return 0, nil +} +func (*mockBackend) ChainID(ctx context.Context) (*big.Int, error) { + return big.NewInt(1), nil } // TransactionReceipt queries the mockBackend for a mined txHash. If none is @@ -193,7 +232,6 @@ func (b *mockBackend) SendTransaction(ctx context.Context, tx *types.Transaction // receipt containing the txHash and the gasFeeCap used in the GasUsed to make // the value accessible from our test framework. func (b *mockBackend) TransactionReceipt(ctx context.Context, txHash common.Hash) (*types.Receipt, error) { - b.mu.RLock() defer b.mu.RUnlock() @@ -237,8 +275,8 @@ func TestTxMgrConfirmAtMinGasPrice(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - receipt, err := h.mgr.Send(ctx, tx) - require.Nil(t, err) + receipt, err := h.mgr.send(ctx, tx) + require.NoError(t, err) require.NotNil(t, receipt) require.Equal(t, gasPricer.expGasFeeCap().Uint64(), receipt.GasUsed) } @@ -265,7 +303,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) } @@ -294,8 +332,8 @@ func TestTxMgrConfirmsAtHigherGasPrice(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - receipt, err := h.mgr.Send(ctx, tx) - require.Nil(t, err) + receipt, err := h.mgr.send(ctx, tx) + require.NoError(t, err) require.NotNil(t, receipt) require.Equal(t, h.gasPricer.expGasFeeCap().Uint64(), receipt.GasUsed) } @@ -325,11 +363,56 @@ 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) } +// TestTxMgr_CraftTx ensures that the tx manager will create transactions as expected. +func TestTxMgr_CraftTx(t *testing.T) { + t.Parallel() + h := newTestHarness(t) + candidate := h.createTxCandidate() + + // Craft the transaction. + gasTipCap, gasFeeCap := h.gasPricer.feesForEpoch(h.gasPricer.epoch + 1) + tx, err := h.mgr.craftTx(context.Background(), candidate) + require.NoError(t, err) + require.NotNil(t, tx) + + // Validate the gas tip cap and fee cap. + require.Equal(t, gasTipCap, tx.GasTipCap()) + require.Equal(t, gasFeeCap, tx.GasFeeCap()) + + // Validate the nonce was set correctly using the backend. + require.Zero(t, tx.Nonce()) + + // Check that the gas was set using the gas limit. + require.Equal(t, candidate.GasLimit, tx.Gas()) +} + +// TestTxMgr_EstimateGas ensures that the tx manager will estimate +// the gas when candidate gas limit is zero in [CraftTx]. +func TestTxMgr_EstimateGas(t *testing.T) { + t.Parallel() + h := newTestHarness(t) + candidate := h.createTxCandidate() + + // Set the gas limit to zero to trigger gas estimation. + candidate.GasLimit = 0 + + // Gas estimate + gasEstimate := h.gasPricer.baseBaseFee.Uint64() + + // Craft the transaction. + tx, err := h.mgr.craftTx(context.Background(), candidate) + require.NoError(t, err) + require.NotNil(t, tx) + + // Check that the gas was estimated correctly. + require.Equal(t, gasEstimate, tx.Gas()) +} + // TestTxMgrOnlyOnePublicationSucceeds asserts that the tx manager will return a // receipt so long as at least one of the publications is able to succeed with a // simulated rpc failure. @@ -358,8 +441,8 @@ func TestTxMgrOnlyOnePublicationSucceeds(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - receipt, err := h.mgr.Send(ctx, tx) - require.Nil(t, err) + receipt, err := h.mgr.send(ctx, tx) + require.NoError(t, err) require.NotNil(t, receipt) require.Equal(t, h.gasPricer.expGasFeeCap().Uint64(), receipt.GasUsed) @@ -393,8 +476,8 @@ func TestTxMgrConfirmsMinGasPriceAfterBumping(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - receipt, err := h.mgr.Send(ctx, tx) - require.Nil(t, err) + receipt, err := h.mgr.send(ctx, tx) + require.NoError(t, err) require.NotNil(t, receipt) require.Equal(t, h.gasPricer.expGasFeeCap().Uint64(), receipt.GasUsed) } @@ -438,8 +521,8 @@ func TestTxMgrDoesntAbortNonceTooLowAfterMiningTx(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - receipt, err := h.mgr.Send(ctx, tx) - require.Nil(t, err) + receipt, err := h.mgr.send(ctx, tx) + require.NoError(t, err) require.NotNil(t, receipt) require.Equal(t, h.gasPricer.expGasFeeCap().Uint64(), receipt.GasUsed) } @@ -458,8 +541,8 @@ func TestWaitMinedReturnsReceiptOnFirstSuccess(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - receipt, err := h.mgr.waitMined(ctx, tx, nil) - require.Nil(t, err) + receipt, err := h.mgr.waitMined(ctx, tx, testSendState()) + require.NoError(t, err) require.NotNil(t, receipt) require.Equal(t, receipt.TxHash, txHash) } @@ -477,7 +560,7 @@ func TestWaitMinedCanBeCanceled(t *testing.T) { // Create an unimined tx. tx := types.NewTx(&types.LegacyTx{}) - receipt, err := h.mgr.waitMined(ctx, tx, nil) + receipt, err := h.mgr.waitMined(ctx, tx, NewSendState(10, time.Hour)) require.Equal(t, err, context.DeadlineExceeded) require.Nil(t, receipt) } @@ -498,7 +581,7 @@ func TestWaitMinedMultipleConfs(t *testing.T) { txHash := tx.Hash() h.backend.mine(&txHash, new(big.Int)) - receipt, err := h.mgr.waitMined(ctx, tx, nil) + receipt, err := h.mgr.waitMined(ctx, tx, NewSendState(10, time.Hour)) require.Equal(t, err, context.DeadlineExceeded) require.Nil(t, receipt) @@ -507,24 +590,19 @@ func TestWaitMinedMultipleConfs(t *testing.T) { // Mine an empty block, tx should now be confirmed. h.backend.mine(nil, nil) - receipt, err = h.mgr.waitMined(ctx, tx, nil) - require.Nil(t, err) + receipt, err = h.mgr.waitMined(ctx, tx, NewSendState(10, time.Hour)) + require.NoError(t, err) require.NotNil(t, receipt) require.Equal(t, txHash, receipt.TxHash) } -// TestManagerPanicOnZeroConfs ensures that the NewSimpleTxManager will panic +// TestManagerErrsOnZeroConfs ensures that the NewSimpleTxManager will error // when attempting to configure with NumConfirmations set to zero. -func TestManagerPanicOnZeroConfs(t *testing.T) { +func TestManagerErrsOnZeroConfs(t *testing.T) { t.Parallel() - defer func() { - if r := recover(); r == nil { - t.Fatal("NewSimpleTxManager should panic when using zero conf") - } - }() - - _ = newTestHarnessWithConfig(t, configWithNumConfs(0)) + _, err := NewSimpleTxManager("TEST", testlog.Logger(t, log.LvlCrit), &metrics.NoopTxMetrics{}, CLIConfig{}) + require.Error(t, err) } // failingBackend implements ReceiptSource, returning a failure on the @@ -550,8 +628,8 @@ func (b *failingBackend) BlockNumber(ctx context.Context) (uint64, error) { // TransactionReceipt for the failingBackend returns errRpcFailure on the first // invocation, and a receipt containing the passed TxHash on the second. func (b *failingBackend) TransactionReceipt( - ctx context.Context, txHash common.Hash) (*types.Receipt, error) { - + ctx context.Context, txHash common.Hash, +) (*types.Receipt, error) { if !b.returnSuccessReceipt { b.returnSuccessReceipt = true return nil, errRpcFailure @@ -577,6 +655,22 @@ func (b *failingBackend) SuggestGasTipCap(_ context.Context) (*big.Int, error) { return b.gasTip, nil } +func (b *failingBackend) EstimateGas(ctx context.Context, msg ethereum.CallMsg) (uint64, error) { + return b.baseFee.Uint64(), nil +} + +func (b *failingBackend) NonceAt(_ context.Context, _ common.Address, _ *big.Int) (uint64, error) { + return 0, errors.New("unimplemented") +} + +func (b *failingBackend) PendingNonceAt(_ context.Context, _ common.Address) (uint64, error) { + return 0, errors.New("unimplemented") +} + +func (b *failingBackend) ChainID(ctx context.Context) (*big.Int, error) { + return nil, errors.New("unimplemented") +} + // TestWaitMinedReturnsReceiptAfterFailure asserts that WaitMined is able to // recover from failed calls to the backend. It uses the failedBackend to // simulate an rpc call failure, followed by the successful return of a receipt. @@ -586,7 +680,7 @@ func TestWaitMinedReturnsReceiptAfterFailure(t *testing.T) { var borkedBackend failingBackend mgr := &SimpleTxManager{ - Config: Config{ + cfg: Config{ ResubmissionTimeout: time.Second, ReceiptQueryInterval: 50 * time.Millisecond, NumConfirmations: 1, @@ -595,6 +689,7 @@ func TestWaitMinedReturnsReceiptAfterFailure(t *testing.T) { name: "TEST", backend: &borkedBackend, l: testlog.Logger(t, log.LvlCrit), + metr: &metrics.NoopTxMetrics{}, } // Don't mine the tx with the default backend. The failingBackend will @@ -604,25 +699,20 @@ func TestWaitMinedReturnsReceiptAfterFailure(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - receipt, err := mgr.waitMined(ctx, tx, nil) - require.Nil(t, err) + receipt, err := mgr.waitMined(ctx, tx, testSendState()) + require.NoError(t, err) require.NotNil(t, receipt) require.Equal(t, receipt.TxHash, txHash) } -// TestIncreaseGasPriceEnforcesMinBump asserts that if the suggest gas tip -// returned from L1 is less than the required price bump the price bump is -// used instead. -func TestIncreaseGasPriceEnforcesMinBump(t *testing.T) { - t.Parallel() - +func doGasPriceIncrease(t *testing.T, txTipCap, txFeeCap, newTip, newBaseFee int64) (*types.Transaction, *types.Transaction) { borkedBackend := failingBackend{ - gasTip: big.NewInt(101), - baseFee: big.NewInt(460), + gasTip: big.NewInt(newTip), + baseFee: big.NewInt(newBaseFee), } mgr := &SimpleTxManager{ - Config: Config{ + cfg: Config{ ResubmissionTimeout: time.Second, ReceiptQueryInterval: 50 * time.Millisecond, NumConfirmations: 1, @@ -634,97 +724,84 @@ func TestIncreaseGasPriceEnforcesMinBump(t *testing.T) { }, name: "TEST", backend: &borkedBackend, - l: testlog.Logger(t, log.LvlTrace), + l: testlog.Logger(t, log.LvlCrit), + metr: &metrics.NoopTxMetrics{}, } tx := types.NewTx(&types.DynamicFeeTx{ - GasTipCap: big.NewInt(100), - GasFeeCap: big.NewInt(1000), + GasTipCap: big.NewInt(txTipCap), + GasFeeCap: big.NewInt(txFeeCap), }) - - ctx := context.Background() - newTx, err := mgr.IncreaseGasPrice(ctx, tx) - require.NoError(t, err) - require.True(t, newTx.GasFeeCap().Cmp(tx.GasFeeCap()) > 0, "new tx fee cap must be larger") - require.True(t, newTx.GasTipCap().Cmp(tx.GasTipCap()) > 0, "new tx tip must be larger") -} - -// TestIncreaseGasPriceEnforcesMinBumpForBothOnTipIncrease asserts that if the gasTip goes up, -// but the baseFee doesn't, both values are increased by 10% -func TestIncreaseGasPriceEnforcesMinBumpForBothOnTipIncrease(t *testing.T) { - t.Parallel() - - borkedBackend := failingBackend{ - gasTip: big.NewInt(101), - baseFee: big.NewInt(440), - } - - mgr := &SimpleTxManager{ - Config: Config{ - ResubmissionTimeout: time.Second, - ReceiptQueryInterval: 50 * time.Millisecond, - NumConfirmations: 1, - SafeAbortNonceTooLowCount: 3, - Signer: func(ctx context.Context, from common.Address, tx *types.Transaction) (*types.Transaction, error) { - return tx, nil + newTx := mgr.increaseGasPrice(context.Background(), tx) + return tx, newTx +} + +func TestIncreaseGasPrice(t *testing.T) { + // t.Parallel() + tests := []struct { + name string + run func(t *testing.T) + }{ + { + name: "enforces min bump", + run: func(t *testing.T) { + tx, newTx := doGasPriceIncrease(t, 100, 1000, 101, 460) + require.True(t, newTx.GasFeeCap().Cmp(tx.GasFeeCap()) > 0, "new tx fee cap must be larger") + require.True(t, newTx.GasTipCap().Cmp(tx.GasTipCap()) > 0, "new tx tip must be larger") }, - From: common.Address{}, }, - name: "TEST", - backend: &borkedBackend, - l: testlog.Logger(t, log.LvlCrit), - } - - tx := types.NewTx(&types.DynamicFeeTx{ - GasTipCap: big.NewInt(100), - GasFeeCap: big.NewInt(1000), - }) - - ctx := context.Background() - newTx, err := mgr.IncreaseGasPrice(ctx, tx) - require.NoError(t, err) - require.True(t, newTx.GasFeeCap().Cmp(tx.GasFeeCap()) > 0, "new tx fee cap must be larger") - require.True(t, newTx.GasTipCap().Cmp(tx.GasTipCap()) > 0, "new tx tip must be larger") -} - -// TestIncreaseGasPriceEnforcesMinBumpForBothOnBaseFeeIncrease asserts that if the baseFee goes up, -// but the tip doesn't, both values are increased by 10% -// TODO(CLI-3620): This test will fail until we implemented CLI-3620. -func TestIncreaseGasPriceEnforcesMinBumpForBothOnBaseFeeIncrease(t *testing.T) { - t.Skip("Failing until CLI-3620 is implemented") - t.Parallel() - - borkedBackend := failingBackend{ - gasTip: big.NewInt(99), - baseFee: big.NewInt(460), - } - - mgr := &SimpleTxManager{ - Config: Config{ - ResubmissionTimeout: time.Second, - ReceiptQueryInterval: 50 * time.Millisecond, - NumConfirmations: 1, - SafeAbortNonceTooLowCount: 3, - Signer: func(ctx context.Context, from common.Address, tx *types.Transaction) (*types.Transaction, error) { - return tx, nil + { + name: "enforces min bump on only tip incrase", + run: func(t *testing.T) { + tx, newTx := doGasPriceIncrease(t, 100, 1000, 101, 440) + require.True(t, newTx.GasFeeCap().Cmp(tx.GasFeeCap()) > 0, "new tx fee cap must be larger") + require.True(t, newTx.GasTipCap().Cmp(tx.GasTipCap()) > 0, "new tx tip must be larger") + }, + }, + { + name: "enforces min bump on only basefee incrase", + run: func(t *testing.T) { + tx, newTx := doGasPriceIncrease(t, 100, 1000, 99, 460) + require.True(t, newTx.GasFeeCap().Cmp(tx.GasFeeCap()) > 0, "new tx fee cap must be larger") + require.True(t, newTx.GasTipCap().Cmp(tx.GasTipCap()) > 0, "new tx tip must be larger") + }, + }, + { + name: "uses L1 values when larger", + run: func(t *testing.T) { + _, newTx := doGasPriceIncrease(t, 10, 100, 50, 200) + require.True(t, newTx.GasFeeCap().Cmp(big.NewInt(450)) == 0, "new tx fee cap must be equal L1") + require.True(t, newTx.GasTipCap().Cmp(big.NewInt(50)) == 0, "new tx tip must be equal L1") + }, + }, + { + name: "uses L1 tip when larger and threshold FC", + run: func(t *testing.T) { + _, newTx := doGasPriceIncrease(t, 100, 2200, 120, 1050) + require.True(t, newTx.GasTipCap().Cmp(big.NewInt(120)) == 0, "new tx tip must be equal L1") + require.True(t, newTx.GasFeeCap().Cmp(big.NewInt(2530)) == 0, "new tx fee cap must be equal to the threshold value") + }, + }, + { + name: "uses L1 FC when larger and threshold tip", + run: func(t *testing.T) { + _, newTx := doGasPriceIncrease(t, 100, 2200, 100, 2000) + require.True(t, newTx.GasTipCap().Cmp(big.NewInt(115)) == 0, "new tx tip must be equal the threshold value") + require.True(t, newTx.GasFeeCap().Cmp(big.NewInt(4115)) == 0, "new tx fee cap must be equal L1") + }, + }, + { + name: "reuses tx when no bump", + run: func(t *testing.T) { + tx, newTx := doGasPriceIncrease(t, 10, 100, 10, 45) + require.Equal(t, tx.Hash(), newTx.Hash(), "tx hash must be the same") }, - From: common.Address{}, }, - name: "TEST", - backend: &borkedBackend, - l: testlog.Logger(t, log.LvlCrit), } - - tx := types.NewTx(&types.DynamicFeeTx{ - GasTipCap: big.NewInt(100), - GasFeeCap: big.NewInt(1000), - }) - - ctx := context.Background() - newTx, err := mgr.IncreaseGasPrice(ctx, tx) - require.NoError(t, err) - require.True(t, newTx.GasFeeCap().Cmp(tx.GasFeeCap()) > 0, "new tx fee cap must be larger") - require.True(t, newTx.GasTipCap().Cmp(tx.GasTipCap()) > 0, "new tx tip must be larger") + for _, test := range tests { + test := test + t.Run(test.name, test.run) + } } // TestIncreaseGasPriceNotExponential asserts that if the L1 basefee & tip remain the @@ -736,10 +813,10 @@ func TestIncreaseGasPriceNotExponential(t *testing.T) { gasTip: big.NewInt(10), baseFee: big.NewInt(45), } - feeCap := CalcGasFeeCap(borkedBackend.baseFee, borkedBackend.gasTip) + feeCap := calcGasFeeCap(borkedBackend.baseFee, borkedBackend.gasTip) mgr := &SimpleTxManager{ - Config: Config{ + cfg: Config{ ResubmissionTimeout: time.Second, ReceiptQueryInterval: 50 * time.Millisecond, NumConfirmations: 1, @@ -752,6 +829,7 @@ func TestIncreaseGasPriceNotExponential(t *testing.T) { name: "TEST", backend: &borkedBackend, l: testlog.Logger(t, log.LvlCrit), + metr: &metrics.NoopTxMetrics{}, } tx := types.NewTx(&types.DynamicFeeTx{ GasTipCap: big.NewInt(10), @@ -761,90 +839,31 @@ func TestIncreaseGasPriceNotExponential(t *testing.T) { // Run IncreaseGasPrice a bunch of times in a row to simulate a very fast resubmit loop. for i := 0; i < 20; i++ { ctx := context.Background() - newTx, err := mgr.IncreaseGasPrice(ctx, tx) - require.NoError(t, err) + newTx := mgr.increaseGasPrice(ctx, tx) require.True(t, newTx.GasFeeCap().Cmp(feeCap) == 0, "new tx fee cap must be equal L1") require.True(t, newTx.GasTipCap().Cmp(borkedBackend.gasTip) == 0, "new tx tip must be equal L1") tx = newTx } - -} - -// TestIncreaseGasPriceUseLargeIncrease asserts that if the suggest gas tip -// returned from L1 is much larger than the required price bump the L1 value -// is used instead of the price bump -func TestIncreaseGasPriceUseLargeIncrease(t *testing.T) { - t.Parallel() - - borkedBackend := failingBackend{ - gasTip: big.NewInt(50), - baseFee: big.NewInt(200), - } - feeCap := CalcGasFeeCap(borkedBackend.baseFee, borkedBackend.gasTip) - - mgr := &SimpleTxManager{ - Config: Config{ - ResubmissionTimeout: time.Second, - ReceiptQueryInterval: 50 * time.Millisecond, - NumConfirmations: 1, - SafeAbortNonceTooLowCount: 3, - Signer: func(ctx context.Context, from common.Address, tx *types.Transaction) (*types.Transaction, error) { - return tx, nil - }, - From: common.Address{}, - }, - name: "TEST", - backend: &borkedBackend, - l: testlog.Logger(t, log.LvlCrit), - } - - tx := types.NewTx(&types.DynamicFeeTx{ - GasTipCap: big.NewInt(10), - GasFeeCap: big.NewInt(100), - }) - - ctx := context.Background() - newTx, err := mgr.IncreaseGasPrice(ctx, tx) - require.NoError(t, err) - require.True(t, newTx.GasFeeCap().Cmp(feeCap) == 0, "new tx fee cap must be equal L1") - require.True(t, newTx.GasTipCap().Cmp(borkedBackend.gasTip) == 0, "new tx tip must be equal L1") } -// TestIncreaseGasPriceReusesTransaction asserts that if the L1 basefee & tip remain the -// same, the transaction is returned with the same signature values. The means that the error -// when submitting the transaction to the network is ErrAlreadyKnown instead of ErrReplacementUnderpriced -func TestIncreaseGasPriceReusesTransaction(t *testing.T) { - t.Parallel() - - borkedBackend := failingBackend{ - gasTip: big.NewInt(10), - baseFee: big.NewInt(45), +func TestErrStringMatch(t *testing.T) { + tests := []struct { + err error + target error + match bool + }{ + {err: nil, target: nil, match: true}, + {err: errors.New("exists"), target: nil, match: false}, + {err: nil, target: errors.New("exists"), match: false}, + {err: errors.New("exact match"), target: errors.New("exact match"), match: true}, + {err: errors.New("partial: match"), target: errors.New("match"), match: true}, } - pk := testutils.InsecureRandomKey(rand.New(rand.NewSource(123))) - signer := kcrypto.PrivateKeySignerFn(pk, big.NewInt(10)) - mgr := &SimpleTxManager{ - Config: Config{ - ResubmissionTimeout: time.Second, - ReceiptQueryInterval: 50 * time.Millisecond, - NumConfirmations: 1, - SafeAbortNonceTooLowCount: 3, - Signer: func(ctx context.Context, from common.Address, tx *types.Transaction) (*types.Transaction, error) { - return signer(from, tx) - }, - From: crypto.PubkeyToAddress(pk.PublicKey), - }, - name: "TEST", - backend: &borkedBackend, - l: testlog.Logger(t, log.LvlCrit), + for i, test := range tests { + i := i + test := test + t.Run(fmt.Sprint(i), func(t *testing.T) { + require.Equal(t, test.match, errStringMatch(test.err, test.target)) + }) } - tx := types.NewTx(&types.DynamicFeeTx{ - GasTipCap: big.NewInt(10), - GasFeeCap: big.NewInt(100), - }) - - ctx := context.Background() - newTx, err := mgr.IncreaseGasPrice(ctx, tx) - require.NoError(t, err) - require.Equal(t, tx.Hash(), newTx.Hash()) } diff --git a/utils/utils.go b/utils/utils.go index f7bfd7db68..2c18007e65 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -3,7 +3,6 @@ package utils import ( "context" "fmt" - "math/big" "time" "github.com/ethereum/go-ethereum/accounts/abi/bind" @@ -15,7 +14,6 @@ import ( "github.com/kroma-network/kroma/components/node/client" "github.com/kroma-network/kroma/components/node/sources" "github.com/kroma-network/kroma/utils/service/crypto" - "github.com/kroma-network/kroma/utils/service/txmgr" ) const ( @@ -59,71 +57,6 @@ func ParseAddress(address string) (common.Address, error) { return common.Address{}, fmt.Errorf("invalid address: %v", address) } -func CalcGasTipAndFeeCap(ctx context.Context, client *ethclient.Client) (*big.Int, *big.Int, error) { - gasTipCap, err := client.SuggestGasTipCap(ctx) - if err != nil { - return nil, nil, err - } - - head, err := client.HeaderByNumber(ctx, nil) - if err != nil { - return nil, nil, err - } - - gasFeeCap := new(big.Int).Add(gasTipCap, new(big.Int).Mul(head.BaseFee, big.NewInt(2))) - - return gasTipCap, gasFeeCap, nil -} - -func UpdateGasPrice(client *ethclient.Client, tx *types.Transaction, addr common.Address, signerFn crypto.SignerFn) txmgr.UpdateGasPriceFunc { - return func(ctx context.Context) (*types.Transaction, error) { - var newTx *types.Transaction - - nonce, err := client.PendingNonceAt(ctx, addr) - if err != nil { - nonce = tx.Nonce() - } - - if tx.ChainId() != nil { - gasTipCap, gasFeeCap, err := CalcGasTipAndFeeCap(ctx, client) - if err != nil { - return nil, err - } - newTx = types.NewTx(&types.DynamicFeeTx{ - ChainID: tx.ChainId(), - Nonce: nonce, - GasTipCap: gasTipCap, - GasFeeCap: gasFeeCap, - Gas: tx.Gas(), - To: tx.To(), - Value: tx.Value(), - Data: tx.Data(), - AccessList: tx.AccessList(), - }) - } else if tx.GasPrice() != nil { - gasPrice, err := client.SuggestGasPrice(ctx) - if err != nil { - return nil, err - } - newTx = types.NewTx(&types.LegacyTx{ - Nonce: nonce, - GasPrice: gasPrice, - Gas: tx.Gas(), - To: tx.To(), - Value: tx.Value(), - Data: tx.Data(), - }) - } - - signedTx, err := signerFn(ctx, addr, newTx) - if err != nil { - return nil, err - } - - return signedTx, nil - } -} - func NewSimpleCallOpts(ctx context.Context) *bind.CallOpts { return &bind.CallOpts{Context: ctx} }