From e55e0424a276416cf849a83a1488a287872824c5 Mon Sep 17 00:00:00 2001 From: amit-momin <108959691+amit-momin@users.noreply.github.com> Date: Mon, 10 Jun 2024 09:53:51 -0500 Subject: [PATCH] Refactor BlockHistoryEstimator check to halt excessive bumping (#13297) * Refactored BlockHistoryEstimator check to halt excessive bumping * Added changeset * Fixed linting * Addressed feedback * Reverted halt bumping check to prevent bumping on nil max gas price or tip cap * Updated comment * Fixed test * Cleaned up gwei formatting --- .changeset/shiny-poems-juggle.md | 6 + .../chains/evm/gas/block_history_estimator.go | 286 +++++++----- .../evm/gas/block_history_estimator_test.go | 441 +++++++++++++----- core/chains/evm/gas/helpers_test.go | 16 +- 4 files changed, 523 insertions(+), 226 deletions(-) create mode 100644 .changeset/shiny-poems-juggle.md diff --git a/.changeset/shiny-poems-juggle.md b/.changeset/shiny-poems-juggle.md new file mode 100644 index 00000000000..32fc2069ee7 --- /dev/null +++ b/.changeset/shiny-poems-juggle.md @@ -0,0 +1,6 @@ +--- +"chainlink": minor +--- + +#changed Refactored the BlockHistoryEstimator check to prevent excessively bumping transactions. Check no longer waits for CheckInclusionBlocks to pass before assessing an attempt. +#bugfix Fixed a bug that would use the oldest blocks in the cached history instead of the latest to perform gas estimations. diff --git a/core/chains/evm/gas/block_history_estimator.go b/core/chains/evm/gas/block_history_estimator.go index 12ccedb4b3b..16a66e1c8be 100644 --- a/core/chains/evm/gas/block_history_estimator.go +++ b/core/chains/evm/gas/block_history_estimator.go @@ -2,6 +2,7 @@ package gas import ( "context" + "errors" "fmt" "math/big" "sort" @@ -11,7 +12,6 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/rpc" - pkgerrors "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" @@ -40,21 +40,18 @@ var ( }, []string{"percentile", "evmChainID"}, ) - promBlockHistoryEstimatorAllTipCapPercentiles = promauto.NewGaugeVec(prometheus.GaugeOpts{ Name: "gas_updater_all_tip_cap_percentiles", Help: "Tip cap at given percentile", }, []string{"percentile", "evmChainID"}, ) - promBlockHistoryEstimatorSetGasPrice = promauto.NewGaugeVec(prometheus.GaugeOpts{ Name: "gas_updater_set_gas_price", Help: "Gas updater set gas price (in Wei)", }, []string{"percentile", "evmChainID"}, ) - promBlockHistoryEstimatorSetTipCap = promauto.NewGaugeVec(prometheus.GaugeOpts{ Name: "gas_updater_set_tip_cap", Help: "Gas updater set gas tip cap (in Wei)", @@ -111,12 +108,15 @@ type BlockHistoryEstimator struct { wg *sync.WaitGroup stopCh services.StopChan - gasPrice *assets.Wei - tipCap *assets.Wei - priceMu sync.RWMutex - latest *evmtypes.Head - latestMu sync.RWMutex - initialFetch atomic.Bool + gasPrice *assets.Wei + tipCap *assets.Wei + priceMu sync.RWMutex + maxPercentileGasPrice *assets.Wei + maxPercentileTipCap *assets.Wei + maxPriceMu sync.RWMutex + latest *evmtypes.Head + latestMu sync.RWMutex + initialFetch atomic.Bool logger logger.SugaredLogger @@ -174,15 +174,6 @@ func (b *BlockHistoryEstimator) getCurrentBaseFee() *assets.Wei { return b.latest.BaseFeePerGas } -func (b *BlockHistoryEstimator) getCurrentBlockNum() *int64 { - b.latestMu.RLock() - defer b.latestMu.RUnlock() - if b.latest == nil { - return nil - } - return &b.latest.Number -} - func (b *BlockHistoryEstimator) getBlocks() []evmtypes.Block { b.blocksMu.RLock() defer b.blocksMu.RUnlock() @@ -196,10 +187,10 @@ func (b *BlockHistoryEstimator) Start(ctx context.Context) error { b.logger.Trace("Starting") if b.bhConfig.CheckInclusionBlocks() > 0 { - b.logger.Infof("Inclusion checking enabled, bumping will be prevented on transactions that have been priced above the %d percentile for %d blocks", b.bhConfig.CheckInclusionPercentile(), b.bhConfig.CheckInclusionBlocks()) + b.logger.Infof("Inclusion checking enabled, bumping will be prevented on transactions that have been priced above the %d percentile of transactions in the latest %d blocks", b.bhConfig.CheckInclusionPercentile(), b.bhConfig.CheckInclusionBlocks()) } if b.bhConfig.BlockHistorySize() == 0 { - return pkgerrors.New("BlockHistorySize must be set to a value greater than 0") + return errors.New("BlockHistorySize must be set to a value greater than 0") } fetchCtx, cancel := context.WithTimeout(ctx, MaxStartTime) @@ -217,7 +208,7 @@ func (b *BlockHistoryEstimator) Start(ctx context.Context) error { // NOTE: This only checks the start context, not the fetch context if ctx.Err() != nil { - return pkgerrors.Wrap(ctx.Err(), "failed to start BlockHistoryEstimator due to main context error") + return fmt.Errorf("failed to start BlockHistoryEstimator due to main context error: %w", ctx.Err()) } b.wg.Add(1) @@ -252,11 +243,11 @@ func (b *BlockHistoryEstimator) GetLegacyGas(_ context.Context, _ []byte, gasLim gasPrice = b.getGasPrice() }) if !ok { - return nil, 0, pkgerrors.New("BlockHistoryEstimator is not started; cannot estimate gas") + return nil, 0, errors.New("BlockHistoryEstimator is not started; cannot estimate gas") } if gasPrice == nil { if !b.initialFetch.Load() { - return nil, 0, pkgerrors.New("BlockHistoryEstimator has not finished the first gas estimation yet, likely because a failure on start") + return nil, 0, errors.New("BlockHistoryEstimator has not finished the first gas estimation yet, likely because a failure on start") } b.logger.Warnw("Failed to estimate gas price. This is likely because there aren't any valid transactions to estimate from."+ "Using Evm.GasEstimator.PriceDefault as fallback.", "blocks", b.getBlockHistoryNumbers()) @@ -273,6 +264,18 @@ func (b *BlockHistoryEstimator) getGasPrice() *assets.Wei { return b.gasPrice } +func (b *BlockHistoryEstimator) getMaxPercentileGasPrice() *assets.Wei { + b.maxPriceMu.RLock() + defer b.maxPriceMu.RUnlock() + return b.maxPercentileGasPrice +} + +func (b *BlockHistoryEstimator) setMaxPercentileGasPrice(gasPrice *assets.Wei) { + b.maxPriceMu.Lock() + defer b.maxPriceMu.Unlock() + b.maxPercentileGasPrice = gasPrice +} + func (b *BlockHistoryEstimator) getBlockHistoryNumbers() (numsInHistory []int64) { for _, b := range b.blocks { numsInHistory = append(numsInHistory, b.Number) @@ -286,10 +289,22 @@ func (b *BlockHistoryEstimator) getTipCap() *assets.Wei { return b.tipCap } +func (b *BlockHistoryEstimator) getMaxPercentileTipCap() *assets.Wei { + b.maxPriceMu.RLock() + defer b.maxPriceMu.RUnlock() + return b.maxPercentileTipCap +} + +func (b *BlockHistoryEstimator) setMaxPercentileTipCap(tipCap *assets.Wei) { + b.maxPriceMu.Lock() + defer b.maxPriceMu.Unlock() + b.maxPercentileTipCap = tipCap +} + func (b *BlockHistoryEstimator) BumpLegacyGas(_ context.Context, originalGasPrice *assets.Wei, gasLimit uint64, maxGasPriceWei *assets.Wei, attempts []EvmPriorAttempt) (bumpedGasPrice *assets.Wei, chainSpecificGasLimit uint64, err error) { if b.bhConfig.CheckInclusionBlocks() > 0 { - if err = b.checkConnectivity(attempts); err != nil { - if pkgerrors.Is(err, commonfee.ErrConnectivity) { + if err = b.haltBumping(attempts); err != nil { + if errors.Is(err, commonfee.ErrConnectivity) { b.logger.Criticalw(BumpingHaltedLabel, "err", err) b.SvcErrBuffer.Append(err) promBlockHistoryEstimatorConnectivityFailureCount.WithLabelValues(b.chainID.String(), "legacy").Inc() @@ -304,76 +319,59 @@ func (b *BlockHistoryEstimator) BumpLegacyGas(_ context.Context, originalGasPric return bumpedGasPrice, gasLimit, err } -// checkConnectivity detects if the transaction is not being included due to -// some kind of mempool propagation or connectivity issue rather than -// insufficiently high pricing and returns error if so -func (b *BlockHistoryEstimator) checkConnectivity(attempts []EvmPriorAttempt) error { +// haltBumping prevents transactions from excessively bumping if an existing attempt's price is above a configurable percentile +// This check is required in case the transaction is not being included due to some kind of mempool propagation +// or connectivity issue rather than insufficiently high pricing +func (b *BlockHistoryEstimator) haltBumping(attempts []EvmPriorAttempt) error { percentile := int(b.bhConfig.CheckInclusionPercentile()) - // how many blocks since broadcast? - latestBlockNum := b.getCurrentBlockNum() - if latestBlockNum == nil { - b.logger.Warn("Latest block is unknown; skipping inclusion check") - // can't determine anything if we don't have/know latest block num yet - return nil + // Get latest CheckInclusionPercentile prices to use in the checks below + var maxGasPrice *assets.Wei + var maxTipCap *assets.Wei + ok := b.IfStarted(func() { + maxGasPrice = b.getMaxPercentileGasPrice() + maxTipCap = b.getMaxPercentileTipCap() + }) + if !ok { + return errors.New("BlockHistoryEstimator is not started; do not have max gas to allow bumping") } - expectInclusionWithinBlocks := int(b.bhConfig.CheckInclusionBlocks()) - blockHistory := b.getBlocks() - if len(blockHistory) < expectInclusionWithinBlocks { - b.logger.Warnf("Block history in memory with length %d is insufficient to determine whether transaction should have been included within the past %d blocks", len(blockHistory), b.bhConfig.CheckInclusionBlocks()) - return nil + if !b.initialFetch.Load() { + return errors.New("BlockHistoryEstimator has not finished the first gas estimation yet, likely because a failure on start") + } + // Return error to prevent bumping if gas price is nil or if EIP1559 is enabled and tip cap is nil + if maxGasPrice == nil || (b.eConfig.EIP1559DynamicFees() && maxTipCap == nil) { + errorMsg := fmt.Sprintf("%d percentile price is not set. This is likely because there aren't any valid transactions to estimate from. Preventing bumping until valid price is available to compare", percentile) + b.logger.Debugf(errorMsg) + return errors.New(errorMsg) } + // Get the latest CheckInclusionBlocks from block history for fee cap check below + blockHistory := b.getBlocks() + blockRange := mathutil.Min(len(blockHistory), int(b.bhConfig.CheckInclusionBlocks())) + startIdx := len(blockHistory) - blockRange + checkInclusionBlocks := blockHistory[startIdx:] + // Check each attempt for any with a gas price or tip cap (if EIP1559 type) exceeds the latest CheckInclusionPercentile prices for _, attempt := range attempts { if attempt.BroadcastBeforeBlockNum == nil { // this shouldn't happen; any broadcast attempt ought to have a // BroadcastBeforeBlockNum otherwise its an assumption violation - return pkgerrors.Errorf("BroadcastBeforeBlockNum was unexpectedly nil for attempt %s", attempt.TxHash) - } - broadcastBeforeBlockNum := *attempt.BroadcastBeforeBlockNum - blocksSinceBroadcast := *latestBlockNum - broadcastBeforeBlockNum - if blocksSinceBroadcast < int64(expectInclusionWithinBlocks) { - // only check attempts that have been waiting around longer than - // CheckInclusionBlocks - continue + return fmt.Errorf("BroadcastBeforeBlockNum was unexpectedly nil for attempt %s", attempt.TxHash) } - // has not been included for at least the required number of blocks - b.logger.Debugw(fmt.Sprintf("transaction %s has been pending inclusion for %d blocks which equals or exceeds expected specified check inclusion blocks of %d", attempt.TxHash, blocksSinceBroadcast, expectInclusionWithinBlocks), "broadcastBeforeBlockNum", broadcastBeforeBlockNum, "latestBlockNum", *latestBlockNum) - // is the price in the right percentile for all of these blocks? - var blocks []evmtypes.Block - l := expectInclusionWithinBlocks - // reverse order since we want to go highest -> lowest block number and bail out early - for i := l - 1; i >= 0; i-- { - block := blockHistory[i] - if block.Number < broadcastBeforeBlockNum { - break - } - blocks = append(blocks, block) - } - var eip1559 bool + var attemptEip1559 bool switch attempt.TxType { case 0x0, 0x1: - eip1559 = false + attemptEip1559 = false case 0x2: - eip1559 = true + attemptEip1559 = true default: - return pkgerrors.Errorf("attempt %s has unknown transaction type 0x%d", attempt.TxHash, attempt.TxType) + return fmt.Errorf("attempt %s has unknown transaction type 0x%d", attempt.TxHash, attempt.TxType) } - gasPrice, tipCap, err := b.calculatePercentilePrices(blocks, percentile, eip1559, nil, nil) - if err != nil { - if pkgerrors.Is(err, ErrNoSuitableTransactions) { - b.logger.Warnf("no suitable transactions found to verify if transaction %s has been included within expected inclusion blocks of %d", attempt.TxHash, expectInclusionWithinBlocks) - return nil - } - b.logger.AssumptionViolationw("unexpected error while verifying transaction inclusion", "err", err, "txHash", attempt.TxHash.String()) - return nil - } - if !eip1559 { - if attempt.GasPrice.Cmp(gasPrice) > 0 { - return pkgerrors.Wrapf(commonfee.ErrConnectivity, "transaction %s has gas price of %s, which is above percentile=%d%% (percentile price: %s) for blocks %d thru %d (checking %d blocks)", attempt.TxHash, attempt.GasPrice, percentile, gasPrice, blockHistory[l-1].Number, blockHistory[0].Number, expectInclusionWithinBlocks) + if !attemptEip1559 { + if attempt.GasPrice.Cmp(maxGasPrice) > 0 { + return fmt.Errorf("transaction %s has gas price of %s, which is above percentile=%d%% (percentile price: %s): %w", attempt.TxHash, attempt.GasPrice, percentile, maxGasPrice, commonfee.ErrConnectivity) } continue } sufficientFeeCap := true - for _, b := range blocks { + for _, b := range checkInclusionBlocks { // feecap must >= tipcap+basefee for the block, otherwise there // is no way this could have been included, and we must bail // out of the check @@ -384,8 +382,8 @@ func (b *BlockHistoryEstimator) checkConnectivity(attempts []EvmPriorAttempt) er break } } - if sufficientFeeCap && attempt.DynamicFee.TipCap.Cmp(tipCap) > 0 { - return pkgerrors.Wrapf(commonfee.ErrConnectivity, "transaction %s has tip cap of %s, which is above percentile=%d%% (percentile tip cap: %s) for blocks %d thru %d (checking %d blocks)", attempt.TxHash, attempt.DynamicFee.TipCap, percentile, tipCap, blockHistory[l-1].Number, blockHistory[0].Number, expectInclusionWithinBlocks) + if sufficientFeeCap && attempt.DynamicFee.TipCap.Cmp(maxTipCap) > 0 { + return fmt.Errorf("transaction %s has tip cap of %s, which is above percentile=%d%% (percentile tip cap: %s): %w", attempt.TxHash, attempt.DynamicFee.TipCap, percentile, maxTipCap, commonfee.ErrConnectivity) } } return nil @@ -393,7 +391,7 @@ func (b *BlockHistoryEstimator) checkConnectivity(attempts []EvmPriorAttempt) er func (b *BlockHistoryEstimator) GetDynamicFee(_ context.Context, maxGasPriceWei *assets.Wei) (fee DynamicFee, err error) { if !b.eConfig.EIP1559DynamicFees() { - return fee, pkgerrors.New("Can't get dynamic fee, EIP1559 is disabled") + return fee, errors.New("can't get dynamic fee, EIP1559 is disabled") } var feeCap *assets.Wei @@ -404,7 +402,7 @@ func (b *BlockHistoryEstimator) GetDynamicFee(_ context.Context, maxGasPriceWei tipCap = b.tipCap if tipCap == nil { if !b.initialFetch.Load() { - err = pkgerrors.New("BlockHistoryEstimator has not finished the first gas estimation yet, likely because a failure on start") + err = errors.New("BlockHistoryEstimator has not finished the first gas estimation yet, likely because a failure on start") return } b.logger.Warnw("Failed to estimate gas price. This is likely because there aren't any valid transactions to estimate from."+ @@ -425,12 +423,12 @@ func (b *BlockHistoryEstimator) GetDynamicFee(_ context.Context, maxGasPriceWei // This shouldn't happen on EIP-1559 blocks, since if the tip cap // is set, Start must have succeeded and we would expect an initial // base fee to be set as well - err = pkgerrors.New("BlockHistoryEstimator: no value for latest block base fee; cannot estimate EIP-1559 base fee. Are you trying to run with EIP1559 enabled on a non-EIP1559 chain?") + err = errors.New("BlockHistoryEstimator: no value for latest block base fee; cannot estimate EIP-1559 base fee. Are you trying to run with EIP1559 enabled on a non-EIP1559 chain?") return } }) if !ok { - return fee, pkgerrors.New("BlockHistoryEstimator is not started; cannot estimate gas") + return fee, errors.New("BlockHistoryEstimator is not started; cannot estimate gas") } if err != nil { return fee, err @@ -462,8 +460,8 @@ func calcFeeCap(latestAvailableBaseFeePerGas *assets.Wei, bufferBlocks int, tipC func (b *BlockHistoryEstimator) BumpDynamicFee(_ context.Context, originalFee DynamicFee, maxGasPriceWei *assets.Wei, attempts []EvmPriorAttempt) (bumped DynamicFee, err error) { if b.bhConfig.CheckInclusionBlocks() > 0 { - if err = b.checkConnectivity(attempts); err != nil { - if pkgerrors.Is(err, commonfee.ErrConnectivity) { + if err = b.haltBumping(attempts); err != nil { + if errors.Is(err, commonfee.ErrConnectivity) { b.logger.Criticalw(BumpingHaltedLabel, "err", err) b.SvcErrBuffer.Append(err) promBlockHistoryEstimatorConnectivityFailureCount.WithLabelValues(b.chainID.String(), "eip1559").Inc() @@ -506,8 +504,6 @@ func (b *BlockHistoryEstimator) FetchBlocksAndRecalculate(ctx context.Context, h // Recalculate adds the given heads to the history and recalculates gas price. func (b *BlockHistoryEstimator) Recalculate(head *evmtypes.Head) { - percentile := int(b.bhConfig.TransactionPercentile()) - lggr := b.logger.With("head", head) blockHistory := b.getBlocks() @@ -516,10 +512,21 @@ func (b *BlockHistoryEstimator) Recalculate(head *evmtypes.Head) { return } - l := mathutil.Min(len(blockHistory), int(b.bhConfig.BlockHistorySize())) - blocks := blockHistory[:l] + // Calculate and set the TransactionPercentile gas price and tip cap to use during gas estimation + b.calculateGasPriceTipCap(lggr, blockHistory, head) + + // Calculate and set the CheckInclusionPercentile gas price and tip cap to halt excessive bumping + b.calculateMaxPercentileGasPriceTipCap(lggr, blockHistory, head) +} +func (b *BlockHistoryEstimator) calculateGasPriceTipCap(lggr logger.SugaredLogger, blockHistory []evmtypes.Block, head *evmtypes.Head) { + percentile := int(b.bhConfig.TransactionPercentile()) eip1559 := b.eConfig.EIP1559DynamicFees() + + blockRange := mathutil.Min(len(blockHistory), int(b.bhConfig.BlockHistorySize())) + startIdx := len(blockHistory) - blockRange + blocks := blockHistory[startIdx:] + percentileGasPrice, percentileTipCap, err := b.calculatePercentilePrices(blocks, percentile, eip1559, func(gasPrices []*assets.Wei) { for i := 0; i <= 100; i += 5 { @@ -533,7 +540,7 @@ func (b *BlockHistoryEstimator) Recalculate(head *evmtypes.Head) { } }) if err != nil { - if pkgerrors.Is(err, ErrNoSuitableTransactions) { + if errors.Is(err, ErrNoSuitableTransactions) { lggr.Debug("No suitable transactions, skipping") } else { lggr.Warnw("Cannot calculate percentile prices", "err", err) @@ -541,32 +548,28 @@ func (b *BlockHistoryEstimator) Recalculate(head *evmtypes.Head) { return } - var numsInHistory []int64 - for _, b := range blockHistory { - numsInHistory = append(numsInHistory, b.Number) + var numsForPrice []int64 + for _, b := range blocks { + numsForPrice = append(numsForPrice, b.Number) } - float := new(big.Float).SetInt(percentileGasPrice.ToInt()) - gwei, _ := big.NewFloat(0).Quo(float, big.NewFloat(1000000000)).Float64() - gasPriceGwei := fmt.Sprintf("%.2f", gwei) - + gasPriceGwei := percentileGasPrice.Text("gwei") lggrFields := []interface{}{ "gasPriceWei", percentileGasPrice, "gasPriceGWei", gasPriceGwei, "maxGasPriceWei", b.eConfig.PriceMax(), "headNum", head.Number, - "blocks", numsInHistory, + "priceBlocks", numsForPrice, } b.setPercentileGasPrice(percentileGasPrice) promBlockHistoryEstimatorSetGasPrice.WithLabelValues(fmt.Sprintf("%v%%", percentile), b.chainID.String()).Set(float64(percentileGasPrice.Int64())) if !eip1559 { - lggr.Debugw(fmt.Sprintf("Setting new default gas price: %v Gwei", gasPriceGwei), lggrFields...) + lggr.Debugw(fmt.Sprintf("Setting new default GasPrice: %v Gwei", gasPriceGwei), lggrFields...) return } - float = new(big.Float).SetInt(percentileTipCap.ToInt()) - gwei, _ = big.NewFloat(0).Quo(float, big.NewFloat(1000000000)).Float64() - tipCapGwei := fmt.Sprintf("%.2f", gwei) + + tipCapGwei := percentileTipCap.Text("gwei") lggrFields = append(lggrFields, []interface{}{ "tipCapWei", percentileTipCap, "tipCapGwei", tipCapGwei, @@ -576,6 +579,56 @@ func (b *BlockHistoryEstimator) Recalculate(head *evmtypes.Head) { promBlockHistoryEstimatorSetTipCap.WithLabelValues(fmt.Sprintf("%v%%", percentile), b.chainID.String()).Set(float64(percentileTipCap.Int64())) } +func (b *BlockHistoryEstimator) calculateMaxPercentileGasPriceTipCap(lggr logger.SugaredLogger, blockHistory []evmtypes.Block, head *evmtypes.Head) { + if b.bhConfig.CheckInclusionBlocks() <= 0 { + return + } + percentile := int(b.bhConfig.CheckInclusionPercentile()) + eip1559 := b.eConfig.EIP1559DynamicFees() + + blockRange := mathutil.Min(len(blockHistory), int(b.bhConfig.CheckInclusionBlocks())) + startIdx := len(blockHistory) - blockRange + checkInclusionBlocks := blockHistory[startIdx:] + + maxPercentileGasPrice, maxPercentileTipCap, err := b.calculatePercentilePrices(checkInclusionBlocks, percentile, eip1559, nil, nil) + if err != nil { + if errors.Is(err, ErrNoSuitableTransactions) { + lggr.Debug("No suitable transactions found to calculate the max percentile prices, skipping") + } else { + lggr.Warnw("Cannot calculate max percentile prices", "err", err) + } + return + } + + var numsForMaxPrices []int64 + for _, b := range checkInclusionBlocks { + numsForMaxPrices = append(numsForMaxPrices, b.Number) + } + + maxPercentileGasPriceGwei := maxPercentileGasPrice.Text("gwei") + lggrFields := []interface{}{ + "maxPercentileGasPriceWei", maxPercentileGasPrice, + "maxPercentileGasPriceGwei", maxPercentileGasPriceGwei, + "headNum", head.Number, + "maxPercentileBlocks", numsForMaxPrices, + } + + b.setMaxPercentileGasPrice(maxPercentileGasPrice) + + if !eip1559 { + lggr.Debugw(fmt.Sprintf("Setting new max percentile GasPrice: %v Gwei", maxPercentileGasPriceGwei), lggrFields...) + return + } + + maxPercentileTipCapGwei := maxPercentileTipCap.Text("gwei") + lggrFields = append(lggrFields, []interface{}{ + "maxPercentileTipCapWei", maxPercentileTipCap, + "maxPercentileTipCapGwei", maxPercentileTipCapGwei, + }...) + lggr.Debugw(fmt.Sprintf("Setting new default prices, max percentile GasPrice: %v Gwei, max percentile TipCap: %v Gwei", maxPercentileGasPriceGwei, maxPercentileTipCapGwei), lggrFields...) + b.setMaxPercentileTipCap(maxPercentileTipCap) +} + // FetchBlocks fetches block history leading up to the given head. func (b *BlockHistoryEstimator) FetchBlocks(ctx context.Context, head *evmtypes.Head) error { // HACK: blockDelay is the number of blocks that the block history estimator trails behind head. @@ -588,12 +641,12 @@ func (b *BlockHistoryEstimator) FetchBlocks(ctx context.Context, head *evmtypes. historySize := b.size if historySize <= 0 { - return pkgerrors.Errorf("BlockHistoryEstimator: history size must be > 0, got: %d", historySize) + return fmt.Errorf("BlockHistoryEstimator: history size must be > 0, got: %d", historySize) } highestBlockToFetch := head.Number - blockDelay if highestBlockToFetch < 0 { - return pkgerrors.Errorf("BlockHistoryEstimator: cannot fetch, current block height %v is lower than EVM.RPCBlockQueryDelay=%v", head.Number, blockDelay) + return fmt.Errorf("BlockHistoryEstimator: cannot fetch, current block height %v is lower than EVM.RPCBlockQueryDelay=%v", head.Number, blockDelay) } lowestBlockToFetch := head.Number - historySize - blockDelay + 1 if lowestBlockToFetch < 0 { @@ -641,7 +694,7 @@ func (b *BlockHistoryEstimator) FetchBlocks(ctx context.Context, head *evmtypes. for _, req := range reqs { result, err := req.Result, req.Error if err != nil { - if pkgerrors.Is(err, evmtypes.ErrMissingBlock) { + if errors.Is(err, evmtypes.ErrMissingBlock) { num := HexToInt64(req.Args[0]) missingBlocks = append(missingBlocks, num) lggr.Debugw( @@ -657,10 +710,10 @@ func (b *BlockHistoryEstimator) FetchBlocks(ctx context.Context, head *evmtypes. block, is := result.(*evmtypes.Block) if !is { - return pkgerrors.Errorf("expected result to be a %T, got %T", &evmtypes.Block{}, result) + return fmt.Errorf("expected result to be a %T, got %T", &evmtypes.Block{}, result) } if block == nil { - return pkgerrors.New("invariant violation: got nil block") + return errors.New("invariant violation: got nil block") } if block.Hash == (common.Hash{}) { lggr.Warnw("Block was missing hash", "block", b, "headNum", head.Number, "blockNum", block.Number) @@ -716,26 +769,26 @@ func (b *BlockHistoryEstimator) batchFetch(ctx context.Context, reqs []rpc.Batch b.logger.Tracew(fmt.Sprintf("Batch fetching blocks %v thru %v", HexToInt64(reqs[i].Args[0]), HexToInt64(reqs[j-1].Args[0]))) err := b.ethClient.BatchCallContext(ctx, reqs[i:j]) - if pkgerrors.Is(err, context.DeadlineExceeded) { + if errors.Is(err, context.DeadlineExceeded) { // We ran out of time, return what we have b.logger.Warnf("Batch fetching timed out; loaded %d/%d results: %v", i, len(reqs), err) for k := i; k < len(reqs); k++ { if k < j { - reqs[k].Error = pkgerrors.Wrap(err, "request failed") + reqs[k].Error = fmt.Errorf("request failed: %w", err) } else { - reqs[k].Error = pkgerrors.Wrap(err, "request skipped; previous request exceeded deadline") + reqs[k].Error = fmt.Errorf("request skipped; previous request exceeded deadline: %w", err) } } return nil } else if err != nil { - return pkgerrors.Wrap(err, "BlockHistoryEstimator#fetchBlocks error fetching blocks with BatchCallContext") + return fmt.Errorf("BlockHistoryEstimator#fetchBlocks error fetching blocks with BatchCallContext: %w", err) } } return nil } var ( - ErrNoSuitableTransactions = pkgerrors.New("no suitable transactions") + ErrNoSuitableTransactions = errors.New("no suitable transactions") ) func (b *BlockHistoryEstimator) calculatePercentilePrices(blocks []evmtypes.Block, percentile int, eip1559 bool, f func(gasPrices []*assets.Wei), f2 func(tipCaps []*assets.Wei)) (gasPrice, tipCap *assets.Wei, err error) { @@ -796,7 +849,7 @@ func (b *BlockHistoryEstimator) getPricesFromBlocks(blocks []evmtypes.Block, eip func verifyBlock(block evmtypes.Block, eip1559 bool) error { if eip1559 && block.BaseFeePerGas == nil { - return pkgerrors.New("EIP-1559 mode was enabled, but block was missing baseFeePerGas") + return errors.New("EIP-1559 mode was enabled, but block was missing baseFeePerGas") } return nil } @@ -896,8 +949,7 @@ func (b *BlockHistoryEstimator) getEffectiveGasPrice(block evmtypes.Block, tx ev priorityFeePerGas = maxFeeMinusBaseFee } - effectiveGasPrice := priorityFeePerGas.Add(block.BaseFeePerGas) - return effectiveGasPrice + return priorityFeePerGas.Add(block.BaseFeePerGas) } func (b *BlockHistoryEstimator) EffectiveTipCap(block evmtypes.Block, tx evmtypes.Transaction) *assets.Wei { diff --git a/core/chains/evm/gas/block_history_estimator_test.go b/core/chains/evm/gas/block_history_estimator_test.go index 4f3fddd562f..714d82aa8c6 100644 --- a/core/chains/evm/gas/block_history_estimator_test.go +++ b/core/chains/evm/gas/block_history_estimator_test.go @@ -721,6 +721,9 @@ func TestBlockHistoryEstimator_FetchBlocksAndRecalculate_NoEIP1559(t *testing.T) bhCfg.TransactionPercentileF = uint16(35) bhCfg.BlockHistorySizeF = uint16(3) bhCfg.BatchSizeF = uint32(0) + // Set CheckInclusionBlocks larger than BlockHistorySize to cache more blocks than needed for calculation + // Helps test whether the latest block is being used or not + bhCfg.CheckInclusionBlocksF = uint16(5) geCfg := &gas.MockGasEstimatorConfig{} geCfg.EIP1559DynamicFeesF = false @@ -729,39 +732,116 @@ func TestBlockHistoryEstimator_FetchBlocksAndRecalculate_NoEIP1559(t *testing.T) bhe := newBlockHistoryEstimator(t, ethClient, cfg, geCfg, bhCfg, l1Oracle) + b0 := evmtypes.Block{ + Number: 0, + Hash: utils.NewHash(), + Transactions: legacyTransactionsFromGasPrices(1, 2, 3), + } b1 := evmtypes.Block{ Number: 1, Hash: utils.NewHash(), - Transactions: legacyTransactionsFromGasPrices(1), + Transactions: legacyTransactionsFromGasPrices(4, 5, 6), } b2 := evmtypes.Block{ Number: 2, Hash: utils.NewHash(), - Transactions: legacyTransactionsFromGasPrices(2), + Transactions: legacyTransactionsFromGasPrices(7, 8, 9), } b3 := evmtypes.Block{ Number: 3, Hash: utils.NewHash(), + Transactions: legacyTransactionsFromGasPrices(100), + } + b4 := evmtypes.Block{ + Number: 4, + Hash: utils.NewHash(), Transactions: legacyTransactionsFromGasPrices(200, 300, 100, 100, 100, 100), } ethClient.On("BatchCallContext", mock.Anything, mock.MatchedBy(func(b []rpc.BatchElem) bool { - return len(b) == 3 && - b[0].Args[0] == "0x3" && - b[1].Args[0] == "0x2" && - b[2].Args[0] == "0x1" + return len(b) == 5 && + b[0].Args[0] == "0x4" && + b[1].Args[0] == "0x3" && + b[2].Args[0] == "0x2" && + b[3].Args[0] == "0x1" && + b[4].Args[0] == "0x0" })).Return(nil).Run(func(args mock.Arguments) { elems := args.Get(1).([]rpc.BatchElem) - elems[0].Result = &b3 - elems[1].Result = &b2 - elems[2].Result = &b1 + elems[0].Result = &b4 + elems[1].Result = &b3 + elems[2].Result = &b2 + elems[3].Result = &b1 + elems[4].Result = &b0 }) - bhe.FetchBlocksAndRecalculate(tests.Context(t), testutils.Head(3)) + bhe.FetchBlocksAndRecalculate(tests.Context(t), testutils.Head(4)) price := gas.GetGasPrice(bhe) require.Equal(t, assets.NewWeiI(100), price) + assert.Len(t, gas.GetRollingBlockHistory(bhe), 5) +} + +func TestBlockHistoryEstimator_FetchBlocksAndRecalculate_EIP1559(t *testing.T) { + t.Parallel() + + ethClient := testutils.NewEthClientMockWithDefaultChain(t) + l1Oracle := rollupMocks.NewL1Oracle(t) + + cfg := gas.NewMockConfig() + bhCfg := newBlockHistoryConfig() + bhCfg.BlockDelayF = uint16(0) + bhCfg.TransactionPercentileF = uint16(50) + bhCfg.BlockHistorySizeF = uint16(1) + bhCfg.BatchSizeF = uint32(0) + // Set CheckInclusionBlocks larger than BlockHistorySize to cache more blocks than needed for calculation + // Helps test whether the latest block is being used or not + bhCfg.CheckInclusionBlocksF = uint16(3) + + geCfg := &gas.MockGasEstimatorConfig{} + geCfg.EIP1559DynamicFeesF = true + geCfg.PriceMaxF = assets.NewWeiI(1000) + geCfg.PriceMinF = assets.NewWeiI(0) + geCfg.TipCapMinF = assets.NewWeiI(0) + + bhe := newBlockHistoryEstimator(t, ethClient, cfg, geCfg, bhCfg, l1Oracle) + + b0 := evmtypes.Block{ + BaseFeePerGas: assets.NewWeiI(1), + Number: 0, + Hash: utils.NewHash(), + Transactions: dynamicFeeTransactionsFromTipCaps(1, 2, 3), + } + b1 := evmtypes.Block{ + BaseFeePerGas: assets.NewWeiI(2), + Number: 1, + Hash: utils.NewHash(), + Transactions: dynamicFeeTransactionsFromTipCaps(4, 5, 6), + } + b2 := evmtypes.Block{ + BaseFeePerGas: assets.NewWeiI(3), + Number: 2, + Hash: utils.NewHash(), + Transactions: dynamicFeeTransactionsFromTipCaps(7, 8, 9), + } + + ethClient.On("BatchCallContext", mock.Anything, mock.MatchedBy(func(b []rpc.BatchElem) bool { + return len(b) == 3 && + b[0].Args[0] == "0x2" && + b[1].Args[0] == "0x1" && + b[2].Args[0] == "0x0" + })).Return(nil).Run(func(args mock.Arguments) { + elems := args.Get(1).([]rpc.BatchElem) + elems[0].Result = &b2 + elems[1].Result = &b1 + elems[2].Result = &b0 + }) + + bhe.FetchBlocksAndRecalculate(tests.Context(t), testutils.Head(2)) + + price := gas.GetTipCap(bhe) + require.Equal(t, assets.NewWeiI(8), price) // 50th percentile of latest block + assert.Len(t, gas.GetRollingBlockHistory(bhe), 3) } @@ -2125,78 +2205,82 @@ func TestBlockHistoryEstimator_GetDynamicFee(t *testing.T) { }) } -func TestBlockHistoryEstimator_CheckConnectivity(t *testing.T) { +func TestBlockHistoryEstimator_HaltBumping(t *testing.T) { cfg := gas.NewMockConfig() bhCfg := newBlockHistoryConfig() bhCfg.CheckInclusionBlocksF = uint16(4) - lggr, obs := logger.TestObserved(t, zapcore.DebugLevel) + bhCfg.CheckInclusionPercentileF = uint16(90) + lggr, _ := logger.TestObserved(t, zapcore.DebugLevel) geCfg := &gas.MockGasEstimatorConfig{} geCfg.EIP1559DynamicFeesF = false + geCfg.PriceMinF = assets.NewWeiI(1) + geCfg.PriceMaxF = assets.NewWeiI(100) l1Oracle := rollupMocks.NewL1Oracle(t) + ethClient := testutils.NewEthClientMockWithDefaultChain(t) + ctx := tests.Context(t) bhe := gas.BlockHistoryEstimatorFromInterface( - gas.NewBlockHistoryEstimator(lggr, nil, cfg, geCfg, bhCfg, testutils.NewRandomEVMChainID(), l1Oracle), + gas.NewBlockHistoryEstimator(lggr, ethClient, cfg, geCfg, bhCfg, testutils.NewRandomEVMChainID(), l1Oracle), ) attempts := []gas.EvmPriorAttempt{ {TxType: 0x0, TxHash: NewEvmHash()}, } - t.Run("skips connectivity check if latest block is not present", func(t *testing.T) { - err := bhe.CheckConnectivity(attempts) - require.NoError(t, err) - - tests.AssertLogEventually(t, obs, "Latest block is unknown; skipping inclusion check") + t.Run("fails halt bumping check if estimator is not started", func(t *testing.T) { + err := bhe.HaltBumping(attempts) + require.Error(t, err, "BlockHistoryEstimator is not started; do not have max gas to allow bumping") }) - h := testutils.Head(1) - h.BaseFeePerGas = assets.NewWeiI(112500) - bhe.OnNewLongestChain(tests.Context(t), h) - + h := testutils.Head(0) b0 := evmtypes.Block{ Number: 0, Hash: utils.NewHash(), Transactions: legacyTransactionsFromGasPrices(), } - gas.SetRollingBlockHistory(bhe, []evmtypes.Block{b0}) - - t.Run("skips connectivity check if block history has insufficient size", func(t *testing.T) { - err := bhe.CheckConnectivity(attempts) - require.NoError(t, err) - - tests.AssertLogEventually(t, obs, "Block history in memory with length 1 is insufficient to determine whether transaction should have been included within the past 4 blocks") - }) + ethClient.On("HeadByNumber", mock.Anything, (*big.Int)(nil)).Return(h, nil) + ethClient.On("BatchCallContext", mock.Anything, mock.MatchedBy(func(b []rpc.BatchElem) bool { + return len(b) == 1 + })).Return(nil).Run(func(args mock.Arguments) { + elems := args.Get(1).([]rpc.BatchElem) + elems[0].Result = &b0 + }).Once() + err := bhe.Start(ctx) + require.NoError(t, err) - t.Run("skips connectivity check if attempts is nil or empty", func(t *testing.T) { - err := bhe.CheckConnectivity(nil) - require.NoError(t, err) + t.Run("allows bumping check if CheckInclusionPercentile price failed to set due to no suitable transactions", func(t *testing.T) { + err := bhe.HaltBumping(attempts) + require.Error(t, err, "90 percentile price is not set. This is likely because there aren't any valid transactions to estimate from. Preventing bumping until valid price is available to compare") }) b1 := evmtypes.Block{ Number: 1, Hash: utils.NewHash(), ParentHash: b0.Hash, - Transactions: legacyTransactionsFromGasPrices(), + Transactions: legacyTransactionsFromGasPrices(10, 20, 30, 40, 50), } b2 := evmtypes.Block{ Number: 2, Hash: utils.NewHash(), ParentHash: b1.Hash, - Transactions: legacyTransactionsFromGasPrices(), + Transactions: legacyTransactionsFromGasPrices(60, 70, 80), } b3 := evmtypes.Block{ Number: 3, Hash: utils.NewHash(), ParentHash: b2.Hash, - Transactions: legacyTransactionsFromGasPrices(), + Transactions: legacyTransactionsFromGasPrices(90, 100), } gas.SetRollingBlockHistory(bhe, []evmtypes.Block{b0, b1, b2, b3}) - h = testutils.Head(5) - h.BaseFeePerGas = assets.NewWeiI(112500) - bhe.OnNewLongestChain(tests.Context(t), h) + bhe.Recalculate(testutils.Head(3)) + + t.Run("skips halt bumping check if attempts is nil or empty", func(t *testing.T) { + err := bhe.HaltBumping(nil) + require.NoError(t, err) + }) t.Run("returns error if one of the supplied attempts is missing BroadcastBeforeBlockNum", func(t *testing.T) { - err := bhe.CheckConnectivity(attempts) + err := bhe.HaltBumping(attempts) require.Error(t, err) assert.Contains(t, err.Error(), fmt.Sprintf("BroadcastBeforeBlockNum was unexpectedly nil for attempt %s", attempts[0].TxHash)) }) @@ -2208,7 +2292,7 @@ func TestBlockHistoryEstimator_CheckConnectivity(t *testing.T) { } t.Run("returns error if one of the supplied attempts has an unknown transaction type", func(t *testing.T) { - err := bhe.CheckConnectivity(attempts) + err := bhe.HaltBumping(attempts) require.Error(t, err) assert.Contains(t, err.Error(), fmt.Sprintf("attempt %s has unknown transaction type 0x3", hash)) }) @@ -2217,13 +2301,6 @@ func TestBlockHistoryEstimator_CheckConnectivity(t *testing.T) { {TxType: 0x0, BroadcastBeforeBlockNum: &num, TxHash: hash}, } - t.Run("skips connectivity check if no transactions are suitable", func(t *testing.T) { - err := bhe.CheckConnectivity(attempts) - require.NoError(t, err) - - tests.AssertLogEventually(t, obs, fmt.Sprintf("no suitable transactions found to verify if transaction %s has been included within expected inclusion blocks of 4", hash)) - }) - t.Run("in legacy mode", func(t *testing.T) { b0 = evmtypes.Block{ Number: 0, @@ -2251,43 +2328,27 @@ func TestBlockHistoryEstimator_CheckConnectivity(t *testing.T) { gas.SetRollingBlockHistory(bhe, []evmtypes.Block{b0, b1, b2, b3}) attempts = []gas.EvmPriorAttempt{ - {TxType: 0x0, TxHash: NewEvmHash(), GasPrice: assets.NewWeiI(1000), BroadcastBeforeBlockNum: ptr(int64(4))}, // This is very expensive but will be ignored due to BroadcastBeforeBlockNum being too recent {TxType: 0x0, TxHash: NewEvmHash(), GasPrice: assets.NewWeiI(3), BroadcastBeforeBlockNum: ptr(int64(0))}, {TxType: 0x0, TxHash: NewEvmHash(), GasPrice: assets.NewWeiI(5), BroadcastBeforeBlockNum: ptr(int64(1))}, {TxType: 0x0, TxHash: NewEvmHash(), GasPrice: assets.NewWeiI(7), BroadcastBeforeBlockNum: ptr(int64(1))}, } - t.Run("passes check if all blocks have percentile price higher or exactly at the highest transaction gas price", func(t *testing.T) { + t.Run("passes check if all attempt gas prices are lower than or equal to percentile price", func(t *testing.T) { bhCfg.CheckInclusionBlocksF = 3 bhCfg.CheckInclusionPercentileF = 80 // percentile price is 7 wei + bhe.Recalculate(testutils.Head(3)) - err := bhe.CheckConnectivity(attempts) + err := bhe.HaltBumping(attempts) require.NoError(t, err) }) - t.Run("fails check if one or more blocks has percentile price higher than highest transaction gas price", func(t *testing.T) { + t.Run("fails check if any attempt price is higher than percentile price", func(t *testing.T) { bhCfg.CheckInclusionBlocksF = 3 bhCfg.CheckInclusionPercentileF = 40 // percentile price is 5 wei + bhe.Recalculate(testutils.Head(3)) - err := bhe.CheckConnectivity(attempts) + err := bhe.HaltBumping(attempts) require.Error(t, err) - assert.Contains(t, err.Error(), fmt.Sprintf("transaction %s has gas price of 7 wei, which is above percentile=40%% (percentile price: 5 wei) for blocks 2 thru 0 (checking 3 blocks)", attempts[3].TxHash)) - require.ErrorIs(t, err, commonfee.ErrConnectivity) - }) - - t.Run("fails check if one or more blocks has percentile price higher than any transaction gas price", func(t *testing.T) { - bhCfg.CheckInclusionBlocksF = 3 - bhCfg.CheckInclusionPercentileF = 30 // percentile price is 4 wei - - err := bhe.CheckConnectivity(attempts) - require.Error(t, err) - assert.Contains(t, err.Error(), fmt.Sprintf("transaction %s has gas price of 5 wei, which is above percentile=30%% (percentile price: 4 wei) for blocks 2 thru 0 (checking 3 blocks)", attempts[2].TxHash)) - - bhCfg.CheckInclusionBlocksF = 3 - bhCfg.CheckInclusionPercentileF = 5 // percentile price is 2 wei - - err = bhe.CheckConnectivity(attempts) - require.Error(t, err) - assert.Contains(t, err.Error(), fmt.Sprintf("transaction %s has gas price of 3 wei, which is above percentile=5%% (percentile price: 2 wei) for blocks 2 thru 0 (checking 3 blocks)", attempts[1].TxHash)) + assert.Contains(t, err.Error(), fmt.Sprintf("transaction %s has gas price of 7 wei, which is above percentile=40%% (percentile price: 5 wei)", attempts[2].TxHash)) require.ErrorIs(t, err, commonfee.ErrConnectivity) }) }) @@ -2309,17 +2370,19 @@ func TestBlockHistoryEstimator_CheckConnectivity(t *testing.T) { t.Run("passes check if both transactions are ok", func(t *testing.T) { bhCfg.CheckInclusionBlocksF = 1 bhCfg.CheckInclusionPercentileF = 90 // percentile price is 5 wei + bhe.Recalculate(testutils.Head(3)) - err := bhe.CheckConnectivity(attempts) + err := bhe.HaltBumping(attempts) require.NoError(t, err) }) t.Run("fails check if legacy transaction fails", func(t *testing.T) { bhCfg.CheckInclusionBlocksF = 1 bhCfg.CheckInclusionPercentileF = 60 + bhe.Recalculate(testutils.Head(3)) - err := bhe.CheckConnectivity(attempts) + err := bhe.HaltBumping(attempts) require.Error(t, err) - assert.Contains(t, err.Error(), fmt.Sprintf("transaction %s has gas price of 10 wei, which is above percentile=60%% (percentile price: 7 wei) for blocks 3 thru 3 (checking 1 blocks)", attempts[1].TxHash)) + assert.Contains(t, err.Error(), fmt.Sprintf("transaction %s has gas price of 10 wei, which is above percentile=60%% (percentile price: 7 wei)", attempts[1].TxHash)) require.ErrorIs(t, err, commonfee.ErrConnectivity) }) @@ -2329,18 +2392,21 @@ func TestBlockHistoryEstimator_CheckConnectivity(t *testing.T) { } t.Run("fails check if dynamic fee transaction fails", func(t *testing.T) { - gas.SetRollingBlockHistory(bhe, []evmtypes.Block{b0}) + geCfg.EIP1559DynamicFeesF = true + geCfg.TipCapMinF = assets.NewWeiI(1) bhCfg.CheckInclusionBlocksF = 1 bhCfg.CheckInclusionPercentileF = 60 + bhe.Recalculate(testutils.Head(3)) - err := bhe.CheckConnectivity(attempts) + err := bhe.HaltBumping(attempts) require.Error(t, err) - assert.Contains(t, err.Error(), fmt.Sprintf("transaction %s has tip cap of 10 wei, which is above percentile=60%% (percentile tip cap: 6 wei) for blocks 3 thru 3 (checking 1 blocks)", attempts[0].TxHash)) + assert.Contains(t, err.Error(), fmt.Sprintf("transaction %s has tip cap of 10 wei, which is above percentile=60%% (percentile tip cap: 6 wei)", attempts[0].TxHash)) require.ErrorIs(t, err, commonfee.ErrConnectivity) }) }) t.Run("in EIP-1559 mode", func(t *testing.T) { + geCfg.EIP1559DynamicFeesF = true b0 = evmtypes.Block{ BaseFeePerGas: assets.NewWeiI(5), Number: 0, @@ -2372,47 +2438,50 @@ func TestBlockHistoryEstimator_CheckConnectivity(t *testing.T) { gas.SetRollingBlockHistory(bhe, blocks) attempts = []gas.EvmPriorAttempt{ - {TxType: 0x2, TxHash: NewEvmHash(), DynamicFee: gas.DynamicFee{FeeCap: assets.NewWeiI(30), TipCap: assets.NewWeiI(1000)}, BroadcastBeforeBlockNum: ptr(int64(4))}, // This is very expensive but will be ignored due to BroadcastBeforeBlockNum being too recent {TxType: 0x2, TxHash: NewEvmHash(), DynamicFee: gas.DynamicFee{FeeCap: assets.NewWeiI(30), TipCap: assets.NewWeiI(3)}, BroadcastBeforeBlockNum: ptr(int64(0))}, {TxType: 0x2, TxHash: NewEvmHash(), DynamicFee: gas.DynamicFee{FeeCap: assets.NewWeiI(30), TipCap: assets.NewWeiI(5)}, BroadcastBeforeBlockNum: ptr(int64(1))}, {TxType: 0x2, TxHash: NewEvmHash(), DynamicFee: gas.DynamicFee{FeeCap: assets.NewWeiI(30), TipCap: assets.NewWeiI(7)}, BroadcastBeforeBlockNum: ptr(int64(1))}, } - t.Run("passes check if all blocks have 90th percentile price higher than highest transaction tip cap", func(t *testing.T) { + t.Run("passes check if 90th percentile price higher than highest transaction tip cap", func(t *testing.T) { bhCfg.CheckInclusionBlocksF = 3 bhCfg.CheckInclusionPercentileF = 80 + bhe.Recalculate(testutils.Head(3)) - err := bhe.CheckConnectivity(attempts) + err := bhe.HaltBumping(attempts) require.NoError(t, err) }) - t.Run("fails check if one or more blocks has percentile tip cap higher than any transaction tip cap, and base fee higher than the block base fee", func(t *testing.T) { + t.Run("fails check if percentile tip cap higher than any transaction tip cap, and base fee higher than the block base fee", func(t *testing.T) { bhCfg.CheckInclusionBlocksF = 3 bhCfg.CheckInclusionPercentileF = 20 + bhe.Recalculate(testutils.Head(3)) - err := bhe.CheckConnectivity(attempts) + err := bhe.HaltBumping(attempts) require.Error(t, err) - assert.Contains(t, err.Error(), fmt.Sprintf("transaction %s has tip cap of 5 wei, which is above percentile=20%% (percentile tip cap: 4 wei) for blocks 2 thru 0 (checking 3 blocks)", attempts[2].TxHash)) + assert.Contains(t, err.Error(), fmt.Sprintf("transaction %s has tip cap of 5 wei, which is above percentile=20%% (percentile tip cap: 4 wei)", attempts[1].TxHash)) require.ErrorIs(t, err, commonfee.ErrConnectivity) bhCfg.CheckInclusionBlocksF = 3 - bhCfg.CheckInclusionPercentileF = 5 + bhCfg.CheckInclusionPercentileF = 2 + bhe.Recalculate(testutils.Head(3)) - err = bhe.CheckConnectivity(attempts) + err = bhe.HaltBumping(attempts) require.Error(t, err) - assert.Contains(t, err.Error(), fmt.Sprintf("transaction %s has tip cap of 3 wei, which is above percentile=5%% (percentile tip cap: 2 wei) for blocks 2 thru 0 (checking 3 blocks)", attempts[1].TxHash)) + assert.Contains(t, err.Error(), fmt.Sprintf("transaction %s has tip cap of 3 wei, which is above percentile=2%% (percentile tip cap: 2 wei)", attempts[0].TxHash)) require.ErrorIs(t, err, commonfee.ErrConnectivity) }) t.Run("passes check if, for at least one block, feecap < tipcap+basefee, even if percentile is not reached", func(t *testing.T) { bhCfg.CheckInclusionBlocksF = 3 bhCfg.CheckInclusionPercentileF = 5 + bhe.Recalculate(testutils.Head(3)) attempts = []gas.EvmPriorAttempt{ {TxType: 0x2, TxHash: NewEvmHash(), DynamicFee: gas.DynamicFee{FeeCap: assets.NewWeiI(4), TipCap: assets.NewWeiI(7)}, BroadcastBeforeBlockNum: ptr(int64(1))}, } - err := bhe.CheckConnectivity(attempts) + err := bhe.HaltBumping(attempts) require.NoError(t, err) }) }) @@ -2423,8 +2492,9 @@ func TestBlockHistoryEstimator_Bumps(t *testing.T) { maxGasPrice := assets.NewWeiI(1000000) bhCfg := newBlockHistoryConfig() - t.Run("BumpLegacyGas checks connectivity", func(t *testing.T) { + t.Run("BumpLegacyGas halts bumping", func(t *testing.T) { cfg := gas.NewMockConfig() + bhCfg.BlockDelayF = 0 bhCfg.CheckInclusionBlocksF = 1 bhCfg.CheckInclusionPercentileF = 10 geCfg := &gas.MockGasEstimatorConfig{} @@ -2432,39 +2502,57 @@ func TestBlockHistoryEstimator_Bumps(t *testing.T) { geCfg.BumpPercentF = 10 geCfg.BumpMinF = assets.NewWeiI(150) geCfg.PriceMaxF = maxGasPrice + geCfg.PriceMinF = assets.NewWeiI(0) l1Oracle := rollupMocks.NewL1Oracle(t) + ethClient := testutils.NewEthClientMockWithDefaultChain(t) + ctx := tests.Context(t) - bhe := newBlockHistoryEstimator(t, nil, cfg, geCfg, bhCfg, l1Oracle) + bhe := gas.BlockHistoryEstimatorFromInterface( + gas.NewBlockHistoryEstimator(logger.Test(t), ethClient, cfg, geCfg, bhCfg, testutils.NewRandomEVMChainID(), l1Oracle), + ) - b1 := evmtypes.Block{ - Number: 1, + b0 := evmtypes.Block{ + Number: 0, Hash: utils.NewHash(), Transactions: legacyTransactionsFromGasPrices(1), } - gas.SetRollingBlockHistory(bhe, []evmtypes.Block{b1}) - head := testutils.Head(1) - bhe.OnNewLongestChain(tests.Context(t), head) + h := testutils.Head(0) + ethClient.On("HeadByNumber", mock.Anything, (*big.Int)(nil)).Return(h, nil).Once() + ethClient.On("BatchCallContext", mock.Anything, mock.MatchedBy(func(b []rpc.BatchElem) bool { + return len(b) == 1 + })).Return(nil).Run(func(args mock.Arguments) { + elems := args.Get(1).([]rpc.BatchElem) + elems[0].Result = &b0 + }).Once() + err := bhe.Start(ctx) + require.NoError(t, err) attempts := []gas.EvmPriorAttempt{ {TxType: 0x0, TxHash: NewEvmHash(), GasPrice: assets.NewWeiI(1000), BroadcastBeforeBlockNum: ptr(int64(0))}, } - - _, _, err := bhe.BumpLegacyGas(tests.Context(t), assets.NewWeiI(42), 100000, maxGasPrice, attempts) + _, _, err = bhe.BumpLegacyGas(ctx, assets.NewWeiI(42), 100000, maxGasPrice, attempts) require.Error(t, err) assert.True(t, pkgerrors.Is(err, commonfee.ErrConnectivity)) - assert.Contains(t, err.Error(), fmt.Sprintf("transaction %s has gas price of 1 kwei, which is above percentile=10%% (percentile price: 1 wei) for blocks 1 thru 1 (checking 1 blocks)", attempts[0].TxHash)) + assert.Contains(t, err.Error(), fmt.Sprintf("transaction %s has gas price of 1 kwei, which is above percentile=10%% (percentile price: 1 wei)", attempts[0].TxHash)) }) t.Run("BumpLegacyGas calls BumpLegacyGasPriceOnly with proper current gas price", func(t *testing.T) { cfg := gas.NewMockConfig() + bhCfg := newBlockHistoryConfig() + bhCfg.CheckInclusionBlocksF = 0 geCfg := &gas.MockGasEstimatorConfig{} geCfg.EIP1559DynamicFeesF = false geCfg.BumpPercentF = 10 geCfg.BumpMinF = assets.NewWeiI(150) geCfg.PriceMaxF = maxGasPrice l1Oracle := rollupMocks.NewL1Oracle(t) + ethClient := testutils.NewEthClientMockWithDefaultChain(t) + ctx := tests.Context(t) - bhe := newBlockHistoryEstimator(t, nil, cfg, geCfg, bhCfg, l1Oracle) + ethClient.On("HeadByNumber", mock.Anything, (*big.Int)(nil)).Return(nil, nil).Once() + bhe := newBlockHistoryEstimator(t, ethClient, cfg, geCfg, bhCfg, l1Oracle) + err := bhe.Start(ctx) + require.NoError(t, err) t.Run("ignores nil current gas price", func(t *testing.T) { gasPrice, gasLimit, err := bhe.BumpLegacyGas(tests.Context(t), assets.NewWeiI(42), 100000, maxGasPrice, nil) @@ -2544,28 +2632,38 @@ func TestBlockHistoryEstimator_Bumps(t *testing.T) { geCfg.BumpPercentF = 10 geCfg.BumpMinF = assets.NewWeiI(150) geCfg.PriceMaxF = maxGasPrice + geCfg.PriceMinF = assets.NewWeiI(0) + geCfg.TipCapMinF = assets.NewWeiI(0) l1Oracle := rollupMocks.NewL1Oracle(t) + ethClient := testutils.NewEthClientMockWithDefaultChain(t) + ctx := tests.Context(t) - bhe := newBlockHistoryEstimator(t, nil, cfg, geCfg, bhCfg, l1Oracle) + bhe := newBlockHistoryEstimator(t, ethClient, cfg, geCfg, bhCfg, l1Oracle) - b1 := evmtypes.Block{ + b0 := evmtypes.Block{ BaseFeePerGas: assets.NewWeiI(1), - Number: 1, + Number: 0, Hash: utils.NewHash(), Transactions: dynamicFeeTransactionsFromTipCaps(1), } - gas.SetRollingBlockHistory(bhe, []evmtypes.Block{b1}) - head := testutils.Head(1) - bhe.OnNewLongestChain(tests.Context(t), head) + ethClient.On("HeadByNumber", mock.Anything, (*big.Int)(nil)).Return(testutils.Head(0), nil).Once() + ethClient.On("BatchCallContext", mock.Anything, mock.MatchedBy(func(b []rpc.BatchElem) bool { + return len(b) == 1 + })).Return(nil).Run(func(args mock.Arguments) { + elems := args.Get(1).([]rpc.BatchElem) + elems[0].Result = &b0 + }).Once() + err := bhe.Start(ctx) + require.NoError(t, err) originalFee := gas.DynamicFee{FeeCap: assets.NewWeiI(100), TipCap: assets.NewWeiI(25)} attempts := []gas.EvmPriorAttempt{ {TxType: 0x2, TxHash: NewEvmHash(), DynamicFee: gas.DynamicFee{TipCap: originalFee.TipCap, FeeCap: originalFee.FeeCap}, BroadcastBeforeBlockNum: ptr(int64(0))}} - _, err := bhe.BumpDynamicFee(tests.Context(t), originalFee, maxGasPrice, attempts) + _, err = bhe.BumpDynamicFee(tests.Context(t), originalFee, maxGasPrice, attempts) require.Error(t, err) assert.True(t, pkgerrors.Is(err, commonfee.ErrConnectivity)) - assert.Contains(t, err.Error(), fmt.Sprintf("transaction %s has tip cap of 25 wei, which is above percentile=10%% (percentile tip cap: 1 wei) for blocks 1 thru 1 (checking 1 blocks)", attempts[0].TxHash)) + assert.Contains(t, err.Error(), fmt.Sprintf("transaction %s has tip cap of 25 wei, which is above percentile=10%% (percentile tip cap: 1 wei)", attempts[0].TxHash)) }) t.Run("BumpDynamicFee bumps the fee", func(t *testing.T) { @@ -2579,8 +2677,13 @@ func TestBlockHistoryEstimator_Bumps(t *testing.T) { geCfg.PriceMaxF = maxGasPrice geCfg.TipCapDefaultF = assets.NewWeiI(52) l1Oracle := rollupMocks.NewL1Oracle(t) + ethClient := testutils.NewEthClientMockWithDefaultChain(t) + ctx := tests.Context(t) - bhe := newBlockHistoryEstimator(t, nil, cfg, geCfg, bhCfg, l1Oracle) + bhe := newBlockHistoryEstimator(t, ethClient, cfg, geCfg, bhCfg, l1Oracle) + ethClient.On("HeadByNumber", mock.Anything, (*big.Int)(nil)).Return(nil, nil).Once() + err := bhe.Start(ctx) + require.NoError(t, err) t.Run("when current tip cap is nil", func(t *testing.T) { originalFee := gas.DynamicFee{FeeCap: assets.NewWeiI(100), TipCap: assets.NewWeiI(25)} @@ -2641,6 +2744,130 @@ func TestBlockHistoryEstimator_Bumps(t *testing.T) { }) } +func TestBlockHistoryEstimator_CheckInclusionPercentile_Calculation(t *testing.T) { + t.Parallel() + + t.Run("sets CheckInclusionPercentile price using the latest blocks, eip-1559 disabled", func(t *testing.T) { + ethClient := testutils.NewEthClientMockWithDefaultChain(t) + l1Oracle := rollupMocks.NewL1Oracle(t) + + cfg := gas.NewMockConfig() + bhCfg := newBlockHistoryConfig() + bhCfg.BlockDelayF = 0 + bhCfg.TransactionPercentileF = 35 + // Set BlockHistorySize larger than CheckInclusionBlocks to cache more blocks than needed for calculation + // Helps test whether the latest block is being used or not + bhCfg.BlockHistorySizeF = 3 + bhCfg.BatchSizeF = 0 + bhCfg.CheckInclusionBlocksF = 1 + bhCfg.CheckInclusionPercentileF = 100 + + geCfg := &gas.MockGasEstimatorConfig{} + geCfg.EIP1559DynamicFeesF = false + geCfg.PriceMaxF = assets.NewWeiI(1000) + geCfg.PriceMinF = assets.NewWeiI(0) + + bhe := newBlockHistoryEstimator(t, ethClient, cfg, geCfg, bhCfg, l1Oracle) + + b0 := evmtypes.Block{ + Number: 0, + Hash: utils.NewHash(), + Transactions: legacyTransactionsFromGasPrices(1, 2, 3), + } + b1 := evmtypes.Block{ + Number: 1, + Hash: utils.NewHash(), + Transactions: legacyTransactionsFromGasPrices(4, 5, 6), + } + b2 := evmtypes.Block{ + Number: 2, + Hash: utils.NewHash(), + Transactions: legacyTransactionsFromGasPrices(7, 8, 9), + } + + ethClient.On("BatchCallContext", mock.Anything, mock.MatchedBy(func(b []rpc.BatchElem) bool { + return len(b) == 3 && + b[0].Args[0] == "0x2" && + b[1].Args[0] == "0x1" && + b[2].Args[0] == "0x0" + })).Return(nil).Run(func(args mock.Arguments) { + elems := args.Get(1).([]rpc.BatchElem) + elems[0].Result = &b2 + elems[1].Result = &b1 + elems[2].Result = &b0 + }) + + bhe.FetchBlocksAndRecalculate(tests.Context(t), testutils.Head(2)) + + price := gas.GetMaxPercentileGasPrice(bhe) + require.Equal(t, assets.NewWeiI(9), price) + + assert.Len(t, gas.GetRollingBlockHistory(bhe), 3) + }) + + t.Run("sets CheckInclusionPercentile price using the latest blocks, eip-1559 enabled", func(t *testing.T) { + ethClient := testutils.NewEthClientMockWithDefaultChain(t) + l1Oracle := rollupMocks.NewL1Oracle(t) + + cfg := gas.NewMockConfig() + bhCfg := newBlockHistoryConfig() + bhCfg.BlockDelayF = 0 + bhCfg.TransactionPercentileF = 35 + // Set BlockHistorySize larger than CheckInclusionBlocks to cache more blocks than needed for calculation + // Helps test whether the latest block is being used or not + bhCfg.BlockHistorySizeF = 3 + bhCfg.BatchSizeF = 0 + bhCfg.CheckInclusionBlocksF = 1 + bhCfg.CheckInclusionPercentileF = 100 + + geCfg := &gas.MockGasEstimatorConfig{} + geCfg.EIP1559DynamicFeesF = true + geCfg.PriceMaxF = assets.NewWeiI(1000) + geCfg.PriceMinF = assets.NewWeiI(0) + geCfg.TipCapMinF = assets.NewWeiI(0) + + bhe := newBlockHistoryEstimator(t, ethClient, cfg, geCfg, bhCfg, l1Oracle) + + b0 := evmtypes.Block{ + BaseFeePerGas: assets.NewWeiI(1), + Number: 0, + Hash: utils.NewHash(), + Transactions: dynamicFeeTransactionsFromTipCaps(1, 2, 3), + } + b1 := evmtypes.Block{ + BaseFeePerGas: assets.NewWeiI(2), + Number: 1, + Hash: utils.NewHash(), + Transactions: dynamicFeeTransactionsFromTipCaps(4, 5, 6), + } + b2 := evmtypes.Block{ + BaseFeePerGas: assets.NewWeiI(3), + Number: 2, + Hash: utils.NewHash(), + Transactions: dynamicFeeTransactionsFromTipCaps(7, 8, 9), + } + + ethClient.On("BatchCallContext", mock.Anything, mock.MatchedBy(func(b []rpc.BatchElem) bool { + return len(b) == 3 && + b[0].Args[0] == "0x2" && + b[1].Args[0] == "0x1" && + b[2].Args[0] == "0x0" + })).Return(nil).Run(func(args mock.Arguments) { + elems := args.Get(1).([]rpc.BatchElem) + elems[0].Result = &b2 + elems[1].Result = &b1 + elems[2].Result = &b0 + }) + + bhe.FetchBlocksAndRecalculate(tests.Context(t), testutils.Head(2)) + + price := gas.GetMaxPercentileTipCap(bhe) + require.Equal(t, assets.NewWeiI(9), price) + + assert.Len(t, gas.GetRollingBlockHistory(bhe), 3) + }) +} + // ptr takes pointer of anything func ptr[T any](v T) *T { return &v diff --git a/core/chains/evm/gas/helpers_test.go b/core/chains/evm/gas/helpers_test.go index e64b6ad4779..2c12ed426a6 100644 --- a/core/chains/evm/gas/helpers_test.go +++ b/core/chains/evm/gas/helpers_test.go @@ -16,8 +16,8 @@ func init() { MaxStartTime = 1 * time.Second } -func (b *BlockHistoryEstimator) CheckConnectivity(attempts []EvmPriorAttempt) error { - return b.checkConnectivity(attempts) +func (b *BlockHistoryEstimator) HaltBumping(attempts []EvmPriorAttempt) error { + return b.haltBumping(attempts) } func BlockHistoryEstimatorFromInterface(bhe EvmEstimator) *BlockHistoryEstimator { @@ -52,12 +52,24 @@ func GetGasPrice(b *BlockHistoryEstimator) *assets.Wei { return b.gasPrice } +func GetMaxPercentileGasPrice(b *BlockHistoryEstimator) *assets.Wei { + b.maxPriceMu.RLock() + defer b.maxPriceMu.RUnlock() + return b.maxPercentileGasPrice +} + func GetTipCap(b *BlockHistoryEstimator) *assets.Wei { b.priceMu.RLock() defer b.priceMu.RUnlock() return b.tipCap } +func GetMaxPercentileTipCap(b *BlockHistoryEstimator) *assets.Wei { + b.maxPriceMu.RLock() + defer b.maxPriceMu.RUnlock() + return b.maxPercentileTipCap +} + func GetLatestBaseFee(b *BlockHistoryEstimator) *assets.Wei { b.latestMu.RLock() defer b.latestMu.RUnlock()