docs: checkCodeHash's rationale counts three entry points, not two

[thedavidmeister]

Closes #53

What was wrong

checkCodeHash's NatSpec in src/lib/LibMigrationRegistry.sol described its own
coverage as "Both entry points check" and closed on "the two cannot drift into
checking different things". The library has three entry points and all three call
it — applied, head and applyMigration.

The count reads as left over from a version with one reader and one writer. In a
repo where the comment is the specification and the correctness of a guard is
argued from it, a doc that miscounts what it covers is the failure mode that lets
a fourth entry point be added later without anyone noticing the sentence stopped
describing the file.

The word "both" was also load-bearing in the sentence's own argument: it
enumerated a read and a write to justify one shared function, which flattened two
distinct readers with distinct failure modes. applied answers a timestamp;
head answers a value that can never legitimately be zero — a failure mode
neither of the two enumerated cases described, and one the file already documents
separately in head's own NatSpec.

What changed

Comment only. No source, no test, no ABI, no bytecode change.

The rationale now enumerates all three entry points with the failure each one
would have against unknown code, and states the drift argument over three rather
than two. It also names why the guard is one function rather than a rule at each
call site, so an entry point added later has one place to call.

Verification

• forge fmt --check — passes.
• forge build — passes.
• Call sites confirmed from source: checkCodeHash is defined at line 93 and
  called at exactly three sites — applied (122), head (139),
  applyMigration (168). grep -rn checkCodeHash --include=*.sol returns the
  definition plus those three and nothing else.
• Swept the repo for the same defect elsewhere. The other entry-point count
  claims are accurate as written and were left alone: script/Build.sol has
  exactly two external entry points (run, cutRelease), and
  MigrationRegistry.head's "the two" refers to a reader and applyMigration,
  not to a call count. The miscount was isolated to this one block.
• No overlap with Correct checkCodeHash's stated justification in LibMigrationRegistry #104 (issue 52), which rewrites the per-function rationale of
  applied, head and applyMigration in the same file but does not touch this
  block. The wording here is framed on "unknown code" throughout, which stays
  consistent with the sharpened rationale that PR lands — under it, a value that
  is not a head is what occupying code returns, which is what this text says.

No test can cover comment text, so there is no test to add.

QA

• Discriminating tests: n/a — the diff changes only NatSpec comment text inside src/lib/LibMigrationRegistry.sol. Solidity comments are not compiled, so no test can observe the change and none can be written that fails on base. The existing coverage of the code the comment describes was run and is green: LibMigrationRegistryTest (23), MigrationRegistryAppliedTest (11), MigrationRegistryHeadTest (9), MigrationRegistryApplyMigrationTest (25) — 68 passed, 0 failed.
• Mutations applied: n/a — docs-only diff, no executable line to mutate. Every changed line is a /// comment; git diff touches nothing else.
• Oracle: the source itself, read independently of the comment. checkCodeHash is defined at src/lib/LibMigrationRegistry.sol:93 and grep -rn "checkCodeHash" --include=*.sol . returns exactly that definition plus three call sites — applied (122), head (139), applyMigration (168). Three, not the "both"/"two" the comment claimed. The per-entry-point failure modes now enumerated come from each function's own NatSpec and signature, not from the sentence being corrected: applied returns a uint256 timestamp, and head's NatSpec (131-135) already states that zero is the one value a head can never hold.
• Category check: the issue asks for one thing — the checkCodeHash rationale at lines 88-92 miscounting its own coverage — and that is fixed. Category swept rather than treating the cited line as exhaustive: grep -rn -iE "both entry|two entry|the two |entry point|three entry" over all *.sol and *.md found two other entry-point count claims, both verified accurate and deliberately left alone — script/Build.sol has exactly two external entry points (run() at 96, cutRelease() at 114), and MigrationRegistry.sol:158's "the two" refers to a reader and applyMigration, not to a call count. The miscount was isolated to this one block.

Checks run

• forge fmt --check — exit 0.
• forge build — exit 0.
• forge test --match-path "test/src/{lib,concrete}/*Migration*" — 68 passed, 0 failed.
• forge test --match-path "test/src/lib/*Snapshot*" — 25 passed, 0 failed. Run deliberately: this repo pins deployed addresses and code hashes, and these assert the generated snapshot shape and pins, so they are what would catch a comment edit somehow moving pinned bytecode. Nothing moved (bytecode_hash = "none", cbor_metadata = false).
• Full forge test — the only failures are 42 fork tests, all vm.createSelectFork: environment variable <NETWORK>_RPC_URL not found, from a fresh clone with no gitignored .env. Every FAIL line in the run names an _RPC_URL (42 of 42); there are zero failures of any other class. CI supplies these via rainix's rpc-preflight.

[coderabbitai]

Warning

Review limit reached

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

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 @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: 3b46c003-1471-4367-8d42-56eb1fe43338

📥 Commits

Reviewing files that changed from the base of the PR and between 86f8d96 and 187c8f6.

📒 Files selected for processing (1)

• src/lib/LibMigrationRegistry.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

• X
• Mastodon
• Reddit
• LinkedIn

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

[thedavidmeister]

Reviewed 187c8f6: ready — Closes #53. Comment-only, and I checked the count against the source rather than the PR body: checkCodeHash() is called by applied (line 122), head (139) and applyMigration (168). Three, not the two the docstring claimed.

The replacement names what each one would do wrong against unknown code, individually and correctly — applied branching a test on whatever timestamp that code returned, head handing back a value that is not a head, applyMigration recording somewhere nothing reads. No behaviour touched.

CI green (rainix static/legal/test), 0 unresolved threads — vacuous rather than clean, since the CodeRabbit check reports Review rate limited.