test: construct the conforming half of the bytesToHex Vm-output property - #102
Conversation
`toStringReturn` was fuzzed independently of `data`, so a run reached the accept arm only if a random string happened to be `0x` plus exactly two characters per input byte. Measured over 2048 runs on each of seeds 1-4, that happened 0 times, so the accept arm of the stated bi-conditional was dead. The conforming string is now constructed from the fuzzed data length with a payload filled from a fuzzed `filler`, selected by a fuzzed `conforming` bool. Conformance is still read off the string's own characters, so a `filler` that coincidentally conforms still lands on the accept arm. Closes #59 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review. WalkthroughThe fuzz test now constructs correctly sized, ChangesHex string conformance test
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized test change constructs conforming VM output so both sides of the existing bytes-to-hex behavior contract are exercised; no actionable merge-blocking risk remains after normal checks and review. Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Closes #59
Problem
testBytesToHexRejectsEveryNonConformingVmOutputstated a bi-conditional in itsdocstring — for any
dataand any string aVmmight return, the library"either reverts or returns exactly that string with its first two characters
removed" — but fuzzed
toStringReturnindependently ofdata. Reaching theaccept arm therefore required a random string to land on exactly
data.length * 2 + 2characters AND start0x. It never did.Fix
The conforming string is constructed rather than waited for. A fuzzed
bool conformingselects which half of the property a run aims at; when it is set,the returned string is
0xplus adata.length * 2payload filled cyclicallyfrom the fuzzed
filler, so an accepted string is arbitrary in everythingexcept the two things the library actually checks (length and prefix).
Whether a string conforms is still read off its own characters, not off
conforming, so afillerthat coincidentally conforms is checked against theaccept arm rather than expected to revert.
Measured: the accept arm was dead, and is not now
Instrumented the accept arm with
vm.writeLine(temporary, not in the diff) andcounted how many of 2048 fuzz runs reached it, on four seeds:
8192 fuzz runs before the change reached the accept arm zero times. The "after"
column is also the positive control for the probe itself: same probe, same file,
same permissions, so the zeros are real zeros and not a probe that never fired.
Failing → passing
A dead arm cannot fail on the unmutated library, so the failure is demonstrated
against a library whose accept path is broken. Every mutant below was applied to
src/lib/LibHexString.sol, run in isolation with--match-test,--fuzz-seed 1.Before the change,
sub(len, 3)in place ofsub(len, 2)— a library thatstrips one character too many from every successful conversion:
After the change, the same broken library:
And on the unmutated library, after the change:
Mutation matrix
Test run in isolation,
--fuzz-seed 1, 2048 runs configured.src/lib/LibHexString.solmstore(newHexString, sub(len, 2))→sub(len, 3)let newHexString := add(hexString, 2)→add(hexString, 3)if eq(len, expectedLength)→if gt(len, 1)0x3078→0x3079(prefix constant)M4's post-fix failure is
[FAIL: UnexpectedHexString("0x/SS\\𑤉?<", 16)]— aconforming string was rejected, which is exactly the half of the property that
had no coverage. M3 mutates the reject path, is killed by both, and is here to
show the reject half was not weakened to buy the accept half.
Proof the mutants really executed, rather than a stale artifact or a zero-match
filter reading as "survived":
no-op
sedwas made to abort rather than report;runs: 2048and a matching test name, so thefilter matched a test and the fuzzer ran the full budget;
LibHexStringExternaldeployedBytecodesha256 (first 32 hex), recompiledfrom scratch per mutant, alongside the pre-fix result each one produced:
5305d03fd60433c68f9f5597ef4d232bruns: 2048, μ 4765957972c518e4dae29b5a24a5d3d0fc6ebcruns: 2048, μ 4765956480f249a4d1bd65bd43f744700f4a53runs: 2048, μ 476595Three different bytecodes, identical mean gas and identical verdict, because
every one of the 2048 runs took the revert path and the mutated instructions
were never reached.
The
runs: 4on killed rows is the fuzzer stopping at its first counterexample,not a short run.
Checks
nix develop -c forge test— 134 passed, 0 failed, 0 skipped, 16 suites.nix develop -c forge fmt --check— exit 0.git statusclean after every mutation pass.Departures from the issue's proposed fix
testBytesToHexRejectsEveryNonConformingVmOutput→testBytesToHexStripsOrRevertsForEveryVmOutput. The issue diagnosed "arejection test wearing a bi-conditional's name"; making the accept half live
fixes the behaviour, and the name has to follow it or the accept half stays
undiscoverable. Nothing else in the repo references the old name.
badVm→stubVm. Half the runs now hand the library aconforming
Vm, sobadVmnames the wrong thing.LibHexStringExternaldeploymentrather than after, so the string is finished before anything consumes it.
QA
testBytesToHexStripsOrRevertsForEveryVmOutput- fails on base behaviour three ways, each verified by running it against a library whose accept path is mutated (M1[FAIL: assertion failed: <mangled> != 𝋫], M2[FAIL: assertion failed], M4[FAIL: UnexpectedHexString("0x/SS\\𑤉?<", 16)]), where the pre-fixtestBytesToHexRejectsEveryNonConformingVmOutputpasses atruns: 2048on all three; independently measured with a temporaryvm.writeLineprobe, the pre-fix accept arm executed 0 times in 8192 runs (seeds 1-4) and the post-fix one executes 1021/991/1036/984 times per 2048.mstore(newHexString, sub(len, 2))->sub(len, 3)-> killed bytestBytesToHexStripsOrRevertsForEveryVmOutput(survived pre-fix);let newHexString := add(hexString, 2)->add(hexString, 3)-> killed bytestBytesToHexStripsOrRevertsForEveryVmOutput(survived pre-fix);if eq(shr(240, mload(add(hexString, 0x20))), 0x3078)->0x3079-> killed bytestBytesToHexStripsOrRevertsForEveryVmOutput(survived pre-fix);if eq(len, expectedLength)->if gt(len, 1)-> killed by both the pre-fix and post-fix test, confirming the reject half was not weakened. Each mutant was diffed against the pristine source before running, a no-opsedaborted rather than reported, every survived row carriesruns: 2048with a matching test name, and the recompiledLibHexStringExternaldeployedBytecodehashes differ per mutant (5305d03f…unmutated /7972c518…M1 /6480f249…M4).Vm.toString(bytes)- "0x" followed by exactly two characters per input byte - not the library.expectedLengthand the conformance predicate are computed in the test fromdata.lengthand from the returned string's own characters, and the expected result is built by copyingreturned[2..]into a fresh buffer, so nothing is read back offLibHexString.bytesToHex.test/src/lib/…after Move every .t.sol into the test/src/lib mirror tree #56) does not apply yet - Move every .t.sol into the test/src/lib mirror tree #56 is still open, so the file is attest/lib/LibHexString.bytesToHex.t.solonmain.Other checks
nix develop -c forge test: 134 passed, 0 failed, 0 skipped across 16 suites.nix develop -c forge fmt --check: exit 0, no diff.git statusclean on the restored tree after every mutation pass.Summary by CodeRabbit
Post-
Build.sol-removal sweep (2026-08-17)main(959d527) merged in. Unaffected by #138's removal — nothing cut.The conflict was not with the removal; it was with #105's refactor of this same
test file.
Conflict resolved, in one function.
mainhoisted the callee into animmutable iExternalbuilt once in the constructor and added avmReturninghelper for the stub
Vm. This branch still built both inline(
new LibHexStringExternal(),new NonConformingVm(...)). Resolved by keepingthis branch's semantics — the constructed conforming payload, the
conformingselector argument, and the rename to
testBytesToHexStripsOrRevertsForEveryVmOutput— on top ofmain's twohelpers. The diff vs
mainis now this one function and nothing else.The accept arm was re-measured on the merge commit, not assumed. A textual
resolution here could have left the constructed arm dead again, which is the
whole deliverable, so the M4 mutant from the matrix above was re-run against the
merged tree:
0x3078→0x3079insrc/lib/LibHexString.sol,--match-test testBytesToHexStripsOrRevertsForEveryVmOutput:Killed at run 7 with
conforming = truein the counterexample — that is theaccept arm executing.
src/lib/LibHexString.solrestored byte-identicalafterwards,
git status --porcelainempty.Overlap with #108, stated with both numbers, not resolved here. #108 adds a
hex-charset check to
bytesToHexand says so in its own body: with #108 merged,an arbitrary-byte payload is non-conforming almost always, so the arm this PR
constructs would land back in the reject arm and the measured ~1000/2048 split
would collapse to ~0 again. The fix belongs in this PR's filler — map each
fillerbyte into0-9a-frather than using it raw — and is not appliedhere, because on
maintodaybytesToHexaccepts any payload and the mappingwould narrow the property for no reason. Whichever of #102 and #108 lands second
carries it. They also collide textually: #108 edits the
conformspredicateinside this same function.
Suite on the merge commit:
Ran 19 test suites: 145 tests passed, 0 failed, 0 skipped— unchanged frommain, right for a rename-in-place.forge fmt --checkclean,git statusclean after the run.