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.

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

Cherry pick gas control commits on release branch #13486

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions .changeset/funny-monkeys-heal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"chainlink": patch
---

#changed:
AUTO-10539: adjust logging for offchain config and gas control
6 changes: 6 additions & 0 deletions .changeset/funny-tips-promise.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"chainlink": patch
---

#added
compare user-defined max gas price with current gas price in automation simulation pipeline
6 changes: 6 additions & 0 deletions .changeset/hungry-apes-hope.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"chainlink": patch
---

#bugfix
fix an automation smoke test flake
6 changes: 6 additions & 0 deletions .changeset/neat-pianos-argue.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"chainlink": patch
---

#added
pass a gas estimator to registry 2.1 pipeline
5 changes: 5 additions & 0 deletions .changeset/witty-weeks-kneel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"chainlink": patch
---

#added an integration test for max gas price check
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const (
UpkeepFailureReasonInvalidRevertDataInput UpkeepFailureReason = 34
UpkeepFailureReasonSimulationFailed UpkeepFailureReason = 35
UpkeepFailureReasonTxHashReorged UpkeepFailureReason = 36
UpkeepFailureReasonGasPriceTooHigh UpkeepFailureReason = 37

// pipeline execution error
NoPipelineError PipelineExecutionState = 0
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package gasprice

import (
"context"
"math/big"

"github.com/ethereum/go-ethereum/common/hexutil"

"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"`
}

// 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, offchainConfigBytes []byte, ge gas.EvmFeeEstimator, lggr logger.Logger) encoding.UpkeepFailureReason {
// check for empty offchain config
if len(offchainConfigBytes) == 0 {
return encoding.UpkeepFailureReasonNone
}

var offchainConfig UpkeepOffchainConfig
if err := cbor.ParseDietCBORToStruct(offchainConfigBytes, &offchainConfig); err != nil {
lggr.Warnw("failed to parse upkeep offchain config, gas price check is disabled", "offchainconfig", hexutil.Encode(offchainConfigBytes), "upkeepId", upkeepId.String(), "err", err)
return encoding.UpkeepFailureReasonNone
}
if offchainConfig.MaxGasPrice == nil || offchainConfig.MaxGasPrice.Int64() <= 0 {
lggr.Debugw("maxGasPrice is not configured or incorrectly configured in upkeep offchain config, gas price check is disabled", "offchainconfig", hexutil.Encode(offchainConfigBytes), "upkeepId", upkeepId.String())
return encoding.UpkeepFailureReasonNone
}
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 {
lggr.Errorw("failed to get fee, gas price check is disabled", "upkeepId", upkeepId.String(), "err", err)
return encoding.UpkeepFailureReasonNone
}

if fee.ValidDynamic() {
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.Debugf("maxGasPrice %s for %s is HIGHER than current gas price %d", offchainConfig.MaxGasPrice.String(), upkeepId.String(), fee.DynamicFeeCap.Int64())
} else {
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.Warnf("maxGasPrice %s for %s is LOWER than current gas price %d", offchainConfig.MaxGasPrice.String(), upkeepId.String(), fee.Legacy.Int64())
return encoding.UpkeepFailureReasonGasPriceTooHigh
}
lggr.Debugf("maxGasPrice %s for %s is HIGHER than current gas price %d", offchainConfig.MaxGasPrice.String(), upkeepId.String(), fee.Legacy.Int64())
}

return encoding.UpkeepFailureReasonNone
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
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
FailedToGetFee bool
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.UpkeepFailureReasonNone,
},
{
Name: "fail to retrieve current gas price",
MaxGasPrice: big.NewInt(8_000_000_000),
FailedToGetFee: true,
ExpectedResult: encoding.UpkeepFailureReasonNone,
},
{
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.FailedToGetFee {
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)
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -113,6 +114,7 @@ func NewEvmRegistry(
bs: blockSub,
finalityDepth: finalityDepth,
streams: streams.NewStreamsLookup(mercuryConfig, blockSub, client.Client(), registry, lggr),
ge: client.GasEstimator(),
}
}

Expand Down Expand Up @@ -194,6 +196,7 @@ type EvmRegistry struct {
logEventProvider logprovider.LogEventProvider
finalityDepth uint32
streams streams.Lookup
ge gas.EvmFeeEstimator
}

func (r *EvmRegistry) Name() string {
Expand Down Expand Up @@ -627,3 +630,13 @@ 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)
if err != nil {
return []byte{}, err
}
return ui.OffchainConfig, nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (

"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 (
Expand Down Expand Up @@ -305,7 +306,19 @@ func (r *EvmRegistry) simulatePerformUpkeeps(ctx context.Context, checkResults [

block, _, upkeepId := r.getBlockAndUpkeepId(cr.UpkeepID, cr.Trigger)

opts := r.buildCallOpts(ctx, block)
oc, err := r.fetchUpkeepOffchainConfig(upkeepId)
if err != nil {
// 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 uint8(fr) == uint8(encoding.UpkeepFailureReasonGasPriceTooHigh) {
r.lggr.Debugf("upkeep %s upkeep failure reason is %d", upkeepId, fr)
checkResults[i].Eligible = false
checkResults[i].Retryable = false
checkResults[i].IneligibilityReason = uint8(fr)
continue
}

// Since checkUpkeep is true, simulate perform upkeep to ensure it doesn't revert
payload, err := r.abi.Pack("simulatePerformUpkeep", upkeepId, cr.PerformData)
Expand All @@ -317,6 +330,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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
ocr2keepers "github.com/smartcontractkit/chainlink-common/pkg/types/automation"

evmClientMocks "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/logpoller"
"github.com/smartcontractkit/chainlink/v2/core/chains/evm/types"
ac "github.com/smartcontractkit/chainlink/v2/core/gethwrappers/generated/i_automation_v21_plus_common"
Expand Down Expand Up @@ -651,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)
Expand All @@ -670,6 +678,7 @@ func setupEVMRegistry(t *testing.T) *EvmRegistry {
mockReg := mocks.NewRegistry(t)
mockHttpClient := mocks.NewHttpClient(t)
client := evmClientMocks.NewClient(t)
ge := gasMocks.NewEvmFeeEstimator(t)

r := &EvmRegistry{
lggr: lggr,
Expand All @@ -694,6 +703,8 @@ func setupEVMRegistry(t *testing.T) *EvmRegistry {
AllowListCache: cache.New(defaultAllowListExpiration, cleanupInterval),
},
hc: mockHttpClient,
bs: &BlockSubscriber{latestBlock: atomic.Pointer[ocr2keepers.BlockKey]{}},
ge: ge,
}
return r
}
Loading
Loading