From 9e733a07fe2082207e6d7884abee26d82d37e808 Mon Sep 17 00:00:00 2001 From: Silas Lenihan <32529249+silaslenihan@users.noreply.github.com> Date: Fri, 14 Jun 2024 17:51:08 -0400 Subject: [PATCH] ChainWriter EVM GetFeeComponents implementation (#13413) * ChainWriter EVM GetFeeComponents implementation * Fixed Write Target CW initialization * added changeset * Added ChainWriter tests * linting fix * Updated comment * Moved MaxGasPrice to CW Config * Changed ExecGasEstimator to CWGasEstimator * removed tests from this PR * Added comments and updated cw struct * renamed gasPriceWei to evmFee * Added more specific error for gasPrice == nil * Update core/services/relay/evm/chain_writer.go Co-authored-by: Jordan Krage * Added tests back in * Reverted to testing EvmFeeEstimator rather than EvmEstimator * reordered imports on CW * Added maxGasPrice default * review requests * Update core/services/relay/evm/write_target.go Co-authored-by: Jordan Krage * Fixed maxGasPrice type * changed maxGasPrice to assets.Wei * mocked gasEstimator --------- Co-authored-by: Jordan Krage --- .changeset/fuzzy-frogs-live.md | 5 + core/services/relay/evm/chain_writer.go | 64 ++++++-- core/services/relay/evm/chain_writer_test.go | 164 +++++++++++++++++++ core/services/relay/evm/types/types.go | 2 + core/services/relay/evm/write_target.go | 4 +- core/services/relay/evm/write_target_test.go | 3 + 6 files changed, 231 insertions(+), 11 deletions(-) create mode 100644 .changeset/fuzzy-frogs-live.md create mode 100644 core/services/relay/evm/chain_writer_test.go diff --git a/.changeset/fuzzy-frogs-live.md b/.changeset/fuzzy-frogs-live.md new file mode 100644 index 00000000000..58a9ffd2288 --- /dev/null +++ b/.changeset/fuzzy-frogs-live.md @@ -0,0 +1,5 @@ +--- +"chainlink": minor +--- + +#added EVM implementation of GetFeeComponents function for ChainWriter diff --git a/core/services/relay/evm/chain_writer.go b/core/services/relay/evm/chain_writer.go index fa62297efce..52bea212b51 100644 --- a/core/services/relay/evm/chain_writer.go +++ b/core/services/relay/evm/chain_writer.go @@ -11,15 +11,17 @@ import ( "github.com/google/uuid" commonservices "github.com/smartcontractkit/chainlink-common/pkg/services" + commontypes "github.com/smartcontractkit/chainlink-common/pkg/types" + "github.com/smartcontractkit/chainlink/v2/common/txmgr" txmgrtypes "github.com/smartcontractkit/chainlink/v2/common/txmgr/types" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/assets" evmclient "github.com/smartcontractkit/chainlink/v2/core/chains/evm/client" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas" evmtxmgr "github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr" "github.com/smartcontractkit/chainlink/v2/core/logger" "github.com/smartcontractkit/chainlink/v2/core/services" "github.com/smartcontractkit/chainlink/v2/core/services/relay/evm/types" - - commontypes "github.com/smartcontractkit/chainlink-common/pkg/types" ) type ChainWriterService interface { @@ -30,11 +32,17 @@ type ChainWriterService interface { // Compile-time assertion that chainWriter implements the ChainWriterService interface. var _ ChainWriterService = (*chainWriter)(nil) -func NewChainWriterService(logger logger.Logger, client evmclient.Client, txm evmtxmgr.TxManager, config types.ChainWriterConfig) (ChainWriterService, error) { +func NewChainWriterService(logger logger.Logger, client evmclient.Client, txm evmtxmgr.TxManager, estimator gas.EvmFeeEstimator, config types.ChainWriterConfig) (ChainWriterService, error) { + if config.MaxGasPrice == nil { + return nil, fmt.Errorf("max gas price is required") + } + w := chainWriter{ - logger: logger, - client: client, - txm: txm, + logger: logger, + client: client, + txm: txm, + ge: estimator, + maxGasPrice: config.MaxGasPrice, sendStrategy: txmgr.NewSendEveryStrategy(), contracts: config.Contracts, @@ -60,9 +68,11 @@ func NewChainWriterService(logger logger.Logger, client evmclient.Client, txm ev type chainWriter struct { commonservices.StateMachine - logger logger.Logger - client evmclient.Client - txm evmtxmgr.TxManager + logger logger.Logger + client evmclient.Client + txm evmtxmgr.TxManager + ge gas.EvmFeeEstimator + maxGasPrice *assets.Wei sendStrategy txmgrtypes.TxStrategy contracts map[string]*types.ContractConfig @@ -155,8 +165,42 @@ func (w *chainWriter) GetTransactionStatus(ctx context.Context, transactionID uu return commontypes.Unknown, fmt.Errorf("not implemented") } +// GetFeeComponents the execution and data availability (L1Oracle) fees for the chain. +// Dynamic fees (introduced in EIP-1559) include a fee cap and a tip cap. If the dyanmic fee is not available, +// (if the chain doesn't support dynamic TXs) the legacy GasPrice is used. func (w *chainWriter) GetFeeComponents(ctx context.Context) (*commontypes.ChainFeeComponents, error) { - return nil, fmt.Errorf("not implemented") + if w.ge == nil { + return nil, fmt.Errorf("gas estimator not available") + } + + fee, _, err := w.ge.GetFee(ctx, nil, 0, w.maxGasPrice) + if err != nil { + return nil, err + } + // Use legacy if no dynamic is available. + gasPrice := fee.Legacy.ToInt() + if fee.DynamicFeeCap != nil { + gasPrice = fee.DynamicFeeCap.ToInt() + } + if gasPrice == nil { + return nil, fmt.Errorf("dynamic fee and legacy gas price missing %+v", fee) + } + l1Oracle := w.ge.L1Oracle() + if l1Oracle == nil { + return &commontypes.ChainFeeComponents{ + ExecutionFee: *gasPrice, + DataAvailabilityFee: *big.NewInt(0), + }, nil + } + l1OracleFee, err := l1Oracle.GasPrice(ctx) + if err != nil { + return nil, err + } + + return &commontypes.ChainFeeComponents{ + ExecutionFee: *gasPrice, + DataAvailabilityFee: *big.NewInt(l1OracleFee.Int64()), + }, nil } func (w *chainWriter) Close() error { diff --git a/core/services/relay/evm/chain_writer_test.go b/core/services/relay/evm/chain_writer_test.go new file mode 100644 index 00000000000..b5aa82ffd82 --- /dev/null +++ b/core/services/relay/evm/chain_writer_test.go @@ -0,0 +1,164 @@ +package evm + +import ( + "fmt" + "math/big" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + "github.com/smartcontractkit/chainlink-common/pkg/types" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/assets" + evmclimocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/client/mocks" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas" + gasmocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas/mocks" + rollupmocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas/rollups/mocks" + txmmocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr/mocks" + "github.com/smartcontractkit/chainlink/v2/core/gethwrappers/keystone/generated/forwarder" + "github.com/smartcontractkit/chainlink/v2/core/internal/testutils" + "github.com/smartcontractkit/chainlink/v2/core/logger" + relayevmtypes "github.com/smartcontractkit/chainlink/v2/core/services/relay/evm/types" +) + +func TestChainWriter(t *testing.T) { + lggr := logger.TestLogger(t) + ctx := testutils.Context(t) + + txm := txmmocks.NewMockEvmTxManager(t) + client := evmclimocks.NewClient(t) + ge := gasmocks.NewEvmFeeEstimator(t) + l1Oracle := rollupmocks.NewL1Oracle(t) + + chainWriterConfig := newBaseChainWriterConfig() + cw, err := NewChainWriterService(lggr, client, txm, ge, chainWriterConfig) + + require.NoError(t, err) + + t.Run("Initialization", func(t *testing.T) { + t.Run("Fails with invalid ABI", func(t *testing.T) { + baseConfig := newBaseChainWriterConfig() + invalidAbiConfig := modifyChainWriterConfig(baseConfig, func(cfg *relayevmtypes.ChainWriterConfig) { + cfg.Contracts["forwarder"].ContractABI = "" + }) + _, err = NewChainWriterService(lggr, client, txm, ge, invalidAbiConfig) + require.Error(t, err) + }) + + t.Run("Fails with invalid method names", func(t *testing.T) { + baseConfig := newBaseChainWriterConfig() + invalidMethodNameConfig := modifyChainWriterConfig(baseConfig, func(cfg *relayevmtypes.ChainWriterConfig) { + cfg.Contracts["forwarder"].Configs["report"].ChainSpecificName = "" + }) + _, err = NewChainWriterService(lggr, client, txm, ge, invalidMethodNameConfig) + require.Error(t, err) + }) + }) + + t.Run("SubmitTransaction", func(t *testing.T) { + // TODO: implement + }) + + t.Run("GetFeeComponents", func(t *testing.T) { + ge.On("GetFee", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(gas.EvmFee{ + Legacy: assets.NewWei(big.NewInt(1000000001)), + DynamicFeeCap: assets.NewWei(big.NewInt(1000000002)), + DynamicTipCap: assets.NewWei(big.NewInt(1000000003)), + }, uint64(0), nil).Twice() + + l1Oracle.On("GasPrice", mock.Anything).Return(assets.NewWei(big.NewInt(1000000004)), nil).Once() + ge.On("L1Oracle", mock.Anything).Return(l1Oracle).Once() + var feeComponents *types.ChainFeeComponents + t.Run("Returns valid FeeComponents", func(t *testing.T) { + feeComponents, err = cw.GetFeeComponents(ctx) + require.NoError(t, err) + assert.Equal(t, big.NewInt(1000000002), &feeComponents.ExecutionFee) + assert.Equal(t, big.NewInt(1000000004), &feeComponents.DataAvailabilityFee) + }) + + ge.On("L1Oracle", mock.Anything).Return(nil).Twice() + + t.Run("Returns valid FeeComponents with no L1Oracle", func(t *testing.T) { + feeComponents, err = cw.GetFeeComponents(ctx) + require.NoError(t, err) + assert.Equal(t, big.NewInt(1000000002), &feeComponents.ExecutionFee) + assert.Equal(t, big.NewInt(0), &feeComponents.DataAvailabilityFee) + }) + + t.Run("Returns Legacy Fee in absence of Dynamic Fee", func(t *testing.T) { + ge.On("GetFee", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(gas.EvmFee{ + Legacy: assets.NewWei(big.NewInt(1000000001)), + DynamicFeeCap: nil, + DynamicTipCap: assets.NewWei(big.NewInt(1000000003)), + }, uint64(0), nil).Once() + feeComponents, err = cw.GetFeeComponents(ctx) + require.NoError(t, err) + assert.Equal(t, big.NewInt(1000000001), &feeComponents.ExecutionFee) + assert.Equal(t, big.NewInt(0), &feeComponents.DataAvailabilityFee) + }) + + t.Run("Fails when neither legacy or dynamic fee is available", func(t *testing.T) { + ge.On("GetFee", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(gas.EvmFee{ + Legacy: nil, + DynamicFeeCap: nil, + DynamicTipCap: nil, + }, uint64(0), nil).Once() + + _, err = cw.GetFeeComponents(ctx) + require.Error(t, err) + }) + + t.Run("Fails when GetFee returns an error", func(t *testing.T) { + expectedErr := fmt.Errorf("GetFee error") + ge.On("GetFee", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(gas.EvmFee{ + Legacy: nil, + DynamicFeeCap: nil, + DynamicTipCap: nil, + }, uint64(0), expectedErr).Once() + _, err = cw.GetFeeComponents(ctx) + require.Equal(t, expectedErr, err) + }) + + t.Run("Fails when L1Oracle returns error", func(t *testing.T) { + ge.On("GetFee", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(gas.EvmFee{ + Legacy: assets.NewWei(big.NewInt(1000000001)), + DynamicFeeCap: assets.NewWei(big.NewInt(1000000002)), + DynamicTipCap: assets.NewWei(big.NewInt(1000000003)), + }, uint64(0), nil).Once() + ge.On("L1Oracle", mock.Anything).Return(l1Oracle).Once() + + expectedErr := fmt.Errorf("l1Oracle error") + l1Oracle.On("GasPrice", mock.Anything).Return(nil, expectedErr).Once() + _, err = cw.GetFeeComponents(ctx) + require.Equal(t, expectedErr, err) + }) + }) +} + +// Helper functions to remove redundant creation of configs +func newBaseChainWriterConfig() relayevmtypes.ChainWriterConfig { + return relayevmtypes.ChainWriterConfig{ + Contracts: map[string]*relayevmtypes.ContractConfig{ + "forwarder": { + // TODO: Use generic ABI / test contract rather than a keystone specific one + ContractABI: forwarder.KeystoneForwarderABI, + Configs: map[string]*relayevmtypes.ChainWriterDefinition{ + "report": { + ChainSpecificName: "report", + Checker: "simulate", + FromAddress: testutils.NewAddress(), + GasLimit: 200_000, + }, + }, + }, + }, + MaxGasPrice: assets.NewWeiI(1000000000000), + } +} + +func modifyChainWriterConfig(baseConfig relayevmtypes.ChainWriterConfig, modifyFn func(*relayevmtypes.ChainWriterConfig)) relayevmtypes.ChainWriterConfig { + modifiedConfig := baseConfig + modifyFn(&modifiedConfig) + return modifiedConfig +} diff --git a/core/services/relay/evm/types/types.go b/core/services/relay/evm/types/types.go index e29b1e6b77f..0d508610389 100644 --- a/core/services/relay/evm/types/types.go +++ b/core/services/relay/evm/types/types.go @@ -18,6 +18,7 @@ import ( "github.com/smartcontractkit/chainlink-common/pkg/codec" "github.com/smartcontractkit/chainlink-common/pkg/services" "github.com/smartcontractkit/chainlink-common/pkg/types" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/assets" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/logpoller" evmtypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/utils/big" @@ -27,6 +28,7 @@ import ( type ChainWriterConfig struct { Contracts map[string]*ContractConfig SendStrategy txmgrtypes.TxStrategy + MaxGasPrice *assets.Wei } type ContractConfig struct { diff --git a/core/services/relay/evm/write_target.go b/core/services/relay/evm/write_target.go index 249c3e257fb..46f4f83b05b 100644 --- a/core/services/relay/evm/write_target.go +++ b/core/services/relay/evm/write_target.go @@ -69,7 +69,9 @@ func NewWriteTarget(ctx context.Context, relayer *Relayer, chain legacyevm.Chain }, }, } - cw, err := NewChainWriterService(lggr.Named("ChainWriter"), chain.Client(), chain.TxManager(), chainWriterConfig) + + chainWriterConfig.MaxGasPrice = chain.Config().EVM().GasEstimator().PriceMax() + cw, err := NewChainWriterService(lggr.Named("ChainWriter"), chain.Client(), chain.TxManager(), chain.GasEstimator(), chainWriterConfig) if err != nil { return nil, err } diff --git a/core/services/relay/evm/write_target_test.go b/core/services/relay/evm/write_target_test.go index 4abbf16cd3b..95d0617db4f 100644 --- a/core/services/relay/evm/write_target_test.go +++ b/core/services/relay/evm/write_target_test.go @@ -13,6 +13,7 @@ import ( "github.com/smartcontractkit/chainlink-common/pkg/capabilities" evmcapabilities "github.com/smartcontractkit/chainlink/v2/core/capabilities" evmclimocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/client/mocks" + gasmocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas/mocks" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr" txmmocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr/mocks" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types" @@ -61,8 +62,10 @@ func TestEvmWrite(t *testing.T) { c.EVM[0].Workflow.ForwarderAddress = &forwarderAddr }) evmCfg := evmtest.NewChainScopedConfig(t, cfg) + ge := gasmocks.NewEvmFeeEstimator(t) chain.On("Config").Return(evmCfg) + chain.On("GasEstimator").Return(ge) db := pgtest.NewSqlxDB(t) keyStore := cltest.NewKeyStore(t, db)