test: parse the literal the library emitted, not one the test formatted

[thedavidmeister]

Closes #62

What changed

testAddressConstantStringRoundTrips, testBytes32ConstantStringRoundTrips and
testUint8ConstantStringRoundTrips each ended in an assertion over
vm.toString(data) — a value the test formatted for itself. Those assertions
stated a property of foundry's own toString/parse* pair and never read the
library's output at all. Each now slices the literal out of the text the library
emitted and parses that.

The slicing is LibCodeGenSlow.betweenSlow, with indexOfSlow under it: the
text between the first open and the first close after it, reverting
NoSlice when either delimiter is missing, so text that does not carry the
literal fails loudly rather than yielding an empty slice that compares equal to
nothing.

The checksum claim

The address docstring claimed the emitted literal is checksummed "so it
round-trips back to the same address rather than silently relying on an
all-lowercase form". vm.parseAddress accepts either form, so no round trip can
state that. The claim itself is true and load bearing, so it is proven rather
than dropped: LibCodeGenSlow.checksumAddressSlow is an EIP-55 reference built
from the address's own bits rather than from vm.toString, and
testAddressConstantStringChecksummed fuzzes the emitted literal against it.

Measured rather than assumed. solc 0.8.25 on
address constant LOWER = address(0xc51a14251b0dcf0ae24a96b7153991378938f5f5);:

Error (9429): This looks like an address but has an invalid checksum. Correct
checksummed address: "0xc51a14251b0dcF0ae24A96b7153991378938f5F5". If this is
not used as an address, please prepend '00'.

An all-lowercase address in a generated file is a compile error in the consumer,
which is exactly what this library's output has to survive.

Decision and ledger

The issue offered two routes: slice the literal out of emitted and parse that,
or drop the assertion and cut the unsupported half of each docstring. Sliced,
and the checksum claim proven.

• Build cost: three reference helpers in test/lib/LibCodeGenSlow.sol
  (indexOfSlow, betweenSlow, checksumAddressSlow), one NoSlice error and
  one new fuzz test. 85 added lines, all under test/.
• Carrying cost: checksumAddressSlow is a second EIP-55 implementation that
  has to stay right; rows B, C and D of the matrix below are what holds it and
  the slicing. betweenSlow is delimiter based, so a change to the emitted
  wrapper (address(, bytes32() surfaces as NoSlice rather than as a silent
  pass.
• Removal cost: delete the three helpers and the four assertions that use them.
  Nothing in src/ depends on any of it.

Dropping instead would have been cheaper to carry and would have left the
library's most consequential output property — that the generated file compiles
— stated nowhere in the suite.

Not a stopgap, so no removal issue is filed.

QA

• Discriminating tests: testAddressConstantStringRoundTrips,
  testBytes32ConstantStringRoundTrips, testUint8ConstantStringRoundTrips
  (final assertion rewritten) and testAddressConstantStringChecksummed (new) -
  the finding is a dead assertion rather than a wrong result, so "fails on base"
  is verified against a gutted base: with the three emitters in
  src/lib/LibCodeGen.sol changed to return "";, the three assertions being
  replaced all PASS and all four replacements FAIL with NoSlice("", ...)
  (probe output in section 1 below).
• Mutations applied: checksumAddressSlow nibble > 7 -> nibble > 6 ->
  killed by testAddressConstantStringChecksummed (134/1 of 135);
  betweenSlow start = openIndex + openBytes.length -> start = openIndex ->
  killed by all four (131/4 of 135); indexOfSlow return i; ->
  return i + 1; -> killed by all four (131/4 of 135);
  addressConstantString vm.toString(data) ->
  vm.toString(address(uint160(data) ^ 1)) -> killed by both address tests
  (127/8); bytes32ConstantString vm.toString(data) ->
  vm.toString(data ^ bytes32(uint256(1))) -> killed by
  testBytes32ConstantStringRoundTrips (111/24); uint8ConstantString
vm.toString(data) -> vm.toString(uint256(data) ^ 1) -> killed by
  testUint8ConstantStringRoundTrips (126/9). Full table in section 4.
• Oracle: vm.parseAddress / vm.parseBytes32 / vm.parseUint read back the
  literal the library emitted, which is the inverse direction from the
  vm.toString the library used to write it; and LibCodeGenSlow. checksumAddressSlow, an EIP-55 reference built from the address's own bits
  and keccak256, never calling vm.toString. The EIP-55 rule itself is
  anchored to solc 0.8.25, which named
  0xc51a14251b0dcF0ae24A96b7153991378938f5F5 as the correct checksum for the
  lowercase literal it rejected (error quoted above).
• Category check: the issue asks for three things - make the round trip read the
  library's output at all three call sites, and either prove the checksum claim
  with an EIP-55 reference in LibCodeGenSlow or stop claiming it. Covered all
  three call sites plus the checksum, proven rather than dropped. No Refs.

All runs are nix develop -c ... from the flake.

1. Red — the three assertions pass with the library gutted

The finding is a dead assertion, not a wrong result, so the reproduction has to
isolate the assertion from the exact-text assertion that sits above it in the
same test. A throwaway test/lib/DeadAssertionProbe.t.sol held the three
original assertions verbatim in one contract and the four replacements in
another, and the three emitters in src/lib/LibCodeGen.sol were changed to
return "";.

nix develop -c forge test --match-path 'test/lib/DeadAssertionProbe.t.sol' --fuzz-runs 64

Ran 4 tests for test/lib/DeadAssertionProbe.t.sol:ProbeNewAssertionsTest
[FAIL: NoSlice("", "address(", ")"); ...] testProbeNewAddress(address) (runs: 0, μ: 0, ~: 0)
[FAIL: NoSlice("", "bytes32(", ")"); ...] testProbeNewBytes32(bytes32) (runs: 0, μ: 0, ~: 0)
[FAIL: NoSlice("", "address(", ")"); ...] testProbeNewChecksum(address) (runs: 0, μ: 0, ~: 0)
[FAIL: NoSlice("", "FUZZ = ", ";"); ...] testProbeNewUint8(uint8) (runs: 0, μ: 0, ~: 0)
Suite result: FAILED. 0 passed; 4 failed; 0 skipped; finished in 3.90ms

Ran 3 tests for test/lib/DeadAssertionProbe.t.sol:ProbeOldAssertionsTest
[PASS] testProbeOldAddress(address) (runs: 64, μ: 4177, ~: 4177)
[PASS] testProbeOldBytes32(bytes32) (runs: 64, μ: 4100, ~: 4100)
[PASS] testProbeOldUint8(uint8) (runs: 64, μ: 4083, ~: 4083)
Suite result: ok. 3 passed; 0 failed; 0 skipped; finished in 6.46ms

Ran 2 test suites: 3 tests passed, 4 failed, 0 skipped (7 total tests)

That is the issue's claim, measured: with LibCodeGen emitting nothing, the
three assertions being replaced still pass and all four replacements fail. The
library was restored with git checkout and the probe file deleted; neither is
in this diff.

2. Green — full suite after the change

nix develop -c forge test

Ran 9 tests for test/lib/LibCodeGen.addressConstantString.t.sol:LibCodeGenAddressConstantStringTest
[PASS] testAddressConstantStringChecksummed(address) (runs: 2048, μ: 93389, ~: 93380)
[PASS] testAddressConstantStringRoundTrips(address) (runs: 2048, μ: 60117, ~: 60117)
...
Suite result: ok. 9 passed; 0 failed; 0 skipped; finished in 3.22s

Ran 8 tests for test/lib/LibCodeGen.bytes32ConstantString.t.sol:LibCodeGenBytes32ConstantStringTest
[PASS] testBytes32ConstantStringRoundTrips(bytes32) (runs: 2048, μ: 76589, ~: 76589)
...
Suite result: ok. 8 passed; 0 failed; 0 skipped; finished in 3.22s

Ran 16 test suites in 3.23s (31.60s CPU time): 135 tests passed, 0 failed, 0 skipped (135 total tests)

origin/main at clone time ran 134 tests in 16 suites, so
testAddressConstantStringChecksummed is the one added and nothing regressed.

3. forge fmt --check

nix develop -c forge fmt --check exits 0 with no diff.

4. Mutation matrix

Each row: break exactly one line, run the whole suite at
--fuzz-runs 256, restore. Every row reports its own
... (135 total tests) line, so a harness error or a zero-match filter cannot
be read as "survived".

Row	Mutation	Tests that failed	Suite totals
A	none (baseline)	—	135 passed, 0 failed (135 total)
B	checksumAddressSlow: nibble > 7 → nibble > 6	testAddressConstantStringChecksummed	134 passed, 1 failed (135 total)
C	betweenSlow: start = openIndex + openBytes.length → start = openIndex	testAddressConstantStringChecksummed, testAddressConstantStringRoundTrips, testBytes32ConstantStringRoundTrips, testUint8ConstantStringRoundTrips	131 passed, 4 failed (135 total)
D	indexOfSlow: return i; → return i + 1;	the same four	131 passed, 4 failed (135 total)
E	LibCodeGen.addressConstantString: vm.toString(data) → vm.toString(address(uint160(data) ^ 1))	all 8 address tests, testAddressConstantStringChecksummed and ...RoundTrips among them	127 passed, 8 failed (135 total)
F	LibCodeGen.bytes32ConstantString: vm.toString(data) → vm.toString(data ^ bytes32(uint256(1)))	24 tests, testBytes32ConstantStringRoundTrips among them	111 passed, 24 failed (135 total)
G	LibCodeGen.uint8ConstantString: vm.toString(data) → vm.toString(uint256(data) ^ 1)	all 9 uint8 tests, testUint8ConstantStringRoundTrips among them	126 passed, 9 failed (135 total)

Rows B, C and D are the ones that matter for this change: they break exactly the
lines this PR adds, and every test they kill is one of the four new or rewritten
assertions. Nothing else in the suite touches checksumAddressSlow,
betweenSlow or indexOfSlow, so those rows isolate the new assertions the way
the probe in section 1 does.

Rows E, F and G break the library itself. They are killed by the pre-existing
exact-text assertions as well, so they do not isolate anything — they are here to
show the new assertions are not vacuous when the emitted value is wrong.

Each row re-applied the mutation with sed and re-checked it with grep before
running; the applied line is recorded per row. Every row reports
(135 total tests), so neither a harness error nor a zero-match filter can be
read here as "survived".

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 57 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: c50b5ad5-7030-4571-8fd7-af074101c610

📥 Commits

Reviewing files that changed from the base of the PR and between 935c725 and 30b6d83.

📒 Files selected for processing (4)

• test/lib/LibCodeGen.addressConstantString.t.sol
• test/lib/LibCodeGen.bytes32ConstantString.t.sol
• test/lib/LibCodeGen.uint8ConstantString.t.sol
• test/lib/LibCodeGenSlow.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]

@coderabbitai review

[coderabbitai]

⚠️ 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.