fix(gas-oracle): nonce too low when resubmission (#1712)

Author: colinlyguo

rollup/internal/controller/sender/sender.go

@@ -63,7 +63,7 @@ type FeeData struct {
 	gasLimit uint64
 }
 
-// Sender Transaction sender to send transaction to l1/l2 geth
+// Sender Transaction sender to send transaction to l1/l2
 type Sender struct {
 	config            *config.SenderConfig
 	gethClient        *gethclient.Client
@@ -105,13 +105,7 @@ func NewSender(ctx context.Context, config *config.SenderConfig, signerConfig *c
 		return nil, fmt.Errorf("failed to create transaction signer, err: %w", err)
 	}
 
-	// Set pending nonce
-	nonce, err := client.PendingNonceAt(ctx, transactionSigner.GetAddr())
-	if err != nil {
-		return nil, fmt.Errorf("failed to get pending nonce for address %s, err: %w", transactionSigner.GetAddr(), err)
-	}
-	transactionSigner.SetNonce(nonce)
-
+	// Create sender instance first and then initialize nonce
 	sender := &Sender{
 		ctx:                   ctx,
 		config:                config,
@@ -127,8 +121,13 @@ func NewSender(ctx context.Context, config *config.SenderConfig, signerConfig *c
 		service:               service,
 		senderType:            senderType,
 	}
-	sender.metrics = initSenderMetrics(reg)
 
+	// Initialize nonce using the new method
+	if err := sender.resetNonce(); err != nil {
+		return nil, fmt.Errorf("failed to reset nonce: %w", err)
+	}
+
+	sender.metrics = initSenderMetrics(reg)
 	go sender.loop(ctx)
 
 	return sender, nil
@@ -242,7 +241,10 @@ func (s *Sender) SendTransaction(contextID string, target *common.Address, data
 		// Check if contain nonce, and reset nonce
 		// only reset nonce when it is not from resubmit
 		if strings.Contains(err.Error(), "nonce too low") {
-			s.resetNonce(context.Background())
+			if err := s.resetNonce(); err != nil {
+				log.Warn("failed to reset nonce after failed send transaction", "address", s.transactionSigner.GetAddr().String(), "err", err)
+				return common.Hash{}, 0, fmt.Errorf("failed to reset nonce after failed send transaction, err: %w", err)
+			}
 		}
 		return common.Hash{}, 0, fmt.Errorf("failed to send transaction, err: %w", err)
 	}
@@ -327,14 +329,46 @@ func (s *Sender) createTx(feeData *FeeData, target *common.Address, data []byte,
 	return signedTx, nil
 }
 
+// initializeNonce initializes the nonce by taking the maximum of database nonce and pending nonce.
+func (s *Sender) initializeNonce() (uint64, error) {
+	// Get maximum nonce from database
+	dbNonce, err := s.pendingTransactionOrm.GetMaxNonceBySenderAddress(s.ctx, s.transactionSigner.GetAddr().Hex())
+	if err != nil {
+		return 0, fmt.Errorf("failed to get max nonce from database for address %s, err: %w", s.transactionSigner.GetAddr().Hex(), err)
+	}
+
+	// Get pending nonce from the client
+	pendingNonce, err := s.client.PendingNonceAt(s.ctx, s.transactionSigner.GetAddr())
+	if err != nil {
+		return 0, fmt.Errorf("failed to get pending nonce for address %s, err: %w", s.transactionSigner.GetAddr().Hex(), err)
+	}
+
+	// Take the maximum of pending nonce and (db nonce + 1)
+	// Database stores the used nonce, so the next available nonce should be dbNonce + 1
+	// When dbNonce is -1 (no records), dbNonce + 1 = 0, which is correct
+	nextDbNonce := uint64(dbNonce + 1)
+	var finalNonce uint64
+	if pendingNonce > nextDbNonce {
+		finalNonce = pendingNonce
+	} else {
+		finalNonce = nextDbNonce
+	}
+
+	log.Info("nonce initialization", "address", s.transactionSigner.GetAddr().Hex(), "maxDbNonce", dbNonce, "nextDbNonce", nextDbNonce, "pendingNonce", pendingNonce, "finalNonce", finalNonce)
+
+	return finalNonce, nil
+}
+
 // resetNonce reset nonce if send signed tx failed.
-func (s *Sender) resetNonce(ctx context.Context) {
-	nonce, err := s.client.PendingNonceAt(ctx, s.transactionSigner.GetAddr())
+func (s *Sender) resetNonce() error {
+	nonce, err := s.initializeNonce()
 	if err != nil {
-		log.Warn("failed to reset nonce", "address", s.transactionSigner.GetAddr().String(), "err", err)
-		return
+		log.Error("failed to reset nonce", "address", s.transactionSigner.GetAddr().String(), "err", err)
+		return fmt.Errorf("failed to reset nonce, err: %w", err)
 	}
+	log.Info("reset nonce", "address", s.transactionSigner.GetAddr().String(), "nonce", nonce)
 	s.transactionSigner.SetNonce(nonce)
+	return nil
 }
 
 func (s *Sender) createReplacingTransaction(tx *gethTypes.Transaction, baseFee, blobBaseFee uint64) (*gethTypes.Transaction, error) {
@@ -612,6 +646,16 @@ func (s *Sender) checkPendingTransaction() {
 			}
 
 			if err := s.client.SendTransaction(s.ctx, newSignedTx); err != nil {
+				if strings.Contains(err.Error(), "nonce too low") {
+					// When we receive a 'nonce too low' error but cannot find the transaction receipt, it indicates another transaction with this nonce has already been processed, so this transaction will never be mined and should be marked as failed.
+					log.Warn("nonce too low detected, marking all non-confirmed transactions with same nonce as failed", "nonce", originalTx.Nonce(), "address", s.transactionSigner.GetAddr().Hex(), "txHash", originalTx.Hash().Hex(), "newTxHash", newSignedTx.Hash().Hex(), "err", err)
+					txHashes := []string{originalTx.Hash().Hex(), newSignedTx.Hash().Hex()}
+					if updateErr := s.pendingTransactionOrm.UpdateTransactionStatusByTxHashes(s.ctx, txHashes, types.TxStatusConfirmedFailed); updateErr != nil {
+						log.Error("failed to update transaction status", "hashes", txHashes, "err", updateErr)
+						return
+					}
+					return
+				}
 				// SendTransaction failed, need to rollback the previous database changes
 				if rollbackErr := s.db.Transaction(func(tx *gorm.DB) error {
 					// Restore original transaction status back to pending

rollup/internal/orm/orm_test.go

@@ -597,3 +597,61 @@ func TestPendingTransactionOrm(t *testing.T) {
 	err = pendingTransactionOrm.DeleteTransactionByTxHash(context.Background(), common.HexToHash("0x123"))
 	assert.Error(t, err) // Should return error for non-existent transaction
 }
+
+func TestPendingTransaction_GetMaxNonceBySenderAddress(t *testing.T) {
+	sqlDB, err := db.DB()
+	assert.NoError(t, err)
+	assert.NoError(t, migrate.ResetDB(sqlDB))
+
+	// When there are no transactions for this sender address, should return -1
+	maxNonce, err := pendingTransactionOrm.GetMaxNonceBySenderAddress(context.Background(), "0xdeadbeef")
+	assert.NoError(t, err)
+	assert.Equal(t, int64(-1), maxNonce)
+
+	// Insert two transactions with different nonces for the same sender address
+	senderMeta := &SenderMeta{
+		Name:    "testName",
+		Service: "testService",
+		Address: common.HexToAddress("0xdeadbeef"),
+		Type:    types.SenderTypeCommitBatch,
+	}
+
+	tx0 := gethTypes.NewTx(&gethTypes.DynamicFeeTx{
+		Nonce:      1,
+		To:         &common.Address{},
+		Data:       []byte{},
+		Gas:        21000,
+		AccessList: gethTypes.AccessList{},
+		Value:      big.NewInt(0),
+		ChainID:    big.NewInt(1),
+		GasTipCap:  big.NewInt(0),
+		GasFeeCap:  big.NewInt(1),
+		V:          big.NewInt(0),
+		R:          big.NewInt(0),
+		S:          big.NewInt(0),
+	})
+	tx1 := gethTypes.NewTx(&gethTypes.DynamicFeeTx{
+		Nonce:      3,
+		To:         &common.Address{},
+		Data:       []byte{},
+		Gas:        22000,
+		AccessList: gethTypes.AccessList{},
+		Value:      big.NewInt(0),
+		ChainID:    big.NewInt(1),
+		GasTipCap:  big.NewInt(1),
+		GasFeeCap:  big.NewInt(2),
+		V:          big.NewInt(0),
+		R:          big.NewInt(0),
+		S:          big.NewInt(0),
+	})
+
+	err = pendingTransactionOrm.InsertPendingTransaction(context.Background(), "test", senderMeta, tx0, 0)
+	assert.NoError(t, err)
+	err = pendingTransactionOrm.InsertPendingTransaction(context.Background(), "test", senderMeta, tx1, 0)
+	assert.NoError(t, err)
+
+	// Now the max nonce for this sender should be 3
+	maxNonce, err = pendingTransactionOrm.GetMaxNonceBySenderAddress(context.Background(), senderMeta.Address.String())
+	assert.NoError(t, err)
+	assert.Equal(t, int64(3), maxNonce)
+}

rollup/internal/orm/pending_transaction.go

@@ -3,6 +3,7 @@ package orm
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"time"
 
@@ -191,6 +192,25 @@ func (o *PendingTransaction) UpdateTransactionStatusByTxHash(ctx context.Context
 	return nil
 }
 
+// UpdateTransactionStatusByTxHashes updates the status of multiple transactions by their hashes in one SQL statement
+func (o *PendingTransaction) UpdateTransactionStatusByTxHashes(ctx context.Context, txHashes []string, status types.TxStatus, dbTX ...*gorm.DB) error {
+	if len(txHashes) == 0 {
+		return nil
+	}
+	db := o.db
+	if len(dbTX) > 0 && dbTX[0] != nil {
+		db = dbTX[0]
+	}
+	db = db.WithContext(ctx)
+	db = db.Model(&PendingTransaction{})
+	db = db.Where("hash IN ?", txHashes)
+	if err := db.Update("status", status).Error; err != nil {
+		return fmt.Errorf("failed to update transaction status for hashes %v to status %d: %w", txHashes, status, err)
+	}
+
+	return nil
+}
+
 // UpdateOtherTransactionsAsFailedByNonce updates the status of all transactions to TxStatusConfirmedFailed for a specific nonce and sender address, excluding a specified transaction hash.
 func (o *PendingTransaction) UpdateOtherTransactionsAsFailedByNonce(ctx context.Context, senderAddress string, nonce uint64, hash common.Hash, dbTX ...*gorm.DB) error {
 	db := o.db
@@ -207,3 +227,27 @@ func (o *PendingTransaction) UpdateOtherTransactionsAsFailedByNonce(ctx context.
 	}
 	return nil
 }
+
+// GetMaxNonceBySenderAddress retrieves the maximum nonce for a specific sender address.
+// Returns -1 if no transactions are found for the given address.
+func (o *PendingTransaction) GetMaxNonceBySenderAddress(ctx context.Context, senderAddress string) (int64, error) {
+	var result struct {
+		Nonce int64 `gorm:"column:nonce"`
+	}
+
+	err := o.db.WithContext(ctx).
+		Model(&PendingTransaction{}).
+		Select("nonce").
+		Where("sender_address = ?", senderAddress).
+		Order("nonce DESC").
+		First(&result).Error
+
+	if err != nil {
+		if errors.Is(err, gorm.ErrRecordNotFound) {
+			return -1, nil
+		}
+		return -1, fmt.Errorf("failed to get max nonce by sender address, address: %s, err: %w", senderAddress, err)
+	}
+
+	return result.Nonce, nil
+}