Reorganise deployment package#22612
Conversation
|
✅ No conflicts with other open PRs targeting |
CORA - Pending Reviewers
Legend: ✅ Approved | ❌ Changes Requested | 💬 Commented | 🚫 Dismissed | ⏳ Pending | ❓ Unknown For more details, see the full review summary. |
|
There was a problem hiding this comment.
Pull request overview
Risk Rating: MEDIUM
This PR reorganizes (and narrows) shared deployment utilities into ownership-aligned packages, removing generic helpers from the root deployment module and introducing smaller, domain-scoped replacements (e.g., CCIP/internal bigint helpers, devenv KMS helpers).
Changes:
- Extracts bigint scaling helpers out of
deployment/environment.gointo CCIP- and integration-test-scopedbigintpackages and updates call sites. - Moves/duplicates a few generic helpers (tx revert reason parsing, address list validation) into the specific packages that use them, and deletes the old shared
deployment/helpers.go. - Renames the devenv internal KMS package from
deploymenttokmsand updates devenv to import it directly.
Targeted areas for scrupulous human review:
deployment/environment/devenv/chain.go: the inlined tx revert-reason parsing logic (correctness of error parsing and behavior differences from the removed helper).deployment/environment/devenv/internal/kms/*: package rename impact (ensure no remaining imports/uses expecting the olddeploymentpackage name).- Broad compile/test pass across the
deploymentandintegration-testsmodules to confirm all references to removed exported helpers were migrated.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| integration-tests/utils/bigint/bigint.go | Adds integration-test-scoped bigint helper functions (UBigInt/E18Mult/EDecMult). |
| integration-tests/smoke/ccip/ccip_message_limitations_test.go | Updates tests to use integration-tests bigint helpers instead of deployment helpers. |
| integration-tests/smoke/ccip/ccip_fees_test.go | Replaces deployment bigint helpers with integration-tests bigint helpers in fee tests. |
| integration-tests/smoke/ccip/ccip_disable_lane_test.go | Switches price constants to integration-tests bigint helpers. |
| integration-tests/smoke/ccip/ccip_aptos_messaging_test.go | Updates token minting/value setting to use integration-tests bigint helpers. |
| integration-tests/load/ccip/helpers.go | Replaces deployment bigint helpers with integration-tests bigint helpers for funding amounts. |
| deployment/ton_chain.go | Deletes the TonChain struct from the root deployment package. |
| deployment/keystone/changeset/state.go | Switches labeled-address filtering to the new internal addrbook package. |
| deployment/keystone/changeset/internal/state.go | Same as above for the internal changeset implementation. |
| deployment/keystone/changeset/internal/addrbook/addrbook.go | Introduces addrbook package (moved from old deployment package location) for label filtering. |
| deployment/keystone/changeset/internal/addrbook/addrbook_test.go | Updates tests for the new addrbook package name/imports. |
| deployment/helpers.go | Removes previously shared helper utilities from the root deployment package. |
| deployment/environment/devenv/internal/kms/kms_client.go | Renames internal KMS package to kms. |
| deployment/environment/devenv/internal/kms/kms_client_test.go | Updates tests to match kms package rename. |
| deployment/environment/devenv/chain.go | Updates devenv to import kms directly and inlines tx revert-reason parsing helper. |
| deployment/environment/crib/helpers.go | Replaces use of removed deployment bigint helper with inline big.Int construction. |
| deployment/environment.go | Removes exported bigint helpers (UBigInt/E18Mult/EDecMult). |
| deployment/ccip/shared/token_info.go | Moves MockLinkPrice computation to CCIP internal bigint helper package. |
| deployment/ccip/shared/stateview/evm/state.go | Replaces removed deployment address-list helpers with local equivalents. |
| deployment/ccip/internal/bigint/bigint.go | Adds CCIP-scoped internal bigint helper package. |
| deployment/ccip/changeset/testhelpers/test_helpers_solana_v0_1_0.go | Updates defaults/prices to use CCIP internal bigint helpers (including non-EVM decimal scaling). |
Comments suppressed due to low confidence (3)
deployment/keystone/changeset/internal/addrbook/addrbook.go:4
- Spelling: "depcreated" should be "deprecated" in the package comment.
This issue also appears on line 8 of the same file.
deployment/keystone/changeset/internal/addrbook/addrbook.go:13
- This file imports
github.com/smartcontractkit/chainlink-deployments-framework/deploymentwithout an alias, but within thedeployment/...tree this identifier is easily confused with the localgithub.com/smartcontractkit/chainlink/deploymentmodule. Elsewhere in this package the deployments-framework import is consistently aliased tocldf(e.g.deployment/keystone/changeset/state.go:13-14). Consider aliasing here too for consistency/readability.
deployment/keystone/changeset/internal/addrbook/addrbook_test.go:10 - Same as
addrbook.go: consider aliasing the deployments-frameworkdeploymentimport tocldfto avoid confusion with the localgithub.com/smartcontractkit/chainlink/deploymentmodule and match the convention used in nearby code (e.g.deployment/keystone/changeset/state.go:13-14).
| if json.Unmarshal(b, &callErr) != nil { | ||
| return "", err | ||
| } |
| evmFundingAmount := new(big.Int).Mul(new( | ||
| big.Int).SetUint64(evmFundingEth), new(big.Int).SetUint64(1e18), | ||
| ) |


Reorganise the deployment package into a structure that aligns with code ownership