diff --git a/src/abstracts/SablierV2Lockup.sol b/src/abstracts/SablierV2Lockup.sol index 158d12aea..d75a1f87d 100644 --- a/src/abstracts/SablierV2Lockup.sol +++ b/src/abstracts/SablierV2Lockup.sol @@ -241,21 +241,6 @@ abstract contract SablierV2Lockup is revert Errors.SablierV2Lockup_StreamDepleted(streamId); } - bool isCallerStreamSender = _isCallerStreamSender(streamId); - - // Checks: `msg.sender` is the stream's sender, the stream's recipient, or an approved third party. - if (!isCallerStreamSender && !_isCallerStreamRecipientOrApproved(streamId)) { - revert Errors.SablierV2Lockup_Unauthorized(streamId, msg.sender); - } - - // Retrieve the recipient from storage. - address recipient = _ownerOf(streamId); - - // Checks: if `msg.sender` is the stream's sender, the withdrawal address must be the recipient. - if (isCallerStreamSender && to != recipient) { - revert Errors.SablierV2Lockup_InvalidSenderWithdrawal(streamId, msg.sender, to); - } - // Checks: the withdrawal address is not zero. if (to == address(0)) { revert Errors.SablierV2Lockup_WithdrawToZeroAddress(); @@ -266,6 +251,15 @@ abstract contract SablierV2Lockup is revert Errors.SablierV2Lockup_WithdrawAmountZero(streamId); } + // Retrieve the recipient from storage. + address recipient = _ownerOf(streamId); + + // Checks: if `msg.sender` is neither the stream's recipient nor an approved third party, the withdrawal address + // must be the recipient. + if (to != recipient && !_isCallerStreamRecipientOrApproved(streamId)) { + revert Errors.SablierV2Lockup_WithdrawalAddressNotRecipient(streamId, msg.sender, to); + } + // Checks: the withdraw amount is not greater than the withdrawable amount. uint128 withdrawableAmount = _withdrawableAmountOf(streamId); if (amount > withdrawableAmount) { @@ -337,7 +331,6 @@ abstract contract SablierV2Lockup is /// @inheritdoc ISablierV2Lockup function withdrawMultiple( uint256[] calldata streamIds, - address to, uint128[] calldata amounts ) external @@ -354,7 +347,7 @@ abstract contract SablierV2Lockup is // Iterate over the provided array of stream ids and withdraw from each stream. for (uint256 i = 0; i < streamIdsCount; ++i) { // Checks, Effects and Interactions: check the parameters and make the withdrawal. - withdraw(streamIds[i], to, amounts[i]); + withdraw({ streamId: streamIds[i], to: _ownerOf(streamIds[i]), amount: amounts[i] }); } } diff --git a/src/interfaces/ISablierV2Lockup.sol b/src/interfaces/ISablierV2Lockup.sol index b21eb3e0e..e129860e8 100644 --- a/src/interfaces/ISablierV2Lockup.sol +++ b/src/interfaces/ISablierV2Lockup.sol @@ -245,16 +245,15 @@ interface ISablierV2Lockup is /// @dev Emits a {Transfer}, {WithdrawFromLockupStream}, and {MetadataUpdate} event. /// /// Notes: - /// - This function attempts to invoke a hook on the stream's recipient, provided that the recipient is a contract - /// and `msg.sender` is either the sender or an approved operator. + /// - This function attempts to call a hook on the recipient of the stream, unless `msg.sender` is the recipient. + /// - This function attempts to call a hook on the sender of the stream, unless `msg.sender` is the sender. /// /// Requirements: /// - Must not be delegate called. /// - `streamId` must not reference a null or depleted stream. - /// - `msg.sender` must be the stream's sender, the stream's recipient or an approved third party. - /// - `to` must be the recipient if `msg.sender` is the stream's sender. /// - `to` must not be the zero address. /// - `amount` must be greater than zero and must not exceed the withdrawable amount. + /// - `to` must be the recipient if `msg.sender` is not the stream's recipient or an approved third party. /// /// @param streamId The id of the stream to withdraw from. /// @param to The address receiving the withdrawn assets. @@ -293,19 +292,21 @@ interface ISablierV2Lockup is /// @param newRecipient The address of the new owner of the stream NFT. function withdrawMaxAndTransfer(uint256 streamId, address newRecipient) external; - /// @notice Withdraws assets from streams to the provided address `to`. + /// @notice Withdraws assets from streams to the recipient of each stream. /// /// @dev Emits multiple {Transfer}, {WithdrawFromLockupStream}, and {MetadataUpdate} events. /// /// Notes: /// - This function attempts to call a hook on the recipient of each stream, unless `msg.sender` is the recipient. + /// - This function attempts to call a hook on the sender of each stream, unless `msg.sender` is the sender. /// /// Requirements: - /// - All requirements from {withdraw} must be met for each stream. + /// - Must not be delegate called. /// - There must be an equal number of `streamIds` and `amounts`. + /// - Each stream id in the array must not reference a null or depleted stream. + /// - Each amount in the array must be greater than zero and must not exceed the withdrawable amount. /// /// @param streamIds The ids of the streams to withdraw from. - /// @param to The address receiving the withdrawn assets. /// @param amounts The amounts to withdraw, denoted in units of the asset's decimals. - function withdrawMultiple(uint256[] calldata streamIds, address to, uint128[] calldata amounts) external; + function withdrawMultiple(uint256[] calldata streamIds, uint128[] calldata amounts) external; } diff --git a/src/interfaces/hooks/ISablierV2Recipient.sol b/src/interfaces/hooks/ISablierV2Recipient.sol index d6e029af6..128d3b958 100644 --- a/src/interfaces/hooks/ISablierV2Recipient.sol +++ b/src/interfaces/hooks/ISablierV2Recipient.sol @@ -33,7 +33,7 @@ interface ISablierV2Recipient { /// @param streamId The id of the renounced stream. function onLockupStreamRenounced(uint256 streamId) external; - /// @notice Responds to withdrawals triggered by either the stream's sender or an approved third party. + /// @notice Responds to withdrawals triggered by any address except the contract implementing this interface. /// /// @dev Notes: /// - This function may revert, but the Sablier contract will ignore the revert. diff --git a/src/interfaces/hooks/ISablierV2Sender.sol b/src/interfaces/hooks/ISablierV2Sender.sol index 632e13065..904380460 100644 --- a/src/interfaces/hooks/ISablierV2Sender.sol +++ b/src/interfaces/hooks/ISablierV2Sender.sol @@ -6,7 +6,7 @@ pragma solidity >=0.8.22; /// @dev Implementation of this interface is optional. If a sender contract doesn't implement this /// interface or implements it partially, function execution will not revert. interface ISablierV2Sender { - /// @notice Responds to withdrawals triggered by either the stream's recipient or an approved third party. + /// @notice Responds to withdrawals triggered by any address except the contract implementing this interface. /// /// @dev Notes: /// - This function may revert, but the Sablier contract will ignore the revert. diff --git a/src/libraries/Errors.sol b/src/libraries/Errors.sol index bf9548520..d410a9255 100644 --- a/src/libraries/Errors.sol +++ b/src/libraries/Errors.sol @@ -38,9 +38,6 @@ library Errors { /// @notice Thrown when trying to create a stream with an end time not in the future. error SablierV2Lockup_EndTimeNotInTheFuture(uint40 currentTime, uint40 endTime); - /// @notice Thrown when the stream's sender tries to withdraw to an address other than the recipient's. - error SablierV2Lockup_InvalidSenderWithdrawal(uint256 streamId, address sender, address to); - /// @notice Thrown when trying to transfer Stream NFT when transferability is disabled. error SablierV2Lockup_NotTransferable(uint256 tokenId); @@ -71,6 +68,9 @@ library Errors { /// @notice Thrown when `msg.sender` lacks authorization to perform an action. error SablierV2Lockup_Unauthorized(uint256 streamId, address caller); + /// @notice Thrown when trying to withdraw to an address other than the recipient's. + error SablierV2Lockup_WithdrawalAddressNotRecipient(uint256 streamId, address caller, address to); + /// @notice Thrown when trying to withdraw zero assets from a stream. error SablierV2Lockup_WithdrawAmountZero(uint256 streamId); diff --git a/test/integration/concrete/lockup/withdraw-multiple/withdrawMultiple.t.sol b/test/integration/concrete/lockup/withdraw-multiple/withdrawMultiple.t.sol index 7b469d53d..915b4c709 100644 --- a/test/integration/concrete/lockup/withdraw-multiple/withdrawMultiple.t.sol +++ b/test/integration/concrete/lockup/withdraw-multiple/withdrawMultiple.t.sol @@ -19,8 +19,7 @@ abstract contract WithdrawMultiple_Integration_Concrete_Test is } function test_RevertWhen_DelegateCalled() external { - bytes memory callData = - abi.encodeCall(ISablierV2Lockup.withdrawMultiple, (testStreamIds, users.recipient, testAmounts)); + bytes memory callData = abi.encodeCall(ISablierV2Lockup.withdrawMultiple, (testStreamIds, testAmounts)); (bool success, bytes memory returnData) = address(lockup).delegatecall(callData); expectRevertDueToDelegateCall(success, returnData); } @@ -33,7 +32,7 @@ abstract contract WithdrawMultiple_Integration_Concrete_Test is Errors.SablierV2Lockup_WithdrawArrayCountsNotEqual.selector, streamIds.length, amounts.length ) ); - lockup.withdrawMultiple({ streamIds: streamIds, to: users.recipient, amounts: amounts }); + lockup.withdrawMultiple(streamIds, amounts); } modifier whenArrayCountsAreEqual() { @@ -43,7 +42,7 @@ abstract contract WithdrawMultiple_Integration_Concrete_Test is function test_WithdrawMultiple_ArrayCountsZero() external whenNotDelegateCalled whenArrayCountsAreEqual { uint256[] memory streamIds = new uint256[](0); uint128[] memory amounts = new uint128[](0); - lockup.withdrawMultiple({ streamIds: streamIds, to: users.recipient, amounts: amounts }); + lockup.withdrawMultiple(streamIds, amounts); } modifier whenArrayCountsNotZero() { @@ -61,7 +60,6 @@ abstract contract WithdrawMultiple_Integration_Concrete_Test is vm.expectRevert(abi.encodeWithSelector(Errors.SablierV2Lockup_Null.selector, nullStreamId)); lockup.withdrawMultiple({ streamIds: Solarray.uint256s(nullStreamId), - to: users.recipient, amounts: Solarray.uint128s(withdrawAmount) }); } @@ -82,7 +80,7 @@ abstract contract WithdrawMultiple_Integration_Concrete_Test is vm.expectRevert(abi.encodeWithSelector(Errors.SablierV2Lockup_Null.selector, nullStreamId)); // Withdraw from multiple streams. - lockup.withdrawMultiple({ streamIds: streamIds, to: users.recipient, amounts: testAmounts }); + lockup.withdrawMultiple({ streamIds: streamIds, amounts: testAmounts }); } function test_RevertGiven_AllStatusesDepleted() @@ -105,7 +103,7 @@ abstract contract WithdrawMultiple_Integration_Concrete_Test is vm.expectRevert(abi.encodeWithSelector(Errors.SablierV2Lockup_StreamDepleted.selector, testStreamIds[0])); // Withdraw from multiple streams. - lockup.withdrawMultiple({ streamIds: streamIds, to: users.recipient, amounts: amounts }); + lockup.withdrawMultiple(streamIds, amounts); } function test_RevertGiven_SomeStatusesDepleted() @@ -125,113 +123,7 @@ abstract contract WithdrawMultiple_Integration_Concrete_Test is vm.expectRevert(abi.encodeWithSelector(Errors.SablierV2Lockup_StreamDepleted.selector, testStreamIds[0])); // Withdraw from multiple streams. - lockup.withdrawMultiple({ streamIds: testStreamIds, to: users.recipient, amounts: testAmounts }); - } - - function test_RevertWhen_CallerUnauthorizedAllStreams_MaliciousThirdParty() - external - whenNotDelegateCalled - whenArrayCountsAreEqual - whenArrayCountsNotZero - givenNoNull - givenNoDepletedStream - whenCallerUnauthorized - { - // Make Eve the caller in this test. - changePrank({ msgSender: users.eve }); - - // Run the test. - vm.expectRevert( - abi.encodeWithSelector(Errors.SablierV2Lockup_Unauthorized.selector, testStreamIds[0], users.eve) - ); - lockup.withdrawMultiple({ streamIds: testStreamIds, to: users.recipient, amounts: testAmounts }); - } - - function test_RevertWhen_CallerUnauthorizedAllStreams_FormerRecipient() - external - whenNotDelegateCalled - whenArrayCountsAreEqual - whenArrayCountsNotZero - givenNoNull - givenNoDepletedStream - whenCallerUnauthorized - { - // Transfer all streams to Alice. - changePrank({ msgSender: users.recipient }); - lockup.transferFrom({ from: users.recipient, to: users.alice, tokenId: testStreamIds[0] }); - lockup.transferFrom({ from: users.recipient, to: users.alice, tokenId: testStreamIds[1] }); - lockup.transferFrom({ from: users.recipient, to: users.alice, tokenId: testStreamIds[2] }); - - // Run the test. - vm.expectRevert( - abi.encodeWithSelector(Errors.SablierV2Lockup_Unauthorized.selector, testStreamIds[0], users.recipient) - ); - lockup.withdrawMultiple({ streamIds: testStreamIds, to: users.recipient, amounts: testAmounts }); - } - - function test_RevertWhen_CallerUnauthorizedSomeStreams_MaliciousThirdParty() - external - whenNotDelegateCalled - whenArrayCountsAreEqual - whenArrayCountsNotZero - givenNoNull - givenNoDepletedStream - whenCallerUnauthorized - { - // Create a stream with Eve as the stream's recipient. - uint256 eveStreamId = createDefaultStreamWithRecipient(users.eve); - - // Make Eve the caller in this test. - changePrank({ msgSender: users.eve }); - - // Simulate the passage of time. - vm.warp({ timestamp: defaults.WARP_26_PERCENT() }); - - // Run the test. - uint256[] memory streamIds = Solarray.uint256s(eveStreamId, testStreamIds[0], testStreamIds[1]); - vm.expectRevert( - abi.encodeWithSelector(Errors.SablierV2Lockup_Unauthorized.selector, testStreamIds[0], users.eve) - ); - lockup.withdrawMultiple({ streamIds: streamIds, to: users.recipient, amounts: testAmounts }); - } - - function test_RevertWhen_CallerUnauthorizedSomeStreams_FormerRecipient() - external - whenNotDelegateCalled - whenArrayCountsAreEqual - whenArrayCountsNotZero - givenNoNull - givenNoDepletedStream - whenCallerUnauthorized - { - // Transfer one of the streams to Eve. - changePrank({ msgSender: users.recipient }); - lockup.transferFrom({ from: users.recipient, to: users.alice, tokenId: testStreamIds[0] }); - - // Simulate the passage of time. - vm.warp({ timestamp: defaults.WARP_26_PERCENT() }); - - // Run the test. - vm.expectRevert( - abi.encodeWithSelector(Errors.SablierV2Lockup_Unauthorized.selector, testStreamIds[0], users.recipient) - ); - lockup.withdrawMultiple({ streamIds: testStreamIds, to: users.recipient, amounts: testAmounts }); - } - - function test_RevertWhen_ToZeroAddress() - external - whenNotDelegateCalled - whenArrayCountsAreEqual - whenArrayCountsNotZero - givenNoNull - givenNoDepletedStream - whenCallerAuthorizedAllStreams - { - if (caller == users.sender) { - return; - } - vm.expectRevert(Errors.SablierV2Lockup_WithdrawToZeroAddress.selector); - lockup.withdrawMultiple({ streamIds: testStreamIds, to: address(0), amounts: testAmounts }); + lockup.withdrawMultiple({ streamIds: testStreamIds, amounts: testAmounts }); } function test_RevertWhen_SomeAmountsZero() @@ -241,7 +133,6 @@ abstract contract WithdrawMultiple_Integration_Concrete_Test is whenArrayCountsNotZero givenNoNull givenNoDepletedStream - whenCallerAuthorizedAllStreams whenToNonZeroAddress { // Simulate the passage of time. @@ -250,7 +141,7 @@ abstract contract WithdrawMultiple_Integration_Concrete_Test is // Run the test. uint128[] memory amounts = Solarray.uint128s(defaults.WITHDRAW_AMOUNT(), 0, 0); vm.expectRevert(abi.encodeWithSelector(Errors.SablierV2Lockup_WithdrawAmountZero.selector, testStreamIds[1])); - lockup.withdrawMultiple({ streamIds: testStreamIds, to: users.recipient, amounts: amounts }); + lockup.withdrawMultiple({ streamIds: testStreamIds, amounts: amounts }); } function test_RevertWhen_SomeAmountsOverdraw() @@ -260,7 +151,6 @@ abstract contract WithdrawMultiple_Integration_Concrete_Test is whenArrayCountsNotZero givenNoNull givenNoDepletedStream - whenCallerAuthorizedAllStreams whenToNonZeroAddress whenNoAmountZero { @@ -275,7 +165,7 @@ abstract contract WithdrawMultiple_Integration_Concrete_Test is Errors.SablierV2Lockup_Overdraw.selector, testStreamIds[2], MAX_UINT128, withdrawableAmount ) ); - lockup.withdrawMultiple({ streamIds: testStreamIds, to: users.recipient, amounts: amounts }); + lockup.withdrawMultiple({ streamIds: testStreamIds, amounts: amounts }); } function test_WithdrawMultiple() @@ -285,7 +175,6 @@ abstract contract WithdrawMultiple_Integration_Concrete_Test is whenArrayCountsNotZero givenNoNull givenNoDepletedStream - whenCallerAuthorizedAllStreams whenToNonZeroAddress whenNoAmountZero whenNoAmountOverdraws @@ -329,7 +218,7 @@ abstract contract WithdrawMultiple_Integration_Concrete_Test is }); // Make the withdrawals. - lockup.withdrawMultiple({ streamIds: testStreamIds, to: users.recipient, amounts: testAmounts }); + lockup.withdrawMultiple({ streamIds: testStreamIds, amounts: testAmounts }); // Assert that the statuses have been updated. assertEq(lockup.statusOf(testStreamIds[0]), Lockup.Status.STREAMING, "status0"); diff --git a/test/integration/concrete/lockup/withdraw-multiple/withdrawMultiple.tree b/test/integration/concrete/lockup/withdraw-multiple/withdrawMultiple.tree index 52ca2f9e7..6759b6aa2 100644 --- a/test/integration/concrete/lockup/withdraw-multiple/withdrawMultiple.tree +++ b/test/integration/concrete/lockup/withdraw-multiple/withdrawMultiple.tree @@ -18,27 +18,13 @@ withdrawMultiple.t.sol ├── given some streams' statuses are "DEPLETED" │ └── it should revert └── given no stream's status is "DEPLETED" - ├── when the caller is unauthorized for all streams - │ ├── when the caller is a malicious third party - │ │ └── it should revert - │ └── when the caller is a former recipient - │ └── it should revert - ├── when the caller is unauthorized for some streams - │ ├── when the caller is a malicious third party - │ │ └── it should revert - │ └── when the caller is a former recipient - │ └── it should revert - └── when the caller is authorized for all streams - ├── when the provided address is zero + ├── when some amounts are zero + │ └── it should revert + └── when none of the amounts are zero + ├── when some amounts overdraw │ └── it should revert - └── when the provided address is not zero - ├── when some amounts are zero - │ └── it should revert - └── when none of the amounts are zero - ├── when some amounts overdraw - │ └── it should revert - └── when no amount overdraws - ├── it should make the withdrawals - ├── it should update the statuses - ├── it should update the withdrawn amounts - └── it should emit multiple {WithdrawFromLockupStream} events + └── when no amount overdraws + ├── it should make the withdrawals + ├── it should update the statuses + ├── it should update the withdrawn amounts + └── it should emit multiple {WithdrawFromLockupStream} events diff --git a/test/integration/concrete/lockup/withdraw/withdraw.t.sol b/test/integration/concrete/lockup/withdraw/withdraw.t.sol index 745a7d5e4..8c488b240 100644 --- a/test/integration/concrete/lockup/withdraw/withdraw.t.sol +++ b/test/integration/concrete/lockup/withdraw/withdraw.t.sol @@ -16,6 +16,10 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr Withdraw_Integration_Shared_Test.setUp(); } + /*////////////////////////////////////////////////////////////////////////// + TESTS + //////////////////////////////////////////////////////////////////////////*/ + function test_RevertWhen_DelegateCalled() external { uint128 withdrawAmount = defaults.WITHDRAW_AMOUNT(); bytes memory callData = @@ -40,100 +44,143 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr lockup.withdraw({ streamId: defaultStreamId, to: users.recipient, amount: withdrawAmount }); } - function test_RevertWhen_CallerUnauthorized_Sender() + function test_RevertWhen_ToZeroAddress() external whenNotDelegateCalled givenNotNull givenStreamNotDepleted { + uint128 withdrawAmount = defaults.WITHDRAW_AMOUNT(); + vm.expectRevert(Errors.SablierV2Lockup_WithdrawToZeroAddress.selector); + lockup.withdraw({ streamId: defaultStreamId, to: address(0), amount: withdrawAmount }); + } + + function test_RevertWhen_WithdrawAmountZero() external whenNotDelegateCalled givenNotNull givenStreamNotDepleted - whenCallerUnauthorized + whenToNonZeroAddress { - // Make the Sender the caller in this test. - changePrank({ msgSender: users.sender }); + vm.expectRevert(abi.encodeWithSelector(Errors.SablierV2Lockup_WithdrawAmountZero.selector, defaultStreamId)); + lockup.withdraw({ streamId: defaultStreamId, to: users.recipient, amount: 0 }); + } - // Run the test. - uint128 withdrawAmount = defaults.WITHDRAW_AMOUNT(); + function test_RevertWhen_Overdraw() + external + whenNotDelegateCalled + givenNotNull + givenStreamNotDepleted + whenToNonZeroAddress + whenWithdrawAmountNotZero + { + uint128 withdrawableAmount = 0; vm.expectRevert( abi.encodeWithSelector( - Errors.SablierV2Lockup_InvalidSenderWithdrawal.selector, defaultStreamId, users.sender, users.sender + Errors.SablierV2Lockup_Overdraw.selector, defaultStreamId, MAX_UINT128, withdrawableAmount ) ); - lockup.withdraw({ streamId: defaultStreamId, to: users.sender, amount: withdrawAmount }); + lockup.withdraw({ streamId: defaultStreamId, to: users.recipient, amount: MAX_UINT128 }); } - function test_RevertWhen_CallerUnauthorized_MaliciousThirdParty() + function test_RevertWhen_CallerUnknown() external whenNotDelegateCalled givenNotNull givenStreamNotDepleted - whenCallerUnauthorized + whenWithdrawAmountNotZero + whenNoOverdraw + whenWithdrawalAddressNotRecipient { - // Make Eve the caller in this test. - changePrank({ msgSender: users.eve }); - - // Run the test. - uint128 withdrawAmount = defaults.WITHDRAW_AMOUNT(); - vm.expectRevert( - abi.encodeWithSelector(Errors.SablierV2Lockup_Unauthorized.selector, defaultStreamId, users.eve) - ); - lockup.withdraw({ streamId: defaultStreamId, to: users.recipient, amount: withdrawAmount }); - } + address unknownCaller = address(0xCAFE); - function test_RevertWhen_FormerRecipient() external whenNotDelegateCalled givenNotNull givenStreamNotDepleted { - // Transfer the stream to Alice. - lockup.transferFrom(users.recipient, users.alice, defaultStreamId); + // Make Eve the caller in this test. + changePrank({ msgSender: unknownCaller }); // Run the test. uint128 withdrawAmount = defaults.WITHDRAW_AMOUNT(); vm.expectRevert( - abi.encodeWithSelector(Errors.SablierV2Lockup_Unauthorized.selector, defaultStreamId, users.recipient) + abi.encodeWithSelector( + Errors.SablierV2Lockup_WithdrawalAddressNotRecipient.selector, + defaultStreamId, + unknownCaller, + unknownCaller + ) ); - lockup.withdraw({ streamId: defaultStreamId, to: users.recipient, amount: withdrawAmount }); + lockup.withdraw({ streamId: defaultStreamId, to: unknownCaller, amount: withdrawAmount }); } - function test_RevertWhen_ToZeroAddress() + function test_RevertWhen_CallerSender() external whenNotDelegateCalled givenNotNull givenStreamNotDepleted - whenCallerAuthorized + whenWithdrawAmountNotZero + whenNoOverdraw + whenWithdrawalAddressNotRecipient { + // Make the Sender the caller in this test. + changePrank({ msgSender: users.sender }); + + // Run the test. uint128 withdrawAmount = defaults.WITHDRAW_AMOUNT(); - vm.expectRevert(Errors.SablierV2Lockup_WithdrawToZeroAddress.selector); - lockup.withdraw({ streamId: defaultStreamId, to: address(0), amount: withdrawAmount }); + vm.expectRevert( + abi.encodeWithSelector( + Errors.SablierV2Lockup_WithdrawalAddressNotRecipient.selector, + defaultStreamId, + users.sender, + users.sender + ) + ); + lockup.withdraw({ streamId: defaultStreamId, to: users.sender, amount: withdrawAmount }); } - function test_RevertWhen_WithdrawAmountZero() + function test_RevertWhen_CallerFormerRecipient() external whenNotDelegateCalled givenNotNull givenStreamNotDepleted - whenCallerAuthorized - whenToNonZeroAddress + whenWithdrawAmountNotZero + whenNoOverdraw + whenWithdrawalAddressNotRecipient { - vm.expectRevert(abi.encodeWithSelector(Errors.SablierV2Lockup_WithdrawAmountZero.selector, defaultStreamId)); - lockup.withdraw({ streamId: defaultStreamId, to: users.recipient, amount: 0 }); + // Transfer the stream to Alice. + lockup.transferFrom(users.recipient, users.alice, defaultStreamId); + + // Run the test. + uint128 withdrawAmount = defaults.WITHDRAW_AMOUNT(); + vm.expectRevert( + abi.encodeWithSelector( + Errors.SablierV2Lockup_WithdrawalAddressNotRecipient.selector, + defaultStreamId, + users.recipient, + users.recipient + ) + ); + lockup.withdraw({ streamId: defaultStreamId, to: users.recipient, amount: withdrawAmount }); } - function test_RevertWhen_Overdraw() + function test_Withdraw_CallerApprovedOperator() external whenNotDelegateCalled givenNotNull givenStreamNotDepleted - whenCallerAuthorized whenToNonZeroAddress whenWithdrawAmountNotZero + whenNoOverdraw + whenWithdrawalAddressNotRecipient { - uint128 withdrawableAmount = 0; - vm.expectRevert( - abi.encodeWithSelector( - Errors.SablierV2Lockup_Overdraw.selector, defaultStreamId, MAX_UINT128, withdrawableAmount - ) - ); - lockup.withdraw({ streamId: defaultStreamId, to: users.recipient, amount: MAX_UINT128 }); - } + // Approve the operator to handle the stream. + lockup.approve({ to: users.operator, tokenId: defaultStreamId }); - modifier whenNoOverdraw() { - _; + // Make the operator the caller in this test. + changePrank({ msgSender: users.operator }); + + // Simulate the passage of time. + vm.warp({ timestamp: defaults.WARP_26_PERCENT() }); + + // Make the withdrawal. + lockup.withdraw({ streamId: defaultStreamId, to: users.operator, amount: defaults.WITHDRAW_AMOUNT() }); + + // Assert that the withdrawn amount has been updated. + uint128 actualWithdrawnAmount = lockup.getWithdrawnAmount(defaultStreamId); + uint128 expectedWithdrawnAmount = defaults.WITHDRAW_AMOUNT(); + assertEq(actualWithdrawnAmount, expectedWithdrawnAmount, "withdrawnAmount"); } function test_Withdraw_SenderNotContract() @@ -141,10 +188,10 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr whenNotDelegateCalled givenNotNull givenStreamNotDepleted - whenCallerAuthorized whenToNonZeroAddress whenWithdrawAmountNotZero whenNoOverdraw + whenWithdrawalAddressNotRecipient whenCallerRecipient { test_Withdraw_CallerRecipient(defaultStreamId, users.sender); @@ -159,10 +206,10 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr whenNotDelegateCalled givenNotNull givenStreamNotDepleted - whenCallerAuthorized whenToNonZeroAddress whenWithdrawAmountNotZero whenNoOverdraw + whenWithdrawalAddressNotRecipient whenCallerRecipient givenSenderContract { @@ -176,15 +223,15 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr _; } - function test_Withdraw_ReetrancySender() + function test_Withdraw_ReentrancySender() external whenNotDelegateCalled givenNotNull givenStreamNotDepleted - whenCallerAuthorized whenToNonZeroAddress whenWithdrawAmountNotZero whenNoOverdraw + whenWithdrawalAddressNotRecipient whenCallerRecipient givenSenderContract givenSenderImplementsHook @@ -229,10 +276,10 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr whenNotDelegateCalled givenNotNull givenStreamNotDepleted - whenCallerAuthorized whenToNonZeroAddress whenWithdrawAmountNotZero whenNoOverdraw + whenWithdrawalAddressNotRecipient whenCallerRecipient givenSenderContract givenSenderImplementsHook @@ -253,10 +300,10 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr whenNotDelegateCalled givenNotNull givenStreamNotDepleted - whenCallerAuthorized whenToNonZeroAddress whenWithdrawAmountNotZero whenNoOverdraw + whenWithdrawalAddressNotRecipient whenCallerRecipient givenSenderContract givenSenderImplementsHook @@ -269,48 +316,41 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr test_Withdraw_CallerRecipient(streamId, address(goodSender)); } - function test_Withdraw_CallerRecipient(uint256 streamId, address sender) internal { + function test_Withdraw_CallerUnknownAddress() + external + whenNotDelegateCalled + givenNotNull + givenStreamNotDepleted + whenToNonZeroAddress + whenWithdrawAmountNotZero + whenNoOverdraw + whenWithdrawalAddressIsRecipient + { + // Make the unknown address the caller in this test. + changePrank({ msgSender: address(0xCAFE) }); + // Simulate the passage of time. vm.warp({ timestamp: defaults.WARP_26_PERCENT() }); - // Set the withdraw amount to the default amount. - uint128 withdrawAmount = defaults.WITHDRAW_AMOUNT(); - - // Expect a call to the hook if the sender is a contract. - if (sender.code.length > 0) { - vm.expectCall( - address(sender), - abi.encodeCall( - ISablierV2Sender.onLockupStreamWithdrawn, (streamId, users.recipient, users.alice, withdrawAmount) - ) - ); - } - // Make the withdrawal. - lockup.withdraw({ streamId: streamId, to: users.alice, amount: withdrawAmount }); + lockup.withdraw({ streamId: defaultStreamId, to: users.recipient, amount: defaults.WITHDRAW_AMOUNT() }); // Assert that the withdrawn amount has been updated. - uint128 actualWithdrawnAmount = lockup.getWithdrawnAmount(streamId); - uint128 expectedWithdrawnAmount = withdrawAmount; + uint128 actualWithdrawnAmount = lockup.getWithdrawnAmount(defaultStreamId); + uint128 expectedWithdrawnAmount = defaults.WITHDRAW_AMOUNT(); assertEq(actualWithdrawnAmount, expectedWithdrawnAmount, "withdrawnAmount"); } - function test_Withdraw_CallerApprovedOperator() + function test_Withdraw_CallerRecipient() external whenNotDelegateCalled givenNotNull givenStreamNotDepleted - whenCallerAuthorized whenToNonZeroAddress whenWithdrawAmountNotZero whenNoOverdraw + whenWithdrawalAddressIsRecipient { - // Approve the operator to handle the stream. - lockup.approve({ to: users.operator, tokenId: defaultStreamId }); - - // Make the operator the caller in this test. - changePrank({ msgSender: users.operator }); - // Simulate the passage of time. vm.warp({ timestamp: defaults.WARP_26_PERCENT() }); @@ -333,10 +373,10 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr whenNotDelegateCalled givenNotNull givenStreamNotDepleted - whenCallerAuthorized whenToNonZeroAddress whenWithdrawAmountNotZero whenNoOverdraw + whenWithdrawalAddressIsRecipient whenCallerSender { // Warp to the stream's end. @@ -371,10 +411,10 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr whenNotDelegateCalled givenNotNull givenStreamNotDepleted - whenCallerAuthorized whenToNonZeroAddress whenWithdrawAmountNotZero whenNoOverdraw + whenWithdrawalAddressIsRecipient whenCallerSender givenEndTimeInTheFuture { @@ -408,15 +448,15 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr whenNotDelegateCalled givenNotNull givenStreamNotDepleted - whenCallerAuthorized whenToNonZeroAddress whenWithdrawAmountNotZero whenNoOverdraw + whenWithdrawalAddressIsRecipient whenCallerSender givenEndTimeInTheFuture whenStreamHasNotBeenCanceled { - test_Withdraw(defaultStreamId, users.recipient); + test_Withdraw_CallerSender(defaultStreamId, users.recipient); } modifier givenRecipientContract() { @@ -428,10 +468,10 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr whenNotDelegateCalled givenNotNull givenStreamNotDepleted - whenCallerAuthorized whenToNonZeroAddress whenWithdrawAmountNotZero whenNoOverdraw + whenWithdrawalAddressIsRecipient whenCallerSender givenEndTimeInTheFuture whenStreamHasNotBeenCanceled @@ -440,7 +480,7 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr // Create the stream with a noop contract as the stream's recipient. uint256 streamId = createDefaultStreamWithRecipient(address(noop)); - test_Withdraw(streamId, address(noop)); + test_Withdraw_CallerSender(streamId, address(noop)); } modifier givenRecipientImplementsHook() { @@ -452,10 +492,10 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr whenNotDelegateCalled givenNotNull givenStreamNotDepleted - whenCallerAuthorized whenToNonZeroAddress whenWithdrawAmountNotZero whenNoOverdraw + whenWithdrawalAddressIsRecipient whenCallerSender givenEndTimeInTheFuture whenStreamHasNotBeenCanceled @@ -465,7 +505,7 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr // Create the stream with a reverting contract as the stream's recipient. uint256 streamId = createDefaultStreamWithRecipient(address(revertingRecipient)); - test_Withdraw(streamId, address(revertingRecipient)); + test_Withdraw_CallerSender(streamId, address(revertingRecipient)); } modifier whenRecipientDoesNotRevert() { @@ -477,10 +517,10 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr whenNotDelegateCalled givenNotNull givenStreamNotDepleted - whenCallerAuthorized whenToNonZeroAddress whenWithdrawAmountNotZero whenNoOverdraw + whenWithdrawalAddressIsRecipient whenCallerSender givenEndTimeInTheFuture whenStreamHasNotBeenCanceled @@ -526,10 +566,10 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr whenNotDelegateCalled givenNotNull givenStreamNotDepleted - whenCallerAuthorized whenToNonZeroAddress whenWithdrawAmountNotZero whenNoOverdraw + whenWithdrawalAddressIsRecipient whenCallerSender givenEndTimeInTheFuture whenStreamHasNotBeenCanceled @@ -541,10 +581,40 @@ abstract contract Withdraw_Integration_Concrete_Test is Integration_Test, Withdr // Create the stream with a contract as the stream's recipient. uint256 streamId = createDefaultStreamWithRecipient(address(goodRecipient)); - test_Withdraw(streamId, address(goodRecipient)); + test_Withdraw_CallerSender(streamId, address(goodRecipient)); + } + + /*////////////////////////////////////////////////////////////////////////// + INTERNAL HELPERS + //////////////////////////////////////////////////////////////////////////*/ + + function test_Withdraw_CallerRecipient(uint256 streamId, address sender) internal { + // Simulate the passage of time. + vm.warp({ timestamp: defaults.WARP_26_PERCENT() }); + + // Set the withdraw amount to the default amount. + uint128 withdrawAmount = defaults.WITHDRAW_AMOUNT(); + + // Expect a call to the hook if the sender is a contract. + if (sender.code.length > 0) { + vm.expectCall( + address(sender), + abi.encodeCall( + ISablierV2Sender.onLockupStreamWithdrawn, (streamId, users.recipient, users.alice, withdrawAmount) + ) + ); + } + + // Make the withdrawal. + lockup.withdraw({ streamId: streamId, to: users.alice, amount: withdrawAmount }); + + // Assert that the withdrawn amount has been updated. + uint128 actualWithdrawnAmount = lockup.getWithdrawnAmount(streamId); + uint128 expectedWithdrawnAmount = withdrawAmount; + assertEq(actualWithdrawnAmount, expectedWithdrawnAmount, "withdrawnAmount"); } - function test_Withdraw(uint256 streamId, address recipient) internal { + function test_Withdraw_CallerSender(uint256 streamId, address recipient) internal { // Set the withdraw amount to the default amount. uint128 withdrawAmount = defaults.WITHDRAW_AMOUNT(); diff --git a/test/integration/concrete/lockup/withdraw/withdraw.tree b/test/integration/concrete/lockup/withdraw/withdraw.tree index 314fa95e1..fccd07685 100644 --- a/test/integration/concrete/lockup/withdraw/withdraw.tree +++ b/test/integration/concrete/lockup/withdraw/withdraw.tree @@ -8,90 +8,96 @@ withdraw.t.sol ├── given the stream's status is "DEPLETED" │ └── it should revert └── given the stream's status is not "DEPLETED" - ├── when the caller is unauthorized - │ ├── when the caller is the sender - │ │ └── it should revert - │ ├── when the caller is a malicious third party - │ │ └── it should revert - │ └── when the caller is a former recipient - │ └── it should revert - └── when the caller is authorized - ├── when the provided address is zero + ├── when the provided address is zero + │ └── it should revert + └── when the provided address is not zero + ├── when the withdraw amount is zero │ └── it should revert - └── when the provided address is not zero - ├── when the withdraw amount is zero + └── when the withdraw amount is not zero + ├── when the withdraw amount overdraws │ └── it should revert - └── when the withdraw amount is not zero - ├── when the withdraw amount overdraws - │ └── it should revert - └── when the withdraw amount does not overdraw - ├── when the caller is the recipient - │ ├── given the sender is not a contract - │ │ ├── it should make the withdrawal - │ │ └── it should update the withdrawn amount - │ └── given the sender is a contract - │ ├── given the sender does not implement the hook - │ │ ├── it should make the withdrawal - │ │ ├── it should update the withdrawn amount - │ │ ├── it should call the sender hook - │ │ └── it should ignore the revert - │ └── given the sender implements the hook - │ ├── when the sender reverts - │ │ ├── it should make the withdrawal - │ │ ├── it should update the withdrawn amount - │ │ ├── it should call the sender hook - │ │ └── it should ignore the revert - │ └── when the sender does not revert - │ ├── when there is reentrancy - │ │ ├── it should make multiple withdrawals - │ │ ├── it should update the withdrawn amounts - │ │ └── it should call the sender hooks - │ └── when there is no reentrancy - │ ├── it should make the withdrawal - │ ├── it should update the withdrawn amount - │ ├── it should call the sender hook - │ ├── it should emit a {MetadataUpdate} event - │ └── it should emit a {WithdrawFromLockupStream} event - └── when the caller is an approved third party + └── when the withdraw amount does not overdraw + ├── when the withdrawal address is not the stream recipient + │ ├── when the caller is unknown + │ │ └── it should revert + │ ├── when the caller is the sender + │ │ └── it should revert + │ ├── when the caller is a former recipient + │ │ └── it should revert + │ ├── when the caller is an approved third party + │ │ ├── it should make the withdrawal + │ │ └── it should update the withdrawn amount + │ └── when the caller is the recipient + │ ├── given the sender is not a contract + │ │ ├── it should make the withdrawal + │ │ └── it should update the withdrawn amount + │ └── given the sender is a contract + │ ├── given the sender does not implement the hook + │ │ ├── it should make the withdrawal + │ │ ├── it should update the withdrawn amount + │ │ ├── it should call the sender hook + │ │ └── it should ignore the revert + │ └── given the sender implements the hook + │ ├── when the sender reverts + │ │ ├── it should make the withdrawal + │ │ ├── it should update the withdrawn amount + │ │ ├── it should call the sender hook + │ │ └── it should ignore the revert + │ └── when the sender does not revert + │ ├── when there is reentrancy + │ │ ├── it should make multiple withdrawals + │ │ ├── it should update the withdrawn amounts + │ │ └── it should call the sender hooks + │ └── when there is no reentrancy + │ ├── it should make the withdrawal + │ ├── it should update the withdrawn amount + │ ├── it should call the sender hook + │ ├── it should emit a {MetadataUpdate} event + │ └── it should emit a {WithdrawFromLockupStream} event + └── when the withdrawal address is the stream recipient + ├── when the caller is unknown + │ ├── it should make the withdrawal + │ └── it should update the withdrawn amount + ├── when the caller is the recipient + │ ├── it should make the withdrawal + │ └── it should update the withdrawn amount + └── when the caller is the sender + ├── given the end time is not in the future │ ├── it should make the withdrawal - │ └── it should update the withdrawn amount - └── when the caller is the sender - ├── given the end time is not in the future + │ ├── it should mark the stream as depleted + │ └── it should make the stream not cancelable + └── given the end time is in the future + ├── given the stream has been canceled │ ├── it should make the withdrawal │ ├── it should mark the stream as depleted - │ └── it should make the stream not cancelable - └── given the end time is in the future - ├── given the stream has been canceled - │ ├── it should make the withdrawal - │ ├── it should mark the stream as depleted - │ ├── it should update the withdrawn amount - │ ├── it should call the recipient hook - │ ├── it should emit a {MetadataUpdate} event - │ └── it should emit a {WithdrawFromLockupStream} event - └── given the stream has not been canceled - ├── given the recipient is not a contract - │ └── it should make the withdrawal - │ └── it should update the withdrawn amount - └── given the recipient is a contract - ├── given the recipient does not implement the hook + │ ├── it should update the withdrawn amount + │ ├── it should call the recipient hook + │ ├── it should emit a {MetadataUpdate} event + │ └── it should emit a {WithdrawFromLockupStream} event + └── given the stream has not been canceled + ├── given the recipient is not a contract + │ └── it should make the withdrawal + │ └── it should update the withdrawn amount + └── given the recipient is a contract + ├── given the recipient does not implement the hook + │ ├── it should make the withdrawal + │ ├── it should update the withdrawn amount + │ ├── it should call the recipient hook + │ └── it should ignore the revert + └── given the recipient implements the hook + ├── when the recipient reverts │ ├── it should make the withdrawal │ ├── it should update the withdrawn amount │ ├── it should call the recipient hook │ └── it should ignore the revert - └── given the recipient implements the hook - ├── when the recipient reverts - │ ├── it should make the withdrawal - │ ├── it should update the withdrawn amount - │ ├── it should call the recipient hook - │ └── it should ignore the revert - └── when the recipient does not revert - ├── when there is reentrancy - │ ├── it should make multiple withdrawals - │ ├── it should update the withdrawn amounts - │ └── it should call the recipient hooks - └── when there is no reentrancy - ├── it should make the withdrawal - ├── it should update the withdrawn amount - ├── it should call the recipient hook - ├── it should emit a {MetadataUpdate} event - └── it should emit a {WithdrawFromLockupStream} event + └── when the recipient does not revert + ├── when there is reentrancy + │ ├── it should make multiple withdrawals + │ ├── it should update the withdrawn amounts + │ └── it should call the recipient hooks + └── when there is no reentrancy + ├── it should make the withdrawal + ├── it should update the withdrawn amount + ├── it should call the recipient hook + ├── it should emit a {MetadataUpdate} event + └── it should emit a {WithdrawFromLockupStream} event diff --git a/test/integration/fuzz/lockup-dynamic/withdraw.t.sol b/test/integration/fuzz/lockup-dynamic/withdraw.t.sol index c73d880f5..45fd367cc 100644 --- a/test/integration/fuzz/lockup-dynamic/withdraw.t.sol +++ b/test/integration/fuzz/lockup-dynamic/withdraw.t.sol @@ -44,10 +44,9 @@ contract Withdraw_LockupDynamic_Integration_Fuzz_Test is external whenNotDelegateCalled givenNotNull - whenCallerAuthorized whenToNonZeroAddress whenWithdrawAmountNotZero - whenWithdrawAmountNotGreaterThanWithdrawableAmount + whenNoOverdraw { vm.assume(params.segments.length != 0); vm.assume(params.to != address(0)); diff --git a/test/integration/fuzz/lockup/withdraw.t.sol b/test/integration/fuzz/lockup/withdraw.t.sol index 6b9c8d3aa..4079206c5 100644 --- a/test/integration/fuzz/lockup/withdraw.t.sol +++ b/test/integration/fuzz/lockup/withdraw.t.sol @@ -11,6 +11,39 @@ abstract contract Withdraw_Integration_Fuzz_Test is Integration_Test, Withdraw_I Withdraw_Integration_Shared_Test.setUp(); } + /// @dev Given enough fuzz runs, all of the following scenarios will be fuzzed: + /// + /// - Multiple caller addresses. + function testFuzz_Withdraw_UnknownCaller(address caller) + external + whenNotDelegateCalled + givenNotNull + whenToNonZeroAddress + whenWithdrawAmountNotZero + whenNoOverdraw + { + vm.assume(caller != users.sender && caller != users.recipient); + + // Make the fuzzed address the caller in this test. + changePrank({ msgSender: caller }); + + // Simulate the passage of time. + vm.warp({ timestamp: defaults.WARP_26_PERCENT() }); + + // Make the withdrawal. + lockup.withdraw({ streamId: defaultStreamId, to: users.recipient, amount: defaults.WITHDRAW_AMOUNT() }); + + // Assert that the stream's status is still "STREAMING". + Lockup.Status actualStatus = lockup.statusOf(defaultStreamId); + Lockup.Status expectedStatus = Lockup.Status.STREAMING; + assertEq(actualStatus, expectedStatus); + + // Assert that the withdrawn amount has been updated. + uint128 actualWithdrawnAmount = lockup.getWithdrawnAmount(defaultStreamId); + uint128 expectedWithdrawnAmount = defaults.WITHDRAW_AMOUNT(); + assertEq(actualWithdrawnAmount, expectedWithdrawnAmount, "withdrawnAmount"); + } + /// @dev Given enough fuzz runs, all of the following scenarios will be fuzzed: /// /// - Multiple values for the withdrawal address. @@ -19,10 +52,9 @@ abstract contract Withdraw_Integration_Fuzz_Test is Integration_Test, Withdraw_I whenNotDelegateCalled givenNotNull givenStreamNotDepleted - whenCallerAuthorized whenToNonZeroAddress whenWithdrawAmountNotZero - whenWithdrawAmountNotGreaterThanWithdrawableAmount + whenNoOverdraw { vm.assume(to != address(0)); @@ -62,10 +94,9 @@ abstract contract Withdraw_Integration_Fuzz_Test is Integration_Test, Withdraw_I external whenNotDelegateCalled givenNotNull - whenCallerAuthorized whenToNonZeroAddress whenWithdrawAmountNotZero - whenWithdrawAmountNotGreaterThanWithdrawableAmount + whenNoOverdraw whenCallerRecipient { timeJump = _bound(timeJump, defaults.CLIFF_DURATION(), defaults.TOTAL_DURATION() - 1 seconds); @@ -130,11 +161,9 @@ abstract contract Withdraw_Integration_Fuzz_Test is Integration_Test, Withdraw_I external whenNotDelegateCalled givenNotNull - whenCallerAuthorized whenToNonZeroAddress whenWithdrawAmountNotZero - whenWithdrawAmountNotGreaterThanWithdrawableAmount - whenCallerRecipient + whenNoOverdraw whenStreamHasNotBeenCanceled { timeJump = _bound(timeJump, defaults.CLIFF_DURATION(), defaults.TOTAL_DURATION() * 2); diff --git a/test/integration/fuzz/lockup/withdrawMultiple.t.sol b/test/integration/fuzz/lockup/withdrawMultiple.t.sol index 307889d33..7cabecba9 100644 --- a/test/integration/fuzz/lockup/withdrawMultiple.t.sol +++ b/test/integration/fuzz/lockup/withdrawMultiple.t.sol @@ -18,7 +18,6 @@ abstract contract WithdrawMultiple_Integration_Fuzz_Test is function testFuzz_WithdrawMultiple( uint256 timeJump, - address to, uint128 ongoingWithdrawAmount ) external @@ -31,14 +30,8 @@ abstract contract WithdrawMultiple_Integration_Fuzz_Test is whenNoAmountZero whenNoAmountOverdraws { - vm.assume(to != address(0)); timeJump = _bound(timeJump, defaults.TOTAL_DURATION(), defaults.TOTAL_DURATION() * 2 - 1 seconds); - // Hard code the withdrawal address if the caller is the stream's sender. - if (caller == users.sender) { - to = users.recipient; - } - // Create a new stream with an end time double that of the default stream. changePrank({ msgSender: users.sender }); uint40 ongoingEndTime = defaults.END_TIME() + defaults.TOTAL_DURATION(); @@ -59,19 +52,29 @@ abstract contract WithdrawMultiple_Integration_Fuzz_Test is ongoingWithdrawAmount = boundUint128(ongoingWithdrawAmount, 1, ongoingWithdrawableAmount); // Expect the withdrawals to be made. - expectCallToTransfer({ to: to, amount: ongoingWithdrawAmount }); - expectCallToTransfer({ to: to, amount: settledWithdrawAmount }); + expectCallToTransfer({ to: users.recipient, amount: ongoingWithdrawAmount }); + expectCallToTransfer({ to: users.recipient, amount: settledWithdrawAmount }); // Expect the relevant events to be emitted. vm.expectEmit({ emitter: address(lockup) }); - emit WithdrawFromLockupStream({ streamId: ongoingStreamId, to: to, asset: dai, amount: ongoingWithdrawAmount }); + emit WithdrawFromLockupStream({ + streamId: ongoingStreamId, + to: users.recipient, + asset: dai, + amount: ongoingWithdrawAmount + }); vm.expectEmit({ emitter: address(lockup) }); - emit WithdrawFromLockupStream({ streamId: settledStreamId, to: to, asset: dai, amount: settledWithdrawAmount }); + emit WithdrawFromLockupStream({ + streamId: settledStreamId, + to: users.recipient, + asset: dai, + amount: settledWithdrawAmount + }); // Make the withdrawals. uint256[] memory streamIds = Solarray.uint256s(ongoingStreamId, settledStreamId); uint128[] memory amounts = Solarray.uint128s(ongoingWithdrawAmount, settledWithdrawAmount); - lockup.withdrawMultiple({ streamIds: streamIds, to: to, amounts: amounts }); + lockup.withdrawMultiple(streamIds, amounts); // Assert that the statuses have been updated. assertEq(lockup.statusOf(streamIds[0]), Lockup.Status.STREAMING, "status0"); diff --git a/test/integration/shared/lockup/withdraw.t.sol b/test/integration/shared/lockup/withdraw.t.sol index 300b89f61..69fc8cb11 100644 --- a/test/integration/shared/lockup/withdraw.t.sol +++ b/test/integration/shared/lockup/withdraw.t.sol @@ -24,23 +24,23 @@ abstract contract Withdraw_Integration_Shared_Test is Lockup_Integration_Shared_ _; } - modifier whenCallerUnauthorized() { + modifier whenToNonZeroAddress() { _; } - modifier whenCallerAuthorized() { + modifier whenWithdrawAmountNotZero() { _; } - modifier whenToNonZeroAddress() { + modifier whenNoOverdraw() { _; } - modifier whenWithdrawAmountNotZero() { + modifier whenWithdrawalAddressNotRecipient() { _; } - modifier whenWithdrawAmountNotGreaterThanWithdrawableAmount() { + modifier whenWithdrawalAddressIsRecipient() { _; }