Pin the reject side of the beacon dirty-bits gate - #140
Conversation
The suite pinned the accept side of `_tryGetAddress`'s dirty-bits boundary (`2 ** 160 - 1` via testMatchesAtMaxAddressBoundary) but never the reject side, so widening the gate past 160 bits shipped green: `raw > type(uint160).max` -> `type(uint168).max` survived the whole default-profile suite (377 passed, 0 failed, identical to baseline). Two additive fixtures pin the reject-side partition: - OneAboveMaxAddressBeacon returns exactly `2 ** 160`, the first value the gate must reject; a widened gate truncates it to `address(0)`. - DirtyUpperByteBeacon returns a real address with only bits 160-167 set above it; a gate widened to 168 bits or more truncates it to the embedded address. One test per fixture per public predicate (`isBeaconOwner`, `isBeaconImplementationBytecode`), each asserting false against exactly the value the truncating mutant would match. All four fail under the widened gate and pass on main. No src changes. Closes #124 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 57 minutes Limit details: You’ve used the included review currently available. 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 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 within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
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 |
Pins the reject side of the beacon dirty-bits gate
(
raw > type(uint160).maxinLibExtrospectERC1967BeaconProxy._tryGetAddress), per the issue's proposedfix. The suite pinned the accept side (
2 ** 160 - 1viatestMatchesAtMaxAddressBoundary) but nothing pinned2 ** 160, the firstvalue the gate must reject, so widening the gate past 160 bits shipped green.
Purely additive:
test/concrete/OneAboveMaxAddressBeacon.sol— fixture whoseimplementation()andowner()return exactly2 ** 160; a widened gatetruncates it to
address(0).test/concrete/DirtyUpperByteBeacon.sol— fixture returning a real addresswith only bits 160-167 set above it; a gate widened to 168 bits or more
truncates it to the embedded address.
isBeaconOwner,isBeaconImplementationBytecode), each asserting false against exactly thevalue a truncating gate would report as a match.
No
src/changes. The issue's masking caveat (default-profile kills beingabsorbed by the compiler-output pins of #84) no longer applies: those tests
have left this repo, and the widening mutant now survives the default profile
on main outright — verified below.
QA
Discriminating tests:
LibExtrospectERC1967BeaconProxyIsBeaconOwnerTest.testReturnsFalseAtOneAboveMaxAddressBoundaryLibExtrospectERC1967BeaconProxyIsBeaconOwnerTest.testReturnsFalseOnDirtyUpperByteAboveRealAddress(fuzz)LibExtrospectERC1967BeaconProxyIsBeaconImplementationBytecodeTest.testReturnsFalseAtOneAboveMaxAddressBoundaryLibExtrospectERC1967BeaconProxyIsBeaconImplementationBytecodeTest.testReturnsFalseOnDirtyUpperByteAboveRealAddressMutations applied (real edits to
src/lib/LibExtrospectERC1967BeaconProxy.sol, default profile, fork fileexcluded for missing RPC env):
raw > type(uint160).max->raw > type(uint168).max(the issue'swidening mutant). Before this PR's tests: 377 passed, 0 failed —
SURVIVED, identical to the clean baseline. With this PR's tests:
377 passed, 4 failed — KILLED, the 4 failures being exactly the 4
discriminating tests above.
if (ok == 0 || raw > type(uint160).max)->if (ok == 0)(dirty-bitsclause deleted). With this PR's tests: 374 passed, 7 failed — KILLED: all 4 discriminating tests plus 3 existing dirty-address tests (
testReturnsFalseOnInvalidReturnEncodingfuzz in both suites andtestFalseOnDirtyAddressin thebeaconImplementationsuite).Oracle: the function's own
@dev— 32-byte returndata whose upper 12 bytesare non-zero is one of the folded failure modes, so the gate is a boundary at
2 ** 160and each side needs a pin. Expected values in the new tests arechosen adversarially: each equals exactly what the truncating mutant would
report as a match, so a false there separates rejection from truncation.
Category check: additive test-only change; no behavior change, no verdict
change. Clean-source run after adding the tests: 381 passed, 0 failed
(fork file excluded; its 3 tests fail locally only on missing
ARBITRUM_RPC_URL, which is environmental).forge fmt --checkclean on allfour touched files.
Closes #124
🤖 Generated with Claude Code