From d2b541e80395110dc58d8c5ce162d3e53fd126c2 Mon Sep 17 00:00:00 2001 From: baku-ccron Date: Sun, 20 Sep 2026 11:59:40 +0000 Subject: [PATCH 1/2] Match each frozen record file to a released suite of its own `checkFrozenSnapshotsReleased` asked, for each file in the frozen record, whether SOME declared released suite derives the address that file records. The derived address is a pure function of the creation code, so that question cannot tell two tags that froze identical bytes apart -- and identical bytes are not exotic: a release that changes only dependencies, metadata or documentation compiles to the same creation code, which is how `0_1_7`/`0_1_8` and `0_1_10`/`0_1_11` came to be two such pairs in this repo`s own record. Either half of such a pair could therefore be dropped from `releasedSuites()` and its own record file would go on being "declared" by its twin: the dropped key stops being dispatchable, the dropped entry`s frozen dependency list stops being checked, and the whole suite stays green. That is the state the record check exists to make impossible. The match is now one for one: N record files recording an address need N released suites deriving it. Still by derived address and by nothing else -- `DeploySuite.suite` is a free-form key on a virtual declaration, so matching on a tag spelled inside it would make this pass or fail on the shape of a string and would refuse every consumer that spells its keys some other way. Counting needs no convention at all. The inequality runs one way only. More declarations than record files at an address stays passing, because a release deployed before a repo adopted this machinery has no frozen record and never will. `FrozenSnapshotsOutnumberReleases` is new and is raised only where `FrozenSnapshotNotReleased` never could be: there ARE matching entries and there are not enough of them, so a reader told "not released" would go looking for an entry that is already there. Reported against a consumer in rainlanguage/rain.factory.deploy#33. Co-Authored-By: Claude Opus 5 (1M context) --- src/abstract/RainDeployVerifySnapshot.sol | 8 ++ src/abstract/RainDeployVerifySnapshotBase.sol | 122 +++++++++++++--- .../RainDeployVerifySnapshotBase.t.sol | 135 ++++++++++++++++++ 3 files changed, 246 insertions(+), 19 deletions(-) diff --git a/src/abstract/RainDeployVerifySnapshot.sol b/src/abstract/RainDeployVerifySnapshot.sol index f10070a..9936c49 100644 --- a/src/abstract/RainDeployVerifySnapshot.sol +++ b/src/abstract/RainDeployVerifySnapshot.sol @@ -55,6 +55,14 @@ abstract contract RainDeployVerifySnapshot is RainDeployVerifySnapshotBase { /// and never will. A size check would red-line that permanently with no way /// to spell the exemption, while the release it names goes on being checked /// by everything anchored to a chain. + /// + /// The per-address tally `checkFrozenSnapshotsReleased` does keep is not + /// that size check wearing a different hat. It counts within ONE derived + /// address and refuses only the direction that hides a release — more + /// record files at an address than declared suites deriving it. A + /// declaration that names releases the record has never held, at that + /// address or at any other, passes it exactly as it passes here, which is + /// what keeps the pre-machinery release spellable. function testEveryFrozenSnapshotIsReleased() external view { checkFrozenSnapshotsReleased(LibRainDeploySnapshot.frozenSnapshotPaths(vm), releasedSuites()); } diff --git a/src/abstract/RainDeployVerifySnapshotBase.sol b/src/abstract/RainDeployVerifySnapshotBase.sol index c7e1ebe..a42c7d7 100644 --- a/src/abstract/RainDeployVerifySnapshotBase.sol +++ b/src/abstract/RainDeployVerifySnapshotBase.sol @@ -35,6 +35,35 @@ error StoredRuntimeCodeHashMismatch(string suite, bytes32 storedBytecodeHash, by /// @param path The frozen record file no released suite declares. error FrozenSnapshotNotReleased(string path); +/// Thrown when the frozen record holds more files recording one deploy address +/// than the declaration holds released suites deriving it. +/// +/// Every record file needs a declaration of ITS OWN. The address is a pure +/// function of the creation code, so two tags that froze identical bytes derive +/// one address — a release that changed only dependencies, metadata or +/// documentation compiles to the same bytes, which is the ordinary way such a +/// pair comes about. A check that asked only whether SOME declared suite +/// derives a file's address would let either of them be dropped from the +/// declaration and go on being "declared" by its twin: the dropped key stops +/// being dispatchable, the dropped entry's own frozen dependency list stops +/// being checked, and nothing anywhere reports it. +/// +/// Distinct from `FrozenSnapshotNotReleased`, which is a record file that no +/// declared suite derives AT ALL. Here there are matching entries and there are +/// not ENOUGH of them, so a reader told the other error would go looking for an +/// entry that is already there. The counts say how many are missing and the +/// address says which releases to count. +/// +/// The path named is the file the record ran out of declarations at, which is +/// not necessarily the release that was dropped — to a match by address the +/// files sharing one are interchangeable, which is the whole defect this +/// exists for. The fix is at the declaration either way. +/// @param path The record file the record ran out of declarations at. +/// @param deployedAddress The address that file declares. +/// @param records The record files declaring it, up to and including `path`. +/// @param released The released suites whose creation code derives it. +error FrozenSnapshotsOutnumberReleases(string path, address deployedAddress, uint256 records, uint256 released); + /// Thrown when a file in the frozen record declares no deployed address. The /// record holds generated snapshots and nothing else, and `DEPLOYED_ADDRESS` is /// what makes one the record of a deployment rather than a file that happens to @@ -96,12 +125,13 @@ error EtherscanEntryUnresolvable(string entry); /// /// **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 -/// what a declared suite records, and it exists because everything anchored to -/// a chain reads `releasedSuites()`, which is a separate file from the record -/// it describes. A release missing from it is not caught anywhere else, by -/// anything: it simply stops being checked, and every check there is stays -/// green. +/// suite OF ITS OWN, one for one, so a release cannot be covered by another +/// that froze the same bytes. This is the one check that is about the +/// DECLARATION rather than about what a declared suite records, and it exists +/// because everything anchored to a chain reads `releasedSuites()`, which is a +/// separate file from the record it describes. A release missing from it is not +/// caught anywhere else, by anything: it simply stops being checked, and every +/// check there is stays green. /// /// None of the three can catch a suite that was never deployed, or that is no /// longer deployed. Only `RainDeployVerifyChain` can, and nothing here is a @@ -246,7 +276,7 @@ abstract contract RainDeployVerifySnapshotBase is RainDeployVerifyBase { } /// Checks the frozen record against the released declaration: every file in - /// the record is declared by a released suite. + /// the record is declared by a released suite OF ITS OWN. /// /// `releasedSuites()` is a generated file, and everything anchored to a /// chain reads it. A frozen tag it does not name is therefore not a missing @@ -271,30 +301,84 @@ abstract contract RainDeployVerifySnapshotBase is RainDeployVerifyBase { /// The match is by address: the address a file DECLARES against the address /// a suite's creation code DERIVES. The derived side is a pure function of /// the creation code, so a suite whose creation code derives the address a - /// file records IS that file's release. + /// file records IS a release of those bytes. + /// + /// It is ONE FOR ONE, which is the whole of what a shared address costs. + /// That derivation is not injective ACROSS RELEASES: two tags that froze + /// identical bytes derive one address and are indistinguishable here, and + /// they are not exotic — a release that changes only dependencies, + /// metadata or documentation compiles to the same bytes. Asking only + /// whether SOME declared suite derives a file's address lets the survivor + /// of such a pair answer for its twin, so either tag can be dropped from + /// the declaration while its own record file goes on being "declared" by + /// the other, and every check stays green. Counting instead — N record + /// files at an address need N declared suites deriving it — hands each file + /// a declaration that is not already spoken for, and no arrangement of + /// twins hides a dropped entry. + /// + /// The inequality runs ONE WAY. More declarations than record files at an + /// address is not a failure: a release deployed before this repo adopted + /// the machinery has no frozen record and never will, and see + /// `testEveryFrozenSnapshotIsReleased` for why refusing that would + /// red-line a legitimate state with no way to spell the exemption. /// /// Nothing is matched by name, which would assert only that a convention - /// was followed. Nothing is matched by searching the file's text either: a - /// record is mostly two hex payloads thousands of digits long, and an - /// address that merely OCCURS somewhere in one of them says nothing about - /// what the file records. - /// @param paths The frozen record's files. + /// was followed — and `DeploySuite.suite` is a free-form key on a virtual + /// declaration, so a consumer is free to spell one that carries no release + /// tag at all. Matching on it would turn this into a check on the shape of + /// a string rather than on what a file records, and would fail every + /// declaration that spells its keys some other way. Nothing is matched by + /// searching the file's text either: a record is mostly two hex payloads + /// thousands of digits long, and an address that merely OCCURS somewhere in + /// one of them says nothing about what the file records. + /// + /// What is left uncovered is an entry RENAMED rather than dropped. The + /// counts are unchanged, so a key that names a tag the record does not hold + /// still answers for the file whose address it derives. Nothing in a + /// `DeploySuite` says which tag it was cut from — only its key does, by a + /// convention the struct does not hold it to — so the record cannot see + /// that at all, and a check that could would be the name match this refuses + /// to be. + /// @param paths The frozen record's files. Distinct files: the walk that + /// produces them lists a directory tree, so it never repeats one, and a + /// list that did would be two claims about one file. /// @param released The declared released suites. function checkFrozenSnapshotsReleased(string[] memory paths, DeploySuite[] memory released) internal view { + // Every record file's address, in walk order, so the tally below can + // ask about the files already passed without reading them again. A + // record file is tens of kilobytes of hex and reading it is what this + // check costs, so each is read exactly once however many files share an + // address. + address[] memory recorded = new address[](paths.length); + for (uint256 i = 0; i < paths.length; i++) { - address recorded = recordedDeployedAddress(paths[i], vm.readFile(paths[i])); + recorded[i] = recordedDeployedAddress(paths[i], vm.readFile(paths[i])); - bool declared = false; + uint256 declarations = 0; for (uint256 j = 0; j < released.length; j++) { - if (recorded == LibRainDeploy.zoltuAddress(released[j].creationCode)) { - declared = true; - break; + if (recorded[i] == LibRainDeploy.zoltuAddress(released[j].creationCode)) { + declarations++; } } - if (!declared) { + if (declarations == 0) { revert FrozenSnapshotNotReleased(paths[i]); } + + // This file and every EARLIER one recording the same address. The + // files after it are not counted, so the failure lands on the file + // the record ran out of declarations at rather than on the first of + // a group that is only short once the walk has reached the last. + uint256 records = 1; + for (uint256 k = 0; k < i; k++) { + if (recorded[k] == recorded[i]) { + records++; + } + } + + if (records > declarations) { + revert FrozenSnapshotsOutnumberReleases(paths[i], recorded[i], records, declarations); + } } } diff --git a/test/src/abstract/RainDeployVerifySnapshotBase.t.sol b/test/src/abstract/RainDeployVerifySnapshotBase.t.sol index 1fc3d5f..aa67e74 100644 --- a/test/src/abstract/RainDeployVerifySnapshotBase.t.sol +++ b/test/src/abstract/RainDeployVerifySnapshotBase.t.sol @@ -8,6 +8,7 @@ import { EtherscanEntryUnresolvable, FrozenSnapshotAmbiguous, FrozenSnapshotNotReleased, + FrozenSnapshotsOutnumberReleases, FrozenSnapshotUnreadable, RainDeployVerifySnapshotBase, StoredAddressMismatch, @@ -26,6 +27,18 @@ import { DEPLOYED_ADDRESS as ADDRESS_REGISTRY_DEPLOYED_ADDRESS, RUNTIME_CODE as ADDRESS_REGISTRY_RUNTIME_CODE } from "../../../src/generated/candidate/AddressRegistry.sol"; +import { + BYTECODE_HASH as ADDRESS_REGISTRY_0_1_10_BYTECODE_HASH, + CREATION_CODE as ADDRESS_REGISTRY_0_1_10_CREATION_CODE, + DEPLOYED_ADDRESS as ADDRESS_REGISTRY_0_1_10_DEPLOYED_ADDRESS, + RUNTIME_CODE as ADDRESS_REGISTRY_0_1_10_RUNTIME_CODE +} from "../../../src/generated/0_1_10/AddressRegistry.sol"; +import { + BYTECODE_HASH as ADDRESS_REGISTRY_0_1_11_BYTECODE_HASH, + CREATION_CODE as ADDRESS_REGISTRY_0_1_11_CREATION_CODE, + DEPLOYED_ADDRESS as ADDRESS_REGISTRY_0_1_11_DEPLOYED_ADDRESS, + RUNTIME_CODE as ADDRESS_REGISTRY_0_1_11_RUNTIME_CODE +} from "../../../src/generated/0_1_11/AddressRegistry.sol"; /// @title RainDeployVerifySnapshotBaseTest /// @notice `RainDeployVerifySnapshotBase` inherited by a exemplar repo, so the @@ -152,6 +165,128 @@ contract RainDeployVerifySnapshotBaseTest is ExampleDeploySuites, RainDeployVeri this.externalCheckFrozenSnapshotsReleased(recordOfTheGeneratedSnapshot(), wrongRelease); } + /// A record of the twin releases `0_1_10` and `0_1_11`: two frozen tags of + /// this repo's own that froze the SAME creation code, so the two files + /// declare one deploy address between them. + /// + /// Real frozen files rather than a fixture, because the pair IS the + /// subject. A record directory is append-only, so these two are byte + /// identical to each other permanently, and a release that changes only + /// dependencies, metadata or documentation compiles to identical bytes — + /// which is exactly how this pair came to be one. Every test below asserts + /// that premise rather than assuming it, so a record that stopped being a + /// twin pair is a red test rather than a green one with nothing in it. + /// @return paths The two-file record. + function recordOfTheTwinReleases() internal pure returns (string[] memory paths) { + paths = new string[](2); + paths[0] = LibRainDeploySnapshot.pathForSnapshot("0_1_10", "AddressRegistry"); + paths[1] = LibRainDeploySnapshot.pathForSnapshot("0_1_11", "AddressRegistry"); + } + + /// The declaration entry for the OLDER twin, `0_1_10`, built from that + /// release's own frozen constants exactly as the generated declaration + /// builds it. + /// @return The `0_1_10` suite. + function declaredOlderTwin() internal pure returns (DeploySuite memory) { + return DeploySuite({ + suite: "address-registry@0_1_10", + creationCode: ADDRESS_REGISTRY_0_1_10_CREATION_CODE, + storedDeployedAddress: ADDRESS_REGISTRY_0_1_10_DEPLOYED_ADDRESS, + storedBytecodeHash: ADDRESS_REGISTRY_0_1_10_BYTECODE_HASH, + storedRuntimeCode: ADDRESS_REGISTRY_0_1_10_RUNTIME_CODE, + artifactPath: "src/concrete/AddressRegistry.sol:AddressRegistry", + dependencies: new address[](0) + }); + } + + /// The declaration entry for the NEWER twin, `0_1_11`, built the same way. + /// @return The `0_1_11` suite. + function declaredNewerTwin() internal pure returns (DeploySuite memory) { + return DeploySuite({ + suite: "address-registry@0_1_11", + creationCode: ADDRESS_REGISTRY_0_1_11_CREATION_CODE, + storedDeployedAddress: ADDRESS_REGISTRY_0_1_11_DEPLOYED_ADDRESS, + storedBytecodeHash: ADDRESS_REGISTRY_0_1_11_BYTECODE_HASH, + storedRuntimeCode: ADDRESS_REGISTRY_0_1_11_RUNTIME_CODE, + artifactPath: "src/concrete/AddressRegistry.sol:AddressRegistry", + dependencies: new address[](0) + }); + } + + /// The two twins really are twins: identical creation code, and therefore + /// one derived address between them. The premise every case below rests + /// on, asserted rather than assumed. + function testTwinReleasesFrozeTheSameCreationCode() external pure { + assertEq(keccak256(ADDRESS_REGISTRY_0_1_10_CREATION_CODE), keccak256(ADDRESS_REGISTRY_0_1_11_CREATION_CODE)); + assertEq(ADDRESS_REGISTRY_0_1_10_DEPLOYED_ADDRESS, ADDRESS_REGISTRY_0_1_11_DEPLOYED_ADDRESS); + assertEq( + LibRainDeploy.zoltuAddress(ADDRESS_REGISTRY_0_1_10_CREATION_CODE), + LibRainDeploy.zoltuAddress(ADDRESS_REGISTRY_0_1_11_CREATION_CODE) + ); + } + + /// A release dropped from the declaration MUST fail even when ANOTHER + /// release froze the same bytes. + /// + /// The derived address is a pure function of the creation code, so two + /// tags that froze identical bytes derive ONE address and are + /// indistinguishable to anything that only asks whether some declared + /// suite derives the address a file records: the survivor answers for its + /// twin, and the dropped release goes on being "declared" by a file it is + /// not. That is the state this whole check exists to make impossible — + /// the dropped entry's key stops being dispatchable and its own + /// dependency list stops being checked, with nothing anywhere reporting + /// it. + /// + /// Two record files sharing an address therefore need TWO declarations, + /// not one that matches both. + function testFrozenSnapshotTwinReleaseDroppedFromTheDeclarationReverts() external { + DeploySuite[] memory released = new DeploySuite[](1); + released[0] = declaredNewerTwin(); + + vm.expectRevert( + abi.encodeWithSelector( + FrozenSnapshotsOutnumberReleases.selector, + recordOfTheTwinReleases()[1], + ADDRESS_REGISTRY_0_1_11_DEPLOYED_ADDRESS, + uint256(2), + uint256(1) + ) + ); + this.externalCheckFrozenSnapshotsReleased(recordOfTheTwinReleases(), released); + } + + /// Both twins declared MUST pass, so the case above is a missing + /// declaration being caught rather than a record with a shared address + /// being refused outright. This is the ordinary state of a repo that cut + /// two releases without a source change between them, and it is this + /// repo's own. + function testFrozenSnapshotTwinReleasesBothDeclaredPasses() external view { + DeploySuite[] memory released = new DeploySuite[](2); + released[0] = declaredOlderTwin(); + released[1] = declaredNewerTwin(); + + this.externalCheckFrozenSnapshotsReleased(recordOfTheTwinReleases(), released); + } + + /// A declaration with MORE entries at an address than the record has files + /// MUST pass. The record is checked against the declaration in one + /// direction only: a release deployed before this repo adopted the + /// machinery has no frozen record and never will, and refusing it would + /// red-line that permanently with no way to spell the exemption — while + /// the release it names goes on being checked by everything anchored to a + /// chain. + function testFrozenSnapshotMoreDeclarationsThanRecordFilesPasses() external view { + string[] memory paths = new string[](1); + paths[0] = recordOfTheTwinReleases()[1]; + + DeploySuite[] memory released = new DeploySuite[](2); + released[0] = declaredOlderTwin(); + released[1] = declaredNewerTwin(); + + this.externalCheckFrozenSnapshotsReleased(paths, released); + } + /// A record in the generated shape that DECLARES one address and merely /// mentions another — in a comment, and in a second address constant. /// @param declared The address the record declares as its deploy address. From 2b8030150be4e01c4b8ed558ddb06ebd24db7354 Mon Sep 17 00:00:00 2001 From: baku-ccron Date: Sun, 20 Sep 2026 13:23:29 +0000 Subject: [PATCH 2/2] Cut the design argument from the one-for-one frozen record check Co-Authored-By: Claude Opus 5 (1M context) --- src/abstract/RainDeployVerifySnapshot.sol | 8 -- src/abstract/RainDeployVerifySnapshotBase.sol | 76 ++----------------- .../RainDeployVerifySnapshotBase.t.sol | 48 ------------ 3 files changed, 7 insertions(+), 125 deletions(-) diff --git a/src/abstract/RainDeployVerifySnapshot.sol b/src/abstract/RainDeployVerifySnapshot.sol index 9936c49..f10070a 100644 --- a/src/abstract/RainDeployVerifySnapshot.sol +++ b/src/abstract/RainDeployVerifySnapshot.sol @@ -55,14 +55,6 @@ abstract contract RainDeployVerifySnapshot is RainDeployVerifySnapshotBase { /// and never will. A size check would red-line that permanently with no way /// to spell the exemption, while the release it names goes on being checked /// by everything anchored to a chain. - /// - /// The per-address tally `checkFrozenSnapshotsReleased` does keep is not - /// that size check wearing a different hat. It counts within ONE derived - /// address and refuses only the direction that hides a release — more - /// record files at an address than declared suites deriving it. A - /// declaration that names releases the record has never held, at that - /// address or at any other, passes it exactly as it passes here, which is - /// what keeps the pre-machinery release spellable. function testEveryFrozenSnapshotIsReleased() external view { checkFrozenSnapshotsReleased(LibRainDeploySnapshot.frozenSnapshotPaths(vm), releasedSuites()); } diff --git a/src/abstract/RainDeployVerifySnapshotBase.sol b/src/abstract/RainDeployVerifySnapshotBase.sol index a42c7d7..a4c4060 100644 --- a/src/abstract/RainDeployVerifySnapshotBase.sol +++ b/src/abstract/RainDeployVerifySnapshotBase.sol @@ -37,27 +37,6 @@ error FrozenSnapshotNotReleased(string path); /// Thrown when the frozen record holds more files recording one deploy address /// than the declaration holds released suites deriving it. -/// -/// Every record file needs a declaration of ITS OWN. The address is a pure -/// function of the creation code, so two tags that froze identical bytes derive -/// one address — a release that changed only dependencies, metadata or -/// documentation compiles to the same bytes, which is the ordinary way such a -/// pair comes about. A check that asked only whether SOME declared suite -/// derives a file's address would let either of them be dropped from the -/// declaration and go on being "declared" by its twin: the dropped key stops -/// being dispatchable, the dropped entry's own frozen dependency list stops -/// being checked, and nothing anywhere reports it. -/// -/// Distinct from `FrozenSnapshotNotReleased`, which is a record file that no -/// declared suite derives AT ALL. Here there are matching entries and there are -/// not ENOUGH of them, so a reader told the other error would go looking for an -/// entry that is already there. The counts say how many are missing and the -/// address says which releases to count. -/// -/// The path named is the file the record ran out of declarations at, which is -/// not necessarily the release that was dropped — to a match by address the -/// files sharing one are interchangeable, which is the whole defect this -/// exists for. The fix is at the declaration either way. /// @param path The record file the record ran out of declarations at. /// @param deployedAddress The address that file declares. /// @param records The record files declaring it, up to and including `path`. @@ -125,8 +104,7 @@ error EtherscanEntryUnresolvable(string entry); /// /// **Anchored to the record.** Every file in the frozen record — the /// append-only `src/generated//` directories — is declared by a released -/// suite OF ITS OWN, one for one, so a release cannot be covered by another -/// that froze the same bytes. This is the one check that is about the +/// suite OF ITS OWN, one for one. This is the one check that is about the /// DECLARATION rather than about what a declared suite records, and it exists /// because everything anchored to a chain reads `releasedSuites()`, which is a /// separate file from the record it describes. A release missing from it is not @@ -303,52 +281,16 @@ abstract contract RainDeployVerifySnapshotBase is RainDeployVerifyBase { /// the creation code, so a suite whose creation code derives the address a /// file records IS a release of those bytes. /// - /// It is ONE FOR ONE, which is the whole of what a shared address costs. - /// That derivation is not injective ACROSS RELEASES: two tags that froze - /// identical bytes derive one address and are indistinguishable here, and - /// they are not exotic — a release that changes only dependencies, - /// metadata or documentation compiles to the same bytes. Asking only - /// whether SOME declared suite derives a file's address lets the survivor - /// of such a pair answer for its twin, so either tag can be dropped from - /// the declaration while its own record file goes on being "declared" by - /// the other, and every check stays green. Counting instead — N record - /// files at an address need N declared suites deriving it — hands each file - /// a declaration that is not already spoken for, and no arrangement of - /// twins hides a dropped entry. - /// - /// The inequality runs ONE WAY. More declarations than record files at an - /// address is not a failure: a release deployed before this repo adopted - /// the machinery has no frozen record and never will, and see - /// `testEveryFrozenSnapshotIsReleased` for why refusing that would - /// red-line a legitimate state with no way to spell the exemption. - /// /// Nothing is matched by name, which would assert only that a convention - /// was followed — and `DeploySuite.suite` is a free-form key on a virtual - /// declaration, so a consumer is free to spell one that carries no release - /// tag at all. Matching on it would turn this into a check on the shape of - /// a string rather than on what a file records, and would fail every - /// declaration that spells its keys some other way. Nothing is matched by - /// searching the file's text either: a record is mostly two hex payloads - /// thousands of digits long, and an address that merely OCCURS somewhere in - /// one of them says nothing about what the file records. + /// was followed. Nothing is matched by searching the file's text either: a + /// record is mostly two hex payloads thousands of digits long, and an + /// address that merely OCCURS somewhere in one of them says nothing about + /// what the file records. /// - /// What is left uncovered is an entry RENAMED rather than dropped. The - /// counts are unchanged, so a key that names a tag the record does not hold - /// still answers for the file whose address it derives. Nothing in a - /// `DeploySuite` says which tag it was cut from — only its key does, by a - /// convention the struct does not hold it to — so the record cannot see - /// that at all, and a check that could would be the name match this refuses - /// to be. - /// @param paths The frozen record's files. Distinct files: the walk that - /// produces them lists a directory tree, so it never repeats one, and a - /// list that did would be two claims about one file. + /// A RENAMED entry is not caught: the counts are unchanged. + /// @param paths The frozen record's files. /// @param released The declared released suites. function checkFrozenSnapshotsReleased(string[] memory paths, DeploySuite[] memory released) internal view { - // Every record file's address, in walk order, so the tally below can - // ask about the files already passed without reading them again. A - // record file is tens of kilobytes of hex and reading it is what this - // check costs, so each is read exactly once however many files share an - // address. address[] memory recorded = new address[](paths.length); for (uint256 i = 0; i < paths.length; i++) { @@ -365,10 +307,6 @@ abstract contract RainDeployVerifySnapshotBase is RainDeployVerifyBase { revert FrozenSnapshotNotReleased(paths[i]); } - // This file and every EARLIER one recording the same address. The - // files after it are not counted, so the failure lands on the file - // the record ran out of declarations at rather than on the first of - // a group that is only short once the walk has reached the last. uint256 records = 1; for (uint256 k = 0; k < i; k++) { if (recorded[k] == recorded[i]) { diff --git a/test/src/abstract/RainDeployVerifySnapshotBase.t.sol b/test/src/abstract/RainDeployVerifySnapshotBase.t.sol index aa67e74..7cf053c 100644 --- a/test/src/abstract/RainDeployVerifySnapshotBase.t.sol +++ b/test/src/abstract/RainDeployVerifySnapshotBase.t.sol @@ -165,28 +165,12 @@ contract RainDeployVerifySnapshotBaseTest is ExampleDeploySuites, RainDeployVeri this.externalCheckFrozenSnapshotsReleased(recordOfTheGeneratedSnapshot(), wrongRelease); } - /// A record of the twin releases `0_1_10` and `0_1_11`: two frozen tags of - /// this repo's own that froze the SAME creation code, so the two files - /// declare one deploy address between them. - /// - /// Real frozen files rather than a fixture, because the pair IS the - /// subject. A record directory is append-only, so these two are byte - /// identical to each other permanently, and a release that changes only - /// dependencies, metadata or documentation compiles to identical bytes — - /// which is exactly how this pair came to be one. Every test below asserts - /// that premise rather than assuming it, so a record that stopped being a - /// twin pair is a red test rather than a green one with nothing in it. - /// @return paths The two-file record. function recordOfTheTwinReleases() internal pure returns (string[] memory paths) { paths = new string[](2); paths[0] = LibRainDeploySnapshot.pathForSnapshot("0_1_10", "AddressRegistry"); paths[1] = LibRainDeploySnapshot.pathForSnapshot("0_1_11", "AddressRegistry"); } - /// The declaration entry for the OLDER twin, `0_1_10`, built from that - /// release's own frozen constants exactly as the generated declaration - /// builds it. - /// @return The `0_1_10` suite. function declaredOlderTwin() internal pure returns (DeploySuite memory) { return DeploySuite({ suite: "address-registry@0_1_10", @@ -199,8 +183,6 @@ contract RainDeployVerifySnapshotBaseTest is ExampleDeploySuites, RainDeployVeri }); } - /// The declaration entry for the NEWER twin, `0_1_11`, built the same way. - /// @return The `0_1_11` suite. function declaredNewerTwin() internal pure returns (DeploySuite memory) { return DeploySuite({ suite: "address-registry@0_1_11", @@ -213,9 +195,6 @@ contract RainDeployVerifySnapshotBaseTest is ExampleDeploySuites, RainDeployVeri }); } - /// The two twins really are twins: identical creation code, and therefore - /// one derived address between them. The premise every case below rests - /// on, asserted rather than assumed. function testTwinReleasesFrozeTheSameCreationCode() external pure { assertEq(keccak256(ADDRESS_REGISTRY_0_1_10_CREATION_CODE), keccak256(ADDRESS_REGISTRY_0_1_11_CREATION_CODE)); assertEq(ADDRESS_REGISTRY_0_1_10_DEPLOYED_ADDRESS, ADDRESS_REGISTRY_0_1_11_DEPLOYED_ADDRESS); @@ -225,21 +204,6 @@ contract RainDeployVerifySnapshotBaseTest is ExampleDeploySuites, RainDeployVeri ); } - /// A release dropped from the declaration MUST fail even when ANOTHER - /// release froze the same bytes. - /// - /// The derived address is a pure function of the creation code, so two - /// tags that froze identical bytes derive ONE address and are - /// indistinguishable to anything that only asks whether some declared - /// suite derives the address a file records: the survivor answers for its - /// twin, and the dropped release goes on being "declared" by a file it is - /// not. That is the state this whole check exists to make impossible — - /// the dropped entry's key stops being dispatchable and its own - /// dependency list stops being checked, with nothing anywhere reporting - /// it. - /// - /// Two record files sharing an address therefore need TWO declarations, - /// not one that matches both. function testFrozenSnapshotTwinReleaseDroppedFromTheDeclarationReverts() external { DeploySuite[] memory released = new DeploySuite[](1); released[0] = declaredNewerTwin(); @@ -256,11 +220,6 @@ contract RainDeployVerifySnapshotBaseTest is ExampleDeploySuites, RainDeployVeri this.externalCheckFrozenSnapshotsReleased(recordOfTheTwinReleases(), released); } - /// Both twins declared MUST pass, so the case above is a missing - /// declaration being caught rather than a record with a shared address - /// being refused outright. This is the ordinary state of a repo that cut - /// two releases without a source change between them, and it is this - /// repo's own. function testFrozenSnapshotTwinReleasesBothDeclaredPasses() external view { DeploySuite[] memory released = new DeploySuite[](2); released[0] = declaredOlderTwin(); @@ -269,13 +228,6 @@ contract RainDeployVerifySnapshotBaseTest is ExampleDeploySuites, RainDeployVeri this.externalCheckFrozenSnapshotsReleased(recordOfTheTwinReleases(), released); } - /// A declaration with MORE entries at an address than the record has files - /// MUST pass. The record is checked against the declaration in one - /// direction only: a release deployed before this repo adopted the - /// machinery has no frozen record and never will, and refusing it would - /// red-line that permanently with no way to spell the exemption — while - /// the release it names goes on being checked by everything anchored to a - /// chain. function testFrozenSnapshotMoreDeclarationsThanRecordFilesPasses() external view { string[] memory paths = new string[](1); paths[0] = recordOfTheTwinReleases()[1];