From 10c5ce7c597e9516f23eebf26a5ac142834edeb2 Mon Sep 17 00:00:00 2001 From: FelixFan1992 Date: Wed, 24 Apr 2024 14:48:03 -0400 Subject: [PATCH 1/7] AUTO-10213: pass an gas estimator to registry 2.1 pipeline --- .../ocr2keeper/evmregistry/v21/registry.go | 3 +++ .../evmregistry/v21/registry_check_pipeline.go | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry.go b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry.go index 206932cf543..8aa481c69be 100644 --- a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry.go +++ b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry.go @@ -27,6 +27,7 @@ import ( ocr2keepers "github.com/smartcontractkit/chainlink-common/pkg/types/automation" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/client" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/logpoller" "github.com/smartcontractkit/chainlink/v2/core/chains/legacyevm" "github.com/smartcontractkit/chainlink/v2/core/gethwrappers/generated" @@ -113,6 +114,7 @@ func NewEvmRegistry( bs: blockSub, finalityDepth: finalityDepth, streams: streams.NewStreamsLookup(mercuryConfig, blockSub, client.Client(), registry, lggr), + ge: client.GasEstimator(), } } @@ -194,6 +196,7 @@ type EvmRegistry struct { logEventProvider logprovider.LogEventProvider finalityDepth uint32 streams streams.Lookup + ge gas.EvmFeeEstimator } func (r *EvmRegistry) Name() string { diff --git a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry_check_pipeline.go b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry_check_pipeline.go index 3e935d0adf1..bd49d2fd61e 100644 --- a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry_check_pipeline.go +++ b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry_check_pipeline.go @@ -14,6 +14,7 @@ import ( ocr2keepers "github.com/smartcontractkit/chainlink-common/pkg/types/automation" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/assets" "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/core" "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/encoding" ) @@ -30,6 +31,10 @@ type checkResult struct { err error } +type UpkeepOffchainConfig struct { + MaxGasPrice *big.Int `json:"maxGasPrice" cbor:"maxGasPrice"` +} + func (r *EvmRegistry) CheckUpkeeps(ctx context.Context, keys ...ocr2keepers.UpkeepPayload) ([]ocr2keepers.CheckResult, error) { r.lggr.Debugw("Checking upkeeps", "upkeeps", keys) for i := range keys { @@ -307,6 +312,19 @@ func (r *EvmRegistry) simulatePerformUpkeeps(ctx context.Context, checkResults [ opts := r.buildCallOpts(ctx, block) + fee, _, err := r.ge.GetFee(ctx, []byte{}, 10_000_000, assets.NewWei(big.NewInt(1_000_000_000_000_000))) + if err != nil { + r.lggr.Error("failed to get fee for %s", upkeepId.String()) + checkResults[performToKeyIdx[i]].Eligible = false + checkResults[performToKeyIdx[i]].IneligibilityReason = uint8(encoding.UpkeepFailureReasonSimulationFailed) + continue + } + if fee.ValidDynamic() { + r.lggr.Infof("current gas price EIP-1559 is fee cap %s, tip cap %s", fee.DynamicFeeCap.String(), fee.DynamicTipCap.String()) + } else { + r.lggr.Infof("current gas price legacy is %s", fee.Legacy.String()) + } + // Since checkUpkeep is true, simulate perform upkeep to ensure it doesn't revert payload, err := r.abi.Pack("simulatePerformUpkeep", upkeepId, cr.PerformData) if err != nil { From 1e6d7b241105968891b20f09bd405a1653f1ef99 Mon Sep 17 00:00:00 2001 From: FelixFan1992 Date: Wed, 24 Apr 2024 15:38:51 -0400 Subject: [PATCH 2/7] update tests and add changeset --- .changeset/neat-pianos-argue.md | 5 +++++ .../evmregistry/v21/registry_check_pipeline.go | 2 +- .../v21/registry_check_pipeline_test.go | 14 ++++++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 .changeset/neat-pianos-argue.md diff --git a/.changeset/neat-pianos-argue.md b/.changeset/neat-pianos-argue.md new file mode 100644 index 00000000000..505bce020ad --- /dev/null +++ b/.changeset/neat-pianos-argue.md @@ -0,0 +1,5 @@ +--- +"chainlink": patch +--- + +pass a gas estimator to registry 2.1 pipeline diff --git a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry_check_pipeline.go b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry_check_pipeline.go index bd49d2fd61e..f8ef98ff813 100644 --- a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry_check_pipeline.go +++ b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry_check_pipeline.go @@ -312,7 +312,7 @@ func (r *EvmRegistry) simulatePerformUpkeeps(ctx context.Context, checkResults [ opts := r.buildCallOpts(ctx, block) - fee, _, err := r.ge.GetFee(ctx, []byte{}, 10_000_000, assets.NewWei(big.NewInt(1_000_000_000_000_000))) + fee, _, err := r.ge.GetFee(ctx, []byte{}, uint64(10_000_000), assets.NewWei(big.NewInt(1_000_000_000_000_000))) if err != nil { r.lggr.Error("failed to get fee for %s", upkeepId.String()) checkResults[performToKeyIdx[i]].Eligible = false diff --git a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry_check_pipeline_test.go b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry_check_pipeline_test.go index 330da44b71b..9dac3b1de1a 100644 --- a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry_check_pipeline_test.go +++ b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry_check_pipeline_test.go @@ -22,7 +22,10 @@ import ( ocr2keepers "github.com/smartcontractkit/chainlink-common/pkg/types/automation" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/assets" evmClientMocks "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" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/logpoller" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types" ac "github.com/smartcontractkit/chainlink/v2/core/gethwrappers/generated/i_automation_v21_plus_common" @@ -670,6 +673,16 @@ func setupEVMRegistry(t *testing.T) *EvmRegistry { mockReg := mocks.NewRegistry(t) mockHttpClient := mocks.NewHttpClient(t) client := evmClientMocks.NewClient(t) + ge := gasMocks.NewEvmFeeEstimator(t) + ge.On("GetFee", mock.Anything, mock.AnythingOfType("[]uint8"), mock.AnythingOfType("uint64"), mock.Anything).Return( + gas.EvmFee{ + Legacy: nil, + DynamicFeeCap: assets.NewWei(big.NewInt(10_000_000_000)), + DynamicTipCap: assets.NewWei(big.NewInt(1_000_000_000)), + }, + uint64(5_000_000), + nil, + ) r := &EvmRegistry{ lggr: lggr, @@ -694,6 +707,7 @@ func setupEVMRegistry(t *testing.T) *EvmRegistry { AllowListCache: cache.New(defaultAllowListExpiration, cleanupInterval), }, hc: mockHttpClient, + ge: ge, } return r } From 7ac169dd27566ce8315f373c450e9852bafb25f4 Mon Sep 17 00:00:00 2001 From: FelixFan1992 Date: Wed, 24 Apr 2024 15:42:18 -0400 Subject: [PATCH 3/7] update changeset --- .changeset/neat-pianos-argue.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.changeset/neat-pianos-argue.md b/.changeset/neat-pianos-argue.md index 505bce020ad..f65c19584db 100644 --- a/.changeset/neat-pianos-argue.md +++ b/.changeset/neat-pianos-argue.md @@ -2,4 +2,5 @@ "chainlink": patch --- +#added pass a gas estimator to registry 2.1 pipeline From 54efcf40d448d7ef7162c6790aa51024904e4768 Mon Sep 17 00:00:00 2001 From: FelixFan1992 Date: Fri, 26 Apr 2024 11:13:00 -0400 Subject: [PATCH 4/7] AUTO-10214: compare max gas price with current gas price in simulation process (#12955) * AUTO-10214: compare max gas price with current gas price in simulation process * refactor and add tests * linting (#12960) * linting * 2 * fix linting * create opts with latest block --- .changeset/funny-tips-promise.md | 6 + .../evmregistry/v21/encoding/interface.go | 14 +- .../evmregistry/v21/gasprice/gasprice.go | 69 ++++++++++ .../evmregistry/v21/gasprice/gasprice_test.go | 126 ++++++++++++++++++ .../ocr2keeper/evmregistry/v21/registry.go | 7 + .../v21/registry_check_pipeline.go | 22 +-- .../v21/registry_check_pipeline_test.go | 19 ++- 7 files changed, 236 insertions(+), 27 deletions(-) create mode 100644 .changeset/funny-tips-promise.md create mode 100644 core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/gasprice/gasprice.go create mode 100644 core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/gasprice/gasprice_test.go diff --git a/.changeset/funny-tips-promise.md b/.changeset/funny-tips-promise.md new file mode 100644 index 00000000000..16fd0a9fc33 --- /dev/null +++ b/.changeset/funny-tips-promise.md @@ -0,0 +1,6 @@ +--- +"chainlink": patch +--- + +#added +compare user-defined max gas price with current gas price in automation simulation pipeline diff --git a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/encoding/interface.go b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/encoding/interface.go index e942078fe54..9455cf32c57 100644 --- a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/encoding/interface.go +++ b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/encoding/interface.go @@ -26,11 +26,15 @@ const ( UpkeepFailureReasonRegistryPaused UpkeepFailureReason = 9 // leaving a gap here for more onchain failure reasons in the future // upkeep failure offchain reasons - UpkeepFailureReasonMercuryAccessNotAllowed UpkeepFailureReason = 32 - UpkeepFailureReasonTxHashNoLongerExists UpkeepFailureReason = 33 - UpkeepFailureReasonInvalidRevertDataInput UpkeepFailureReason = 34 - UpkeepFailureReasonSimulationFailed UpkeepFailureReason = 35 - UpkeepFailureReasonTxHashReorged UpkeepFailureReason = 36 + UpkeepFailureReasonMercuryAccessNotAllowed UpkeepFailureReason = 32 + UpkeepFailureReasonTxHashNoLongerExists UpkeepFailureReason = 33 + UpkeepFailureReasonInvalidRevertDataInput UpkeepFailureReason = 34 + UpkeepFailureReasonSimulationFailed UpkeepFailureReason = 35 + UpkeepFailureReasonTxHashReorged UpkeepFailureReason = 36 + UpkeepFailureReasonFailToRetrieveOffchainConfig UpkeepFailureReason = 37 + UpkeepFailureReasonFailToParseOffchainConfig UpkeepFailureReason = 38 + UpkeepFailureReasonFailToRetrieveGasPrice UpkeepFailureReason = 39 + UpkeepFailureReasonGasPriceTooHigh UpkeepFailureReason = 40 // pipeline execution error NoPipelineError PipelineExecutionState = 0 diff --git a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/gasprice/gasprice.go b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/gasprice/gasprice.go new file mode 100644 index 00000000000..4565fb677c6 --- /dev/null +++ b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/gasprice/gasprice.go @@ -0,0 +1,69 @@ +package gasprice + +import ( + "context" + "math/big" + + "github.com/smartcontractkit/chainlink/v2/core/cbor" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/assets" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas" + "github.com/smartcontractkit/chainlink/v2/core/logger" + "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/encoding" +) + +const ( + // feeLimit is a placeholder when getting current price from gas estimator. it does not impact gas price calculation + feeLimit = uint64(1_000_000) + // maxFeePrice is a placeholder when getting current price from gas estimator. it caps the returned gas price from + // the estimator. it's set to a very high value because the gas price will be compared with user-defined gas price + // later. + maxFeePrice = 1_000_000_000_000_000 +) + +type UpkeepOffchainConfig struct { + MaxGasPrice *big.Int `json:"maxGasPrice" cbor:"maxGasPrice"` +} + +func CheckGasPrice(ctx context.Context, upkeepId *big.Int, oc []byte, ge gas.EvmFeeEstimator, lggr logger.Logger) encoding.UpkeepFailureReason { + if len(oc) == 0 { + return encoding.UpkeepFailureReasonNone + } + + var offchainConfig UpkeepOffchainConfig + if err := cbor.ParseDietCBORToStruct(oc, &offchainConfig); err != nil { + lggr.Errorw("failed to parse upkeep offchain config", "upkeepId", upkeepId.String(), "err", err) + return encoding.UpkeepFailureReasonFailToParseOffchainConfig + } + if offchainConfig.MaxGasPrice == nil { + lggr.Infow("maxGasPrice is not configured in upkeep offchain config", "upkeepId", upkeepId.String()) + return encoding.UpkeepFailureReasonNone + } + lggr.Infof("successfully decode offchain config for %s", upkeepId.String()) + lggr.Infof("max gas price for %s is %s", upkeepId.String(), offchainConfig.MaxGasPrice.String()) + + fee, _, err := ge.GetFee(ctx, []byte{}, feeLimit, assets.NewWei(big.NewInt(maxFeePrice))) + if err != nil { + lggr.Errorw("failed to get fee", "upkeepId", upkeepId.String(), "err", err) + return encoding.UpkeepFailureReasonFailToRetrieveGasPrice + } + + if fee.ValidDynamic() { + lggr.Infof("current gas price EIP-1559 is fee cap %s, tip cap %s", fee.DynamicFeeCap.String(), fee.DynamicTipCap.String()) + if fee.DynamicFeeCap.Cmp(assets.NewWei(offchainConfig.MaxGasPrice)) > 0 { + // current gas price is higher than max gas price + lggr.Warnf("max gas price %s for %s is LOWER than current gas price %s", offchainConfig.MaxGasPrice.String(), upkeepId.String(), fee.DynamicFeeCap.String()) + return encoding.UpkeepFailureReasonGasPriceTooHigh + } + lggr.Infof("max gas price %s for %s is HIGHER than current gas price %s", offchainConfig.MaxGasPrice.String(), upkeepId.String(), fee.DynamicFeeCap.String()) + } else { + lggr.Infof("current gas price legacy is %s", fee.Legacy.String()) + if fee.Legacy.Cmp(assets.NewWei(offchainConfig.MaxGasPrice)) > 0 { + // current gas price is higher than max gas price + lggr.Infof("max gas price %s for %s is LOWER than current gas price %s", offchainConfig.MaxGasPrice.String(), upkeepId.String(), fee.Legacy.String()) + return encoding.UpkeepFailureReasonGasPriceTooHigh + } + lggr.Infof("max gas price %s for %s is HIGHER than current gas price %s", offchainConfig.MaxGasPrice.String(), upkeepId.String(), fee.Legacy.String()) + } + + return encoding.UpkeepFailureReasonNone +} diff --git a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/gasprice/gasprice_test.go b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/gasprice/gasprice_test.go new file mode 100644 index 00000000000..24d48609082 --- /dev/null +++ b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/gasprice/gasprice_test.go @@ -0,0 +1,126 @@ +package gasprice + +import ( + "math/big" + "testing" + + "github.com/fxamacker/cbor/v2" + "github.com/pkg/errors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/assets" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas" + gasMocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas/mocks" + "github.com/smartcontractkit/chainlink/v2/core/internal/testutils" + "github.com/smartcontractkit/chainlink/v2/core/logger" + "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/encoding" +) + +type WrongOffchainConfig struct { + MaxGasPrice1 []int `json:"maxGasPrice1" cbor:"maxGasPrice1"` +} + +func TestGasPrice_Check(t *testing.T) { + lggr := logger.TestLogger(t) + uid, _ := new(big.Int).SetString("1843548457736589226156809205796175506139185429616502850435279853710366065936", 10) + + tests := []struct { + Name string + MaxGasPrice *big.Int + CurrentLegacyGasPrice *big.Int + CurrentDynamicGasPrice *big.Int + ExpectedResult encoding.UpkeepFailureReason + NotConfigured bool + ParsingFailed bool + }{ + { + Name: "no offchain config", + ExpectedResult: encoding.UpkeepFailureReasonNone, + }, + { + Name: "maxGasPrice not configured in offchain config", + NotConfigured: true, + ExpectedResult: encoding.UpkeepFailureReasonNone, + }, + { + Name: "fail to parse offchain config", + ParsingFailed: true, + MaxGasPrice: big.NewInt(10_000_000_000), + ExpectedResult: encoding.UpkeepFailureReasonFailToParseOffchainConfig, + }, + { + Name: "fail to retrieve current gas price", + MaxGasPrice: big.NewInt(8_000_000_000), + ExpectedResult: encoding.UpkeepFailureReasonFailToRetrieveGasPrice, + }, + { + Name: "current gas price is too high - legacy", + MaxGasPrice: big.NewInt(10_000_000_000), + CurrentLegacyGasPrice: big.NewInt(18_000_000_000), + ExpectedResult: encoding.UpkeepFailureReasonGasPriceTooHigh, + }, + { + Name: "current gas price is too high - dynamic", + MaxGasPrice: big.NewInt(10_000_000_000), + CurrentDynamicGasPrice: big.NewInt(15_000_000_000), + ExpectedResult: encoding.UpkeepFailureReasonGasPriceTooHigh, + }, + { + Name: "current gas price is less than user's max gas price - legacy", + MaxGasPrice: big.NewInt(8_000_000_000), + CurrentLegacyGasPrice: big.NewInt(5_000_000_000), + ExpectedResult: encoding.UpkeepFailureReasonNone, + }, + { + Name: "current gas price is less than user's max gas price - dynamic", + MaxGasPrice: big.NewInt(10_000_000_000), + CurrentDynamicGasPrice: big.NewInt(8_000_000_000), + ExpectedResult: encoding.UpkeepFailureReasonNone, + }, + } + for _, test := range tests { + t.Run(test.Name, func(t *testing.T) { + ctx := testutils.Context(t) + ge := gasMocks.NewEvmFeeEstimator(t) + if test.ExpectedResult == encoding.UpkeepFailureReasonFailToRetrieveGasPrice { + ge.On("GetFee", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return( + gas.EvmFee{}, + feeLimit, + errors.New("failed to retrieve gas price"), + ) + } else if test.CurrentLegacyGasPrice != nil { + ge.On("GetFee", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return( + gas.EvmFee{ + Legacy: assets.NewWei(test.CurrentLegacyGasPrice), + }, + feeLimit, + nil, + ) + } else if test.CurrentDynamicGasPrice != nil { + ge.On("GetFee", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return( + gas.EvmFee{ + DynamicFeeCap: assets.NewWei(test.CurrentDynamicGasPrice), + DynamicTipCap: assets.NewWei(big.NewInt(1_000_000_000)), + }, + feeLimit, + nil, + ) + } + + var oc []byte + if test.ParsingFailed { + oc, _ = cbor.Marshal(WrongOffchainConfig{MaxGasPrice1: []int{1, 2, 3}}) + if len(oc) > 0 { + oc[len(oc)-1] = 0x99 + } + } else if test.NotConfigured { + oc = []byte{1, 2, 3, 4} // parsing this will set maxGasPrice field to nil + } else if test.MaxGasPrice != nil { + oc, _ = cbor.Marshal(UpkeepOffchainConfig{MaxGasPrice: test.MaxGasPrice}) + } + fr := CheckGasPrice(ctx, uid, oc, ge, lggr) + assert.Equal(t, test.ExpectedResult, fr) + }) + } +} diff --git a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry.go b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry.go index 8aa481c69be..a4b1cc56915 100644 --- a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry.go +++ b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry.go @@ -630,3 +630,10 @@ func (r *EvmRegistry) fetchTriggerConfig(id *big.Int) ([]byte, error) { } return cfg, nil } + +// fetchUpkeepOffchainConfig fetches upkeep offchain config in raw bytes for an upkeep. +func (r *EvmRegistry) fetchUpkeepOffchainConfig(id *big.Int) ([]byte, error) { + opts := r.buildCallOpts(r.ctx, nil) + ui, err := r.registry.GetUpkeep(opts, id) + return ui.OffchainConfig, err +} diff --git a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry_check_pipeline.go b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry_check_pipeline.go index f8ef98ff813..f737a462fe5 100644 --- a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry_check_pipeline.go +++ b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry_check_pipeline.go @@ -14,9 +14,9 @@ import ( ocr2keepers "github.com/smartcontractkit/chainlink-common/pkg/types/automation" - "github.com/smartcontractkit/chainlink/v2/core/chains/evm/assets" "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/core" "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/encoding" + "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/gasprice" ) const ( @@ -310,19 +310,18 @@ func (r *EvmRegistry) simulatePerformUpkeeps(ctx context.Context, checkResults [ block, _, upkeepId := r.getBlockAndUpkeepId(cr.UpkeepID, cr.Trigger) - opts := r.buildCallOpts(ctx, block) - - fee, _, err := r.ge.GetFee(ctx, []byte{}, uint64(10_000_000), assets.NewWei(big.NewInt(1_000_000_000_000_000))) + oc, err := r.fetchUpkeepOffchainConfig(upkeepId) if err != nil { - r.lggr.Error("failed to get fee for %s", upkeepId.String()) - checkResults[performToKeyIdx[i]].Eligible = false - checkResults[performToKeyIdx[i]].IneligibilityReason = uint8(encoding.UpkeepFailureReasonSimulationFailed) + r.lggr.Warnw("failed get offchain config", "err", err, "upkeepId", upkeepId, "block", block) + checkResults[i].Eligible = false + checkResults[i].PipelineExecutionState = uint8(encoding.UpkeepFailureReasonFailToRetrieveOffchainConfig) continue } - if fee.ValidDynamic() { - r.lggr.Infof("current gas price EIP-1559 is fee cap %s, tip cap %s", fee.DynamicFeeCap.String(), fee.DynamicTipCap.String()) - } else { - r.lggr.Infof("current gas price legacy is %s", fee.Legacy.String()) + fr := gasprice.CheckGasPrice(ctx, upkeepId, oc, r.ge, r.lggr) + if fr != encoding.UpkeepFailureReasonNone { + checkResults[i].Eligible = false + checkResults[i].PipelineExecutionState = uint8(fr) + continue } // Since checkUpkeep is true, simulate perform upkeep to ensure it doesn't revert @@ -335,6 +334,7 @@ func (r *EvmRegistry) simulatePerformUpkeeps(ctx context.Context, checkResults [ continue } + opts := r.buildCallOpts(ctx, block) var result string performReqs = append(performReqs, rpc.BatchElem{ Method: "eth_call", diff --git a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry_check_pipeline_test.go b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry_check_pipeline_test.go index 9dac3b1de1a..e74ad4821a6 100644 --- a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry_check_pipeline_test.go +++ b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry_check_pipeline_test.go @@ -22,9 +22,7 @@ import ( ocr2keepers "github.com/smartcontractkit/chainlink-common/pkg/types/automation" - "github.com/smartcontractkit/chainlink/v2/core/chains/evm/assets" evmClientMocks "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" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/logpoller" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types" @@ -654,6 +652,13 @@ func TestRegistry_SimulatePerformUpkeeps(t *testing.T) { }).Once() e.client = client + mockReg := mocks.NewRegistry(t) + mockReg.On("GetUpkeep", mock.Anything, mock.Anything).Return( + encoding.UpkeepInfo{OffchainConfig: make([]byte, 0)}, + nil, + ).Times(2) + e.registry = mockReg + results, err := e.simulatePerformUpkeeps(testutils.Context(t), tc.inputs) assert.Equal(t, tc.results, results) assert.Equal(t, tc.err, err) @@ -674,15 +679,6 @@ func setupEVMRegistry(t *testing.T) *EvmRegistry { mockHttpClient := mocks.NewHttpClient(t) client := evmClientMocks.NewClient(t) ge := gasMocks.NewEvmFeeEstimator(t) - ge.On("GetFee", mock.Anything, mock.AnythingOfType("[]uint8"), mock.AnythingOfType("uint64"), mock.Anything).Return( - gas.EvmFee{ - Legacy: nil, - DynamicFeeCap: assets.NewWei(big.NewInt(10_000_000_000)), - DynamicTipCap: assets.NewWei(big.NewInt(1_000_000_000)), - }, - uint64(5_000_000), - nil, - ) r := &EvmRegistry{ lggr: lggr, @@ -707,6 +703,7 @@ func setupEVMRegistry(t *testing.T) *EvmRegistry { AllowListCache: cache.New(defaultAllowListExpiration, cleanupInterval), }, hc: mockHttpClient, + bs: &BlockSubscriber{latestBlock: atomic.Pointer[ocr2keepers.BlockKey]{}}, ge: ge, } return r From 84ae5bf93ee93dfcdfcccdc1e12828d2d3253468 Mon Sep 17 00:00:00 2001 From: FelixFan1992 Date: Thu, 2 May 2024 09:18:46 -0400 Subject: [PATCH 5/7] AUTO-10236: add integration tests for max gas price check (#12974) * AUTO-10214: compare max gas price with current gas price in simulation process * refactor and add tests * linting (#12960) * linting * 2 * fix linting * AUTO-10236 * fix go mod * update test json * improve max gas price integration tests * AUTO-10214: compare max gas price with current gas price in simulation process * refactor and add tests * linting (#12960) * linting * 2 * fix linting * create opts with latest block * add some logs * fix bug and update logs * update * update --- .changeset/witty-weeks-kneel.md | 5 + .../evmregistry/v21/encoding/interface.go | 15 +-- .../evmregistry/v21/gasprice/gasprice.go | 20 +-- .../evmregistry/v21/gasprice/gasprice_test.go | 8 +- .../ocr2keeper/evmregistry/v21/registry.go | 5 +- .../v21/registry_check_pipeline.go | 16 +-- .../contracts/ethereum_keeper_contracts.go | 41 ++++++ integration-tests/go.mod | 2 +- integration-tests/smoke/automation_test.go | 120 +++++++++++++++++- .../smoke/automation_test.go_test_list.json | 4 + 10 files changed, 200 insertions(+), 36 deletions(-) create mode 100644 .changeset/witty-weeks-kneel.md diff --git a/.changeset/witty-weeks-kneel.md b/.changeset/witty-weeks-kneel.md new file mode 100644 index 00000000000..d638d037081 --- /dev/null +++ b/.changeset/witty-weeks-kneel.md @@ -0,0 +1,5 @@ +--- +"chainlink": patch +--- + +#added an integration test for max gas price check diff --git a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/encoding/interface.go b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/encoding/interface.go index 9455cf32c57..39d738fa7c6 100644 --- a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/encoding/interface.go +++ b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/encoding/interface.go @@ -26,15 +26,12 @@ const ( UpkeepFailureReasonRegistryPaused UpkeepFailureReason = 9 // leaving a gap here for more onchain failure reasons in the future // upkeep failure offchain reasons - UpkeepFailureReasonMercuryAccessNotAllowed UpkeepFailureReason = 32 - UpkeepFailureReasonTxHashNoLongerExists UpkeepFailureReason = 33 - UpkeepFailureReasonInvalidRevertDataInput UpkeepFailureReason = 34 - UpkeepFailureReasonSimulationFailed UpkeepFailureReason = 35 - UpkeepFailureReasonTxHashReorged UpkeepFailureReason = 36 - UpkeepFailureReasonFailToRetrieveOffchainConfig UpkeepFailureReason = 37 - UpkeepFailureReasonFailToParseOffchainConfig UpkeepFailureReason = 38 - UpkeepFailureReasonFailToRetrieveGasPrice UpkeepFailureReason = 39 - UpkeepFailureReasonGasPriceTooHigh UpkeepFailureReason = 40 + UpkeepFailureReasonMercuryAccessNotAllowed UpkeepFailureReason = 32 + UpkeepFailureReasonTxHashNoLongerExists UpkeepFailureReason = 33 + UpkeepFailureReasonInvalidRevertDataInput UpkeepFailureReason = 34 + UpkeepFailureReasonSimulationFailed UpkeepFailureReason = 35 + UpkeepFailureReasonTxHashReorged UpkeepFailureReason = 36 + UpkeepFailureReasonGasPriceTooHigh UpkeepFailureReason = 37 // pipeline execution error NoPipelineError PipelineExecutionState = 0 diff --git a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/gasprice/gasprice.go b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/gasprice/gasprice.go index 4565fb677c6..82514477312 100644 --- a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/gasprice/gasprice.go +++ b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/gasprice/gasprice.go @@ -24,6 +24,8 @@ type UpkeepOffchainConfig struct { MaxGasPrice *big.Int `json:"maxGasPrice" cbor:"maxGasPrice"` } +// CheckGasPrice retrieves the current gas price and compare against the max gas price configured in upkeep's offchain config +// any errors in offchain config decoding will result in max gas price check disabled func CheckGasPrice(ctx context.Context, upkeepId *big.Int, oc []byte, ge gas.EvmFeeEstimator, lggr logger.Logger) encoding.UpkeepFailureReason { if len(oc) == 0 { return encoding.UpkeepFailureReasonNone @@ -31,11 +33,11 @@ func CheckGasPrice(ctx context.Context, upkeepId *big.Int, oc []byte, ge gas.Evm var offchainConfig UpkeepOffchainConfig if err := cbor.ParseDietCBORToStruct(oc, &offchainConfig); err != nil { - lggr.Errorw("failed to parse upkeep offchain config", "upkeepId", upkeepId.String(), "err", err) - return encoding.UpkeepFailureReasonFailToParseOffchainConfig + lggr.Errorw("failed to parse upkeep offchain config, gas price check is disabled", "upkeepId", upkeepId.String(), "err", err) + return encoding.UpkeepFailureReasonNone } if offchainConfig.MaxGasPrice == nil { - lggr.Infow("maxGasPrice is not configured in upkeep offchain config", "upkeepId", upkeepId.String()) + lggr.Infow("maxGasPrice is not configured in upkeep offchain config, gas price check is disabled", "upkeepId", upkeepId.String()) return encoding.UpkeepFailureReasonNone } lggr.Infof("successfully decode offchain config for %s", upkeepId.String()) @@ -43,26 +45,26 @@ func CheckGasPrice(ctx context.Context, upkeepId *big.Int, oc []byte, ge gas.Evm fee, _, err := ge.GetFee(ctx, []byte{}, feeLimit, assets.NewWei(big.NewInt(maxFeePrice))) if err != nil { - lggr.Errorw("failed to get fee", "upkeepId", upkeepId.String(), "err", err) - return encoding.UpkeepFailureReasonFailToRetrieveGasPrice + lggr.Errorw("failed to get fee, gas price check is disabled", "upkeepId", upkeepId.String(), "err", err) + return encoding.UpkeepFailureReasonNone } if fee.ValidDynamic() { lggr.Infof("current gas price EIP-1559 is fee cap %s, tip cap %s", fee.DynamicFeeCap.String(), fee.DynamicTipCap.String()) if fee.DynamicFeeCap.Cmp(assets.NewWei(offchainConfig.MaxGasPrice)) > 0 { // current gas price is higher than max gas price - lggr.Warnf("max gas price %s for %s is LOWER than current gas price %s", offchainConfig.MaxGasPrice.String(), upkeepId.String(), fee.DynamicFeeCap.String()) + lggr.Warnf("maxGasPrice %s for %s is LOWER than current gas price %d", offchainConfig.MaxGasPrice.String(), upkeepId.String(), fee.DynamicFeeCap.Int64()) return encoding.UpkeepFailureReasonGasPriceTooHigh } - lggr.Infof("max gas price %s for %s is HIGHER than current gas price %s", offchainConfig.MaxGasPrice.String(), upkeepId.String(), fee.DynamicFeeCap.String()) + lggr.Infof("maxGasPrice %s for %s is HIGHER than current gas price %d", offchainConfig.MaxGasPrice.String(), upkeepId.String(), fee.DynamicFeeCap.Int64()) } else { lggr.Infof("current gas price legacy is %s", fee.Legacy.String()) if fee.Legacy.Cmp(assets.NewWei(offchainConfig.MaxGasPrice)) > 0 { // current gas price is higher than max gas price - lggr.Infof("max gas price %s for %s is LOWER than current gas price %s", offchainConfig.MaxGasPrice.String(), upkeepId.String(), fee.Legacy.String()) + lggr.Infof("maxGasPrice %s for %s is LOWER than current gas price %d", offchainConfig.MaxGasPrice.String(), upkeepId.String(), fee.Legacy.Int64()) return encoding.UpkeepFailureReasonGasPriceTooHigh } - lggr.Infof("max gas price %s for %s is HIGHER than current gas price %s", offchainConfig.MaxGasPrice.String(), upkeepId.String(), fee.Legacy.String()) + lggr.Infof("maxGasPrice %s for %s is HIGHER than current gas price %d", offchainConfig.MaxGasPrice.String(), upkeepId.String(), fee.Legacy.Int64()) } return encoding.UpkeepFailureReasonNone diff --git a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/gasprice/gasprice_test.go b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/gasprice/gasprice_test.go index 24d48609082..9b5640051df 100644 --- a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/gasprice/gasprice_test.go +++ b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/gasprice/gasprice_test.go @@ -31,6 +31,7 @@ func TestGasPrice_Check(t *testing.T) { CurrentLegacyGasPrice *big.Int CurrentDynamicGasPrice *big.Int ExpectedResult encoding.UpkeepFailureReason + FailedToGetFee bool NotConfigured bool ParsingFailed bool }{ @@ -47,12 +48,13 @@ func TestGasPrice_Check(t *testing.T) { Name: "fail to parse offchain config", ParsingFailed: true, MaxGasPrice: big.NewInt(10_000_000_000), - ExpectedResult: encoding.UpkeepFailureReasonFailToParseOffchainConfig, + ExpectedResult: encoding.UpkeepFailureReasonNone, }, { Name: "fail to retrieve current gas price", MaxGasPrice: big.NewInt(8_000_000_000), - ExpectedResult: encoding.UpkeepFailureReasonFailToRetrieveGasPrice, + FailedToGetFee: true, + ExpectedResult: encoding.UpkeepFailureReasonNone, }, { Name: "current gas price is too high - legacy", @@ -83,7 +85,7 @@ func TestGasPrice_Check(t *testing.T) { t.Run(test.Name, func(t *testing.T) { ctx := testutils.Context(t) ge := gasMocks.NewEvmFeeEstimator(t) - if test.ExpectedResult == encoding.UpkeepFailureReasonFailToRetrieveGasPrice { + if test.FailedToGetFee { ge.On("GetFee", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return( gas.EvmFee{}, feeLimit, diff --git a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry.go b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry.go index a4b1cc56915..5a6466a8b15 100644 --- a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry.go +++ b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry.go @@ -635,5 +635,8 @@ func (r *EvmRegistry) fetchTriggerConfig(id *big.Int) ([]byte, error) { func (r *EvmRegistry) fetchUpkeepOffchainConfig(id *big.Int) ([]byte, error) { opts := r.buildCallOpts(r.ctx, nil) ui, err := r.registry.GetUpkeep(opts, id) - return ui.OffchainConfig, err + if err != nil { + return []byte{}, err + } + return ui.OffchainConfig, nil } diff --git a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry_check_pipeline.go b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry_check_pipeline.go index f737a462fe5..e341730c794 100644 --- a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry_check_pipeline.go +++ b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry_check_pipeline.go @@ -31,10 +31,6 @@ type checkResult struct { err error } -type UpkeepOffchainConfig struct { - MaxGasPrice *big.Int `json:"maxGasPrice" cbor:"maxGasPrice"` -} - func (r *EvmRegistry) CheckUpkeeps(ctx context.Context, keys ...ocr2keepers.UpkeepPayload) ([]ocr2keepers.CheckResult, error) { r.lggr.Debugw("Checking upkeeps", "upkeeps", keys) for i := range keys { @@ -312,15 +308,15 @@ func (r *EvmRegistry) simulatePerformUpkeeps(ctx context.Context, checkResults [ oc, err := r.fetchUpkeepOffchainConfig(upkeepId) if err != nil { - r.lggr.Warnw("failed get offchain config", "err", err, "upkeepId", upkeepId, "block", block) - checkResults[i].Eligible = false - checkResults[i].PipelineExecutionState = uint8(encoding.UpkeepFailureReasonFailToRetrieveOffchainConfig) - continue + // this is mostly caused by RPC flakiness + r.lggr.Errorw("failed get offchain config, gas price check will be disabled", "err", err, "upkeepId", upkeepId, "block", block) } fr := gasprice.CheckGasPrice(ctx, upkeepId, oc, r.ge, r.lggr) - if fr != encoding.UpkeepFailureReasonNone { + if uint8(fr) == uint8(encoding.UpkeepFailureReasonGasPriceTooHigh) { + r.lggr.Infof("upkeep %s upkeep failure reason is %d", upkeepId, fr) checkResults[i].Eligible = false - checkResults[i].PipelineExecutionState = uint8(fr) + checkResults[i].Retryable = false + checkResults[i].IneligibilityReason = uint8(fr) continue } diff --git a/integration-tests/contracts/ethereum_keeper_contracts.go b/integration-tests/contracts/ethereum_keeper_contracts.go index 337e3009f16..8ec6a547b55 100644 --- a/integration-tests/contracts/ethereum_keeper_contracts.go +++ b/integration-tests/contracts/ethereum_keeper_contracts.go @@ -87,6 +87,7 @@ type KeeperRegistry interface { UpdateCheckData(id *big.Int, newCheckData []byte) error SetUpkeepTriggerConfig(id *big.Int, triggerConfig []byte) error SetUpkeepPrivilegeConfig(id *big.Int, privilegeConfig []byte) error + SetUpkeepOffchainConfig(id *big.Int, offchainConfig []byte) error RegistryOwnerAddress() common.Address ChainModuleAddress() common.Address ReorgProtectionEnabled() bool @@ -1225,6 +1226,46 @@ func (v *EthereumKeeperRegistry) UnpauseUpkeep(id *big.Int) error { } } +func (v *EthereumKeeperRegistry) SetUpkeepOffchainConfig(id *big.Int, offchainConfig []byte) error { + switch v.version { + case ethereum.RegistryVersion_2_0: + opts, err := v.client.TransactionOpts(v.client.GetDefaultWallet()) + if err != nil { + return err + } + + tx, err := v.registry2_0.SetUpkeepOffchainConfig(opts, id, offchainConfig) + if err != nil { + return err + } + return v.client.ProcessTransaction(tx) + case ethereum.RegistryVersion_2_1: + opts, err := v.client.TransactionOpts(v.client.GetDefaultWallet()) + if err != nil { + return err + } + + tx, err := v.registry2_1.SetUpkeepOffchainConfig(opts, id, offchainConfig) + if err != nil { + return err + } + return v.client.ProcessTransaction(tx) + case ethereum.RegistryVersion_2_2: + opts, err := v.client.TransactionOpts(v.client.GetDefaultWallet()) + if err != nil { + return err + } + + tx, err := v.registry2_2.SetUpkeepOffchainConfig(opts, id, offchainConfig) + if err != nil { + return err + } + return v.client.ProcessTransaction(tx) + default: + return fmt.Errorf("SetUpkeepOffchainConfig is not supported by keeper registry version %d", v.version) + } +} + // Parses upkeep performed log func (v *EthereumKeeperRegistry) ParseUpkeepPerformedLog(log *types.Log) (*UpkeepPerformedLog, error) { switch v.version { diff --git a/integration-tests/go.mod b/integration-tests/go.mod index 3d76a656be5..3ad1a4c0ace 100644 --- a/integration-tests/go.mod +++ b/integration-tests/go.mod @@ -9,6 +9,7 @@ require ( github.com/barkimedes/go-deepcopy v0.0.0-20220514131651-17c30cfc62df github.com/cli/go-gh/v2 v2.0.0 github.com/ethereum/go-ethereum v1.13.8 + github.com/fxamacker/cbor/v2 v2.5.0 github.com/go-resty/resty/v2 v2.7.0 github.com/google/go-cmp v0.6.0 github.com/google/uuid v1.6.0 @@ -174,7 +175,6 @@ require ( github.com/felixge/httpsnoop v1.0.3 // indirect github.com/fsnotify/fsnotify v1.6.0 // indirect github.com/fvbommel/sortorder v1.0.2 // indirect - github.com/fxamacker/cbor/v2 v2.5.0 // indirect github.com/gabriel-vasile/mimetype v1.4.2 // indirect github.com/gagliardetto/binary v0.7.7 // indirect github.com/gagliardetto/solana-go v1.8.4 // indirect diff --git a/integration-tests/smoke/automation_test.go b/integration-tests/smoke/automation_test.go index 73a7749c4e1..154c6cc0dc5 100644 --- a/integration-tests/smoke/automation_test.go +++ b/integration-tests/smoke/automation_test.go @@ -11,6 +11,8 @@ import ( "time" "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/common/hexutil" + "github.com/fxamacker/cbor/v2" "github.com/onsi/gomega" "github.com/stretchr/testify/require" @@ -34,6 +36,7 @@ import ( "github.com/smartcontractkit/chainlink/integration-tests/types/config/node" ac "github.com/smartcontractkit/chainlink/v2/core/gethwrappers/generated/automation_compatible_utils" "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/core" + "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/gasprice" "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/mercury/streams" ) @@ -190,7 +193,7 @@ func SetupAutomationBasic(t *testing.T, nodeUpgrade bool, automationTestConfig t for i := 0; i < len(upkeepIDs); i++ { counter, err := consumers[i].Counter(testcontext.Get(t)) require.NoError(t, err, "Failed to retrieve consumer counter for upkeep at index %d", i) - l.Info().Int64("Upkeeps Performed", counter.Int64()).Int("Upkeep ID", i).Msg("Number of upkeeps performed") + l.Info().Int64("Upkeeps Performed", counter.Int64()).Int("Upkeep index", i).Msg("Number of upkeeps performed") g.Expect(counter.Int64()).Should(gomega.BeNumerically(">=", int64(expect)), "Expected consumer counter to be greater than %d, but got %d", expect, counter.Int64()) } @@ -631,7 +634,7 @@ func TestAutomationRegisterUpkeep(t *testing.T) { "Expected consumer counter to be greater than 0, but got %d", counter.Int64()) l.Info(). Int64("Upkeep counter", counter.Int64()). - Int64("Upkeep ID", int64(i)). + Int64("Upkeep index", int64(i)). Msg("Number of upkeeps performed") } }, "4m", "1s").Should(gomega.Succeed()) // ~1m for cluster setup, ~1m for performing each upkeep once, ~2m buffer @@ -657,7 +660,7 @@ func TestAutomationRegisterUpkeep(t *testing.T) { g.Expect(err).ShouldNot(gomega.HaveOccurred(), "Calling consumer's counter shouldn't fail") l.Info(). - Int64("Upkeep ID", int64(i)). + Int64("Upkeep index", int64(i)). Int64("Upkeep counter", currentCounter.Int64()). Int64("initial counter", initialCounters[i].Int64()). Msg("Number of upkeeps performed") @@ -1120,6 +1123,117 @@ func TestUpdateCheckData(t *testing.T) { } } +func TestSetOffchainConfigWithMaxGasPrice(t *testing.T) { + t.Parallel() + registryVersions := map[string]ethereum.KeeperRegistryVersion{ + // registry20 also has upkeep offchain config but the max gas price check is not implemented + "registry_2_1": ethereum.RegistryVersion_2_1, + "registry_2_2": ethereum.RegistryVersion_2_2, + } + + for n, rv := range registryVersions { + name := n + registryVersion := rv + t.Run(name, func(t *testing.T) { + t.Parallel() + l := logging.GetTestLogger(t) + config, err := tc.GetConfig("Smoke", tc.Automation) + if err != nil { + t.Fatal(err) + } + a := setupAutomationTestDocker( + t, registryVersion, automationDefaultRegistryConfig(config), false, false, &config, + ) + + consumers, upkeepIDs := actions.DeployConsumers( + t, + a.Registry, + a.Registrar, + a.LinkToken, + a.Deployer, + a.ChainClient, + defaultAmountOfUpkeeps, + big.NewInt(automationDefaultLinkFunds), + automationDefaultUpkeepGasLimit, + false, + false, + ) + gom := gomega.NewGomegaWithT(t) + + l.Info().Msg("waiting for all upkeeps to be performed at least once") + gom.Eventually(func(g gomega.Gomega) { + for i := 0; i < len(upkeepIDs); i++ { + counter, err := consumers[i].Counter(testcontext.Get(t)) + g.Expect(err).ShouldNot(gomega.HaveOccurred(), "Failed to retrieve consumer counter for upkeep at index %d", i) + g.Expect(counter.Int64()).Should(gomega.BeNumerically(">", int64(0)), + "Expected consumer counter to be greater than 0, but got %d") + } + }, "3m", "1s").Should(gomega.Succeed()) // ~1m for cluster setup, ~1m for performing each upkeep once, ~2m buffer + + // set the maxGasPrice to 1 wei + uoc, _ := cbor.Marshal(gasprice.UpkeepOffchainConfig{MaxGasPrice: big.NewInt(1)}) + l.Info().Msgf("setting all upkeeps' offchain config to %s, which means maxGasPrice is 1 wei", hexutil.Encode(uoc)) + for _, uid := range upkeepIDs { + err = a.Registry.SetUpkeepOffchainConfig(uid, uoc) + require.NoError(t, err, "Error setting upkeep offchain config") + err = a.ChainClient.WaitForEvents() + require.NoError(t, err, "Error waiting for events from setting upkeep offchain config") + } + + // Store how many times each upkeep performed once their offchain config is set with maxGasPrice = 1 wei + var countersAfterSettingLowMaxGasPrice = make([]*big.Int, len(upkeepIDs)) + for i := 0; i < len(upkeepIDs); i++ { + countersAfterSettingLowMaxGasPrice[i], err = consumers[i].Counter(testcontext.Get(t)) + require.NoError(t, err, "Failed to retrieve consumer counter for upkeep at index %d", i) + l.Info().Int64("Upkeep Performed times", countersAfterSettingLowMaxGasPrice[i].Int64()).Int("Upkeep index", i).Msg("Number of upkeeps performed") + } + + var latestCounter *big.Int + // the counters of all the upkeeps should stay constant because they are no longer getting serviced + gom.Consistently(func(g gomega.Gomega) { + for i := 0; i < len(upkeepIDs); i++ { + latestCounter, err = consumers[i].Counter(testcontext.Get(t)) + g.Expect(err).ShouldNot(gomega.HaveOccurred(), "Failed to retrieve consumer counter for upkeep at index %d", i) + g.Expect(latestCounter.Int64()).Should(gomega.Equal(countersAfterSettingLowMaxGasPrice[i].Int64()), + "Expected consumer counter to remain constant at %d, but got %d", + countersAfterSettingLowMaxGasPrice[i].Int64(), latestCounter.Int64()) + } + }, "2m", "1s").Should(gomega.Succeed()) + l.Info().Msg("no upkeeps is performed because their max gas price is only 1 wei") + + // setting offchain config with a high max gas price for the first upkeep, it should perform again while + // other upkeeps should not perform + // set the maxGasPrice to 500 gwei for the first upkeep + uoc, _ = cbor.Marshal(gasprice.UpkeepOffchainConfig{MaxGasPrice: big.NewInt(500_000_000_000)}) + l.Info().Msgf("setting the first upkeeps' offchain config to %s, which means maxGasPrice is 500 gwei", hexutil.Encode(uoc)) + err = a.Registry.SetUpkeepOffchainConfig(upkeepIDs[0], uoc) + require.NoError(t, err, "Error setting upkeep offchain config") + + // the counters of all other upkeeps should stay constant because their max gas price remains very low + gom.Consistently(func(g gomega.Gomega) { + for i := 1; i < len(upkeepIDs); i++ { + latestCounter, err = consumers[i].Counter(testcontext.Get(t)) + g.Expect(err).ShouldNot(gomega.HaveOccurred(), "Failed to retrieve consumer counter for upkeep at index %d", i) + g.Expect(latestCounter.Int64()).Should(gomega.Equal(countersAfterSettingLowMaxGasPrice[i].Int64()), + "Expected consumer counter to remain constant at %d, but got %d", + countersAfterSettingLowMaxGasPrice[i].Int64(), latestCounter.Int64()) + } + }, "2m", "1s").Should(gomega.Succeed()) + l.Info().Msg("all the rest upkeeps did not perform again because their max gas price remains 1 wei") + + // the first upkeep should start performing again + gom.Eventually(func(g gomega.Gomega) { + latestCounter, err = consumers[0].Counter(testcontext.Get(t)) + g.Expect(err).ShouldNot(gomega.HaveOccurred(), "Failed to retrieve consumer counter for upkeep at index 0") + g.Expect(latestCounter.Int64()).Should(gomega.BeNumerically(">", countersAfterSettingLowMaxGasPrice[0].Int64()), + "Expected consumer counter to be greater than %d, but got %d", + countersAfterSettingLowMaxGasPrice[0].Int64(), latestCounter.Int64()) + }, "2m", "1s").Should(gomega.Succeed()) // ~1m for cluster setup, ~1m for performing each upkeep once, ~2m buffer + l.Info().Int64("Upkeep Performed times", latestCounter.Int64()).Msg("the first upkeep performed again") + }) + } +} + func setupAutomationTestDocker( t *testing.T, registryVersion ethereum.KeeperRegistryVersion, diff --git a/integration-tests/smoke/automation_test.go_test_list.json b/integration-tests/smoke/automation_test.go_test_list.json index 03029c9018b..e8f0f838dfd 100644 --- a/integration-tests/smoke/automation_test.go_test_list.json +++ b/integration-tests/smoke/automation_test.go_test_list.json @@ -70,6 +70,10 @@ { "name": "TestUpdateCheckData", "nodes": 3 + }, + { + "name": "TestSetOffchainConfigWithMaxGasPrice", + "nodes": 2 } ] } \ No newline at end of file From d1e7eca34399b1cd06ec45453ea01f14af3798e0 Mon Sep 17 00:00:00 2001 From: FelixFan1992 Date: Mon, 6 May 2024 18:04:53 -0400 Subject: [PATCH 6/7] update logs --- .../evmregistry/v21/gasprice/gasprice.go | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/gasprice/gasprice.go b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/gasprice/gasprice.go index 82514477312..1d678e1340a 100644 --- a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/gasprice/gasprice.go +++ b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/gasprice/gasprice.go @@ -26,22 +26,21 @@ type UpkeepOffchainConfig struct { // CheckGasPrice retrieves the current gas price and compare against the max gas price configured in upkeep's offchain config // any errors in offchain config decoding will result in max gas price check disabled -func CheckGasPrice(ctx context.Context, upkeepId *big.Int, oc []byte, ge gas.EvmFeeEstimator, lggr logger.Logger) encoding.UpkeepFailureReason { - if len(oc) == 0 { +func CheckGasPrice(ctx context.Context, upkeepId *big.Int, offchainConfigBytes []byte, ge gas.EvmFeeEstimator, lggr logger.Logger) encoding.UpkeepFailureReason { + if len(offchainConfigBytes) == 0 { return encoding.UpkeepFailureReasonNone } var offchainConfig UpkeepOffchainConfig - if err := cbor.ParseDietCBORToStruct(oc, &offchainConfig); err != nil { + if err := cbor.ParseDietCBORToStruct(offchainConfigBytes, &offchainConfig); err != nil { lggr.Errorw("failed to parse upkeep offchain config, gas price check is disabled", "upkeepId", upkeepId.String(), "err", err) return encoding.UpkeepFailureReasonNone } - if offchainConfig.MaxGasPrice == nil { + if offchainConfig.MaxGasPrice == nil || offchainConfig.MaxGasPrice.Int64() <= 0 { lggr.Infow("maxGasPrice is not configured in upkeep offchain config, gas price check is disabled", "upkeepId", upkeepId.String()) return encoding.UpkeepFailureReasonNone } - lggr.Infof("successfully decode offchain config for %s", upkeepId.String()) - lggr.Infof("max gas price for %s is %s", upkeepId.String(), offchainConfig.MaxGasPrice.String()) + lggr.Debugf("successfully decode offchain config for %s, max gas price is %s", upkeepId.String(), offchainConfig.MaxGasPrice.String()) fee, _, err := ge.GetFee(ctx, []byte{}, feeLimit, assets.NewWei(big.NewInt(maxFeePrice))) if err != nil { @@ -50,21 +49,21 @@ func CheckGasPrice(ctx context.Context, upkeepId *big.Int, oc []byte, ge gas.Evm } if fee.ValidDynamic() { - lggr.Infof("current gas price EIP-1559 is fee cap %s, tip cap %s", fee.DynamicFeeCap.String(), fee.DynamicTipCap.String()) + lggr.Debugf("current gas price EIP-1559 is fee cap %s, tip cap %s", fee.DynamicFeeCap.String(), fee.DynamicTipCap.String()) if fee.DynamicFeeCap.Cmp(assets.NewWei(offchainConfig.MaxGasPrice)) > 0 { // current gas price is higher than max gas price lggr.Warnf("maxGasPrice %s for %s is LOWER than current gas price %d", offchainConfig.MaxGasPrice.String(), upkeepId.String(), fee.DynamicFeeCap.Int64()) return encoding.UpkeepFailureReasonGasPriceTooHigh } - lggr.Infof("maxGasPrice %s for %s is HIGHER than current gas price %d", offchainConfig.MaxGasPrice.String(), upkeepId.String(), fee.DynamicFeeCap.Int64()) + lggr.Debugf("maxGasPrice %s for %s is HIGHER than current gas price %d", offchainConfig.MaxGasPrice.String(), upkeepId.String(), fee.DynamicFeeCap.Int64()) } else { - lggr.Infof("current gas price legacy is %s", fee.Legacy.String()) + lggr.Debugf("current gas price legacy is %s", fee.Legacy.String()) if fee.Legacy.Cmp(assets.NewWei(offchainConfig.MaxGasPrice)) > 0 { // current gas price is higher than max gas price - lggr.Infof("maxGasPrice %s for %s is LOWER than current gas price %d", offchainConfig.MaxGasPrice.String(), upkeepId.String(), fee.Legacy.Int64()) + lggr.Warnf("maxGasPrice %s for %s is LOWER than current gas price %d", offchainConfig.MaxGasPrice.String(), upkeepId.String(), fee.Legacy.Int64()) return encoding.UpkeepFailureReasonGasPriceTooHigh } - lggr.Infof("maxGasPrice %s for %s is HIGHER than current gas price %d", offchainConfig.MaxGasPrice.String(), upkeepId.String(), fee.Legacy.Int64()) + lggr.Debugf("maxGasPrice %s for %s is HIGHER than current gas price %d", offchainConfig.MaxGasPrice.String(), upkeepId.String(), fee.Legacy.Int64()) } return encoding.UpkeepFailureReasonNone From 98be798ccf94c89749580264949ee4cf8b449f12 Mon Sep 17 00:00:00 2001 From: FelixFan1992 Date: Mon, 6 May 2024 18:07:36 -0400 Subject: [PATCH 7/7] fix --- .../plugins/ocr2keeper/evmregistry/v21/gasprice/gasprice.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/gasprice/gasprice.go b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/gasprice/gasprice.go index 1d678e1340a..2c376443fa5 100644 --- a/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/gasprice/gasprice.go +++ b/core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/gasprice/gasprice.go @@ -37,7 +37,7 @@ func CheckGasPrice(ctx context.Context, upkeepId *big.Int, offchainConfigBytes [ return encoding.UpkeepFailureReasonNone } if offchainConfig.MaxGasPrice == nil || offchainConfig.MaxGasPrice.Int64() <= 0 { - lggr.Infow("maxGasPrice is not configured in upkeep offchain config, gas price check is disabled", "upkeepId", upkeepId.String()) + lggr.Warnw("maxGasPrice is not configured or incorrectly configured in upkeep offchain config, gas price check is disabled", "upkeepId", upkeepId.String()) return encoding.UpkeepFailureReasonNone } lggr.Debugf("successfully decode offchain config for %s, max gas price is %s", upkeepId.String(), offchainConfig.MaxGasPrice.String())