From d813894d383662c3be433ea447ca70a7e0850da7 Mon Sep 17 00:00:00 2001 From: chris-de-leon-cll <147140544+chris-de-leon-cll@users.noreply.github.com> Date: Mon, 20 Apr 2026 02:26:17 -0700 Subject: [PATCH 1/2] chore: add style guides for each team --- deployment/docs/style-guide-contributions.md | 173 +++++++++++++++++++ docs/style-guides/evm.md | 95 ++++++++++ docs/style-guides/ops.md | 97 +++++++++++ docs/style-guides/platform.md | 100 +++++++++++ 4 files changed, 465 insertions(+) create mode 100644 deployment/docs/style-guide-contributions.md create mode 100644 docs/style-guides/evm.md create mode 100644 docs/style-guides/ops.md create mode 100644 docs/style-guides/platform.md diff --git a/deployment/docs/style-guide-contributions.md b/deployment/docs/style-guide-contributions.md new file mode 100644 index 0000000000..1338d2fa2f --- /dev/null +++ b/deployment/docs/style-guide-contributions.md @@ -0,0 +1,173 @@ +--- +title: Maintaining the Changeset Style Guide +sidebar_label: Style Guide Contributions +sidebar_position: 9 +--- + +# Maintaining the Changeset Style Guide + +This page explains when and how to update the [Changeset Style Guide](./style-guide.md). It does not restate the rules themselves; read the style guide for the actual guidance. + +## Scope and purpose + +The Changeset Style Guide turns recurring review feedback into a practical reference for writing and reviewing changesets. + +This document explains how to maintain that guide so it stays useful, scannable, and reviewable over time. It is about maintaining the guide itself, not changeset implementation. + +## When to add or update a rule + +Good reasons to add or update a rule: + +- The same feedback appears on multiple PRs. +- There is a real risk to correctness, safety, or operability that a short rule can prevent. +- A pattern is widespread enough that a durable principle, not just a one-line fix, is worth documenting. +- A current rule needs clarification because reviewers or authors are interpreting it inconsistently. + +Signals to pause, narrow the scope, or avoid adding a rule: + +- The issue is a one-off bug or a temporary API quirk that will disappear with the next refactor. +- The proposed rule would duplicate [Cross-Family Deployment Architecture](./architecture.md) or [Implementing Adapters](./implementing-adapters.md); prefer linking to those for deep implementation detail. +- The change would read as a mandate to rewrite unrelated, already-safe code. The style guide explicitly says it is not a mandate to rewrite older code that is correct and operating safely. +- The guidance is really a local implementation note, not a stable review principle. + +## When to remove or consolidate a rule + +Revise or remove a rule when: + +- The underlying API, helper, or workflow has changed enough that the example now teaches the wrong instinct. +- Repo conventions have shifted and the old rule now conflicts with the default local pattern. +- Two sections now cover the same idea and can be merged into one clearer rule. +- The rule documents an edge case that no longer exists in practice. + +When removing a rule, prefer either: + +- folding the still-valid principle into a broader section, or +- deleting it entirely if the guidance is no longer useful. + +Do not keep obsolete guidance only for historical continuity. + +## Deciding between a new rule and expanding an existing one + +Create a new section when the guidance introduces a distinct principle that could stand alone in review comments or the review checklist. + +Expand an existing section when the change only adds: + +- another example of the same failure mode, +- a small exception, +- a clarification of wording, or +- an additional repo convention that supports the same rule. + +Keep each section focused on one rule or one closely related exception. + +## How to write a rule + +Match the structure used in the [Changeset Style Guide](./style-guide.md): + +1. **`##` section title** — short, specific, and stable. +2. **Rule:** one clear sentence stating what to do or avoid. +3. **Why it matters:** explain the operational, correctness, review, or debugging impact. +4. **Examples:** use `// ❌ BAD:` and `// ✅ BETTER:` Go snippets when they make the pattern easier to recognize. +5. **Exceptions (optional):** if the rule is not universal, say when it does not apply. + +When writing the **Rule:** line: + +- Prefer direct verbs such as “Use”, “Resolve”, “Infer”, “Handle”, or “Avoid”. +- State the default behavior first. +- Keep it specific enough that a reviewer could quote it in one sentence. + +When writing **Why it matters:** + +- Focus on correctness, safety, operability, debuggability, or review clarity. +- Avoid repeating the rule in different words. + +Examples should be updated when names, helpers, or APIs change, but example code does not need to be rewritten just to match every new local convention if the principle is still clear. + +Use stable section titles. Avoid renaming existing `##` headings unless the old title is clearly misleading, because heading text determines link anchors used in reviews, docs, and PR discussions. + +### Copyable rule template + +Use this template when adding a new section to the style guide: + +````md +## Short, Stable Section Title + +**Rule:** State the default behavior in one clear sentence. + +**Why it matters:** Explain the correctness, safety, operability, or review impact. + +```go +// ❌ BAD: show the common mistake + +// ✅ BETTER: show the preferred pattern +``` +```` + +## Editorial standards + +Keep contributions aligned with the tone and goals of the [Changeset Style Guide](./style-guide.md): + +- Prefer durable principles over guidance tied to temporary implementation details. +- Prefer concrete BAD/BETTER examples when they make the rule faster to recognize. +- Keep sections short, scannable, and linkable. +- Split large topics into separate rules instead of growing one section indefinitely. +- Avoid duplicating deep implementation guidance that belongs in [Cross-Family Deployment Architecture](./architecture.md) or [Implementing Adapters](./implementing-adapters.md). +- Write for both human reviewers and AI-assisted authoring: the rule should be easy to quote, apply, and review against. + +Match the main guide’s tone: direct, practical, and review-oriented. Prefer “what to do and why” over abstract documentation language. + +## Required updates when editing the guide + +When you add, remove, rename, split, or merge a rule in [Changeset Style Guide](./style-guide.md), also check the following: + +- Update the **Table of Contents** so new sections appear and removed sections disappear. +- Update the **Review Checklist** so it matches the current rule set. +- Update any **cross-links** in this guide and related docs if section names or anchors changed. +- Check whether nearby examples should be revised for naming consistency or terminology changes. +- Place the rule where it fits the main guide’s conceptual flow. Preserve the existing section order unless there is a clear reason to reorganize it. + +## Contribution process + +1. Open a PR that updates [style-guide.md](./style-guide.md). +2. If this maintenance document needs new process guidance, update this file in the same PR. +3. In the PR description, state: + - the rule being added, changed, merged, or removed, + - the problem it addresses, + - whether it reflects recurring review feedback, an incident, or a clarification of existing guidance, + - any required updates to the checklist, table of contents, or cross-links. +4. For substantive new rules, include brief evidence in the PR description, such as recurring review feedback, an incident, or repeated confusion in authoring. +5. Request review from the owners for deployment and changeset tooling. If there is a CODEOWNERS entry or team-specific review convention for this area, follow that source of truth. + +## Non-goals + +This guide is not meant to: + +- document every helper or package used in changesets, +- replace architecture or adapter implementation docs, +- preserve historical exceptions that are no longer useful, +- require cleanup of unrelated older code that is already safe and correct. + +## Maintainer checklist + +Before merging a style guide update, confirm the following: + +- The rule describes a recurring pattern or meaningful operational risk. +- The guidance is framed as a durable principle, not a temporary implementation detail. +- The section title is specific and stable. +- The rule and rationale are concise. +- Examples are accurate and materially improve comprehension. +- Any required companion updates to the Table of Contents, Review Checklist, and cross-links have been made. +- For substantive new rules, the PR includes brief evidence that the guidance reflects a recurring theme or real risk. +- Cross-links and anchors still work. +- Related docs are linked instead of duplicated. + +## Contribution flow at a glance + +```mermaid +flowchart LR + hazard[Recurring review feedback or operational hazard] + decide[Decide: new rule, clarification, consolidation, or no guide change] + draft[Draft or revise section in style-guide.md] + sync[Update TOC, checklist, ordering, and cross-links] + review[Open PR with rationale and evidence] + hazard --> decide --> draft --> sync --> review +``` diff --git a/docs/style-guides/evm.md b/docs/style-guides/evm.md new file mode 100644 index 0000000000..7ee60cfa47 --- /dev/null +++ b/docs/style-guides/evm.md @@ -0,0 +1,95 @@ +# EVM changeset style guide + +**Owning team (tracking reference):** Simon B., Robert, Anindita Ghosh. + +This file is a **harness**: consistent structure and **direction** for what belongs here. **Teams own all substantive rules, examples, and updates**—replace placeholders and grow this document with durable, reviewable guidance. + +## Purpose + +Capture **EVM-specific** practices for writing and reviewing CCIP tooling changesets: selectors, contract surfaces, adapter usage, and footguns that **do not** generalize to other chain families. + +## How to use this guide (once populated) + +- **While authoring:** use the rules when touching EVM adapters, bytecode/deploy steps, or chain-specific configuration in changesets. +- **Before review or merge:** run the checklist (when added) for EVM-facing changeset PRs. + +## What this guide should optimize for + +- Correct chain and contract identity (selectors, types, versions) in changeset inputs +- Safe sequencing of EVM deploy/upgrade/configure steps +- Reviewable EVM-specific error modes (reverts, gas, nonces) at the changeset boundary where relevant + +## Related documentation + +- [Platform (chain-agnostic) style guide](./platform.md) — shared changeset shape and conventions. +- [Ops / chainlink-deployments style guide](./ops.md) — how runs and pipelines are operated. +- [Changeset Style Guide](../../deployment/docs/style-guide.md) — existing deployment changeset reference. +- [Maintaining the Changeset Style Guide](../../deployment/docs/style-guide-contributions.md) — how to add rules, TOC, checklist, and links when editing any changeset-related guide. + +## Scope: what belongs here (and what does not) + +| In scope (this guide) | Out of scope (use instead) | +| ----------------------- | ---------------------------- | +| EVM **chain selectors**, contract **types/ABIs**, and adapter-specific patterns in changesets | Generic idempotency, shared datastore rules, cross-family input shape → [platform.md](./platform.md) | +| EVM sequencing (deploy/upgrade/configure) **as it differs** from other families | Running **chainlink-deployments** pipelines, environments, operational review → [ops.md](./ops.md) | +| Footguns that are meaningless outside EVM (e.g. chain-ID or EVM-specific ref semantics at this layer) | Adapter implementation internals → deployment architecture / adapter docs | + +If a rule applies equally to non-EVM families, it belongs in **platform**, not here. + +## Table of contents + + + +## Illustrative rule shape (replace with real team rules) + +The section below shows the **format** only. Delete this section when you add real content, or replace it rule-by-rule. + +### ExampleSectionTitleToReplace + +**Rule:** (One clear sentence: default behavior.) + +**Why it matters:** (Operational / correctness / review impact—do not repeat the rule verbatim.) + +```go +// ❌ BAD: (common mistake) + +// ✅ BETTER: (preferred pattern) +``` + +**Exceptions (optional):** (When the rule does not apply.) + +--- + +## Copyable rule template + +Use this when adding a new section: + +````md +## Short, Stable Section Title + +**Rule:** State the default behavior in one clear sentence. + +**Why it matters:** Explain the correctness, safety, operability, or review impact. + +```go +// ❌ BAD: show the common mistake + +// ✅ BETTER: show the preferred pattern +``` + +**Exceptions (optional):** When the rule does not apply. +```` + +## Direction: topics for the owning team to cover + +Turn these **prompts** into short rules with BAD/BETTER examples where they help. Avoid duplicating topics listed in [platform.md](./platform.md) or [ops.md](./ops.md). + +- **Chain selectors** and EVM identity in changeset inputs (what must be explicit vs inferred). +- **Contract types, versions, and ABI** handling in changesets—naming and evolution patterns teams should follow. +- **EVM adapter** usage: patterns that are review-friendly and hard to misconfigure **in EVM changesets** (not generic platform idempotency—see platform). +- **Deploy/upgrade/configure ordering** when EVM-specific ordering differs from other families. +- **Operational footguns at the EVM boundary** reviewers should watch for (e.g. wrong network, mismatched bytecode expectations)—staying at changeset/review level, not full chain ops (see ops for CLD runs). + +## Contributing and updating + +Teams update this guide as EVM patterns evolve. Follow the maintenance pattern in [Maintaining the Changeset Style Guide](../../deployment/docs/style-guide-contributions.md): stable headings for anchors, TOC/checklist hygiene, and links to deeper docs instead of duplicating them. diff --git a/docs/style-guides/ops.md b/docs/style-guides/ops.md new file mode 100644 index 0000000000..563d195023 --- /dev/null +++ b/docs/style-guides/ops.md @@ -0,0 +1,97 @@ +# Ops style guide: chainlink-deployments and deployment runs + +**Owning team (tracking reference):** Jasmin Bakalović, Phil Ó Condúin. + +This file is a **harness**: consistent structure and **direction** for what belongs here. **Teams own all substantive rules, examples, and updates**—replace placeholders and grow this document with durable, reviewable guidance. + +## Purpose + +Capture **operational** practices for running and reviewing CCIP-related work through **chainlink-deployments (CLD)** and adjacent deployment workflows: environments, pipelines, how changesets are exercised in CI/CD or release paths, and what reviewers should verify for a deployment run. + +## How to use this guide (once populated) + +- **While preparing a change:** use the rules to decide how the change should be validated through CLD (or documented exceptions). +- **Before promoting or closing a deployment-related PR:** run the checklist (when added) for operational completeness. + +## What this guide should optimize for + +- Repeatable, reviewable deployment runs with clear ownership +- Safe promotion between environments (where applicable) +- Observable failures and straightforward rollback or retry semantics **at the ops layer** + +## Related documentation + +- [Platform (chain-agnostic) style guide](./platform.md) — shared changeset conventions. +- [EVM changeset style guide](./evm.md) — EVM-specific changeset authoring. +- [Changeset Style Guide](../../deployment/docs/style-guide.md) — existing deployment changeset reference. +- [Maintaining the Changeset Style Guide](../../deployment/docs/style-guide-contributions.md) — how to add rules, TOC, checklist, and links when editing any changeset-related guide. + +## Scope: what belongs here (and what does not) + +| In scope (this guide) | Out of scope (use instead) | +| ----------------------- | ---------------------------- | +| **chainlink-deployments** workflows: pipelines, jobs, how runs are triggered and reviewed | Changeset **code structure** (idempotency, refs, shared helpers) → [platform.md](./platform.md) | +| Environment selection, promotion, and operational checklists for deployment **runs** | EVM-specific authoring (selectors, contract typing in changesets) → [evm.md](./evm.md) | +| Expectations for testing/validating changes through CLD vs ad-hoc paths | Low-level adapter implementation → deployment architecture docs | + +Authoring guidance for **what goes inside a changeset** belongs in **platform** or **evm**, not here—unless the guidance is specifically about **how that changeset is run or verified** through CLD. + +## Table of contents + + + +## Illustrative rule shape (replace with real team rules) + +The section below shows the **format** only. Delete this section when you add real content, or replace it rule-by-rule. + +### ExampleSectionTitleToReplace + +**Rule:** (One clear sentence: default behavior.) + +**Why it matters:** (Operational / correctness / review impact—do not repeat the rule verbatim.) + +```go +// ❌ BAD: (common mistake) + +// ✅ BETTER: (preferred pattern) +``` + +**Exceptions (optional):** (When the rule does not apply.) + +--- + +## Copyable rule template + +Use this when adding a new section (adapt language if the example is not Go): + +````md +## Short, Stable Section Title + +**Rule:** State the default behavior in one clear sentence. + +**Why it matters:** Explain the correctness, safety, operability, or review impact. + +```go +// ❌ BAD: show the common mistake + +// ✅ BETTER: show the preferred pattern +``` + +**Exceptions (optional):** When the rule does not apply. +```` + +## Direction: topics for the owning team to cover + +Turn these **prompts** into short rules with BAD/BETTER examples where they help. Avoid duplicating topics listed in [platform.md](./platform.md) or [evm.md](./evm.md). + +- How **chainlink-deployments** is expected to be used for CCIP-related changes (vs ad-hoc or legacy paths). +- **Pipeline/job** conventions: naming, required stages, and what “green” must mean before promotion. +- **Environments:** dev/stage/prod (or your equivalents)—rules of thumb for what runs where. +- **Review of deployment outputs:** logs, artifacts, or checkpoints reviewers should expect. +- **Secrets and configuration** at the ops layer (without duplicating in-repo changeset defaults—point to the right separation of concerns). +- **One-off CLD pipelines:** when they are unacceptable versus durable, tested paths (tie to org risk, without naming specific incidents unless public). +- **Observability:** what operators need to see during/after a run to trust the deployment. + +## Contributing and updating + +Teams update this guide as CLD and release practices evolve. Follow the maintenance pattern in [Maintaining the Changeset Style Guide](../../deployment/docs/style-guide-contributions.md): stable headings for anchors, TOC/checklist hygiene, and links to runbooks or internal docs instead of pasting secrets or environment-specific values. diff --git a/docs/style-guides/platform.md b/docs/style-guides/platform.md new file mode 100644 index 0000000000..c213723f8e --- /dev/null +++ b/docs/style-guides/platform.md @@ -0,0 +1,100 @@ +# Platform changeset style guide (chain-agnostic) + +**Owning team (tracking reference):** Terry Tata, Will Winder. + +This file is a **harness**: consistent structure and **direction** for what belongs here. **Teams own all substantive rules, examples, and updates**—replace placeholders and grow this document with durable, reviewable guidance. + +## Purpose + +Capture **cross-chain** (chain-family-agnostic) practices for writing and reviewing CCIP tooling changesets: shapes, inputs, persistence conventions, and shared abstractions that should stay consistent no matter which adapter or chain family sits underneath. + +## How to use this guide (once populated) + +- **While authoring:** use the rules to structure inputs, refs, apply behavior, and error handling before review. +- **Before review or merge:** run the checklist (when added) against changeset PRs touching shared platform code paths. + +## What this guide should optimize for + +- Safe retries and predictable apply semantics **at the platform layer** +- Clear, explicit configuration that reviewers can reason about without chain-specific trivia +- Consistency with repo-wide datastore and helper conventions +- Minimal, well-bounded abstractions shared across chain families + +## Related documentation + +- [EVM changeset style guide](./evm.md) — EVM-only changeset concerns. +- [Ops / chainlink-deployments style guide](./ops.md) — how runs and pipelines are operated. +- [Changeset Style Guide](../../deployment/docs/style-guide.md) — existing deployment changeset reference (tooling API patterns). +- [Maintaining the Changeset Style Guide](../../deployment/docs/style-guide-contributions.md) — how to add rules, TOC, checklist, and links when editing any changeset-related guide. + +## Scope: what belongs here (and what does not) + +| In scope (this guide) | Out of scope (use instead) | +| ----------------------- | ---------------------------- | +| Idempotency, ref resolution, datastore usage, and other patterns that apply **across** chain families | EVM-specific selectors, contract typing, or adapter quirks → [evm.md](./evm.md) | +| Naming, tagging, and structural conventions for changeset inputs that are not family-specific | How to run jobs/pipelines in **chainlink-deployments** → [ops.md](./ops.md) | +| Shared helpers, qualifiers, and “narrowest clear abstraction” at the platform layer | Deep adapter implementation detail → deployment docs such as [Cross-Family Deployment Architecture](../../deployment/docs/architecture.md) | + +If guidance only makes sense on EVM, it belongs in **evm**, not here. If guidance is about **operating** a deployment (environments, pipeline, review of a run), it belongs in **ops**. + +## Table of contents + + + +## Illustrative rule shape (replace with real team rules) + +The section below shows the **format** only. Delete this section when you add real content, or replace it rule-by-rule. + +### ExampleSectionTitleToReplace + +**Rule:** (One clear sentence: default behavior.) + +**Why it matters:** (Operational / correctness / review impact—do not repeat the rule verbatim.) + +```go +// ❌ BAD: (common mistake) + +// ✅ BETTER: (preferred pattern) +``` + +**Exceptions (optional):** (When the rule does not apply.) + +--- + +## Copyable rule template + +Use this when adding a new section: + +````md +## Short, Stable Section Title + +**Rule:** State the default behavior in one clear sentence. + +**Why it matters:** Explain the correctness, safety, operability, or review impact. + +```go +// ❌ BAD: show the common mistake + +// ✅ BETTER: show the preferred pattern +``` + +**Exceptions (optional):** When the rule does not apply. +```` + +## Direction: topics for the owning team to cover + +Turn these **prompts** into short rules with BAD/BETTER examples where they help. Avoid duplicating topics listed in [evm.md](./evm.md) or [ops.md](./ops.md). + +- Apply paths that must be **safe to retry** without double-writing or double-deploying at the platform layer. +- **Explicit, self-documenting inputs** (field names, tags) for YAML/JSON that are not EVM-specific. +- **Refs and lookups:** resolving `AddressRef`-style inputs before use; avoiding “existence check” helpers that encode the wrong semantics. +- **Caching and stale reads:** when cached reads are unsafe across steps in a changeset. +- **Datastore qualifiers and naming** shared across families. +- **Shared helpers:** when to reuse them versus local duplication. +- **Abstractions:** prefer the narrowest abstraction that stays clear for reviewers. +- **Validation:** avoid redundant validation; validate at the right layer. +- **Conflicts with local conventions:** when consistency wins over a “perfect” abstraction. + +## Contributing and updating + +Teams update this guide as patterns emerge. Follow the maintenance pattern in [Maintaining the Changeset Style Guide](../../deployment/docs/style-guide-contributions.md): keep section titles stable for anchors, update any checklist/TOC you add here, and link to architecture docs instead of copying implementation detail. From a5d81284b38c088fa4ba6c54e2d082ec4349e18e Mon Sep 17 00:00:00 2001 From: Terry Tata Date: Wed, 22 Apr 2026 15:37:47 -0700 Subject: [PATCH 2/2] update platform --- docs/style-guides/platform.md | 243 ++++++++++++++++++++++++++++------ 1 file changed, 204 insertions(+), 39 deletions(-) diff --git a/docs/style-guides/platform.md b/docs/style-guides/platform.md index c213723f8e..566cb42581 100644 --- a/docs/style-guides/platform.md +++ b/docs/style-guides/platform.md @@ -2,65 +2,239 @@ **Owning team (tracking reference):** Terry Tata, Will Winder. -This file is a **harness**: consistent structure and **direction** for what belongs here. **Teams own all substantive rules, examples, and updates**—replace placeholders and grow this document with durable, reviewable guidance. +This guide captures **chain-family-agnostic** practices for writing and reviewing CCIP tooling changesets: input shapes, family routing, datastore conventions, and shared abstractions that should stay consistent no matter which adapter or chain family sits underneath. -## Purpose +It complements—and intentionally does not duplicate—the rules in [Changeset Style Guide](../../deployment/docs/style-guide.md). Read that guide first; the rules here cover platform-level concerns it does not. -Capture **cross-chain** (chain-family-agnostic) practices for writing and reviewing CCIP tooling changesets: shapes, inputs, persistence conventions, and shared abstractions that should stay consistent no matter which adapter or chain family sits underneath. +When a recommendation here conflicts with established local conventions, prefer consistency unless there is a clear correctness, safety, or operability reason not to. -## How to use this guide (once populated) +## How to use this guide -- **While authoring:** use the rules to structure inputs, refs, apply behavior, and error handling before review. -- **Before review or merge:** run the checklist (when added) against changeset PRs touching shared platform code paths. +- **While authoring:** apply the rules when shaping shared changeset code (inputs, registry routing, error wrapping, sequence composition) before review. +- **Before review or merge:** run the [Review Checklist](#review-checklist) against changeset PRs that touch shared platform code paths. -## What this guide should optimize for +## What this guide optimizes for -- Safe retries and predictable apply semantics **at the platform layer** -- Clear, explicit configuration that reviewers can reason about without chain-specific trivia -- Consistency with repo-wide datastore and helper conventions -- Minimal, well-bounded abstractions shared across chain families +- Predictable, deterministic apply semantics at the platform layer +- Configuration that operators can reason about without chain-specific trivia +- Clean separation between platform code and family-specific adapters +- Errors that can be triaged from a log line without re-running the changeset ## Related documentation +- [Changeset Style Guide](../../deployment/docs/style-guide.md) — the foundational changeset rules. Read first. - [EVM changeset style guide](./evm.md) — EVM-only changeset concerns. - [Ops / chainlink-deployments style guide](./ops.md) — how runs and pipelines are operated. -- [Changeset Style Guide](../../deployment/docs/style-guide.md) — existing deployment changeset reference (tooling API patterns). - [Maintaining the Changeset Style Guide](../../deployment/docs/style-guide-contributions.md) — how to add rules, TOC, checklist, and links when editing any changeset-related guide. ## Scope: what belongs here (and what does not) | In scope (this guide) | Out of scope (use instead) | | ----------------------- | ---------------------------- | -| Idempotency, ref resolution, datastore usage, and other patterns that apply **across** chain families | EVM-specific selectors, contract typing, or adapter quirks → [evm.md](./evm.md) | -| Naming, tagging, and structural conventions for changeset inputs that are not family-specific | How to run jobs/pipelines in **chainlink-deployments** → [ops.md](./ops.md) | -| Shared helpers, qualifiers, and “narrowest clear abstraction” at the platform layer | Deep adapter implementation detail → deployment docs such as [Cross-Family Deployment Architecture](../../deployment/docs/architecture.md) | +| Patterns that apply **across** chain families: input shape, family routing, error wrapping, sequence composition | EVM-specific selectors, contract surfaces, or adapter quirks → [evm.md](./evm.md) | +| Conventions for changeset code that does not know (or should not know) which family it runs against | How to run jobs/pipelines in **chainlink-deployments** → [ops.md](./ops.md) | +| Cross-cutting concerns not already covered by the main [Changeset Style Guide](../../deployment/docs/style-guide.md) | Anything already covered there — link to it instead of restating | If guidance only makes sense on EVM, it belongs in **evm**, not here. If guidance is about **operating** a deployment (environments, pipeline, review of a run), it belongs in **ops**. ## Table of contents - + -## Illustrative rule shape (replace with real team rules) +- [Resolve Chain Family From the Selector, Not From Inputs](#resolve-chain-family-from-the-selector-not-from-inputs) +- [Route Family-Specific Logic Through a Registry, Not a `switch family`](#route-family-specific-logic-through-a-registry-not-a-switch-family) +- [Keep Changeset Inputs Fully Serializable](#keep-changeset-inputs-fully-serializable) +- [Wrap Errors With Identifying Context](#wrap-errors-with-identifying-context) +- [Compose Logic Into Sequences; Keep `apply` a Thin Wirer](#compose-logic-into-sequences-keep-apply-a-thin-wirer) +- [Review Checklist](#review-checklist) -The section below shows the **format** only. Delete this section when you add real content, or replace it rule-by-rule. + -### ExampleSectionTitleToReplace +## Resolve Chain Family From the Selector, Not From Inputs -**Rule:** (One clear sentence: default behavior.) +**Rule:** Derive chain family from the chain selector at runtime via `chain_selectors.GetSelectorFamily`. Do not accept `family` (or equivalent) as a separate user-provided input. -**Why it matters:** (Operational / correctness / review impact—do not repeat the rule verbatim.) +**Why it matters:** A chain selector already encodes its family. Adding a parallel `family` input creates a second source of truth that can drift from the selector and silently route work to the wrong adapter. Operators should not be able to misconfigure family at all. ```go -// ❌ BAD: (common mistake) +// ❌ BAD: family is a separate input that can disagree with the selector +type ConfigureRouterInput struct { + ChainSelector uint64 `json:"chainSelector,string" yaml:"chainSelector"` + Family string `json:"family" yaml:"family"` +} + +func apply(e deployment.Environment, in ConfigureRouterInput) error { + reader, ok := registry.GetMCMSReader(in.Family) // could be inconsistent with ChainSelector + if !ok { /* handle error */ } + // ... +} + +// ✅ BETTER: derive family from the selector — single source of truth +type ConfigureRouterInput struct { + ChainSelector uint64 `json:"chainSelector,string" yaml:"chainSelector"` +} + +func apply(e deployment.Environment, in ConfigureRouterInput) error { + family, err := chain_selectors.GetSelectorFamily(in.ChainSelector) + if err != nil { + return fmt.Errorf("unknown chain selector %d: %w", in.ChainSelector, err) + } + reader, ok := registry.GetMCMSReader(family) + if !ok { + return fmt.Errorf("no MCMS reader registered for family %q (selector %d)", family, in.ChainSelector) + } + // ... +} +``` + +**Exceptions:** Tests or fixtures that intentionally exercise a mismatched family (negative cases) may set family explicitly, but production changeset inputs should not. + +--- + +## Route Family-Specific Logic Through a Registry, Not a `switch family` + +**Rule:** When a changeset must do something family-specific (resolve a contract address, read MCMS metadata, build a transaction), look up the behavior through a registry such as `MCMSReaderRegistry` or `ChainFamilyRegistry`. Do not branch on family with a `switch` or `if family == "evm"`. + +**Why it matters:** Family branching forces every new chain family to touch every call site that already cares about family. Registry routing keeps platform changesets closed to family changes—adding a new family is a single registration, not a sweep across the repo. It also keeps platform code from importing chain-specific packages, which protects build hygiene. + +```go +// ❌ BAD: every new family means editing every call site +func getTimelock(e deployment.Environment, sel uint64, in mcms.Input) (string, error) { + family, _ := chain_selectors.GetSelectorFamily(sel) + switch family { + case chain_selectors.FamilyEVM: + return evm.GetTimelock(e, sel, in) + case chain_selectors.FamilySolana: + return solana.GetTimelock(e, sel, in) + // ...add another case for every new family, in every changeset that does this + default: + return "", fmt.Errorf("unsupported family %s", family) + } +} + +// ✅ BETTER: register once, look up everywhere +func getTimelock(e deployment.Environment, sel uint64, in mcms.Input, reg *MCMSReaderRegistry) (string, error) { + family, err := chain_selectors.GetSelectorFamily(sel) + if err != nil { + return "", fmt.Errorf("unknown chain selector %d: %w", sel, err) + } + reader, ok := reg.GetMCMSReader(family) + if !ok { + return "", fmt.Errorf("no MCMS reader registered for family %q (selector %d)", family, sel) + } + ref, err := reader.GetTimelockRef(e, sel, in) + if err != nil { + return "", fmt.Errorf("get timelock ref for selector %d: %w", sel, err) + } + return ref.Address, nil +} +``` + +**Exceptions:** Code that itself defines the registry or tests a single family in isolation may reference a family by name. Cross-family changeset code should not. + +--- + +## Keep Changeset Inputs Fully Serializable + +**Rule:** Public changeset input structs (and any nested types) must be fully serializable to JSON and YAML. Do not put functions, channels, `*deployment.Environment`, live chain handles, or other runtime values into them. Pass-through containers like `map[string]any` (e.g. `FamilyExtras`) must hold only serializable values. + +**Why it matters:** Operators run changesets from YAML/JSON in `chainlink-deployments` pipelines. Anything non-serializable in an input cannot be expressed in config and turns the changeset into something only callable from Go code. It also makes inputs impossible to round-trip for audit, replay, or dry-run review. + +```go +// ❌ BAD: input carries runtime state that cannot be expressed in YAML +type DeployPoolInput struct { + ChainSelector uint64 `json:"chainSelector,string" yaml:"chainSelector"` + Env *deployment.Environment // not serializable + OnDeploy func(addr string) error // not serializable + Extras map[string]any // OK only if every value is serializable +} + +// ✅ BETTER: inputs describe what to do; the runtime is provided separately +type DeployPoolInput struct { + ChainSelector uint64 `json:"chainSelector,string" yaml:"chainSelector"` + TokenRef datastore.AddressRef `json:"tokenRef" yaml:"tokenRef"` + Extras map[string]any `json:"extras,omitempty" yaml:"extras,omitempty"` // serializable values only +} +``` + +A practical check: would `json.Unmarshal(yaml.Marshal(in), &in)` round-trip without losing fields? If not, the input is not pipeline-safe. + +**Exceptions:** Internal sequence inputs that are constructed in-process (e.g. by `ResolveInput`) may carry richer types, because they are not user-facing config. The boundary is the changeset's public input struct. + +--- + +## Wrap Errors With Identifying Context + +**Rule:** When wrapping or returning errors from changeset code, include the identifying context the operator needs to triage the failure: chain selector, contract type, qualifier, and/or sequence ID. Always wrap the underlying error with `%w` so callers can `errors.Is` / `errors.As`. + +**Why it matters:** Changeset failures are often first seen as a single log line in a pipeline run. `"failed to deploy"` forces a re-run with more logging; `"deploy Router on selector 16015286601757825753 (qualifier=router-mainnet): execution reverted"` is usually triageable on the spot. `%w` (instead of `%v`/`%s`) preserves error identity for typed handling upstream. + +```go +// ❌ BAD: nothing here helps an operator find the failing chain or contract +if err := deployRouter(ref); err != nil { + return fmt.Errorf("failed to deploy: %v", err) +} + +// ✅ BETTER: identifying context + wrapped error +if err := deployRouter(ref); err != nil { + return fmt.Errorf( + "deploy %s on selector %d (qualifier=%q): %w", + ref.Type, ref.ChainSelector, ref.Qualifier, err, + ) +} +``` + +When wrapping at a boundary that already adds context (for example, a sequence wrapper that prepends its sequence ID), avoid restating the same fields lower down—prefer adding context once, at the layer that owns it. + +**Exceptions:** Sentinel errors returned for `errors.Is` matching (e.g. `ErrAlreadyDeployed`) may be returned bare; let the caller add context. + +--- -// ✅ BETTER: (preferred pattern) +## Compose Logic Into Sequences; Keep `apply` a Thin Wirer + +**Rule:** Put the substantive logic (deploys, writes, datastore updates, batch-op assembly) into `operations.Sequence`s and reuse them via helpers like `NewFromOnChainSequence`. The changeset's `apply` function should resolve inputs and dependencies, execute the sequence, and build the output—nothing more. + +**Why it matters:** Sequences are the unit of replay, reporting, and reuse in this repo. Logic that lives directly inside `apply` is not visible to the operations bundle, is harder to compose into larger flows, and tends to grow chain-aware branches that should live in a family adapter. Keeping `apply` thin also makes it obvious whether a changeset is safe to retry: idempotency and ordering become properties of the sequence, not the changeset glue. + +```go +// ❌ BAD: apply does the work directly; nothing is reusable or replayable +func apply(e deployment.Environment, cfg DeployRouterCfg) (deployment.ChangesetOutput, error) { + addr, err := deployRouterDirect(e, cfg) + if err != nil { /* handle error */ } + if err := saveRefDirect(e, addr); err != nil { /* handle error */ } + if err := configureDirect(e, addr, cfg); err != nil { /* handle error */ } + return deployment.ChangesetOutput{ /* hand-built */ }, nil +} + +// ✅ BETTER: sequence owns the logic; apply only wires env, deps, and output +var DeployRouter = changesets.NewFromOnChainSequence(changesets.NewFromOnChainSequenceParams[ + DeployRouterInput, DeployRouterDeps, DeployRouterCfg, +]{ + Sequence: deployRouterSeq, // operations.Sequence — contains the actual logic + ResolveInput: func(e deployment.Environment, cfg DeployRouterCfg) (DeployRouterInput, error) { + // build input from cfg + datastore lookups + }, + ResolveDep: func(e deployment.Environment, cfg DeployRouterCfg) (DeployRouterDeps, error) { + // resolve chains/clients from env + }, +}) ``` -**Exceptions (optional):** (When the rule does not apply.) +**Exceptions:** Trivial changesets that genuinely have no on-chain work (for example, a datastore-only annotation) may skip the sequence and use `apply` directly. As soon as there is on-chain or datastore-mutating logic worth replaying, move it into a sequence. --- +## Review Checklist + +Before sending a platform-layer changeset for review, confirm: + +- Confirm chain family is derived from the selector, not taken as input. +- Confirm family-specific behavior is resolved through a registry, not a `switch`. +- Confirm public input structs and their nested types are fully YAML/JSON serializable. +- Confirm errors include identifying context (selector, type, qualifier, sequence ID) and wrap underlying errors with `%w`. +- Confirm substantive logic lives in a sequence and `apply` only resolves, executes, and builds output. +- Confirm none of the rules in the main [Changeset Style Guide](../../deployment/docs/style-guide.md) have regressed (idempotency, ref resolution, datastore qualifiers, etc.). + ## Copyable rule template Use this when adding a new section: @@ -81,20 +255,11 @@ Use this when adding a new section: **Exceptions (optional):** When the rule does not apply. ```` -## Direction: topics for the owning team to cover - -Turn these **prompts** into short rules with BAD/BETTER examples where they help. Avoid duplicating topics listed in [evm.md](./evm.md) or [ops.md](./ops.md). - -- Apply paths that must be **safe to retry** without double-writing or double-deploying at the platform layer. -- **Explicit, self-documenting inputs** (field names, tags) for YAML/JSON that are not EVM-specific. -- **Refs and lookups:** resolving `AddressRef`-style inputs before use; avoiding “existence check” helpers that encode the wrong semantics. -- **Caching and stale reads:** when cached reads are unsafe across steps in a changeset. -- **Datastore qualifiers and naming** shared across families. -- **Shared helpers:** when to reuse them versus local duplication. -- **Abstractions:** prefer the narrowest abstraction that stays clear for reviewers. -- **Validation:** avoid redundant validation; validate at the right layer. -- **Conflicts with local conventions:** when consistency wins over a “perfect” abstraction. - ## Contributing and updating -Teams update this guide as patterns emerge. Follow the maintenance pattern in [Maintaining the Changeset Style Guide](../../deployment/docs/style-guide-contributions.md): keep section titles stable for anchors, update any checklist/TOC you add here, and link to architecture docs instead of copying implementation detail. +Follow the maintenance pattern in [Maintaining the Changeset Style Guide](../../deployment/docs/style-guide-contributions.md): + +- Keep section titles short and stable—they are the anchors used in PR comments. +- Update the [Table of contents](#table-of-contents) and [Review Checklist](#review-checklist) whenever you add, remove, or rename a rule. +- Link to architecture docs and the main [Changeset Style Guide](../../deployment/docs/style-guide.md) instead of duplicating their content. +- Prefer recurring review feedback over one-off bugs as the source of new rules.