Ship the network-config check, behind one binding consumers keep - #155
Merged
Conversation
`testSupportedNetworksAreFullyConfigured` lived in `test/`, which `.soldeerignore` strips, so no consumer had it. Moving it into `RainDeployVerifySnapshot` makes it available; `RainDeployVerify` is what makes it arrive. The union is the whole verification binding, so the next check added upstream reaches every binder on a version bump rather than waiting on four independent downstream edits that nothing red-lines. `LibStringSet` moves to `src/lib/` because a shipped abstract cannot depend on a stripped one. Closes #151 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 55 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: Pro Plus Run ID: 📒 Files selected for processing (17)
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 |
thedavidmeister
pushed a commit
that referenced
this pull request
Aug 27, 2026
#155 renamed RegistryDeployChainTest to RegistryDeployVerifyTest inside the paragraph this branch deletes, so main re-added it in updated form still carrying the false claim. The deletion stands. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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 #151.
testSupportedNetworksAreFullyConfiguredlived intest/src/lib/LibRainDeploy.t.sol, which.soldeerignorestrips from the published package, so no consumer had it. Per the scope correction on the issue, moving it into a shipped abstract only makes it available — four repos would each owe a binding, and nothing red-lines the one that skips it. So this ships the check and the single entrypoint that makes it, and every check after it, arrive.What shipped
src/abstract/RainDeployVerify.sol—abstract contract RainDeployVerify is RainDeployVerifyChain, RainDeployVerifySnapshot {}. The one contract a deploy repo binds. A check added to either half, or to the union, reaches every binder on a version bump with no downstream edit. The convention it carries: a new check goes into something this already inherits, never into an abstract beside it.testSupportedNetworksAreFullyConfiguredmoved intosrc/abstract/RainDeployVerifySnapshot.sol— the network-free side, body unchanged.vm.readFile("foundry.toml")resolves against the project root of whatever runs it, so a consumer's binding reads the consumer'sfoundry.tomlagainst this package'ssupportedNetworks().LibStringSetmovedtest/lib/→src/lib/— a shipped abstract cannot depend on a stripped one. Four test imports follow it.RegistryDeployChainTest+RegistryDeploySnapshotTest→RegistryDeployVerifyTest— this repo's exemplar is now the same single line a consumer writes, so a check that does not reach the exemplar never shipped. Fork load is unchanged: one contract, seven forks, once.slither.config.jsongains the new abstract by exact filename, never by thesrc/abstract/prefix. README documents the union, adds the config group to the verification table, and states thefs_permissionsa consumer needs.RainDeployVerifyChainandRainDeployVerifySnapshotstay separate contracts underneath. That split is runtime, not binding — the chain half forks every supported network, the snapshot half touches none — so a credential-free job can still bind and--match-contractthe offline half.Verified from a consumer's position
A check that only runs upstream is the defect being fixed, so the evidence is downstream, not here. All of it is against a clone of
rain.tofu.erc20-decimals.deploy.forge soldeer push --dry-runproduces a zip carryingsrc/abstract/RainDeployVerify.sol,src/abstract/RainDeployVerifySnapshot.solandsrc/lib/LibStringSet.sol, and zerotest/entries.flarefrom its[etherscan]leaves the suite green — 3 passed, 0 failed. That is the issue, reproduced downstream.dependencies/rain-deploy-0.1.7/, the two bindings replaced with oneis TOFUTokenDecimalsDeploySuites, RainDeployVerify {}:testSupportedNetworksAreFullyConfiguredruns there and passes against that repo's ownfoundry.toml.RainDeployVerify.solinside the consumer'sdependencies/— no consumer source touched — showed up and ran in that consumer's bound contract on the nextforge test. That is the property the issue asks for, demonstrated rather than asserted.What each consumer must do
Nothing in this PR touches any consumer repo.
rain-deploy0.1.7MetaBoardDeployChainTest+MetaBoardDeploySnapshotTestwith oneMetaBoardDeployVerifyTest is MetaBoardDeploySuites, RainDeployVerify {}rain-deploy0.1.7DecimalFloatDeploySuitesrain-deploy0.1.7TOFUTokenDecimalsDeploySuitesrain-deploy0.1.4RainDeployVerifyline. It deploys to Base, Ethereum and HyperEVM in production, so it is where a missing[etherscan]key costs the most: one unresolvable entry takes verification down for every entry, not only its ownAll three of the first group already carry
{ access = "read", path = "./foundry.toml" }infs_permissions; a repo that does not needs it, and fails loudly without it. Nothing here can be checked for st0x.deploy from this account — that row is from the issue.Expect the bump to go red wherever a consumer has already drifted. That is the check working, and the version-bump PR is where it surfaces. Every check after this one is free.
QA
testSupportedNetworksAreFullyConfigured, now onRainDeployVerifySnapshotand therefore reached throughRainDeployVerify. It fails on base in the only sense available to it: on base it does not exist downstream at all, so every mutation below passes silently there — verified by running the flare[etherscan]deletion against the pristine published 0.1.7 dependency and the consumer's own Chain + Snapshot bindings, which stayed green (3 passed, 0 failed). Upstream it also runs inRegistryDeployVerifyTest(4 passed offline), which is now the same one-line binding a consumer writes.foundry.tomldelete[etherscan] flare→supported network has no [etherscan] key: flarefoundry.tomldelete[rpc_endpoints] flare→supported network has no [rpc_endpoints] alias: flarefoundry.tomladd[rpc_endpoints] optimism→[rpc_endpoints] alias is not a supported network: optimismfoundry.tomladd[etherscan] optimism→[etherscan] key is not a supported network: optimismfoundry.tomldrop{ access = "read", path = "./foundry.toml" }fromfs_permissions→vm.readFile: the path foundry.toml is not allowed to be accessed for read operations, i.e. a repo that cannot read its own config fails rather than passing on a file it never openedfoundry.tomlmutation — a test function added toRainDeployVerify.solinside the consumer'sdependencies/— is the propagation check in point 4, not a kill.LibRainDeploy.supportedNetworks(), which is the list the deploy broadcasts to and the chain group forks, read independently of thefoundry.tomlunder test. The failure modes are foundry's own:[rpc_endpoints]is what makes an alias forkable,[etherscan]is what makes--verifyresolve, and an[etherscan]entry with neitherchainnorurlunder an alias foundry cannot resolve takes the whole section down. Thefs_permissionsbehaviour is forge's cheatcode boundary, not this code's.test/for a shipped abstract and (b) the exemplars bind it. The scope-correction comment supersedes the remedy with (c) a singleRainDeployVerifyunion consumers bind once, (d) that union being the only thing they bind, and (e)Chain/Snapshotkept separately runnable. Covered a, b, c, d, e. (e) is kept at the abstract level: both remain distinct contracts, so--match-contractcan still select the offline half; a repo binding only the union trades that selection for the guarantee, which is what the ruling prescribes. Not covered, and out of scope by the ruling: the downstream bumps and st0x.deploy's first binding, which no upstream change can remove.