From 330b8b8cd5bcbcb7ba3dacf27f96a03c0476f17b Mon Sep 17 00:00:00 2001 From: Kingter <83567446+kingster-will@users.noreply.github.com> Date: Thu, 25 Apr 2024 16:24:51 -0700 Subject: [PATCH] Integration with Solday's ERC6551 Implementation (#133) * forge install: solady v0.0.192 * integration with Solady 6551 --- contracts/IPAccountImpl.sol | 144 ++++++++------ contracts/IPAccountStorage.sol | 4 +- contracts/interfaces/IIPAccount.sol | 13 +- contracts/interfaces/IIPAccountStorage.sol | 4 +- contracts/lib/Errors.sol | 3 + contracts/lib/MetaTx.sol | 2 +- package.json | 3 +- remappings.txt | 3 +- test/foundry/IPAccount.t.sol | 125 ++++++++++++- test/foundry/IPAccountImpl.btt.t.sol | 6 +- test/foundry/IPAccountMetaTx.t.sol | 208 ++++++++++++++++++--- test/foundry/access/AccessController.t.sol | 24 ++- yarn.lock | 5 + 13 files changed, 437 insertions(+), 107 deletions(-) diff --git a/contracts/IPAccountImpl.sol b/contracts/IPAccountImpl.sol index 1f2921cc..d4427493 100644 --- a/contracts/IPAccountImpl.sol +++ b/contracts/IPAccountImpl.sol @@ -2,12 +2,12 @@ pragma solidity 0.8.23; import { IERC165 } from "@openzeppelin/contracts/utils/introspection/ERC165.sol"; -import { IERC721 } from "@openzeppelin/contracts/token/ERC721/IERC721.sol"; import { IERC721Receiver } from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol"; import { IERC1155Receiver } from "@openzeppelin/contracts/token/ERC1155/IERC1155Receiver.sol"; import { SignatureChecker } from "@openzeppelin/contracts/utils/cryptography/SignatureChecker.sol"; import { MessageHashUtils } from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; import { IERC6551Account } from "erc6551/interfaces/IERC6551Account.sol"; +import { ERC6551, Receiver } from "@solady/src/accounts/ERC6551.sol"; import { IAccessController } from "./interfaces/access/IAccessController.sol"; import { IIPAccount } from "./interfaces/IIPAccount.sol"; @@ -17,13 +17,10 @@ import { IPAccountStorage } from "./IPAccountStorage.sol"; /// @title IPAccountImpl /// @notice The Story Protocol's implementation of the IPAccount. -contract IPAccountImpl is IPAccountStorage, IIPAccount { +contract IPAccountImpl is ERC6551, IPAccountStorage, IIPAccount { address public immutable ACCESS_CONTROLLER; - /// @notice Returns the IPAccount's internal nonce for transaction ordering. - uint256 public state; - - receive() external payable override(IERC6551Account) {} + receive() external payable override(Receiver, IIPAccount) {} /// @notice Creates a new IPAccountImpl contract instance /// @dev Initializes the IPAccountImpl with an AccessController address which is stored @@ -44,7 +41,9 @@ contract IPAccountImpl is IPAccountStorage, IIPAccount { /// @notice Checks if the contract supports a specific interface /// @param interfaceId The interface identifier, as specified in ERC-165 /// @return bool is true if the contract supports the interface, false otherwise - function supportsInterface(bytes4 interfaceId) public view override(IPAccountStorage, IERC165) returns (bool) { + function supportsInterface( + bytes4 interfaceId + ) public view override(ERC6551, IPAccountStorage, IERC165) returns (bool) { return (interfaceId == type(IIPAccount).interfaceId || interfaceId == type(IERC6551Account).interfaceId || interfaceId == type(IERC1155Receiver).interfaceId || @@ -56,42 +55,30 @@ contract IPAccountImpl is IPAccountStorage, IIPAccount { /// @return chainId The EIP-155 ID of the chain the token exists on /// @return tokenContract The contract address of the token /// @return tokenId The ID of the token - function token() public view override returns (uint256, address, uint256) { - bytes memory footer = new bytes(0x60); - // 0x4d = 77 bytes (ERC-1167 Header, address, ERC-1167 Footer, salt) - // 0x60 = 96 bytes (chainId, tokenContract, tokenId) - // ERC-1167 Header (10 bytes) - // (20 bytes) - // ERC-1167 Footer (15 bytes) - // (32 bytes) - // (32 bytes) - // (32 bytes) - // (32 bytes) - assembly { - extcodecopy(address(), add(footer, 0x20), 0x4d, 0x60) - } - - return abi.decode(footer, (uint256, address, uint256)); + function token() public view override(ERC6551, IIPAccount) returns (uint256, address, uint256) { + return super.token(); } /// @notice Checks if the signer is valid for the given data /// @param signer The signer to check /// @param data The data to check against /// @return The function selector if the signer is valid, 0 otherwise - function isValidSigner(address signer, bytes calldata data) external view returns (bytes4) { - if (_isValidSigner(signer, address(0), data)) { - return IERC6551Account.isValidSigner.selector; - } - - return bytes4(0); + function isValidSigner( + address signer, + bytes calldata data + ) public view override(ERC6551, IIPAccount) returns (bytes4) { + return super.isValidSigner(signer, data); } /// @notice Returns the owner of the IP Account. /// @return The address of the owner. - function owner() public view returns (address) { - (uint256 chainId, address contractAddress, uint256 tokenId) = token(); - if (chainId != block.chainid) return address(0); - return IERC721(contractAddress).ownerOf(tokenId); + function owner() public view override(ERC6551, IIPAccount) returns (address) { + return super.owner(); + } + + /// @notice Returns the IPAccount's internal nonce for transaction ordering. + function state() public view override(ERC6551, IIPAccount) returns (bytes32 result) { + return super.state(); } /// @dev Checks if the signer is valid for the given data and recipient via the AccessController permission system. @@ -135,12 +122,12 @@ contract IPAccountImpl is IPAccountStorage, IIPAccount { revert Errors.IPAccount__ExpiredSignature(); } - ++state; + _updateStateForExecute(to, value, data); bytes32 digest = MessageHashUtils.toTypedDataHash( MetaTx.calculateDomainSeparator(), MetaTx.getExecuteStructHash( - MetaTx.Execute({ to: to, value: value, data: data, nonce: state, deadline: deadline }) + MetaTx.Execute({ to: to, value: value, data: data, nonce: state(), deadline: deadline }) ) ); @@ -149,7 +136,7 @@ contract IPAccountImpl is IPAccountStorage, IIPAccount { } result = _execute(signer, to, value, data); - emit ExecutedWithSig(to, value, data, state, deadline, signer, signature); + emit ExecutedWithSig(to, value, data, state(), deadline, signer, signature); } /// @notice Executes a transaction from the IP Account. @@ -158,30 +145,45 @@ contract IPAccountImpl is IPAccountStorage, IIPAccount { /// @param data The data to send along with the transaction. /// @return result The return data from the transaction. function execute(address to, uint256 value, bytes calldata data) external payable returns (bytes memory result) { - ++state; + _updateStateForExecute(to, value, data); result = _execute(msg.sender, to, value, data); - emit Executed(to, value, data, state); + emit Executed(to, value, data, state()); } - /// @inheritdoc IERC721Receiver - function onERC721Received(address, address, uint256, bytes memory) public pure returns (bytes4) { - return this.onERC721Received.selector; - } - - /// @inheritdoc IERC1155Receiver - function onERC1155Received(address, address, uint256, uint256, bytes memory) public pure returns (bytes4) { - return this.onERC1155Received.selector; + /// @dev Override 6551 execute function. + /// Only "CALL" operation is supported. + /// @param to The recipient of the transaction. + /// @param value The amount of Ether to send. + /// @param data The data to send along with the transaction. + /// @param operation The operation type to perform, only 0 - CALL is supported. + /// @return result The return data from the transaction. + function execute( + address to, + uint256 value, + bytes calldata data, + uint8 operation + ) public payable override returns (bytes memory result) { + // Only "CALL" operation is supported. + if (operation != 0) { + revert Errors.IPAccount__InvalidOperation(); + } + _updateStateForExecute(to, value, data); + result = _execute(msg.sender, to, value, data); + emit Executed(to, value, data, state()); } - /// @inheritdoc IERC1155Receiver - function onERC1155BatchReceived( - address, - address, - uint256[] memory, - uint256[] memory, - bytes memory - ) public pure returns (bytes4) { - return this.onERC1155BatchReceived.selector; + /// @notice Executes a batch of transactions from the IP Account. + /// @param calls The array of calls to execute. + /// @param operation The operation type to perform, only 0 - CALL is supported. + /// @return results The return data from the transactions. + function executeBatch( + Call[] calldata calls, + uint8 operation + ) public payable override returns (bytes[] memory results) { + results = new bytes[](calls.length); + for (uint256 i = 0; i < calls.length; i++) { + results[i] = execute(calls[i].target, calls[i].value, calls[i].data, operation); + } } /// @dev Executes a transaction from the IP Account. @@ -202,4 +204,36 @@ contract IPAccountImpl is IPAccountStorage, IIPAccount { } } } + + /// @dev Updates the IP Account's state all execute transactions. + /// @param to The "target" of the execute transactions. + /// @param value The amount of Ether to send. + /// @param data The data to send along with the transaction. + function _updateStateForExecute(address to, uint256 value, bytes calldata data) internal { + bytes32 newState = keccak256( + abi.encode(state(), abi.encodeWithSignature("execute(address,uint256,bytes)", to, value, data)) + ); + assembly { + sstore(_ERC6551_STATE_SLOT, newState) + } + } + + /// @dev Override Solady 6551 _isValidSigner function. + /// @param signer The signer to check + /// @param extraData The extra data to check against, it should bethe address of the recipient for IPAccount + /// @param context The context for validating the signer + /// @return bool is true if the signer is valid, false otherwise + function _isValidSigner( + address signer, + bytes32 extraData, + bytes calldata context + ) internal view override returns (bool) { + return _isValidSigner(signer, address(uint160(uint256(extraData))), context); + } + + /// @dev Override Solady EIP712 function and return EIP712 domain name for IPAccount. + function _domainNameAndVersion() internal view override returns (string memory name, string memory version) { + name = "Story Protocol IP Account"; + version = "1"; + } } diff --git a/contracts/IPAccountStorage.sol b/contracts/IPAccountStorage.sol index cc772f86..0243b47c 100644 --- a/contracts/IPAccountStorage.sol +++ b/contracts/IPAccountStorage.sol @@ -4,7 +4,7 @@ pragma solidity ^0.8.23; import { IIPAccountStorage } from "./interfaces/IIPAccountStorage.sol"; import { IModuleRegistry } from "./interfaces/registries/IModuleRegistry.sol"; import { Errors } from "./lib/Errors.sol"; -import { ERC165 } from "@openzeppelin/contracts/utils/introspection/ERC165.sol"; +import { ERC165, IERC165 } from "@openzeppelin/contracts/utils/introspection/ERC165.sol"; import { ShortString, ShortStrings } from "@openzeppelin/contracts/utils/ShortStrings.sol"; /// @title IPAccount Storage /// @dev Implements the IIPAccountStorage interface for managing IPAccount's state using a namespaced storage pattern. @@ -66,7 +66,7 @@ contract IPAccountStorage is ERC165, IIPAccountStorage { } /// @notice ERC165 interface identifier for IIPAccountStorage - function supportsInterface(bytes4 interfaceId) public view virtual override(ERC165) returns (bool) { + function supportsInterface(bytes4 interfaceId) public view virtual override(ERC165, IERC165) returns (bool) { return interfaceId == type(IIPAccountStorage).interfaceId || super.supportsInterface(interfaceId); } diff --git a/contracts/interfaces/IIPAccount.sol b/contracts/interfaces/IIPAccount.sol index a62de039..afee047d 100644 --- a/contracts/interfaces/IIPAccount.sol +++ b/contracts/interfaces/IIPAccount.sol @@ -1,9 +1,6 @@ // SPDX-License-Identifier: BUSL-1.1 pragma solidity 0.8.23; -import { IERC721Receiver } from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol"; -import { IERC1155Receiver } from "@openzeppelin/contracts/token/ERC1155/IERC1155Receiver.sol"; -import { IERC6551Account } from "erc6551/interfaces/IERC6551Account.sol"; import { IIPAccountStorage } from "./IIPAccountStorage.sol"; /// @title IIPAccount @@ -14,13 +11,13 @@ import { IIPAccountStorage } from "./IIPAccountStorage.sol"; /// IPAccount can interact with modules by making calls as a normal transaction sender. /// This allows for seamless operations on the state and data of IP. /// IPAccount is core identity for all actions. -interface IIPAccount is IERC6551Account, IERC721Receiver, IERC1155Receiver, IIPAccountStorage { +interface IIPAccount is IIPAccountStorage { /// @notice Emitted when a transaction is executed. /// @param to The recipient of the transaction. /// @param value The amount of Ether sent. /// @param data The data sent along with the transaction. /// @param nonce The nonce of the transaction. - event Executed(address indexed to, uint256 value, bytes data, uint256 nonce); + event Executed(address indexed to, uint256 value, bytes data, bytes32 nonce); /// @notice Emitted when a transaction is executed on behalf of the signer. /// @param to The recipient of the transaction. @@ -34,14 +31,16 @@ interface IIPAccount is IERC6551Account, IERC721Receiver, IERC1155Receiver, IIPA address indexed to, uint256 value, bytes data, - uint256 nonce, + bytes32 nonce, uint256 deadline, address indexed signer, bytes signature ); + receive() external payable; + /// @notice Returns the IPAccount's internal nonce for transaction ordering. - function state() external view returns (uint256); + function state() external view returns (bytes32); /// @notice Returns the identifier of the non-fungible token which owns the account /// @return chainId The EIP-155 ID of the chain the token exists on diff --git a/contracts/interfaces/IIPAccountStorage.sol b/contracts/interfaces/IIPAccountStorage.sol index f088dd19..18407af6 100644 --- a/contracts/interfaces/IIPAccountStorage.sol +++ b/contracts/interfaces/IIPAccountStorage.sol @@ -2,6 +2,8 @@ // See https://github.com/storyprotocol/protocol-contracts/blob/main/StoryProtocol-AlphaTestingAgreement-17942166.3.pdf pragma solidity ^0.8.23; +import { IERC165 } from "@openzeppelin/contracts/utils/introspection/ERC165.sol"; + /// @title IPAccount Namespaced Storage Interface /// @dev Provides a structured way to store IPAccount's state using a namespaced storage pattern. /// This interface facilitates conflict-free data writing by different Modules into the same IPAccount @@ -15,7 +17,7 @@ pragma solidity ^0.8.23; /// - Every Module can read data from any namespace. /// - Only the owning Module (i.e., the Module whose address is used as the namespace) can write data into /// its respective namespace. -interface IIPAccountStorage { +interface IIPAccountStorage is IERC165 { /// @dev Sets a bytes value under a given key within the default namespace, determined by `msg.sender`. /// @param key The key under which to store the value. /// @param value The bytes value to be stored. diff --git a/contracts/lib/Errors.sol b/contracts/lib/Errors.sol index 11d87bd3..71f5da30 100644 --- a/contracts/lib/Errors.sol +++ b/contracts/lib/Errors.sol @@ -23,6 +23,9 @@ library Errors { /// @notice Provided calldata is invalid. error IPAccount__InvalidCalldata(); + /// @notice Execute operation type is not supported. + error IPAccount__InvalidOperation(); + //////////////////////////////////////////////////////////////////////////// // IP Account Storage // //////////////////////////////////////////////////////////////////////////// diff --git a/contracts/lib/MetaTx.sol b/contracts/lib/MetaTx.sol index 018854c3..c0461b54 100644 --- a/contracts/lib/MetaTx.sol +++ b/contracts/lib/MetaTx.sol @@ -20,7 +20,7 @@ library MetaTx { address to; uint256 value; bytes data; - uint256 nonce; + bytes32 nonce; uint256 deadline; } diff --git a/package.json b/package.json index 8dc1b78e..d14abee2 100644 --- a/package.json +++ b/package.json @@ -59,6 +59,7 @@ "@openzeppelin/contracts": "5.0.2", "@openzeppelin/contracts-upgradeable": "5.0.2", "@openzeppelin/contracts-upgradeable-v4": "npm:@openzeppelin/contracts-upgradeable@4.9.6", - "erc6551": "^0.3.1" + "erc6551": "^0.3.1", + "solady": "^0.0.192" } } diff --git a/remappings.txt b/remappings.txt index c7c20eb8..3ec6c11b 100644 --- a/remappings.txt +++ b/remappings.txt @@ -2,4 +2,5 @@ ds-test/=node_modules/ds-test/src/ forge-std/=node_modules/forge-std/src/ @openzeppelin/=node_modules/@openzeppelin/ @openzeppelin-foundry-upgrades/=lib/openzeppelin-foundry-upgrades -@create3-deployer/=lib/create3-deployer \ No newline at end of file +@create3-deployer/=lib/create3-deployer +@solady/=node_modules/solady/ \ No newline at end of file diff --git a/test/foundry/IPAccount.t.sol b/test/foundry/IPAccount.t.sol index 5a8c26e3..95ae0a91 100644 --- a/test/foundry/IPAccount.t.sol +++ b/test/foundry/IPAccount.t.sol @@ -2,6 +2,7 @@ pragma solidity 0.8.23; import { IERC6551Account } from "erc6551/interfaces/IERC6551Account.sol"; +import { ERC6551 } from "@solady/src/accounts/ERC6551.sol"; import { IIPAccount } from "../../contracts/interfaces/IIPAccount.sol"; import { Errors } from "../../contracts/lib/Errors.sol"; @@ -92,6 +93,18 @@ contract IPAccountTest is BaseTest { IIPAccount ipAccount = IIPAccount(payable(account)); + bytes32 expectedState = keccak256( + abi.encode( + ipAccount.state(), + abi.encodeWithSignature( + "execute(address,uint256,bytes)", + address(module), + 0, + abi.encodeWithSignature("executeSuccessfully(string)", "test") + ) + ) + ); + vm.prank(owner); bytes memory result = ipAccount.execute( address(module), @@ -100,7 +113,7 @@ contract IPAccountTest is BaseTest { ); assertEq("test", abi.decode(result, (string))); - assertEq(ipAccount.state(), 1); + assertEq(ipAccount.state(), expectedState); } function test_IPAccount_revert_NonOwnerNoPermissionToExecute() public { @@ -166,4 +179,114 @@ contract IPAccountTest is BaseTest { assertEq(mockNFT.balanceOf(account), 1); assertEq(mockNFT.ownerOf(otherTokenId), account); } + + function test_IPAccount_ExecuteWithOperationType() public { + address owner = vm.addr(1); + uint256 tokenId = 100; + + mockNFT.mintId(owner, tokenId); + + vm.prank(owner, owner); + address account = ipAssetRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId); + + ERC6551 ipAccount = ERC6551(payable(account)); + + bytes32 expectedState = keccak256( + abi.encode( + ipAccount.state(), + abi.encodeWithSignature( + "execute(address,uint256,bytes)", + address(module), + 0, + abi.encodeWithSignature("executeSuccessfully(string)", "test") + ) + ) + ); + + vm.prank(owner); + bytes memory result = ipAccount.execute( + address(module), + 0, + abi.encodeWithSignature("executeSuccessfully(string)", "test"), + 0 + ); + assertEq("test", abi.decode(result, (string))); + + assertEq(ipAccount.state(), expectedState); + } + + function test_IPAccount_revert_ExecuteWithUnsupportedOperationType() public { + address owner = vm.addr(1); + uint256 tokenId = 100; + + mockNFT.mintId(owner, tokenId); + + vm.prank(owner, owner); + address account = ipAssetRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId); + + ERC6551 ipAccount = ERC6551(payable(account)); + + vm.expectRevert(Errors.IPAccount__InvalidOperation.selector); + vm.prank(owner); + bytes memory result = ipAccount.execute( + address(module), + 0, + abi.encodeWithSignature("executeSuccessfully(string)", "test"), + 1 // unsupported operation type + ); + } + + function test_IPAccount_executeBatch() public { + address owner = vm.addr(1); + uint256 tokenId = 100; + + mockNFT.mintId(owner, tokenId); + + vm.prank(owner, owner); + address account = ipAssetRegistry.registerIpAccount(block.chainid, address(mockNFT), tokenId); + + ERC6551 ipAccount = ERC6551(payable(account)); + + bytes32 firstState = keccak256( + abi.encode( + ipAccount.state(), + abi.encodeWithSignature( + "execute(address,uint256,bytes)", + address(module), + 0, + abi.encodeWithSignature("executeSuccessfully(string)", "test") + ) + ) + ); + + bytes32 finalState = keccak256( + abi.encode( + firstState, + abi.encodeWithSignature( + "execute(address,uint256,bytes)", + address(module), + 0, + abi.encodeWithSignature("executeSuccessfully(string)", "another test") + ) + ) + ); + + ERC6551.Call[] memory calls = new ERC6551.Call[](2); + calls[0] = ERC6551.Call({ + target: address(module), + value: 0, + data: abi.encodeWithSignature("executeSuccessfully(string)", "test") + }); + calls[1] = ERC6551.Call({ + target: address(module), + value: 0, + data: abi.encodeWithSignature("executeSuccessfully(string)", "another test") + }); + vm.prank(owner); + bytes[] memory results = ipAccount.executeBatch(calls, 0); + assertEq("test", abi.decode(results[0], (string))); + assertEq("another test", abi.decode(results[1], (string))); + + assertEq(ipAccount.state(), finalState); + } } diff --git a/test/foundry/IPAccountImpl.btt.t.sol b/test/foundry/IPAccountImpl.btt.t.sol index d9a4c3f9..8eb38e42 100644 --- a/test/foundry/IPAccountImpl.btt.t.sol +++ b/test/foundry/IPAccountImpl.btt.t.sol @@ -139,7 +139,9 @@ contract IPAccountImplBTT is BaseTest { whenSignerIsOwner toIsRegisteredModule { - uint256 expectedState = ipAcct.state() + 1; + bytes32 expectedState = keccak256( + abi.encode(ipAcct.state(), abi.encodeWithSelector(ipAcct.execute.selector, to, 0, data)) + ); vm.expectEmit(address(ipAcct)); emit IIPAccount.Executed(to, 0, data, expectedState); @@ -155,7 +157,7 @@ contract IPAccountImplBTT is BaseTest { whenSignerIsOwner toIsRegisteredModule { - uint256 expectedState = ipAcct.state(); + bytes32 expectedState = ipAcct.state(); vm.expectRevert("MockModule: executeRevert"); ipAcct.execute(to, 0, data); diff --git a/test/foundry/IPAccountMetaTx.t.sol b/test/foundry/IPAccountMetaTx.t.sol index 3faf686a..434f0c5d 100644 --- a/test/foundry/IPAccountMetaTx.t.sol +++ b/test/foundry/IPAccountMetaTx.t.sol @@ -73,6 +73,17 @@ contract IPAccountMetaTxTest is BaseTest { uint deadline = block.timestamp + 1000; + bytes32 expectedState = keccak256( + abi.encode( + ipAccount.state(), + abi.encodeWithSignature( + "execute(address,uint256,bytes)", + address(module), + 0, + abi.encodeWithSignature("executeSuccessfully(string)", "test") + ) + ) + ); bytes32 digest = MessageHashUtils.toTypedDataHash( MetaTx.calculateDomainSeparator(address(ipAccount)), MetaTx.getExecuteStructHash( @@ -80,7 +91,7 @@ contract IPAccountMetaTxTest is BaseTest { to: address(module), value: 0, data: abi.encodeWithSignature("executeSuccessfully(string)", "test"), - nonce: ipAccount.state() + 1, + nonce: expectedState, deadline: deadline }) ) @@ -98,7 +109,7 @@ contract IPAccountMetaTxTest is BaseTest { ); assertEq("test", abi.decode(result, (string))); - assertEq(ipAccount.state(), 1); + assertEq(ipAccount.state(), expectedState); } function test_IPAccount_setPermissionWithSignature() public { @@ -112,6 +123,36 @@ contract IPAccountMetaTxTest is BaseTest { uint deadline = block.timestamp + 1000; + bytes32 setPermissionState = keccak256( + abi.encode( + ipAccount.state(), + abi.encodeWithSignature( + "execute(address,uint256,bytes)", + address(accessController), + 0, + abi.encodeWithSignature( + "setPermission(address,address,address,bytes4,uint8)", + address(ipAccount), + address(metaTxModule), + address(module), + bytes4(0), + AccessPermission.ALLOW + ) + ) + ) + ); + bytes32 expectedState = keccak256( + abi.encode( + setPermissionState, + abi.encodeWithSignature( + "execute(address,uint256,bytes)", + address(module), + 0, + abi.encodeWithSignature("executeSuccessfully(string)", "test") + ) + ) + ); + bytes32 digest = MessageHashUtils.toTypedDataHash( MetaTx.calculateDomainSeparator(address(ipAccount)), MetaTx.getExecuteStructHash( @@ -126,7 +167,7 @@ contract IPAccountMetaTxTest is BaseTest { bytes4(0), AccessPermission.ALLOW ), - nonce: ipAccount.state() + 1, + nonce: setPermissionState, deadline: deadline }) ) @@ -144,7 +185,7 @@ contract IPAccountMetaTxTest is BaseTest { ); assertEq("test", abi.decode(result, (string))); - assertEq(ipAccount.state(), 2); + assertEq(ipAccount.state(), expectedState); } function test_IPAccount_setPermissionWithSignatureThenCallAccessControlledModule() public { @@ -158,6 +199,25 @@ contract IPAccountMetaTxTest is BaseTest { uint deadline = block.timestamp + 1000; + bytes32 expectedState = keccak256( + abi.encode( + ipAccount.state(), + abi.encodeWithSignature( + "execute(address,uint256,bytes)", + address(accessController), + 0, + abi.encodeWithSignature( + "setPermission(address,address,address,bytes4,uint8)", + address(ipAccount), + address(metaTxModule), + address(accessControlledModule), + bytes4(0), + AccessPermission.ALLOW + ) + ) + ) + ); + bytes32 digest = MessageHashUtils.toTypedDataHash( MetaTx.calculateDomainSeparator(address(ipAccount)), MetaTx.getExecuteStructHash( @@ -172,7 +232,7 @@ contract IPAccountMetaTxTest is BaseTest { bytes4(0), AccessPermission.ALLOW ), - nonce: ipAccount.state() + 1, + nonce: expectedState, deadline: deadline }) ) @@ -190,7 +250,7 @@ contract IPAccountMetaTxTest is BaseTest { ); assertEq("test", result); - assertEq(ipAccount.state(), 1); + assertEq(ipAccount.state(), expectedState); } function test_IPAccount_revert_SignatureExpired() public { @@ -204,6 +264,19 @@ contract IPAccountMetaTxTest is BaseTest { uint deadline = 0; + bytes32 currentState = ipAccount.state(); + bytes32 newState = keccak256( + abi.encode( + ipAccount.state(), + abi.encodeWithSignature( + "execute(address,uint256,bytes)", + address(module), + 0, + abi.encodeWithSignature("executeSuccessfully(string)", "test") + ) + ) + ); + bytes32 digest = MessageHashUtils.toTypedDataHash( MetaTx.calculateDomainSeparator(address(ipAccount)), MetaTx.getExecuteStructHash( @@ -211,7 +284,7 @@ contract IPAccountMetaTxTest is BaseTest { to: address(module), value: 0, data: abi.encodeWithSignature("executeSuccessfully(string)", "test"), - nonce: ipAccount.state() + 1, + nonce: newState, deadline: deadline }) ) @@ -224,7 +297,7 @@ contract IPAccountMetaTxTest is BaseTest { vm.expectRevert(abi.encodeWithSelector(Errors.IPAccount__ExpiredSignature.selector)); vm.prank(caller); metaTxModule.callAnotherModuleWithSignature(payable(address(ipAccount)), owner, deadline, signature); - assertEq(ipAccount.state(), 0); + assertEq(ipAccount.state(), currentState); } function test_IPAccount_revert_InvalidSignature() public { @@ -238,6 +311,18 @@ contract IPAccountMetaTxTest is BaseTest { uint deadline = block.timestamp + 1000; + bytes32 currentState = ipAccount.state(); + bytes32 newState = keccak256( + abi.encode( + ipAccount.state(), + abi.encodeWithSignature( + "execute(address,uint256,bytes)", + address(module), + 0, + abi.encodeWithSignature("executeSuccessfully(string)", "test") + ) + ) + ); bytes32 digest = MessageHashUtils.toTypedDataHash( MetaTx.calculateDomainSeparator(address(ipAccount)), MetaTx.getExecuteStructHash( @@ -245,7 +330,7 @@ contract IPAccountMetaTxTest is BaseTest { to: address(module), value: 0, data: abi.encodeWithSignature("executeSuccessfully(string)", "test"), - nonce: ipAccount.state() + 1, + nonce: newState, deadline: deadline }) ) @@ -258,7 +343,7 @@ contract IPAccountMetaTxTest is BaseTest { vm.expectRevert(abi.encodeWithSelector(Errors.IPAccount__InvalidSignature.selector)); vm.prank(caller); metaTxModule.callAnotherModuleWithSignature(payable(address(ipAccount)), owner, deadline, invalidSignature); - assertEq(ipAccount.state(), 0); + assertEq(ipAccount.state(), currentState); } function test_IPAccount_revert_SignatureNotMatchExecuteTargetFunction() public { @@ -271,7 +356,18 @@ contract IPAccountMetaTxTest is BaseTest { IIPAccount ipAccount = IIPAccount(payable(account)); uint deadline = block.timestamp + 1000; - + bytes32 currentState = ipAccount.state(); + bytes32 newState = keccak256( + abi.encode( + ipAccount.state(), + abi.encodeWithSignature( + "execute(address,uint256,bytes)", + address(module), + 0, + abi.encodeWithSignature("executeSuccessfully(string)", "test") + ) + ) + ); bytes32 digest = MessageHashUtils.toTypedDataHash( MetaTx.calculateDomainSeparator(address(ipAccount)), MetaTx.getExecuteStructHash( @@ -279,7 +375,7 @@ contract IPAccountMetaTxTest is BaseTest { to: address(module), value: 0, data: abi.encodeWithSignature("UnMatchedFunction(string)", "test"), - nonce: ipAccount.state() + 1, + nonce: newState, deadline: deadline }) ) @@ -292,7 +388,7 @@ contract IPAccountMetaTxTest is BaseTest { vm.expectRevert(abi.encodeWithSelector(Errors.IPAccount__InvalidSignature.selector)); vm.prank(caller); metaTxModule.callAnotherModuleWithSignature(payable(address(ipAccount)), owner, deadline, signature); - assertEq(ipAccount.state(), 0); + assertEq(ipAccount.state(), currentState); } function test_IPAccount_revert_WrongSigner() public { @@ -305,7 +401,18 @@ contract IPAccountMetaTxTest is BaseTest { IIPAccount ipAccount = IIPAccount(payable(account)); uint deadline = block.timestamp + 1000; - + bytes32 currentState = ipAccount.state(); + bytes32 newState = keccak256( + abi.encode( + ipAccount.state(), + abi.encodeWithSignature( + "execute(address,uint256,bytes)", + address(module), + 0, + abi.encodeWithSignature("executeSuccessfully(string)", "test") + ) + ) + ); bytes32 digest = MessageHashUtils.toTypedDataHash( MetaTx.calculateDomainSeparator(address(ipAccount)), MetaTx.getExecuteStructHash( @@ -313,7 +420,7 @@ contract IPAccountMetaTxTest is BaseTest { to: address(module), value: 0, data: abi.encodeWithSignature("executeSuccessfully(string)", "test"), - nonce: ipAccount.state() + 1, + nonce: newState, deadline: deadline }) ) @@ -327,7 +434,7 @@ contract IPAccountMetaTxTest is BaseTest { vm.expectRevert(abi.encodeWithSelector(Errors.IPAccount__InvalidSignature.selector)); vm.prank(caller); metaTxModule.callAnotherModuleWithSignature(payable(address(ipAccount)), owner, deadline, signature); - assertEq(ipAccount.state(), 0); + assertEq(ipAccount.state(), currentState); } function test_IPAccount_revert_SignatureForAnotherIPAccount() public { @@ -380,7 +487,18 @@ contract IPAccountMetaTxTest is BaseTest { IIPAccount ipAccount = IIPAccount(payable(account)); uint deadline = block.timestamp + 1000; - + bytes32 currentState = ipAccount.state(); + bytes32 newState = keccak256( + abi.encode( + ipAccount.state(), + abi.encodeWithSignature( + "execute(address,uint256,bytes)", + address(module), + 0, + abi.encodeWithSignature("executeSuccessfully(string)", "test") + ) + ) + ); bytes32 digest = MessageHashUtils.toTypedDataHash( MetaTx.calculateDomainSeparator(address(ipAccount)), MetaTx.getExecuteStructHash( @@ -388,7 +506,7 @@ contract IPAccountMetaTxTest is BaseTest { to: address(module), value: 0, data: abi.encodeWithSignature("executeSuccessfully(string)", "test"), - nonce: ipAccount.state() + 1, + nonce: newState, deadline: deadline }) ) @@ -410,7 +528,7 @@ contract IPAccountMetaTxTest is BaseTest { ); vm.prank(caller); metaTxModule.callAnotherModuleWithSignature(payable(address(ipAccount)), caller, deadline, signature); - assertEq(ipAccount.state(), 0); + assertEq(ipAccount.state(), currentState); } function test_IPAccount_revert_UseSignatureTwice() public { @@ -423,7 +541,17 @@ contract IPAccountMetaTxTest is BaseTest { IIPAccount ipAccount = IIPAccount(payable(account)); uint deadline = block.timestamp + 1000; - + bytes32 newState = keccak256( + abi.encode( + ipAccount.state(), + abi.encodeWithSignature( + "execute(address,uint256,bytes)", + address(module), + 0, + abi.encodeWithSignature("executeSuccessfully(string)", "test") + ) + ) + ); bytes32 digest = MessageHashUtils.toTypedDataHash( MetaTx.calculateDomainSeparator(address(ipAccount)), MetaTx.getExecuteStructHash( @@ -431,7 +559,7 @@ contract IPAccountMetaTxTest is BaseTest { to: address(module), value: 0, data: abi.encodeWithSignature("executeSuccessfully(string)", "test"), - nonce: ipAccount.state() + 1, + nonce: newState, deadline: deadline }) ) @@ -442,12 +570,12 @@ contract IPAccountMetaTxTest is BaseTest { // first time pass vm.prank(caller); metaTxModule.callAnotherModuleWithSignature(payable(address(ipAccount)), owner, deadline, signature); - assertEq(ipAccount.state(), 1); + assertEq(ipAccount.state(), newState); // second time fail vm.expectRevert(abi.encodeWithSelector(Errors.IPAccount__InvalidSignature.selector)); vm.prank(caller); metaTxModule.callAnotherModuleWithSignature(payable(address(ipAccount)), owner, deadline, signature); - assertEq(ipAccount.state(), 1); + assertEq(ipAccount.state(), newState); } function test_IPAccount_revert_signerZeroAddress() public { @@ -460,7 +588,18 @@ contract IPAccountMetaTxTest is BaseTest { IIPAccount ipAccount = IIPAccount(payable(account)); uint deadline = block.timestamp + 1000; - + bytes32 currentState = ipAccount.state(); + bytes32 newState = keccak256( + abi.encode( + ipAccount.state(), + abi.encodeWithSignature( + "execute(address,uint256,bytes)", + address(module), + 0, + abi.encodeWithSignature("executeSuccessfully(string)", "test") + ) + ) + ); bytes32 digest = MessageHashUtils.toTypedDataHash( MetaTx.calculateDomainSeparator(address(ipAccount)), MetaTx.getExecuteStructHash( @@ -468,7 +607,7 @@ contract IPAccountMetaTxTest is BaseTest { to: address(module), value: 0, data: abi.encodeWithSignature("executeSuccessfully(string)", "test"), - nonce: ipAccount.state() + 1, + nonce: newState, deadline: deadline }) ) @@ -481,7 +620,7 @@ contract IPAccountMetaTxTest is BaseTest { vm.expectRevert(abi.encodeWithSelector(Errors.IPAccount__InvalidSigner.selector)); vm.prank(caller); metaTxModule.callAnotherModuleWithSignature(payable(address(ipAccount)), address(0), deadline, signature); - assertEq(ipAccount.state(), 0); + assertEq(ipAccount.state(), currentState); } function test_IPAccount_revert_workflowFailureWithSig() public { @@ -494,7 +633,18 @@ contract IPAccountMetaTxTest is BaseTest { IIPAccount ipAccount = IIPAccount(payable(account)); uint deadline = block.timestamp + 1000; - + bytes32 currentState = ipAccount.state(); + bytes32 newState = keccak256( + abi.encode( + ipAccount.state(), + abi.encodeWithSignature( + "execute(address,uint256,bytes)", + address(module), + 0, + abi.encodeWithSignature("executeSuccessfully(string)", "test") + ) + ) + ); bytes32 digest = MessageHashUtils.toTypedDataHash( MetaTx.calculateDomainSeparator(address(ipAccount)), MetaTx.getExecuteStructHash( @@ -502,7 +652,7 @@ contract IPAccountMetaTxTest is BaseTest { to: address(module), value: 0, data: abi.encodeWithSignature("executeSuccessfully(string)", "test"), - nonce: ipAccount.state() + 1, + nonce: newState, deadline: deadline }) ) @@ -530,6 +680,6 @@ contract IPAccountMetaTxTest is BaseTest { ); vm.prank(caller); metaTxModule.workflowFailureWithSignature(payable(address(ipAccount)), owner, deadline, signature); - assertEq(ipAccount.state(), 0); + assertEq(ipAccount.state(), currentState); } } diff --git a/test/foundry/access/AccessController.t.sol b/test/foundry/access/AccessController.t.sol index 8fd2ec03..a38145cb 100644 --- a/test/foundry/access/AccessController.t.sol +++ b/test/foundry/access/AccessController.t.sol @@ -1372,12 +1372,14 @@ contract AccessControllerTest is BaseTest { ); vm.prank(owner); mockNFT.burn(tokenId); - vm.expectRevert(abi.encodeWithSelector(ERC721NonexistentToken.selector, tokenId)); - accessController.getPermission( - address(ipAccount), - signer, - address(mockModule), - mockModule.executeSuccessfully.selector + assertEq( + AccessPermission.ABSTAIN, + accessController.getPermission( + address(ipAccount), + signer, + address(mockModule), + mockModule.executeSuccessfully.selector + ) ); } @@ -1406,7 +1408,15 @@ contract AccessControllerTest is BaseTest { vm.prank(owner); mockNFT.burn(tokenId); - vm.expectRevert(abi.encodeWithSelector(ERC721NonexistentToken.selector, tokenId)); + vm.expectRevert( + abi.encodeWithSelector( + Errors.AccessController__PermissionDenied.selector, + address(ipAccount), + signer, + address(mockModule), + mockModule.executeSuccessfully.selector + ) + ); accessController.checkPermission( address(ipAccount), signer, diff --git a/yarn.lock b/yarn.lock index 4d9271c3..4760bafd 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4936,6 +4936,11 @@ slice-ansi@^4.0.0: astral-regex "^2.0.0" is-fullwidth-code-point "^3.0.0" +solady@^0.0.192: + version "0.0.192" + resolved "https://registry.yarnpkg.com/solady/-/solady-0.0.192.tgz#f80ea061903ba1914d2c96c3c69f53d66865f88f" + integrity sha512-96A4dhYkSB/xUyZIuc6l8RMbQ+nqk7GFr0hEci7/64D3G63Ial06puqXA14ZVy/xFe8nzBJbz3Mxssn7SH/1Ag== + solc@0.7.3: version "0.7.3" resolved "https://registry.yarnpkg.com/solc/-/solc-0.7.3.tgz#04646961bd867a744f63d2b4e3c0701ffdc7d78a"