diff --git a/.github/workflows/config-drift.yml b/.github/workflows/config-drift.yml index e2aac30..dfe8429 100644 --- a/.github/workflows/config-drift.yml +++ b/.github/workflows/config-drift.yml @@ -97,7 +97,7 @@ jobs: code=$? echo "$out" if [ "$code" -eq 1 ]; then - echo "::warning::roles-check found authority drift - a declared role no longer matches the live chain; investigate (docs/roles.md drift runbook)" + echo "::warning::roles-check exit 1: authority drift OR a refused read (a declared value that could not be read) - see the [FAIL] lines; docs/roles.md drift runbook, verdict (c) for refused reads" elif [ "$code" -eq 2 ]; then echo "::warning::roles-check: an RPC was unavailable - authority drift could not be determined (flake/missing secret, not drift)" elif echo "$out" | grep -q "checked nothing"; then diff --git a/README.md b/README.md index ce65bb8..b163a75 100644 --- a/README.md +++ b/README.md @@ -70,6 +70,8 @@ its `rpcEnv` and `KEYSTORE_NAME` in `.env`, then `source .env`). make deploy-token CHAIN= TOKEN_NAME="My Token" TOKEN_SYMBOL=MTK make deploy-pool CHAIN= # BurnMint pool; token resolved from the registry make doctor CHAIN= # verify on-chain code at the recorded addresses + # doctor ends VERIFIED (exit 0), INCOMPLETE, or FAILED (both nonzero); a run without the chain's + # RPC env is INCOMPLETE by design - see docs/operations/chains.md ``` `make deploy-new-chain CHAIN= SELECTOR=` runs `add-chain -> deploy-token -> deploy-pool -> diff --git a/docs/config-architecture.md b/docs/config-architecture.md index ab5f989..8ceba5c 100644 --- a/docs/config-architecture.md +++ b/docs/config-architecture.md @@ -32,7 +32,7 @@ that need it, never exported. Targets that touch the API need only `curl` + `jq` | `fmt-config` | Repair the canonical JSON of **both** stores: `config/chains/*.json` (`jq --indent 2 -S`, trailing newline) and the project store in every group (`project/*.json` **and** `project/*/*.json`, `jq --indent 2 -S`, **no** trailing newline). Repair tool only - never a required step | - | `jq` over every config + project file (all groups) | | `snapshot-chain` | Backfill the declared `roles{}` authority block FROM the live chain into the project store (the bootstrap for `roles-check`; see [`roles.md`](roles.md)) | `CHAIN=` (required); `GROUP=` (optional) scopes to a token group; `TOKEN=` `TOKEN_POOL=` `TAR=` `SCAN_FROM_BLOCK=` (optional overrides) | `SnapshotChain.s.sol --sig "run(string)" ` → canonicalize the project file. Exit: pass/fail via make | -| `roles-check` | READ-ONLY reconcile of a chain's declared `roles{}` vs the live chain (see [`roles.md`](roles.md)). Make remaps the script's exit code to pass/fail; CI calls the script directly for the full contract: 0 `CLEAN` / 1 `ROLES_DRIFT` / 2 `RPC_UNAVAILABLE` | `CHAIN=` (optional - no arg checks every declaring chain); `GROUP=` (optional) scopes to one token group | `bash script/config/roles-check.sh []` → `RolesCheck.s.sol --sig "run(string)"` per chain | +| `roles-check` | READ-ONLY reconcile of a chain's declared `roles{}` vs the live chain (see [`roles.md`](roles.md)). Make remaps the script's exit code to pass/fail; CI calls the script directly for the full contract: 0 `CLEAN` / 1 `ROLES_DRIFT` (a declared value mismatches, or its read was refused - see [`roles.md`](roles.md) verdict (c)) / 2 `RPC_UNAVAILABLE` | `CHAIN=` (optional - no arg checks every declaring chain); `GROUP=` (optional) scopes to one token group | `bash script/config/roles-check.sh []` → `RolesCheck.s.sol --sig "run(string)"` per chain | | `roles-check-all`| The same reconcile for every chain that declares `roles{}`, across the default group AND every `project//` (exit contract as above) | - | `bash script/config/roles-check.sh` (no args, no group filter) | | `clean-scratch` | Remove test-scratch fixtures (`zz-scratch-*`, `zz-tt-*`, `local-*`) from `config/chains`, `project`, and `history` via explicit patterns - never `git clean -X`, which would also delete real gitignored project state | - | explicit `rm` patterns | @@ -240,9 +240,11 @@ lane); the doctor's **mesh rung** proves the property across the whole directory **to** e.g. `solana-devnet` is checked for resolution but exempt from reciprocity (a SKIP, not a FAIL). The mesh rung proves the committed policy agrees with itself; the **lanes rung** (the doctor's last -rung) proves it agrees with the **chain**. It is RPC-gated like the TAR reconciliation (a clean SKIP -when the chain's `rpcEnv` is unset) and pool-gated (a SKIP naming `make adopt-token` / the deploy -scripts when no `tokenPool` is recorded in the registry). With a fork and a pool it resolves the pool's +rung) proves it agrees with the **chain**. It is RPC-gated like the TAR reconciliation (a +`[SKIP] UNVERIFIED` when the chain's `rpcEnv` is unset - the run then ends INCOMPLETE, see +[operations/chains.md](operations/chains.md)) and pool-gated (a plain SKIP naming `make adopt-token` / +the deploy scripts when no `tokenPool` is recorded in the registry: nothing deployed means nothing +checkable, so that skip never makes the run INCOMPLETE). With a fork and a pool it resolves the pool's contract version (`PoolVersion.tryResolve`; an unrecognized version WARNs and reads degrade to best effort) and reconciles **both directions**: diff --git a/docs/config-schema.md b/docs/config-schema.md index be8afb9..79f5f33 100644 --- a/docs/config-schema.md +++ b/docs/config-schema.md @@ -589,6 +589,16 @@ carry an honest **`complete` marker**: `true` only when the token enumerates its `snapshot-chain SCAN_FROM_BLOCK=` event scan proved the list; `false` (candidate seed) otherwise, and the auditor WARNs so a partial list is never read as full. +**Absence is the unread state.** A chain-readable field the snapshot could not read is **not written** - +never written as `0x0` / `false` / `[]`, which are what a failed probe returns and are indistinguishable +from real values once on disk. The snapshot logs each such field as `UNREAD` and omits it; the audit then +refuses to compare a declared value against a read that failed (`could not be read, so nothing was +compared` - a FAIL, not a PASS), and a declared `hooks`/`lockbox` contract that answers none of its +getters fails as unauditable outright. A declaration can still carry zeros +that were produced by failed reads (a snapshot written by older tooling, or a hand edit): they were +never facts, and against a still-unreadable contract they FAIL instead of reconciling - re-run +`make snapshot-chain` on a working RPC to rebuild the block from actual reads. + The token block **dispatches on a declared `type`**, because the admin model differs per template - the engine never assumes one: diff --git a/docs/guides/expand-the-mesh.md b/docs/guides/expand-the-mesh.md index 453652b..993baf6 100644 --- a/docs/guides/expand-the-mesh.md +++ b/docs/guides/expand-the-mesh.md @@ -51,8 +51,8 @@ verification loop for `2N` lanes, not a fixed two. ## Prove the mesh is complete -`make doctor` at 0 FAIL on every chain does not prove the mesh transfers. The doctor checks two things, -and neither reads the remote pool address that a release validates against: +`make doctor` ending VERIFIED on every chain does not prove the mesh transfers. The doctor checks two +things, and neither reads the remote pool address that a release validates against: - The **mesh rung** checks declaration reciprocity only: for each declared lane it confirms the remote's `config/chains/.json` exists and its stored `remoteSelector` matches, and that the reciprocal @@ -61,8 +61,8 @@ and neither reads the remote pool address that a release validates against: rate-limit and policy value matches the live local pool. It does not read which remote pool address the local pool has registered for that lane. -So a pool can be doctor 0-FAIL for a lane, with `isSupportedChain(remote)` true and every rate limit -matching, while the remote pool it has registered points at a decommissioned pool. A transfer over that +So a pool can pass the doctor (VERIFIED) for a lane, with `isSupportedChain(remote)` true and every +rate limit matching, while the remote pool it has registered points at a decommissioned pool. A transfer over that lane then reverts `InvalidSourcePoolAddress` on release, because the destination pool validates the message's `sourcePoolAddress` against its registered remote pools and the wired-in pool is not in the set. See [Remove a remote pool](../operations/lanes-and-remotes.md#remove-a-remote-pool) for the same check at diff --git a/docs/guides/health-check.md b/docs/guides/health-check.md index 4c05a71..54f7ad2 100644 --- a/docs/guides/health-check.md +++ b/docs/guides/health-check.md @@ -22,6 +22,8 @@ the complete gate. ## What a clean run looks like -`make doctor` green up to the expected project-placeholder warnings, `make roles-check` at exit 0, every -contract verified on its explorer, and a `SUCCESS` smoke transfer each way. Anything less is a launch -blocker, not a warning to note and move past. +`make doctor` ends VERIFIED (with any warnings beyond the expected project-placeholder ones +investigated), `make roles-check` at exit 0, every contract verified on its explorer, and a `SUCCESS` +smoke transfer each way. A doctor run that ends INCOMPLETE checked less than it claims - close the +`[SKIP] UNVERIFIED` gaps (usually an unset RPC env) before reading anything into it. Anything less is a +launch blocker, not a warning to note and move past. diff --git a/docs/operations/chains.md b/docs/operations/chains.md index 4cf4564..0f81a03 100644 --- a/docs/operations/chains.md +++ b/docs/operations/chains.md @@ -37,7 +37,17 @@ with `_` so the derived `rpcEnv` is a valid shell variable name (`0g-testnet-gal `_0G_TESTNET_GALILEO_1`). `add-chain` prints the exact `chainNameIdentifier` and `rpcEnv` names it generated, plus your next steps: add the chain's RPC env var to `.env`, review the generated defaults in the config file, wire a lane with `make add-lane`, and re-run the doctor until it -reports 0 FAIL. +reports VERIFIED. + +The doctor ends in one of three verdicts, not a pass/fail boolean. FAILED means a check ran and +contradicted the config - including a declared `roles{}` value whose read failed, which the roles rung +refuses as a FAIL rather than skipping. INCOMPLETE means no check failed but an infrastructure gap kept +checks from running at all - an unset RPC env, a pool that did not answer the lanes reverse-check - and +the run exits nonzero rather than clean: a wrapper reading exit 0 there would treat an unchecked +chain as a healthy one. Each counted gap is tagged +`[SKIP] UNVERIFIED` in the output. VERIFIED (exit 0) means every applicable check ran. Designed +absences (a non-EVM chain's EVM rungs, an optional block that is not declared) are plain SKIPs and +never make a run INCOMPLETE. Names may contain underscores, which some CCIP selectorNames use (e.g. `binance_smart_chain-mainnet`); pass the name from `make discover` verbatim. diff --git a/docs/operations/dynamic-config.md b/docs/operations/dynamic-config.md index 55ee9f3..4328f77 100644 --- a/docs/operations/dynamic-config.md +++ b/docs/operations/dynamic-config.md @@ -31,5 +31,10 @@ ROUTER=0xYourRouterAddress \ | Env var | Required | Description | | ------------------ | -------- | -------------------------------------------------------------------------------------------------------------------------------------- | | `ROUTER` | No | The CCIP Router address to set on the pool (default: current on-chain value) | -| `RATE_LIMIT_ADMIN` | No | Rate limit admin address (default: current on-chain value, then broadcaster) | -| `FEE_ADMIN` | No | Fee admin address (default: current on-chain value, then broadcaster). Set to `address(0)` to restrict fee withdrawal to the owner only | +| `RATE_LIMIT_ADMIN` | No | Rate limit admin address (default: current on-chain value, verbatim) | +| `FEE_ADMIN` | No | Fee admin address (default: current on-chain value, verbatim). Set to `address(0)` to restrict fee withdrawal to the owner only | + +An unset variable preserves the pool's current value exactly, `address(0)` included. The call writes the +whole struct, so any other default would turn a one-field update into a silent grant of the others: a +broadcaster fallback, for example, would hand both admin slots to the acting account on a `ROUTER`-only +run whenever they are unset on chain. diff --git a/docs/operations/finality.md b/docs/operations/finality.md index cdc5090..337ea4d 100644 --- a/docs/operations/finality.md +++ b/docs/operations/finality.md @@ -109,3 +109,6 @@ it is not. | `INBOUND_RATE_LIMIT_CAPACITY` | No | uint128, inbound token bucket capacity (fast finality bucket) | | `INBOUND_RATE_LIMIT_RATE` | No | uint128, inbound token bucket refill rate (tokens/second) | | `INBOUND_RATE_LIMIT_ENABLED` | No | Override `isEnabled` explicitly (`true`/`false`; defaults to `true` when `CAPACITY` or `RATE` are set) | + +The rate-limit inputs are all-or-nothing per direction; a partial set is refused naming the missing +variable. The full rule is in [Rate limits](rate-limits.md). diff --git a/docs/operations/lanes-and-remotes.md b/docs/operations/lanes-and-remotes.md index d4f4b33..f3a386d 100644 --- a/docs/operations/lanes-and-remotes.md +++ b/docs/operations/lanes-and-remotes.md @@ -169,6 +169,9 @@ DEST_CHAIN=MANTLE_SEPOLIA \ | `INBOUND_RATE_LIMIT_RATE` | No | Token bucket refill rate (tokens/second) for inbound transfers | | `INBOUND_RATE_LIMIT_ENABLED` | No | Override `isEnabled` explicitly (`true`/`false`; defaults to `true` when CAPACITY or RATE are set) | +These env buckets follow the same all-or-nothing rule per direction as `UpdateRateLimiters`; see +[Rate limits](rate-limits.md). + `ApplyChainUpdates` only configures the standard finality rate limit bucket. To configure the fast finality bucket, run `UpdateRateLimiters` with `FAST_FINALITY=true` after the lane is set up (see [Rate limits](rate-limits.md)). diff --git a/docs/operations/rate-limits.md b/docs/operations/rate-limits.md index 5233a9e..cfccd59 100644 --- a/docs/operations/rate-limits.md +++ b/docs/operations/rate-limits.md @@ -35,6 +35,14 @@ is no need to pass a separate enabled flag. A bucket is enabled automatically wh `*_RATE` is set; pass `OUTBOUND_RATE_LIMIT_ENABLED=false` (or the inbound equivalent) to disable it explicitly. +Within a direction the inputs are all-or-nothing: an enabled bucket needs `*_CAPACITY` and `*_RATE` +supplied together, and a run that supplies only one is refused naming the missing variable. An unset +field never becomes a `0` written on chain - a capacity without a rate would enable a bucket that lets N +tokens through and then stays shut with `TokenRateLimitReached`, while the run prints success. The +one exception is `*_ENABLED=false` on its own, which is a complete instruction: a disabled bucket's +capacity and rate are 0 by protocol rule. Values that do not fit `uint128` are refused rather than +truncated into a different live value. + The golden path for v2 lanes is to declare the policy in the local chain config and apply from the declaration: with no rate-limit env vars, a direction resolves from the `lanes{}` entry in `project/.json` (the standard bucket from `capacity`/`rate` plus the optional `inbound{}` block, diff --git a/docs/primitives/_meta.json b/docs/primitives/_meta.json index e918324..686ce38 100644 --- a/docs/primitives/_meta.json +++ b/docs/primitives/_meta.json @@ -4,7 +4,15 @@ "when_to_use": "Configure a remote chain on a token pool: the remote pool addresses, the remote token address, and both rate limiter configs.", "preconditions": "The executing account owns the pool. Every remote address is already chain-family encoded (EVM: abi.encode(address); SVM: raw 32 bytes).", "postconditions": "The chain's entry on the pool matches the payload exactly.", - "failure_modes": "REPLACE, NOT MERGE: re-applying an already-configured chain removes its whole entry first, including every registered remote pool and both rate limiter configs, so anything the payload omits is lost. Mid-migration that drops the old remote pool a lane still needs (in-flight messages then fail InvalidSourcePoolAddress) or un-throttles a lane throttled on purpose. Repeating a selector inside one payload reverts NonExistentChain, because the first removal already took it out." + "failure_modes": "REPLACE, NOT MERGE: re-applying an already-configured chain removes its whole entry first, including every registered remote pool and both rate limiter configs, so anything the payload omits is lost. Mid-migration that drops the old remote pool a lane still needs (in-flight messages then fail InvalidSourcePoolAddress) or un-throttles a lane throttled on purpose. Repeating a selector inside one payload reverts NonExistentChain, because the first removal already took it out.", + "inputs": { + "OUTBOUND_RATE_LIMIT_CAPACITY": "uint128 token-bucket capacity. Per direction the inputs are all-or-nothing: an enabled bucket needs CAPACITY and RATE together, and a partial set is refused naming the missing variable.", + "OUTBOUND_RATE_LIMIT_RATE": "uint128 token-bucket refill rate. An enabled bucket needs CAPACITY and RATE together.", + "OUTBOUND_RATE_LIMIT_ENABLED": "true/false; defaults to true when CAPACITY or RATE are set. false stands alone as a disable.", + "INBOUND_RATE_LIMIT_CAPACITY": "uint128 token-bucket capacity (inbound). Same all-or-nothing rule per direction.", + "INBOUND_RATE_LIMIT_RATE": "uint128 token-bucket refill rate (inbound).", + "INBOUND_RATE_LIMIT_ENABLED": "true/false; defaults to true when CAPACITY or RATE are set. false stands alone as a disable." + } }, "ClaimAndAcceptAdmin": { "when_to_use": "Register a token you control in the TokenAdminRegistry in one atomic step. Prefer this over separate ClaimAdmin then AcceptAdminRole when you want a single Safe batch: AcceptAdminRole on its own preflight-requires the pending administrator to already be set, so the two standalone steps cannot be deferred into one batch, whereas this pair executes together.", @@ -12,5 +20,32 @@ "postconditions": "The TokenAdminRegistry administrator for the token is the executing account.", "example": "CCIP_ADMIN_ADDRESS=0xYourAdmin forge script script/setup/ClaimAndAcceptAdmin.s.sol --rpc-url $ETHEREUM_SEPOLIA_RPC_URL --account $KEYSTORE_NAME --broadcast", "failure_modes": "Reverts if no claim path resolves the executing account as the token admin, or if the token is already registered to a different administrator." + }, + "SetDynamicConfig": { + "inputs": { + "ROUTER": "Optional. Unset preserves the pool's current on-chain router verbatim.", + "RATE_LIMIT_ADMIN": "Optional. Unset preserves the current on-chain value verbatim, address(0) included.", + "FEE_ADMIN": "Optional. Unset preserves the current value verbatim; an explicit address(0) restricts fee withdrawal to the owner." + } + }, + "UpdateRateLimiters": { + "inputs": { + "OUTBOUND_RATE_LIMIT_CAPACITY": "uint128 token-bucket capacity. Per direction the inputs are all-or-nothing: an enabled bucket needs CAPACITY and RATE together, and a partial set is refused naming the missing variable.", + "OUTBOUND_RATE_LIMIT_RATE": "uint128 token-bucket refill rate. An enabled bucket needs CAPACITY and RATE together.", + "OUTBOUND_RATE_LIMIT_ENABLED": "true/false; defaults to true when CAPACITY or RATE are set. false stands alone as a disable.", + "INBOUND_RATE_LIMIT_CAPACITY": "uint128 token-bucket capacity (inbound). Same all-or-nothing rule per direction.", + "INBOUND_RATE_LIMIT_RATE": "uint128 token-bucket refill rate (inbound).", + "INBOUND_RATE_LIMIT_ENABLED": "true/false; defaults to true when CAPACITY or RATE are set. false stands alone as a disable." + } + }, + "SetFinalityConfig": { + "inputs": { + "OUTBOUND_RATE_LIMIT_CAPACITY": "uint128 token-bucket capacity. Per direction the inputs are all-or-nothing: an enabled bucket needs CAPACITY and RATE together, and a partial set is refused naming the missing variable.", + "OUTBOUND_RATE_LIMIT_RATE": "uint128 token-bucket refill rate. An enabled bucket needs CAPACITY and RATE together.", + "OUTBOUND_RATE_LIMIT_ENABLED": "true/false; defaults to true when CAPACITY or RATE are set. false stands alone as a disable.", + "INBOUND_RATE_LIMIT_CAPACITY": "uint128 token-bucket capacity (inbound). Same all-or-nothing rule per direction.", + "INBOUND_RATE_LIMIT_RATE": "uint128 token-bucket refill rate (inbound).", + "INBOUND_RATE_LIMIT_ENABLED": "true/false; defaults to true when CAPACITY or RATE are set. false stands alone as a disable." + } } } diff --git a/docs/primitives/catalog.json b/docs/primitives/catalog.json index 0598f90..16e3802 100644 --- a/docs/primitives/catalog.json +++ b/docs/primitives/catalog.json @@ -335,7 +335,13 @@ "destructive": false, "inputs": [ "DEST_CHAIN", - "DEST_CHAIN_SELECTOR" + "DEST_CHAIN_SELECTOR", + "INBOUND_RATE_LIMIT_CAPACITY", + "INBOUND_RATE_LIMIT_ENABLED", + "INBOUND_RATE_LIMIT_RATE", + "OUTBOUND_RATE_LIMIT_CAPACITY", + "OUTBOUND_RATE_LIMIT_ENABLED", + "OUTBOUND_RATE_LIMIT_RATE" ] }, { @@ -444,7 +450,13 @@ "inputs": [ "DEST_CHAIN", "DEST_CHAIN_SELECTOR", - "FAST_FINALITY" + "FAST_FINALITY", + "INBOUND_RATE_LIMIT_CAPACITY", + "INBOUND_RATE_LIMIT_ENABLED", + "INBOUND_RATE_LIMIT_RATE", + "OUTBOUND_RATE_LIMIT_CAPACITY", + "OUTBOUND_RATE_LIMIT_ENABLED", + "OUTBOUND_RATE_LIMIT_RATE" ] }, { @@ -799,6 +811,12 @@ "DEST_CHAIN_SELECTOR", "DEST_TOKEN", "DEST_TOKEN_POOL", + "INBOUND_RATE_LIMIT_CAPACITY", + "INBOUND_RATE_LIMIT_ENABLED", + "INBOUND_RATE_LIMIT_RATE", + "OUTBOUND_RATE_LIMIT_CAPACITY", + "OUTBOUND_RATE_LIMIT_ENABLED", + "OUTBOUND_RATE_LIMIT_RATE", "VIA_JSON_FILE" ] }, diff --git a/docs/primitives/dynamic-config/SetDynamicConfig.md b/docs/primitives/dynamic-config/SetDynamicConfig.md index de496f1..074ee76 100644 --- a/docs/primitives/dynamic-config/SetDynamicConfig.md +++ b/docs/primitives/dynamic-config/SetDynamicConfig.md @@ -17,9 +17,9 @@ Updates the dynamic configuration of a TokenPool (router, rateLimitAdmin, feeAdm | Env var | Description | | --- | --- | -| `FEE_ADMIN` | See the script header. | -| `RATE_LIMIT_ADMIN` | See the script header. | -| `ROUTER` | See the script header. | +| `FEE_ADMIN` | Optional. Unset preserves the current value verbatim; an explicit address(0) restricts fee withdrawal to the owner. | +| `RATE_LIMIT_ADMIN` | Optional. Unset preserves the current on-chain value verbatim, address(0) included. | +| `ROUTER` | Optional. Unset preserves the pool's current on-chain router verbatim. | ## Reference diff --git a/docs/primitives/finality-config/SetFinalityConfig.md b/docs/primitives/finality-config/SetFinalityConfig.md index 0ec9946..d28ec66 100644 --- a/docs/primitives/finality-config/SetFinalityConfig.md +++ b/docs/primitives/finality-config/SetFinalityConfig.md @@ -19,6 +19,12 @@ Sets the allowed finality configuration on a TokenPool, and optionally updates r | --- | --- | | `DEST_CHAIN` | See the script header. | | `DEST_CHAIN_SELECTOR` | See the script header. | +| `INBOUND_RATE_LIMIT_CAPACITY` | uint128 token-bucket capacity (inbound). Same all-or-nothing rule per direction. | +| `INBOUND_RATE_LIMIT_ENABLED` | true/false; defaults to true when CAPACITY or RATE are set. false stands alone as a disable. | +| `INBOUND_RATE_LIMIT_RATE` | uint128 token-bucket refill rate (inbound). | +| `OUTBOUND_RATE_LIMIT_CAPACITY` | uint128 token-bucket capacity. Per direction the inputs are all-or-nothing: an enabled bucket needs CAPACITY and RATE together, and a partial set is refused naming the missing variable. | +| `OUTBOUND_RATE_LIMIT_ENABLED` | true/false; defaults to true when CAPACITY or RATE are set. false stands alone as a disable. | +| `OUTBOUND_RATE_LIMIT_RATE` | uint128 token-bucket refill rate. An enabled bucket needs CAPACITY and RATE together. | ## Reference diff --git a/docs/primitives/rate-limiter/UpdateRateLimiters.md b/docs/primitives/rate-limiter/UpdateRateLimiters.md index 386a430..5a55a49 100644 --- a/docs/primitives/rate-limiter/UpdateRateLimiters.md +++ b/docs/primitives/rate-limiter/UpdateRateLimiters.md @@ -20,6 +20,12 @@ Updates rate limiter configuration on a TokenPool, compatible with both v1 and v | `DEST_CHAIN` | See the script header. | | `DEST_CHAIN_SELECTOR` | See the script header. | | `FAST_FINALITY` | See the script header. | +| `INBOUND_RATE_LIMIT_CAPACITY` | uint128 token-bucket capacity (inbound). Same all-or-nothing rule per direction. | +| `INBOUND_RATE_LIMIT_ENABLED` | true/false; defaults to true when CAPACITY or RATE are set. false stands alone as a disable. | +| `INBOUND_RATE_LIMIT_RATE` | uint128 token-bucket refill rate (inbound). | +| `OUTBOUND_RATE_LIMIT_CAPACITY` | uint128 token-bucket capacity. Per direction the inputs are all-or-nothing: an enabled bucket needs CAPACITY and RATE together, and a partial set is refused naming the missing variable. | +| `OUTBOUND_RATE_LIMIT_ENABLED` | true/false; defaults to true when CAPACITY or RATE are set. false stands alone as a disable. | +| `OUTBOUND_RATE_LIMIT_RATE` | uint128 token-bucket refill rate. An enabled bucket needs CAPACITY and RATE together. | ## Reference diff --git a/docs/primitives/token-admin-registry/ApplyChainUpdates.md b/docs/primitives/token-admin-registry/ApplyChainUpdates.md index b8bf7d0..3177622 100644 --- a/docs/primitives/token-admin-registry/ApplyChainUpdates.md +++ b/docs/primitives/token-admin-registry/ApplyChainUpdates.md @@ -24,6 +24,12 @@ Configures cross-chain lanes on the source TokenPool by calling applyChainUpdate | `DEST_CHAIN_SELECTOR` | See the script header. | | `DEST_TOKEN` | See the script header. | | `DEST_TOKEN_POOL` | See the script header. | +| `INBOUND_RATE_LIMIT_CAPACITY` | uint128 token-bucket capacity (inbound). Same all-or-nothing rule per direction. | +| `INBOUND_RATE_LIMIT_ENABLED` | true/false; defaults to true when CAPACITY or RATE are set. false stands alone as a disable. | +| `INBOUND_RATE_LIMIT_RATE` | uint128 token-bucket refill rate (inbound). | +| `OUTBOUND_RATE_LIMIT_CAPACITY` | uint128 token-bucket capacity. Per direction the inputs are all-or-nothing: an enabled bucket needs CAPACITY and RATE together, and a partial set is refused naming the missing variable. | +| `OUTBOUND_RATE_LIMIT_ENABLED` | true/false; defaults to true when CAPACITY or RATE are set. false stands alone as a disable. | +| `OUTBOUND_RATE_LIMIT_RATE` | uint128 token-bucket refill rate. An enabled bucket needs CAPACITY and RATE together. | | `VIA_JSON_FILE` | See the script header. | ## Preconditions diff --git a/docs/roles.md b/docs/roles.md index 26c22e3..9bf1220 100644 --- a/docs/roles.md +++ b/docs/roles.md @@ -86,7 +86,7 @@ never read and never a FAIL. | Command | Reads | Writes | Exit contract | | ---------------------------------- | ---------------------------- | ----------------------------------------------------------- | ------------------------------------------------------------- | -| `make roles-check CHAIN= [GROUP=]` | live chain + the declaration | **nothing** | `0` clean / `1` drift (names the field) / `2` RPC unavailable | +| `make roles-check CHAIN= [GROUP=]` | live chain + the declaration | **nothing** | `0` clean / `1` drift or a refused read (names the field) / `2` RPC unavailable | | `make snapshot-chain CHAIN= [GROUP=]` | live chain | **only** the `.roles` subtree of that chain's project store | writes the declaration; canonicalizes the file | - **`make roles-check` is READ-ONLY.** It reads the live chain, compares to the declaration, prints one @@ -98,7 +98,10 @@ never read and never a FAIL. - **`make snapshot-chain` is the ONLY writer.** It backfills the declaration FROM the chain (preserve-and-replace on the `.roles` subtree only), for the initial bootstrap or an intentional resync. Same no-silent-writeback rule as `lanes{}`: a read check never edits your files, and the one - writer is an explicit, reviewed command. + writer is an explicit, reviewed command. A field the snapshot cannot read is **not written**: each is + logged as `[snapshot] UNREAD` and omitted, and the closing output warns that the block is partial. Do + not commit a partial block - re-run on a working RPC first. (Absence is the unread state; see the + `roles{}` schema notes in [config-schema.md](config-schema.md).) Bootstrap a chain that has no `roles{}` yet: @@ -171,6 +174,11 @@ flowchart TD - **(b) The change was INTENDED** - a deliberate authority change (e.g. you moved a role to a new governance address on purpose). Update the **declaration** through a reviewed edit or `make snapshot-chain`, and PR the diff so the new intent is recorded and approved. +- **(c) Nothing was compared** - the FAIL reads `could not be read, so nothing was compared` (or names a + contract that `cannot be audited`). This is not drift: the chain neither confirmed nor contradicted + the declaration, because the read itself failed. Fix the read, not the roles: check the RPC's health, + and check that the declared address points at the contract you think it does (a wrong or unauditable + address answers no getter). Then re-run; only a completed read can produce verdict (a) or (b). `make doctor`'s roles rung and the scheduled CI `roles-check` (in `.github/workflows/config-drift.yml`, non-blocking) keep surfacing the drift as a `[FAIL]`/`::warning::` until it is reconciled one way or the diff --git a/script/config/SnapshotChain.s.sol b/script/config/SnapshotChain.s.sol index 974a20e..07d2a7a 100644 --- a/script/config/SnapshotChain.s.sol +++ b/script/config/SnapshotChain.s.sol @@ -59,9 +59,22 @@ contract SnapshotChain is Script { string memory projectPath = ProjectStore._path(name); string memory projectJson = vm.readFile(projectPath); - string memory rolesJson = (new RolesSnapshot()).build(name, configJson, projectJson); + RolesSnapshot snap = new RolesSnapshot(); + string memory rolesJson = snap.build(name, configJson, projectJson); vm.writeJson(rolesJson, projectPath, ".roles"); console.log(string.concat("[snapshot] wrote .roles block for ", name, " -> ", ProjectStore._display(name))); + uint256 unread = snap.unreadCount(); + if (unread != 0) { + console.log( + string.concat( + "[snapshot] WARNING: ", + vm.toString(unread), + " field(s) were UNREAD (see the lines above) - the block is PARTIAL. Re-run on a", + " working RPC before committing it; the missing fields would be SKIPped as undeclared,", + " so a green roles-check on this block proves nothing about them." + ) + ); + } string memory grp = ProjectStore._group(); console.log( string.concat( diff --git a/script/config/SyncCcipConfig.s.sol b/script/config/SyncCcipConfig.s.sol index 3cae6c4..9b039ba 100644 --- a/script/config/SyncCcipConfig.s.sol +++ b/script/config/SyncCcipConfig.s.sol @@ -555,7 +555,7 @@ contract SyncCcipConfig is Script { string.concat( " 5. verify: FOUNDRY_PROFILE=sync forge script script/config/VerifyChain.s.sol --tc VerifyChain --sig \"run(string)\" ", localName, - " (re-run until it reports 0 FAIL)" + " (re-run until it reports VERIFIED)" ) ); } else { diff --git a/script/config/VerifyChain.s.sol b/script/config/VerifyChain.s.sol index 2886d5c..dbbd76b 100644 --- a/script/config/VerifyChain.s.sol +++ b/script/config/VerifyChain.s.sol @@ -222,8 +222,11 @@ contract ChainProbe { /// @title VerifyChain /// @notice The layered chain-config doctor. One aligned [PASS]/[FAIL]/[WARN]/[SKIP] line per check, -/// reverting at the end iff any FAIL, so a chain can be verified end-to-end between "config file -/// edited" and "scripts run against it". Layers: +/// ending in a three-way verdict: FAILED (any [FAIL] - reverts), INCOMPLETE (no failure, but a +/// declared or deployed check could not run, e.g. an unset RPC - also reverts, with its own marker, +/// so a wrapper gating on the exit code cannot read an unchecked chain as a verified one), or +/// VERIFIED (exit 0). Designed absences (non-EVM rungs, undeclared optional blocks) are plain SKIPs +/// and never make a run INCOMPLETE. Layers: /// 1. TOOLS curl + jq present (the ffi fetch preflight) /// 2. SCHEMA every key the real `ChainConfig._load` path consumes, incl. the quoted-decimal /// big-int rule, plus an actual `ChainConfig._load` parse, the optional @@ -256,6 +259,7 @@ contract ChainProbe { contract VerifyChain is Script { uint256 private s_fails; uint256 private s_warns; + uint256 private s_skips; // unverified-gap skips only (see _skipUnverified); designed skips do not count bool private s_forked; ChainProbe private s_probe; @@ -273,10 +277,21 @@ contract VerifyChain is Script { console.log(string.concat("[WARN] ", msg_)); } + /// @dev A DESIGNED skip: there is nothing to verify (non-EVM rung, an undeclared optional block, a + /// bootstrap state). Printed, never counted - it cannot make the verdict INCOMPLETE. function _skip(string memory msg_) private pure { console.log(string.concat("[SKIP] ", msg_)); } + /// @dev An UNVERIFIED gap: something IS declared or deployed and could not be checked (an unset + /// RPC, a contract that did not answer). Counted into the verdict - enough of these and the run is + /// INCOMPLETE rather than clean, because a check that never ran proves nothing. The UNVERIFIED tag + /// separates these lines from designed skips, so the INCOMPLETE verdict points at exactly them. + function _skipUnverified(string memory msg_) private { + s_skips++; + console.log(string.concat("[SKIP] UNVERIFIED ", msg_)); + } + function _path(string memory name) private pure returns (string memory) { return string.concat("config/chains/", name, ".json"); } @@ -401,13 +416,40 @@ contract VerifyChain is Script { _verdict(name); } + /// @dev Three outcomes, not a boolean. The one that matters here is INCOMPLETE - checks that were + /// declared or deployed but could NOT run (an unset RPC, a contract that did not answer). + /// Without it, a run that verifies nothing prints + /// `0 FAIL, 0 WARN` and exits 0, indistinguishable from a run that verified everything, and any + /// wrapper gating on the exit code passes a chain the doctor never actually checked. Designed + /// absences (a non-EVM chain's EVM rungs, an undeclared optional block) stay plain SKIPs and do + /// not affect the verdict: there, nothing claimable was left unchecked. function _verdict(string memory name) private view { console.log( string.concat( - "== check-chain ", name, ": ", vm.toString(s_fails), " FAIL, ", vm.toString(s_warns), " WARN ==" + "== check-chain ", + name, + ": ", + vm.toString(s_fails), + " FAIL, ", + vm.toString(s_warns), + " WARN, ", + vm.toString(s_skips), + " UNVERIFIED ==" ) ); require(s_fails == 0, string.concat("check-chain FAILED for ", name, " - see [FAIL] lines above")); + require( + s_skips == 0, + string.concat( + "check-chain INCOMPLETE for ", + name, + " - ", + vm.toString(s_skips), + " check(s) could not run: see the [SKIP] UNVERIFIED lines above, set the named RPC env" + " or fix the unanswering contract, then re-run" + ) + ); + console.log(string.concat("== check-chain ", name, ": VERIFIED ==")); } // ---------------------------------------------------------------- 1. TOOLS @@ -776,7 +818,11 @@ contract VerifyChain is Script { } string memory url = vm.envOr(rpcEnv, string("")); if (bytes(url).length == 0) { - _skip(string.concat("rpc: env ", rpcEnv, " unset - add it to your .env to enable fork checks")); + _skipUnverified( + string.concat( + "rpc: env ", rpcEnv, " unset - add it to your .env; without it the RPC-gated rungs cannot run" + ) + ); return false; } try s_probe.forkTo(url) returns (uint256 forkChainId) { @@ -961,7 +1007,7 @@ contract VerifyChain is Script { /// unhandled revert that would kill the whole doctor run. function _reconcilePoolWithTar(address tar, address token, address pool) private { if (!s_forked) { - _skip( + _skipUnverified( "registry: TAR reconciliation needs an RPC (no fork) - registry pool not checked against on-chain wiring" ); return; @@ -1283,6 +1329,33 @@ contract VerifyChain is Script { return (s_fails, s_warns); } + /// @notice Test hook: the unverified-gap counter, so rung tests can pin WHICH skips count toward + /// the INCOMPLETE verdict. Not used by any production path. + function unverifiedForTest() public view returns (uint256) { + return s_skips; + } + + /// @notice Test hook: drives the three RPC-gated rungs in their no-fork state and returns the + /// unverified-gap count, so each counted site is pinned individually (a regression demoting one + /// back to a designed skip fails this). `rolesJson` must declare `.roles.token` so the roles rung + /// reaches its RPC gate rather than the designed no-roles skip. Not used by any production path. + function rpcGatedSkipsForTest(string memory rolesJson) public returns (uint256 skipsOut) { + _reconcilePoolWithTar(address(1), address(1), address(1)); + _checkRoles("zz-tt-rpcgaps", rolesJson, false); + _checkLanesOnChain("zz-tt-rpcgaps", "", "{}"); + return s_skips; + } + + /// @notice Test hook: seeds the outcome counters and runs `_verdict`, so the three-outcome + /// contract (VERIFIED / INCOMPLETE / FAILED, and that designed skips never taint the verdict) is + /// pinnable without the ffi/API/RPC doctor run. Not used by any production path. + function verdictForTest(string memory name, bool designedSkip, bool unverifiedSkip, bool failed) public { + if (designedSkip) _skip("test: designed absence - nothing to verify"); + if (unverifiedSkip) _skipUnverified("test: declared but could not be checked"); + if (failed) _fail("test: induced failure"); + _verdict(name); + } + /// @notice Test hook: runs ONLY the SCHEMA rung (`_checkSchema`) against `name`'s config and returns /// `(isEvm, fails, warns)`. Lets a UNIT test (no fork, no ffi/API) assert the clean-chain PASS /// (every key `ChainConfig._load` consumes is present → 0 FAIL) and the induced-FAIL-naming-the-field @@ -1351,7 +1424,9 @@ contract VerifyChain is Script { _warnAnchorDrift(name, projectJson, ".roles.token.address", "token", "roles.token.address"); _warnAnchorDrift(name, projectJson, ".roles.pool.address", "tokenPool", "roles.pool.address"); if (!rpcOk || !s_forked) { - _skip("roles: authority reconciliation needs an RPC (no fork) - declared roles{} not checked against chain"); + _skipUnverified( + "roles: authority reconciliation needs an RPC (no fork) - declared roles{} not checked against chain" + ); return; } // Mirror every other rung in this file: a revert inside the auditor (a typo'd token.type, a @@ -1422,7 +1497,9 @@ contract VerifyChain is Script { private { if (!s_forked) { - _skip("lanes: on-chain reconciliation needs an RPC (no fork) - declared lanes not checked against the pool"); + _skipUnverified( + "lanes: on-chain reconciliation needs an RPC (no fork) - declared lanes not checked against the pool" + ); return; } address pool = RegistryWriter._read(name, "tokenPool"); @@ -2031,7 +2108,9 @@ contract VerifyChain is Script { try s_probe.poolSupportedChains(pool) returns (uint64[] memory chains) { onChain = chains; } catch { - _skip( + // A deployed pool that does not answer is an unverified gap, not a designed absence: the + // reverse reconcile was claimable and did not run. + _skipUnverified( string.concat( "lanes: pool ", vm.toString(pool), diff --git a/script/config/roles-check.sh b/script/config/roles-check.sh index 8e925aa..cef562f 100755 --- a/script/config/roles-check.sh +++ b/script/config/roles-check.sh @@ -12,7 +12,9 @@ # recipe failure to exit 2, so `make roles-check` is pass/fail only; CI calls the script directly, the # same lesson as sync-check.sh): # 0 CLEAN every checked chain's declared roles{} matches the live chain -# 1 ROLES_DRIFT at least one declared holder/config mismatches (or a real config error) +# 1 ROLES_DRIFT at least one declared holder/config mismatches, or a declared value's read was +# refused ("could not be read, so nothing was compared" - not drift; fix the +# read, see docs/roles.md verdict (c)), or a real config error # 2 RPC_UNAVAILABLE an RPC was unset/unreachable for at least one chain and NOTHING drifted # (flake/missing secret, not drift — CI should warn-and-pass, never go red) # @@ -162,7 +164,7 @@ for i in "${!pair_chain[@]}"; do done if [ $drift -ne 0 ]; then - echo "roles-check: ROLES_DRIFT (or config error) for: ${drifted[*]} - remediate on-chain or re-declare via make snapshot-chain CHAIN= [GROUP=]" + echo "roles-check: ROLES_DRIFT (or config error) for: ${drifted[*]} - remediate on-chain or re-declare via make snapshot-chain CHAIN= [GROUP=]; a FAIL reading 'could not be read' is a refused read, not drift - fix the RPC or the declared address (docs/roles.md verdict (c))" exit 1 elif [ $unreachable -ne 0 ]; then echo "roles-check: RPC_UNAVAILABLE for: ${flaked[*]} - flake/missing secret, not drift; retry with the RPC env set" diff --git a/script/config/test-tooling.sh b/script/config/test-tooling.sh index 689653b..b9b7d24 100755 --- a/script/config/test-tooling.sh +++ b/script/config/test-tooling.sh @@ -603,7 +603,7 @@ print('SKELETON_OK') failures+=("non-EVM skeleton shape") echo "[FAIL] non-EVM skeleton shape: $out" fi - run_case "doctor passes the freshly add-chain'd non-EVM config (0 FAIL)" zero "check-chain $SVMO_CHAIN: 0 FAIL" -- \ + run_case "doctor ends VERIFIED on the freshly add-chain'd non-EVM config" zero "check-chain $SVMO_CHAIN: VERIFIED" -- \ env CCIP_API_BASE="http://127.0.0.1:$port" bash -c \ "FOUNDRY_PROFILE=sync forge script script/config/VerifyChain.s.sol --tc VerifyChain --sig 'run(string)' $SVMO_CHAIN" rm_fixture_config "$SVMO_FILE" @@ -712,16 +712,20 @@ fi run_case "check-chain unknown chain FAILs with the add-chain hint" nonzero "config: no config/chains/doesnotexist" -- \ env FOUNDRY_PROFILE=sync forge script script/config/VerifyChain.s.sol --tc VerifyChain --sig "run(string)" doesnotexist -# 15. non-EVM chain -> schema parse only, 0 FAIL -run_case_live "check-chain on solana-devnet passes (non-EVM path)" zero "0 FAIL" -- \ +# 15. non-EVM chain -> schema parse only, ends VERIFIED +run_case_live "check-chain on solana-devnet ends VERIFIED (non-EVM path)" zero "check-chain solana-devnet: VERIFIED" -- \ env FOUNDRY_PROFILE=sync forge script script/config/VerifyChain.s.sol --tc VerifyChain --sig "run(string)" solana-devnet -# 16. EVM chain with rpcEnv unset -> rpc SKIP (not FAIL), overall 0 FAIL (live API for the drift rung). +# 16. EVM chain with rpcEnv unset -> rpc SKIP (not FAIL), and the run is INCOMPLETE, not clean: the +# RPC-gated rungs never ran (the doctor's three-outcome contract, docs/operations/chains.md). # Injected present-but-empty, NOT `env -u`: forge auto-loads the repo-root .env and re-sets a var # that is absent from the environment, while dotenv never overrides a var that is present, even # empty. `_checkRpc` treats empty as unset (`vm.envOr` + zero-length -> SKIP), so an empty -# injection proves the SKIP path on any machine, whatever the local .env defines. -run_case_live "check-chain SKIPs rpc when the rpcEnv var is unset" zero "\[SKIP\] rpc: env MANTLE_SEPOLIA_RPC_URL unset" -- \ +# injection proves the path on any machine, whatever the local .env defines. +run_case_live "check-chain is INCOMPLETE (nonzero) when the rpcEnv var is unset" nonzero "check-chain INCOMPLETE for" -- \ + env MANTLE_SEPOLIA_RPC_URL= FOUNDRY_PROFILE=sync \ + forge script script/config/VerifyChain.s.sol --tc VerifyChain --sig "run(string)" ethereum-testnet-sepolia-mantle-1 +run_case_live "check-chain tags the rpc gap UNVERIFIED when the rpcEnv var is unset" nonzero "\[SKIP\] UNVERIFIED rpc: env MANTLE_SEPOLIA_RPC_URL unset" -- \ env MANTLE_SEPOLIA_RPC_URL= FOUNDRY_PROFILE=sync \ forge script script/config/VerifyChain.s.sol --tc VerifyChain --sig "run(string)" ethereum-testnet-sepolia-mantle-1 @@ -870,15 +874,18 @@ run_case "doctor FAILs a one-sided lane naming both chains" nonzero \ "one-sided lane $TMP_CHAIN -> $TMP_CHAIN_B ($TMP_CHAIN_B has no lanes.$TMP_CHAIN entry)" -- \ env FOUNDRY_PROFILE=sync forge script script/config/VerifyChain.s.sol --tc VerifyChain --sig "run(string)" "$TMP_CHAIN" -# 28. the reciprocal entry clears it: doctor back to 0 FAIL +# 28. the reciprocal entry clears the mesh FAIL. The INCOMPLETE marker is the proof: _verdict prints +# it only when s_fails == 0 (a FAILED run reverts on the fails require first), and the run stays +# nonzero because the scratch chain's RPC env is unset. make add-lane LOCAL="$TMP_CHAIN_B" REMOTE="$TMP_CHAIN" CAPACITY=1000 RATE=10 > /dev/null 2>&1 -run_case "doctor passes once the lane is reciprocated" zero "0 FAIL" -- \ +run_case "doctor mesh FAIL clears once the lane is reciprocated (INCOMPLETE, no residual FAIL)" nonzero "check-chain INCOMPLETE for" -- \ env FOUNDRY_PROFILE=sync forge script script/config/VerifyChain.s.sol --tc VerifyChain --sig "run(string)" "$TMP_CHAIN" # 28b. the LANES rung (on-chain lane reconciliation) is RPC-gated: with TOOLING_TMP_RPC_URL unset it -# SKIPs cleanly instead of blocking an offline doctor run. The reconciliation logic itself needs -# a fork and is covered by test/config/VerifyChainLaneReconcile.t.sol. -run_case "doctor lanes rung SKIPs cleanly without an RPC" zero \ +# prints its SKIP line (never a FAIL) and counts into the INCOMPLETE verdict - the declared lanes +# were not checked against the pool, so the run must not exit clean. The reconciliation logic +# itself needs a fork and is covered by test/config/VerifyChainLaneReconcile.t.sol. +run_case "doctor lanes rung prints its SKIP without an RPC (run INCOMPLETE)" nonzero \ "lanes: on-chain reconciliation needs an RPC" -- \ env FOUNDRY_PROFILE=sync forge script script/config/VerifyChain.s.sol --tc VerifyChain --sig "run(string)" "$TMP_CHAIN" @@ -1293,14 +1300,14 @@ for name, cid, sel in [('$TMP_CHAIN','990001','9900010000000000001'), env FOUNDRY_PROFILE=sync PROJECT_GROUP="$GRP_X" \ forge script script/config/VerifyChain.s.sol --tc VerifyChain --sig "run(string)" "$TMP_CHAIN" - # verdict equivalence: the SAME store content flat vs in a group yields identical FAIL/WARN - # tallies (the group changes only the file location, never the doctor's verdict). + # verdict equivalence: the SAME store content flat vs in a group yields identical + # FAIL/WARN/UNVERIFIED tallies (the group changes only the file location, never the verdict). seed_group_pair make add-lane LOCAL="$TMP_CHAIN" REMOTE="$TMP_CHAIN_B" CAPACITY=1000 RATE=10 BOTH=1 > /dev/null 2>&1 # flat, reciprocated make add-lane LOCAL="$TMP_CHAIN" REMOTE="$TMP_CHAIN_B" CAPACITY=1000 RATE=10 BOTH=1 GROUP="$GRP_X" > /dev/null 2>&1 # same, grouped - flat_verdict="$(FOUNDRY_PROFILE=sync forge script script/config/VerifyChain.s.sol --tc VerifyChain --sig "run(string)" "$TMP_CHAIN" 2>&1 | grep -oE "[0-9]+ FAIL, [0-9]+ WARN" | tail -1)" + flat_verdict="$(FOUNDRY_PROFILE=sync forge script script/config/VerifyChain.s.sol --tc VerifyChain --sig "run(string)" "$TMP_CHAIN" 2>&1 | grep -oE "[0-9]+ FAIL, [0-9]+ WARN, [0-9]+ UNVERIFIED" | tail -1)" grp_out="$(FOUNDRY_PROFILE=sync PROJECT_GROUP="$GRP_X" forge script script/config/VerifyChain.s.sol --tc VerifyChain --sig "run(string)" "$TMP_CHAIN" 2>&1)" - grp_verdict="$(echo "$grp_out" | grep -oE "[0-9]+ FAIL, [0-9]+ WARN" | tail -1)" + grp_verdict="$(echo "$grp_out" | grep -oE "[0-9]+ FAIL, [0-9]+ WARN, [0-9]+ UNVERIFIED" | tail -1)" # Non-vacuous: the grouped run must have read the GROUPED file (not silently the flat one), so the # tally match is between two runs that genuinely resolved different paths. if [ -n "$flat_verdict" ] && [ "$flat_verdict" = "$grp_verdict" ] && grep -q "project/$GRP_X/$TMP_CHAIN.json" <<< "$grp_out"; then @@ -1320,7 +1327,9 @@ for name, cid, sel in [('$TMP_CHAIN','990001','9900010000000000001'), printf '{"addresses":{"active":{},"deployments":{}},"lanes":{},"roles":{},"schema":999}' > "project/$GRP_Y/$TMP_CHAIN.json" out="$(FOUNDRY_PROFILE=sync PROJECT_GROUP="$GRP_X" forge script script/config/VerifyChain.s.sol --tc VerifyChain --sig "run(string)" "$TMP_CHAIN" 2>&1)" status=$? - if [ $status -eq 0 ] && grep -q "0 FAIL" <<< "$out" && + # 0 FAIL proves the corrupt sibling did not poison the run; the exit is INCOMPLETE (nonzero) + # because the scratch chain's RPC env is unset, so the run must not read as verified. + if grep -q "0 FAIL" <<< "$out" && grep -q "check-chain INCOMPLETE for" <<< "$out" && grep -q "project/$GRP_X/$TMP_CHAIN.json" <<< "$out" && ! grep -q "project/$GRP_Y/" <<< "$out"; then pass=$((pass + 1)) @@ -1331,7 +1340,7 @@ for name, cid, sel in [('$TMP_CHAIN','990001','9900010000000000001'), echo "[FAIL] doctor GROUP=$GRP_X scoping (exit=$status; read the sibling group or FAILed)" echo "$out" | tail -6 | sed 's/^/ | /' fi - run_case "doctor GROUP=$GRP_Y reads its OWN group file (scoping is real)" zero \ + run_case "doctor GROUP=$GRP_Y reads its OWN group file (scoping is real)" nonzero \ "project/$GRP_Y/$TMP_CHAIN.json" -- \ env FOUNDRY_PROFILE=sync PROJECT_GROUP="$GRP_Y" \ forge script script/config/VerifyChain.s.sol --tc VerifyChain --sig "run(string)" "$TMP_CHAIN" diff --git a/script/configure/dynamic-config/SetDynamicConfig.s.sol b/script/configure/dynamic-config/SetDynamicConfig.s.sol index 8762334..bafaacc 100644 --- a/script/configure/dynamic-config/SetDynamicConfig.s.sol +++ b/script/configure/dynamic-config/SetDynamicConfig.s.sol @@ -11,12 +11,15 @@ import {EoaExecutor} from "../../../src/base/EoaExecutor.s.sol"; /// /// Environment Variables (optional): /// ROUTER - The new router address (default: current on-chain value) -/// RATE_LIMIT_ADMIN - The new rate limit admin address -/// Default: current on-chain value (if set), otherwise the broadcaster address. -/// FEE_ADMIN - The new fee admin address -/// Default: current on-chain value (if set), otherwise the broadcaster address. +/// RATE_LIMIT_ADMIN - The new rate limit admin address (default: current on-chain value) +/// FEE_ADMIN - The new fee admin address (default: current on-chain value) /// Set to address(0) to restrict fee withdrawal to the owner only. /// +/// An unset variable preserves the pool's current value verbatim, address(0) included: this script +/// writes the whole struct, so anything else would turn a one-field update into a silent grant of the +/// other fields (a broadcaster fallback would hand both admin slots to the acting account on a +/// ROUTER-only run whenever they are unset on chain). +/// /// Usage example: /// ROUTER=0xYourRouterAddress \ /// RATE_LIMIT_ADMIN=0xYourRateLimitAdminAddress \ @@ -25,6 +28,26 @@ import {EoaExecutor} from "../../../src/base/EoaExecutor.s.sol"; contract SetDynamicConfig is EoaExecutor { HelperConfig public helperConfig; + /// @dev An unset variable preserves the current on-chain value VERBATIM, address(0) included. The + /// call writes the whole struct, so any other default turns a one-field update into a silent + /// grant of the others: a broadcaster fallback would hand both admin slots to the acting + /// account on a ROUTER-only run whenever they are unset on chain. + function _resolveNewConfig(address currentRouter, address currentRateLimitAdmin, address currentFeeAdmin) + internal + view + returns (address router, address rateLimitAdmin, address feeAdmin) + { + router = _dcEnvOr("ROUTER", currentRouter); + rateLimitAdmin = _dcEnvOr("RATE_LIMIT_ADMIN", currentRateLimitAdmin); + feeAdmin = _dcEnvOr("FEE_ADMIN", currentFeeAdmin); + } + + /// @dev Virtual input seam (like the `_rlEnv*` seams elsewhere): env vars are process-wide and + /// suites run in parallel, so tests pin the resolution through an override, never `vm.setEnv`. + function _dcEnvOr(string memory name, address defaultValue) internal view virtual returns (address) { + return vm.envOr(name, defaultValue); + } + function run() external { // ── Resolve chain ID ────────────────────────────────────────────── helperConfig = new HelperConfig(); @@ -63,12 +86,8 @@ contract SetDynamicConfig is EoaExecutor { console.log(string.concat(" Fee Admin: ", vm.toString(currentFeeAdmin))); console.log(""); - // Defaults: env var → current on-chain value → broadcaster (as last resort if unset) - address broadcasterAddr = _broadcaster(); - address router = vm.envOr("ROUTER", currentRouter); - address rateLimitAdmin = - vm.envOr("RATE_LIMIT_ADMIN", currentRateLimitAdmin != address(0) ? currentRateLimitAdmin : broadcasterAddr); - address feeAdmin = vm.envOr("FEE_ADMIN", currentFeeAdmin != address(0) ? currentFeeAdmin : broadcasterAddr); + (address router, address rateLimitAdmin, address feeAdmin) = + _resolveNewConfig(currentRouter, currentRateLimitAdmin, currentFeeAdmin); console.log("New Configuration:"); console.log(string.concat(" Router: ", vm.toString(router))); diff --git a/script/configure/finality-config/SetFinalityConfig.s.sol b/script/configure/finality-config/SetFinalityConfig.s.sol index ee40ba4..58dd299 100644 --- a/script/configure/finality-config/SetFinalityConfig.s.sol +++ b/script/configure/finality-config/SetFinalityConfig.s.sol @@ -51,6 +51,10 @@ import {ProjectStore} from "../../../src/utils/ProjectStore.sol"; /// INBOUND_RATE_LIMIT_RATE - uint128, inbound token bucket refill rate /// INBOUND_RATE_LIMIT_ENABLED - true/false (defaults to true when CAPACITY or RATE are set) /// +/// Per direction the rate-limit inputs are all-or-nothing (`RateLimiterUtils._establishBucket`, +/// the same decision UpdateRateLimiters applies): a partial set is refused naming the missing +/// variable. +/// /// Behaviour (rate limiter section): /// * DEST_CHAIN only -> logs current rate limits for the fast finality bucket /// * DEST_CHAIN + rate limit vars -> logs current, applies updates, logs updated state diff --git a/script/configure/rate-limiter/UpdateRateLimiters.s.sol b/script/configure/rate-limiter/UpdateRateLimiters.s.sol index 650b9a4..f57d99d 100644 --- a/script/configure/rate-limiter/UpdateRateLimiters.s.sol +++ b/script/configure/rate-limiter/UpdateRateLimiters.s.sol @@ -29,12 +29,13 @@ import {ProjectStore} from "../../../src/utils/ProjectStore.sol"; /// DEST_CHAIN_FAMILY - Override destination family (default: from DEST_CHAIN config) /// DEST_CHAIN_SELECTOR - Override destination selector (default: from DEST_CHAIN config) /// -/// Environment Variables (set to update outbound - any one triggers the direction): +/// Environment Variables (set to update outbound - any one triggers the direction; the direction's +/// inputs are then all-or-nothing, decided by `RateLimiterUtils._establishBucket`): /// OUTBOUND_RATE_LIMIT_CAPACITY - uint128, token bucket capacity (isEnabled defaults to true when set) /// OUTBOUND_RATE_LIMIT_RATE - uint128, token bucket refill rate (isEnabled defaults to true when set) /// OUTBOUND_RATE_LIMIT_ENABLED - true/false (optional override; defaults to true if CAPACITY/RATE provided) /// -/// Environment Variables (set to update inbound - any one triggers the direction): +/// Environment Variables (set to update inbound - same all-or-nothing rule per direction): /// INBOUND_RATE_LIMIT_CAPACITY - uint128, token bucket capacity (isEnabled defaults to true when set) /// INBOUND_RATE_LIMIT_RATE - uint128, token bucket refill rate (isEnabled defaults to true when set) /// INBOUND_RATE_LIMIT_ENABLED - true/false (optional override; defaults to true if CAPACITY/RATE provided) @@ -216,27 +217,35 @@ contract UpdateRateLimiters is EoaExecutor, LanePolicySource { /// @dev Reads the rate-limit env vars through the env seams - the same semantics as /// `RateLimiterUtils._readRateLimitUpdate` (which reads the process env directly and stays - /// untouched for its other consumers): any of a direction's vars triggers the direction, - /// isEnabled defaults to true when CAPACITY or RATE is set, ENABLED overrides explicitly. + /// untouched for its other consumers): any of a direction's vars triggers the direction, and + /// the direction's inputs are then all-or-nothing (`RateLimiterUtils._establishBucket`), so + /// an unset variable never becomes a 0 written on chain. function _readRateLimitUpdate() internal view returns (RateLimiterUtils.RateLimitUpdate memory u) { + bool outboundEnabledSet = _envExists("OUTBOUND_RATE_LIMIT_ENABLED"); bool outboundCapacitySet = _envExists("OUTBOUND_RATE_LIMIT_CAPACITY"); bool outboundRateSet = _envExists("OUTBOUND_RATE_LIMIT_RATE"); + bool inboundEnabledSet = _envExists("INBOUND_RATE_LIMIT_ENABLED"); bool inboundCapacitySet = _envExists("INBOUND_RATE_LIMIT_CAPACITY"); bool inboundRateSet = _envExists("INBOUND_RATE_LIMIT_RATE"); - u.updateOutbound = _envExists("OUTBOUND_RATE_LIMIT_ENABLED") || outboundCapacitySet || outboundRateSet; - u.updateInbound = _envExists("INBOUND_RATE_LIMIT_ENABLED") || inboundCapacitySet || inboundRateSet; - - if (u.updateOutbound) { - u.outboundEnabled = _envBool("OUTBOUND_RATE_LIMIT_ENABLED", outboundCapacitySet || outboundRateSet); - u.outboundCapacity = uint128(_envUint("OUTBOUND_RATE_LIMIT_CAPACITY")); - u.outboundRate = uint128(_envUint("OUTBOUND_RATE_LIMIT_RATE")); - } - if (u.updateInbound) { - u.inboundEnabled = _envBool("INBOUND_RATE_LIMIT_ENABLED", inboundCapacitySet || inboundRateSet); - u.inboundCapacity = uint128(_envUint("INBOUND_RATE_LIMIT_CAPACITY")); - u.inboundRate = uint128(_envUint("INBOUND_RATE_LIMIT_RATE")); - } + (u.updateOutbound, u.outboundEnabled, u.outboundCapacity, u.outboundRate) = RateLimiterUtils._establishBucket( + "OUTBOUND", + outboundEnabledSet, + _envBool("OUTBOUND_RATE_LIMIT_ENABLED", false), + outboundCapacitySet, + _envUint("OUTBOUND_RATE_LIMIT_CAPACITY"), + outboundRateSet, + _envUint("OUTBOUND_RATE_LIMIT_RATE") + ); + (u.updateInbound, u.inboundEnabled, u.inboundCapacity, u.inboundRate) = RateLimiterUtils._establishBucket( + "INBOUND", + inboundEnabledSet, + _envBool("INBOUND_RATE_LIMIT_ENABLED", false), + inboundCapacitySet, + _envUint("INBOUND_RATE_LIMIT_CAPACITY"), + inboundRateSet, + _envUint("INBOUND_RATE_LIMIT_RATE") + ); } /// @dev Resolves the per-direction buckets through the input ladder (see the contract natspec). diff --git a/script/docs/gen-primitives.mjs b/script/docs/gen-primitives.mjs index 61dc329..5f30b65 100644 --- a/script/docs/gen-primitives.mjs +++ b/script/docs/gen-primitives.mjs @@ -352,6 +352,14 @@ function main() { // reached only the .md page and left the catalog and index on the name-heuristic default. for (const s of scripts) s.destructive = meta[s.name]?.destructive ?? s.destructive; + // Same at-the-source rule for authored inputs: a var read through an env seam is invisible to the + // vm.env* regex, and an authored `inputs` entry that reached only the .md page would leave the + // machine catalog advertising `"inputs": []` for a script that does read env vars. + for (const s of scripts) { + const authored = Object.keys(meta[s.name]?.inputs ?? {}); + if (authored.length) s.inputs = [...new Set([...s.inputs, ...authored])].sort(); + } + // Coverage: no authored meta entry may name a script that no longer exists. const names = new Set(scripts.map((s) => s.name)); const orphans = Object.keys(meta).filter((k) => !names.has(k)); diff --git a/script/governance/VerifyRoles.s.sol b/script/governance/VerifyRoles.s.sol index 9fa0d14..4956dde 100644 --- a/script/governance/VerifyRoles.s.sol +++ b/script/governance/VerifyRoles.s.sol @@ -19,7 +19,8 @@ import {RolesProbes} from "../../src/roles/RolesProbes.sol"; /// standalone audit of who holds what right now. /// /// Every read goes through `RolesProbes`'s tolerant staticcalls, so a slot a given template/version -/// does not expose prints as "(absent)" rather than reverting the whole report - one reader covers +/// does not expose (or that does not answer) prints as "( did not answer)" rather than +/// reverting the whole report - one reader covers /// `CrossChainToken` / `BurnMintERC20` / `FactoryBurnMintERC20` / BYO tokens and v1.x / v2.0 pools. /// /// forge script script/governance/VerifyRoles.s.sol --sig "run(string)" ethereum-testnet-sepolia --rpc-url @@ -141,9 +142,10 @@ contract VerifyRoles is Script { if (!has || lockbox == address(0)) return; console.log("--- lockbox authority (", lockbox, ") ---"); _logAddr("owner ", lockbox, "owner()"); - (, address[] memory callers) = + (bool okCallers, address[] memory callers) = RolesProbes._tryAddressArray(lockbox, abi.encodeWithSignature("getAllAuthorizedCallers()")); - _logSet("authorizedCallers ", callers); + if (okCallers) _logSet("authorizedCallers ", callers); + else console.log(" authorizedCallers : (getAllAuthorizedCallers() did not answer)"); } function _reportHooks(address pool) private view { @@ -152,11 +154,15 @@ contract VerifyRoles is Script { console.log("--- hooks authority (", hooks, ") ---"); _logAddr("owner ", hooks, "owner()"); _logAddr("policyEngine ", hooks, "getPolicyEngine()"); - (, bool allowlistEnabled) = RolesProbes._tryBool(hooks, "getAllowListEnabled()"); - console.log(" allowlistEnabled :", allowlistEnabled); - (, address[] memory callers) = + (bool okEnabled, bool allowlistEnabled) = RolesProbes._tryBool(hooks, "getAllowListEnabled()"); + // A failed read is named as such: printing the probe's zero would report "allowlistEnabled: + // false" about a contract that never answered. + if (okEnabled) console.log(" allowlistEnabled :", allowlistEnabled); + else console.log(" allowlistEnabled : (getAllowListEnabled() did not answer)"); + (bool okCallers, address[] memory callers) = RolesProbes._tryAddressArray(hooks, abi.encodeWithSignature("getAllAuthorizedCallers()")); - _logSet("authorizedCallers ", callers); + if (okCallers) _logSet("authorizedCallers ", callers); + else console.log(" authorizedCallers : (getAllAuthorizedCallers() did not answer)"); } // ---------------------------------------------------------------- helpers @@ -164,7 +170,7 @@ contract VerifyRoles is Script { function _logAddr(string memory label, address target, string memory sig) private view { (bool ok, address val) = RolesProbes._tryAddress(target, sig); if (ok) console.log(string.concat(" ", label, ":"), val); - else console.log(string.concat(" ", label, ": (absent)")); + else console.log(string.concat(" ", label, ": (", sig, " did not answer)")); } function _logRole(string memory label, address token, bytes32 role) private view { diff --git a/script/setup/ApplyChainUpdates.s.sol b/script/setup/ApplyChainUpdates.s.sol index 517fd18..9ac5017 100644 --- a/script/setup/ApplyChainUpdates.s.sol +++ b/script/setup/ApplyChainUpdates.s.sol @@ -8,6 +8,7 @@ import {RateLimiter} from "@chainlink/contracts-ccip/contracts/libraries/RateLim import {ChainHandlers} from "../utils/ChainHandlers.s.sol"; import {ChainConfig} from "../../src/config/ChainConfig.sol"; import {PoolVersion} from "../utils/PoolVersion.s.sol"; +import {RateLimiterUtils} from "../utils/RateLimiterUtils.s.sol"; import {PoolVersions} from "../../src/PoolVersions.sol"; import {CctActions, ITokenPoolV150} from "../../src/actions/CctActions.sol"; import {EoaExecutor} from "../../src/base/EoaExecutor.s.sol"; @@ -949,23 +950,27 @@ contract ApplyChainUpdates is EoaExecutor { } } - /// @dev Reads one direction's rate-limit env vars: - /// isEnabled defaults to true when CAPACITY or RATE is set, ENABLED overrides it - /// explicitly, and a disabled bucket zeroes its values. `provided` is true when ANY of the - /// direction's env vars is set (the rung-1 trigger). + /// @dev Reads one direction's rate-limit env vars through the shared all-or-nothing decision + /// (`RateLimiterUtils._establishBucket`): an enabled bucket needs CAPACITY and RATE supplied + /// together (refused naming the missing variable), ENABLED=false stands alone, values must + /// fit uint128, and an unset variable never becomes a 0 written on chain. `provided` is true + /// when ANY of the direction's env vars is set (the rung-1 trigger). function _envBucket(string memory prefix) internal view returns (bool provided, RateLimiter.Config memory config) { - string memory capacityVar = string.concat(prefix, "_RATE_LIMIT_CAPACITY"); - string memory rateVar = string.concat(prefix, "_RATE_LIMIT_RATE"); - string memory enabledVar = string.concat(prefix, "_RATE_LIMIT_ENABLED"); - - bool valuesProvided = _rlEnvExists(capacityVar) || _rlEnvExists(rateVar); - provided = valuesProvided || _rlEnvExists(enabledVar); - bool enabled = _rlEnvBool(enabledVar, valuesProvided); - config = RateLimiter.Config({ - isEnabled: enabled, - capacity: enabled ? uint128(_rlEnvUint(capacityVar)) : 0, - rate: enabled ? uint128(_rlEnvUint(rateVar)) : 0 - }); + bool capacitySet = _rlEnvExists(string.concat(prefix, "_RATE_LIMIT_CAPACITY")); + bool rateSet = _rlEnvExists(string.concat(prefix, "_RATE_LIMIT_RATE")); + bool isEnabled; + uint128 capacity; + uint128 rate; + (provided, isEnabled, capacity, rate) = RateLimiterUtils._establishBucket( + prefix, + _rlEnvExists(string.concat(prefix, "_RATE_LIMIT_ENABLED")), + _rlEnvBool(string.concat(prefix, "_RATE_LIMIT_ENABLED"), false), + capacitySet, + _rlEnvUint(string.concat(prefix, "_RATE_LIMIT_CAPACITY")), + rateSet, + _rlEnvUint(string.concat(prefix, "_RATE_LIMIT_RATE")) + ); + config = RateLimiter.Config({isEnabled: isEnabled, capacity: capacity, rate: rate}); } /// @dev A declared bucket as a RateLimiter.Config: enabled iff capacity or rate is non-zero - diff --git a/script/setup/token-roles/SetCCIPAdmin.s.sol b/script/setup/token-roles/SetCCIPAdmin.s.sol index 1a94763..4ec4404 100644 --- a/script/setup/token-roles/SetCCIPAdmin.s.sol +++ b/script/setup/token-roles/SetCCIPAdmin.s.sol @@ -56,7 +56,8 @@ contract SetCCIPAdmin is TokenRoleScript { // DEFAULT_ADMIN_ROLE; the factory template with owner(). A BYO token's gate is unknown - // the simulation run (no --broadcast) surfaces an unauthorized actor before anything is sent. if (template == RolesProbes.TokenTemplate.FactoryBurnMintERC20) { - (, address owner_) = RolesProbes._tryAddress(token, "owner()"); + (bool okOwner, address owner_) = RolesProbes._tryAddress(token, "owner()"); + require(okOwner, "owner() could not be read from the token, so authority cannot be verified"); require( owner_ == actor, string.concat( diff --git a/script/setup/token-roles/TokenRoleScript.s.sol b/script/setup/token-roles/TokenRoleScript.s.sol index 658f289..59d9a62 100644 --- a/script/setup/token-roles/TokenRoleScript.s.sol +++ b/script/setup/token-roles/TokenRoleScript.s.sol @@ -39,7 +39,8 @@ abstract contract TokenRoleScript is EoaExecutor { address actor ) internal view { if (template == RolesProbes.TokenTemplate.FactoryBurnMintERC20) { - (, address owner_) = RolesProbes._tryAddress(token, "owner()"); + (bool okOwner, address owner_) = RolesProbes._tryAddress(token, "owner()"); + require(okOwner, "owner() could not be read from the token, so authority cannot be verified"); require( owner_ == actor, string.concat( diff --git a/script/setup/token-roles/TransferTokenAdmin.s.sol b/script/setup/token-roles/TransferTokenAdmin.s.sol index 0422e57..6df50cb 100644 --- a/script/setup/token-roles/TransferTokenAdmin.s.sol +++ b/script/setup/token-roles/TransferTokenAdmin.s.sol @@ -126,7 +126,8 @@ contract TransferTokenAdmin is TokenRoleScript { console.log(string.concat("New Admin: ", vm.toString(newAdmin))); if (template == RolesProbes.TokenTemplate.CrossChainToken) { - (, address current) = RolesProbes._tryAddress(token, "defaultAdmin()"); + (bool okCurrent, address current) = RolesProbes._tryAddress(token, "defaultAdmin()"); + require(okCurrent, "defaultAdmin() could not be read from the token, so authority cannot be verified"); require( current == actor, string.concat( @@ -160,7 +161,8 @@ contract TransferTokenAdmin is TokenRoleScript { return; } // factory (Ownable2Step) - (, address owner_) = RolesProbes._tryAddress(token, "owner()"); + (bool okOwner, address owner_) = RolesProbes._tryAddress(token, "owner()"); + require(okOwner, "owner() could not be read from the token, so authority cannot be verified"); require( owner_ == actor, string.concat( @@ -176,7 +178,10 @@ contract TransferTokenAdmin is TokenRoleScript { private { if (template == RolesProbes.TokenTemplate.CrossChainToken) { - (, address pending) = RolesProbes._tryAddress(token, "pendingDefaultAdmin()"); + (bool okPending, address pending) = RolesProbes._tryAddress(token, "pendingDefaultAdmin()"); + require( + okPending, "pendingDefaultAdmin() could not be read from the token, so the transfer cannot be verified" + ); require( pending == actor, string.concat( diff --git a/script/utils/RateLimiterUtils.s.sol b/script/utils/RateLimiterUtils.s.sol index 20840e1..51426bc 100644 --- a/script/utils/RateLimiterUtils.s.sol +++ b/script/utils/RateLimiterUtils.s.sol @@ -45,8 +45,8 @@ library RateLimiterUtils { } /// @dev Reads optional rate limit env vars using a sentinel to detect which were actually set. - /// Direction is inferred from any OUTBOUND_* / INBOUND_* var being present. - /// isEnabled defaults to true when CAPACITY or RATE are provided; override with ENABLED=false. + /// Direction is inferred from any OUTBOUND_* / INBOUND_* var being present. Per-direction the + /// inputs are all-or-nothing (`_establishBucket`): an unset variable never becomes a 0 on chain. function _readRateLimitUpdate() internal view returns (RateLimitUpdate memory u) { string memory sentinel = "__not_set__"; bool outboundEnabledSet = @@ -62,19 +62,94 @@ library RateLimiterUtils { bool inboundRateSet = keccak256(bytes(VM.envOr("INBOUND_RATE_LIMIT_RATE", sentinel))) != keccak256(bytes(sentinel)); - u.updateOutbound = outboundEnabledSet || outboundCapacitySet || outboundRateSet; - u.updateInbound = inboundEnabledSet || inboundCapacitySet || inboundRateSet; + (u.updateOutbound, u.outboundEnabled, u.outboundCapacity, u.outboundRate) = _establishBucket( + "OUTBOUND", + outboundEnabledSet, + VM.envOr("OUTBOUND_RATE_LIMIT_ENABLED", false), + outboundCapacitySet, + VM.envOr("OUTBOUND_RATE_LIMIT_CAPACITY", uint256(0)), + outboundRateSet, + VM.envOr("OUTBOUND_RATE_LIMIT_RATE", uint256(0)) + ); + (u.updateInbound, u.inboundEnabled, u.inboundCapacity, u.inboundRate) = _establishBucket( + "INBOUND", + inboundEnabledSet, + VM.envOr("INBOUND_RATE_LIMIT_ENABLED", false), + inboundCapacitySet, + VM.envOr("INBOUND_RATE_LIMIT_CAPACITY", uint256(0)), + inboundRateSet, + VM.envOr("INBOUND_RATE_LIMIT_RATE", uint256(0)) + ); + } - if (u.updateOutbound) { - u.outboundEnabled = VM.envOr("OUTBOUND_RATE_LIMIT_ENABLED", outboundCapacitySet || outboundRateSet); - u.outboundCapacity = uint128(VM.envOr("OUTBOUND_RATE_LIMIT_CAPACITY", uint256(0))); - u.outboundRate = uint128(VM.envOr("OUTBOUND_RATE_LIMIT_RATE", uint256(0))); - } - if (u.updateInbound) { - u.inboundEnabled = VM.envOr("INBOUND_RATE_LIMIT_ENABLED", inboundCapacitySet || inboundRateSet); - u.inboundCapacity = uint128(VM.envOr("INBOUND_RATE_LIMIT_CAPACITY", uint256(0))); - u.inboundRate = uint128(VM.envOr("INBOUND_RATE_LIMIT_RATE", uint256(0))); + /// @notice One direction's bucket from its three inputs, all-or-nothing: an unset variable must + /// never become a 0 written on chain. Letting one variable trigger the direction with the rest + /// defaulted would enable a bucket with rate 0 from capacity alone (the lane works for N tokens, + /// then is permanently dead with `TokenRateLimitReached`) or write capacity 0 from ENABLED=true + /// alone (every transfer reverts `TokenMaxCapacityExceeded`), under a success message. + /// @dev Pure so the decision is pinnable without touching the process env. `enabled` is the + /// EXPLICIT value and is read only when `enabledSet`; the implied default (supplying capacity + /// or rate enables the bucket) is computed here, once, so no consumer can wire it + /// differently. The shapes: + /// - nothing set: the direction is untouched; + /// - ENABLED=false: a valid disable on its own - a disabled bucket's capacity and rate are 0 + /// by protocol rule, so the zeros are forced, not guessed; a NONZERO capacity/rate alongside + /// is refused here rather than left to revert on chain; + /// - enabled (explicitly, or implied by supplying capacity or rate): CAPACITY and RATE are + /// required together, and each must fit uint128 rather than truncate. + function _establishBucket( + string memory direction, + bool enabledSet, + bool enabled, + bool capacitySet, + uint256 capacity, + bool rateSet, + uint256 rate + ) internal pure returns (bool update, bool isEnabled, uint128 capacity128, uint128 rate128) { + update = enabledSet || capacitySet || rateSet; + if (!update) return (false, false, 0, 0); + isEnabled = enabledSet ? enabled : (capacitySet || rateSet); + if (!isEnabled) { + require( + (!capacitySet || capacity == 0) && (!rateSet || rate == 0), + string.concat( + direction, + ": a disabled rate-limit bucket has capacity 0 and rate 0 by protocol rule - drop the", + " CAPACITY/RATE variables or set ", + direction, + "_RATE_LIMIT_ENABLED=true" + ) + ); + return (true, false, 0, 0); } + require( + capacitySet, + string.concat( + direction, + "_RATE_LIMIT_CAPACITY is not set - an enabled bucket needs CAPACITY and RATE supplied", + " together; an unset field must not become a 0 written on chain" + ) + ); + require( + rateSet, + string.concat( + direction, + "_RATE_LIMIT_RATE is not set - an enabled bucket needs CAPACITY and RATE supplied", + " together; an unset field must not become a 0 written on chain" + ) + ); + capacity128 = _requireUint128(string.concat(direction, "_RATE_LIMIT_CAPACITY"), capacity); + rate128 = _requireUint128(string.concat(direction, "_RATE_LIMIT_RATE"), rate); + } + + /// @notice Range-check before the uint128 narrowing: 1e39 would otherwise wrap to a plausible + /// wrong capacity and 2^128 to 0, both written on chain without a trace. + function _requireUint128(string memory field, uint256 value) internal pure returns (uint128) { + require( + value <= type(uint128).max, + string.concat(field, " does not fit uint128 - refusing to truncate it into a different live value") + ); + return uint128(value); } /// @notice Returns the current outbound and inbound TokenBuckets for a given lane, dispatched diff --git a/src/roles/RolesAuditor.sol b/src/roles/RolesAuditor.sol index c4b8f89..e2f21e8 100644 --- a/src/roles/RolesAuditor.sol +++ b/src/roles/RolesAuditor.sol @@ -137,6 +137,38 @@ contract RolesAuditor { } } + /// @dev A failed read refuses the comparison instead of comparing against the probe's zero value. + /// Without this, a declaration of `0x0` / `false` / `[]` reconciles PASS against a contract the + /// audit could not read, because both sides hold the zero value for opposite reasons: one was + /// declared, the other is a failed read. + function _refuseUnread(string memory field, string memory sig) private { + _fail(field, string.concat(sig, " could not be read, so nothing was compared")); + } + + function _checkReadAddress(string memory field, string memory sig, bool ok, address declared, address live) + private + { + if (!ok) { + _refuseUnread(field, sig); + return; + } + _checkAddress(field, declared, live); + } + + function _checkReadSet( + string memory field, + string memory sig, + bool ok, + address[] memory declared, + address[] memory live + ) private { + if (!ok) { + _refuseUnread(field, sig); + return; + } + _checkSet(field, declared, live); + } + function _checkSet(string memory field, address[] memory declared, address[] memory live) private { if (RolesProbes._sameSet(declared, live)) { _pass(field, string.concat(VM.toString(live.length), " member(s), sets match")); @@ -229,16 +261,24 @@ contract RolesAuditor { function _auditTokenAdminPoint(string memory json, address token, RolesProbes.TokenTemplate t) private { if (t == RolesProbes.TokenTemplate.CrossChainToken) { if (VM.keyExistsJson(json, ".roles.token.defaultAdmin")) { - (, address live) = RolesProbes._tryAddress(token, "defaultAdmin()"); - _checkAddress("token.defaultAdmin", VM.parseJsonAddress(json, ".roles.token.defaultAdmin"), live); + (bool ok, address live) = RolesProbes._tryAddress(token, "defaultAdmin()"); + _checkReadAddress( + "token.defaultAdmin", + "defaultAdmin()", + ok, + VM.parseJsonAddress(json, ".roles.token.defaultAdmin"), + live + ); } else { _skip("token.defaultAdmin", "not declared"); } - (, address pending) = RolesProbes._tryAddress(token, "pendingDefaultAdmin()"); + (bool okPending, address pending) = RolesProbes._tryAddress(token, "pendingDefaultAdmin()"); address declaredPending = VM.keyExistsJson(json, ".roles.token.pendingDefaultAdmin") ? VM.parseJsonAddress(json, ".roles.token.pendingDefaultAdmin") : address(0); - if (pending == declaredPending) { + if (!okPending) { + _refuseUnread("token.pendingDefaultAdmin", "pendingDefaultAdmin()"); + } else if (pending == declaredPending) { if (pending != address(0)) { _warn( "token.pendingDefaultAdmin", @@ -263,8 +303,8 @@ contract RolesAuditor { } if (t == RolesProbes.TokenTemplate.FactoryBurnMintERC20) { if (VM.keyExistsJson(json, ".roles.token.owner")) { - (, address live) = RolesProbes._tryAddress(token, "owner()"); - _checkAddress("token.owner", VM.parseJsonAddress(json, ".roles.token.owner"), live); + (bool ok, address live) = RolesProbes._tryAddress(token, "owner()"); + _checkReadAddress("token.owner", "owner()", ok, VM.parseJsonAddress(json, ".roles.token.owner"), live); } else { _skip("token.owner", "not declared"); } @@ -481,17 +521,21 @@ contract RolesAuditor { // ---------------------------------------------------------------- pool function _auditPool(string memory json, address pool) private { - (bool isV2,, address rateLimitAdmin, address feeAdmin) = RolesProbes._readPoolAdmins(pool); - (, address owner_) = RolesProbes._tryAddress(pool, "owner()"); + (bool isV2, bool adminsOk, address rateLimitAdmin, address feeAdmin) = _poolAdmins(pool); + (bool okOwner, address owner_) = RolesProbes._tryAddress(pool, "owner()"); if (VM.keyExistsJson(json, ".roles.pool.owner")) { - _checkAddress("pool.owner", VM.parseJsonAddress(json, ".roles.pool.owner"), owner_); + _checkReadAddress("pool.owner", "owner()", okOwner, VM.parseJsonAddress(json, ".roles.pool.owner"), owner_); } else { _skip("pool.owner", "not declared"); } _auditPendingOwner("pool", pool); if (VM.keyExistsJson(json, ".roles.pool.rateLimitAdmin")) { - _checkAddress( - "pool.rateLimitAdmin", VM.parseJsonAddress(json, ".roles.pool.rateLimitAdmin"), rateLimitAdmin + _checkReadAddress( + "pool.rateLimitAdmin", + "getDynamicConfig()/getRateLimitAdmin()", + adminsOk, + VM.parseJsonAddress(json, ".roles.pool.rateLimitAdmin"), + rateLimitAdmin ); } else { // Governance-critical slot: an undeclared one is a visible SKIP, never a silent CLEAN. @@ -510,14 +554,30 @@ contract RolesAuditor { } if (VM.keyExistsJson(json, ".roles.pool.hooks")) { if (isV2) { - (, address hooks) = RolesProbes._tryAddress(pool, "getAdvancedPoolHooks()"); - _checkAddress("pool.hooks", VM.parseJsonAddress(json, ".roles.pool.hooks"), hooks); + (bool okHooks, address hooks) = RolesProbes._tryAddress(pool, "getAdvancedPoolHooks()"); + _checkReadAddress( + "pool.hooks", + "getAdvancedPoolHooks()", + okHooks, + VM.parseJsonAddress(json, ".roles.pool.hooks"), + hooks + ); } else { _fail("pool.hooks", "declared, but the pool is v1.x (no AdvancedPoolHooks surface)"); } } } + /// @dev `isV2 == true` implies the admins were decoded from `getDynamicConfig()`, so `adminsOk` is + /// only ever false on the v1.x path where `getRateLimitAdmin()` did not answer. + function _poolAdmins(address pool) + private + view + returns (bool isV2, bool adminsOk, address rateLimitAdmin, address feeAdmin) + { + (isV2, adminsOk,, rateLimitAdmin, feeAdmin) = RolesProbes._tryPoolAdmins(pool); + } + /// @dev Chainlink `Ownable2Step`/`ConfirmedOwner` keep `s_pendingOwner` private with no getter; /// `RolesProbes._tryPendingOwner` reads it from storage, self-checked against the live `owner()` /// (see its natspec). A non-zero pending is a WARN - an ownership transfer is in flight and the @@ -549,15 +609,45 @@ contract RolesAuditor { } address declared = VM.parseJsonAddress(json, ".roles.lockbox.address"); (bool has, address live) = RolesProbes._tryAddress(pool, "getLockBox()"); - if (has) _checkAddress("lockbox.address", declared, live); - (, address owner_) = RolesProbes._tryAddress(declared, "owner()"); - _checkAddress("lockbox.owner", VM.parseJsonAddress(json, ".roles.lockbox.owner"), owner_); - _auditPendingOwner("lockbox", declared); - (, address[] memory callers) = + if (has) { + _checkAddress("lockbox.address", declared, live); + } else { + _fail("lockbox.address", "declared, but the pool did not answer getLockBox(), so it was not confirmed"); + } + (bool okOwner, address owner_) = RolesProbes._tryAddress(declared, "owner()"); + (bool okCallers, address[] memory callers) = RolesProbes._tryAddressArray(declared, abi.encodeWithSignature("getAllAuthorizedCallers()")); - _checkSet( - "lockbox.authorizedCallers", VM.parseJsonAddressArray(json, ".roles.lockbox.authorizedCallers"), callers - ); + // The surface gate: a declared lockbox that answers neither getter cannot be audited at all, + // and per-field refusals below it would only repeat the same fact. One FAIL, then stop. + if (!okOwner && !okCallers) { + _fail( + "lockbox.address", + string.concat( + VM.toString(declared), + " answers neither owner() nor getAllAuthorizedCallers(), so it cannot be audited as an ERC20LockBox" + ) + ); + return; + } + if (VM.keyExistsJson(json, ".roles.lockbox.owner")) { + _checkReadAddress( + "lockbox.owner", "owner()", okOwner, VM.parseJsonAddress(json, ".roles.lockbox.owner"), owner_ + ); + } else { + _skip("lockbox.owner", "not declared - run snapshot-chain to backfill it"); + } + _auditPendingOwner("lockbox", declared); + if (VM.keyExistsJson(json, ".roles.lockbox.authorizedCallers")) { + _checkReadSet( + "lockbox.authorizedCallers", + "getAllAuthorizedCallers()", + okCallers, + VM.parseJsonAddressArray(json, ".roles.lockbox.authorizedCallers"), + callers + ); + } else { + _skip("lockbox.authorizedCallers", "not declared - run snapshot-chain to backfill it"); + } } function _auditHooks(string memory json) private { @@ -566,10 +656,68 @@ contract RolesAuditor { return; } address hooks = VM.parseJsonAddress(json, ".roles.hooks.address"); - (, address owner_) = RolesProbes._tryAddress(hooks, "owner()"); - _checkAddress("hooks.owner", VM.parseJsonAddress(json, ".roles.hooks.owner"), owner_); + // The surface gate: a hooks contract that answers none of its getters is unauditable, and every + // declared field under it would only refuse for the same reason. One FAIL naming the address, + // then stop - this is what separates "the chain confirmed an empty allowlist" from "nothing + // was read and empty is what failure looks like". + if (_hooksAnswersNothing(hooks)) { + _fail( + "hooks.address", + string.concat( + VM.toString(hooks), + " answers no AdvancedPoolHooks getter (owner/getAllowListEnabled/getAllowList/", + "getAllAuthorizedCallers), so it cannot be audited as a hooks contract" + ) + ); + return; + } + _auditHooksOwner(json, hooks); _auditPendingOwner("hooks", hooks); - (, bool allowlistEnabled) = RolesProbes._tryBool(hooks, "getAllowListEnabled()"); + _auditHooksAllowlistEnabled(json, hooks); + _auditHooksSet(json, hooks, "allowlist", "getAllowList()"); + _auditHooksSet(json, hooks, "authorizedCallers", "getAllAuthorizedCallers()"); + if (VM.keyExistsJson(json, ".roles.hooks.policyEngine")) { + (bool okEngine, address engine) = RolesProbes._tryAddress(hooks, "getPolicyEngine()"); + _checkReadAddress( + "hooks.policyEngine", + "getPolicyEngine()", + okEngine, + VM.parseJsonAddress(json, ".roles.hooks.policyEngine"), + engine + ); + } + } + + function _hooksAnswersNothing(address hooks) private view returns (bool) { + (bool okOwner,) = RolesProbes._tryAddress(hooks, "owner()"); + if (okOwner) return false; + (bool okEnabled,) = RolesProbes._tryBool(hooks, "getAllowListEnabled()"); + if (okEnabled) return false; + (bool okAllowlist,) = RolesProbes._tryAddressArray(hooks, abi.encodeWithSignature("getAllowList()")); + if (okAllowlist) return false; + (bool okCallers,) = RolesProbes._tryAddressArray(hooks, abi.encodeWithSignature("getAllAuthorizedCallers()")); + return !okCallers; + } + + function _auditHooksOwner(string memory json, address hooks) private { + if (!VM.keyExistsJson(json, ".roles.hooks.owner")) { + _skip("hooks.owner", "not declared - run snapshot-chain to backfill it"); + return; + } + (bool ok, address owner_) = RolesProbes._tryAddress(hooks, "owner()"); + _checkReadAddress("hooks.owner", "owner()", ok, VM.parseJsonAddress(json, ".roles.hooks.owner"), owner_); + } + + function _auditHooksAllowlistEnabled(string memory json, address hooks) private { + if (!VM.keyExistsJson(json, ".roles.hooks.allowlistEnabled")) { + _skip("hooks.allowlistEnabled", "not declared - run snapshot-chain to backfill it"); + return; + } + (bool ok, bool allowlistEnabled) = RolesProbes._tryBool(hooks, "getAllowListEnabled()"); + if (!ok) { + _refuseUnread("hooks.allowlistEnabled", "getAllowListEnabled()"); + return; + } bool declaredEnabled = VM.parseJsonBool(json, ".roles.hooks.allowlistEnabled"); if (declaredEnabled == allowlistEnabled) { _pass("hooks.allowlistEnabled", allowlistEnabled ? "true (IMMUTABLE - set at deploy)" : "false"); @@ -585,15 +733,16 @@ contract RolesAuditor { ) ); } - (, address[] memory allowlist) = RolesProbes._tryAddressArray(hooks, abi.encodeWithSignature("getAllowList()")); - _checkSet("hooks.allowlist", VM.parseJsonAddressArray(json, ".roles.hooks.allowlist"), allowlist); - (, address[] memory callers) = - RolesProbes._tryAddressArray(hooks, abi.encodeWithSignature("getAllAuthorizedCallers()")); - _checkSet("hooks.authorizedCallers", VM.parseJsonAddressArray(json, ".roles.hooks.authorizedCallers"), callers); - if (VM.keyExistsJson(json, ".roles.hooks.policyEngine")) { - (, address engine) = RolesProbes._tryAddress(hooks, "getPolicyEngine()"); - _checkAddress("hooks.policyEngine", VM.parseJsonAddress(json, ".roles.hooks.policyEngine"), engine); + } + + function _auditHooksSet(string memory json, address hooks, string memory label, string memory sig) private { + string memory field = string.concat("hooks.", label); + if (!VM.keyExistsJson(json, string.concat(".roles.hooks.", label))) { + _skip(field, "not declared - run snapshot-chain to backfill it"); + return; } + (bool ok, address[] memory live) = RolesProbes._tryAddressArray(hooks, abi.encodeWithSignature(sig)); + _checkReadSet(field, sig, ok, VM.parseJsonAddressArray(json, string.concat(".roles.hooks.", label)), live); } function _auditRebalancer(string memory json, address pool) private { @@ -859,7 +1008,7 @@ contract RolesAuditor { uint256 declared = VM.parseJsonUint(json, ".roles.governance.safe.threshold"); (bool ok, uint256 live) = RolesProbes._tryUint(safe, "getThreshold()"); if (!ok) { - _fail("governance.safe.threshold", "getThreshold() could not be read, so nothing was compared"); + _refuseUnread("governance.safe.threshold", "getThreshold()"); } else if (declared == live) { _pass("governance.safe.threshold", VM.toString(live)); } else { @@ -872,13 +1021,13 @@ contract RolesAuditor { if (VM.keyExistsJson(json, ".roles.governance.safe.owners")) { (bool ok, address[] memory owners) = RolesProbes._tryAddressArray(safe, abi.encodeWithSignature("getOwners()")); - if (!ok) { - _fail("governance.safe.owners", "getOwners() could not be read, so nothing was compared"); - } else { - _checkSet( - "governance.safe.owners", VM.parseJsonAddressArray(json, ".roles.governance.safe.owners"), owners - ); - } + _checkReadSet( + "governance.safe.owners", + "getOwners()", + ok, + VM.parseJsonAddressArray(json, ".roles.governance.safe.owners"), + owners + ); } } @@ -891,8 +1040,10 @@ contract RolesAuditor { _pass("governance.timelock.address", string.concat(VM.toString(tl), " (has code)")); if (VM.keyExistsJson(json, ".roles.governance.timelock.minDelay")) { uint256 declared = VM.parseJsonUint(json, ".roles.governance.timelock.minDelay"); - (, uint256 live) = RolesProbes._tryUint(tl, "getMinDelay()"); - if (declared == live) { + (bool ok, uint256 live) = RolesProbes._tryUint(tl, "getMinDelay()"); + if (!ok) { + _refuseUnread("governance.timelock.minDelay", "getMinDelay()"); + } else if (declared == live) { _pass("governance.timelock.minDelay", VM.toString(live)); } else { _fail( diff --git a/src/roles/RolesProbes.sol b/src/roles/RolesProbes.sol index 9131264..37a8f91 100644 --- a/src/roles/RolesProbes.sol +++ b/src/roles/RolesProbes.sol @@ -45,6 +45,9 @@ library RolesProbes { // ---------------------------------------------------------------- generic tolerant getters + /// @dev Callers gate on `ok` before using the value: the zero value is what a failed read returns, + /// not a fact about the target, so comparing or authorizing against it asserts something nobody + /// observed. The same contract holds for every probe below. function _tryAddress(address target, string memory sig) internal view returns (bool ok, address val) { (bool s, bytes memory ret) = target.staticcall(abi.encodeWithSignature(sig)); if (s && ret.length >= 32) return (true, abi.decode(ret, (address))); @@ -279,15 +282,27 @@ library RolesProbes { internal view returns (bool isV2, address router, address rateLimitAdmin, address feeAdmin) + { + (isV2,, router, rateLimitAdmin, feeAdmin) = _tryPoolAdmins(pool); + } + + /// @notice `_readPoolAdmins` with the read outcome kept: `adminsOk` is false when NEITHER admin + /// surface answered, in which case the three addresses are the zero value of a failed read, not a + /// fact about the pool. Callers that record or reconcile the admins use this variant; callers that + /// only need the values for a comparison a zero cannot satisfy may keep `_readPoolAdmins`. + function _tryPoolAdmins(address pool) + internal + view + returns (bool isV2, bool adminsOk, address router, address rateLimitAdmin, address feeAdmin) { (bool s, bytes memory ret) = pool.staticcall(abi.encodeWithSignature("getDynamicConfig()")); if (s && ret.length >= 96) { (router, rateLimitAdmin, feeAdmin) = abi.decode(ret, (address, address, address)); - return (true, router, rateLimitAdmin, feeAdmin); + return (true, true, router, rateLimitAdmin, feeAdmin); } (, router) = _tryAddress(pool, "getRouter()"); - (, rateLimitAdmin) = _tryAddress(pool, "getRateLimitAdmin()"); - return (false, router, rateLimitAdmin, address(0)); + (bool okRla, address rla) = _tryAddress(pool, "getRateLimitAdmin()"); + return (false, okRla, router, rla, address(0)); } // ---------------------------------------------------------------- governance probes diff --git a/src/roles/RolesSnapshot.sol b/src/roles/RolesSnapshot.sol index a628fcb..0be8b5c 100644 --- a/src/roles/RolesSnapshot.sol +++ b/src/roles/RolesSnapshot.sol @@ -25,6 +25,11 @@ import {ProjectStore} from "../utils/ProjectStore.sol"; contract RolesSnapshot { Vm private constant VM = Vm(address(uint160(uint256(keccak256("hevm cheat code"))))); + /// @notice How many chain-readable fields the last `build` could NOT read (each is logged UNREAD + /// and omitted from the written block). A caller that persists the block uses this to say whether + /// the snapshot is complete or partial. + uint256 public unreadCount; + bytes32 private constant ROLE_GRANTED_TOPIC = keccak256("RoleGranted(bytes32,address,address)"); bytes32 private constant ROLE_REVOKED_TOPIC = keccak256("RoleRevoked(bytes32,address,address)"); @@ -36,8 +41,10 @@ contract RolesSnapshot { address pool; address tar; address poolOwner; + bool poolOwnerOk; // false = owner() did not answer; the field is then NOT written address tarAdmin; bool isV2; + bool poolAdminsOk; // false = neither admin surface answered; rateLimitAdmin is then NOT written address rateLimitAdmin; address feeAdmin; address ccipAdmin; @@ -53,6 +60,7 @@ contract RolesSnapshot { external returns (string memory) { + unreadCount = 0; Ctx memory c; c.name = name; (c.token, c.pool) = _resolveProject(name, projectJson); @@ -61,8 +69,8 @@ contract RolesSnapshot { : address(0); c.tar = _resolveTar(projectJson, ccipTar); c.template = RolesProbes._detectTemplate(c.token); - (c.isV2,, c.rateLimitAdmin, c.feeAdmin) = RolesProbes._readPoolAdmins(c.pool); - (, c.poolOwner) = RolesProbes._tryAddress(c.pool, "owner()"); + (c.isV2, c.poolAdminsOk,, c.rateLimitAdmin, c.feeAdmin) = RolesProbes._tryPoolAdmins(c.pool); + (c.poolOwnerOk, c.poolOwner) = RolesProbes._tryAddress(c.pool, "owner()"); (c.tarAdmin,) = _tarConfig(c.tar, c.token); return _assemble(c, projectJson); } @@ -140,8 +148,10 @@ contract RolesSnapshot { c.adminHolder = _firstAdminCandidate(c, json); VM.serializeString(obj, "defaultAdmins", _defaultAdminsBlock(c, json)); } else if (c.template == RolesProbes.TokenTemplate.FactoryBurnMintERC20) { - (, c.adminHolder) = RolesProbes._tryAddress(c.token, "owner()"); - VM.serializeAddress(obj, "owner", c.adminHolder); + (bool okOwner, address tokenOwner) = RolesProbes._tryAddress(c.token, "owner()"); + c.adminHolder = tokenOwner; + if (okOwner) VM.serializeAddress(obj, "owner", c.adminHolder); + else _logUnread("token.owner", "owner()"); } else { _tokenAdminByo(c, json, obj); } @@ -161,10 +171,13 @@ contract RolesSnapshot { /// @dev CrossChainToken admin surface: the single-holder two-step default admin + the separate /// `BURN_MINT_ADMIN_ROLE` that admins MINTER/BURNER - NOT moved by a defaultAdmin transfer. function _tokenAdminCrossChain(Ctx memory c, string memory json, string memory obj) private { - (, c.adminHolder) = RolesProbes._tryAddress(c.token, "defaultAdmin()"); - VM.serializeAddress(obj, "defaultAdmin", c.adminHolder); - (, address pending) = RolesProbes._tryAddress(c.token, "pendingDefaultAdmin()"); - if (pending != address(0)) VM.serializeAddress(obj, "pendingDefaultAdmin", pending); + (bool okAdmin, address adminHolder) = RolesProbes._tryAddress(c.token, "defaultAdmin()"); + c.adminHolder = adminHolder; + if (okAdmin) VM.serializeAddress(obj, "defaultAdmin", c.adminHolder); + else _logUnread("token.defaultAdmin", "defaultAdmin()"); + (bool okPending, address pending) = RolesProbes._tryAddress(c.token, "pendingDefaultAdmin()"); + if (!okPending) _logUnread("token.pendingDefaultAdmin", "pendingDefaultAdmin()"); + else if (pending != address(0)) VM.serializeAddress(obj, "pendingDefaultAdmin", pending); bytes32 role = RolesProbes._roleIdOrDefault(c.token, "BURN_MINT_ADMIN_ROLE()", RolesProbes.BURN_MINT_ADMIN_ROLE); address[] memory candidates = _candidates(c, json, ".roles.token.burnMintRoleAdmins.holders"); VM.serializeString(obj, "burnMintRoleAdmins", _holdersBlock(c, "burnMintRoleAdmins", "", role, candidates)); @@ -441,16 +454,95 @@ contract RolesSnapshot { function _poolBlock(Ctx memory c) private returns (string memory) { string memory obj = string.concat("roles-pool-", c.name); - VM.serializeAddress(obj, "address", c.pool); - VM.serializeAddress(obj, "rateLimitAdmin", c.rateLimitAdmin); + string memory out = VM.serializeAddress(obj, "address", c.pool); + if (c.poolAdminsOk) out = VM.serializeAddress(obj, "rateLimitAdmin", c.rateLimitAdmin); + else _logUnread("pool.rateLimitAdmin", "getDynamicConfig()/getRateLimitAdmin()"); if (c.isV2) { - VM.serializeAddress(obj, "feeAdmin", c.feeAdmin); - (, address hooks) = RolesProbes._tryAddress(c.pool, "getAdvancedPoolHooks()"); - VM.serializeAddress(obj, "hooks", hooks); + out = VM.serializeAddress(obj, "feeAdmin", c.feeAdmin); + (bool okHooks, address hooks) = RolesProbes._tryAddress(c.pool, "getAdvancedPoolHooks()"); + if (okHooks) out = VM.serializeAddress(obj, "hooks", hooks); + else _logUnread("pool.hooks", "getAdvancedPoolHooks()"); } - // NOTE: a 1.5.0 pool's pendingOwner has NO getter (ConfirmedOwner) - the owner read below is + // NOTE: a 1.5.0 pool's pendingOwner has NO getter (ConfirmedOwner) - the owner read here is // the only ownership fact snapshottable across all cataloged versions. - return VM.serializeAddress(obj, "owner", c.poolOwner); + if (c.poolOwnerOk) out = VM.serializeAddress(obj, "owner", c.poolOwner); + else _logUnread("pool.owner", "owner()"); + return out; + } + + /// @dev A field the chain did not supply is NOT written - absence is the unread state + /// (docs/config-schema.md). Writing the probe's zero value instead would let the next audit + /// reconcile the failed read against itself and pass. + function _logUnread(string memory field, string memory sig) private { + unreadCount++; + console.log(string.concat("[snapshot] UNREAD roles.", field, ": ", sig, " did not answer - field not written")); + } + + /// @dev Probe-and-serialize one field: written when the read answered, logged as UNREAD (and the + /// accumulated JSON passed through unchanged) when it did not. One helper per probed type keeps + /// the ok flags out of the block builders' stack frames. + function _putAddr( + string memory obj, + string memory prefix, + string memory key, + string memory out, + address target, + string memory sig + ) private returns (string memory) { + (bool ok, address v) = RolesProbes._tryAddress(target, sig); + if (!ok) { + _logUnread(string.concat(prefix, ".", key), sig); + return out; + } + return VM.serializeAddress(obj, key, v); + } + + function _putBool( + string memory obj, + string memory prefix, + string memory key, + string memory out, + address target, + string memory sig + ) private returns (string memory) { + (bool ok, bool v) = RolesProbes._tryBool(target, sig); + if (!ok) { + _logUnread(string.concat(prefix, ".", key), sig); + return out; + } + return VM.serializeBool(obj, key, v); + } + + function _putUint( + string memory obj, + string memory prefix, + string memory key, + string memory out, + address target, + string memory sig + ) private returns (string memory) { + (bool ok, uint256 v) = RolesProbes._tryUint(target, sig); + if (!ok) { + _logUnread(string.concat(prefix, ".", key), sig); + return out; + } + return VM.serializeUint(obj, key, v); + } + + function _putAddrArray( + string memory obj, + string memory prefix, + string memory key, + string memory out, + address target, + string memory sig + ) private returns (string memory) { + (bool ok, address[] memory v) = RolesProbes._tryAddressArray(target, abi.encodeWithSignature(sig)); + if (!ok) { + _logUnread(string.concat(prefix, ".", key), sig); + return out; + } + return VM.serializeAddress(obj, key, v); } // ---------------------------------------------------------------- lockbox / hooks @@ -458,13 +550,11 @@ contract RolesSnapshot { function _lockboxBlock(string memory name, address pool) private returns (string memory) { (bool has, address lockbox) = RolesProbes._tryAddress(pool, "getLockBox()"); if (!has || lockbox == address(0)) return ""; - (, address owner_) = RolesProbes._tryAddress(lockbox, "owner()"); - (, address[] memory callers) = - RolesProbes._tryAddressArray(lockbox, abi.encodeWithSignature("getAllAuthorizedCallers()")); string memory obj = string.concat("roles-lockbox-", name); - VM.serializeAddress(obj, "address", lockbox); - VM.serializeAddress(obj, "owner", owner_); - return VM.serializeAddress(obj, "authorizedCallers", callers); + string memory out = VM.serializeAddress(obj, "address", lockbox); + out = _putAddr(obj, "lockbox", "owner", out, lockbox, "owner()"); + out = _putAddrArray(obj, "lockbox", "authorizedCallers", out, lockbox, "getAllAuthorizedCallers()"); + return out; } /// @dev Hooks resolution: the pool's live `getAdvancedPoolHooks()` (v2, when attached) > the @@ -479,19 +569,14 @@ contract RolesSnapshot { hooks = VM.parseJsonAddress(json, ".roles.hooks.address"); } if (hooks == address(0)) return ""; - (, address owner_) = RolesProbes._tryAddress(hooks, "owner()"); - (, bool allowlistEnabled) = RolesProbes._tryBool(hooks, "getAllowListEnabled()"); - (, address[] memory allowlist) = RolesProbes._tryAddressArray(hooks, abi.encodeWithSignature("getAllowList()")); - (, address[] memory callers) = - RolesProbes._tryAddressArray(hooks, abi.encodeWithSignature("getAllAuthorizedCallers()")); - (, address policyEngine) = RolesProbes._tryAddress(hooks, "getPolicyEngine()"); string memory obj = string.concat("roles-hooks-", name); - VM.serializeAddress(obj, "address", hooks); - VM.serializeAddress(obj, "owner", owner_); - VM.serializeBool(obj, "allowlistEnabled", allowlistEnabled); - VM.serializeAddress(obj, "allowlist", allowlist); - VM.serializeAddress(obj, "policyEngine", policyEngine); - return VM.serializeAddress(obj, "authorizedCallers", callers); + string memory out = VM.serializeAddress(obj, "address", hooks); + out = _putAddr(obj, "hooks", "owner", out, hooks, "owner()"); + out = _putBool(obj, "hooks", "allowlistEnabled", out, hooks, "getAllowListEnabled()"); + out = _putAddrArray(obj, "hooks", "allowlist", out, hooks, "getAllowList()"); + out = _putAddr(obj, "hooks", "policyEngine", out, hooks, "getPolicyEngine()"); + out = _putAddrArray(obj, "hooks", "authorizedCallers", out, hooks, "getAllAuthorizedCallers()"); + return out; } // ---------------------------------------------------------------- governance (OPTIONAL, three shapes) @@ -527,17 +612,15 @@ contract RolesSnapshot { string memory out = ""; if (safe != address(0)) { string memory sObj = string.concat("roles-gov-safe-", name); - VM.serializeAddress(sObj, "address", safe); - (, uint256 threshold) = RolesProbes._tryUint(safe, "getThreshold()"); - VM.serializeUint(sObj, "threshold", threshold); - (, address[] memory owners) = RolesProbes._tryAddressArray(safe, abi.encodeWithSignature("getOwners()")); - out = VM.serializeString(obj, "safe", VM.serializeAddress(sObj, "owners", owners)); + string memory sOut = VM.serializeAddress(sObj, "address", safe); + sOut = _putUint(sObj, "governance.safe", "threshold", sOut, safe, "getThreshold()"); + sOut = _putAddrArray(sObj, "governance.safe", "owners", sOut, safe, "getOwners()"); + out = VM.serializeString(obj, "safe", sOut); } if (timelock != address(0)) { string memory tObj = string.concat("roles-gov-tl-", name); - VM.serializeAddress(tObj, "address", timelock); - (, uint256 minDelay) = RolesProbes._tryUint(timelock, "getMinDelay()"); - string memory tl = VM.serializeUint(tObj, "minDelay", minDelay); + string memory tl = VM.serializeAddress(tObj, "address", timelock); + tl = _putUint(tObj, "governance.timelock", "minDelay", tl, timelock, "getMinDelay()"); tl = _copyDeclaredList(json, ".roles.governance.timelock.proposers", tObj, "proposers", tl); tl = _copyDeclaredList(json, ".roles.governance.timelock.cancellers", tObj, "cancellers", tl); tl = _copyDeclaredList(json, ".roles.governance.timelock.executors", tObj, "executors", tl); diff --git a/test/config/VerifyChainLaneReconcile.t.sol b/test/config/VerifyChainLaneReconcile.t.sol index 57fa84b..0fd7317 100644 --- a/test/config/VerifyChainLaneReconcile.t.sol +++ b/test/config/VerifyChainLaneReconcile.t.sol @@ -457,6 +457,44 @@ contract VerifyChainLaneReconcileForkTest is BaseForkTest, LaneReconcileScratch /// @dev A minimal v1-surface pool mock: `typeAndVersion()` is constructor-set, the chain-membership /// getters answer for one selector once "applied", and ONLY the per-direction v1 rate-limit getters +/// @dev A 1.5.0-shaped pool whose `getSupportedChains()` REVERTS: the forward reconcile works, the +/// reverse (on-chain -> declared) check cannot run. Pins that an unanswering deployed pool is an +/// unverified gap, not a designed absence. +contract MockNoReversePool { + uint64 private immutable i_selector; + RateLimiter.TokenBucket private s_outbound; + RateLimiter.TokenBucket private s_inbound; + + constructor(uint64 selector_) { + i_selector = selector_; + } + + function applyLane(RateLimiter.TokenBucket memory outbound, RateLimiter.TokenBucket memory inbound) external { + s_outbound = outbound; + s_inbound = inbound; + } + + function typeAndVersion() external pure returns (string memory) { + return "BurnMintTokenPool 1.5.0"; + } + + function isSupportedChain(uint64 remoteChainSelector) external view returns (bool) { + return remoteChainSelector == i_selector; + } + + function getSupportedChains() external pure returns (uint64[] memory) { + revert("no reverse enumeration"); + } + + function getCurrentOutboundRateLimiterState(uint64) external view returns (RateLimiter.TokenBucket memory) { + return s_outbound; + } + + function getCurrentInboundRateLimiterState(uint64) external view returns (RateLimiter.TokenBucket memory) { + return s_inbound; + } +} + /// exist (no `getCurrentRateLimiterState(uint64,bool)`), so any v2-first read on it reverts. That /// absence is the point: a clean reconcile proves the rung dispatched the v1 getters. contract MockV1Pool { @@ -624,6 +662,25 @@ contract VerifyChainLaneReconcileMockTest is LaneReconcileScratch { // An unrecognized typeAndVersion: one best-effort WARN, then reads degrade (v2 getter first, // v1 fallback - the mock only answers v1) and still reconcile the declared policy. + // A deployed pool that does not answer getSupportedChains(): the forward reconcile stays clean, + // and the skipped reverse check counts as an unverified gap (toward the INCOMPLETE verdict), + // never a FAIL or a designed skip. + function test_Lanes_PoolNotAnsweringReverse_CountsUnverifiedGap() public { + string memory name = "zz-scratch-lanechk-m4"; + _writeScratchChain(name, 887001401, 8_870_014_010_000_000_001); + _declareLane(name, "zz-scratch-lanechk-mr4", _laneEntry(SEL, CAPACITY, RATE, _inboundBlock())); + + MockNoReversePool mockPool = new MockNoReversePool(SEL); + mockPool.applyLane(_bucket(true, CAPACITY, RATE), _bucket(true, CAPACITY, RATE)); + + VerifyChain vc = new VerifyChain(); + (uint256 fails,) = vc.checkLanesOnChainForTest(name, address(mockPool)); + assertEq(fails, 0, "an unanswering reverse enumeration must not FAIL"); + assertEq(vc.unverifiedForTest(), 1, "the skipped reverse check must count as an unverified gap"); + + _cleanupScratchOne(name); + } + function test_Lanes_Warn_UnknownVersionDegradesToBestEffort() public { string memory name = "zz-scratch-lanechk-m3"; _writeScratchChain(name, 887001301, 8_870_013_010_000_000_001); diff --git a/test/config/VerifyChainVerdict.t.sol b/test/config/VerifyChainVerdict.t.sol new file mode 100644 index 0000000..583f91a --- /dev/null +++ b/test/config/VerifyChainVerdict.t.sol @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; + +import {Test} from "forge-std/Test.sol"; +import {VerifyChain} from "../../script/config/VerifyChain.s.sol"; + +/// @title VerifyChainVerdictTest - the doctor's three-outcome contract +/// @notice The doctor returns VERIFIED / INCOMPLETE / FAILED, not a boolean. The state this pins is +/// INCOMPLETE: a run whose declared or deployed checks could not run (an unset RPC, a contract that +/// did not answer) must not exit clean (the rationale lives on `_verdict`). Designed absences (non-EVM +/// rungs, undeclared optional blocks) stay plain SKIPs and never taint the verdict: there, nothing +/// claimable was left unchecked. Driven through `verdictForTest`, which seeds the counters exactly as +/// the rungs do; the full-run path (rpcEnv unset -> nonzero exit) is exercised by +/// `script/config/test-tooling.sh`. +contract VerifyChainVerdictTest is Test { + string internal constant NAME = "zz-tt-verdict"; + + function test_Verdict_NothingSkipped_IsVerified() public { + new VerifyChain().verdictForTest(NAME, false, false, false); + } + + function test_Verdict_DesignedSkip_StaysVerified() public { + new VerifyChain().verdictForTest(NAME, true, false, false); + } + + function test_Verdict_UnverifiedGap_IsIncompleteNotClean() public { + try new VerifyChain().verdictForTest(NAME, false, true, false) { + assertTrue(false, "a run with an unverified gap must not exit clean"); + } catch Error(string memory reason) { + _assertContains(reason, string.concat("check-chain INCOMPLETE for ", NAME)); + } + } + + /// @dev A genuine failure outranks incompleteness: the operator fixes the FAIL first, and the + /// gap resurfaces on the re-run. + function test_Verdict_FailWithGap_IsFailedNotIncomplete() public { + try new VerifyChain().verdictForTest(NAME, false, true, true) { + assertTrue(false, "a run with a failure must not exit clean"); + } catch Error(string memory reason) { + _assertContains(reason, string.concat("check-chain FAILED for ", NAME)); + } + } + + /// @dev Pins each RPC-gated site individually: registry TAR reconcile, roles rung, lanes rung. + /// A regression demoting any of them back to a designed skip changes the count. + function test_RpcGatedRungs_EachCountsOneUnverifiedGap() public { + uint256 skips = new VerifyChain() + .rpcGatedSkipsForTest('{"roles":{"token":{"address":"0x0000000000000000000000000000000000000001"}}}'); + assertEq(skips, 3, "registry TAR, roles, and lanes each count one unverified gap without an RPC"); + } + + function _assertContains(string memory haystack, string memory needle) internal pure { + assertTrue(_contains(haystack, needle), string.concat("expected \"", needle, "\" in: ", haystack)); + } + + function _contains(string memory s, string memory needle) internal pure returns (bool) { + bytes memory b = bytes(s); + bytes memory n = bytes(needle); + if (n.length > b.length) return false; + for (uint256 i = 0; i + n.length <= b.length; i++) { + bool hit = true; + for (uint256 j = 0; j < n.length; j++) { + if (b[i + j] != n[j]) { + hit = false; + break; + } + } + if (hit) return true; + } + return false; + } +} diff --git a/test/configure/RateLimitBucketInputs.t.sol b/test/configure/RateLimitBucketInputs.t.sol new file mode 100644 index 0000000..c7bce41 --- /dev/null +++ b/test/configure/RateLimitBucketInputs.t.sol @@ -0,0 +1,120 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; + +import {Test} from "forge-std/Test.sol"; +import {RateLimiterUtils} from "../../script/utils/RateLimiterUtils.s.sol"; + +/// @notice What a rate-limit bucket accepts as input, all-or-nothing per direction. +/// +/// An unset variable must never become a 0 written on chain; the failure modes that rule prevents are +/// documented on `RateLimiterUtils._establishBucket`, the single decision every consumer routes +/// through. The decision is +/// pinned through `_establishBucket`, which takes the set-flags and values as arguments: `vm.setEnv` is +/// process-wide while tests run in parallel, and the apply scripts read the same variables, so these +/// tests never touch the environment. +contract RateLimitBucketInputsTest is Test { + function test_NothingSet_LeavesTheDirectionUntouched() public pure { + (bool update,,,) = RateLimiterUtils._establishBucket("OUTBOUND", false, false, false, 0, false, 0); + assertFalse(update); + } + + function test_FullBucket_Applies() public pure { + (bool update, bool enabled, uint128 capacity, uint128 rate) = + RateLimiterUtils._establishBucket("OUTBOUND", true, true, true, 1000, true, 10); + assertTrue(update); + assertTrue(enabled); + assertEq(capacity, 1000); + assertEq(rate, 10); + } + + function test_CapacityAndRate_ImplyEnabled() public pure { + // enabled=false here is deliberately ignorable: with enabledSet=false the helper computes the + // implied default itself, so a consumer cannot wire it differently. + (bool update, bool enabled, uint128 capacity, uint128 rate) = + RateLimiterUtils._establishBucket("INBOUND", false, false, true, 500, true, 5); + assertTrue(update); + assertTrue(enabled, "supplying capacity and rate implies enabling the bucket"); + assertEq(capacity, 500); + assertEq(rate, 5); + } + + /// @dev The incident-response shape: an explicit disable stands alone. A disabled bucket's + /// capacity and rate are 0 by protocol rule, so the zeros are forced, not guessed. + function test_DisableAlone_IsValid() public pure { + (bool update, bool enabled, uint128 capacity, uint128 rate) = + RateLimiterUtils._establishBucket("OUTBOUND", true, false, false, 0, false, 0); + assertTrue(update); + assertFalse(enabled); + assertEq(capacity, 0); + assertEq(rate, 0); + } + + function test_DisableWithNonZeroCapacity_Refuses() public { + vm.expectRevert( + bytes( + "OUTBOUND: a disabled rate-limit bucket has capacity 0 and rate 0 by protocol rule - drop the" + " CAPACITY/RATE variables or set OUTBOUND_RATE_LIMIT_ENABLED=true" + ) + ); + this.establish("OUTBOUND", true, false, true, 7, false, 0); + } + + function test_CapacityAlone_RefusesNamingRate() public { + vm.expectRevert( + bytes( + "OUTBOUND_RATE_LIMIT_RATE is not set - an enabled bucket needs CAPACITY and RATE supplied" + " together; an unset field must not become a 0 written on chain" + ) + ); + this.establish("OUTBOUND", false, false, true, 1000, false, 0); + } + + function test_RateAlone_RefusesNamingCapacity() public { + vm.expectRevert( + bytes( + "INBOUND_RATE_LIMIT_CAPACITY is not set - an enabled bucket needs CAPACITY and RATE supplied" + " together; an unset field must not become a 0 written on chain" + ) + ); + this.establish("INBOUND", false, false, false, 0, true, 10); + } + + function test_EnabledAlone_RefusesNamingCapacity() public { + vm.expectRevert( + bytes( + "OUTBOUND_RATE_LIMIT_CAPACITY is not set - an enabled bucket needs CAPACITY and RATE supplied" + " together; an unset field must not become a 0 written on chain" + ) + ); + this.establish("OUTBOUND", true, true, false, 0, false, 0); + } + + /// @dev 1e39 wraps to a plausible wrong capacity under a raw uint128 cast; 2^128 wraps to exactly + /// 0. Both are refused instead of becoming a different live value. + function test_Overflow_RefusesInsteadOfTruncating() public { + vm.expectRevert( + bytes( + "OUTBOUND_RATE_LIMIT_CAPACITY does not fit uint128 - refusing to truncate it into a different live value" + ) + ); + this.establish("OUTBOUND", false, false, true, 1e39, true, 10); + + vm.expectRevert( + bytes("INBOUND_RATE_LIMIT_RATE does not fit uint128 - refusing to truncate it into a different live value") + ); + this.establish("INBOUND", false, false, true, 1000, true, 2 ** 128); + } + + /// @notice External for `expectRevert`: an internal library call would revert the test itself. + function establish( + string memory direction, + bool enabledSet, + bool enabled, + bool capacitySet, + uint256 capacity, + bool rateSet, + uint256 rate + ) external pure returns (bool, bool, uint128, uint128) { + return RateLimiterUtils._establishBucket(direction, enabledSet, enabled, capacitySet, capacity, rateSet, rate); + } +} diff --git a/test/configure/SetDynamicConfigPreserve.t.sol b/test/configure/SetDynamicConfigPreserve.t.sol new file mode 100644 index 0000000..cba5da9 --- /dev/null +++ b/test/configure/SetDynamicConfigPreserve.t.sol @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; + +import {Test} from "forge-std/Test.sol"; +import {SetDynamicConfig} from "../../script/configure/dynamic-config/SetDynamicConfig.s.sol"; + +/// @dev Exposes the resolution with the env access swapped for an injectable fake (the `_dcEnvOr` +/// seam exists for exactly this: ROUTER/RATE_LIMIT_ADMIN/FEE_ADMIN are process-wide names and +/// suites run in parallel, so tests must never `vm.setEnv` them). +contract DynamicConfigHarness is SetDynamicConfig { + mapping(bytes32 => address) private fakeEnv; + mapping(bytes32 => bool) private fakeSet; + + function setFakeEnv(string memory name, address value) external { + fakeEnv[keccak256(bytes(name))] = value; + fakeSet[keccak256(bytes(name))] = true; + } + + function _dcEnvOr(string memory name, address defaultValue) internal view override returns (address) { + return fakeSet[keccak256(bytes(name))] ? fakeEnv[keccak256(bytes(name))] : defaultValue; + } + + function resolve(address currentRouter, address currentRla, address currentFee) + external + view + returns (address, address, address) + { + return _resolveNewConfig(currentRouter, currentRla, currentFee); + } +} + +/// @notice `SetDynamicConfig` writes the pool's whole dynamic-config struct, so an unset variable must +/// preserve the current on-chain value VERBATIM, `address(0)` included. Anything else turns a +/// one-field update into a silent grant of the others: a broadcaster fallback for zero admins would +/// hand both admin slots to the acting account on a `ROUTER`-only run. +contract SetDynamicConfigPreserveTest is Test { + DynamicConfigHarness internal harness; + + address internal constant NEW_ROUTER = address(0xA111); + address internal constant CUR_RLA = address(0xB222); + address internal constant CUR_FEE = address(0xC333); + + function setUp() public { + harness = new DynamicConfigHarness(); + } + + /// @dev The defect shape this pins out: ROUTER alone, both admins unset ON CHAIN. The zeros must + /// come back verbatim, not become the acting account. + function test_RouterOnly_PreservesZeroAdminsVerbatim() public { + harness.setFakeEnv("ROUTER", NEW_ROUTER); + + (address router, address rla, address fee) = harness.resolve(address(0xD00D), address(0), address(0)); + + assertEq(router, NEW_ROUTER, "ROUTER override applies"); + assertEq(rla, address(0), "an unset RATE_LIMIT_ADMIN preserves address(0) verbatim"); + assertEq(fee, address(0), "an unset FEE_ADMIN preserves address(0) verbatim"); + } + + function test_NoOverrides_PreservesEverything() public view { + (address router, address rla, address fee) = harness.resolve(address(0xD00D), CUR_RLA, CUR_FEE); + assertEq(router, address(0xD00D)); + assertEq(rla, CUR_RLA); + assertEq(fee, CUR_FEE); + } + + function test_ExplicitZeroFeeAdmin_IsAppliedNotConfusedWithUnset() public { + harness.setFakeEnv("FEE_ADMIN", address(0)); + + (,, address fee) = harness.resolve(address(0xD00D), CUR_RLA, CUR_FEE); + assertEq(fee, address(0), "FEE_ADMIN=0x0 is an explicit restrict-to-owner instruction"); + } +} diff --git a/test/configure/UpdateRateLimitersLaneSource.t.sol b/test/configure/UpdateRateLimitersLaneSource.t.sol index 6f5bf27..7237524 100644 --- a/test/configure/UpdateRateLimitersLaneSource.t.sol +++ b/test/configure/UpdateRateLimitersLaneSource.t.sol @@ -202,6 +202,38 @@ contract UpdateRateLimitersLaneSourceTest is LaneReconcileScratch { vm.removeFile(_path("zz-scratch-rlsrc-r1")); } + // INBOUND capacity alone through the seam wiring: refused naming the missing RATE. Pins the + // inbound leg of the 12-argument _readRateLimitUpdate -> _establishBucket wiring, which the + // pure bucket tests cannot reach. + function test_Std_InboundCapacityAlone_RefusedNamingRate() public { + harness.setFakeEnv("INBOUND_RATE_LIMIT_CAPACITY", vm.toString(uint256(ENV_CAPACITY))); + + vm.expectRevert( + bytes( + "INBOUND_RATE_LIMIT_RATE is not set - an enabled bucket needs CAPACITY and RATE supplied" + " together; an unset field must not become a 0 written on chain" + ) + ); + harness.resolve("zz-scratch-rlsrc-none", REMOTE_SELECTOR, false); + } + + // A full INBOUND bucket lands through the same wiring, outbound untouched. + function test_Std_InboundFullBucket_LandsThroughTheWiring() public { + string memory local = _localChain(91); + harness.setFakeEnv("INBOUND_RATE_LIMIT_CAPACITY", vm.toString(uint256(ENV_CAPACITY))); + harness.setFakeEnv("INBOUND_RATE_LIMIT_RATE", vm.toString(uint256(ENV_RATE))); + + UpdateRateLimiters.RateLimitResolution memory res = + harness.resolve("zz-scratch-rlsrc-none91", REMOTE_SELECTOR, false); + + _assertInbound(res.update, true, true, ENV_CAPACITY, ENV_RATE); + _assertOutbound(res.update, false, false, 0, 0); + assertTrue(res.inboundFromEnv, "inbound must come from env"); + assertFalse(res.outboundFromEnv, "outbound env vars are unset"); + + _cleanupScratchOne(local); + } + // Env set, lanes{} entry AGREES on the core fields: env used, no divergence, no hint. function test_Std_EnvAndLaneAgree_NoDivergence_NoHint() public { string memory local = _localChain(2); diff --git a/test/roles/UnreadableIsNotEmpty.t.sol b/test/roles/UnreadableIsNotEmpty.t.sol new file mode 100644 index 0000000..77d9763 --- /dev/null +++ b/test/roles/UnreadableIsNotEmpty.t.sol @@ -0,0 +1,126 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; + +import {BaseForkTest} from "../BaseForkTest.t.sol"; +import {RolesAuditor} from "../../src/roles/RolesAuditor.sol"; +import {RolesSnapshot} from "../../src/roles/RolesSnapshot.sol"; + +/// @dev A contract that has code and answers nothing. Every probe against it fails, which is the shape +/// an operator-supplied hooks contract takes when the audit cannot read it. +contract SilentContract { + // Deliberately empty: any call reverts, so every probe fails. + fallback() external { + revert("silent"); + } +} + +/// @notice A field the audit could not read must not reconcile clean. +/// +/// The probes return a zero value alongside an `ok` flag, and where a caller drops that flag, a failed +/// read becomes the assertion "the chain holds 0x0 / false / an empty set". A snapshot then writes those +/// zeros into the declaration and the audit compares them against the same failed reads, so the two +/// agree because neither observed anything. The verdict says the declared authority matches the chain +/// while nothing about that contract was established. +contract UnreadableIsNotEmptyTest is BaseForkTest { + RolesAuditor internal auditor; + address internal silent; + address internal fixtureToken; + address internal fixturePool; + + function setUp() public override { + super.setUp(); + auditor = new RolesAuditor(); + silent = address(new SilentContract()); + (fixtureToken, fixturePool) = deployTokenAndPoolFixture(); + } + + /// @dev Wrap a built `roles{}` object as a project document the auditor can read. + function _wrap(string memory rolesJson) private pure returns (string memory) { + return string.concat("{\"roles\":", rolesJson, "}"); + } + + /// @dev The declaration says the hooks contract holds nothing, which is exactly what the failed + /// probes report. Agreement here is an artefact of the read failing, not a fact about the chain. + function test_HooksThatAnswerNothing_DoNotReconcileClean() public { + string memory roles = string.concat( + '{"token":{"address":"', + vm.toString(fixtureToken), + '","type":"crosschain"},"pool":{"address":"', + vm.toString(fixturePool), + '"},"hooks":{"address":"', + vm.toString(silent), + '","owner":"0x0000000000000000000000000000000000000000",', + '"allowlistEnabled":false,"allowlist":[],"authorizedCallers":[]}}' + ); + + RolesAuditor.Result memory r = auditor.auditJson("ethereum-testnet-sepolia", _wrap(roles)); + + assertGt(r.fails, 0, "a hooks contract whose every probe failed must FAIL: the audit observed nothing about it"); + } + + /// @dev The same address declared with a NON-zero owner. This is the control: it already fails today, + /// which is what makes the test above meaningful rather than a tautology. If the audit reported + /// correctly, the two declarations would be distinguishable only by what the chain actually says, + /// and here the chain says nothing either way. + function test_HooksThatAnswerNothing_FailAgainstANonZeroDeclaration() public { + string memory roles = string.concat( + '{"token":{"address":"', + vm.toString(fixtureToken), + '","type":"crosschain"},"pool":{"address":"', + vm.toString(fixturePool), + '"},"hooks":{"address":"', + vm.toString(silent), + '","owner":"0x000000000000000000000000000000000000BEEF",', + '"allowlistEnabled":false,"allowlist":[],"authorizedCallers":[]}}' + ); + + RolesAuditor.Result memory r = auditor.auditJson("ethereum-testnet-sepolia", _wrap(roles)); + assertGt(r.fails, 0, "a declared owner the chain does not confirm must fail"); + } + + /// @dev The snapshot half of the same contract: a field the chain did not supply is written as + /// ABSENT, never as the probe's zero value, and the audit of that snapshot does not pass. + /// Writing zeros instead would hand the next audit a declaration built from the same failed + /// reads it is about to repeat, and the two would agree about nothing. + function test_Snapshot_WritesUnreadAsAbsent_AndTheAuditDoesNotPassIt() public { + string memory prior = string.concat( + '{"roles":{"token":{"address":"', + vm.toString(fixtureToken), + '"},"pool":{"address":"', + vm.toString(fixturePool), + '"},"hooks":{"address":"', + vm.toString(silent), + '"}}}' + ); + RolesSnapshot snap = new RolesSnapshot(); + string memory built = _wrap( + snap.build("ethereum-testnet-sepolia", vm.readFile("config/chains/ethereum-testnet-sepolia.json"), prior) + ); + + assertTrue(vm.keyExistsJson(built, ".roles.hooks.address"), "the anchor address is still recorded"); + assertFalse(vm.keyExistsJson(built, ".roles.hooks.owner"), "an unread owner is not written"); + assertFalse(vm.keyExistsJson(built, ".roles.hooks.allowlistEnabled"), "an unread flag is not written"); + assertFalse(vm.keyExistsJson(built, ".roles.hooks.allowlist"), "an unread allowlist is not written"); + assertFalse(vm.keyExistsJson(built, ".roles.hooks.authorizedCallers"), "an unread caller set is not written"); + + assertGt(snap.unreadCount(), 0, "unread fields must be counted so SnapshotChain reports the block PARTIAL"); + + RolesAuditor.Result memory r = auditor.auditJson("ethereum-testnet-sepolia", built); + assertGt(r.fails, 0, "auditing a snapshot whose hooks answered nothing must not pass"); + } + + /// @dev The complement: a fully readable fixture snapshots with ZERO unread fields, pinning both + /// the counter's meaning and its per-build reset. + function test_Snapshot_ReadableFixture_CountsZeroUnread() public { + string memory prior = string.concat( + '{"roles":{"token":{"address":"', + vm.toString(fixtureToken), + '"},"pool":{"address":"', + vm.toString(fixturePool), + '"}}}' + ); + RolesSnapshot snap = new RolesSnapshot(); + snap.build("ethereum-testnet-sepolia", vm.readFile("config/chains/ethereum-testnet-sepolia.json"), prior); + assertEq(snap.unreadCount(), 0, "a fully readable fixture must snapshot with no unread fields"); + } +} diff --git a/test/setup/ApplyChainUpdatesLaneSource.t.sol b/test/setup/ApplyChainUpdatesLaneSource.t.sol index 05c391e..04f0627 100644 --- a/test/setup/ApplyChainUpdatesLaneSource.t.sol +++ b/test/setup/ApplyChainUpdatesLaneSource.t.sol @@ -213,17 +213,20 @@ contract ApplyChainUpdatesLaneSourceTest is LaneReconcileScratch { _cleanupScratchOne(local); } - // CAPACITY alone: isEnabled defaults to true and the unset RATE reads 0 - the exact historical - // env semantics, pinned. - function test_CapacityOnly_EnabledDefaultsTrue_RateZero() public { + // CAPACITY alone is refused naming the missing RATE: an unset variable must never become a 0 + // written on chain (an enabled bucket with rate 0 lets N tokens through, then stays shut with + // TokenRateLimitReached). The same shared decision gates UpdateRateLimiters and SetFinalityConfig. + function test_CapacityOnly_IsRefusedNamingRate() public { string memory local = _localChain(13); harness.setFakeEnv("OUTBOUND_RATE_LIMIT_CAPACITY", vm.toString(uint256(ENV_CAPACITY))); - ApplyChainUpdates.RateLimitResolution memory res = harness.resolve("zz-scratch-lanesrc-none13", REMOTE_SELECTOR); - - _assertBucket(res.outbound, true, ENV_CAPACITY, 0); - _assertBucket(res.inbound, false, 0, 0); - assertTrue(res.outboundFromEnv, "CAPACITY alone must select the env rung"); + vm.expectRevert( + bytes( + "OUTBOUND_RATE_LIMIT_RATE is not set - an enabled bucket needs CAPACITY and RATE supplied" + " together; an unset field must not become a 0 written on chain" + ) + ); + harness.resolve("zz-scratch-lanesrc-none13", REMOTE_SELECTOR); _cleanupScratchOne(local); } diff --git a/test/setup/TokenRoleAuthorityGates.t.sol b/test/setup/TokenRoleAuthorityGates.t.sol new file mode 100644 index 0000000..0ab61dd --- /dev/null +++ b/test/setup/TokenRoleAuthorityGates.t.sol @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; + +import {Test} from "forge-std/Test.sol"; +import {TokenRoleScript} from "../../script/setup/token-roles/TokenRoleScript.s.sol"; +import {RolesProbes} from "../../src/roles/RolesProbes.sol"; + +/// @dev A token that has code and answers nothing: the shape a wrong or unreadable TOKEN address +/// takes when an authority preflight probes it. +contract SilentToken { + fallback() external { + revert("silent"); + } +} + +contract TokenRoleGateHarness is TokenRoleScript { + function requireAuthority( + address token, + RolesProbes.TokenTemplate template, + RolesProbes.TokenRole role, + address actor + ) external view { + _requireTokenRoleAuthority(token, template, role, actor); + } +} + +/// @notice The authority preflight refuses a failed read BEFORE the equality check. Without the gate, +/// a token whose `owner()` does not answer reads as owner `0x0`, and the refusal message would +/// attribute that owner to a token nobody observed. +contract TokenRoleAuthorityGatesTest is Test { + TokenRoleGateHarness internal harness; + address internal silent; + + function setUp() public { + harness = new TokenRoleGateHarness(); + silent = address(new SilentToken()); + } + + function test_FactoryGate_RefusesAFailedOwnerRead() public { + vm.expectRevert(bytes("owner() could not be read from the token, so authority cannot be verified")); + harness.requireAuthority( + silent, RolesProbes.TokenTemplate.FactoryBurnMintERC20, RolesProbes.TokenRole.Minter, address(0xBEEF) + ); + } +}