Generate the network config from the roster instead of comparing it - #237
Open
thedavidmeister wants to merge 3 commits into
Open
thedavidmeister wants to merge 3 commits into
thedavidmeister wants to merge 3 commits into
Conversation
`LibRainDeploy.supportedNetworkConfigs()` becomes the single statement of the supported network set — name, chain id, explorer url and default endpoint. `LibRainDeployConfig` emits `[rpc_endpoints]`, `[etherscan]` and the `.env.example` endpoint variables from it and splices each between its markers, and `BuildScript.run()` writes both files. `supportedNetworks()` is now the roster's names. `testSupportedNetworksAreFullyConfigured` goes: with both sides written from one list there is nothing left for it to compare. What generation cannot settle is whether a declared chain id is the one the bound endpoint reports, so `RainDeployVerifyChain` gains `testSupportedNetworkChainIdsAreBound`, which forks every supported network and checks `block.chainid`. Every generated `[etherscan]` entry states `chain`, which carries #229's requirement across as a property of the generator rather than an assertion about a hand-written file. KNOWN BLOCKER, unresolved: foundry refuses every fs cheatcode write to the project-root `foundry.toml` — `ensure_not_foundry_toml`, "access to `foundry.toml` is not allowed" — regardless of `fs_permissions`. `writeFile`, `writeLine` and `copyFile` are all refused, under every path spelling (`foundry.toml`, `./foundry.toml`, `src/../foundry.toml`, absolute, absolute with `..`). So `forge script ./script/Build.sol` reverts, and the `Git is clean` job that runs it goes red. The `.env.example` half writes fine. The suite does not see this because `BuildScriptHarness` points `configPath()` at a fixture root, where the guard does not apply. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V8ViHcKLVk2YoS2joH4HdN
Foundry refuses every filesystem cheatcode write to the project root's own `foundry.toml` — "access to `foundry.toml` is not allowed", a guard on the path that no `fs_permissions` grant and no spelling of the path gets past, and that refuses `writeFile`, `writeLine` and `copyFile` alike. So `forge script ./script/Build.sol` reverted, and the `Git is clean` job that runs it went red, while the suite stayed green because the harness pointed the writer at a fixture root the guard does not apply to. Reads are allowed, which is what makes this possible. `run()` now reads each file, splices its blocks and writes the result to `.staged-config/` under the same name; `script/build.sh` copies each staged file onto the file of that name at the root and removes the directory. That hook is rainix's own consumer hook: `rainix-copy-artifacts` runs it outside any devshell, after the regeneration and before the `git diff` that fails a stale tree. It needs no forge, no nix and no `--ffi` — the alternative, and not taken, because the invocation that matters passes no `--ffi` and granting it there would hand FFI to every consumer's build. `.env.example` is staged too, though foundry would allow that one written directly, so which file foundry happens to guard is not something the design depends on. Staging also retires the hazard the direct write carried: nothing under `forge test` can race a rewrite of the config every other test reads. A repo with no `script/build.sh` is refused — `BuildHookMissing` — because nothing else installs a staged file, and generating for such a repo would write the roster where nothing reads it while the config went on saying whatever it said, green. Presence is the same condition `rainix-copy-artifacts` runs the hook on. `configPath()` and `envExamplePath()` collapse into one `configRoot()` hook, and `fs_permissions` on both generated files goes to READ. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V8ViHcKLVk2YoS2joH4HdN
Six conflicts, five of them the same decision: #233 GENERATES the network config from `LibRainDeploy.supportedNetworkConfigs()`, so a check that reads `foundry.toml` back is the generator reading its own output. #229 landed on main in the other direction — it strengthened those reads. Generation wins everywhere the two meet. - `README.md`, `foundry.toml`: the branch's text and the generated blocks. The `[etherscan]` block is emitted, so main's prose inside it and its hand-maintained comment cannot survive there; the rationale for stating `chain` on every entry lives in `LibRainDeployConfig` instead. - `RainDeployVerifySnapshot.sol`: `testSupportedNetworksAreFullyConfigured` is gone. With both sides written from one list there is nothing to compare. - `RainDeployVerifyChain.sol`, `RainDeployVerifyChain.t.sol`: the branch's roster-based `checkNetworkChainIds` is kept and main's `declaredChainIds`, `DeclaredChainId` and `NoDeclaredChainIds` are deleted, for the same reason. Main's prose about what a wrong `chain` costs is kept; so is its property that every entry is checked and not only the first, ported to the roster as `testChainIdChecksEveryEntry`. - `BuildScript.t.sol`: both constant sets, which do not overlap. Two things followed from those resolutions rather than being conflicts. `checkNetworksConfigured` and `checkEtherscanEntriesResolvable` are deleted from `RainDeployVerifySnapshotBase`, with `EtherscanEntryUnresolvable` and their tests. #229 moved the comparison's body there from the test; deleting the test leaves it dead, and it IS the comparison — keeping it would leave a tested, consumer-callable assertion about a file this package now writes. `testRunCallsEveryHookThatRegenerates` becomes `testRunCallsEveryGenerator`. It required `run()`'s calls to be exactly the `internal virtual` hooks that write, and `run()` now also calls `regenerateConfig`, which is deliberately NOT a hook: the roster is this package's own, and a repo able to override the emission would deploy to and verify fewer chains with nothing red. The set it enumerates is now every `internal` function that can write, which holds the same two claims over a strictly larger set and no longer turns on `virtual`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01V8ViHcKLVk2YoS2joH4HdN
|
Warning Review limit reachedNext included review available in 38 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (20)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #233. Closes #194.
LibRainDeploy.supportedNetworkConfigs()becomes the single statement of thesupported network set — name, chain id, explorer url and default endpoint.
LibRainDeployConfigemits[rpc_endpoints],[etherscan]and the.env.exampleendpoint variables from it and splices each between its markers;BuildScript.run()generates both files.supportedNetworks()is now theroster's names.
That closes #194 by removing its subject rather than answering it. #194 said
nothing in the suite could make the
foundry.tomlconfig assertions fail: theycompared two hand-maintained statements of one set, and no test could drive
either side. With both sides written from one list there is nothing left to
compare, so
testSupportedNetworksAreFullyConfiguredis deleted — and with itcheckNetworksConfigured,checkEtherscanEntriesResolvableandEtherscanEntryUnresolvable, which #229 had just moved the comparison's bodyinto.
Enforcement moves to
Git is clean, the mechanism already holdingsrc/generated/: a tree whose config has drifted from the roster it pins failsthe job every push runs.
foundry refuses to write its own config
forge script ./script/Build.solcannot write the project root'sfoundry.toml— "access tofoundry.tomlis not allowed", a guard on the paththat no
fs_permissionsgrant and no spelling of the path gets past, refusingwriteFile,writeLineandcopyFilealike. So thefs_permissionschange#233 asked for (read → read-write on
./foundry.toml) is not the one thatworks.
Reads are allowed, which is what makes the splice possible.
run()reads eachfile, splices its blocks and writes the result to
.staged-config/;script/build.shinstalls each staged file onto the file of that name at theroot. That hook is rainix's own
rainix-copy-artifactsconsumer hook, whichruns outside any devshell, after the regeneration and before the
git diffthat fails a stale tree. It needs no forge, no nix and no
--ffi; granting--ffiis the alternative and is not taken, because it would be granted toevery consumer's build rather than to this one step.
A repo with no
script/build.shis refused —BuildHookMissing— becausenothing else installs a staged file, and generating for such a repo would write
the roster where nothing reads it while the config went on saying whatever it
said, green.
Staging also retires the race #233 flagged as a requirement: nothing under
forge testcan race a rewrite of the config every other test reads, becausenothing rewrites it.
what generation cannot settle
Whether a declared chain id is the one the bound endpoint reports is a claim
about the world rather than about the text, and
chainis what--verifysubmits. So
RainDeployVerifyChain.testSupportedNetworkChainIdsAreBoundforksevery supported network and compares
block.chainidagainst the roster.merging #229
main's #229 strengthened the reads this deletes, so the merge decides between
them everywhere the two meet, and generation wins: prose and assertions that
read
foundry.tomlback are the generator reading its own output. Kept from#229: its reasoning about what a wrong
chaincosts, and its property thatevery entry is checked and not only the first — ported to the roster as
testChainIdChecksEveryEntry.testRunCallsEveryHookThatRegeneratesbecomestestRunCallsEveryGenerator:run()now also callsregenerateConfig, whichis deliberately not a hook, so the set it enumerates is every
internalfunction that can write rather than the
internal virtualones.consumer impact
A repo inheriting
BuildScripthas to:script/build.sh, and gitignore.staged-config;foundry.tomland.env.example, once each,begin before end — everything outside them stays the consumer's, and the
build neither reads nor moves it;
fs_permissionson./foundry.tomland./.env.exampleatread, andadd
read-writeon./.staged-configandreadon./script/build.sh.A consumer that called
checkNetworksConfiguredorcheckEtherscanEntriesResolvableby hand loses them. What replaces theassertion is that the sections are written rather than checked. The roster is
deliberately not overridable: a repo able to narrow it would deploy to and
verify fewer chains with nothing red, so a network arrives in a consumer's
config by a version bump and by nothing else.
QA
testRpcEndpointsSectionIsTheRoster,testEtherscanSectionStatesChainOnEveryEntry,testEtherscanSectionEntriesAreResolvable,testEtherscanSectionOfTheSupportedNetworksIsResolvable,testEtherscanSectionZeroChainIdReverts,testEnvExampleSectionIsTheRosterDefaults,testVariableNamesAreUppercased,testEmptyRosterReverts, the tentestSplice*/testWrite*cases over themarker splice and the staged write,
testStagedPathsAreNamedAsTheFilesTheyInstallOver,testWriteStagedConfigWithoutBuildHookReverts,testRunStagesTheNetworkConfig,testRunCallsEveryGenerator,testCutReleaseLeavesTheConfigAlone,testChainIdChecksEveryEntry,testChainIdEmptyRosterReverts,testSupportedNetworksAreTheRosterNames,testSupportedNetworkChainIds,testSupportedNetworkExplorerUrls. None of these can be run against base tofail there:
LibRainDeployConfig,BuildScript.regenerateConfigand theSupportedNetworkroster they bind are added by this PR, so the suite does notcompile on
origin/mainat all. Discrimination is shown by mutation instead —each line below is reverted to a shape base behaves as, and a named test fails.
mutation-probeover/home/thedavidmeister/code/scratch/fix-233-mutants.toml, baseline green at585 passed / 0 failed, 7 applied, 7 KILLED, 0 survived, 0 no-run, 0 harness
errors.
RainDeployVerifyChain.checkChainIds:if (declared != reported)->if (false)(the mismatch never reverts) ->testChainIdChecksEveryEntry,testChainIdIsReadFromTheForkedEndpoint,testChainIdMismatchReverts,testZoltuFactoryCodehash.RainDeployVerifyChain.checkChainIds:if (declared != reported)->if (true)(every match reverts) ->testChainIdMatchPasses.RainDeployVerifyChain.checkChainIds:i < networks.length->i < 1(only the first roster entry is checked) ->
testChainIdChecksEveryEntry.RainDeployVerifyChain.checkChainIds:if (networks.length == 0)->if (false)(an empty roster passes having forked nothing) ->testChainIdEmptyRosterReverts.BuildScript.run():regenerateConfig();deleted (run stops generating theconfig) ->
testEveryHookIsReachedFromAnEntryPoint,testRunCallsEveryGenerator,testRunStagesTheNetworkConfig.BuildScript.run():recordRoot();added beside it (run holds a call thatgenerates nothing) ->
testRunCallsEveryGenerator.LibRainDeployConfig.etherscanSection:'}", chain = ', vm.toString(networks[i].chainId)->'}"'(the generated entry states nochain, which is The config check passes an [etherscan] entry that takes verification down for every network #192's bug re-introduced on the generating side) ->
testEtherscanSectionEntriesAreResolvable,testEtherscanSectionOfTheSupportedNetworksIsResolvable. The first passscored this KILLED but named no killer, because its
fail-patterndid notmatch forge's failure line for these two; re-probing this mutant alone with
a widened pattern named them, same KILLED verdict.
test/src/lib/LibRainDeployConfig.t.solrather than concatenated the way thesource concatenates — an expectation built by the source's own spelling would
pass for any spelling, including a broken one — over a fixture roster
alpha/beta/gammathat names no real network, so nothing passes againstthis repo's own config by accident. For
run()'s wiring, the compiler's ASTfor the base contract, enumerating the generators the base declares rather
than a hand-kept list, so a generator added and left uncalled fails without
the test being touched. For the chain ids, the forked endpoint's own
block.chainid, which is the world rather than the text. The intent oracle isGenerate the network config sections from supportedNetworks() instead of comparing them #233's:
RainDeployVerifySnapshot's NatSpec that the config sections "MUST beEXACTLY
supportedNetworks(), which makes the three lists one".[rpc_endpoints], (b)[etherscan]witheach chain id, and (c)
.env.examplegenerated from the roster, (d) delimitedso hand-written config around them survives, (e) from a hook in
run(),(f) enforced by
Git is clean, (g) the membership assertions removed, (h) afork test on
block.chainidwith a real subject, (i)fs_permissionson./foundry.tomlmoved read -> read-write, and (j) no test racing the rewrite.Covered: a, b, c, d, e, f, g, h, j. NOT covered, deliberately: (i) — forge
refuses to write the project root's
foundry.tomlwhateverfs_permissionssays, so the grant stays
readandscript/build.shinstalls from.staged-config/; staging is also what settles (j), since nothing rewritesthe config under
forge testat all. Nothing in the suite can make the foundry.toml config assertions fail #194 asks that the config assertions becapable of failing; covered by removing them, their subject being the
comparison this generates away.
🤖 Generated with Claude Code
https://claude.ai/code/session_01V8ViHcKLVk2YoS2joH4HdN