|
1 | | -# Copilot instructions — doublezero |
| 1 | +Write every review comment in Simplified Technical English. |
2 | 2 |
|
3 | | -DoubleZero is a protocol for building and operating high-performance, permissionless networks — a |
4 | | -global dedicated-fiber network for distributed systems like blockchains. Service contributors |
5 | | -deploy devices and register them and their links onchain; users connect over GRE tunnels and receive |
6 | | -optimized routes via BGP, in unicast (IBRL) or multicast modes. |
| 3 | +Use the active voice. Name the actor. Keep one idea in one sentence. Do not use an internal type or field name as a noun. Use a real value when you can. Do not use = as a verb. Do not coin a term. Do not use em dashes. |
7 | 4 |
|
8 | | -Two distinct chains are in play. The **DoubleZero Ledger** is the protocol's own Solana-based |
9 | | -cluster, run by its own validators; the serviceability, telemetry, geolocation, and record |
10 | | -programs in this repo are deployed there, reached via `ledger_rpc_url`. |
11 | | -**Solana L1** is a separate network reached via `solana_l1_rpc_url`: it carries the 2Z utility |
12 | | -token and hosts the shred-subscription program. The environment mapping between the two is not the |
13 | | -identity — DZ testnet's shred-subscription program lives on Solana devnet, not Solana testnet (see |
14 | | -`config/src/constants.rs`). |
| 5 | +Write each finding in this shape: |
15 | 6 |
|
16 | | -The repository is a hybrid Rust/Go monorepo: the Ledger programs and their CLI/SDK in |
17 | | -`smartcontract/` and `crates/`, the client daemon and controller in `client/` and `controlplane/`, |
18 | | -telemetry in `telemetry/`, end-to-end tests in `e2e/`, and read-only account decoders for |
19 | | -Go/Python/TypeScript in `sdk/`. |
| 7 | +**Issue** |
| 8 | +What fails, in one or two sentences. |
20 | 9 |
|
21 | | -## Review posture |
| 10 | +**Context** |
| 11 | +What the user or operator does, and the state that leads to the failure. |
22 | 12 |
|
23 | | -Report defects, not impressions. Every finding must name a concrete failure scenario — the input or |
24 | | -state that triggers it and the wrong behavior that results — and rate it by consequence. Label a nit |
25 | | -as a nit, and prefer a small number of well-evidenced findings over broad coverage. Check the |
26 | | -sibling implementation and the callers before reporting; several of the rules below exist because |
27 | | -the defect was only visible from outside the diff. |
28 | | - |
29 | | -Most pull requests are fine. Reporting nothing is a valid and useful outcome — say the change looks |
30 | | -sound and stop there. Do not pad a review to look thorough, do not summarize back what the diff |
31 | | -already says, and do not raise a rule from these files merely because the diff touched the area it |
32 | | -covers. Every rule here describes a defect to look for, not a checklist to walk; a rule with no |
33 | | -matching problem in the diff has nothing to say. |
34 | | - |
35 | | -A pre-existing defect is in scope when the diff widens its window, rewrites the lines that carry it, |
36 | | -or adds a second copy of the pattern. Say explicitly that it predates the change and what the change |
37 | | -did to make it matter, rate it accordingly — usually non-blocking — and do not attribute it to the |
38 | | -author. |
39 | | - |
40 | | -When the root cause sits in a file the diff does not touch, anchor the comment on the nearest |
41 | | -changed line, say where the fix belongs, and say why this diff is what surfaces it. |
42 | | - |
43 | | -State what you could not verify. Do not claim a test fails on the base branch, that a build is |
44 | | -clean, or that a downstream consumer breaks, unless the diff itself shows it — ask the author to |
45 | | -confirm instead. |
46 | | - |
47 | | -## Universal rules |
48 | | - |
49 | | -- Match the sibling. When a diff touches one member of a create/update/delete family or an A/B pair, |
50 | | - the other members define the convention: guard placement, error variants, logging, layout |
51 | | - comments, fail-fast strength. Two behaviors for one condition inside a single file is a finding. |
52 | | -- A comment, doc comment, RFC row, or CHANGELOG line that the diff makes false is worth reporting |
53 | | - even when the code is correct. Rate it by what a reader would do wrong because of it: a rationale |
54 | | - paragraph that survived a design pivot and now invites someone to "fix" working code is a real |
55 | | - finding; a stale index in a layout comment is a nit. |
56 | | -- Claims made in the PR description and the CHANGELOG must be supported by the diff. Flag overstated |
57 | | - ones: "asserts the specific error" when every branch returns the same code, "tightened |
58 | | - expectations" when assertions were in fact loosened, "covers X" when no test reaches X. |
59 | | -- A new comparison, field, or branch that nothing reaches is a finding. Check the callers and say |
60 | | - which fix applies — wire it up (a caller gates on `Equal` before ever consulting it) or delete it |
61 | | - (nothing will ever reach it). |
62 | | -- Code that reads onchain state must use the endpoint for the chain that state lives on — the |
63 | | - Ledger programs via `ledger_rpc_url`, the shred-subscription program and anything 2Z via |
64 | | - `solana_l1_rpc_url` or its shred-specific override. Pointing a lookup at the wrong cluster does |
65 | | - not error: the account simply does not exist there, so the caller silently takes its not-found |
66 | | - path. Check the environment mapping rather than assuming DZ testnet means Solana testnet. |
67 | | -- Fail-open behavior and discarded errors (`|| true`, `_ = err`, `.map_err(|_| ...)`, a swallowed |
68 | | - parse error) must be deliberate and must say so in a comment at the decision point. |
69 | | -- Use "onchain" as one word, never "on-chain". |
| 13 | +**Proposed Fix** |
| 14 | +What to change, in one sentence. |
0 commit comments