Skip to content

Commit

Permalink
test: fix 'Space' withdraw-related tests
Browse files Browse the repository at this point in the history
  • Loading branch information
gabrielstoica committed Nov 28, 2024
1 parent 897e26b commit 849a55c
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 31 deletions.
35 changes: 18 additions & 17 deletions test/mocks/MockBadSpace.sol
Original file line number Diff line number Diff line change
Expand Up @@ -117,41 +117,42 @@ contract MockBadSpace 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
// 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) {
Expand All @@ -161,21 +162,21 @@ contract MockBadSpace is ISpace, AccountCore, ERC1271, ModuleManager {
}

// 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ contract WithdrawERC1155_Unit_Concrete_Test is Space_Unit_Concrete_Test {
vm.expectRevert("Account: not admin or EntryPoint.");

// Run the test
space.withdrawERC1155({ collection: IERC1155(address(0x0)), ids: ids, amounts: amounts });
space.withdrawERC1155({ to: users.bob, collection: IERC1155(address(0x0)), ids: ids, amounts: amounts });
}

modifier whenCallerOwner() {
Expand All @@ -46,7 +46,7 @@ contract WithdrawERC1155_Unit_Concrete_Test is Space_Unit_Concrete_Test {
);

// Run the test by attempting to withdraw a nonexistent ERC1155 token
space.withdrawERC1155({ collection: mockERC1155, ids: ids, amounts: amounts });
space.withdrawERC1155({ to: users.eve, collection: mockERC1155, ids: ids, amounts: amounts });
}

modifier whenExistingERC1155Token() {
Expand All @@ -71,7 +71,7 @@ contract WithdrawERC1155_Unit_Concrete_Test is Space_Unit_Concrete_Test {
});

// Run the test
space.withdrawERC1155({ collection: mockERC1155, ids: idsToWithdraw, amounts: amountsToWithdraw });
space.withdrawERC1155({ to: users.eve, collection: mockERC1155, ids: idsToWithdraw, amounts: amountsToWithdraw });

// Assert the actual and expected token type 1 ERC1155 balance of Eve
uint256 actualBalanceOfEve = mockERC1155.balanceOf(users.eve, idsToWithdraw[0]);
Expand All @@ -84,7 +84,7 @@ contract WithdrawERC1155_Unit_Concrete_Test is Space_Unit_Concrete_Test {
emit Events.ERC1155Withdrawn({ to: users.eve, collection: address(mockERC1155), ids: ids, amounts: amounts });

// Run the test
space.withdrawERC1155({ collection: mockERC1155, ids: ids, amounts: amounts });
space.withdrawERC1155({ to: users.eve, collection: mockERC1155, ids: ids, amounts: amounts });

// Assert the actual and expected balance of any ERC1155 tokens
uint256 numberOfTokens = ids.length;
Expand Down
6 changes: 3 additions & 3 deletions test/unit/concrete/space/withdraw-erc20/withdrawERC20.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ contract WithdrawERC20_Unit_Concrete_Test is Space_Unit_Concrete_Test {
vm.expectRevert("Account: not admin or EntryPoint.");

// Run the test
space.withdrawERC20({ asset: IERC20(address(0x0)), amount: 100e6 });
space.withdrawERC20({ to: users.bob, asset: IERC20(address(0x0)), amount: 100e6 });
}

modifier whenCallerOwner() {
Expand All @@ -33,7 +33,7 @@ contract WithdrawERC20_Unit_Concrete_Test is Space_Unit_Concrete_Test {
vm.expectRevert(Errors.InsufficientERC20ToWithdraw.selector);

// Run the test
space.withdrawERC20({ asset: IERC20(address(usdt)), amount: 100e6 });
space.withdrawERC20({ to: users.eve, asset: IERC20(address(usdt)), amount: 100e6 });
}

modifier whenSufficientERC20ToWithdraw() {
Expand All @@ -54,7 +54,7 @@ contract WithdrawERC20_Unit_Concrete_Test is Space_Unit_Concrete_Test {
emit Events.AssetWithdrawn({ to: users.eve, asset: address(usdt), amount: 10e6 });

// Run the test
space.withdrawERC20({ asset: IERC20(address(usdt)), amount: 10e6 });
space.withdrawERC20({ to: users.eve, asset: IERC20(address(usdt)), amount: 10e6 });

// Assert the USDT balance of the {Space} contract
uint256 actualBalanceOfSpace = usdt.balanceOf(address(space));
Expand Down
6 changes: 3 additions & 3 deletions test/unit/concrete/space/withdraw-erc721/withdrawERC721.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ contract WithdrawERC721_Unit_Concrete_Test is Space_Unit_Concrete_Test {
vm.expectRevert("Account: not admin or EntryPoint.");

// Run the test
space.withdrawERC721({ collection: IERC721(address(0x0)), tokenId: 1 });
space.withdrawERC721({ to: users.bob, collection: IERC721(address(0x0)), tokenId: 1 });
}

modifier whenCallerOwner() {
Expand All @@ -33,7 +33,7 @@ contract WithdrawERC721_Unit_Concrete_Test is Space_Unit_Concrete_Test {
vm.expectRevert(abi.encodeWithSelector(Errors.ERC721NonexistentToken.selector, 1));

// Run the test by attempting to withdraw a nonexistent ERC721 token
space.withdrawERC721({ collection: mockERC721, tokenId: 1 });
space.withdrawERC721({ to: users.eve, collection: mockERC721, tokenId: 1 });
}

modifier whenExistingERC721Token() {
Expand All @@ -48,7 +48,7 @@ contract WithdrawERC721_Unit_Concrete_Test is Space_Unit_Concrete_Test {
emit Events.ERC721Withdrawn({ to: users.eve, collection: address(mockERC721), tokenId: 1 });

// Run the test
space.withdrawERC721({ collection: mockERC721, tokenId: 1 });
space.withdrawERC721({ to: users.eve, collection: mockERC721, tokenId: 1 });

// Assert the actual and expected owner of the ERC721 token
address actualOwner = mockERC721.ownerOf(1);
Expand Down
8 changes: 4 additions & 4 deletions test/unit/concrete/space/withdraw-native/withdrawNative.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ contract WithdrawNative_Unit_Concrete_Test is Space_Unit_Concrete_Test {
vm.expectRevert("Account: not admin or EntryPoint.");

// Run the test
space.withdrawNative({ amount: 2 ether });
space.withdrawNative({ to: users.bob, amount: 2 ether });
}

modifier whenCallerOwner(address caller) {
Expand All @@ -46,7 +46,7 @@ contract WithdrawNative_Unit_Concrete_Test is Space_Unit_Concrete_Test {
vm.expectRevert(Errors.InsufficientNativeToWithdraw.selector);

// Run the test
space.withdrawNative({ amount: 2 ether });
space.withdrawNative({ to: users.eve, amount: 2 ether });
}

modifier whenSufficientNativeToWithdraw(Space space) {
Expand All @@ -65,7 +65,7 @@ contract WithdrawNative_Unit_Concrete_Test is Space_Unit_Concrete_Test {
vm.expectRevert(Errors.NativeWithdrawFailed.selector);

// Run the test
badSpace.withdrawNative({ amount: 1 ether });
badSpace.withdrawNative({ to: badReceiver, amount: 1 ether });
}

modifier whenNativeWithdrawSucceeds() {
Expand All @@ -88,7 +88,7 @@ contract WithdrawNative_Unit_Concrete_Test is Space_Unit_Concrete_Test {
emit Events.AssetWithdrawn({ to: users.eve, asset: address(0x0), amount: ethToWithdraw });

// Run the test
space.withdrawNative({ amount: ethToWithdraw });
space.withdrawNative({ to: users.eve, amount: ethToWithdraw });

// Assert the ETH balance of the {Space} contract
uint256 actualBalanceOfSpace = address(space).balance;
Expand Down

0 comments on commit 849a55c

Please sign in to comment.