feat(evm): standalone migration e2e - #1309
Closed
tt-cll wants to merge 15 commits into
Closed
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds an end-to-end “CL mode → standalone” migration path for EVM node operators, aiming to preserve existing on-chain/off-chain identities (OCR2 onchain signing key + funded transmitter) while cutting over runtime processes and Job Distributor (JD) records without manual config conversion.
Changes:
- Extend the EVM accessor config loader to accept a Chainlink node’s TOML (
[[EVM]],[[EVM.Nodes]]) and convert it to standalone format with operator-visible warnings. - Add bootstrap support for importing a Chainlink-exported key via a new
[key_import]config section (format auto-detected, optional identity pinning). - Introduce devenv migration orchestration + an E2E smoke test, plus operator docs and a changelog entry.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| integration/pkg/accessors/evm/factory_constructor.go | Load EVM config from either standalone TOML or Chainlink node TOML; log conversion warnings. |
| integration/pkg/accessors/evm/factory_constructor_test.go | Update unit tests for new loadConfig return signature. |
| integration/pkg/accessors/evm/clnode_config.go | Implement conversion from Chainlink node EVM config ([[EVM]]) to standalone chains.<selector>. |
| integration/pkg/accessors/evm/clnode_config_test.go | Add unit tests validating conversion correctness and strict reload behavior. |
| docs/migration/cl-to-standalone.md | Add operator runbook for CL-to-standalone cutover (EVM-focused). |
| changelog/2026-07-29_cl_to_standalone_key_migration.md | Document new migration capabilities and rationale. |
| build/devenv/tests/e2e/smoke_migration_test.go | Add E2E smoke test that performs the cutover mid-test and asserts identities remain unchanged. |
| build/devenv/services/keyimports.go | Add helper to wire host export files into container + generate [key_import] config fragment. |
| build/devenv/services/keyimports_test.go | Add tests ensuring config paths match mounted files and required inputs are validated. |
| build/devenv/services/executor/base.go | Mount key import files into executor container during migration. |
| build/devenv/services/committeeverifier/base.go | Mount key import files into verifier container during migration. |
| build/devenv/services/bootstrap.go | Extend devenv bootstrap input to include key import config + mounted files. |
| build/devenv/migration/jobspecs.go | Implement retargeting of CL-mode job-spec envelopes to standalone appConfig. |
| build/devenv/migration/jobspecs_test.go | Add tests for retarget correctness, idempotence, and parser round-tripping. |
| build/devenv/migration/identity.go | Add JD node lookup by CSA key, identity adoption (UpdateNode), and container-stop helper. |
| build/devenv/migration/export.go | Export OCR2 + ETH keys from a running CL node and validate exported identities. |
| build/devenv/migration/cutover.go | Orchestrate cutover steps: export, stop CL, launch standalone, adopt JD identity, relaunch executors, mutate topology. |
| build/devenv/migration.clnode.profile | Add devenv profile for local migration testing. |
| build/devenv/migration.clnode.ci.profile | Add CI profile for migration testing using pre-built CL node image. |
| build/devenv/env-cl-migration.toml | Define a CL-mode topology sized for migration (one committee/aggregator) to match standalone constraints. |
| bootstrap/README.md | Document new [key_import] bootstrap config section and migration rationale. |
| bootstrap/keys/import.go | Implement key export format detection + import into keystore, with optional identity pinning. |
| bootstrap/keys/import_test.go | Add fixtures and tests for OCR2/ETH export decoding, import behavior, and expected_id validation. |
| bootstrap/config.go | Add [key_import] config struct, validation, and conversion into keys import spec. |
| bootstrap/bootstrap.go | Wire key import into keystore initialization and add logic to resolve which declared key is importable. |
| .github/workflows/test-cl-smoke.yaml | Add CI job entry to run the new migration E2E smoke test under CL-mode smoke workflow. |
Comments suppressed due to low confidence (1)
integration/pkg/accessors/evm/factory_constructor.go:149
- convertedFromNodeConfig is derived from len(warnings)>0, but converting a node config can legitimately produce zero warnings. This log field will incorrectly report false for a successful conversion with no dropped settings.
lggr.Infow("loaded EVM config", "numChains", len(infos), "convertedFromNodeConfig", len(warnings) > 0)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+43
to
+49
| if isNodeConfig { | ||
| conversion, cerr := convertChainlinkNodeConfig(data) | ||
| if cerr != nil { | ||
| return nil, nil, fmt.Errorf("failed to convert Chainlink node config %s: %w", path, cerr) | ||
| } | ||
| return &conversion.Config, conversion.Warnings, nil | ||
| } |
Comment on lines
+65
to
73
| func isChainlinkNodeConfig(data []byte) (bool, error) { | ||
| var probe struct { | ||
| EVM []toml.Primitive `toml:"EVM"` | ||
| } | ||
| if _, err := toml.Decode(string(data), &probe); err != nil { | ||
| return false, err | ||
| } | ||
| return len(probe.EVM) > 0, nil | ||
| } |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 26 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
bootstrap/keys/import.go:156
- Format is often omitted for auto-detection, so using
%shere can yieldfailed to decode import .... Prefer%qso the message is still readable when Format is empty.
return fmt.Errorf("failed to decode %s import for key %q from %q: %w", spec.Format, keyName, spec.Path, err)
bootstrap/keys/import.go:163
- Same as above:
%swith an empty, auto-detected Format can produce a confusing message. Using%qkeeps the error actionable whether Format was explicitly set or auto-detected.
return fmt.Errorf(
"%s import for key %q from %q holds identity %s but expected_id is %s: the wrong node's export is mounted",
spec.Format, keyName, spec.Path, id, want,
)
bootstrap/keys/import.go:198
- InspectImport has the same
%s/empty Format issue; switching to%qavoids an empty placeholder when Format is auto-detected.
return "", fmt.Errorf("failed to decode %s export %q: %w", spec.Format, spec.Path, err)
bootstrap/keys/import.go:140
- This error message uses
%sfor spec.Format, but Format is commonly left empty to auto-detect from the file, which produces an unhelpful blank in the message. Using%qmakes the empty/explicit case unambiguous.
This issue also appears in the following locations of the same file:
- line 156
- line 160
- line 198
return fmt.Errorf(
"key %q is declared as %s but %s imports carry a secp256k1 key and can only populate %s",
keyName, keyType, spec.Format, keystore.ECDSA_S256,
)
}
integration/pkg/accessors/evm/factory_constructor.go:32
- The comment says configs with neither
chainsnorEVMare rejected, but the loader only rejects unknown fields; an empty/irrelevant TOML can still decode to an empty Config. Either enforce the check in code or adjust the comment to match the current behavior.
// The two formats are told apart by their top-level table: `chains` is the standalone format,
// `EVM` is a node config. Anything with neither is rejected by the strict decode below.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 28 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
docs/migration/cl-to-standalone.md:113
- This section says "Two things" are dropped, but it lists three items, and the converter also drops IsLoadBalancedRPC (with a warning). The runbook should match the actual conversion behavior so operators aren’t surprised by an extra warning or a silently ignored setting.
Two things a Chainlink node can express have no standalone equivalent and are dropped: send-only
nodes, and per-node `Order` and `HTTPURLExtraWrite`. Each one is logged at startup saying exactly
what was dropped. If you rely on a send-only endpoint, add it as a full node.
changelog/2026-07-29_cl_to_standalone_key_migration.md:51
- The changelog lists Order and HTTPURLExtraWrite as dropped, but the conversion also drops IsLoadBalancedRPC (see integration/pkg/accessors/evm/clnode_config.go). Mentioning it here keeps the operator-facing behavior aligned with the implementation.
The node's chain defaults are applied before finality is translated, so a chain the operator never
configured explicitly keeps the behavior it had instead of moving onto finality tags. Send-only
nodes, `Order` and `HTTPURLExtraWrite` have no standalone equivalent and are dropped, each logged at
warn so nothing goes missing quietly.
deployment/shared/signing_identity.go:26
- CanonicalEVMAddress lowercases/prefixes the string but does not trim whitespace. If JD ever returns an address with leading/trailing whitespace (or a caller passes one through), the result will still contain whitespace and will fail downstream hex decoding even though the function is intended to canonicalize addresses.
func CanonicalEVMAddress(addr string) string {
lower := strings.ToLower(addr)
if !strings.HasPrefix(lower, "0x") {
return "0x" + lower
}
return lower
bootstrap/keys/import.go:157
- When Import.Format is left empty (the normal auto-detect path), this error message formats it with %s and can end up reading "failed to decode import...". That makes troubleshooting harder in the exact scenario operators are expected to use.
privateKey, id, err := decodeImport(spec)
if err != nil {
return fmt.Errorf("failed to decode %s import for key %q from %q: %w", spec.Format, keyName, spec.Path, err)
}
bootstrap/keys/import.go:199
- Same issue as EnsureImportedKey: when Import.Format is empty (auto-detect), this error can render as "failed to decode export" with a blank format label.
_, id, err := decodeImport(spec)
if err != nil {
return "", fmt.Errorf("failed to decode %s export %q: %w", spec.Format, spec.Path, err)
}
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 26 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
integration/pkg/accessors/evm/clnode_config.go:164
- convertNodes currently iterates over a map of dropped settings (random order) and then sorts the entire warnings slice. This makes warning output deterministic but no longer reflect the operator's config order (and it undermines mergeByChainID's note about preserving order for warnings). You can keep output deterministic without sorting by iterating the settings in a fixed order and returning warnings as-collected.
for setting, isSet := range map[string]bool{
"HTTPURLExtraWrite": node.HTTPURLExtraWrite != nil,
"Order": node.Order != nil,
"IsLoadBalancedRPC": node.IsLoadBalancedRPC != nil,
} {
bootstrap/keys/import.go:83
- The ExpectedID docstring says it must be hex "without a 0x prefix", but EnsureImportedKey accepts values with or without the prefix (it trims "0x"). Updating this comment will better match actual behavior and reduce operator confusion.
// ExpectedID, when set, must equal the identity encoded in the export: the onchain signing
// address for ImportFormatOCR2, the account address for ImportFormatETH, both hex without a
// 0x prefix and case-insensitive. An operator migrating several nodes can mount the wrong
// node's export, and without this check the node comes up signing with another node's key —
tt-cll
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 26 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
integration/pkg/accessors/evm/factory_constructor.go:82
- isChainlinkNodeConfig classifies purely on the presence of the top-level "EVM" key. If a mounted TOML accidentally contains both a standalone [chains.*] config and an EVM node config (e.g., concatenated/merged files), this will route to the node-config converter and silently ignore the standalone chains section (since convertChainlinkNodeConfig unmarshals only the EVM section). That breaks the stated goal that nothing is dropped silently.
Consider rejecting configs that contain both top-level keys ("EVM" and "chains") so operators get a clear error instead of an implicit precedence rule.
func isChainlinkNodeConfig(data []byte) (bool, error) {
var probe map[string]toml.Primitive
if _, err := toml.Decode(string(data), &probe); err != nil {
return false, err
}
_, ok := probe["EVM"]
return ok, nil
bootstrap/keys/import.go:165
- EnsureImportedKey intentionally allows spec.Format to be empty (auto-detected), but the decode/expected_id error paths format errors using spec.Format. When Format is empty, errors like "failed to decode import..." / " import for key ..." end up with a blank format label, which is confusing.
Consider using a local non-empty label (e.g., "auto-detected") for messaging/logging whenever spec.Format is empty.
privateKey, id, err := decodeImport(spec)
if err != nil {
return fmt.Errorf("failed to decode %s import for key %q from %q: %w", spec.Format, keyName, spec.Path, err)
}
if want := strings.TrimSpace(spec.ExpectedID); want != "" {
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 42 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
bootstrap/keys/import.go:149
- EnsureImportedKey skips import as soon as a key with the same name exists, but it doesn’t verify the existing key’s type matches the declared
keyType. If a key with the same name exists with a different type (e.g., from a prior misconfiguration), this will silently keep an unusable key and likely fail later in a less obvious place.
resp, err := ks.GetKeys(ctx, keystore.GetKeysRequest{KeyNames: []string{keyName}})
if err == nil && len(resp.Keys) > 0 {
lggr.Infow("key already exists, skipping import",
"keyName", keyName, "keyType", keyType, "purpose", purpose, "format", spec.Format,
"publicKey", hex.EncodeToString(resp.Keys[0].KeyInfo.PublicKey),
)
return nil
}
docs/migration/cl-to-standalone.md:113
- This section says only send-only nodes,
Order, andHTTPURLExtraWriteare dropped, but the converter also dropsIsLoadBalancedRPC(with a warning) when converting from Chainlink node TOML. The doc should mention it so operators aren’t surprised by the warning/behavior change.
Two things a Chainlink node can express have no standalone equivalent and are dropped: send-only
nodes, and per-node `Order` and `HTTPURLExtraWrite`. Each one is logged at startup saying exactly
what was dropped. If you rely on a send-only endpoint, add it as a full node.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
bootstrap/keys/import.go:236
- InspectImport has the same formatting issue: when Import.Format is omitted, the error renders as "failed to decode export...". Since format detection happens internally, the placeholder isn't adding useful signal here.
_, id, err := decodeImport(spec)
if err != nil {
return "", fmt.Errorf("failed to decode %s export %q: %w", spec.Format, spec.Path, err)
}
bootstrap/keys/import.go:175
- Same as above: when Import.Format is omitted, this expected_id mismatch message starts with an empty format prefix (" import for key..."). Consider removing the unused format placeholder.
if want := strings.TrimSpace(spec.ExpectedID); want != "" {
if !strings.EqualFold(strings.TrimPrefix(want, "0x"), id) {
return fmt.Errorf(
"%s import for key %q from %q holds identity %s but expected_id is %s: the wrong node's export is mounted",
spec.Format, keyName, spec.Path, id, want,
)
bootstrap/keys/import.go:169
- The error message will render with an empty format when Import.Format is omitted (the normal case), producing strings like "failed to decode import...". This makes failures harder to understand.
This issue also appears in the following locations of the same file:
- line 170
- line 233
privateKey, id, err := decodeImport(spec)
if err != nil {
return fmt.Errorf("failed to decode %s import for key %q from %q: %w", spec.Format, keyName, spec.Path, err)
}
tools/configdoc/registry/registry.go:286
- The documented [key_import] example paths here differ from the migration runbook and bootstrap README (which use /etc/ccv/migration/key.json and /etc/ccv/migration/export-password.txt). Since this struct instance drives generated docs, consider standardizing the example paths to reduce copy/paste confusion.
KeyImport: &bootstrap.KeyImport{ //nolint:gosec // G101: password_path is where a password file is mounted, not a password
Path: "/etc/bootstrap/keys/exported-key.json",
PasswordPath: "/etc/bootstrap/keys/export-password.txt",
ExpectedID: "0x0123456789abcdef0123456789abcdef01234567",
docs/config/bootstrap/config.documented.toml:40
- The [key_import] example paths here use /etc/bootstrap/keys/... but the migration runbook/changelog/bootstrap README use /etc/ccv/migration/.... Standardizing on one set of example paths would make the docs easier to follow.
[key_import]
# path is the mounted path of the key file exported from the Chainlink node.
path = "/etc/bootstrap/keys/exported-key.json"
# password_path is the mounted path of a file holding the password used at export time.
password_path = "/etc/bootstrap/keys/export-password.txt"
# expected_id optionally pins the address the export must carry. Set it when migrating more than
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
bootstrap/keys/import.go:332
- Inconsistent use of geth keystore auth types: this file calls EncryptDataV3 with a []byte auth (see encodeForImport), but DecryptDataV3 is currently passed a string. The go-ethereum keystore helpers are expected to take the same auth type, so this is likely a compile-time type mismatch (or at least inconsistent API usage).
decrypted, err := gethkeystore.DecryptDataV3(export.Crypto, ocr2PasswordPrefix+password)
docs/config/bootstrap/config.documented.toml:39
- The example mount paths for [key_import] in this documented config use
/etc/bootstrap/keys/..., but the operator docs and bootstrap README use/etc/ccv/migration/...(see docs/migration/cl-to-standalone.md and bootstrap/README.md). This inconsistency can confuse operators copying snippets; consider aligning the example paths across docs.
path = "/etc/bootstrap/keys/exported-key.json"
# password_path is the mounted path of a file holding the password used at export time.
password_path = "/etc/bootstrap/keys/export-password.txt"
tools/configdoc/registry/registry.go:286
- The generated bootstrap config example uses
/etc/bootstrap/keys/...for the [key_import] mount paths, but other operator-facing docs in this PR (bootstrap/README.md and docs/migration/cl-to-standalone.md) use/etc/ccv/migration/...and devenv mounts there as well (services/keyimports.go). To avoid conflicting guidance, consider using the same example paths everywhere.
KeyImport: &bootstrap.KeyImport{ //nolint:gosec // G101: password_path is where a password file is mounted, not a password
Path: "/etc/bootstrap/keys/exported-key.json",
PasswordPath: "/etc/bootstrap/keys/export-password.txt",
ExpectedID: "0x0123456789abcdef0123456789abcdef01234567",
},
docs/migration/cl-to-standalone.md:113
- This section lists the Chainlink-node-only settings that are dropped during conversion, but the converter also drops
IsLoadBalancedRPC(see integration/pkg/accessors/evm/clnode_config.go). The doc should mention it so operators aren’t surprised by the startup warning.
Two things a Chainlink node can express have no standalone equivalent and are dropped: send-only
nodes, and per-node `Order` and `HTTPURLExtraWrite`. Each one is logged at startup saying exactly
what was dropped. If you rely on a send-only endpoint, add it as a full node.
changelog/2026-07-29_cl_to_standalone_key_migration.md:51
- The changelog describes which Chainlink-node-only EVM node settings are dropped during conversion, but the converter also drops
IsLoadBalancedRPC(see integration/pkg/accessors/evm/clnode_config.go). Consider listing it here as well so the operator-facing summary matches actual behavior.
configured explicitly keeps the behavior it had instead of moving onto finality tags. Send-only
nodes, `Order` and `HTTPURLExtraWrite` have no standalone equivalent and are dropped, each logged at
warn so nothing goes missing quietly.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
docs/migration/cl-to-standalone.md:115
- The migration guide’s Step 2 list of node-only settings that are dropped is incomplete: the converter also drops
IsLoadBalancedRPC(and logs a warning for it), in addition toOrderandHTTPURLExtraWrite(see integration/pkg/accessors/evm/clnode_config.go:169-172). The doc should mention it so operators aren’t surprised by an additional warning / lost setting.
Two things a Chainlink node can express have no standalone equivalent and are dropped: send-only
nodes, and per-node `Order` and `HTTPURLExtraWrite`. Each one is logged at startup saying exactly
what was dropped. If you rely on a send-only endpoint, add it as a full node.
changelog/2026-07-29_cl_to_standalone_key_migration.md:51
- This changelog entry says only SendOnly,
Order, andHTTPURLExtraWriteare dropped from a Chainlink node EVM config, but the converter also dropsIsLoadBalancedRPCwith a warning (integration/pkg/accessors/evm/clnode_config.go:171-176). The changelog should reflect that additional dropped setting.
The node's chain defaults are applied before finality is translated, so a chain the operator never
configured explicitly keeps the behavior it had instead of moving onto finality tags. Send-only
nodes, `Order` and `HTTPURLExtraWrite` have no standalone equivalent and are dropped, each logged at
warn so nothing goes missing quietly.
bootstrap/keys/import.go:179
- When the import format is auto-detected (the normal case when [key_import] omits
format),spec.Formatremains empty becausedecodeImportstores the detected value in a localformatvariable but doesn’t propagate it back. As a result, this error message (and the subsequent logs at import/skip) can end up with an empty format prefix (" import for key ..."), which makes debugging harder.
if want := strings.TrimSpace(spec.ExpectedID); want != "" {
if !strings.EqualFold(strings.TrimPrefix(want, "0x"), id) {
return fmt.Errorf(
"%s import for key %q from %q holds identity %s but expected_id is %s: the wrong node's export is mounted",
spec.Format, keyName, spec.Path, id, want,
)
}
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Code coverage report:
Files added (in
|
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 38 changed files in this pull request and generated no new comments.
Suppressed comments (2)
docs/migration/cl-to-standalone.md:147
- This section says only send-only nodes,
Order, andHTTPURLExtraWriteare dropped during Chainlink node EVM config conversion, but the converter also dropsIsLoadBalancedRPC(and logs a warning for it). Update the runbook so operators aren’t surprised by an additional dropped setting.
changelog/2026-07-29_cl_to_standalone_key_migration.md:67 - This changelog entry lists dropped Chainlink node EVM settings as SendOnly nodes,
Order, andHTTPURLExtraWrite, but the conversion code also dropsIsLoadBalancedRPCwith a warning. The changelog should mention it to match actual behavior.
The node's chain defaults are applied before finality is translated, so a chain the operator never
configured explicitly keeps the behavior it had instead of moving onto finality tags. Send-only
nodes, `Order` and `HTTPURLExtraWrite` have no standalone equivalent and are dropped, each logged at
warn so nothing goes missing quietly.
makramkd
reviewed
Jul 31, 2026
Comment on lines
+5
to
+6
| Distributor. This document is the procedure. It assumes EVM; Solana and Canton were standalone from | ||
| the start and have nothing to migrate. |
Collaborator
There was a problem hiding this comment.
Suggested change
| Distributor. This document is the procedure. It assumes EVM; Solana and Canton were standalone from | |
| the start and have nothing to migrate. | |
| Distributor. This document is the procedure. It assumes EVM; other families are standalone from | |
| the start and have nothing to migrate. |
Comment on lines
+17
to
+19
| from there when it configures the contract. Generate a new one and the committee's signer set has to | ||
| be updated on every chain, which needs a config transaction per chain and coordination with the | ||
| other operators in the committee. |
Collaborator
There was a problem hiding this comment.
Suggested change
| from there when it configures the contract. Generate a new one and the committee's signer set has to | |
| be updated on every chain, which needs a config transaction per chain and coordination with the | |
| other operators in the committee. | |
| from there when it configures the contract. If you generate a new one, the committee's signer set has to | |
| be updated on every chain, which needs a config transaction per chain and coordination with the | |
| other operators in the committee. |
I'm not sure of the intended audience of the doc - if its node operators, its probably not necessary to have this sentence.
Comment on lines
+28
to
+29
| This is also why the Chainlink node must be stopped before the standalone verifier starts: JD | ||
| identifies a node by the key it authenticates with, and one record cannot have two owners. |
Collaborator
There was a problem hiding this comment.
Question: it must only be stopped once we intend to send jobs to the standalone verifier and executor nodes, right?
Comment on lines
+62
to
+63
| configures or can change. It has to be resolved on that side first, by consolidating the committee's | ||
| verifier jobs into one that writes to every aggregator, and only then does this procedure apply. |
Collaborator
There was a problem hiding this comment.
Aside: any idea if we've done this migration yet in prod (single job, multiple aggregators)
makramkd
reviewed
Jul 31, 2026
Comment on lines
+825
to
+828
| if importKeyName != "" && k.name == importKeyName { | ||
| if err := keys.EnsureImportedKey(ctx, lggr, ks, k.name, k.purpose, k.keyType, importSpec); err != nil { | ||
| return nil, nil, fmt.Errorf("failed to import key %q (purpose=%q, type=%v): %w", k.name, k.purpose, k.keyType, err) | ||
| } |
Collaborator
There was a problem hiding this comment.
Question: what would happen if there already exists a key w/ this name, would it get overwritten or would this error out?
Practically, how this could happen: a nop launches the standalone verifier/executor, and w/out any jobs/etc. configured and the key_import not present, I believe the keystore would be initialized w/ the keys declared in the bootstrap.Run options, if I'm not mistaken.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 39 out of 39 changed files in this pull request and generated no new comments.
Suppressed comments (3)
migration/node_client.go:49
- NewNodeClient accepts URLs that include a path/query/fragment (e.g. http://host:6688/v2). In that case the client will build requests like ".../v2/sessions" and ".../v2/v2/keys/...", which is very likely to fail with confusing 404s. Since this function is documented as taking a base URL, it should either normalize to scheme+host or reject non-base URLs up front.
bootstrap/config.go:388 - Config validation currently only checks that [key_import] is well-formed, but it does not catch the incompatible combination of key_import + keystore backend "kms". That config will pass validation and then fail later during keystore initialization; it would be more actionable to fail validation with a direct message.
// The key import populates the keystore, which both modes initialize when [db] and [keystore]
// are present, so it is validated in every mode rather than only alongside the JD infra bundle.
errs = append(errs, validateKeyImport(c.KeyImport)...)
return errors.Join(errs...)
cli/migrate/commands.go:167
- inspectKey already detects the export format, but then calls keys.InspectImport without passing it, which forces a second format-detection pass. Passing the detected format avoids redundant work and keeps DetectFormat/InspectImport behavior aligned if either is extended later.
format, err := keys.DetectFormat(data)
if err != nil {
return err
}
id, err := keys.InspectImport(keys.Import{Path: keyFile, PasswordPath: passwordFile})
if err != nil {
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 39 out of 39 changed files in this pull request and generated no new comments.
Suppressed comments (3)
migration/node_client.go:96
- This error message doesn't tell the operator which flag to use to disambiguate multiple EVM OCR2 bundles. The tests also expect the hint to mention
--bundle-id, and without it the CLI output is less actionable.
cli/migrate/commands.go:146 - The CLI summary points operators at a non-existent doc path (
docs/migration/evm-evm-cl-to-standalone.md). This will break the runbook link in the primary operator-facing output.
3. Continue from step 4 of docs/migration/evm-evm-cl-to-standalone.md (stop the Chainlink node).
migration/node_client.go:127
- This error message is missing the flag name the operator should use to resolve ambiguity when multiple enabled accounts exist on the same chain. The accompanying unit test also expects
--accountto be mentioned.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
https://smartcontract-it.atlassian.net/browse/CCIP-12567
https://smartcontract-it.atlassian.net/browse/CCIP-12568
Lets an EVM node operator running CCV as jobs on a Chainlink node move to the standalone verifier and executor without reconfiguring contracts, moving funds, or creating a duplicate JD entry.
[key_import]in the bootstrap config adopts a key exported from the Chainlink node — the OCR2 bundle's onchain signing key for the verifier, the funded EVM account for the executor — with an optionalexpected_idpin against mounting the wrong node's export. The CSA key is never exported; the existing JD record is repointed at the standalone verifier's own CSA key, keeping the node ID, NOP alias, and job history.[[EVM]]/[[EVM.Nodes]]at startup: chain IDs resolved to selectors, node chain defaults applied before finality translation, unsupported settings (send-only nodes,Order,HTTPURLExtraWrite) dropped with warnings.build/devenv/migrationcarries out the same cutover in devenv (key export, CL node stop, JD identity adoption, job spec retargeting preservingexternalJobID), anddocs/migration/cl-to-standalone.mdis the operator procedure.Testing
bootstrap/keys), CL node config conversion (integration/pkg/accessors/evm), job spec retargeting, and cutover helpers.TestE2EMigration_CLToStandalone(added to the CL smoke workflow) runs the full cutover on a CL-mode environment and asserts messages verify and execute before and after, with the signing address, transmitter address, and JD node record unchanged.Checklist
changelogdirectory)