Conversation
|
👋 tt-cll, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
There was a problem hiding this comment.
Pull request overview
Adds a dedicated CCIP 2.0 integration guide to the deployment tooling documentation, and links it from the main docs index and the existing “Implementing Adapters” guide so integrators can follow a 2.0-specific path.
Changes:
- Add new
CCIP 2.0 Integration Guidedoc covering 2.0-specific interfaces, registries/dispatch flow, and an integration checklist. - Link the new guide from
deployment/docs/index.md. - Add an upfront callout in
implementing-adapters.mdredirecting 2.0 integrators to the new guide.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| deployment/docs/index.md | Adds the new 2.0 guide to the documentation index table. |
| deployment/docs/implementing-adapters.md | Adds a prominent note directing CCIP 2.0 integrators to the new guide. |
| deployment/docs/ccip-2.0-integration-guide.md | New end-to-end CCIP 2.0 integration documentation (interfaces, flow, registration, checklist). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| **Registration:** | ||
| ```go | ||
| adapters.GetDeployChainContractsRegistry().Register(chainsel.FamilyMyChain, &MyDeployAdapter{}) |
There was a problem hiding this comment.
These registration snippets use chainsel.FamilyMyChain, but most other docs in deployment/docs/ use the chain_selectors import name (e.g. implementing-adapters.md). For consistency and to reduce copy/paste friction, consider using chain_selectors.FamilyMyChain (or explicitly show the import/alias you expect readers to use).
| adapters.GetDeployChainContractsRegistry().Register(chainsel.FamilyMyChain, &MyDeployAdapter{}) | |
| adapters.GetDeployChainContractsRegistry().Register(chain_selectors.FamilyMyChain, &MyDeployAdapter{}) |
|
|
||
| Note over Devenv: Phase 2: Token Deployment (per chain) | ||
| Devenv->>CS: TokenExpansion changeset | ||
| CS->>Reg: TokenAdapterRegistry.GetTokenAdapter() |
There was a problem hiding this comment.
In the sequence diagram, TokenAdapterRegistry.GetTokenAdapter() is shown without parameters. In code the lookup requires (chainFamily, version), so it may be clearer to annotate that here (or rename the callout to something like "Resolve token adapter by (family, poolVersion)"), to prevent readers from looking for a no-arg method.
| CS->>Reg: TokenAdapterRegistry.GetTokenAdapter() | |
| CS->>Reg: TokenAdapterRegistry.GetTokenAdapter(chainFamily, version) |
| | Concept | 1.6 | 2.0 | | ||
| |---------|-----|-----| | ||
| | **Execution** | Implicit (part of OffRamp) | Explicit Executor contracts with qualifiers | | ||
| | **OCR** | OCR3 configured on OffRamp | No OCR -- off-chain consensus handled differently | |
There was a problem hiding this comment.
The comparison table says "No OCR" for 2.0, but the codebase still relies on OCR key bundles (OCR2) for node chain config/signing keys in the 2.0 offchain flow. Consider changing this cell to something more precise like "No OCR3 (OffRamp OCR3 config not used in 2.0)" to avoid implying that OCR-related configuration disappears entirely.
| | **OCR** | OCR3 configured on OffRamp | No OCR -- off-chain consensus handled differently | | |
| | **OCR** | OCR3 configured on OffRamp | No OCR3 on OffRamp -- OCR key bundles still used elsewhere in 2.0 offchain flow | |
| **Source:** [v2_0_0/adapters/deploy_chain_contracts.go](../v2_0_0/adapters/deploy_chain_contracts.go) | ||
| **Registry:** `DeployChainContractsRegistry` via `adapters.GetDeployChainContractsRegistry()` | ||
| **Key:** `chainFamily` (version-agnostic within 2.0) |
There was a problem hiding this comment.
nit: this all renders on the same line, bulleting them may look cleaner:
| **Source:** [v2_0_0/adapters/deploy_chain_contracts.go](../v2_0_0/adapters/deploy_chain_contracts.go) | |
| **Registry:** `DeployChainContractsRegistry` via `adapters.GetDeployChainContractsRegistry()` | |
| **Key:** `chainFamily` (version-agnostic within 2.0) | |
| * **Source:** [v2_0_0/adapters/deploy_chain_contracts.go](../v2_0_0/adapters/deploy_chain_contracts.go) | |
| * **Registry:** `DeployChainContractsRegistry` via `adapters.GetDeployChainContractsRegistry()` | |
| * **Key:** `chainFamily` (version-agnostic within 2.0) |
| **Source:** [v2_0_0/adapters/chain_family.go](../v2_0_0/adapters/chain_family.go) | ||
| **Registry:** `ChainFamilyRegistry` via `adapters.GetChainFamilyRegistry()` | ||
| **Key:** `chainFamily` (version-agnostic within 2.0) |
There was a problem hiding this comment.
| **Source:** [v2_0_0/adapters/chain_family.go](../v2_0_0/adapters/chain_family.go) | |
| **Registry:** `ChainFamilyRegistry` via `adapters.GetChainFamilyRegistry()` | |
| **Key:** `chainFamily` (version-agnostic within 2.0) | |
| * **Source:** [v2_0_0/adapters/chain_family.go](../v2_0_0/adapters/chain_family.go) | |
| * **Registry:** `ChainFamilyRegistry` via `adapters.GetChainFamilyRegistry()` | |
| * **Key:** `chainFamily` (version-agnostic within 2.0) |
| | `CCTPChain` / `RemoteCCTPChain` | [v2_0_0/adapters/cctp.go](../v2_0_0/adapters/cctp.go) | Only if your chain supports USDC via CCTP | | ||
| | `LombardChain` / `RemoteLombardChain` | [v2_0_0/adapters/lombard.go](../v2_0_0/adapters/lombard.go) | Only if your chain supports Lombard | | ||
|
|
||
| ## CCV Devenv Interfaces |
There was a problem hiding this comment.
This is good documentation - though I wonder if we should have it in the devenv package as well (or instead?).
There was a problem hiding this comment.
yeah maybe this section can be sparser on devenv. I wanted to have something explaining how the tooling API is called for someone performing an integration, but maybe that detail should be in a different doc
|
No description provided.