From a25a3570f732edff5758b981413537fd3213faf6 Mon Sep 17 00:00:00 2001 From: David Meister Date: Sat, 15 Aug 2026 12:07:54 +0000 Subject: [PATCH 1/2] Anchor the candidate snapshot to source on the broadcast path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #56. `RainDeployBroadcast.run()` deployed whatever `src/generated/candidate/*.sol` held. The only guard in front of the CREATE2 is `LibRainDeploy` comparing the recorded address to what the recorded creation code derives, and both sides come out of the same generated file — so it proved that file internally consistent and nothing more. The check that catches a snapshot of the wrong CONTRACT lived on `RainDeployVerifySnapshot`, which inherits `Test` and which the broadcast cannot reach. Move the anchor onto `RainDeploySuitesBase` so there is ONE definition both the broadcast and the tests run: `checkCandidatesAnchoredToSource()` reads the declaration through `checkedCandidateSuites()` and is called at the top of `run()`, before a suite is selected and before `DEPLOYMENT_KEY` is read. `RainDeployVerifySnapshot` loses its local error and both helpers, and `testSnapshotMatchesSource` calls the inherited one. The negative case is now a whole broken DECLARATION rather than a set handed to a helper, because the anchor takes no argument: `SourceMismatchDeploySuites` declares a genuinely anchored candidate followed by a consistent snapshot of the wrong contract, and `SourceMismatchDeploy` drives it through `run()`. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 27 +++- README.md | 10 ++ src/abstract/RainDeployBroadcast.sol | 15 ++ src/abstract/RainDeploySuitesBase.sol | 76 +++++++-- src/abstract/RainDeployVerifySnapshot.sol | 58 ++----- test/abstract/ExternalDeploySuites.sol | 12 +- test/abstract/SourceMismatchDeploySuites.sol | 80 ++++++++++ test/concrete/SourceMismatchDeploy.sol | 20 +++ test/src/abstract/RainDeployBroadcast.t.sol | 66 +++++++- test/src/abstract/RainDeploySuitesBase.t.sol | 17 +- .../abstract/RainDeployVerifySnapshot.t.sol | 149 +++++++----------- 11 files changed, 366 insertions(+), 164 deletions(-) create mode 100644 test/abstract/SourceMismatchDeploySuites.sol create mode 100644 test/concrete/SourceMismatchDeploy.sol diff --git a/CLAUDE.md b/CLAUDE.md index e04e3be..231510e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -285,7 +285,8 @@ adds a suite by adding an array entry; the keys reported by a mistyped fall behind the suites it describes. Keys are checked unique, because the key is what selects what gets broadcast. -**`src/abstract/RainDeployBroadcast.sol`** — the broadcast. Selects one suite by +**`src/abstract/RainDeployBroadcast.sol`** — the broadcast. Runs the source +anchor over the whole declaration first, then selects one suite by `DEPLOYMENT_SUITE` and deploys it, before reading `DEPLOYMENT_KEY` so a mistyped suite fails naming the valid ones rather than on a missing key. `deployNetworks()` defaults to `supportedNetworks()` and is overridable for @@ -318,13 +319,23 @@ Four groups, sorted by what they are anchored to: generated inconsistently. CANNOT catch a snapshot of the wrong contract: a consistent snapshot of the wrong thing satisfies all of it, which `testWrongContractSnapshotPassesInternalConsistency` pins. -2. **Anchored to source** (`RainDeployVerifySnapshot`) — EVERY candidate's - recorded creation code is `type(X).creationCode`. The only check that catches - a wrong-contract snapshot. Candidates only, because a released tag is MEANT - to diverge from current source; there is no field on a released version to - spell it, so it cannot be opted into or out of. Every one, and refusing an - empty list, because a candidate the loop never reaches is a contract whose - snapshot nothing anywhere anchors — see `NoDeployCandidates`. +2. **Anchored to source** (`RainDeploySuitesBase`) — EVERY candidate's recorded + creation code is `type(X).creationCode`. The only check that catches a + wrong-contract snapshot. Candidates only, because a released tag is MEANT to + diverge from current source; there is no field on a released version to spell + it, so it cannot be opted into or out of. Every one, and refusing an empty + list, because a candidate the loop never reaches is a contract whose snapshot + nothing anywhere anchors — see `NoDeployCandidates`. + + The only group that is not only a test. It is defined on the DECLARATION, and + `RainDeployBroadcast.run()` calls it before it selects a suite or reads a + key, because the broadcast deploys the recorded bytes and the only other + guard in front of the `CREATE2` — `LibRainDeploy` comparing the recorded + address to what the recorded creation code derives — takes both sides out of + the same generated file. An anchor reachable only from a contract that + inherits `Test` is an anchor the irreversible action does not run, and + `CREATE2` at a zero salt puts the wrong bytes at their own permanent address + on every chain a dispatch reaches. 3. **Anchored to the record** (`RainDeployVerifySnapshot`) — every file in the append-only `src/generated//` tree is declared by a released suite, matched by the address that file's creation code derives. `releasedSuites()` diff --git a/README.md b/README.md index 88ccf44..3e43ac3 100644 --- a/README.md +++ b/README.md @@ -122,6 +122,16 @@ it. A candidate the source anchor never reaches is a contract whose snapshot nothing anywhere anchors, and a repo with several contracts is exactly where a snapshot generated from the wrong one comes from. +The source group is also the only one that is not only a test. It is defined on +the suite declaration, and the broadcast runs it before it selects a suite or +reads a key. The deploy reads the same recorded bytes, and the only other guard +in front of the `CREATE2` compares the recorded address against what the +recorded creation code derives — both out of the same generated file, so it +catches a stale pin and cannot catch a snapshot of the wrong contract. An anchor +only a test contract could reach would be an anchor the irreversible action does +not run, and `CREATE2` at a zero salt puts the wrong bytes at their own +permanent address on every chain the dispatch reached. + The chain group carries the mirror image of that exemption: it applies to **released versions only**. A release IS a deployment that happened, so "it is live on every supported network" is either true of it or a defect. A candidate diff --git a/src/abstract/RainDeployBroadcast.sol b/src/abstract/RainDeployBroadcast.sol index 723d74b..87b382f 100644 --- a/src/abstract/RainDeployBroadcast.sol +++ b/src/abstract/RainDeployBroadcast.sol @@ -65,6 +65,19 @@ abstract contract RainDeployBroadcast is RainDeploySuitesBase, Script { /// Broadcasts the suite `DEPLOYMENT_SUITE` names. /// + /// The source anchor runs FIRST, before a suite is selected and before the + /// key is read. A snapshot the deploy reads and nothing anchors is a deploy + /// of unknown bytes: everything else asked of a snapshot is internal to it, + /// and the recorded-address guard `LibRainDeploy` applies before it forks + /// anything compares two values out of the same generated file. That guard + /// is what catches a stale PIN; only the anchor catches a snapshot of the + /// wrong CONTRACT, and `CREATE2` at a zero salt means the wrong bytes take + /// their own permanent address on every chain this reaches. It is run over + /// the whole declaration rather than over the selected suite because the + /// declaration is what a repo maintains and regenerates as a unit — a + /// dispatch of one suite from a tree where another candidate has gone stale + /// is a tree nobody should be broadcasting from at all. + /// /// The suite is resolved before the key is read, so a mistyped suite fails /// in seconds listing the valid ones rather than failing on a missing /// `DEPLOYMENT_KEY` and sending the reader after the wrong thing. @@ -75,6 +88,8 @@ abstract contract RainDeployBroadcast is RainDeploySuitesBase, Script { /// `dependencies` enforces per network, and which a caller satisfies by /// dispatching in order. function run() external { + checkCandidatesAnchoredToSource(); + DeploySuite memory suite = suiteByName(vm.envOr("DEPLOYMENT_SUITE", string(""))); uint256 deployerPrivateKey = vm.envUint("DEPLOYMENT_KEY"); diff --git a/src/abstract/RainDeploySuitesBase.sol b/src/abstract/RainDeploySuitesBase.sol index 24785f5..350d3b2 100644 --- a/src/abstract/RainDeploySuitesBase.sol +++ b/src/abstract/RainDeploySuitesBase.sol @@ -26,11 +26,22 @@ error UnknownDeploymentSuite(string requested, string validSuites); /// to declare. When the candidate was a single struct this was true by /// construction; a list has to say it. /// -/// Raised from `allSuites`, which is the only way anything reads the -/// declaration — `suiteNames` and `suiteByName` both go through it — so there -/// is no reader that answers from an empty one. +/// Raised from `checkedCandidateSuites`, which is the only way anything reads +/// the candidates — `allSuites` goes through it and so does the source anchor, +/// and `suiteNames` and `suiteByName` go through `allSuites` — so there is no +/// reader that answers from an empty one. error NoDeployCandidates(); +/// Thrown when a candidate's recorded creation code is not the creation code +/// this repo currently compiles. Hashes rather than the bytes themselves, which +/// run to tens of kilobytes. +/// @param suite The candidate's key. +/// @param storedCreationCodeHash Hash of the creation code the candidate +/// records. +/// @param sourceCreationCodeHash Hash of `type(X).creationCode` for the +/// contract the candidate claims to be. +error CandidateSourceMismatch(string suite, bytes32 storedCreationCodeHash, bytes32 sourceCreationCodeHash); + /// One deployable unit: a named snapshot of one contract. /// /// `creationCode` is the ONLY input. The Zoltu factory is `CREATE2` over its @@ -148,12 +159,12 @@ abstract contract RainDeploySuitesBase { /// The declared candidates, refusing an empty list. /// /// The ONE place `NoDeployCandidates` is raised, and the only way anything - /// reads the candidates. `allSuites` goes through it, and so does the - /// source anchor in `RainDeployVerifySnapshot` — which matters, because the - /// source anchor loops over the candidates and a loop over an empty list - /// passes. Guarding each reader separately would be two spellings of one - /// rule, and the reader that got the second spelling wrong is the one that - /// silently stops asserting. + /// reads the candidates. `allSuites` goes through it, and so does + /// `checkCandidatesAnchoredToSource` — which matters, because the source + /// anchor loops over the candidates and a loop over an empty list passes. + /// Guarding each reader separately would be two spellings of one rule, and + /// the reader that got the second spelling wrong is the one that silently + /// stops asserting. /// @return candidates The candidates. function checkedCandidateSuites() internal pure returns (DeployCandidate[] memory candidates) { candidates = candidateSuites(); @@ -162,6 +173,53 @@ abstract contract RainDeploySuitesBase { } } + /// EVERY candidate MUST record the creation code this repo compiles. + /// + /// This is the ONLY check that catches a snapshot of the wrong contract. + /// Everything else a snapshot is asked is internal to the snapshot — the + /// recorded address is what the recorded creation code derives, the + /// recorded code hash is what it produces — and a consistent snapshot of + /// the wrong thing satisfies all of it, because the wrong contract's bytes + /// agree with each other perfectly. + /// + /// It lives on the DECLARATION rather than on the verification abstract + /// because the broadcast runs it too. `RainDeployBroadcast` deploys the + /// bytes a candidate records, and the only guard between it and the Zoltu + /// factory is `LibRainDeploy`'s recorded-address-against-recorded-creation- + /// code comparison — both sides of which come out of the same generated + /// file, so it proves that file is internally consistent and nothing more. + /// A source anchor reachable only from a test contract is an anchor the + /// irreversible action does not run: broadcasting is `workflow_dispatch` on + /// a ref with no required-green gate, so "CI is red on that ref" is a + /// signal a human may not have read, and CREATE2 at a zero salt puts the + /// wrong bytes at their own permanent address on every chain the dispatch + /// reached. One definition, both callers, no way to deploy past it. + /// + /// EVERY candidate, because a candidate the loop never reaches is a + /// contract whose snapshot nothing anywhere anchors — and a repo with + /// several contracts is exactly where a snapshot generated from the wrong + /// one comes from. + /// + /// Read through `checkedCandidateSuites` rather than `candidateSuites`: a + /// loop over an empty list passes, so a declaration with no candidate at + /// all would turn this into a green check that asserts nothing. + /// + /// Candidates alone, and there is no way to spell an exemption. A released + /// suite is MEANT to diverge from current source — it records bytes that + /// are already on chain — so anchoring one to source asserts something + /// false by design, which is why `DeploySuite` carries no source at all and + /// only `DeployCandidate` does. + function checkCandidatesAnchoredToSource() internal pure { + DeployCandidate[] memory candidates = checkedCandidateSuites(); + for (uint256 i = 0; i < candidates.length; i++) { + bytes32 stored = keccak256(candidates[i].snapshot.creationCode); + bytes32 source = keccak256(candidates[i].sourceCreationCode); + if (stored != source) { + revert CandidateSourceMismatch(candidates[i].snapshot.suite, stored, source); + } + } + } + /// Every suite this repo declares: the released ones followed by the /// candidates. This is the verification set and the deploy registry, which /// are the same set because they are the same declaration. diff --git a/src/abstract/RainDeployVerifySnapshot.sol b/src/abstract/RainDeployVerifySnapshot.sol index 05416d7..c6758b1 100644 --- a/src/abstract/RainDeployVerifySnapshot.sol +++ b/src/abstract/RainDeployVerifySnapshot.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.25; import {DerivedDeploy, RainDeployVerifyBase} from "./RainDeployVerifyBase.sol"; -import {DeployCandidate, DeploySuite} from "./RainDeploySuitesBase.sol"; +import {DeploySuite} from "./RainDeploySuitesBase.sol"; import {LibRainDeploy} from "../lib/LibRainDeploy.sol"; import {LibRainDeploySnapshot} from "../lib/LibRainDeploySnapshot.sol"; @@ -28,16 +28,6 @@ error StoredCodeHashMismatch(string suite, bytes32 storedCodeHash, bytes32 deriv /// @param runtimeCodeHash The hash of the runtime code the suite records. error StoredRuntimeCodeHashMismatch(string suite, bytes32 storedBytecodeHash, bytes32 runtimeCodeHash); -/// Thrown when the candidate's recorded creation code is not the creation code -/// this repo currently compiles. Hashes rather than the bytes themselves, which -/// run to tens of kilobytes. -/// @param suite The candidate's key. -/// @param storedCreationCodeHash Hash of the creation code the candidate -/// records. -/// @param sourceCreationCodeHash Hash of `type(X).creationCode` for the -/// contract the candidate claims to be. -error CandidateSourceMismatch(string suite, bytes32 storedCreationCodeHash, bytes32 sourceCreationCodeHash); - /// Thrown when a file in the frozen record is declared by no released suite. /// The record is append-only, so this never goes away by itself: a release the /// declaration missed is a release the chain group never asks about, and the @@ -80,6 +70,12 @@ error FrozenSnapshotUnreadable(string path); /// source, so anchoring one to source asserts something that is false by /// design. /// +/// That one is not defined here. It lives on `RainDeploySuitesBase`, because +/// `RainDeployBroadcast` runs it before it broadcasts and cannot reach anything +/// on this side — this inherits `Test`. Here it is a test; there it is the last +/// thing standing between a stale generated file and a permanent `CREATE2` +/// address on every chain a dispatch reaches. +/// /// **Anchored to the record.** Every file in the frozen record — the /// append-only `src/generated//` directories — is declared by a released /// suite. This is the one check that is about the DECLARATION rather than about @@ -208,18 +204,6 @@ abstract contract RainDeployVerifySnapshot is RainDeployVerifyBase { } } - /// Checks the candidate against the source this repo compiles. - /// @param candidate The candidate to check. - function checkAnchoredToSource(DeployCandidate memory candidate) internal pure { - if (keccak256(candidate.snapshot.creationCode) != keccak256(candidate.sourceCreationCode)) { - revert CandidateSourceMismatch( - candidate.snapshot.suite, - keccak256(candidate.snapshot.creationCode), - keccak256(candidate.sourceCreationCode) - ); - } - } - /// Every declared suite MUST be internally consistent: what it records is /// what its own creation code derives. function testSnapshotInternallyConsistent() external { @@ -229,32 +213,16 @@ abstract contract RainDeployVerifySnapshot is RainDeployVerifyBase { } } - /// Checks every candidate in a set against the source it claims to be. - /// - /// Every one, because this is the only check that catches a wrong-contract - /// snapshot at all: a candidate the loop never reaches is a contract whose - /// snapshot nothing anywhere anchors, and a repo with several contracts is - /// exactly where a snapshot generated from the wrong one comes from. - /// - /// Takes the set as an argument, as `checkFrozenSnapshotsReleased` does, so - /// the loop is drivable with a set built to break it rather than only with - /// whatever the inheriting repo happens to declare. - /// @param candidates The candidates to check. - function checkCandidatesAnchoredToSource(DeployCandidate[] memory candidates) internal pure { - for (uint256 i = 0; i < candidates.length; i++) { - checkAnchoredToSource(candidates[i]); - } - } - /// EVERY candidate MUST be a snapshot of the contract this repo compiles, /// not of some other contract that happens to be internally consistent. /// - /// Read through `checkedCandidateSuites` rather than `candidateSuites`: a - /// loop over an empty list passes, so a declaration with no candidate at - /// all would turn the one check that catches a wrong-contract snapshot into - /// a green test that asserts nothing. + /// The check itself is `RainDeploySuitesBase.checkCandidatesAnchoredToSource` + /// rather than anything here, because `RainDeployBroadcast` runs the same + /// definition before it broadcasts. A second spelling on this side is a + /// spelling the deploy does not run, which is exactly the state this test + /// would otherwise be reporting green about. function testSnapshotMatchesSource() external pure { - checkCandidatesAnchoredToSource(checkedCandidateSuites()); + checkCandidatesAnchoredToSource(); } /// Every release in the frozen record MUST be declared, so that the set the diff --git a/test/abstract/ExternalDeploySuites.sol b/test/abstract/ExternalDeploySuites.sol index 31c5f7f..a05de4d 100644 --- a/test/abstract/ExternalDeploySuites.sol +++ b/test/abstract/ExternalDeploySuites.sol @@ -5,10 +5,11 @@ pragma solidity ^0.8.25; import {DeployCandidate, DeploySuite, RainDeploySuitesBase} from "../../src/abstract/RainDeploySuitesBase.sol"; /// @title ExternalDeploySuites -/// @notice The three registry reads, exposed externally so a plain `Test` -/// contract can drive them and `vm.expectRevert` lands at the right call depth. +/// @notice Every reader of the declaration, exposed externally so a plain +/// `Test` contract can drive them and `vm.expectRevert` lands at the right call +/// depth. /// -/// Here rather than on each fixture because every fixture needs the same three, +/// Here rather than on each fixture because every fixture needs all of them, /// and a declaration that is refused has to be refused on ALL of them — a /// wrapper a fixture forgot to carry is a reader nothing checks that fixture /// through. @@ -33,4 +34,9 @@ abstract contract ExternalDeploySuites is RainDeploySuitesBase { function externalCheckedCandidateSuites() external pure returns (DeployCandidate[] memory) { return checkedCandidateSuites(); } + + /// Runs the source anchor over the fixture's own declaration. + function externalCheckCandidatesAnchoredToSource() external pure { + checkCandidatesAnchoredToSource(); + } } diff --git a/test/abstract/SourceMismatchDeploySuites.sol b/test/abstract/SourceMismatchDeploySuites.sol new file mode 100644 index 0000000..767309a --- /dev/null +++ b/test/abstract/SourceMismatchDeploySuites.sol @@ -0,0 +1,80 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity ^0.8.25; + +import {DeployCandidate, DeploySuite, RainDeploySuitesBase} from "../../src/abstract/RainDeploySuitesBase.sol"; +import {AddressRegistry} from "../../src/concrete/AddressRegistry.sol"; +import { + BYTECODE_HASH as ADDRESS_REGISTRY_BYTECODE_HASH, + CREATION_CODE as ADDRESS_REGISTRY_CREATION_CODE, + DEPLOYED_ADDRESS as ADDRESS_REGISTRY_DEPLOYED_ADDRESS, + RUNTIME_CODE as ADDRESS_REGISTRY_RUNTIME_CODE +} from "../../src/generated/candidate/AddressRegistry.sol"; +import {LibRainDeploy} from "../../src/lib/LibRainDeploy.sol"; +import {MockDeployable} from "../concrete/MockDeployable.sol"; +import {MockDeployableV2} from "../concrete/MockDeployableV2.sol"; + +/// @title SourceMismatchDeploySuites +/// @notice A declaration whose second candidate records a contract that is not +/// the one it claims to be — the stale generated file, as a repo actually holds +/// it. +/// +/// The broken candidate is a CONSISTENT snapshot: the address it records is the +/// address its recorded creation code derives, the code hash is the one that +/// creation code produces, and the runtime code hashes to it. Every check +/// internal to a snapshot passes on it. Only the pairing with +/// `sourceCreationCode` says it describes the wrong contract, which is why the +/// source anchor is the only thing that can catch it. +/// +/// `MockDeployable` and `MockDeployableV2` are the pair, deliberately: the +/// snapshot is `V2`'s while the source is `MockDeployable`'s, which is exactly +/// the shape of a snapshot regenerated from a build that has since moved, or +/// generated from the wrong contract in a repo that compiles several. +/// +/// TWO candidates, broken one LAST, behind a genuinely anchored one. A loop +/// that stops at the first entry is invisible against a single candidate and +/// silently stops anchoring the moment a repo declares two — and a repo with +/// several contracts is precisely where a snapshot of the wrong one comes from. +/// The first is the real `AddressRegistry` this repo compiles, so nothing fails +/// before the loop has to advance, and the failure has to name the SECOND key +/// rather than a fixed one or the first. +/// +/// No releases. A release is meant to diverge from current source and is never +/// anchored, so declaring one here would add a suite that this fixture makes no +/// claim about. The keys are its own, shared with no other fixture, because +/// `DEPLOYMENT_SUITE` is a process-wide variable other tests write. +abstract contract SourceMismatchDeploySuites is RainDeploySuitesBase { + /// @inheritdoc RainDeploySuitesBase + function releasedSuites() internal pure override returns (DeploySuite[] memory suites) { + suites = new DeploySuite[](0); + } + + /// @inheritdoc RainDeploySuitesBase + function candidateSuites() internal pure override returns (DeployCandidate[] memory candidates) { + candidates = new DeployCandidate[](2); + candidates[0] = DeployCandidate({ + snapshot: DeploySuite({ + suite: "anchored-candidate", + creationCode: ADDRESS_REGISTRY_CREATION_CODE, + storedDeployedAddress: ADDRESS_REGISTRY_DEPLOYED_ADDRESS, + storedBytecodeHash: ADDRESS_REGISTRY_BYTECODE_HASH, + storedRuntimeCode: ADDRESS_REGISTRY_RUNTIME_CODE, + artifactPath: "src/concrete/AddressRegistry.sol:AddressRegistry", + dependencies: new address[](0) + }), + sourceCreationCode: type(AddressRegistry).creationCode + }); + candidates[1] = DeployCandidate({ + snapshot: DeploySuite({ + suite: "mismatched-candidate", + creationCode: type(MockDeployableV2).creationCode, + storedDeployedAddress: LibRainDeploy.zoltuAddress(type(MockDeployableV2).creationCode), + storedBytecodeHash: keccak256(type(MockDeployableV2).runtimeCode), + storedRuntimeCode: type(MockDeployableV2).runtimeCode, + artifactPath: "test/concrete/MockDeployableV2.sol:MockDeployableV2", + dependencies: new address[](0) + }), + sourceCreationCode: type(MockDeployable).creationCode + }); + } +} diff --git a/test/concrete/SourceMismatchDeploy.sol b/test/concrete/SourceMismatchDeploy.sol new file mode 100644 index 0000000..53062ff --- /dev/null +++ b/test/concrete/SourceMismatchDeploy.sol @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +import {RainDeployBroadcast} from "../../src/abstract/RainDeployBroadcast.sol"; +import {ExternalDeploySuites} from "../abstract/ExternalDeploySuites.sol"; +import {SourceMismatchDeploySuites} from "../abstract/SourceMismatchDeploySuites.sol"; + +/// @title SourceMismatchDeploy +/// A deploy repo's whole script, over a declaration whose candidate is a +/// snapshot of the wrong contract — `ExampleDeploy` with one thing wrong, and +/// the thing that is wrong is the one nothing about a snapshot can see by +/// itself. +/// +/// It is a real script rather than a declaration fixture because that is the +/// claim under test: the anchor has to run on the path that BROADCASTS, not +/// only on the path that tests. A fixture that could only be driven through +/// external wrappers would leave `run()` — the irreversible, multi-chain, +/// key-custody action — asserted about by nothing. +contract SourceMismatchDeploy is SourceMismatchDeploySuites, ExternalDeploySuites, RainDeployBroadcast {} diff --git a/test/src/abstract/RainDeployBroadcast.t.sol b/test/src/abstract/RainDeployBroadcast.t.sol index 1c2c178..2fb43ff 100644 --- a/test/src/abstract/RainDeployBroadcast.t.sol +++ b/test/src/abstract/RainDeployBroadcast.t.sol @@ -4,9 +4,12 @@ pragma solidity =0.8.25; import {Test} from "forge-std-1.16.1/src/Test.sol"; -import {UnknownDeploymentSuite} from "../../../src/abstract/RainDeploySuitesBase.sol"; +import {CandidateSourceMismatch, UnknownDeploymentSuite} from "../../../src/abstract/RainDeploySuitesBase.sol"; import {LibRainDeploy} from "../../../src/lib/LibRainDeploy.sol"; import {ExampleDeploy} from "../../concrete/ExampleDeploy.sol"; +import {SourceMismatchDeploy} from "../../concrete/SourceMismatchDeploy.sol"; +import {MockDeployable} from "../../concrete/MockDeployable.sol"; +import {MockDeployableV2} from "../../concrete/MockDeployableV2.sol"; /// @title RainDeployBroadcastTest /// @notice The broadcast entry point, driven exactly as the `Manual sol @@ -105,6 +108,67 @@ contract RainDeployBroadcastTest is Test { sDeploy.run(); } + /// The broadcast MUST refuse a candidate that is not the contract this repo + /// compiles, and refuse it BEFORE anything else happens. + /// + /// This is the whole reason the anchor is on the declaration rather than on + /// the verification abstract. `RainDeployBroadcast` cannot reach anything on + /// `RainDeployVerifySnapshot`, which inherits `Test`, so an anchor defined + /// there is an anchor the deploy does not run — and the only other guard + /// before the `CREATE2` goes out compares the recorded address against the + /// recorded creation code, both of which come out of the same generated + /// file. That catches a stale PIN and cannot catch a snapshot of the wrong + /// CONTRACT, so without this the bytes reaching five chains are whatever the + /// generated file happens to hold. `CREATE2` at a zero salt makes that + /// permanent: the wrong bytes take the wrong bytes' own address, on every + /// chain the dispatch reached, and the dispatch is `workflow_dispatch` on a + /// ref with no required-green gate — so "CI was red" is a signal a human may + /// not have read. + /// + /// ## Before the suite, and therefore before the key + /// + /// This declaration names no suite anything sets `DEPLOYMENT_SUITE` to, so + /// selection is a revert waiting to happen whatever that variable holds — + /// asserted here rather than assumed. The revert that actually arrives from + /// `run()` is the anchor's instead, which is what says the anchor ran first; + /// an anchor placed after the selection would produce the other one. The key + /// is read after the suite, which + /// `testRunSelectsTheSuiteFromTheEnvBeforeTheKeyAndNeverDefaults` pins, so + /// before the suite is before the key. + /// + /// Nothing here writes an env var. Both values are process-wide and forge + /// runs tests concurrently, so a second writer is a race, and this test does + /// not need one: the ordering is observable from a declaration that cannot + /// resolve any key at all. + /// + /// ## Discriminating + /// + /// That same env test is the passing case — `ExampleDeploy.run()` reaches + /// suite selection and fails there, which it can only do by getting past an + /// anchor that had nothing to say about a declaration whose candidates are + /// its source. + function testRunRefusesToBroadcastACandidateThatIsNotItsSource() external { + SourceMismatchDeploy mismatch = new SourceMismatchDeploy(); + + string memory requested = vm.envOr("DEPLOYMENT_SUITE", string("")); + vm.expectRevert( + abi.encodeWithSelector( + UnknownDeploymentSuite.selector, requested, "anchored-candidate, mismatched-candidate" + ) + ); + mismatch.externalSuiteByName(requested); + + vm.expectRevert( + abi.encodeWithSelector( + CandidateSourceMismatch.selector, + "mismatched-candidate", + keccak256(type(MockDeployableV2).creationCode), + keccak256(type(MockDeployable).creationCode) + ) + ); + mismatch.run(); + } + /// The default target set MUST be every supported network, so a /// deterministic deployment reaches one address on every chain from one /// dispatch and no repo restates the list. diff --git a/test/src/abstract/RainDeploySuitesBase.t.sol b/test/src/abstract/RainDeploySuitesBase.t.sol index 5a619ff..1619cd2 100644 --- a/test/src/abstract/RainDeploySuitesBase.t.sol +++ b/test/src/abstract/RainDeploySuitesBase.t.sol @@ -145,9 +145,9 @@ contract RainDeploySuitesBaseTest is Test { /// An empty candidate list reads as a repo with nothing left to declare and /// is a repo whose source anchor — the only check that catches a snapshot /// of the wrong contract — has been handed nothing to run over. It is - /// refused rather than tolerated, and refused on all three readers, because - /// a reader that answers from an empty declaration is a reader through - /// which the whole registry can be empty and green. + /// refused rather than tolerated, and refused on EVERY reader, because a + /// reader that answers from an empty declaration is a reader through which + /// the whole registry can be empty and green. function testNoCandidateReverts() external { NoCandidateDeploySuites none = new NoCandidateDeploySuites(); @@ -168,12 +168,19 @@ contract RainDeploySuitesBaseTest is Test { // passes, so that check cannot be the thing that catches this. vm.expectRevert(abi.encodeWithSelector(NoDeployCandidates.selector)); none.externalCheckedCandidateSuites(); + + // Including through the anchor itself, which the BROADCAST runs. An + // empty declaration that reached it would be a deploy whose only + // wrong-contract check ran over nothing and passed. + vm.expectRevert(abi.encodeWithSelector(NoDeployCandidates.selector)); + none.externalCheckCandidatesAnchoredToSource(); } /// The refusal MUST be discriminating: a declaration that DOES name a - /// candidate answers all four readers rather than reverting, so the test - /// above is about emptiness and not about the fixture. + /// candidate answers every reader rather than reverting, so the test above + /// is about emptiness and not about the fixture. function testCandidatesPresentAnswers() external view { assertEq(sSuites.externalCheckedCandidateSuites().length, 2); + sSuites.externalCheckCandidatesAnchoredToSource(); } } diff --git a/test/src/abstract/RainDeployVerifySnapshot.t.sol b/test/src/abstract/RainDeployVerifySnapshot.t.sol index 0da9f97..e187431 100644 --- a/test/src/abstract/RainDeployVerifySnapshot.t.sol +++ b/test/src/abstract/RainDeployVerifySnapshot.t.sol @@ -3,9 +3,8 @@ pragma solidity =0.8.25; import {ZoltuDerivationMismatch} from "../../../src/abstract/RainDeployVerifyBase.sol"; -import {DeployCandidate, DeploySuite} from "../../../src/abstract/RainDeploySuitesBase.sol"; +import {CandidateSourceMismatch, DeployCandidate, DeploySuite} from "../../../src/abstract/RainDeploySuitesBase.sol"; import { - CandidateSourceMismatch, FrozenSnapshotNotReleased, FrozenSnapshotUnreadable, RainDeployVerifySnapshot, @@ -15,9 +14,10 @@ import { } from "../../../src/abstract/RainDeployVerifySnapshot.sol"; import {LibRainDeploySnapshot} from "../../../src/lib/LibRainDeploySnapshot.sol"; import {LibRainDeploy} from "../../../src/lib/LibRainDeploy.sol"; -import {AddressRegistry} from "../../../src/concrete/AddressRegistry.sol"; import {ExampleDeploySuites} from "../../abstract/ExampleDeploySuites.sol"; +import {MockDeployable} from "../../concrete/MockDeployable.sol"; import {MockDeployableV2} from "../../concrete/MockDeployableV2.sol"; +import {SourceMismatchDeploy} from "../../concrete/SourceMismatchDeploy.sol"; import { BYTECODE_HASH as ADDRESS_REGISTRY_BYTECODE_HASH, CREATION_CODE as ADDRESS_REGISTRY_CREATION_CODE, @@ -35,10 +35,29 @@ import { /// /// The rest is what each group CATCHES, and — for the internal group — what it /// provably does not. Every case drives the same internal functions the -/// inherited tests do, through external wrappers so `vm.expectRevert` lands at -/// the right call depth, with the exemplar data deliberately broken one field at -/// a time. +/// inherited tests do, at a call depth `vm.expectRevert` lands at. +/// +/// The groups that take their subject as an argument are driven with the +/// exemplar data deliberately broken one field at a time. The source anchor +/// takes none: it reads the declaration, because `RainDeployBroadcast` runs it +/// with nothing to hand it. Its negative case is therefore a whole broken +/// DECLARATION — `SourceMismatchDeploy` — which is also the shape a repo holding +/// a stale generated file is actually in. contract RainDeployVerifySnapshotTest is ExampleDeploySuites, RainDeployVerifySnapshot { + /// A declaration whose second candidate is a consistent snapshot of the + /// wrong contract. + /// + /// A whole declaration rather than a candidate built here, because the + /// source anchor reads the declaration itself — it has to, so that the + /// broadcast can run the same definition without being handed anything. + /// A set passed in as an argument would be a set only a test can supply. + SourceMismatchDeploy internal sMismatch; + + /// The broken declaration, as a repo holding a stale generated file has it. + function setUp() external { + sMismatch = new SourceMismatchDeploy(); + } + /// External wrapper for `checkInternallyConsistent` so `vm.expectRevert` /// works at the correct call depth. /// @param suite The suite to check. @@ -46,20 +65,6 @@ contract RainDeployVerifySnapshotTest is ExampleDeploySuites, RainDeployVerifySn checkInternallyConsistent(suite); } - /// External wrapper for `checkAnchoredToSource` so `vm.expectRevert` works - /// at the correct call depth. - /// @param candidate The candidate to check. - function externalCheckAnchoredToSource(DeployCandidate memory candidate) external pure { - checkAnchoredToSource(candidate); - } - - /// External wrapper for `checkCandidatesAnchoredToSource` so - /// `vm.expectRevert` works at the correct call depth. - /// @param candidates The candidates to check. - function externalCheckCandidatesAnchoredToSource(DeployCandidate[] memory candidates) external pure { - checkCandidatesAnchoredToSource(candidates); - } - /// External wrapper for `checkFrozenSnapshotsReleased` so `vm.expectRevert` /// works at the correct call depth. /// @param paths The frozen record's files. @@ -214,27 +219,6 @@ contract RainDeployVerifySnapshotTest is ExampleDeploySuites, RainDeployVerifySn this.externalCheckFrozenSnapshotsReleased(paths, releasedSuites()); } - /// A consistent snapshot of the WRONG contract: every recorded field is - /// `MockDeployable`'s and they all agree with each other, but it is - /// presented as the candidate for a repo whose source is - /// `MockDeployableV2`. This is the shape of a snapshot generated from a - /// stale build, or from the wrong contract in a repo with several. - /// @return The wrong-contract candidate. - function wrongContractCandidate() internal pure returns (DeployCandidate memory) { - return DeployCandidate({ - snapshot: DeploySuite({ - suite: "address-registry-candidate", - creationCode: ADDRESS_REGISTRY_CREATION_CODE, - storedDeployedAddress: ADDRESS_REGISTRY_DEPLOYED_ADDRESS, - storedBytecodeHash: ADDRESS_REGISTRY_BYTECODE_HASH, - storedRuntimeCode: ADDRESS_REGISTRY_RUNTIME_CODE, - artifactPath: "src/concrete/AddressRegistry.sol:AddressRegistry", - dependencies: new address[](0) - }), - sourceCreationCode: type(MockDeployableV2).creationCode - }); - } - /// The frozen `0_0_1` release, which every negative case below breaks one /// field of. /// @return The consistent `0_0_1` suite. @@ -315,74 +299,53 @@ contract RainDeployVerifySnapshotTest is ExampleDeploySuites, RainDeployVerifySn /// covering the source-anchored one, and what makes the next test the only /// thing standing between a stale snapshot and a green suite. function testWrongContractSnapshotPassesInternalConsistency() external { - DeployCandidate memory candidate = wrongContractCandidate(); + DeployCandidate memory candidate = sMismatch.externalCheckedCandidateSuites()[1]; - // It really is the wrong contract: the recorded creation code is not - // the creation code this repo compiles for the candidate. - assertNotEq(keccak256(candidate.snapshot.creationCode), keccak256(type(MockDeployableV2).creationCode)); - assertEq(keccak256(candidate.snapshot.creationCode), keccak256(type(AddressRegistry).creationCode)); + // It really is the wrong contract: the snapshot records `MockDeployableV2` + // while the source it claims to be is `MockDeployable`. + assertEq(keccak256(candidate.snapshot.creationCode), keccak256(type(MockDeployableV2).creationCode)); + assertEq(keccak256(candidate.sourceCreationCode), keccak256(type(MockDeployable).creationCode)); + assertNotEq(keccak256(candidate.snapshot.creationCode), keccak256(candidate.sourceCreationCode)); // Every internal check passes anyway. this.externalCheckInternallyConsistent(candidate.snapshot); } /// The source-anchored group MUST catch exactly the snapshot the internal - /// group just let through, naming the candidate and both creation code - /// hashes. This is the only check in the whole suite that can. - function testWrongContractSnapshotCaughtBySource() external { - DeployCandidate memory candidate = wrongContractCandidate(); - - vm.expectRevert( - abi.encodeWithSelector( - CandidateSourceMismatch.selector, - "address-registry-candidate", - keccak256(ADDRESS_REGISTRY_CREATION_CODE), - keccak256(type(MockDeployableV2).creationCode) - ) - ); - this.externalCheckAnchoredToSource(candidate); - } - - /// A candidate whose recorded creation code IS the source's MUST pass, so - /// the previous test is discriminating rather than a check that always - /// fails. - function testCandidateAnchoredToSourcePasses() external view { - this.externalCheckCandidatesAnchoredToSource(checkedCandidateSuites()); - } - - /// The source anchor MUST reach EVERY candidate, not just the first. + /// group just let through, MUST reach every candidate to do it, and MUST + /// name the one that failed. This is the only check in the whole suite that + /// can catch any of it. /// - /// A loop that stops early is invisible while a repo declares one - /// candidate, and silently stops anchoring the moment it declares two — and - /// a repo with several contracts is precisely where a snapshot generated - /// from the wrong one comes from. So the broken candidate is the LAST one, - /// behind a good one, and the failure has to name it. - function testSourceAnchorReachesEveryCandidate() external { - DeployCandidate[] memory candidates = checkedCandidateSuites(); + /// The broken candidate is the LAST one, behind a genuinely anchored one. A + /// loop that stops early is invisible while a repo declares one candidate + /// and silently stops anchoring the moment it declares two — and a repo with + /// several contracts is precisely where a snapshot generated from the wrong + /// one comes from. The failure naming the SECOND key is what separates a + /// loop that reached it from one that reported a fixed entry or the first. + /// + /// The inherited `testSnapshotMatchesSource` is the passing case: it runs + /// this same function over `ExampleDeploySuites`, whose candidates are their + /// own source. + function testWrongContractSnapshotCaughtBySource() external { + DeployCandidate[] memory candidates = sMismatch.externalCheckedCandidateSuites(); assertEq(candidates.length, 2); - // The first is genuinely fine, so nothing fails before the loop has to - // advance. - this.externalCheckAnchoredToSource(candidates[0]); - - // The break keeps the SECOND candidate's own key, so the failure names - // the entry that actually failed rather than the good one sitting in - // front of it — a loop that reported a fixed entry, or the first, would - // otherwise be indistinguishable from one that reported the right one. - DeployCandidate memory broken = wrongContractCandidate(); - broken.snapshot.suite = candidates[1].snapshot.suite; - assertEq(broken.snapshot.suite, "second-address-candidate"); - candidates[1] = broken; + // The first is genuinely anchored, so nothing fails before the loop has + // to advance, and it is a different contract at a different address + // rather than the same entry under two keys. + assertEq(candidates[0].snapshot.suite, "anchored-candidate"); + assertEq(keccak256(candidates[0].snapshot.creationCode), keccak256(candidates[0].sourceCreationCode)); + assertNotEq(candidates[0].snapshot.storedDeployedAddress, candidates[1].snapshot.storedDeployedAddress); vm.expectRevert( abi.encodeWithSelector( CandidateSourceMismatch.selector, - "second-address-candidate", - keccak256(ADDRESS_REGISTRY_CREATION_CODE), - keccak256(type(MockDeployableV2).creationCode) + "mismatched-candidate", + keccak256(type(MockDeployableV2).creationCode), + keccak256(type(MockDeployable).creationCode) ) ); - this.externalCheckCandidatesAnchoredToSource(candidates); + sMismatch.externalCheckCandidatesAnchoredToSource(); } /// Two suites that record the SAME creation code MUST both derive, which From c976e3a023c6527e7b75b68746cf48442ba19029 Mon Sep 17 00:00:00 2001 From: David Meister Date: Sun, 16 Aug 2026 13:19:43 +0000 Subject: [PATCH 2/2] Point the ordering reference at the name main renamed it to The broadcast ordering test became `testRunSelectsTheSuiteFromTheEnvBeforeTheKeyNeverDefaultsAndBroadcastsIt` on main while this branch was open, leaving the anchor test's docstring naming a function that no longer exists. Co-Authored-By: Claude Opus 5 (1M context) --- test/src/abstract/RainDeployBroadcast.t.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/src/abstract/RainDeployBroadcast.t.sol b/test/src/abstract/RainDeployBroadcast.t.sol index 2f754ab..8923d3c 100644 --- a/test/src/abstract/RainDeployBroadcast.t.sol +++ b/test/src/abstract/RainDeployBroadcast.t.sol @@ -221,8 +221,8 @@ contract RainDeployBroadcastTest is Test { /// `run()` is the anchor's instead, which is what says the anchor ran first; /// an anchor placed after the selection would produce the other one. The key /// is read after the suite, which - /// `testRunSelectsTheSuiteFromTheEnvBeforeTheKeyAndNeverDefaults` pins, so - /// before the suite is before the key. + /// `testRunSelectsTheSuiteFromTheEnvBeforeTheKeyNeverDefaultsAndBroadcastsIt` + /// pins, so before the suite is before the key. /// /// Nothing here writes an env var. Both values are process-wide and forge /// runs tests concurrently, so a second writer is a race, and this test does