Merge pull request #36 from xWerk/fix/withdraw-to

Fix `Space` withdrawal methods

.env.example

@@ -2,7 +2,4 @@ MAINNET_RPC_URL=""
 SEPOLIA_RPC_URL="https://rpc.sepolia.org"
 BASE_SEPOLIA_RPC_URL="https://sepolia.base.org"
 ETHERSCAN_API_KEY=""
-CREATE2SALT="""
-DEPLOYER=""
-PRIVATE_KEY="0x{YOUR_PRIVATE_KEY}"
-MNEMONIC=""
+CREATE2SALT="""

Makefile

@@ -43,12 +43,12 @@ deploy-deterministic-module-keeper:
 #	- {ENTRYPOINT} with the address of the {Entrypoint} contract (currently v6)
 #	- {MODULE_KEEPER} with the address of the {ModuleKeeper} deployment
 #	- {RPC_URL} with the network RPC used for deployment
-deploy-deterministic-dock-registry:
+deploy-deterministic-station-registry:
 					forge script script/DeployDeterministicStationRegistry.s.sol:DeployDeterministicStationRegistry \
 					$(CREATE2SALT) {INITIAL_OWNER} {ENTRYPOINT} {MODULE_KEEPER} \
-					--sig "run(string,address,address)" --rpc-url {RPC_URL} \
+					--sig "run(string,address,address,address)" --rpc-url {RPC_URL} \
 					--account dev --etherscan-api-key $(ETHERSCAN_API_KEY) \
-					--broadcast --verify
+					--broadcast --verify --ffi
 
 # Deploys the {PaymentModule} contract deterministically 
 #
@@ -77,6 +77,6 @@ deploy-payment-module:
 #	- {RPC_URL} with the network RPC used for deployment
 deploy-core: 
 					forge script script/DeployDeterministicCore.s.sol:DeployDeterministicCore \
-					$(CREATE2SALT) "0xfe7fc0bbde84c239c0ab89111d617dc7cc58049f" "0xb8c724df3ec8f2bf8fa808df2cb5dbab22f3e68c" "0x85E094B259718Be1AF0D8CbBD41dd7409c2200aa" "0x85E094B259718Be1AF0D8CbBD41dd7409c2200aa" "0x5FF137D4b0FDCD49DcA30c7CF57E578a026d2789" \
-					--sig "run(string,address,address,address,address,address)" --rpc-url https://sepolia.base.org --account dev \
+					$(CREATE2SALT) {SABLIER_LOCKUP_LINEAR} {SABLIER_LOCKUP_TRANCHED} {INITIAL_OWNER} {BROKER_ACCOUNT} {ENTRYPOINT} \
+					--sig "run(string,address,address,address,address,address)" --rpc-url {RPC_URL} --account dev \
 					--broadcast --verify --etherscan-api-key $(ETHERSCAN_API_KEY) --ffi				

src/ModuleKeeper.sol

@@ -20,7 +20,7 @@ contract ModuleKeeper is IModuleKeeper, Ownable {
     //////////////////////////////////////////////////////////////////////////*/
 
     /// @dev Initializes the initial owner of the {ModuleKeeper}
-    constructor(address _initialOwner) Ownable(_initialOwner) {}
+    constructor(address _initialOwner) Ownable(_initialOwner) { }
 
     /*//////////////////////////////////////////////////////////////////////////
                                 NON-CONSTANT FUNCTIONS

src/Space.sol

@@ -37,11 +37,11 @@ contract Space is ISpace, AccountCore, ERC1271, ModuleManager {
     //////////////////////////////////////////////////////////////////////////*/
 
     /// @dev Initializes the address of the EIP 4337 factory and EntryPoint contract
-    constructor(IEntryPoint _entrypoint, address _factory) AccountCore(_entrypoint, _factory) {}
+    constructor(IEntryPoint _entrypoint, address _factory) AccountCore(_entrypoint, _factory) { }
 
     /// @notice Initializes the {ModuleKeeper}, enables initial modules and configures the {Space} smart account
     function initialize(address _defaultAdmin, bytes calldata _data) public override {
-        (, , address[] memory initialModules) = abi.decode(_data, (uint256, uint256, address[]));
+        (,, address[] memory initialModules) = abi.decode(_data, (uint256, uint256, address[]));
 
         // Enable the initial module(s)
         ModuleKeeper moduleKeeper = StationRegistry(factory).moduleKeeper();
@@ -87,7 +87,11 @@ contract Space is ISpace, AccountCore, ERC1271, ModuleManager {
         address module,
         uint256 value,
         bytes calldata data
-    ) public onlyAdminOrEntrypoint returns (bool success) {
+    )
+        public
+        onlyAdminOrEntrypoint
+        returns (bool success)
+    {
         // Checks: the `module` module is enabled on the smart account
         _checkIfModuleIsEnabled(module);
 
@@ -100,7 +104,10 @@ contract Space is ISpace, AccountCore, ERC1271, ModuleManager {
         address[] calldata modules,
         uint256[] calldata values,
         bytes[] calldata data
-    ) external onlyAdminOrEntrypoint {
+    )
+        external
+        onlyAdminOrEntrypoint
+    {
         // Cache the length of the modules array
         uint256 modulesLength = modules.length;
 
@@ -118,74 +125,66 @@ contract Space is ISpace, AccountCore, ERC1271, ModuleManager {
     }
 
     /// @inheritdoc ISpace
-    function withdrawERC20(IERC20 asset, uint256 amount) public onlyAdminOrEntrypoint {
+    function withdrawERC20(address to, IERC20 asset, uint256 amount) public onlyAdminOrEntrypoint {
         // Checks: the available ERC20 balance of the space is greater enough to support the withdrawal
         if (amount > asset.balanceOf(address(this))) revert Errors.InsufficientERC20ToWithdraw();
 
-        // Interactions: withdraw by transferring the amount to the sender
-        asset.safeTransfer({ to: msg.sender, value: amount });
+        // Interactions: withdraw by transferring the `amount` to the `to` address
+        asset.safeTransfer({ to: to, value: amount });
 
         // Log the successful ERC-20 token withdrawal
-        emit AssetWithdrawn({ to: msg.sender, asset: address(asset), amount: amount });
+        emit AssetWithdrawn({ to: to, asset: address(asset), amount: amount });
     }
 
     /// @inheritdoc ISpace
-    function withdrawERC721(IERC721 collection, uint256 tokenId) public onlyAdminOrEntrypoint {
-        // Checks, Effects, Interactions: withdraw by transferring the token to the space owner
+    function withdrawERC721(address to, IERC721 collection, uint256 tokenId) public onlyAdminOrEntrypoint {
+        // Checks, Effects, Interactions: withdraw by transferring the `tokenId` token to the `to` address
         // Notes:
-        // - we're using `safeTransferFrom` as the owner can be an ERC-4337 smart account
+        // - we're using `safeTransferFrom` as the owner can be a smart contract
         // therefore the `onERC721Received` hook must be implemented
-        collection.safeTransferFrom(address(this), msg.sender, tokenId);
+        collection.safeTransferFrom({ from: address(this), to: to, tokenId: tokenId });
 
         // Log the successful ERC-721 token withdrawal
-        emit ERC721Withdrawn({ to: msg.sender, collection: address(collection), tokenId: tokenId });
+        emit ERC721Withdrawn({ to: to, collection: address(collection), tokenId: tokenId });
     }
 
     /// @inheritdoc ISpace
     function withdrawERC1155(
+        address to,
         IERC1155 collection,
         uint256[] memory ids,
         uint256[] memory amounts
-    ) public onlyAdminOrEntrypoint {
-        // Checks, Effects, Interactions: withdraw by transferring the tokens to the space owner
+    )
+        public
+        onlyAdminOrEntrypoint
+    {
+        // Checks, Effects, Interactions: withdraw by transferring the tokens to the `to` address
         // Notes:
-        // - we're using `safeTransferFrom` as the owner can be an ERC-4337 smart account
+        // - we're using `safeTransferFrom` as the owner can be a smart contract
         // therefore the `onERC1155Received` hook must be implemented
         // - depending on the length of the `ids` array, we're using `safeBatchTransferFrom` or `safeTransferFrom`
         if (ids.length > 1) {
-            collection.safeBatchTransferFrom({
-                from: address(this),
-                to: msg.sender,
-                ids: ids,
-                values: amounts,
-                data: ""
-            });
+            collection.safeBatchTransferFrom({ from: address(this), to: msg.sender, ids: ids, values: amounts, data: "" });
         } else {
-            collection.safeTransferFrom({
-                from: address(this),
-                to: msg.sender,
-                id: ids[0],
-                value: amounts[0],
-                data: ""
-            });
+            collection.safeTransferFrom({ from: address(this), to: msg.sender, id: ids[0], value: amounts[0], data: "" });
         }
 
         // Log the successful ERC-1155 token withdrawal
-        emit ERC1155Withdrawn(msg.sender, address(collection), ids, amounts);
+        emit ERC1155Withdrawn({ to: to, collection: address(collection), ids: ids, values: amounts });
     }
 
     /// @inheritdoc ISpace
-    function withdrawNative(uint256 amount) public onlyAdminOrEntrypoint {
-        // Checks: the native balance of the space minus the amount locked for operations is greater than the requested amount
+    function withdrawNative(address to, uint256 amount) public onlyAdminOrEntrypoint {
+        // Checks: the native balance of the space is greater enough to support the withdrawal
         if (amount > address(this).balance) revert Errors.InsufficientNativeToWithdraw();
 
-        // Interactions: withdraw by transferring the amount to the sender
-        (bool success, ) = msg.sender.call{ value: amount }("");
+        // Interactions: withdraw by transferring the `amount` to the `to` address
+        (bool success,) = to.call{ value: amount }("");
         // Revert if the call failed
         if (!success) revert Errors.NativeWithdrawFailed();
 
         // Log the successful native token withdrawal
-        emit AssetWithdrawn({ to: msg.sender, asset: address(0), amount: amount });
+        emit AssetWithdrawn({ to: to, asset: address(0), amount: amount });
     }
 
     /// @inheritdoc IModuleManager
@@ -208,7 +207,15 @@ contract Space is ISpace, AccountCore, ERC1271, ModuleManager {
     //////////////////////////////////////////////////////////////////////////*/
 
     /// @inheritdoc ERC1271
-    function isValidSignature(bytes32 _hash, bytes memory _signature) public view override returns (bytes4 magicValue) {
+    function isValidSignature(
+        bytes32 _hash,
+        bytes memory _signature
+    )
+        public
+        view
+        override
+        returns (bytes4 magicValue)
+    {
         // Compute the hash of message the should be signed
         bytes32 targetDigest = getMessageHash(_hash);
 
@@ -241,11 +248,8 @@ contract Space is ISpace, AccountCore, ERC1271, ModuleManager {
 
     /// @inheritdoc IERC165
     function supportsInterface(bytes4 interfaceId) public pure returns (bool) {
-        return
-            interfaceId == type(ISpace).interfaceId ||
-            interfaceId == type(IERC1155Receiver).interfaceId ||
-            interfaceId == type(IERC721Receiver).interfaceId ||
-            interfaceId == type(IERC165).interfaceId;
+        return interfaceId == type(ISpace).interfaceId || interfaceId == type(IERC1155Receiver).interfaceId
+            || interfaceId == type(IERC721Receiver).interfaceId || interfaceId == type(IERC165).interfaceId;
     }
 
     /// @inheritdoc IERC721Receiver
@@ -254,7 +258,11 @@ contract Space is ISpace, AccountCore, ERC1271, ModuleManager {
         address from,
         uint256 tokenId,
         bytes memory
-    ) public override returns (bytes4) {
+    )
+        public
+        override
+        returns (bytes4)
+    {
         // Silence unused variable warning
         operator = operator;
 
@@ -271,7 +279,11 @@ contract Space is ISpace, AccountCore, ERC1271, ModuleManager {
         uint256 id,
         uint256 value,
         bytes memory
-    ) public override returns (bytes4) {
+    )
+        public
+        override
+        returns (bytes4)
+    {
         // Silence unused variable warning
         operator = operator;
 
@@ -288,7 +300,11 @@ contract Space is ISpace, AccountCore, ERC1271, ModuleManager {
         uint256[] memory ids,
         uint256[] memory values,
         bytes memory
-    ) public override returns (bytes4) {
+    )
+        public
+        override
+        returns (bytes4)
+    {
         for (uint256 i; i < ids.length; ++i) {
             // Log the successful ERC-1155 token receipt
             emit ERC1155Received(from, ids[i], values[i]);

src/interfaces/ISpace.sol

@@ -87,34 +87,44 @@ interface ISpace is IERC165, IERC721Receiver, IERC1155Receiver {
     /// Requirements:
     /// - `msg.sender` must be the owner of the space
     ///
+    /// @param to The address to which the ERC-20 token will be transferred
     /// @param asset The address of the ERC-20 token to withdraw
     /// @param amount The amount of the ERC-20 token to withdraw
-    function withdrawERC20(IERC20 asset, uint256 amount) external;
+    function withdrawERC20(address to, IERC20 asset, uint256 amount) external;
 
     /// @notice Withdraws the `tokenId` token of the ERC-721 `collection` collection
     ///
     /// Requirements:
     /// - `msg.sender` must be the owner of the space
     ///
+    /// @param to The address to which the ERC-721 token will be transferred
     /// @param collection The address of the ERC-721 collection
     /// @param tokenId The ID of the token to withdraw
-    function withdrawERC721(IERC721 collection, uint256 tokenId) external;
+    function withdrawERC721(address to, IERC721 collection, uint256 tokenId) external;
 
     /// @notice Withdraws an `amount` amount of the ERC-1155 `id` token
     ///
     /// Requirements:
     /// - `msg.sender` must be the owner of the space
     ///
+    /// @param to The address to which the ERC-1155 tokens will be transferred
     /// @param collection The address of the ERC-1155 collection
     /// @param ids The IDs of tokens to withdraw
     /// @param amounts The amounts of tokens to withdraw
-    function withdrawERC1155(IERC1155 collection, uint256[] memory ids, uint256[] memory amounts) external;
+    function withdrawERC1155(
+        address to,
+        IERC1155 collection,
+        uint256[] memory ids,
+        uint256[] memory amounts
+    )
+        external;
 
     /// @notice Withdraws an `amount` amount of native token (ETH)
     ///
     /// Requirements:
     /// - `msg.sender` must be the owner of the space
     ///
+    /// @param to The address to which the native token will be transferred
     /// @param amount The amount of the native token to withdraw
-    function withdrawNative(uint256 amount) external;
+    function withdrawNative(address to, uint256 amount) external;
 }