Repository navigation
Scope verification to the networks the declaration names - #261
Conversation
Closes #246. `RainDeploySuitesBase` gains one `internal view virtual supportedNetworks()`, defaulting to `LibRainDeploy.supportedNetworks()`. Both sides read it: `RainDeployBroadcast.deployNetworks()` defaults to it, and the three verification reads that were hardwired to the library's nine — `RainDeployVerifyChain.checkDeployedOnSupportedNetworks`, `RainDeployVerifyChain.testSupportedNetworkChainIdsAreBound` and `RainDeployVerifySnapshot.testSupportedNetworksAreFullyConfigured` — now read the declaration instead. A repo that deploys to a subset says so once and is held to exactly that subset. ONE hook, on the declaration, rather than a `virtual` per verification function. Three independent hooks would be three ways to spell a verification set narrower than the one the repo broadcasts to, which is every release held to nothing at all on the networks that were dropped, with no assertion left to notice. On the declaration because that is the contract both sides already inherit, so verifying a different set from the one deployed to is unspellable rather than discouraged. `deployNetworks()` staying overridable is a different thing and is kept. That is the target set of ONE dispatch — st0x.deploy selects between Ethereum and HyperEVM per dispatch — and the networks a dispatch skips are ones the repo still deploys to and is still verified on. This hook does not unblock st0x on its own, and that half is #259 rather than this issue. `checkNetworksConfigured` requires an `[etherscan]` entry for every network in the set it is given, and whether a network HAS an Etherscan deployment is a fact about the network rather than about the repo: Robinhood (4663) is not indexed by Etherscan v2 and verifies through Sourcify, so st0x.deploy states no entry for it on purpose and still fails the config group with its config correct. #259 wants the networks that have an Etherscan deployment, which no scoping of this hook is. QA: three new discriminating tests plus two fixtures. `testSupportedNetworksDefaultsToEveryRainNetwork` pins the default to the library's list by membership and position. `testDeployNetworksFollowsTheDeclaredNetworks` drives `ExampleDeployNarrowNetworks`, which declares base alone — neither end of the library's list, so a reader that ignored the override lands on polygon or arbitrum rather than base. `RainDeployVerifyChainNarrowNetworksTest` runs both inherited chain tests on that narrowed set and adds the fork COUNT and the chain LEFT SELECTED as the observable that the set really narrowed, since the subject is etched on every fork and this repo's ids are right on all nine, so passing alone says nothing. `testConfigIsHeldToTheDeclaredNetworks` flips the declaration narrow at test time and expects the reverse-direction failure naming `arbitrum`; it is a flag rather than a static narrowing because the only config on disk names all nine, so a statically narrow contract would fail the very inherited test it exists to drive. Mutation probes: six applied, six killed, every file restored byte-clean. Each production read reverted to `LibRainDeploy.supportedNetworks()` in turn, and the `supportedNetworks` default broken twice (one network dropped; first two entries swapped) because reverting the default to the library is a no-op. Probes 4 and 5 die at `createFork` on `arbitrum` in the strict environment, so they were re-run with all nine aliases bound to one base endpoint — unmutated green there, and the mutants then fail on the fork-count assertion and on `NetworkChainIdMismatch("arbitrum", 42161, 8453)` respectively. `forge fmt --check`, `forge lint` and `forge build` clean on the rainix `8657b83` sol-shell. Full suite: 624 tests, 567 passed, 57 failed, every failure a missing `<NETWORK>_RPC_URL` and none an assertion; main at `63b0558` re-run in the same environment fails the identical 57. The nine-network fork suites were not exercised — only `BASE_RPC_URL` was available. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 35 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (11)
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 |
|
23 lines of NatSpec on a 3-line hook, 37 on a one-test contract. Cut throughout: the restatements of "one fact, one hook" in three files, the alternatives-considered paragraphs, and the cross-references. One substantive fix came with it. The `[etherscan]` paragraph in `RainDeployVerifySnapshot` said a repo deploying to Robinhood and stating no entry for it "fails this with its config correct". That premise is wrong — #259 is closed over it. A chain Etherscan does not index carries its explorer's API url in the entry instead, which is what this repo's own `foundry.toml` does for 4663 and what st0x.deploy#412 adopts. The paragraph now says that. 27/27 of the touched suites pass; fmt and lint clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
main's #261 added the `supportedNetworks()` hook on `RainDeploySuitesBase`; this branch changed `DeploySuite.dependencies` to `DeployDependency[]`. The two touch the same declaration but not the same field, so everything merged except the tail of `RainDeploySuitesBase.t.sol`, where both sides appended tests to the end of the same contract. Both sets are kept. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reconciles generation (this branch) with #261's declaration scoping. Config is generated from the DECLARED network set, not the catalogue: `supportedNetworkConfigs()` becomes the catalogue of per-network facts and `declaredNetworkConfigs(networks)` selects from it in declaration order, reverting `NetworkNotInCatalogue` for a declared name with no entry. That named refusal is what replaces the membership comparison this branch deletes. `BuildScript` inherits `RainDeploySuitesBase` rather than growing a networks hook of its own, so the declaration stays one hook with no consumer boilerplate — `supportedNetworks()` has a body, and a same-signature virtual on `BuildScript` would force every consumer's `Build` to write a disambiguating override. #261's `testConfigIsHeldToTheDeclaredNetworks` is renamed and re-expressed rather than deleted, keeping its name and its arbitrum message: a generator that ignored the declaration fails on the alias rather than on a text diff. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
One conflict, in `RainDeploySuitesBase`: main's #261 added the `supportedNetworks()` hook immediately above the NatSpec line this branch rewrote to name the artifact-path refusal. Both kept. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes #246.
RainDeploySuitesBasegains one hook —internal view virtual supportedNetworks(), defaulting toLibRainDeploy.supportedNetworks()— and both sides read it:RainDeployBroadcast.deployNetworks()defaults to it rather than to thelibrary;
RainDeployVerifyChain.checkDeployedOnSupportedNetworksforks it;RainDeployVerifyChain.testSupportedNetworkChainIdsAreBoundreads the[etherscan]entries of it;RainDeployVerifySnapshot.testSupportedNetworksAreFullyConfiguredholdsfoundry.tomlto it.A repo that deploys to a subset of the org's nine says so once, on its suites
declaration, and is held to exactly that subset. A repo that deploys to all of
them declares nothing.
One hook, on the declaration
A
virtualon each verification function instead would be three independentways to spell a verification set narrower than the one the repo broadcasts to
— which is every release held to nothing at all on the networks that were
dropped, with no assertion anywhere left to notice. The set of networks a repo
deals with is one fact, so it is one hook.
It lives on
RainDeploySuitesBasebecause that is the contract both sidesalready inherit. Verifying a different set from the one deployed to is
therefore unspellable rather than discouraged.
deployNetworks()staying overridable is a different thing and is kept. Thatis the target set of ONE dispatch — st0x.deploy selects between Ethereum and
HyperEVM per dispatch, and the reusable workflow carries a
network:input forit — and the networks a dispatch skips are ones the repo still deploys to and
is still verified on. Its NatSpec now says so, and points at
supportedNetworksfor the other thing.This does not by itself unblock st0x
checkNetworksConfiguredrequires an[etherscan]entry for every network inthe set it is given, and scoping that set to the consumer does not help:
st0x.deploy deliberately has no
[etherscan]entry for Robinhood — chain 4663is not indexed by Etherscan v2, an entry would only make foundry demand an API
key it cannot use, and it verifies with
--verifier sourcify. So even overst0x's own five, the assertion fails on a config that is correct.
That direction wants its own set — the networks that HAVE an Etherscan
deployment, which is a property of the networks rather than of any consumer —
and is #259, not a scoping of this hook.
Interaction with #237
Open PR #237 (
fix-233) generates[rpc_endpoints]and[etherscan]fromLibRainDeploy.supportedNetworkConfigs()and DELETES bothtestSupportedNetworksAreFullyConfiguredandcheckNetworksConfigured. Twoconsequences if it lands after this:
RainDeployVerifySnapshotread this PR scopes goes away with theassertion, and with it
RainDeployVerifySnapshotNarrowNetworksTest, whosewhole subject is which networks that inherited test reads. The chain group's
two reads and
deployNetworks()are unaffected;argument that a repo able to narrow it would deploy to and verify fewer
chains with nothing red. This PR's answer to that argument is the single
hook: narrowing moves the deploy target and the verification set together,
so there is no spelling in which fewer chains are verified than are deployed
to. The two positions have to be reconciled when Generate the network config from the roster instead of comparing it #237 is merged rather than
silently resolved by merge order.
QA
testSupportedNetworksDefaultsToEveryRainNetwork,testDeployNetworksFollowsTheDeclaredNetworks,testChainMatrixForksOnlyTheDeclaredNetworks,testChainIdBindingReadsOnlyTheDeclaredNetworks,testConfigIsHeldToTheDeclaredNetworks— none can be run against base to fail there: all five bind or driveRainDeploySuitesBase.supportedNetworks(), which this PR adds, so the suite does not compile onorigin/main(63b0558) at all. Discrimination is shown by mutation instead — each line below is reverted to the shape base behaves as, and a named test fails. Baseline over all five was green first (forge exit 0).git checkout HEAD --and confirmed byte-clean bygit status --porcelain.RainDeployVerifySnapshot.testSupportedNetworksAreFullyConfigured:115:supportedNetworks()->LibRainDeploy.supportedNetworks()(the config group reads this package's nine again) ->testConfigIsHeldToTheDeclaredNetworks, onnext call did not revert as expected.RainDeployBroadcast.deployNetworks:70:supportedNetworks()->LibRainDeploy.supportedNetworks()(the broadcast ignores the declaration) ->testDeployNetworksFollowsTheDeclaredNetworks, onassertion failed: 9 != 1.RainDeploySuitesBase.supportedNetworks:237: the default's last network dropped (all.length - 1) ->testSupportedNetworksDefaultsToEveryRainNetwork, onassertion failed: 8 != 9.RainDeploySuitesBase.supportedNetworks:237: the default's first two entries swapped ->testSupportedNetworksDefaultsToEveryRainNetwork, onassertion failed: base != arbitrum. Two mutations on this line rather than one, because reverting this default toLibRainDeploy.supportedNetworks()IS the default — a no-op mutation proves nothing — and what the test claims is membership AND position, so each is broken separately.RainDeployVerifyChain.checkDeployedOnSupportedNetworks:171:supportedNetworks()->LibRainDeploy.supportedNetworks()(the matrix forks the nine again) ->testChainMatrixForksOnlyTheDeclaredNetworks, onthe matrix forked a network the declaration does not name.RainDeployVerifyChain.testSupportedNetworkChainIdsAreBound:285:supportedNetworks()->LibRainDeploy.supportedNetworks()->testChainIdBindingReadsOnlyTheDeclaredNetworks, onNetworkChainIdMismatch("arbitrum", 42161, 8453).BASE_RPC_URLonly) both mutants die instead atvm.createFork: environment variable `ARBITRUM_RPC_URL` not found— a real kill, and exactly the consumer symptom Verification is hardwired to supportedNetworks(), so a consumer that deploys to a subset cannot bind RainDeployVerify #246 describes, but NOT the discriminating assertion. So both were re-run with all nine[rpc_endpoints]aliases bound to the same base endpoint, which makes every fork reachable and leaves only an assertion to fail. The UNMUTATED code passes under that same environment (forge exit 0), and the two failures quoted above are what the mutants then produce: the fork-COUNT assertion (selectFork(1)succeeding, i.e. a second fork exists) and the chain-id comparison made on an alias the declaration does not name.LibRainDeploy.supportedNetworks()itself for the default, by membership and by position, since the deploy and the matrix both walk the list in order and the last entry is what each leaves selected. For the narrowed fixtures,basechosen deliberately as neither the first nor the last of the nine, so a reader that ignored the override lands onpolygon(last walked) orarbitrum(first) and neither reads asbase; base's chain id is the literal8453written out rather than read back through the alias, so an[rpc_endpoints]entry pointed at another chain cannot agree with itself. For the chain group, the fork COUNT and the chain LEFT SELECTED rather than the group passing — the subject isvm.etched persistent on every fork and this repo's[etherscan]ids are right on all nine, so a group that ignored the declaration and walked the nine would pass too, andselectFork(1)reverting is the world's answer to "is there a second fork". For the config group, the reverse-direction message namingarbitrum: every alias in this repo'sfoundry.tomlis one the library's list names, so that string is only producible by a check reading the declaration.RainDeployVerify; (b) that the named call sites be checked against the current tree rather than either list trusted —checkDeployedOnSupportedNetworks,testSupportedNetworkChainIdsAreBound,RainDeployVerifySnapshot.testSupportedNetworksAreFullyConfigured, and A consumer deploying to a subset of supportedNetworks() cannot bind RainDeployVerify #250'stestSuitesLiveOnEverySupportedNetwork; (c) that the broadcast side, already scopable, stay so. Covered: a, b, c. For (b), all four were located in the current tree and all four are now scoped —testSuitesLiveOnEverySupportedNetworkthroughcheckDeployedOnSupportedNetworks, which is its whole body, rather than separately — andgrep -rn "LibRainDeploy.supportedNetworks()" src/now returns exactly one hit, the hook's own default, so no production read is left hardwired. NOT covered, deliberately: the[etherscan]membership direction, which was split out of this issue into The [etherscan] config assertion needs the networks that have an Etherscan deployment, not the deploy roster #259 — it needs the networks that HAVE an Etherscan deployment, a property of the networks, which no scoping of this hook is.Checks run
Toolchain:
nix develop github:rainlanguage/rainix/8657b83b68f41957ab85da91132c3f652c1f32c0#sol-shell, forge 1.7.2-nightly (43923a4) — the one CI uses, not the repo's own devshell.forge build— clean.forge fmt --check— clean. It first flagged one over-width line intest/src/abstract/RainDeployBroadcast.t.sol;forge fmtwrapped it and the re-check is clean.forge lint— clean, no findings.BASE_RPC_URL=... forge test --match-contract RainDeployVerifyChainNarrowNetworksTest— 4 passed, 0 failed. Both inherited chain tests run on the narrowed set, which is the consumer-facing claim: the binding is bindable.forge test— 624 tests, 567 passed, 57 failed. Every one of the 57 isvm.createFork/vm.createSelectFork: environment variable <NETWORK>_RPC_URL not found; not one is an assertion failure. The tree reverted tomain(63b0558) and the same command re-run in the same environment gives 613 tests, 556 passed, and the identical 57 failures by test name. So this PR adds 11 passing tests and no failures.Not run
The nine-network fork suites. They need one
<NETWORK>_RPC_URLper network and onlyBASE_RPC_URLwas available, soARBITRUM_RPC_URL,BASE_SEPOLIA_RPC_URL,BSC_RPC_URL,ETHEREUM_RPC_URL,FLARE_RPC_URL,HYPEREVM_RPC_URL,POLYGON_RPC_URLandROBINHOOD_RPC_URLare what CI has to settle. That is the pre-existing local condition rather than anything this PR introduces, which is what the reverted-tree comparison above establishes.Why
testConfigIsHeldToTheDeclaredNetworksflips a flagThe only
foundry.tomlon disk is this repo's own and it names all nine, so a contract whose declaration was statically narrow would fail the very inherited test it exists to drive, every run. The narrowing is therefore something the test turns on, which makes the scoped failure reachable and leaves the default case passing beside it — and the default passing is what makes the narrowed failure discriminating, since a group reading the library's list passes both and a group that could not pass at all fails both.🤖 Generated with Claude Code