From fc427ecc81ce32d349bcff640a6797e56d6b8066 Mon Sep 17 00:00:00 2001 From: krehermann Date: Mon, 13 Feb 2023 09:23:38 -0800 Subject: [PATCH 01/15] Support dynamic TOML (#8421) (cherry picked from commit ee9fd178aff52f0379b48246a2a371673983c504) --- core/cmd/app.go | 40 ++++++++------ core/cmd/app_test.go | 128 +++++++++++++++++++++++++++++++++++++++++++ core/main_test.go | 2 +- docs/CHANGELOG.md | 2 + 4 files changed, 154 insertions(+), 18 deletions(-) create mode 100644 core/cmd/app_test.go diff --git a/core/cmd/app.go b/core/cmd/app.go index f71444e83f8..de9de8fb2af 100644 --- a/core/cmd/app.go +++ b/core/cmd/app.go @@ -71,7 +71,7 @@ func NewApp(client *Client) *cli.App { }, cli.StringSliceFlag{ Name: "config, c", - Usage: "TOML configuration file(s) via flag, or raw TOML via env var. If used, legacy env vars must not be set. Multiple files can be used (-c configA.toml -c configB.toml), and they are applied in order with duplicated fields overriding any earlier values.", + Usage: "TOML configuration file(s) via flag, or raw TOML via env var. If used, legacy env vars must not be set. Multiple files can be used (-c configA.toml -c configB.toml), and they are applied in order with duplicated fields overriding any earlier values. If the env var is specified, it is always processed last with the effect of being the final override.", EnvVar: "CL_CONFIG", }, cli.StringFlag{ @@ -83,22 +83,9 @@ func NewApp(client *Client) *cli.App { if c.IsSet("config") { // TOML var opts chainlink.GeneralConfigOpts - if configTOML := v2.EnvConfig.Get(); configTOML != "" { - if err := opts.ParseConfig(configTOML); err != nil { - return errors.Wrapf(err, "failed to parse env var %q", v2.EnvConfig) - } - } else { - fileNames := c.StringSlice("config") - for _, fileName := range fileNames { - b, err := os.ReadFile(fileName) - if err != nil { - return errors.Wrapf(err, "failed to read config file: %s", fileName) - } - if err := opts.ParseConfig(string(b)); err != nil { - return errors.Wrapf(err, "failed to parse file: %s", fileName) - } - } - } + + fileNames := c.StringSlice("config") + loadOpts(&opts, fileNames...) secretsTOML := "" if c.IsSet("secrets") { @@ -1231,3 +1218,22 @@ func logDeprecatedClientEnvWarnings(lggr logger.Logger) { lggr.Errorf("ADMIN_CREDENTIALS_FILE env var has been deprecated and will be removed in a future release. Use flag instead: --admin-credentials-file=%s", s) } } + +// loadOpts applies file configs and then overlays env config +func loadOpts(opts *chainlink.GeneralConfigOpts, fileNames ...string) error { + for _, fileName := range fileNames { + b, err := os.ReadFile(fileName) + if err != nil { + return errors.Wrapf(err, "failed to read config file: %s", fileName) + } + if err := opts.ParseConfig(string(b)); err != nil { + return errors.Wrapf(err, "failed to parse file: %s", fileName) + } + } + if configTOML := v2.EnvConfig.Get(); configTOML != "" { + if err := opts.ParseConfig(configTOML); err != nil { + return errors.Wrapf(err, "failed to parse env var %q", v2.EnvConfig) + } + } + return nil +} diff --git a/core/cmd/app_test.go b/core/cmd/app_test.go new file mode 100644 index 00000000000..07203bf232c --- /dev/null +++ b/core/cmd/app_test.go @@ -0,0 +1,128 @@ +package cmd + +import ( + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/pelletier/go-toml/v2" + v2 "github.com/smartcontractkit/chainlink/core/config/v2" + "github.com/smartcontractkit/chainlink/core/services/chainlink" + "github.com/stretchr/testify/require" +) + +var ( + setInFile = "set in config file" + setInEnv = "set in env" + + testEnvContents = fmt.Sprintf("P2P.V2.AnnounceAddresses = ['%s']", setInEnv) + + testConfigFileContents = chainlink.Config{ + Core: v2.Core{ + RootDir: &setInFile, + P2P: v2.P2P{ + V2: v2.P2PV2{ + AnnounceAddresses: &[]string{setInFile}, + ListenAddresses: &[]string{setInFile}, + }, + }, + }, + } +) + +func makeTestConfigFile(t *testing.T) string { + d := t.TempDir() + p := filepath.Join(d, "test.toml") + + b, err := toml.Marshal(testConfigFileContents) + require.NoError(t, err) + + require.NoError(t, os.WriteFile(p, b, 0777)) + return p +} + +func Test_loadOpts(t *testing.T) { + type args struct { + opts *chainlink.GeneralConfigOpts + fileNames []string + envVar string + } + tests := []struct { + name string + args args + wantErr bool + wantOpts *chainlink.GeneralConfigOpts + }{ + { + name: "env only", + args: args{ + opts: new(chainlink.GeneralConfigOpts), + envVar: testEnvContents, + }, + wantOpts: &chainlink.GeneralConfigOpts{ + Config: chainlink.Config{ + Core: v2.Core{ + P2P: v2.P2P{ + V2: v2.P2PV2{ + AnnounceAddresses: &[]string{setInEnv}, + }, + }, + }, + }, + }, + }, + + { + name: "files only", + args: args{ + opts: new(chainlink.GeneralConfigOpts), + fileNames: []string{makeTestConfigFile(t)}, + }, + wantOpts: &chainlink.GeneralConfigOpts{ + Config: testConfigFileContents, + }, + }, + { + name: "file error", + args: args{ + opts: new(chainlink.GeneralConfigOpts), + fileNames: []string{"notexist"}, + }, + wantErr: true, + }, + + { + name: "env overlay of file", + args: args{ + opts: new(chainlink.GeneralConfigOpts), + fileNames: []string{makeTestConfigFile(t)}, + envVar: testEnvContents, + }, + wantOpts: &chainlink.GeneralConfigOpts{ + Config: chainlink.Config{ + Core: v2.Core{ + RootDir: &setInFile, + P2P: v2.P2P{ + V2: v2.P2PV2{ + // env should override this specific field + AnnounceAddresses: &[]string{setInEnv}, + ListenAddresses: &[]string{setInFile}, + }, + }, + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.args.envVar != "" { + t.Setenv(string(v2.EnvConfig), tt.args.envVar) + } + if err := loadOpts(tt.args.opts, tt.args.fileNames...); (err != nil) != tt.wantErr { + t.Errorf("loadOpts() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/core/main_test.go b/core/main_test.go index d0d8b173084..843549ff68f 100644 --- a/core/main_test.go +++ b/core/main_test.go @@ -76,7 +76,7 @@ func ExampleRun() { // --admin-credentials-file FILE optional, applies only in client mode when making remote API calls. If provided, FILE containing admin credentials will be used for logging in, allowing to avoid an additional login step. If `FILE` is missing, it will be ignored. Defaults to /apicredentials // --remote-node-url URL optional, applies only in client mode when making remote API calls. If provided, URL will be used as the remote Chainlink API endpoint (default: "http://localhost:6688") // --insecure-skip-verify optional, applies only in client mode when making remote API calls. If turned on, SSL certificate verification will be disabled. This is mostly useful for people who want to use Chainlink with a self-signed TLS certificate - // --config value, -c value TOML configuration file(s) via flag, or raw TOML via env var. If used, legacy env vars must not be set. Multiple files can be used (-c configA.toml -c configB.toml), and they are applied in order with duplicated fields overriding any earlier values. [$CL_CONFIG] + // --config value, -c value TOML configuration file(s) via flag, or raw TOML via env var. If used, legacy env vars must not be set. Multiple files can be used (-c configA.toml -c configB.toml), and they are applied in order with duplicated fields overriding any earlier values. If the env var is specified, it is always processed last with the effect of being the final override. [$CL_CONFIG] // --secrets value, -s value TOML configuration file for secrets. Must be set if and only if config is set. // --help, -h show help // --version, -v print the version diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index be9d796d308..aeff0a44c0f 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -29,6 +29,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Removed `KEEPER_TURN_FLAG_ENABLED` as all networks/nodes have switched this to `true` now. The variable should be completely removed my NOPs. - Removed `Keeper.UpkeepCheckGasPriceEnabled` config (`KEEPER_CHECK_UPKEEP_GAS_PRICE_FEATURE_ENABLED` in old env var configuration) as this feature is deprecated now. The variable should be completely removed by NOPs. +- TOML env var `CL_CONFIG` always processed as the last configuration, with the effect of being the final override +of any values provided via configuration files. ### Fixed From 988d4b83ef77af512c470232cfa10a4a3f117cf5 Mon Sep 17 00:00:00 2001 From: chainchad <96362174+chainchad@users.noreply.github.com> Date: Thu, 16 Feb 2023 08:47:36 -0500 Subject: [PATCH 02/15] Bump version and update CHANGELOG for core v1.12.1 --- VERSION | 2 +- docs/CHANGELOG.md | 13 +++++++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/VERSION b/VERSION index 0eed1a29efd..f8f4f03b3dc 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.12.0 +1.12.1 diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index aeff0a44c0f..022f7dbce42 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -11,6 +11,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ... + + +## 1.12.1 - UNRELEASED + +### Updated + +- TOML env var `CL_CONFIG` always processed as the last configuration, with the effect of being the final override of any values provided via configuration files. + + ## 1.12.0 - 2023-02-14 ### Added @@ -29,8 +38,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Removed `KEEPER_TURN_FLAG_ENABLED` as all networks/nodes have switched this to `true` now. The variable should be completely removed my NOPs. - Removed `Keeper.UpkeepCheckGasPriceEnabled` config (`KEEPER_CHECK_UPKEEP_GAS_PRICE_FEATURE_ENABLED` in old env var configuration) as this feature is deprecated now. The variable should be completely removed by NOPs. -- TOML env var `CL_CONFIG` always processed as the last configuration, with the effect of being the final override -of any values provided via configuration files. ### Fixed @@ -39,8 +46,6 @@ of any values provided via configuration files. - The `P2P.V1.Enabled` config logic incorrectly matched V2, by only setting explicit true values so that otherwise the default is used. The `V1.Enabled` default value is actually true already, and is now updated to only set explicit false values. - The `[EVM.Transactions]` config fields `MaxQueued` & `MaxInFlight` will now correctly match `ETH_MAX_QUEUED_TRANSACTIONS` & `ETH_MAX_IN_FLIGHT_TRANSACTIONS`. - - ## 1.11.0 - 2022-12-12 ### Added From d1336c88cfac28469d2a5e93026f3fb10969275c Mon Sep 17 00:00:00 2001 From: Jordan Krage Date: Fri, 17 Feb 2023 06:59:45 -0600 Subject: [PATCH 03/15] core/cmd: handle loadOpts errs --- core/cmd/app.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/cmd/app.go b/core/cmd/app.go index de9de8fb2af..136d3f5762d 100644 --- a/core/cmd/app.go +++ b/core/cmd/app.go @@ -85,7 +85,9 @@ func NewApp(client *Client) *cli.App { var opts chainlink.GeneralConfigOpts fileNames := c.StringSlice("config") - loadOpts(&opts, fileNames...) + if err := loadOpts(&opts, fileNames...); err != nil { + return err + } secretsTOML := "" if c.IsSet("secrets") { From f3c101f4c1642d974c2a946160c7b24fc617a109 Mon Sep 17 00:00:00 2001 From: Jordan Krage Date: Fri, 17 Feb 2023 09:25:58 -0600 Subject: [PATCH 04/15] core/cmd: fix CL_CONFIG handling --- core/cmd/app.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/cmd/app.go b/core/cmd/app.go index 136d3f5762d..3e6b510987c 100644 --- a/core/cmd/app.go +++ b/core/cmd/app.go @@ -70,9 +70,9 @@ func NewApp(client *Client) *cli.App { Usage: "optional, applies only in client mode when making remote API calls. If turned on, SSL certificate verification will be disabled. This is mostly useful for people who want to use Chainlink with a self-signed TLS certificate", }, cli.StringSliceFlag{ - Name: "config, c", - Usage: "TOML configuration file(s) via flag, or raw TOML via env var. If used, legacy env vars must not be set. Multiple files can be used (-c configA.toml -c configB.toml), and they are applied in order with duplicated fields overriding any earlier values. If the env var is specified, it is always processed last with the effect of being the final override.", - EnvVar: "CL_CONFIG", + Name: "config, c", + Usage: "TOML configuration file(s) via flag, or raw TOML via env var. If used, legacy env vars must not be set. Multiple files can be used (-c configA.toml -c configB.toml), and they are applied in order with duplicated fields overriding any earlier values. If the env var is specified, it is always processed last with the effect of being the final override. [$CL_CONFIG]", + // Note: we cannot use the EnvVar field since it will combine with the flags. }, cli.StringFlag{ Name: "secrets, s", @@ -80,7 +80,7 @@ func NewApp(client *Client) *cli.App { }, } app.Before = func(c *cli.Context) error { - if c.IsSet("config") { + if c.IsSet("config") || v2.EnvConfig != "" { // TOML var opts chainlink.GeneralConfigOpts From 047798a87509d5ab5cb5c12a3e33ed6f61e3e523 Mon Sep 17 00:00:00 2001 From: Jordan Krage Date: Fri, 17 Feb 2023 09:51:18 -0600 Subject: [PATCH 05/15] Update core/cmd/app.go Co-authored-by: chainchad <96362174+chainchad@users.noreply.github.com> --- core/cmd/app.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/cmd/app.go b/core/cmd/app.go index 3e6b510987c..23c9688c769 100644 --- a/core/cmd/app.go +++ b/core/cmd/app.go @@ -71,7 +71,7 @@ func NewApp(client *Client) *cli.App { }, cli.StringSliceFlag{ Name: "config, c", - Usage: "TOML configuration file(s) via flag, or raw TOML via env var. If used, legacy env vars must not be set. Multiple files can be used (-c configA.toml -c configB.toml), and they are applied in order with duplicated fields overriding any earlier values. If the env var is specified, it is always processed last with the effect of being the final override. [$CL_CONFIG]", + Usage: "TOML configuration file(s) via flag, or raw TOML via env var. If used, legacy env vars must not be set. Multiple files can be used (-c configA.toml -c configB.toml), and they are applied in order with duplicated fields overriding any earlier values. If the `CL_CONFIG` env var is specified, it is always processed last with the effect of being the final override. [$CL_CONFIG]", // Note: we cannot use the EnvVar field since it will combine with the flags. }, cli.StringFlag{ From 199e89b427460e42d0b5d501d28233daea0d1697 Mon Sep 17 00:00:00 2001 From: Jordan Krage Date: Fri, 17 Feb 2023 15:09:38 -0600 Subject: [PATCH 06/15] core: fix config flag help text (cherry picked from commit b3669107f2494f7ed655b406f73932373609874c) --- core/cmd/app.go | 2 +- core/main_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/cmd/app.go b/core/cmd/app.go index 23c9688c769..f538c799547 100644 --- a/core/cmd/app.go +++ b/core/cmd/app.go @@ -71,7 +71,7 @@ func NewApp(client *Client) *cli.App { }, cli.StringSliceFlag{ Name: "config, c", - Usage: "TOML configuration file(s) via flag, or raw TOML via env var. If used, legacy env vars must not be set. Multiple files can be used (-c configA.toml -c configB.toml), and they are applied in order with duplicated fields overriding any earlier values. If the `CL_CONFIG` env var is specified, it is always processed last with the effect of being the final override. [$CL_CONFIG]", + Usage: "TOML configuration file(s) via flag, or raw TOML via env var. If used, legacy env vars must not be set. Multiple files can be used (-c configA.toml -c configB.toml), and they are applied in order with duplicated fields overriding any earlier values. If the 'CL_CONFIG' env var is specified, it is always processed last with the effect of being the final override. [$CL_CONFIG]", // Note: we cannot use the EnvVar field since it will combine with the flags. }, cli.StringFlag{ diff --git a/core/main_test.go b/core/main_test.go index 843549ff68f..e2627c3b7ab 100644 --- a/core/main_test.go +++ b/core/main_test.go @@ -76,7 +76,7 @@ func ExampleRun() { // --admin-credentials-file FILE optional, applies only in client mode when making remote API calls. If provided, FILE containing admin credentials will be used for logging in, allowing to avoid an additional login step. If `FILE` is missing, it will be ignored. Defaults to /apicredentials // --remote-node-url URL optional, applies only in client mode when making remote API calls. If provided, URL will be used as the remote Chainlink API endpoint (default: "http://localhost:6688") // --insecure-skip-verify optional, applies only in client mode when making remote API calls. If turned on, SSL certificate verification will be disabled. This is mostly useful for people who want to use Chainlink with a self-signed TLS certificate - // --config value, -c value TOML configuration file(s) via flag, or raw TOML via env var. If used, legacy env vars must not be set. Multiple files can be used (-c configA.toml -c configB.toml), and they are applied in order with duplicated fields overriding any earlier values. If the env var is specified, it is always processed last with the effect of being the final override. [$CL_CONFIG] + // --config value, -c value TOML configuration file(s) via flag, or raw TOML via env var. If used, legacy env vars must not be set. Multiple files can be used (-c configA.toml -c configB.toml), and they are applied in order with duplicated fields overriding any earlier values. If the 'CL_CONFIG' env var is specified, it is always processed last with the effect of being the final override. [$CL_CONFIG] // --secrets value, -s value TOML configuration file for secrets. Must be set if and only if config is set. // --help, -h show help // --version, -v print the version From 4b0728fbaa9993031c10b10c86a26f43995c17e2 Mon Sep 17 00:00:00 2001 From: Anirudh Warrier Date: Mon, 20 Feb 2023 13:32:57 +0530 Subject: [PATCH 07/15] Fix config v2 check (cherry picked from commit 10659349d90dedfd948ca76bc44419cabbca9c84) --- core/cmd/app.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/cmd/app.go b/core/cmd/app.go index f538c799547..98b6d1d9993 100644 --- a/core/cmd/app.go +++ b/core/cmd/app.go @@ -80,7 +80,7 @@ func NewApp(client *Client) *cli.App { }, } app.Before = func(c *cli.Context) error { - if c.IsSet("config") || v2.EnvConfig != "" { + if c.IsSet("config") || v2.EnvConfig.Get() != "" { // TOML var opts chainlink.GeneralConfigOpts From e7355c4d6802fa2516044729998f0d786e0472e3 Mon Sep 17 00:00:00 2001 From: Jordan Krage Date: Tue, 21 Feb 2023 08:13:51 -0500 Subject: [PATCH 08/15] core/services/chainlink: dump LogPoller and UICSAKeys Feature flags (#8508) (cherry picked from commit 3442b87490c892522e564369a43e1b7b1baef704) --- core/services/chainlink/cfgtest/dump/full-custom.toml | 2 ++ core/services/chainlink/config_dump.go | 2 ++ 2 files changed, 4 insertions(+) diff --git a/core/services/chainlink/cfgtest/dump/full-custom.toml b/core/services/chainlink/cfgtest/dump/full-custom.toml index 21bf0ca1cbd..02c7df71ea1 100644 --- a/core/services/chainlink/cfgtest/dump/full-custom.toml +++ b/core/services/chainlink/cfgtest/dump/full-custom.toml @@ -5,6 +5,8 @@ ShutdownGracePeriod = '10s' [Feature] FeedsManager = true +LogPoller = true +UICSAKeys = true [Database] DefaultIdleInTxSessionTimeout = '1h0m0s' diff --git a/core/services/chainlink/config_dump.go b/core/services/chainlink/config_dump.go index dcba28c4e34..610294fb223 100644 --- a/core/services/chainlink/config_dump.go +++ b/core/services/chainlink/config_dump.go @@ -520,6 +520,8 @@ func (c *Config) loadLegacyCoreEnv() error { c.Feature = config.Feature{ FeedsManager: envvar.NewBool("FeatureFeedsManager").ParsePtr(), + LogPoller: envvar.NewBool("FeatureLogPoller").ParsePtr(), + UICSAKeys: envvar.NewBool("FeatureUICSAKeys").ParsePtr(), } c.AuditLogger = audit.AuditLoggerConfig{ Enabled: envvar.NewBool("AuditLoggerEnabled").ParsePtr(), From bee9b1c46390ca8ff0498b439b1cdbcbe85f23ff Mon Sep 17 00:00:00 2001 From: chainchad <96362174+chainchad@users.noreply.github.com> Date: Tue, 28 Feb 2023 18:22:05 -0500 Subject: [PATCH 09/15] Bump version and update CHANGELOG for core v1.13.0 --- VERSION | 2 +- docs/CHANGELOG.md | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/VERSION b/VERSION index f8f4f03b3dc..feaae22bac7 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.12.1 +1.13.0 diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index f3f0cedc558..62c7f674196 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [dev] +... + +## 1.13.0 - UNRELEASED + ### Added - Support for sending OCR2 job specs to the feeds manager @@ -22,6 +26,8 @@ of any values provided via configuration files. - Terra is no longer supported + + ## 1.12.0 - 2023-02-15 ### Added From 387c81b63ffdb25f65a3793415dfdf8c2c189f36 Mon Sep 17 00:00:00 2001 From: Augustus <14297860+augustbleeds@users.noreply.github.com> Date: Wed, 1 Mar 2023 20:01:04 -0500 Subject: [PATCH 10/15] BCI-268: fix distinct query (#8606) Co-authored-by: Prashant Yadav <34992934+prashantkumar1982@users.noreply.github.com> (cherry picked from commit de3c326c701428f068046d5646cc0cc58506b6eb) --- core/chains/evm/txmgr/orm.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/chains/evm/txmgr/orm.go b/core/chains/evm/txmgr/orm.go index 10dd0cbafb3..627118f1c50 100644 --- a/core/chains/evm/txmgr/orm.go +++ b/core/chains/evm/txmgr/orm.go @@ -376,7 +376,7 @@ func (o *orm) FindEthTxAttemptsRequiringResend(olderThan time.Time, maxInFlightT // this select distinct works because of unique index on eth_txes // (evm_chain_id, from_address, nonce) err = o.q.Select(&attempts, ` -SELECT DISTINCT ON (nonce) eth_tx_attempts.* +SELECT DISTINCT ON (eth_txes.nonce) eth_tx_attempts.* FROM eth_tx_attempts JOIN eth_txes ON eth_txes.id = eth_tx_attempts.eth_tx_id AND eth_txes.state IN ('unconfirmed', 'confirmed_missing_receipt') WHERE eth_tx_attempts.state <> 'in_progress' AND eth_txes.broadcast_at <= $1 AND evm_chain_id = $2 AND from_address = $3 From 2713e22a6225cbb901ae76ee82c6e2e99d514a82 Mon Sep 17 00:00:00 2001 From: Chris C <104409744+vreff@users.noreply.github.com> Date: Wed, 8 Mar 2023 18:44:44 -0500 Subject: [PATCH 11/15] VRF-367 Fix use of getRoundRobinAddress in VRF listener (#8653) * VRF-367 Fix use of getRoundRobinAddress in VRF listener * Move round robin call to inside SQLX callback * Add empty slice check * Add comments * Revert "Move round robin call to inside SQLX callback" This reverts commit 8a215183680d3541d7a6d0b7f9fb5c52e1c3252f. * Move fromAddress validation to validate.go * Update job spec tests * Revert "Move fromAddress validation to validate.go" This reverts commit 28d0279467cf11acf127d9c6d1ab90609016088f. * Remove specs test changes * Fix nit * Fix integration tst * Revert "Revert "Move fromAddress validation to validate.go"" This reverts commit b8b2a2251028ef62f1b21dcb4160b4d030b884ba. * Adjust validation * Fix integration test model * Fix other integration test * Adjust integration test * Fix lint --- core/services/vrf/integration_v2_test.go | 2 ++ core/services/vrf/listener_v2.go | 29 +++++++--------- core/services/vrf/listener_v2_types.go | 10 +++++- core/services/vrf/validate.go | 6 ++++ core/services/vrf/validate_test.go | 36 ++++++++++++++++++++ core/testdata/testspecs/v2_specs.go | 1 + integration-tests/actions/vrfv2_helpers.go | 2 +- integration-tests/client/chainlink_models.go | 4 +-- integration-tests/smoke/vrfv2_test.go | 6 ++-- 9 files changed, 73 insertions(+), 23 deletions(-) diff --git a/core/services/vrf/integration_v2_test.go b/core/services/vrf/integration_v2_test.go index 6beb05564b1..62288d0e5d7 100644 --- a/core/services/vrf/integration_v2_test.go +++ b/core/services/vrf/integration_v2_test.go @@ -1989,9 +1989,11 @@ func TestMaliciousConsumer(t *testing.T) { s := testspecs.GenerateVRFSpec(testspecs.VRFSpecParams{ JobID: jid.String(), Name: "vrf-primary", + FromAddresses: []string{key.Address.String()}, CoordinatorAddress: uni.rootContractAddress.String(), BatchCoordinatorAddress: uni.batchCoordinatorContractAddress.String(), MinIncomingConfirmations: incomingConfs, + GasLanePrice: assets.GWei(1), PublicKey: vrfkey.PublicKey.String(), V2: true, }).Toml() diff --git a/core/services/vrf/listener_v2.go b/core/services/vrf/listener_v2.go index ced1552b8cd..1692e9e873b 100644 --- a/core/services/vrf/listener_v2.go +++ b/core/services/vrf/listener_v2.go @@ -570,13 +570,9 @@ func (lsn *listenerV2) processRequestsPerSubBatch( } } + // All fromAddresses passed to the VRFv2 job have the same KeySpecificMaxGasPriceWei value. fromAddresses := lsn.fromAddresses() - fromAddress, err := lsn.gethks.GetRoundRobinAddress(lsn.chainID, fromAddresses...) - if err != nil { - l.Errorw("Couldn't get next from address", "err", err) - continue - } - maxGasPriceWei := lsn.cfg.KeySpecificMaxGasPriceWei(fromAddress) + maxGasPriceWei := lsn.cfg.KeySpecificMaxGasPriceWei(fromAddresses[0]) // Cases: // 1. Never simulated: in this case, we want to observe the time until simulated @@ -593,7 +589,6 @@ func (lsn *listenerV2) processRequestsPerSubBatch( ll := l.With("reqID", p.req.req.RequestId.String(), "txHash", p.req.req.Raw.TxHash, "maxGasPrice", maxGasPriceWei.String(), - "fromAddress", fromAddress, "juelsNeeded", p.juelsNeeded.String(), "maxLink", p.maxLink.String(), "gasLimit", p.gasLimit, @@ -647,7 +642,7 @@ func (lsn *listenerV2) processRequestsPerSubBatch( var processedRequestIDs []string for _, batch := range batches.fulfillments { l.Debugw("Processing batch", "batchSize", len(batch.proofs)) - p := lsn.processBatch(l, subID, fromAddress, startBalanceNoReserveLink, batchMaxGas, batch) + p := lsn.processBatch(l, subID, startBalanceNoReserveLink, batchMaxGas, batch) processedRequestIDs = append(processedRequestIDs, p...) } @@ -752,22 +747,15 @@ func (lsn *listenerV2) processRequestsPerSub( } } + // All fromAddresses passed to the VRFv2 job have the same KeySpecificMaxGasPriceWei value. fromAddresses := lsn.fromAddresses() - fromAddress, err := lsn.gethks.GetRoundRobinAddress(lsn.chainID, fromAddresses...) - if err != nil { - l.Errorw("Couldn't get next from address", "err", err) - continue - } - maxGasPriceWei := lsn.cfg.KeySpecificMaxGasPriceWei(fromAddress) - + maxGasPriceWei := lsn.cfg.KeySpecificMaxGasPriceWei(fromAddresses[0]) observeRequestSimDuration(lsn.job.Name.ValueOrZero(), lsn.job.ExternalJobID, v2, unfulfilled) - pipelines := lsn.runPipelines(ctx, l, maxGasPriceWei, unfulfilled) for _, p := range pipelines { ll := l.With("reqID", p.req.req.RequestId.String(), "txHash", p.req.req.Raw.TxHash, "maxGasPrice", maxGasPriceWei.String(), - "fromAddress", fromAddress, "juelsNeeded", p.juelsNeeded.String(), "maxLink", p.maxLink.String(), "gasLimit", p.gasLimit, @@ -809,6 +797,13 @@ func (lsn *listenerV2) processRequestsPerSub( return processed } + fromAddress, err := lsn.gethks.GetRoundRobinAddress(lsn.chainID, fromAddresses...) + if err != nil { + l.Errorw("Couldn't get next from address", "err", err) + continue + } + ll = ll.With("fromAddress", fromAddress) + ll.Infow("Enqueuing fulfillment") var ethTX txmgr.EthTx err = lsn.q.Transaction(func(tx pg.Queryer) error { diff --git a/core/services/vrf/listener_v2_types.go b/core/services/vrf/listener_v2_types.go index 5afa8d71771..78e7334dd69 100644 --- a/core/services/vrf/listener_v2_types.go +++ b/core/services/vrf/listener_v2_types.go @@ -101,7 +101,6 @@ func (b *batchFulfillments) addRun(result vrfPipelineResult) { func (lsn *listenerV2) processBatch( l logger.Logger, subID uint64, - fromAddress common.Address, startBalanceNoReserveLink *big.Int, maxCallbackGasLimit uint32, batch *batchFulfillment, @@ -125,9 +124,18 @@ func (lsn *listenerV2) processBatch( maxCallbackGasLimit, float64(lsn.job.VRFSpec.BatchFulfillmentGasMultiplier), ) + + fromAddresses := lsn.fromAddresses() + fromAddress, err := lsn.gethks.GetRoundRobinAddress(lsn.chainID, fromAddresses...) + if err != nil { + l.Errorw("Couldn't get next from address", "err", err) + return + } + ll := l.With("numRequestsInBatch", len(batch.reqIDs), "requestIDs", batch.reqIDs, "batchSumGasLimit", batch.totalGasLimit, + "fromAddress", fromAddress, "linkBalance", startBalanceNoReserveLink, "totalGasLimitBumped", totalGasLimitBumped, "gasMultiplier", lsn.job.VRFSpec.BatchFulfillmentGasMultiplier, diff --git a/core/services/vrf/validate.go b/core/services/vrf/validate.go index ed8311c4456..8ea0a909b98 100644 --- a/core/services/vrf/validate.go +++ b/core/services/vrf/validate.go @@ -86,6 +86,12 @@ func ValidatedVRFSpec(tomlString string) (job.Job, error) { if t.Type() == pipeline.TaskTypeVRF || t.Type() == pipeline.TaskTypeVRFV2 { foundVRFTask = true } + + if t.Type() == pipeline.TaskTypeVRFV2 { + if len(spec.FromAddresses) == 0 { + return jb, errors.Wrap(ErrKeyNotSet, "fromAddreses needs to have a non-zero length") + } + } } if !foundVRFTask { return jb, errors.Wrapf(ErrKeyNotSet, "invalid pipeline, expected a vrf task") diff --git a/core/services/vrf/validate_test.go b/core/services/vrf/validate_test.go index 2c737bd6d8e..8c7241ce63d 100644 --- a/core/services/vrf/validate_test.go +++ b/core/services/vrf/validate_test.go @@ -92,6 +92,42 @@ decode_log->vrf->encode_tx->submit_tx require.True(t, errors.Is(ErrKeyNotSet, errors.Cause(err))) }, }, + { + name: "missing fromAddresses", + 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=vrfv2 + 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.Error(t, err) + require.True(t, errors.Is(ErrKeyNotSet, errors.Cause(err))) + }, + }, { name: "missing coordinator address", toml: ` diff --git a/core/testdata/testspecs/v2_specs.go b/core/testdata/testspecs/v2_specs.go index 3f591837ce8..6d377f9ab8e 100644 --- a/core/testdata/testspecs/v2_specs.go +++ b/core/testdata/testspecs/v2_specs.go @@ -264,6 +264,7 @@ func GenerateVRFSpec(params VRFSpecParams) VRFSpec { if params.BatchCoordinatorAddress != "" { batchCoordinatorAddress = params.BatchCoordinatorAddress } + batchFulfillmentGasMultiplier := 1.0 if params.BatchFulfillmentGasMultiplier >= 1.0 { batchFulfillmentGasMultiplier = params.BatchFulfillmentGasMultiplier diff --git a/integration-tests/actions/vrfv2_helpers.go b/integration-tests/actions/vrfv2_helpers.go index 9dc705f0554..18a19978051 100644 --- a/integration-tests/actions/vrfv2_helpers.go +++ b/integration-tests/actions/vrfv2_helpers.go @@ -69,7 +69,7 @@ func CreateVRFV2Jobs( job, err := n.MustCreateJob(&client.VRFV2JobSpec{ Name: fmt.Sprintf("vrf-%s", jobUUID), CoordinatorAddress: coordinator.Address(), - FromAddress: oracleAddr, + FromAddresses: []string{oracleAddr}, EVMChainID: c.GetChainID().String(), MinIncomingConfirmations: minIncomingConfirmations, PublicKey: pubKeyCompressed, diff --git a/integration-tests/client/chainlink_models.go b/integration-tests/client/chainlink_models.go index f56c69cce00..57511c14e2a 100644 --- a/integration-tests/client/chainlink_models.go +++ b/integration-tests/client/chainlink_models.go @@ -1024,7 +1024,7 @@ type VRFV2JobSpec struct { ExternalJobID string `toml:"externalJobID"` ObservationSource string `toml:"observationSource"` // List of commands for the Chainlink node MinIncomingConfirmations int `toml:"minIncomingConfirmations"` - FromAddress string `toml:"fromAddress"` + FromAddresses []string `toml:"fromAddresses"` EVMChainID string `toml:"evmChainID"` BatchFulfillmentEnabled bool `toml:"batchFulfillmentEnabled"` BackOffInitialDelay time.Duration `toml:"backOffInitialDelay"` @@ -1041,7 +1041,7 @@ type = "vrf" schemaVersion = 1 name = "{{.Name}}" coordinatorAddress = "{{.CoordinatorAddress}}" -fromAddress = "{{.FromAddress}}" +fromAddresses = [{{range .FromAddresses}}"{{.}}",{{end}}] evmChainID = "{{.EVMChainID}}" minIncomingConfirmations = {{.MinIncomingConfirmations}} publicKey = "{{.PublicKey}}" diff --git a/integration-tests/smoke/vrfv2_test.go b/integration-tests/smoke/vrfv2_test.go index dc947e99fed..23eac87aa36 100644 --- a/integration-tests/smoke/vrfv2_test.go +++ b/integration-tests/smoke/vrfv2_test.go @@ -8,15 +8,17 @@ import ( "testing" "time" + "github.com/rs/zerolog" "github.com/smartcontractkit/chainlink-testing-framework/utils" "go.uber.org/zap/zapcore" "github.com/onsi/gomega" + "github.com/stretchr/testify/require" + "github.com/smartcontractkit/chainlink-env/environment" "github.com/smartcontractkit/chainlink-env/pkg/helm/chainlink" "github.com/smartcontractkit/chainlink-testing-framework/blockchain" "github.com/smartcontractkit/chainlink-testing-framework/contracts/ethereum" - "github.com/stretchr/testify/require" eth "github.com/smartcontractkit/chainlink-env/pkg/helm/ethereum" @@ -117,7 +119,7 @@ func TestVRFv2Basic(t *testing.T) { job, err = n.MustCreateJob(&client.VRFV2JobSpec{ Name: fmt.Sprintf("vrf-%s", jobUUID), CoordinatorAddress: coordinator.Address(), - FromAddress: oracleAddr, + FromAddresses: []string{oracleAddr}, EVMChainID: fmt.Sprint(chainClient.GetNetworkConfig().ChainID), MinIncomingConfirmations: minimumConfirmations, PublicKey: pubKeyCompressed, From db002182a2cb386eb8d1b182c6bd59940b5b4577 Mon Sep 17 00:00:00 2001 From: Makram Date: Thu, 9 Mar 2023 12:55:18 +0200 Subject: [PATCH 12/15] Fix integration test build (#8662) --- integration-tests/smoke/vrfv2_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/integration-tests/smoke/vrfv2_test.go b/integration-tests/smoke/vrfv2_test.go index 23eac87aa36..c8e505696f0 100644 --- a/integration-tests/smoke/vrfv2_test.go +++ b/integration-tests/smoke/vrfv2_test.go @@ -8,7 +8,6 @@ import ( "testing" "time" - "github.com/rs/zerolog" "github.com/smartcontractkit/chainlink-testing-framework/utils" "go.uber.org/zap/zapcore" From c6505d38c0de00f09d7b25e17290e0ca5145058b Mon Sep 17 00:00:00 2001 From: James Kong Date: Tue, 14 Mar 2023 22:43:30 +0800 Subject: [PATCH 13/15] Fixes new job proposals not storing the name when received (#8699) (#8702) Job proposals were not deserializing the TOML data correctly, leading the job proposal name to be set to null. This fixes the issue by correctly using a pointer to the target struct, so that the value can be returned. --- core/services/feeds/service.go | 2 +- core/services/feeds/service_test.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/core/services/feeds/service.go b/core/services/feeds/service.go index 6a1e6ee93e5..89f427a4d5d 100644 --- a/core/services/feeds/service.go +++ b/core/services/feeds/service.go @@ -1143,7 +1143,7 @@ func extractName(defn string) null.String { Name null.String }{} - if err := toml.Unmarshal([]byte(defn), spec); err != nil { + if err := toml.Unmarshal([]byte(defn), &spec); err != nil { return null.StringFromPtr(nil) } diff --git a/core/services/feeds/service_test.go b/core/services/feeds/service_test.go index 2424fdee799..b9f50e5380f 100644 --- a/core/services/feeds/service_test.go +++ b/core/services/feeds/service_test.go @@ -495,6 +495,7 @@ func Test_Service_ProposeJob(t *testing.T) { } jp = feeds.JobProposal{ FeedsManagerID: 1, + Name: null.StringFrom("example flux monitor spec"), RemoteUUID: remoteUUID, Status: feeds.JobProposalStatusPending, } From 6bb65342a855a06c731b495fdee7df1dcadaec13 Mon Sep 17 00:00:00 2001 From: Jordan Krage Date: Wed, 15 Mar 2023 17:50:05 -0500 Subject: [PATCH 14/15] core/cmd: only run fallback API initializer when file not present (#8718) (cherry picked from commit 5622de9fec4d459d5c2a8fc2fddd9c6d78119404) --- core/cmd/local_client.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/core/cmd/local_client.go b/core/cmd/local_client.go index 814afbe1ee7..e5887b903e5 100644 --- a/core/cmd/local_client.go +++ b/core/cmd/local_client.go @@ -377,14 +377,16 @@ func (cli *Client) runNode(c *clipkg.Context) error { } var user sessions.User - if _, err = NewFileAPIInitializer(c.String("api")).Initialize(sessionORM, lggr); err != nil && !errors.Is(err, ErrNoCredentialFile) { - return errors.Wrap(err, "error creating api initializer") - } - if user, err = cli.FallbackAPIInitializer.Initialize(sessionORM, lggr); err != nil { - if errors.Is(err, ErrorNoAPICredentialsAvailable) { - return errors.WithStack(err) + if user, err = NewFileAPIInitializer(c.String("api")).Initialize(sessionORM, lggr); err != nil { + if !errors.Is(err, ErrNoCredentialFile) { + return errors.Wrap(err, "error creating api initializer") + } + if user, err = cli.FallbackAPIInitializer.Initialize(sessionORM, lggr); err != nil { + if errors.Is(err, ErrorNoAPICredentialsAvailable) { + return errors.WithStack(err) + } + return errors.Wrap(err, "error creating fallback initializer") } - return errors.Wrap(err, "error creating fallback initializer") } lggr.Info("API exposed for user ", user.Email) From 006716e0282fc35836384f46ea94b6542b46e11b Mon Sep 17 00:00:00 2001 From: chainchad <96362174+chainchad@users.noreply.github.com> Date: Thu, 16 Mar 2023 14:37:42 -0400 Subject: [PATCH 15/15] Finalize date on changelog for 1.13.0 --- docs/CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 41b006629df..33111cb01e3 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -10,7 +10,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ... -## 1.13.0 - UNRELEASED + + +## 1.13.0 - 2023-03-16 ### Added @@ -30,8 +32,6 @@ of any values provided via configuration files. - Terra is no longer supported - - ## 1.12.0 - 2023-02-15 ### Added