Ship the network-config check, behind one binding consumers keep

.github/workflows/manual-sol-artifacts.yaml

@@ -6,7 +6,7 @@ name: Manual sol artifacts
 # pins and never broadcasts. So the deploy has to happen first, and separately,
 # which is this.
 #
-# Order is: dispatch this once per suite, confirm `RegistryDeployChainTest`
+# Order is: dispatch this once per suite, confirm `RegistryDeployVerifyTest`
 # passes on every supported network, then push the `sol-v*` tag.
 #
 # Deliberately `workflow_dispatch` only. Broadcasting is key custody and real

README.md

@@ -70,13 +70,21 @@ contract Build is MyDeploySuites, BuildScript {
     function snapshotContractNames() internal view override returns (string[] memory);
 }
 
-// test/src/abstract/MyDeploySnapshot.t.sol
-contract MyDeploySnapshotTest is MyDeploySuites, RainDeployVerifySnapshot {}
-
-// test/src/abstract/MyDeployChain.t.sol
-contract MyDeployChainTest is MyDeploySuites, RainDeployVerifyChain {}
+// test/src/abstract/MyDeployVerify.t.sol
+contract MyDeployVerifyTest is MyDeploySuites, RainDeployVerify {}
 ```
 
+`RainDeployVerify` is the whole of the verification binding, deliberately. It is
+the union of `RainDeployVerifyChain` and `RainDeployVerifySnapshot`, so a check
+added to either — or to the union — reaches every consumer on a version bump
+with no downstream edit. A check a consumer has to remember to bind is a check
+most consumers do not run, so a new assertion goes into something this already
+inherits rather than into an abstract beside it. The pair underneath stays two
+contracts because that split is runtime rather than binding: the chain half
+forks every supported network and the snapshot half touches none, so a job with
+no RPC credentials binds `RainDeployVerifySnapshot` alone and `--match-contract`
+selects it.
+
 The broadcast and the verification read the SAME array. "The deploy script ships
 one contract while the tests verify another" is therefore not a statement that
 can be true — not because something checks for it, but because there is nothing
@@ -113,7 +121,7 @@ Deriving the pins at broadcast time would make that comparison
 derived-against-derived, and a guard that compares a value to itself is not a
 guard.
 
-Four groups, sorted by what each is anchored to and therefore by what each can
+Five groups, sorted by what each is anchored to and therefore by what each can
 catch:
 
 | Group    | Anchored to            | Catches                               | Cannot catch                     |
@@ -122,6 +130,7 @@ catch:
 | Source   | `type(X).creationCode` | a snapshot of the wrong contract      | anything about any chain         |
 | Record   | the frozen record      | a release the declaration missed      | what a declared suite records    |
 | Chain    | the networks           | never deployed, or not there any more | anything about a candidate       |
+| Config   | `foundry.toml`         | a network it cannot fork or verify on | anything about a suite           |
 
 The internal group's blind spot is not a gap to close there: every check in it
 asks the recorded bytes to agree with each other, and the wrong contract's bytes
@@ -170,6 +179,23 @@ chain: deploying through Zoltu buys address predictability, and such a
 constructor spends it. So a per-chain difference fails hard, naming the chain
 and both hashes, and there is deliberately no per-chain code hash to record.
 
+The config group is the only one whose subject is the CONSUMER's own
+`foundry.toml` rather than its suites. `supportedNetworks()` is what the deploy
+broadcasts to and what the chain group forks, `[rpc_endpoints]` is what makes an
+alias forkable and `[etherscan]` is what makes `--verify` resolve, so the three
+lists are one list and drift between them is a defect in either direction: a
+supported network missing from a section broadcasts and then fails after the gas
+is spent, and a section entry no supported network names is config nothing ever
+reads. An `[etherscan]` entry carrying neither `chain` nor `url` under an alias
+foundry cannot resolve is worse than missing — it takes verification down for
+every entry in the section, not only its own.
+
+It reads the raw file rather than forge's resolved config, because the values
+are `${VAR}` interpolations that only exist in CI while the KEYS are the whole
+contract, and the keys are in the text. So it needs no RPC and fails on the pull
+request that drifts rather than at dispatch time. Reading the file at all is
+what a consumer has to allow: see [Install](#install).
+
 ## Address registry
 
 `AddressRegistry` binds an opaque `bytes32` name to an address. An immutable
@@ -388,7 +414,7 @@ Three separate steps, in this order. Nothing automatic ever broadcasts.
    is key custody and real money, and no merge or tag should be able to trigger
    it. It is idempotent — a network that already has the code is skipped — so a
    partial run is fixed by running it again rather than by unpicking anything.
-2. **Verify.** `RegistryDeployChainTest` passes only once every **released**
+2. **Verify.** `RegistryDeployVerifyTest` passes only once every **released**
    suite is live on every supported network, with the code that release froze.
    This repo has released none, so today it has nothing to check and passes; it
    gets a subject the moment step 3 freezes one, and is red from then until step
@@ -548,6 +574,18 @@ The versions have to match: the import paths are version-qualified, which is
 deliberate — it is what stops a consumer's incompatible copy from silently
 satisfying these imports.
 
+The config group reads the CONSUMING repo's `foundry.toml`, so that repo has to
+allow it and has to have the sections to be read:
+
+```toml
+fs_permissions = [{ access = "read", path = "./foundry.toml" }]
+```
+
+`[rpc_endpoints]` and `[etherscan]` then have to name exactly the networks in
+`supportedNetworks()`. Missing permission fails the check rather than skipping
+it, which is the intended direction: a repo that cannot read its own config is a
+repo whose config nothing has checked.
+
 ## Develop
 
 This repo uses [nix](https://nixos.org/download.html). The default shell is the

script/Deploy.sol

@@ -28,7 +28,7 @@ import {RegistryDeploySuites} from "../src/abstract/RegistryDeploySuites.sol";
 /// chains of seven, one RPC down — is fixed by running it again rather than by
 /// unpicking anything.
 ///
-/// `RegistryDeployChainTest` is what says whether this has been run and worked
+/// `RegistryDeployVerifyTest` is what says whether this has been run and worked
 /// — but only for RELEASED suites, and this repo has released none, so today it
 /// has nothing to check and passes. It gets a subject once a release is frozen,
 /// and then fails until every supported network has that release's code, which

slither.config.json

@@ -1,4 +1,4 @@
 {
-  "filter_paths": "dependencies/forge-std-|src/abstract/(RainDeploy(SuitesBase|Broadcast|VerifyBase|VerifyChain|VerifySnapshotBase|VerifySnapshot)|RegistryDeploySuites|BuildScript)\\.sol",
+  "filter_paths": "dependencies/forge-std-|src/abstract/(RainDeploy(SuitesBase|Broadcast|VerifyBase|VerifyChain|VerifySnapshotBase|VerifySnapshot|Verify)|RegistryDeploySuites|BuildScript)\\.sol",
   "detectors_to_exclude": "assembly"
 }

src/abstract/RainDeployVerify.sol

@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: LicenseRef-DCL-1.0
+// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd
+pragma solidity ^0.8.25;
+
+import {RainDeployVerifyChain} from "./RainDeployVerifyChain.sol";
+import {RainDeployVerifySnapshot} from "./RainDeployVerifySnapshot.sol";
+
+/// @title RainDeployVerify
+/// @notice The ONE contract a deploy repo binds: every deploy verification this
+/// package has, network-free and chain-anchored alike, over that repo's own
+/// declaration.
+///
+/// A check a repo has to opt into is a check most repos do not run. Binding is
+/// a downstream edit, every repo owes it independently, and nothing red-lines
+/// the one that skips it — so an assertion added to a contract nobody binds
+/// ships without arriving. Bound here it arrives on a version bump with no
+/// downstream edit, which puts delivering it on the author who knows it exists.
+///
+/// That holds only while this is the whole of what there is to bind. A new
+/// check therefore goes into a contract this already inherits, or into this. An
+/// abstract added beside it, that every consumer has to remember, reproduces
+/// the gap this exists to close.
+///
+/// `RainDeployVerifyChain` and `RainDeployVerifySnapshot` stay separate
+/// contracts underneath, because that split is runtime rather than binding: the
+/// chain group forks every supported network and the snapshot group touches
+/// none, so binding one of them alone is what runs the offline half with no RPC
+/// credentials. `--match-contract` selects at a contract boundary, so the pair
+/// has to remain two contracts for that to be selectable at all. This is what a
+/// repo binds; that pair is what a job narrows to.
+abstract contract RainDeployVerify is RainDeployVerifyChain, RainDeployVerifySnapshot {}

src/abstract/RainDeployVerifyChain.sol

@@ -81,6 +81,9 @@ error CodeHashMismatchOnNetwork(
 /// missing deployment. A contract boundary is what `forge test
 /// --match-contract` and a CI job select at, and it is structural rather than
 /// conventional — nothing reachable from the snapshot contract forks anything.
+/// `RainDeployVerify` is what a repo binds, and this stays a contract of its
+/// own so that binding the snapshot half alone remains a thing a
+/// credential-free job can do.
 abstract contract RainDeployVerifyChain is RainDeployVerifyBase {
     /// Checks one derived suite against whichever network is currently
     /// selected.

src/abstract/RainDeployVerifySnapshot.sol

@@ -3,16 +3,18 @@
 pragma solidity ^0.8.25;
 
 import {RainDeployVerifySnapshotBase} from "./RainDeployVerifySnapshotBase.sol";
+import {LibRainDeploy} from "../lib/LibRainDeploy.sol";
 import {LibRainDeploySnapshot} from "../lib/LibRainDeploySnapshot.sol";
+import {LibStringSet} from "../lib/LibStringSet.sol";
 
 /// @title RainDeployVerifySnapshot
-/// @notice What a deploy repo inherits: every deploy-pin assertion that needs
-/// no network, bound to that repo. `RainDeployVerifySnapshotBase` is where all
-/// three groups are defined and documented; this adds the one test whose
-/// subject is the repo's real frozen record on disk rather than anything the
-/// inheriting contract declares.
+/// @notice What a deploy repo inherits: every assertion that needs no network,
+/// bound to that repo. `RainDeployVerifySnapshotBase` is where the three
+/// deploy-pin groups are defined and documented; this adds the tests whose
+/// subject is the repo's own state on disk — its frozen record, and its
+/// `foundry.toml` — rather than anything the inheriting contract declares.
 ///
-/// The split is which contract carries that one test, and nothing else. A
+/// The split is which contract carries those tests, and nothing else. A
 /// consumer inherits this and gets all three groups, exactly as it does when
 /// they are one contract. The base is for a contract whose declaration is a
 /// FIXTURE — the record is not its subject, and see the base for why asking it
@@ -53,4 +55,63 @@ abstract contract RainDeployVerifySnapshot is RainDeployVerifySnapshotBase {
             LibRainDeploySnapshot.frozenSnapshotPaths(vm, LibRainDeploySnapshot.LIB_FS_ROOT), releasedSuites()
         );
     }
+
+    /// `[rpc_endpoints]` and `[etherscan]` in the binding repo's `foundry.toml`
+    /// MUST be EXACTLY `supportedNetworks()`, which makes the three lists one
+    /// list.
+    ///
+    /// The deploy forks by the first and `--verify` resolves the second, so a
+    /// supported network missing from either broadcasts and then fails after
+    /// the gas is spent, and a section entry no supported network names is
+    /// config nothing ever reads. Both are the same defect — the lists having
+    /// drifted — so both directions are asserted, by membership: containment
+    /// one way alone passes for a section carrying an alias nothing deploys
+    /// to, and the other way alone passes for a network with no config at all.
+    /// Membership rather than position, because a config section is keyed
+    /// rather than ordered and there is no order in it to assert.
+    ///
+    /// This is what makes the `[etherscan]` half enforced at all. The RPC half
+    /// is enforced only incidentally, by the fork tests, and only forwards.
+    ///
+    /// The raw file is read rather than forge's resolved config because the
+    /// values are `${VAR}` interpolations that exist only in CI. The KEYS are
+    /// the whole contract here, and they are in the text — so this needs no
+    /// RPC and fails on the PR that drifts rather than at dispatch time.
+    ///
+    /// `vm.readFile` resolves against the project root of whatever runs it, so
+    /// the file read is the binder's own and the networks are this package's.
+    /// A binding repo therefore needs `{ access = "read", path =
+    /// "./foundry.toml" }` in `fs_permissions`, and one without it fails here
+    /// rather than passing on a file it never opened.
+    function testSupportedNetworksAreFullyConfigured() external view {
+        string memory config = vm.readFile("foundry.toml");
+        string[] memory networks = LibRainDeploy.supportedNetworks();
+
+        for (uint256 i = 0; i < networks.length; i++) {
+            assertTrue(
+                vm.keyExistsToml(config, string.concat(".rpc_endpoints.", networks[i])),
+                string.concat("supported network has no [rpc_endpoints] alias: ", networks[i])
+            );
+            assertTrue(
+                vm.keyExistsToml(config, string.concat(".etherscan.", networks[i])),
+                string.concat("supported network has no [etherscan] key: ", networks[i])
+            );
+        }
+
+        string[] memory rpcAliases = vm.parseTomlKeys(config, ".rpc_endpoints");
+        for (uint256 i = 0; i < rpcAliases.length; i++) {
+            assertTrue(
+                LibStringSet.holds(networks, rpcAliases[i]),
+                string.concat("[rpc_endpoints] alias is not a supported network: ", rpcAliases[i])
+            );
+        }
+
+        string[] memory etherscanKeys = vm.parseTomlKeys(config, ".etherscan");
+        for (uint256 i = 0; i < etherscanKeys.length; i++) {
+            assertTrue(
+                LibStringSet.holds(networks, etherscanKeys[i]),
+                string.concat("[etherscan] key is not a supported network: ", etherscanKeys[i])
+            );
+        }
+    }
 }

src/abstract/RegistryDeploySuites.sol

@@ -27,8 +27,8 @@ import {LibReleasedSuites} from "../lib/LibReleasedSuites.sol";
 ///   released-suites libs from it — the named candidates below are the
 ///   templates it emits from, which is why it inherits the declaration rather
 ///   than restating the key, the artifact path and the dependency list
-/// - `RegistryDeploySnapshotTest` checks its records against its creation code
-/// - `RegistryDeployChainTest` checks it against every chain
+/// - `RegistryDeployVerifyTest` checks its records against its creation code
+///   and against every chain
 /// - `GeneratedSnapshotShapeTest` checks the shape of the files that generation
 ///   writes
 ///

test/lib/LibStringSet.sol → src/lib/LibStringSet.sol

@@ -3,11 +3,11 @@
 pragma solidity ^0.8.25;
 
 /// @title LibStringSet
-/// @notice Membership over a `string[]`, for tests that assert about a set
-/// whose order they do not fix.
+/// @notice Membership over a `string[]`, for an assertion about a set whose
+/// order it does not fix.
 ///
-/// Solidity has no string equality and no set, so a test that wants "this list
-/// holds this string" writes a keccak loop. WHY a caller wants membership
+/// Solidity has no string equality and no set, so a caller that wants "this
+/// list holds this string" writes a keccak loop. WHY a caller wants membership
 /// rather than an index or an ordering is the caller's own reasoning and stays
 /// with the caller; what happens here is only the comparison, which is the same
 /// comparison wherever it is asked for.

test/script/Build.t.sol

@@ -8,7 +8,7 @@ import {DeployCandidate} from "../../src/abstract/RainDeploySuitesBase.sol";
 import {LibRainDeploySnapshot} from "../../src/lib/LibRainDeploySnapshot.sol";
 import {LibReleasedSuitesAggregate} from "../lib/LibReleasedSuitesAggregate.sol";
 import {BuildHarness} from "../concrete/BuildHarness.sol";
-import {LibStringSet} from "../lib/LibStringSet.sol";
+import {LibStringSet} from "../../src/lib/LibStringSet.sol";
 
 /// @title BuildTest
 /// @notice `script/Build.sol`'s own declaration.
@@ -26,7 +26,7 @@ import {LibStringSet} from "../lib/LibStringSet.sol";
 /// names, `regenerateLibs()` writes a released-suites lib only for those, and
 /// the release permanently omits that contract. Nothing downstream can see it:
 /// `testEveryFrozenSnapshotIsReleased` walks record -> declaration, so a record
-/// entry never written is invisible to it; `RegistryDeployChainTest` is never
+/// entry never written is invisible to it; `RegistryDeployVerifyTest` is never
 /// handed the omitted suite; and `testEveryCandidateHasASnapshot` compares the
 /// candidate DIRECTORY against the declaration, where the stale committed file
 /// keeps both sides equal. `SnapshotAlreadyFrozen` then makes the hole

test/src/abstract/RainDeployVerifySnapshotBase.t.sol

@@ -40,7 +40,7 @@ import {
 /// this repo's real releases are declared by the exemplar — a claim that is
 /// vacuously true while the repo has released nothing and false from its first
 /// release, which is not a thing this contract is about either way. The real
-/// record is bound where the real declaration is, in `RegistryDeploySnapshotTest`.
+/// record is bound where the real declaration is, in `RegistryDeployVerifyTest`.
 ///
 /// The record check ITSELF is exercised here, at every position and against
 /// every shape of declaration, because `checkFrozenSnapshotsReleased` takes the