From 4909dc8cd2a393ff47de07cbe053fe6725488a0b Mon Sep 17 00:00:00 2001 From: colin <102356659+colinlyguo@users.noreply.github.com> Date: Thu, 28 Dec 2023 11:18:34 +0800 Subject: [PATCH] fix(sender): gas price bump corner case when resubmitting tx (#1058) Co-authored-by: colinlyguo --- common/cmd/cmd.go | 3 +- common/version/version.go | 2 +- rollup/internal/controller/sender/sender.go | 60 +++++++++++----- .../internal/controller/sender/sender_test.go | 69 +++++++++++++++++-- 4 files changed, 106 insertions(+), 28 deletions(-) diff --git a/common/cmd/cmd.go b/common/cmd/cmd.go index 15750fea7b..f98404fd59 100644 --- a/common/cmd/cmd.go +++ b/common/cmd/cmd.go @@ -95,8 +95,7 @@ func (c *Cmd) Write(data []byte) (int, error) { if verbose || c.openLog { fmt.Printf("%s:\n\t%v", c.name, out) } else if strings.Contains(strings.ToLower(out), "error") || - strings.Contains(strings.ToLower(out), "warning") || - strings.Contains(strings.ToLower(out), "info") { + strings.Contains(strings.ToLower(out), "warning") { fmt.Printf("%s:\n\t%v", c.name, out) } go c.checkFuncs.IterCb(func(_ string, value interface{}) { diff --git a/common/version/version.go b/common/version/version.go index c8b2aed094..08f65b8210 100644 --- a/common/version/version.go +++ b/common/version/version.go @@ -5,7 +5,7 @@ import ( "runtime/debug" ) -var tag = "v4.3.44" +var tag = "v4.3.45" var commit = func() string { if info, ok := debug.ReadBuildInfo(); ok { diff --git a/rollup/internal/controller/sender/sender.go b/rollup/internal/controller/sender/sender.go index 719f31d7f8..b496b43b5b 100644 --- a/rollup/internal/controller/sender/sender.go +++ b/rollup/internal/controller/sender/sender.go @@ -222,12 +222,13 @@ func (s *Sender) SendTransaction(ID string, target *common.Address, value *big.I if feeData, err = s.getFeeData(s.auth, target, value, data, fallbackGasLimit); err != nil { s.metrics.sendTransactionFailureGetFee.WithLabelValues(s.service, s.name).Inc() - log.Error("failed to get fee data", "err", err) + log.Error("failed to get fee data", "from", s.auth.From.String(), "nonce", s.auth.Nonce.Uint64(), "fallback gas limit", fallbackGasLimit, "err", err) return common.Hash{}, fmt.Errorf("failed to get fee data, err: %w", err) } if tx, err = s.createAndSendTx(s.auth, feeData, target, value, data, nil); err != nil { s.metrics.sendTransactionFailureSendTx.WithLabelValues(s.service, s.name).Inc() + log.Error("failed to create and send tx (non-resubmit case)", "from", s.auth.From.String(), "nonce", s.auth.Nonce.Uint64(), "err", err) return common.Hash{}, fmt.Errorf("failed to create and send transaction, err: %w", err) } @@ -303,11 +304,11 @@ func (s *Sender) createAndSendTx(auth *bind.TransactOpts, feeData *FeeData, targ // sign and send tx, err := auth.Signer(auth.From, types.NewTx(txData)) if err != nil { - log.Error("failed to sign tx", "err", err) + log.Error("failed to sign tx", "address", auth.From.String(), "err", err) return nil, err } if err = s.client.SendTransaction(s.ctx, tx); err != nil { - log.Error("failed to send tx", "tx hash", tx.Hash().String(), "err", err) + log.Error("failed to send tx", "tx hash", tx.Hash().String(), "from", auth.From.String(), "nonce", tx.Nonce(), "err", err) // Check if contain nonce, and reset nonce // only reset nonce when it is not from resubmit if strings.Contains(err.Error(), "nonce") && overrideNonce == nil { @@ -356,6 +357,7 @@ func (s *Sender) resubmitTransaction(feeData *FeeData, auth *bind.TransactOpts, "tx_hash": tx.Hash().String(), "tx_type": s.config.TxType, "from": auth.From.String(), + "nonce": tx.Nonce(), } switch s.config.TxType { @@ -369,10 +371,15 @@ func (s *Sender) resubmitTransaction(feeData *FeeData, auth *bind.TransactOpts, if gasPrice.Cmp(maxGasPrice) > 0 { gasPrice = maxGasPrice } - feeData.gasPrice = gasPrice - txInfo["original_gas_price"] = originalGasPrice - txInfo["adjusted_gas_price"] = gasPrice + if originalGasPrice.Cmp(gasPrice) == 0 { + log.Warn("gas price bump corner case, add 1 wei", "original", originalGasPrice.Uint64(), "adjusted", gasPrice.Uint64()) + gasPrice = new(big.Int).Add(gasPrice, big.NewInt(1)) + } + + feeData.gasPrice = gasPrice + txInfo["original_gas_price"] = originalGasPrice.Uint64() + txInfo["adjusted_gas_price"] = gasPrice.Uint64() default: originalGasTipCap := big.NewInt(feeData.gasTipCap.Int64()) originalGasFeeCap := big.NewInt(feeData.gasFeeCap.Int64()) @@ -406,20 +413,35 @@ func (s *Sender) resubmitTransaction(feeData *FeeData, auth *bind.TransactOpts, if gasFeeCap.Cmp(maxGasPrice) > 0 { gasFeeCap = maxGasPrice } + + if originalGasTipCap.Cmp(gasTipCap) == 0 { + log.Warn("gas tip cap bump corner case, add 1 wei", "original", originalGasTipCap.Uint64(), "adjusted", gasTipCap.Uint64()) + gasTipCap = new(big.Int).Add(gasTipCap, big.NewInt(1)) + } + + if originalGasFeeCap.Cmp(gasFeeCap) == 0 { + log.Warn("gas fee cap bump corner case, add 1 wei", "original", originalGasFeeCap.Uint64(), "adjusted", gasFeeCap.Uint64()) + gasFeeCap = new(big.Int).Add(gasFeeCap, big.NewInt(1)) + } + feeData.gasFeeCap = gasFeeCap feeData.gasTipCap = gasTipCap - - txInfo["original_gas_tip_cap"] = originalGasTipCap - txInfo["adjusted_gas_tip_cap"] = gasTipCap - txInfo["original_gas_fee_cap"] = originalGasFeeCap - txInfo["adjusted_gas_fee_cap"] = gasFeeCap + txInfo["original_gas_tip_cap"] = originalGasTipCap.Uint64() + txInfo["adjusted_gas_tip_cap"] = gasTipCap.Uint64() + txInfo["original_gas_fee_cap"] = originalGasFeeCap.Uint64() + txInfo["adjusted_gas_fee_cap"] = gasFeeCap.Uint64() } - log.Debug("Transaction gas adjustment details", txInfo) + log.Info("Transaction gas adjustment details", txInfo) nonce := tx.Nonce() s.metrics.resubmitTransactionTotal.WithLabelValues(s.service, s.name).Inc() - return s.createAndSendTx(auth, feeData, tx.To(), tx.Value(), tx.Data(), &nonce) + tx, err := s.createAndSendTx(auth, feeData, tx.To(), tx.Value(), tx.Data(), &nonce) + if err != nil { + log.Error("failed to create and send tx (resubmit case)", "from", s.auth.From.String(), "nonce", nonce, "err", err) + return nil, err + } + return tx, nil } // checkPendingTransaction checks the confirmation status of pending transactions against the latest confirmed block number. @@ -455,15 +477,15 @@ func (s *Sender) checkPendingTransaction(header *types.Header, confirmed uint64) } } } else if s.config.EscalateBlocks+pending.submitAt < number { - log.Debug("resubmit transaction", - "tx hash", pending.tx.Hash().String(), + log.Info("resubmit transaction", + "hash", pending.tx.Hash().String(), + "from", pending.signer.From.String(), + "nonce", pending.tx.Nonce(), "submit block number", pending.submitAt, "current block number", number, - "escalateBlocks", s.config.EscalateBlocks) + "configured escalateBlocks", s.config.EscalateBlocks) - var tx *types.Transaction - tx, err := s.resubmitTransaction(pending.feeData, pending.signer, pending.tx) - if err != nil { + if tx, err := s.resubmitTransaction(pending.feeData, pending.signer, pending.tx); err != nil { // If account pool is empty, it will try again in next loop. if !errors.Is(err, ErrNoAvailableAccount) { log.Error("failed to resubmit transaction, reset submitAt", "tx hash", pending.tx.Hash().String(), "err", err) diff --git a/rollup/internal/controller/sender/sender_test.go b/rollup/internal/controller/sender/sender_test.go index 2dea5d41d9..def344a3c1 100644 --- a/rollup/internal/controller/sender/sender_test.go +++ b/rollup/internal/controller/sender/sender_test.go @@ -43,7 +43,7 @@ func setupEnv(t *testing.T) { var err error cfg, err = config.NewConfig("../../../conf/config.json") assert.NoError(t, err) - base.RunImages(t) + base.RunL1Geth(t) priv, err := crypto.HexToECDSA("1212121212121212121212121212121212121212121212121212121212121212") assert.NoError(t, err) // Load default private key. @@ -60,7 +60,9 @@ func TestSender(t *testing.T) { t.Run("test new sender", testNewSender) t.Run("test pending limit", testPendLimit) t.Run("test fallback gas limit", testFallbackGasLimit) - t.Run("test resubmit transaction", testResubmitTransaction) + t.Run("test resubmit zero gas price transaction", testResubmitZeroGasPriceTransaction) + t.Run("test resubmit non-zero gas price transaction", testResubmitNonZeroGasPriceTransaction) + t.Run("test resubmit under priced transaction", testResubmitUnderpricedTransaction) t.Run("test resubmit transaction with rising base fee", testResubmitTransactionWithRisingBaseFee) t.Run("test check pending transaction", testCheckPendingTransaction) } @@ -138,21 +140,76 @@ func testFallbackGasLimit(t *testing.T) { } } -func testResubmitTransaction(t *testing.T) { +func testResubmitZeroGasPriceTransaction(t *testing.T) { for _, txType := range txTypes { cfgCopy := *cfg.L1Config.RelayerConfig.SenderConfig cfgCopy.TxType = txType s, err := NewSender(context.Background(), &cfgCopy, privateKey, "test", "test", nil) assert.NoError(t, err) - tx := types.NewTransaction(s.auth.Nonce.Uint64(), common.Address{}, big.NewInt(0), 0, big.NewInt(0), nil) - feeData, err := s.getFeeData(s.auth, &common.Address{}, big.NewInt(0), nil, 0) + feeData := &FeeData{ + gasPrice: big.NewInt(0), + gasTipCap: big.NewInt(0), + gasFeeCap: big.NewInt(0), + gasLimit: 50000, + } + tx, err := s.createAndSendTx(s.auth, feeData, &common.Address{}, big.NewInt(0), nil, nil) assert.NoError(t, err) + assert.NotNil(t, tx) + // Increase at least 1 wei in gas price, gas tip cap and gas fee cap. _, err = s.resubmitTransaction(feeData, s.auth, tx) assert.NoError(t, err) s.Stop() } } +func testResubmitNonZeroGasPriceTransaction(t *testing.T) { + for _, txType := range txTypes { + cfgCopy := *cfg.L1Config.RelayerConfig.SenderConfig + // Bump gas price, gas tip cap and gas fee cap just touch the minimum threshold of 10% (default config of geth). + cfgCopy.EscalateMultipleNum = 110 + cfgCopy.EscalateMultipleDen = 100 + cfgCopy.TxType = txType + s, err := NewSender(context.Background(), &cfgCopy, privateKey, "test", "test", nil) + assert.NoError(t, err) + feeData := &FeeData{ + gasPrice: big.NewInt(100000), + gasTipCap: big.NewInt(100000), + gasFeeCap: big.NewInt(100000), + gasLimit: 50000, + } + tx, err := s.createAndSendTx(s.auth, feeData, &common.Address{}, big.NewInt(0), nil, nil) + assert.NoError(t, err) + assert.NotNil(t, tx) + _, err = s.resubmitTransaction(feeData, s.auth, tx) + assert.NoError(t, err) + s.Stop() + } +} + +func testResubmitUnderpricedTransaction(t *testing.T) { + for _, txType := range txTypes { + cfgCopy := *cfg.L1Config.RelayerConfig.SenderConfig + // Bump gas price, gas tip cap and gas fee cap less than 10% (default config of geth). + cfgCopy.EscalateMultipleNum = 109 + cfgCopy.EscalateMultipleDen = 100 + cfgCopy.TxType = txType + s, err := NewSender(context.Background(), &cfgCopy, privateKey, "test", "test", nil) + assert.NoError(t, err) + feeData := &FeeData{ + gasPrice: big.NewInt(100000), + gasTipCap: big.NewInt(100000), + gasFeeCap: big.NewInt(100000), + gasLimit: 50000, + } + tx, err := s.createAndSendTx(s.auth, feeData, &common.Address{}, big.NewInt(0), nil, nil) + assert.NoError(t, err) + assert.NotNil(t, tx) + _, err = s.resubmitTransaction(feeData, s.auth, tx) + assert.Error(t, err, "replacement transaction underpriced") + s.Stop() + } +} + func testResubmitTransactionWithRisingBaseFee(t *testing.T) { txType := "DynamicFeeTx" @@ -247,7 +304,7 @@ func testCheckPendingTransaction(t *testing.T) { }, ) - pendingTx := &PendingTransaction{id: "abc", tx: tx, submitAt: header.Number.Uint64() - s.config.EscalateBlocks - 1} + pendingTx := &PendingTransaction{id: "abc", tx: tx, signer: s.auth, submitAt: header.Number.Uint64() - s.config.EscalateBlocks - 1} s.pendingTxs.Set(pendingTx.id, pendingTx) s.checkPendingTransaction(header, confirmed)