From eebea972e9c66ddad1100d6d0cfcc7255375fd2d Mon Sep 17 00:00:00 2001 From: baku-ccron Date: Tue, 15 Sep 2026 15:33:21 +0000 Subject: [PATCH 1/6] Generate the rolling snapshots under the record root they are frozen from MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `cutRelease()` regenerates and then freezes from `recordRoot()`, but `Build.regenerateSnapshots` reached `writeSnapshot`, whose only output root was `LIB_FS_ROOT`. Under an overridden root the freeze read a rolling snapshot the regeneration had never written — `NothingToFreeze` — after rewriting the real `src/generated/candidate/`, which is the tree the override exists to keep a caller's hands off. `writeSnapshot` now takes the record root, required for the same reason `freeze` and `writeReleasedSuitesLib` require theirs, and writes through `LibFs.buildFileForContract` at `dirForSnapshot(root, dir)`. At `LIB_FS_ROOT` that is the directory `LibFs.dirForTag` names, so the real lifecycle is unchanged. Closes #206 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01V8ViHcKLVk2YoS2joH4HdN --- script/Build.sol | 1 + src/lib/LibRainDeploySnapshot.sol | 44 +++++----- test/concrete/BuildRecordRootHarness.sol | 35 ++++++++ test/script/Build.t.sol | 73 ++++++++++++++++ test/src/lib/LibRainDeploySnapshot.t.sol | 103 ++++++++++++++++++++++- 5 files changed, 234 insertions(+), 22 deletions(-) create mode 100644 test/concrete/BuildRecordRootHarness.sol diff --git a/script/Build.sol b/script/Build.sol index 8fd0585..1ff8d5d 100644 --- a/script/Build.sol +++ b/script/Build.sol @@ -89,6 +89,7 @@ contract Build is BuildScript, RegistryDeploySuites { for (uint256 i = 0; i < contracts.length; i++) { LibRainDeploySnapshot.writeSnapshot( vm, + recordRoot(), LibRainDeploySnapshot.CANDIDATE, contracts[i].contractName, contracts[i].candidate.sourceCreationCode, diff --git a/src/lib/LibRainDeploySnapshot.sol b/src/lib/LibRainDeploySnapshot.sol index 72750c3..a623fdb 100644 --- a/src/lib/LibRainDeploySnapshot.sol +++ b/src/lib/LibRainDeploySnapshot.sol @@ -423,20 +423,20 @@ library LibRainDeploySnapshot { /// Generate one snapshot for one contract. /// - /// There is no output root to choose. `LibFs.buildFileForTaggedContract` - /// derives its directory from `LIB_FS_ROOT` and the snapshot directory it is - /// handed, and this is the repo's real deploy record, which belongs under - /// that root and nowhere else. This is the one place a snapshot's bytes come - /// into existence, and they come from the compiler rather than from another - /// tree, so there is nothing for a root to select between. + /// The output root is the one `freeze` is handed, and it is required for + /// the same reason: `cutRelease()` regenerates and then freezes within ONE + /// record tree. A generator that could only write under `LIB_FS_ROOT` would + /// leave a release cut under any other root frozen from a rolling snapshot + /// its own regeneration never wrote — after writing the real record on the + /// way there, which is the tree a `recordRoot()` override exists to keep a + /// caller's hands off. /// - /// `freeze` does take a root and that is not the same freedom: it COPIES, - /// within one record tree, reading a rolling snapshot under the root it is - /// handed and writing the frozen copy under that same root. Pointing a - /// copier at a tree of its own is a thing a test genuinely needs, exactly - /// as pointing `frozenSnapshotPaths` at one is; GENERATING this repo's - /// record anywhere but under `LIB_FS_ROOT` remains something nothing here - /// can express. + /// `LibFs.buildFileForContract` takes the directory it writes into, so the + /// root reaches the writer through `dirForSnapshot(root, dir)` and `dir` is + /// still held to the tag alphabet by it. At `LIB_FS_ROOT` that directory is + /// `LibFs.dirForTag(dir)`, which is where + /// `testRootAwareSnapshotPathIsTheWritersAtTheRealRoot` holds the two + /// spellings to being one path. /// /// The dependency list is frozen here with the rest, and it is not /// metadata. `RainDeployBroadcast.run` hands a suite's `dependencies` to @@ -455,6 +455,8 @@ library LibRainDeploySnapshot { /// repo's statement. Repos outside this org call this overload; repos /// inside it call the one that defaults to the org's values. /// @param vm The Vm instance for file operations. + /// @param root The record root to generate into — `LIB_FS_ROOT` for a + /// repo's real record. /// @param dir The snapshot directory name — a release tag, or `CANDIDATE`. /// @param contractName The contract the snapshot describes. /// @param spdxLicenseIdentifier The SPDX licence identifier the written @@ -466,6 +468,7 @@ library LibRainDeploySnapshot { /// @return The path written. function writeSnapshot( Vm vm, + string memory root, string memory dir, string memory contractName, string memory spdxLicenseIdentifier, @@ -478,18 +481,20 @@ library LibRainDeploySnapshot { address deployed = LibRainDeploy.deployZoltu(creationCode); string memory constants = snapshotConstants(vm, deployed, creationCode, dependencies); - // The directory is created by the writer, from the same tag this path is - // derived from, so there is no `createDir` here to disagree with it. - LibFs.buildFileForTaggedContract( - vm, deployed, dir, contractName, spdxLicenseIdentifier, copyrightText, constants + // The directory is created by the writer, from the same root and tag + // this path is derived from, so there is no `createDir` here to + // disagree with it. + LibFs.buildFileForContract( + vm, deployed, dirForSnapshot(root, dir), contractName, spdxLicenseIdentifier, copyrightText, constants ); - return pathForSnapshot(dir, contractName); + return pathForSnapshot(root, dir, contractName); } /// `writeSnapshot` applied to `RAIN_SPDX_LICENSE_IDENTIFIER` and /// `RAIN_COPYRIGHT_TEXT`, for a repo this org owns. /// @param vm The Vm instance for file operations. + /// @param root The record root — `LIB_FS_ROOT` for a repo's real record. /// @param dir The snapshot directory name — a release tag, or `CANDIDATE`. /// @param contractName The contract the snapshot describes. /// @param creationCode That contract's creation code. @@ -498,13 +503,14 @@ library LibRainDeploySnapshot { /// @return The path written. function writeSnapshot( Vm vm, + string memory root, string memory dir, string memory contractName, bytes memory creationCode, address[] memory dependencies ) internal returns (string memory) { return writeSnapshot( - vm, dir, contractName, RAIN_SPDX_LICENSE_IDENTIFIER, RAIN_COPYRIGHT_TEXT, creationCode, dependencies + vm, root, dir, contractName, RAIN_SPDX_LICENSE_IDENTIFIER, RAIN_COPYRIGHT_TEXT, creationCode, dependencies ); } diff --git a/test/concrete/BuildRecordRootHarness.sol b/test/concrete/BuildRecordRootHarness.sol new file mode 100644 index 0000000..8b8f13c --- /dev/null +++ b/test/concrete/BuildRecordRootHarness.sol @@ -0,0 +1,35 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +import {BuildScript} from "../../src/abstract/BuildScript.sol"; +import {BuildHarness} from "./BuildHarness.sol"; + +/// @title BuildRecordRootHarness +/// @notice `Build` with its record root overridden — the one thing +/// `BuildScript` documents the root as being overridable for — and the +/// regeneration `cutRelease()` freezes from reachable from a test. +/// +/// The regeneration alone. `run()` and `cutRelease()` also regenerate the libs, +/// and those are written into `LIB_DIR`, which no override moves, so either +/// entry point would rewrite committed libs that other test contracts read +/// while forge runs them in parallel. +contract BuildRecordRootHarness is BuildHarness { + /// The record root this harness freezes into and regenerates under. + string internal sRoot; + + /// @param root The fixture record root. + constructor(string memory root) { + sRoot = root; + } + + /// @inheritdoc BuildScript + function recordRoot() internal view override returns (string memory) { + return sRoot; + } + + /// Runs the hook `cutRelease()` regenerates through. + function externalRegenerateSnapshots() external { + regenerateSnapshots(); + } +} diff --git a/test/script/Build.t.sol b/test/script/Build.t.sol index 8d0e92f..eb30bbf 100644 --- a/test/script/Build.t.sol +++ b/test/script/Build.t.sol @@ -9,6 +9,7 @@ import {LibCodeGen} from "rain-sol-codegen-0.1.37/src/lib/LibCodeGen.sol"; import {LibRainDeploySnapshot} from "../../src/lib/LibRainDeploySnapshot.sol"; import {LibReleasedSuitesAggregate} from "../lib/LibReleasedSuitesAggregate.sol"; import {BuildHarness} from "../concrete/BuildHarness.sol"; +import {BuildRecordRootHarness} from "../concrete/BuildRecordRootHarness.sol"; import {LibMemoryKV, MemoryKV, MemoryKVKey, MemoryKVVal} from "rain-lib-memkv-0.1.4/src/lib/LibMemoryKV.sol"; /// @title BuildTest @@ -313,3 +314,75 @@ contract BuildTest is Test { } } } + +/// @title BuildRecordRootTest +/// @notice Where `script/Build.sol` writes when `recordRoot()` is overridden — +/// the one thing `BuildScript` documents the root as being overridable for. +/// +/// A contract of its own because this one WRITES, and `BuildTest` states that +/// nothing in it does. What is asserted here is which tree the write lands in, +/// so the write is the subject rather than a side effect: it goes under this +/// contract's own fixture root, and the committed record it would otherwise +/// have gone into is read and left alone. +contract BuildRecordRootTest is Test { + /// The fixture record root the regeneration is pointed at. + string constant FIXTURE_ROOT = "test/generated-build-record-root"; + + /// Clears a fixture record an earlier failure left behind. A cheatcode + /// write is not undone by a revert, so a failure leaves generated sources + /// on disk and the next run reads THOSE. + /// @param root The fixture record root to clear. + function resetFixture(string memory root) internal { + if (vm.exists(root)) { + //forge-lint: disable-next-line(unsafe-cheatcode) + vm.removeDir(root, true); + } + } + + /// PROPERTY: `Build`'s regeneration writes every rolling snapshot under + /// `recordRoot()`. + /// + /// `cutRelease()` freezes each contract from `pathForSnapshot(recordRoot(), + /// CANDIDATE, name)`, so a regeneration that ignores the root hands the + /// freeze a record nothing wrote — `NothingToFreeze` for any repo that + /// overrides the root — and rewrites the real `src/generated/candidate/` on + /// the way to that revert, which is the tree the override exists to keep a + /// caller's hands off. + /// + /// The bytes are the committed candidate's, read from the real record: the + /// regeneration is a function of what this repo compiles and the committed + /// snapshot is what it last compiled to, so an equal file under the fixture + /// root is the whole snapshot having moved rather than a file having been + /// created there. + function testRegenerateSnapshotsWritesUnderTheRecordRoot() external { + resetFixture(FIXTURE_ROOT); + BuildRecordRootHarness harness = new BuildRecordRootHarness(FIXTURE_ROOT); + string[] memory names = harness.externalSnapshotContractNames(); + + harness.externalRegenerateSnapshots(); + + // Read while the fixture is still there, asserted once it is gone. + bool[] memory written = new bool[](names.length); + string[] memory regenerated = new string[](names.length); + string[] memory committed = new string[](names.length); + for (uint256 i = 0; i < names.length; i++) { + string memory path = + LibRainDeploySnapshot.pathForSnapshot(FIXTURE_ROOT, LibRainDeploySnapshot.CANDIDATE, names[i]); + written[i] = vm.exists(path); + regenerated[i] = written[i] ? vm.readFile(path) : ""; + committed[i] = vm.readFile(LibRainDeploySnapshot.pathForSnapshot(LibRainDeploySnapshot.CANDIDATE, names[i])); + } + + resetFixture(FIXTURE_ROOT); + + assertTrue(names.length > 0, "no contract is generated, so nothing was asserted"); + for (uint256 i = 0; i < names.length; i++) { + assertTrue(written[i], string.concat("regeneration wrote nothing under the record root: ", names[i])); + assertEq( + regenerated[i], + committed[i], + string.concat("snapshot under the record root is not the committed one: ", names[i]) + ); + } + } +} diff --git a/test/src/lib/LibRainDeploySnapshot.t.sol b/test/src/lib/LibRainDeploySnapshot.t.sol index 8d4b725..f4ceea2 100644 --- a/test/src/lib/LibRainDeploySnapshot.t.sol +++ b/test/src/lib/LibRainDeploySnapshot.t.sol @@ -496,6 +496,7 @@ contract LibRainDeploySnapshotTest is Test { string memory written = LibRainDeploySnapshot.writeSnapshot( vm, + LibRainDeploySnapshot.LIB_FS_ROOT, dir, "MockDeployable", RAIN_SPDX_LICENSE_IDENTIFIER, @@ -513,6 +514,83 @@ contract LibRainDeploySnapshotTest is Test { assertTrue(exists); } + /// The fixture record root the rooted write is pointed at. Under `test/`, + /// which nothing walks for releases. + string constant ROOTED_FIXTURE_ROOT = "test/generated-write-snapshot-root"; + + /// The directory the rooted write writes into, under the fixture root and + /// nowhere else. Not tag shaped, for the reason + /// `testWriteSnapshotWritesTheSnapshotAtItsPath` gives, and drawn from the + /// tag alphabet because the writer places files only in directories whose + /// names are. + string constant ROOTED_FIXTURE_DIR = "writeSnapshotRootedNotATag"; + + /// The directory the same snapshot is written into at the REAL root, to + /// compare the bytes against. A second name so that nothing this test wrote + /// can stand in for what the rooted write did. + string constant ROOTED_FIXTURE_REAL_DIR = "writeSnapshotRootedRealNotATag"; + + /// PROPERTY: a snapshot is generated under the record root it is HANDED, + /// and the real record is not written on the way there. + /// + /// `freeze` reads every rolling snapshot at `pathForSnapshot(root, + /// CANDIDATE, name)`, so a generator that wrote under `LIB_FS_ROOT` + /// whatever root it was handed would leave a release cut under any other + /// root frozen from a record its own regeneration never wrote — after + /// rewriting the append-only tree the other root exists to keep clear. + /// + /// The bytes are the real root's for the same inputs: the root selects the + /// PATH and nothing else, so a fixture record holds the layout the real + /// record holds rather than one only a test can be pointed at. State is + /// reverted between the two writes for the reason + /// `testWriteSnapshotDefaultsToTheOrgHeader` gives. + function testWriteSnapshotWritesUnderTheRootItIsHanded() external { + uint256 undeployed = vm.snapshotState(); + string memory atRealRoot = vm.readFile( + LibRainDeploySnapshot.writeSnapshot( + vm, + LibRainDeploySnapshot.LIB_FS_ROOT, + ROOTED_FIXTURE_REAL_DIR, + FIXTURE_CONTRACT, + type(MockDeployable).creationCode, + new address[](0) + ) + ); + vm.revertToState(undeployed); + + string memory written = LibRainDeploySnapshot.writeSnapshot( + vm, + ROOTED_FIXTURE_ROOT, + ROOTED_FIXTURE_DIR, + FIXTURE_CONTRACT, + type(MockDeployable).creationCode, + new address[](0) + ); + + // Read while the fixtures are still there, asserted once they are gone. + bool rooted = vm.exists(written); + string memory atFixtureRoot = rooted ? vm.readFile(written) : ""; + bool inTheRealRecord = vm.exists(LibRainDeploySnapshot.pathForSnapshot(ROOTED_FIXTURE_DIR, FIXTURE_CONTRACT)); + + //forge-lint: disable-next-line(unsafe-cheatcode) + vm.removeDir(LibRainDeploySnapshot.dirForSnapshot(ROOTED_FIXTURE_REAL_DIR), true); + if (vm.exists(LibRainDeploySnapshot.dirForSnapshot(ROOTED_FIXTURE_DIR))) { + //forge-lint: disable-next-line(unsafe-cheatcode) + vm.removeDir(LibRainDeploySnapshot.dirForSnapshot(ROOTED_FIXTURE_DIR), true); + } + if (vm.exists(ROOTED_FIXTURE_ROOT)) { + //forge-lint: disable-next-line(unsafe-cheatcode) + vm.removeDir(ROOTED_FIXTURE_ROOT, true); + } + + assertEq( + written, LibRainDeploySnapshot.pathForSnapshot(ROOTED_FIXTURE_ROOT, ROOTED_FIXTURE_DIR, FIXTURE_CONTRACT) + ); + assertTrue(rooted, "nothing was written under the record root"); + assertFalse(inTheRealRecord, "the real record was written under the record root's name"); + assertEq(atFixtureRoot, atRealRoot); + } + /// The directory the licence-header fixture snapshot is written into. Not /// tag shaped, for the reason `testWriteSnapshotWritesTheSnapshotAtItsPath` /// gives, and drawn from the tag alphabet because the writer places files @@ -555,6 +633,7 @@ contract LibRainDeploySnapshotTest is Test { string memory source = vm.readFile( LibRainDeploySnapshot.writeSnapshot( vm, + LibRainDeploySnapshot.LIB_FS_ROOT, HEADER_FIXTURE_DIR, FIXTURE_CONTRACT, RAIN_SPDX_LICENSE_IDENTIFIER, @@ -639,7 +718,12 @@ contract LibRainDeploySnapshotTest is Test { string memory source = vm.readFile( LibRainDeploySnapshot.writeSnapshot( - vm, RECORD_FIXTURE_DIR, FIXTURE_CONTRACT, creationCode, new address[](0) + vm, + LibRainDeploySnapshot.LIB_FS_ROOT, + RECORD_FIXTURE_DIR, + FIXTURE_CONTRACT, + creationCode, + new address[](0) ) ); //forge-lint: disable-next-line(unsafe-cheatcode) @@ -673,7 +757,12 @@ contract LibRainDeploySnapshotTest is Test { function testWriteSnapshotDeclaresTheDeployConstantsInOrder() external { string memory source = vm.readFile( LibRainDeploySnapshot.writeSnapshot( - vm, ORDER_FIXTURE_DIR, FIXTURE_CONTRACT, type(MockDeployable).creationCode, new address[](0) + vm, + LibRainDeploySnapshot.LIB_FS_ROOT, + ORDER_FIXTURE_DIR, + FIXTURE_CONTRACT, + type(MockDeployable).creationCode, + new address[](0) ) ); @@ -1247,6 +1336,7 @@ contract LibRainDeploySnapshotTest is Test { string memory source = vm.readFile( LibRainDeploySnapshot.writeSnapshot( vm, + LibRainDeploySnapshot.LIB_FS_ROOT, DEPENDENCIES_FIXTURE_DIR, FIXTURE_CONTRACT, RAIN_SPDX_LICENSE_IDENTIFIER, @@ -1366,6 +1456,7 @@ contract LibRainDeploySnapshotTest is Test { string memory source = vm.readFile( LibRainDeploySnapshot.writeSnapshot( vm, + LibRainDeploySnapshot.LIB_FS_ROOT, CONSENSUS_FIXTURE_DIR, FIXTURE_CONTRACT, RAIN_SPDX_LICENSE_IDENTIFIER, @@ -2718,13 +2809,19 @@ contract LibRainDeploySnapshotTest is Test { uint256 undeployed = vm.snapshotState(); string memory defaulted = vm.readFile( LibRainDeploySnapshot.writeSnapshot( - vm, dir, "MockDeployable", type(MockDeployable).creationCode, new address[](0) + vm, + LibRainDeploySnapshot.LIB_FS_ROOT, + dir, + "MockDeployable", + type(MockDeployable).creationCode, + new address[](0) ) ); vm.revertToState(undeployed); string memory explicitly = vm.readFile( LibRainDeploySnapshot.writeSnapshot( vm, + LibRainDeploySnapshot.LIB_FS_ROOT, dir, "MockDeployable", RAIN_SPDX_LICENSE_IDENTIFIER, From 324c57b8342396a93d2dc24179e7565be81201d8 Mon Sep 17 00:00:00 2001 From: baku-ccron Date: Tue, 15 Sep 2026 16:02:33 +0000 Subject: [PATCH 2/6] Split BuildRecordRootTest into its own file Rain convention is one contract per file; static caught the second contract in Build.t.sol. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01V8ViHcKLVk2YoS2joH4HdN --- test/script/Build.t.sol | 73 ---------------------------- test/script/BuildRecordRoot.t.sol | 79 +++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 73 deletions(-) create mode 100644 test/script/BuildRecordRoot.t.sol diff --git a/test/script/Build.t.sol b/test/script/Build.t.sol index eb30bbf..8d0e92f 100644 --- a/test/script/Build.t.sol +++ b/test/script/Build.t.sol @@ -9,7 +9,6 @@ import {LibCodeGen} from "rain-sol-codegen-0.1.37/src/lib/LibCodeGen.sol"; import {LibRainDeploySnapshot} from "../../src/lib/LibRainDeploySnapshot.sol"; import {LibReleasedSuitesAggregate} from "../lib/LibReleasedSuitesAggregate.sol"; import {BuildHarness} from "../concrete/BuildHarness.sol"; -import {BuildRecordRootHarness} from "../concrete/BuildRecordRootHarness.sol"; import {LibMemoryKV, MemoryKV, MemoryKVKey, MemoryKVVal} from "rain-lib-memkv-0.1.4/src/lib/LibMemoryKV.sol"; /// @title BuildTest @@ -314,75 +313,3 @@ contract BuildTest is Test { } } } - -/// @title BuildRecordRootTest -/// @notice Where `script/Build.sol` writes when `recordRoot()` is overridden — -/// the one thing `BuildScript` documents the root as being overridable for. -/// -/// A contract of its own because this one WRITES, and `BuildTest` states that -/// nothing in it does. What is asserted here is which tree the write lands in, -/// so the write is the subject rather than a side effect: it goes under this -/// contract's own fixture root, and the committed record it would otherwise -/// have gone into is read and left alone. -contract BuildRecordRootTest is Test { - /// The fixture record root the regeneration is pointed at. - string constant FIXTURE_ROOT = "test/generated-build-record-root"; - - /// Clears a fixture record an earlier failure left behind. A cheatcode - /// write is not undone by a revert, so a failure leaves generated sources - /// on disk and the next run reads THOSE. - /// @param root The fixture record root to clear. - function resetFixture(string memory root) internal { - if (vm.exists(root)) { - //forge-lint: disable-next-line(unsafe-cheatcode) - vm.removeDir(root, true); - } - } - - /// PROPERTY: `Build`'s regeneration writes every rolling snapshot under - /// `recordRoot()`. - /// - /// `cutRelease()` freezes each contract from `pathForSnapshot(recordRoot(), - /// CANDIDATE, name)`, so a regeneration that ignores the root hands the - /// freeze a record nothing wrote — `NothingToFreeze` for any repo that - /// overrides the root — and rewrites the real `src/generated/candidate/` on - /// the way to that revert, which is the tree the override exists to keep a - /// caller's hands off. - /// - /// The bytes are the committed candidate's, read from the real record: the - /// regeneration is a function of what this repo compiles and the committed - /// snapshot is what it last compiled to, so an equal file under the fixture - /// root is the whole snapshot having moved rather than a file having been - /// created there. - function testRegenerateSnapshotsWritesUnderTheRecordRoot() external { - resetFixture(FIXTURE_ROOT); - BuildRecordRootHarness harness = new BuildRecordRootHarness(FIXTURE_ROOT); - string[] memory names = harness.externalSnapshotContractNames(); - - harness.externalRegenerateSnapshots(); - - // Read while the fixture is still there, asserted once it is gone. - bool[] memory written = new bool[](names.length); - string[] memory regenerated = new string[](names.length); - string[] memory committed = new string[](names.length); - for (uint256 i = 0; i < names.length; i++) { - string memory path = - LibRainDeploySnapshot.pathForSnapshot(FIXTURE_ROOT, LibRainDeploySnapshot.CANDIDATE, names[i]); - written[i] = vm.exists(path); - regenerated[i] = written[i] ? vm.readFile(path) : ""; - committed[i] = vm.readFile(LibRainDeploySnapshot.pathForSnapshot(LibRainDeploySnapshot.CANDIDATE, names[i])); - } - - resetFixture(FIXTURE_ROOT); - - assertTrue(names.length > 0, "no contract is generated, so nothing was asserted"); - for (uint256 i = 0; i < names.length; i++) { - assertTrue(written[i], string.concat("regeneration wrote nothing under the record root: ", names[i])); - assertEq( - regenerated[i], - committed[i], - string.concat("snapshot under the record root is not the committed one: ", names[i]) - ); - } - } -} diff --git a/test/script/BuildRecordRoot.t.sol b/test/script/BuildRecordRoot.t.sol new file mode 100644 index 0000000..a9a81cc --- /dev/null +++ b/test/script/BuildRecordRoot.t.sol @@ -0,0 +1,79 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +import {Test} from "forge-std-1.16.2/src/Test.sol"; +import {LibRainDeploySnapshot} from "../../src/lib/LibRainDeploySnapshot.sol"; +import {BuildRecordRootHarness} from "../concrete/BuildRecordRootHarness.sol"; + +/// @title BuildRecordRootTest +/// @notice Where `script/Build.sol` writes when `recordRoot()` is overridden — +/// the one thing `BuildScript` documents the root as being overridable for. +/// +/// A contract of its own because this one WRITES, and `BuildTest` states that +/// nothing in it does. What is asserted here is which tree the write lands in, +/// so the write is the subject rather than a side effect: it goes under this +/// contract's own fixture root, and the committed record it would otherwise +/// have gone into is read and left alone. +contract BuildRecordRootTest is Test { + /// The fixture record root the regeneration is pointed at. + string constant FIXTURE_ROOT = "test/generated-build-record-root"; + + /// Clears a fixture record an earlier failure left behind. A cheatcode + /// write is not undone by a revert, so a failure leaves generated sources + /// on disk and the next run reads THOSE. + /// @param root The fixture record root to clear. + function resetFixture(string memory root) internal { + if (vm.exists(root)) { + //forge-lint: disable-next-line(unsafe-cheatcode) + vm.removeDir(root, true); + } + } + + /// PROPERTY: `Build`'s regeneration writes every rolling snapshot under + /// `recordRoot()`. + /// + /// `cutRelease()` freezes each contract from `pathForSnapshot(recordRoot(), + /// CANDIDATE, name)`, so a regeneration that ignores the root hands the + /// freeze a record nothing wrote — `NothingToFreeze` for any repo that + /// overrides the root — and rewrites the real `src/generated/candidate/` on + /// the way to that revert, which is the tree the override exists to keep a + /// caller's hands off. + /// + /// The bytes are the committed candidate's, read from the real record: the + /// regeneration is a function of what this repo compiles and the committed + /// snapshot is what it last compiled to, so an equal file under the fixture + /// root is the whole snapshot having moved rather than a file having been + /// created there. + function testRegenerateSnapshotsWritesUnderTheRecordRoot() external { + resetFixture(FIXTURE_ROOT); + BuildRecordRootHarness harness = new BuildRecordRootHarness(FIXTURE_ROOT); + string[] memory names = harness.externalSnapshotContractNames(); + + harness.externalRegenerateSnapshots(); + + // Read while the fixture is still there, asserted once it is gone. + bool[] memory written = new bool[](names.length); + string[] memory regenerated = new string[](names.length); + string[] memory committed = new string[](names.length); + for (uint256 i = 0; i < names.length; i++) { + string memory path = + LibRainDeploySnapshot.pathForSnapshot(FIXTURE_ROOT, LibRainDeploySnapshot.CANDIDATE, names[i]); + written[i] = vm.exists(path); + regenerated[i] = written[i] ? vm.readFile(path) : ""; + committed[i] = vm.readFile(LibRainDeploySnapshot.pathForSnapshot(LibRainDeploySnapshot.CANDIDATE, names[i])); + } + + resetFixture(FIXTURE_ROOT); + + assertTrue(names.length > 0, "no contract is generated, so nothing was asserted"); + for (uint256 i = 0; i < names.length; i++) { + assertTrue(written[i], string.concat("regeneration wrote nothing under the record root: ", names[i])); + assertEq( + regenerated[i], + committed[i], + string.concat("snapshot under the record root is not the committed one: ", names[i]) + ); + } + } +} From fafe38ceec8ed1faf318e517bf2b346a7163fe74 Mon Sep 17 00:00:00 2001 From: baku-ccron Date: Tue, 15 Sep 2026 20:16:15 +0000 Subject: [PATCH 3/6] Check the record root the way the snapshot directory is checked `dirForSnapshot(root, dir)` held `dir` to `LibFs`'s tag alphabet and concatenated `root` as given, so the half of the path that decides where the tree IS was the only half nothing checked. `requireRecordRoot` holds it to that alphabet plus `-`, segment by segment, which is what this repo's own roots are spelled in. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01V8ViHcKLVk2YoS2joH4HdN --- src/lib/LibRainDeploySnapshot.sol | 82 +++++++++++- test/src/lib/LibRainDeploySnapshot.t.sol | 156 +++++++++++++++++++++++ 2 files changed, 235 insertions(+), 3 deletions(-) diff --git a/src/lib/LibRainDeploySnapshot.sol b/src/lib/LibRainDeploySnapshot.sol index a623fdb..b995f4c 100644 --- a/src/lib/LibRainDeploySnapshot.sol +++ b/src/lib/LibRainDeploySnapshot.sol @@ -49,6 +49,15 @@ error EmptyRelease(string tag); /// @param newestFrozenTag The newest tag already in the record. error NonMonotonicRelease(string tag, string newestFrozenTag); +/// Thrown when a record root is not a path this library can place a record at. +/// A root is interpolated into every snapshot path a caller hands it, so one +/// that climbs out of the tree, starts at `/` or is empty makes the paths this +/// library returns paths to somewhere else entirely — and `fs_permissions`, a +/// consuming repo's config rather than this library's argument, the only thing +/// standing between a generated file and an arbitrary location on disk. +/// @param root The rejected root. +error InvalidRecordRoot(string root); + /// @title LibRainDeploySnapshot /// @notice Which release is being built, where its record lives, and how it is /// frozen. Release machinery, not code generation. @@ -196,6 +205,51 @@ library LibRainDeploySnapshot { /// assertion was standing in for. string constant LIB_FS_ROOT = GENERATED_DIR; + /// Reverts unless `root` is a path of record root segments: at least one + /// segment, separated by single `/`, each of them at least one character + /// and every character an ASCII letter, a digit, `_`, `$` or `-`. + /// + /// That is `LibFs.requireTag`'s alphabet with `-` admitted as well, and a + /// root is held to it segment by segment for the reason `requireTag` states + /// of a tag: no character in the set is a path separator and none of them is + /// `.`, so no segment is `.` or `..` and none reaches past the single + /// directory it names. Refusing the empty segment is what carries that from + /// a segment to a path — it takes the leading `/` of an absolute path, the + /// trailing one, the doubled one, and the empty root itself. + /// + /// `requireTag` cannot be asked this, because the separators that make a + /// path a path are exactly what it refuses; the alphabet BETWEEN them is a + /// copy of its, held to it byte for byte by + /// `testRecordRootSegmentIsTheTagAlphabetPlusHyphen`. The `-` is the whole + /// of the widening and it is what this repo's own roots need: `src/generated` + /// is tag segments already, while every fixture root the tests build is + /// `test/generated-` or `test/fixture-record`. + /// @param root The record root to check. + function requireRecordRoot(string memory root) internal pure { + bytes memory rootBytes = bytes(root); + uint256 segmentLength = 0; + for (uint256 i = 0; i < rootBytes.length; i++) { + bytes1 char = rootBytes[i]; + if (char == "/") { + if (segmentLength == 0) { + revert InvalidRecordRoot(root); + } + segmentLength = 0; + continue; + } + bool isLetter = (char >= 0x41 && char <= 0x5A) || (char >= 0x61 && char <= 0x7A); + bool isDigit = char >= 0x30 && char <= 0x39; + bool isUnderscoreOrDollar = char == 0x5F || char == 0x24; + if (!(isLetter || isDigit || isUnderscoreOrDollar || char == "-")) { + revert InvalidRecordRoot(root); + } + segmentLength++; + } + if (segmentLength == 0) { + revert InvalidRecordRoot(root); + } + } + /// The directory holding a snapshot, rolling or frozen, under a record /// root. /// @@ -204,11 +258,18 @@ library LibRainDeploySnapshot { /// that admitted a name the writer refuses is a reader pointed at a path /// nothing can ever have written, and a fixture record that admitted one /// would be a fixture of a layout the real record cannot hold. + /// + /// The root is checked here too, and this is where it has to be: it is the + /// one place the root becomes a path, and the two halves of that path are + /// concatenated caller input. A checked `dir` beside an unchecked root is + /// only the shorter half of the path confined. /// @param root The record root — `LIB_FS_ROOT` for a repo's real record. + /// MUST be a path of record root segments. /// @param dir The snapshot directory name — a release tag, or `CANDIDATE`. /// MUST be drawn from `LibFs`'s tag alphabet. /// @return The directory path. function dirForSnapshot(string memory root, string memory dir) internal pure returns (string memory) { + requireRecordRoot(root); LibFs.requireTag(dir); return string.concat(root, "/", dir); } @@ -242,6 +303,7 @@ library LibRainDeploySnapshot { /// as on how they are spelled: a reader that accepted what the writer /// refuses is the same divergence one step quieter. /// @param root The record root — `LIB_FS_ROOT` for a repo's real record. + /// MUST be a path of record root segments. /// @param dir The snapshot directory name — a release tag, or `CANDIDATE`. /// MUST be drawn from `LibFs`'s tag alphabet. /// @param contractName The name of the contract. MUST be a Solidity @@ -335,10 +397,17 @@ library LibRainDeploySnapshot { /// - the entry is a file directly inside it. Everything in a release /// directory belongs to that release's record — there is no extension to /// filter on, because nothing else has any business being in there. + /// The root is checked before the walk, because a root nothing can be + /// written under is not a record that happens to be empty. This is the one + /// root-taking entry point that does not reach `dirForSnapshot`, so the two + /// together are every way a root gets into this library. /// @param vm The Vm instance for file operations. /// @param root The record root — `LIB_FS_ROOT` for a repo's real record. + /// MUST be a path of record root segments. /// @return Every frozen record file. function frozenSnapshotPaths(Vm vm, string memory root) internal view returns (string[] memory) { + requireRecordRoot(root); + // A repo with no generated directory at all has released nothing. That // is a real state — it is this repo's own, before its first release — // rather than a missing file to fail on. @@ -432,8 +501,14 @@ library LibRainDeploySnapshot { /// caller's hands off. /// /// `LibFs.buildFileForContract` takes the directory it writes into, so the - /// root reaches the writer through `dirForSnapshot(root, dir)` and `dir` is - /// still held to the tag alphabet by it. At `LIB_FS_ROOT` that directory is + /// root reaches the writer through `dirForSnapshot(root, dir)`, which is + /// where both halves of the directory are checked: `dir` against `LibFs`'s + /// tag alphabet and the root against `requireRecordRoot`. The root is a + /// caller's string and it is the half that names where the tree IS, so an + /// unchecked one would make the output directory of every write here the + /// caller's to place anywhere `fs_permissions` allows — which is a + /// consuming repo's config, not an argument this library gets to see. At + /// `LIB_FS_ROOT` that directory is /// `LibFs.dirForTag(dir)`, which is where /// `testRootAwareSnapshotPathIsTheWritersAtTheRealRoot` holds the two /// spellings to being one path. @@ -456,7 +531,7 @@ library LibRainDeploySnapshot { /// inside it call the one that defaults to the org's values. /// @param vm The Vm instance for file operations. /// @param root The record root to generate into — `LIB_FS_ROOT` for a - /// repo's real record. + /// repo's real record. MUST be a path of record root segments. /// @param dir The snapshot directory name — a release tag, or `CANDIDATE`. /// @param contractName The contract the snapshot describes. /// @param spdxLicenseIdentifier The SPDX licence identifier the written @@ -495,6 +570,7 @@ library LibRainDeploySnapshot { /// `RAIN_COPYRIGHT_TEXT`, for a repo this org owns. /// @param vm The Vm instance for file operations. /// @param root The record root — `LIB_FS_ROOT` for a repo's real record. + /// MUST be a path of record root segments. /// @param dir The snapshot directory name — a release tag, or `CANDIDATE`. /// @param contractName The contract the snapshot describes. /// @param creationCode That contract's creation code. diff --git a/test/src/lib/LibRainDeploySnapshot.t.sol b/test/src/lib/LibRainDeploySnapshot.t.sol index f4ceea2..9f5241b 100644 --- a/test/src/lib/LibRainDeploySnapshot.t.sol +++ b/test/src/lib/LibRainDeploySnapshot.t.sol @@ -9,9 +9,11 @@ import { RAIN_COPYRIGHT_TEXT, RAIN_SPDX_LICENSE_IDENTIFIER } from "rain-sol-codegen-0.1.37/src/lib/LibCodeGen.sol"; +import {LibFs} from "rain-sol-codegen-0.1.37/src/lib/LibFs.sol"; import {DeploySuite} from "../../../src/abstract/RainDeploySuitesBase.sol"; import { EmptyRelease, + InvalidRecordRoot, LibRainDeploySnapshot, NonMonotonicRelease, NothingToFreeze, @@ -591,6 +593,160 @@ contract LibRainDeploySnapshotTest is Test { assertEq(atFixtureRoot, atRealRoot); } + /// The directory the escaping write is pointed at, under whatever root it + /// is handed. Not tag shaped, for the reason + /// `testWriteSnapshotWritesTheSnapshotAtItsPath` gives. + string constant ESCAPE_FIXTURE_DIR = "writeSnapshotEscapeNotATag"; + + /// A record root spelled as a path that leaves the tree it names: two + /// segments under `test/`, then back out of both and into the REAL record. + /// + /// The real record is where it is pointed deliberately. It is the tree + /// `BuildScript.recordRoot` is overridable to keep a caller's hands off, it + /// is append-only, and `fs_permissions` grants `./src` — so a root that + /// reaches it is inside everything the config can refuse and is exactly the + /// write nothing outside this library is left to catch. + string constant ESCAPE_FIXTURE_ROOT = "test/generated-escape-root/../../src/generated"; + + /// The directory `ESCAPE_FIXTURE_ROOT`'s first segment names, created on the + /// way through by a recursive create and removed with the rest. + string constant ESCAPE_FIXTURE_CLIMB_DIR = "test/generated-escape-root"; + + /// External wrapper so a refusal is a failed call rather than a reverted + /// test, for the write that is not supposed to happen at all. + /// @param root The record root to generate into. + /// @param dir The snapshot directory name. + function externalWriteSnapshotAt(string memory root, string memory dir) external { + LibRainDeploySnapshot.writeSnapshot( + vm, root, dir, FIXTURE_CONTRACT, type(MockDeployable).creationCode, new address[](0) + ); + } + + /// PROPERTY: a root that resolves to somewhere other than the tree it names + /// is refused, and nothing is written. + /// + /// A root is concatenated with a directory and a contract name, both of + /// which are checked, and the root is the half that decides where the tree + /// IS. Unchecked, `..` in it walks the write out of the root the caller + /// named and into one it did not — here the append-only record — and the + /// only thing that would have stood between the two is `fs_permissions`, + /// which is a consuming repo's config rather than an argument of this + /// library's and which grants the record's own tree. + /// + /// The landing path is the WRITER's own spelling at the real root rather + /// than a literal, so what is asserted absent is the same path a real + /// generation into the record would produce. + function testWriteSnapshotRefusesARootThatClimbsOutOfTheTreeItNames() external { + string memory landing = LibRainDeploySnapshot.pathForSnapshot(ESCAPE_FIXTURE_DIR, FIXTURE_CONTRACT); + + (bool accepted,) = + address(this).call(abi.encodeCall(this.externalWriteSnapshotAt, (ESCAPE_FIXTURE_ROOT, ESCAPE_FIXTURE_DIR))); + + // Read while any residue is still there, asserted once it is gone. + bool landedInTheRecord = vm.exists(landing); + if (vm.exists(LibRainDeploySnapshot.dirForSnapshot(ESCAPE_FIXTURE_DIR))) { + //forge-lint: disable-next-line(unsafe-cheatcode) + vm.removeDir(LibRainDeploySnapshot.dirForSnapshot(ESCAPE_FIXTURE_DIR), true); + } + if (vm.exists(ESCAPE_FIXTURE_CLIMB_DIR)) { + //forge-lint: disable-next-line(unsafe-cheatcode) + vm.removeDir(ESCAPE_FIXTURE_CLIMB_DIR, true); + } + + assertFalse(landedInTheRecord, "the write landed in the real record, outside the root it was handed"); + assertFalse(accepted, "a root that resolves outside the tree it names was accepted"); + } + + /// External wrapper so a refusal is a failed call rather than a reverted + /// test, and so the root rule can be asked about a root on its own. + /// @param root The record root to check. + function externalRequireRecordRoot(string memory root) external pure { + LibRainDeploySnapshot.requireRecordRoot(root); + } + + /// External wrapper for `LibFs`'s own tag rule, the counterpart to + /// `externalRequireRecordRoot`. + /// @param tag The tag to check. + function externalRequireTag(string memory tag) external pure { + LibFs.requireTag(tag); + } + + /// PROPERTY: every root that is not a path of record root segments is + /// refused, and the refusal names the root. + /// + /// The cases are the shapes a root can take that a concatenation cannot + /// survive, each of which puts the written file somewhere other than under + /// the root the caller named: climbing out of the tree, naming the tree's + /// own parent, starting at the filesystem root, ending in a separator so + /// the next one doubles, doubling one already, and carrying nothing at all. + /// A character outside the alphabet is last, because it is the one that is + /// not about separators. + function testRecordRootRefusesEveryRootThatIsNotOne() external { + string[8] memory bad = [ + "../src/generated", + "src/generated/..", + "..", + "/src/generated", + "src/generated/", + "src//generated", + "", + "src/gen erated" + ]; + + for (uint256 i = 0; i < bad.length; i++) { + vm.expectRevert(abi.encodeWithSelector(InvalidRecordRoot.selector, bad[i])); + this.externalRequireRecordRoot(bad[i]); + } + } + + /// PROPERTY: a root of ONE segment is accepted exactly when `LibFs` accepts + /// that segment as a tag, `-` alone excepted. + /// + /// The root rule cannot be asked of `LibFs.requireTag` — a root is a path + /// and `requireTag` refuses the separators that make it one — so the + /// alphabet between the separators is a copy of `LibFs`'s, and this is where + /// the copy is held to the original. Exhaustive over all 256 byte values + /// rather than fuzzed, because the alphabet is a property of every byte and + /// a copy that has drifted by one of them is a copy that has drifted. + /// + /// `-` is the whole of the widening and it is asserted as such: the two + /// rules are required to disagree about it, so dropping it from the root + /// alphabet fails here as loudly as widening the root alphabet further + /// does. + function testRecordRootSegmentIsTheTagAlphabetPlusHyphen() external view { + for (uint256 i = 0; i < 256; i++) { + bytes memory segmentBytes = new bytes(1); + // forge-lint: disable-next-line(unsafe-typecast) + segmentBytes[0] = bytes1(uint8(i)); + string memory segment = string(segmentBytes); + + (bool rootOk,) = address(this).staticcall(abi.encodeCall(this.externalRequireRecordRoot, (segment))); + (bool tagOk,) = address(this).staticcall(abi.encodeCall(this.externalRequireTag, (segment))); + + assertEq(rootOk, tagOk || segmentBytes[0] == "-", segment); + } + } + + /// External wrapper so a refusal is a failed call rather than a reverted + /// test, for the record walk. + /// @param root The record root to walk. + function externalFrozenSnapshotPaths(string memory root) external view { + LibRainDeploySnapshot.frozenSnapshotPaths(vm, root); + } + + /// PROPERTY: the record WALK holds a root to the same rule the writers do. + /// + /// It is the one root-taking entry point that does not reach + /// `dirForSnapshot`, and it answers a missing root with an empty record — + /// which is a real state for a repo that has released nothing, and silence + /// for a root nothing could ever have been written under. The same root is + /// refused by both, so a reader cannot be pointed somewhere a writer would + /// not go. + function testFrozenSnapshotPathsRefusesARootThatIsNotOne() external { + vm.expectRevert(abi.encodeWithSelector(InvalidRecordRoot.selector, ESCAPE_FIXTURE_ROOT)); + this.externalFrozenSnapshotPaths(ESCAPE_FIXTURE_ROOT); + } + /// The directory the licence-header fixture snapshot is written into. Not /// tag shaped, for the reason `testWriteSnapshotWritesTheSnapshotAtItsPath` /// gives, and drawn from the tag alphabet because the writer places files From cff2b9b7f77332cc9dd0265d674018a3759e7bd9 Mon Sep 17 00:00:00 2001 From: baku-ccron Date: Tue, 15 Sep 2026 20:22:05 +0000 Subject: [PATCH 4/6] Name the byte in hex when the two alphabets disagree A raw byte as the assertion message is unreadable for exactly the bytes the test is about. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01V8ViHcKLVk2YoS2joH4HdN --- test/src/lib/LibRainDeploySnapshot.t.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/src/lib/LibRainDeploySnapshot.t.sol b/test/src/lib/LibRainDeploySnapshot.t.sol index 9f5241b..1f778a3 100644 --- a/test/src/lib/LibRainDeploySnapshot.t.sol +++ b/test/src/lib/LibRainDeploySnapshot.t.sol @@ -723,7 +723,7 @@ contract LibRainDeploySnapshotTest is Test { (bool rootOk,) = address(this).staticcall(abi.encodeCall(this.externalRequireRecordRoot, (segment))); (bool tagOk,) = address(this).staticcall(abi.encodeCall(this.externalRequireTag, (segment))); - assertEq(rootOk, tagOk || segmentBytes[0] == "-", segment); + assertEq(rootOk, tagOk || segmentBytes[0] == "-", vm.toString(segmentBytes)); } } From 222ee7ba482d136045d9fa16070c6c2d8d5dd380 Mon Sep 17 00:00:00 2001 From: baku-ccron Date: Tue, 15 Sep 2026 22:52:06 +0000 Subject: [PATCH 5/6] Cut the comments this PR added Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01V8ViHcKLVk2YoS2joH4HdN --- src/lib/LibRainDeploySnapshot.sol | 54 ------------- test/concrete/BuildRecordRootHarness.sol | 12 --- test/script/BuildRecordRoot.t.sol | 32 +------- test/src/lib/LibRainDeploySnapshot.t.sol | 98 ------------------------ 4 files changed, 2 insertions(+), 194 deletions(-) diff --git a/src/lib/LibRainDeploySnapshot.sol b/src/lib/LibRainDeploySnapshot.sol index b995f4c..c87ab64 100644 --- a/src/lib/LibRainDeploySnapshot.sol +++ b/src/lib/LibRainDeploySnapshot.sol @@ -50,11 +50,6 @@ error EmptyRelease(string tag); error NonMonotonicRelease(string tag, string newestFrozenTag); /// Thrown when a record root is not a path this library can place a record at. -/// A root is interpolated into every snapshot path a caller hands it, so one -/// that climbs out of the tree, starts at `/` or is empty makes the paths this -/// library returns paths to somewhere else entirely — and `fs_permissions`, a -/// consuming repo's config rather than this library's argument, the only thing -/// standing between a generated file and an arbitrary location on disk. /// @param root The rejected root. error InvalidRecordRoot(string root); @@ -208,22 +203,6 @@ library LibRainDeploySnapshot { /// Reverts unless `root` is a path of record root segments: at least one /// segment, separated by single `/`, each of them at least one character /// and every character an ASCII letter, a digit, `_`, `$` or `-`. - /// - /// That is `LibFs.requireTag`'s alphabet with `-` admitted as well, and a - /// root is held to it segment by segment for the reason `requireTag` states - /// of a tag: no character in the set is a path separator and none of them is - /// `.`, so no segment is `.` or `..` and none reaches past the single - /// directory it names. Refusing the empty segment is what carries that from - /// a segment to a path — it takes the leading `/` of an absolute path, the - /// trailing one, the doubled one, and the empty root itself. - /// - /// `requireTag` cannot be asked this, because the separators that make a - /// path a path are exactly what it refuses; the alphabet BETWEEN them is a - /// copy of its, held to it byte for byte by - /// `testRecordRootSegmentIsTheTagAlphabetPlusHyphen`. The `-` is the whole - /// of the widening and it is what this repo's own roots need: `src/generated` - /// is tag segments already, while every fixture root the tests build is - /// `test/generated-` or `test/fixture-record`. /// @param root The record root to check. function requireRecordRoot(string memory root) internal pure { bytes memory rootBytes = bytes(root); @@ -258,11 +237,6 @@ library LibRainDeploySnapshot { /// that admitted a name the writer refuses is a reader pointed at a path /// nothing can ever have written, and a fixture record that admitted one /// would be a fixture of a layout the real record cannot hold. - /// - /// The root is checked here too, and this is where it has to be: it is the - /// one place the root becomes a path, and the two halves of that path are - /// concatenated caller input. A checked `dir` beside an unchecked root is - /// only the shorter half of the path confined. /// @param root The record root — `LIB_FS_ROOT` for a repo's real record. /// MUST be a path of record root segments. /// @param dir The snapshot directory name — a release tag, or `CANDIDATE`. @@ -397,10 +371,6 @@ library LibRainDeploySnapshot { /// - the entry is a file directly inside it. Everything in a release /// directory belongs to that release's record — there is no extension to /// filter on, because nothing else has any business being in there. - /// The root is checked before the walk, because a root nothing can be - /// written under is not a record that happens to be empty. This is the one - /// root-taking entry point that does not reach `dirForSnapshot`, so the two - /// together are every way a root gets into this library. /// @param vm The Vm instance for file operations. /// @param root The record root — `LIB_FS_ROOT` for a repo's real record. /// MUST be a path of record root segments. @@ -492,27 +462,6 @@ library LibRainDeploySnapshot { /// Generate one snapshot for one contract. /// - /// The output root is the one `freeze` is handed, and it is required for - /// the same reason: `cutRelease()` regenerates and then freezes within ONE - /// record tree. A generator that could only write under `LIB_FS_ROOT` would - /// leave a release cut under any other root frozen from a rolling snapshot - /// its own regeneration never wrote — after writing the real record on the - /// way there, which is the tree a `recordRoot()` override exists to keep a - /// caller's hands off. - /// - /// `LibFs.buildFileForContract` takes the directory it writes into, so the - /// root reaches the writer through `dirForSnapshot(root, dir)`, which is - /// where both halves of the directory are checked: `dir` against `LibFs`'s - /// tag alphabet and the root against `requireRecordRoot`. The root is a - /// caller's string and it is the half that names where the tree IS, so an - /// unchecked one would make the output directory of every write here the - /// caller's to place anywhere `fs_permissions` allows — which is a - /// consuming repo's config, not an argument this library gets to see. At - /// `LIB_FS_ROOT` that directory is - /// `LibFs.dirForTag(dir)`, which is where - /// `testRootAwareSnapshotPathIsTheWritersAtTheRealRoot` holds the two - /// spellings to being one path. - /// /// The dependency list is frozen here with the rest, and it is not /// metadata. `RainDeployBroadcast.run` hands a suite's `dependencies` to /// `LibRainDeploy.deployToNetworks`, which refuses to broadcast on any @@ -556,9 +505,6 @@ library LibRainDeploySnapshot { address deployed = LibRainDeploy.deployZoltu(creationCode); string memory constants = snapshotConstants(vm, deployed, creationCode, dependencies); - // The directory is created by the writer, from the same root and tag - // this path is derived from, so there is no `createDir` here to - // disagree with it. LibFs.buildFileForContract( vm, deployed, dirForSnapshot(root, dir), contractName, spdxLicenseIdentifier, copyrightText, constants ); diff --git a/test/concrete/BuildRecordRootHarness.sol b/test/concrete/BuildRecordRootHarness.sol index 8b8f13c..836baa4 100644 --- a/test/concrete/BuildRecordRootHarness.sol +++ b/test/concrete/BuildRecordRootHarness.sol @@ -5,20 +5,9 @@ pragma solidity =0.8.25; import {BuildScript} from "../../src/abstract/BuildScript.sol"; import {BuildHarness} from "./BuildHarness.sol"; -/// @title BuildRecordRootHarness -/// @notice `Build` with its record root overridden — the one thing -/// `BuildScript` documents the root as being overridable for — and the -/// regeneration `cutRelease()` freezes from reachable from a test. -/// -/// The regeneration alone. `run()` and `cutRelease()` also regenerate the libs, -/// and those are written into `LIB_DIR`, which no override moves, so either -/// entry point would rewrite committed libs that other test contracts read -/// while forge runs them in parallel. contract BuildRecordRootHarness is BuildHarness { - /// The record root this harness freezes into and regenerates under. string internal sRoot; - /// @param root The fixture record root. constructor(string memory root) { sRoot = root; } @@ -28,7 +17,6 @@ contract BuildRecordRootHarness is BuildHarness { return sRoot; } - /// Runs the hook `cutRelease()` regenerates through. function externalRegenerateSnapshots() external { regenerateSnapshots(); } diff --git a/test/script/BuildRecordRoot.t.sol b/test/script/BuildRecordRoot.t.sol index a9a81cc..f6a832d 100644 --- a/test/script/BuildRecordRoot.t.sol +++ b/test/script/BuildRecordRoot.t.sol @@ -6,23 +6,11 @@ import {Test} from "forge-std-1.16.2/src/Test.sol"; import {LibRainDeploySnapshot} from "../../src/lib/LibRainDeploySnapshot.sol"; import {BuildRecordRootHarness} from "../concrete/BuildRecordRootHarness.sol"; -/// @title BuildRecordRootTest -/// @notice Where `script/Build.sol` writes when `recordRoot()` is overridden — -/// the one thing `BuildScript` documents the root as being overridable for. -/// -/// A contract of its own because this one WRITES, and `BuildTest` states that -/// nothing in it does. What is asserted here is which tree the write lands in, -/// so the write is the subject rather than a side effect: it goes under this -/// contract's own fixture root, and the committed record it would otherwise -/// have gone into is read and left alone. contract BuildRecordRootTest is Test { - /// The fixture record root the regeneration is pointed at. string constant FIXTURE_ROOT = "test/generated-build-record-root"; - /// Clears a fixture record an earlier failure left behind. A cheatcode - /// write is not undone by a revert, so a failure leaves generated sources - /// on disk and the next run reads THOSE. - /// @param root The fixture record root to clear. + /// A cheatcode write is not undone by a revert, so a failure leaves + /// generated sources on disk and the next run reads THOSE. function resetFixture(string memory root) internal { if (vm.exists(root)) { //forge-lint: disable-next-line(unsafe-cheatcode) @@ -30,21 +18,6 @@ contract BuildRecordRootTest is Test { } } - /// PROPERTY: `Build`'s regeneration writes every rolling snapshot under - /// `recordRoot()`. - /// - /// `cutRelease()` freezes each contract from `pathForSnapshot(recordRoot(), - /// CANDIDATE, name)`, so a regeneration that ignores the root hands the - /// freeze a record nothing wrote — `NothingToFreeze` for any repo that - /// overrides the root — and rewrites the real `src/generated/candidate/` on - /// the way to that revert, which is the tree the override exists to keep a - /// caller's hands off. - /// - /// The bytes are the committed candidate's, read from the real record: the - /// regeneration is a function of what this repo compiles and the committed - /// snapshot is what it last compiled to, so an equal file under the fixture - /// root is the whole snapshot having moved rather than a file having been - /// created there. function testRegenerateSnapshotsWritesUnderTheRecordRoot() external { resetFixture(FIXTURE_ROOT); BuildRecordRootHarness harness = new BuildRecordRootHarness(FIXTURE_ROOT); @@ -52,7 +25,6 @@ contract BuildRecordRootTest is Test { harness.externalRegenerateSnapshots(); - // Read while the fixture is still there, asserted once it is gone. bool[] memory written = new bool[](names.length); string[] memory regenerated = new string[](names.length); string[] memory committed = new string[](names.length); diff --git a/test/src/lib/LibRainDeploySnapshot.t.sol b/test/src/lib/LibRainDeploySnapshot.t.sol index 1f778a3..6ac0fbf 100644 --- a/test/src/lib/LibRainDeploySnapshot.t.sol +++ b/test/src/lib/LibRainDeploySnapshot.t.sol @@ -516,36 +516,12 @@ contract LibRainDeploySnapshotTest is Test { assertTrue(exists); } - /// The fixture record root the rooted write is pointed at. Under `test/`, - /// which nothing walks for releases. string constant ROOTED_FIXTURE_ROOT = "test/generated-write-snapshot-root"; - /// The directory the rooted write writes into, under the fixture root and - /// nowhere else. Not tag shaped, for the reason - /// `testWriteSnapshotWritesTheSnapshotAtItsPath` gives, and drawn from the - /// tag alphabet because the writer places files only in directories whose - /// names are. string constant ROOTED_FIXTURE_DIR = "writeSnapshotRootedNotATag"; - /// The directory the same snapshot is written into at the REAL root, to - /// compare the bytes against. A second name so that nothing this test wrote - /// can stand in for what the rooted write did. string constant ROOTED_FIXTURE_REAL_DIR = "writeSnapshotRootedRealNotATag"; - /// PROPERTY: a snapshot is generated under the record root it is HANDED, - /// and the real record is not written on the way there. - /// - /// `freeze` reads every rolling snapshot at `pathForSnapshot(root, - /// CANDIDATE, name)`, so a generator that wrote under `LIB_FS_ROOT` - /// whatever root it was handed would leave a release cut under any other - /// root frozen from a record its own regeneration never wrote — after - /// rewriting the append-only tree the other root exists to keep clear. - /// - /// The bytes are the real root's for the same inputs: the root selects the - /// PATH and nothing else, so a fixture record holds the layout the real - /// record holds rather than one only a test can be pointed at. State is - /// reverted between the two writes for the reason - /// `testWriteSnapshotDefaultsToTheOrgHeader` gives. function testWriteSnapshotWritesUnderTheRootItIsHanded() external { uint256 undeployed = vm.snapshotState(); string memory atRealRoot = vm.readFile( @@ -569,7 +545,6 @@ contract LibRainDeploySnapshotTest is Test { new address[](0) ); - // Read while the fixtures are still there, asserted once they are gone. bool rooted = vm.exists(written); string memory atFixtureRoot = rooted ? vm.readFile(written) : ""; bool inTheRealRecord = vm.exists(LibRainDeploySnapshot.pathForSnapshot(ROOTED_FIXTURE_DIR, FIXTURE_CONTRACT)); @@ -593,56 +568,24 @@ contract LibRainDeploySnapshotTest is Test { assertEq(atFixtureRoot, atRealRoot); } - /// The directory the escaping write is pointed at, under whatever root it - /// is handed. Not tag shaped, for the reason - /// `testWriteSnapshotWritesTheSnapshotAtItsPath` gives. string constant ESCAPE_FIXTURE_DIR = "writeSnapshotEscapeNotATag"; - /// A record root spelled as a path that leaves the tree it names: two - /// segments under `test/`, then back out of both and into the REAL record. - /// - /// The real record is where it is pointed deliberately. It is the tree - /// `BuildScript.recordRoot` is overridable to keep a caller's hands off, it - /// is append-only, and `fs_permissions` grants `./src` — so a root that - /// reaches it is inside everything the config can refuse and is exactly the - /// write nothing outside this library is left to catch. string constant ESCAPE_FIXTURE_ROOT = "test/generated-escape-root/../../src/generated"; - /// The directory `ESCAPE_FIXTURE_ROOT`'s first segment names, created on the - /// way through by a recursive create and removed with the rest. string constant ESCAPE_FIXTURE_CLIMB_DIR = "test/generated-escape-root"; - /// External wrapper so a refusal is a failed call rather than a reverted - /// test, for the write that is not supposed to happen at all. - /// @param root The record root to generate into. - /// @param dir The snapshot directory name. function externalWriteSnapshotAt(string memory root, string memory dir) external { LibRainDeploySnapshot.writeSnapshot( vm, root, dir, FIXTURE_CONTRACT, type(MockDeployable).creationCode, new address[](0) ); } - /// PROPERTY: a root that resolves to somewhere other than the tree it names - /// is refused, and nothing is written. - /// - /// A root is concatenated with a directory and a contract name, both of - /// which are checked, and the root is the half that decides where the tree - /// IS. Unchecked, `..` in it walks the write out of the root the caller - /// named and into one it did not — here the append-only record — and the - /// only thing that would have stood between the two is `fs_permissions`, - /// which is a consuming repo's config rather than an argument of this - /// library's and which grants the record's own tree. - /// - /// The landing path is the WRITER's own spelling at the real root rather - /// than a literal, so what is asserted absent is the same path a real - /// generation into the record would produce. function testWriteSnapshotRefusesARootThatClimbsOutOfTheTreeItNames() external { string memory landing = LibRainDeploySnapshot.pathForSnapshot(ESCAPE_FIXTURE_DIR, FIXTURE_CONTRACT); (bool accepted,) = address(this).call(abi.encodeCall(this.externalWriteSnapshotAt, (ESCAPE_FIXTURE_ROOT, ESCAPE_FIXTURE_DIR))); - // Read while any residue is still there, asserted once it is gone. bool landedInTheRecord = vm.exists(landing); if (vm.exists(LibRainDeploySnapshot.dirForSnapshot(ESCAPE_FIXTURE_DIR))) { //forge-lint: disable-next-line(unsafe-cheatcode) @@ -657,30 +600,14 @@ contract LibRainDeploySnapshotTest is Test { assertFalse(accepted, "a root that resolves outside the tree it names was accepted"); } - /// External wrapper so a refusal is a failed call rather than a reverted - /// test, and so the root rule can be asked about a root on its own. - /// @param root The record root to check. function externalRequireRecordRoot(string memory root) external pure { LibRainDeploySnapshot.requireRecordRoot(root); } - /// External wrapper for `LibFs`'s own tag rule, the counterpart to - /// `externalRequireRecordRoot`. - /// @param tag The tag to check. function externalRequireTag(string memory tag) external pure { LibFs.requireTag(tag); } - /// PROPERTY: every root that is not a path of record root segments is - /// refused, and the refusal names the root. - /// - /// The cases are the shapes a root can take that a concatenation cannot - /// survive, each of which puts the written file somewhere other than under - /// the root the caller named: climbing out of the tree, naming the tree's - /// own parent, starting at the filesystem root, ending in a separator so - /// the next one doubles, doubling one already, and carrying nothing at all. - /// A character outside the alphabet is last, because it is the one that is - /// not about separators. function testRecordRootRefusesEveryRootThatIsNotOne() external { string[8] memory bad = [ "../src/generated", @@ -699,20 +626,6 @@ contract LibRainDeploySnapshotTest is Test { } } - /// PROPERTY: a root of ONE segment is accepted exactly when `LibFs` accepts - /// that segment as a tag, `-` alone excepted. - /// - /// The root rule cannot be asked of `LibFs.requireTag` — a root is a path - /// and `requireTag` refuses the separators that make it one — so the - /// alphabet between the separators is a copy of `LibFs`'s, and this is where - /// the copy is held to the original. Exhaustive over all 256 byte values - /// rather than fuzzed, because the alphabet is a property of every byte and - /// a copy that has drifted by one of them is a copy that has drifted. - /// - /// `-` is the whole of the widening and it is asserted as such: the two - /// rules are required to disagree about it, so dropping it from the root - /// alphabet fails here as loudly as widening the root alphabet further - /// does. function testRecordRootSegmentIsTheTagAlphabetPlusHyphen() external view { for (uint256 i = 0; i < 256; i++) { bytes memory segmentBytes = new bytes(1); @@ -727,21 +640,10 @@ contract LibRainDeploySnapshotTest is Test { } } - /// External wrapper so a refusal is a failed call rather than a reverted - /// test, for the record walk. - /// @param root The record root to walk. function externalFrozenSnapshotPaths(string memory root) external view { LibRainDeploySnapshot.frozenSnapshotPaths(vm, root); } - /// PROPERTY: the record WALK holds a root to the same rule the writers do. - /// - /// It is the one root-taking entry point that does not reach - /// `dirForSnapshot`, and it answers a missing root with an empty record — - /// which is a real state for a repo that has released nothing, and silence - /// for a root nothing could ever have been written under. The same root is - /// refused by both, so a reader cannot be pointed somewhere a writer would - /// not go. function testFrozenSnapshotPathsRefusesARootThatIsNotOne() external { vm.expectRevert(abi.encodeWithSelector(InvalidRecordRoot.selector, ESCAPE_FIXTURE_ROOT)); this.externalFrozenSnapshotPaths(ESCAPE_FIXTURE_ROOT); From fd90becc43ea74f55d6a922cace168f1fa265cc4 Mon Sep 17 00:00:00 2001 From: baku-ccron Date: Wed, 16 Sep 2026 00:14:37 +0000 Subject: [PATCH 6/6] docs: put back the reasons 222ee7b cut with the restatements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 222ee7b took 194 lines off this branch under the heading of restated comments. Most of what went was not restatement: `src/lib/LibRainDeploySnapshot.sol` lost every sentence saying why a record root is checked at all, why it is checked where it is, why `LibFs.requireTag` cannot be asked the question, and why the generator takes a root instead of writing under `LIB_FS_ROOT` — the thesis of this PR. The tests lost the threat model the escape-root fixture is built from, the argument that makes each oracle independent of the writer under test, and the reason each external wrapper exists. Those come back here. The cuts that were restatement stay cut: the constant docstrings that spell the constant's name back, `sRoot`, and the `@param` and `@return` lines on the test-local wrappers, whose signatures already say it. `resetFixture` keeps 222ee7b's shorter form. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01V8ViHcKLVk2YoS2joH4HdN --- src/lib/LibRainDeploySnapshot.sol | 54 ++++++++++++++ test/concrete/BuildRecordRootHarness.sol | 9 +++ test/script/BuildRecordRoot.t.sol | 25 +++++++ test/src/lib/LibRainDeploySnapshot.t.sol | 89 ++++++++++++++++++++++++ 4 files changed, 177 insertions(+) diff --git a/src/lib/LibRainDeploySnapshot.sol b/src/lib/LibRainDeploySnapshot.sol index c87ab64..b995f4c 100644 --- a/src/lib/LibRainDeploySnapshot.sol +++ b/src/lib/LibRainDeploySnapshot.sol @@ -50,6 +50,11 @@ error EmptyRelease(string tag); error NonMonotonicRelease(string tag, string newestFrozenTag); /// Thrown when a record root is not a path this library can place a record at. +/// A root is interpolated into every snapshot path a caller hands it, so one +/// that climbs out of the tree, starts at `/` or is empty makes the paths this +/// library returns paths to somewhere else entirely — and `fs_permissions`, a +/// consuming repo's config rather than this library's argument, the only thing +/// standing between a generated file and an arbitrary location on disk. /// @param root The rejected root. error InvalidRecordRoot(string root); @@ -203,6 +208,22 @@ library LibRainDeploySnapshot { /// Reverts unless `root` is a path of record root segments: at least one /// segment, separated by single `/`, each of them at least one character /// and every character an ASCII letter, a digit, `_`, `$` or `-`. + /// + /// That is `LibFs.requireTag`'s alphabet with `-` admitted as well, and a + /// root is held to it segment by segment for the reason `requireTag` states + /// of a tag: no character in the set is a path separator and none of them is + /// `.`, so no segment is `.` or `..` and none reaches past the single + /// directory it names. Refusing the empty segment is what carries that from + /// a segment to a path — it takes the leading `/` of an absolute path, the + /// trailing one, the doubled one, and the empty root itself. + /// + /// `requireTag` cannot be asked this, because the separators that make a + /// path a path are exactly what it refuses; the alphabet BETWEEN them is a + /// copy of its, held to it byte for byte by + /// `testRecordRootSegmentIsTheTagAlphabetPlusHyphen`. The `-` is the whole + /// of the widening and it is what this repo's own roots need: `src/generated` + /// is tag segments already, while every fixture root the tests build is + /// `test/generated-` or `test/fixture-record`. /// @param root The record root to check. function requireRecordRoot(string memory root) internal pure { bytes memory rootBytes = bytes(root); @@ -237,6 +258,11 @@ library LibRainDeploySnapshot { /// that admitted a name the writer refuses is a reader pointed at a path /// nothing can ever have written, and a fixture record that admitted one /// would be a fixture of a layout the real record cannot hold. + /// + /// The root is checked here too, and this is where it has to be: it is the + /// one place the root becomes a path, and the two halves of that path are + /// concatenated caller input. A checked `dir` beside an unchecked root is + /// only the shorter half of the path confined. /// @param root The record root — `LIB_FS_ROOT` for a repo's real record. /// MUST be a path of record root segments. /// @param dir The snapshot directory name — a release tag, or `CANDIDATE`. @@ -371,6 +397,10 @@ library LibRainDeploySnapshot { /// - the entry is a file directly inside it. Everything in a release /// directory belongs to that release's record — there is no extension to /// filter on, because nothing else has any business being in there. + /// The root is checked before the walk, because a root nothing can be + /// written under is not a record that happens to be empty. This is the one + /// root-taking entry point that does not reach `dirForSnapshot`, so the two + /// together are every way a root gets into this library. /// @param vm The Vm instance for file operations. /// @param root The record root — `LIB_FS_ROOT` for a repo's real record. /// MUST be a path of record root segments. @@ -462,6 +492,27 @@ library LibRainDeploySnapshot { /// Generate one snapshot for one contract. /// + /// The output root is the one `freeze` is handed, and it is required for + /// the same reason: `cutRelease()` regenerates and then freezes within ONE + /// record tree. A generator that could only write under `LIB_FS_ROOT` would + /// leave a release cut under any other root frozen from a rolling snapshot + /// its own regeneration never wrote — after writing the real record on the + /// way there, which is the tree a `recordRoot()` override exists to keep a + /// caller's hands off. + /// + /// `LibFs.buildFileForContract` takes the directory it writes into, so the + /// root reaches the writer through `dirForSnapshot(root, dir)`, which is + /// where both halves of the directory are checked: `dir` against `LibFs`'s + /// tag alphabet and the root against `requireRecordRoot`. The root is a + /// caller's string and it is the half that names where the tree IS, so an + /// unchecked one would make the output directory of every write here the + /// caller's to place anywhere `fs_permissions` allows — which is a + /// consuming repo's config, not an argument this library gets to see. At + /// `LIB_FS_ROOT` that directory is + /// `LibFs.dirForTag(dir)`, which is where + /// `testRootAwareSnapshotPathIsTheWritersAtTheRealRoot` holds the two + /// spellings to being one path. + /// /// The dependency list is frozen here with the rest, and it is not /// metadata. `RainDeployBroadcast.run` hands a suite's `dependencies` to /// `LibRainDeploy.deployToNetworks`, which refuses to broadcast on any @@ -505,6 +556,9 @@ library LibRainDeploySnapshot { address deployed = LibRainDeploy.deployZoltu(creationCode); string memory constants = snapshotConstants(vm, deployed, creationCode, dependencies); + // The directory is created by the writer, from the same root and tag + // this path is derived from, so there is no `createDir` here to + // disagree with it. LibFs.buildFileForContract( vm, deployed, dirForSnapshot(root, dir), contractName, spdxLicenseIdentifier, copyrightText, constants ); diff --git a/test/concrete/BuildRecordRootHarness.sol b/test/concrete/BuildRecordRootHarness.sol index 836baa4..d18ebf7 100644 --- a/test/concrete/BuildRecordRootHarness.sol +++ b/test/concrete/BuildRecordRootHarness.sol @@ -5,6 +5,15 @@ pragma solidity =0.8.25; import {BuildScript} from "../../src/abstract/BuildScript.sol"; import {BuildHarness} from "./BuildHarness.sol"; +/// @title BuildRecordRootHarness +/// @notice `Build` with its record root overridden — the one thing +/// `BuildScript` documents the root as being overridable for — and the +/// regeneration `cutRelease()` freezes from reachable from a test. +/// +/// The regeneration alone. `run()` and `cutRelease()` also regenerate the libs, +/// and those are written into `LIB_DIR`, which no override moves, so either +/// entry point would rewrite committed libs that other test contracts read +/// while forge runs them in parallel. contract BuildRecordRootHarness is BuildHarness { string internal sRoot; diff --git a/test/script/BuildRecordRoot.t.sol b/test/script/BuildRecordRoot.t.sol index f6a832d..6f2495c 100644 --- a/test/script/BuildRecordRoot.t.sol +++ b/test/script/BuildRecordRoot.t.sol @@ -6,6 +6,15 @@ import {Test} from "forge-std-1.16.2/src/Test.sol"; import {LibRainDeploySnapshot} from "../../src/lib/LibRainDeploySnapshot.sol"; import {BuildRecordRootHarness} from "../concrete/BuildRecordRootHarness.sol"; +/// @title BuildRecordRootTest +/// @notice Where `script/Build.sol` writes when `recordRoot()` is overridden — +/// the one thing `BuildScript` documents the root as being overridable for. +/// +/// A contract of its own because this one WRITES, and `BuildTest` states that +/// nothing in it does. What is asserted here is which tree the write lands in, +/// so the write is the subject rather than a side effect: it goes under this +/// contract's own fixture root, and the committed record it would otherwise +/// have gone into is read and left alone. contract BuildRecordRootTest is Test { string constant FIXTURE_ROOT = "test/generated-build-record-root"; @@ -18,6 +27,21 @@ contract BuildRecordRootTest is Test { } } + /// PROPERTY: `Build`'s regeneration writes every rolling snapshot under + /// `recordRoot()`. + /// + /// `cutRelease()` freezes each contract from `pathForSnapshot(recordRoot(), + /// CANDIDATE, name)`, so a regeneration that ignores the root hands the + /// freeze a record nothing wrote — `NothingToFreeze` for any repo that + /// overrides the root — and rewrites the real `src/generated/candidate/` on + /// the way to that revert, which is the tree the override exists to keep a + /// caller's hands off. + /// + /// The bytes are the committed candidate's, read from the real record: the + /// regeneration is a function of what this repo compiles and the committed + /// snapshot is what it last compiled to, so an equal file under the fixture + /// root is the whole snapshot having moved rather than a file having been + /// created there. function testRegenerateSnapshotsWritesUnderTheRecordRoot() external { resetFixture(FIXTURE_ROOT); BuildRecordRootHarness harness = new BuildRecordRootHarness(FIXTURE_ROOT); @@ -25,6 +49,7 @@ contract BuildRecordRootTest is Test { harness.externalRegenerateSnapshots(); + // Read while the fixture is still there, asserted once it is gone. bool[] memory written = new bool[](names.length); string[] memory regenerated = new string[](names.length); string[] memory committed = new string[](names.length); diff --git a/test/src/lib/LibRainDeploySnapshot.t.sol b/test/src/lib/LibRainDeploySnapshot.t.sol index 6ac0fbf..5471af1 100644 --- a/test/src/lib/LibRainDeploySnapshot.t.sol +++ b/test/src/lib/LibRainDeploySnapshot.t.sol @@ -516,12 +516,33 @@ contract LibRainDeploySnapshotTest is Test { assertTrue(exists); } + /// Under `test/`, which nothing walks for releases. string constant ROOTED_FIXTURE_ROOT = "test/generated-write-snapshot-root"; + /// Not tag shaped, for the reason + /// `testWriteSnapshotWritesTheSnapshotAtItsPath` gives, and drawn from the + /// tag alphabet because the writer places files only in directories whose + /// names are. string constant ROOTED_FIXTURE_DIR = "writeSnapshotRootedNotATag"; + /// A second name so that nothing this test wrote can stand in for what the + /// rooted write did. string constant ROOTED_FIXTURE_REAL_DIR = "writeSnapshotRootedRealNotATag"; + /// PROPERTY: a snapshot is generated under the record root it is HANDED, + /// and the real record is not written on the way there. + /// + /// `freeze` reads every rolling snapshot at `pathForSnapshot(root, + /// CANDIDATE, name)`, so a generator that wrote under `LIB_FS_ROOT` + /// whatever root it was handed would leave a release cut under any other + /// root frozen from a record its own regeneration never wrote — after + /// rewriting the append-only tree the other root exists to keep clear. + /// + /// The bytes are the real root's for the same inputs: the root selects the + /// PATH and nothing else, so a fixture record holds the layout the real + /// record holds rather than one only a test can be pointed at. State is + /// reverted between the two writes for the reason + /// `testWriteSnapshotDefaultsToTheOrgHeader` gives. function testWriteSnapshotWritesUnderTheRootItIsHanded() external { uint256 undeployed = vm.snapshotState(); string memory atRealRoot = vm.readFile( @@ -545,6 +566,7 @@ contract LibRainDeploySnapshotTest is Test { new address[](0) ); + // Read while the fixtures are still there, asserted once they are gone. bool rooted = vm.exists(written); string memory atFixtureRoot = rooted ? vm.readFile(written) : ""; bool inTheRealRecord = vm.exists(LibRainDeploySnapshot.pathForSnapshot(ROOTED_FIXTURE_DIR, FIXTURE_CONTRACT)); @@ -568,24 +590,53 @@ contract LibRainDeploySnapshotTest is Test { assertEq(atFixtureRoot, atRealRoot); } + /// Not tag shaped, for the reason + /// `testWriteSnapshotWritesTheSnapshotAtItsPath` gives. string constant ESCAPE_FIXTURE_DIR = "writeSnapshotEscapeNotATag"; + /// A record root spelled as a path that leaves the tree it names: two + /// segments under `test/`, then back out of both and into the REAL record. + /// + /// The real record is where it is pointed deliberately. It is the tree + /// `BuildScript.recordRoot` is overridable to keep a caller's hands off, it + /// is append-only, and `fs_permissions` grants `./src` — so a root that + /// reaches it is inside everything the config can refuse and is exactly the + /// write nothing outside this library is left to catch. string constant ESCAPE_FIXTURE_ROOT = "test/generated-escape-root/../../src/generated"; + /// The directory `ESCAPE_FIXTURE_ROOT`'s first segment names, created on the + /// way through by a recursive create and removed with the rest. string constant ESCAPE_FIXTURE_CLIMB_DIR = "test/generated-escape-root"; + /// External wrapper so a refusal is a failed call rather than a reverted + /// test, for the write that is not supposed to happen at all. function externalWriteSnapshotAt(string memory root, string memory dir) external { LibRainDeploySnapshot.writeSnapshot( vm, root, dir, FIXTURE_CONTRACT, type(MockDeployable).creationCode, new address[](0) ); } + /// PROPERTY: a root that resolves to somewhere other than the tree it names + /// is refused, and nothing is written. + /// + /// A root is concatenated with a directory and a contract name, both of + /// which are checked, and the root is the half that decides where the tree + /// IS. Unchecked, `..` in it walks the write out of the root the caller + /// named and into one it did not — here the append-only record — and the + /// only thing that would have stood between the two is `fs_permissions`, + /// which is a consuming repo's config rather than an argument of this + /// library's and which grants the record's own tree. + /// + /// The landing path is the WRITER's own spelling at the real root rather + /// than a literal, so what is asserted absent is the same path a real + /// generation into the record would produce. function testWriteSnapshotRefusesARootThatClimbsOutOfTheTreeItNames() external { string memory landing = LibRainDeploySnapshot.pathForSnapshot(ESCAPE_FIXTURE_DIR, FIXTURE_CONTRACT); (bool accepted,) = address(this).call(abi.encodeCall(this.externalWriteSnapshotAt, (ESCAPE_FIXTURE_ROOT, ESCAPE_FIXTURE_DIR))); + // Read while any residue is still there, asserted once it is gone. bool landedInTheRecord = vm.exists(landing); if (vm.exists(LibRainDeploySnapshot.dirForSnapshot(ESCAPE_FIXTURE_DIR))) { //forge-lint: disable-next-line(unsafe-cheatcode) @@ -600,14 +651,28 @@ contract LibRainDeploySnapshotTest is Test { assertFalse(accepted, "a root that resolves outside the tree it names was accepted"); } + /// External wrapper so a refusal is a failed call rather than a reverted + /// test, and so the root rule can be asked about a root on its own. function externalRequireRecordRoot(string memory root) external pure { LibRainDeploySnapshot.requireRecordRoot(root); } + /// External wrapper for `LibFs`'s own tag rule, the counterpart to + /// `externalRequireRecordRoot`. function externalRequireTag(string memory tag) external pure { LibFs.requireTag(tag); } + /// PROPERTY: every root that is not a path of record root segments is + /// refused, and the refusal names the root. + /// + /// The cases are the shapes a root can take that a concatenation cannot + /// survive, each of which puts the written file somewhere other than under + /// the root the caller named: climbing out of the tree, naming the tree's + /// own parent, starting at the filesystem root, ending in a separator so + /// the next one doubles, doubling one already, and carrying nothing at all. + /// A character outside the alphabet is last, because it is the one that is + /// not about separators. function testRecordRootRefusesEveryRootThatIsNotOne() external { string[8] memory bad = [ "../src/generated", @@ -626,6 +691,20 @@ contract LibRainDeploySnapshotTest is Test { } } + /// PROPERTY: a root of ONE segment is accepted exactly when `LibFs` accepts + /// that segment as a tag, `-` alone excepted. + /// + /// The root rule cannot be asked of `LibFs.requireTag` — a root is a path + /// and `requireTag` refuses the separators that make it one — so the + /// alphabet between the separators is a copy of `LibFs`'s, and this is where + /// the copy is held to the original. Exhaustive over all 256 byte values + /// rather than fuzzed, because the alphabet is a property of every byte and + /// a copy that has drifted by one of them is a copy that has drifted. + /// + /// `-` is the whole of the widening and it is asserted as such: the two + /// rules are required to disagree about it, so dropping it from the root + /// alphabet fails here as loudly as widening the root alphabet further + /// does. function testRecordRootSegmentIsTheTagAlphabetPlusHyphen() external view { for (uint256 i = 0; i < 256; i++) { bytes memory segmentBytes = new bytes(1); @@ -640,10 +719,20 @@ contract LibRainDeploySnapshotTest is Test { } } + /// External wrapper so a refusal is a failed call rather than a reverted + /// test, for the record walk. function externalFrozenSnapshotPaths(string memory root) external view { LibRainDeploySnapshot.frozenSnapshotPaths(vm, root); } + /// PROPERTY: the record WALK holds a root to the same rule the writers do. + /// + /// It is the one root-taking entry point that does not reach + /// `dirForSnapshot`, and it answers a missing root with an empty record — + /// which is a real state for a repo that has released nothing, and silence + /// for a root nothing could ever have been written under. The same root is + /// refused by both, so a reader cannot be pointed somewhere a writer would + /// not go. function testFrozenSnapshotPathsRefusesARootThatIsNotOne() external { vm.expectRevert(abi.encodeWithSelector(InvalidRecordRoot.selector, ESCAPE_FIXTURE_ROOT)); this.externalFrozenSnapshotPaths(ESCAPE_FIXTURE_ROOT);