From b47ea6beedaac2494f32002507f44a600d7678ed Mon Sep 17 00:00:00 2001 From: Jayden Windle Date: Fri, 6 Jan 2023 10:15:25 -0800 Subject: [PATCH] Added MinimalProxyStore + Execution Modules (#4) * moved to custom proxy implementation, added example for locked vault * Removed lock implementation using module * Moved to MinimalProxyStore + execution module pattern * Removed MinimalReceiver * Added natspec to MinimalProxyStore * Cleaned up imports and modified contract natspec * Improved natspec docs * Added test for minimal proxy store reverts * Cleaned up tests * Added test for custom execution module --- .gitmodules | 3 + lib/sstore2 | 1 + remappings.txt | 1 + script/Counter.s.sol | 12 -- src/Vault.sol | 203 ++++++++++++++++++++-------- src/VaultRegistry.sol | 198 ++++++++++++++++----------- src/interfaces/IExecutionModule.sol | 11 ++ src/interfaces/IVault.sol | 53 ++++++++ src/lib/Delegate.sol | 43 ++++++ src/lib/MinimalProxyStore.sol | 147 ++++++++++++++++++++ test/MinimalProxyStore.t.sol | 122 +++++++++++++++++ test/Vault.t.sol | 192 +++++++++----------------- test/VaultRegistry.t.sol | 32 +++++ test/mocks/MockERC1155.sol | 16 +++ test/mocks/MockERC20.sol | 12 ++ test/mocks/MockERC721.sol | 12 ++ test/mocks/MockExecutionModule.sol | 23 ++++ 17 files changed, 812 insertions(+), 269 deletions(-) create mode 160000 lib/sstore2 delete mode 100644 script/Counter.s.sol create mode 100644 src/interfaces/IExecutionModule.sol create mode 100644 src/interfaces/IVault.sol create mode 100644 src/lib/Delegate.sol create mode 100644 src/lib/MinimalProxyStore.sol create mode 100644 test/MinimalProxyStore.t.sol create mode 100644 test/VaultRegistry.t.sol create mode 100644 test/mocks/MockERC1155.sol create mode 100644 test/mocks/MockERC20.sol create mode 100644 test/mocks/MockERC721.sol create mode 100644 test/mocks/MockExecutionModule.sol diff --git a/.gitmodules b/.gitmodules index e80ffd8..b8836d4 100644 --- a/.gitmodules +++ b/.gitmodules @@ -4,3 +4,6 @@ [submodule "lib/openzeppelin-contracts"] path = lib/openzeppelin-contracts url = https://github.com/openzeppelin/openzeppelin-contracts +[submodule "lib/sstore2"] + path = lib/sstore2 + url = https://github.com/0xsequence/sstore2 diff --git a/lib/sstore2 b/lib/sstore2 new file mode 160000 index 0000000..0a28fe6 --- /dev/null +++ b/lib/sstore2 @@ -0,0 +1 @@ +Subproject commit 0a28fe61b6e81de9a05b462a24b9f4ba8c70d5b7 diff --git a/remappings.txt b/remappings.txt index a87803a..3f73cb3 100644 --- a/remappings.txt +++ b/remappings.txt @@ -2,3 +2,4 @@ ds-test/=lib/forge-std/lib/ds-test/src/ erc4626-tests/=lib/openzeppelin-contracts/lib/erc4626-tests/ forge-std/=lib/forge-std/src/ openzeppelin-contracts/=lib/openzeppelin-contracts/contracts/ +sstore2/=lib/sstore2/contracts/ diff --git a/script/Counter.s.sol b/script/Counter.s.sol deleted file mode 100644 index 0e546ab..0000000 --- a/script/Counter.s.sol +++ /dev/null @@ -1,12 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.13; - -import "forge-std/Script.sol"; - -contract CounterScript is Script { - function setUp() public {} - - function run() public { - vm.broadcast(); - } -} diff --git a/src/Vault.sol b/src/Vault.sol index fdc2d34..89d0379 100644 --- a/src/Vault.sol +++ b/src/Vault.sol @@ -1,38 +1,32 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.13; -import "forge-std/Script.sol"; - import "openzeppelin-contracts/token/ERC721/IERC721Receiver.sol"; import "openzeppelin-contracts/token/ERC1155/IERC1155Receiver.sol"; import "openzeppelin-contracts/interfaces/IERC1271.sol"; +import "openzeppelin-contracts/utils/cryptography/SignatureChecker.sol"; import "./VaultRegistry.sol"; +import "./interfaces/IVault.sol"; +import "./interfaces/IExecutionModule.sol"; +import "./lib/MinimalProxyStore.sol"; +import "./lib/Delegate.sol"; -error NotAuthorized(); - -/// @title Tokenbound Vault -/// @notice A smart contract wallet owned by a single ERC721 token. -/// @author Jayden Windle -contract Vault { - // before any transfer - // check nft ownership - // extensible as fuck - - /// @dev Address of VaultRegistry - VaultRegistry public immutable vaultRegistry; - - constructor() { - vaultRegistry = VaultRegistry(msg.sender); - } - - /// @dev Returns the owner of the token that controls this Vault - function owner() public view returns (address) { - return vaultRegistry.vaultOwner(address(this)); - } +/** + * @title A smart contract wallet owned by a single ERC721 token + * @author Jayden Windle (jaydenwindle) + */ +contract Vault is IVault { + error NotAuthorized(); /** - * @dev Executes a transaction from the Vault. Must be called by Vault owner + * @dev Address of VaultRegistry + */ + VaultRegistry public immutable registry = VaultRegistry(msg.sender); + + /** + * @dev Executes a transaction from the Vault. Must be called by an authorized sender. + * * @param to Destination address of the transaction * @param value Ether value of the transaction * @param data Encoded payload of the transaction @@ -40,14 +34,11 @@ contract Vault { function executeCall( address payable to, uint256 value, - bytes calldata data + bytes calldata data, + bool useExecutionModule ) external payable { - bool isAuthorized = vaultRegistry.isAuthorizedCaller( - address(this), - msg.sender - ); - - if (!isAuthorized) revert NotAuthorized(); + if (!_isAuthorized(msg.sender, useExecutionModule)) + revert NotAuthorized(); (bool success, bytes memory result) = to.call{value: value}(data); @@ -60,20 +51,18 @@ contract Vault { /** * @dev Executes a delegated transaction from the Vault, allowing vault - * functionality to be expanded without upgradability. Must be called by the Vault owner + * functionality to be expanded without setting an execution module. Must be called by an authorized sender. + * * @param to Contract address of the delegated call * @param data Encoded payload of the delegated call */ - function executeDelegateCall(address payable to, bytes calldata data) - external - payable - { - bool isAuthorized = vaultRegistry.isAuthorizedCaller( - address(this), - msg.sender - ); - - if (!isAuthorized) revert NotAuthorized(); + function executeDelegateCall( + address payable to, + bytes calldata data, + bool useExecutionModule + ) external payable { + if (!_isAuthorized(msg.sender, useExecutionModule)) + revert NotAuthorized(); (bool success, bytes memory result) = to.delegatecall(data); if (!success) { @@ -85,6 +74,7 @@ contract Vault { /** * @dev Implements EIP-1271 signature validation + * * @param hash Hash of the signed data * @param signature Signature to validate */ @@ -93,54 +83,157 @@ contract Vault { view returns (bytes4 magicValue) { - bool isValid = vaultRegistry.isAuthorizedSigner( + // If vault is locked, return invalid for all signatures + bool isLocked = registry.vaultLocked(address(this)); + if (isLocked) { + return ""; + } + + // If vault has an executionModule, return its verification result + address _owner = owner(); + address executionModule = registry.vaultExecutionModule( address(this), + _owner + ); + if (executionModule != address(0)) { + return + IExecutionModule(executionModule).isValidSignature( + hash, + signature + ); + } + + // Default - check if signature is valid for vault owner + bool isValid = SignatureChecker.isValidSignatureNow( + _owner, hash, signature ); - if (isValid) { return IERC1271.isValidSignature.selector; } + + return ""; } - // receiver functions + /** + * @dev Returns the owner of the token that controls this Vault (for Ownable compatibility) + * + * @return the address of the Vault owner + */ + function owner() public view returns (address) { + bytes memory context = MinimalProxyStore.getContext(address(this), 64); - /// @dev allows contract to receive Ether - receive() external payable {} + if (context.length == 0) return address(0); - /// @dev ensures that fallback calls are a noop - fallback() external payable {} + (address tokenCollection, uint256 tokenId) = abi.decode( + context, + (address, uint256) + ); - /// @dev Allows all ERC721 tokens to be received + return IERC721(tokenCollection).ownerOf(tokenId); + } + + /** + * @dev Returns true if caller is authorized to execute actions on this vault. Only uses execution module for auth + * if useExecutionModule is set to true. + * + * @param caller the address to query authorization for + * @return bool true if caller is authorized, false otherwise + */ + function _isAuthorized(address caller, bool useExecutionModule) + internal + view + returns (bool) + { + // If vault is locked, return false for all auth queries + bool isLocked = registry.vaultLocked(address(this)); + if (isLocked) { + return false; + } + + address _owner = owner(); + + // If useExecutionModule is set, lookup executionModule + address executionModule; + if (useExecutionModule) { + executionModule = registry.vaultExecutionModule( + address(this), + _owner + ); + } + + // if useExecutionModule is false or executionModule is not set, return default auth + if (executionModule == address(0)) return caller == _owner; + + // If executionModule is set, query it for auth status + return IExecutionModule(executionModule).isAuthorized(caller); + } + + /** + * @dev Returns true if caller is authorized to execute actions on this vault + * + * @param caller the address to query authorization for + * @return bool true if caller is authorized, false otherwise + */ + function isAuthorized(address caller) public view virtual returns (bool) { + return _isAuthorized(caller, true); + } + + /** + * @dev If vault is unlocked and an execution module is defined, delegate execution to the execution module + */ + fallback() external payable virtual { + address _owner = owner(); + address executionModule = registry.vaultExecutionModule( + address(this), + _owner + ); + bool isLocked = registry.vaultLocked(address(this)); + + if (!isLocked) Delegate.delegate(executionModule); + } + + /** + * @dev Allows all Ether transfers + */ + receive() external payable virtual {} + + /** + * @dev Allows all ERC721 tokens to be received + */ function onERC721Received( address, address, uint256, bytes calldata - ) external pure returns (bytes4) { + ) external pure virtual returns (bytes4) { return IERC721Receiver.onERC721Received.selector; } - /// @dev Allows all ERC1155 tokens to be received + /** + * @dev Allows all ERC1155 tokens to be received + */ function onERC1155Received( address, address, uint256, uint256, bytes calldata /* data */ - ) external pure returns (bytes4) { + ) external pure virtual returns (bytes4) { return IERC1155Receiver.onERC1155Received.selector; } - /// @dev Allows all ERC1155 token batches to be received + /** + * @dev Allows all ERC1155 token batches to be received + */ function onERC1155BatchReceived( address, address, uint256[] calldata, uint256[] calldata, bytes calldata - ) external pure returns (bytes4) { + ) external pure virtual returns (bytes4) { return IERC1155Receiver.onERC1155BatchReceived.selector; } } diff --git a/src/VaultRegistry.sol b/src/VaultRegistry.sol index 8a490e2..2342dc5 100644 --- a/src/VaultRegistry.sol +++ b/src/VaultRegistry.sol @@ -4,124 +4,170 @@ pragma solidity ^0.8.13; import "forge-std/Script.sol"; import "openzeppelin-contracts/proxy/Clones.sol"; +import "openzeppelin-contracts/utils/Create2.sol"; import "openzeppelin-contracts/token/ERC721/IERC721.sol"; import "openzeppelin-contracts/utils/cryptography/SignatureChecker.sol"; import "./Vault.sol"; +import "./lib/MinimalProxyStore.sol"; -/// @title VaultRegistry -/// @notice Determines the address for each tokenbound Vault and performs deployment of Vault instances -/// @author Jayden Windle +/** + * @title A registry for tokenbound Vaults + * @dev Determines the address for each tokenbound Vault and performs deployment of vault instances + * @author Jayden Windle (jaydenwindle) + */ contract VaultRegistry { + error NotAuthorized(); + error VaultLocked(); + + /** + * @dev Address of the default vault implementation + */ address public vaultImplementation; - struct VaultData { - address tokenCollection; - uint256 tokenId; - } + /** + * @dev Mapping from vault address to owner address to execution module address + */ + mapping(address => mapping(address => address)) private executionModule; - /// @dev mapping from vault address to VaultData - mapping(address => VaultData) public vaultData; - - /// @dev mapping from vault owner address to unlock timestamp + /** + * @dev Mapping from vault address unlock timestamp + */ mapping(address => uint256) public unlockTimestamp; - /// @dev deploys the canonical vault implementation + /** + * @dev Deploys the default Vault implementation + */ constructor() { vaultImplementation = address(new Vault()); } /** - * @dev Deploys the Vault instance for an ERC721 token. + * @dev Deploys the Vault instance for an ERC721 token. Will revert if Vault has already been deployed + * + * @param tokenCollection the contract address of the ERC721 token which will control the deployed Vault + * @param tokenId the token ID of the ERC721 token which will control the deployed Vault * @return The address of the deployed Vault */ function deployVault(address tokenCollection, uint256 tokenId) - public + external returns (address payable) { - bytes32 salt = keccak256(abi.encodePacked(tokenCollection, tokenId)); - address vaultClone = Clones.cloneDeterministic( + bytes memory encodedTokenData = abi.encode(tokenCollection, tokenId); + bytes32 salt = keccak256(encodedTokenData); + address vaultProxy = MinimalProxyStore.cloneDeterministic( vaultImplementation, + encodedTokenData, salt ); - vaultData[vaultClone] = VaultData(tokenCollection, tokenId); - - return payable(vaultClone); + return payable(vaultProxy); } /** - * @dev Gets the address of the Vault for an ERC721 token. If Vault is not deployed, - * the return value is the address that the Vault will eventually be deployed to - * @return The Vault address + * @dev Sets the execution module address for a Vault, allowing for vault owners to use a custom implementation if + * they choose to. When the token controlling the vault is transferred, the implementation address will reset + * + * @param vault the address of the vault whose execution module is being set + * @param _executionModule the address of the execution module + */ + function setExecutionModule(address vault, address _executionModule) + external + { + if (vaultLocked(vault)) revert VaultLocked(); + + if (vault.code.length == 0) revert NotAuthorized(); + + address owner = vaultOwner(vault); + if (owner != msg.sender) revert NotAuthorized(); + + executionModule[vault][owner] = _executionModule; + } + + /** + * @dev Locks a vault, preventing transactions from being executed until a certain time + * + * @param vault the vault to lock + * @param _unlockTimestamp timestamp when the vault will become unlocked + */ + function lockVault(address payable vault, uint256 _unlockTimestamp) + external + { + if (vaultLocked(vault)) revert VaultLocked(); + + if (vault.code.length == 0) revert NotAuthorized(); + + address owner = vaultOwner(vault); + if (owner != msg.sender) revert NotAuthorized(); + + unlockTimestamp[vault] = _unlockTimestamp; + } + + /** + * @dev Gets the address of the VaultProxy for an ERC721 token. If VaultProxy is + * not yet deployed, returns the address it will be deployed to + * + * @param tokenCollection the address of the ERC721 token contract + * @param tokenId the tokenId of the ERC721 token that controls the vault + * @return The VaultProxy address */ function vaultAddress(address tokenCollection, uint256 tokenId) - public + external view returns (address payable) { - bytes32 salt = keccak256(abi.encodePacked(tokenCollection, tokenId)); - address vaultClone = Clones.predictDeterministicAddress( + bytes memory encodedTokenData = abi.encode(tokenCollection, tokenId); + bytes32 salt = keccak256(encodedTokenData); + + address vaultProxy = MinimalProxyStore.predictDeterministicAddress( vaultImplementation, + encodedTokenData, salt ); - return payable(vaultClone); - } - /// @dev Returns the owner of the Vault, which is the owner of the underlying ERC721 token - function vaultOwner(address vault) public view returns (address) { - VaultData memory data = vaultData[vault]; - - if (data.tokenCollection == address(0)) { - return address(0); - } - - return IERC721(data.tokenCollection).ownerOf(data.tokenId); - } - - /// @dev Returns true if caller is authorized to call vault, false otherwise - function isAuthorizedCaller(address vault, address caller) - public - view - returns (bool) - { - return vaultOwner(vault) == caller && !isLocked(vault); - } - - /// @dev Returns true if caller is authorized to sign on behalf of vault, false otherwise - function isAuthorizedSigner( - address vault, - bytes32 hash, - bytes memory signature - ) external view returns (bool) { - address _owner = vaultOwner(vault); - - bool isAuthorized = isAuthorizedCaller(vault, _owner); - - bool isValid = SignatureChecker.isValidSignatureNow( - _owner, - hash, - signature - ); - - return isAuthorized && isValid; + return payable(vaultProxy); } /** - * @dev Disables all actions on the Vault until a certain time. Vault is - * automatically unlocked when ownership token is transferred - * @param _unlockTimestamp Timestamp at which the vault will be unlocked + * @dev Returns the implementation address for a vault + * + * @param vault the address of the vault to query implementation for + * @return the address of the vault implementation */ - function lockVault(uint256 _unlockTimestamp) external { - address _owner = vaultOwner(msg.sender); - if (unlockTimestamp[_owner] < block.timestamp) { - unlockTimestamp[_owner] = _unlockTimestamp; - } + function vaultExecutionModule(address vault, address owner) + external + view + returns (address) + { + return executionModule[vault][owner]; } - /// @dev returns true if vault is locked, false otherwise - function isLocked(address vault) public view returns (bool) { - address _owner = vaultOwner(vault); - return unlockTimestamp[_owner] > block.timestamp; + /** + * @dev Returns the owner of the Vault, which is the owner of the underlying ERC721 token + * + * @param vault the address of the vault to query ownership for + * @return the address of the vault owner + */ + function vaultOwner(address vault) public view returns (address) { + bytes memory context = MinimalProxyStore.getContext(vault, 64); + + if (context.length == 0) return address(0); + + (address tokenCollection, uint256 tokenId) = abi.decode( + context, + (address, uint256) + ); + + return IERC721(tokenCollection).ownerOf(tokenId); + } + + /** + * @dev Returns the lock status for a vault + * + * @param vault the address of the vault to query lock status for + * @return true if vault is locked, false otherwise + */ + function vaultLocked(address vault) public view returns (bool) { + return unlockTimestamp[vault] > block.timestamp; } } diff --git a/src/interfaces/IExecutionModule.sol b/src/interfaces/IExecutionModule.sol new file mode 100644 index 0000000..6bad229 --- /dev/null +++ b/src/interfaces/IExecutionModule.sol @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.13; + +interface IExecutionModule { + function isAuthorized(address caller) external view returns (bool); + + function isValidSignature(bytes32 hash, bytes memory signature) + external + view + returns (bytes4 magicValue); +} diff --git a/src/interfaces/IVault.sol b/src/interfaces/IVault.sol new file mode 100644 index 0000000..0842b2e --- /dev/null +++ b/src/interfaces/IVault.sol @@ -0,0 +1,53 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.13; + +interface IVault { + function executeCall( + address payable to, + uint256 value, + bytes calldata data, + bool useExecutionModule + ) external payable; + + function executeDelegateCall( + address payable to, + bytes calldata data, + bool useExecutionModule + ) external payable; + + receive() external payable; + + fallback() external payable; + + function onERC721Received( + address, + address, + uint256, + bytes calldata + ) external returns (bytes4); + + function onERC1155Received( + address, + address, + uint256, + uint256, + bytes calldata /* data */ + ) external returns (bytes4); + + function onERC1155BatchReceived( + address, + address, + uint256[] calldata, + uint256[] calldata, + bytes calldata + ) external returns (bytes4); + + function owner() external view returns (address); + + function isAuthorized(address caller) external view returns (bool); + + function isValidSignature(bytes32 hash, bytes memory signature) + external + view + returns (bytes4 magicValue); +} diff --git a/src/lib/Delegate.sol b/src/lib/Delegate.sol new file mode 100644 index 0000000..389dd10 --- /dev/null +++ b/src/lib/Delegate.sol @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.13; + +library Delegate { + /** + * @dev Delegates the current call to `implementation`. + * + * This function does not return to its internal call site, it will return directly to the external caller. + * + * Extracted from openzeppelin-contracts v4.6.0 (https://github.com/OpenZeppelin/openzeppelin-contracts/blob/4e5b11919e91b18b6683b6f49a1b4fdede579969/contracts/proxy/Proxy.sol#L16-L45)) + */ + function delegate(address implementation) internal { + assembly { + // Copy msg.data. We take full control of memory in this inline assembly + // block because it will not return to Solidity code. We overwrite the + // Solidity scratch pad at memory position 0. + calldatacopy(0, 0, calldatasize()) + + // Call the implementation. + // out and outsize are 0 because we don't know the size yet. + let result := delegatecall( + gas(), + implementation, + 0, + calldatasize(), + 0, + 0 + ) + + // Copy the returned data. + returndatacopy(0, 0, returndatasize()) + + switch result + // delegatecall returns 0 on error. + case 0 { + revert(0, returndatasize()) + } + default { + return(0, returndatasize()) + } + } + } +} diff --git a/src/lib/MinimalProxyStore.sol b/src/lib/MinimalProxyStore.sol new file mode 100644 index 0000000..22ae955 --- /dev/null +++ b/src/lib/MinimalProxyStore.sol @@ -0,0 +1,147 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.13; + +import "openzeppelin-contracts/utils/Create2.sol"; +import "sstore2/utils/Bytecode.sol"; + +/** + * @title A library for deploying EIP-1167 minimal proxy contracts with embedded constant data + * @author Jayden Windle (jaydenwindle) + */ +library MinimalProxyStore { + error CreateError(); + error ContextOverflow(); + + /** + * @dev Returns bytecode for a minmal proxy with additional context data appended to it + * + * @param implementation the implementation this proxy will delegate to + * @param context the data to be appended to the proxy + * @return the generated bytecode + */ + function getBytecode(address implementation, bytes memory context) + internal + pure + returns (bytes memory) + { + if (context.length > 210) revert ContextOverflow(); + + return + abi.encodePacked( + hex"3d60", // RETURNDATASIZE, PUSH1 + uint8(0x2d + context.length + 1), // size of minimal proxy (45 bytes) + size of context + hex"80600a3d3981f3363d3d373d3d3d363d73", + implementation, + hex"5af43d82803e903d91602b57fd5bf3", + hex"00", // stop byte (prevents context from executing as code) + context // appended context + ); + } + + /** + * @dev Fetches the context data stored in a deployed proxy + * + * @param instance the proxy to query context data for + * @param contextSize the length in bytes of the data that should be queried + * @return the queried context data + */ + function getContext(address instance, uint256 contextSize) + internal + view + returns (bytes memory) + { + if (contextSize > 210) revert ContextOverflow(); + + uint256 instanceCodeLength = instance.code.length; + + return + Bytecode.codeAt( + instance, + instanceCodeLength - contextSize, + instanceCodeLength + ); + } + + /** + * @dev Deploys and returns the address of a clone with stored context data that mimics the behaviour of `implementation`. + * + * This function uses the create opcode, which should never revert. + * + * @param implementation the implementation to delegate to + * @param context context data to be stored in the proxy + * @return instance the address of the deployed proxy + */ + function clone(address implementation, bytes memory context) + internal + returns (address instance) + { + // Generate bytecode for proxy + bytes memory code = getBytecode(implementation, context); + + // Deploy contract using create + assembly { + instance := create(0, add(code, 32), mload(code)) + } + + // If address is zero, deployment failed + if (instance == address(0)) revert CreateError(); + } + + /** + * @dev Deploys and returns the address of a clone with stored context data that mimics the behaviour of `implementation`. + * + * This function uses the create2 opcode and a `salt` to deterministically deploy + * the clone. Using the same `implementation` and `salt` multiple time will revert, since + * the clones cannot be deployed twice at the same address. + * + * @param implementation the implementation to delegate to + * @param context context data to be stored in the proxy + * @return instance the address of the deployed proxy + */ + function cloneDeterministic( + address implementation, + bytes memory context, + bytes32 salt + ) internal returns (address instance) { + bytes memory code = getBytecode(implementation, context); + + // Deploy contract using create2 + assembly { + instance := create2(0, add(code, 32), mload(code), salt) + } + + // If address is zero, deployment failed + if (instance == address(0)) revert CreateError(); + } + + /** + * @dev Computes the address of a clone deployed using {MinimalProxyStore-cloneDeterministic}. + */ + function predictDeterministicAddress( + address implementation, + bytes memory context, + bytes32 salt, + address deployer + ) internal pure returns (address predicted) { + bytes memory code = getBytecode(implementation, context); + + return Create2.computeAddress(salt, keccak256(code), deployer); + } + + /** + * @dev Computes the address of a clone deployed using {MinimalProxyStore-cloneDeterministic}. + */ + function predictDeterministicAddress( + address implementation, + bytes memory context, + bytes32 salt + ) internal view returns (address predicted) { + return + predictDeterministicAddress( + implementation, + context, + salt, + address(this) + ); + } +} diff --git a/test/MinimalProxyStore.t.sol b/test/MinimalProxyStore.t.sol new file mode 100644 index 0000000..afd9b71 --- /dev/null +++ b/test/MinimalProxyStore.t.sol @@ -0,0 +1,122 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.13; + +import "forge-std/Test.sol"; + +import "../src/lib/MinimalProxyStore.sol"; + +contract TestContract { + error Failed(); + + function test() public pure returns (uint256) { + return 123; + } + + function fails() public pure { + revert Failed(); + } +} + +contract MinimalProxyStoreTest is Test { + function testDeploymentSucceeds() public { + TestContract testContract = new TestContract(); + + address clone = MinimalProxyStore.clone( + address(testContract), + abi.encode("hello") + ); + + assertTrue(clone != address(0)); + assertEq(TestContract(clone).test(), 123); + } + + function testReverts() public { + TestContract testContract = new TestContract(); + + address clone = MinimalProxyStore.clone( + address(testContract), + abi.encode("hello") + ); + + assertTrue(clone != address(0)); + vm.expectRevert(TestContract.Failed.selector); + TestContract(clone).fails(); + } + + function testGetContext() public { + TestContract testContract = new TestContract(); + + bytes memory context = abi.encode("hello"); + + address clone = MinimalProxyStore.clone(address(testContract), context); + + assertTrue(clone != address(0)); + assertEq(TestContract(clone).test(), 123); + + bytes memory recoveredContext = MinimalProxyStore.getContext( + clone, + context.length + ); + + assertEq(recoveredContext, context); + } + + function testCreate2() public { + TestContract testContract = new TestContract(); + + bytes memory context = abi.encode("hello"); + + address clone = MinimalProxyStore.cloneDeterministic( + address(testContract), + context, + keccak256("hello") + ); + + assertTrue(clone != address(0)); + assertEq(TestContract(clone).test(), 123); + + bytes memory recoveredContext = MinimalProxyStore.getContext( + clone, + context.length + ); + + assertEq(recoveredContext, context); + + address predictedAddress = MinimalProxyStore + .predictDeterministicAddress( + address(testContract), + context, + keccak256("hello") + ); + + assertEq(clone, predictedAddress); + } + + function testRedeploymentFails() public { + TestContract testContract = new TestContract(); + + bytes memory context = abi.encode("hello"); + + MinimalProxyStore.cloneDeterministic( + address(testContract), + context, + keccak256("hello") + ); + vm.expectRevert(MinimalProxyStore.CreateError.selector); + MinimalProxyStore.cloneDeterministic( + address(testContract), + context, + keccak256("hello") + ); + } + + function testCannotOverflowContext() public { + bytes memory largeContext = new bytes(211); + + vm.expectRevert(MinimalProxyStore.ContextOverflow.selector); + MinimalProxyStore.getBytecode(address(this), largeContext); + + vm.expectRevert(MinimalProxyStore.ContextOverflow.selector); + MinimalProxyStore.getContext(address(this), 211); + } +} diff --git a/test/Vault.t.sol b/test/Vault.t.sol index 4ff86be..541f1b6 100644 --- a/test/Vault.t.sol +++ b/test/Vault.t.sol @@ -3,52 +3,37 @@ pragma solidity ^0.8.13; import "forge-std/Test.sol"; -import "openzeppelin-contracts/token/ERC721/ERC721.sol"; -import "openzeppelin-contracts/token/ERC1155/ERC1155.sol"; import "openzeppelin-contracts/token/ERC20/ERC20.sol"; import "../src/Vault.sol"; import "../src/VaultRegistry.sol"; -contract VaultCollectionTest is Test { - DummyERC721 public dummyERC721; - DummyERC1155 public dummyERC1155; - DummyERC20 public dummyERC20; +import "./mocks/MockERC721.sol"; +import "./mocks/MockERC1155.sol"; +import "./mocks/MockERC20.sol"; +import "./mocks/MockExecutionModule.sol"; + +contract VaultTest is Test { + MockERC721 public dummyERC721; + MockERC1155 public dummyERC1155; + MockERC20 public dummyERC20; VaultRegistry public vaultRegistry; - TokenCollection public tokenCollection; - - event Initialized(uint8 version); + MockERC721 public tokenCollection; function setUp() public { - dummyERC721 = new DummyERC721(); - dummyERC1155 = new DummyERC1155(); - dummyERC20 = new DummyERC20(); + dummyERC721 = new MockERC721(); + dummyERC1155 = new MockERC1155(); + dummyERC20 = new MockERC20(); vaultRegistry = new VaultRegistry(); - tokenCollection = new TokenCollection(); + tokenCollection = new MockERC721(); } - function testDeployVault(uint256 tokenId) public { - assertTrue(address(vaultRegistry) != address(0)); - - address predictedVaultAddress = vaultRegistry.vaultAddress( - address(tokenCollection), - tokenId - ); - - address vaultAddress = vaultRegistry.deployVault( - address(tokenCollection), - tokenId - ); - - assertTrue(vaultAddress != address(0)); - assertTrue(vaultAddress == predictedVaultAddress); - } - - function testTransferETHPreDeploy(uint256 tokenId) public { + function testTransferETHPreDeploy() public { + uint256 tokenId = 1; address user1 = vm.addr(1); vm.deal(user1, 0.2 ether); @@ -82,7 +67,7 @@ contract VaultCollectionTest is Test { // user1 executes transaction to send ETH from vault vm.prank(user1); - vault.executeCall(payable(user1), 0.1 ether, ""); + vault.executeCall(payable(user1), 0.1 ether, "", false); // success! assertEq(vaultAddress.balance, 0.1 ether); @@ -111,7 +96,7 @@ contract VaultCollectionTest is Test { Vault vault = Vault(vaultAddress); vm.prank(user1); - vault.executeCall(payable(user1), 0.1 ether, ""); + vault.executeCall(payable(user1), 0.1 ether, "", false); assertEq(vaultAddress.balance, 0.1 ether); assertEq(user1.balance, 0.1 ether); @@ -145,7 +130,12 @@ contract VaultCollectionTest is Test { 1 ether ); vm.prank(user1); - vault.executeCall(payable(address(dummyERC20)), 0, erc20TransferCall); + vault.executeCall( + payable(address(dummyERC20)), + 0, + erc20TransferCall, + false + ); assertEq(dummyERC20.balanceOf(vaultAddress), 0); assertEq(dummyERC20.balanceOf(user1), 1 ether); @@ -174,7 +164,12 @@ contract VaultCollectionTest is Test { 1 ether ); vm.prank(user1); - vault.executeCall(payable(address(dummyERC20)), 0, erc20TransferCall); + vault.executeCall( + payable(address(dummyERC20)), + 0, + erc20TransferCall, + false + ); assertEq(dummyERC20.balanceOf(vaultAddress), 0); assertEq(dummyERC20.balanceOf(user1), 1 ether); @@ -214,7 +209,8 @@ contract VaultCollectionTest is Test { vault.executeCall( payable(address(dummyERC1155)), 0, - erc1155TransferCall + erc1155TransferCall, + false ); assertEq(dummyERC1155.balanceOf(vaultAddress, 1), 0); @@ -250,7 +246,8 @@ contract VaultCollectionTest is Test { vault.executeCall( payable(address(dummyERC1155)), 0, - erc1155TransferCall + erc1155TransferCall, + false ); assertEq(dummyERC1155.balanceOf(vaultAddress, 1), 0); @@ -287,7 +284,12 @@ contract VaultCollectionTest is Test { 1 ); vm.prank(user1); - vault.executeCall(payable(address(dummyERC721)), 0, erc721TransferCall); + vault.executeCall( + payable(address(dummyERC721)), + 0, + erc721TransferCall, + false + ); assertEq(dummyERC721.balanceOf(address(vaultAddress)), 0); assertEq(dummyERC721.balanceOf(user1), 1); @@ -319,7 +321,12 @@ contract VaultCollectionTest is Test { 1 ); vm.prank(user1); - vault.executeCall(payable(address(dummyERC721)), 0, erc721TransferCall); + vault.executeCall( + payable(address(dummyERC721)), + 0, + erc721TransferCall, + false + ); assertEq(dummyERC721.balanceOf(vaultAddress), 0); assertEq(dummyERC721.balanceOf(user1), 1); @@ -344,8 +351,8 @@ contract VaultCollectionTest is Test { // should fail if user2 tries to use vault vm.prank(user2); - vm.expectRevert(NotAuthorized.selector); - vault.executeCall(payable(user2), 0.1 ether, ""); + vm.expectRevert(Vault.NotAuthorized.selector); + vault.executeCall(payable(user2), 0.1 ether, "", false); } function testVaultOwnershipTransfer(uint256 tokenId) public { @@ -366,15 +373,15 @@ contract VaultCollectionTest is Test { // should fail if user2 tries to use vault vm.prank(user2); - vm.expectRevert(NotAuthorized.selector); - vault.executeCall(payable(user2), 0.1 ether, ""); + vm.expectRevert(Vault.NotAuthorized.selector); + vault.executeCall(payable(user2), 0.1 ether, "", false); vm.prank(user1); tokenCollection.safeTransferFrom(user1, user2, tokenId); // should succeed now that user2 is owner vm.prank(user2); - vault.executeCall(payable(user2), 0.1 ether, ""); + vault.executeCall(payable(user2), 0.1 ether, "", false); assertEq(user2.balance, 0.1 ether); } @@ -447,16 +454,12 @@ contract VaultCollectionTest is Test { // lock vault for 10 days uint256 unlockTimestamp = block.timestamp + 10 days; vm.prank(user1); - vault.executeCall( - payable(address(vaultRegistry)), - 0, - abi.encodeWithSignature("lockVault(uint256)", unlockTimestamp) - ); + vaultRegistry.lockVault(vaultAddress, unlockTimestamp); - // transaction should fail if vault is locked + // transaction should revert if vault is locked vm.prank(user1); - vm.expectRevert(NotAuthorized.selector); - vault.executeCall(payable(user1), 0.1 ether, ""); + vm.expectRevert(Vault.NotAuthorized.selector); + vault.executeCall(payable(user1), 1 ether, "", false); // signing should fail if vault is locked bytes32 hash = keccak256("This is a signed message"); @@ -470,7 +473,7 @@ contract VaultCollectionTest is Test { // transaction succeed now that vault lock has expired vm.prank(user1); - vault.executeCall(payable(user1), 1 ether, ""); + vault.executeCall(payable(user1), 1 ether, "", false); assertEq(user1.balance, 1 ether); // signing should now that vault lock has expired @@ -484,9 +487,8 @@ contract VaultCollectionTest is Test { assertEq(returnValue1, IERC1271.isValidSignature.selector); } - function testVaultUnlocksAfterTransfer(uint256 tokenId) public { + function testCustomExecutionModule(uint256 tokenId) public { address user1 = vm.addr(1); - address user2 = vm.addr(2); tokenCollection.mint(user1, tokenId); assertEq(tokenCollection.ownerOf(tokenId), user1); @@ -500,80 +502,18 @@ contract VaultCollectionTest is Test { Vault vault = Vault(vaultAddress); - // lock vault for 10 days - uint256 unlockTimestamp = block.timestamp + 10 days; + MockExecutionModule mockExecutionModule = new MockExecutionModule(); + vm.prank(user1); - vault.executeCall( - payable(address(vaultRegistry)), - 0, - abi.encodeWithSignature("lockVault(uint256)", unlockTimestamp) + vaultRegistry.setExecutionModule( + vaultAddress, + address(mockExecutionModule) ); - // transaction should fail if vault is locked - vm.prank(user1); - vm.expectRevert(NotAuthorized.selector); - vault.executeCall(payable(user1), 0.1 ether, ""); + // execution module overrides authorization check + assertTrue(vault.isAuthorized(vm.addr(2))); - // signing should fail if vault is locked - bytes32 hash = keccak256("This is a signed message"); - (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(2, hash); - bytes memory signature1 = abi.encodePacked(r1, s1, v1); - bytes4 returnValue = vault.isValidSignature(hash, signature1); - assertEq(returnValue, 0); - - // transfer vault to new owner - vm.prank(user1); - tokenCollection.safeTransferFrom(user1, user2, tokenId); - - // transaction succeed now that vault ownership has transferred - vm.prank(user2); - vault.executeCall(payable(user2), 1 ether, ""); - assertEq(user2.balance, 1 ether); - - // signing should now that vault vault ownership has transferred - bytes32 hashAfterUnlock = keccak256("This is a signed message"); - (uint8 v2, bytes32 r2, bytes32 s2) = vm.sign(2, hashAfterUnlock); - bytes memory signature2 = abi.encodePacked(r2, s2, v2); - bytes4 returnValue1 = vault.isValidSignature( - hashAfterUnlock, - signature2 - ); - assertEq(returnValue1, IERC1271.isValidSignature.selector); - } -} - -contract TokenCollection is ERC721 { - constructor() ERC721("TokenCollection", "TC") {} - - function mint(address to, uint256 tokenId) external { - _safeMint(to, tokenId); - } -} - -contract DummyERC721 is ERC721 { - constructor() ERC721("DummyERC721", "T721") {} - - function mint(address to, uint256 tokenId) external { - _safeMint(to, tokenId); - } -} - -contract DummyERC1155 is ERC1155 { - constructor() ERC1155("http://DummyERC1155.com") {} - - function mint( - address to, - uint256 tokenId, - uint256 amount - ) external { - _mint(to, tokenId, amount, ""); - } -} - -contract DummyERC20 is ERC20 { - constructor() ERC20("DummyERC20", "T20") {} - - function mint(address to, uint256 amount) external { - _mint(to, amount); + // execution module handles fallback calls + assertEq(MockExecutionModule(vaultAddress).customFunction(), 12345); } } diff --git a/test/VaultRegistry.t.sol b/test/VaultRegistry.t.sol new file mode 100644 index 0000000..6ba5302 --- /dev/null +++ b/test/VaultRegistry.t.sol @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.13; + +import "forge-std/Test.sol"; + +import "../src/Vault.sol"; +import "../src/VaultRegistry.sol"; + +contract VaultRegistryTest is Test { + VaultRegistry public vaultRegistry; + + function setUp() public { + vaultRegistry = new VaultRegistry(); + } + + function testDeployVault(uint256 tokenId) public { + assertTrue(address(vaultRegistry) != address(0)); + + address predictedVaultAddress = vaultRegistry.vaultAddress( + vm.addr(1337), + tokenId + ); + + address vaultAddress = vaultRegistry.deployVault( + vm.addr(1337), + tokenId + ); + + assertTrue(vaultAddress != address(0)); + assertTrue(vaultAddress == predictedVaultAddress); + } +} diff --git a/test/mocks/MockERC1155.sol b/test/mocks/MockERC1155.sol new file mode 100644 index 0000000..7462141 --- /dev/null +++ b/test/mocks/MockERC1155.sol @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.13; + +import "openzeppelin-contracts/token/ERC1155/ERC1155.sol"; + +contract MockERC1155 is ERC1155 { + constructor() ERC1155("http://MockERC1155.com") {} + + function mint( + address to, + uint256 tokenId, + uint256 amount + ) external { + _mint(to, tokenId, amount, ""); + } +} diff --git a/test/mocks/MockERC20.sol b/test/mocks/MockERC20.sol new file mode 100644 index 0000000..bd82327 --- /dev/null +++ b/test/mocks/MockERC20.sol @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.13; + +import "openzeppelin-contracts/token/ERC20/ERC20.sol"; + +contract MockERC20 is ERC20 { + constructor() ERC20("MockERC20", "T20") {} + + function mint(address to, uint256 amount) external { + _mint(to, amount); + } +} diff --git a/test/mocks/MockERC721.sol b/test/mocks/MockERC721.sol new file mode 100644 index 0000000..9171ee4 --- /dev/null +++ b/test/mocks/MockERC721.sol @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.13; + +import "openzeppelin-contracts/token/ERC721/ERC721.sol"; + +contract MockERC721 is ERC721 { + constructor() ERC721("MockERC721", "M721") {} + + function mint(address to, uint256 tokenId) external { + _safeMint(to, tokenId); + } +} diff --git a/test/mocks/MockExecutionModule.sol b/test/mocks/MockExecutionModule.sol new file mode 100644 index 0000000..56622f2 --- /dev/null +++ b/test/mocks/MockExecutionModule.sol @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.13; + +import "openzeppelin-contracts/interfaces/IERC1271.sol"; +import "../../src/interfaces/IExecutionModule.sol"; + +contract MockExecutionModule is IExecutionModule { + function isAuthorized(address) external pure returns (bool) { + return true; + } + + function isValidSignature(bytes32, bytes memory) + external + pure + returns (bytes4 magicValue) + { + return IERC1271.isValidSignature.selector; + } + + function customFunction() external pure returns (uint256) { + return 12345; + } +}