Oev 671 txm improvements

pkg/.mockery.yaml

@@ -53,7 +53,7 @@ packages:
     config:
       dir: txm
       outpkg: txm
-      mockname: "mock{{ .InterfaceName }}"
+      mockname: "Mock{{ .InterfaceName }}"
       filename: "mock_{{ .InterfaceName | snakecase }}_test.go"
     interfaces:
       Client:

pkg/chains/legacyevm/evm_txm.go

@@ -60,6 +60,7 @@ func newEvmTxm(
 				logPoller,
 				opts.KeyStore,
 				estimator,
+				cfg.GasEstimator(),
 			)
 			if cfg.Transactions().TransactionManagerV2().DualBroadcast() == nil || !*cfg.Transactions().TransactionManagerV2().DualBroadcast() {
 				return txmv2, err

pkg/txm/attempt_builder.go

@@ -2,6 +2,7 @@ package txm
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"math/big"
 
@@ -13,24 +14,34 @@ import (
 	"github.com/smartcontractkit/chainlink-evm/pkg/gas"
 	"github.com/smartcontractkit/chainlink-evm/pkg/keys"
 	"github.com/smartcontractkit/chainlink-evm/pkg/txm/types"
+	"github.com/smartcontractkit/chainlink-framework/chains/fees"
 )
 
+// maxBumpThreshold controls the maximum number of bumps for an attempt.
+const maxBumpThreshold = 5
+
 type attemptBuilder struct {
 	gas.EvmFeeEstimator
-	priceMaxKey func(common.Address) *assets.Wei
-	keystore    keys.TxSigner
+	priceMaxKey         func(common.Address) *assets.Wei
+	keystore            keys.TxSigner
+	emptyTxLimitDefault uint64
 }
 
-func NewAttemptBuilder(priceMaxKey func(common.Address) *assets.Wei, estimator gas.EvmFeeEstimator, keystore keys.TxSigner) *attemptBuilder {
+func NewAttemptBuilder(priceMaxKey func(common.Address) *assets.Wei, estimator gas.EvmFeeEstimator, keystore keys.TxSigner, emptyTxLimitDefault uint64) *attemptBuilder {
 	return &attemptBuilder{
-		priceMaxKey:     priceMaxKey,
-		EvmFeeEstimator: estimator,
-		keystore:        keystore,
+		priceMaxKey:         priceMaxKey,
+		EvmFeeEstimator:     estimator,
+		keystore:            keystore,
+		emptyTxLimitDefault: emptyTxLimitDefault,
 	}
 }
 
 func (a *attemptBuilder) NewAttempt(ctx context.Context, lggr logger.Logger, tx *types.Transaction, dynamic bool) (*types.Attempt, error) {
-	fee, estimatedGasLimit, err := a.EvmFeeEstimator.GetFee(ctx, tx.Data, tx.SpecifiedGasLimit, a.priceMaxKey(tx.FromAddress), &tx.FromAddress, &tx.ToAddress)
+	gasLimit := tx.SpecifiedGasLimit
+	if tx.IsPurgeable {
+		gasLimit = a.emptyTxLimitDefault
+	}
+	fee, estimatedGasLimit, err := a.EvmFeeEstimator.GetFee(ctx, tx.Data, gasLimit, a.priceMaxKey(tx.FromAddress), &tx.FromAddress, &tx.ToAddress)
 	if err != nil {
 		return nil, err
 	}
@@ -42,13 +53,52 @@ func (a *attemptBuilder) NewAttempt(ctx context.Context, lggr logger.Logger, tx
 }
 
 func (a *attemptBuilder) NewBumpAttempt(ctx context.Context, lggr logger.Logger, tx *types.Transaction, previousAttempt types.Attempt) (*types.Attempt, error) {
-	bumpedFee, bumpedFeeLimit, err := a.EvmFeeEstimator.BumpFee(ctx, previousAttempt.Fee, tx.SpecifiedGasLimit, a.priceMaxKey(tx.FromAddress), nil)
+	gasLimit := tx.SpecifiedGasLimit
+	if tx.IsPurgeable {
+		gasLimit = a.emptyTxLimitDefault
+	}
+	bumpedFee, bumpedFeeLimit, err := a.EvmFeeEstimator.BumpFee(ctx, previousAttempt.Fee, gasLimit, a.priceMaxKey(tx.FromAddress), nil)
 	if err != nil {
 		return nil, err
 	}
 	return a.newCustomAttempt(ctx, tx, bumpedFee, bumpedFeeLimit, previousAttempt.Type, lggr)
 }
 
+func (a *attemptBuilder) NewAgnosticBumpAttempt(ctx context.Context, lggr logger.Logger, tx *types.Transaction, dynamic bool) (*types.Attempt, error) {
+	attempt, err := a.NewAttempt(ctx, lggr, tx, dynamic)
+	if err != nil {
+		return nil, err
+	}
+
+	// bump purge attempts
+	if tx.IsPurgeable {
+		// TODO: add better handling
+		for {
+			bumpedAttempt, err := a.NewBumpAttempt(ctx, lggr, tx, *attempt)
+			if err != nil {
+				if errors.Is(err, fees.ErrConnectivity) {
+					return attempt, nil
+				}
+				return nil, fmt.Errorf("error bumping attempt for txID: %v, err: %w", tx.ID, err)
+			}
+			attempt = bumpedAttempt
+		}
+	} else {
+		// bump regular attempts
+		bumps := min(maxBumpThreshold, tx.AttemptCount)
+		for range bumps {
+			bumpedAttempt, err := a.NewBumpAttempt(ctx, lggr, tx, *attempt)
+			if err != nil {
+				lggr.Errorf("error bumping attempt: %v for txID: %v", err, tx.ID)
+				return attempt, nil
+			}
+			attempt = bumpedAttempt
+		}
+	}
+
+	return attempt, nil
+}
+
 func (a *attemptBuilder) newCustomAttempt(
 	ctx context.Context,
 	tx *types.Transaction,

pkg/txm/attempt_builder_test.go

@@ -1,22 +1,28 @@
 package txm
 
 import (
+	"errors"
 	"testing"
 
+	"github.com/ethereum/go-ethereum/common"
 	evmtypes "github.com/ethereum/go-ethereum/core/types"
+
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 
 	"github.com/smartcontractkit/chainlink-common/pkg/logger"
 	"github.com/smartcontractkit/chainlink-evm/pkg/assets"
 	"github.com/smartcontractkit/chainlink-evm/pkg/gas"
+	"github.com/smartcontractkit/chainlink-evm/pkg/gas/mocks"
 	"github.com/smartcontractkit/chainlink-evm/pkg/keys/keystest"
 	"github.com/smartcontractkit/chainlink-evm/pkg/testutils"
 	"github.com/smartcontractkit/chainlink-evm/pkg/txm/types"
+	"github.com/smartcontractkit/chainlink-framework/chains/fees"
 )
 
 func TestAttemptBuilder_newLegacyAttempt(t *testing.T) {
-	ab := NewAttemptBuilder(nil, nil, keystest.TxSigner(nil))
+	ab := NewAttemptBuilder(nil, nil, keystest.TxSigner(nil), 100)
 	address := testutils.NewAddress()
 	lggr := logger.Test(t)
 	var gasLimit uint64 = 100
@@ -51,7 +57,7 @@ func TestAttemptBuilder_newLegacyAttempt(t *testing.T) {
 }
 
 func TestAttemptBuilder_newDynamicFeeAttempt(t *testing.T) {
-	ab := NewAttemptBuilder(nil, nil, keystest.TxSigner(nil))
+	ab := NewAttemptBuilder(nil, nil, keystest.TxSigner(nil), 100)
 	address := testutils.NewAddress()
 
 	lggr := logger.Test(t)
@@ -85,3 +91,214 @@ func TestAttemptBuilder_newDynamicFeeAttempt(t *testing.T) {
 		assert.Equal(t, gasLimit, a.GasLimit)
 	})
 }
+
+func TestAttemptBuilder_NewAttempt(t *testing.T) {
+	mockEstimator := mocks.NewEvmFeeEstimator(t)
+	priceMaxKey := func(addr common.Address) *assets.Wei {
+		return assets.NewWeiI(1000)
+	}
+	var nonce uint64 = 1
+	var gasLimit uint64 = 100
+	ab := NewAttemptBuilder(priceMaxKey, mockEstimator, keystest.TxSigner(nil), gasLimit)
+	address := testutils.NewAddress()
+	lggr := logger.Test(t)
+
+	t.Run("creates legacy attempt with fields", func(t *testing.T) {
+		tx := &types.Transaction{ID: 10, FromAddress: address, Nonce: &nonce}
+		mockEstimator.On("GetFee", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
+			Return(gas.EvmFee{GasPrice: assets.NewWeiI(100)}, gasLimit, nil).Once()
+		a, err := ab.NewAttempt(t.Context(), lggr, tx, false)
+		require.NoError(t, err)
+		assert.Equal(t, tx.ID, a.TxID)
+		assert.Equal(t, evmtypes.LegacyTxType, int(a.Type))
+		assert.NotNil(t, a.Fee.GasPrice)
+		assert.Equal(t, "100 wei", a.Fee.GasPrice.String())
+		assert.Nil(t, a.Fee.GasTipCap)
+		assert.Nil(t, a.Fee.GasFeeCap)
+		assert.Equal(t, gasLimit, a.GasLimit)
+	})
+
+	t.Run("creates dynamic fee attempt with fields", func(t *testing.T) {
+		tx := &types.Transaction{ID: 10, FromAddress: address, Nonce: &nonce}
+		mockEstimator.On("GetFee", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
+			Return(gas.EvmFee{DynamicFee: gas.DynamicFee{GasTipCap: assets.NewWeiI(1), GasFeeCap: assets.NewWeiI(2)}}, gasLimit, nil).Once()
+		a, err := ab.NewAttempt(t.Context(), lggr, tx, true)
+		require.NoError(t, err)
+		assert.Equal(t, tx.ID, a.TxID)
+		assert.Equal(t, evmtypes.DynamicFeeTxType, int(a.Type))
+	})
+
+	t.Run("creates purgeable attempt with fields", func(t *testing.T) {
+		tx := &types.Transaction{ID: 10, FromAddress: address, IsPurgeable: true, Nonce: &nonce}
+		mockEstimator.On("GetFee", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
+			Return(gas.EvmFee{GasPrice: assets.NewWeiI(100)}, gasLimit, nil).Once()
+		a, err := ab.NewAttempt(t.Context(), lggr, tx, false)
+		require.NoError(t, err)
+		assert.Equal(t, tx.ID, a.TxID)
+		assert.Equal(t, evmtypes.LegacyTxType, int(a.Type))
+	})
+
+	t.Run("creates dynamic fee purgeable attempt with fields", func(t *testing.T) {
+		tx := &types.Transaction{ID: 10, FromAddress: address, IsPurgeable: true, Nonce: &nonce}
+		mockEstimator.On("GetFee", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
+			Return(gas.EvmFee{DynamicFee: gas.DynamicFee{GasTipCap: assets.NewWeiI(1), GasFeeCap: assets.NewWeiI(2)}}, gasLimit, nil).Once()
+		a, err := ab.NewAttempt(t.Context(), lggr, tx, true)
+		require.NoError(t, err)
+		assert.Equal(t, tx.ID, a.TxID)
+		assert.Equal(t, evmtypes.DynamicFeeTxType, int(a.Type))
+	})
+
+	t.Run("fails if estimator returns error", func(t *testing.T) {
+		tx := &types.Transaction{ID: 10, FromAddress: address}
+		mockEstimator.On("GetFee", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
+			Return(gas.EvmFee{}, uint64(0), errors.New("estimator error")).Once()
+		_, err := ab.NewAttempt(t.Context(), lggr, tx, false)
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "estimator error")
+		mockEstimator.AssertExpectations(t)
+	})
+}
+
+func TestAttemptBuilder_NewAgnosticBumpAttempt(t *testing.T) {
+	address := testutils.NewAddress()
+	lggr := logger.Test(t)
+	var nonce uint64 = 77
+	priceMaxKey := func(addr common.Address) *assets.Wei {
+		return assets.NewWeiI(1000)
+	}
+
+	t.Run("returns original attempt when AttemptCount is 0", func(t *testing.T) {
+		mockEstimator := mocks.NewEvmFeeEstimator(t)
+		ab := NewAttemptBuilder(priceMaxKey, mockEstimator, keystest.TxSigner(nil), 100)
+
+		tx := &types.Transaction{
+			ID:           10,
+			FromAddress:  address,
+			Nonce:        &nonce,
+			AttemptCount: 0,
+		}
+
+		gasPrice := assets.NewWeiI(100)
+		initialFee := gas.EvmFee{GasPrice: gasPrice}
+		mockEstimator.On("GetFee", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
+			Return(initialFee, uint64(21000), nil).Once()
+
+		attempt, err := ab.NewAgnosticBumpAttempt(t.Context(), lggr, tx, false)
+		require.NoError(t, err)
+		assert.Equal(t, tx.ID, attempt.TxID)
+		assert.Equal(t, gasPrice.String(), attempt.Fee.GasPrice.String())
+		assert.Equal(t, evmtypes.LegacyTxType, int(attempt.Type))
+		mockEstimator.AssertExpectations(t)
+	})
+
+	t.Run("bumps once when AttemptCount is 1", func(t *testing.T) {
+		mockEstimator := mocks.NewEvmFeeEstimator(t)
+		ab := NewAttemptBuilder(priceMaxKey, mockEstimator, keystest.TxSigner(nil), 100)
+
+		tx := &types.Transaction{
+			ID:           10,
+			FromAddress:  address,
+			Nonce:        &nonce,
+			AttemptCount: 1,
+		}
+
+		gasPrice := assets.NewWeiI(100)
+		initialFee := gas.EvmFee{GasPrice: gasPrice}
+		bumpedFee := gas.EvmFee{GasPrice: gasPrice.Add(assets.NewWeiI(20))}
+		mockEstimator.On("GetFee", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
+			Return(initialFee, uint64(21000), nil).Once()
+		mockEstimator.On("BumpFee", mock.Anything, initialFee, mock.Anything, mock.Anything, mock.Anything).
+			Return(bumpedFee, uint64(21000), nil).Once()
+
+		attempt, err := ab.NewAgnosticBumpAttempt(t.Context(), lggr, tx, false)
+		require.NoError(t, err)
+		assert.Equal(t, tx.ID, attempt.TxID)
+		assert.Equal(t, bumpedFee.GasPrice.String(), attempt.Fee.GasPrice.String())
+		mockEstimator.AssertExpectations(t)
+	})
+
+	t.Run("bumps N times when AttemptCount is N", func(t *testing.T) {
+		mockEstimator := mocks.NewEvmFeeEstimator(t)
+		ab := NewAttemptBuilder(priceMaxKey, mockEstimator, keystest.TxSigner(nil), 100)
+
+		tx := &types.Transaction{
+			ID:           10,
+			FromAddress:  address,
+			Nonce:        &nonce,
+			AttemptCount: 3,
+		}
+
+		initialFee := gas.EvmFee{GasPrice: assets.NewWeiI(100)}
+		firstBump := gas.EvmFee{GasPrice: assets.NewWeiI(110)}
+		secondBump := gas.EvmFee{GasPrice: assets.NewWeiI(121)}
+		thirdBump := gas.EvmFee{GasPrice: assets.NewWeiI(133)}
+		mockEstimator.On("GetFee", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
+			Return(initialFee, uint64(21000), nil).Once()
+		mockEstimator.On("BumpFee", mock.Anything, initialFee, mock.Anything, mock.Anything, mock.Anything).
+			Return(firstBump, uint64(21000), nil).Once()
+		mockEstimator.On("BumpFee", mock.Anything, firstBump, mock.Anything, mock.Anything, mock.Anything).
+			Return(secondBump, uint64(21000), nil).Once()
+		mockEstimator.On("BumpFee", mock.Anything, secondBump, mock.Anything, mock.Anything, mock.Anything).
+			Return(thirdBump, uint64(21000), nil).Once()
+
+		attempt, err := ab.NewAgnosticBumpAttempt(t.Context(), lggr, tx, false)
+		require.NoError(t, err)
+		assert.Equal(t, tx.ID, attempt.TxID)
+		assert.Equal(t, thirdBump.GasPrice.String(), attempt.Fee.GasPrice.String())
+		mockEstimator.AssertExpectations(t)
+	})
+
+	t.Run("returns last valid attempt when BumpFee fails", func(t *testing.T) {
+		mockEstimator := mocks.NewEvmFeeEstimator(t)
+		ab := NewAttemptBuilder(priceMaxKey, mockEstimator, keystest.TxSigner(nil), 100)
+
+		tx := &types.Transaction{
+			ID:           10,
+			FromAddress:  address,
+			Nonce:        &nonce,
+			AttemptCount: 3,
+		}
+
+		gasPrice := assets.NewWeiI(100)
+		initialFee := gas.EvmFee{GasPrice: gasPrice}
+		firstBump := gas.EvmFee{GasPrice: gasPrice.Add(assets.NewWeiI(20))}
+		mockEstimator.On("GetFee", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
+			Return(initialFee, uint64(21000), nil).Once()
+		mockEstimator.On("BumpFee", mock.Anything, initialFee, mock.Anything, mock.Anything, mock.Anything).
+			Return(firstBump, uint64(21000), nil).Once()
+		mockEstimator.On("BumpFee", mock.Anything, firstBump, mock.Anything, mock.Anything, mock.Anything).
+			Return(gas.EvmFee{}, uint64(0), fees.ErrConnectivity).Once()
+
+		attempt, err := ab.NewAgnosticBumpAttempt(t.Context(), lggr, tx, false)
+		require.NoError(t, err)
+		assert.Equal(t, tx.ID, attempt.TxID)
+		// Should return the last valid bumped attempt
+		assert.Equal(t, firstBump.GasPrice.String(), attempt.Fee.GasPrice.String())
+		mockEstimator.AssertExpectations(t)
+	})
+
+	t.Run("caps bumps at maxBumpThreshold", func(t *testing.T) {
+		mockEstimator := mocks.NewEvmFeeEstimator(t)
+		ab := NewAttemptBuilder(priceMaxKey, mockEstimator, keystest.TxSigner(nil), 100)
+
+		tx := &types.Transaction{
+			ID:           10,
+			FromAddress:  address,
+			Nonce:        &nonce,
+			AttemptCount: 10, // More than maxBumpThreshold (5)
+		}
+
+		initialFee := gas.EvmFee{GasPrice: assets.NewWeiI(100)}
+		bumpedFee := gas.EvmFee{GasPrice: initialFee.GasPrice.Add(assets.NewWeiI(20))}
+		mockEstimator.On("GetFee", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
+			Return(initialFee, uint64(21000), nil).Once()
+		// Should only bump 5 times (maxBumpThreshold)
+		mockEstimator.On("BumpFee", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
+			Return(bumpedFee, uint64(21000), nil).Times(5)
+
+		attempt, err := ab.NewAgnosticBumpAttempt(t.Context(), lggr, tx, false)
+		require.NoError(t, err)
+		assert.Equal(t, tx.ID, attempt.TxID)
+		mockEstimator.AssertExpectations(t)
+	})
+}