diff --git a/src/MinimalReceiver.sol b/src/MinimalReceiver.sol new file mode 100644 index 0000000..25e6c40 --- /dev/null +++ b/src/MinimalReceiver.sol @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.13; + +import "openzeppelin-contracts/token/ERC721/utils/ERC721Holder.sol"; +import "openzeppelin-contracts/token/ERC1155/utils/ERC1155Holder.sol"; + +contract MinimalReceiver is ERC721Holder, ERC1155Holder { + /** + * @dev Allows all Ether transfers + */ + receive() external payable virtual {} +} diff --git a/src/Vault.sol b/src/Vault.sol index 18fea70..78e20fa 100644 --- a/src/Vault.sol +++ b/src/Vault.sol @@ -1,28 +1,63 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.13; +import "forge-std/Script.sol"; + +import "openzeppelin-contracts/token/ERC721/IERC721.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 "openzeppelin-contracts/utils/Address.sol"; -import "./VaultRegistry.sol"; +import "./MinimalReceiver.sol"; import "./interfaces/IVault.sol"; -import "./interfaces/IExecutionModule.sol"; import "./lib/MinimalProxyStore.sol"; -import "./lib/Delegate.sol"; /** * @title A smart contract wallet owned by a single ERC721 token * @author Jayden Windle (jaydenwindle) */ -contract Vault is IVault { +contract Vault is IVault, MinimalReceiver { error NotAuthorized(); + error VaultLocked(); /** - * @dev Address of VaultRegistry + * @dev Timestamp at which Vault will unlock */ - VaultRegistry public immutable registry = VaultRegistry(msg.sender); + uint256 public unlockTimestamp; + + /** + * @dev Mapping from owner address to executor address + */ + mapping(address => address) public executor; + + /** + * @dev If vault is unlocked and an executor is set, pass call to executor + */ + fallback(bytes calldata data) + external + payable + returns (bytes memory result) + { + if (unlockTimestamp > block.timestamp) revert VaultLocked(); + + address _owner = owner(); + address _executor = executor[_owner]; + + // accept funds if executor is undefined or cannot be called + if (_executor == address(0)) return ""; + if (_executor.code.length == 0) return ""; + + bool success; + (success, result) = _executor.call(data); + + if (!success) { + assembly { + revert(add(result, 32), mload(result)) + } + } + } /** * @dev Executes a transaction from the Vault. Must be called by an authorized sender. @@ -34,13 +69,13 @@ contract Vault is IVault { function executeCall( address payable to, uint256 value, - bytes calldata data, - bool useExecutionModule - ) external payable { - if (!_isAuthorized(msg.sender, useExecutionModule)) - revert NotAuthorized(); + bytes calldata data + ) external payable returns (bytes memory result) { + if (unlockTimestamp > block.timestamp) revert VaultLocked(); + if (!isOwnerOrExecutor(msg.sender)) revert NotAuthorized(); - (bool success, bytes memory result) = to.call{value: value}(data); + bool success; + (success, result) = to.call{value: value}(data); if (!success) { assembly { @@ -50,26 +85,51 @@ contract Vault is IVault { } /** - * @dev Executes a delegated transaction from the Vault, allowing vault - * functionality to be expanded without setting an execution module. Must be called by an authorized sender. + * @dev Sets executior address for Vault, allowing owner to use a custom implementation if they choose to. + * When the token controlling the vault is transferred, the implementation address will reset * - * @param to Contract address of the delegated call - * @param data Encoded payload of the delegated call + * @param _executionModule the address of the execution module */ - function executeDelegateCall( - address payable to, - bytes calldata data, - bool useExecutionModule - ) external payable { - if (!_isAuthorized(msg.sender, useExecutionModule)) - revert NotAuthorized(); + function setExecutor(address _executionModule) external { + if (unlockTimestamp > block.timestamp) revert VaultLocked(); - (bool success, bytes memory result) = to.delegatecall(data); - if (!success) { - assembly { - revert(add(result, 32), mload(result)) - } - } + address _owner = owner(); + if (_owner != msg.sender) revert NotAuthorized(); + + executor[_owner] = _executionModule; + } + + /** + * @dev Locks Vault, preventing transactions from being executed until a certain time + * + * @param _unlockTimestamp timestamp when the vault will become unlocked + */ + function lock(uint256 _unlockTimestamp) external { + if (unlockTimestamp > block.timestamp) revert VaultLocked(); + + address _owner = owner(); + if (_owner != msg.sender) revert NotAuthorized(); + + unlockTimestamp = _unlockTimestamp; + } + + /** + * @dev Returns Vault lock status + * + * @return true if Vault is locked, false otherwise + */ + function isLocked() external view returns (bool) { + return unlockTimestamp > block.timestamp; + } + + /** + * @dev Returns true if caller is authorized to execute actions on this vault + * + * @param caller the address to query authorization for + * @return true if caller is authorized, false otherwise + */ + function isAuthorized(address caller) external view returns (bool) { + return isOwnerOrExecutor(caller); } /** @@ -83,33 +143,22 @@ contract Vault is IVault { view returns (bytes4 magicValue) { - // If vault is locked, return invalid for all signatures - bool isLocked = registry.vaultLocked(address(this)); - if (isLocked) { - return ""; - } + // If vault is locked, disable signing + if (unlockTimestamp > block.timestamp) return ""; - // If vault has an executionModule, return its verification result + // If vault has an executor, check if executor signature is valid address _owner = owner(); - address executionModule = registry.vaultExecutionModule( - address(this), - _owner - ); - if (executionModule != address(0)) { - return - IExecutionModule(executionModule).isValidSignature( - hash, - signature - ); + address _executor = executor[_owner]; + + if ( + _executor != address(0) && + SignatureChecker.isValidSignatureNow(_executor, hash, signature) + ) { + return IERC1271.isValidSignature.selector; } // Default - check if signature is valid for vault owner - bool isValid = SignatureChecker.isValidSignatureNow( - _owner, - hash, - signature - ); - if (isValid) { + if (SignatureChecker.isValidSignatureNow(_owner, hash, signature)) { return IERC1271.isValidSignature.selector; } @@ -117,7 +166,7 @@ contract Vault is IVault { } /** - * @dev Returns the owner of the token that controls this Vault (for Ownable compatibility) + * @dev Returns the owner of the token that controls this Vault (public for Ownable compatibility) * * @return the address of the Vault owner */ @@ -135,105 +184,18 @@ contract Vault is IVault { } /** - * @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. + * @dev Returns true if caller is owner or ececutor * - * @param caller the address to query authorization for - * @return bool true if caller is authorized, false otherwise + * @param caller the address to query for + * @return true if caller is owner or executor, 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; - } - + function isOwnerOrExecutor(address caller) internal view returns (bool) { address _owner = owner(); + if (caller == _owner) return true; - // If useExecutionModule is set, lookup executionModule - address executionModule; - if (useExecutionModule) { - executionModule = registry.vaultExecutionModule( - address(this), - _owner - ); - } + address _executor = executor[_owner]; + if (caller == _executor) return true; - // 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 virtual returns (bytes4) { - return IERC721Receiver.onERC721Received.selector; - } - - /** - * @dev Allows all ERC1155 tokens to be received - */ - function onERC1155Received( - address, - address, - uint256, - uint256, - bytes calldata /* data */ - ) external pure virtual returns (bytes4) { - return IERC1155Receiver.onERC1155Received.selector; - } - - /** - * @dev Allows all ERC1155 token batches to be received - */ - function onERC1155BatchReceived( - address, - address, - uint256[] calldata, - uint256[] calldata, - bytes calldata - ) external pure virtual returns (bytes4) { - return IERC1155Receiver.onERC1155BatchReceived.selector; + return false; } } diff --git a/src/VaultRegistry.sol b/src/VaultRegistry.sol index 57aa92e..6e60983 100644 --- a/src/VaultRegistry.sol +++ b/src/VaultRegistry.sol @@ -25,16 +25,6 @@ contract VaultRegistry { */ address public vaultImplementation; - /** - * @dev Mapping from vault address to owner address to execution module address - */ - mapping(address => mapping(address => address)) private executionModule; - - /** - * @dev Mapping from vault address unlock timestamp - */ - mapping(address => uint256) public unlockTimestamp; - /** * @dev Deploys the default Vault implementation */ @@ -64,45 +54,6 @@ contract VaultRegistry { return payable(vaultProxy); } - /** - * @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 @@ -127,47 +78,4 @@ contract VaultRegistry { return payable(vaultProxy); } - - /** - * @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 vaultExecutionModule(address vault, address owner) - external - view - returns (address) - { - return executionModule[vault][owner]; - } - - /** - * @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); - - 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 deleted file mode 100644 index 6bad229..0000000 --- a/src/interfaces/IExecutionModule.sol +++ /dev/null @@ -1,11 +0,0 @@ -// 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 index 0842b2e..4b7600f 100644 --- a/src/interfaces/IVault.sol +++ b/src/interfaces/IVault.sol @@ -1,53 +1,22 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.13; -interface IVault { +import "openzeppelin-contracts/interfaces/IERC1271.sol"; + +interface IVault is IERC1271 { function executeCall( address payable to, uint256 value, - bytes calldata data, - bool useExecutionModule - ) external payable; + bytes calldata data + ) external payable returns (bytes memory); - function executeDelegateCall( - address payable to, - bytes calldata data, - bool useExecutionModule - ) external payable; + function executor(address owner) external view returns (address); + function setExecutor(address _executionModule) external; - 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 isLocked() external view returns (bool); + function lock(uint256 _unlockTimestamp) external; function isAuthorized(address caller) external view returns (bool); - function isValidSignature(bytes32 hash, bytes memory signature) - external - view - returns (bytes4 magicValue); + function owner() external view returns (address); } diff --git a/src/lib/Delegate.sol b/src/lib/Delegate.sol deleted file mode 100644 index 389dd10..0000000 --- a/src/lib/Delegate.sol +++ /dev/null @@ -1,43 +0,0 @@ -// 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/test/Vault.t.sol b/test/Vault.t.sol index 541f1b6..6339d14 100644 --- a/test/Vault.t.sol +++ b/test/Vault.t.sol @@ -11,7 +11,7 @@ import "../src/VaultRegistry.sol"; import "./mocks/MockERC721.sol"; import "./mocks/MockERC1155.sol"; import "./mocks/MockERC20.sol"; -import "./mocks/MockExecutionModule.sol"; +import "./mocks/MockExecutor.sol"; contract VaultTest is Test { MockERC721 public dummyERC721; @@ -67,7 +67,7 @@ contract VaultTest is Test { // user1 executes transaction to send ETH from vault vm.prank(user1); - vault.executeCall(payable(user1), 0.1 ether, "", false); + vault.executeCall(payable(user1), 0.1 ether, ""); // success! assertEq(vaultAddress.balance, 0.1 ether); @@ -96,7 +96,7 @@ contract VaultTest is Test { Vault vault = Vault(vaultAddress); vm.prank(user1); - vault.executeCall(payable(user1), 0.1 ether, "", false); + vault.executeCall(payable(user1), 0.1 ether, ""); assertEq(vaultAddress.balance, 0.1 ether); assertEq(user1.balance, 0.1 ether); @@ -130,12 +130,7 @@ contract VaultTest is Test { 1 ether ); vm.prank(user1); - vault.executeCall( - payable(address(dummyERC20)), - 0, - erc20TransferCall, - false - ); + vault.executeCall(payable(address(dummyERC20)), 0, erc20TransferCall); assertEq(dummyERC20.balanceOf(vaultAddress), 0); assertEq(dummyERC20.balanceOf(user1), 1 ether); @@ -164,12 +159,7 @@ contract VaultTest is Test { 1 ether ); vm.prank(user1); - vault.executeCall( - payable(address(dummyERC20)), - 0, - erc20TransferCall, - false - ); + vault.executeCall(payable(address(dummyERC20)), 0, erc20TransferCall); assertEq(dummyERC20.balanceOf(vaultAddress), 0); assertEq(dummyERC20.balanceOf(user1), 1 ether); @@ -209,8 +199,7 @@ contract VaultTest is Test { vault.executeCall( payable(address(dummyERC1155)), 0, - erc1155TransferCall, - false + erc1155TransferCall ); assertEq(dummyERC1155.balanceOf(vaultAddress, 1), 0); @@ -246,8 +235,7 @@ contract VaultTest is Test { vault.executeCall( payable(address(dummyERC1155)), 0, - erc1155TransferCall, - false + erc1155TransferCall ); assertEq(dummyERC1155.balanceOf(vaultAddress, 1), 0); @@ -284,12 +272,7 @@ contract VaultTest is Test { 1 ); vm.prank(user1); - vault.executeCall( - payable(address(dummyERC721)), - 0, - erc721TransferCall, - false - ); + vault.executeCall(payable(address(dummyERC721)), 0, erc721TransferCall); assertEq(dummyERC721.balanceOf(address(vaultAddress)), 0); assertEq(dummyERC721.balanceOf(user1), 1); @@ -321,12 +304,7 @@ contract VaultTest is Test { 1 ); vm.prank(user1); - vault.executeCall( - payable(address(dummyERC721)), - 0, - erc721TransferCall, - false - ); + vault.executeCall(payable(address(dummyERC721)), 0, erc721TransferCall); assertEq(dummyERC721.balanceOf(vaultAddress), 0); assertEq(dummyERC721.balanceOf(user1), 1); @@ -352,7 +330,7 @@ contract VaultTest is Test { // should fail if user2 tries to use vault vm.prank(user2); vm.expectRevert(Vault.NotAuthorized.selector); - vault.executeCall(payable(user2), 0.1 ether, "", false); + vault.executeCall(payable(user2), 0.1 ether, ""); } function testVaultOwnershipTransfer(uint256 tokenId) public { @@ -374,14 +352,14 @@ contract VaultTest is Test { // should fail if user2 tries to use vault vm.prank(user2); vm.expectRevert(Vault.NotAuthorized.selector); - vault.executeCall(payable(user2), 0.1 ether, "", false); + vault.executeCall(payable(user2), 0.1 ether, ""); 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, "", false); + vault.executeCall(payable(user2), 0.1 ether, ""); assertEq(user2.balance, 0.1 ether); } @@ -454,12 +432,12 @@ contract VaultTest is Test { // lock vault for 10 days uint256 unlockTimestamp = block.timestamp + 10 days; vm.prank(user1); - vaultRegistry.lockVault(vaultAddress, unlockTimestamp); + vault.lock(unlockTimestamp); // transaction should revert if vault is locked vm.prank(user1); - vm.expectRevert(Vault.NotAuthorized.selector); - vault.executeCall(payable(user1), 1 ether, "", false); + vm.expectRevert(Vault.VaultLocked.selector); + vault.executeCall(payable(user1), 1 ether, ""); // signing should fail if vault is locked bytes32 hash = keccak256("This is a signed message"); @@ -473,7 +451,7 @@ contract VaultTest is Test { // transaction succeed now that vault lock has expired vm.prank(user1); - vault.executeCall(payable(user1), 1 ether, "", false); + vault.executeCall(payable(user1), 1 ether, ""); assertEq(user1.balance, 1 ether); // signing should now that vault lock has expired @@ -502,18 +480,12 @@ contract VaultTest is Test { Vault vault = Vault(vaultAddress); - MockExecutionModule mockExecutionModule = new MockExecutionModule(); + MockExecutor mockExecutor = new MockExecutor(); vm.prank(user1); - vaultRegistry.setExecutionModule( - vaultAddress, - address(mockExecutionModule) - ); - - // execution module overrides authorization check - assertTrue(vault.isAuthorized(vm.addr(2))); + vault.setExecutor(address(mockExecutor)); // execution module handles fallback calls - assertEq(MockExecutionModule(vaultAddress).customFunction(), 12345); + assertEq(MockExecutor(vaultAddress).customFunction(), 12345); } } diff --git a/test/mocks/MockExecutionModule.sol b/test/mocks/MockExecutionModule.sol deleted file mode 100644 index 56622f2..0000000 --- a/test/mocks/MockExecutionModule.sol +++ /dev/null @@ -1,23 +0,0 @@ -// 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; - } -} diff --git a/test/mocks/MockExecutor.sol b/test/mocks/MockExecutor.sol new file mode 100644 index 0000000..aa71a3a --- /dev/null +++ b/test/mocks/MockExecutor.sol @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.13; + +import "openzeppelin-contracts/interfaces/IERC1271.sol"; + +contract MockExecutor { + function customFunction() external pure returns (uint256) { + return 12345; + } +}