Skip to content

Commit

Permalink
Cherry pick gas control commits on release branch (#13486)
Browse files Browse the repository at this point in the history
  • Loading branch information
infiloop2 authored Jun 11, 2024
1 parent ef8a79c commit d87b24c
Show file tree
Hide file tree
Showing 15 changed files with 434 additions and 5 deletions.
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)
})
}
}
13 changes: 13 additions & 0 deletions core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/registry.go
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

0 comments on commit d87b24c

Please sign in to comment.