Updated logic to add a buffer on top of the new suggested gas price w…

…hen bumping

common/fee/models.go

@@ -41,7 +41,7 @@ func CalculateBumpedFee(
 	toChainUnit feeUnitToChainUnit,
 ) (*big.Int, error) {
 	maxFeePrice := bigmath.Min(maxFeePriceInput, maxBumpPrice)
-	bumpedFeePrice := maxBumpedFee(originalfeePrice, bumpPercent, bumpMin)
+	bumpedFeePrice := MaxBumpedFee(originalfeePrice, bumpPercent, bumpMin)
 
 	// Update bumpedFeePrice if currentfeePrice is higher than bumpedFeePrice and within maxFeePrice
 	bumpedFeePrice = maxFee(lggr, currentfeePrice, bumpedFeePrice, maxFeePrice, "fee price", toChainUnit)
@@ -61,7 +61,7 @@ func CalculateBumpedFee(
 }
 
 // Returns highest bumped fee price of originalFeePrice bumped by fixed units or percentage.
-func maxBumpedFee(originalFeePrice *big.Int, feeBumpPercent uint16, feeBumpUnits *big.Int) *big.Int {
+func MaxBumpedFee(originalFeePrice *big.Int, feeBumpPercent uint16, feeBumpUnits *big.Int) *big.Int {
 	return bigmath.Max(
 		addPercentage(originalFeePrice, feeBumpPercent),
 		new(big.Int).Add(originalFeePrice, feeBumpUnits),

core/chains/evm/gas/arbitrum_estimator.go

@@ -24,6 +24,8 @@ import (
 
 type ArbConfig interface {
 	LimitMax() uint32
+	BumpPercent() uint16
+	BumpMin() *assets.Wei
 }
 
 //go:generate mockery --quiet --name ethClient --output ./mocks/ --case=underscore --structname ETHClient
@@ -56,7 +58,7 @@ func NewArbitrumEstimator(lggr logger.Logger, cfg ArbConfig, rpcClient rpcClient
 	lggr = logger.Named(lggr, "ArbitrumEstimator")
 	return &arbitrumEstimator{
 		cfg:            cfg,
-		EvmEstimator:   NewSuggestedPriceEstimator(lggr, rpcClient),
+		EvmEstimator:   NewSuggestedPriceEstimator(lggr, rpcClient, cfg),
 		client:         ethClient,
 		pollPeriod:     10 * time.Second,
 		logger:         lggr,

core/chains/evm/gas/arbitrum_estimator_test.go

@@ -23,13 +23,23 @@ import (
 )
 
 type arbConfig struct {
-	v uint32
+	v           uint32
+	bumpPercent uint16
+	bumpMin     *assets.Wei
 }
 
 func (a *arbConfig) LimitMax() uint32 {
 	return a.v
 }
 
+func (a *arbConfig) BumpPercent() uint16 {
+	return a.bumpPercent
+}
+
+func (a *arbConfig) BumpMin() *assets.Wei {
+	return a.bumpMin
+}
+
 func TestArbitrumEstimator(t *testing.T) {
 	t.Parallel()
 
@@ -38,6 +48,8 @@ func TestArbitrumEstimator(t *testing.T) {
 	calldata := []byte{0x00, 0x00, 0x01, 0x02, 0x03}
 	const gasLimit uint32 = 80000
 	const gasPriceBufferPercentage = 50
+	const bumpPercent = 10
+	var bumpMin = assets.NewWei(big.NewInt(1))
 
 	t.Run("calling GetLegacyGas on unstarted estimator returns error", func(t *testing.T) {
 		rpcClient := mocks.NewRPCClient(t)
@@ -66,7 +78,7 @@ func TestArbitrumEstimator(t *testing.T) {
 			assert.Equal(t, big.NewInt(-1), blockNumber)
 		}).Return(zeros.Bytes(), nil)
 
-		o := gas.NewArbitrumEstimator(logger.Test(t), &arbConfig{v: maxGasLimit}, rpcClient, ethClient)
+		o := gas.NewArbitrumEstimator(logger.Test(t), &arbConfig{v: maxGasLimit, bumpPercent: bumpPercent, bumpMin: bumpMin}, rpcClient, ethClient)
 		servicetest.RunHealthy(t, o)
 		gasPrice, chainSpecificGasLimit, err := o.GetLegacyGas(testutils.Context(t), calldata, gasLimit, maxGasPrice)
 		require.NoError(t, err)
@@ -197,7 +209,7 @@ func TestArbitrumEstimator(t *testing.T) {
 			assert.Equal(t, big.NewInt(-1), blockNumber)
 		}).Return(b.Bytes(), nil)
 
-		o := gas.NewArbitrumEstimator(logger.Test(t), &arbConfig{v: maxGasLimit}, rpcClient, ethClient)
+		o := gas.NewArbitrumEstimator(logger.Test(t), &arbConfig{v: maxGasLimit, bumpPercent: bumpPercent, bumpMin: bumpMin}, rpcClient, ethClient)
 		servicetest.RunHealthy(t, o)
 		gasPrice, chainSpecificGasLimit, err := o.GetLegacyGas(testutils.Context(t), calldata, gasLimit, maxGasPrice)
 		require.NoError(t, err)
@@ -231,7 +243,7 @@ func TestArbitrumEstimator(t *testing.T) {
 			assert.Equal(t, big.NewInt(-1), blockNumber)
 		}).Return(b.Bytes(), nil)
 
-		o := gas.NewArbitrumEstimator(logger.Test(t), &arbConfig{v: maxGasLimit}, rpcClient, ethClient)
+		o := gas.NewArbitrumEstimator(logger.Test(t), &arbConfig{v: maxGasLimit, bumpPercent: bumpPercent, bumpMin: bumpMin}, rpcClient, ethClient)
 		servicetest.RunHealthy(t, o)
 		gasPrice, chainSpecificGasLimit, err := o.GetLegacyGas(testutils.Context(t), calldata, gasLimit, maxGasPrice)
 		require.Error(t, err, "expected error but got (%s, %d)", gasPrice, chainSpecificGasLimit)

core/chains/evm/gas/cmd/arbgas/main.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"fmt"
 	"log"
+	"math/big"
 	"os"
 
 	"github.com/ethereum/go-ethereum/ethclient"
@@ -45,14 +46,17 @@ func printGetLegacyGas(ctx context.Context, e gas.EvmEstimator, calldata []byte,
 }
 
 const max = 50_000_000
+const bumpPercent = 20
+
+var bumpMin = assets.NewWei(big.NewInt(5_000_000_000))
 
 func withEstimator(ctx context.Context, lggr logger.SugaredLogger, url string, f func(e gas.EvmEstimator)) {
 	rc, err := rpc.Dial(url)
 	if err != nil {
 		log.Fatal(err)
 	}
 	ec := ethclient.NewClient(rc)
-	e := gas.NewArbitrumEstimator(lggr, &config{max: max}, rc, ec)
+	e := gas.NewArbitrumEstimator(lggr, &config{max: max, bumpPercent: bumpPercent, bumpMin: bumpMin}, rc, ec)
 	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
 	err = e.Start(ctx)
@@ -67,9 +71,19 @@ func withEstimator(ctx context.Context, lggr logger.SugaredLogger, url string, f
 var _ gas.ArbConfig = &config{}
 
 type config struct {
-	max uint32
+	max         uint32
+	bumpPercent uint16
+	bumpMin     *assets.Wei
 }
 
 func (c *config) LimitMax() uint32 {
 	return c.max
 }
+
+func (a *config) BumpPercent() uint16 {
+	return a.bumpPercent
+}
+
+func (a *config) BumpMin() *assets.Wei {
+	return a.bumpMin
+}

core/chains/evm/gas/models.go

@@ -87,7 +87,7 @@ func NewEstimator(lggr logger.Logger, ethClient evmclient.Client, cfg Config, ge
 		}
 	case "L2Suggested", "SuggestedPrice":
 		newEstimator = func(l logger.Logger) EvmEstimator {
-			return NewSuggestedPriceEstimator(lggr, ethClient)
+			return NewSuggestedPriceEstimator(lggr, ethClient, geCfg)
 		}
 	default:
 		lggr.Warnf("GasEstimator: unrecognised mode '%s', falling back to FixedPriceEstimator", s)

core/chains/evm/gas/suggested_price_estimator.go

@@ -2,6 +2,7 @@ package gas
 
 import (
 	"context"
+	"fmt"
 	"slices"
 	"sync"
 	"time"
@@ -12,7 +13,9 @@ import (
 	"github.com/smartcontractkit/chainlink-common/pkg/logger"
 	"github.com/smartcontractkit/chainlink-common/pkg/services"
 	"github.com/smartcontractkit/chainlink-common/pkg/utils"
+	bigmath "github.com/smartcontractkit/chainlink-common/pkg/utils/big_math"
 
+	"github.com/smartcontractkit/chainlink/v2/common/fee"
 	feetypes "github.com/smartcontractkit/chainlink/v2/common/fee/types"
 	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/assets"
 	evmclient "github.com/smartcontractkit/chainlink/v2/core/chains/evm/client"
@@ -23,6 +26,11 @@ var (
 	_ EvmEstimator = &SuggestedPriceEstimator{}
 )
 
+type suggestedPriceConfig interface {
+	BumpPercent() uint16
+	BumpMin() *assets.Wei
+}
+
 //go:generate mockery --quiet --name rpcClient --output ./mocks/ --case=underscore --structname RPCClient
 type rpcClient interface {
 	CallContext(ctx context.Context, result interface{}, method string, args ...interface{}) error
@@ -32,6 +40,7 @@ type rpcClient interface {
 type SuggestedPriceEstimator struct {
 	services.StateMachine
 
+	cfg        suggestedPriceConfig
 	client     rpcClient
 	pollPeriod time.Duration
 	logger     logger.Logger
@@ -46,11 +55,12 @@ type SuggestedPriceEstimator struct {
 }
 
 // NewSuggestedPriceEstimator returns a new Estimator which uses the suggested gas price.
-func NewSuggestedPriceEstimator(lggr logger.Logger, client rpcClient) EvmEstimator {
+func NewSuggestedPriceEstimator(lggr logger.Logger, client rpcClient, cfg suggestedPriceConfig) EvmEstimator {
 	return &SuggestedPriceEstimator{
 		client:         client,
 		pollPeriod:     10 * time.Second,
 		logger:         logger.Named(lggr, "SuggestedPriceEstimator"),
+		cfg:            cfg,
 		chForceRefetch: make(chan (chan struct{})),
 		chInitialised:  make(chan struct{}),
 		chStop:         make(chan struct{}),
@@ -184,6 +194,12 @@ func (o *SuggestedPriceEstimator) GetLegacyGas(ctx context.Context, _ []byte, Ga
 func (o *SuggestedPriceEstimator) BumpLegacyGas(ctx context.Context, originalFee *assets.Wei, feeLimit uint32, maxGasPriceWei *assets.Wei, _ []EvmPriorAttempt) (newGasPrice *assets.Wei, chainSpecificGasLimit uint32, err error) {
 	chainSpecificGasLimit = feeLimit
 	ok := o.IfStarted(func() {
+		// Immediately return error if original fee is greater than or equal to the max gas price
+		// Prevents a loop of resubmitting the attempt with the max gas price
+		if originalFee.Cmp(maxGasPriceWei) >= 0 {
+			err = fmt.Errorf("original fee (%s) greater than or equal to max gas price (%s) so cannot be bumped further", originalFee.String(), maxGasPriceWei.String())
+			return
+		}
 		err = o.forceRefresh(ctx)
 		if newGasPrice = o.getGasPrice(); newGasPrice == nil {
 			err = errors.New("failed to refresh and return gas; gas price not set")
@@ -199,7 +215,15 @@ func (o *SuggestedPriceEstimator) BumpLegacyGas(ctx context.Context, originalFee
 	if newGasPrice != nil && newGasPrice.Cmp(maxGasPriceWei) > 0 {
 		return nil, 0, errors.Errorf("estimated gas price: %s is greater than the maximum gas price configured: %s", newGasPrice.String(), maxGasPriceWei.String())
 	}
-	// Return the original price if the refreshed price is lower to ensure the bumped gas price is always equal or higher to the previous attempt
+	// Add a buffer on top of the gas price returned by the RPC.
+	// Bump logic when using the suggested gas price from an RPC is realistically only needed when there is increased volatility in gas price.
+	// This buffer is a precaution to increase the chance of getting this tx on chain
+	bufferedPrice := fee.MaxBumpedFee(newGasPrice.ToInt(), o.cfg.BumpPercent(), o.cfg.BumpMin().ToInt())
+	// If the new suggested price is less than or equal to the max and the buffer puts the new price over the max, return the max price instead
+	// The buffer is added on top of the suggested price during bumping as just a precaution. It is better to resubmit the transaction with the max gas price instead of erroring.
+	newGasPrice = assets.NewWei(bigmath.Min(bufferedPrice, maxGasPriceWei.ToInt()))
+
+	// Return the original price if the refreshed price with the buffer is lower to ensure the bumped gas price is always equal or higher to the previous attempt
 	if originalFee != nil && originalFee.Cmp(newGasPrice) > 0 {
 		return originalFee, chainSpecificGasLimit, nil
 	}