docs: @param tags for UnexpectedDeployedAddress - #118
Conversation
`error UnexpectedDeployedAddress(address expected, address actual)` had a single prose line and no `@param` tags, while every other error-declaring file in `src/` documents its error params. The error is raised at two points, both in `deployToNetworks` and so also reachable via `deployAndBroadcast`: before any fork, when the creation code derives an address other than `expectedAddress`, and after broadcasting, when the address deployed to is not `expectedAddress`. `actual` means a different thing at each, which is what went undocumented. The summary line is corrected rather than kept. "Thrown when the deployed address does not match the expected address" is false at the pre-fork site, where nothing has been deployed at all, so `@param` tags underneath it would have documented the parameters and left the wrong statement standing. `expected` is described from the signature — the caller-supplied `expectedAddress` — not as "the address the suite records", which is `RainDeployVerifySnapshot`'s notion and not what either site passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 12 minutes 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 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 configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
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 |
|
Reviewed f4381de: ready — Closes #70. Docs only, and I checked the claim it adds against the source: CI green, 0 unresolved threads — vacuous, CodeRabbit reports |
Closes #70
error UnexpectedDeployedAddress(address expected, address actual)insrc/lib/LibRainDeploy.solcarried a single prose line and zero@paramtags,while every other error-declaring file in
src/documents its error params with@param. This adds the tags.The two revert sites
The error is raised at two points, both inside
deployToNetworks(and soreachable through
deployAndBroadcast, which delegates to it):src/lib/LibRainDeploy.sol:399— before any fork, whenzoltuAddress(creationCode)derives an address other thanexpectedAddress.Nothing is deployed at this point.
src/lib/LibRainDeploy.sol:431— aftervm.stopBroadcast(), when the addressactually deployed to is not
expectedAddress.actualtherefore means something different at each site, which is what themissing
@paramtags left undocumented.Two corrections to the issue's proposed text
The issue's proposed fix is not applied verbatim. Its own collapsed verification
block identifies both problems, and the diff resolves them:
The summary line is corrected, not preserved. The proposed fix keeps
Thrown when the deployed address does not match the expected address.Theverification block states this line "is factually wrong for that site" —
nothing is deployed when the pre-fork check fires, so there is no deployed
address to mismatch. Adding
@paramtags underneath a false summary wouldhave left the documented defect in place. The summary now covers both sites
and says outright that nothing is deployed at the first.
expectedis described from the signature, not from a sibling file. Theproposed
The address the suite records.is flagged in the verificationblock as "imported phrasing from
RainDeployVerifySnapshot" — hereexpectedis the caller-supplied
expectedAddressargument, not necessarilysuite-sourced. Confirmed against both revert sites: each passes the
expectedAddressparameter through unmodified.QA
is no runtime behaviour a test could discriminate on, and Solidity has no
assertion surface over
@paramtags. The obligation this change has is theinverse one — that it changes NOTHING — which is what the bytecode equality
below establishes.
mutation produces a distinguishable program:
cbor_metadata = falseandbytecode_hash = "none"infoundry.tomlstrip metadata, so comments are notreachable from the compiled output at all. Verified, not assumed — see below.
were read in full (
src/lib/LibRainDeploy.sol:399and:431) and theargument each passes as
expectedtraced back to theexpectedAddressparameter of
deployToNetworks;deployAndBroadcastwas read and confirmed todelegate rather than raise the error itself. This is what caught both defects
in the issue's proposed text, which its own verification block had flagged.
@paramtags on both parameters and(b) the two distinct meanings of
actualdocumented. Both covered. Thesummary-line correction is not extra scope — the issue's verification block
records that line as "factually wrong for that site", so it is part of the
documented defect.
Verification
Comment-only change — no bytecode, address or code-hash pin can move. Verified
rather than assumed:
AddressRegistryandMigrationRegistrywere built atthis branch and at
main, and the sha256 of bothbytecode.objectanddeployedBytecode.objectis identical across all four pairs.The repo's own pin authority agrees:
RegistryDeploySnapshotTest,RainDeployVerifySnapshotTestandGeneratedSnapshotShapeTestare 27/27 green,including
testSnapshotMatchesSource, which asserts each recordedCREATION_CODEequalstype(X).creationCode.Full suite: 168 passed, 47 failed — every one of the 47 failing on a missing
*_RPC_URLenv var (no.envin this environment), which is the fork-testrequirement CLAUDE.md documents. Zero failures with any other cause; checked by
grepping every
[FAIL]line, not by sampling.forge fmt --checkandforge buildare clean, pre-commit hooks pass, and allnew comment lines sit within the file's 80-column comment convention.