Skip to content

Anchor the candidate snapshot to source on the broadcast path - #89

Merged
thedavidmeister merged 3 commits into
mainfrom
56-broadcast-source-anchor
Aug 16, 2026
Merged

Anchor the candidate snapshot to source on the broadcast path#89
thedavidmeister merged 3 commits into
mainfrom
56-broadcast-source-anchor

Conversation

@thedavidmeister

Copy link
Copy Markdown
Contributor

Fixes #56.

The defect

script/Deploy.sol is contract Deploy is RegistryDeploySuites, RainDeployBroadcast {}, so the whole deploy is RainDeployBroadcast.run(). It read the suite from the declaration — whose candidates take creationCode / storedDeployedAddress / storedBytecodeHash from src/generated/candidate/<Name>.sol — and handed it to LibRainDeploy.deployAndBroadcast.

The only guard in front of the CREATE2 is deployToNetworks' zoltuAddress(creationCode) != expectedAddress. Both sides come out of the SAME generated file, so it proves that file internally consistent and nothing more — exactly the property RainDeployVerifySnapshot's own NatSpec says "CANNOT catch a snapshot of the wrong contract".

The check that can catch it lived only on RainDeployVerifySnapshot, which inherits Test. RainDeployBroadcast cannot reach it, and run() -> suiteByName -> allSuites -> checkedCandidateSuites ran only the empty-list and duplicate-key checks. So the bytes broadcast to five chains were whatever the generated tree happened to hold, with nothing anywhere saying they belong to the contract this repo compiles.

The fix

One definition, both callers.

  • src/abstract/RainDeploySuitesBase.sol gains error CandidateSourceMismatch and checkCandidatesAnchoredToSource(), which reads the declaration through checkedCandidateSuites() and compares every candidate's recorded creation code against its sourceCreationCode.
  • RainDeployBroadcast.run() calls it FIRST — before a suite is selected and before DEPLOYMENT_KEY is read.
  • RainDeployVerifySnapshot loses its local error, checkAnchoredToSource and its checkCandidatesAnchoredToSource(candidates); testSnapshotMatchesSource calls the inherited one. A second spelling on the test side is a spelling the deploy does not run.

The anchor now takes no argument, because the broadcast has nothing to hand it. Its negative case is therefore a whole broken DECLARATION rather than a set built in a test — which is also the shape a repo holding a stale generated file is actually in:

  • test/abstract/SourceMismatchDeploySuites.sol — a genuinely anchored AddressRegistry candidate followed by a consistent snapshot of the wrong contract (MockDeployableV2's bytes, MockDeployable as source). Broken one LAST, so a loop that stops early is caught and the failure has to name the second key.
  • test/concrete/SourceMismatchDeploy.sol — that declaration plus RainDeployBroadcast, so run() itself is what the test drives.

ExternalDeploySuites gains the matching wrapper, and testNoCandidateReverts now also refuses an empty declaration through the anchor — the reader the broadcast uses.

Ordering, without a new env-var race

testRunRefusesToBroadcastACandidateThatIsNotItsSource writes no env var. SourceMismatchDeploySuites declares keys (anchored-candidate, mismatched-candidate) that nothing anywhere sets DEPLOYMENT_SUITE to, so suite selection is asserted to be a revert waiting to happen whatever that process-wide variable holds — and the revert that arrives from run() is the anchor's instead. testRunSelectsTheSuiteFromTheEnvBeforeTheKeyAndNeverDefaults already pins that the key is read after the suite, so before the suite is before the key.

That same existing test is the discriminating passing case: ExampleDeploy.run() still reaches suite selection, which it can only do by getting past an anchor with nothing to say about a declaration whose candidates are their own source.

QA

  • Discriminating tests: testRunRefusesToBroadcastACandidateThatIsNotItsSource (new), testWrongContractSnapshotCaughtBySource (rewritten to drive the declaration), testNoCandidateReverts (new assertion through the anchor) — each fails on base. None can COMPILE against base, because base has no no-argument checkCandidatesAnchoredToSource(), so each was verified failing against base BEHAVIOUR by a mutant that restores it: M1 deletes the run() anchor call, which is base's run() byte for byte, and the broadcast test then fails with UnknownDeploymentSuite instead of CandidateSourceMismatch. M3 (if (false)) restores base's absence of an executed anchor for testWrongContractSnapshotCaughtBySource, and M4 restores base's unguarded read for testNoCandidateReverts.
  • Mutations applied: 6 applied to src/abstract/, 6 killed, 0 survived. Every run reported 31 total tests, so a mutant cannot look survived because nothing ran.
    • RainDeployBroadcast.sol:run() -> delete checkCandidatesAnchoredToSource(); -> testRunRefusesToBroadcastACandidateThatIsNotItsSource
    • RainDeployBroadcast.sol:run() -> move that call AFTER suiteByName(...) -> testRunRefusesToBroadcastACandidateThatIsNotItsSource. This is the mutant that pins the ORDER rather than the presence: the call is still there, still runs, and the test still fails because the revert becomes UnknownDeploymentSuite.
    • RainDeploySuitesBase.sol -> i < candidates.length -> i < 1 -> testWrongContractSnapshotCaughtBySource + testRunRefusesToBroadcastACandidateThatIsNotItsSource
    • RainDeploySuitesBase.sol -> if (stored != source) -> if (false) -> the same two
    • RainDeploySuitesBase.sol -> checkedCandidateSuites() -> candidateSuites() -> testNoCandidateReverts
    • RainDeploySuitesBase.sol -> revert CandidateSourceMismatch(candidates[i]...) -> candidates[0]... -> testWrongContractSnapshotCaughtBySource + testRunRefusesToBroadcastACandidateThatIsNotItsSource
  • Oracle: the fixture's declared intent, and the compiler. SourceMismatchDeploySuites states that its second candidate records MockDeployableV2 while claiming MockDeployable as its source; the expected revert payload is spelled in the tests as keccak256(type(MockDeployableV2).creationCode) and keccak256(type(MockDeployable).creationCode), read from the compiler and never from the anchor's own output. The expected ORDERING comes from the issue and from RainDeployVerifySnapshot's pre-existing NatSpec ("the ONLY check that catches a snapshot of the wrong contract"), not from the new code.
  • Category check: the issue asks for (1) the error + anchor moved onto RainDeploySuitesBase, (2) run() calling it before the suite and the key, (3) RainDeployVerifySnapshot deleting the local error and both helpers with testSnapshotMatchesSource calling the inherited one, (4) the drive-a-broken-set cases replaced by a suites fixture whose candidate disagrees with its source. Covered (1), (2), (3), (4). The issue's list is illustrative, so the same category is covered where it stopped at examples: the empty-declaration refusal is now asserted through the anchor as well (testNoCandidateReverts), because checkedCandidateSuites() has become a broadcast-path reader, and README.md / CLAUDE.md are corrected where they attributed the source-anchor group to RainDeployVerifySnapshot.

Other verification

31/31 pass across RainDeployBroadcast.t.sol, RainDeploySuitesBase.t.sol and RainDeployVerifySnapshot.t.sol. Full suite: 167 pass, 47 fail — all 47 are vm.createSelectFork: environment variable *_RPC_URL not found, pre-existing on main in a shell without .env.

forge fmt --check exits 0, reuse lint reports full compliance (73/73 files), slither . analyses 49 contracts with 100 detectors and finds 0 results.

Scope note

The issue's finding is left where it stood on consequence. Stale bytes occupy the STALE bytes' CREATE2 address, so the intended address is untouched and remains deployable — recovery is regenerate and redeploy, at the cost of gas plus a stray contract. What this closes is that the irreversible, multi-chain, key-custody action ran with the repo's own stated only-check-that-catches-a-wrong-contract-snapshot unexecuted.

🤖 Generated with Claude Code

Fixes #56.

`RainDeployBroadcast.run()` deployed whatever `src/generated/candidate/*.sol`
held. The only guard in front of the CREATE2 is `LibRainDeploy` comparing the
recorded address to what the recorded creation code derives, and both sides come
out of the same generated file — so it proved that file internally consistent
and nothing more. The check that catches a snapshot of the wrong CONTRACT lived
on `RainDeployVerifySnapshot`, which inherits `Test` and which the broadcast
cannot reach.

Move the anchor onto `RainDeploySuitesBase` so there is ONE definition both the
broadcast and the tests run: `checkCandidatesAnchoredToSource()` reads the
declaration through `checkedCandidateSuites()` and is called at the top of
`run()`, before a suite is selected and before `DEPLOYMENT_KEY` is read.
`RainDeployVerifySnapshot` loses its local error and both helpers, and
`testSnapshotMatchesSource` calls the inherited one.

The negative case is now a whole broken DECLARATION rather than a set handed to
a helper, because the anchor takes no argument: `SourceMismatchDeploySuites`
declares a genuinely anchored candidate followed by a consistent snapshot of the
wrong contract, and `SourceMismatchDeploy` drives it through `run()`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@thedavidmeister thedavidmeister self-assigned this Aug 15, 2026
@coderabbitai

coderabbitai Bot commented Aug 15, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@thedavidmeister, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 40 minutes

Limit details: You’ve used all 1 included review currently available under your plan.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: eb97ca22-ca76-418f-8595-9319c319ec1f

📥 Commits

Reviewing files that changed from the base of the PR and between aca580c and c976e3a.

📒 Files selected for processing (11)
  • CLAUDE.md
  • README.md
  • src/abstract/RainDeployBroadcast.sol
  • src/abstract/RainDeploySuitesBase.sol
  • src/abstract/RainDeployVerifySnapshot.sol
  • test/abstract/ExternalDeploySuites.sol
  • test/abstract/SourceMismatchDeploySuites.sol
  • test/concrete/SourceMismatchDeploy.sol
  • test/src/abstract/RainDeployBroadcast.t.sol
  • test/src/abstract/RainDeploySuitesBase.t.sol
  • test/src/abstract/RainDeployVerifySnapshot.t.sol

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@thedavidmeister

Copy link
Copy Markdown
Contributor Author

End-to-end against the real script/Deploy.sol

The tests drive a fixture. This drives the actual deploy script, reproducing the trigger the issue names — edit src/concrete/AddressRegistry.sol, do NOT re-run script/Build.sol, then dispatch.

Control, clean treeDEPLOYMENT_SUITE=address-registry forge script script/Deploy.sol --sig 'run()':

└─ ← [Revert] vm.envUint: environment variable "DEPLOYMENT_KEY" not found
Error: script failed: vm.envUint: environment variable "DEPLOYMENT_KEY" not found

The anchor has nothing to say about a healthy tree, and the run proceeds to the key exactly as before.

Same command, after adding a uint256 public constant STALE_PROBE = 1; to AddressRegistry and not regenerating:

└─ ← [Revert] CandidateSourceMismatch("address-registry", 0xf7fcb46e49ae2a15cd94322b636522885ac112ec71801cf510e09306e7b93fb5, 0x8797d93e6a1631eff52fd0fde657a891744604001d30aba0a11abf6620e35e08)
Error: script failed: CandidateSourceMismatch("address-registry", 0x...fb5, 0x...e08)

The deploy refuses, naming the suite and both creation-code hashes. That it arrives instead of the DEPLOYMENT_KEY error the control produced is the ordering: the anchor ran before the key was read, so nothing reached deployAndBroadcast and no fork was opened.

script/Build.sol is deliberately not gated by this — it inherits RegistryDeploySuites and not RainDeployBroadcast — so the remedy (forge script script/Build.sol) still runs from the very tree the deploy now refuses. A guard that also blocked its own fix would be a worse defect than the one it closes.

Cost of the added work on this repo's real declaration: 5,173 gas, against foundry's script default of ~1.07e9 and no gas_limit in foundry.toml. It cannot push a broadcast out of gas.

@thedavidmeister

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 15, 2026

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@thedavidmeister

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 15, 2026

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@thedavidmeister

Copy link
Copy Markdown
Contributor Author

Reviewed 8b3c7e1: ready — Fixes #56. This is the one in today's batch that changes what the irreversible action does, and the diagnosis holds on its own terms.

The only guard in front of the CREATE2 was zoltuAddress(creationCode) != expectedAddress inside deployToNetworks, and both sides of that comparison come out of the same generated file — so it proves that file internally consistent and nothing more, which is exactly what RainDeployVerifySnapshot's own NatSpec already said it cannot catch. The check that CAN catch a consistent snapshot of the wrong contract lived on a contract inheriting Test, which the broadcast cannot reach. So the bytes going to five chains were anchored to nothing.

Moving the anchor onto RainDeploySuitesBase and calling it from run() before suite selection and before DEPLOYMENT_KEY is the right shape: one definition, both callers, and the test side loses its local copy rather than keeping a second spelling the deploy does not run.

The mutation matrix pins ORDER and not just presence — moving the call after suiteByName leaves it present and still running, and the test fails because the revert becomes UnknownDeploymentSuite. 6 of 6 killed, with the total test count reported per run so a mutant cannot read as survived because nothing executed.

The scope note is honest and I agree with it: stale bytes occupy the STALE bytes' address, so the intended address stays deployable and recovery is regenerate-and-redeploy at the cost of gas plus a stray contract. What this closes is that the check the repo itself calls the only one that catches a wrong-contract snapshot was not executed by the action that matters.

CI green, 0 unresolved threads — vacuous, CodeRabbit reports Review rate limited.

thedavidmeister and others added 2 commits August 16, 2026 13:17
…nchor

Resolved four conflicts, keeping the PR's invariant intact: ONE definition of
the source anchor on RainDeploySuitesBase, called by RainDeployBroadcast.run()
before suite selection and before DEPLOYMENT_KEY is read.

- src/abstract/RainDeploySuitesBase.sol: took main's NoDeployCandidates
  docstring (defers to checkedCandidateSuites, plus the releasedSuites note).
  checkedCandidateSuites' own docstring already names
  checkCandidatesAnchoredToSource, so the deferral resolves correctly.
- test/src/abstract/RainDeployBroadcast.t.sol: union of both sides' imports.
- test/src/abstract/RainDeploySuitesBase.t.sol: kept "EVERY reader" —
  testNoCandidateReverts now drives five readers, so main's "all four" is stale.
- test/src/abstract/RainDeployVerifySnapshot.t.sol: kept main's five new
  frozen-record tests; dropped wrongContractCandidate(), which this branch
  replaces with the SourceMismatchDeploySuites declaration.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The broadcast ordering test became
`testRunSelectsTheSuiteFromTheEnvBeforeTheKeyNeverDefaultsAndBroadcastsIt` on
main while this branch was open, leaving the anchor test's docstring naming a
function that no longer exists.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@thedavidmeister

Copy link
Copy Markdown
Contributor Author

Reviewed c976e3a: ready — conflicts resolved by merging main in, and the invariant this PR exists for is intact after the merge.

The merge

main merged INTO the branch at 73a010e (never rebased); aca580c was the tip. Four conflicts, resolved as:

  • src/abstract/RainDeploySuitesBase.sol — took main's NoDeployCandidates docstring from docs: NoDeployCandidates docstring names its real raise site #101. It defers the rationale to checkedCandidateSuites and adds the releasedSuites paragraph, and this branch had already rewritten checkedCandidateSuites' own docstring to name checkCandidatesAnchoredToSource, so the deferral lands on the right text rather than on the old RainDeployVerifySnapshot spelling.
  • test/src/abstract/RainDeployBroadcast.t.sol — union of both sides' imports (ExampleDeploySingleNetwork from test: drive run() through a successful broadcast #90, SourceMismatchDeploy + MockDeployable from here).
  • test/src/abstract/RainDeploySuitesBase.t.sol — kept this branch's "refused on EVERY reader". main's "all four readers" is a count this branch makes stale: testNoCandidateReverts now drives five, the fifth being the anchor itself.
  • test/src/abstract/RainDeployVerifySnapshot.t.sol — kept all five of test: drive checkFrozenSnapshotsReleased over a multi-file record #94's new frozen-record tests and dropped wrongContractCandidate(), which this branch replaces with the SourceMismatchDeploySuites declaration. That is the whole conflict: test: drive checkFrozenSnapshotsReleased over a multi-file record #94 added tests immediately in front of the helper this branch deletes.

One follow-up commit, c976e3a: #90 renamed the ordering test to testRunSelectsTheSuiteFromTheEnvBeforeTheKeyNeverDefaultsAndBroadcastsIt while this branch was open, so the anchor test's docstring was left naming a function that no longer exists. Repointed.

The invariant, checked on the merged tree

ONE definition, both callers, and the ordering:

  • error CandidateSourceMismatch and checkCandidatesAnchoredToSource() are each defined exactly once, on RainDeploySuitesBase. Grep over src/, test/ and script/ finds no second definition anywhere; the only other occurrences are call sites — RainDeployBroadcast.run(), RainDeployVerifySnapshot.testSnapshotMatchesSource, and ExternalDeploySuites' external wrapper, which forwards to the inherited one rather than restating it.
  • run() calls it on its first line, before suiteByName(...) and before vm.envUint("DEPLOYMENT_KEY").

Mutation matrix, re-run on the merged tree

Baseline for forge test --match-path "test/src/abstract/*" on c976e3a: 11 failing, 41 passing, 52 ran — the 11 are vm.createSelectFork: environment variable *_RPC_URL not found in a shell without .env. Every mutant run below also reported 52, so none can look survived because nothing ran.

line mutation result killing test
RainDeployBroadcast.sol run() delete checkCandidatesAnchoredToSource(); killed (12/40) testRunRefusesToBroadcastACandidateThatIsNotItsSource — revert becomes UnknownDeploymentSuite
RainDeployBroadcast.sol run() move that call AFTER suiteByName(...) killed (12/40) testRunRefusesToBroadcastACandidateThatIsNotItsSource — call still present, still executes, revert becomes UnknownDeploymentSuite("", ...)
RainDeploySuitesBase.sol i < candidates.lengthi < 1 killed (13/39) testRunRefusesToBroadcastACandidateThatIsNotItsSource + testWrongContractSnapshotCaughtBySource
RainDeploySuitesBase.sol if (stored != source)if (false) killed (13/39) the same two
RainDeploySuitesBase.sol checkedCandidateSuites()candidateSuites() killed (12/40) testNoCandidateReverts
RainDeploySuitesBase.sol revert CandidateSourceMismatch(candidates[i]…)candidates[0]… killed (13/39) the same two — payload names anchored-candidate instead of mismatched-candidate

6 applied, 6 killed, 0 survived. The second row is the one that pins the ORDER rather than the presence, and it is still killed after the merge.

Other verification

  • Full suite on the merge commit: 199 pass, 51 fail. Every one of the 51 names a missing *_RPC_URL; none is a behavioural failure. main at aca580c in the same shell: 200 pass, 51 fail, also all RPC. Compared rather than setting the env, as the numbers show.
  • The one-test delta is this PR's own: testCandidateAnchoredToSourcePasses and testSourceAnchorReachesEveryCandidate are removed and testRunRefusesToBroadcastACandidateThatIsNotItsSource added. Neither removal drops an assertion — the passing case is now the inherited testSnapshotMatchesSource over ExampleDeploySuites plus testCandidatesPresentAnswers, and reach-every-candidate is asserted by testWrongContractSnapshotCaughtBySource driving a two-candidate declaration with the broken one LAST. Diffing the full test-name sets against main shows no other test lost.
  • All five of test: drive checkFrozenSnapshotsReleased over a multi-file record #94's frozen-record tests present and passing on the merged tree: testFrozenSnapshotCheckReachesEveryRecordFile, testFrozenSnapshotUnreadableFileIsNamedAtEveryRecordPosition, testFrozenSnapshotCheckReachesEveryReleasedSuite, testFrozenSnapshotEmptyDeclarationFailsOnTheFirstRecordFile, testFrozenSnapshotEmptyRecordPasses.
  • forge fmt --check exit 0. slither . — 49 contracts, 100 detectors, 0 results. reuse lint — compliant, 78/78 files.
  • Docs read end to end rather than diffed: README's four-group table and the paragraphs under it, and CLAUDE.md's group list, both describe the code as it now stands — group 2 attributed to RainDeploySuitesBase with the broadcast calling it, groups 1 and 3 still RainDeployVerifySnapshot. No leftover sentence anywhere attributes the source anchor to the verification abstract. No dangling doc reference to a renamed or deleted function remains, checked by extracting every backticked identifier in src/, test/ and script/ and differencing it against the declared function names.
  • CodeRabbit reports Review rate limited — a green check with no review behind it, so its zero unresolved-thread count is vacuous and this verdict rests on the read above rather than on it.

CI on this head

rainix / test and rainix / legal pass. rainix / static fails, and the failure is not this branch's: it is the new agent-context byte cap (rainlanguage/rainix#298), which rejects CLAUDE.md at 24946 bytes against a 4096-byte cap. main fails identically at its own tip — ERROR: this repo loads 24227 bytes of agent context … 20131 over the 4096-byte cap, same single file — so every rain.deploy ref is red on it right now. Trimming ~20KB of CLAUDE.md is its own piece of work and is not in this PR's scope. Merging with --admin over that one red.

Checked against issue #56 as filed: the four things it asks for are all present, and the ordering it specifies is the one the second mutant pins.

@thedavidmeister
thedavidmeister merged commit fdd4b13 into main Aug 16, 2026
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Broadcast path never anchors the candidate snapshot to source, so a stale generated file is deployed to five chains

1 participant