Skip to content

Commit

Permalink
BCF-2637 Add evm chain id removal migrations (#10669)
Browse files Browse the repository at this point in the history
* Add evm chain id removal migrations

* Remove _test db url check for 0195 evm chain id migration

* Improve 0195 migration, improve error msg and sql string constant names

* Update 0195 migration to only add/remove evmChainID not nil constraints

* Remove dangling err handling (leftover from evmChainID removal)

* Update 0195 migration naming

* Add helper function for not null evmChainID migration

* Update changelog for evm chain id not null migration helper function

* Update error message for sql open in shell MigrateDatabase

* Fix job_orm_test test that set evm_chain_id to null

* Fix evmChainID migration helper err when db is init for the first time

* Move goose migration ver check for evmChainID migration in helper func
  • Loading branch information
ilija42 authored Sep 18, 2023
1 parent c8762ee commit eba9a27
Show file tree
Hide file tree
Showing 6 changed files with 131 additions and 104 deletions.
44 changes: 44 additions & 0 deletions core/cmd/shell.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,12 @@ type ChainlinkAppFactory struct{}
func (n ChainlinkAppFactory) NewApplication(ctx context.Context, cfg chainlink.GeneralConfig, appLggr logger.Logger, db *sqlx.DB) (app chainlink.Application, err error) {
initGlobals(cfg.Prometheus())

// TODO TO BE REMOVED IN v2.7.0
err = evmChainIDMigration(cfg, db.DB, appLggr)
if err != nil {
return nil, err
}

err = handleNodeVersioning(db, appLggr, cfg.RootDir(), cfg.Database(), cfg.WebServer().HTTPPort())
if err != nil {
return nil, err
Expand Down Expand Up @@ -295,6 +301,44 @@ func takeBackupIfVersionUpgrade(dbUrl url.URL, rootDir string, cfg periodicbacku
return err
}

// evmChainIDMigration TODO TO BE REMOVED IN v2.7.0. This is a helper function for evmChainID 0195 migration in v2.6.0 only, so that we don't have to inject evmChainID into goose.
func evmChainIDMigration(generalConfig chainlink.GeneralConfig, db *sql.DB, lggr logger.Logger) error {
migrationVer, err := migrate.Current(db, lggr)
if err != nil {
return err
}
if migrationVer != 194 {
return nil
}

if generalConfig.EVMEnabled() {
if generalConfig.EVMConfigs() == nil {
return errors.New("evm configs are missing")
}
if generalConfig.EVMConfigs()[0] == nil {
return errors.New("evm config is nil")
}
updateQueries := []string{
`UPDATE direct_request_specs SET evm_chain_id = $1 WHERE evm_chain_id IS NULL;`,
`UPDATE flux_monitor_specs SET evm_chain_id = $1 WHERE evm_chain_id IS NULL;`,
`UPDATE ocr_oracle_specs SET evm_chain_id = $1 WHERE evm_chain_id IS NULL;`,
`UPDATE keeper_specs SET evm_chain_id = $1 WHERE evm_chain_id IS NULL;`,
`UPDATE vrf_specs SET evm_chain_id = $1 WHERE evm_chain_id IS NULL;`,
`UPDATE blockhash_store_specs SET evm_chain_id = $1 WHERE evm_chain_id IS NULL;`,
`UPDATE block_header_feeder_specs SET evm_chain_id = $1 WHERE evm_chain_id IS NULL;`,
}

chainID := generalConfig.EVMConfigs()[0].ChainID.String()
for i := range updateQueries {
_, err := db.Exec(updateQueries[i], chainID)
if err != nil {
return errors.Wrap(err, "failed to set missing evm chain ids")
}
}
}
return nil
}

// Runner implements the Run method.
type Runner interface {
Run(context.Context, chainlink.Application) error
Expand Down
21 changes: 18 additions & 3 deletions core/cmd/shell_local.go
Original file line number Diff line number Diff line change
Expand Up @@ -376,9 +376,6 @@ func (s *Shell) runNode(c *cli.Context) error {
legacyEVMChains := app.GetRelayers().LegacyEVMChains()

if s.Config.EVMEnabled() {
if err != nil {
return errors.Wrap(err, "error migrating keystore")
}
chainList, err := legacyEVMChains.List()
if err != nil {
return fmt.Errorf("error listing legacy evm chains: %w", err)
Expand Down Expand Up @@ -843,6 +840,23 @@ func (s *Shell) MigrateDatabase(_ *cli.Context) error {
return s.errorOut(errDBURLMissing)
}

// TODO TO BE REMOVED IN v2.7.0
db, err := sql.Open(string(dialects.Postgres), parsed.String())
if err != nil {
return fmt.Errorf("unable to open postgres database for evmChainID helper migration: %+v", err)
}
defer func() {
if cerr := db.Close(); cerr != nil {
err = multierr.Append(err, cerr)
}
}()

// TODO TO BE REMOVED IN v2.7.0
err = evmChainIDMigration(s.Config, db, s.Logger)
if err != nil {
return err
}

s.Logger.Infof("Migrating database: %#v", parsed.String())
if err := migrateDB(cfg, s.Logger); err != nil {
return s.errorOut(err)
Expand Down Expand Up @@ -1026,6 +1040,7 @@ func migrateDB(config dbConfig, lggr logger.Logger) error {
if err != nil {
return fmt.Errorf("failed to initialize orm: %v", err)
}

if err = migrate.Migrate(db.DB, lggr); err != nil {
return fmt.Errorf("migrateDB failed: %v", err)
}
Expand Down
4 changes: 2 additions & 2 deletions core/internal/cltest/factories.go
Original file line number Diff line number Diff line change
Expand Up @@ -624,8 +624,8 @@ func MustInsertOffchainreportingOracleSpec(t *testing.T, db *sqlx.DB, transmitte

ocrKeyID := models.MustSha256HashFromHex(DefaultOCRKeyBundleID)
spec := job.OCROracleSpec{}
require.NoError(t, db.Get(&spec, `INSERT INTO ocr_oracle_specs (created_at, updated_at, contract_address, p2p_bootstrap_peers, is_bootstrap_peer, encrypted_ocr_key_bundle_id, transmitter_address, observation_timeout, blockchain_timeout, contract_config_tracker_subscribe_interval, contract_config_tracker_poll_interval, contract_config_confirmations, database_timeout, observation_grace_period, contract_transmitter_transmit_timeout) VALUES (
NOW(),NOW(),$1,'{}',false,$2,$3,0,0,0,0,0,0,0,0
require.NoError(t, db.Get(&spec, `INSERT INTO ocr_oracle_specs (created_at, updated_at, contract_address, p2p_bootstrap_peers, is_bootstrap_peer, encrypted_ocr_key_bundle_id, transmitter_address, observation_timeout, blockchain_timeout, contract_config_tracker_subscribe_interval, contract_config_tracker_poll_interval, contract_config_confirmations, database_timeout, observation_grace_period, contract_transmitter_transmit_timeout, evm_chain_id) VALUES (
NOW(),NOW(),$1,'{}',false,$2,$3,0,0,0,0,0,0,0,0,0
) RETURNING *`, NewEIP55Address(), &ocrKeyID, &transmitterAddress))
return spec
}
Expand Down
113 changes: 14 additions & 99 deletions core/services/job/job_orm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -721,87 +721,42 @@ func TestORM_CreateJob_OCR_DuplicatedContractAddress(t *testing.T) {
_, bridge := cltest.MustCreateBridge(t, db, cltest.BridgeOpts{}, config.Database())
_, bridge2 := cltest.MustCreateBridge(t, db, cltest.BridgeOpts{}, config.Database())

spec := testspecs.GenerateOCRSpec(testspecs.OCRSpecParams{
Name: "job1",
EVMChainID: testutils.FixtureChainID.String(),
DS1BridgeName: bridge.Name.String(),
DS2BridgeName: bridge2.Name.String(),
TransmitterAddress: address.Hex(),
})

jb, err := ocr.ValidatedOracleSpecToml(legacyChains, spec.Toml())
require.NoError(t, err)

// Default Chain Job
externalJobID := uuid.NullUUID{UUID: uuid.New(), Valid: true}
spec2 := testspecs.GenerateOCRSpec(testspecs.OCRSpecParams{
Name: "job2",
EVMChainID: defaultChainID.String(),
DS1BridgeName: bridge.Name.String(),
DS2BridgeName: bridge2.Name.String(),
TransmitterAddress: address.Hex(),
JobID: externalJobID.UUID.String(),
})
jb2, err := ocr.ValidatedOracleSpecToml(legacyChains, spec2.Toml())
require.NoError(t, err)

// Custom Chain Job
externalJobID = uuid.NullUUID{UUID: uuid.New(), Valid: true}
spec3 := testspecs.GenerateOCRSpec(testspecs.OCRSpecParams{
externalJobID := uuid.NullUUID{UUID: uuid.New(), Valid: true}
spec := testspecs.GenerateOCRSpec(testspecs.OCRSpecParams{
Name: "job3",
EVMChainID: customChainID.String(),
DS1BridgeName: bridge.Name.String(),
DS2BridgeName: bridge2.Name.String(),
TransmitterAddress: address.Hex(),
JobID: externalJobID.UUID.String(),
})
jb3, err := ocr.ValidatedOracleSpecToml(legacyChains, spec3.Toml())
jb, err := ocr.ValidatedOracleSpecToml(legacyChains, spec.Toml())
require.NoError(t, err)

t.Run("with legacy NULL chain id", func(t *testing.T) {
err = jobORM.CreateJob(&jb)
require.NoError(t, err)
_, err := db.ExecContext(testutils.Context(t),
"UPDATE ocr_oracle_specs o SET evm_chain_id=NULL FROM jobs j WHERE o.id = j.ocr_oracle_spec_id AND j.id=$1", jb.ID)
require.NoError(t, err)

cltest.AssertCount(t, db, "ocr_oracle_specs", 1)
cltest.AssertCount(t, db, "jobs", 1)

err = jobORM.CreateJob(&jb2) // try adding job for same contract with default chain id
require.Error(t, err)
assert.Equal(t, fmt.Sprintf("CreateJobFailed: a job with contract address %s already exists for chain ID %d", jb2.OCROracleSpec.ContractAddress, jb2.OCROracleSpec.EVMChainID.ToInt()), err.Error())

err = jobORM.CreateJob(&jb3) // Try adding job with custom chain id
require.Error(t, err)
assert.Equal(t, fmt.Sprintf("CreateJobFailed: a job with contract address %s already exists for chain ID %d", jb3.OCROracleSpec.ContractAddress, jb3.OCROracleSpec.EVMChainID.ToInt()), err.Error())
})

require.NoError(t, jobORM.DeleteJob(jb.ID))

t.Run("with a set chain id", func(t *testing.T) {
err = jobORM.CreateJob(&jb3) // Add job with custom chain id
err = jobORM.CreateJob(&jb) // Add job with custom chain id
require.NoError(t, err)

cltest.AssertCount(t, db, "ocr_oracle_specs", 1)
cltest.AssertCount(t, db, "jobs", 1)

externalJobID = uuid.NullUUID{UUID: uuid.New(), Valid: true}
spec3.JobID = externalJobID.UUID.String()
jb3a, err := ocr.ValidatedOracleSpecToml(legacyChains, spec3.Toml())
spec.JobID = externalJobID.UUID.String()
jba, err := ocr.ValidatedOracleSpecToml(legacyChains, spec.Toml())
require.NoError(t, err)
err = jobORM.CreateJob(&jb3a) // Try to add duplicate job with default id
err = jobORM.CreateJob(&jba) // Try to add duplicate job with default id
require.Error(t, err)
assert.Equal(t, fmt.Sprintf("CreateJobFailed: a job with contract address %s already exists for chain ID %s", jb3.OCROracleSpec.ContractAddress, defaultChainID.String()), err.Error())
assert.Equal(t, fmt.Sprintf("CreateJobFailed: a job with contract address %s already exists for chain ID %s", jb.OCROracleSpec.ContractAddress, defaultChainID.String()), err.Error())

externalJobID = uuid.NullUUID{UUID: uuid.New(), Valid: true}
spec3.JobID = externalJobID.UUID.String()
jb4, err := ocr.ValidatedOracleSpecToml(legacyChains, spec3.Toml())
spec.JobID = externalJobID.UUID.String()
jb2, err := ocr.ValidatedOracleSpecToml(legacyChains, spec.Toml())
require.NoError(t, err)

err = jobORM.CreateJob(&jb4) // Try to add duplicate job with custom id
err = jobORM.CreateJob(&jb2) // Try to add duplicate job with custom id
require.Error(t, err)
assert.Equal(t, fmt.Sprintf("CreateJobFailed: a job with contract address %s already exists for chain ID %s", jb4.OCROracleSpec.ContractAddress, customChainID), err.Error())
assert.Equal(t, fmt.Sprintf("CreateJobFailed: a job with contract address %s already exists for chain ID %s", jb2.OCROracleSpec.ContractAddress, customChainID), err.Error())
})
}

Expand Down Expand Up @@ -1060,9 +1015,8 @@ func Test_FindJobs(t *testing.T) {
func Test_FindJob(t *testing.T) {
t.Parallel()

// Create a config with multiple EVM chains. The test fixtures already load a 1337 and the
// default EVM chain ID. Additional chains will need additional fixture statements to add
// a chain to evm_chains.
// Create a config with multiple EVM chains. The test fixtures already load 1337
// Additional chains will need additional fixture statements to add a chain to evm_chains.
config := configtest.NewGeneralConfig(t, func(c *chainlink.Config, s *chainlink.Secrets) {
chainID := utils.NewBigI(1337)
enabled := true
Expand Down Expand Up @@ -1146,28 +1100,12 @@ func Test_FindJob(t *testing.T) {
jobOCR2WithFeedID2.Name = null.StringFrom("new name")
require.NoError(t, err)

// Create a job with the legacy null evm chain id.
jobWithNullChain, err := ocr.ValidatedOracleSpecToml(legacyChains,
testspecs.GenerateOCRSpec(testspecs.OCRSpecParams{
JobID: uuid.New().String(),
ContractAddress: "0xB47f9a6D281B2A82F8692F8dE058E4249363A6fc",
TransmitterAddress: address.Hex(),
Name: "ocr legacy null chain id",
DS1BridgeName: bridge.Name.String(),
DS2BridgeName: bridge2.Name.String(),
}).Toml(),
)
require.NoError(t, err)

err = orm.CreateJob(&job)
require.NoError(t, err)

err = orm.CreateJob(&jobSameAddress)
require.NoError(t, err)

err = orm.CreateJob(&jobWithNullChain)
require.NoError(t, err)

err = orm.CreateJob(&jobOCR2)
require.NoError(t, err)

Expand All @@ -1178,11 +1116,6 @@ func Test_FindJob(t *testing.T) {
err = orm.CreateJob(&jobOCR2WithFeedID2)
require.NoError(t, err)

// Set the ChainID to null manually since we can't do this in the test helper
_, err = db.ExecContext(testutils.Context(t),
"UPDATE ocr_oracle_specs o SET evm_chain_id=NULL FROM jobs j WHERE o.id = j.ocr_oracle_spec_id AND j.id=$1", jobWithNullChain.ID)
require.NoError(t, err)

t.Run("by id", func(t *testing.T) {
ctx, cancel := context.WithTimeout(testutils.Context(t), 5*time.Second)
defer cancel()
Expand Down Expand Up @@ -1222,24 +1155,6 @@ func Test_FindJob(t *testing.T) {
require.ErrorIs(t, err, sql.ErrNoRows)
})

t.Run("by address with legacy null evm chain id", func(t *testing.T) {
jbID, err := orm.FindJobIDByAddress(
jobWithNullChain.OCROracleSpec.ContractAddress,
jobWithNullChain.OCROracleSpec.EVMChainID,
)
require.NoError(t, err)

assert.Equal(t, jobWithNullChain.ID, jbID)

jbID, err = orm.FindJobIDByAddress(
jobWithNullChain.OCROracleSpec.ContractAddress,
utils.NewBig(nil),
)
require.NoError(t, err)

assert.Equal(t, jobWithNullChain.ID, jbID)
})

t.Run("by address yet chain scoped", func(t *testing.T) {
commonAddr := jobSameAddress.OCROracleSpec.ContractAddress

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package migrations

import (
"context"
"database/sql"

"github.com/pkg/errors"
"github.com/pressly/goose/v3"
)

func init() {
goose.AddMigrationContext(Up195, Down195)
}

const (
addNullConstraintsToSpecs = `
ALTER TABLE direct_request_specs ALTER COLUMN evm_chain_id SET NOT NULL;
ALTER TABLE flux_monitor_specs ALTER COLUMN evm_chain_id SET NOT NULL;
ALTER TABLE ocr_oracle_specs ALTER COLUMN evm_chain_id SET NOT NULL;
ALTER TABLE keeper_specs ALTER COLUMN evm_chain_id SET NOT NULL;
ALTER TABLE vrf_specs ALTER COLUMN evm_chain_id SET NOT NULL;
ALTER TABLE blockhash_store_specs ALTER COLUMN evm_chain_id SET NOT NULL;
ALTER TABLE block_header_feeder_specs ALTER COLUMN evm_chain_id SET NOT NULL;
`

dropNullConstraintsFromSpecs = `
ALTER TABLE direct_request_specs ALTER COLUMN evm_chain_id DROP NOT NULL;
ALTER TABLE flux_monitor_specs ALTER COLUMN evm_chain_id DROP NOT NULL;
ALTER TABLE ocr_oracle_specs ALTER COLUMN evm_chain_id DROP NOT NULL;
ALTER TABLE keeper_specs ALTER COLUMN evm_chain_id DROP NOT NULL;
ALTER TABLE vrf_specs ALTER COLUMN evm_chain_id DROP NOT NULL;
ALTER TABLE blockhash_store_specs ALTER COLUMN evm_chain_id DROP NOT NULL;
ALTER TABLE block_header_feeder_specs ALTER COLUMN evm_chain_id DROP NOT NULL;
`
)

// nolint
func Up195(ctx context.Context, tx *sql.Tx) error {
_, err := tx.ExecContext(ctx, addNullConstraintsToSpecs)
return errors.Wrap(err, "failed to add null constraints")
}

// nolint
func Down195(ctx context.Context, tx *sql.Tx) error {
if _, err := tx.ExecContext(ctx, dropNullConstraintsFromSpecs); err != nil {
return err
}
return nil
}
4 changes: 4 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [dev]

### Added

- Temporary helper function for proper migration of evmChainID not null in specs, that fills in missing chainIDs. This needs to be removed after one version after everyone migrated properly. For proper migrations all nodes must upgrade sequentially without skipping this version.

### Removed

- Removed support for sending telemetry to the deprecated Explorer service. All nodes will have to remove `Explorer` related keys from TOML configuration and env vars.
Expand Down

0 comments on commit eba9a27

Please sign in to comment.