Skip to content

LibHexString.bytesToHex gets a test file - #37

Merged
thedavidmeister merged 3 commits into
mainfrom
2026-08-16-libhexstring-coverage
Aug 16, 2026
Merged

LibHexString.bytesToHex gets a test file#37
thedavidmeister merged 3 commits into
mainfrom
2026-08-16-libhexstring-coverage

Conversation

@thedavidmeister

@thedavidmeisterthedavidmeister commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

src/lib/LibHexString.sol had no test file. Its one function, bytesToHex, is
the only raw assembly pointer arithmetic in the repo: it takes the string
vm.toString(bytes) returns, moves the pointer forward two bytes, and rewrites
the length word two bytes lower so the leading 0x falls off. Every part of
that — the offset, the subtraction, the pointer reassignment, and the promise in
the memory-safe annotation — was unpinned.

This adds test/lib/LibHexString.bytesToHex.t.sol. Tests only; no source change.

What the tests pin

The emitted characters. Exact strings for a known vector, a zero byte, a
single high byte, leading zero bytes, both sides of the first word boundary, and
the end of the allocated buffer. Fuzzed: the result is exactly two characters per
input byte, every character is a lower case hex nibble, no x survives anywhere,
the result round trips back through vm.parseBytes to the input bytes, and the
result is vm.toString(data) with its first two characters removed and nothing
else touched.

The length property is load bearing rather than cosmetic. The output is spliced
straight into a hex"..." literal in generated source, and solc 0.8.25 rejects
an odd nibble count outright (hex"abc" → "Expected even number of
hex-nibbles"). An off-by-one in the subtraction emits source that does not
compile. Case is not load bearing — solc accepts hex"AABB" and even mixed case
hex"aAbB" — but stability under regeneration is, so the exact lower case output
is pinned.

Empty bytes, end to end.vm.toString(bytes("")) is "0x", so the whole
string is prefix and 2 - 2 is the tightest input the subtraction ever sees.
testBytesToHexEmpty reads the raw length word rather than comparing strings, so
an underflow to 2**256 - 2 cannot pass as "".
testBytesToHexEmptyReachesCallerAsEmptyHexLiteral follows it into
LibCodeGen.bytesConstantString and pins the emitted declaration as
bytes constant NOTHING = hex"";, which solc accepts.

The memory-safe promise. The annotation claims the block touches nothing
outside the string it was handed. Memory allocated before the call survives it,
memory allocated after it is not clobbered, the free memory pointer never moves
backwards, the caller's input bytes come back unchanged, and the returned string
lies entirely inside memory allocated during the call. The returned pointer is
deliberately not word aligned (ptr % 32 == 2), so the result is also forced
across an ABI boundary and through a string.concat copy at
data.length % 16 == 15 — the lengths where the payload ends flush with the end
of the buffer vm.toString allocated, which is where a copier reading past that
buffer would surface.

The annotation is a promise to the optimiser, so the suite was also run under the
pipelines a consumer might build this library with: --via-ir, and
--evm-version shanghai where there is no mcopy and the compiler falls back to
a word-rounded copy loop — the case where reading two bytes past the buffer would
actually happen. All 20 pass under each, and under --via-ir --evm-version paris.

QA

  • Discriminating tests: 20 new tests, all asserting exact values or exact memory
    facts rather than "it did not revert". Full suite 29 tests / 5 suites, 0
    failures; forge fmt --check clean. Every test passes on the clean tree, and
    each behaviour it claims to cover has a mutation below that it fails under.

  • Mutations applied: 18, one behaviour each, run with mutation-probe against
    the whole suite from a green 29-test baseline. 17 KILLED, 1 SURVIVED. Killer
    lists are the first five the probe reports, not the complete set:

    #mutationverdict / killed by
    M01add(hexString, 2)add(hexString, 0)KILLED — testBytesToHexAllocationBoundary, testBytesToHexCharset, testBytesToHexConcatenatesIntoHexLiteral, testBytesToHexHasNoPrefix, testBytesToHexIsVmToStringWithoutPrefix
    M02add(hexString, 2)add(hexString, 1)KILLED — same five
    M03add(hexString, 2)add(hexString, 3)KILLED — testBytesToHexAllocationBoundary, testBytesToHexCharset, testBytesToHexConcatenatesIntoHexLiteral, testBytesToHexIsVmToStringWithoutPrefix, testBytesToHexKnown
    M04add(hexString, 2)add(hexString, 32)KILLED — same five as M03
    M05sub(mload(hexString), 2)mload(hexString) (term dropped)KILLED — testBytesToHexAllocationBoundary, testBytesToHexCharset, testBytesToHexConcatenatesIntoHexLiteral, testBytesToHexEmpty, testBytesToHexIsVmToStringWithoutPrefix
    M06sub(…, 2)sub(…, 1) (odd nibble count)KILLED — same five as M05
    M07sub(…, 2)sub(…, 3)KILLED — testBytesToHexAllocationBoundary, testBytesToHexCharset, testBytesToHexConcatenatesIntoHexLiteral, testBytesToHexEmpty, testBytesToHexEmptyReachesCallerAsEmptyHexLiteral
    M08sub(…, 2)add(…, 2)KILLED — same five as M05
    M09length read from newHexString instead of hexStringKILLED — testBytesToHexAllocationBoundary, testBytesToHexCharset, testBytesToHexConcatenatesIntoHexLiteral, testBytesToHexEmpty, testBytesToHexHasNoPrefix
    M10the mstore deleted entirelyKILLED — same five as M09
    M11length written to the old pointerKILLED — same five as M09
    M12hexString := newHexStringhexString := hexStringKILLED — testBytesToHexAllocationBoundary, testBytesToHexConcatenatesIntoHexLiteral, testBytesToHexIsVmToStringWithoutPrefix, testBytesToHexKnown, testBytesToHexLeadingZeroBytes
    M13vm.toString(data)vm.toString(bytes("")) (input ignored)KILLED — same five as M12
    M14return hexString;return "";KILLED — same five as M12
    M15stray mstore(0x80, 0) — writes memory the block does not ownKILLED — testBytesToHexDoesNotMutateInput, testBytesToHexLeavesNeighbouringMemoryAlone, testBytesToHexLength, testBytesToHexLongData, testBytesToHexRoundTrips
    M16assembly ("memory-safe")assemblySURVIVED — see below
    M17free memory pointer moved backwardsKILLED — testBytesToHexAllocationBoundary, testBytesToHexCharset, testBytesToHexConcatenatesIntoHexLiteral, testBytesToHexEmpty, testBytesToHexHasNoPrefix
    M18caller's input bytes clobberedKILLED — testBytesToHexDoesNotMutateInput, testBytesToHexLongData, testBytesToHexRoundTrips

    M16 is a non-behaviour control and its survival is the correct verdict, not a
    gap: the annotation is a promise made to the optimiser, and removing it only
    makes the optimiser more conservative, so nothing observable changes at
    runtime. The promise itself is checked directly instead, by M15, M17 and M18 —
    each injects a distinct violation of what the annotation claims, and each dies.

  • Oracle: the generated text has to compile, so intent was derived from what solc
    accepts rather than from what the library returns. Checked against solc 0.8.25
    directly: hex"" compiles, hex"AABB" and mixed case hex"aAbB" compile,
    hex"abc" does not. That is what makes "exactly two characters per byte" the
    property worth pinning, and makes case a stability concern rather than a
    correctness one. The docstring's claim about vm.toString ("the leading 0x
    which is unconditionally added") is checked rather than assumed:
    testBytesToHexIsVmToStringWithoutPrefix asserts the prefix is present and the
    length is 2 * data.length + 2 for every fuzz input, and it holds — including
    for empty bytes, where vm.toString returns exactly "0x" and the subtraction
    lands on zero rather than underflowing.

  • Category check: the ask is coverage for LibHexString, and the category is
    every behaviour in it, not the interesting ones. The file is one function with
    six behaviours — the conversion call, the pointer offset, the length
    subtraction, the length store, the pointer reassignment, and the return — plus
    the memory-safe promise and the handoff to bytesConstantString. All eight are
    in the matrix above, each probed by at least one mutation. A second pass added
    input immutability and the buffer boundary; a third re-survey of the function
    surfaced no behaviour that is not already probed.

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

coderabbitaiBot commented Aug 16, 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:55 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: ce7482fd-c10f-4d03-aca3-b85c2e7b4b3a

📥 Commits

Reviewing files that changed from the base of the PR and between c72eb89 and 0814859.

📒 Files selected for processing (2)
  • test/concrete/LibHexStringExternal.sol
  • test/lib/LibHexString.bytesToHex.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.

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

Copy link
Copy Markdown
ContributorAuthor

Reviewed 0814859: ready. Tests only — test/lib/LibHexString.bytesToHex.t.sol and test/concrete/LibHexStringExternal.sol. This file had no test at all before.

18 mutants, 17 killed. The survivor is M16, removing the memory-safe annotation — a control rather than a gap. The annotation is a promise to the optimiser with no runtime observable, so its survival is the correct verdict, and the promise itself is probed directly by three other mutants that all die: a stray write to memory the block does not own, the free memory pointer moved backwards, and the caller's input clobbered.

The adversarial half refuted the candidate I seeded it with, on evidence rather than argument. vm.toString(bytes("")) is "0x", so sub(mload(hexString), 2) is 2 - 2 == 0 and not an underflow; over 2048 fuzz runs vm.toString(data) is always 2 * len + 2 characters and always 0x-prefixed, so the docstring's "unconditionally" holds. Empty data reaches the caller as bytes constant X = hex"";, which solc 0.8.25 accepts — it rejects an odd nibble count, which is what makes the length property matter, and it is case-insensitive.

The memory-safe annotation is honest, checked beyond this repo's own profile: all tests pass under --via-ir, under --evm-version shanghai (no mcopy, word-rounded copy loop — the case where a 2-byte over-read would actually show), and under --via-ir --evm-version paris. The returned pointer is unaligned at ptr % 32 == 2, which is unusual but not unsafe.

What survived as a finding is filed as #43 and deliberately not blessed here: the length arithmetic guards nothing against a caller-supplied Vm that returns fewer than two characters, and the failure mode is silent truncation rather than a revert.

Second helper contract in its own file under test/concrete/ rather than beside the tests, because rainix static enforces one contract per file — the first push went red on exactly that and it is fixed rather than worked around.

All five checks green. They ran before #36 merged, so this head has not been exercised against #36's new committed-artifact staleness test; the diff is two new test files touching neither filePrefix nor bytecodeHashConstantString, so it cannot affect it, and GitHub reports MERGEABLE/CLEAN.

CodeRabbit reports Review rate limited, so the 0 unresolved threads is vacuous rather than clean.

@thedavidmeister
thedavidmeister merged commit a003022 into mainAug 16, 2026
5 checks passed
Sign up for freeto 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.

2 participants

@thedavidmeister@claude