From 42c5a002592825be16f8c4ec2e5216e55d2a0d7f Mon Sep 17 00:00:00 2001 From: Matt Yang Date: Fri, 15 Sep 2023 18:45:37 -0400 Subject: [PATCH] address comments --- .../chains/evm/gas/mocks/evm_fee_estimator.go | 12 +++--- core/chains/evm/gas/models.go | 16 ++++---- core/chains/evm/gas/models_test.go | 12 +++--- .../l1_gas_price_oracle.go | 6 +-- .../l1_gas_price_oracle_test.go | 15 +++---- .../mocks/eth_client.go | 3 +- .../mocks/l1_oracle.go | 40 +++++++++---------- .../gas/{chainoracles => rollups}/models.go | 4 +- 8 files changed, 54 insertions(+), 54 deletions(-) rename core/chains/evm/gas/{chainoracles => rollups}/l1_gas_price_oracle.go (96%) rename core/chains/evm/gas/{chainoracles => rollups}/l1_gas_price_oracle_test.go (84%) rename core/chains/evm/gas/{chainoracles => rollups}/mocks/eth_client.go (99%) rename core/chains/evm/gas/{chainoracles => rollups}/mocks/l1_oracle.go (94%) rename core/chains/evm/gas/{chainoracles => rollups}/models.go (85%) diff --git a/core/chains/evm/gas/mocks/evm_fee_estimator.go b/core/chains/evm/gas/mocks/evm_fee_estimator.go index 8fb0409154f..dbca58dcdd5 100644 --- a/core/chains/evm/gas/mocks/evm_fee_estimator.go +++ b/core/chains/evm/gas/mocks/evm_fee_estimator.go @@ -7,8 +7,6 @@ import ( assets "github.com/smartcontractkit/chainlink/v2/core/assets" - chainoracles "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas/chainoracles" - context "context" evmtypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types" @@ -17,6 +15,8 @@ import ( mock "github.com/stretchr/testify/mock" + rollups "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas/rollups" + types "github.com/smartcontractkit/chainlink/v2/common/fee/types" ) @@ -158,15 +158,15 @@ func (_m *EvmFeeEstimator) HealthReport() map[string]error { } // L1Oracle provides a mock function with given fields: -func (_m *EvmFeeEstimator) L1Oracle() chainoracles.L1Oracle { +func (_m *EvmFeeEstimator) L1Oracle() rollups.L1Oracle { ret := _m.Called() - var r0 chainoracles.L1Oracle - if rf, ok := ret.Get(0).(func() chainoracles.L1Oracle); ok { + var r0 rollups.L1Oracle + if rf, ok := ret.Get(0).(func() rollups.L1Oracle); ok { r0 = rf() } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(chainoracles.L1Oracle) + r0 = ret.Get(0).(rollups.L1Oracle) } } diff --git a/core/chains/evm/gas/models.go b/core/chains/evm/gas/models.go index b7f4377767f..c6f8edbf04b 100644 --- a/core/chains/evm/gas/models.go +++ b/core/chains/evm/gas/models.go @@ -16,7 +16,7 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/assets" evmclient "github.com/smartcontractkit/chainlink/v2/core/chains/evm/client" evmconfig "github.com/smartcontractkit/chainlink/v2/core/chains/evm/config" - "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas/chainoracles" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas/rollups" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/label" evmtypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types" "github.com/smartcontractkit/chainlink/v2/core/config" @@ -34,7 +34,7 @@ type EvmFeeEstimator interface { commontypes.HeadTrackable[*evmtypes.Head, common.Hash] // L1Oracle returns the L1 gas price oracle only if the chain has one, e.g. OP stack L2s and Arbitrum. - L1Oracle() chainoracles.L1Oracle + L1Oracle() rollups.L1Oracle GetFee(ctx context.Context, calldata []byte, feeLimit uint32, maxFeePrice *assets.Wei, opts ...feetypes.Opt) (fee EvmFee, chainSpecificFeeLimit uint32, err error) BumpFee(ctx context.Context, originalFee EvmFee, feeLimit uint32, maxFeePrice *assets.Wei, attempts []EvmPriorAttempt) (bumpedFee EvmFee, chainSpecificFeeLimit uint32, err error) @@ -68,9 +68,9 @@ func NewEstimator(lggr logger.Logger, ethClient evmclient.Client, cfg Config, ge df := geCfg.EIP1559DynamicFees() // create l1Oracle only if it is supported for the chain - var l1Oracle chainoracles.L1Oracle - if chainoracles.IsL1OracleChain(cfg.ChainType()) { - l1Oracle = chainoracles.NewL1GasPriceOracle(lggr, ethClient, cfg.ChainType()) + var l1Oracle rollups.L1Oracle + if rollups.IsRollupWithL1Support(cfg.ChainType()) { + l1Oracle = rollups.NewL1GasPriceOracle(lggr, ethClient, cfg.ChainType()) } switch s { case "Arbitrum": @@ -152,13 +152,13 @@ func (fee EvmFee) ValidDynamic() bool { type WrappedEvmEstimator struct { EvmEstimator EIP1559Enabled bool - l1Oracle chainoracles.L1Oracle + l1Oracle rollups.L1Oracle utils.StartStopOnce } var _ EvmFeeEstimator = (*WrappedEvmEstimator)(nil) -func NewWrappedEvmEstimator(e EvmEstimator, eip1559Enabled bool, l1Oracle chainoracles.L1Oracle) EvmFeeEstimator { +func NewWrappedEvmEstimator(e EvmEstimator, eip1559Enabled bool, l1Oracle rollups.L1Oracle) EvmFeeEstimator { return &WrappedEvmEstimator{ EvmEstimator: e, EIP1559Enabled: eip1559Enabled, @@ -223,7 +223,7 @@ func (e *WrappedEvmEstimator) HealthReport() map[string]error { return report } -func (e *WrappedEvmEstimator) L1Oracle() chainoracles.L1Oracle { +func (e *WrappedEvmEstimator) L1Oracle() rollups.L1Oracle { return e.l1Oracle } diff --git a/core/chains/evm/gas/models_test.go b/core/chains/evm/gas/models_test.go index a4cac2db0cb..c1dd9e44ffc 100644 --- a/core/chains/evm/gas/models_test.go +++ b/core/chains/evm/gas/models_test.go @@ -12,8 +12,8 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/assets" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas" - oraclesMocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas/chainoracles/mocks" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas/mocks" + rollupMocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas/rollups/mocks" ) func TestWrappedEvmEstimator(t *testing.T) { @@ -49,7 +49,7 @@ func TestWrappedEvmEstimator(t *testing.T) { assert.Nil(t, l1Oracle) // expect l1Oracle - oracle := oraclesMocks.NewL1Oracle(t) + oracle := rollupMocks.NewL1Oracle(t) estimator = gas.NewWrappedEvmEstimator(e, false, oracle) l1Oracle = estimator.L1Oracle() assert.Equal(t, oracle, l1Oracle) @@ -135,7 +135,7 @@ func TestWrappedEvmEstimator(t *testing.T) { t.Run("Name", func(t *testing.T) { evmEstimator := mocks.NewEvmEstimator(t) - oracle := oraclesMocks.NewL1Oracle(t) + oracle := rollupMocks.NewL1Oracle(t) evmEstimator.On("Name").Return(mockEvmEstimatorName, nil).Once() @@ -146,7 +146,7 @@ func TestWrappedEvmEstimator(t *testing.T) { t.Run("Start and stop calls both EVM estimator and L1Oracle", func(t *testing.T) { evmEstimator := mocks.NewEvmEstimator(t) - oracle := oraclesMocks.NewL1Oracle(t) + oracle := rollupMocks.NewL1Oracle(t) evmEstimator.On("Name").Return(mockEvmEstimatorName, nil).Times(4) evmEstimator.On("Start", mock.Anything).Return(nil).Twice() @@ -169,7 +169,7 @@ func TestWrappedEvmEstimator(t *testing.T) { t.Run("Read calls both EVM estimator and L1Oracle", func(t *testing.T) { evmEstimator := mocks.NewEvmEstimator(t) - oracle := oraclesMocks.NewL1Oracle(t) + oracle := rollupMocks.NewL1Oracle(t) evmEstimator.On("Ready").Return(nil).Twice() oracle.On("Ready").Return(nil).Once() @@ -185,7 +185,7 @@ func TestWrappedEvmEstimator(t *testing.T) { t.Run("HealthReport merges report from EVM estimator and L1Oracle", func(t *testing.T) { evmEstimator := mocks.NewEvmEstimator(t) - oracle := oraclesMocks.NewL1Oracle(t) + oracle := rollupMocks.NewL1Oracle(t) evmEstimatorKey := "evm" evmEstimatorError := errors.New("evm error") diff --git a/core/chains/evm/gas/chainoracles/l1_gas_price_oracle.go b/core/chains/evm/gas/rollups/l1_gas_price_oracle.go similarity index 96% rename from core/chains/evm/gas/chainoracles/l1_gas_price_oracle.go rename to core/chains/evm/gas/rollups/l1_gas_price_oracle.go index e031ed115be..13ec5e29dd8 100644 --- a/core/chains/evm/gas/chainoracles/l1_gas_price_oracle.go +++ b/core/chains/evm/gas/rollups/l1_gas_price_oracle.go @@ -1,4 +1,4 @@ -package chainoracles +package rollups import ( "context" @@ -62,7 +62,7 @@ const ( var supportedChainTypes = []config.ChainType{config.ChainArbitrum, config.ChainOptimismBedrock} -func IsL1OracleChain(chainType config.ChainType) bool { +func IsRollupWithL1Support(chainType config.ChainType) bool { return slices.Contains(supportedChainTypes, chainType) } @@ -160,7 +160,7 @@ func (o *l1GasPriceOracle) refresh() (t *time.Timer) { return } -func (o *l1GasPriceOracle) L1GasPrice(_ context.Context) (l1GasPrice *assets.Wei, err error) { +func (o *l1GasPriceOracle) GasPrice(_ context.Context) (l1GasPrice *assets.Wei, err error) { ok := o.IfStarted(func() { o.l1GasPriceMu.RLock() l1GasPrice = o.l1GasPrice diff --git a/core/chains/evm/gas/chainoracles/l1_gas_price_oracle_test.go b/core/chains/evm/gas/rollups/l1_gas_price_oracle_test.go similarity index 84% rename from core/chains/evm/gas/chainoracles/l1_gas_price_oracle_test.go rename to core/chains/evm/gas/rollups/l1_gas_price_oracle_test.go index f3d23de0867..9fd2a66201c 100644 --- a/core/chains/evm/gas/chainoracles/l1_gas_price_oracle_test.go +++ b/core/chains/evm/gas/rollups/l1_gas_price_oracle_test.go @@ -1,4 +1,4 @@ -package chainoracles +package rollups import ( "fmt" @@ -11,8 +11,9 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas/rollups/mocks" + "github.com/smartcontractkit/chainlink/v2/core/assets" - "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas/chainoracles/mocks" "github.com/smartcontractkit/chainlink/v2/core/config" "github.com/smartcontractkit/chainlink/v2/core/internal/testutils" "github.com/smartcontractkit/chainlink/v2/core/logger" @@ -32,11 +33,11 @@ func TestL1GasPriceOracle(t *testing.T) { oracle := NewL1GasPriceOracle(logger.TestLogger(t), ethClient, config.ChainOptimismBedrock) - _, err := oracle.L1GasPrice(testutils.Context(t)) + _, err := oracle.GasPrice(testutils.Context(t)) assert.EqualError(t, err, "L1GasPriceOracle is not started; cannot estimate gas") }) - t.Run("Calling L1GasPrice on started Arbitrum L1Oracle returns Arbitrum l1GasPrice", func(t *testing.T) { + t.Run("Calling GasPrice on started Arbitrum L1Oracle returns Arbitrum l1GasPrice", func(t *testing.T) { l1BaseFee := big.NewInt(100) ethClient := mocks.NewETHClient(t) @@ -52,13 +53,13 @@ func TestL1GasPriceOracle(t *testing.T) { require.NoError(t, oracle.Start(testutils.Context(t))) t.Cleanup(func() { assert.NoError(t, oracle.Close()) }) - gasPrice, err := oracle.L1GasPrice(testutils.Context(t)) + gasPrice, err := oracle.GasPrice(testutils.Context(t)) require.NoError(t, err) assert.Equal(t, assets.NewWei(l1BaseFee), gasPrice) }) - t.Run("Calling L1GasPrice on started OPStack L1Oracle returns OPStack l1GasPrice", func(t *testing.T) { + t.Run("Calling GasPrice on started OPStack L1Oracle returns OPStack l1GasPrice", func(t *testing.T) { l1BaseFee := big.NewInt(200) ethClient := mocks.NewETHClient(t) @@ -74,7 +75,7 @@ func TestL1GasPriceOracle(t *testing.T) { require.NoError(t, oracle.Start(testutils.Context(t))) t.Cleanup(func() { assert.NoError(t, oracle.Close()) }) - gasPrice, err := oracle.L1GasPrice(testutils.Context(t)) + gasPrice, err := oracle.GasPrice(testutils.Context(t)) require.NoError(t, err) assert.Equal(t, assets.NewWei(l1BaseFee), gasPrice) diff --git a/core/chains/evm/gas/chainoracles/mocks/eth_client.go b/core/chains/evm/gas/rollups/mocks/eth_client.go similarity index 99% rename from core/chains/evm/gas/chainoracles/mocks/eth_client.go rename to core/chains/evm/gas/rollups/mocks/eth_client.go index 86228d4d1fe..5389661bc56 100644 --- a/core/chains/evm/gas/chainoracles/mocks/eth_client.go +++ b/core/chains/evm/gas/rollups/mocks/eth_client.go @@ -3,9 +3,8 @@ package mocks import ( - big "math/big" - context "context" + big "math/big" ethereum "github.com/ethereum/go-ethereum" diff --git a/core/chains/evm/gas/chainoracles/mocks/l1_oracle.go b/core/chains/evm/gas/rollups/mocks/l1_oracle.go similarity index 94% rename from core/chains/evm/gas/chainoracles/mocks/l1_oracle.go rename to core/chains/evm/gas/rollups/mocks/l1_oracle.go index cd896370e81..e148c0e9ac4 100644 --- a/core/chains/evm/gas/chainoracles/mocks/l1_oracle.go +++ b/core/chains/evm/gas/rollups/mocks/l1_oracle.go @@ -3,10 +3,10 @@ package mocks import ( - assets "github.com/smartcontractkit/chainlink/v2/core/assets" - context "context" + assets "github.com/smartcontractkit/chainlink/v2/core/assets" + mock "github.com/stretchr/testify/mock" ) @@ -29,24 +29,8 @@ func (_m *L1Oracle) Close() error { return r0 } -// HealthReport provides a mock function with given fields: -func (_m *L1Oracle) HealthReport() map[string]error { - ret := _m.Called() - - var r0 map[string]error - if rf, ok := ret.Get(0).(func() map[string]error); ok { - r0 = rf() - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(map[string]error) - } - } - - return r0 -} - -// L1GasPrice provides a mock function with given fields: ctx -func (_m *L1Oracle) L1GasPrice(ctx context.Context) (*assets.Wei, error) { +// GasPrice provides a mock function with given fields: ctx +func (_m *L1Oracle) GasPrice(ctx context.Context) (*assets.Wei, error) { ret := _m.Called(ctx) var r0 *assets.Wei @@ -71,6 +55,22 @@ func (_m *L1Oracle) L1GasPrice(ctx context.Context) (*assets.Wei, error) { return r0, r1 } +// HealthReport provides a mock function with given fields: +func (_m *L1Oracle) HealthReport() map[string]error { + ret := _m.Called() + + var r0 map[string]error + if rf, ok := ret.Get(0).(func() map[string]error); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(map[string]error) + } + } + + return r0 +} + // Name provides a mock function with given fields: func (_m *L1Oracle) Name() string { ret := _m.Called() diff --git a/core/chains/evm/gas/chainoracles/models.go b/core/chains/evm/gas/rollups/models.go similarity index 85% rename from core/chains/evm/gas/chainoracles/models.go rename to core/chains/evm/gas/rollups/models.go index e95fb036aaf..83ae29f4ea9 100644 --- a/core/chains/evm/gas/chainoracles/models.go +++ b/core/chains/evm/gas/rollups/models.go @@ -1,4 +1,4 @@ -package chainoracles +package rollups import ( "context" @@ -14,5 +14,5 @@ import ( type L1Oracle interface { services.ServiceCtx - L1GasPrice(ctx context.Context) (*assets.Wei, error) + GasPrice(ctx context.Context) (*assets.Wei, error) }