Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EvmFeeEstimator return Optimistic Rollup's L1BaseFee #10557

Merged
merged 15 commits into from
Sep 16, 2023
Merged

EvmFeeEstimator return Optimistic Rollup's L1BaseFee #10557

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
af3e893
Draft impl
matYang Sep 8, 2023
33af306
update configs
matYang Sep 8, 2023
e5f5aff
move l1 knowledge from EVMEstimator to WrappedEVMFeeEstimator
matYang Sep 10, 2023
d6e8274
nit fixes
matYang Sep 10, 2023
d5ab581
change L1BaseFee to L1GasPrice
matYang Sep 10, 2023
56efe1d
update mock
matYang Sep 10, 2023
41f5bd4
use chain type instead of modifying chain config
matYang Sep 13, 2023
055aeb1
add mocks
matYang Sep 13, 2023
20272a4
beef up models tests
matYang Sep 13, 2023
e5d1295
beef up l1 oracle tests
matYang Sep 14, 2023
008a455
small nits
matYang Sep 14, 2023
19282f2
address comments
matYang Sep 14, 2023
7074410
Merge branch 'develop' into evm-estimator-return-gas-price
matYang Sep 14, 2023
06b61ba
fix chaintype panic
matYang Sep 14, 2023
42c5a00
address comments
matYang Sep 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions core/chains/evm/gas/mocks/evm_fee_estimator.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

93 changes: 84 additions & 9 deletions core/chains/evm/gas/models.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,21 @@ import (
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/common/hexutil"
"github.com/pkg/errors"
"golang.org/x/exp/maps"

commonfee "github.com/smartcontractkit/chainlink/v2/common/fee"
feetypes "github.com/smartcontractkit/chainlink/v2/common/fee/types"
commontypes "github.com/smartcontractkit/chainlink/v2/common/types"
"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/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"
"github.com/smartcontractkit/chainlink/v2/core/logger"
"github.com/smartcontractkit/chainlink/v2/core/services"
"github.com/smartcontractkit/chainlink/v2/core/utils"
bigmath "github.com/smartcontractkit/chainlink/v2/core/utils/big_math"
)

Expand All @@ -30,6 +33,8 @@ type EvmFeeEstimator interface {
services.ServiceCtx
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() 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)

Expand Down Expand Up @@ -61,18 +66,24 @@ func NewEstimator(lggr logger.Logger, ethClient evmclient.Client, cfg Config, ge
"priceMin", geCfg.PriceMin(),
)
df := geCfg.EIP1559DynamicFees()

// create l1Oracle only if it is supported for the chain
var l1Oracle rollups.L1Oracle
if rollups.IsRollupWithL1Support(cfg.ChainType()) {
l1Oracle = rollups.NewL1GasPriceOracle(lggr, ethClient, cfg.ChainType())
}
switch s {
case "Arbitrum":
return NewWrappedEvmEstimator(NewArbitrumEstimator(lggr, geCfg, ethClient, ethClient), df)
return NewWrappedEvmEstimator(NewArbitrumEstimator(lggr, geCfg, ethClient, ethClient), df, l1Oracle)
case "BlockHistory":
return NewWrappedEvmEstimator(NewBlockHistoryEstimator(lggr, ethClient, cfg, geCfg, bh, *ethClient.ConfiguredChainID()), df)
return NewWrappedEvmEstimator(NewBlockHistoryEstimator(lggr, ethClient, cfg, geCfg, bh, *ethClient.ConfiguredChainID()), df, l1Oracle)
case "FixedPrice":
return NewWrappedEvmEstimator(NewFixedPriceEstimator(geCfg, bh, lggr), df)
return NewWrappedEvmEstimator(NewFixedPriceEstimator(geCfg, bh, lggr), df, l1Oracle)
case "Optimism2", "L2Suggested":
return NewWrappedEvmEstimator(NewL2SuggestedPriceEstimator(lggr, ethClient), df)
return NewWrappedEvmEstimator(NewL2SuggestedPriceEstimator(lggr, ethClient), df, l1Oracle)
default:
lggr.Warnf("GasEstimator: unrecognised mode '%s', falling back to FixedPriceEstimator", s)
return NewWrappedEvmEstimator(NewFixedPriceEstimator(geCfg, bh, lggr), df)
return NewWrappedEvmEstimator(NewFixedPriceEstimator(geCfg, bh, lggr), df, l1Oracle)
}
}

Expand Down Expand Up @@ -141,18 +152,82 @@ func (fee EvmFee) ValidDynamic() bool {
type WrappedEvmEstimator struct {
EvmEstimator
EIP1559Enabled bool
l1Oracle rollups.L1Oracle
utils.StartStopOnce
}

var _ EvmFeeEstimator = (*WrappedEvmEstimator)(nil)

func NewWrappedEvmEstimator(e EvmEstimator, eip1559Enabled bool) EvmFeeEstimator {
func NewWrappedEvmEstimator(e EvmEstimator, eip1559Enabled bool, l1Oracle rollups.L1Oracle) EvmFeeEstimator {
return &WrappedEvmEstimator{
EvmEstimator: e,
EIP1559Enabled: eip1559Enabled,
l1Oracle: l1Oracle,
}
}

func (e WrappedEvmEstimator) GetFee(ctx context.Context, calldata []byte, feeLimit uint32, maxFeePrice *assets.Wei, opts ...feetypes.Opt) (fee EvmFee, chainSpecificFeeLimit uint32, err error) {
func (e *WrappedEvmEstimator) Name() string {
return fmt.Sprintf("WrappedEvmEstimator(%s)", e.EvmEstimator.Name())
}

func (e *WrappedEvmEstimator) Start(ctx context.Context) error {
return e.StartOnce(e.Name(), func() error {
if err := e.EvmEstimator.Start(ctx); err != nil {
return errors.Wrap(err, "failed to start EVMEstimator")
}
if e.l1Oracle != nil {
if err := e.l1Oracle.Start(ctx); err != nil {
return errors.Wrap(err, "failed to start L1Oracle")
}
}
return nil
})
}
func (e *WrappedEvmEstimator) Close() error {
return e.StopOnce(e.Name(), func() error {
var errEVM, errOracle error

errEVM = errors.Wrap(e.EvmEstimator.Close(), "failed to stop EVMEstimator")
if e.l1Oracle != nil {
errOracle = errors.Wrap(e.l1Oracle.Close(), "failed to stop L1Oracle")
}

if errEVM != nil {
return errEVM
}
return errOracle
})
}

func (e *WrappedEvmEstimator) Ready() error {
var errEVM, errOracle error

errEVM = e.EvmEstimator.Ready()
if e.l1Oracle != nil {
errOracle = e.l1Oracle.Ready()
}

if errEVM != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block of code can happen before even calling l1Oracle.Ready()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it'd only execute either when e.l1Oracle is nil, or e.l1Oracle.Ready() has been called. The drawback is if EvmEstimator and l1Oracle both error, only errEVM will be surfaced. I think that's fine since it doesn't alter the outcome and EvmEstimator is the more critical component.

return errEVM
}
return errOracle
}

func (e *WrappedEvmEstimator) HealthReport() map[string]error {
report := map[string]error{e.Name(): e.StartStopOnce.Healthy()}
maps.Copy(report, e.EvmEstimator.HealthReport())
if e.l1Oracle != nil {
maps.Copy(report, e.l1Oracle.HealthReport())
}

return report
}

func (e *WrappedEvmEstimator) L1Oracle() rollups.L1Oracle {
return e.l1Oracle
}

func (e *WrappedEvmEstimator) GetFee(ctx context.Context, calldata []byte, feeLimit uint32, maxFeePrice *assets.Wei, opts ...feetypes.Opt) (fee EvmFee, chainSpecificFeeLimit uint32, err error) {
// get dynamic fee
if e.EIP1559Enabled {
var dynamicFee DynamicFee
Expand All @@ -167,7 +242,7 @@ func (e WrappedEvmEstimator) GetFee(ctx context.Context, calldata []byte, feeLim
return
}

func (e WrappedEvmEstimator) GetMaxCost(ctx context.Context, amount assets.Eth, calldata []byte, feeLimit uint32, maxFeePrice *assets.Wei, opts ...feetypes.Opt) (*big.Int, error) {
func (e *WrappedEvmEstimator) GetMaxCost(ctx context.Context, amount assets.Eth, calldata []byte, feeLimit uint32, maxFeePrice *assets.Wei, opts ...feetypes.Opt) (*big.Int, error) {
fees, gasLimit, err := e.GetFee(ctx, calldata, feeLimit, maxFeePrice, opts...)
if err != nil {
return nil, err
Expand All @@ -185,7 +260,7 @@ func (e WrappedEvmEstimator) GetMaxCost(ctx context.Context, amount assets.Eth,
return amountWithFees, nil
}

func (e WrappedEvmEstimator) BumpFee(ctx context.Context, originalFee EvmFee, feeLimit uint32, maxFeePrice *assets.Wei, attempts []EvmPriorAttempt) (bumpedFee EvmFee, chainSpecificFeeLimit uint32, err error) {
func (e *WrappedEvmEstimator) BumpFee(ctx context.Context, originalFee EvmFee, feeLimit uint32, maxFeePrice *assets.Wei, attempts []EvmPriorAttempt) (bumpedFee EvmFee, chainSpecificFeeLimit uint32, err error) {
// validate only 1 fee type is present
if (!originalFee.ValidDynamic() && originalFee.Legacy == nil) || (originalFee.ValidDynamic() && originalFee.Legacy != nil) {
err = errors.New("only one dynamic or legacy fee can be defined")
Expand Down
105 changes: 100 additions & 5 deletions core/chains/evm/gas/models_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@ import (
"math/big"
"testing"

"github.com/pkg/errors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"

"github.com/smartcontractkit/chainlink/v2/core/assets"
"github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas"
"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) {
Expand All @@ -36,11 +38,28 @@ func TestWrappedEvmEstimator(t *testing.T) {
e.On("BumpLegacyGas", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
Return(legacyFee, gasLimit, nil).Once()

mockEvmEstimatorName := "MockEstimator"
mockEstimatorName := "WrappedEvmEstimator(MockEstimator)"

// L1Oracle returns the correct L1Oracle interface
t.Run("L1Oracle", func(t *testing.T) {
// expect nil
estimator := gas.NewWrappedEvmEstimator(e, false, nil)
l1Oracle := estimator.L1Oracle()
assert.Nil(t, l1Oracle)

// expect l1Oracle
oracle := rollupMocks.NewL1Oracle(t)
estimator = gas.NewWrappedEvmEstimator(e, false, oracle)
l1Oracle = estimator.L1Oracle()
assert.Equal(t, oracle, l1Oracle)
})

// GetFee returns gas estimation based on configuration value
t.Run("GetFee", func(t *testing.T) {
// expect legacy fee data
dynamicFees := false
estimator := gas.NewWrappedEvmEstimator(e, dynamicFees)
estimator := gas.NewWrappedEvmEstimator(e, dynamicFees, nil)
fee, max, err := estimator.GetFee(ctx, nil, 0, nil)
require.NoError(t, err)
assert.Equal(t, gasLimit, max)
Expand All @@ -50,7 +69,7 @@ func TestWrappedEvmEstimator(t *testing.T) {

// expect dynamic fee data
dynamicFees = true
estimator = gas.NewWrappedEvmEstimator(e, dynamicFees)
estimator = gas.NewWrappedEvmEstimator(e, dynamicFees, nil)
fee, max, err = estimator.GetFee(ctx, nil, 0, nil)
require.NoError(t, err)
assert.Equal(t, gasLimit, max)
Expand All @@ -62,7 +81,7 @@ func TestWrappedEvmEstimator(t *testing.T) {
// BumpFee returns bumped fee type based on original fee calculation
t.Run("BumpFee", func(t *testing.T) {
dynamicFees := false
estimator := gas.NewWrappedEvmEstimator(e, dynamicFees)
estimator := gas.NewWrappedEvmEstimator(e, dynamicFees, nil)

// expect legacy fee data
fee, max, err := estimator.BumpFee(ctx, gas.EvmFee{Legacy: assets.NewWeiI(0)}, 0, nil, nil)
Expand Down Expand Up @@ -99,18 +118,94 @@ func TestWrappedEvmEstimator(t *testing.T) {

// expect legacy fee data
dynamicFees := false
estimator := gas.NewWrappedEvmEstimator(e, dynamicFees)
estimator := gas.NewWrappedEvmEstimator(e, dynamicFees, nil)
total, err := estimator.GetMaxCost(ctx, val, nil, gasLimit, nil)
require.NoError(t, err)
fee := new(big.Int).Mul(legacyFee.ToInt(), big.NewInt(int64(gasLimit)))
assert.Equal(t, new(big.Int).Add(val.ToInt(), fee), total)

// expect dynamic fee data
dynamicFees = true
estimator = gas.NewWrappedEvmEstimator(e, dynamicFees)
estimator = gas.NewWrappedEvmEstimator(e, dynamicFees, nil)
total, err = estimator.GetMaxCost(ctx, val, nil, gasLimit, nil)
require.NoError(t, err)
fee = new(big.Int).Mul(dynamicFee.FeeCap.ToInt(), big.NewInt(int64(gasLimit)))
assert.Equal(t, new(big.Int).Add(val.ToInt(), fee), total)
})

t.Run("Name", func(t *testing.T) {
evmEstimator := mocks.NewEvmEstimator(t)
oracle := rollupMocks.NewL1Oracle(t)

evmEstimator.On("Name").Return(mockEvmEstimatorName, nil).Once()

estimator := gas.NewWrappedEvmEstimator(evmEstimator, false, oracle)
name := estimator.Name()
require.Equal(t, mockEstimatorName, name)
})

t.Run("Start and stop calls both EVM estimator and L1Oracle", func(t *testing.T) {
evmEstimator := mocks.NewEvmEstimator(t)
oracle := rollupMocks.NewL1Oracle(t)

evmEstimator.On("Name").Return(mockEvmEstimatorName, nil).Times(4)
evmEstimator.On("Start", mock.Anything).Return(nil).Twice()
evmEstimator.On("Close").Return(nil).Twice()
oracle.On("Start", mock.Anything).Return(nil).Once()
oracle.On("Close").Return(nil).Once()

estimator := gas.NewWrappedEvmEstimator(evmEstimator, false, nil)
err := estimator.Start(ctx)
require.NoError(t, err)
err = estimator.Close()
require.NoError(t, err)

estimator = gas.NewWrappedEvmEstimator(evmEstimator, false, oracle)
err = estimator.Start(ctx)
require.NoError(t, err)
err = estimator.Close()
require.NoError(t, err)
})

t.Run("Read calls both EVM estimator and L1Oracle", func(t *testing.T) {
evmEstimator := mocks.NewEvmEstimator(t)
oracle := rollupMocks.NewL1Oracle(t)

evmEstimator.On("Ready").Return(nil).Twice()
oracle.On("Ready").Return(nil).Once()

estimator := gas.NewWrappedEvmEstimator(evmEstimator, false, nil)
err := estimator.Ready()
require.NoError(t, err)

estimator = gas.NewWrappedEvmEstimator(evmEstimator, false, oracle)
err = estimator.Ready()
require.NoError(t, err)
})

t.Run("HealthReport merges report from EVM estimator and L1Oracle", func(t *testing.T) {
evmEstimator := mocks.NewEvmEstimator(t)
oracle := rollupMocks.NewL1Oracle(t)

evmEstimatorKey := "evm"
evmEstimatorError := errors.New("evm error")
oracleKey := "oracle"
oracleError := errors.New("oracle error")

evmEstimator.On("Name").Return(mockEvmEstimatorName, nil).Twice()
evmEstimator.On("HealthReport").Return(map[string]error{evmEstimatorKey: evmEstimatorError}).Twice()
oracle.On("HealthReport").Return(map[string]error{oracleKey: oracleError}).Once()

estimator := gas.NewWrappedEvmEstimator(evmEstimator, false, nil)
report := estimator.HealthReport()
require.True(t, errors.Is(report[evmEstimatorKey], evmEstimatorError))
require.Nil(t, report[oracleKey])
require.NotNil(t, report[mockEstimatorName])

estimator = gas.NewWrappedEvmEstimator(evmEstimator, false, oracle)
report = estimator.HealthReport()
require.True(t, errors.Is(report[evmEstimatorKey], evmEstimatorError))
require.True(t, errors.Is(report[oracleKey], oracleError))
require.NotNil(t, report[mockEstimatorName])
})
}
Loading