From d84c214a5fa752726e374256b066b6921ff6aa19 Mon Sep 17 00:00:00 2001 From: Silas Lenihan Date: Tue, 26 Mar 2024 16:17:50 -0400 Subject: [PATCH] removed chainSpecificGasLimit from BumpDynamicFee, GetDynamicFee --- .../chains/evm/gas/arbitrum_estimator_test.go | 4 +- .../chains/evm/gas/block_history_estimator.go | 21 ++++--- .../evm/gas/block_history_estimator_test.go | 63 +++++++------------ core/chains/evm/gas/fixed_price_estimator.go | 10 ++- .../evm/gas/fixed_price_estimator_test.go | 18 +++--- core/chains/evm/gas/gas_test.go | 17 +++-- core/chains/evm/gas/mocks/evm_estimator.go | 38 ++++------- core/chains/evm/gas/models.go | 24 ++++--- .../evm/gas/suggested_price_estimator.go | 4 +- .../evm/gas/suggested_price_estimator_test.go | 4 +- 10 files changed, 83 insertions(+), 120 deletions(-) diff --git a/core/chains/evm/gas/arbitrum_estimator_test.go b/core/chains/evm/gas/arbitrum_estimator_test.go index afac9e03276..31b32ec3d6f 100644 --- a/core/chains/evm/gas/arbitrum_estimator_test.go +++ b/core/chains/evm/gas/arbitrum_estimator_test.go @@ -168,7 +168,7 @@ func TestArbitrumEstimator(t *testing.T) { rpcClient := mocks.NewRPCClient(t) ethClient := mocks.NewETHClient(t) o := gas.NewArbitrumEstimator(logger.Test(t), &arbConfig{}, rpcClient, ethClient) - _, _, err := o.GetDynamicFee(testutils.Context(t), gasLimit, maxGasPrice) + _, err := o.GetDynamicFee(testutils.Context(t), gasLimit, maxGasPrice) assert.EqualError(t, err, "dynamic fees are not implemented for this estimator") }) @@ -180,7 +180,7 @@ func TestArbitrumEstimator(t *testing.T) { FeeCap: assets.NewWeiI(42), TipCap: assets.NewWeiI(5), } - _, _, err := o.BumpDynamicFee(testutils.Context(t), fee, gasLimit, maxGasPrice, nil) + _, err := o.BumpDynamicFee(testutils.Context(t), fee, gasLimit, maxGasPrice, nil) assert.EqualError(t, err, "dynamic fees are not implemented for this estimator") }) diff --git a/core/chains/evm/gas/block_history_estimator.go b/core/chains/evm/gas/block_history_estimator.go index a4bcd849e2c..d186ef04937 100644 --- a/core/chains/evm/gas/block_history_estimator.go +++ b/core/chains/evm/gas/block_history_estimator.go @@ -297,7 +297,11 @@ func (b *BlockHistoryEstimator) BumpLegacyGas(_ context.Context, originalGasPric return nil, 0, err } } - return BumpLegacyGasPriceOnly(b.eConfig, b.logger, b.getGasPrice(), originalGasPrice, gasLimit, maxGasPriceWei) + bumpedGasPrice, err = BumpLegacyGasPriceOnly(b.eConfig, b.logger, b.getGasPrice(), originalGasPrice, maxGasPriceWei) + if err != nil { + return nil, 0, err + } + return bumpedGasPrice, gasLimit, err } // checkConnectivity detects if the transaction is not being included due to @@ -387,15 +391,14 @@ func (b *BlockHistoryEstimator) checkConnectivity(attempts []EvmPriorAttempt) er return nil } -func (b *BlockHistoryEstimator) GetDynamicFee(_ context.Context, gasLimit uint64, maxGasPriceWei *assets.Wei) (fee DynamicFee, chainSpecificGasLimit uint64, err error) { +func (b *BlockHistoryEstimator) GetDynamicFee(_ context.Context, gasLimit uint64, maxGasPriceWei *assets.Wei) (fee DynamicFee, err error) { if !b.eConfig.EIP1559DynamicFees() { - return fee, 0, pkgerrors.New("Can't get dynamic fee, EIP1559 is disabled") + return fee, pkgerrors.New("Can't get dynamic fee, EIP1559 is disabled") } var feeCap *assets.Wei var tipCap *assets.Wei ok := b.IfStarted(func() { - chainSpecificGasLimit = gasLimit b.priceMu.RLock() defer b.priceMu.RUnlock() tipCap = b.tipCap @@ -427,10 +430,10 @@ func (b *BlockHistoryEstimator) GetDynamicFee(_ context.Context, gasLimit uint64 } }) if !ok { - return fee, 0, pkgerrors.New("BlockHistoryEstimator is not started; cannot estimate gas") + return fee, pkgerrors.New("BlockHistoryEstimator is not started; cannot estimate gas") } if err != nil { - return fee, 0, err + return fee, err } fee.FeeCap = feeCap fee.TipCap = tipCap @@ -457,7 +460,7 @@ func calcFeeCap(latestAvailableBaseFeePerGas *assets.Wei, bufferBlocks int, tipC return feeCap } -func (b *BlockHistoryEstimator) BumpDynamicFee(_ context.Context, originalFee DynamicFee, originalGasLimit uint64, maxGasPriceWei *assets.Wei, attempts []EvmPriorAttempt) (bumped DynamicFee, chainSpecificGasLimit uint64, err error) { +func (b *BlockHistoryEstimator) BumpDynamicFee(_ context.Context, originalFee DynamicFee, originalGasLimit uint64, 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) { @@ -465,10 +468,10 @@ func (b *BlockHistoryEstimator) BumpDynamicFee(_ context.Context, originalFee Dy b.SvcErrBuffer.Append(err) promBlockHistoryEstimatorConnectivityFailureCount.WithLabelValues(b.chainID.String(), "eip1559").Inc() } - return bumped, 0, err + return bumped, err } } - return BumpDynamicFeeOnly(b.eConfig, b.bhConfig.EIP1559FeeCapBufferBlocks(), b.logger, b.getTipCap(), b.getCurrentBaseFee(), originalFee, originalGasLimit, maxGasPriceWei) + return BumpDynamicFeeOnly(b.eConfig, b.bhConfig.EIP1559FeeCapBufferBlocks(), b.logger, b.getTipCap(), b.getCurrentBaseFee(), originalFee, maxGasPriceWei) } func (b *BlockHistoryEstimator) runLoop() { diff --git a/core/chains/evm/gas/block_history_estimator_test.go b/core/chains/evm/gas/block_history_estimator_test.go index 8cb0e03f308..143987c4587 100644 --- a/core/chains/evm/gas/block_history_estimator_test.go +++ b/core/chains/evm/gas/block_history_estimator_test.go @@ -186,7 +186,7 @@ func TestBlockHistoryEstimator_Start(t *testing.T) { require.Error(t, err) require.Contains(t, err.Error(), "has not finished the first gas estimation yet, likely because a failure on start") - _, _, err = bhe.GetDynamicFee(testutils.Context(t), 100, maxGasPrice) + _, err = bhe.GetDynamicFee(testutils.Context(t), 100, maxGasPrice) require.Error(t, err) require.Contains(t, err.Error(), "has not finished the first gas estimation yet, likely because a failure on start") }) @@ -209,7 +209,7 @@ func TestBlockHistoryEstimator_Start(t *testing.T) { require.Error(t, err) require.Contains(t, err.Error(), "has not finished the first gas estimation yet, likely because a failure on start") - _, _, err = bhe.GetDynamicFee(testutils.Context(t), 100, maxGasPrice) + _, err = bhe.GetDynamicFee(testutils.Context(t), 100, maxGasPrice) require.Error(t, err) require.Contains(t, err.Error(), "has not finished the first gas estimation yet, likely because a failure on start") }) @@ -250,7 +250,7 @@ func TestBlockHistoryEstimator_Start(t *testing.T) { require.Error(t, err) require.Contains(t, err.Error(), "has not finished the first gas estimation yet, likely because a failure on start") - _, _, err = bhe.GetDynamicFee(testutils.Context(t), 100, maxGasPrice) + _, err = bhe.GetDynamicFee(testutils.Context(t), 100, maxGasPrice) require.Error(t, err) require.Contains(t, err.Error(), "has not finished the first gas estimation yet, likely because a failure on start") }) @@ -1946,11 +1946,10 @@ func TestBlockHistoryEstimator_UseDefaultPriceAsFallback(t *testing.T) { err := bhe.Start(testutils.Context(t)) require.NoError(t, err) - fee, limit, err := bhe.GetDynamicFee(testutils.Context(t), 100000, assets.NewWeiI(200)) + fee, err := bhe.GetDynamicFee(testutils.Context(t), 100000, assets.NewWeiI(200)) require.NoError(t, err) assert.Equal(t, gas.DynamicFee{FeeCap: assets.NewWeiI(114), TipCap: geCfg.TipCapDefault()}, fee) - assert.Equal(t, 100000, int(limit)) }) } @@ -1992,7 +1991,7 @@ func TestBlockHistoryEstimator_GetDynamicFee(t *testing.T) { t.Run("if estimator is missing base fee and gas bumping is enabled", func(t *testing.T) { geCfg.BumpThresholdF = uint64(1) - _, _, err := bhe.GetDynamicFee(testutils.Context(t), 100000, maxGasPrice) + _, err := bhe.GetDynamicFee(testutils.Context(t), 100000, maxGasPrice) require.Error(t, err) assert.Contains(t, err.Error(), "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?") }) @@ -2000,10 +1999,9 @@ func TestBlockHistoryEstimator_GetDynamicFee(t *testing.T) { t.Run("if estimator is missing base fee and gas bumping is disabled", func(t *testing.T) { geCfg.BumpThresholdF = uint64(0) - fee, limit, err := bhe.GetDynamicFee(testutils.Context(t), 100000, maxGasPrice) + fee, err := bhe.GetDynamicFee(testutils.Context(t), 100000, maxGasPrice) require.NoError(t, err) assert.Equal(t, gas.DynamicFee{FeeCap: maxGasPrice, TipCap: assets.NewWeiI(6000)}, fee) - assert.Equal(t, 100000, int(limit)) }) h := cltest.Head(1) @@ -2013,41 +2011,37 @@ func TestBlockHistoryEstimator_GetDynamicFee(t *testing.T) { t.Run("if gas bumping is enabled", func(t *testing.T) { geCfg.BumpThresholdF = uint64(1) - fee, limit, err := bhe.GetDynamicFee(testutils.Context(t), 100000, maxGasPrice) + fee, err := bhe.GetDynamicFee(testutils.Context(t), 100000, maxGasPrice) require.NoError(t, err) assert.Equal(t, gas.DynamicFee{FeeCap: assets.NewWeiI(186203), TipCap: assets.NewWeiI(6000)}, fee) - assert.Equal(t, 100000, int(limit)) }) t.Run("if gas bumping is disabled", func(t *testing.T) { geCfg.BumpThresholdF = uint64(0) - fee, limit, err := bhe.GetDynamicFee(testutils.Context(t), 100000, maxGasPrice) + fee, err := bhe.GetDynamicFee(testutils.Context(t), 100000, maxGasPrice) require.NoError(t, err) assert.Equal(t, gas.DynamicFee{FeeCap: maxGasPrice, TipCap: assets.NewWeiI(6000)}, fee) - assert.Equal(t, 100000, int(limit)) }) t.Run("if gas bumping is enabled and local max gas price set", func(t *testing.T) { geCfg.BumpThresholdF = uint64(1) - fee, limit, err := bhe.GetDynamicFee(testutils.Context(t), 100000, assets.NewWeiI(180000)) + fee, err := bhe.GetDynamicFee(testutils.Context(t), 100000, assets.NewWeiI(180000)) require.NoError(t, err) assert.Equal(t, gas.DynamicFee{FeeCap: assets.NewWeiI(180000), TipCap: assets.NewWeiI(6000)}, fee) - assert.Equal(t, 100000, int(limit)) }) t.Run("if bump threshold is 0 and local max gas price set", func(t *testing.T) { geCfg.BumpThresholdF = uint64(0) - fee, limit, err := bhe.GetDynamicFee(testutils.Context(t), 100000, assets.NewWeiI(100)) + fee, err := bhe.GetDynamicFee(testutils.Context(t), 100000, assets.NewWeiI(100)) require.NoError(t, err) assert.Equal(t, gas.DynamicFee{FeeCap: assets.NewWeiI(100), TipCap: assets.NewWeiI(6000)}, fee) - assert.Equal(t, 100000, int(limit)) }) h = cltest.Head(1) @@ -2057,11 +2051,10 @@ func TestBlockHistoryEstimator_GetDynamicFee(t *testing.T) { t.Run("if gas bumping is enabled and global max gas price lower than local max gas price", func(t *testing.T) { geCfg.BumpThresholdF = uint64(1) - fee, limit, err := bhe.GetDynamicFee(testutils.Context(t), 100000, assets.NewWeiI(1200000)) + fee, err := bhe.GetDynamicFee(testutils.Context(t), 100000, assets.NewWeiI(1200000)) require.NoError(t, err) assert.Equal(t, gas.DynamicFee{FeeCap: assets.NewWeiI(1000000), TipCap: assets.NewWeiI(6000)}, fee) - assert.Equal(t, 100000, int(limit)) }) } @@ -2401,7 +2394,6 @@ func TestBlockHistoryEstimator_Bumps(t *testing.T) { geCfg.BumpPercentF = 10 geCfg.BumpMinF = assets.NewWeiI(150) geCfg.PriceMaxF = maxGasPrice - geCfg.LimitMultiplierF = float32(1.1) bhe := newBlockHistoryEstimator(t, nil, cfg, geCfg, bhCfg) @@ -2409,10 +2401,10 @@ func TestBlockHistoryEstimator_Bumps(t *testing.T) { gasPrice, gasLimit, err := bhe.BumpLegacyGas(testutils.Context(t), assets.NewWeiI(42), 100000, maxGasPrice, nil) require.NoError(t, err) - expectedGasPrice, expectedGasLimit, err := gas.BumpLegacyGasPriceOnly(geCfg, logger.TestSugared(t), nil, assets.NewWeiI(42), 100000, maxGasPrice) + expectedGasPrice, err := gas.BumpLegacyGasPriceOnly(geCfg, logger.TestSugared(t), nil, assets.NewWeiI(42), maxGasPrice) require.NoError(t, err) - assert.Equal(t, expectedGasLimit, gasLimit) + assert.Equal(t, 100000, int(gasLimit)) assert.Equal(t, expectedGasPrice, gasPrice) }) @@ -2423,10 +2415,10 @@ func TestBlockHistoryEstimator_Bumps(t *testing.T) { massive := assets.NewWeiI(100000000000000) gas.SetGasPrice(bhe, massive) - expectedGasPrice, expectedGasLimit, err := gas.BumpLegacyGasPriceOnly(geCfg, logger.TestSugared(t), massive, assets.NewWeiI(42), 100000, maxGasPrice) + expectedGasPrice, err := gas.BumpLegacyGasPriceOnly(geCfg, logger.TestSugared(t), massive, assets.NewWeiI(42), maxGasPrice) require.NoError(t, err) - assert.Equal(t, expectedGasLimit, gasLimit) + assert.Equal(t, 100000, int(gasLimit)) assert.Equal(t, expectedGasPrice, gasPrice) }) @@ -2436,7 +2428,7 @@ func TestBlockHistoryEstimator_Bumps(t *testing.T) { gasPrice, gasLimit, err := bhe.BumpLegacyGas(testutils.Context(t), assets.NewWeiI(42), 100000, maxGasPrice, nil) require.NoError(t, err) - assert.Equal(t, 110000, int(gasLimit)) + assert.Equal(t, 100000, int(gasLimit)) assert.Equal(t, assets.NewWeiI(192), gasPrice) }) @@ -2446,7 +2438,7 @@ func TestBlockHistoryEstimator_Bumps(t *testing.T) { gasPrice, gasLimit, err := bhe.BumpLegacyGas(testutils.Context(t), assets.NewWeiI(42), 100000, maxGasPrice, nil) require.NoError(t, err) - assert.Equal(t, 110000, int(gasLimit)) + assert.Equal(t, 100000, int(gasLimit)) assert.Equal(t, assets.NewWeiI(193), gasPrice) }) @@ -2500,7 +2492,7 @@ func TestBlockHistoryEstimator_Bumps(t *testing.T) { attempts := []gas.EvmPriorAttempt{ {TxType: 0x2, TxHash: NewEvmHash(), DynamicFee: gas.DynamicFee{TipCap: originalFee.TipCap, FeeCap: originalFee.FeeCap}, BroadcastBeforeBlockNum: testutils.Ptr(int64(0))}} - _, _, err := bhe.BumpDynamicFee(testutils.Context(t), originalFee, 100000, maxGasPrice, attempts) + _, err := bhe.BumpDynamicFee(testutils.Context(t), originalFee, 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 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)) @@ -2515,47 +2507,42 @@ func TestBlockHistoryEstimator_Bumps(t *testing.T) { geCfg.BumpPercentF = 10 geCfg.BumpMinF = assets.NewWeiI(150) geCfg.PriceMaxF = maxGasPrice - geCfg.LimitMultiplierF = float32(1.1) geCfg.TipCapDefaultF = assets.NewWeiI(52) bhe := newBlockHistoryEstimator(t, nil, cfg, geCfg, bhCfg) t.Run("when current tip cap is nil", func(t *testing.T) { originalFee := gas.DynamicFee{FeeCap: assets.NewWeiI(100), TipCap: assets.NewWeiI(25)} - fee, gasLimit, err := bhe.BumpDynamicFee(testutils.Context(t), originalFee, 100000, maxGasPrice, nil) + fee, err := bhe.BumpDynamicFee(testutils.Context(t), originalFee, 100000, maxGasPrice, nil) require.NoError(t, err) - assert.Equal(t, 110000, int(gasLimit)) assert.Equal(t, gas.DynamicFee{FeeCap: assets.NewWeiI(250), TipCap: assets.NewWeiI(202)}, fee) }) t.Run("ignores current tip cap that is smaller than original fee with bump applied", func(t *testing.T) { gas.SetTipCap(bhe, assets.NewWeiI(201)) originalFee := gas.DynamicFee{FeeCap: assets.NewWeiI(100), TipCap: assets.NewWeiI(25)} - fee, gasLimit, err := bhe.BumpDynamicFee(testutils.Context(t), originalFee, 100000, maxGasPrice, nil) + fee, err := bhe.BumpDynamicFee(testutils.Context(t), originalFee, 100000, maxGasPrice, nil) require.NoError(t, err) - assert.Equal(t, 110000, int(gasLimit)) assert.Equal(t, gas.DynamicFee{FeeCap: assets.NewWeiI(250), TipCap: assets.NewWeiI(202)}, fee) }) t.Run("uses current tip cap that is larger than original fee with bump applied", func(t *testing.T) { gas.SetTipCap(bhe, assets.NewWeiI(203)) originalFee := gas.DynamicFee{FeeCap: assets.NewWeiI(100), TipCap: assets.NewWeiI(25)} - fee, gasLimit, err := bhe.BumpDynamicFee(testutils.Context(t), originalFee, 100000, maxGasPrice, nil) + fee, err := bhe.BumpDynamicFee(testutils.Context(t), originalFee, 100000, maxGasPrice, nil) require.NoError(t, err) - assert.Equal(t, 110000, int(gasLimit)) assert.Equal(t, gas.DynamicFee{FeeCap: assets.NewWeiI(250), TipCap: assets.NewWeiI(203)}, fee) }) t.Run("ignores absurdly large current tip cap", func(t *testing.T) { gas.SetTipCap(bhe, assets.NewWeiI(1000000000000000)) originalFee := gas.DynamicFee{FeeCap: assets.NewWeiI(100), TipCap: assets.NewWeiI(25)} - fee, gasLimit, err := bhe.BumpDynamicFee(testutils.Context(t), originalFee, 100000, maxGasPrice, nil) + fee, err := bhe.BumpDynamicFee(testutils.Context(t), originalFee, 100000, maxGasPrice, nil) require.NoError(t, err) - assert.Equal(t, 110000, int(gasLimit)) assert.Equal(t, gas.DynamicFee{FeeCap: assets.NewWeiI(250), TipCap: assets.NewWeiI(202)}, fee) }) @@ -2563,10 +2550,9 @@ func TestBlockHistoryEstimator_Bumps(t *testing.T) { gas.SetTipCap(bhe, assets.NewWeiI(203)) originalFee := gas.DynamicFee{FeeCap: assets.NewWeiI(100), TipCap: assets.NewWeiI(990000)} - fee, gasLimit, err := bhe.BumpDynamicFee(testutils.Context(t), originalFee, 100000, maxGasPrice, nil) + fee, err := bhe.BumpDynamicFee(testutils.Context(t), originalFee, 100000, maxGasPrice, nil) require.Error(t, err) - assert.Equal(t, 0, int(gasLimit)) assert.Equal(t, gas.DynamicFee{}, fee) assert.Contains(t, err.Error(), "bumped tip cap of 1.089 mwei would exceed configured max gas price of 1 mwei (original fee: tip cap 990 kwei, fee cap 100 wei)") }) @@ -2575,10 +2561,9 @@ func TestBlockHistoryEstimator_Bumps(t *testing.T) { gas.SetTipCap(bhe, assets.NewWeiI(203)) originalFee := gas.DynamicFee{FeeCap: assets.NewWeiI(990000), TipCap: assets.NewWeiI(25)} - fee, gasLimit, err := bhe.BumpDynamicFee(testutils.Context(t), originalFee, 100000, maxGasPrice, nil) + fee, err := bhe.BumpDynamicFee(testutils.Context(t), originalFee, 100000, maxGasPrice, nil) require.Error(t, err) - assert.Equal(t, 0, int(gasLimit)) assert.Equal(t, gas.DynamicFee{}, fee) assert.Contains(t, err.Error(), "bumped fee cap of 1.089 mwei would exceed configured max gas price of 1 mwei (original fee: tip cap 25 wei, fee cap 990 kwei)") }) diff --git a/core/chains/evm/gas/fixed_price_estimator.go b/core/chains/evm/gas/fixed_price_estimator.go index 15ec0e5b598..7d04c9040f4 100644 --- a/core/chains/evm/gas/fixed_price_estimator.go +++ b/core/chains/evm/gas/fixed_price_estimator.go @@ -88,13 +88,12 @@ func (f *fixedPriceEstimator) BumpLegacyGas( return assets.NewWei(gasPrice), chainSpecificGasLimit, err } -func (f *fixedPriceEstimator) GetDynamicFee(_ context.Context, originalGasLimit uint64, maxGasPriceWei *assets.Wei) (d DynamicFee, chainSpecificGasLimit uint64, err error) { +func (f *fixedPriceEstimator) GetDynamicFee(_ context.Context, originalGasLimit uint64, maxGasPriceWei *assets.Wei) (d DynamicFee, err error) { gasTipCap := f.config.TipCapDefault() if gasTipCap == nil { - return d, 0, pkgerrors.New("cannot calculate dynamic fee: EthGasTipCapDefault was not set") + return d, pkgerrors.New("cannot calculate dynamic fee: EthGasTipCapDefault was not set") } - chainSpecificGasLimit = originalGasLimit var feeCap *assets.Wei if f.config.BumpThreshold() == 0 { @@ -108,7 +107,7 @@ func (f *fixedPriceEstimator) GetDynamicFee(_ context.Context, originalGasLimit return DynamicFee{ FeeCap: feeCap, TipCap: gasTipCap, - }, chainSpecificGasLimit, nil + }, nil } func (f *fixedPriceEstimator) BumpDynamicFee( @@ -117,7 +116,7 @@ func (f *fixedPriceEstimator) BumpDynamicFee( originalGasLimit uint64, maxGasPriceWei *assets.Wei, _ []EvmPriorAttempt, -) (bumped DynamicFee, chainSpecificGasLimit uint64, err error) { +) (bumped DynamicFee, err error) { return BumpDynamicFeeOnly( f.config, @@ -126,7 +125,6 @@ func (f *fixedPriceEstimator) BumpDynamicFee( f.config.TipCapDefault(), nil, originalFee, - originalGasLimit, maxGasPriceWei, ) } diff --git a/core/chains/evm/gas/fixed_price_estimator_test.go b/core/chains/evm/gas/fixed_price_estimator_test.go index 4bc3f19f50a..a1f415e3454 100644 --- a/core/chains/evm/gas/fixed_price_estimator_test.go +++ b/core/chains/evm/gas/fixed_price_estimator_test.go @@ -74,10 +74,10 @@ func Test_FixedPriceEstimator(t *testing.T) { gasPrice, gasLimit, err := f.BumpLegacyGas(testutils.Context(t), assets.NewWeiI(42), 100000, maxGasPrice, nil) require.NoError(t, err) - expectedGasPrice, expectedGasLimit, err := gas.BumpLegacyGasPriceOnly(config, lggr, nil, assets.NewWeiI(42), 100000, maxGasPrice) + expectedGasPrice, err := gas.BumpLegacyGasPriceOnly(config, lggr, nil, assets.NewWeiI(42), maxGasPrice) require.NoError(t, err) - assert.Equal(t, expectedGasLimit, gasLimit) + assert.Equal(t, 100000, int(gasLimit)) assert.Equal(t, expectedGasPrice, gasPrice) }) @@ -91,9 +91,8 @@ func Test_FixedPriceEstimator(t *testing.T) { lggr := logger.Test(t) f := gas.NewFixedPriceEstimator(config, &blockHistoryConfig{}, lggr) - fee, gasLimit, err := f.GetDynamicFee(testutils.Context(t), 100000, maxGasPrice) + fee, err := f.GetDynamicFee(testutils.Context(t), 100000, maxGasPrice) require.NoError(t, err) - assert.Equal(t, 100000, int(gasLimit)) assert.Equal(t, assets.NewWeiI(52), fee.TipCap) assert.Equal(t, assets.NewWeiI(100), fee.FeeCap) @@ -101,17 +100,15 @@ func Test_FixedPriceEstimator(t *testing.T) { // Gas bumping disabled config.BumpThresholdF = uint64(0) - fee, gasLimit, err = f.GetDynamicFee(testutils.Context(t), 100000, maxGasPrice) + fee, err = f.GetDynamicFee(testutils.Context(t), 100000, maxGasPrice) require.NoError(t, err) - assert.Equal(t, 100000, int(gasLimit)) assert.Equal(t, assets.NewWeiI(52), fee.TipCap) assert.Equal(t, maxGasPrice, fee.FeeCap) // override max gas price - fee, gasLimit, err = f.GetDynamicFee(testutils.Context(t), 100000, assets.NewWeiI(10)) + fee, err = f.GetDynamicFee(testutils.Context(t), 100000, assets.NewWeiI(10)) require.NoError(t, err) - assert.Equal(t, 100000, int(gasLimit)) assert.Equal(t, assets.NewWeiI(52), fee.TipCap) assert.Equal(t, assets.NewWeiI(10), fee.FeeCap) @@ -128,13 +125,12 @@ func Test_FixedPriceEstimator(t *testing.T) { f := gas.NewFixedPriceEstimator(config, &blockHistoryConfig{}, lggr) originalFee := gas.DynamicFee{FeeCap: assets.NewWeiI(100), TipCap: assets.NewWeiI(25)} - fee, gasLimit, err := f.BumpDynamicFee(testutils.Context(t), originalFee, 100000, maxGasPrice, nil) + fee, err := f.BumpDynamicFee(testutils.Context(t), originalFee, 100000, maxGasPrice, nil) require.NoError(t, err) - expectedFee, expectedGasLimit, err := gas.BumpDynamicFeeOnly(config, 0, lggr, nil, nil, originalFee, 100000, maxGasPrice) + expectedFee, err := gas.BumpDynamicFeeOnly(config, 0, lggr, nil, nil, originalFee, maxGasPrice) require.NoError(t, err) - assert.Equal(t, expectedGasLimit, gasLimit) assert.Equal(t, expectedFee, fee) }) } diff --git a/core/chains/evm/gas/gas_test.go b/core/chains/evm/gas/gas_test.go index 43a1506bc24..8f3d56b54e7 100644 --- a/core/chains/evm/gas/gas_test.go +++ b/core/chains/evm/gas/gas_test.go @@ -5,7 +5,6 @@ import ( "math/big" "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/smartcontractkit/chainlink-common/pkg/logger" @@ -95,12 +94,11 @@ func Test_BumpLegacyGasPriceOnly(t *testing.T) { cfg.BumpMinF = test.bumpMin cfg.PriceMaxF = test.priceMax cfg.LimitMultiplierF = test.limitMultiplierPercent - actual, limit, err := gas.BumpLegacyGasPriceOnly(cfg, logger.TestSugared(t), test.currentGasPrice, test.originalGasPrice, test.originalLimit, test.priceMax) + actual, err := gas.BumpLegacyGasPriceOnly(cfg, logger.TestSugared(t), test.currentGasPrice, test.originalGasPrice, test.priceMax) require.NoError(t, err) if actual.Cmp(test.expectedGasPrice) != 0 { t.Fatalf("Expected %s but got %s", test.expectedGasPrice.String(), actual.String()) } - assert.Equal(t, int(test.expectedLimit), int(limit)) }) } } @@ -115,7 +113,7 @@ func Test_BumpLegacyGasPriceOnly_HitsMaxError(t *testing.T) { cfg.PriceMaxF = priceMax originalGasPrice := toWei("3e10") // 30 GWei - _, _, err := gas.BumpLegacyGasPriceOnly(cfg, logger.TestSugared(t), nil, originalGasPrice, 42, priceMax) + _, err := gas.BumpLegacyGasPriceOnly(cfg, logger.TestSugared(t), nil, originalGasPrice, priceMax) require.Error(t, err) require.Contains(t, err.Error(), "bumped gas price of 45 gwei would exceed configured max gas price of 40 gwei (original price was 30 gwei)") } @@ -132,13 +130,13 @@ func Test_BumpLegacyGasPriceOnly_NoBumpError(t *testing.T) { cfg.PriceMaxF = priceMax originalGasPrice := toWei("3e10") // 30 GWei - _, _, err := gas.BumpLegacyGasPriceOnly(cfg, lggr, nil, originalGasPrice, 42, priceMax) + _, err := gas.BumpLegacyGasPriceOnly(cfg, lggr, nil, originalGasPrice, priceMax) require.Error(t, err) require.Contains(t, err.Error(), "bumped gas price of 30 gwei is equal to original gas price of 30 gwei. ACTION REQUIRED: This is a configuration error, you must increase either EVM.GasEstimator.BumpPercent or EVM.GasEstimator.BumpMin") // Even if it's exactly the maximum originalGasPrice = toWei("4e10") // 40 GWei - _, _, err = gas.BumpLegacyGasPriceOnly(cfg, lggr, nil, originalGasPrice, 42, priceMax) + _, err = gas.BumpLegacyGasPriceOnly(cfg, lggr, nil, originalGasPrice, priceMax) require.Error(t, err) require.Contains(t, err.Error(), "bumped gas price of 40 gwei is equal to original gas price of 40 gwei. ACTION REQUIRED: This is a configuration error, you must increase either EVM.GasEstimator.BumpPercent or EVM.GasEstimator.BumpMin") } @@ -298,7 +296,7 @@ func Test_BumpDynamicFeeOnly(t *testing.T) { cfg.LimitMultiplierF = test.limitMultiplierPercent bufferBlocks := uint16(4) - actual, limit, err := gas.BumpDynamicFeeOnly(cfg, bufferBlocks, logger.TestSugared(t), test.currentTipCap, test.currentBaseFee, test.originalFee, test.originalLimit, test.priceMax) + actual, err := gas.BumpDynamicFeeOnly(cfg, bufferBlocks, logger.TestSugared(t), test.currentTipCap, test.currentBaseFee, test.originalFee, test.priceMax) require.NoError(t, err) if actual.TipCap.Cmp(test.expectedFee.TipCap) != 0 { t.Fatalf("TipCap not equal, expected %s but got %s", test.expectedFee.TipCap.String(), actual.TipCap.String()) @@ -306,7 +304,6 @@ func Test_BumpDynamicFeeOnly(t *testing.T) { if actual.FeeCap.Cmp(test.expectedFee.FeeCap) != 0 { t.Fatalf("FeeCap not equal, expected %s but got %s", test.expectedFee.FeeCap.String(), actual.FeeCap.String()) } - assert.Equal(t, int(test.expectedLimit), int(limit)) }) } } @@ -324,14 +321,14 @@ func Test_BumpDynamicFeeOnly_HitsMaxError(t *testing.T) { t.Run("tip cap hits max", func(t *testing.T) { originalFee := gas.DynamicFee{TipCap: assets.GWei(30), FeeCap: assets.GWei(100)} - _, _, err := gas.BumpDynamicFeeOnly(cfg, 0, logger.TestSugared(t), nil, nil, originalFee, 42, priceMax) + _, err := gas.BumpDynamicFeeOnly(cfg, 0, logger.TestSugared(t), nil, nil, originalFee, priceMax) require.Error(t, err) require.Contains(t, err.Error(), "bumped tip cap of 45 gwei would exceed configured max gas price of 40 gwei (original fee: tip cap 30 gwei, fee cap 100 gwei)") }) t.Run("fee cap hits max", func(t *testing.T) { originalFee := gas.DynamicFee{TipCap: assets.GWei(10), FeeCap: assets.GWei(100)} - _, _, err := gas.BumpDynamicFeeOnly(cfg, 0, logger.TestSugared(t), nil, nil, originalFee, 42, priceMax) + _, err := gas.BumpDynamicFeeOnly(cfg, 0, logger.TestSugared(t), nil, nil, originalFee, priceMax) require.Error(t, err) require.Contains(t, err.Error(), "bumped fee cap of 150 gwei would exceed configured max gas price of 40 gwei (original fee: tip cap 10 gwei, fee cap 100 gwei)") }) diff --git a/core/chains/evm/gas/mocks/evm_estimator.go b/core/chains/evm/gas/mocks/evm_estimator.go index f9ea34b830d..03f9bec88c3 100644 --- a/core/chains/evm/gas/mocks/evm_estimator.go +++ b/core/chains/evm/gas/mocks/evm_estimator.go @@ -22,7 +22,7 @@ type EvmEstimator struct { } // BumpDynamicFee provides a mock function with given fields: ctx, original, gasLimit, maxGasPriceWei, attempts -func (_m *EvmEstimator) BumpDynamicFee(ctx context.Context, original gas.DynamicFee, gasLimit uint64, maxGasPriceWei *assets.Wei, attempts []gas.EvmPriorAttempt) (gas.DynamicFee, uint64, error) { +func (_m *EvmEstimator) BumpDynamicFee(ctx context.Context, original gas.DynamicFee, gasLimit uint64, maxGasPriceWei *assets.Wei, attempts []gas.EvmPriorAttempt) (gas.DynamicFee, error) { ret := _m.Called(ctx, original, gasLimit, maxGasPriceWei, attempts) if len(ret) == 0 { @@ -30,9 +30,8 @@ func (_m *EvmEstimator) BumpDynamicFee(ctx context.Context, original gas.Dynamic } var r0 gas.DynamicFee - var r1 uint64 - var r2 error - if rf, ok := ret.Get(0).(func(context.Context, gas.DynamicFee, uint64, *assets.Wei, []gas.EvmPriorAttempt) (gas.DynamicFee, uint64, error)); ok { + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, gas.DynamicFee, uint64, *assets.Wei, []gas.EvmPriorAttempt) (gas.DynamicFee, error)); ok { return rf(ctx, original, gasLimit, maxGasPriceWei, attempts) } if rf, ok := ret.Get(0).(func(context.Context, gas.DynamicFee, uint64, *assets.Wei, []gas.EvmPriorAttempt) gas.DynamicFee); ok { @@ -41,19 +40,13 @@ func (_m *EvmEstimator) BumpDynamicFee(ctx context.Context, original gas.Dynamic r0 = ret.Get(0).(gas.DynamicFee) } - if rf, ok := ret.Get(1).(func(context.Context, gas.DynamicFee, uint64, *assets.Wei, []gas.EvmPriorAttempt) uint64); ok { + if rf, ok := ret.Get(1).(func(context.Context, gas.DynamicFee, uint64, *assets.Wei, []gas.EvmPriorAttempt) error); ok { r1 = rf(ctx, original, gasLimit, maxGasPriceWei, attempts) } else { - r1 = ret.Get(1).(uint64) - } - - if rf, ok := ret.Get(2).(func(context.Context, gas.DynamicFee, uint64, *assets.Wei, []gas.EvmPriorAttempt) error); ok { - r2 = rf(ctx, original, gasLimit, maxGasPriceWei, attempts) - } else { - r2 = ret.Error(2) + r1 = ret.Error(1) } - return r0, r1, r2 + return r0, r1 } // BumpLegacyGas provides a mock function with given fields: ctx, originalGasPrice, gasLimit, maxGasPriceWei, attempts @@ -112,7 +105,7 @@ func (_m *EvmEstimator) Close() error { } // GetDynamicFee provides a mock function with given fields: ctx, gasLimit, maxGasPriceWei -func (_m *EvmEstimator) GetDynamicFee(ctx context.Context, gasLimit uint64, maxGasPriceWei *assets.Wei) (gas.DynamicFee, uint64, error) { +func (_m *EvmEstimator) GetDynamicFee(ctx context.Context, gasLimit uint64, maxGasPriceWei *assets.Wei) (gas.DynamicFee, error) { ret := _m.Called(ctx, gasLimit, maxGasPriceWei) if len(ret) == 0 { @@ -120,9 +113,8 @@ func (_m *EvmEstimator) GetDynamicFee(ctx context.Context, gasLimit uint64, maxG } var r0 gas.DynamicFee - var r1 uint64 - var r2 error - if rf, ok := ret.Get(0).(func(context.Context, uint64, *assets.Wei) (gas.DynamicFee, uint64, error)); ok { + var r1 error + if rf, ok := ret.Get(0).(func(context.Context, uint64, *assets.Wei) (gas.DynamicFee, error)); ok { return rf(ctx, gasLimit, maxGasPriceWei) } if rf, ok := ret.Get(0).(func(context.Context, uint64, *assets.Wei) gas.DynamicFee); ok { @@ -131,19 +123,13 @@ func (_m *EvmEstimator) GetDynamicFee(ctx context.Context, gasLimit uint64, maxG r0 = ret.Get(0).(gas.DynamicFee) } - if rf, ok := ret.Get(1).(func(context.Context, uint64, *assets.Wei) uint64); ok { + if rf, ok := ret.Get(1).(func(context.Context, uint64, *assets.Wei) error); ok { r1 = rf(ctx, gasLimit, maxGasPriceWei) } else { - r1 = ret.Get(1).(uint64) - } - - if rf, ok := ret.Get(2).(func(context.Context, uint64, *assets.Wei) error); ok { - r2 = rf(ctx, gasLimit, maxGasPriceWei) - } else { - r2 = ret.Error(2) + r1 = ret.Error(1) } - return r0, r1, r2 + return r0, r1 } // GetLegacyGas provides a mock function with given fields: ctx, calldata, gasLimit, maxGasPriceWei, opts diff --git a/core/chains/evm/gas/models.go b/core/chains/evm/gas/models.go index 202b689c177..54a0bf13096 100644 --- a/core/chains/evm/gas/models.go +++ b/core/chains/evm/gas/models.go @@ -131,13 +131,13 @@ type EvmEstimator interface { BumpLegacyGas(ctx context.Context, originalGasPrice *assets.Wei, gasLimit uint64, maxGasPriceWei *assets.Wei, attempts []EvmPriorAttempt) (bumpedGasPrice *assets.Wei, chainSpecificGasLimit uint64, err error) // GetDynamicFee Calculates initial gas fee for gas for EIP1559 transactions // maxGasPriceWei parameter is the highest possible gas fee cap that the function will return - GetDynamicFee(ctx context.Context, gasLimit uint64, maxGasPriceWei *assets.Wei) (fee DynamicFee, chainSpecificGasLimit uint64, err error) + GetDynamicFee(ctx context.Context, gasLimit uint64, maxGasPriceWei *assets.Wei) (fee DynamicFee, err error) // BumpDynamicFee Increases gas price and/or limit for non-EIP1559 transactions // if the bumped gas fee or tip caps are greater than maxGasPriceWei, the method returns an error // attempts must: // - be sorted in order from highest price to lowest price // - all be of transaction type 0x2 - BumpDynamicFee(ctx context.Context, original DynamicFee, gasLimit uint64, maxGasPriceWei *assets.Wei, attempts []EvmPriorAttempt) (bumped DynamicFee, chainSpecificGasLimit uint64, err error) + BumpDynamicFee(ctx context.Context, original DynamicFee, gasLimit uint64, maxGasPriceWei *assets.Wei, attempts []EvmPriorAttempt) (bumped DynamicFee, err error) } var _ feetypes.Fee = (*EvmFee)(nil) @@ -247,11 +247,11 @@ func (e *WrappedEvmEstimator) GetFee(ctx context.Context, calldata []byte, feeLi // get dynamic fee if e.EIP1559Enabled { var dynamicFee DynamicFee - dynamicFee, chainSpecificFeeLimit, err = e.EvmEstimator.GetDynamicFee(ctx, feeLimit, maxFeePrice) + dynamicFee, err = e.EvmEstimator.GetDynamicFee(ctx, feeLimit, maxFeePrice) if err != nil { return } - chainSpecificFeeLimit, err = commonfee.ApplyMultiplier(chainSpecificFeeLimit, e.geCfg.LimitMultiplier()) + chainSpecificFeeLimit, err = commonfee.ApplyMultiplier(feeLimit, e.geCfg.LimitMultiplier()) fee.DynamicFeeCap = dynamicFee.FeeCap fee.DynamicTipCap = dynamicFee.TipCap return @@ -296,7 +296,7 @@ func (e *WrappedEvmEstimator) BumpFee(ctx context.Context, originalFee EvmFee, f // bump dynamic original if originalFee.ValidDynamic() { var bumpedDynamic DynamicFee - bumpedDynamic, chainSpecificFeeLimit, err = e.EvmEstimator.BumpDynamicFee(ctx, + bumpedDynamic, err = e.EvmEstimator.BumpDynamicFee(ctx, DynamicFee{ TipCap: originalFee.DynamicTipCap, FeeCap: originalFee.DynamicFeeCap, @@ -304,7 +304,7 @@ func (e *WrappedEvmEstimator) BumpFee(ctx context.Context, originalFee EvmFee, f if err != nil { return } - chainSpecificFeeLimit, err = commonfee.ApplyMultiplier(chainSpecificFeeLimit, e.geCfg.LimitMultiplier()) + chainSpecificFeeLimit, err = commonfee.ApplyMultiplier(feeLimit, e.geCfg.LimitMultiplier()) bumpedFee.DynamicFeeCap = bumpedDynamic.FeeCap bumpedFee.DynamicTipCap = bumpedDynamic.TipCap return @@ -374,13 +374,12 @@ func HexToInt64(input interface{}) int64 { } } -// BumpLegacyGasPriceOnly will increase the price and apply multiplier to the gas limit -func BumpLegacyGasPriceOnly(cfg bumpConfig, lggr logger.SugaredLogger, currentGasPrice, originalGasPrice *assets.Wei, originalGasLimit uint64, maxGasPriceWei *assets.Wei) (gasPrice *assets.Wei, chainSpecificGasLimit uint64, err error) { +// BumpLegacyGasPriceOnly will increase the price +func BumpLegacyGasPriceOnly(cfg bumpConfig, 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, 0, err + return nil, err } - chainSpecificGasLimit, err = commonfee.ApplyMultiplier(originalGasLimit, cfg.LimitMultiplier()) return } @@ -410,12 +409,11 @@ 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, originalGasLimit uint64, maxGasPriceWei *assets.Wei) (bumped DynamicFee, chainSpecificGasLimit uint64, err error) { +func BumpDynamicFeeOnly(config bumpConfig, 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, 0, err + return bumped, err } - chainSpecificGasLimit, err = commonfee.ApplyMultiplier(originalGasLimit, config.LimitMultiplier()) return } diff --git a/core/chains/evm/gas/suggested_price_estimator.go b/core/chains/evm/gas/suggested_price_estimator.go index fda7387a211..2e77b57ffeb 100644 --- a/core/chains/evm/gas/suggested_price_estimator.go +++ b/core/chains/evm/gas/suggested_price_estimator.go @@ -154,12 +154,12 @@ func (o *SuggestedPriceEstimator) forceRefresh(ctx context.Context) (err error) func (o *SuggestedPriceEstimator) OnNewLongestChain(context.Context, *evmtypes.Head) {} -func (*SuggestedPriceEstimator) GetDynamicFee(_ context.Context, _ uint64, _ *assets.Wei) (fee DynamicFee, chainSpecificGasLimit uint64, err error) { +func (*SuggestedPriceEstimator) GetDynamicFee(_ context.Context, _ uint64, _ *assets.Wei) (fee DynamicFee, err error) { err = pkgerrors.New("dynamic fees are not implemented for this estimator") return } -func (*SuggestedPriceEstimator) BumpDynamicFee(_ context.Context, _ DynamicFee, _ uint64, _ *assets.Wei, _ []EvmPriorAttempt) (bumped DynamicFee, chainSpecificGasLimit uint64, err error) { +func (*SuggestedPriceEstimator) BumpDynamicFee(_ context.Context, _ DynamicFee, _ uint64, _ *assets.Wei, _ []EvmPriorAttempt) (bumped DynamicFee, err error) { err = pkgerrors.New("dynamic fees are not implemented for this estimator") return } diff --git a/core/chains/evm/gas/suggested_price_estimator_test.go b/core/chains/evm/gas/suggested_price_estimator_test.go index ff5e004031b..0f65bfbad10 100644 --- a/core/chains/evm/gas/suggested_price_estimator_test.go +++ b/core/chains/evm/gas/suggested_price_estimator_test.go @@ -98,7 +98,7 @@ func TestSuggestedPriceEstimator(t *testing.T) { t.Run("calling GetDynamicFee always returns error", func(t *testing.T) { client := mocks.NewRPCClient(t) o := gas.NewSuggestedPriceEstimator(logger.Test(t), client, cfg) - _, _, err := o.GetDynamicFee(testutils.Context(t), gasLimit, maxGasPrice) + _, err := o.GetDynamicFee(testutils.Context(t), gasLimit, maxGasPrice) assert.EqualError(t, err, "dynamic fees are not implemented for this estimator") }) @@ -116,7 +116,7 @@ func TestSuggestedPriceEstimator(t *testing.T) { FeeCap: assets.NewWeiI(42), TipCap: assets.NewWeiI(5), } - _, _, err := o.BumpDynamicFee(testutils.Context(t), fee, gasLimit, maxGasPrice, nil) + _, err := o.BumpDynamicFee(testutils.Context(t), fee, gasLimit, maxGasPrice, nil) assert.EqualError(t, err, "dynamic fees are not implemented for this estimator") })