Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
48 changes: 48 additions & 0 deletions .claude/rules/coding-style.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
---
paths:
- "src/**"
---

# Coding style (`src/**`)

The reference aesthetic is MakerDAO's dss: small, flat, self-contained contracts where the entire system's core ledger (vat.sol) fits in ~250 lines. Core contracts are groomed like bonsai trees, every line earns its place and what remains is shaped deliberately. These principles apply to all new code and guide refactoring of existing code; `src/core` is held to them most strictly.

## Principles (enforced)

1. **Avoid inheritance for business logic.** Compose via injected interface references (constructor or `file()`), not base contracts.
- Inheriting **generic** behavior is fine: mixins that fit in `src/misc` or similar (`Auth`, `Recoverable`, `ReentrancyProtection`, `BatchedMulticall`, `Escrow`), the contract's own interface(s), and token standards (e.g. `ShareToken is ERC20`).
- What's not fine: inheriting behavior that is business logic related to the contract. Domain logic belongs in the contract itself or behind an injected interface, never in a shared base.
- When extending a parent, call `super.<fn>()` rather than duplicating its logic, since copies diverge as the parent changes.
- Constants or storage used by only one child belong in that child, not the shared base.

2. **Straightforward control flow.** A reader should follow any function top-to-bottom in one pass.
- Guard clauses (`require`) first, then effects, then interactions (CEI).
- Max ~2 levels of nesting. No `else` after a branch that reverts or returns.
- No `try/catch` in core (the gateway's `excessivelySafeCall` boundary is the one sanctioned exception).
- Dispatch chains (`if/else if` on message type or `file` param) are fine, being flat and auditable, but group them into internal helpers per module once they exceed ~10 branches.

3. **SSA (single static assignment).** Each local variable is assigned exactly once.
- Resolve conditional values with a ternary at the declaration site, not by reassigning later.
- Loop iterators and byte-slicing accumulators in explicit loops are exempt.
- Storage struct mutation is what storage is for, so SSA applies to locals only; but if a storage pointer is mutated in 3+ branches (as in BalanceSheet's queue netting), extract the branching into a named pure helper that computes the new value once.

4. **Files ≤ 400 LOC (target).** Aim for contracts and libraries under 400 lines (interfaces exempt). Approaching it is a signal to split by concern or move convenience outward, not to compress formatting. But when splitting via composition isn't feasible (e.g. a vault implementation), one large contract beats splitting through inheritance; never trade file size for an inheritance hierarchy.

5. **YAGNI: keep core minimal, convenience lives in the periphery.** Core exposes one canonical, fully-parameterized function per operation. Wrappers, overloads with defaulted parameters, batched getters, and compatibility shims belong in facade/router/manager contracts (e.g. `SpokeV3_1_0`, `VaultRouter`), never in core.

6. **Aesthetics.** The shape of the file communicates the design.
- Section headers: 3-line blocks for code sections (a `//----` divider line, `// <Label>`, then another `//----` divider line); 1-line `// <Label>` comments for state-variable groups (see Declaration Ordering). Order: Administration → main operations (grouped by flow direction or role) → view methods → internal methods. Small single-concern files (e.g. `Envoy`, `PoolEscrow`, the factories) omit headers entirely.
- Errors and events declared in the interface, never in the contract body. Libraries are the exception: they declare their own errors locally (e.g. `PricingLib.DivisionByZero`, `MessageLib.UnknownMessageType`).
- Documentation lives in the interface (natspec on functions, params, errors, events). In the contract itself: a top-level `@title`/`@notice` natspec block, `/// @inheritdoc` on implementations, and minimal inline comments reserved for non-obvious code.
- State variables and modifiers almost never carry comments; a well-named `onlyManager`/`sender` explains itself. Only comment one when its purpose is genuinely non-obvious from the name and type.
- Symmetry: paired operations (`deposit`/`withdraw`, `issue`/`revoke`, `rely`/`deny`) should mirror each other visually and structurally.
- Names are short verbs and nouns; if a function name needs a conjunction, it does two things.
- Follow the Declaration Ordering rules below for imports and state variables.

## Declaration Ordering (line-length sorting)

Both imports and contract-level declarations are sorted by **full line length, ascending** (shortest line first) within each blank-line-separated group. Sorting is by the whole line, so a shorter type with a longer variable name can sort *after* a longer type with a short name (e.g. `ISpokeMessageSender public sender;` before `ISpokeRegistry public spokeRegistry;`).

- **Imports**: grouped by source area (local `./`, then `../../misc`, then `../core`, then remote libs), each group separated by a blank line and sorted ascending by line length.
- **State variables / constants**: grouped by kind/purpose (constants & immutables, dependency references, mappings/storage), each group separated by a blank line and sorted ascending by line length within the group.
- **Section comments**: add a `// <Label>` above each state-variable group (e.g. `// Dependencies`, `// Assets & prices`, `// Vaults`) when the contract has several functionally-distinct storage groups whose purpose isn't self-evident (as in `MultiAdapter`, `Gateway`, `BatchRequestManager`, `SpokeRegistry`). A single dependency list uses blank-line separation without labels.
35 changes: 31 additions & 4 deletions .claude/rules/registry.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,14 @@ paths:

| Area | Role |
|------|------|
| `abi-registry.js` | Builds `registry/registry-{mainnet,testnet}.json` from `env/*.json`, explorer APIs, deltas vs live registry or `SOURCE_IPFS`. |
| `abi-registry.js` | Builds `registry/registry-{mainnet,testnet}.json` from `env/*.json`, explorer APIs, deltas vs the flattened published chain (walked from the live registry or `SOURCE_IPFS`). |
| `utils/registry-chain.js` | Walks `previousRegistry.ipfsHash` to the base snapshot and flattens layers into the accumulated published state. Call `collectRegistryChain` and take its `accumulated` — it flattens for you; `flattenRegistryChain` is exported for tests and requires oldest → newest ordering. |
| `utils/registry-fetch.js` | **All** published-registry network I/O: `REGISTRY_URLS`, `DNSLINK_HOSTNAMES`, `IPFS_GATEWAYS`, `isValidIpfsHash`, `resolveLiveCid`, `fetchRegistryFromIpfs`, `fetchLiveRegistry`, CID layer cache. Never declare these a second time elsewhere. |
| `utils/registry-delta.js` | Delta membership against the accumulated state: `hasContractChanged`, `computeChainDelta`. |
| `utils/registry-invariants.js` | `checkDeltaInvariants` (projection / minimality / size), `loadEnvChains`. |
| `utils/env-dirs.js` | Which `env/` directory each registry publishes: `env/registry.json` (`resolveEnvironmentDir`, `envFilesOf`, `allEnvFiles`). **Never enumerate `env/mainnet`/`env/testnet` by hand** — `env/testnet-<id>/` directories are other deployments of the same chains, and only the pointed one is published. |
| `walk-registry-chain.js` | Ops CLI: per-layer chain audit (contracts + tombstones per layer, flags snapshot-sized layers). |
| `test/` | `node:test` suite, no extra deps: `cd script/registry && npm test`. |
| `utils/abi-cache.js` | Per-tag Forge ABI cache (worktree + build + `out/` copy); `collectContractTags`, `findAbiInOutput`, `resolveArtifactName` / `artifactNamesForContractKey` (single `ABI_NAME_ALIASES` table) — reusable outside the registry script. |
| `build-abi-cache.js` | CLI: `node script/registry/build-abi-cache.js <tag> [...]` to warm `cache/abi-registry/`. |
| `utils/tag-resolution.js` | Maps env contract `version` → local git tag (`resolveVersionTag`, candidates). |
Expand All @@ -33,18 +40,38 @@ paths:

## Delta mode & deprecations

- **Delta only** (not `--full`): compare each chain to **previous registry** (default live URL or `SOURCE_IPFS`).
- **Deprecated contract:** name exists on previous registry for that chain with non-null `address`, but **missing** from current `env` for that chain → emit `{ address: null, blockNumber: null, txHash: null }`. Skip if live entry already has `address: null` (avoid re-emitting every run).
- **Delta only** (not `--full`): compare each chain to the **accumulated state of the whole published chain** — `collectRegistryChain` + `flattenRegistryChain` from the tip (live URL or `SOURCE_IPFS`) back to the base snapshot, newest layer winning. **Never compare against the tip document alone:** each layer carries only its own changes, so that inflates the delta to a near-full snapshot, makes successive publishes oscillate between complementary halves of the contract set, and hides deprecations.
- **Incomplete chain** (broken pointer, unreachable layer, cycle, depth bound) → **exit non-zero**; `ALLOW_PARTIAL_REGISTRY_CHAIN=1` overrides, `REGISTRY_CHAIN_MAX_DEPTH` raises the fetch bound (default 500; testnet is already ~50 layers).
- **Unreadable tip** (live endpoint + dnslink fallback both down, or unfetchable `SOURCE_IPFS`) → **exit non-zero, no override**: with no baseline every contract reads as new *and* `previousRegistry` stays null, so an outage would pin itself as a base registry. Use `REGISTRY_MODE=full` to publish a base snapshot deliberately.
- **CID hygiene:** `previousRegistry.ipfsHash` comes from published documents, so `fetchRegistryFromIpfs` refuses anything that is not a plain alphanumeric segment before it reaches a gateway URL or the `cache/registry-chain/<cid>.json` path.
- **Wrong-network layer** → `collectRegistryChain` returns `fatal: true` with an empty state, and **no override applies**. Pass `expectedNetwork` from every call site; a layer with no `network` field is unknown, not wrong. Mixing environments would otherwise pass the delta invariants too, since they compare against the same accumulated state.
- **Changed contract:** different address, **or** a `blockNumber` env states that the accumulated state does not resolve to — including when the chain carried `null` and env now has a number (explorer backfill; otherwise the indexer keeps starting that listener from the chain-level `startBlock`). Env *dropping* a `blockNumber` the chain has is **not** a change: the entry would be emitted with `blockNumber: null` and the indexer's merge takes leaf nulls literally, erasing what is published.
- **Deprecated contract:** name exists in the accumulated state for that chain with non-null `address`, but **missing** from current `env` for that chain → emit `{ address: null, blockNumber: null, txHash: null }`. Skip if a later layer already set `address: null` (avoid re-emitting every run).
- **`collectContractTags`:** skip `address === null`; no ABI for tombstones.
- **ABI gaps:** an active env contract whose required artifact names (`artifactNamesForContractKey`) appear in **no** layer's `abis` is pulled into the delta even when its address is unchanged, so the gap heals — `computeChainDelta` returns these in `abiGaps`. `checkDeltaInvariants` errors if any live contract still has no ABI after the delta, and downgrades the minimality error to a warning for a restatement that ships a missing ABI. Retired contracts need no ABI.
- **Layer cache:** published layers are immutable, so `utils/registry-fetch.js` caches them at `cache/registry-chain/<cid>.json` (gitignored, restored in CI via `actions/cache`). `REGISTRY_CHAIN_CACHE_DIR` relocates, `REGISTRY_CHAIN_NO_CACHE=1` bypasses.

## Delta invariants (`utils/registry-invariants.js`)

Structural validity does not imply a correct delta — a delta re-stating published contracts is valid JSON, which is how a near-full snapshot shipped three times. `validate-registry.js` also asserts, against the flattened chain:

- **Projection** (error): `flatten(published chain + delta)` must equal `env/*.json` — no missing change, no invented address, no wrong `blockNumber` for a contract whose blockNumber env states, no contract dropped from env without a tombstone.
- **Minimality** (error): no delta entry may restate the published address + blockNumber, and no re-tombstoning of an already-retired contract.
- **Size** (warning): delta carrying >30% of published contracts.

Skipped for `full` mode, under `SKIP_LIVE_REGISTRY_CHECK=1`, and when the chain cannot be reconstructed (warns instead — generation already fails hard there). Stats land in `summary.delta` of the sidecar and the PR comment's **Delta size** row.

## Env contracts

- Preserve **`version`** when writing env after explorer fetch (`fetchedNewData` path); stripping it breaks the next run and CI.
- Mainnet/testnet env entries should be **objects** with `address` + `version` (validator rejects bare address strings).
- `network.environment` restates the directory (`testnet-rev2` for `env/testnet-rev2/`) and `network.namespace` is required; both are checked by `validate-env-schema.js`. Neither reaches the published registry (`abi-registry.js` strips them with the other deploy-time fields).
- Switching which deployment a registry publishes (`env/registry.json`) changes every address in it — run that build with `REGISTRY_MODE=full`.

## CI

- `.github/workflows/registry.yml`: `git fetch --tags`, `validate-env-schema.js`, `validate-env-contract-version-tags.js`, `abi-registry.js`, `validate-registry.js`, PR preview comment (no separate pre-build of `./out` at deployment commit).
- `.github/workflows/registry.yml`: `git fetch --tags`, `npm test`, restore the `cache/registry-chain` layer cache, `validate-env-schema.js`, `validate-env-contract-version-tags.js`, `abi-registry.js`, `validate-registry.js`, PR preview comment (no separate pre-build of `./out` at deployment commit).
- **Disabled (`if: false` on `generate`) until the v3.3 deployment writes the configs.** Root-only configs read as "everything else retired" and publish tombstones for the whole protocol — that happened on 2026-08-19. Never re-enable while a published environment's configs record only `root`; `live-checks.yml` runs `validate-env-schema.js` meanwhile.

## Cursor vs Claude Code

Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ jobs:
FOUNDRY_PROFILE: ci

coverage:
# Temporarily disabled: the runner has been flaking with mid-run shutdown/cancellation
# unrelated to test correctness. Re-enable once that infra issue is resolved.
if: false
runs-on: ubuntu-latest
permissions:
contents: read
Expand Down
25 changes: 18 additions & 7 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,32 @@ lcov.info
## Recon
/medusa
/crytic-export
/echidna
/echidna*
!/echidna-cov.txt
recon.json
**/*.log
**/anvil/*.json
**/env/anvil.json
**/anvil
**/anvil/*.log
# Written by script/anvil/anvil.sh from the fixtures next to it, one directory per run. env/anvil/ is where
# runs recorded themselves before the id was in the directory name; anvil.sh removes it, and it stays
# ignored so a checkout that has not run it since is not left with an untracked directory
env/anvil-*/
env/anvil/
**/*.bak
## Recon Separate Configs
/medusa-core
/medusa-aggregator
/registry/**/*.json
.cursor

## Spell validation cache
spell-cache/
## Benchmarking
src/admin/GasService_temp.*.sol

## Local tooling artifacts (Claude Code skills & generated reports)
.claude/skills/
x-ray/

## Tooling of the live branch (registry pipeline, spell validation). Ignored here too, so switching
## between branches with build artifacts present leaves the tree clean either way
spell-cache/
script/registry/node_modules
script/registry/package-lock.json
/registry/**/*.json
3 changes: 3 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,6 @@
[submodule "lib/enso-weiroll"]
path = lib/enso-weiroll
url = https://github.com/ensobuild/enso-weiroll
[submodule "lib/create3-gate"]
path = lib/create3-gate
url = https://github.com/centrifuge/create3-gate
Loading
Loading