Replace the inlined implementation copy in the identifier fuzz oracle

[thedavidmeister]

Closes#60

testRequireContractNameAcceptedNamesAreIdentifiers restated
LibCodeGen.requireContractName's own range comparisons character for
character. A copy of the implementation is not an oracle: it agrees with the
implementation by construction, so it cannot see a design error that is present
in both. The file already carries an independent oracle in
LibCodeGenSlow.isContractNameSlow, which decides by membership of an alphabet
spelled out character by character. This test now uses it.

Path note: PR #56 is still open, so on main today the file is
test/lib/LibCodeGen.requireContractName.t.sol, and that is where this is
edited.

The defect is silence, not redness

This finding cannot be reproduced as a test that goes red — the defect is that
the test stays green when it should be red. The discriminating experiment is
the paired edit a copy invites: move a boundary in the library, and reflexively
move the copy in the test to match. Both experiments below were run against
main's test body, before the fix.

Paired mutation 1 — library and inlined copy both drop the && i > 0 guard,
so a leading digit is accepted:

[PASS] testRequireContractNameAcceptedNamesAreIdentifiers(string) (runs: 2048, μ: 6744, ~: 6307) <-- blind
[FAIL: next call did not revert as expected; counterexample: args=[0x39]] testRequireContractNameMatchesAlphabet(bytes) (runs: 936)
[FAIL: next call did not revert as expected] testRequireContractNameEveryLeadingByte()
[FAIL: next call did not revert as expected] testRequireContractNameRejectsLeadingDigit()
Suite result: FAILED. 12 passed; 3 failed; 0 skipped

The independent oracle catches it with the counterexample "9". The copy does
not, over all 2048 runs.

Paired mutation 2 — library char <= 0x5A becomes 0x5B and the copy's
char <= "Z" becomes char <= "[", so [ is accepted:

[PASS] testRequireContractNameAcceptedNamesAreIdentifiers(string) (runs: 2048, μ: 6831, ~: 6345) <-- blind
[FAIL: next call did not revert as expected] testRequireContractNameEveryLeadingByte()
[FAIL: next call did not revert as expected] testRequireContractNameEveryTrailingByte()
[FAIL: next call did not revert as expected] testRequireContractNameRangeBoundaries()
[FAIL: ... args=[0x466f6f00, 17, 91]] testRequireContractNameRejectsOneBadByte(bytes,uint256,uint8) (runs: 206)
Suite result: FAILED. 11 passed; 4 failed; 0 skipped

After the fix there is no copy left to move, so the same library-only mutations
are exactly M2 and M4 in the matrix below — both killed.

Mutation matrix

Every entry mutates one line, runs only
testRequireContractNameAcceptedNamesAreIdentifiers, then restores from HEAD.
[fuzz] runs = 2048.

#	mutated line	verdict	counterexample	runs to kill
M0	none (baseline)	test passed	–	2048
M1	the line this PR adds — oracle condition inverted	KILLED	""	0
M2	LibCodeGen drops && i > 0 (leading digit allowed)	KILLED	"9"	49
M3	LibCodeGen drops the empty-name revert	KILLED	""	34
M4	LibCodeGen letter range 0x5A → 0x5B	KILLED	"["	854
M5	LibCodeGen drops $ from the accepted set	KILLED	"ABC…xyz_$0123456789"	111

M2 and M4 are the two design errors the pre-fix test was blind to above.

Proof the suite actually ran, rather than a filter matching nothing: the harness
asserts a [PASS]/[FAIL] line for the target test name appears in forge's
output, and reports HARNESS-ERROR-OR-ZERO-MATCH otherwise. It never did — every
row above carries a real counterexample and a run count. The first version of
that check demanded exactly one matching line, but forge prints each [FAIL]
line twice (suite body and summary), so it mislabelled all five kills; the check
was corrected and the entire matrix re-run from scratch, with identical verdicts
and counterexamples both times.

Why this test is kept rather than deleted

Once the oracle is shared, this test's body is identical to
testRequireContractNameMatchesAlphabet; only the fuzzed type differs
(string vs bytes). That difference is load-bearing, so I measured it rather
than asserting it — instrumented copies of both generators, 2048 runs each,
counting names the reference alphabet accepts:

generator	accepted names (run 1)	accepted names (run 2)	accepted lengths seen
string	56 / 2048	65 / 2048	1, 2, 3, 54, 64
bytes	13 / 2048	11 / 2048	1, 54, 64

The string generator reaches the accepted half of the domain several times
more often, and is the only one of the two that produced accepted names of
length 2 and 3. (Lengths 54 and 64 are the two alphabet constants themselves,
fed back in from the fuzz dictionary.) The docstring now states this as the
reason the test exists alongside its bytes twin.

Where the issue's proposed fix was adjusted

• The issue proposed replacing the body with a bare
  if (LibCodeGenSlow.isContractNameSlow(name)) {. I used the file's existing
  assertAccepted/assertRejected helpers for the two branches instead of
  re-spelling the vm.expectRevert(abi.encodeWithSelector(...)) longhand, which
  is what the neighbouring testRequireContractNameMatchesAlphabet already does.
• The issue's docstring was left describing an inlined byte walk. Rewritten to
  describe what the test now does and why its generator is a string.
• The issue reported the accept branch as thin (101/2048) and implied the bytes
  domain is what makes the two tests differ. Measured, the bytes domain does
  reach accepts too (11-13/2048): the difference is density and length, not
  presence. Stated that way in the docstring rather than overclaiming.

QA

• Discriminating tests: testRequireContractNameAcceptedNamesAreIdentifiers - it
  does NOT fail on base, and that is the finding: on base its oracle is a copy of
  the implementation, so it agrees by construction. Verified by running two paired
  mutations (library boundary moved AND the test's copy moved to match) against
  base: the test PASSED all 2048 runs both times, while
  testRequireContractNameMatchesAlphabet (counterexample 0x39),
  testRequireContractNameEveryLeadingByte, testRequireContractNameEveryTrailingByte,
  testRequireContractNameRangeBoundaries, testRequireContractNameRejectsLeadingDigit
  and testRequireContractNameRejectsOneBadByte FAILED. With the fix applied, the
  same mutations kill it (M2, M4 below). Full suite on the branch head:
  nix develop -c forge test = 134 passed, 0 failed, 16 suites.
• Mutations applied: (1) test/lib/LibCodeGen.requireContractName.t.sol:138
if (LibCodeGenSlow.isContractNameSlow(name)) -> condition inverted -> killed by
  testRequireContractNameAcceptedNamesAreIdentifiers, counterexample "", run 0.
  (2) src/lib/LibCodeGen.sol:51(isDigit && i > 0) -> isDigit -> killed by the
  same test, counterexample "9", run 49. (3) src/lib/LibCodeGen.sol:43-45 the
  empty-name revert -> deleted -> killed, counterexample "", run 34.
  (4) src/lib/LibCodeGen.sol:48char <= 0x5A -> char <= 0x5B -> killed,
  counterexample "[", run 854. (5) src/lib/LibCodeGen.sol:50
char == 0x5F || char == 0x24 -> char == 0x5F -> killed, counterexample
  "ABC...xyz_$0123456789", run 111. 5 applied, 5 killed, 0 survived; each run
  restored from HEAD and the tree verified clean afterwards. The harness asserts
  a [PASS]/[FAIL] line for the target test name is present in forge output,
  so a zero-match filter or compile error cannot read as "survived"; the whole
  matrix was run twice end to end with identical verdicts and counterexamples.
• Oracle: LibCodeGenSlow.isContractNameSlow, which decides by membership of
  SLOW_HEAD_ALPHABET/SLOW_TAIL_ALPHABET — the identifier alphabet spelled out
  character by character in the test tree, never importing or restating the
  char >= 0x41 && char <= 0x5A-style range arithmetic LibCodeGen decides with.
  Before this PR the oracle was a character-for-character copy of that arithmetic.
• Category check: issue asks for one thing — replace the inlined implementation
  copy at lines 134-145 with the independent isContractNameSlow oracle while
  keeping the string generator; covered exactly that. The issue's incidental
  observation about the thin accept branch (101/2048) was measured rather than
  acted on: 56 and 65 accepts per 2048 for string vs 13 and 11 for bytes,
  recorded in the docstring as the reason the generator stays a string. No
  other file, test or behaviour touched.
• nix develop -c forge fmt --check — exit 0, no diff.

[coderabbitai]

Warning

Review limit reached

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

Next review available in:34 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: a3c9c7c0-dbde-4d0a-ad07-c9953b0e86d6

📥 Commits

Reviewing files that changed from the base of the PR and between 1bcf2f9 and faf0eae.

📒 Files selected for processing (1)

• test/lib/LibCodeGen.requireContractName.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

• X
• Mastodon
• Reddit
• LinkedIn

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