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

chore: remove vrf maxGasPriceGwei job parameter #7721

Merged
merged 4 commits into from
Oct 24, 2022
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
4 changes: 0 additions & 4 deletions core/services/job/job_orm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,6 @@ func TestORM_CreateJob_VRFV2(t *testing.T) {
ChunkSize: 25,
BackoffInitialDelay: time.Minute,
BackoffMaxDelay: time.Hour,
MaxGasPriceGWei: 100,
}).
Toml())
require.NoError(t, err)
Expand Down Expand Up @@ -388,9 +387,6 @@ func TestORM_CreateJob_VRFV2(t *testing.T) {
var chunkSize int
require.NoError(t, db.Get(&chunkSize, `SELECT chunk_size FROM vrf_specs LIMIT 1`))
require.Equal(t, 25, chunkSize)
var maxGasPriceGWei uint32
require.NoError(t, db.Get(&maxGasPriceGWei, `SELECT max_gas_price_gwei FROM vrf_specs LIMIT 1`))
require.Equal(t, *jb.VRFSpec.MaxGasPriceGWei, maxGasPriceGWei)
var fa pq.ByteaArray
require.NoError(t, db.Get(&fa, `SELECT from_addresses FROM vrf_specs LIMIT 1`))
var actual []string
Expand Down
17 changes: 0 additions & 17 deletions core/services/job/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -443,23 +443,6 @@ type VRFSpec struct {
RequestedConfsDelay int64 `toml:"requestedConfsDelay"` // For v2 jobs. Optional, defaults to 0 if not provided.
RequestTimeout time.Duration `toml:"requestTimeout"` // Optional, defaults to 24hr if not provided.

// MaxGasPriceGwei sets the maximum gas price in gwei on the addresses
// specified in VRFSpec.FromAddresses.
//
// This is optional. If this is not set, then the job will not attempt to set any
// key specific gas prices on the addresses specified in VRFSpec.FromAddresses,
// and as a result, will use any previously set key specific max gas price (i.e, set via CLI or REST API)
// or the global max gas price if a key-specific gas price is not set.
//
// This is essentially the "gas lane" gas price.
//
// This will end up overriding any previously set key-specific gas prices
// set via CLI and/or REST API. However, environment variables can end up
// taking precedence depending on their values relative to the key-specific prices.
//
// V2 only.
MaxGasPriceGWei *uint32 `toml:"maxGasPriceGWei" db:"max_gas_price_gwei"`

// ChunkSize is the number of pending VRF V2 requests to process in parallel. Optional, defaults
// to 20 if not provided.
ChunkSize uint32 `toml:"chunkSize"`
Expand Down
2 changes: 0 additions & 2 deletions core/services/job/orm.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,14 +315,12 @@ func (o *orm) CreateJob(jb *Job, qopts ...pg.QOpt) error {
evm_chain_id, from_addresses, poll_period, requested_confs_delay,
request_timeout, chunk_size, batch_coordinator_address, batch_fulfillment_enabled,
batch_fulfillment_gas_multiplier, backoff_initial_delay, backoff_max_delay,
max_gas_price_gwei,
created_at, updated_at)
VALUES (
:coordinator_address, :public_key, :min_incoming_confirmations,
:evm_chain_id, :from_addresses, :poll_period, :requested_confs_delay,
:request_timeout, :chunk_size, :batch_coordinator_address, :batch_fulfillment_enabled,
:batch_fulfillment_gas_multiplier, :backoff_initial_delay, :backoff_max_delay,
:max_gas_price_gwei,
NOW(), NOW())
RETURNING id;`

Expand Down
39 changes: 0 additions & 39 deletions core/services/vrf/delegate.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"github.com/smartcontractkit/chainlink/core/logger"
"github.com/smartcontractkit/chainlink/core/services/job"
"github.com/smartcontractkit/chainlink/core/services/keystore"
"github.com/smartcontractkit/chainlink/core/services/keystore/keys/ethkey"
"github.com/smartcontractkit/chainlink/core/services/pg"
"github.com/smartcontractkit/chainlink/core/services/pipeline"
"github.com/smartcontractkit/chainlink/core/utils"
Expand Down Expand Up @@ -115,17 +114,6 @@ func (d *Delegate) ServicesForSpec(jb job.Job) ([]job.ServiceCtx, error) {
lV1 := l.Named("VRFListener")
lV2 := l.Named("VRFListenerV2")

// This is not a terminal error, but it isn't ideal.
// We can operate under these potentially degraded conditions, however.
err = setMaxGasPriceGWei(
toAddrSlice(jb.VRFSpec.FromAddresses),
jb.VRFSpec.MaxGasPriceGWei,
d.cc,
chain.ID())
if err != nil {
l.With("err", err).Warn("unable to set max gas price gwei on from addresses, continuing anyway")
}

for _, task := range pl.Tasks {
if _, ok := task.(*pipeline.VRFTaskV2); ok {
linkEthFeedAddress, err := coordinatorV2.LINKETHFEED(nil)
Expand All @@ -142,7 +130,6 @@ func (d *Delegate) ServicesForSpec(jb job.Job) ([]job.ServiceCtx, error) {
lV2,
chain.Client(),
chain.ID(),
d.cc,
chain.LogBroadcaster(),
d.q,
coordinatorV2,
Expand Down Expand Up @@ -283,29 +270,3 @@ GROUP BY meta->'RequestID'
}
return counts, nil
}

// setMaxGasPriceGWei attempts to set the max gas price for each key in the fromAddresses slice
// of the VRF job spec to the given maxGasPriceGWei parameter.
// If maxGasPriceGWei is nil, this is a no-op.
func setMaxGasPriceGWei(fromAddresses []common.Address, maxGasPriceGWei *uint32, keyUpdater keyConfigUpdater, chainID *big.Int) error {
if maxGasPriceGWei == nil {
return nil
}

for _, addr := range fromAddresses {
updater := evm.UpdateKeySpecificMaxGasPrice(addr, assets.GWei(int64(*maxGasPriceGWei)))
err := keyUpdater.UpdateConfig(chainID, updater)
if err != nil {
return errors.Wrap(err, "update key specific max gas price")
}
}

return nil
}

func toAddrSlice(addrs []ethkey.EIP55Address) (r []common.Address) {
for _, a := range addrs {
r = append(r, a.Address())
}
return
}
27 changes: 12 additions & 15 deletions core/services/vrf/integration_v2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,6 @@ func subscribeVRF(
func createVRFJobs(
t *testing.T,
fromKeys [][]ethkey.KeyV2,
maxGasPricesGWei []int,
app *cltest.TestApplication,
uni coordinatorV2Universe,
batchEnabled bool,
Expand Down Expand Up @@ -345,7 +344,6 @@ func createVRFJobs(
FromAddresses: keyStrs,
BackoffInitialDelay: 10 * time.Millisecond,
BackoffMaxDelay: time.Second,
MaxGasPriceGWei: maxGasPricesGWei[i],
V2: true,
}).Toml()
jb, err := vrf.ValidatedVRFSpec(s)
Expand Down Expand Up @@ -622,7 +620,7 @@ func TestVRFV2Integration_SingleConsumer_HappyPath_BatchFulfillment(t *testing.T
require.NoError(t, app.Start(testutils.Context(t)))

// Create VRF job using key1 and key2 on the same gas lane.
jbs := createVRFJobs(t, [][]ethkey.KeyV2{{key1}}, []int{10}, app, uni, true)
jbs := createVRFJobs(t, [][]ethkey.KeyV2{{key1}}, app, uni, true)
keyHash := jbs[0].VRFSpec.PublicKey.MustHash()

// Make some randomness requests.
Expand Down Expand Up @@ -685,7 +683,7 @@ func TestVRFV2Integration_SingleConsumer_HappyPath_BatchFulfillment_BigGasCallba
require.NoError(t, app.Start(testutils.Context(t)))

// Create VRF job using key1 and key2 on the same gas lane.
jbs := createVRFJobs(t, [][]ethkey.KeyV2{{key1}}, []int{10}, app, uni, true)
jbs := createVRFJobs(t, [][]ethkey.KeyV2{{key1}}, app, uni, true)
keyHash := jbs[0].VRFSpec.PublicKey.MustHash()

// Make some randomness requests with low max gas callback limits.
Expand Down Expand Up @@ -759,7 +757,7 @@ func TestVRFV2Integration_SingleConsumer_HappyPath(t *testing.T) {
require.NoError(t, app.Start(testutils.Context(t)))

// Create VRF job using key1 and key2 on the same gas lane.
jbs := createVRFJobs(t, [][]ethkey.KeyV2{{key1, key2}}, []int{10, 10}, app, uni, false)
jbs := createVRFJobs(t, [][]ethkey.KeyV2{{key1, key2}}, app, uni, false)
keyHash := jbs[0].VRFSpec.PublicKey.MustHash()

// Make the first randomness request.
Expand Down Expand Up @@ -840,7 +838,7 @@ func TestVRFV2Integration_SingleConsumer_EIP150_HappyPath(t *testing.T) {
require.NoError(t, app.Start(testutils.Context(t)))

// Create VRF job.
jbs := createVRFJobs(t, [][]ethkey.KeyV2{{key1}}, []int{10, 10}, app, uni, false)
jbs := createVRFJobs(t, [][]ethkey.KeyV2{{key1}}, app, uni, false)
keyHash := jbs[0].VRFSpec.PublicKey.MustHash()

// Make the first randomness request.
Expand Down Expand Up @@ -890,7 +888,7 @@ func TestVRFV2Integration_SingleConsumer_EIP150_Revert(t *testing.T) {
require.NoError(t, app.Start(testutils.Context(t)))

// Create VRF job.
jbs := createVRFJobs(t, [][]ethkey.KeyV2{{key1}}, []int{10, 10}, app, uni, false)
jbs := createVRFJobs(t, [][]ethkey.KeyV2{{key1}}, app, uni, false)
keyHash := jbs[0].VRFSpec.PublicKey.MustHash()

// Make the first randomness request.
Expand Down Expand Up @@ -956,7 +954,7 @@ func TestVRFV2Integration_SingleConsumer_Wrapper(t *testing.T) {
require.NoError(t, app.Start(testutils.Context(t)))

// Create VRF job.
jbs := createVRFJobs(t, [][]ethkey.KeyV2{{key1}}, []int{10, 10}, app, uni, false)
jbs := createVRFJobs(t, [][]ethkey.KeyV2{{key1}}, app, uni, false)
keyHash := jbs[0].VRFSpec.PublicKey.MustHash()

wrapper, _, consumer, consumerAddress := deployWrapper(t, uni, wrapperOverhead, coordinatorOverhead, keyHash, maxNumWords)
Expand Down Expand Up @@ -1024,7 +1022,7 @@ func TestVRFV2Integration_Wrapper_High_Gas(t *testing.T) {
require.NoError(t, app.Start(testutils.Context(t)))

// Create VRF job.
jbs := createVRFJobs(t, [][]ethkey.KeyV2{{key1}}, []int{10, 10}, app, uni, false)
jbs := createVRFJobs(t, [][]ethkey.KeyV2{{key1}}, app, uni, false)
keyHash := jbs[0].VRFSpec.PublicKey.MustHash()

wrapper, _, consumer, consumerAddress := deployWrapper(t, uni, wrapperOverhead, coordinatorOverhead, keyHash, maxNumWords)
Expand Down Expand Up @@ -1099,7 +1097,7 @@ func TestVRFV2Integration_SingleConsumer_NeedsBlockhashStore(t *testing.T) {
sendEth(t, ownerKey, uni.backend, bhsKey.Address, 10)

// Create VRF job.
vrfJobs := createVRFJobs(t, [][]ethkey.KeyV2{{vrfKey}}, []int{10}, app, uni, false)
vrfJobs := createVRFJobs(t, [][]ethkey.KeyV2{{vrfKey}}, app, uni, false)
keyHash := vrfJobs[0].VRFSpec.PublicKey.MustHash()

_ = createAndStartBHSJob(
Expand Down Expand Up @@ -1187,7 +1185,7 @@ func TestVRFV2Integration_SingleConsumer_NeedsTopUp(t *testing.T) {
require.NoError(t, app.Start(testutils.Context(t)))

// Create VRF job.
jbs := createVRFJobs(t, [][]ethkey.KeyV2{{key}}, []int{1000}, app, uni, false)
jbs := createVRFJobs(t, [][]ethkey.KeyV2{{key}}, app, uni, false)
keyHash := jbs[0].VRFSpec.PublicKey.MustHash()

numWords := uint32(20)
Expand Down Expand Up @@ -1251,7 +1249,7 @@ func TestVRFV2Integration_SingleConsumer_BigGasCallback_Sandwich(t *testing.T) {
require.NoError(t, app.Start(testutils.Context(t)))

// Create VRF job.
jbs := createVRFJobs(t, [][]ethkey.KeyV2{{key1}}, []int{100}, app, uni, false)
jbs := createVRFJobs(t, [][]ethkey.KeyV2{{key1}}, app, uni, false)
keyHash := jbs[0].VRFSpec.PublicKey.MustHash()

// Make some randomness requests, each one block apart, which contain a single low-gas request sandwiched between two high-gas requests.
Expand Down Expand Up @@ -1349,7 +1347,7 @@ func TestVRFV2Integration_SingleConsumer_MultipleGasLanes(t *testing.T) {
require.NoError(t, app.Start(testutils.Context(t)))

// Create VRF jobs.
jbs := createVRFJobs(t, [][]ethkey.KeyV2{{cheapKey}, {expensiveKey}}, []int{10, 1000}, app, uni, false)
jbs := createVRFJobs(t, [][]ethkey.KeyV2{{cheapKey}, {expensiveKey}}, app, uni, false)
cheapHash := jbs[0].VRFSpec.PublicKey.MustHash()
expensiveHash := jbs[1].VRFSpec.PublicKey.MustHash()

Expand Down Expand Up @@ -1433,7 +1431,7 @@ func TestVRFV2Integration_SingleConsumer_AlwaysRevertingCallback_StillFulfilled(
require.NoError(t, app.Start(testutils.Context(t)))

// Create VRF job.
jbs := createVRFJobs(t, [][]ethkey.KeyV2{{key}}, []int{10}, app, uni, false)
jbs := createVRFJobs(t, [][]ethkey.KeyV2{{key}}, app, uni, false)
keyHash := jbs[0].VRFSpec.PublicKey.MustHash()

// Make the randomness request.
Expand Down Expand Up @@ -1626,7 +1624,6 @@ func TestIntegrationVRFV2(t *testing.T) {
BatchCoordinatorAddress: uni.batchCoordinatorContractAddress.String(),
MinIncomingConfirmations: incomingConfs,
PublicKey: vrfkey.PublicKey.String(),
MaxGasPriceGWei: 10,
FromAddresses: []string{key.EIP55Address.String()},
V2: true,
}).Toml()
Expand Down
23 changes: 0 additions & 23 deletions core/services/vrf/listener_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"golang.org/x/exp/slices"

"github.com/smartcontractkit/chainlink/core/assets"
"github.com/smartcontractkit/chainlink/core/chains/evm"
evmclient "github.com/smartcontractkit/chainlink/core/chains/evm/client"
httypes "github.com/smartcontractkit/chainlink/core/chains/evm/headtracker/types"
"github.com/smartcontractkit/chainlink/core/chains/evm/log"
Expand Down Expand Up @@ -74,17 +73,11 @@ func (errBlockhashNotInStore) Error() string {
return "Blockhash not in store"
}

// keyConfigUpdater can update key-specific max gas prices for a given chain.
type keyConfigUpdater interface {
UpdateConfig(id *big.Int, updaters ...evm.ChainConfigUpdater) error
}

func newListenerV2(
cfg Config,
l logger.Logger,
ethClient evmclient.Client,
chainID *big.Int,
keyUpdater keyConfigUpdater,
logBroadcaster log.Broadcaster,
q pg.Q,
coordinator vrf_coordinator_v2.VRFCoordinatorV2Interface,
Expand Down Expand Up @@ -123,7 +116,6 @@ func newListenerV2(
wg: &sync.WaitGroup{},
aggregator: aggregator,
deduper: deduper,
keyUpdater: keyUpdater,
}
}

Expand Down Expand Up @@ -159,9 +151,6 @@ type listenerV2 struct {
logBroadcaster log.Broadcaster
txm txmgr.TxManager

// to update key-specific max gas prices if job spec field is set.
keyUpdater keyConfigUpdater

coordinator vrf_coordinator_v2.VRFCoordinatorV2Interface
batchCoordinator batch_vrf_coordinator_v2.BatchVRFCoordinatorV2Interface

Expand Down Expand Up @@ -577,12 +566,6 @@ func (lsn *listenerV2) processRequestsPerSubBatch(
}

fromAddresses := lsn.fromAddresses()
err = setMaxGasPriceGWei(fromAddresses, lsn.job.VRFSpec.MaxGasPriceGWei, lsn.keyUpdater, lsn.chainID)
if err != nil {
l.Warnw("Couldn't set max gas price gwei on configured from addresses, processing anyway",
"err", err, "fromAddresses", fromAddresses)
}

fromAddress, err := lsn.gethks.GetRoundRobinAddress(lsn.chainID, fromAddresses...)
if err != nil {
l.Errorw("Couldn't get next from address", "err", err)
Expand Down Expand Up @@ -735,12 +718,6 @@ func (lsn *listenerV2) processRequestsPerSub(
}

fromAddresses := lsn.fromAddresses()
err = setMaxGasPriceGWei(fromAddresses, lsn.job.VRFSpec.MaxGasPriceGWei, lsn.keyUpdater, lsn.chainID)
if err != nil {
l.Warnw("Couldn't set max gas price gwei on configured from addresses, processing anyway",
"err", err, "fromAddresses", fromAddresses)
}

fromAddress, err := lsn.gethks.GetRoundRobinAddress(lsn.chainID, fromAddresses...)
if err != nil {
l.Errorw("Couldn't get next from address", "err", err)
Expand Down
46 changes: 0 additions & 46 deletions core/services/vrf/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ requestTimeout = "168h" # 7 days
chunkSize = 25
backoffInitialDelay = "1m"
backoffMaxDelay = "2h"
maxGasPriceGWei = 200
observationSource = """
decode_log [type=ethabidecodelog
abi="RandomnessRequest(bytes32 keyHash,uint256 seed,bytes32 indexed jobID,address sender,uint256 fee,bytes32 requestID)"
Expand All @@ -55,7 +54,6 @@ decode_log->vrf->encode_tx->submit_tx
assert.Equal(t, uint32(10), s.VRFSpec.MinIncomingConfirmations)
assert.Equal(t, "0xB3b7874F13387D44a3398D298B075B7A3505D8d4", s.VRFSpec.CoordinatorAddress.String())
assert.Equal(t, "0x79be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f8179800", s.VRFSpec.PublicKey.String())
assert.Equal(t, uint32(200), *s.VRFSpec.MaxGasPriceGWei)
require.Equal(t, 168*time.Hour, s.VRFSpec.RequestTimeout)
require.Equal(t, time.Minute, s.VRFSpec.BackoffInitialDelay)
require.Equal(t, 2*time.Hour, s.VRFSpec.BackoffMaxDelay)
Expand Down Expand Up @@ -431,50 +429,6 @@ decode_log->vrf->encode_tx->submit_tx
require.Error(t, err)
},
},
{
name: "max gas price gwei not provided",
toml: `
type = "vrf"
schemaVersion = 1
minIncomingConfirmations = 10
publicKey = "0x79BE667EF9DCBBAC55A06295CE870B07029BFCDB2DCE28D959F2815B16F8179800"
coordinatorAddress = "0xB3b7874F13387D44a3398D298B075B7A3505D8d4"
requestTimeout = "168h" # 7 days
chunkSize = 25
backoffInitialDelay = "1m"
backoffMaxDelay = "2h"
observationSource = """
decode_log [type=ethabidecodelog
abi="RandomnessRequest(bytes32 keyHash,uint256 seed,bytes32 indexed jobID,address sender,uint256 fee,bytes32 requestID)"
data="$(jobRun.logData)"
topics="$(jobRun.logTopics)"]
vrf [type=vrf
publicKey="$(jobSpec.publicKey)"
requestBlockHash="$(jobRun.logBlockHash)"
requestBlockNumber="$(jobRun.logBlockNumber)"
topics="$(jobRun.logTopics)"]
encode_tx [type=ethabiencode
abi="fulfillRandomnessRequest(bytes proof)"
data="{\\"proof\\": $(vrf)}"]
submit_tx [type=ethtx to="%s"
data="$(encode_tx)"
txMeta="{\\"requestTxHash\\": $(jobRun.logTxHash),\\"requestID\\": $(decode_log.requestID),\\"jobID\\": $(jobSpec.databaseID)}"]
decode_log->vrf->encode_tx->submit_tx
"""
`,
assertion: func(t *testing.T, s job.Job, err error) {
require.NoError(t, err)
require.NotNil(t, s.VRFSpec)
assert.Equal(t, uint32(10), s.VRFSpec.MinIncomingConfirmations)
assert.Equal(t, "0xB3b7874F13387D44a3398D298B075B7A3505D8d4", s.VRFSpec.CoordinatorAddress.String())
assert.Equal(t, "0x79be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f8179800", s.VRFSpec.PublicKey.String())
assert.Nil(t, s.VRFSpec.MaxGasPriceGWei)
require.Equal(t, 168*time.Hour, s.VRFSpec.RequestTimeout)
require.Equal(t, time.Minute, s.VRFSpec.BackoffInitialDelay)
require.Equal(t, 2*time.Hour, s.VRFSpec.BackoffMaxDelay)
require.EqualValues(t, 25, s.VRFSpec.ChunkSize)
},
},
}
for _, tc := range tt {
t.Run(tc.name, func(t *testing.T) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
-- +goose Up
ALTER TABLE vrf_specs DROP COLUMN "max_gas_price_gwei";

-- +goose Down
ALTER TABLE vrf_specs
ADD COLUMN "max_gas_price_gwei" BIGINT
CHECK (max_gas_price_gwei >= 0);
Loading