From 06ce8dd341c8fb17719dd0c6ee4e0d03b0f6fd6b Mon Sep 17 00:00:00 2001 From: David Meister Date: Mon, 17 Aug 2026 03:08:42 +0000 Subject: [PATCH 1/3] Strip the script/Build.sol worked example, leave the library lean rain.deploy is the org's worked example. This repo is the library and now carries no example of its own. Deleted: - script/Build.sol (and the script/ directory with it) - src/generated/CodeGennable.sol, the artifact it produced. Forced, not a choice: rainix's rainix-copy-artifacts.yaml hard-fails any repo that commits src/generated/ without a script/Build.sol. - .github/workflows/build-pointers.yaml, which existed only to call that reusable. - testBuildFileForContractCommittedArtifactIsCurrent, which read the deleted artifact off disk. Rewrote the five interface docstrings that pointed at the deleted workflow, the README paragraphs that described the example, and the two test comments that described a committed artifact. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/build-pointers.yaml | 9 ------ README.md | 26 ++++++++++------ script/Build.sol | 37 ----------------------- src/generated/CodeGennable.sol | 15 --------- src/interface/IIntegrityToolingV1.sol | 5 ++- src/interface/IOpcodeToolingV1.sol | 5 ++- src/interface/IParserToolingV1.sol | 14 ++++++--- src/interface/ISubParserToolingV1.sol | 5 ++- test/concrete/CodeGennable.sol | 9 ++++-- test/lib/LibFs.buildFileForContract.t.sol | 36 ++++------------------ 10 files changed, 51 insertions(+), 110 deletions(-) delete mode 100644 .github/workflows/build-pointers.yaml delete mode 100644 script/Build.sol delete mode 100644 src/generated/CodeGennable.sol diff --git a/.github/workflows/build-pointers.yaml b/.github/workflows/build-pointers.yaml deleted file mode 100644 index 050e881..0000000 --- a/.github/workflows/build-pointers.yaml +++ /dev/null @@ -1,9 +0,0 @@ -name: build-pointers -on: [push] -jobs: - build-pointers: - # Runs script/Build.sol (this repo has no CopyArtifacts.sol, so that step is - # skipped) and asserts the committed generated sources still match. - # secrets: inherit carries CACHIX_AUTH_TOKEN. - uses: rainlanguage/rainix/.github/workflows/rainix-copy-artifacts.yaml@main - secrets: inherit diff --git a/README.md b/README.md index 8e26680..1d09e42 100644 --- a/README.md +++ b/README.md @@ -9,11 +9,17 @@ Also exposes the tooling interfaces (`IIntegrityToolingV1`, `IOpcodeToolingV1`, `IParserToolingV1`, `ISubParserToolingV1`) that Rain contracts implement to build the pointers this library caches. -`script/Build.sol` is an example implementation. The name is not a free choice: -rainix's `rainix-copy-artifacts.yaml` reusable regenerates from that exact path, -and hard-fails any repo that commits `src/generated/` without it. -`.github/workflows/build-pointers.yaml` wires that reusable up here, so CI fails -when the committed generated sources drift from a fresh regeneration. +A consumer drives this library from a build script, and the path of that script +is not a free choice: rainix's `rainix-copy-artifacts.yaml` reusable regenerates +from `script/Build.sol` exactly, and hard-fails any repo that commits +`src/generated/` without one. A consumer that names its script anything else +gets no regeneration and no currency check. + +The org's worked example is +[`rainlanguage/rain.deploy`](https://github.com/rainlanguage/rain.deploy): +[`script/Build.sol`](https://github.com/rainlanguage/rain.deploy/blob/main/script/Build.sol) +generates into its committed `src/generated/`. This repo carries no example of +its own — it is the library, and nothing here is generated. Generated code is imported downstream by contracts that themselves expose pointers, which pointers feed back into the generation. This cycle means @@ -59,11 +65,11 @@ Checks, each of which CI also runs: - `slither .` - `reuse lint` -Regenerate the committed example artifact under `src/generated/`: - -```sh -forge script script/Build.sol -``` +`forge test` writes scratch files under `src/generated/`, creating the directory +if it is absent. Nothing there is committed, and every test removes its own +file, so a completed run leaves the directory empty and invisible to git. Same +arrangement as `meta/`, which the meta-hash tests use the same way. Anything +left there after an interrupted run is scratch, and `git status` will say so. On top of the above, CI applies rainix's org-wide static checks via [`.github/workflows/rainix.yaml`](.github/workflows/rainix.yaml). diff --git a/script/Build.sol b/script/Build.sol deleted file mode 100644 index b2735f2..0000000 --- a/script/Build.sol +++ /dev/null @@ -1,37 +0,0 @@ -// SPDX-License-Identifier: LicenseRef-DCL-1.0 -// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd -pragma solidity =0.8.25; - -import {Script} from "forge-std-1.16.1/src/Script.sol"; -import {LibFs} from "../src/lib/LibFs.sol"; -import {LibCodeGen} from "../src/lib/LibCodeGen.sol"; -import {CodeGennable} from "../test/concrete/CodeGennable.sol"; - -/// @title Build -/// @notice Script to build the generated file for the CodeGennable contract. -/// @dev This shows an example of how to use the bytes constant generation -/// utility in LibCodeGen. -contract Build is Script { - /// Builds the generated file for the CodeGennable contract to show an example - /// of how to use the bytes constant generation utility. - function run() external { - CodeGennable codeGennable = new CodeGennable(); - - LibFs.buildFileForContract( - vm, - address(codeGennable), - "CodeGennable", - string.concat( - LibCodeGen.bytesConstantString( - vm, "/// @dev Some bytes comment.", "SOME_BYTES_CONSTANT", hex"12345678" - ), - LibCodeGen.bytesConstantString( - vm, - "/// @dev Longer constant.", - "LONGER_BYTES_CONSTANT", - hex"e2bafcba65b2c99d33f5096307bc57c2e7f195d2a178f56e45d720bb64344998e2bafcba65b2c99d33f5096307bc57c2e7f195d2a178f56e45d720bb64344998" - ) - ) - ); - } -} diff --git a/src/generated/CodeGennable.sol b/src/generated/CodeGennable.sol deleted file mode 100644 index a9db139..0000000 --- a/src/generated/CodeGennable.sol +++ /dev/null @@ -1,15 +0,0 @@ -// SPDX-License-Identifier: LicenseRef-DCL-1.0 -// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd -pragma solidity ^0.8.25; - -// THIS FILE IS AUTOGENERATED BY THE BUILD SCRIPT. DO NOT EDIT BY HAND. - -/// @dev Hash of the known bytecode. -bytes32 constant BYTECODE_HASH = bytes32(0x8016e5305d25be28ac4d72616cfd999756e36eded3ec88562375e2e1c9eca84d); - -/// @dev Some bytes comment. -bytes constant SOME_BYTES_CONSTANT = hex"12345678"; - -/// @dev Longer constant. -bytes constant LONGER_BYTES_CONSTANT = - hex"e2bafcba65b2c99d33f5096307bc57c2e7f195d2a178f56e45d720bb64344998e2bafcba65b2c99d33f5096307bc57c2e7f195d2a178f56e45d720bb64344998"; diff --git a/src/interface/IIntegrityToolingV1.sol b/src/interface/IIntegrityToolingV1.sol index 593bdc0..7cb7686 100644 --- a/src/interface/IIntegrityToolingV1.sol +++ b/src/interface/IIntegrityToolingV1.sol @@ -15,7 +15,10 @@ interface IIntegrityToolingV1 { /// for efficiency. As the process is deterministic for a given source and /// compiler configuration, the output can be tested against the used value /// in CI and the translation from source to pointers can also be tested in - /// CI. See .github/workflows/build-pointers.yaml for an example of such a test. + /// CI. rainix's `rainix-copy-artifacts.yaml` reusable workflow is that test: + /// it regenerates from the consumer's `script/Build.sol` and fails on any + /// diff against the committed sources. Worked example of the generation + /// side: https://github.com/rainlanguage/rain.deploy/blob/main/script/Build.sol /// Declared `view` so an implementation may read storage or an immutable to /// build its answer. A `pure` implementation still conforms, as an override /// may only tighten mutability. diff --git a/src/interface/IOpcodeToolingV1.sol b/src/interface/IOpcodeToolingV1.sol index fedb92e..b5d4d0d 100644 --- a/src/interface/IOpcodeToolingV1.sol +++ b/src/interface/IOpcodeToolingV1.sol @@ -14,7 +14,10 @@ interface IOpcodeToolingV1 { /// efficiency. As the process is deterministic for a given source and /// compiler configuration, the output can be tested against the used value /// in CI and the translation from source to pointers can also be tested in - /// CI. See .github/workflows/build-pointers.yaml for an example of such a test. + /// CI. rainix's `rainix-copy-artifacts.yaml` reusable workflow is that test: + /// it regenerates from the consumer's `script/Build.sol` and fails on any + /// diff against the committed sources. Worked example of the generation + /// side: https://github.com/rainlanguage/rain.deploy/blob/main/script/Build.sol /// Declared `view` so an implementation may read storage or an immutable to /// build its answer. A `pure` implementation still conforms, as an override /// may only tighten mutability. diff --git a/src/interface/IParserToolingV1.sol b/src/interface/IParserToolingV1.sol index b117f78..a97a685 100644 --- a/src/interface/IParserToolingV1.sol +++ b/src/interface/IParserToolingV1.sol @@ -13,8 +13,11 @@ interface IParserToolingV1 { /// will be built ahead of time and cached in a constant for efficiency. As /// the process is deterministic for a given source and compiler /// configuration, the output can be tested against the used value in CI and - /// the translation from source to pointers can also be tested in CI. See - /// .github/workflows/build-pointers.yaml for an example of such a test. + /// the translation from source to pointers can also be tested in CI. + /// rainix's `rainix-copy-artifacts.yaml` reusable workflow is that test: it + /// regenerates from the consumer's `script/Build.sol` and fails on any diff + /// against the committed sources. Worked example of the generation side: + /// https://github.com/rainlanguage/rain.deploy/blob/main/script/Build.sol /// Declared `view` so an implementation may read storage or an immutable to /// build its answer. A `pure` implementation still conforms, as an override /// may only tighten mutability. @@ -26,8 +29,11 @@ interface IParserToolingV1 { /// will be built ahead of time and cached in a constant for efficiency. As /// the process is deterministic for a given source and compiler /// configuration, the output can be tested against the used value in CI and - /// the translation from source to pointers can also be tested in CI. See - /// .github/workflows/build-pointers.yaml for an example of such a test. + /// the translation from source to pointers can also be tested in CI. + /// rainix's `rainix-copy-artifacts.yaml` reusable workflow is that test: it + /// regenerates from the consumer's `script/Build.sol` and fails on any diff + /// against the committed sources. Worked example of the generation side: + /// https://github.com/rainlanguage/rain.deploy/blob/main/script/Build.sol /// Declared `view` so an implementation may read storage or an immutable to /// build its answer. A `pure` implementation still conforms, as an override /// may only tighten mutability. diff --git a/src/interface/ISubParserToolingV1.sol b/src/interface/ISubParserToolingV1.sol index c4479dc..5009d3e 100644 --- a/src/interface/ISubParserToolingV1.sol +++ b/src/interface/ISubParserToolingV1.sol @@ -13,7 +13,10 @@ interface ISubParserToolingV1 { /// efficiency. As the process is deterministic for a given source and /// compiler configuration, the output can be tested against the used value /// in CI and the translation from source to parsers can also be tested in - /// CI. See .github/workflows/build-pointers.yaml for an example of such a test. + /// CI. rainix's `rainix-copy-artifacts.yaml` reusable workflow is that test: + /// it regenerates from the consumer's `script/Build.sol` and fails on any + /// diff against the committed sources. Worked example of the generation + /// side: https://github.com/rainlanguage/rain.deploy/blob/main/script/Build.sol /// Declared `view` so an implementation may read storage or an immutable to /// build its answer. A `pure` implementation still conforms, as an override /// may only tighten mutability. diff --git a/test/concrete/CodeGennable.sol b/test/concrete/CodeGennable.sol index 3ee25be..dde87a0 100644 --- a/test/concrete/CodeGennable.sol +++ b/test/concrete/CodeGennable.sol @@ -3,6 +3,11 @@ pragma solidity =0.8.25; /// @title CodeGennable -/// An empty contract used by `script/Build.sol` as a way to show an -/// example of the code generation capabilities of this repo. +/// An empty contract the suite deploys whenever it needs an address that holds +/// code. It carries no behaviour of its own: what the tests want from it is a +/// stable, non-zero `codehash` to feed to `bytecodeHashConstantString` and +/// `buildFileForContract`. The name is asserted on in +/// `LibCodeGen.requireContractName.t.sol` and used as a contract name in +/// `LibCodeGen.describedByMetaHashConstantString.t.sol`, so it is not free to +/// change. contract CodeGennable {} diff --git a/test/lib/LibFs.buildFileForContract.t.sol b/test/lib/LibFs.buildFileForContract.t.sol index a5ed2af..93f26aa 100644 --- a/test/lib/LibFs.buildFileForContract.t.sol +++ b/test/lib/LibFs.buildFileForContract.t.sol @@ -4,7 +4,7 @@ pragma solidity =0.8.25; import {Test} from "forge-std-1.16.1/src/Test.sol"; import {LibFs} from "src/lib/LibFs.sol"; -import {LibCodeGen, InvalidContractName} from "src/lib/LibCodeGen.sol"; +import {InvalidContractName} from "src/lib/LibCodeGen.sol"; import {CodeGennable} from "test/concrete/CodeGennable.sol"; import {LibFsExternal} from "test/concrete/LibFsExternal.sol"; import {LibCodeGenSlow} from "test/lib/LibCodeGenSlow.sol"; @@ -30,10 +30,11 @@ contract LibFsBuildFileForContractTest is Test { iExternal = new LibFsExternal(); } - /// Every test writes under `src/generated/`, which is a committed directory - /// in this repo. Each test owns a distinct name so parallel suites cannot - /// collide, none of them is `CodeGennable` (the committed artifact), and - /// each removes its file again. + /// Every test writes under `src/generated/`, which holds nothing committed + /// in this repo and is created on demand by `buildFileForContract` itself. + /// Everything that lands there during a run is this suite's scratch. Each + /// test owns a distinct name so parallel suites cannot collide, and each + /// removes its file again. function cleanup(string memory contractName) internal { string memory path = LibFs.pathForContract(contractName); if (vm.exists(path)) { @@ -198,31 +199,6 @@ contract LibFsBuildFileForContractTest is Test { cleanup(nameB); } - /// `src/generated/CodeGennable.sol` is committed, and `script/Build.sol` - /// builds it through this function. Nothing in `forge test` noticed when it - /// went stale — only the separate `rainix-copy-artifacts` job did, by - /// regenerating and diffing. This asserts the committed file still opens - /// with what `buildFileForContract` writes today, so drift between the - /// library and the artifact it produced reds the suite too. - /// - /// Deliberately built from `LibCodeGen` here, unlike the tests above: the - /// claim is that the committed bytes match what the library emits now, so - /// the library is the correct side to read it from and the file on disk is - /// the oracle. - function testBuildFileForContractCommittedArtifactIsCurrent() external { - address instance = address(new CodeGennable()); - string memory header = - string.concat(LibCodeGen.filePrefix(), LibCodeGen.bytecodeHashConstantString(vm, instance)); - bytes memory committed = bytes(vm.readFile(LibFs.pathForContract("CodeGennable"))); - - assertTrue(committed.length >= bytes(header).length, "committed artifact is shorter than the header"); - bytes memory actual = new bytes(bytes(header).length); - for (uint256 i = 0; i < actual.length; i++) { - actual[i] = committed[i]; - } - assertEq(actual, bytes(header), "committed artifact is stale, regenerate with script/Build.sol"); - } - /// The bytecode hash is read from the instance that was passed in, not from /// the caller and not from a fixed address. Two addresses holding different /// code produce different files. From 4c61eadc7b7c0863f77b3fef3be5e6420b5e11ac Mon Sep 17 00:00:00 2001 From: David Meister Date: Mon, 17 Aug 2026 03:15:18 +0000 Subject: [PATCH 2/3] Create src/generated in setUp where the deleted artifact used to Deleting src/generated/CodeGennable.sol means src/generated/ does not exist in a fresh checkout. Two suites wrote into it without creating it, relying on the committed artifact to drag the directory into every clone: neither ln, nor vm.writeFile, nor vm.createDir on a child creates the parent. LibFsIsPresentTest went red on CI, 5 of 6 tests. LibFsBuildFileForContractTest passed on CI only because another test in the same contract creates the directory as a side effect before testBuildFileForContractReplacesExistingContent writes there directly; that one fails under --match-test on a fresh tree. Both now create GENERATED_DIR in setUp, the same arrangement LibCodeGenDescribedByMetaHashConstantStringTest already uses for meta/. Co-Authored-By: Claude Opus 5 (1M context) --- test/lib/LibFs.buildFileForContract.t.sol | 12 +++++++++++- test/src/lib/LibFs.isPresent.t.sol | 10 ++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/test/lib/LibFs.buildFileForContract.t.sol b/test/lib/LibFs.buildFileForContract.t.sol index 93f26aa..a383574 100644 --- a/test/lib/LibFs.buildFileForContract.t.sol +++ b/test/lib/LibFs.buildFileForContract.t.sol @@ -3,7 +3,7 @@ pragma solidity =0.8.25; import {Test} from "forge-std-1.16.1/src/Test.sol"; -import {LibFs} from "src/lib/LibFs.sol"; +import {LibFs, GENERATED_DIR} from "src/lib/LibFs.sol"; import {InvalidContractName} from "src/lib/LibCodeGen.sol"; import {CodeGennable} from "test/concrete/CodeGennable.sol"; import {LibFsExternal} from "test/concrete/LibFsExternal.sol"; @@ -30,6 +30,16 @@ contract LibFsBuildFileForContractTest is Test { iExternal = new LibFsExternal(); } + /// `src/generated/` holds no committed file, so nothing in a fresh clone + /// creates it. `buildFileForContract` creates it for itself, but + /// `testBuildFileForContractReplacesExistingContent` writes its stale + /// content there directly first, and a filtered run may be only that test, + /// so the directory is not something this contract can inherit from a test + /// that happened to run earlier. + function setUp() external { + vm.createDir(GENERATED_DIR, true); + } + /// Every test writes under `src/generated/`, which holds nothing committed /// in this repo and is created on demand by `buildFileForContract` itself. /// Everything that lands there during a run is this suite's scratch. Each diff --git a/test/src/lib/LibFs.isPresent.t.sol b/test/src/lib/LibFs.isPresent.t.sol index 45c5866..3249d30 100644 --- a/test/src/lib/LibFs.isPresent.t.sol +++ b/test/src/lib/LibFs.isPresent.t.sol @@ -16,6 +16,16 @@ import {LibFs, GENERATED_DIR} from "src/lib/LibFs.sol"; /// itself and fails on anything that is not a symlink. `vm.readLink` is what the /// library uses, so it is deliberately not what asserts here. contract LibFsIsPresentTest is Test { + /// `src/generated/` holds no committed file, so nothing in a fresh clone + /// creates it, and none of `ln`, `vm.writeFile` or `vm.createDir` for a + /// child of it creates the parent. `buildFileForContract` creates it for + /// itself, but suites run in any order and a filtered run may be only this + /// one, so this contract creates it rather than inheriting it from whatever + /// ran first. + function setUp() external { + vm.createDir(GENERATED_DIR, true); + } + /// Every path this contract hands to the shell is built here from a bare /// name, so no test in it can name a path outside the generated directory. function pathFor(string memory name) internal pure returns (string memory) { From 0445cb378648229101e5a2e7a5476d97eaf1a586 Mon Sep 17 00:00:00 2001 From: David Meister Date: Mon, 17 Aug 2026 03:17:36 +0000 Subject: [PATCH 3/3] Scope the cleanup docstring to the tests that generate a file The rejection tests assert that nothing is written and never call cleanup, so "every test writes under src/generated/" was wrong about them. Raised by CodeRabbit on #138. Co-Authored-By: Claude Opus 5 (1M context) --- test/lib/LibFs.buildFileForContract.t.sol | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/test/lib/LibFs.buildFileForContract.t.sol b/test/lib/LibFs.buildFileForContract.t.sol index a383574..79d6858 100644 --- a/test/lib/LibFs.buildFileForContract.t.sol +++ b/test/lib/LibFs.buildFileForContract.t.sol @@ -40,11 +40,12 @@ contract LibFsBuildFileForContractTest is Test { vm.createDir(GENERATED_DIR, true); } - /// Every test writes under `src/generated/`, which holds nothing committed - /// in this repo and is created on demand by `buildFileForContract` itself. - /// Everything that lands there during a run is this suite's scratch. Each - /// test owns a distinct name so parallel suites cannot collide, and each - /// removes its file again. + /// Called by the tests that generate a file. `src/generated/` holds nothing + /// committed in this repo, so everything that lands there during a run is + /// this suite's scratch. Each generating test owns a distinct name so + /// parallel suites cannot collide, and removes its file again. The tests + /// that assert a name is refused write nothing at all and do not come + /// through here. function cleanup(string memory contractName) internal { string memory path = LibFs.pathForContract(contractName); if (vm.exists(path)) {