Skip to content

Commit

Permalink
fix(sender): gas price bump corner case when resubmitting tx (#1058)
Browse files Browse the repository at this point in the history 
Co-authored-by: colinlyguo <colinlyguo@users.noreply.github.com>
  • Loading branch information
@colinlyguo
colinlyguo and colinlyguo authored Dec 28, 2023
1 parent b0609fa commit 4909dc8
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 28 deletions.
3 changes: 1 addition & 2 deletions common/cmd/cmd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{}) {
Expand Down
2 changes: 1 addition & 1 deletion common/version/version.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
60 changes: 41 additions & 19 deletions rollup/internal/controller/sender/sender.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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())
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down
69 changes: 63 additions & 6 deletions rollup/internal/controller/sender/sender_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
}
Expand Down Expand Up @@ -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"

Expand Down Expand Up @@ -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)

Expand Down

0 comments on commit 4909dc8

Please sign in to comment.