Skip to content

Commit

Permalink
♻️ Use immutable chainId instead for ERC6551 (#884)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Vectorized
Vectorized authored Apr 9, 2024
1 parent 59b038f commit 0dd02ad
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 88 deletions.
32 changes: 16 additions & 16 deletions .gas-snapshot
View file
Original file line number Diff line number Diff line change
Expand Up @@ -267,24 +267,24 @@ ERC4626Test:testWithdrawWithNoUnderlyingAmountReverts() (gas: 13102)
ERC4626Test:testWithdrawWithNotEnoughUnderlyingAmountReverts() (gas: 144096)
ERC4626Test:testWithdrawZero() (gas: 51777)
ERC4626Test:test__codesize() (gas: 36848)
ERC6551Test:testCdFallback() (gas: 894338)
ERC6551Test:testDeployERC6551(uint256) (runs: 276, μ: 171159, ~: 168814)
ERC6551Test:testDeployERC6551Proxy() (gas: 80351)
ERC6551Test:testExecute() (gas: 508156)
ERC6551Test:testExecuteBatch() (gas: 817275)
ERC6551Test:testExecuteBatch(uint256) (runs: 276, μ: 615557, ~: 483186)
ERC6551Test:testCdFallback() (gas: 894347)
ERC6551Test:testDeployERC6551(uint256) (runs: 276, μ: 171161, ~: 168772)
ERC6551Test:testDeployERC6551Proxy() (gas: 80373)
ERC6551Test:testExecute() (gas: 508172)
ERC6551Test:testExecuteBatch() (gas: 817309)
ERC6551Test:testExecuteBatch(uint256) (runs: 276, μ: 607569, ~: 483190)
ERC6551Test:testInitializeERC6551ProxyImplementation() (gas: 189802)
ERC6551Test:testIsValidSignature() (gas: 187826)
ERC6551Test:testIsValidSigner(address) (runs: 276, μ: 167383, ~: 167375)
ERC6551Test:testOnERC1155BatchReceived() (gas: 1526556)
ERC6551Test:testOnERC1155Received() (gas: 1523868)
ERC6551Test:testOnERC721Received() (gas: 1501319)
ERC6551Test:testOnERC721ReceivedCycles() (gas: 1714496)
ERC6551Test:testOnERC721ReceivedCyclesWithDifferentChainIds(uint256) (runs: 276, μ: 453875, ~: 458570)
ERC6551Test:testSaveChainId(uint256,uint256) (runs: 276, μ: 198525, ~: 174675)
ERC6551Test:testIsValidSignature() (gas: 187852)
ERC6551Test:testIsValidSigner(address) (runs: 276, μ: 167410, ~: 167380)
ERC6551Test:testOnERC1155BatchReceived() (gas: 1526562)
ERC6551Test:testOnERC1155Received() (gas: 1523896)
ERC6551Test:testOnERC721Received() (gas: 1501345)
ERC6551Test:testOnERC721ReceivedCycles() (gas: 1714457)
ERC6551Test:testOnERC721ReceivedCyclesWithDifferentChainIds(uint256) (runs: 276, μ: 448645, ~: 454180)
ERC6551Test:testOwnerWorksWithChainIdChange(uint256,uint256) (runs: 276, μ: 1212002, ~: 1211998)
ERC6551Test:testSupportsInterface() (gas: 169338)
ERC6551Test:testUpgrade() (gas: 1233149)
ERC6551Test:test__codesize() (gas: 47833)
ERC6551Test:testUpgrade() (gas: 1215934)
ERC6551Test:test__codesize() (gas: 47050)
ERC6909Test:testApprove() (gas: 36771)
ERC6909Test:testApprove(address,uint256,uint256) (runs: 276, μ: 36403, ~: 37413)
ERC6909Test:testBurn() (gas: 40676)
Expand Down
65 changes: 16 additions & 49 deletions src/accounts/ERC6551.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,6 @@ abstract contract ERC6551 is UUPSUpgradeable, Receiver, ERC1271 {
bytes data;
}

/*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
/* EVENTS */
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/

/// @dev Emitted when the chain ID is saved to storage.
event ChainIdSaved(uint256 indexed chainId);

/// @dev `keccak256(bytes("ChainIdSaved(uint256)"))`.
uint256 private constant _CHAIN_ID_SAVED_EVENT_SIGNATURE =
0xa220d2547c767f2e114b44272960ca6b3144fa65180cd2b94adc4ed4980353f6;

/*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
/* CUSTOM ERRORS */
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/
Expand All @@ -73,7 +62,7 @@ abstract contract ERC6551 is UUPSUpgradeable, Receiver, ERC1271 {
error SelfOwnDetected();

/*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
/* CONSTANTS */
/* CONSTANTS AND IMMUTABLES */
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/

/// @dev The ERC6551 state slot is given by:
Expand All @@ -85,12 +74,17 @@ abstract contract ERC6551 is UUPSUpgradeable, Receiver, ERC1271 {
uint256 internal constant _ERC6551_STATE_SLOT =
0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffb919c7a5;

/// @dev The ERC6551 chain ID save slot is given by:
/// `bytes32(~uint256(uint32(bytes4(keccak256("_ERC6551_CHAIN_ID_SAVE_SLOT_NOT")))))`.
/// The slot of whether the chain ID has been saved is given by:
/// `_ERC6551_CHAIN_ID_SAVE_SLOT + 1`.
uint256 internal constant _ERC6551_CHAIN_ID_SAVE_SLOT =
0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffba274b31;
/// @dev Caches the chain ID in the deployed bytecode,
/// so that in the rare case of a hard fork, `owner` will still work.
uint256 private immutable _cachedChainId;

/*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
/* CONSTRUCTOR */
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/

constructor() {
_cachedChainId = block.chainid;
}

/*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
/* TOKEN-BOUND OWNERSHIP OPERATIONS */
Expand All @@ -116,16 +110,12 @@ abstract contract ERC6551 is UUPSUpgradeable, Receiver, ERC1271 {

/// @dev Returns the owner of the contract.
function owner() public view virtual returns (address result) {
uint256 cachedChainId = _cachedChainId;
/// @solidity memory-safe-assembly
assembly {
let m := mload(0x40) // Cache the free memory pointer.
extcodecopy(address(), 0x00, 0x4d, 0x60)
let chainsEq := eq(mload(0x00), chainid())
if iszero(chainsEq) {
let saveSlot := _ERC6551_CHAIN_ID_SAVE_SLOT
if sload(add(saveSlot, 1)) { chainsEq := eq(mload(0x00), sload(saveSlot)) }
}
if chainsEq {
if or(eq(mload(0x00), chainid()), eq(mload(0x00), cachedChainId)) {
let tokenContract := mload(0x20)
// `tokenId` is already at 0x40.
mstore(0x20, 0x6352211e) // `ownerOf(uint256)`.
Expand Down Expand Up @@ -316,6 +306,7 @@ abstract contract ERC6551 is UUPSUpgradeable, Receiver, ERC1271 {
/// @dev For handling token callbacks, and the hidden `saveChainId()` function.
/// Safe-transferred ERC721 tokens will trigger a ownership cycle check.
modifier receiverFallback() override(Receiver) {
uint256 cachedChainId = _cachedChainId;
/// @solidity memory-safe-assembly
assembly {
let s := shr(224, calldataload(0x00))
Expand All @@ -327,11 +318,7 @@ abstract contract ERC6551 is UUPSUpgradeable, Receiver, ERC1271 {
let tokenContract := mload(0x20)
// `tokenId` is already at 0x40.
mstore(0x20, 0x6352211e) // `ownerOf(uint256)`.
let chainsEq := eq(mload(0x00), chainid())
if iszero(chainsEq) {
let saveSlot := _ERC6551_CHAIN_ID_SAVE_SLOT
if sload(add(saveSlot, 1)) { chainsEq := eq(mload(0x00), sload(saveSlot)) }
}
let chainsEq := or(eq(mload(0x00), chainid()), eq(mload(0x00), cachedChainId))
let currentOwner :=
mul(
mload(0x20),
Expand Down Expand Up @@ -370,26 +357,6 @@ abstract contract ERC6551 is UUPSUpgradeable, Receiver, ERC1271 {
mstore(0x20, s) // Store `msg.sig`.
return(0x3c, 0x20) // Return `msg.sig`.
}
// Hidden `saveChainId()` function:
// Saves the chain ID into storage. In case of the super rare event of a hard fork,
// anyone can call this to save the chain ID in storage before the asteroid hits,
// allowing `owner` to still work after the hard fork.
// No-op if the chain ID has already been saved.
// Returns the saved chain ID.
if eq(s, 0x7d870d25) {
let saveSlot := _ERC6551_CHAIN_ID_SAVE_SLOT
let alreadySavedSlot := add(saveSlot, 1)
if sload(alreadySavedSlot) {
mstore(0x00, sload(saveSlot))
return(0x00, 0x20)
}
sstore(alreadySavedSlot, 1)
sstore(saveSlot, chainid())
// Emit the {ChainIdSaved} event.
log2(codesize(), 0x00, _CHAIN_ID_SAVED_EVENT_SIGNATURE, chainid())
mstore(0x00, chainid())
return(0x00, 0x20)
}
}
_;
}
Expand Down
28 changes: 5 additions & 23 deletions test/ERC6551.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@ contract Target {
}
}

interface IERC6551WithChainIdSaver {
function saveChainId() external returns (uint256);
}

contract ERC6551Test is SoladyTest {
MockERC6551Registry internal _registry;

Expand Down Expand Up @@ -124,30 +120,16 @@ contract ERC6551Test is SoladyTest {
}
}

function testSaveChainId(uint256 oldChainId, uint256 newChainId) public {
function testOwnerWorksWithChainIdChange(uint256 oldChainId, uint256 newChainId) public {
oldChainId = _bound(oldChainId, 0, 2 ** 64 - 1);
newChainId = _bound(newChainId, 0, 2 ** 64 - 1);
vm.chainId(oldChainId);
_erc6551 = address(new MockERC6551());
_proxy = address(new ERC6551Proxy(_erc6551));
_TestTemps memory t = _testTemps();
assertEq(t.account.owner(), t.owner);
if (_random() % 2 == 0) {
vm.expectEmit(true, true, true, true);
emit ChainIdSaved(t.chainId);
assertEq(IERC6551WithChainIdSaver(address(t.account)).saveChainId(), t.chainId);
if (_random() % 2 == 0) {
assertEq(IERC6551WithChainIdSaver(address(t.account)).saveChainId(), t.chainId);
}
assertEq(t.account.owner(), t.owner);
vm.chainId(newChainId);
assertEq(t.account.owner(), t.owner);
} else {
vm.chainId(newChainId);
if (newChainId == oldChainId) {
assertEq(t.account.owner(), t.owner);
} else {
assertEq(t.account.owner(), address(0));
}
}
vm.chainId(newChainId);
assertEq(t.account.owner(), t.owner);
}

function testOnERC721ReceivedCycles() public {
Expand Down

0 comments on commit 0dd02ad

Please sign in to comment.