Reject codeless accounts in address-taking absence checks - #136
Conversation
Issue #132 (ruling E+D on #55): no absence check may answer "no code" as a pass. This commit adds the discriminating tests plus the minimal scaffold they need to compile: the `CodelessAccount(address)` error and the address-taking metamorphic entry points, deliberately WITHOUT the codeless guard, so the new tests fail behaviourally: - testCheckNoMetadataRevertsOnCodelessAccount - testCheckNotMetamorphicAddressRevertsOnCodelessAccount - testScanMetamorphicRiskAddressRevertsOnCodelessAccount `nix develop -c forge test`: 374 passed, 6 failed — the 3 above plus the 3 fork tests needing ARBITRUM_RPC_URL (environmental, fail on main too). Also pins the empty-input meaning of the non-reverting predicates (isEOFBytecode, isERC1167Proxy, both opcode scans) with deterministic tests at both the empty-literal and codeless-account readings; those pass already. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Implements issue #132 (ruling E+D on #55): no absence check may answer "no code" as a pass. An empty account is the maximally metamorphic state — an unoccupied CREATE2 target, a self-destructed account between incarnations, an EOA that can gain code by EIP-7702 delegation — and the chain cannot distinguish "can never gain code" from "can gain anything", so only the address boundary can refuse to vouch for it. - `checkNoSolidityCBORMetadata(address)` gains the `code.length == 0` guard and reverts `CodelessAccount(account)`; no new API. - `scanMetamorphicRisk(address)` and `checkNotMetamorphic(address)` are new entry points that revert `CodelessAccount(account)` on a codeless account, then delegate to the bytes functions (the metamorphic half of #83). - Bytes-taking functions stay total over bytes; their NatSpec now cross-references the address entry points. - Non-reverting predicates (isEOFBytecode, isERC1167Proxy, both opcode scans) state their empty-input meaning explicitly in NatSpec. - IExtrospectV1 and README updated to describe the new behaviour. `nix develop -c forge test`: 377 passed, 3 failed — the 3 fork tests needing ARBITRUM_RPC_URL, environmental, identical on main. Closes #132 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 4 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 (12)
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 |
scanMetamorphicRisk(address) returns a bare uint256 with an explicit return statement, matching every other function in the PR and the org's types-only returns rule. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Closes #132
Implements ruling E+D on #55 exactly as #132 scopes it: no absence check may answer "no code" as a pass. An empty account is the maximally metamorphic state — an unoccupied
CREATE2target, a self-destructed account between incarnations, an EOA that can gain code by EIP-7702 delegation — and the chain cannot distinguish "can never gain code" from "can gain anything", so only the address boundary has the information to refuse to vouch for it.What changed
LibExtrospectBytecode.checkNoSolidityCBORMetadata(address)gains thecode.length == 0guard and reverts with the newCodelessAccount(address)error carrying the address. No new API for this function.LibExtrospectMetamorphicgains the address-taking entry pointsscanMetamorphicRisk(address)andcheckNotMetamorphic(address): both revertCodelessAccount(account)on a codeless account, then delegate to the bytes functions. This is the metamorphic half of No address-taking metamorphic or opcode scan exists, so no verdict is bound to an account #83; the opcode-scan half of No address-taking metamorphic or opcode scan exists, so no verdict is bound to an account #83 stays open.scanMetamorphicRisk(bytes)/checkNotMetamorphic(bytes)keep the behaviour documented and pinned in Document and pin the empty-bytecode metamorphic verdict #96; their NatSpec now cross-references the address entry points as the way to bind a verdict to an account.isEOFBytecode,isERC1167Proxy,scanEVMOpcodesPresentInBytecode,scanEVMOpcodesReachableInBytecode) now state their empty-input meaning explicitly in NatSpec, pinned by deterministic tests at both the empty-literal and codeless-account (codeless.code) readings.IExtrospectV1NatSpec updated:checkNoSolidityCBORMetadatano longer documents "returns nothing ... including an account with no code" — it documents theCodelessAccountrevert — and the error roster gainsCodelessAccount. No function was added to or removed from the interface.Out of scope, untouched: #54 (EIP-7702 designator vs the metamorphic scan is a separate bytes-level defect;
isEOFBytecodematching is unchanged).TDD
Failing-tests commit d91665a adds the discriminating tests plus the minimal scaffold they need to compile (
CodelessAccountdeclared, address entry points present deliberately WITHOUT the guard).nix develop -c forge test: 374 passed, 6 failed — the 3 new codeless tests failing "next call did not revert as expected", plus the 3 fork tests needingARBITRUM_RPC_URL(environmental, identical on main).Implementation commit aaa05ea adds the guards and docs.
nix develop -c forge test: 377 passed, 3 failed — only the environmental fork tests.QA
Discriminating tests:
testCheckNoMetadataRevertsOnCodelessAccount— codeless account revertsCodelessAccount(0xdead)fromcheckNoSolidityCBORMetadata(flips the previoustestCheckNoMetadataEmptyAccount, which pinned the pass).testScanMetamorphicRiskAddressRevertsOnCodelessAccount,testCheckNotMetamorphicAddressRevertsOnCodelessAccount— codeless account revertsCodelessAccount(0xC2)from both address entry points, while the bytes entry points still pass the same account's empty code (testScanMetamorphicRiskCodelessAccount,testCheckNotMetamorphicCodelessAccount, unchanged from Document and pin the empty-bytecode metamorphic verdict #96).testScanMetamorphicRiskAddressClean/testCheckNotMetamorphicAddressClean(pass),testScanMetamorphicRiskAddressSelfdestruct/testCheckNotMetamorphicAddressRevertsOnSelfdestruct(risk bit /Metamorphicrevert),test*AddressRevertsOnEOF(EOFBytecodeNotSupportedthrough the delegate), andtest*AddressEquivalenceFuzz(address vs bytes agreement over nonempty non-EOF etched code).testIsEOFBytecodeCodelessAccount,testIsERC1167ProxyEmpty,testIsERC1167ProxyCodelessAccount,testScanEVMOpcodesPresentCodelessAccount,testScanEVMOpcodesReachableCodelessAccount(new, deterministic), alongside the pre-existing empty-literal tests.Mutations applied (each applied to the working tree,
nix develop -c forge test --no-match-path "*fork*"run in full, then reverted; baseline for that command is 377 passed, 0 failed):src, then reverted)checkNoSolidityCBORMetadata: codeless guard deletedtestCheckNoMetadataRevertsOnCodelessAccount("next call did not revert as expected"); 376 passed, 1 failedCodelessAccount(address(0))instead of the accountCodelessAccount(0x0000...0000) != CodelessAccount(0x0...dEaD)— the error's address payload is pinned; 376 passed, 1 failedscanMetamorphicRisk(address): codeless guard deletedtestScanMetamorphicRiskAddressRevertsOnCodelessAccountandtestCheckNotMetamorphicAddressRevertsOnCodelessAccount(the check inherits the scan's guard); 375 passed, 2 failedscanMetamorphicRisk(address): delegation dropped (riskyOpcodes = 0)testScanMetamorphicRiskAddressSelfdestruct,testCheckNotMetamorphicAddressRevertsOnSelfdestruct, both*AddressRevertsOnEOFtests, both*AddressEquivalenceFuzztests; 371 passed, 6 failedcheckNotMetamorphic(address):Metamorphicrevert made unreachable (riskyOpcodes != riskyOpcodes)testCheckNotMetamorphicAddressRevertsOnSelfdestructandtestCheckNotMetamorphicAddressEquivalenceFuzz; 375 passed, 2 failedbytecode.length == 0->== 1*AddressRevertsOnCodelessAccounttests (length 0 no longer reverts) and both*AddressEquivalenceFuzztests with a 1-byte-code counterexample (args=[0xa9]revertingCodelessAccount(0xbEEF)); 373 passed, 4 failedAll 6 mutants killed, 0 survivors.
Oracle: the ruling text on checkNotMetamorphic reports an account with no code as NOT metamorphic #55 (checkNotMetamorphic reports an account with no code as NOT metamorphic #55 (comment)) as scoped by Account-taking absence checks reject codeless accounts (ruling E+D on #55) #132, read independently of the implementation: every address-taking absence check must revert on a codeless account with a typed error carrying the address; bytes functions stay total; non-reverting predicates document their empty-input meaning. Expected revert data in tests is constructed from the ruling's required shape (
CodelessAccount(address)) viaabi.encodeWithSelector, not observed from the code under test.Category check: Account-taking absence checks reject codeless accounts (ruling E+D on #55) #132 asks for (1) the CBOR guard with a typed address-carrying error — covered,
src/lib/LibExtrospectBytecode.sol; (2) address-taking metamorphic entry points reverting on codeless then delegating — covered,src/lib/LibExtrospectMetamorphic.sol; (3) bytes functions total with empty meaning documented and pinned — already done in Document and pin the empty-bytecode metamorphic verdict #96, cross-references added, pins retained; (4) non-reverting predicates' empty-input meaning explicit in NatSpec with deterministic pins — covered,LibExtrospectBytecode.sol,LibExtrospectERC1167Proxy.soland the four test files. Nothing outside that scope was changed; EIP-7702 delegated accounts are reported as NOT metamorphic #54 untouched.🤖 Generated with Claude Code