Skip to content

Commit

Permalink
removed chainSpecificGasLimit from BumpDynamicFee, GetDynamicFee
Browse files Browse the repository at this point in the history
  • Loading branch information
@silaslenihan
silaslenihan committed Mar 26, 2024
1 parent ebf465a commit d84c214
Show file tree
Hide file tree
Showing 10 changed files with 83 additions and 120 deletions.
4 changes: 2 additions & 2 deletions core/chains/evm/gas/arbitrum_estimator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})

Expand All @@ -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")
})

Expand Down
21 changes: 12 additions & 9 deletions core/chains/evm/gas/block_history_estimator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -457,18 +460,18 @@ 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) {
b.logger.Criticalw(BumpingHaltedLabel, "err", err)
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() {
Expand Down
63 changes: 24 additions & 39 deletions core/chains/evm/gas/block_history_estimator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})
Expand All @@ -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")
})
Expand Down Expand Up @@ -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")
})
Expand Down Expand Up @@ -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))
})
}

Expand Down Expand Up @@ -1992,18 +1991,17 @@ 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?")
})

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)
Expand All @@ -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)
Expand All @@ -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))
})
}

Expand Down Expand Up @@ -2401,18 +2394,17 @@ 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)

t.Run("ignores nil current gas price", func(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)
})

Expand All @@ -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)
})

Expand All @@ -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)
})

Expand All @@ -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)
})

Expand Down Expand Up @@ -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))
Expand All @@ -2515,58 +2507,52 @@ 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)
})

t.Run("bumped tip cap price > max gas price", func(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)")
})
Expand All @@ -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)")
})
Expand Down
10 changes: 4 additions & 6 deletions core/chains/evm/gas/fixed_price_estimator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -108,7 +107,7 @@ func (f *fixedPriceEstimator) GetDynamicFee(_ context.Context, originalGasLimit
return DynamicFee{
FeeCap: feeCap,
TipCap: gasTipCap,
}, chainSpecificGasLimit, nil
}, nil
}

func (f *fixedPriceEstimator) BumpDynamicFee(
Expand All @@ -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,
Expand All @@ -126,7 +125,6 @@ func (f *fixedPriceEstimator) BumpDynamicFee(
f.config.TipCapDefault(),
nil,
originalFee,
originalGasLimit,
maxGasPriceWei,
)
}
Expand Down
Loading

0 comments on commit d84c214

Please sign in to comment.