Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/lib/LibICloneableFactoryV4.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ import {
/// `initialize` included — to nothing.
error ZeroImplementationCodeSize();

/// Thrown when an implementation's code begins with `0xef`: an EIP-7702
/// delegated account.
error DelegatedImplementation();

/// Thrown when the clone address already has code. On the open-salt path the
/// occupant is the exact clone asked for; on the namespaced path it is the
/// clone the same deployer deployed at that salt, with whatever `data` that
Expand Down Expand Up @@ -125,12 +129,20 @@ library LibICloneableFactoryV4 {
}

/// Reverts with `ZeroImplementationCodeSize` if `implementation` has no
/// code. Always a mistake: the clone would delegate every call to nothing.
/// code, or `DelegatedImplementation` if its code begins with `0xef`.
/// @param implementation The contract to clone.
function checkImplementationCode(address implementation) internal view {
if (implementation.code.length == 0) {
revert ZeroImplementationCodeSize();
}
uint256 firstByte;
assembly ("memory-safe") {
extcodecopy(implementation, 0, 0, 1)
firstByte := byte(0, mload(0))
}
if (firstByte == 0xef) {
revert DelegatedImplementation();
}
}

/// The shared tail of both clone entry points: guard the implementation,
Expand Down
60 changes: 11 additions & 49 deletions test/src/lib/LibICloneableFactoryV4.checkImplementationCode.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@ pragma solidity =0.8.25;

import {Test} from "forge-std-1.16.1/src/Test.sol";

import {LibICloneableFactoryV4, ZeroImplementationCodeSize} from "src/lib/LibICloneableFactoryV4.sol";
import {
LibICloneableFactoryV4,
DelegatedImplementation,
ZeroImplementationCodeSize
} from "src/lib/LibICloneableFactoryV4.sol";
import {TestCloneable} from "test/concrete/TestCloneable.sol";

/// @title LibICloneableFactoryV4CheckImplementationCodeTest
/// @notice Tests `LibICloneableFactoryV4.checkImplementationCode`: a codeless
/// implementation is always a mistake — its clone would delegate every call,
/// `initialize` included, to nothing — so it reverts with a typed error
/// before any deploy happens.
/// @notice Tests `LibICloneableFactoryV4.checkImplementationCode`.
contract LibICloneableFactoryV4CheckImplementationCodeTest is Test {
/// External wrapper so `vm.expectRevert` sees the internal library call at
/// its own call depth.
Expand All @@ -32,61 +33,22 @@ contract LibICloneableFactoryV4CheckImplementationCodeTest is Test {
LibICloneableFactoryV4.checkImplementationCode(address(implementation));
}

/// Any nonempty code is enough to pass: the guard is a code-size check,
/// not a validation of what the code is.
/// Any nonempty code not beginning with `0xef` passes.
function testCheckImplementationCodeEtched(address implementation, bytes memory code) external {
vm.assume(implementation.code.length == 0);
vm.assume(uint160(implementation) > 0x0a);
vm.assume(code.length > 0);
// EIP-3541 forbids DEPLOYING any code whose first byte is `0xef`, so
// no CREATE or CREATE2 can put such code at an address. That leaves
// exactly one way an account can hold it — an EIP-7702 delegation
// designator, which is `0xef0100` followed by an address and is
// therefore EXACTLY 23 bytes. That case is real, so it is not excluded
// here, it is pinned by its own test below.
//
// What this exclusion drops is the rest: `0xef`-leading blobs of any
// other length, which no chain can produce. It cannot weaken the
// property under test, because the guard only ever looks at code
// LENGTH.
//
// It is also what keeps this test from failing for a harness reason:
// `vm.etch` parses a `0xef01` prefix as an EIP-7702 delegation
// designator and rejects it unless the blob is exactly 23 bytes
// ("Eip7702 is not 23 bytes long"), so a fuzz run that drew one died
// in the cheatcode rather than in the code under test.
vm.assume(code[0] != 0xef);
vm.etch(implementation, code);
LibICloneableFactoryV4.checkImplementationCode(implementation);
}

/// The one `0xef`-leading code a real account can hold: an EIP-7702
/// delegation designator, `0xef0100 || address`, exactly 23 bytes. The
/// fuzz test above cannot reach it, so it is pinned here as a fixed case.
///
/// It PASSES the guard, and that is the point worth having on the record.
/// `EXTCODESIZE` on a delegated EOA returns 23, not zero, so the size
/// check cannot tell an ordinary implementation contract from an EOA that
/// has delegated — and unlike a deployed contract, a delegation is
/// REVOCABLE by the account holder at any time. A caller who wants an
/// immutable implementation does not get that from this guard; the guard
/// promises only that something is there.
///
/// Scoped honestly: `foundry.toml` pins `evm_version = "cancun"`, which
/// predates EIP-7702, so what is asserted here is that the 23-byte
/// designator is storable at an address and passes the SIZE check. The
/// execution semantics of delegation are not exercised and this test does
/// not claim them.
/// An EIP-7702 delegation designator reverts `DelegatedImplementation`.
function testCheckImplementationCodeEip7702Designator(address delegated, address delegate) external {
vm.assume(delegated.code.length == 0);
vm.assume(uint160(delegated) > 0x0a);

bytes memory designator = abi.encodePacked(hex"ef0100", delegate);
assertEq(designator.length, 23, "an EIP-7702 designator is 23 bytes");

vm.etch(delegated, designator);

assertEq(delegated.code.length, 23, "EXTCODESIZE sees the designator, not zero");
LibICloneableFactoryV4.checkImplementationCode(delegated);
vm.etch(delegated, abi.encodePacked(hex"ef0100", delegate));
vm.expectRevert(abi.encodeWithSelector(DelegatedImplementation.selector));
this.checkImplementationCodeExternal(delegated);
}
}
Loading