From 96e642929983991a8ca09db3f291c27e1c21a194 Mon Sep 17 00:00:00 2001 From: Jayden Windle Date: Wed, 10 Apr 2024 14:29:21 -0600 Subject: [PATCH 1/2] Updated reference implementation to use deterministic state, added support for post-hardfork account usage --- src/examples/simple/ERC6551Account.sol | 10 ++++++---- .../upgradeable/ERC6551AccountUpgradeable.sol | 6 +++--- src/interfaces/IERC6551Account.sol | 2 +- test/examples/simple/ERC6551Account.t.sol | 11 ++++++++++- .../upgradeable/ERC6551AccountUpgradeable.t.sol | 14 +++++++++++--- test/mocks/MockERC6551Account.sol | 2 +- 6 files changed, 32 insertions(+), 13 deletions(-) diff --git a/src/examples/simple/ERC6551Account.sol b/src/examples/simple/ERC6551Account.sol index ff0f47e..f8fcc94 100644 --- a/src/examples/simple/ERC6551Account.sol +++ b/src/examples/simple/ERC6551Account.sol @@ -14,7 +14,7 @@ interface IERC6551Account { view returns (uint256 chainId, address tokenContract, uint256 tokenId); - function state() external view returns (uint256); + function state() external view returns (bytes32); function isValidSigner(address signer, bytes calldata context) external @@ -30,7 +30,9 @@ interface IERC6551Executable { } contract ERC6551Account is IERC165, IERC1271, IERC6551Account, IERC6551Executable { - uint256 public state; + uint256 immutable deploymentChainId = block.chainid; + + bytes32 public state; receive() external payable {} @@ -43,7 +45,7 @@ contract ERC6551Account is IERC165, IERC1271, IERC6551Account, IERC6551Executabl require(_isValidSigner(msg.sender), "Invalid signer"); require(operation == 0, "Only call operations are supported"); - ++state; + state = keccak256(abi.encode(state, msg.data)); bool success; (success, result) = to.call{value: value}(data); @@ -96,7 +98,7 @@ contract ERC6551Account is IERC165, IERC1271, IERC6551Account, IERC6551Executabl function owner() public view virtual returns (address) { (uint256 chainId, address tokenContract, uint256 tokenId) = token(); - if (chainId != block.chainid) return address(0); + if (chainId != block.chainid && block.chainid == deploymentChainId) return address(0); return IERC721(tokenContract).ownerOf(tokenId); } diff --git a/src/examples/upgradeable/ERC6551AccountUpgradeable.sol b/src/examples/upgradeable/ERC6551AccountUpgradeable.sol index 1be1881..b0325af 100644 --- a/src/examples/upgradeable/ERC6551AccountUpgradeable.sol +++ b/src/examples/upgradeable/ERC6551AccountUpgradeable.sol @@ -36,7 +36,7 @@ contract ERC6551AccountUpgradeable is bytes32 internal constant _IMPLEMENTATION_SLOT = 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc; - uint256 public state; + bytes32 public state; receive() external payable {} @@ -51,7 +51,7 @@ contract ERC6551AccountUpgradeable is { require(_isValidSigner(msg.sender), "Caller is not owner"); require(_operation == 0, "Only call operations are supported"); - ++state; + state = keccak256(abi.encode(state, msg.data)); bool success; // solhint-disable-next-line avoid-low-level-calls (success, _result) = _target.call{value: _value}(_data); @@ -65,7 +65,7 @@ contract ERC6551AccountUpgradeable is function upgrade(address implementation_) external virtual { require(_isValidSigner(msg.sender), "Caller is not owner"); require(implementation_ != address(0), "Invalid implementation address"); - ++state; + state = keccak256(abi.encode(state, msg.data)); StorageSlot.getAddressSlot(_IMPLEMENTATION_SLOT).value = implementation_; } diff --git a/src/interfaces/IERC6551Account.sol b/src/interfaces/IERC6551Account.sol index f3dcb97..980247d 100644 --- a/src/interfaces/IERC6551Account.sol +++ b/src/interfaces/IERC6551Account.sol @@ -32,7 +32,7 @@ interface IERC6551Account { * * @return The current account state */ - function state() external view returns (uint256); + function state() external view returns (bytes32); /** * @dev Returns a magic value indicating whether a given signer is authorized to act on behalf diff --git a/test/examples/simple/ERC6551Account.t.sol b/test/examples/simple/ERC6551Account.t.sol index 3502c36..c80761e 100644 --- a/test/examples/simple/ERC6551Account.t.sol +++ b/test/examples/simple/ERC6551Account.t.sol @@ -49,11 +49,20 @@ contract AccountTest is Test { vm.deal(account, 1 ether); + bytes32 expectedState = keccak256( + abi.encode( + bytes32(0), + abi.encodeWithSignature( + "execute(address,uint256,bytes,uint8)", vm.addr(2), 0.5 ether, "", 0 + ) + ) + ); + vm.prank(vm.addr(1)); executableAccountInstance.execute(payable(vm.addr(2)), 0.5 ether, "", 0); assertEq(account.balance, 0.5 ether); assertEq(vm.addr(2).balance, 0.5 ether); - assertEq(accountInstance.state(), 1); + assertEq(accountInstance.state(), expectedState); } } diff --git a/test/examples/upgradeable/ERC6551AccountUpgradeable.t.sol b/test/examples/upgradeable/ERC6551AccountUpgradeable.t.sol index 7d15b72..84eb8a6 100644 --- a/test/examples/upgradeable/ERC6551AccountUpgradeable.t.sol +++ b/test/examples/upgradeable/ERC6551AccountUpgradeable.t.sol @@ -94,6 +94,15 @@ contract AccountProxyTest is Test { IERC6551Account accountInstance = IERC6551Account(payable(account)); IERC6551Executable executableAccountInstance = IERC6551Executable(account); + bytes32 expectedState = keccak256( + abi.encode( + bytes32(0), + abi.encodeWithSignature( + "execute(address,uint256,bytes,uint8)", vm.addr(2), 0.5 ether, "", 0 + ) + ) + ); + vm.prank(vm.addr(3)); vm.expectRevert("Caller is not owner"); executableAccountInstance.execute(payable(vm.addr(2)), 0.5 ether, "", 0); @@ -103,7 +112,7 @@ contract AccountProxyTest is Test { assertEq(account.balance, 0.5 ether); assertEq(vm.addr(2).balance, 0.5 ether); - assertEq(accountInstance.state(), 1); + assertEq(accountInstance.state(), expectedState); } function testCannotOwnSelf() public { @@ -287,14 +296,13 @@ contract AccountProxyTest is Test { assertEq(address(uint160(uint256(rawImplementation))), address(0)); // Send ETH to initialize. - (bool success, ) = payable(account).call{value: 0}(""); + (bool success,) = payable(account).call{value: 0}(""); assertTrue(success); rawImplementation = vm.load(account, 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc); assertEq(address(uint160(uint256(rawImplementation))), address(implementation)); - } function testERC721Receive() public { diff --git a/test/mocks/MockERC6551Account.sol b/test/mocks/MockERC6551Account.sol index 52bca74..b1eb965 100644 --- a/test/mocks/MockERC6551Account.sol +++ b/test/mocks/MockERC6551Account.sol @@ -8,7 +8,7 @@ import "../../src/interfaces/IERC6551Executable.sol"; import "../../src/lib/ERC6551AccountLib.sol"; contract MockERC6551Account is IERC165, IERC6551Account, IERC6551Executable { - uint256 public state; + bytes32 public state; bool private _initialized; receive() external payable {} From 8caa54becffe14b139211127fa99d3f5ad1c4f64 Mon Sep 17 00:00:00 2001 From: Jayden Windle Date: Thu, 11 Apr 2024 10:42:52 -0600 Subject: [PATCH 2/2] Added hardfork test case, simplified hardfork compatibility logic, added hardfork compatibility to upgradeable example --- src/examples/simple/ERC6551Account.sol | 3 ++- .../upgradeable/ERC6551AccountUpgradeable.sol | 4 +++- test/examples/simple/ERC6551Account.t.sol | 24 +++++++++++++++++++ .../ERC6551AccountUpgradeable.t.sol | 24 +++++++++++++++++++ 4 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/examples/simple/ERC6551Account.sol b/src/examples/simple/ERC6551Account.sol index f8fcc94..f011c32 100644 --- a/src/examples/simple/ERC6551Account.sol +++ b/src/examples/simple/ERC6551Account.sol @@ -98,7 +98,8 @@ contract ERC6551Account is IERC165, IERC1271, IERC6551Account, IERC6551Executabl function owner() public view virtual returns (address) { (uint256 chainId, address tokenContract, uint256 tokenId) = token(); - if (chainId != block.chainid && block.chainid == deploymentChainId) return address(0); + + if (chainId != deploymentChainId) return address(0); return IERC721(tokenContract).ownerOf(tokenId); } diff --git a/src/examples/upgradeable/ERC6551AccountUpgradeable.sol b/src/examples/upgradeable/ERC6551AccountUpgradeable.sol index b0325af..0296231 100644 --- a/src/examples/upgradeable/ERC6551AccountUpgradeable.sol +++ b/src/examples/upgradeable/ERC6551AccountUpgradeable.sol @@ -36,6 +36,8 @@ contract ERC6551AccountUpgradeable is bytes32 internal constant _IMPLEMENTATION_SLOT = 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc; + uint256 immutable deploymentChainId = block.chainid; + bytes32 public state; receive() external payable {} @@ -139,7 +141,7 @@ contract ERC6551AccountUpgradeable is function owner() public view virtual returns (address) { (uint256 chainId, address contractAddress, uint256 tokenId) = token(); - if (chainId != block.chainid) return address(0); + if (chainId != deploymentChainId) return address(0); return IERC721(contractAddress).ownerOf(tokenId); } diff --git a/test/examples/simple/ERC6551Account.t.sol b/test/examples/simple/ERC6551Account.t.sol index c80761e..24b0951 100644 --- a/test/examples/simple/ERC6551Account.t.sol +++ b/test/examples/simple/ERC6551Account.t.sol @@ -65,4 +65,28 @@ contract AccountTest is Test { assertEq(vm.addr(2).balance, 0.5 ether); assertEq(accountInstance.state(), expectedState); } + + function testChainIdPostHardfork() public { + nft.mint(vm.addr(1), 1); + + uint256 anvilChainId = block.chainid; + uint256 anvilHardforkChainId = anvilChainId + 1; + uint256 otherChainId = anvilChainId + 2; + + address anvilAccount = + registry.createAccount(address(implementation), 0, anvilChainId, address(nft), 1); + address otherChainOnAnvilAccount = + registry.createAccount(address(implementation), 0, otherChainId, address(nft), 1); + + assertTrue(anvilAccount != address(0)); + assertTrue(otherChainOnAnvilAccount != address(0)); + + assertEq(ERC6551Account(payable(anvilAccount)).owner(), vm.addr(1)); + assertEq(ERC6551Account(payable(otherChainOnAnvilAccount)).owner(), address(0)); + + vm.chainId(anvilHardforkChainId); + + assertEq(ERC6551Account(payable(anvilAccount)).owner(), vm.addr(1)); + assertEq(ERC6551Account(payable(otherChainOnAnvilAccount)).owner(), address(0)); + } } diff --git a/test/examples/upgradeable/ERC6551AccountUpgradeable.t.sol b/test/examples/upgradeable/ERC6551AccountUpgradeable.t.sol index 84eb8a6..6d2f372 100644 --- a/test/examples/upgradeable/ERC6551AccountUpgradeable.t.sol +++ b/test/examples/upgradeable/ERC6551AccountUpgradeable.t.sol @@ -404,4 +404,28 @@ contract AccountProxyTest is Test { vm.expectRevert(InvalidImplementation.selector); new ERC6551AccountProxy(address(0)); } + + function testChainIdPostHardfork() public { + nft.mint(vm.addr(1), 1); + + uint256 anvilChainId = block.chainid; + uint256 anvilHardforkChainId = anvilChainId + 1; + uint256 otherChainId = anvilChainId + 2; + + address anvilAccount = + registry.createAccount(address(implementation), 0, anvilChainId, address(nft), 1); + address otherChainOnAnvilAccount = + registry.createAccount(address(implementation), 0, otherChainId, address(nft), 1); + + assertTrue(anvilAccount != address(0)); + assertTrue(otherChainOnAnvilAccount != address(0)); + + assertEq(ERC6551AccountUpgradeable(payable(anvilAccount)).owner(), vm.addr(1)); + assertEq(ERC6551AccountUpgradeable(payable(otherChainOnAnvilAccount)).owner(), address(0)); + + vm.chainId(anvilHardforkChainId); + + assertEq(ERC6551AccountUpgradeable(payable(anvilAccount)).owner(), vm.addr(1)); + assertEq(ERC6551AccountUpgradeable(payable(otherChainOnAnvilAccount)).owner(), address(0)); + } }