From 9e9858d2bf357fabea5a98345871661a981fdad8 Mon Sep 17 00:00:00 2001 From: Dimitris Date: Thu, 4 Jul 2024 15:27:23 +0300 Subject: [PATCH 01/11] Introduce universal estimator --- common/fee/models.go | 2 +- common/fee/utils.go | 2 +- core/chains/evm/client/chain_client.go | 8 + core/chains/evm/client/mocks/rpc_client.go | 30 ++ core/chains/evm/client/rpc_client.go | 25 + .../gas/mocks/universal_estimator_client.go | 91 ++++ core/chains/evm/gas/universal_estimator.go | 352 +++++++++++++ .../evm/gas/universal_estimator_test.go | 498 ++++++++++++++++++ 8 files changed, 1006 insertions(+), 2 deletions(-) create mode 100644 core/chains/evm/gas/mocks/universal_estimator_client.go create mode 100644 core/chains/evm/gas/universal_estimator.go create mode 100644 core/chains/evm/gas/universal_estimator_test.go diff --git a/common/fee/models.go b/common/fee/models.go index 0568a2f1433..2f0c6ce50bd 100644 --- a/common/fee/models.go +++ b/common/fee/models.go @@ -63,7 +63,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 { return bigmath.Max( - addPercentage(originalFeePrice, feeBumpPercent), + AddPercentage(originalFeePrice, feeBumpPercent), new(big.Int).Add(originalFeePrice, feeBumpUnits), ) } diff --git a/common/fee/utils.go b/common/fee/utils.go index 26323e11e26..3d4b001e839 100644 --- a/common/fee/utils.go +++ b/common/fee/utils.go @@ -18,7 +18,7 @@ func ApplyMultiplier(feeLimit uint64, multiplier float32) (uint64, error) { } // Returns the input value increased by the given percentage. -func addPercentage(value *big.Int, percentage uint16) *big.Int { +func AddPercentage(value *big.Int, percentage uint16) *big.Int { bumped := new(big.Int) bumped.Mul(value, big.NewInt(int64(100+percentage))) bumped.Div(bumped, big.NewInt(100)) diff --git a/core/chains/evm/client/chain_client.go b/core/chains/evm/client/chain_client.go index 8d1dcb6cc8c..8e50bfb560f 100644 --- a/core/chains/evm/client/chain_client.go +++ b/core/chains/evm/client/chain_client.go @@ -352,6 +352,14 @@ func (c *chainClient) LatestFinalizedBlock(ctx context.Context) (*evmtypes.Head, return c.multiNode.LatestFinalizedBlock(ctx) } +func (r *chainClient) FeeHistory(ctx context.Context, blockCount uint64, rewardPercentiles []float64) (feeHistory *ethereum.FeeHistory, err error) { + rpc, err := r.multiNode.SelectNodeRPC() + if err != nil { + return feeHistory, err + } + return rpc.FeeHistory(ctx, blockCount, rewardPercentiles) +} + func (c *chainClient) CheckTxValidity(ctx context.Context, from common.Address, to common.Address, data []byte) *SendError { msg := ethereum.CallMsg{ From: from, diff --git a/core/chains/evm/client/mocks/rpc_client.go b/core/chains/evm/client/mocks/rpc_client.go index 980a215ccfe..4a81b60d655 100644 --- a/core/chains/evm/client/mocks/rpc_client.go +++ b/core/chains/evm/client/mocks/rpc_client.go @@ -412,6 +412,36 @@ func (_m *RPCClient) EstimateGas(ctx context.Context, call interface{}) (uint64, return r0, r1 } +// FeeHistory provides a mock function with given fields: ctx, blockCount, rewardPercentiles +func (_m *RPCClient) FeeHistory(ctx context.Context, blockCount uint64, rewardPercentiles []float64) (*ethereum.FeeHistory, error) { + ret := _m.Called(ctx, blockCount, rewardPercentiles) + + if len(ret) == 0 { + panic("no return value specified for FeeHistory") + } + + var r0 *ethereum.FeeHistory + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, uint64, []float64) (*ethereum.FeeHistory, error)); ok { + return rf(ctx, blockCount, rewardPercentiles) + } + if rf, ok := ret.Get(0).(func(context.Context, uint64, []float64) *ethereum.FeeHistory); ok { + r0 = rf(ctx, blockCount, rewardPercentiles) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*ethereum.FeeHistory) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, uint64, []float64) error); ok { + r1 = rf(ctx, blockCount, rewardPercentiles) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // FilterEvents provides a mock function with given fields: ctx, query func (_m *RPCClient) FilterEvents(ctx context.Context, query ethereum.FilterQuery) ([]coretypes.Log, error) { ret := _m.Called(ctx, query) diff --git a/core/chains/evm/client/rpc_client.go b/core/chains/evm/client/rpc_client.go index 5b64900a0cb..177169be434 100644 --- a/core/chains/evm/client/rpc_client.go +++ b/core/chains/evm/client/rpc_client.go @@ -101,6 +101,7 @@ type RPCClient interface { SuggestGasPrice(ctx context.Context) (p *big.Int, err error) SuggestGasTipCap(ctx context.Context) (t *big.Int, err error) TransactionReceiptGeth(ctx context.Context, txHash common.Hash) (r *types.Receipt, err error) + FeeHistory(ctx context.Context, blockCount uint64, rewardPercentiles []float64) (feeHistory *ethereum.FeeHistory, err error) } type rawclient struct { @@ -445,6 +446,7 @@ func (r *rpcClient) TransactionReceiptGeth(ctx context.Context, txHash common.Ha return } + func (r *rpcClient) TransactionByHash(ctx context.Context, txHash common.Hash) (tx *types.Transaction, err error) { ctx, cancel, ws, http := r.makeLiveQueryCtxAndSafeGetClients(ctx) defer cancel() @@ -888,6 +890,29 @@ func (r *rpcClient) BalanceAt(ctx context.Context, account common.Address, block return } +func (r *rpcClient) FeeHistory(ctx context.Context, blockCount uint64, rewardPercentiles []float64) (feeHistory *ethereum.FeeHistory, err error) { + ctx, cancel, ws, http := r.makeLiveQueryCtxAndSafeGetClients(ctx) + defer cancel() + lggr := r.newRqLggr().With("blockCount", blockCount, "rewardPercentiles", rewardPercentiles) + + lggr.Debug("RPC call: evmclient.Client#FeeHistory") + start := time.Now() + if http != nil { + feeHistory, err = http.geth.FeeHistory(ctx, blockCount, nil, rewardPercentiles) + err = r.wrapHTTP(err) + } else { + feeHistory, err = ws.geth.FeeHistory(ctx, blockCount, nil, rewardPercentiles) + err = r.wrapWS(err) + } + duration := time.Since(start) + + r.logResult(lggr, err, duration, r.getRPCDomain(), "FeeHistory", + "feeHistory", feeHistory, + ) + + return +} + // CallArgs represents the data used to call the balance method of a contract. // "To" is the address of the ERC contract. "Data" is the message sent // to the contract. "From" is the sender address. diff --git a/core/chains/evm/gas/mocks/universal_estimator_client.go b/core/chains/evm/gas/mocks/universal_estimator_client.go new file mode 100644 index 00000000000..f549b21f92e --- /dev/null +++ b/core/chains/evm/gas/mocks/universal_estimator_client.go @@ -0,0 +1,91 @@ +// Code generated by mockery v2.42.2. DO NOT EDIT. + +package mocks + +import ( + context "context" + big "math/big" + + ethereum "github.com/ethereum/go-ethereum" + + mock "github.com/stretchr/testify/mock" +) + +// UniversalEstimatorClient is an autogenerated mock type for the universalEstimatorClient type +type UniversalEstimatorClient struct { + mock.Mock +} + +// FeeHistory provides a mock function with given fields: ctx, blockCount, rewardPercentiles +func (_m *UniversalEstimatorClient) FeeHistory(ctx context.Context, blockCount uint64, rewardPercentiles []float64) (*ethereum.FeeHistory, error) { + ret := _m.Called(ctx, blockCount, rewardPercentiles) + + if len(ret) == 0 { + panic("no return value specified for FeeHistory") + } + + var r0 *ethereum.FeeHistory + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, uint64, []float64) (*ethereum.FeeHistory, error)); ok { + return rf(ctx, blockCount, rewardPercentiles) + } + if rf, ok := ret.Get(0).(func(context.Context, uint64, []float64) *ethereum.FeeHistory); ok { + r0 = rf(ctx, blockCount, rewardPercentiles) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*ethereum.FeeHistory) + } + } + + if rf, ok := ret.Get(1).(func(context.Context, uint64, []float64) error); ok { + r1 = rf(ctx, blockCount, rewardPercentiles) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// SuggestGasPrice provides a mock function with given fields: ctx +func (_m *UniversalEstimatorClient) SuggestGasPrice(ctx context.Context) (*big.Int, error) { + ret := _m.Called(ctx) + + if len(ret) == 0 { + panic("no return value specified for SuggestGasPrice") + } + + var r0 *big.Int + var r1 error + if rf, ok := ret.Get(0).(func(context.Context) (*big.Int, error)); ok { + return rf(ctx) + } + if rf, ok := ret.Get(0).(func(context.Context) *big.Int); ok { + r0 = rf(ctx) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*big.Int) + } + } + + if rf, ok := ret.Get(1).(func(context.Context) error); ok { + r1 = rf(ctx) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// NewUniversalEstimatorClient creates a new instance of UniversalEstimatorClient. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewUniversalEstimatorClient(t interface { + mock.TestingT + Cleanup(func()) +}) *UniversalEstimatorClient { + mock := &UniversalEstimatorClient{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/core/chains/evm/gas/universal_estimator.go b/core/chains/evm/gas/universal_estimator.go new file mode 100644 index 00000000000..b6dfc028d61 --- /dev/null +++ b/core/chains/evm/gas/universal_estimator.go @@ -0,0 +1,352 @@ +package gas + +import ( + "context" + "fmt" + "math/big" + "slices" + "strconv" + "sync" + "time" + + "github.com/ethereum/go-ethereum" + + "github.com/smartcontractkit/chainlink-common/pkg/logger" + "github.com/smartcontractkit/chainlink-common/pkg/services" + bigmath "github.com/smartcontractkit/chainlink-common/pkg/utils/big_math" + + feetypes "github.com/smartcontractkit/chainlink/v2/common/fee/types" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/assets" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas/rollups" + evmtypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types" +) + +const ( + queryTimeout = 10 * time.Second + minimumBumpPercentage = 10 // based on geth's spec + + ConnectivityPercentile = 80 + BaseFeeBufferPercentage = 40 +) + +type UniversalEstimatorConfig struct { + CacheTimeout time.Duration + BumpPercent uint16 + + BlockHistoryRange uint64 // inclusive range + RewardPercentile float64 +} + +//go:generate mockery --quiet --name universalEstimatorClient --output ./mocks/ --case=underscore --structname UniversalEstimatorClient +type universalEstimatorClient interface { + SuggestGasPrice(ctx context.Context) (*big.Int, error) + FeeHistory(ctx context.Context, blockCount uint64, rewardPercentiles []float64) (feeHistory *ethereum.FeeHistory, err error) +} + +type UniversalEstimator struct { + services.StateMachine + + client universalEstimatorClient + logger logger.Logger + config UniversalEstimatorConfig + + gasPriceMu sync.RWMutex + gasPrice *assets.Wei + gasPriceLastUpdate time.Time + + dynamicPriceMu sync.RWMutex + dynamicPrice DynamicFee + dynamicPriceLastUpdate time.Time + + priorityFeeThresholdMu sync.RWMutex + priorityFeeThreshold *assets.Wei + + l1Oracle rollups.L1Oracle +} + +func NewUniversalEstimator(lggr logger.Logger, client universalEstimatorClient, cfg UniversalEstimatorConfig, l1Oracle rollups.L1Oracle) EvmEstimator { + return &UniversalEstimator{ + client: client, + logger: logger.Named(lggr, "UniversalEstimator"), + config: cfg, + l1Oracle: l1Oracle, + } +} + +func (u *UniversalEstimator) Start(context.Context) error { + // This is not an actual start since it's not a service, just a sanity check for configs + if u.config.BumpPercent < minimumBumpPercentage { + u.logger.Warnf("BumpPercent: %s is less than minimum allowed percentage: %s. Bumping attempts might result in rejections due to replacement transaction underpriced error!", + strconv.FormatUint(uint64(u.config.BumpPercent), 10), strconv.Itoa(minimumBumpPercentage)) + } + if u.config.RewardPercentile > ConnectivityPercentile { + u.logger.Warnf("RewardPercentile: %s is greater than maximum allowed connectivity percentage: %s. Lower reward percentile percentage otherwise connectivity checks will fail!", + strconv.FormatUint(uint64(u.config.RewardPercentile), 10), strconv.Itoa(ConnectivityPercentile)) + } + if u.config.BlockHistoryRange == 0 { + u.logger.Warnf("BlockHistoryRange: %s is greater than maximum allowed connectivity percentage: %s. Lower reward percentile percentage otherwise connectivity checks will fail!", + strconv.FormatUint(uint64(u.config.RewardPercentile), 10), strconv.Itoa(ConnectivityPercentile)) + } + return nil +} + +// GetLegacyGas will use eth_gasPrice to fetch the latest gas price from the RPC. +// It returns a cached value if the price was recently changed. Caching can be skipped. +func (u *UniversalEstimator) GetLegacyGas(ctx context.Context, _ []byte, gasLimit uint64, maxPrice *assets.Wei, opts ...feetypes.Opt) (gasPrice *assets.Wei, chainSpecificGasLimit uint64, err error) { + chainSpecificGasLimit = gasLimit + // TODO: fix this + refresh := false + if slices.Contains(opts, feetypes.OptForceRefetch) { + refresh = true + } + if gasPrice, err = u.fetchGasPrice(ctx, refresh); err != nil { + return + } + + if gasPrice.Cmp(maxPrice) > 0 { + u.logger.Warnf("estimated gas price: %s is greater than the maximum gas price configured: %s, returning the maximum price instead.", gasPrice, maxPrice) + return maxPrice, chainSpecificGasLimit, nil + } + return +} + +func (u *UniversalEstimator) fetchGasPrice(parentCtx context.Context, forceRefetch bool) (*assets.Wei, error) { + if !u.checkIfStale(false) && !forceRefetch { + return u.getGasPrice() + } + + ctx, cancel := context.WithTimeout(parentCtx, queryTimeout) + defer cancel() + + gasPrice, err := u.client.SuggestGasPrice(ctx) + if err != nil { + return nil, fmt.Errorf("failed to fetch gas price: %s", err) + } + + bi := (*assets.Wei)(gasPrice) + + u.logger.Debugf("fetched new gas price: %v", bi) + + u.gasPriceMu.Lock() + defer u.gasPriceMu.Unlock() + u.gasPrice = bi + u.gasPriceLastUpdate = time.Now() + return u.gasPrice, nil +} + +func (u *UniversalEstimator) getGasPrice() (*assets.Wei, error) { + u.gasPriceMu.RLock() + defer u.gasPriceMu.RUnlock() + if u.gasPrice == nil { + return u.gasPrice, fmt.Errorf("gas price not set") + } + return u.gasPrice, nil +} + +// GetDynamicFee will utilize eth_feeHistory to estimate an accurate maxFeePerGas and maxPriorityFeePerGas. +// It also has a mechanism to store the highest Nth percentile maxPriorityFeePerGas value of the latest X blocks, +// to prevent excessive bumping during connectivity incidents. +// It returns cached value if the prices were recently changed. Caching can be skipped. +func (u *UniversalEstimator) GetDynamicFee(ctx context.Context, maxPrice *assets.Wei) (fee DynamicFee, err error) { + if fee, err = u.fetchDynamicPrice(ctx, false); err != nil { + return + } + + if fee.FeeCap == nil || fee.TipCap == nil { + return fee, fmt.Errorf("dynamic price not set") + } + if fee.FeeCap.Cmp(maxPrice) > 0 { + u.logger.Warnf("estimated maxFeePerGas: %s is greater than the maximum price configured: %s, returning the maximum price instead.", + fee.FeeCap, maxPrice) + fee.FeeCap = maxPrice + if fee.TipCap.Cmp(maxPrice) > 0 { + u.logger.Warnf("estimated maxPriorityFeePerGas: %s is greater than the maximum price configured: %s, returning the maximum price instead. There won't be any room for base fee!", + fee.FeeCap, maxPrice) + fee.TipCap = maxPrice + } + } + + return +} + +// fetchDynamicPrice uses eth_feeHistory to fetch the basFee of the latest block and the Nth maxPriorityFeePerGas percentiles +// of the past X blocks. It also fetches the highest Zth maxPriorityFeePerGas percentile of the past X blocks. Z is configurable +// and it represents the highest percentile we're willing to pay. +// A buffer is added on top of the latest basFee to catch fluctuations in the next blocks. On Ethereum the increase is baseFee*1.125 per block +func (u *UniversalEstimator) fetchDynamicPrice(parentCtx context.Context, forceRefetch bool) (fee DynamicFee, err error) { + if !u.checkIfStale(true) && !forceRefetch { + return u.getDynamicPrice() + } + + ctx, cancel := context.WithTimeout(parentCtx, queryTimeout) + defer cancel() + + if u.config.BlockHistoryRange == 0 { + return fee, fmt.Errorf("BlockHistoryRange cannot be 0") + } + // RewardPercentile will be used for maxPriorityFeePerGas estimations and connectivityPercentile to set the highest threshold for bumping. + feeHistory, err := u.client.FeeHistory(ctx, u.config.BlockHistoryRange, []float64{u.config.RewardPercentile, ConnectivityPercentile}) + if err != nil { + return fee, fmt.Errorf("failed to fetch dynamic prices: %s", err) + } + + // Latest base fee + baseFee := (*assets.Wei)(feeHistory.BaseFee[len(feeHistory.BaseFee)-1]) + priorityFee := big.NewInt(0) + priorityFeeThreshold := big.NewInt(0) + for _, fee := range feeHistory.Reward { + priorityFee = priorityFee.Add(priorityFee, fee[0]) + // We don't need an average, we need the max value + priorityFeeThreshold = bigmath.Max(priorityFeeThreshold, fee[1]) + } + + u.priorityFeeThresholdMu.Lock() + u.priorityFeeThreshold = (*assets.Wei)(priorityFeeThreshold) + u.priorityFeeThresholdMu.Unlock() + + maxPriorityFeePerGas := (*assets.Wei)(priorityFee.Div(priorityFee, big.NewInt(int64(u.config.BlockHistoryRange)))) + // baseFeeBufferPercentage is used as a safety to catch fluctuations in the next block. + maxFeePerGas := baseFee.AddPercentage(BaseFeeBufferPercentage).Add((maxPriorityFeePerGas)) + + u.logger.Debugf("fetched new dynamic prices, maxFeePerGas: %v - maxPriorityFeePerGas: %v - maxPriorityFeeThreshold: %v", + maxFeePerGas, maxPriorityFeePerGas, priorityFeeThreshold) + + u.dynamicPriceMu.Lock() + defer u.dynamicPriceMu.Unlock() + u.dynamicPrice.FeeCap = maxFeePerGas + u.dynamicPrice.TipCap = maxPriorityFeePerGas + u.dynamicPriceLastUpdate = time.Now() + return u.dynamicPrice, nil +} + +func (o *UniversalEstimator) getDynamicPrice() (fee DynamicFee, err error) { + o.dynamicPriceMu.RLock() + defer o.dynamicPriceMu.RUnlock() + if o.dynamicPrice.FeeCap == nil || o.dynamicPrice.TipCap == nil { + return fee, fmt.Errorf("dynamic price not set") + } + return o.dynamicPrice, nil +} + +// checkIfStale enables caching +func (u *UniversalEstimator) checkIfStale(dynamic bool) bool { + if dynamic { + u.dynamicPriceMu.Lock() + defer u.dynamicPriceMu.Unlock() + return time.Since(u.dynamicPriceLastUpdate) >= u.config.CacheTimeout + } + u.gasPriceMu.Lock() + defer u.gasPriceMu.Unlock() + return time.Since(u.gasPriceLastUpdate) >= u.config.CacheTimeout +} + +// BumpLegacyGas provides a bumped gas price value by bumping a previous one by BumpPercent. It refreshes the market gas price by making a call to the RPC +// in case it has gone stale. If the original value is higher than the max price it returns an error as there is no room for bumping. +// It aggregates the market, bumped, and max gas price to provide a correct value. +func (u *UniversalEstimator) BumpLegacyGas(ctx context.Context, originalGasPrice *assets.Wei, gasLimit uint64, maxPrice *assets.Wei, _ []EvmPriorAttempt) (*assets.Wei, uint64, error) { + // Sanitize original fee input + if originalGasPrice == nil || originalGasPrice.Cmp(maxPrice) >= 0 { + return nil, 0, fmt.Errorf("error while retrieving original gas price: originalGasPrice: %s. Maximum price configured: %s", originalGasPrice, maxPrice) + } + + // Always refresh prices when bumping + currentGasPrice, err := u.fetchGasPrice(ctx, true) + if err != nil { + return nil, 0, err + } + + bumpedGasPrice := originalGasPrice.AddPercentage(u.config.BumpPercent) + bumpedGasPrice, err = u.limitBumpedFee(originalGasPrice, currentGasPrice, bumpedGasPrice, maxPrice) + if err != nil { + return nil, 0, fmt.Errorf("gas price error: %s", err.Error()) + } + + u.logger.Debugw("bumped gas price", "originalGasPrice", originalGasPrice, "bumpedGasPrice", bumpedGasPrice) + + return bumpedGasPrice, gasLimit, nil +} + +// BumpDynamicFee provides a bumped dynamic fee by bumping a previous one by BumpPercent. It refreshes the market prices by making a call to the RPC +// in case they have gone stale. If the original values are higher than the max price it returns an error as there is no room for bumping. Both maxFeePerGas +// as well as maxPriorityFerPergas need to be bumped otherwise the RPC won't accept the transaction and throw an error. +// See: https://github.com/ethereum/go-ethereum/issues/24284 +// It aggregates the market, bumped, and max price to provide a correct value, for both maxFeePerGas as well as maxPriorityFerPergas. +func (u *UniversalEstimator) BumpDynamicFee(ctx context.Context, originalFee DynamicFee, maxPrice *assets.Wei, _ []EvmPriorAttempt) (bumped DynamicFee, err error) { + // Sanitize original fee input + // According to geth's spec we need to bump both maxFeePerGas and maxPriorityFeePerGas for the new attempt to be accepted by the RPC + if originalFee.FeeCap == nil || + originalFee.TipCap == nil || + ((originalFee.TipCap.Cmp(originalFee.FeeCap)) > 0) || + (originalFee.FeeCap.Cmp(maxPrice) >= 0) { + return bumped, fmt.Errorf("error while retrieving original dynamic fees: (originalFeePerGas: %s - originalPriorityFeePerGas: %s). Maximum price configured: %s", + originalFee.FeeCap, originalFee.TipCap, maxPrice) + } + + // Always refresh prices when bumping + currentDynamicPrice, err := u.fetchDynamicPrice(ctx, true) + if err != nil { + return + } + + bumpedMaxPriorityFeePerGas := originalFee.TipCap.AddPercentage(u.config.BumpPercent) + bumpedMaxFeePerGas := originalFee.FeeCap.AddPercentage(u.config.BumpPercent) + + bumpedMaxPriorityFeePerGas, err = u.limitBumpedFee(originalFee.TipCap, currentDynamicPrice.TipCap, bumpedMaxPriorityFeePerGas, maxPrice) + if err != nil { + return bumped, fmt.Errorf("maxPriorityFeePerGas error: %s", err.Error()) + } + priorityFeeThreshold, err := u.getPriorityFeeThreshold() + if err != nil { + return + } + if bumpedMaxPriorityFeePerGas.Cmp(priorityFeeThreshold) > 0 { + return bumped, fmt.Errorf("bumpedMaxPriorityFeePergas: %s is above market's %sth percentile: %s, bumping is halted", + bumpedMaxPriorityFeePerGas, strconv.Itoa(ConnectivityPercentile), priorityFeeThreshold) + + } + bumpedMaxFeePerGas, err = u.limitBumpedFee(originalFee.FeeCap, currentDynamicPrice.FeeCap, bumpedMaxFeePerGas, maxPrice) + if err != nil { + return bumped, fmt.Errorf("maxFeePerGas error: %s", err.Error()) + } + + bumpedFee := DynamicFee{FeeCap: bumpedMaxFeePerGas, TipCap: bumpedMaxPriorityFeePerGas} + u.logger.Debugw("bumped dynamic fee", "originalFee", originalFee, "bumpedFee", bumpedFee) + + return bumpedFee, nil +} + +// limitBumpedFee selects the maximum value between the original fee and the bumped attempt. If the result is higher than the max price it gets capped. +// Geth's implementation has a hard 10% minimum limit of the bumped values, otherwise it rejects the transaction with an error. +// See: https://github.com/ethereum/go-ethereum/blob/bff330335b94af3643ac2fb809793f77de3069d4/core/tx_list.go#L298 +// +// Note: for chains that support EIP-1559 but we still choose to send Legacy transactions, the limit is still enforcable due to the fact that Legacy transactions +// are treated the same way as Dynamic transactions. For chains that don't support EIP-1559 at all, the limit isn't enforcable but a 10% minimum bump percentage +// makes sense anyway. +func (u *UniversalEstimator) limitBumpedFee(originalFee *assets.Wei, currentFee *assets.Wei, bufferedFee *assets.Wei, maxPrice *assets.Wei) (*assets.Wei, error) { + bumpedFee := assets.WeiMax(currentFee, bufferedFee) + if bumpedFee.Cmp(maxPrice) > 0 { + bumpedFee = maxPrice + } + + if bumpedFee.Cmp(originalFee.AddPercentage(minimumBumpPercentage)) < 0 { + return nil, fmt.Errorf("bumpedFee: %s is bumped less than minimum allowed percentage(%s) from originalFee: %s - maxPrice: %s", + bumpedFee, strconv.Itoa(minimumBumpPercentage), originalFee, maxPrice) + } + return bumpedFee, nil +} + +func (u *UniversalEstimator) getPriorityFeeThreshold() (*assets.Wei, error) { + u.priorityFeeThresholdMu.RLock() + defer u.priorityFeeThresholdMu.RUnlock() + if u.priorityFeeThreshold == nil { + return u.priorityFeeThreshold, fmt.Errorf("priorityFeeThreshold not set") + } + return u.priorityFeeThreshold, nil +} + +// These are required because Gas Estimators have been treated as services. +func (u *UniversalEstimator) Close() error { return nil } +func (u *UniversalEstimator) Name() string { return u.logger.Name() } +func (u *UniversalEstimator) L1Oracle() rollups.L1Oracle { return u.l1Oracle } +func (u *UniversalEstimator) HealthReport() map[string]error { return map[string]error{u.Name(): nil} } +func (u *UniversalEstimator) OnNewLongestChain(context.Context, *evmtypes.Head) {} diff --git a/core/chains/evm/gas/universal_estimator_test.go b/core/chains/evm/gas/universal_estimator_test.go new file mode 100644 index 00000000000..a9c1da9321a --- /dev/null +++ b/core/chains/evm/gas/universal_estimator_test.go @@ -0,0 +1,498 @@ +package gas_test + +import ( + "math/big" + "testing" + "time" + + "github.com/ethereum/go-ethereum" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + + "github.com/smartcontractkit/chainlink-common/pkg/logger" + "github.com/smartcontractkit/chainlink-common/pkg/utils/tests" + + feetypes "github.com/smartcontractkit/chainlink/v2/common/fee/types" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/assets" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas/mocks" +) + +func TestUniversalEstimatorGetLegacyGas(t *testing.T) { + t.Parallel() + + var gasLimit uint64 = 21000 + maxPrice := assets.NewWeiI(100) + + t.Run("fetches a new gas price when first called", func(t *testing.T) { + client := mocks.NewUniversalEstimatorClient(t) + client.On("SuggestGasPrice", mock.Anything).Return(big.NewInt(10), nil).Once() + + cfg := gas.UniversalEstimatorConfig{} + + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + gasPrice, _, err := u.GetLegacyGas(tests.Context(t), nil, gasLimit, maxPrice) + assert.NoError(t, err) + assert.Equal(t, assets.NewWeiI(10), gasPrice) + }) + + t.Run("without forceRefetch enabled it fetches the cached gas price if not stale", func(t *testing.T) { + client := mocks.NewUniversalEstimatorClient(t) + client.On("SuggestGasPrice", mock.Anything).Return(big.NewInt(10), nil).Once() + + cfg := gas.UniversalEstimatorConfig{CacheTimeout: 4 * time.Hour} + + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + gas1, _, err := u.GetLegacyGas(tests.Context(t), nil, gasLimit, maxPrice) + assert.NoError(t, err) + assert.Equal(t, assets.NewWeiI(10), gas1) + + gas2, _, err := u.GetLegacyGas(tests.Context(t), nil, gasLimit, maxPrice) + assert.NoError(t, err) + assert.Equal(t, assets.NewWeiI(10), gas2) + }) + + t.Run("without forceRefetch enabled it fetches the a new gas price if the cached one is stale", func(t *testing.T) { + client := mocks.NewUniversalEstimatorClient(t) + client.On("SuggestGasPrice", mock.Anything).Return(big.NewInt(10), nil).Once() + client.On("SuggestGasPrice", mock.Anything).Return(big.NewInt(15), nil).Once() + + cfg := gas.UniversalEstimatorConfig{CacheTimeout: 0 * time.Second} + + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + gas1, _, err := u.GetLegacyGas(tests.Context(t), nil, gasLimit, maxPrice) + assert.NoError(t, err) + assert.Equal(t, assets.NewWeiI(10), gas1) + + gas2, _, err := u.GetLegacyGas(tests.Context(t), nil, gasLimit, maxPrice) + assert.NoError(t, err) + assert.Equal(t, assets.NewWeiI(15), gas2) + }) + + t.Run("with forceRefetch enabled it updates the price even if not stale", func(t *testing.T) { + client := mocks.NewUniversalEstimatorClient(t) + client.On("SuggestGasPrice", mock.Anything).Return(big.NewInt(10), nil).Once() + client.On("SuggestGasPrice", mock.Anything).Return(big.NewInt(15), nil).Once() + + cfg := gas.UniversalEstimatorConfig{CacheTimeout: 4 * time.Hour} + + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + gas1, _, err := u.GetLegacyGas(tests.Context(t), nil, gasLimit, maxPrice) + assert.NoError(t, err) + assert.Equal(t, assets.NewWeiI(10), gas1) + + gas2, _, err := u.GetLegacyGas(tests.Context(t), nil, gasLimit, maxPrice, feetypes.OptForceRefetch) + assert.NoError(t, err) + assert.Equal(t, assets.NewWeiI(15), gas2) + }) + + t.Run("will return max price if estimation exceeds it", func(t *testing.T) { + client := mocks.NewUniversalEstimatorClient(t) + client.On("SuggestGasPrice", mock.Anything).Return(big.NewInt(10), nil).Once() + + cfg := gas.UniversalEstimatorConfig{} + + maxPrice := assets.NewWeiI(1) + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + gas1, _, err := u.GetLegacyGas(tests.Context(t), nil, gasLimit, maxPrice) + assert.NoError(t, err) + assert.Equal(t, maxPrice, gas1) + }) +} + +func TestUniversalEstimatorBumpLegacyGas(t *testing.T) { + t.Parallel() + + var gasLimit uint64 = 21000 + maxPrice := assets.NewWeiI(100) + + t.Run("bumps a previous attempt by BumpPercent", func(t *testing.T) { + client := mocks.NewUniversalEstimatorClient(t) + originalGasPrice := assets.NewWeiI(10) + client.On("SuggestGasPrice", mock.Anything).Return(big.NewInt(10), nil).Once() + + cfg := gas.UniversalEstimatorConfig{BumpPercent: 50} + + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + gasPrice, _, err := u.BumpLegacyGas(tests.Context(t), originalGasPrice, gasLimit, maxPrice, nil) + assert.NoError(t, err) + assert.Equal(t, assets.NewWeiI(15), gasPrice) + }) + + t.Run("fails if the original attempt is nil, or equal or higher than the max price", func(t *testing.T) { + client := mocks.NewUniversalEstimatorClient(t) + + cfg := gas.UniversalEstimatorConfig{} + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + + var originalPrice *assets.Wei + _, _, err := u.BumpLegacyGas(tests.Context(t), originalPrice, gasLimit, maxPrice, nil) + assert.Error(t, err) + + originalPrice = assets.NewWeiI(100) + _, _, err = u.BumpLegacyGas(tests.Context(t), originalPrice, gasLimit, maxPrice, nil) + assert.Error(t, err) + + }) + + t.Run("returns market gas price if bumped original fee is lower", func(t *testing.T) { + client := mocks.NewUniversalEstimatorClient(t) + client.On("SuggestGasPrice", mock.Anything).Return(big.NewInt(80), nil).Once() + originalGasPrice := assets.NewWeiI(10) + + cfg := gas.UniversalEstimatorConfig{} + + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + gas, _, err := u.BumpLegacyGas(tests.Context(t), originalGasPrice, gasLimit, maxPrice, nil) + assert.NoError(t, err) + assert.Equal(t, assets.NewWeiI(80), gas) + }) + + t.Run("returns max gas price if bumped original fee is higher", func(t *testing.T) { + client := mocks.NewUniversalEstimatorClient(t) + client.On("SuggestGasPrice", mock.Anything).Return(big.NewInt(1), nil).Once() + originalGasPrice := assets.NewWeiI(10) + + cfg := gas.UniversalEstimatorConfig{BumpPercent: 50} + + maxPrice := assets.NewWeiI(14) + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + gas, _, err := u.BumpLegacyGas(tests.Context(t), originalGasPrice, gasLimit, maxPrice, nil) + assert.NoError(t, err) + assert.Equal(t, maxPrice, gas) + }) + + t.Run("returns max gas price if the aggregation of max and original bumped fee is higher", func(t *testing.T) { + client := mocks.NewUniversalEstimatorClient(t) + client.On("SuggestGasPrice", mock.Anything).Return(big.NewInt(1), nil).Once() + originalGasPrice := assets.NewWeiI(10) + + cfg := gas.UniversalEstimatorConfig{BumpPercent: 50} + + maxPrice := assets.NewWeiI(14) + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + gas, _, err := u.BumpLegacyGas(tests.Context(t), originalGasPrice, gasLimit, maxPrice, nil) + assert.NoError(t, err) + assert.Equal(t, maxPrice, gas) + }) + + t.Run("fails if the bumped gas price is lower than the minimum bump percentage", func(t *testing.T) { + client := mocks.NewUniversalEstimatorClient(t) + client.On("SuggestGasPrice", mock.Anything).Return(big.NewInt(100), nil).Once() + originalGasPrice := assets.NewWeiI(100) + + cfg := gas.UniversalEstimatorConfig{BumpPercent: 20} + + // Price will be capped by the max price + maxPrice := assets.NewWeiI(101) + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + _, _, err := u.BumpLegacyGas(tests.Context(t), originalGasPrice, gasLimit, maxPrice, nil) + assert.Error(t, err) + }) +} + +func TestUniversalEstimatorGetDynamicFee(t *testing.T) { + t.Parallel() + + maxPrice := assets.NewWeiI(100) + + t.Run("fetches a new dynamic fee when first called", func(t *testing.T) { + client := mocks.NewUniversalEstimatorClient(t) + baseFee := big.NewInt(5) + maxPriorityFeePerGas1 := big.NewInt(33) + maxPriorityFeePerGas2 := big.NewInt(20) + + feeHistoryResult := ðereum.FeeHistory{ + OldestBlock: big.NewInt(1), + Reward: [][]*big.Int{{maxPriorityFeePerGas1, big.NewInt(5)}, {maxPriorityFeePerGas2, big.NewInt(5)}}, // first one represents market price and second one connectivity price + BaseFee: []*big.Int{baseFee, baseFee}, + GasUsedRatio: nil, + } + client.On("FeeHistory", mock.Anything, mock.Anything, mock.Anything).Return(feeHistoryResult, nil).Once() + + blockHistoryLength := 2 + cfg := gas.UniversalEstimatorConfig{BlockHistoryRange: uint64(blockHistoryLength)} + avrgPriorityFee := big.NewInt(0) + avrgPriorityFee.Add(maxPriorityFeePerGas1, maxPriorityFeePerGas2).Div(avrgPriorityFee, big.NewInt(int64(blockHistoryLength))) + maxFee := (*assets.Wei)(baseFee).AddPercentage(gas.BaseFeeBufferPercentage).Add((*assets.Wei)(avrgPriorityFee)) + + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + dynamicFee, err := u.GetDynamicFee(tests.Context(t), maxPrice) + assert.NoError(t, err) + assert.Equal(t, maxFee, dynamicFee.FeeCap) + assert.Equal(t, (*assets.Wei)(avrgPriorityFee), dynamicFee.TipCap) + }) + + t.Run("fails if BlockHistoryRange is zero and tries to fetch new prices", func(t *testing.T) { + client := mocks.NewUniversalEstimatorClient(t) + + cfg := gas.UniversalEstimatorConfig{BlockHistoryRange: 0} + + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + _, err := u.GetDynamicFee(tests.Context(t), maxPrice) + assert.Error(t, err) + }) + + t.Run("without forceRefetch enabled it fetches the cached dynamic fees if not stale", func(t *testing.T) { + client := mocks.NewUniversalEstimatorClient(t) + baseFee := big.NewInt(1) + maxPriorityFeePerGas := big.NewInt(1) + + feeHistoryResult := ðereum.FeeHistory{ + OldestBlock: big.NewInt(1), + Reward: [][]*big.Int{{maxPriorityFeePerGas, big.NewInt(2)}}, // first one represents market price and second one connectivity price + BaseFee: []*big.Int{baseFee}, + GasUsedRatio: nil, + } + client.On("FeeHistory", mock.Anything, mock.Anything, mock.Anything).Return(feeHistoryResult, nil).Once() + + cfg := gas.UniversalEstimatorConfig{ + CacheTimeout: 4 * time.Hour, + BlockHistoryRange: 1, + } + maxFee := (*assets.Wei)(baseFee).AddPercentage(gas.BaseFeeBufferPercentage).Add((*assets.Wei)(maxPriorityFeePerGas)) + + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + dynamicFee, err := u.GetDynamicFee(tests.Context(t), maxPrice) + assert.NoError(t, err) + assert.Equal(t, maxFee, dynamicFee.FeeCap) + assert.Equal(t, (*assets.Wei)(maxPriorityFeePerGas), dynamicFee.TipCap) + + dynamicFee2, err := u.GetDynamicFee(tests.Context(t), maxPrice) + assert.NoError(t, err) + assert.Equal(t, maxFee, dynamicFee2.FeeCap) + assert.Equal(t, (*assets.Wei)(maxPriorityFeePerGas), dynamicFee2.TipCap) + + }) + + t.Run("fetches a new dynamic fee when first called", func(t *testing.T) { + client := mocks.NewUniversalEstimatorClient(t) + baseFee := big.NewInt(1) + maxPriorityFeePerGas := big.NewInt(1) + + feeHistoryResult := ðereum.FeeHistory{ + OldestBlock: big.NewInt(1), + Reward: [][]*big.Int{{maxPriorityFeePerGas, big.NewInt(2)}}, // first one represents market price and second one connectivity price + BaseFee: []*big.Int{baseFee}, + GasUsedRatio: nil, + } + client.On("FeeHistory", mock.Anything, mock.Anything, mock.Anything).Return(feeHistoryResult, nil).Once() + + cfg := gas.UniversalEstimatorConfig{BlockHistoryRange: 1} + maxFee := (*assets.Wei)(baseFee).AddPercentage(gas.BaseFeeBufferPercentage).Add((*assets.Wei)(maxPriorityFeePerGas)) + + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + dynamicFee, err := u.GetDynamicFee(tests.Context(t), maxPrice) + assert.NoError(t, err) + assert.Equal(t, maxFee, dynamicFee.FeeCap) + assert.Equal(t, (*assets.Wei)(maxPriorityFeePerGas), dynamicFee.TipCap) + }) + + t.Run("will return max price if tip cap or fee cap exceed it", func(t *testing.T) { + client := mocks.NewUniversalEstimatorClient(t) + baseFee := big.NewInt(1) + maxPriorityFeePerGas := big.NewInt(3) + maxPrice := assets.NewWeiI(2) + + feeHistoryResult := ðereum.FeeHistory{ + OldestBlock: big.NewInt(1), + Reward: [][]*big.Int{{maxPriorityFeePerGas, big.NewInt(5)}}, // first one represents market price and second one connectivity price + BaseFee: []*big.Int{baseFee}, + GasUsedRatio: nil, + } + client.On("FeeHistory", mock.Anything, mock.Anything, mock.Anything).Return(feeHistoryResult, nil).Once() + + cfg := gas.UniversalEstimatorConfig{BlockHistoryRange: 1} + + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + dynamicFee, err := u.GetDynamicFee(tests.Context(t), maxPrice) + assert.NoError(t, err) + assert.Equal(t, maxPrice, dynamicFee.FeeCap) + assert.Equal(t, maxPrice, dynamicFee.TipCap) + }) +} + +func TestUniversalEstimatorBumpDynamicFee(t *testing.T) { + t.Parallel() + + maxPrice := assets.NewWeiI(100) + + t.Run("bumps a previous attempt by BumpPercent", func(t *testing.T) { + client := mocks.NewUniversalEstimatorClient(t) + originalFee := gas.DynamicFee{ + FeeCap: assets.NewWeiI(20), + TipCap: assets.NewWeiI(10), + } + + // These values will be ignored because they are lower prices than the originalFee + feeHistoryResult := ðereum.FeeHistory{ + OldestBlock: big.NewInt(1), + Reward: [][]*big.Int{{big.NewInt(5), big.NewInt(50)}}, // first one represents market price and second one connectivity price + BaseFee: []*big.Int{big.NewInt(5)}, + GasUsedRatio: nil, + } + client.On("FeeHistory", mock.Anything, mock.Anything, mock.Anything).Return(feeHistoryResult, nil).Once() + + cfg := gas.UniversalEstimatorConfig{ + BlockHistoryRange: 2, + BumpPercent: 50, + } + + expectedFeeCap := originalFee.FeeCap.AddPercentage(cfg.BumpPercent) + expectedTipCap := originalFee.TipCap.AddPercentage(cfg.BumpPercent) + + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + dynamicFee, err := u.BumpDynamicFee(tests.Context(t), originalFee, maxPrice, nil) + assert.NoError(t, err) + assert.Equal(t, expectedFeeCap, dynamicFee.FeeCap) + assert.Equal(t, expectedTipCap, dynamicFee.TipCap) + }) + + t.Run("fails if the original attempt is invalid", func(t *testing.T) { + client := mocks.NewUniversalEstimatorClient(t) + maxPrice := assets.NewWeiI(20) + cfg := gas.UniversalEstimatorConfig{BlockHistoryRange: 1} + + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + // nil original fee + var originalFee gas.DynamicFee + _, err := u.BumpDynamicFee(tests.Context(t), originalFee, maxPrice, nil) + assert.Error(t, err) + + // tip cap is higher than fee cap + originalFee = gas.DynamicFee{ + FeeCap: assets.NewWeiI(10), + TipCap: assets.NewWeiI(11), + } + _, err = u.BumpDynamicFee(tests.Context(t), originalFee, maxPrice, nil) + assert.Error(t, err) + + // fee cap is equal or higher to max price + originalFee = gas.DynamicFee{ + FeeCap: assets.NewWeiI(20), + TipCap: assets.NewWeiI(10), + } + _, err = u.BumpDynamicFee(tests.Context(t), originalFee, maxPrice, nil) + assert.Error(t, err) + }) + + t.Run("returns market prices bumped by BumpPercent if bumped original fee is lower", func(t *testing.T) { + client := mocks.NewUniversalEstimatorClient(t) + originalFee := gas.DynamicFee{ + FeeCap: assets.NewWeiI(20), + TipCap: assets.NewWeiI(10), + } + + // Market fees + baseFee := big.NewInt(5) + maxPriorityFeePerGas := big.NewInt(33) + feeHistoryResult := ðereum.FeeHistory{ + OldestBlock: big.NewInt(1), + Reward: [][]*big.Int{{maxPriorityFeePerGas, big.NewInt(100)}}, // first one represents market price and second one connectivity price + BaseFee: []*big.Int{baseFee}, + GasUsedRatio: nil, + } + client.On("FeeHistory", mock.Anything, mock.Anything, mock.Anything).Return(feeHistoryResult, nil).Once() + + maxFee := (*assets.Wei)(baseFee).AddPercentage(gas.BaseFeeBufferPercentage).Add((*assets.Wei)(maxPriorityFeePerGas)) + + cfg := gas.UniversalEstimatorConfig{ + BlockHistoryRange: 1, + BumpPercent: 50, + } + + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + bumpedFee, err := u.BumpDynamicFee(tests.Context(t), originalFee, maxPrice, nil) + assert.NoError(t, err) + assert.Equal(t, (*assets.Wei)(maxPriorityFeePerGas), bumpedFee.TipCap) + assert.Equal(t, maxFee, bumpedFee.FeeCap) + }) + + t.Run("fails if connectivity percentile value is reached", func(t *testing.T) { + client := mocks.NewUniversalEstimatorClient(t) + originalFee := gas.DynamicFee{ + FeeCap: assets.NewWeiI(20), + TipCap: assets.NewWeiI(10), + } + + // Market fees + baseFee := big.NewInt(5) + maxPriorityFeePerGas := big.NewInt(33) + feeHistoryResult := ðereum.FeeHistory{ + OldestBlock: big.NewInt(1), + Reward: [][]*big.Int{{maxPriorityFeePerGas, big.NewInt(30)}}, // first one represents market price and second one connectivity price + BaseFee: []*big.Int{baseFee}, + GasUsedRatio: nil, + } + client.On("FeeHistory", mock.Anything, mock.Anything, mock.Anything).Return(feeHistoryResult, nil).Once() + + cfg := gas.UniversalEstimatorConfig{ + BlockHistoryRange: 1, + BumpPercent: 50, + } + + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + _, err := u.BumpDynamicFee(tests.Context(t), originalFee, maxPrice, nil) + assert.Error(t, err) + }) + + t.Run("returns max price if the aggregation of max and original bumped fee is higher", func(t *testing.T) { + client := mocks.NewUniversalEstimatorClient(t) + originalFee := gas.DynamicFee{ + FeeCap: assets.NewWeiI(20), + TipCap: assets.NewWeiI(18), + } + + maxPrice := assets.NewWeiI(25) + // Market fees + baseFee := big.NewInt(1) + maxPriorityFeePerGas := big.NewInt(1) + feeHistoryResult := ðereum.FeeHistory{ + OldestBlock: big.NewInt(1), + Reward: [][]*big.Int{{maxPriorityFeePerGas, big.NewInt(30)}}, // first one represents market price and second one connectivity price + BaseFee: []*big.Int{baseFee}, + GasUsedRatio: nil, + } + client.On("FeeHistory", mock.Anything, mock.Anything, mock.Anything).Return(feeHistoryResult, nil).Once() + + cfg := gas.UniversalEstimatorConfig{ + BlockHistoryRange: 1, + BumpPercent: 50, + } + + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + bumpedFee, err := u.BumpDynamicFee(tests.Context(t), originalFee, maxPrice, nil) + assert.NoError(t, err) + assert.Equal(t, maxPrice, bumpedFee.TipCap) + assert.Equal(t, maxPrice, bumpedFee.FeeCap) + }) + + t.Run("fails if the bumped gas price is lower than the minimum bump percentage", func(t *testing.T) { + client := mocks.NewUniversalEstimatorClient(t) + originalFee := gas.DynamicFee{ + FeeCap: assets.NewWeiI(20), + TipCap: assets.NewWeiI(18), + } + + maxPrice := assets.NewWeiI(21) + // Market fees + baseFee := big.NewInt(1) + maxPriorityFeePerGas := big.NewInt(1) + feeHistoryResult := ðereum.FeeHistory{ + OldestBlock: big.NewInt(1), + Reward: [][]*big.Int{{maxPriorityFeePerGas, big.NewInt(30)}}, // first one represents market price and second one connectivity price + BaseFee: []*big.Int{baseFee}, + GasUsedRatio: nil, + } + client.On("FeeHistory", mock.Anything, mock.Anything, mock.Anything).Return(feeHistoryResult, nil).Once() + + cfg := gas.UniversalEstimatorConfig{ + BlockHistoryRange: 1, + BumpPercent: 50, + } + + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + _, err := u.BumpDynamicFee(tests.Context(t), originalFee, maxPrice, nil) + assert.Error(t, err) + }) +} From ed3f70a6252d1c948bf4dedd2ad307e866b13a03 Mon Sep 17 00:00:00 2001 From: Dimitris Date: Fri, 5 Jul 2024 15:34:03 +0300 Subject: [PATCH 02/11] Refactor fixed price estimator --- common/fee/models.go | 49 ----- common/fee/utils.go | 3 - core/chains/evm/assets/wei.go | 4 - .../chains/evm/gas/block_history_estimator.go | 5 +- core/chains/evm/gas/fixed_price_estimator.go | 144 ++++--------- .../evm/gas/fixed_price_estimator_test.go | 204 ++++++++++-------- .../gas/mocks/fixed_price_estimator_config.go | 106 +++++++++ core/chains/evm/gas/models.go | 25 +-- core/chains/evm/txmgr/broadcaster_test.go | 6 +- core/chains/evm/txmgr/confirmer_test.go | 2 +- 10 files changed, 285 insertions(+), 263 deletions(-) create mode 100644 core/chains/evm/gas/mocks/fixed_price_estimator_config.go diff --git a/common/fee/models.go b/common/fee/models.go index 2f0c6ce50bd..cebdaac3215 100644 --- a/common/fee/models.go +++ b/common/fee/models.go @@ -2,11 +2,8 @@ package fee import ( "errors" - "fmt" "math/big" - "github.com/smartcontractkit/chainlink-common/pkg/chains/label" - "github.com/smartcontractkit/chainlink-common/pkg/logger" bigmath "github.com/smartcontractkit/chainlink-common/pkg/utils/big_math" ) @@ -30,36 +27,6 @@ func CalculateFee( return bigmath.Min(defaultPrice, maxFeePriceAllowed) } -// CalculateBumpedFee computes the next fee price to attempt as the largest of: -// - A configured percentage bump (bumpPercent) on top of the baseline price. -// - A configured fixed amount of Unit (bumpMin) on top of the baseline price. -// The baseline price is the maximum of the previous fee price attempt and the node's current fee price. -func CalculateBumpedFee( - lggr logger.SugaredLogger, - currentfeePrice, originalfeePrice, maxFeePriceInput, maxBumpPrice, bumpMin *big.Int, - bumpPercent uint16, - toChainUnit feeUnitToChainUnit, -) (*big.Int, error) { - maxFeePrice := bigmath.Min(maxFeePriceInput, maxBumpPrice) - 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) - - if bumpedFeePrice.Cmp(maxFeePrice) > 0 { - return maxFeePrice, fmt.Errorf("bumped fee price of %s would exceed configured max fee price of %s (original price was %s). %s: %w", - toChainUnit(bumpedFeePrice), toChainUnit(maxFeePrice), toChainUnit(originalfeePrice), label.NodeConnectivityProblemWarning, ErrBumpFeeExceedsLimit) - } else if bumpedFeePrice.Cmp(originalfeePrice) == 0 { - // NOTE: This really shouldn't happen since we enforce minimums for - // FeeEstimator.BumpPercent and FeeEstimator.BumpMin in the config validation, - // but it's here anyway for a "belts and braces" approach - return bumpedFeePrice, fmt.Errorf("bumped fee price of %s is equal to original fee price of %s."+ - " ACTION REQUIRED: This is a configuration error, you must increase either "+ - "FeeEstimator.BumpPercent or FeeEstimator.BumpMin: %w", toChainUnit(bumpedFeePrice), toChainUnit(bumpedFeePrice), ErrBump) - } - return bumpedFeePrice, nil -} - // 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 { return bigmath.Max( @@ -67,19 +34,3 @@ func MaxBumpedFee(originalFeePrice *big.Int, feeBumpPercent uint16, feeBumpUnits new(big.Int).Add(originalFeePrice, feeBumpUnits), ) } - -// Returns the max of currentFeePrice, bumpedFeePrice, and maxFeePrice -func maxFee(lggr logger.SugaredLogger, currentFeePrice, bumpedFeePrice, maxFeePrice *big.Int, feeType string, toChainUnit feeUnitToChainUnit) *big.Int { - if currentFeePrice == nil { - return bumpedFeePrice - } - if currentFeePrice.Cmp(maxFeePrice) > 0 { - // Shouldn't happen because the estimator should not be allowed to - // estimate a higher fee than the maximum allowed - lggr.AssumptionViolationf("Ignoring current %s of %s that would exceed max %s of %s", feeType, toChainUnit(currentFeePrice), feeType, toChainUnit(maxFeePrice)) - } else if bumpedFeePrice.Cmp(currentFeePrice) < 0 { - // If the current fee price is higher than the old price bumped, use that instead - return currentFeePrice - } - return bumpedFeePrice -} diff --git a/common/fee/utils.go b/common/fee/utils.go index 3d4b001e839..7ea8c132269 100644 --- a/common/fee/utils.go +++ b/common/fee/utils.go @@ -24,6 +24,3 @@ func AddPercentage(value *big.Int, percentage uint16) *big.Int { bumped.Div(bumped, big.NewInt(100)) return bumped } - -// Returns the fee in its chain specific unit. -type feeUnitToChainUnit func(fee *big.Int) string diff --git a/core/chains/evm/assets/wei.go b/core/chains/evm/assets/wei.go index 4fcdc15146b..66ed3441fe8 100644 --- a/core/chains/evm/assets/wei.go +++ b/core/chains/evm/assets/wei.go @@ -63,10 +63,6 @@ func suffixExp(suf string) int32 { // additional functions type Wei ubig.Big -func MaxWei(w, x *Wei) *Wei { - return NewWei(bigmath.Max(w.ToInt(), x.ToInt())) -} - // NewWei constructs a Wei from *big.Int. func NewWei(i *big.Int) *Wei { return (*Wei)(i) diff --git a/core/chains/evm/gas/block_history_estimator.go b/core/chains/evm/gas/block_history_estimator.go index 16a66e1c8be..dfc5ee5a2df 100644 --- a/core/chains/evm/gas/block_history_estimator.go +++ b/core/chains/evm/gas/block_history_estimator.go @@ -88,7 +88,8 @@ type estimatorGasEstimatorConfig interface { TipCapMin() *assets.Wei PriceMax() *assets.Wei PriceMin() *assets.Wei - bumpConfig + BumpPercent() uint16 + BumpMin() *assets.Wei } //go:generate mockery --quiet --name Config --output ./mocks/ --case=underscore @@ -409,7 +410,7 @@ func (b *BlockHistoryEstimator) GetDynamicFee(_ context.Context, maxGasPriceWei "Using Evm.GasEstimator.TipCapDefault as fallback.", "blocks", b.getBlockHistoryNumbers()) tipCap = b.eConfig.TipCapDefault() } - maxGasPrice := getMaxGasPrice(maxGasPriceWei, b.eConfig.PriceMax()) + maxGasPrice := assets.WeiMin(maxGasPriceWei, b.eConfig.PriceMax()) if b.eConfig.BumpThreshold() == 0 { // just use the max gas price if gas bumping is disabled feeCap = maxGasPrice diff --git a/core/chains/evm/gas/fixed_price_estimator.go b/core/chains/evm/gas/fixed_price_estimator.go index 7ac086bf067..a30604a47c6 100644 --- a/core/chains/evm/gas/fixed_price_estimator.go +++ b/core/chains/evm/gas/fixed_price_estimator.go @@ -2,139 +2,85 @@ package gas import ( "context" - - pkgerrors "github.com/pkg/errors" + "fmt" "github.com/smartcontractkit/chainlink-common/pkg/logger" - commonfee "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" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas/rollups" evmtypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types" ) -var _ EvmEstimator = (*fixedPriceEstimator)(nil) - -type fixedPriceEstimator struct { - config fixedPriceEstimatorConfig - bhConfig fixedPriceEstimatorBlockHistoryConfig +type FixedPriceEstimator struct { lggr logger.SugaredLogger + config fixedPriceEstimatorConfig l1Oracle rollups.L1Oracle } -type bumpConfig interface { - LimitMultiplier() float32 - PriceMax() *assets.Wei - BumpPercent() uint16 - BumpMin() *assets.Wei - TipCapDefault() *assets.Wei -} +//go:generate mockery --quiet --name fixedPriceEstimatorConfig --output ./mocks/ --case=underscore --structname FixedPriceEstimatorConfig type fixedPriceEstimatorConfig interface { - BumpThreshold() uint64 - FeeCapDefault() *assets.Wei PriceDefault() *assets.Wei TipCapDefault() *assets.Wei - PriceMax() *assets.Wei - Mode() string - bumpConfig -} + FeeCapDefault() *assets.Wei -type fixedPriceEstimatorBlockHistoryConfig interface { - EIP1559FeeCapBufferBlocks() uint16 + BumpPercent() uint16 } -// NewFixedPriceEstimator returns a new "FixedPrice" estimator which will -// always use the config default values for gas prices and limits -func NewFixedPriceEstimator(cfg fixedPriceEstimatorConfig, ethClient feeEstimatorClient, bhCfg fixedPriceEstimatorBlockHistoryConfig, lggr logger.Logger, l1Oracle rollups.L1Oracle) EvmEstimator { - return &fixedPriceEstimator{cfg, bhCfg, logger.Sugared(logger.Named(lggr, "FixedPriceEstimator")), l1Oracle} +func NewFixedPriceEstimator(lggr logger.Logger, config fixedPriceEstimatorConfig, l1Oracle rollups.L1Oracle) EvmEstimator { + return &FixedPriceEstimator{logger.Sugared(logger.Named(lggr, "FixedPriceEstimator")), config, l1Oracle} } -func (f *fixedPriceEstimator) Start(context.Context) error { - if f.config.BumpThreshold() == 0 && f.config.Mode() == "FixedPrice" { - // EvmGasFeeCapDefault is ignored if fixed estimator mode is on and gas bumping is disabled - if f.config.FeeCapDefault().Cmp(f.config.PriceMax()) != 0 { - f.lggr.Infof("You are using FixedPrice estimator with gas bumping disabled. EVM.GasEstimator.PriceMax (value: %s) will be used as the FeeCap for transactions", f.config.PriceMax()) - } - } - return nil -} +func (f *FixedPriceEstimator) Start(context.Context) error { return nil } -func (f *fixedPriceEstimator) GetLegacyGas(_ context.Context, _ []byte, gasLimit uint64, maxGasPriceWei *assets.Wei, _ ...feetypes.Opt) (*assets.Wei, uint64, error) { - gasPrice := commonfee.CalculateFee(f.config.PriceDefault().ToInt(), maxGasPriceWei.ToInt(), f.config.PriceMax().ToInt()) - chainSpecificGasLimit := gasLimit - return assets.NewWei(gasPrice), chainSpecificGasLimit, nil +func (f *FixedPriceEstimator) GetLegacyGas(_ context.Context, _ []byte, gasLimit uint64, maxPrice *assets.Wei, _ ...feetypes.Opt) (*assets.Wei, uint64, error) { + gasPrice := assets.WeiMin(f.config.PriceDefault(), maxPrice) + return gasPrice, gasLimit, nil } -func (f *fixedPriceEstimator) BumpLegacyGas( - _ context.Context, - originalGasPrice *assets.Wei, - originalGasLimit uint64, - maxGasPriceWei *assets.Wei, - _ []EvmPriorAttempt, -) (*assets.Wei, uint64, error) { - gasPrice, err := commonfee.CalculateBumpedFee( - f.lggr, - f.config.PriceDefault().ToInt(), - originalGasPrice.ToInt(), - maxGasPriceWei.ToInt(), - f.config.PriceMax().ToInt(), - f.config.BumpMin().ToInt(), - f.config.BumpPercent(), - assets.FormatWei, - ) - if err != nil { - return nil, 0, err +func (f *FixedPriceEstimator) BumpLegacyGas(_ context.Context, originalGasPrice *assets.Wei, gasLimit uint64, maxPrice *assets.Wei, _ []EvmPriorAttempt) (*assets.Wei, uint64, error) { + // Sanitize original fee input + if originalGasPrice == nil || originalGasPrice.Cmp(maxPrice) >= 0 { + return nil, 0, fmt.Errorf("error while retrieving original gas price: originalGasPrice: %s. Maximum price configured: %s", originalGasPrice, maxPrice) } - chainSpecificGasLimit := originalGasLimit - return assets.NewWei(gasPrice), chainSpecificGasLimit, err + bumpedGasPrice := originalGasPrice.AddPercentage(f.config.BumpPercent()) + bumpedGasPrice = assets.WeiMin(bumpedGasPrice, maxPrice) + return bumpedGasPrice, gasLimit, nil } -func (f *fixedPriceEstimator) GetDynamicFee(_ context.Context, maxGasPriceWei *assets.Wei) (d DynamicFee, err error) { - gasTipCap := f.config.TipCapDefault() +func (f *FixedPriceEstimator) GetDynamicFee(_ context.Context, maxPrice *assets.Wei) (d DynamicFee, err error) { + maxPriorityFeePerGas := assets.WeiMin(f.config.TipCapDefault(), maxPrice) + maxFeePerGas := assets.WeiMin(f.config.FeeCapDefault(), maxPrice) - if gasTipCap == nil { - return d, pkgerrors.New("cannot calculate dynamic fee: EthGasTipCapDefault was not set") - } + return DynamicFee{FeeCap: maxFeePerGas, TipCap: maxPriorityFeePerGas}, nil +} - var feeCap *assets.Wei - if f.config.BumpThreshold() == 0 { - // Gas bumping is disabled, just use the max fee cap - feeCap = getMaxGasPrice(maxGasPriceWei, f.config.PriceMax()) - } else { - // Need to leave headroom for bumping so we fallback to the default value here - feeCap = f.config.FeeCapDefault() +func (f *FixedPriceEstimator) BumpDynamicFee(_ context.Context, originalFee DynamicFee, maxPrice *assets.Wei, _ []EvmPriorAttempt) (bumpedFee DynamicFee, err error) { + // Sanitize original fee input + if originalFee.FeeCap == nil || + originalFee.TipCap == nil || + ((originalFee.TipCap.Cmp(originalFee.FeeCap)) > 0) || + (originalFee.FeeCap.Cmp(maxPrice) >= 0) { + return bumpedFee, fmt.Errorf("error while retrieving original dynamic fees: (originalFeePerGas: %s - originalPriorityFeePerGas: %s). Maximum price configured: %s", + originalFee.FeeCap, originalFee.TipCap, maxPrice) } - return DynamicFee{ - FeeCap: feeCap, - TipCap: gasTipCap, - }, nil -} + bumpedMaxPriorityFeePerGas := originalFee.TipCap.AddPercentage(f.config.BumpPercent()) + bumpedMaxFeePerGas := originalFee.FeeCap.AddPercentage(f.config.BumpPercent()) + + bumpedMaxFeePerGas = assets.WeiMin(bumpedMaxFeePerGas, maxPrice) + bumpedMaxPriorityFeePerGas = assets.WeiMin(bumpedMaxPriorityFeePerGas, maxPrice) -func (f *fixedPriceEstimator) BumpDynamicFee( - _ context.Context, - originalFee DynamicFee, - maxGasPriceWei *assets.Wei, - _ []EvmPriorAttempt, -) (bumped DynamicFee, err error) { - return BumpDynamicFeeOnly( - f.config, - f.bhConfig.EIP1559FeeCapBufferBlocks(), - f.lggr, - f.config.TipCapDefault(), - nil, - originalFee, - maxGasPriceWei, - ) + bumpedFee = DynamicFee{FeeCap: bumpedMaxFeePerGas, TipCap: bumpedMaxPriorityFeePerGas} + return bumpedFee, nil } -func (f *fixedPriceEstimator) L1Oracle() rollups.L1Oracle { +func (f *FixedPriceEstimator) L1Oracle() rollups.L1Oracle { return f.l1Oracle } -func (f *fixedPriceEstimator) Name() string { return f.lggr.Name() } -func (f *fixedPriceEstimator) Ready() error { return nil } -func (f *fixedPriceEstimator) HealthReport() map[string]error { return map[string]error{} } -func (f *fixedPriceEstimator) Close() error { return nil } -func (f *fixedPriceEstimator) OnNewLongestChain(_ context.Context, _ *evmtypes.Head) {} +func (f *FixedPriceEstimator) Name() string { return f.lggr.Name() } +func (f *FixedPriceEstimator) Ready() error { return nil } +func (f *FixedPriceEstimator) HealthReport() map[string]error { return map[string]error{} } +func (f *FixedPriceEstimator) Close() error { return nil } +func (f *FixedPriceEstimator) OnNewLongestChain(_ context.Context, _ *evmtypes.Head) {} diff --git a/core/chains/evm/gas/fixed_price_estimator_test.go b/core/chains/evm/gas/fixed_price_estimator_test.go index 80641ae3cc5..d4627611c87 100644 --- a/core/chains/evm/gas/fixed_price_estimator_test.go +++ b/core/chains/evm/gas/fixed_price_estimator_test.go @@ -11,136 +11,166 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/chains/evm/assets" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas" + gasmocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas/mocks" rollupMocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas/rollups/mocks" ) -type blockHistoryConfig struct { - v uint16 -} - -func (b *blockHistoryConfig) EIP1559FeeCapBufferBlocks() uint16 { - return b.v -} - func Test_FixedPriceEstimator(t *testing.T) { t.Parallel() - maxGasPrice := assets.NewWeiI(1000000) + maxPrice := assets.NewWeiI(1000000) - t.Run("GetLegacyGas returns EvmGasPriceDefault from config", func(t *testing.T) { - config := &gas.MockGasEstimatorConfig{} + t.Run("GetLegacyGas returns PriceDefault from config", func(t *testing.T) { + config := gasmocks.NewFixedPriceEstimatorConfig(t) + config.On("PriceDefault").Return(assets.NewWeiI(42)) l1Oracle := rollupMocks.NewL1Oracle(t) + f := gas.NewFixedPriceEstimator(logger.Test(t), config, l1Oracle) - f := gas.NewFixedPriceEstimator(config, nil, &blockHistoryConfig{}, logger.Test(t), l1Oracle) - - config.PriceDefaultF = assets.NewWeiI(42) - config.PriceMaxF = maxGasPrice - - gasPrice, gasLimit, err := f.GetLegacyGas(tests.Context(t), nil, 100000, maxGasPrice) + gasPrice, _, err := f.GetLegacyGas(tests.Context(t), nil, 100000, maxPrice) require.NoError(t, err) - assert.Equal(t, 100000, int(gasLimit)) assert.Equal(t, assets.NewWeiI(42), gasPrice) }) - t.Run("GetLegacyGas returns user specified maximum gas price", func(t *testing.T) { - config := &gas.MockGasEstimatorConfig{} - config.PriceDefaultF = assets.NewWeiI(42) - config.PriceMaxF = assets.NewWeiI(35) + t.Run("GetLegacyGas returns user specified maximum gas price if default is higher", func(t *testing.T) { + config := gasmocks.NewFixedPriceEstimatorConfig(t) + config.On("PriceDefault").Return(assets.NewWeiI(42)) l1Oracle := rollupMocks.NewL1Oracle(t) + f := gas.NewFixedPriceEstimator(logger.Test(t), config, l1Oracle) - f := gas.NewFixedPriceEstimator(config, nil, &blockHistoryConfig{}, logger.Test(t), l1Oracle) - - gasPrice, gasLimit, err := f.GetLegacyGas(tests.Context(t), nil, 100000, assets.NewWeiI(30)) + maxPrice := assets.NewWeiI(30) + gasPrice, _, err := f.GetLegacyGas(tests.Context(t), nil, 100000, maxPrice) require.NoError(t, err) - assert.Equal(t, 100000, int(gasLimit)) - assert.Equal(t, assets.NewWeiI(30), gasPrice) + assert.Equal(t, maxPrice, gasPrice) }) - t.Run("GetLegacyGas returns global maximum gas price", func(t *testing.T) { - config := &gas.MockGasEstimatorConfig{} - config.PriceDefaultF = assets.NewWeiI(42) - config.PriceMaxF = assets.NewWeiI(20) + t.Run("BumpLegacyGas fails if original gas price is invalid", func(t *testing.T) { + config := gasmocks.NewFixedPriceEstimatorConfig(t) l1Oracle := rollupMocks.NewL1Oracle(t) - - f := gas.NewFixedPriceEstimator(config, nil, &blockHistoryConfig{}, logger.Test(t), l1Oracle) - gasPrice, gasLimit, err := f.GetLegacyGas(tests.Context(t), nil, 100000, assets.NewWeiI(30)) - require.NoError(t, err) - assert.Equal(t, 100000, int(gasLimit)) - assert.Equal(t, assets.NewWeiI(20), gasPrice) + f := gas.NewFixedPriceEstimator(logger.Test(t), config, l1Oracle) + + // original gas price is nil + maxPrice := assets.NewWeiI(30) + var originalGasPrice *assets.Wei + _, _, err := f.BumpLegacyGas(tests.Context(t), originalGasPrice, 100000, maxPrice, nil) + require.Error(t, err) + + // original gas price is higher than max + originalGasPrice = assets.NewWeiI(40) + _, _, err = f.BumpLegacyGas(tests.Context(t), originalGasPrice, 100000, maxPrice, nil) + require.Error(t, err) }) - t.Run("BumpLegacyGas calls BumpLegacyGasPriceOnly", func(t *testing.T) { - config := &gas.MockGasEstimatorConfig{} - config.PriceDefaultF = assets.NewWeiI(42) - config.PriceMaxF = maxGasPrice - config.BumpPercentF = uint16(10) - config.BumpMinF = assets.NewWeiI(150) + t.Run("BumpLegacyGas bumps original gas price by BumpPercent", func(t *testing.T) { + config := gasmocks.NewFixedPriceEstimatorConfig(t) + config.On("BumpPercent").Return(uint16(20)) l1Oracle := rollupMocks.NewL1Oracle(t) + f := gas.NewFixedPriceEstimator(logger.Test(t), config, l1Oracle) - lggr := logger.TestSugared(t) - f := gas.NewFixedPriceEstimator(config, nil, &blockHistoryConfig{}, lggr, l1Oracle) - - gasPrice, gasLimit, err := f.BumpLegacyGas(tests.Context(t), assets.NewWeiI(42), 100000, maxGasPrice, nil) + // original gas price is nil + maxPrice := assets.NewWeiI(100) + originalGasPrice := assets.NewWeiI(40) + bumpedGas, _, err := f.BumpLegacyGas(tests.Context(t), originalGasPrice, 100000, maxPrice, nil) require.NoError(t, err) + assert.Equal(t, originalGasPrice.AddPercentage(20), bumpedGas) - expectedGasPrice, err := gas.BumpLegacyGasPriceOnly(config, lggr, nil, assets.NewWeiI(42), maxGasPrice) - require.NoError(t, err) - - assert.Equal(t, 100000, int(gasLimit)) - assert.Equal(t, expectedGasPrice, gasPrice) }) - t.Run("GetDynamicFee returns defaults from config", func(t *testing.T) { - config := &gas.MockGasEstimatorConfig{} - config.PriceMaxF = maxGasPrice - config.TipCapDefaultF = assets.NewWeiI(52) - config.FeeCapDefaultF = assets.NewWeiI(100) - config.BumpThresholdF = uint64(3) + t.Run("BumpLegacyGas bumps original gas price by BumpPercent but caps on max price", func(t *testing.T) { + config := gasmocks.NewFixedPriceEstimatorConfig(t) + config.On("BumpPercent").Return(uint16(20)) l1Oracle := rollupMocks.NewL1Oracle(t) + f := gas.NewFixedPriceEstimator(logger.Test(t), config, l1Oracle) - lggr := logger.Test(t) - f := gas.NewFixedPriceEstimator(config, nil, &blockHistoryConfig{}, lggr, l1Oracle) - - fee, err := f.GetDynamicFee(tests.Context(t), maxGasPrice) + // original gas price is nil + maxPrice := assets.NewWeiI(41) + originalGasPrice := assets.NewWeiI(40) + bumpedGas, _, err := f.BumpLegacyGas(tests.Context(t), originalGasPrice, 100000, maxPrice, nil) require.NoError(t, err) + assert.Equal(t, maxPrice, bumpedGas) - assert.Equal(t, assets.NewWeiI(52), fee.TipCap) - assert.Equal(t, assets.NewWeiI(100), fee.FeeCap) + }) - // Gas bumping disabled - config.BumpThresholdF = uint64(0) + t.Run("GetDynamicFee returns TipCapDefault and FeeCapDefault from config", func(t *testing.T) { + config := gasmocks.NewFixedPriceEstimatorConfig(t) + config.On("TipCapDefault").Return(assets.NewWeiI(10)) + config.On("FeeCapDefault").Return(assets.NewWeiI(20)) + l1Oracle := rollupMocks.NewL1Oracle(t) + f := gas.NewFixedPriceEstimator(logger.Test(t), config, l1Oracle) - fee, err = f.GetDynamicFee(tests.Context(t), maxGasPrice) + dynamicFee, err := f.GetDynamicFee(tests.Context(t), maxPrice) require.NoError(t, err) + assert.Equal(t, assets.NewWeiI(10), dynamicFee.TipCap) + assert.Equal(t, assets.NewWeiI(20), dynamicFee.FeeCap) + }) - assert.Equal(t, assets.NewWeiI(52), fee.TipCap) - assert.Equal(t, maxGasPrice, fee.FeeCap) + t.Run("GetDynamicFee returns user specified maximum price if defaults are higher", func(t *testing.T) { + config := gasmocks.NewFixedPriceEstimatorConfig(t) + config.On("TipCapDefault").Return(assets.NewWeiI(10)) + config.On("FeeCapDefault").Return(assets.NewWeiI(20)) + l1Oracle := rollupMocks.NewL1Oracle(t) + f := gas.NewFixedPriceEstimator(logger.Test(t), config, l1Oracle) - // override max gas price - fee, err = f.GetDynamicFee(tests.Context(t), assets.NewWeiI(10)) - require.NoError(t, err) + maxPrice := assets.NewWeiI(8) - assert.Equal(t, assets.NewWeiI(52), fee.TipCap) - assert.Equal(t, assets.NewWeiI(10), fee.FeeCap) + dynamicFee, err := f.GetDynamicFee(tests.Context(t), maxPrice) + require.NoError(t, err) + assert.Equal(t, maxPrice, dynamicFee.TipCap) + assert.Equal(t, maxPrice, dynamicFee.FeeCap) }) - t.Run("BumpDynamicFee calls BumpDynamicFeeOnly", func(t *testing.T) { - config := &gas.MockGasEstimatorConfig{} - config.PriceMaxF = maxGasPrice - config.TipCapDefaultF = assets.NewWeiI(52) - config.BumpMinF = assets.NewWeiI(150) - config.BumpPercentF = uint16(10) + t.Run("BumpDynamicFee fails if original fee is invalid", func(t *testing.T) { + config := gasmocks.NewFixedPriceEstimatorConfig(t) l1Oracle := rollupMocks.NewL1Oracle(t) + f := gas.NewFixedPriceEstimator(logger.Test(t), config, l1Oracle) + + maxPrice := assets.NewWeiI(8) + + // original fee is nil + var dynamicFee gas.DynamicFee + _, err := f.BumpDynamicFee(tests.Context(t), dynamicFee, maxPrice, nil) + require.Error(t, err) + + // TipCap is higher than FeeCap + dynamicFee.FeeCap = assets.NewWeiI(10) + dynamicFee.TipCap = assets.NewWeiI(11) + _, err = f.BumpDynamicFee(tests.Context(t), dynamicFee, maxPrice, nil) + require.Error(t, err) + + // FeeCap is higher than max price + dynamicFee.FeeCap = assets.NewWeiI(10) + dynamicFee.TipCap = assets.NewWeiI(8) + _, err = f.BumpDynamicFee(tests.Context(t), dynamicFee, maxPrice, nil) + require.Error(t, err) + }) - lggr := logger.TestSugared(t) - f := gas.NewFixedPriceEstimator(config, nil, &blockHistoryConfig{}, lggr, l1Oracle) + t.Run("BumpDynamicFee bumps original fee by BumpPercent", func(t *testing.T) { + config := gasmocks.NewFixedPriceEstimatorConfig(t) + config.On("BumpPercent").Return(uint16(20)) + l1Oracle := rollupMocks.NewL1Oracle(t) + f := gas.NewFixedPriceEstimator(logger.Test(t), config, l1Oracle) - originalFee := gas.DynamicFee{FeeCap: assets.NewWeiI(100), TipCap: assets.NewWeiI(25)} - fee, err := f.BumpDynamicFee(tests.Context(t), originalFee, maxGasPrice, nil) + maxPrice := assets.NewWeiI(100) + feeCap := assets.NewWeiI(20) + tipCap := assets.NewWeiI(10) + dynamicFee := gas.DynamicFee{FeeCap: feeCap, TipCap: tipCap} + bumpedFee, err := f.BumpDynamicFee(tests.Context(t), dynamicFee, maxPrice, nil) require.NoError(t, err) + assert.Equal(t, feeCap.AddPercentage(20), bumpedFee.FeeCap) + assert.Equal(t, tipCap.AddPercentage(20), bumpedFee.TipCap) + }) - expectedFee, err := gas.BumpDynamicFeeOnly(config, 0, lggr, nil, nil, originalFee, maxGasPrice) - require.NoError(t, err) + t.Run("BumpDynamicFee bumps original fee by BumpPercent but caps on max price", func(t *testing.T) { + config := gasmocks.NewFixedPriceEstimatorConfig(t) + config.On("BumpPercent").Return(uint16(20)) + l1Oracle := rollupMocks.NewL1Oracle(t) + f := gas.NewFixedPriceEstimator(logger.Test(t), config, l1Oracle) - assert.Equal(t, expectedFee, fee) + maxPrice := assets.NewWeiI(22) + feeCap := assets.NewWeiI(20) + tipCap := assets.NewWeiI(19) + dynamicFee := gas.DynamicFee{FeeCap: feeCap, TipCap: tipCap} + bumpedFee, err := f.BumpDynamicFee(tests.Context(t), dynamicFee, maxPrice, nil) + require.NoError(t, err) + assert.Equal(t, maxPrice, bumpedFee.FeeCap) + assert.Equal(t, maxPrice, bumpedFee.TipCap) }) } diff --git a/core/chains/evm/gas/mocks/fixed_price_estimator_config.go b/core/chains/evm/gas/mocks/fixed_price_estimator_config.go new file mode 100644 index 00000000000..addf0ce0424 --- /dev/null +++ b/core/chains/evm/gas/mocks/fixed_price_estimator_config.go @@ -0,0 +1,106 @@ +// Code generated by mockery v2.43.2. DO NOT EDIT. + +package mocks + +import ( + assets "github.com/smartcontractkit/chainlink/v2/core/chains/evm/assets" + + mock "github.com/stretchr/testify/mock" +) + +// FixedPriceEstimatorConfig is an autogenerated mock type for the fixedPriceEstimatorConfig type +type FixedPriceEstimatorConfig struct { + mock.Mock +} + +// BumpPercent provides a mock function with given fields: +func (_m *FixedPriceEstimatorConfig) BumpPercent() uint16 { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for BumpPercent") + } + + var r0 uint16 + if rf, ok := ret.Get(0).(func() uint16); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(uint16) + } + + return r0 +} + +// FeeCapDefault provides a mock function with given fields: +func (_m *FixedPriceEstimatorConfig) FeeCapDefault() *assets.Wei { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for FeeCapDefault") + } + + var r0 *assets.Wei + if rf, ok := ret.Get(0).(func() *assets.Wei); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*assets.Wei) + } + } + + return r0 +} + +// PriceDefault provides a mock function with given fields: +func (_m *FixedPriceEstimatorConfig) PriceDefault() *assets.Wei { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for PriceDefault") + } + + var r0 *assets.Wei + if rf, ok := ret.Get(0).(func() *assets.Wei); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*assets.Wei) + } + } + + return r0 +} + +// TipCapDefault provides a mock function with given fields: +func (_m *FixedPriceEstimatorConfig) TipCapDefault() *assets.Wei { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for TipCapDefault") + } + + var r0 *assets.Wei + if rf, ok := ret.Get(0).(func() *assets.Wei); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*assets.Wei) + } + } + + return r0 +} + +// NewFixedPriceEstimatorConfig creates a new instance of FixedPriceEstimatorConfig. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewFixedPriceEstimatorConfig(t interface { + mock.TestingT + Cleanup(func()) +}) *FixedPriceEstimatorConfig { + mock := &FixedPriceEstimatorConfig{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/core/chains/evm/gas/models.go b/core/chains/evm/gas/models.go index 8c1d19280ae..c0f31a4ee68 100644 --- a/core/chains/evm/gas/models.go +++ b/core/chains/evm/gas/models.go @@ -13,7 +13,6 @@ import ( "github.com/smartcontractkit/chainlink-common/pkg/logger" "github.com/smartcontractkit/chainlink-common/pkg/services" - bigmath "github.com/smartcontractkit/chainlink-common/pkg/utils/big_math" commonfee "github.com/smartcontractkit/chainlink/v2/common/fee" feetypes "github.com/smartcontractkit/chainlink/v2/common/fee/types" @@ -101,7 +100,7 @@ func NewEstimator(lggr logger.Logger, ethClient feeEstimatorClient, cfg Config, } case "FixedPrice": newEstimator = func(l logger.Logger) EvmEstimator { - return NewFixedPriceEstimator(geCfg, ethClient, bh, lggr, l1Oracle) + return NewFixedPriceEstimator(lggr, geCfg, l1Oracle) } case "L2Suggested", "SuggestedPrice": newEstimator = func(l logger.Logger) EvmEstimator { @@ -110,7 +109,7 @@ func NewEstimator(lggr logger.Logger, ethClient feeEstimatorClient, cfg Config, default: lggr.Warnf("GasEstimator: unrecognised mode '%s', falling back to FixedPriceEstimator", s) newEstimator = func(l logger.Logger) EvmEstimator { - return NewFixedPriceEstimator(geCfg, ethClient, bh, lggr, l1Oracle) + return NewFixedPriceEstimator(lggr, geCfg, l1Oracle) } } return NewEvmFeeEstimator(lggr, newEstimator, df, geCfg), nil @@ -398,7 +397,7 @@ func HexToInt64(input interface{}) int64 { } // BumpLegacyGasPriceOnly will increase the price -func BumpLegacyGasPriceOnly(cfg bumpConfig, lggr logger.SugaredLogger, currentGasPrice, originalGasPrice *assets.Wei, maxGasPriceWei *assets.Wei) (gasPrice *assets.Wei, err error) { +func BumpLegacyGasPriceOnly(cfg estimatorGasEstimatorConfig, lggr logger.SugaredLogger, currentGasPrice, originalGasPrice *assets.Wei, maxGasPriceWei *assets.Wei) (gasPrice *assets.Wei, err error) { gasPrice, err = bumpGasPrice(cfg, lggr, currentGasPrice, originalGasPrice, maxGasPriceWei) if err != nil { return nil, err @@ -410,8 +409,8 @@ func BumpLegacyGasPriceOnly(cfg bumpConfig, lggr logger.SugaredLogger, currentGa // - A configured percentage bump (EVM.GasEstimator.BumpPercent) on top of the baseline price. // - A configured fixed amount of Wei (ETH_GAS_PRICE_WEI) on top of the baseline price. // The baseline price is the maximum of the previous gas price attempt and the node's current gas price. -func bumpGasPrice(cfg bumpConfig, lggr logger.SugaredLogger, currentGasPrice, originalGasPrice, maxGasPriceWei *assets.Wei) (*assets.Wei, error) { - maxGasPrice := getMaxGasPrice(maxGasPriceWei, cfg.PriceMax()) +func bumpGasPrice(cfg estimatorGasEstimatorConfig, lggr logger.SugaredLogger, currentGasPrice, originalGasPrice, maxGasPriceWei *assets.Wei) (*assets.Wei, error) { + maxGasPrice := assets.WeiMin(maxGasPriceWei, cfg.PriceMax()) bumpedGasPrice := bumpFeePrice(originalGasPrice, cfg.BumpPercent(), cfg.BumpMin()) // Update bumpedGasPrice if currentGasPrice is higher than bumpedGasPrice and within maxGasPrice @@ -432,7 +431,7 @@ func bumpGasPrice(cfg bumpConfig, lggr logger.SugaredLogger, currentGasPrice, or } // BumpDynamicFeeOnly bumps the tip cap and max gas price if necessary -func BumpDynamicFeeOnly(config bumpConfig, feeCapBufferBlocks uint16, lggr logger.SugaredLogger, currentTipCap, currentBaseFee *assets.Wei, originalFee DynamicFee, maxGasPriceWei *assets.Wei) (bumped DynamicFee, err error) { +func BumpDynamicFeeOnly(config estimatorGasEstimatorConfig, feeCapBufferBlocks uint16, lggr logger.SugaredLogger, currentTipCap, currentBaseFee *assets.Wei, originalFee DynamicFee, maxGasPriceWei *assets.Wei) (bumped DynamicFee, err error) { bumped, err = bumpDynamicFee(config, feeCapBufferBlocks, lggr, currentTipCap, currentBaseFee, originalFee, maxGasPriceWei) if err != nil { return bumped, err @@ -450,9 +449,9 @@ func BumpDynamicFeeOnly(config bumpConfig, feeCapBufferBlocks uint16, lggr logge // the Tip only. Unfortunately due to a flaw of how EIP-1559 is implemented we // have to bump FeeCap by at least 10% each time we bump the tip cap. // See: https://github.com/ethereum/go-ethereum/issues/24284 -func bumpDynamicFee(cfg bumpConfig, feeCapBufferBlocks uint16, lggr logger.SugaredLogger, currentTipCap, currentBaseFee *assets.Wei, originalFee DynamicFee, maxGasPriceWei *assets.Wei) (bumpedFee DynamicFee, err error) { - maxGasPrice := getMaxGasPrice(maxGasPriceWei, cfg.PriceMax()) - baselineTipCap := assets.MaxWei(originalFee.TipCap, cfg.TipCapDefault()) +func bumpDynamicFee(cfg estimatorGasEstimatorConfig, feeCapBufferBlocks uint16, lggr logger.SugaredLogger, currentTipCap, currentBaseFee *assets.Wei, originalFee DynamicFee, maxGasPriceWei *assets.Wei) (bumpedFee DynamicFee, err error) { + maxGasPrice := assets.WeiMin(maxGasPriceWei, cfg.PriceMax()) + baselineTipCap := assets.WeiMax(originalFee.TipCap, cfg.TipCapDefault()) bumpedTipCap := bumpFeePrice(baselineTipCap, cfg.BumpPercent(), cfg.BumpMin()) // Update bumpedTipCap if currentTipCap is higher than bumpedTipCap and within maxGasPrice @@ -493,7 +492,7 @@ func bumpDynamicFee(cfg bumpConfig, feeCapBufferBlocks uint16, lggr logger.Sugar } func bumpFeePrice(originalFeePrice *assets.Wei, feeBumpPercent uint16, feeBumpUnits *assets.Wei) *assets.Wei { - bumpedFeePrice := assets.MaxWei( + bumpedFeePrice := assets.WeiMax( originalFeePrice.AddPercentage(feeBumpPercent), originalFeePrice.Add(feeBumpUnits), ) @@ -514,10 +513,6 @@ func maxBumpedFee(lggr logger.SugaredLogger, currentFeePrice, bumpedFeePrice, ma return bumpedFeePrice } -func getMaxGasPrice(userSpecifiedMax, maxGasPriceWei *assets.Wei) *assets.Wei { - return assets.NewWei(bigmath.Min(userSpecifiedMax.ToInt(), maxGasPriceWei.ToInt())) -} - func capGasPrice(calculatedGasPrice, userSpecifiedMax, maxGasPriceWei *assets.Wei) *assets.Wei { maxGasPrice := commonfee.CalculateFee(calculatedGasPrice.ToInt(), userSpecifiedMax.ToInt(), maxGasPriceWei.ToInt()) return assets.NewWei(maxGasPrice) diff --git a/core/chains/evm/txmgr/broadcaster_test.go b/core/chains/evm/txmgr/broadcaster_test.go index 3559c329dee..784679fe363 100644 --- a/core/chains/evm/txmgr/broadcaster_test.go +++ b/core/chains/evm/txmgr/broadcaster_test.go @@ -67,7 +67,7 @@ func NewTestEthBroadcaster( ge := config.EVM().GasEstimator() estimator := gas.NewEvmFeeEstimator(lggr, func(lggr logger.Logger) gas.EvmEstimator { - return gas.NewFixedPriceEstimator(config.EVM().GasEstimator(), nil, ge.BlockHistory(), lggr, nil) + return gas.NewFixedPriceEstimator(lggr, config.EVM().GasEstimator(), nil) }, ge.EIP1559DynamicFees(), ge) txBuilder := txmgr.NewEvmTxAttemptBuilder(*ethClient.ConfiguredChainID(), ge, keyStore, estimator) ethBroadcaster := txmgrcommon.NewBroadcaster(txStore, txmgr.NewEvmTxmClient(ethClient, nil), txmgr.NewEvmTxmConfig(config.EVM()), txmgr.NewEvmTxmFeeConfig(config.EVM().GasEstimator()), config.EVM().Transactions(), gconfig.Database().Listener(), keyStore, txBuilder, nonceTracker, lggr, checkerFactory, nonceAutoSync) @@ -1153,7 +1153,7 @@ func TestEthBroadcaster_ProcessUnstartedEthTxs_Errors(t *testing.T) { // same as the parent test, but callback is set by ctor t.Run("callback set by ctor", func(t *testing.T) { estimator := gas.NewEvmFeeEstimator(lggr, func(lggr logger.Logger) gas.EvmEstimator { - return gas.NewFixedPriceEstimator(evmcfg.EVM().GasEstimator(), nil, evmcfg.EVM().GasEstimator().BlockHistory(), lggr, nil) + return gas.NewFixedPriceEstimator(lggr, evmcfg.EVM().GasEstimator(), nil) }, evmcfg.EVM().GasEstimator().EIP1559DynamicFees(), evmcfg.EVM().GasEstimator()) txBuilder := txmgr.NewEvmTxAttemptBuilder(*ethClient.ConfiguredChainID(), evmcfg.EVM().GasEstimator(), ethKeyStore, estimator) localNextNonce = getLocalNextNonce(t, nonceTracker, fromAddress) @@ -1738,7 +1738,7 @@ func TestEthBroadcaster_SyncNonce(t *testing.T) { _, fromAddress := cltest.RandomKey{Disabled: false}.MustInsertWithState(t, kst) estimator := gas.NewEvmFeeEstimator(lggr, func(lggr logger.Logger) gas.EvmEstimator { - return gas.NewFixedPriceEstimator(evmcfg.EVM().GasEstimator(), nil, evmcfg.EVM().GasEstimator().BlockHistory(), lggr, nil) + return gas.NewFixedPriceEstimator(lggr, evmcfg.EVM().GasEstimator(), nil) }, evmcfg.EVM().GasEstimator().EIP1559DynamicFees(), evmcfg.EVM().GasEstimator()) checkerFactory := &testCheckerFactory{} diff --git a/core/chains/evm/txmgr/confirmer_test.go b/core/chains/evm/txmgr/confirmer_test.go index 6b107b222a6..6c7837571e5 100644 --- a/core/chains/evm/txmgr/confirmer_test.go +++ b/core/chains/evm/txmgr/confirmer_test.go @@ -3233,7 +3233,7 @@ func newEthConfirmer(t testing.TB, txStore txmgr.EvmTxStore, ethClient client.Cl lggr := logger.Test(t) ge := config.EVM().GasEstimator() estimator := gas.NewEvmFeeEstimator(lggr, func(lggr logger.Logger) gas.EvmEstimator { - return gas.NewFixedPriceEstimator(ge, nil, ge.BlockHistory(), lggr, nil) + return gas.NewFixedPriceEstimator(lggr, ge, nil) }, ge.EIP1559DynamicFees(), ge) txBuilder := txmgr.NewEvmTxAttemptBuilder(*ethClient.ConfiguredChainID(), ge, ks, estimator) stuckTxDetector := txmgr.NewStuckTxDetector(lggr, testutils.FixtureChainID, "", assets.NewWei(assets.NewEth(100).ToInt()), config.EVM().Transactions().AutoPurge(), estimator, txStore, ethClient) From f750b0761bf8576169270f28f8dfd16fa082697d Mon Sep 17 00:00:00 2001 From: Dimitris Date: Fri, 5 Jul 2024 17:52:32 +0300 Subject: [PATCH 03/11] Fixes --- core/chains/evm/gas/universal_estimator.go | 78 ++++++++++++++----- .../evm/gas/universal_estimator_test.go | 48 ++++++------ 2 files changed, 84 insertions(+), 42 deletions(-) diff --git a/core/chains/evm/gas/universal_estimator.go b/core/chains/evm/gas/universal_estimator.go index b6dfc028d61..737195bfc67 100644 --- a/core/chains/evm/gas/universal_estimator.go +++ b/core/chains/evm/gas/universal_estimator.go @@ -10,6 +10,8 @@ import ( "time" "github.com/ethereum/go-ethereum" + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" "github.com/smartcontractkit/chainlink-common/pkg/logger" "github.com/smartcontractkit/chainlink-common/pkg/services" @@ -21,6 +23,34 @@ import ( evmtypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types" ) +// metrics are thread safe +var ( + promUniversalEstimatorGasPrice = promauto.NewGaugeVec(prometheus.GaugeOpts{ + Name: "gas_price_updater", + Help: "Sets latest gas price (in Wei)", + }, + []string{"evmChainID"}, + ) + promUniversalEstimatorBaseFee = promauto.NewGaugeVec(prometheus.GaugeOpts{ + Name: "base_fee_updater", + Help: "Sets latest BaseFee (in Wei)", + }, + []string{"evmChainID"}, + ) + promUniversalEstimatorMaxPriorityFeePerGas = promauto.NewGaugeVec(prometheus.GaugeOpts{ + Name: "max_priority_fee_per_gas_updater", + Help: "Sets latest MaxPriorityFeePerGas (in Wei)", + }, + []string{"evmChainID"}, + ) + promUniversalEstimatorMaxFeePerGas = promauto.NewGaugeVec(prometheus.GaugeOpts{ + Name: "max_fee_per_gas_updater", + Help: "Sets latest MaxFeePerGas (in Wei)", + }, + []string{"evmChainID"}, + ) +) + const ( queryTimeout = 10 * time.Second minimumBumpPercentage = 10 // based on geth's spec @@ -46,9 +76,10 @@ type universalEstimatorClient interface { type UniversalEstimator struct { services.StateMachine - client universalEstimatorClient - logger logger.Logger - config UniversalEstimatorConfig + client universalEstimatorClient + logger logger.Logger + config UniversalEstimatorConfig + chainID *big.Int gasPriceMu sync.RWMutex gasPrice *assets.Wei @@ -64,11 +95,12 @@ type UniversalEstimator struct { l1Oracle rollups.L1Oracle } -func NewUniversalEstimator(lggr logger.Logger, client universalEstimatorClient, cfg UniversalEstimatorConfig, l1Oracle rollups.L1Oracle) EvmEstimator { +func NewUniversalEstimator(lggr logger.Logger, client universalEstimatorClient, cfg UniversalEstimatorConfig, chainID *big.Int, l1Oracle rollups.L1Oracle) EvmEstimator { return &UniversalEstimator{ client: client, logger: logger.Named(lggr, "UniversalEstimator"), config: cfg, + chainID: chainID, l1Oracle: l1Oracle, } } @@ -84,7 +116,7 @@ func (u *UniversalEstimator) Start(context.Context) error { strconv.FormatUint(uint64(u.config.RewardPercentile), 10), strconv.Itoa(ConnectivityPercentile)) } if u.config.BlockHistoryRange == 0 { - u.logger.Warnf("BlockHistoryRange: %s is greater than maximum allowed connectivity percentage: %s. Lower reward percentile percentage otherwise connectivity checks will fail!", + u.logger.Warnf("BlockHistoryRange is set to 0. Using dynamic transactions will result in an error!", strconv.FormatUint(uint64(u.config.RewardPercentile), 10), strconv.Itoa(ConnectivityPercentile)) } return nil @@ -123,13 +155,15 @@ func (u *UniversalEstimator) fetchGasPrice(parentCtx context.Context, forceRefet return nil, fmt.Errorf("failed to fetch gas price: %s", err) } - bi := (*assets.Wei)(gasPrice) + promUniversalEstimatorGasPrice.WithLabelValues(u.chainID.String()).Set(float64(gasPrice.Int64())) - u.logger.Debugf("fetched new gas price: %v", bi) + gasPriceWei := (*assets.Wei)(gasPrice) + + u.logger.Debugf("fetched new gas price: %v", gasPriceWei) u.gasPriceMu.Lock() defer u.gasPriceMu.Unlock() - u.gasPrice = bi + u.gasPrice = gasPriceWei u.gasPriceLastUpdate = time.Now() return u.gasPrice, nil } @@ -145,8 +179,8 @@ func (u *UniversalEstimator) getGasPrice() (*assets.Wei, error) { // GetDynamicFee will utilize eth_feeHistory to estimate an accurate maxFeePerGas and maxPriorityFeePerGas. // It also has a mechanism to store the highest Nth percentile maxPriorityFeePerGas value of the latest X blocks, -// to prevent excessive bumping during connectivity incidents. -// It returns cached value if the prices were recently changed. Caching can be skipped. +// to prevent excessive gas bumping during connectivity incidents. +// It returns cached values if the prices were recently changed. Caching can be skipped. func (u *UniversalEstimator) GetDynamicFee(ctx context.Context, maxPrice *assets.Wei) (fee DynamicFee, err error) { if fee, err = u.fetchDynamicPrice(ctx, false); err != nil { return @@ -156,12 +190,12 @@ func (u *UniversalEstimator) GetDynamicFee(ctx context.Context, maxPrice *assets return fee, fmt.Errorf("dynamic price not set") } if fee.FeeCap.Cmp(maxPrice) > 0 { - u.logger.Warnf("estimated maxFeePerGas: %s is greater than the maximum price configured: %s, returning the maximum price instead.", + u.logger.Warnf("estimated maxFeePerGas: %v is greater than the maximum price configured: %v, returning the maximum price instead.", fee.FeeCap, maxPrice) fee.FeeCap = maxPrice if fee.TipCap.Cmp(maxPrice) > 0 { - u.logger.Warnf("estimated maxPriorityFeePerGas: %s is greater than the maximum price configured: %s, returning the maximum price instead. There won't be any room for base fee!", - fee.FeeCap, maxPrice) + u.logger.Warnf("estimated maxPriorityFeePerGas: %v is greater than the maximum price configured: %v, returning the maximum price instead. There won't be any room for base fee!", + fee.TipCap, maxPrice) fee.TipCap = maxPrice } } @@ -172,7 +206,7 @@ func (u *UniversalEstimator) GetDynamicFee(ctx context.Context, maxPrice *assets // fetchDynamicPrice uses eth_feeHistory to fetch the basFee of the latest block and the Nth maxPriorityFeePerGas percentiles // of the past X blocks. It also fetches the highest Zth maxPriorityFeePerGas percentile of the past X blocks. Z is configurable // and it represents the highest percentile we're willing to pay. -// A buffer is added on top of the latest basFee to catch fluctuations in the next blocks. On Ethereum the increase is baseFee*1.125 per block +// A buffer is added on top of the latest basFee to catch fluctuations in the next blocks. On Ethereum the increase is baseFee*1.125 per block. func (u *UniversalEstimator) fetchDynamicPrice(parentCtx context.Context, forceRefetch bool) (fee DynamicFee, err error) { if !u.checkIfStale(true) && !forceRefetch { return u.getDynamicPrice() @@ -208,8 +242,12 @@ func (u *UniversalEstimator) fetchDynamicPrice(parentCtx context.Context, forceR // baseFeeBufferPercentage is used as a safety to catch fluctuations in the next block. maxFeePerGas := baseFee.AddPercentage(BaseFeeBufferPercentage).Add((maxPriorityFeePerGas)) + promUniversalEstimatorBaseFee.WithLabelValues(u.chainID.String()).Set(float64(baseFee.Int64())) + promUniversalEstimatorMaxPriorityFeePerGas.WithLabelValues(u.chainID.String()).Set(float64(maxPriorityFeePerGas.Int64())) + promUniversalEstimatorMaxFeePerGas.WithLabelValues(u.chainID.String()).Set(float64(maxFeePerGas.Int64())) + u.logger.Debugf("fetched new dynamic prices, maxFeePerGas: %v - maxPriorityFeePerGas: %v - maxPriorityFeeThreshold: %v", - maxFeePerGas, maxPriorityFeePerGas, priorityFeeThreshold) + maxFeePerGas, maxPriorityFeePerGas, (*assets.Wei)(priorityFeeThreshold)) u.dynamicPriceMu.Lock() defer u.dynamicPriceMu.Unlock() @@ -240,7 +278,7 @@ func (u *UniversalEstimator) checkIfStale(dynamic bool) bool { return time.Since(u.gasPriceLastUpdate) >= u.config.CacheTimeout } -// BumpLegacyGas provides a bumped gas price value by bumping a previous one by BumpPercent. It refreshes the market gas price by making a call to the RPC +// BumpLegacyGas provides a bumped gas price value by bumping the previous one by BumpPercent. It refreshes the market gas price by making a call to the RPC // in case it has gone stale. If the original value is higher than the max price it returns an error as there is no room for bumping. // It aggregates the market, bumped, and max gas price to provide a correct value. func (u *UniversalEstimator) BumpLegacyGas(ctx context.Context, originalGasPrice *assets.Wei, gasLimit uint64, maxPrice *assets.Wei, _ []EvmPriorAttempt) (*assets.Wei, uint64, error) { @@ -266,7 +304,7 @@ func (u *UniversalEstimator) BumpLegacyGas(ctx context.Context, originalGasPrice return bumpedGasPrice, gasLimit, nil } -// BumpDynamicFee provides a bumped dynamic fee by bumping a previous one by BumpPercent. It refreshes the market prices by making a call to the RPC +// BumpDynamicFee provides a bumped dynamic fee by bumping the previous one by BumpPercent. It refreshes the market prices by making a call to the RPC // in case they have gone stale. If the original values are higher than the max price it returns an error as there is no room for bumping. Both maxFeePerGas // as well as maxPriorityFerPergas need to be bumped otherwise the RPC won't accept the transaction and throw an error. // See: https://github.com/ethereum/go-ethereum/issues/24284 @@ -316,11 +354,11 @@ func (u *UniversalEstimator) BumpDynamicFee(ctx context.Context, originalFee Dyn } // limitBumpedFee selects the maximum value between the original fee and the bumped attempt. If the result is higher than the max price it gets capped. -// Geth's implementation has a hard 10% minimum limit of the bumped values, otherwise it rejects the transaction with an error. +// Geth's implementation has a hard 10% minimum limit for the bumped values, otherwise it rejects the transaction with an error. // See: https://github.com/ethereum/go-ethereum/blob/bff330335b94af3643ac2fb809793f77de3069d4/core/tx_list.go#L298 // -// Note: for chains that support EIP-1559 but we still choose to send Legacy transactions, the limit is still enforcable due to the fact that Legacy transactions -// are treated the same way as Dynamic transactions. For chains that don't support EIP-1559 at all, the limit isn't enforcable but a 10% minimum bump percentage +// Note: for chains that support EIP-1559 but we still choose to send Legacy transactions to them, the limit is still enforcable due to the fact that Legacy transactions +// are treated the same way as Dynamic transactions under the hood. For chains that don't support EIP-1559 at all, the limit isn't enforcable but a 10% minimum bump percentage // makes sense anyway. func (u *UniversalEstimator) limitBumpedFee(originalFee *assets.Wei, currentFee *assets.Wei, bufferedFee *assets.Wei, maxPrice *assets.Wei) (*assets.Wei, error) { bumpedFee := assets.WeiMax(currentFee, bufferedFee) diff --git a/core/chains/evm/gas/universal_estimator_test.go b/core/chains/evm/gas/universal_estimator_test.go index a9c1da9321a..2829a59edb7 100644 --- a/core/chains/evm/gas/universal_estimator_test.go +++ b/core/chains/evm/gas/universal_estimator_test.go @@ -23,6 +23,7 @@ func TestUniversalEstimatorGetLegacyGas(t *testing.T) { var gasLimit uint64 = 21000 maxPrice := assets.NewWeiI(100) + chainID := big.NewInt(0) t.Run("fetches a new gas price when first called", func(t *testing.T) { client := mocks.NewUniversalEstimatorClient(t) @@ -30,7 +31,7 @@ func TestUniversalEstimatorGetLegacyGas(t *testing.T) { cfg := gas.UniversalEstimatorConfig{} - u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, chainID, nil) gasPrice, _, err := u.GetLegacyGas(tests.Context(t), nil, gasLimit, maxPrice) assert.NoError(t, err) assert.Equal(t, assets.NewWeiI(10), gasPrice) @@ -42,7 +43,7 @@ func TestUniversalEstimatorGetLegacyGas(t *testing.T) { cfg := gas.UniversalEstimatorConfig{CacheTimeout: 4 * time.Hour} - u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, chainID, nil) gas1, _, err := u.GetLegacyGas(tests.Context(t), nil, gasLimit, maxPrice) assert.NoError(t, err) assert.Equal(t, assets.NewWeiI(10), gas1) @@ -59,7 +60,7 @@ func TestUniversalEstimatorGetLegacyGas(t *testing.T) { cfg := gas.UniversalEstimatorConfig{CacheTimeout: 0 * time.Second} - u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, chainID, nil) gas1, _, err := u.GetLegacyGas(tests.Context(t), nil, gasLimit, maxPrice) assert.NoError(t, err) assert.Equal(t, assets.NewWeiI(10), gas1) @@ -76,7 +77,7 @@ func TestUniversalEstimatorGetLegacyGas(t *testing.T) { cfg := gas.UniversalEstimatorConfig{CacheTimeout: 4 * time.Hour} - u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, chainID, nil) gas1, _, err := u.GetLegacyGas(tests.Context(t), nil, gasLimit, maxPrice) assert.NoError(t, err) assert.Equal(t, assets.NewWeiI(10), gas1) @@ -93,7 +94,7 @@ func TestUniversalEstimatorGetLegacyGas(t *testing.T) { cfg := gas.UniversalEstimatorConfig{} maxPrice := assets.NewWeiI(1) - u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, chainID, nil) gas1, _, err := u.GetLegacyGas(tests.Context(t), nil, gasLimit, maxPrice) assert.NoError(t, err) assert.Equal(t, maxPrice, gas1) @@ -105,6 +106,7 @@ func TestUniversalEstimatorBumpLegacyGas(t *testing.T) { var gasLimit uint64 = 21000 maxPrice := assets.NewWeiI(100) + chainID := big.NewInt(0) t.Run("bumps a previous attempt by BumpPercent", func(t *testing.T) { client := mocks.NewUniversalEstimatorClient(t) @@ -113,7 +115,7 @@ func TestUniversalEstimatorBumpLegacyGas(t *testing.T) { cfg := gas.UniversalEstimatorConfig{BumpPercent: 50} - u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, chainID, nil) gasPrice, _, err := u.BumpLegacyGas(tests.Context(t), originalGasPrice, gasLimit, maxPrice, nil) assert.NoError(t, err) assert.Equal(t, assets.NewWeiI(15), gasPrice) @@ -123,7 +125,7 @@ func TestUniversalEstimatorBumpLegacyGas(t *testing.T) { client := mocks.NewUniversalEstimatorClient(t) cfg := gas.UniversalEstimatorConfig{} - u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, chainID, nil) var originalPrice *assets.Wei _, _, err := u.BumpLegacyGas(tests.Context(t), originalPrice, gasLimit, maxPrice, nil) @@ -142,7 +144,7 @@ func TestUniversalEstimatorBumpLegacyGas(t *testing.T) { cfg := gas.UniversalEstimatorConfig{} - u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, chainID, nil) gas, _, err := u.BumpLegacyGas(tests.Context(t), originalGasPrice, gasLimit, maxPrice, nil) assert.NoError(t, err) assert.Equal(t, assets.NewWeiI(80), gas) @@ -156,7 +158,7 @@ func TestUniversalEstimatorBumpLegacyGas(t *testing.T) { cfg := gas.UniversalEstimatorConfig{BumpPercent: 50} maxPrice := assets.NewWeiI(14) - u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, chainID, nil) gas, _, err := u.BumpLegacyGas(tests.Context(t), originalGasPrice, gasLimit, maxPrice, nil) assert.NoError(t, err) assert.Equal(t, maxPrice, gas) @@ -170,7 +172,7 @@ func TestUniversalEstimatorBumpLegacyGas(t *testing.T) { cfg := gas.UniversalEstimatorConfig{BumpPercent: 50} maxPrice := assets.NewWeiI(14) - u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, chainID, nil) gas, _, err := u.BumpLegacyGas(tests.Context(t), originalGasPrice, gasLimit, maxPrice, nil) assert.NoError(t, err) assert.Equal(t, maxPrice, gas) @@ -185,7 +187,7 @@ func TestUniversalEstimatorBumpLegacyGas(t *testing.T) { // Price will be capped by the max price maxPrice := assets.NewWeiI(101) - u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, chainID, nil) _, _, err := u.BumpLegacyGas(tests.Context(t), originalGasPrice, gasLimit, maxPrice, nil) assert.Error(t, err) }) @@ -195,6 +197,7 @@ func TestUniversalEstimatorGetDynamicFee(t *testing.T) { t.Parallel() maxPrice := assets.NewWeiI(100) + chainID := big.NewInt(0) t.Run("fetches a new dynamic fee when first called", func(t *testing.T) { client := mocks.NewUniversalEstimatorClient(t) @@ -216,7 +219,7 @@ func TestUniversalEstimatorGetDynamicFee(t *testing.T) { avrgPriorityFee.Add(maxPriorityFeePerGas1, maxPriorityFeePerGas2).Div(avrgPriorityFee, big.NewInt(int64(blockHistoryLength))) maxFee := (*assets.Wei)(baseFee).AddPercentage(gas.BaseFeeBufferPercentage).Add((*assets.Wei)(avrgPriorityFee)) - u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, chainID, nil) dynamicFee, err := u.GetDynamicFee(tests.Context(t), maxPrice) assert.NoError(t, err) assert.Equal(t, maxFee, dynamicFee.FeeCap) @@ -228,7 +231,7 @@ func TestUniversalEstimatorGetDynamicFee(t *testing.T) { cfg := gas.UniversalEstimatorConfig{BlockHistoryRange: 0} - u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, chainID, nil) _, err := u.GetDynamicFee(tests.Context(t), maxPrice) assert.Error(t, err) }) @@ -252,7 +255,7 @@ func TestUniversalEstimatorGetDynamicFee(t *testing.T) { } maxFee := (*assets.Wei)(baseFee).AddPercentage(gas.BaseFeeBufferPercentage).Add((*assets.Wei)(maxPriorityFeePerGas)) - u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, chainID, nil) dynamicFee, err := u.GetDynamicFee(tests.Context(t), maxPrice) assert.NoError(t, err) assert.Equal(t, maxFee, dynamicFee.FeeCap) @@ -281,7 +284,7 @@ func TestUniversalEstimatorGetDynamicFee(t *testing.T) { cfg := gas.UniversalEstimatorConfig{BlockHistoryRange: 1} maxFee := (*assets.Wei)(baseFee).AddPercentage(gas.BaseFeeBufferPercentage).Add((*assets.Wei)(maxPriorityFeePerGas)) - u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, chainID, nil) dynamicFee, err := u.GetDynamicFee(tests.Context(t), maxPrice) assert.NoError(t, err) assert.Equal(t, maxFee, dynamicFee.FeeCap) @@ -304,7 +307,7 @@ func TestUniversalEstimatorGetDynamicFee(t *testing.T) { cfg := gas.UniversalEstimatorConfig{BlockHistoryRange: 1} - u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, chainID, nil) dynamicFee, err := u.GetDynamicFee(tests.Context(t), maxPrice) assert.NoError(t, err) assert.Equal(t, maxPrice, dynamicFee.FeeCap) @@ -316,6 +319,7 @@ func TestUniversalEstimatorBumpDynamicFee(t *testing.T) { t.Parallel() maxPrice := assets.NewWeiI(100) + chainID := big.NewInt(0) t.Run("bumps a previous attempt by BumpPercent", func(t *testing.T) { client := mocks.NewUniversalEstimatorClient(t) @@ -341,7 +345,7 @@ func TestUniversalEstimatorBumpDynamicFee(t *testing.T) { expectedFeeCap := originalFee.FeeCap.AddPercentage(cfg.BumpPercent) expectedTipCap := originalFee.TipCap.AddPercentage(cfg.BumpPercent) - u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, chainID, nil) dynamicFee, err := u.BumpDynamicFee(tests.Context(t), originalFee, maxPrice, nil) assert.NoError(t, err) assert.Equal(t, expectedFeeCap, dynamicFee.FeeCap) @@ -353,7 +357,7 @@ func TestUniversalEstimatorBumpDynamicFee(t *testing.T) { maxPrice := assets.NewWeiI(20) cfg := gas.UniversalEstimatorConfig{BlockHistoryRange: 1} - u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, chainID, nil) // nil original fee var originalFee gas.DynamicFee _, err := u.BumpDynamicFee(tests.Context(t), originalFee, maxPrice, nil) @@ -401,7 +405,7 @@ func TestUniversalEstimatorBumpDynamicFee(t *testing.T) { BumpPercent: 50, } - u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, chainID, nil) bumpedFee, err := u.BumpDynamicFee(tests.Context(t), originalFee, maxPrice, nil) assert.NoError(t, err) assert.Equal(t, (*assets.Wei)(maxPriorityFeePerGas), bumpedFee.TipCap) @@ -431,7 +435,7 @@ func TestUniversalEstimatorBumpDynamicFee(t *testing.T) { BumpPercent: 50, } - u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, chainID, nil) _, err := u.BumpDynamicFee(tests.Context(t), originalFee, maxPrice, nil) assert.Error(t, err) }) @@ -460,7 +464,7 @@ func TestUniversalEstimatorBumpDynamicFee(t *testing.T) { BumpPercent: 50, } - u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, chainID, nil) bumpedFee, err := u.BumpDynamicFee(tests.Context(t), originalFee, maxPrice, nil) assert.NoError(t, err) assert.Equal(t, maxPrice, bumpedFee.TipCap) @@ -491,7 +495,7 @@ func TestUniversalEstimatorBumpDynamicFee(t *testing.T) { BumpPercent: 50, } - u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, nil) + u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, chainID, nil) _, err := u.BumpDynamicFee(tests.Context(t), originalFee, maxPrice, nil) assert.Error(t, err) }) From 176102126f462f40b45dc8dbbf7c670b42c6a118 Mon Sep 17 00:00:00 2001 From: Dimitris Date: Mon, 8 Jul 2024 17:04:22 +0300 Subject: [PATCH 04/11] Use WeiMin to cap bump price --- core/chains/evm/gas/universal_estimator.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/core/chains/evm/gas/universal_estimator.go b/core/chains/evm/gas/universal_estimator.go index 737195bfc67..abc1350c204 100644 --- a/core/chains/evm/gas/universal_estimator.go +++ b/core/chains/evm/gas/universal_estimator.go @@ -362,9 +362,7 @@ func (u *UniversalEstimator) BumpDynamicFee(ctx context.Context, originalFee Dyn // makes sense anyway. func (u *UniversalEstimator) limitBumpedFee(originalFee *assets.Wei, currentFee *assets.Wei, bufferedFee *assets.Wei, maxPrice *assets.Wei) (*assets.Wei, error) { bumpedFee := assets.WeiMax(currentFee, bufferedFee) - if bumpedFee.Cmp(maxPrice) > 0 { - bumpedFee = maxPrice - } + bumpedFee = assets.WeiMin(bumpedFee, maxPrice) if bumpedFee.Cmp(originalFee.AddPercentage(minimumBumpPercentage)) < 0 { return nil, fmt.Errorf("bumpedFee: %s is bumped less than minimum allowed percentage(%s) from originalFee: %s - maxPrice: %s", From 4128630c1d34c907986115c3795c0e67222e4558 Mon Sep 17 00:00:00 2001 From: Dimitris Date: Thu, 11 Jul 2024 12:54:12 +0300 Subject: [PATCH 05/11] Update connectivity logic --- core/chains/evm/gas/universal_estimator.go | 40 +++++++++++++--------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/core/chains/evm/gas/universal_estimator.go b/core/chains/evm/gas/universal_estimator.go index abc1350c204..55217ea592d 100644 --- a/core/chains/evm/gas/universal_estimator.go +++ b/core/chains/evm/gas/universal_estimator.go @@ -17,6 +17,7 @@ import ( "github.com/smartcontractkit/chainlink-common/pkg/services" bigmath "github.com/smartcontractkit/chainlink-common/pkg/utils/big_math" + commonfee "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" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas/rollups" @@ -53,7 +54,7 @@ var ( const ( queryTimeout = 10 * time.Second - minimumBumpPercentage = 10 // based on geth's spec + MinimumBumpPercentage = 10 // based on geth's spec ConnectivityPercentile = 80 BaseFeeBufferPercentage = 40 @@ -107,9 +108,9 @@ func NewUniversalEstimator(lggr logger.Logger, client universalEstimatorClient, func (u *UniversalEstimator) Start(context.Context) error { // This is not an actual start since it's not a service, just a sanity check for configs - if u.config.BumpPercent < minimumBumpPercentage { + if u.config.BumpPercent < MinimumBumpPercentage { u.logger.Warnf("BumpPercent: %s is less than minimum allowed percentage: %s. Bumping attempts might result in rejections due to replacement transaction underpriced error!", - strconv.FormatUint(uint64(u.config.BumpPercent), 10), strconv.Itoa(minimumBumpPercentage)) + strconv.FormatUint(uint64(u.config.BumpPercent), 10), strconv.Itoa(MinimumBumpPercentage)) } if u.config.RewardPercentile > ConnectivityPercentile { u.logger.Warnf("RewardPercentile: %s is greater than maximum allowed connectivity percentage: %s. Lower reward percentile percentage otherwise connectivity checks will fail!", @@ -126,7 +127,7 @@ func (u *UniversalEstimator) Start(context.Context) error { // It returns a cached value if the price was recently changed. Caching can be skipped. func (u *UniversalEstimator) GetLegacyGas(ctx context.Context, _ []byte, gasLimit uint64, maxPrice *assets.Wei, opts ...feetypes.Opt) (gasPrice *assets.Wei, chainSpecificGasLimit uint64, err error) { chainSpecificGasLimit = gasLimit - // TODO: fix this + // TODO: fix this when the interface requirements change. refresh := false if slices.Contains(opts, feetypes.OptForceRefetch) { refresh = true @@ -284,7 +285,8 @@ func (u *UniversalEstimator) checkIfStale(dynamic bool) bool { func (u *UniversalEstimator) BumpLegacyGas(ctx context.Context, originalGasPrice *assets.Wei, gasLimit uint64, maxPrice *assets.Wei, _ []EvmPriorAttempt) (*assets.Wei, uint64, error) { // Sanitize original fee input if originalGasPrice == nil || originalGasPrice.Cmp(maxPrice) >= 0 { - return nil, 0, fmt.Errorf("error while retrieving original gas price: originalGasPrice: %s. Maximum price configured: %s", originalGasPrice, maxPrice) + return nil, 0, fmt.Errorf("%w: error while retrieving original gas price: originalGasPrice: %s. Maximum price configured: %s", + commonfee.ErrBump, originalGasPrice, maxPrice) } // Always refresh prices when bumping @@ -294,7 +296,7 @@ func (u *UniversalEstimator) BumpLegacyGas(ctx context.Context, originalGasPrice } bumpedGasPrice := originalGasPrice.AddPercentage(u.config.BumpPercent) - bumpedGasPrice, err = u.limitBumpedFee(originalGasPrice, currentGasPrice, bumpedGasPrice, maxPrice) + bumpedGasPrice, err = LimitBumpedFee(originalGasPrice, currentGasPrice, bumpedGasPrice, maxPrice) if err != nil { return nil, 0, fmt.Errorf("gas price error: %s", err.Error()) } @@ -316,8 +318,8 @@ func (u *UniversalEstimator) BumpDynamicFee(ctx context.Context, originalFee Dyn originalFee.TipCap == nil || ((originalFee.TipCap.Cmp(originalFee.FeeCap)) > 0) || (originalFee.FeeCap.Cmp(maxPrice) >= 0) { - return bumped, fmt.Errorf("error while retrieving original dynamic fees: (originalFeePerGas: %s - originalPriorityFeePerGas: %s). Maximum price configured: %s", - originalFee.FeeCap, originalFee.TipCap, maxPrice) + return bumped, fmt.Errorf("%w: error while retrieving original dynamic fees: (originalFeePerGas: %s - originalPriorityFeePerGas: %s). Maximum price configured: %s", + commonfee.ErrBump, originalFee.FeeCap, originalFee.TipCap, maxPrice) } // Always refresh prices when bumping @@ -329,7 +331,7 @@ func (u *UniversalEstimator) BumpDynamicFee(ctx context.Context, originalFee Dyn bumpedMaxPriorityFeePerGas := originalFee.TipCap.AddPercentage(u.config.BumpPercent) bumpedMaxFeePerGas := originalFee.FeeCap.AddPercentage(u.config.BumpPercent) - bumpedMaxPriorityFeePerGas, err = u.limitBumpedFee(originalFee.TipCap, currentDynamicPrice.TipCap, bumpedMaxPriorityFeePerGas, maxPrice) + bumpedMaxPriorityFeePerGas, err = LimitBumpedFee(originalFee.TipCap, currentDynamicPrice.TipCap, bumpedMaxPriorityFeePerGas, maxPrice) if err != nil { return bumped, fmt.Errorf("maxPriorityFeePerGas error: %s", err.Error()) } @@ -337,12 +339,16 @@ func (u *UniversalEstimator) BumpDynamicFee(ctx context.Context, originalFee Dyn if err != nil { return } - if bumpedMaxPriorityFeePerGas.Cmp(priorityFeeThreshold) > 0 { + // If either of these two values are 0 it could be that the network has extremely low priority fees or most likely it doesn't have + // a mempool and priority fees are not taken into account. Either way, we should skip the connectivity check because we're only + // going to be charged for the base fee, which is mandatory. + if (priorityFeeThreshold.Cmp(assets.NewWeiI(0)) > 0) && (bumpedMaxPriorityFeePerGas.Cmp(assets.NewWeiI(0)) > 0) && + bumpedMaxPriorityFeePerGas.Cmp(priorityFeeThreshold) > 0 { return bumped, fmt.Errorf("bumpedMaxPriorityFeePergas: %s is above market's %sth percentile: %s, bumping is halted", bumpedMaxPriorityFeePerGas, strconv.Itoa(ConnectivityPercentile), priorityFeeThreshold) } - bumpedMaxFeePerGas, err = u.limitBumpedFee(originalFee.FeeCap, currentDynamicPrice.FeeCap, bumpedMaxFeePerGas, maxPrice) + bumpedMaxFeePerGas, err = LimitBumpedFee(originalFee.FeeCap, currentDynamicPrice.FeeCap, bumpedMaxFeePerGas, maxPrice) if err != nil { return bumped, fmt.Errorf("maxFeePerGas error: %s", err.Error()) } @@ -360,13 +366,15 @@ func (u *UniversalEstimator) BumpDynamicFee(ctx context.Context, originalFee Dyn // Note: for chains that support EIP-1559 but we still choose to send Legacy transactions to them, the limit is still enforcable due to the fact that Legacy transactions // are treated the same way as Dynamic transactions under the hood. For chains that don't support EIP-1559 at all, the limit isn't enforcable but a 10% minimum bump percentage // makes sense anyway. -func (u *UniversalEstimator) limitBumpedFee(originalFee *assets.Wei, currentFee *assets.Wei, bufferedFee *assets.Wei, maxPrice *assets.Wei) (*assets.Wei, error) { - bumpedFee := assets.WeiMax(currentFee, bufferedFee) +func LimitBumpedFee(originalFee *assets.Wei, currentFee *assets.Wei, bumpedFee *assets.Wei, maxPrice *assets.Wei) (*assets.Wei, error) { + if currentFee != nil { + bumpedFee = assets.WeiMax(currentFee, bumpedFee) + } bumpedFee = assets.WeiMin(bumpedFee, maxPrice) - if bumpedFee.Cmp(originalFee.AddPercentage(minimumBumpPercentage)) < 0 { - return nil, fmt.Errorf("bumpedFee: %s is bumped less than minimum allowed percentage(%s) from originalFee: %s - maxPrice: %s", - bumpedFee, strconv.Itoa(minimumBumpPercentage), originalFee, maxPrice) + if bumpedFee.Cmp(originalFee.AddPercentage(MinimumBumpPercentage)) < 0 { + return nil, fmt.Errorf("%s: %s is bumped less than minimum allowed percentage(%s) from originalFee: %s - maxPrice: %s", + commonfee.ErrBump, bumpedFee, strconv.Itoa(MinimumBumpPercentage), originalFee, maxPrice) } return bumpedFee, nil } From 7ec8c3ff62375bdebb079280b68983aeb65394d5 Mon Sep 17 00:00:00 2001 From: Dimitris Date: Thu, 11 Jul 2024 13:52:05 +0300 Subject: [PATCH 06/11] Fix error --- core/chains/evm/gas/universal_estimator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/chains/evm/gas/universal_estimator.go b/core/chains/evm/gas/universal_estimator.go index 55217ea592d..a5fa1479d81 100644 --- a/core/chains/evm/gas/universal_estimator.go +++ b/core/chains/evm/gas/universal_estimator.go @@ -373,7 +373,7 @@ func LimitBumpedFee(originalFee *assets.Wei, currentFee *assets.Wei, bumpedFee * bumpedFee = assets.WeiMin(bumpedFee, maxPrice) if bumpedFee.Cmp(originalFee.AddPercentage(MinimumBumpPercentage)) < 0 { - return nil, fmt.Errorf("%s: %s is bumped less than minimum allowed percentage(%s) from originalFee: %s - maxPrice: %s", + return nil, fmt.Errorf("%w: %s is bumped less than minimum allowed percentage(%s) from originalFee: %s - maxPrice: %s", commonfee.ErrBump, bumpedFee, strconv.Itoa(MinimumBumpPercentage), originalFee, maxPrice) } return bumpedFee, nil From 45f663c016e6c71ba597be657efea3937c1e7fae Mon Sep 17 00:00:00 2001 From: Dimitris Date: Fri, 12 Jul 2024 00:02:18 +0300 Subject: [PATCH 07/11] Refactor Fixed Price estimator --- core/chains/evm/gas/fixed_price_estimator.go | 34 ++++++--- .../evm/gas/fixed_price_estimator_test.go | 30 +++++++- core/chains/evm/txmgr/broadcaster_test.go | 75 ++----------------- core/chains/evm/txmgr/confirmer_test.go | 62 ++++++++------- 4 files changed, 99 insertions(+), 102 deletions(-) diff --git a/core/chains/evm/gas/fixed_price_estimator.go b/core/chains/evm/gas/fixed_price_estimator.go index a30604a47c6..de38a0609be 100644 --- a/core/chains/evm/gas/fixed_price_estimator.go +++ b/core/chains/evm/gas/fixed_price_estimator.go @@ -3,8 +3,10 @@ package gas import ( "context" "fmt" + "strconv" "github.com/smartcontractkit/chainlink-common/pkg/logger" + commonfee "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" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas/rollups" @@ -30,7 +32,14 @@ func NewFixedPriceEstimator(lggr logger.Logger, config fixedPriceEstimatorConfig return &FixedPriceEstimator{logger.Sugared(logger.Named(lggr, "FixedPriceEstimator")), config, l1Oracle} } -func (f *FixedPriceEstimator) Start(context.Context) error { return nil } +func (f *FixedPriceEstimator) Start(context.Context) error { + // This is not an actual start since it's not a service, just a sanity check for configs + if f.config.BumpPercent() < MinimumBumpPercentage { + f.lggr.Warnf("BumpPercent: %s is less than minimum allowed percentage: %s. Bumping attempts might result in rejections due to replacement transaction underpriced error!", + strconv.FormatUint(uint64(f.config.BumpPercent()), 10), strconv.Itoa(MinimumBumpPercentage)) + } + return nil +} func (f *FixedPriceEstimator) GetLegacyGas(_ context.Context, _ []byte, gasLimit uint64, maxPrice *assets.Wei, _ ...feetypes.Opt) (*assets.Wei, uint64, error) { gasPrice := assets.WeiMin(f.config.PriceDefault(), maxPrice) @@ -40,12 +49,13 @@ func (f *FixedPriceEstimator) GetLegacyGas(_ context.Context, _ []byte, gasLimit func (f *FixedPriceEstimator) BumpLegacyGas(_ context.Context, originalGasPrice *assets.Wei, gasLimit uint64, maxPrice *assets.Wei, _ []EvmPriorAttempt) (*assets.Wei, uint64, error) { // Sanitize original fee input if originalGasPrice == nil || originalGasPrice.Cmp(maxPrice) >= 0 { - return nil, 0, fmt.Errorf("error while retrieving original gas price: originalGasPrice: %s. Maximum price configured: %s", originalGasPrice, maxPrice) + return nil, 0, fmt.Errorf("%w: error while retrieving original gas price: originalGasPrice: %s, maximum price configured: %s", + commonfee.ErrBump, originalGasPrice, maxPrice) } bumpedGasPrice := originalGasPrice.AddPercentage(f.config.BumpPercent()) - bumpedGasPrice = assets.WeiMin(bumpedGasPrice, maxPrice) - return bumpedGasPrice, gasLimit, nil + bumpedGasPrice, err := LimitBumpedFee(originalGasPrice, nil, bumpedGasPrice, maxPrice) + return bumpedGasPrice, gasLimit, err } func (f *FixedPriceEstimator) GetDynamicFee(_ context.Context, maxPrice *assets.Wei) (d DynamicFee, err error) { @@ -61,15 +71,21 @@ func (f *FixedPriceEstimator) BumpDynamicFee(_ context.Context, originalFee Dyna originalFee.TipCap == nil || ((originalFee.TipCap.Cmp(originalFee.FeeCap)) > 0) || (originalFee.FeeCap.Cmp(maxPrice) >= 0) { - return bumpedFee, fmt.Errorf("error while retrieving original dynamic fees: (originalFeePerGas: %s - originalPriorityFeePerGas: %s). Maximum price configured: %s", - originalFee.FeeCap, originalFee.TipCap, maxPrice) + return bumpedFee, fmt.Errorf("%w: error while retrieving original dynamic fees: (originalFeePerGas: %s - originalPriorityFeePerGas: %s), maximum price configured: %s", + commonfee.ErrBump, originalFee.FeeCap, originalFee.TipCap, maxPrice) } bumpedMaxPriorityFeePerGas := originalFee.TipCap.AddPercentage(f.config.BumpPercent()) - bumpedMaxFeePerGas := originalFee.FeeCap.AddPercentage(f.config.BumpPercent()) + bumpedMaxPriorityFeePerGas, err = LimitBumpedFee(originalFee.TipCap, nil, bumpedMaxPriorityFeePerGas, maxPrice) + if err != nil { + return bumpedFee, err + } - bumpedMaxFeePerGas = assets.WeiMin(bumpedMaxFeePerGas, maxPrice) - bumpedMaxPriorityFeePerGas = assets.WeiMin(bumpedMaxPriorityFeePerGas, maxPrice) + bumpedMaxFeePerGas := originalFee.FeeCap.AddPercentage(f.config.BumpPercent()) + bumpedMaxFeePerGas, err = LimitBumpedFee(originalFee.FeeCap, nil, bumpedMaxFeePerGas, maxPrice) + if err != nil { + return bumpedFee, err + } bumpedFee = DynamicFee{FeeCap: bumpedMaxFeePerGas, TipCap: bumpedMaxPriorityFeePerGas} return bumpedFee, nil diff --git a/core/chains/evm/gas/fixed_price_estimator_test.go b/core/chains/evm/gas/fixed_price_estimator_test.go index d4627611c87..d5d0ce3cd86 100644 --- a/core/chains/evm/gas/fixed_price_estimator_test.go +++ b/core/chains/evm/gas/fixed_price_estimator_test.go @@ -81,7 +81,7 @@ func Test_FixedPriceEstimator(t *testing.T) { f := gas.NewFixedPriceEstimator(logger.Test(t), config, l1Oracle) // original gas price is nil - maxPrice := assets.NewWeiI(41) + maxPrice := assets.NewWeiI(47) originalGasPrice := assets.NewWeiI(40) bumpedGas, _, err := f.BumpLegacyGas(tests.Context(t), originalGasPrice, 100000, maxPrice, nil) require.NoError(t, err) @@ -89,6 +89,20 @@ func Test_FixedPriceEstimator(t *testing.T) { }) + t.Run("BumpLegacyGas bumps original gas price by BumpPercent, caps on max price and errors due to minimum bump percentage", func(t *testing.T) { + config := gasmocks.NewFixedPriceEstimatorConfig(t) + config.On("BumpPercent").Return(uint16(20)) + l1Oracle := rollupMocks.NewL1Oracle(t) + f := gas.NewFixedPriceEstimator(logger.Test(t), config, l1Oracle) + + // original gas price is nil + maxPrice := assets.NewWeiI(41) + originalGasPrice := assets.NewWeiI(40) + _, _, err := f.BumpLegacyGas(tests.Context(t), originalGasPrice, 100000, maxPrice, nil) + require.Error(t, err) + + }) + t.Run("GetDynamicFee returns TipCapDefault and FeeCapDefault from config", func(t *testing.T) { config := gasmocks.NewFixedPriceEstimatorConfig(t) config.On("TipCapDefault").Return(assets.NewWeiI(10)) @@ -173,4 +187,18 @@ func Test_FixedPriceEstimator(t *testing.T) { assert.Equal(t, maxPrice, bumpedFee.FeeCap) assert.Equal(t, maxPrice, bumpedFee.TipCap) }) + + t.Run("BumpDynamicFee bumps original fee by BumpPercent, caps on max price and errors due to minimum bump percentage", func(t *testing.T) { + config := gasmocks.NewFixedPriceEstimatorConfig(t) + config.On("BumpPercent").Return(uint16(20)) + l1Oracle := rollupMocks.NewL1Oracle(t) + f := gas.NewFixedPriceEstimator(logger.Test(t), config, l1Oracle) + + maxPrice := assets.NewWeiI(21) + feeCap := assets.NewWeiI(20) + tipCap := assets.NewWeiI(19) + dynamicFee := gas.DynamicFee{FeeCap: feeCap, TipCap: tipCap} + _, err := f.BumpDynamicFee(tests.Context(t), dynamicFee, maxPrice, nil) + require.Error(t, err) + }) } diff --git a/core/chains/evm/txmgr/broadcaster_test.go b/core/chains/evm/txmgr/broadcaster_test.go index 784679fe363..7e73df52a23 100644 --- a/core/chains/evm/txmgr/broadcaster_test.go +++ b/core/chains/evm/txmgr/broadcaster_test.go @@ -29,6 +29,7 @@ import ( "github.com/smartcontractkit/chainlink-common/pkg/utils/tests" commonclient "github.com/smartcontractkit/chainlink/v2/common/client" + commonfee "github.com/smartcontractkit/chainlink/v2/common/fee" txmgrcommon "github.com/smartcontractkit/chainlink/v2/common/txmgr" txmgrtypes "github.com/smartcontractkit/chainlink/v2/common/txmgr/types" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/assets" @@ -1375,12 +1376,12 @@ func TestEthBroadcaster_ProcessUnstartedEthTxs_Errors(t *testing.T) { // Second with gas bump was still underpriced ethClient.On("SendTransactionReturnCode", mock.Anything, mock.MatchedBy(func(tx *gethTypes.Transaction) bool { - return tx.Nonce() == localNextNonce && tx.GasPrice().Cmp(big.NewInt(25000000000)) == 0 + return tx.Nonce() == localNextNonce && tx.GasPrice().Cmp(big.NewInt(24000000000)) == 0 }), fromAddress).Return(commonclient.Underpriced, errors.New(underpricedError)).Once() // Third succeeded ethClient.On("SendTransactionReturnCode", mock.Anything, mock.MatchedBy(func(tx *gethTypes.Transaction) bool { - return tx.Nonce() == localNextNonce && tx.GasPrice().Cmp(big.NewInt(30000000000)) == 0 + return tx.Nonce() == localNextNonce && tx.GasPrice().Cmp(big.NewInt(28800000000)) == 0 }), fromAddress).Return(commonclient.Successful, nil).Once() // Do the thing @@ -1398,7 +1399,7 @@ func TestEthBroadcaster_ProcessUnstartedEthTxs_Errors(t *testing.T) { assert.False(t, etx.Error.Valid) assert.Len(t, etx.TxAttempts, 1) attempt := etx.TxAttempts[0] - assert.Equal(t, "30 gwei", attempt.TxFee.Legacy.String()) + assert.Equal(t, "28.8 gwei", attempt.TxFee.Legacy.String()) }) etxUnfinished := txmgr.Tx{ @@ -1468,7 +1469,7 @@ func TestEthBroadcaster_ProcessUnstartedEthTxs_Errors(t *testing.T) { assert.Equal(t, "20 gwei", attempt.TxFee.Legacy.String()) }) - t.Run("eth node returns underpriced transaction and bumping gas doesn't increase it", func(t *testing.T) { + t.Run("eth node returns underpriced transaction and bumping with 0 returns error", func(t *testing.T) { // This happens if a transaction's gas price is below the minimum // configured for the transaction pool. // This is a configuration error by the node operator, since it means they set the base gas level too low. @@ -1492,7 +1493,7 @@ func TestEthBroadcaster_ProcessUnstartedEthTxs_Errors(t *testing.T) { // Do the thing retryable, err := eb2.ProcessUnstartedTxs(ctx, fromAddress) require.Error(t, err) - require.Contains(t, err.Error(), "bumped fee price of 20 gwei is equal to original fee price of 20 gwei. ACTION REQUIRED: This is a configuration error, you must increase either FeeEstimator.BumpPercent or FeeEstimator.BumpMin") + assert.True(t, commonfee.IsBumpErr(err)) assert.True(t, retryable) // TEARDOWN: Clear out the unsent tx before the next test @@ -1558,17 +1559,9 @@ func TestEthBroadcaster_ProcessUnstartedEthTxs_Errors(t *testing.T) { pgtest.MustExec(t, db, `DELETE FROM evm.txes`) }) - t.Run("eth node returns underpriced transaction and bumping gas doesn't increase it in EIP-1559 mode", func(t *testing.T) { - // This happens if a transaction's gas price is below the minimum - // configured for the transaction pool. - // This is a configuration error by the node operator, since it means they set the base gas level too low. - - // In this scenario the node operator REALLY fucked up and set the bump - // to zero (even though that should not be possible due to config - // validation) + t.Run("eth node returns underpriced transaction and bumping gas doesn't increase it in EIP-1559 mode, resulting in error", func(t *testing.T) { evmcfg2 := evmtest.NewChainScopedConfig(t, configtest.NewGeneralConfig(t, func(c *chainlink.Config, s *chainlink.Secrets) { c.EVM[0].GasEstimator.EIP1559DynamicFees = ptr(true) - c.EVM[0].GasEstimator.BumpMin = assets.NewWeiI(0) c.EVM[0].GasEstimator.BumpPercent = ptr[uint16](0) })) localNextNonce := getLocalNextNonce(t, nonceTracker, fromAddress) @@ -1584,63 +1577,11 @@ func TestEthBroadcaster_ProcessUnstartedEthTxs_Errors(t *testing.T) { // Check gas tip cap verification retryable, err := eb2.ProcessUnstartedTxs(ctx, fromAddress) require.Error(t, err) - require.Contains(t, err.Error(), "bumped gas tip cap of 1 wei is less than or equal to original gas tip cap of 1 wei") + assert.True(t, commonfee.IsBumpErr(err)) assert.True(t, retryable) pgtest.MustExec(t, db, `DELETE FROM evm.txes`) }) - - t.Run("eth node returns underpriced transaction in EIP-1559 mode, bumps until inclusion", func(t *testing.T) { - // This happens if a transaction's gas price is below the minimum - // configured for the transaction pool. - // This is a configuration error by the node operator, since it means they set the base gas level too low. - underpricedError := "transaction underpriced" - localNextNonce := getLocalNextNonce(t, nonceTracker, fromAddress) - mustCreateUnstartedTx(t, txStore, fromAddress, toAddress, encodedPayload, gasLimit, value, testutils.FixtureChainID) - - // Check gas tip cap verification - evmcfg2 := evmtest.NewChainScopedConfig(t, configtest.NewGeneralConfig(t, func(c *chainlink.Config, s *chainlink.Secrets) { - c.EVM[0].GasEstimator.EIP1559DynamicFees = ptr(true) - c.EVM[0].GasEstimator.TipCapDefault = assets.NewWeiI(0) - })) - ethClient.On("PendingNonceAt", mock.Anything, fromAddress).Return(localNextNonce, nil).Once() - eb2 := NewTestEthBroadcaster(t, txStore, ethClient, ethKeyStore, cfg, evmcfg2, &testCheckerFactory{}, false, nonceTracker) - - retryable, err := eb2.ProcessUnstartedTxs(ctx, fromAddress) - require.Error(t, err) - require.Contains(t, err.Error(), "specified gas tip cap of 0 is below min configured gas tip of 1 wei for key") - assert.True(t, retryable) - - gasTipCapDefault := assets.NewWeiI(42) - - evmcfg2 = evmtest.NewChainScopedConfig(t, configtest.NewGeneralConfig(t, func(c *chainlink.Config, s *chainlink.Secrets) { - c.EVM[0].GasEstimator.EIP1559DynamicFees = ptr(true) - c.EVM[0].GasEstimator.TipCapDefault = gasTipCapDefault - })) - localNextNonce = getLocalNextNonce(t, nonceTracker, fromAddress) - ethClient.On("PendingNonceAt", mock.Anything, fromAddress).Return(localNextNonce, nil).Once() - eb2 = NewTestEthBroadcaster(t, txStore, ethClient, ethKeyStore, cfg, evmcfg2, &testCheckerFactory{}, false, nonceTracker) - - // Second was underpriced but above minimum - ethClient.On("SendTransactionReturnCode", mock.Anything, mock.MatchedBy(func(tx *gethTypes.Transaction) bool { - return tx.Nonce() == localNextNonce && tx.GasTipCap().Cmp(gasTipCapDefault.ToInt()) == 0 - }), fromAddress).Return(commonclient.Underpriced, errors.New(underpricedError)).Once() - // Resend at the bumped price - ethClient.On("SendTransactionReturnCode", mock.Anything, mock.MatchedBy(func(tx *gethTypes.Transaction) bool { - return tx.Nonce() == localNextNonce && tx.GasTipCap().Cmp(big.NewInt(0).Add(gasTipCapDefault.ToInt(), evmcfg2.EVM().GasEstimator().BumpMin().ToInt())) == 0 - }), fromAddress).Return(commonclient.Underpriced, errors.New(underpricedError)).Once() - // Final bump succeeds - ethClient.On("SendTransactionReturnCode", mock.Anything, mock.MatchedBy(func(tx *gethTypes.Transaction) bool { - return tx.Nonce() == localNextNonce && tx.GasTipCap().Cmp(big.NewInt(0).Add(gasTipCapDefault.ToInt(), big.NewInt(0).Mul(evmcfg2.EVM().GasEstimator().BumpMin().ToInt(), big.NewInt(2)))) == 0 - }), fromAddress).Return(commonclient.Successful, nil).Once() - - retryable, err = eb2.ProcessUnstartedTxs(ctx, fromAddress) - require.NoError(t, err) - assert.False(t, retryable) - - // TEARDOWN: Clear out the unsent tx before the next test - pgtest.MustExec(t, db, `DELETE FROM evm.txes WHERE nonce = $1`, localNextNonce) - }) } func TestEthBroadcaster_ProcessUnstartedEthTxs_KeystoreErrors(t *testing.T) { diff --git a/core/chains/evm/txmgr/confirmer_test.go b/core/chains/evm/txmgr/confirmer_test.go index 6c7837571e5..31cababc92b 100644 --- a/core/chains/evm/txmgr/confirmer_test.go +++ b/core/chains/evm/txmgr/confirmer_test.go @@ -1756,7 +1756,8 @@ func TestEthConfirmer_RebroadcastWhereNecessary_MaxFeeScenario(t *testing.T) { etx := cltest.MustInsertUnconfirmedEthTxWithBroadcastLegacyAttempt(t, txStore, nonce, fromAddress, originalBroadcastAt) attempt1_1 := etx.TxAttempts[0] var dbAttempt txmgr.DbEthTxAttempt - require.NoError(t, db.Get(&dbAttempt, `UPDATE evm.tx_attempts SET broadcast_before_block_num=$1 WHERE id=$2 RETURNING *`, oldEnough, attempt1_1.ID)) + require.NoError(t, db.Get(&dbAttempt, `UPDATE evm.tx_attempts SET broadcast_before_block_num=$1, gas_price=$2 WHERE id=$3 RETURNING *`, + oldEnough, evmcfg.EVM().GasEstimator().PriceDefault(), attempt1_1.ID)) t.Run("treats an exceeds max fee attempt as a success", func(t *testing.T) { ethTx := *types.NewTx(&types.LegacyTx{}) @@ -1775,7 +1776,7 @@ func TestEthConfirmer_RebroadcastWhereNecessary_MaxFeeScenario(t *testing.T) { // Once for the bumped attempt which exceeds limit ethClient.On("SendTransactionReturnCode", mock.Anything, mock.MatchedBy(func(tx *types.Transaction) bool { - return tx.Nonce() == uint64(*etx.Sequence) && tx.GasPrice().Int64() == int64(20000000000) + return tx.Nonce() == uint64(*etx.Sequence) && tx.GasPrice().Int64() == int64(24000000000) }), fromAddress).Return(commonclient.ExceedsMaxFee, errors.New("tx fee (1.10 ether) exceeds the configured cap (1.00 ether)")).Once() // Do the thing @@ -1829,7 +1830,8 @@ func TestEthConfirmer_RebroadcastWhereNecessary(t *testing.T) { nonce++ attempt1_1 := etx.TxAttempts[0] var dbAttempt txmgr.DbEthTxAttempt - require.NoError(t, db.Get(&dbAttempt, `UPDATE evm.tx_attempts SET broadcast_before_block_num=$1 WHERE id=$2 RETURNING *`, oldEnough, attempt1_1.ID)) + require.NoError(t, db.Get(&dbAttempt, `UPDATE evm.tx_attempts SET broadcast_before_block_num=$1, gas_price=$2 WHERE id=$3 RETURNING *`, + oldEnough, evmcfg.EVM().GasEstimator().PriceDefault(), attempt1_1.ID)) t.Run("re-sends previous transaction on keystore error", func(t *testing.T) { // simulate bumped transaction that is somehow impossible to sign @@ -1883,7 +1885,7 @@ func TestEthConfirmer_RebroadcastWhereNecessary(t *testing.T) { ec.XXXTestSetClient(txmgr.NewEvmTxmClient(ethClient, nil)) t.Run("creates new attempt with higher gas price if transaction has an attempt older than threshold", func(t *testing.T) { - expectedBumpedGasPrice := big.NewInt(20000000000) + expectedBumpedGasPrice := big.NewInt(24000000000) require.Greater(t, expectedBumpedGasPrice.Int64(), attempt1_1.TxFee.Legacy.ToInt().Int64()) ethTx := *types.NewTx(&types.LegacyTx{}) @@ -1931,7 +1933,7 @@ func TestEthConfirmer_RebroadcastWhereNecessary(t *testing.T) { var attempt1_3 txmgr.TxAttempt t.Run("creates new attempt with higher gas price if transaction is already in mempool (e.g. due to previous crash before we could save the new attempt)", func(t *testing.T) { - expectedBumpedGasPrice := big.NewInt(25000000000) + expectedBumpedGasPrice := big.NewInt(28800000000) require.Greater(t, expectedBumpedGasPrice.Int64(), attempt1_2.TxFee.Legacy.ToInt().Int64()) ethTx := *types.NewTx(&types.LegacyTx{}) @@ -1969,7 +1971,7 @@ func TestEthConfirmer_RebroadcastWhereNecessary(t *testing.T) { var attempt1_4 txmgr.TxAttempt t.Run("saves new attempt even for transaction that has already been confirmed (nonce already used)", func(t *testing.T) { - expectedBumpedGasPrice := big.NewInt(30000000000) + expectedBumpedGasPrice := big.NewInt(34560000000) require.Greater(t, expectedBumpedGasPrice.Int64(), attempt1_2.TxFee.Legacy.ToInt().Int64()) ethTx := *types.NewTx(&types.LegacyTx{}) @@ -2018,11 +2020,12 @@ func TestEthConfirmer_RebroadcastWhereNecessary(t *testing.T) { etx2 := cltest.MustInsertUnconfirmedEthTxWithBroadcastLegacyAttempt(t, txStore, nonce, fromAddress) nonce++ attempt2_1 := etx2.TxAttempts[0] - require.NoError(t, db.Get(&dbAttempt, `UPDATE evm.tx_attempts SET broadcast_before_block_num=$1 WHERE id=$2 RETURNING *`, oldEnough, attempt2_1.ID)) + require.NoError(t, db.Get(&dbAttempt, `UPDATE evm.tx_attempts SET broadcast_before_block_num=$1, gas_price=$2 WHERE id=$3 RETURNING *`, + oldEnough, evmcfg.EVM().GasEstimator().PriceDefault(), attempt2_1.ID)) var attempt2_2 txmgr.TxAttempt t.Run("saves in-progress attempt on temporary error and returns error", func(t *testing.T) { - expectedBumpedGasPrice := big.NewInt(20000000000) + expectedBumpedGasPrice := big.NewInt(24000000000) require.Greater(t, expectedBumpedGasPrice.Int64(), attempt2_1.TxFee.Legacy.ToInt().Int64()) ethTx := *types.NewTx(&types.LegacyTx{}) @@ -2089,7 +2092,7 @@ func TestEthConfirmer_RebroadcastWhereNecessary(t *testing.T) { require.NoError(t, db.Get(&dbAttempt, `UPDATE evm.tx_attempts SET broadcast_before_block_num=$1 WHERE id=$2 RETURNING *`, oldEnough, attempt2_2.ID)) t.Run("assumes that 'nonce too low' error means confirmed_missing_receipt", func(t *testing.T) { - expectedBumpedGasPrice := big.NewInt(25000000000) + expectedBumpedGasPrice := big.NewInt(28800000000) require.Greater(t, expectedBumpedGasPrice.Int64(), attempt2_1.TxFee.Legacy.ToInt().Int64()) ethTx := *types.NewTx(&types.LegacyTx{}) @@ -2240,18 +2243,18 @@ func TestEthConfirmer_RebroadcastWhereNecessary(t *testing.T) { require.NoError(t, db.Get(&dbAttempt, `UPDATE evm.tx_attempts SET broadcast_before_block_num=$1 WHERE id=$2 RETURNING *`, oldEnough, attempt3_4.ID)) - t.Run("resubmits at the old price and does not create a new attempt if one of the bumped transactions would exceed EVM.GasEstimator.PriceMax", func(t *testing.T) { + t.Run("resubmits at the max price and creates a new attempt if one of the bumped transactions would exceed EVM.GasEstimator.PriceMax", func(t *testing.T) { // Set price such that the next bump will exceed EVM.GasEstimator.PriceMax // Existing gas price is: 60480000000 - gasPrice := attempt3_4.TxFee.Legacy.ToInt() + priceMax := big.NewInt(66530000000) gcfg := configtest.NewGeneralConfig(t, func(c *chainlink.Config, s *chainlink.Secrets) { - c.EVM[0].GasEstimator.PriceMax = assets.NewWeiI(60500000000) + c.EVM[0].GasEstimator.PriceMax = (*assets.Wei)(priceMax) }) newCfg := evmtest.NewChainScopedConfig(t, gcfg) ec2 := newEthConfirmer(t, txStore, ethClient, gcfg, newCfg, ethKeyStore, nil) ethClient.On("SendTransactionReturnCode", mock.Anything, mock.MatchedBy(func(tx *types.Transaction) bool { - return evmtypes.Nonce(tx.Nonce()) == *etx3.Sequence && gasPrice.Cmp(tx.GasPrice()) == 0 + return evmtypes.Nonce(tx.Nonce()) == *etx3.Sequence && priceMax.Cmp(tx.GasPrice()) == 0 }), fromAddress).Return(commonclient.Successful, errors.New("already known")).Once() // we already submitted at this price, now it's time to bump and submit again but since we simply resubmitted rather than increasing gas price, geth already knows about this tx // Do the thing @@ -2262,10 +2265,10 @@ func TestEthConfirmer_RebroadcastWhereNecessary(t *testing.T) { assert.Equal(t, txmgrcommon.TxUnconfirmed, etx3.State) - // No new tx attempts - require.Len(t, etx3.TxAttempts, 4) + // New tx attempts + require.Len(t, etx3.TxAttempts, 5) attempt3_4 = etx3.TxAttempts[0] - assert.Equal(t, gasPrice.Int64(), attempt3_4.TxFee.Legacy.ToInt().Int64()) + assert.Equal(t, (*assets.Wei)(priceMax), attempt3_4.TxFee.Legacy) }) require.NoError(t, db.Get(&dbAttempt, `UPDATE evm.tx_attempts SET broadcast_before_block_num=$1 WHERE id=$2 RETURNING *`, oldEnough, attempt3_4.ID)) @@ -2275,7 +2278,7 @@ func TestEthConfirmer_RebroadcastWhereNecessary(t *testing.T) { // Existing gas price is: 60480000000 gasPrice := attempt3_4.TxFee.Legacy.ToInt() gcfg := configtest.NewGeneralConfig(t, func(c *chainlink.Config, s *chainlink.Secrets) { - c.EVM[0].GasEstimator.PriceMax = assets.NewWeiI(60480000000) + c.EVM[0].GasEstimator.PriceMax = assets.NewWeiI(66530000000) }) newCfg := evmtest.NewChainScopedConfig(t, gcfg) ec2 := newEthConfirmer(t, txStore, ethClient, gcfg, newCfg, ethKeyStore, nil) @@ -2293,7 +2296,7 @@ func TestEthConfirmer_RebroadcastWhereNecessary(t *testing.T) { assert.Equal(t, txmgrcommon.TxUnconfirmed, etx3.State) // No new tx attempts - require.Len(t, etx3.TxAttempts, 4) + require.Len(t, etx3.TxAttempts, 5) attempt3_4 = etx3.TxAttempts[0] assert.Equal(t, gasPrice.Int64(), attempt3_4.TxFee.Legacy.ToInt().Int64()) }) @@ -2411,7 +2414,8 @@ func TestEthConfirmer_RebroadcastWhereNecessary_TerminallyUnderpriced_ThenGoesTh db := pgtest.NewSqlxDB(t) cfg := configtest.NewGeneralConfig(t, func(c *chainlink.Config, s *chainlink.Secrets) { - c.EVM[0].GasEstimator.PriceMax = assets.GWei(500) + c.EVM[0].GasEstimator.PriceMax = assets.GWei(5000) + c.EVM[0].GasEstimator.PriceMin = assets.NewWeiI(0) }) txStore := cltest.NewTestTxStore(t, db) @@ -2463,7 +2467,8 @@ func TestEthConfirmer_RebroadcastWhereNecessary_TerminallyUnderpriced_ThenGoesTh legacyAttempt := etx.TxAttempts[0] var dbAttempt txmgr.DbEthTxAttempt dbAttempt.FromTxAttempt(&legacyAttempt) - require.NoError(t, db.Get(&dbAttempt, `UPDATE evm.tx_attempts SET broadcast_before_block_num=$1 WHERE id=$2 RETURNING *`, oldEnough, legacyAttempt.ID)) + require.NoError(t, db.Get(&dbAttempt, `UPDATE evm.tx_attempts SET broadcast_before_block_num=$1, gas_price=$2 WHERE id=$3 RETURNING *`, + oldEnough, evmcfg.EVM().GasEstimator().PriceDefault(), legacyAttempt.ID)) // Fail a few times with terminally underpriced ethClient.On("SendTransactionReturnCode", mock.Anything, mock.Anything, fromAddress).Return( @@ -2495,7 +2500,9 @@ func TestEthConfirmer_RebroadcastWhereNecessary_TerminallyUnderpriced_ThenGoesTh dxFeeAttempt := etx.TxAttempts[0] var dbAttempt txmgr.DbEthTxAttempt dbAttempt.FromTxAttempt(&dxFeeAttempt) - require.NoError(t, db.Get(&dbAttempt, `UPDATE evm.tx_attempts SET broadcast_before_block_num=$1 WHERE id=$2 RETURNING *`, oldEnough, dxFeeAttempt.ID)) + tipCapDefault := assets.NewWeiI(100) + require.NoError(t, db.Get(&dbAttempt, `UPDATE evm.tx_attempts SET broadcast_before_block_num=$1, gas_fee_cap=$2, gas_tip_cap=$3 WHERE id=$4 RETURNING *`, + oldEnough, evmcfg.EVM().GasEstimator().FeeCapDefault(), tipCapDefault, dxFeeAttempt.ID)) // Fail a few times with terminally underpriced ethClient.On("SendTransactionReturnCode", mock.Anything, mock.Anything, fromAddress).Return( @@ -2536,7 +2543,10 @@ func TestEthConfirmer_RebroadcastWhereNecessary_WhenOutOfEth(t *testing.T) { // keyStates, err := ethKeyStore.GetStatesForKeys(keys) // require.NoError(t, err) - gconfig, config := newTestChainScopedConfig(t) + gconfig := configtest.NewGeneralConfig(t, func(c *chainlink.Config, s *chainlink.Secrets) { + c.EVM[0].GasEstimator.PriceMin = assets.NewWeiI(0) + }) + config := evmtest.NewChainScopedConfig(t, gconfig) currentHead := int64(30) oldEnough := int64(19) nonce := int64(0) @@ -2546,6 +2556,8 @@ func TestEthConfirmer_RebroadcastWhereNecessary_WhenOutOfEth(t *testing.T) { attempt1_1 := etx.TxAttempts[0] var dbAttempt txmgr.DbEthTxAttempt dbAttempt.FromTxAttempt(&attempt1_1) + require.NoError(t, db.Get(&dbAttempt, `UPDATE evm.tx_attempts SET broadcast_before_block_num=$1, gas_price=$2 WHERE id=$3 RETURNING *`, + oldEnough, config.EVM().GasEstimator().PriceDefault(), attempt1_1.ID)) require.NoError(t, db.Get(&dbAttempt, `UPDATE evm.tx_attempts SET broadcast_before_block_num=$1 WHERE id=$2 RETURNING *`, oldEnough, attempt1_1.ID)) var attempt1_2 txmgr.TxAttempt @@ -2554,7 +2566,7 @@ func TestEthConfirmer_RebroadcastWhereNecessary_WhenOutOfEth(t *testing.T) { t.Run("saves attempt with state 'insufficient_eth' if eth node returns this error", func(t *testing.T) { ec := newEthConfirmer(t, txStore, ethClient, gconfig, config, ethKeyStore, nil) - expectedBumpedGasPrice := big.NewInt(20000000000) + expectedBumpedGasPrice := big.NewInt(24000000000) require.Greater(t, expectedBumpedGasPrice.Int64(), attempt1_1.TxFee.Legacy.ToInt().Int64()) ethClient.On("SendTransactionReturnCode", mock.Anything, mock.MatchedBy(func(tx *types.Transaction) bool { @@ -2580,7 +2592,7 @@ func TestEthConfirmer_RebroadcastWhereNecessary_WhenOutOfEth(t *testing.T) { t.Run("does not bump gas when previous error was 'out of eth', instead resubmits existing transaction", func(t *testing.T) { ec := newEthConfirmer(t, txStore, ethClient, gconfig, config, ethKeyStore, nil) - expectedBumpedGasPrice := big.NewInt(20000000000) + expectedBumpedGasPrice := big.NewInt(24000000000) require.Greater(t, expectedBumpedGasPrice.Int64(), attempt1_1.TxFee.Legacy.ToInt().Int64()) ethClient.On("SendTransactionReturnCode", mock.Anything, mock.MatchedBy(func(tx *types.Transaction) bool { @@ -2605,7 +2617,7 @@ func TestEthConfirmer_RebroadcastWhereNecessary_WhenOutOfEth(t *testing.T) { t.Run("saves the attempt as broadcast after node wallet has been topped up with sufficient balance", func(t *testing.T) { ec := newEthConfirmer(t, txStore, ethClient, gconfig, config, ethKeyStore, nil) - expectedBumpedGasPrice := big.NewInt(20000000000) + expectedBumpedGasPrice := big.NewInt(24000000000) require.Greater(t, expectedBumpedGasPrice.Int64(), attempt1_1.TxFee.Legacy.ToInt().Int64()) ethClient.On("SendTransactionReturnCode", mock.Anything, mock.MatchedBy(func(tx *types.Transaction) bool { From 937ad92aa318e6601e112b6f1c301e006153ce15 Mon Sep 17 00:00:00 2001 From: Dimitris Date: Fri, 12 Jul 2024 00:19:44 +0300 Subject: [PATCH 08/11] Fixes --- core/chains/evm/client/chain_client.go | 4 ++-- .../evm/gas/mocks/universal_estimator_client.go | 2 +- core/chains/evm/gas/universal_estimator.go | 14 +++++++------- core/chains/evm/gas/universal_estimator_test.go | 10 ++++------ 4 files changed, 14 insertions(+), 16 deletions(-) diff --git a/core/chains/evm/client/chain_client.go b/core/chains/evm/client/chain_client.go index 5e8b3d94582..319ece6b55a 100644 --- a/core/chains/evm/client/chain_client.go +++ b/core/chains/evm/client/chain_client.go @@ -352,8 +352,8 @@ func (c *chainClient) LatestFinalizedBlock(ctx context.Context) (*evmtypes.Head, return c.multiNode.LatestFinalizedBlock(ctx) } -func (r *chainClient) FeeHistory(ctx context.Context, blockCount uint64, rewardPercentiles []float64) (feeHistory *ethereum.FeeHistory, err error) { - rpc, err := r.multiNode.SelectNodeRPC() +func (c *chainClient) FeeHistory(ctx context.Context, blockCount uint64, rewardPercentiles []float64) (feeHistory *ethereum.FeeHistory, err error) { + rpc, err := c.multiNode.SelectNodeRPC() if err != nil { return feeHistory, err } diff --git a/core/chains/evm/gas/mocks/universal_estimator_client.go b/core/chains/evm/gas/mocks/universal_estimator_client.go index f549b21f92e..e58bf173791 100644 --- a/core/chains/evm/gas/mocks/universal_estimator_client.go +++ b/core/chains/evm/gas/mocks/universal_estimator_client.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.42.2. DO NOT EDIT. +// Code generated by mockery v2.43.2. DO NOT EDIT. package mocks diff --git a/core/chains/evm/gas/universal_estimator.go b/core/chains/evm/gas/universal_estimator.go index a5fa1479d81..26186826a5e 100644 --- a/core/chains/evm/gas/universal_estimator.go +++ b/core/chains/evm/gas/universal_estimator.go @@ -117,7 +117,7 @@ func (u *UniversalEstimator) Start(context.Context) error { strconv.FormatUint(uint64(u.config.RewardPercentile), 10), strconv.Itoa(ConnectivityPercentile)) } if u.config.BlockHistoryRange == 0 { - u.logger.Warnf("BlockHistoryRange is set to 0. Using dynamic transactions will result in an error!", + u.logger.Warn("BlockHistoryRange is set to 0. Using dynamic transactions will result in an error!", strconv.FormatUint(uint64(u.config.RewardPercentile), 10), strconv.Itoa(ConnectivityPercentile)) } return nil @@ -258,13 +258,13 @@ func (u *UniversalEstimator) fetchDynamicPrice(parentCtx context.Context, forceR return u.dynamicPrice, nil } -func (o *UniversalEstimator) getDynamicPrice() (fee DynamicFee, err error) { - o.dynamicPriceMu.RLock() - defer o.dynamicPriceMu.RUnlock() - if o.dynamicPrice.FeeCap == nil || o.dynamicPrice.TipCap == nil { +func (u *UniversalEstimator) getDynamicPrice() (fee DynamicFee, err error) { + u.dynamicPriceMu.RLock() + defer u.dynamicPriceMu.RUnlock() + if u.dynamicPrice.FeeCap == nil || u.dynamicPrice.TipCap == nil { return fee, fmt.Errorf("dynamic price not set") } - return o.dynamicPrice, nil + return u.dynamicPrice, nil } // checkIfStale enables caching @@ -346,8 +346,8 @@ func (u *UniversalEstimator) BumpDynamicFee(ctx context.Context, originalFee Dyn bumpedMaxPriorityFeePerGas.Cmp(priorityFeeThreshold) > 0 { return bumped, fmt.Errorf("bumpedMaxPriorityFeePergas: %s is above market's %sth percentile: %s, bumping is halted", bumpedMaxPriorityFeePerGas, strconv.Itoa(ConnectivityPercentile), priorityFeeThreshold) - } + bumpedMaxFeePerGas, err = LimitBumpedFee(originalFee.FeeCap, currentDynamicPrice.FeeCap, bumpedMaxFeePerGas, maxPrice) if err != nil { return bumped, fmt.Errorf("maxFeePerGas error: %s", err.Error()) diff --git a/core/chains/evm/gas/universal_estimator_test.go b/core/chains/evm/gas/universal_estimator_test.go index 2829a59edb7..30eab673495 100644 --- a/core/chains/evm/gas/universal_estimator_test.go +++ b/core/chains/evm/gas/universal_estimator_test.go @@ -134,7 +134,6 @@ func TestUniversalEstimatorBumpLegacyGas(t *testing.T) { originalPrice = assets.NewWeiI(100) _, _, err = u.BumpLegacyGas(tests.Context(t), originalPrice, gasLimit, maxPrice, nil) assert.Error(t, err) - }) t.Run("returns market gas price if bumped original fee is lower", func(t *testing.T) { @@ -265,7 +264,6 @@ func TestUniversalEstimatorGetDynamicFee(t *testing.T) { assert.NoError(t, err) assert.Equal(t, maxFee, dynamicFee2.FeeCap) assert.Equal(t, (*assets.Wei)(maxPriorityFeePerGas), dynamicFee2.TipCap) - }) t.Run("fetches a new dynamic fee when first called", func(t *testing.T) { @@ -318,7 +316,7 @@ func TestUniversalEstimatorGetDynamicFee(t *testing.T) { func TestUniversalEstimatorBumpDynamicFee(t *testing.T) { t.Parallel() - maxPrice := assets.NewWeiI(100) + globalMaxPrice := assets.NewWeiI(100) chainID := big.NewInt(0) t.Run("bumps a previous attempt by BumpPercent", func(t *testing.T) { @@ -346,7 +344,7 @@ func TestUniversalEstimatorBumpDynamicFee(t *testing.T) { expectedTipCap := originalFee.TipCap.AddPercentage(cfg.BumpPercent) u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, chainID, nil) - dynamicFee, err := u.BumpDynamicFee(tests.Context(t), originalFee, maxPrice, nil) + dynamicFee, err := u.BumpDynamicFee(tests.Context(t), originalFee, globalMaxPrice, nil) assert.NoError(t, err) assert.Equal(t, expectedFeeCap, dynamicFee.FeeCap) assert.Equal(t, expectedTipCap, dynamicFee.TipCap) @@ -406,7 +404,7 @@ func TestUniversalEstimatorBumpDynamicFee(t *testing.T) { } u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, chainID, nil) - bumpedFee, err := u.BumpDynamicFee(tests.Context(t), originalFee, maxPrice, nil) + bumpedFee, err := u.BumpDynamicFee(tests.Context(t), originalFee, globalMaxPrice, nil) assert.NoError(t, err) assert.Equal(t, (*assets.Wei)(maxPriorityFeePerGas), bumpedFee.TipCap) assert.Equal(t, maxFee, bumpedFee.FeeCap) @@ -436,7 +434,7 @@ func TestUniversalEstimatorBumpDynamicFee(t *testing.T) { } u := gas.NewUniversalEstimator(logger.Test(t), client, cfg, chainID, nil) - _, err := u.BumpDynamicFee(tests.Context(t), originalFee, maxPrice, nil) + _, err := u.BumpDynamicFee(tests.Context(t), originalFee, globalMaxPrice, nil) assert.Error(t, err) }) From 5e9716a14477befbf6afb1b28ea77c5cd98ad121 Mon Sep 17 00:00:00 2001 From: Dimitris Date: Fri, 12 Jul 2024 12:12:30 +0300 Subject: [PATCH 09/11] Cover an edge case when enforcing limits --- core/chains/evm/gas/universal_estimator.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/core/chains/evm/gas/universal_estimator.go b/core/chains/evm/gas/universal_estimator.go index 26186826a5e..6493642ee42 100644 --- a/core/chains/evm/gas/universal_estimator.go +++ b/core/chains/evm/gas/universal_estimator.go @@ -372,7 +372,12 @@ func LimitBumpedFee(originalFee *assets.Wei, currentFee *assets.Wei, bumpedFee * } bumpedFee = assets.WeiMin(bumpedFee, maxPrice) - if bumpedFee.Cmp(originalFee.AddPercentage(MinimumBumpPercentage)) < 0 { + // The first check is added for the following edge case: + // If originalFee is below 10 wei, then adding the minimum bump percentage won't have any effect on the final value because of rounding down. + // Similarly for bumpedFee, it can have the exact same value as the originalFee, even if we bumped, given an originalFee of less than 10 wei + // and a small BumpPercent. + if bumpedFee.Cmp(originalFee) == 0 || + bumpedFee.Cmp(originalFee.AddPercentage(MinimumBumpPercentage)) < 0 { return nil, fmt.Errorf("%w: %s is bumped less than minimum allowed percentage(%s) from originalFee: %s - maxPrice: %s", commonfee.ErrBump, bumpedFee, strconv.Itoa(MinimumBumpPercentage), originalFee, maxPrice) } From 1ca80e480955e41063747bf48b0fd2c61208ab66 Mon Sep 17 00:00:00 2001 From: Dimitris Date: Fri, 12 Jul 2024 12:49:34 +0300 Subject: [PATCH 10/11] Fix test case --- core/chains/evm/gas/universal_estimator.go | 7 ++++++- core/chains/evm/txmgr/confirmer_test.go | 2 ++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/core/chains/evm/gas/universal_estimator.go b/core/chains/evm/gas/universal_estimator.go index 26186826a5e..6493642ee42 100644 --- a/core/chains/evm/gas/universal_estimator.go +++ b/core/chains/evm/gas/universal_estimator.go @@ -372,7 +372,12 @@ func LimitBumpedFee(originalFee *assets.Wei, currentFee *assets.Wei, bumpedFee * } bumpedFee = assets.WeiMin(bumpedFee, maxPrice) - if bumpedFee.Cmp(originalFee.AddPercentage(MinimumBumpPercentage)) < 0 { + // The first check is added for the following edge case: + // If originalFee is below 10 wei, then adding the minimum bump percentage won't have any effect on the final value because of rounding down. + // Similarly for bumpedFee, it can have the exact same value as the originalFee, even if we bumped, given an originalFee of less than 10 wei + // and a small BumpPercent. + if bumpedFee.Cmp(originalFee) == 0 || + bumpedFee.Cmp(originalFee.AddPercentage(MinimumBumpPercentage)) < 0 { return nil, fmt.Errorf("%w: %s is bumped less than minimum allowed percentage(%s) from originalFee: %s - maxPrice: %s", commonfee.ErrBump, bumpedFee, strconv.Itoa(MinimumBumpPercentage), originalFee, maxPrice) } diff --git a/core/chains/evm/txmgr/confirmer_test.go b/core/chains/evm/txmgr/confirmer_test.go index 31cababc92b..fc00c57a4b3 100644 --- a/core/chains/evm/txmgr/confirmer_test.go +++ b/core/chains/evm/txmgr/confirmer_test.go @@ -2443,6 +2443,8 @@ func TestEthConfirmer_RebroadcastWhereNecessary_TerminallyUnderpriced_ThenGoesTh require.Equal(t, originalBroadcastAt, *etx.BroadcastAt) nonce++ attempt := etx.TxAttempts[0] + _, err := db.Exec(`UPDATE evm.tx_attempts SET gas_price=$1 WHERE id=$2`, evmcfg.EVM().GasEstimator().PriceDefault(), attempt.ID) + require.NoError(t, err) signedTx, err := txmgr.GetGethSignedTx(attempt.SignedRawTx) require.NoError(t, err) From c9fdf713c61571b040dddd4227400e89284ce017 Mon Sep 17 00:00:00 2001 From: Dimitris Date: Fri, 12 Jul 2024 13:37:33 +0300 Subject: [PATCH 11/11] Add changeset --- .changeset/breezy-eggs-glow.md | 5 +++++ core/chains/evm/gas/fixed_price_estimator_test.go | 9 +++------ 2 files changed, 8 insertions(+), 6 deletions(-) create mode 100644 .changeset/breezy-eggs-glow.md diff --git a/.changeset/breezy-eggs-glow.md b/.changeset/breezy-eggs-glow.md new file mode 100644 index 00000000000..711dfdf1abb --- /dev/null +++ b/.changeset/breezy-eggs-glow.md @@ -0,0 +1,5 @@ +--- +"chainlink": minor +--- + +Refactor Fixed Price estimator #internal diff --git a/core/chains/evm/gas/fixed_price_estimator_test.go b/core/chains/evm/gas/fixed_price_estimator_test.go index d5d0ce3cd86..d997190506b 100644 --- a/core/chains/evm/gas/fixed_price_estimator_test.go +++ b/core/chains/evm/gas/fixed_price_estimator_test.go @@ -17,7 +17,7 @@ import ( func Test_FixedPriceEstimator(t *testing.T) { t.Parallel() - maxPrice := assets.NewWeiI(1000000) + globalMaxPrice := assets.NewWeiI(1000000) t.Run("GetLegacyGas returns PriceDefault from config", func(t *testing.T) { config := gasmocks.NewFixedPriceEstimatorConfig(t) @@ -25,7 +25,7 @@ func Test_FixedPriceEstimator(t *testing.T) { l1Oracle := rollupMocks.NewL1Oracle(t) f := gas.NewFixedPriceEstimator(logger.Test(t), config, l1Oracle) - gasPrice, _, err := f.GetLegacyGas(tests.Context(t), nil, 100000, maxPrice) + gasPrice, _, err := f.GetLegacyGas(tests.Context(t), nil, 100000, globalMaxPrice) require.NoError(t, err) assert.Equal(t, assets.NewWeiI(42), gasPrice) }) @@ -71,7 +71,6 @@ func Test_FixedPriceEstimator(t *testing.T) { bumpedGas, _, err := f.BumpLegacyGas(tests.Context(t), originalGasPrice, 100000, maxPrice, nil) require.NoError(t, err) assert.Equal(t, originalGasPrice.AddPercentage(20), bumpedGas) - }) t.Run("BumpLegacyGas bumps original gas price by BumpPercent but caps on max price", func(t *testing.T) { @@ -86,7 +85,6 @@ func Test_FixedPriceEstimator(t *testing.T) { bumpedGas, _, err := f.BumpLegacyGas(tests.Context(t), originalGasPrice, 100000, maxPrice, nil) require.NoError(t, err) assert.Equal(t, maxPrice, bumpedGas) - }) t.Run("BumpLegacyGas bumps original gas price by BumpPercent, caps on max price and errors due to minimum bump percentage", func(t *testing.T) { @@ -100,7 +98,6 @@ func Test_FixedPriceEstimator(t *testing.T) { originalGasPrice := assets.NewWeiI(40) _, _, err := f.BumpLegacyGas(tests.Context(t), originalGasPrice, 100000, maxPrice, nil) require.Error(t, err) - }) t.Run("GetDynamicFee returns TipCapDefault and FeeCapDefault from config", func(t *testing.T) { @@ -110,7 +107,7 @@ func Test_FixedPriceEstimator(t *testing.T) { l1Oracle := rollupMocks.NewL1Oracle(t) f := gas.NewFixedPriceEstimator(logger.Test(t), config, l1Oracle) - dynamicFee, err := f.GetDynamicFee(tests.Context(t), maxPrice) + dynamicFee, err := f.GetDynamicFee(tests.Context(t), globalMaxPrice) require.NoError(t, err) assert.Equal(t, assets.NewWeiI(10), dynamicFee.TipCap) assert.Equal(t, assets.NewWeiI(20), dynamicFee.FeeCap)