Skip to content

Commit

Permalink
Add optimizations
Browse files Browse the repository at this point in the history
  • Loading branch information
@dimriou
dimriou committed Sep 3, 2024
1 parent 3f70efa commit 8fcf025
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 46 deletions.
51 changes: 32 additions & 19 deletions core/chains/evm/gas/fee_history_estimator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,19 +91,21 @@ type FeeHistoryEstimator struct {

l1Oracle rollups.L1Oracle

wg *sync.WaitGroup
stopCh services.StopChan
wg *sync.WaitGroup
stopCh services.StopChan
refreshCh chan struct{}
}

func NewFeeHistoryEstimator(lggr logger.Logger, client feeHistoryEstimatorClient, cfg FeeHistoryEstimatorConfig, chainID *big.Int, l1Oracle rollups.L1Oracle) *FeeHistoryEstimator {
return &FeeHistoryEstimator{
client: client,
logger: logger.Named(lggr, "FeeHistoryEstimator"),
config: cfg,
chainID: chainID,
l1Oracle: l1Oracle,
wg: new(sync.WaitGroup),
stopCh: make(chan struct{}),
client: client,
logger: logger.Named(lggr, "FeeHistoryEstimator"),
config: cfg,
chainID: chainID,
l1Oracle: l1Oracle,
wg: new(sync.WaitGroup),
stopCh: make(chan struct{}),
refreshCh: make(chan struct{}),
}
}

Expand Down Expand Up @@ -143,13 +145,15 @@ func (f *FeeHistoryEstimator) run() {
select {
case <-f.stopCh:
return
case <-f.refreshCh:
t.Reset()
case <-t.C:
if f.config.EIP1559 {
if err := f.RefreshDynamicPrice(); err != nil {
f.logger.Error(err)
}
} else {
if err := f.RefreshGasPrice(); err != nil {
if _, err := f.RefreshGasPrice(); err != nil {
f.logger.Error(err)
}
}
Expand All @@ -172,13 +176,13 @@ func (f *FeeHistoryEstimator) GetLegacyGas(ctx context.Context, _ []byte, gasLim
}

// RefreshGasPrice will use eth_gasPrice to fetch and cache the latest gas price from the RPC.
func (f *FeeHistoryEstimator) RefreshGasPrice() error {
func (f *FeeHistoryEstimator) RefreshGasPrice() (*assets.Wei, error) {
ctx, cancel := f.stopCh.CtxCancel(evmclient.ContextWithDefaultTimeout())
defer cancel()

gasPrice, err := f.client.SuggestGasPrice(ctx)
if err != nil {
return fmt.Errorf("failed to fetch gas price: %w", err)
return nil, err
}

promFeeHistoryEstimatorGasPrice.WithLabelValues(f.chainID.String()).Set(float64(gasPrice.Int64()))
Expand All @@ -190,7 +194,7 @@ func (f *FeeHistoryEstimator) RefreshGasPrice() error {
f.gasPriceMu.Lock()
defer f.gasPriceMu.Unlock()
f.gasPrice = gasPriceWei
return nil
return f.gasPrice, nil
}

func (f *FeeHistoryEstimator) getGasPrice() (*assets.Wei, error) {
Expand Down Expand Up @@ -233,7 +237,7 @@ func (f *FeeHistoryEstimator) RefreshDynamicPrice() error {
// RewardPercentile will be used for maxPriorityFeePerGas estimations and connectivityPercentile to set the highest threshold for bumping.
feeHistory, err := f.client.FeeHistory(ctx, max(f.config.BlockHistorySize, 1), []float64{f.config.RewardPercentile, ConnectivityPercentile})
if err != nil {
return fmt.Errorf("failed to fetch dynamic prices: %w", err)
return err
}

// eth_feeHistory doesn't return the latest baseFee of the range but rather the latest + 1, because it can be derived from the existing
Expand Down Expand Up @@ -313,10 +317,11 @@ func (f *FeeHistoryEstimator) BumpLegacyGas(ctx context.Context, originalGasPric
commonfee.ErrBump, originalGasPrice, maxPrice)
}

currentGasPrice, err := f.getGasPrice()
currentGasPrice, err := f.RefreshGasPrice()
if err != nil {
return nil, 0, err
}
f.IfStarted(func() { f.refreshCh <- struct{}{} })

bumpedGasPrice := originalGasPrice.AddPercentage(f.config.BumpPercent)
bumpedGasPrice, err = LimitBumpedFee(originalGasPrice, currentGasPrice, bumpedGasPrice, maxPrice)
Expand All @@ -336,13 +341,21 @@ func (f *FeeHistoryEstimator) BumpLegacyGas(ctx context.Context, originalGasPric
// See: https://github.com/ethereum/go-ethereum/issues/24284
// It aggregates the market, bumped, and max price to provide a correct value, for both maxFeePerGas as well as maxPriorityFerPergas.
func (f *FeeHistoryEstimator) BumpDynamicFee(ctx context.Context, originalFee DynamicFee, maxPrice *assets.Wei, _ []EvmPriorAttempt) (bumped DynamicFee, err error) {
// For chains that don't have a mempool there is no concept of gas bumping so we force-call FetchDynamicPrice to update the underlying base fee value
// For chains that don't have a mempool there is no concept of gas bumping so we force-call RefreshDynamicPrice to update the underlying base fee value
if f.config.BlockHistorySize == 0 {
if err = f.RefreshDynamicPrice(); err != nil {
return
if !f.IfStarted(func() {
if refreshErr := f.RefreshDynamicPrice(); refreshErr != nil {
err = refreshErr
return
}
f.refreshCh <- struct{}{}
bumped, err = f.GetDynamicFee(ctx, maxPrice)
}) {
return bumped, fmt.Errorf("estimator not started")
}
return f.GetDynamicFee(ctx, maxPrice)
return bumped, err
}

// Sanitize original fee input
// According to geth's spec we need to bump both maxFeePerGas and maxPriorityFeePerGas for the new attempt to be accepted by the RPC
if originalFee.FeeCap == nil ||
Expand Down
38 changes: 11 additions & 27 deletions core/chains/evm/gas/fee_history_estimator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/stretchr/testify/mock"

"github.com/smartcontractkit/chainlink-common/pkg/logger"
"github.com/smartcontractkit/chainlink-common/pkg/services/servicetest"
"github.com/smartcontractkit/chainlink-common/pkg/utils/tests"

"github.com/smartcontractkit/chainlink/v2/core/chains/evm/assets"
Expand Down Expand Up @@ -85,7 +86,7 @@ func TestFeeHistoryEstimatorGetLegacyGas(t *testing.T) {
cfg := gas.FeeHistoryEstimatorConfig{}

u := gas.NewFeeHistoryEstimator(logger.Test(t), client, cfg, chainID, nil)
err := u.RefreshGasPrice()
_, err := u.RefreshGasPrice()
assert.NoError(t, err)
gasPrice, _, err := u.GetLegacyGas(tests.Context(t), nil, gasLimit, maxPrice)
assert.NoError(t, err)
Expand All @@ -100,7 +101,7 @@ func TestFeeHistoryEstimatorGetLegacyGas(t *testing.T) {

maxPrice := assets.NewWeiI(1)
u := gas.NewFeeHistoryEstimator(logger.Test(t), client, cfg, chainID, nil)
err := u.RefreshGasPrice()
_, err := u.RefreshGasPrice()
assert.NoError(t, err)
gas1, _, err := u.GetLegacyGas(tests.Context(t), nil, gasLimit, maxPrice)
assert.NoError(t, err)
Expand Down Expand Up @@ -128,13 +129,12 @@ func TestFeeHistoryEstimatorBumpLegacyGas(t *testing.T) {
t.Run("bumps a previous attempt by BumpPercent", func(t *testing.T) {
client := mocks.NewFeeHistoryEstimatorClient(t)
originalGasPrice := assets.NewWeiI(10)
client.On("SuggestGasPrice", mock.Anything).Return(big.NewInt(10), nil).Once()
client.On("SuggestGasPrice", mock.Anything).Return(big.NewInt(10), nil)

cfg := gas.FeeHistoryEstimatorConfig{BumpPercent: 50}
cfg := gas.FeeHistoryEstimatorConfig{BumpPercent: 50, CacheTimeout: 5 * time.Second}

u := gas.NewFeeHistoryEstimator(logger.Test(t), client, cfg, chainID, nil)
err := u.RefreshGasPrice()
assert.NoError(t, err)
servicetest.RunHealthy(t, u)
gasPrice, _, err := u.BumpLegacyGas(tests.Context(t), originalGasPrice, gasLimit, maxPrice, nil)
assert.NoError(t, err)
assert.Equal(t, assets.NewWeiI(15), gasPrice)
Expand All @@ -155,17 +155,6 @@ func TestFeeHistoryEstimatorBumpLegacyGas(t *testing.T) {
assert.Error(t, err)
})

t.Run("fails if we try to bump but gas price has not been set", func(t *testing.T) {
originalGasPrice := assets.NewWeiI(10)

cfg := gas.FeeHistoryEstimatorConfig{}

u := gas.NewFeeHistoryEstimator(logger.Test(t), nil, cfg, chainID, nil)
_, _, err := u.BumpLegacyGas(tests.Context(t), originalGasPrice, gasLimit, maxPrice, nil)
assert.Error(t, err)
assert.ErrorContains(t, err, "gas price not set")
})

t.Run("returns market gas price if bumped original fee is lower", func(t *testing.T) {
client := mocks.NewFeeHistoryEstimatorClient(t)
client.On("SuggestGasPrice", mock.Anything).Return(big.NewInt(80), nil).Once()
Expand All @@ -174,8 +163,6 @@ func TestFeeHistoryEstimatorBumpLegacyGas(t *testing.T) {
cfg := gas.FeeHistoryEstimatorConfig{}

u := gas.NewFeeHistoryEstimator(logger.Test(t), client, cfg, chainID, nil)
err := u.RefreshGasPrice()
assert.NoError(t, err)
gas, _, err := u.BumpLegacyGas(tests.Context(t), originalGasPrice, gasLimit, maxPrice, nil)
assert.NoError(t, err)
assert.Equal(t, assets.NewWeiI(80), gas)
Expand All @@ -190,8 +177,6 @@ func TestFeeHistoryEstimatorBumpLegacyGas(t *testing.T) {

maxPrice := assets.NewWeiI(14)
u := gas.NewFeeHistoryEstimator(logger.Test(t), client, cfg, chainID, nil)
err := u.RefreshGasPrice()
assert.NoError(t, err)
gas, _, err := u.BumpLegacyGas(tests.Context(t), originalGasPrice, gasLimit, maxPrice, nil)
assert.NoError(t, err)
assert.Equal(t, maxPrice, gas)
Expand All @@ -206,8 +191,6 @@ func TestFeeHistoryEstimatorBumpLegacyGas(t *testing.T) {

maxPrice := assets.NewWeiI(14)
u := gas.NewFeeHistoryEstimator(logger.Test(t), client, cfg, chainID, nil)
err := u.RefreshGasPrice()
assert.NoError(t, err)
gas, _, err := u.BumpLegacyGas(tests.Context(t), originalGasPrice, gasLimit, maxPrice, nil)
assert.NoError(t, err)
assert.Equal(t, maxPrice, gas)
Expand All @@ -223,9 +206,7 @@ func TestFeeHistoryEstimatorBumpLegacyGas(t *testing.T) {
// Price will be capped by the max price
maxPrice := assets.NewWeiI(101)
u := gas.NewFeeHistoryEstimator(logger.Test(t), client, cfg, chainID, nil)
err := u.RefreshGasPrice()
assert.NoError(t, err)
_, _, err = u.BumpLegacyGas(tests.Context(t), originalGasPrice, gasLimit, maxPrice, nil)
_, _, err := u.BumpLegacyGas(tests.Context(t), originalGasPrice, gasLimit, maxPrice, nil)
assert.Error(t, err)
})
}
Expand Down Expand Up @@ -512,15 +493,18 @@ func TestFeeHistoryEstimatorBumpDynamicFee(t *testing.T) {
BaseFee: []*big.Int{baseFee},
GasUsedRatio: nil,
}
client.On("FeeHistory", mock.Anything, mock.Anything, mock.Anything).Return(feeHistoryResult, nil).Once()
client.On("FeeHistory", mock.Anything, mock.Anything, mock.Anything).Return(feeHistoryResult, nil)

cfg := gas.FeeHistoryEstimatorConfig{
BlockHistorySize: 0,
BumpPercent: 20,
CacheTimeout: 10 * time.Second,
EIP1559: true,
}

maxFeePerGas := assets.NewWei(baseFee).AddPercentage(gas.BaseFeeBufferPercentage)
u := gas.NewFeeHistoryEstimator(logger.Test(t), client, cfg, chainID, nil)
servicetest.RunHealthy(t, u)
bumpedFee, err := u.BumpDynamicFee(tests.Context(t), originalFee, globalMaxPrice, nil)
assert.NoError(t, err)
assert.Equal(t, assets.NewWeiI(0), (*assets.Wei)(maxPriorityFeePerGas))
Expand Down

0 comments on commit 8fcf025

Please sign in to comment.