Skip to content

Commit

Permalink
[BCI-2313] Rename L2Suggested to SuggestedPrice (#11093)
Browse files Browse the repository at this point in the history
* Rename L2Suggested to SuggestedPrice

* Rename L2Suggested to SuggestedPrice

* update test error string

* update comment

* update docs

* generate docs

* L2Suggested backwards compatibility

* L2Suggested backwards compatibility

* Update CHANGELOG

* Update comments

* Update docs/CHANGELOG.md

Co-authored-by: amit-momin <108959691+amit-momin@users.noreply.github.com>

* Fix error message

* Move import

* deprecate L2Suggested

* Update CONFIG.md

---------

Co-authored-by: amit-momin <108959691+amit-momin@users.noreply.github.com>
Co-authored-by: Prashant Yadav <34992934+prashantkumar1982@users.noreply.github.com>
  • Loading branch information
3 people committed Nov 1, 2023
1 parent 08c9f89 commit 1a69590
Show file tree
Hide file tree
Showing 20 changed files with 83 additions and 74 deletions.
2 changes: 1 addition & 1 deletion core/chains/evm/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ func Test_chainScopedConfig_Validate(t *testing.T) {
t.Run("testnet", func(t *testing.T) {
cfg := configWithChains(t, 421611, &toml.Chain{
GasEstimator: toml.GasEstimator{
Mode: ptr("L2Suggested"),
Mode: ptr("SuggestedPrice"),
},
})
assert.NoError(t, cfg.Validate())
Expand Down
5 changes: 2 additions & 3 deletions core/chains/evm/config/toml/defaults/Fantom_Mainnet.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,8 @@ RPCBlockQueryDelay = 2
Enabled = true

[GasEstimator]
# Fantom network has been slow to include txs at times when using the BlockHistory estimator, and the recommendation is to use L2Suggested mode.
# There is work under way to improve L2Suggested mode's name so that its use on non-L2 chains will be less confusing in the future.
Mode = 'L2Suggested'
# Fantom network has been slow to include txs at times when using the BlockHistory estimator, and the recommendation is to use SuggestedPrice mode.
Mode = 'SuggestedPrice'

[OCR2.Automation]
GasLimit = 3800000
2 changes: 1 addition & 1 deletion core/chains/evm/config/toml/defaults/Fantom_Testnet.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ NoNewHeadsThreshold = '0'
RPCBlockQueryDelay = 2

[GasEstimator]
Mode = 'L2Suggested'
Mode = 'SuggestedPrice'

[OCR2.Automation]
GasLimit = 3800000
2 changes: 1 addition & 1 deletion core/chains/evm/config/toml/defaults/Klaytn_Mainnet.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ NoNewHeadsThreshold = '30s'
OCR.ContractConfirmations = 1

[GasEstimator]
Mode = 'L2Suggested'
Mode = 'SuggestedPrice'
PriceDefault = '750 gwei' # gwei = ston
BumpThreshold = 0
2 changes: 1 addition & 1 deletion core/chains/evm/config/toml/defaults/Klaytn_Testnet.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ NoNewHeadsThreshold = '30s'
OCR.ContractConfirmations = 1

[GasEstimator]
Mode = 'L2Suggested'
Mode = 'SuggestedPrice'
PriceDefault = '750 gwei' # gwei = ston
BumpThreshold = 0
4 changes: 2 additions & 2 deletions core/chains/evm/config/toml/defaults/Metis_Mainnet.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ NoNewHeadsThreshold = '0'
OCR.ContractConfirmations = 1

[GasEstimator]
Mode = 'L2Suggested'
# Metis uses the L2Suggested estimator; we don't want to place any limits on the minimum gas price
Mode = 'SuggestedPrice'
# Metis uses the SuggestedPrice estimator; we don't want to place any limits on the minimum gas price
PriceMin = '0'
# Never bump gas on metis
BumpThreshold = 0
Expand Down
2 changes: 1 addition & 1 deletion core/chains/evm/config/toml/defaults/Metis_Rinkeby.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ OCR.ContractConfirmations = 1
Enabled = true

[GasEstimator]
Mode = 'L2Suggested'
Mode = 'SuggestedPrice'
PriceMin = '0'
BumpThreshold = 0

Expand Down
4 changes: 2 additions & 2 deletions core/chains/evm/config/toml/defaults/Scroll_Mainnet.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ NoNewHeadsThreshold = '0'
OCR.ContractConfirmations = 1

[GasEstimator]
Mode = 'L2Suggested'
# Scroll uses the L2Suggested estimator; we don't want to place any limits on the minimum gas price
Mode = 'SuggestedPrice'
# Scroll uses the SuggestedPrice estimator; we don't want to place any limits on the minimum gas price
PriceMin = '0'
# Never bump gas on Scroll
BumpThreshold = 0
Expand Down
4 changes: 2 additions & 2 deletions core/chains/evm/config/toml/defaults/Scroll_Sepolia.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ NoNewHeadsThreshold = '0'
OCR.ContractConfirmations = 1

[GasEstimator]
Mode = 'L2Suggested'
# Scroll uses the L2Suggested estimator; we don't want to place any limits on the minimum gas price
Mode = 'SuggestedPrice'
# Scroll uses the SuggestedPrice estimator; we don't want to place any limits on the minimum gas price
PriceMin = '0'
# Never bump gas on Scroll
BumpThreshold = 0
Expand Down
8 changes: 4 additions & 4 deletions core/chains/evm/gas/arbitrum_estimator.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ type ethClient interface {
CallContract(ctx context.Context, msg ethereum.CallMsg, blockNumber *big.Int) ([]byte, error)
}

// arbitrumEstimator is an Estimator which extends l2SuggestedPriceEstimator to use getPricesInArbGas() for gas limit estimation.
// arbitrumEstimator is an Estimator which extends SuggestedPriceEstimator to use getPricesInArbGas() for gas limit estimation.
type arbitrumEstimator struct {
services.StateMachine
cfg ArbConfig

EvmEstimator // *l2SuggestedPriceEstimator
EvmEstimator // *SuggestedPriceEstimator

client ethClient
pollPeriod time.Duration
Expand All @@ -56,7 +56,7 @@ func NewArbitrumEstimator(lggr logger.Logger, cfg ArbConfig, rpcClient rpcClient
lggr = lggr.Named("ArbitrumEstimator")
return &arbitrumEstimator{
cfg: cfg,
EvmEstimator: NewL2SuggestedPriceEstimator(lggr, rpcClient),
EvmEstimator: NewSuggestedPriceEstimator(lggr, rpcClient),
client: ethClient,
pollPeriod: 10 * time.Second,
logger: lggr,
Expand Down Expand Up @@ -99,7 +99,7 @@ func (a *arbitrumEstimator) HealthReport() map[string]error {
}

// GetLegacyGas estimates both the gas price and the gas limit.
// - Price is delegated to the embedded l2SuggestedPriceEstimator.
// - Price is delegated to the embedded SuggestedPriceEstimator.
// - Limit is computed from the dynamic values perL2Tx and perL1CalldataUnit, provided by the getPricesInArbGas() method
// of the precompilie contract at ArbGasInfoAddress. perL2Tx is a constant amount of gas, and perL1CalldataUnit is
// multiplied by the length of the tx calldata. The sum of these two values plus the original l2GasLimit is returned.
Expand Down
4 changes: 2 additions & 2 deletions core/chains/evm/gas/arbitrum_estimator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ func TestArbitrumEstimator(t *testing.T) {
ethClient := mocks.NewETHClient(t)
o := gas.NewArbitrumEstimator(logger.TestLogger(t), &arbConfig{}, rpcClient, ethClient)
_, _, err := o.BumpLegacyGas(testutils.Context(t), assets.NewWeiI(42), gasLimit, assets.NewWeiI(10), nil)
assert.EqualError(t, err, "bump gas is not supported for this l2")
assert.EqualError(t, err, "bump gas is not supported for this chain")
})

t.Run("calling GetLegacyGas on started estimator if initial call failed returns error", func(t *testing.T) {
Expand All @@ -152,7 +152,7 @@ func TestArbitrumEstimator(t *testing.T) {
t.Cleanup(func() { assert.NoError(t, o.Close()) })

_, _, err := o.GetLegacyGas(testutils.Context(t), calldata, gasLimit, maxGasPrice)
assert.EqualError(t, err, "failed to estimate l2 gas; gas price not set")
assert.EqualError(t, err, "failed to estimate gas; gas price not set")
})

t.Run("limit computes", func(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions core/chains/evm/gas/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ func NewEstimator(lggr logger.Logger, ethClient evmclient.Client, cfg Config, ge
return NewWrappedEvmEstimator(NewBlockHistoryEstimator(lggr, ethClient, cfg, geCfg, bh, *ethClient.ConfiguredChainID()), df, l1Oracle)
case "FixedPrice":
return NewWrappedEvmEstimator(NewFixedPriceEstimator(geCfg, bh, lggr), df, l1Oracle)
case "Optimism2", "L2Suggested":
return NewWrappedEvmEstimator(NewL2SuggestedPriceEstimator(lggr, ethClient), df, l1Oracle)
case "L2Suggested", "SuggestedPrice":
return NewWrappedEvmEstimator(NewSuggestedPriceEstimator(lggr, ethClient), df, l1Oracle)
default:
lggr.Warnf("GasEstimator: unrecognised mode '%s', falling back to FixedPriceEstimator", s)
return NewWrappedEvmEstimator(NewFixedPriceEstimator(geCfg, bh, lggr), df, l1Oracle)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,68 +19,68 @@ import (
)

var (
_ EvmEstimator = &l2SuggestedPriceEstimator{}
_ EvmEstimator = &SuggestedPriceEstimator{}
)

//go:generate mockery --quiet --name rpcClient --output ./mocks/ --case=underscore --structname RPCClient
type rpcClient interface {
CallContext(ctx context.Context, result interface{}, method string, args ...interface{}) error
}

// l2SuggestedPriceEstimator is an Estimator which uses the L2 suggested gas price from eth_gasPrice.
type l2SuggestedPriceEstimator struct {
// SuggestedPriceEstimator is an Estimator which uses the suggested gas price from eth_gasPrice.
type SuggestedPriceEstimator struct {
services.StateMachine

client rpcClient
pollPeriod time.Duration
logger logger.Logger

gasPriceMu sync.RWMutex
l2GasPrice *assets.Wei
GasPrice *assets.Wei

chForceRefetch chan (chan struct{})
chInitialised chan struct{}
chStop utils.StopChan
chDone chan struct{}
}

// NewL2SuggestedPriceEstimator returns a new Estimator which uses the L2 suggested gas price.
func NewL2SuggestedPriceEstimator(lggr logger.Logger, client rpcClient) EvmEstimator {
return &l2SuggestedPriceEstimator{
// NewSuggestedPriceEstimator returns a new Estimator which uses the suggested gas price.
func NewSuggestedPriceEstimator(lggr logger.Logger, client rpcClient) EvmEstimator {
return &SuggestedPriceEstimator{
client: client,
pollPeriod: 10 * time.Second,
logger: lggr.Named("L2SuggestedEstimator"),
logger: lggr.Named("SuggestedPriceEstimator"),
chForceRefetch: make(chan (chan struct{})),
chInitialised: make(chan struct{}),
chStop: make(chan struct{}),
chDone: make(chan struct{}),
}
}

func (o *l2SuggestedPriceEstimator) Name() string {
func (o *SuggestedPriceEstimator) Name() string {
return o.logger.Name()
}

func (o *l2SuggestedPriceEstimator) Start(context.Context) error {
return o.StartOnce("L2SuggestedEstimator", func() error {
func (o *SuggestedPriceEstimator) Start(context.Context) error {
return o.StartOnce("SuggestedPriceEstimator", func() error {
go o.run()
<-o.chInitialised
return nil
})
}
func (o *l2SuggestedPriceEstimator) Close() error {
return o.StopOnce("L2SuggestedEstimator", func() error {
func (o *SuggestedPriceEstimator) Close() error {
return o.StopOnce("SuggestedPriceEstimator", func() error {
close(o.chStop)
<-o.chDone
return nil
})
}

func (o *l2SuggestedPriceEstimator) HealthReport() map[string]error {
func (o *SuggestedPriceEstimator) HealthReport() map[string]error {
return map[string]error{o.Name(): o.Healthy()}
}

func (o *l2SuggestedPriceEstimator) run() {
func (o *SuggestedPriceEstimator) run() {
defer close(o.chDone)

t := o.refreshPrice()
Expand All @@ -100,7 +100,7 @@ func (o *l2SuggestedPriceEstimator) run() {
}
}

func (o *l2SuggestedPriceEstimator) refreshPrice() (t *time.Timer) {
func (o *SuggestedPriceEstimator) refreshPrice() (t *time.Timer) {
t = time.NewTimer(utils.WithJitter(o.pollPeriod))

var res hexutil.Big
Expand All @@ -113,28 +113,28 @@ func (o *l2SuggestedPriceEstimator) refreshPrice() (t *time.Timer) {
}
bi := (*assets.Wei)(&res)

o.logger.Debugw("refreshPrice", "l2GasPrice", bi)
o.logger.Debugw("refreshPrice", "GasPrice", bi)

o.gasPriceMu.Lock()
defer o.gasPriceMu.Unlock()
o.l2GasPrice = bi
o.GasPrice = bi
return
}

func (o *l2SuggestedPriceEstimator) OnNewLongestChain(context.Context, *evmtypes.Head) {}
func (o *SuggestedPriceEstimator) OnNewLongestChain(context.Context, *evmtypes.Head) {}

func (*l2SuggestedPriceEstimator) GetDynamicFee(_ context.Context, _ uint32, _ *assets.Wei) (fee DynamicFee, chainSpecificGasLimit uint32, err error) {
func (*SuggestedPriceEstimator) GetDynamicFee(_ context.Context, _ uint32, _ *assets.Wei) (fee DynamicFee, chainSpecificGasLimit uint32, err error) {
err = errors.New("dynamic fees are not implemented for this layer 2")
return
}

func (*l2SuggestedPriceEstimator) BumpDynamicFee(_ context.Context, _ DynamicFee, _ uint32, _ *assets.Wei, _ []EvmPriorAttempt) (bumped DynamicFee, chainSpecificGasLimit uint32, err error) {
func (*SuggestedPriceEstimator) BumpDynamicFee(_ context.Context, _ DynamicFee, _ uint32, _ *assets.Wei, _ []EvmPriorAttempt) (bumped DynamicFee, chainSpecificGasLimit uint32, err error) {
err = errors.New("dynamic fees are not implemented for this layer 2")
return
}

func (o *l2SuggestedPriceEstimator) GetLegacyGas(ctx context.Context, _ []byte, l2GasLimit uint32, maxGasPriceWei *assets.Wei, opts ...feetypes.Opt) (gasPrice *assets.Wei, chainSpecificGasLimit uint32, err error) {
chainSpecificGasLimit = l2GasLimit
func (o *SuggestedPriceEstimator) GetLegacyGas(ctx context.Context, _ []byte, GasLimit uint32, maxGasPriceWei *assets.Wei, opts ...feetypes.Opt) (gasPrice *assets.Wei, chainSpecificGasLimit uint32, err error) {
chainSpecificGasLimit = GasLimit

ok := o.IfStarted(func() {
if slices.Contains(opts, feetypes.OptForceRefetch) {
Expand All @@ -159,10 +159,10 @@ func (o *l2SuggestedPriceEstimator) GetLegacyGas(ctx context.Context, _ []byte,
}
}
if gasPrice = o.getGasPrice(); gasPrice == nil {
err = errors.New("failed to estimate l2 gas; gas price not set")
err = errors.New("failed to estimate gas; gas price not set")
return
}
o.logger.Debugw("GetLegacyGas", "l2GasPrice", gasPrice, "l2GasLimit", l2GasLimit)
o.logger.Debugw("GetLegacyGas", "GasPrice", gasPrice, "GasLimit", GasLimit)
})
if !ok {
return nil, 0, errors.New("estimator is not started")
Expand All @@ -176,12 +176,12 @@ func (o *l2SuggestedPriceEstimator) GetLegacyGas(ctx context.Context, _ []byte,
return
}

func (o *l2SuggestedPriceEstimator) BumpLegacyGas(_ context.Context, _ *assets.Wei, _ uint32, _ *assets.Wei, _ []EvmPriorAttempt) (bumpedGasPrice *assets.Wei, chainSpecificGasLimit uint32, err error) {
return nil, 0, errors.New("bump gas is not supported for this l2")
func (o *SuggestedPriceEstimator) BumpLegacyGas(_ context.Context, _ *assets.Wei, _ uint32, _ *assets.Wei, _ []EvmPriorAttempt) (bumpedGasPrice *assets.Wei, chainSpecificGasLimit uint32, err error) {
return nil, 0, errors.New("bump gas is not supported for this chain")
}

func (o *l2SuggestedPriceEstimator) getGasPrice() (l2GasPrice *assets.Wei) {
func (o *SuggestedPriceEstimator) getGasPrice() (GasPrice *assets.Wei) {
o.gasPriceMu.RLock()
defer o.gasPriceMu.RUnlock()
return o.l2GasPrice
return o.GasPrice
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
"github.com/smartcontractkit/chainlink/v2/core/logger"
)

func TestL2SuggestedEstimator(t *testing.T) {
func TestSuggestedPriceEstimator(t *testing.T) {
t.Parallel()

maxGasPrice := assets.NewWeiI(100)
Expand All @@ -27,7 +27,7 @@ func TestL2SuggestedEstimator(t *testing.T) {

t.Run("calling GetLegacyGas on unstarted estimator returns error", func(t *testing.T) {
client := mocks.NewRPCClient(t)
o := gas.NewL2SuggestedPriceEstimator(logger.TestLogger(t), client)
o := gas.NewSuggestedPriceEstimator(logger.TestLogger(t), client)
_, _, err := o.GetLegacyGas(testutils.Context(t), calldata, gasLimit, maxGasPrice)
assert.EqualError(t, err, "estimator is not started")
})
Expand All @@ -39,7 +39,7 @@ func TestL2SuggestedEstimator(t *testing.T) {
(*big.Int)(res).SetInt64(42)
})

o := gas.NewL2SuggestedPriceEstimator(logger.TestLogger(t), client)
o := gas.NewSuggestedPriceEstimator(logger.TestLogger(t), client)
require.NoError(t, o.Start(testutils.Context(t)))
t.Cleanup(func() { assert.NoError(t, o.Close()) })
gasPrice, chainSpecificGasLimit, err := o.GetLegacyGas(testutils.Context(t), calldata, gasLimit, maxGasPrice)
Expand All @@ -50,7 +50,7 @@ func TestL2SuggestedEstimator(t *testing.T) {

t.Run("gas price is lower than user specified max gas price", func(t *testing.T) {
client := mocks.NewRPCClient(t)
o := gas.NewL2SuggestedPriceEstimator(logger.TestLogger(t), client)
o := gas.NewSuggestedPriceEstimator(logger.TestLogger(t), client)

client.On("CallContext", mock.Anything, mock.Anything, "eth_gasPrice").Return(nil).Run(func(args mock.Arguments) {
res := args.Get(1).(*hexutil.Big)
Expand All @@ -68,7 +68,7 @@ func TestL2SuggestedEstimator(t *testing.T) {

t.Run("gas price is lower than global max gas price", func(t *testing.T) {
client := mocks.NewRPCClient(t)
o := gas.NewL2SuggestedPriceEstimator(logger.TestLogger(t), client)
o := gas.NewSuggestedPriceEstimator(logger.TestLogger(t), client)

client.On("CallContext", mock.Anything, mock.Anything, "eth_gasPrice").Return(nil).Run(func(args mock.Arguments) {
res := args.Get(1).(*hexutil.Big)
Expand All @@ -85,21 +85,21 @@ func TestL2SuggestedEstimator(t *testing.T) {

t.Run("calling BumpLegacyGas always returns error", func(t *testing.T) {
client := mocks.NewRPCClient(t)
o := gas.NewL2SuggestedPriceEstimator(logger.TestLogger(t), client)
o := gas.NewSuggestedPriceEstimator(logger.TestLogger(t), client)
_, _, err := o.BumpLegacyGas(testutils.Context(t), assets.NewWeiI(42), gasLimit, assets.NewWeiI(10), nil)
assert.EqualError(t, err, "bump gas is not supported for this l2")
assert.EqualError(t, err, "bump gas is not supported for this chain")
})

t.Run("calling GetLegacyGas on started estimator if initial call failed returns error", func(t *testing.T) {
client := mocks.NewRPCClient(t)
o := gas.NewL2SuggestedPriceEstimator(logger.TestLogger(t), client)
o := gas.NewSuggestedPriceEstimator(logger.TestLogger(t), client)

client.On("CallContext", mock.Anything, mock.Anything, "eth_gasPrice").Return(errors.New("kaboom"))

require.NoError(t, o.Start(testutils.Context(t)))
t.Cleanup(func() { assert.NoError(t, o.Close()) })

_, _, err := o.GetLegacyGas(testutils.Context(t), calldata, gasLimit, maxGasPrice)
assert.EqualError(t, err, "failed to estimate l2 gas; gas price not set")
assert.EqualError(t, err, "failed to estimate gas; gas price not set")
})
}
3 changes: 2 additions & 1 deletion core/config/docs/chains-evm.toml
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ Enabled = true # Default
#
# - `FixedPrice` uses static configured values for gas price (can be set via API call).
# - `BlockHistory` dynamically adjusts default gas price based on heuristics from mined blocks.
# - `L2Suggested` is a special mode only for use with L2 blockchains. This mode will use the gas price suggested by the rpc endpoint via `eth_gasPrice`.
# - `L2Suggested` mode is deprecated and replaced with `SuggestedPrice`.
# - `SuggestedPrice` is a mode which uses the gas price suggested by the rpc endpoint via `eth_gasPrice`.
# - `Arbitrum` is a special mode only for use with Arbitrum blockchains. It uses the suggested gas price (up to `ETH_MAX_GAS_PRICE_WEI`, with `1000 gwei` default) as well as an estimated gas limit (up to `ETH_GAS_LIMIT_MAX`, with `1,000,000,000` default).
#
# Chainlink nodes decide what gas price to use using an `Estimator`. It ships with several simple and battle-hardened built-in estimators that should work well for almost all use-cases. Note that estimators will change their behaviour slightly depending on if you are in EIP-1559 mode or not.
Expand Down
Loading

0 comments on commit 1a69590

Please sign in to comment.