From 9cfe2ffbb91adee7f3438de39064066d050a8506 Mon Sep 17 00:00:00 2001 From: andreivladbrg Date: Mon, 15 Apr 2024 17:14:44 +0200 Subject: [PATCH 01/21] feat: add withdrawMultiple function feat: add cancelMultiple functions --- src/SablierV2OpenEnded.sol | 28 +++++++++++++++++++++++++- src/interfaces/ISablierV2OpenEnded.sol | 26 +++++++++++++++++++++++- src/libraries/Errors.sol | 4 ++++ 3 files changed, 56 insertions(+), 2 deletions(-) diff --git a/src/SablierV2OpenEnded.sol b/src/SablierV2OpenEnded.sol index 6687cb59..d09ca3ea 100644 --- a/src/SablierV2OpenEnded.sol +++ b/src/SablierV2OpenEnded.sol @@ -119,10 +119,20 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope } /// @inheritdoc ISablierV2OpenEnded - function cancel(uint256 streamId) external noDelegateCall notCanceled(streamId) onlySender(streamId) { + function cancel(uint256 streamId) public noDelegateCall notCanceled(streamId) onlySender(streamId) { _cancel(streamId); } + /// @inheritdoc ISablierV2OpenEnded + function cancelMultiple(uint256[] calldata streamIds) external override { + // Iterate over the provided array of stream IDs and cancel each stream. + uint256 count = streamIds.length; + for (uint256 i = 0; i < count; ++i) { + // Effects and Interactions: cancel the stream. + cancel(streamIds[i]); + } + } + /// @inheritdoc ISablierV2OpenEnded function create( address sender, @@ -223,6 +233,22 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope _withdraw(streamId, to, time); } + /// @inheritdoc ISablierV2OpenEnded + function withdrawMultiple(uint256[] calldata streamIds, uint40[] calldata times) external override noDelegateCall { + // Check: there is an equal number of `streamIds` and `amounts`. + uint256 streamIdsCount = streamIds.length; + uint256 timesCount = times.length; + if (streamIdsCount != timesCount) { + revert Errors.SablierV2OpenEnded_WithdrawMultipleArrayCountsNotEqual(streamIdsCount, timesCount); + } + + // Iterate over the provided array of stream IDs, and withdraw from each stream to the recipient. + for (uint256 i = 0; i < streamIdsCount; ++i) { + // Checks, Effects and Interactions: check the parameters and make the withdrawal. + _withdraw({ streamId: streamIds[i], to: _streams[streamIds[i]].recipient, time: times[i] }); + } + } + /// @inheritdoc ISablierV2OpenEnded function withdrawMax(uint256 streamId, address to) external { // Checks, Effects and Interactions: make the withdrawal. diff --git a/src/interfaces/ISablierV2OpenEnded.sol b/src/interfaces/ISablierV2OpenEnded.sol index f323e404..c35fd529 100644 --- a/src/interfaces/ISablierV2OpenEnded.sol +++ b/src/interfaces/ISablierV2OpenEnded.sol @@ -162,7 +162,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// @param newRatePerSecond The new rate per second of the open-ended stream, denoted in 18 decimals. function adjustRatePerSecond(uint256 streamId, uint128 newRatePerSecond) external; - /// @notice Cancels the stream and refunds any remaining assets to the sender. + /// @notice Cancels the stream and refunds available assets to the sender and recipient. /// /// @dev Emits a {Transfer} and {CancelOpenEndedStream} event. /// @@ -175,6 +175,16 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// @param streamId The id of the stream to cancel. function cancel(uint256 streamId) external; + /// @notice Cancels multiple streams and refunds available assets to the sender and to the recipient of each stream. + /// + /// @dev Emits multiple {Transfer} and {CancelOpenEndedStream} events. + /// + /// Requirements: + /// - All requirements from {cancel} must be met for each stream. + /// + /// @param streamIds The IDs of the streams to cancel. + function cancelMultiple(uint256[] calldata streamIds) external; + /// @notice Creates a new open-ended stream with the `block.timestamp` as the time reference and with zero balance. /// /// @dev Emits a {CreateOpenEndedStream} event. @@ -326,4 +336,18 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// @param streamId The id of the stream to withdraw from. /// @param to The address receiving the withdrawn assets. function withdrawMax(uint256 streamId, address to) external; + + /// @notice Withdraws assets from streams to the recipient of each stream. + /// + /// @dev Emits multiple {Transfer} and {WithdrawFromOpenEndedStream} events. + /// + /// Requirements: + /// - Must not be delegate called. + /// - There must be an equal number of `streamIds` and `times`. + /// - Each stream ID in the array must not reference a null stream. + /// - Each time in the array must be greater than the last time update and must not exceed `block.timestamp`. + /// + /// @param streamIds The IDs of the streams to withdraw from. + /// @param times The time references to calculate the streamed amount for each stream. + function withdrawMultiple(uint256[] calldata streamIds, uint40[] calldata times) external; } diff --git a/src/libraries/Errors.sol b/src/libraries/Errors.sol index c86ff401..03d85f23 100644 --- a/src/libraries/Errors.sol +++ b/src/libraries/Errors.sol @@ -63,6 +63,10 @@ library Errors { /// @notice Thrown when trying to withdraw to an address other than the recipient's. error SablierV2OpenEnded_WithdrawalAddressNotRecipient(uint256 streamId, address caller, address to); + /// @notice Thrown when trying to withdraw from multiple streams and the number of stream IDs does + /// not match the number of withdraw times. + error SablierV2OpenEnded_WithdrawMultipleArrayCountsNotEqual(uint256 streamIdCount, uint256 timesCount); + /// @notice Thrown when trying to withdraw assets with a withdrawal time in the future. error SablierV2OpenEnded_WithdrawalTimeInTheFuture(uint40 time, uint256 currentTime); From cb8fe0afafa4b061658b35367eda6b57f75d4160 Mon Sep 17 00:00:00 2001 From: andreivladbrg Date: Mon, 15 Apr 2024 17:23:47 +0200 Subject: [PATCH 02/21] chore: capitalize ID in comments --- src/abstracts/SablierV2OpenEndedState.sol | 2 +- src/interfaces/ISablierV2OpenEnded.sol | 48 ++++++++++----------- src/interfaces/ISablierV2OpenEndedState.sol | 20 ++++----- src/libraries/Errors.sol | 2 +- 4 files changed, 36 insertions(+), 36 deletions(-) diff --git a/src/abstracts/SablierV2OpenEndedState.sol b/src/abstracts/SablierV2OpenEndedState.sol index 2b0221a6..d1b0e0b2 100644 --- a/src/abstracts/SablierV2OpenEndedState.sol +++ b/src/abstracts/SablierV2OpenEndedState.sol @@ -133,7 +133,7 @@ abstract contract SablierV2OpenEndedState is ISablierV2OpenEndedState { //////////////////////////////////////////////////////////////////////////*/ /// @notice Checks whether `msg.sender` is the stream's sender. - /// @param streamId The stream id for the query. + /// @param streamId The stream ID for the query. function _isCallerStreamSender(uint256 streamId) internal view returns (bool) { return msg.sender == _streams[streamId].sender; } diff --git a/src/interfaces/ISablierV2OpenEnded.sol b/src/interfaces/ISablierV2OpenEnded.sol index c35fd529..c5c73154 100644 --- a/src/interfaces/ISablierV2OpenEnded.sol +++ b/src/interfaces/ISablierV2OpenEnded.sol @@ -13,7 +13,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { //////////////////////////////////////////////////////////////////////////*/ /// @notice Emitted when the sender changes the rate per second. - /// @param streamId The id of the stream. + /// @param streamId The ID of the stream. /// @param recipientAmount The amount of assets withdrawn to the recipient, denoted in 18 decimals. /// @param oldRatePerSecond The rate per second to change. /// @param newRatePerSecond The newly changed rate per second. @@ -26,7 +26,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { ); /// @notice Emitted when a open-ended stream is canceled. - /// @param streamId The id of the stream. + /// @param streamId The ID of the stream. /// @param sender The address of the stream's sender. /// @param recipient The address of the stream's recipient. /// @param asset The contract address of the ERC-20 asset used for streaming. @@ -42,7 +42,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { ); /// @notice Emitted when a open-ended stream is created. - /// @param streamId The id of the newly created stream. + /// @param streamId The ID of the newly created stream. /// @param sender The address from which to stream the assets, which has the ability to /// adjust and cancel the stream. /// @param recipient The address toward which to stream the assets. @@ -59,7 +59,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { ); /// @notice Emitted when a open-ended stream is funded. - /// @param streamId The id of the open-ended stream. + /// @param streamId The ID of the open-ended stream. /// @param funder The address which funded the stream. /// @param asset The contract address of the ERC-20 asset used for streaming. /// @param amount The amount of assets deposited, denoted in 18 decimals. @@ -68,7 +68,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { ); /// @notice Emitted when assets are refunded from a open-ended stream. - /// @param streamId The id of the open-ended stream. + /// @param streamId The ID of the open-ended stream. /// @param sender The address of the stream's sender. /// @param asset The contract address of the ERC-20 asset used for streaming. /// @param amount The amount of assets deposited, denoted in 18 decimals. @@ -77,7 +77,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { ); /// @notice Emitted when a open-ended stream is re-started. - /// @param streamId The id of the open-ended stream. + /// @param streamId The ID of the open-ended stream. /// @param sender The address of the stream's sender. /// @param asset The contract address of the ERC-20 asset used for streaming. /// @param ratePerSecond The amount of assets that is increasing by every second, denoted in 18 decimals. @@ -86,7 +86,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { ); /// @notice Emitted when assets are withdrawn from a open-ended stream. - /// @param streamId The id of the stream. + /// @param streamId The ID of the stream. /// @param to The address that has received the withdrawn assets. /// @param asset The contract address of the ERC-20 asset used for streaming. /// @param amount The amount of assets withdrawn, denoted in 18 decimals. @@ -100,42 +100,42 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// @notice Calculates the amount that the sender can refund from stream, denoted in 18 decimals. /// @dev Reverts if `streamId` references a canceled stream. - /// @param streamId The stream id for the query. + /// @param streamId The stream ID for the query. function refundableAmountOf(uint256 streamId) external view returns (uint128 refundableAmount); /// @notice Calculates the amount that the sender can refund from stream at `time`, denoted in 18 decimals. /// @dev Reverts if `streamId` references a canceled stream. - /// @param streamId The stream id for the query. + /// @param streamId The stream ID for the query. /// @param time The Unix timestamp for the streamed amount calculation. function refundableAmountOf(uint256 streamId, uint40 time) external view returns (uint128 refundableAmount); /// @notice Calculates the amount that the sender owes on the stream, i.e. if more assets have been streamed than /// its balance, denoted in 18 decimals. If there is no debt, it will return zero. /// @dev Reverts if `streamId` references a canceled stream. - /// @param streamId The stream id for the query. + /// @param streamId The stream ID for the query. function streamDebt(uint256 streamId) external view returns (uint128 debt); /// @notice Calculates the amount streamed to the recipient from the last time update to the current time, /// denoted in 18 decimals. /// @dev Reverts if `streamId` references a canceled stream. - /// @param streamId The stream id for the query. + /// @param streamId The stream ID for the query. function streamedAmountOf(uint256 streamId) external view returns (uint128 streamedAmount); /// @notice Calculates the amount streamed to the recipient from the last time update to `time` passed as parameter, /// denoted in 18 decimals. /// @dev Reverts if `streamId` references a canceled stream. - /// @param streamId The stream id for the query. + /// @param streamId The stream ID for the query. /// @param time The Unix timestamp for the streamed amount calculation. function streamedAmountOf(uint256 streamId, uint40 time) external view returns (uint128 streamedAmount); /// @notice Calculates the amount that the recipient can withdraw from the stream, denoted in 18 decimals. /// @dev Reverts if `streamId` references a canceled stream. - /// @param streamId The stream id for the query. + /// @param streamId The stream ID for the query. function withdrawableAmountOf(uint256 streamId) external view returns (uint128 withdrawableAmount); /// @notice Calculates the amount that the recipient can withdraw from the stream at `time`, denoted in 18 decimals. /// @dev Reverts if `streamId` references a canceled stream. - /// @param streamId The stream id for the query. + /// @param streamId The stream ID for the query. /// @param time The Unix timestamp for the streamed amount calculation. function withdrawableAmountOf(uint256 streamId, uint40 time) external view returns (uint128 withdrawableAmount); @@ -158,7 +158,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// - `newRatePerSecond` must be greater than zero. /// - `newRatePerSecond` must not be equal to the actual rate per second. /// - /// @param streamId The id of the stream to adjust. + /// @param streamId The ID of the stream to adjust. /// @param newRatePerSecond The new rate per second of the open-ended stream, denoted in 18 decimals. function adjustRatePerSecond(uint256 streamId, uint128 newRatePerSecond) external; @@ -172,7 +172,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// - `streamId` must not reference a canceled stream. /// - `msg.sender` must be the stream's sender. /// - /// @param streamId The id of the stream to cancel. + /// @param streamId The ID of the stream to cancel. function cancel(uint256 streamId) external; /// @notice Cancels multiple streams and refunds available assets to the sender and to the recipient of each stream. @@ -202,7 +202,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// to be the same as `msg.sender`. /// @param ratePerSecond The amount of assets that is increasing by every second, denoted in 18 decimals. /// @param asset The contract address of the ERC-20 asset used for streaming. - /// @return streamId The id of the newly created stream. + /// @return streamId The ID of the newly created stream. function create( address recipient, address sender, @@ -227,7 +227,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// @param ratePerSecond The amount of assets that is increasing by every second, denoted in 18 decimals. /// @param asset The contract address of the ERC-20 asset used for streaming. /// @param depositAmount The amount deposited in the stream. - /// @return streamId The id of the newly created stream. + /// @return streamId The ID of the newly created stream. function createAndDeposit( address recipient, address sender, @@ -248,7 +248,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// - `streamId` must not reference a canceled stream. /// - `amount` must be greater than zero. /// - /// @param streamId The id of the stream to deposit on. + /// @param streamId The ID of the stream to deposit on. /// @param amount The amount deposited in the stream, denoted in 18 decimals. function deposit(uint256 streamId, uint128 amount) external; @@ -275,7 +275,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// - `msg.sender` must be the sender. /// - `amount` must be greater than zero and must not exceed the refundable amount. /// - /// @param streamId The id of the stream to refund from. + /// @param streamId The ID of the stream to refund from. /// @param amount The amount to refund, in units of the ERC-20 asset's decimals. function refundFromStream(uint256 streamId, uint128 amount) external; @@ -290,7 +290,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// - `msg.sender` must be the stream's sender. /// - `ratePerSecond` must be greater than zero. /// - /// @param streamId The id of the stream to restart. + /// @param streamId The ID of the stream to restart. /// @param ratePerSecond The amount of assets that is increasing by every second, denoted in 18 decimals. function restartStream(uint256 streamId, uint128 ratePerSecond) external; @@ -303,7 +303,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// - `depositAmount` must be greater than zero. /// - Refer to the requirements in {restartStream}. /// - /// @param streamId The id of the stream to restart. + /// @param streamId The ID of the stream to restart. /// @param ratePerSecond The amount of assets that is increasing by every second, denoted in 18 decimals. /// @param depositAmount The amount deposited in the stream. function restartStreamAndDeposit(uint256 streamId, uint128 ratePerSecond, uint128 depositAmount) external; @@ -321,7 +321,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// - `time` must be greater than the stream's `lastTimeUpdate` and must not be in the future. /// - The stream balance must be greater than zero. /// - /// @param streamId The id of the stream to withdraw from. + /// @param streamId The ID of the stream to withdraw from. /// @param to The address receiving the withdrawn assets. /// @param time The Unix timestamp for the streamed amount calculation. function withdraw(uint256 streamId, address to, uint40 time) external; @@ -333,7 +333,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// Requirements: /// - Refer to the requirements in {withdraw}. /// - /// @param streamId The id of the stream to withdraw from. + /// @param streamId The ID of the stream to withdraw from. /// @param to The address receiving the withdrawn assets. function withdrawMax(uint256 streamId, address to) external; diff --git a/src/interfaces/ISablierV2OpenEndedState.sol b/src/interfaces/ISablierV2OpenEndedState.sol index f17e6df1..dd60c3e5 100644 --- a/src/interfaces/ISablierV2OpenEndedState.sol +++ b/src/interfaces/ISablierV2OpenEndedState.sol @@ -15,53 +15,53 @@ interface ISablierV2OpenEndedState { /// @notice Retrieves the asset of the stream. /// @dev Reverts if `streamId` references a null stream. - /// @param streamId The id of the stream to make the query for. + /// @param streamId The ID of the stream to make the query for. function getAsset(uint256 streamId) external view returns (IERC20 asset); /// @notice Retrieves the asset decimals of the stream. /// @dev Reverts if `streamId` references a null stream. - /// @param streamId The id of the stream to make the query for. + /// @param streamId The ID of the stream to make the query for. function getAssetDecimals(uint256 streamId) external view returns (uint8 assetDecimals); /// @notice Retrieves the balance of the stream, i.e. the total deposited amounts subtracted by the total withdrawn /// amounts, denoted in 18 decimals. /// @dev Reverts if `streamId` references a null stream. - /// @param streamId The stream id for the query. + /// @param streamId The stream ID for the query. function getBalance(uint256 streamId) external view returns (uint128 balance); /// @notice Retrieves the last time update of the stream, which is a Unix timestamp. /// @dev Reverts if `streamId` references a null stream. - /// @param streamId The id of the stream to make the query for. + /// @param streamId The ID of the stream to make the query for. function getLastTimeUpdate(uint256 streamId) external view returns (uint40 lastTimeUpdate); /// @notice Retrieves the rate per second of the stream, denoted in 18 decimals. /// @dev Reverts if `streamId` references a null stream. - /// @param streamId The id of the stream to make the query for. + /// @param streamId The ID of the stream to make the query for. function getRatePerSecond(uint256 streamId) external view returns (uint128 ratePerSecond); /// @notice Retrieves the stream's recipient. /// @dev Reverts if `streamId` references a null stream. - /// @param streamId The stream id for the query. + /// @param streamId The stream ID for the query. function getRecipient(uint256 streamId) external view returns (address recipient); /// @notice Retrieves the stream's sender. /// @dev Reverts if `streamId` references a null stream. - /// @param streamId The stream id for the query. + /// @param streamId The stream ID for the query. function getSender(uint256 streamId) external view returns (address sender); /// @notice Retrieves the stream entity. /// @dev Reverts if `streamId` references a null stream. - /// @param streamId The stream id for the query. + /// @param streamId The stream ID for the query. function getStream(uint256 streamId) external view returns (OpenEnded.Stream memory stream); /// @notice Retrieves a flag indicating whether the stream is canceled. /// @dev Reverts if `streamId` references a null stream. - /// @param streamId The stream id for the query. + /// @param streamId The stream ID for the query. function isCanceled(uint256 streamId) external view returns (bool result); /// @notice Retrieves a flag indicating whether the stream exists. /// @dev Does not revert if `streamId` references a null stream. - /// @param streamId The stream id for the query. + /// @param streamId The stream ID for the query. function isStream(uint256 streamId) external view returns (bool result); /// @notice Counter for stream ids. diff --git a/src/libraries/Errors.sol b/src/libraries/Errors.sol index 03d85f23..20eee260 100644 --- a/src/libraries/Errors.sol +++ b/src/libraries/Errors.sol @@ -36,7 +36,7 @@ library Errors { /// @notice Thrown when an unexpected error occurs during the calculation of an amount. error SablierV2OpenEnded_InvalidCalculation(uint256 streamId, uint128 balance, uint128 amount); - /// @notice Thrown when the id references a null stream. + /// @notice Thrown when the ID references a null stream. error SablierV2OpenEnded_Null(uint256 streamId); /// @notice Thrown when trying to refund an amount greater than the refundable amount. From ea433609fe71b53c2d2db0bc2632224833b1509c Mon Sep 17 00:00:00 2001 From: andreivladbrg Date: Thu, 18 Apr 2024 19:27:34 +0300 Subject: [PATCH 03/21] test: rename function to expectRevertNull test: rename function to expectRevertCanceled --- test/integration/Integration.t.sol | 6 +++--- .../adjust-rate-per-second/adjustRatePerSecond.t.sol | 4 ++-- test/integration/cancel/cancel.t.sol | 4 ++-- test/integration/deposit/deposit.t.sol | 4 ++-- test/integration/refund-from-stream/refundFromStream.t.sol | 4 ++-- .../refundable-amount-of/refundableAmountOf.t.sol | 4 ++-- test/integration/restart-stream/restartStream.t.sol | 2 +- test/integration/stream-debt/streamDebt.t.sol | 4 ++-- test/integration/streamed-amount-of/streamedAmountOf.t.sol | 4 ++-- test/integration/withdraw/withdraw.t.sol | 4 ++-- .../withdrawable-amount-of/withdrawableAmountOf.t.sol | 4 ++-- 11 files changed, 22 insertions(+), 22 deletions(-) diff --git a/test/integration/Integration.t.sol b/test/integration/Integration.t.sol index a65f4e2a..501d8854 100644 --- a/test/integration/Integration.t.sol +++ b/test/integration/Integration.t.sol @@ -38,7 +38,7 @@ abstract contract Integration_Test is Base_Test { } /*////////////////////////////////////////////////////////////////////////// - COMMON-TESTS + COMMON //////////////////////////////////////////////////////////////////////////*/ function _test_RevertWhen_DelegateCall(bytes memory callData) internal { @@ -49,11 +49,11 @@ abstract contract Integration_Test is Base_Test { uint256 internal nullStreamId = 420; - function _test_RevertGiven_Null() internal { + function expectRevertNull() internal { vm.expectRevert(abi.encodeWithSelector(Errors.SablierV2OpenEnded_Null.selector, nullStreamId)); } - function _test_RevertGiven_Canceled() internal whenNotDelegateCalled givenNotNull { + function expectRevertCanceled() internal { openEnded.cancel(defaultStreamId); vm.expectRevert(abi.encodeWithSelector(Errors.SablierV2OpenEnded_StreamCanceled.selector, defaultStreamId)); } diff --git a/test/integration/adjust-rate-per-second/adjustRatePerSecond.t.sol b/test/integration/adjust-rate-per-second/adjustRatePerSecond.t.sol index 9860030f..11c3f04d 100644 --- a/test/integration/adjust-rate-per-second/adjustRatePerSecond.t.sol +++ b/test/integration/adjust-rate-per-second/adjustRatePerSecond.t.sol @@ -20,12 +20,12 @@ contract adjustRatePerSecond_Integration_Test is Integration_Test { } function test_RevertGiven_Null() external whenNotDelegateCalled { - _test_RevertGiven_Null(); + expectRevertNull(); openEnded.adjustRatePerSecond({ streamId: nullStreamId, newRatePerSecond: RATE_PER_SECOND }); } function test_RevertGiven_Canceled() external whenNotDelegateCalled givenNotNull { - _test_RevertGiven_Canceled(); + expectRevertCanceled(); openEnded.adjustRatePerSecond({ streamId: defaultStreamId, newRatePerSecond: RATE_PER_SECOND }); } diff --git a/test/integration/cancel/cancel.t.sol b/test/integration/cancel/cancel.t.sol index 3723d8e4..74481a78 100644 --- a/test/integration/cancel/cancel.t.sol +++ b/test/integration/cancel/cancel.t.sol @@ -21,12 +21,12 @@ contract Cancel_Integration_Test is Integration_Test { } function test_RevertGiven_Null() external whenNotDelegateCalled { - _test_RevertGiven_Null(); + expectRevertNull(); openEnded.cancel(nullStreamId); } function test_RevertGiven_Canceled() external whenNotDelegateCalled givenNotNull { - _test_RevertGiven_Canceled(); + expectRevertCanceled(); openEnded.cancel(defaultStreamId); } diff --git a/test/integration/deposit/deposit.t.sol b/test/integration/deposit/deposit.t.sol index 8aca0fee..4a9cf5a0 100644 --- a/test/integration/deposit/deposit.t.sol +++ b/test/integration/deposit/deposit.t.sol @@ -19,12 +19,12 @@ contract Deposit_Integration_Test is Integration_Test { } function test_RevertGiven_Null() external whenNotDelegateCalled { - _test_RevertGiven_Null(); + expectRevertNull(); openEnded.deposit(nullStreamId, DEPOSIT_AMOUNT); } function test_RevertGiven_Canceled() external whenNotDelegateCalled givenNotNull { - _test_RevertGiven_Canceled(); + expectRevertCanceled(); openEnded.deposit(defaultStreamId, DEPOSIT_AMOUNT); } diff --git a/test/integration/refund-from-stream/refundFromStream.t.sol b/test/integration/refund-from-stream/refundFromStream.t.sol index aa160e83..633aa6ec 100644 --- a/test/integration/refund-from-stream/refundFromStream.t.sol +++ b/test/integration/refund-from-stream/refundFromStream.t.sol @@ -23,12 +23,12 @@ contract RefundFromStream_Integration_Test is Integration_Test { } function test_RevertGiven_Null() external whenNotDelegateCalled { - _test_RevertGiven_Null(); + expectRevertNull(); openEnded.refundFromStream({ streamId: nullStreamId, amount: REFUND_AMOUNT }); } function test_RevertGiven_Canceled() external whenNotDelegateCalled givenNotNull { - _test_RevertGiven_Canceled(); + expectRevertCanceled(); openEnded.refundFromStream({ streamId: defaultStreamId, amount: REFUND_AMOUNT }); } diff --git a/test/integration/refundable-amount-of/refundableAmountOf.t.sol b/test/integration/refundable-amount-of/refundableAmountOf.t.sol index d75d41f1..7af87ead 100644 --- a/test/integration/refundable-amount-of/refundableAmountOf.t.sol +++ b/test/integration/refundable-amount-of/refundableAmountOf.t.sol @@ -9,12 +9,12 @@ contract RefundableAmountOf_Integration_Test is Integration_Test { } function test_RevertGiven_Null() external { - _test_RevertGiven_Null(); + expectRevertNull(); openEnded.refundableAmountOf(nullStreamId); } function test_RevertGiven_Canceled() external givenNotNull { - _test_RevertGiven_Canceled(); + expectRevertCanceled(); openEnded.refundableAmountOf(defaultStreamId); } diff --git a/test/integration/restart-stream/restartStream.t.sol b/test/integration/restart-stream/restartStream.t.sol index 45edecc2..9af36c4c 100644 --- a/test/integration/restart-stream/restartStream.t.sol +++ b/test/integration/restart-stream/restartStream.t.sol @@ -19,7 +19,7 @@ contract RestartStream_Integration_Test is Integration_Test { } function test_RevertGiven_Null() external whenNotDelegateCalled { - _test_RevertGiven_Null(); + expectRevertNull(); openEnded.restartStream({ streamId: nullStreamId, ratePerSecond: RATE_PER_SECOND }); } diff --git a/test/integration/stream-debt/streamDebt.t.sol b/test/integration/stream-debt/streamDebt.t.sol index e7cce4d9..d4a28a7b 100644 --- a/test/integration/stream-debt/streamDebt.t.sol +++ b/test/integration/stream-debt/streamDebt.t.sol @@ -9,12 +9,12 @@ contract StreamDebt_Integration_Test is Integration_Test { } function test_RevertGiven_Null() external { - _test_RevertGiven_Null(); + expectRevertNull(); openEnded.streamDebt(nullStreamId); } function test_RevertGiven_Canceled() external givenNotNull { - _test_RevertGiven_Canceled(); + expectRevertCanceled(); openEnded.streamDebt(defaultStreamId); } diff --git a/test/integration/streamed-amount-of/streamedAmountOf.t.sol b/test/integration/streamed-amount-of/streamedAmountOf.t.sol index 61b4c7e4..814bb589 100644 --- a/test/integration/streamed-amount-of/streamedAmountOf.t.sol +++ b/test/integration/streamed-amount-of/streamedAmountOf.t.sol @@ -9,12 +9,12 @@ contract StreamedAmountOf_Integration_Test is Integration_Test { } function test_RevertGiven_Null() external { - _test_RevertGiven_Null(); + expectRevertNull(); openEnded.streamedAmountOf(nullStreamId); } function test_RevertGiven_Canceled() external givenNotNull { - _test_RevertGiven_Canceled(); + expectRevertCanceled(); openEnded.streamedAmountOf(defaultStreamId); } diff --git a/test/integration/withdraw/withdraw.t.sol b/test/integration/withdraw/withdraw.t.sol index 40cfe9de..dc483c8d 100644 --- a/test/integration/withdraw/withdraw.t.sol +++ b/test/integration/withdraw/withdraw.t.sol @@ -24,12 +24,12 @@ contract Withdraw_Integration_Test is Integration_Test { } function test_RevertGiven_Null() external whenNotDelegateCalled { - _test_RevertGiven_Null(); + expectRevertNull(); openEnded.withdraw({ streamId: nullStreamId, to: users.recipient, time: WITHDRAW_TIME }); } function test_RevertGiven_Canceled() external whenNotDelegateCalled givenNotNull { - _test_RevertGiven_Canceled(); + expectRevertCanceled(); openEnded.withdraw({ streamId: defaultStreamId, to: users.recipient, time: WITHDRAW_TIME }); } diff --git a/test/integration/withdrawable-amount-of/withdrawableAmountOf.t.sol b/test/integration/withdrawable-amount-of/withdrawableAmountOf.t.sol index a9467f26..d9d678f2 100644 --- a/test/integration/withdrawable-amount-of/withdrawableAmountOf.t.sol +++ b/test/integration/withdrawable-amount-of/withdrawableAmountOf.t.sol @@ -9,12 +9,12 @@ contract WithdrawableAmountOf_Integration_Test is Integration_Test { } function test_RevertGiven_Null() external { - _test_RevertGiven_Null(); + expectRevertNull(); openEnded.withdrawableAmountOf(nullStreamId); } function test_RevertGiven_Canceled() external givenNotNull { - _test_RevertGiven_Canceled(); + expectRevertCanceled(); openEnded.withdrawableAmountOf(defaultStreamId); } From 7683de230c3c25fcecf48d35f042190caf84b59d Mon Sep 17 00:00:00 2001 From: andreivladbrg Date: Thu, 18 Apr 2024 19:27:53 +0300 Subject: [PATCH 04/21] test: cancelMultiple function test: add user eve and use for the malicious third party tests --- test/Base.t.sol | 2 + .../adjustRatePerSecond.t.sol | 9 +- .../cancel-multiple/cancelMultiple.t.sol | 130 ++++++++++++++++++ .../cancel-multiple/cancelMultiple.tree | 26 ++++ test/integration/cancel/cancel.t.sol | 9 +- .../refund-from-stream/refundFromStream.t.sol | 9 +- .../restart-stream/restartStream.t.sol | 9 +- test/utils/Modifiers.sol | 8 ++ 8 files changed, 178 insertions(+), 24 deletions(-) create mode 100644 test/integration/cancel-multiple/cancelMultiple.t.sol create mode 100644 test/integration/cancel-multiple/cancelMultiple.tree diff --git a/test/Base.t.sol b/test/Base.t.sol index 0e7d42ae..18da4963 100644 --- a/test/Base.t.sol +++ b/test/Base.t.sol @@ -17,6 +17,7 @@ import { Utils } from "./utils/Utils.sol"; struct Users { address sender; address recipient; + address eve; } abstract contract Base_Test is Assertions, Events, Modifiers, Test, Utils { @@ -64,6 +65,7 @@ abstract contract Base_Test is Assertions, Events, Modifiers, Test, Utils { users.sender = createUser("sender"); users.recipient = createUser("recipient"); + users.eve = createUser("eve"); labelConctracts(); diff --git a/test/integration/adjust-rate-per-second/adjustRatePerSecond.t.sol b/test/integration/adjust-rate-per-second/adjustRatePerSecond.t.sol index 11c3f04d..319811e1 100644 --- a/test/integration/adjust-rate-per-second/adjustRatePerSecond.t.sol +++ b/test/integration/adjust-rate-per-second/adjustRatePerSecond.t.sol @@ -43,19 +43,16 @@ contract adjustRatePerSecond_Integration_Test is Integration_Test { openEnded.adjustRatePerSecond({ streamId: defaultStreamId, newRatePerSecond: RATE_PER_SECOND }); } - function test_RevertWhen_CallerUnauthorized_MaliciousThirdParty(address maliciousThirdParty) + function test_RevertWhen_CallerUnauthorized_MaliciousThirdParty() external whenNotDelegateCalled givenNotNull givenNotCanceled whenCallerUnauthorized { - vm.assume(maliciousThirdParty != users.sender && maliciousThirdParty != users.recipient); - resetPrank({ msgSender: maliciousThirdParty }); + resetPrank({ msgSender: users.eve }); vm.expectRevert( - abi.encodeWithSelector( - Errors.SablierV2OpenEnded_Unauthorized.selector, defaultStreamId, maliciousThirdParty - ) + abi.encodeWithSelector(Errors.SablierV2OpenEnded_Unauthorized.selector, defaultStreamId, users.eve) ); openEnded.adjustRatePerSecond({ streamId: defaultStreamId, newRatePerSecond: RATE_PER_SECOND }); } diff --git a/test/integration/cancel-multiple/cancelMultiple.t.sol b/test/integration/cancel-multiple/cancelMultiple.t.sol new file mode 100644 index 00000000..5eeeb95c --- /dev/null +++ b/test/integration/cancel-multiple/cancelMultiple.t.sol @@ -0,0 +1,130 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.22; + +import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; + +import { ISablierV2OpenEnded } from "src/interfaces/ISablierV2OpenEnded.sol"; +import { Errors } from "src/libraries/Errors.sol"; + +import { Integration_Test } from "../Integration.t.sol"; + +contract CancelMultiple_Integration_Concrete_Test is Integration_Test { + uint256[] internal testStreamIds; + + function setUp() public override { + Integration_Test.setUp(); + + vm.warp({ newTimestamp: WARP_ONE_MONTH }); + + testStreamIds = new uint256[](2); + testStreamIds[0] = defaultStreamId; + testStreamIds[1] = createDefaultStream(); + } + + function test_RevertWhen_DelegateCall() external { + bytes memory callData = abi.encodeCall(ISablierV2OpenEnded.cancelMultiple, (testStreamIds)); + _test_RevertWhen_DelegateCall(callData); + } + + function test_CancelMultiple_ArrayCountZero() external whenNotDelegateCalled { + uint256[] memory streamIds = new uint256[](0); + openEnded.cancelMultiple(streamIds); + } + + function test_RevertGiven_OnlyNull() external whenNotDelegateCalled whenArrayCountNotZero { + testStreamIds[0] = nullStreamId; + testStreamIds[1] = nullStreamId; + vm.expectRevert(abi.encodeWithSelector(Errors.SablierV2OpenEnded_Null.selector, nullStreamId)); + openEnded.cancelMultiple({ streamIds: testStreamIds }); + } + + function test_RevertGiven_SomeNull() external whenNotDelegateCalled whenArrayCountNotZero { + testStreamIds[0] = nullStreamId; + vm.expectRevert(abi.encodeWithSelector(Errors.SablierV2OpenEnded_Null.selector, nullStreamId)); + openEnded.cancelMultiple({ streamIds: testStreamIds }); + } + + function test_RevertWhen_CallerUnauthorizedAllStreams_MaliciousThirdParty() + external + whenNotDelegateCalled + whenArrayCountNotZero + givenNotNull + whenCallerUnauthorized + { + resetPrank({ msgSender: users.eve }); + + vm.expectRevert( + abi.encodeWithSelector(Errors.SablierV2OpenEnded_Unauthorized.selector, testStreamIds[0], users.eve) + ); + openEnded.cancelMultiple(testStreamIds); + } + + function test_RevertWhen_CallerUnauthorizedAllStreams_Recipient() + external + whenNotDelegateCalled + whenArrayCountNotZero + givenNotNull + whenCallerUnauthorized + { + resetPrank({ msgSender: users.recipient }); + + vm.expectRevert( + abi.encodeWithSelector(Errors.SablierV2OpenEnded_Unauthorized.selector, testStreamIds[0], users.recipient) + ); + openEnded.cancelMultiple(testStreamIds); + } + + function test_RevertWhen_CallerUnauthorizedSomeStreams_MaliciousThirdParty() + external + whenNotDelegateCalled + whenArrayCountNotZero + givenNotNull + whenCallerUnauthorized + { + uint256 eveStreamId = openEnded.create({ + sender: users.eve, + recipient: users.recipient, + ratePerSecond: RATE_PER_SECOND, + asset: dai + }); + + resetPrank({ msgSender: users.eve }); + testStreamIds[0] = eveStreamId; + vm.expectRevert( + abi.encodeWithSelector(Errors.SablierV2OpenEnded_Unauthorized.selector, testStreamIds[1], users.eve) + ); + openEnded.cancelMultiple(testStreamIds); + } + + function test_RevertWhen_CallerUnauthorizedSomeStreams_Recipient() + external + whenNotDelegateCalled + whenArrayCountNotZero + givenNotNull + whenCallerUnauthorized + { + testStreamIds[0] = openEnded.create({ + sender: users.recipient, + recipient: users.recipient, + ratePerSecond: RATE_PER_SECOND, + asset: dai + }); + + resetPrank({ msgSender: users.recipient }); + + vm.expectRevert( + abi.encodeWithSelector(Errors.SablierV2OpenEnded_Unauthorized.selector, testStreamIds[1], users.recipient) + ); + openEnded.cancelMultiple(testStreamIds); + } + + function test_CancelMultiple() external { + openEnded.cancelMultiple(testStreamIds); + + assertTrue(openEnded.isCanceled(testStreamIds[0])); + assertTrue(openEnded.isCanceled(testStreamIds[1])); + + assertEq(openEnded.getRatePerSecond(testStreamIds[0]), 0); + assertEq(openEnded.getRatePerSecond(testStreamIds[1]), 0); + } +} diff --git a/test/integration/cancel-multiple/cancelMultiple.tree b/test/integration/cancel-multiple/cancelMultiple.tree new file mode 100644 index 00000000..e5718f7c --- /dev/null +++ b/test/integration/cancel-multiple/cancelMultiple.tree @@ -0,0 +1,26 @@ +cancelMultiple.t.sol +├── when delegate called +│ └── it should revert +└── when not delegate called + ├── when the array count is zero + │ └── it should do nothing + └── when the array count is not zero + ├── given the stream IDs array references only null streams + │ └── it should revert + ├── given the stream IDs array references some null streams + │ └── it should revert + └── given the stream IDs array references only streams that are not null + ├── when the caller is unauthorized for all streams + │ ├── when the caller is a malicious third party + │ │ └── it should revert + │ └── when the caller is the 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 the recipient + │ └── it should revert + └── when the caller is authorized for all streams + ├── it should cancel the streams + ├── it should update the rate per second for each stream + └── it should emit {CancelOpenEndedStream} events diff --git a/test/integration/cancel/cancel.t.sol b/test/integration/cancel/cancel.t.sol index 74481a78..b1398904 100644 --- a/test/integration/cancel/cancel.t.sol +++ b/test/integration/cancel/cancel.t.sol @@ -44,19 +44,16 @@ contract Cancel_Integration_Test is Integration_Test { openEnded.cancel(defaultStreamId); } - function test_RevertWhen_CallerUnauthorized_MaliciousThirdParty(address maliciousThirdParty) + function test_RevertWhen_CallerUnauthorized_MaliciousThirdParty() external whenNotDelegateCalled givenNotNull givenNotCanceled whenCallerUnauthorized { - vm.assume(maliciousThirdParty != users.sender && maliciousThirdParty != users.recipient); - resetPrank({ msgSender: maliciousThirdParty }); + resetPrank({ msgSender: users.eve }); vm.expectRevert( - abi.encodeWithSelector( - Errors.SablierV2OpenEnded_Unauthorized.selector, defaultStreamId, maliciousThirdParty - ) + abi.encodeWithSelector(Errors.SablierV2OpenEnded_Unauthorized.selector, defaultStreamId, users.eve) ); openEnded.cancel(defaultStreamId); } diff --git a/test/integration/refund-from-stream/refundFromStream.t.sol b/test/integration/refund-from-stream/refundFromStream.t.sol index 633aa6ec..3c559660 100644 --- a/test/integration/refund-from-stream/refundFromStream.t.sol +++ b/test/integration/refund-from-stream/refundFromStream.t.sol @@ -46,19 +46,16 @@ contract RefundFromStream_Integration_Test is Integration_Test { openEnded.refundFromStream({ streamId: defaultStreamId, amount: REFUND_AMOUNT }); } - function test_RevertWhen_CallerUnauthorized_MaliciousThirdParty(address maliciousThirdParty) + function test_RevertWhen_CallerUnauthorized_MaliciousThirdParty() external whenNotDelegateCalled givenNotNull givenNotCanceled whenCallerUnauthorized { - vm.assume(maliciousThirdParty != users.sender && maliciousThirdParty != users.recipient); - resetPrank({ msgSender: maliciousThirdParty }); + resetPrank({ msgSender: users.eve }); vm.expectRevert( - abi.encodeWithSelector( - Errors.SablierV2OpenEnded_Unauthorized.selector, defaultStreamId, maliciousThirdParty - ) + abi.encodeWithSelector(Errors.SablierV2OpenEnded_Unauthorized.selector, defaultStreamId, users.eve) ); openEnded.refundFromStream({ streamId: defaultStreamId, amount: REFUND_AMOUNT }); } diff --git a/test/integration/restart-stream/restartStream.t.sol b/test/integration/restart-stream/restartStream.t.sol index 9af36c4c..8e61b22d 100644 --- a/test/integration/restart-stream/restartStream.t.sol +++ b/test/integration/restart-stream/restartStream.t.sol @@ -43,19 +43,16 @@ contract RestartStream_Integration_Test is Integration_Test { openEnded.restartStream({ streamId: defaultStreamId, ratePerSecond: RATE_PER_SECOND }); } - function test_RevertWhen_CallerUnauthorized_MaliciousThirdParty(address maliciousThirdParty) + function test_RevertWhen_CallerUnauthorized_MaliciousThirdParty() external whenNotDelegateCalled givenNotNull givenCanceled whenCallerUnauthorized { - vm.assume(maliciousThirdParty != users.sender && maliciousThirdParty != users.recipient); - resetPrank({ msgSender: maliciousThirdParty }); + resetPrank({ msgSender: users.eve }); vm.expectRevert( - abi.encodeWithSelector( - Errors.SablierV2OpenEnded_Unauthorized.selector, defaultStreamId, maliciousThirdParty - ) + abi.encodeWithSelector(Errors.SablierV2OpenEnded_Unauthorized.selector, defaultStreamId, users.eve) ); openEnded.restartStream({ streamId: defaultStreamId, ratePerSecond: RATE_PER_SECOND }); } diff --git a/test/utils/Modifiers.sol b/test/utils/Modifiers.sol index 40ec01d0..ea2230fb 100644 --- a/test/utils/Modifiers.sol +++ b/test/utils/Modifiers.sol @@ -38,6 +38,14 @@ abstract contract Modifiers { _; } + /*////////////////////////////////////////////////////////////////////////// + CANCEL-MULTIPLE + //////////////////////////////////////////////////////////////////////////*/ + + modifier whenArrayCountNotZero() { + _; + } + /*////////////////////////////////////////////////////////////////////////// CREATE //////////////////////////////////////////////////////////////////////////*/ From 14f10240013081b89f7ec737bb517317c5ddd49a Mon Sep 17 00:00:00 2001 From: andreivladbrg Date: Fri, 19 Apr 2024 17:17:17 +0300 Subject: [PATCH 05/21] test: add defaultStreamIds array in Integration_Test --- test/integration/Integration.t.sol | 3 ++ .../cancel-multiple/cancelMultiple.t.sol | 54 +++++++++---------- 2 files changed, 28 insertions(+), 29 deletions(-) diff --git a/test/integration/Integration.t.sol b/test/integration/Integration.t.sol index 501d8854..6e83760c 100644 --- a/test/integration/Integration.t.sol +++ b/test/integration/Integration.t.sol @@ -9,11 +9,14 @@ import { Base_Test } from "../Base.t.sol"; abstract contract Integration_Test is Base_Test { uint256 internal defaultStreamId; + uint256[] internal defaultStreamIds; function setUp() public virtual override { Base_Test.setUp(); defaultStreamId = createDefaultStream(); + defaultStreamIds.push(defaultStreamId); + defaultStreamIds.push(createDefaultStream()); } /*////////////////////////////////////////////////////////////////////////// diff --git a/test/integration/cancel-multiple/cancelMultiple.t.sol b/test/integration/cancel-multiple/cancelMultiple.t.sol index 5eeeb95c..0e07b221 100644 --- a/test/integration/cancel-multiple/cancelMultiple.t.sol +++ b/test/integration/cancel-multiple/cancelMultiple.t.sol @@ -1,28 +1,20 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity >=0.8.22; -import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; - import { ISablierV2OpenEnded } from "src/interfaces/ISablierV2OpenEnded.sol"; import { Errors } from "src/libraries/Errors.sol"; import { Integration_Test } from "../Integration.t.sol"; contract CancelMultiple_Integration_Concrete_Test is Integration_Test { - uint256[] internal testStreamIds; - function setUp() public override { Integration_Test.setUp(); vm.warp({ newTimestamp: WARP_ONE_MONTH }); - - testStreamIds = new uint256[](2); - testStreamIds[0] = defaultStreamId; - testStreamIds[1] = createDefaultStream(); } function test_RevertWhen_DelegateCall() external { - bytes memory callData = abi.encodeCall(ISablierV2OpenEnded.cancelMultiple, (testStreamIds)); + bytes memory callData = abi.encodeCall(ISablierV2OpenEnded.cancelMultiple, (defaultStreamIds)); _test_RevertWhen_DelegateCall(callData); } @@ -32,16 +24,16 @@ contract CancelMultiple_Integration_Concrete_Test is Integration_Test { } function test_RevertGiven_OnlyNull() external whenNotDelegateCalled whenArrayCountNotZero { - testStreamIds[0] = nullStreamId; - testStreamIds[1] = nullStreamId; + defaultStreamIds[0] = nullStreamId; + defaultStreamIds[1] = nullStreamId; vm.expectRevert(abi.encodeWithSelector(Errors.SablierV2OpenEnded_Null.selector, nullStreamId)); - openEnded.cancelMultiple({ streamIds: testStreamIds }); + openEnded.cancelMultiple({ streamIds: defaultStreamIds }); } function test_RevertGiven_SomeNull() external whenNotDelegateCalled whenArrayCountNotZero { - testStreamIds[0] = nullStreamId; + defaultStreamIds[0] = nullStreamId; vm.expectRevert(abi.encodeWithSelector(Errors.SablierV2OpenEnded_Null.selector, nullStreamId)); - openEnded.cancelMultiple({ streamIds: testStreamIds }); + openEnded.cancelMultiple({ streamIds: defaultStreamIds }); } function test_RevertWhen_CallerUnauthorizedAllStreams_MaliciousThirdParty() @@ -54,9 +46,9 @@ contract CancelMultiple_Integration_Concrete_Test is Integration_Test { resetPrank({ msgSender: users.eve }); vm.expectRevert( - abi.encodeWithSelector(Errors.SablierV2OpenEnded_Unauthorized.selector, testStreamIds[0], users.eve) + abi.encodeWithSelector(Errors.SablierV2OpenEnded_Unauthorized.selector, defaultStreamIds[0], users.eve) ); - openEnded.cancelMultiple(testStreamIds); + openEnded.cancelMultiple(defaultStreamIds); } function test_RevertWhen_CallerUnauthorizedAllStreams_Recipient() @@ -69,9 +61,11 @@ contract CancelMultiple_Integration_Concrete_Test is Integration_Test { resetPrank({ msgSender: users.recipient }); vm.expectRevert( - abi.encodeWithSelector(Errors.SablierV2OpenEnded_Unauthorized.selector, testStreamIds[0], users.recipient) + abi.encodeWithSelector( + Errors.SablierV2OpenEnded_Unauthorized.selector, defaultStreamIds[0], users.recipient + ) ); - openEnded.cancelMultiple(testStreamIds); + openEnded.cancelMultiple(defaultStreamIds); } function test_RevertWhen_CallerUnauthorizedSomeStreams_MaliciousThirdParty() @@ -89,11 +83,11 @@ contract CancelMultiple_Integration_Concrete_Test is Integration_Test { }); resetPrank({ msgSender: users.eve }); - testStreamIds[0] = eveStreamId; + defaultStreamIds[0] = eveStreamId; vm.expectRevert( - abi.encodeWithSelector(Errors.SablierV2OpenEnded_Unauthorized.selector, testStreamIds[1], users.eve) + abi.encodeWithSelector(Errors.SablierV2OpenEnded_Unauthorized.selector, defaultStreamIds[1], users.eve) ); - openEnded.cancelMultiple(testStreamIds); + openEnded.cancelMultiple(defaultStreamIds); } function test_RevertWhen_CallerUnauthorizedSomeStreams_Recipient() @@ -103,7 +97,7 @@ contract CancelMultiple_Integration_Concrete_Test is Integration_Test { givenNotNull whenCallerUnauthorized { - testStreamIds[0] = openEnded.create({ + defaultStreamIds[0] = openEnded.create({ sender: users.recipient, recipient: users.recipient, ratePerSecond: RATE_PER_SECOND, @@ -113,18 +107,20 @@ contract CancelMultiple_Integration_Concrete_Test is Integration_Test { resetPrank({ msgSender: users.recipient }); vm.expectRevert( - abi.encodeWithSelector(Errors.SablierV2OpenEnded_Unauthorized.selector, testStreamIds[1], users.recipient) + abi.encodeWithSelector( + Errors.SablierV2OpenEnded_Unauthorized.selector, defaultStreamIds[1], users.recipient + ) ); - openEnded.cancelMultiple(testStreamIds); + openEnded.cancelMultiple(defaultStreamIds); } function test_CancelMultiple() external { - openEnded.cancelMultiple(testStreamIds); + openEnded.cancelMultiple(defaultStreamIds); - assertTrue(openEnded.isCanceled(testStreamIds[0])); - assertTrue(openEnded.isCanceled(testStreamIds[1])); + assertTrue(openEnded.isCanceled(defaultStreamIds[0])); + assertTrue(openEnded.isCanceled(defaultStreamIds[1])); - assertEq(openEnded.getRatePerSecond(testStreamIds[0]), 0); - assertEq(openEnded.getRatePerSecond(testStreamIds[1]), 0); + assertEq(openEnded.getRatePerSecond(defaultStreamIds[0]), 0); + assertEq(openEnded.getRatePerSecond(defaultStreamIds[1]), 0); } } From 8e5241638fdadf50af6cd6bb12c1cfcb6c235cf7 Mon Sep 17 00:00:00 2001 From: andreivladbrg Date: Fri, 19 Apr 2024 17:37:40 +0300 Subject: [PATCH 06/21] test: withdrawMultiple function test: set the block.timestamp to May 1 2024 --- test/Base.t.sol | 6 +- test/integration/Integration.t.sol | 6 +- .../withdraw-multiple/withdrawMultiple.t.sol | 265 ++++++++++++++++++ .../withdraw-multiple/withdrawMultiple.tree | 38 +++ 4 files changed, 313 insertions(+), 2 deletions(-) create mode 100644 test/integration/withdraw-multiple/withdrawMultiple.t.sol create mode 100644 test/integration/withdraw-multiple/withdrawMultiple.tree diff --git a/test/Base.t.sol b/test/Base.t.sol index 18da4963..710ef3e7 100644 --- a/test/Base.t.sol +++ b/test/Base.t.sol @@ -29,7 +29,8 @@ abstract contract Base_Test is Assertions, Events, Modifiers, Test, Utils { uint128 public constant RATE_PER_SECOND = 0.001e18; // 86.4 daily uint128 public constant DEPOSIT_AMOUNT = 50_000e18; - uint40 public immutable ONE_MONTH = 1 days * 30; // "30/360" convention + uint40 internal constant MAY_1_2024 = 1_714_518_000; + uint40 public immutable ONE_MONTH = 30 days; // "30/360" convention uint128 public constant ONE_MONTH_STREAMED_AMOUNT = 2592e18; // 86.4 * 30 uint128 public constant ONE_MONTH_REFUNDABLE_AMOUNT = DEPOSIT_AMOUNT - ONE_MONTH_STREAMED_AMOUNT; uint128 public constant REFUND_AMOUNT = 10_000e18; @@ -52,6 +53,9 @@ abstract contract Base_Test is Assertions, Events, Modifiers, Test, Utils { //////////////////////////////////////////////////////////////////////////*/ constructor() { + // Warp to May 1, 2024 at 00:00 GMT to provide a more realistic testing environment. + vm.warp({ newTimestamp: MAY_1_2024 }); + WARP_ONE_MONTH = uint40(block.timestamp + ONE_MONTH); WITHDRAW_TIME = uint40(block.timestamp) + 2_500_000; } diff --git a/test/integration/Integration.t.sol b/test/integration/Integration.t.sol index 6e83760c..dd776fac 100644 --- a/test/integration/Integration.t.sol +++ b/test/integration/Integration.t.sol @@ -37,7 +37,11 @@ abstract contract Integration_Test is Base_Test { } function defaultDeposit() internal { - openEnded.deposit(defaultStreamId, DEPOSIT_AMOUNT); + defaultDeposit(defaultStreamId); + } + + function defaultDeposit(uint256 streamId) internal { + openEnded.deposit(streamId, DEPOSIT_AMOUNT); } /*////////////////////////////////////////////////////////////////////////// diff --git a/test/integration/withdraw-multiple/withdrawMultiple.t.sol b/test/integration/withdraw-multiple/withdrawMultiple.t.sol new file mode 100644 index 00000000..42d497d0 --- /dev/null +++ b/test/integration/withdraw-multiple/withdrawMultiple.t.sol @@ -0,0 +1,265 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.22; + +import { ISablierV2OpenEnded } from "src/interfaces/ISablierV2OpenEnded.sol"; +import { Errors } from "src/libraries/Errors.sol"; + +import { Integration_Test } from "../Integration.t.sol"; + +contract WithdrawMultiple_Integration_Concrete_Test is Integration_Test { + uint40[] internal times; + + function setUp() public override { + Integration_Test.setUp(); + + times.push(WITHDRAW_TIME); + times.push(WITHDRAW_TIME); + vm.warp({ newTimestamp: WARP_ONE_MONTH }); + } + + function test_RevertWhen_DelegateCall() external { + bytes memory callData = abi.encodeCall(ISablierV2OpenEnded.withdrawMultiple, (defaultStreamIds, times)); + _test_RevertWhen_DelegateCall(callData); + } + + function test_RevertWhen_ArrayCountsNotEqual() external whenNotDelegateCalled { + uint256[] memory streamIds = new uint256[](0); + uint40[] memory _times = new uint40[](1); + vm.expectRevert( + abi.encodeWithSelector(Errors.SablierV2OpenEnded_WithdrawMultipleArrayCountsNotEqual.selector, 0, 1) + ); + openEnded.withdrawMultiple(streamIds, _times); + } + + modifier whenArrayCountsAreEqual() { + _; + } + + function test_WithdrawMultiple_ArrayCountsZero() external whenNotDelegateCalled whenArrayCountsAreEqual { + uint256[] memory streamIds = new uint256[](0); + uint40[] memory _times = new uint40[](0); + openEnded.withdrawMultiple(streamIds, _times); + } + + modifier whenArrayCountsNotZero() { + _; + } + + function test_RevertGiven_OnlyNull() + external + whenNotDelegateCalled + whenArrayCountsAreEqual + whenArrayCountsNotZero + { + defaultStreamIds[0] = nullStreamId; + defaultStreamIds[1] = nullStreamId; + vm.expectRevert(abi.encodeWithSelector(Errors.SablierV2OpenEnded_Null.selector, nullStreamId)); + openEnded.withdrawMultiple({ streamIds: defaultStreamIds, times: times }); + } + + function test_RevertGiven_SomeNull() + external + whenNotDelegateCalled + whenArrayCountsAreEqual + whenArrayCountsNotZero + { + defaultStreamIds[0] = nullStreamId; + vm.expectRevert(abi.encodeWithSelector(Errors.SablierV2OpenEnded_Null.selector, nullStreamId)); + openEnded.withdrawMultiple({ streamIds: defaultStreamIds, times: times }); + } + + function test_RevertGiven_OnlyCanceled() + external + whenNotDelegateCalled + whenArrayCountsAreEqual + whenArrayCountsNotZero + givenNotNull + { + openEnded.cancel(defaultStreamIds[1]); + expectRevertCanceled(); + openEnded.withdrawMultiple({ streamIds: defaultStreamIds, times: times }); + } + + function test_RevertGiven_SomeCanceled() + external + whenNotDelegateCalled + whenArrayCountsAreEqual + whenArrayCountsNotZero + givenNotNull + { + expectRevertCanceled(); + openEnded.withdrawMultiple({ streamIds: defaultStreamIds, times: times }); + } + + function test_RevertWhen_OnlyWithdrawalTimesNotGreaterThanLastTimeUpdate() + external + whenNotDelegateCalled + whenArrayCountsAreEqual + whenArrayCountsNotZero + givenNotNull + givenNotCanceled + { + uint40 lastTimeUpdate = openEnded.getLastTimeUpdate(defaultStreamIds[0]); + times[0] = lastTimeUpdate; + + vm.expectRevert( + abi.encodeWithSelector( + Errors.SablierV2OpenEnded_WithdrawalTimeNotGreaterThanLastUpdate.selector, + lastTimeUpdate, + lastTimeUpdate + ) + ); + openEnded.withdrawMultiple({ streamIds: defaultStreamIds, times: times }); + } + + function test_RevertWhen_SomeWithdrawalTimesNotGreaterThanLastTimeUpdate() + external + whenNotDelegateCalled + whenArrayCountsAreEqual + whenArrayCountsNotZero + givenNotNull + givenNotCanceled + { + uint40 lastTimeUpdate = openEnded.getLastTimeUpdate(defaultStreamIds[0]); + times[0] = lastTimeUpdate; + times[1] = lastTimeUpdate; + + vm.expectRevert( + abi.encodeWithSelector( + Errors.SablierV2OpenEnded_WithdrawalTimeNotGreaterThanLastUpdate.selector, + lastTimeUpdate, + lastTimeUpdate + ) + ); + openEnded.withdrawMultiple({ streamIds: defaultStreamIds, times: times }); + } + + function test_RevertWhen_OnlyWithdrawalTimesInTheFuture() + external + whenNotDelegateCalled + whenArrayCountsAreEqual + whenArrayCountsNotZero + givenNotNull + givenNotCanceled + whenWithdrawalTimeGreaterThanLastUpdate + { + uint40 futureTime = uint40(block.timestamp + 1); + times[0] = futureTime; + times[1] = futureTime; + + vm.expectRevert( + abi.encodeWithSelector( + Errors.SablierV2OpenEnded_WithdrawalTimeInTheFuture.selector, futureTime, WARP_ONE_MONTH + ) + ); + openEnded.withdrawMultiple({ streamIds: defaultStreamIds, times: times }); + } + + function test_RevertWhen_SomeWithdrawalTimesInTheFuture() + external + whenNotDelegateCalled + whenArrayCountsAreEqual + whenArrayCountsNotZero + givenNotNull + givenNotCanceled + whenWithdrawalTimeGreaterThanLastUpdate + { + defaultDeposit(); + + uint40 futureTime = uint40(block.timestamp + 1); + times[1] = futureTime; + + vm.expectRevert( + abi.encodeWithSelector( + Errors.SablierV2OpenEnded_WithdrawalTimeInTheFuture.selector, futureTime, WARP_ONE_MONTH + ) + ); + openEnded.withdrawMultiple({ streamIds: defaultStreamIds, times: times }); + } + + function test_RevertGiven_OnlyZeroBalances() + external + whenNotDelegateCalled + whenArrayCountsAreEqual + whenArrayCountsNotZero + givenNotNull + givenNotCanceled + whenWithdrawalTimeGreaterThanLastUpdate + whenWithdrawalTimeNotInTheFuture + { + vm.expectRevert( + abi.encodeWithSelector(Errors.SablierV2OpenEnded_WithdrawBalanceZero.selector, defaultStreamIds[0]) + ); + openEnded.withdrawMultiple({ streamIds: defaultStreamIds, times: times }); + } + + function test_RevertGiven_SomeZeroBalances() + external + whenNotDelegateCalled + whenArrayCountsAreEqual + whenArrayCountsNotZero + givenNotNull + givenNotCanceled + whenWithdrawalTimeGreaterThanLastUpdate + whenWithdrawalTimeNotInTheFuture + { + defaultDeposit(); + + vm.expectRevert( + abi.encodeWithSelector(Errors.SablierV2OpenEnded_WithdrawBalanceZero.selector, defaultStreamIds[1]) + ); + openEnded.withdrawMultiple({ streamIds: defaultStreamIds, times: times }); + } + + function test_WithdrawMultiple() + external + whenNotDelegateCalled + whenArrayCountsAreEqual + whenArrayCountsNotZero + givenNotNull + givenNotCanceled + whenWithdrawalTimeGreaterThanLastUpdate + whenWithdrawalTimeNotInTheFuture + { + defaultDeposit(); + defaultDeposit(defaultStreamIds[1]); + + uint40 actualLastTimeUpdate = openEnded.getLastTimeUpdate(defaultStreamIds[0]); + uint40 expectedLastTimeUpdate = uint40(block.timestamp - ONE_MONTH); + assertEq(actualLastTimeUpdate, expectedLastTimeUpdate, "last time updated"); + + actualLastTimeUpdate = openEnded.getLastTimeUpdate(defaultStreamIds[1]); + assertEq(actualLastTimeUpdate, expectedLastTimeUpdate, "last time updated"); + + vm.expectEmit({ emitter: address(openEnded) }); + emit WithdrawFromOpenEndedStream({ + streamId: defaultStreamIds[0], + to: users.recipient, + asset: dai, + amount: WITHDRAW_AMOUNT + }); + vm.expectEmit({ emitter: address(openEnded) }); + emit WithdrawFromOpenEndedStream({ + streamId: defaultStreamIds[1], + to: users.recipient, + asset: dai, + amount: WITHDRAW_AMOUNT + }); + + openEnded.withdrawMultiple({ streamIds: defaultStreamIds, times: times }); + + actualLastTimeUpdate = openEnded.getLastTimeUpdate(defaultStreamIds[0]); + expectedLastTimeUpdate = WITHDRAW_TIME; + assertEq(actualLastTimeUpdate, expectedLastTimeUpdate, "last time updated"); + + actualLastTimeUpdate = openEnded.getLastTimeUpdate(defaultStreamIds[1]); + assertEq(actualLastTimeUpdate, expectedLastTimeUpdate, "last time updated"); + + uint128 actualStreamBalance = openEnded.getBalance(defaultStreamIds[0]); + uint128 expectedStreamBalance = DEPOSIT_AMOUNT - WITHDRAW_AMOUNT; + assertEq(actualStreamBalance, expectedStreamBalance, "stream balance"); + + actualStreamBalance = openEnded.getBalance(defaultStreamIds[1]); + assertEq(actualStreamBalance, expectedStreamBalance, "stream balance"); + } +} diff --git a/test/integration/withdraw-multiple/withdrawMultiple.tree b/test/integration/withdraw-multiple/withdrawMultiple.tree new file mode 100644 index 00000000..5e60a542 --- /dev/null +++ b/test/integration/withdraw-multiple/withdrawMultiple.tree @@ -0,0 +1,38 @@ +withdrawMultiple.t.sol +├── when delegate called +│ └── it should revert +└── when not delegate called + ├── when the input array counts are not equal + │ └── it should revert + └── when the input array counts are equal + ├── when the array counts are zero + │ └── it should do nothing + └── when the array counts are not zero + ├── given the stream IDs array references only null streams + │ └── it should revert + ├── given the stream IDs array references some null streams + │ └── it should revert + └── given the stream IDs array references only non-null streams + ├── given the stream IDs array references only canceled streams + │ └── it should revert + ├── given the stream IDs array references some canceled streams + │ └── it should revert + └── given the stream IDs array references only non-canceled streams + ├── when all withdrawal times are not strictly greater than the last time update + │ └── it should revert + ├── when some withdrawal times are not strictly greater than the last time update + │ └── it should revert + └── when none withdrawal times are strictly greater than the last time update + ├── when all withdrawal times are in the future + │ └── it should revert + ├── when some withdrawal times are in the future + │ └── it should revert + └── when none withdrawal times are in the future + ├── when all balances are zero + │ └── it should revert + ├── when some balances are zero + │ └── it should revert + └── when all balances are greater than zero + ├── it should make the withdrawals + ├── it should update the times + └── it should emit multiple {WithdrawFromOpenEndedStream} events \ No newline at end of file From b4d9d471428d350907ec7d7d1894a1c5a2c3d445 Mon Sep 17 00:00:00 2001 From: andreivladbrg Date: Sat, 20 Apr 2024 18:40:28 +0300 Subject: [PATCH 07/21] feat: implement createMultiple function --- src/SablierV2OpenEnded.sol | 28 ++++++++++++++++++++++++++ src/interfaces/ISablierV2OpenEnded.sol | 25 +++++++++++++++++++++-- src/libraries/Errors.sol | 6 ++++++ 3 files changed, 57 insertions(+), 2 deletions(-) diff --git a/src/SablierV2OpenEnded.sol b/src/SablierV2OpenEnded.sol index d09ca3ea..ded3eef4 100644 --- a/src/SablierV2OpenEnded.sol +++ b/src/SablierV2OpenEnded.sol @@ -165,6 +165,34 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope _deposit(streamId, depositAmount); } + /// @inheritdoc ISablierV2OpenEnded + function createMultiple( + address[] calldata senders, + address[] calldata recipients, + uint128[] calldata ratesPerSecond, + IERC20 asset + ) + external + returns (uint256[] memory streamIds) + { + uint256 sendersCount = senders.length; + uint256 recipientsCount = recipients.length; + uint256 ratesPerSecondCount = ratesPerSecond.length; + + // Check: count of `senders`, `recipients`, `ratesPerSecond` matches. + if (sendersCount != recipientsCount || sendersCount != ratesPerSecondCount) { + revert Errors.SablierV2OpenEnded_CreateArrayCountsNotEqual( + sendersCount, recipientsCount, ratesPerSecondCount + ); + } + + streamIds = new uint256[](sendersCount); + for (uint256 i = 0; i < sendersCount; ++i) { + // Checks, Effects and Interactions: create the stream. + streamIds[i] = _create(senders[i], recipients[i], ratesPerSecond[i], asset); + } + } + /// @inheritdoc ISablierV2OpenEnded function deposit(uint256 streamId, uint128 amount) external noDelegateCall notCanceled(streamId) { // Checks, Effects and Interactions: deposit on stream. diff --git a/src/interfaces/ISablierV2OpenEnded.sol b/src/interfaces/ISablierV2OpenEnded.sol index c5c73154..95c7d531 100644 --- a/src/interfaces/ISablierV2OpenEnded.sol +++ b/src/interfaces/ISablierV2OpenEnded.sol @@ -198,8 +198,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// /// @param recipient The address receiving the assets. /// @param sender The address streaming the assets, with the ability to adjust and cancel the stream. It doesn't - /// have - /// to be the same as `msg.sender`. + /// have to be the same as `msg.sender`. /// @param ratePerSecond The amount of assets that is increasing by every second, denoted in 18 decimals. /// @param asset The contract address of the ERC-20 asset used for streaming. /// @return streamId The ID of the newly created stream. @@ -238,6 +237,28 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { external returns (uint256 streamId); + /// @notice Creates multiple open-ended streams with the `block.timestamp` as the time reference and with zero + /// balance. + /// + /// @dev Emits multiple {CreateOpenEndedStream} events. + /// + /// Requirements: + /// - There must be an equal number of `recipients`, `senders` and `ratesPerSecond`. + /// - All requirements from {create} must be met for each stream. + /// + /// @param recipients The addresses receiving the assets. + /// @param senders The addresses streaming the assets, with the ability to adjust and cancel the stream. + /// @param ratesPerSecond The amounts of assets that are increasing by every second, denoted in 18 decimals. + /// @param asset The contract address of the ERC-20 asset used for streaming. + function createMultiple( + address[] calldata recipients, + address[] calldata senders, + uint128[] calldata ratesPerSecond, + IERC20 asset + ) + external + returns (uint256[] memory streamIds); + /// @notice Deposits assets in a stream. /// /// @dev Emits a {Transfer} and {DepositOpenEndedStream} event. diff --git a/src/libraries/Errors.sol b/src/libraries/Errors.sol index 20eee260..4d442a66 100644 --- a/src/libraries/Errors.sol +++ b/src/libraries/Errors.sol @@ -17,6 +17,12 @@ library Errors { SABLIER-V2-OpenEnded //////////////////////////////////////////////////////////////////////////*/ + /// @notice Thrown when trying to create multiple streams and the number of senders, recipients and rates per second + /// does not match. + error SablierV2OpenEnded_CreateArrayCountsNotEqual( + uint256 sendersCount, uint256 recipientsCount, uint256 ratesPerSecondCount + ); + /// @notice Thrown when trying to set the rate per second of a stream to zero. error SablierV2OpenEnded_RatePerSecondZero(); From 34ed95cba60a93a7fd13947bba23cc55a6fc8bdb Mon Sep 17 00:00:00 2001 From: andreivladbrg Date: Mon, 22 Apr 2024 15:43:46 +0300 Subject: [PATCH 08/21] feat: implement createAndDepositMultiple function refactor: use specific amount names instead of a generic one --- src/SablierV2OpenEnded.sol | 83 +++++++++++-------- src/interfaces/ISablierV2OpenEnded.sol | 58 +++++++++---- src/libraries/Errors.sol | 8 +- test/integration/deposit/deposit.t.sol | 7 +- .../refund-from-stream/refundFromStream.t.sol | 21 +++-- .../withdraw-multiple/withdrawMultiple.t.sol | 4 +- test/integration/withdraw/withdraw.t.sol | 2 +- test/invariant/handlers/OpenEndedHandler.sol | 2 +- test/utils/Events.sol | 6 +- 9 files changed, 118 insertions(+), 73 deletions(-) diff --git a/src/SablierV2OpenEnded.sol b/src/SablierV2OpenEnded.sol index ded3eef4..f51e955f 100644 --- a/src/SablierV2OpenEnded.sol +++ b/src/SablierV2OpenEnded.sol @@ -167,62 +167,73 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope /// @inheritdoc ISablierV2OpenEnded function createMultiple( - address[] calldata senders, address[] calldata recipients, + address[] calldata senders, uint128[] calldata ratesPerSecond, IERC20 asset ) - external + public returns (uint256[] memory streamIds) { - uint256 sendersCount = senders.length; uint256 recipientsCount = recipients.length; + uint256 sendersCount = senders.length; uint256 ratesPerSecondCount = ratesPerSecond.length; // Check: count of `senders`, `recipients`, `ratesPerSecond` matches. - if (sendersCount != recipientsCount || sendersCount != ratesPerSecondCount) { + if (recipientsCount != sendersCount || recipientsCount != ratesPerSecondCount) { revert Errors.SablierV2OpenEnded_CreateArrayCountsNotEqual( sendersCount, recipientsCount, ratesPerSecondCount ); } - streamIds = new uint256[](sendersCount); - for (uint256 i = 0; i < sendersCount; ++i) { + streamIds = new uint256[](recipientsCount); + for (uint256 i = 0; i < recipientsCount; ++i) { // Checks, Effects and Interactions: create the stream. streamIds[i] = _create(senders[i], recipients[i], ratesPerSecond[i], asset); } } /// @inheritdoc ISablierV2OpenEnded - function deposit(uint256 streamId, uint128 amount) external noDelegateCall notCanceled(streamId) { + function createAndDepositMultiple( + address[] calldata recipients, + address[] calldata senders, + uint128[] calldata ratesPerSecond, + IERC20 asset, + uint128[] calldata depositAmounts + ) + external + returns (uint256[] memory streamIds) + { + streamIds = new uint256[](recipients.length); + streamIds = createMultiple(recipients, senders, ratesPerSecond, asset); + + depositMultiple(streamIds, depositAmounts); + } + + /// @inheritdoc ISablierV2OpenEnded + function deposit(uint256 streamId, uint128 depositAmount) external noDelegateCall notCanceled(streamId) { // Checks, Effects and Interactions: deposit on stream. - _deposit(streamId, amount); + _deposit(streamId, depositAmount); } /// @inheritdoc ISablierV2OpenEnded - function depositMultiple(uint256[] calldata streamIds, uint128[] calldata amounts) external noDelegateCall { + function depositMultiple(uint256[] memory streamIds, uint128[] calldata amounts) public noDelegateCall { uint256 streamIdsCount = streamIds.length; - uint256 amountsCount = amounts.length; + uint256 depositAmountsCount = amounts.length; // Check: count of `streamIds` matches count of `amounts`. - if (streamIdsCount != amountsCount) { - revert Errors.SablierV2OpenEnded_DepositArrayCountsNotEqual(streamIdsCount, amountsCount); + if (streamIdsCount != depositAmountsCount) { + revert Errors.SablierV2OpenEnded_DepositArrayCountsNotEqual(streamIdsCount, depositAmountsCount); } - uint256 streamId; - uint128 amount; for (uint256 i = 0; i < streamIdsCount; ++i) { - streamId = streamIds[i]; - // Check: the stream is not canceled. - if (isCanceled(streamId)) { - revert Errors.SablierV2OpenEnded_StreamCanceled(streamId); + if (isCanceled(streamIds[i])) { + revert Errors.SablierV2OpenEnded_StreamCanceled(streamIds[i]); } - amount = amounts[i]; - // Checks, Effects and Interactions: deposit on stream. - _deposit(streamId, amount); + _deposit(streamIds[i], amounts[i]); } } @@ -244,7 +255,7 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope /// @inheritdoc ISablierV2OpenEnded function refundFromStream( uint256 streamId, - uint128 amount + uint128 refundAmount ) external noDelegateCall @@ -252,7 +263,7 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope onlySender(streamId) { // Checks, Effects and Interactions: make the refund. - _refundFromStream(streamId, amount); + _refundFromStream(streamId, refundAmount); } /// @inheritdoc ISablierV2OpenEnded @@ -346,7 +357,7 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope /// @dev Calculates the streamed amount. function _streamedAmountOf(uint256 streamId, uint40 time) internal view returns (uint128) { - uint128 lastTimeUpdate = uint128(_streams[streamId].lastTimeUpdate); + uint40 lastTimeUpdate = _streams[streamId].lastTimeUpdate; // If the time reference is less than or equal to the `lastTimeUpdate`, return zero. if (time <= lastTimeUpdate) { @@ -526,26 +537,26 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope } /// @dev See the documentation for the user-facing functions that call this internal function. - function _deposit(uint256 streamId, uint128 amount) internal { - // Check: the amount is not zero. - if (amount == 0) { + function _deposit(uint256 streamId, uint128 depositAmount) internal { + // Check: the deposit amount is not zero. + if (depositAmount == 0) { revert Errors.SablierV2OpenEnded_DepositAmountZero(); } // Effect: update the stream balance. - _streams[streamId].balance += amount; + _streams[streamId].balance += depositAmount; // Retrieve the ERC-20 asset from storage. IERC20 asset = _streams[streamId].asset; // Calculate the transfer amount. - uint128 transferAmount = _calculateTransferAmount(streamId, amount); + uint128 transferAmount = _calculateTransferAmount(streamId, depositAmount); // Interaction: transfer the deposit amount. asset.safeTransferFrom(msg.sender, address(this), transferAmount); // Log the deposit. - emit ISablierV2OpenEnded.DepositOpenEndedStream(streamId, msg.sender, asset, amount); + emit ISablierV2OpenEnded.DepositOpenEndedStream(streamId, msg.sender, asset, depositAmount); } /// @dev Helper function to update the `balance` and to perform the ERC-20 transfer. @@ -561,25 +572,25 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope } /// @dev See the documentation for the user-facing functions that call this internal function. - function _refundFromStream(uint256 streamId, uint128 amount) internal { + function _refundFromStream(uint256 streamId, uint128 refundAmount) internal { address sender = _streams[streamId].sender; uint128 refundableAmount = _refundableAmountOf(streamId, uint40(block.timestamp)); // Check: the amount is not zero. - if (amount == 0) { + if (refundAmount == 0) { revert Errors.SablierV2OpenEnded_RefundAmountZero(); } // Check: the withdraw amount is not greater than the refundable amount. - if (amount > refundableAmount) { - revert Errors.SablierV2OpenEnded_Overrefund(streamId, amount, refundableAmount); + if (refundAmount > refundableAmount) { + revert Errors.SablierV2OpenEnded_Overrefund(streamId, refundAmount, refundableAmount); } // Effects and interactions: update the `balance` and perform the ERC-20 transfer. - _extractFromStream(streamId, sender, amount); + _extractFromStream(streamId, sender, refundAmount); // Log the refund. - emit ISablierV2OpenEnded.RefundFromOpenEndedStream(streamId, sender, _streams[streamId].asset, amount); + emit ISablierV2OpenEnded.RefundFromOpenEndedStream(streamId, sender, _streams[streamId].asset, refundAmount); } /// @dev See the documentation for the user-facing functions that call this internal function. diff --git a/src/interfaces/ISablierV2OpenEnded.sol b/src/interfaces/ISablierV2OpenEnded.sol index 95c7d531..4579078e 100644 --- a/src/interfaces/ISablierV2OpenEnded.sol +++ b/src/interfaces/ISablierV2OpenEnded.sol @@ -62,18 +62,18 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// @param streamId The ID of the open-ended stream. /// @param funder The address which funded the stream. /// @param asset The contract address of the ERC-20 asset used for streaming. - /// @param amount The amount of assets deposited, denoted in 18 decimals. + /// @param depositAmount The amount of assets deposited, denoted in 18 decimals. event DepositOpenEndedStream( - uint256 indexed streamId, address indexed funder, IERC20 indexed asset, uint128 amount + uint256 indexed streamId, address indexed funder, IERC20 indexed asset, uint128 depositAmount ); /// @notice Emitted when assets are refunded from a open-ended stream. /// @param streamId The ID of the open-ended stream. /// @param sender The address of the stream's sender. /// @param asset The contract address of the ERC-20 asset used for streaming. - /// @param amount The amount of assets deposited, denoted in 18 decimals. + /// @param refundAmount The amount of assets refunded to the sender, denoted in 18 decimals. event RefundFromOpenEndedStream( - uint256 indexed streamId, address indexed sender, IERC20 indexed asset, uint128 amount + uint256 indexed streamId, address indexed sender, IERC20 indexed asset, uint128 refundAmount ); /// @notice Emitted when a open-ended stream is re-started. @@ -89,9 +89,9 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// @param streamId The ID of the stream. /// @param to The address that has received the withdrawn assets. /// @param asset The contract address of the ERC-20 asset used for streaming. - /// @param amount The amount of assets withdrawn, denoted in 18 decimals. + /// @param withdrawAmount The amount of assets withdrawn, denoted in 18 decimals. event WithdrawFromOpenEndedStream( - uint256 indexed streamId, address indexed to, IERC20 indexed asset, uint128 amount + uint256 indexed streamId, address indexed to, IERC20 indexed asset, uint128 withdrawAmount ); /*////////////////////////////////////////////////////////////////////////// @@ -155,8 +155,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// - `streamId` must not reference a null stream. /// - `streamId` must not reference a canceled stream. /// - `msg.sender` must be the stream's sender. - /// - `newRatePerSecond` must be greater than zero. - /// - `newRatePerSecond` must not be equal to the actual rate per second. + /// - `newRatePerSecond` must be greater than zero and not equal to the current rate per second. /// /// @param streamId The ID of the stream to adjust. /// @param newRatePerSecond The new rate per second of the open-ended stream, denoted in 18 decimals. @@ -237,6 +236,31 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { external returns (uint256 streamId); + /// @notice Creates multiple open-ended streams with the `block.timestamp` as the time reference and with + /// `depositAmounts` balances. + /// + /// @dev Emits multiple {CreateOpenEndedStream}, {Transfer} and {DepositOpenEndedStream} events. + /// + /// Requirements: + /// - All requirements from {create} must be met for each stream. + /// - There must be an equal number of `recipients`, `senders`, `ratesPerSecond` and `depositAmounts`. + /// + /// @param recipients The addresses receiving the assets. + /// @param senders The addresses streaming the assets, with the ability to adjust and cancel the stream. + /// @param ratesPerSecond The amounts of assets that are increasing by every second, denoted in 18 decimals. + /// @param asset The contract address of the ERC-20 asset used for streaming. + /// @param depositAmounts The amounts deposited in the streams. + /// @return streamIds The IDs of the newly created streams. + function createAndDepositMultiple( + address[] calldata recipients, + address[] calldata senders, + uint128[] calldata ratesPerSecond, + IERC20 asset, + uint128[] calldata depositAmounts + ) + external + returns (uint256[] memory streamIds); + /// @notice Creates multiple open-ended streams with the `block.timestamp` as the time reference and with zero /// balance. /// @@ -267,11 +291,11 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// - Must not be delegate called. /// - `streamId` must not reference a null stream. /// - `streamId` must not reference a canceled stream. - /// - `amount` must be greater than zero. + /// - `depositAmount` must be greater than zero. /// /// @param streamId The ID of the stream to deposit on. - /// @param amount The amount deposited in the stream, denoted in 18 decimals. - function deposit(uint256 streamId, uint128 amount) external; + /// @param depositAmount The amount deposited in the stream, denoted in 18 decimals. + function deposit(uint256 streamId, uint128 depositAmount) external; /// @notice Deposits assets in multiple streams. /// @@ -279,11 +303,11 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// /// Requirements: /// - All requirements from {deposit} must be met for each stream. - /// - There must be an equal number of `streamIds` and `amounts`. + /// - There must be an equal number of `streamIds` and `depositAmount`. /// /// @param streamIds The ids of the streams to deposit on. - /// @param amounts The amounts of assets to be deposited, denoted in 18 decimals. - function depositMultiple(uint256[] calldata streamIds, uint128[] calldata amounts) external; + /// @param depositAmounts The amount of assets to be deposited, denoted in 18 decimals. + function depositMultiple(uint256[] calldata streamIds, uint128[] calldata depositAmounts) external; /// @notice Refunds the provided amount of assets from the stream to the sender's address. /// @@ -294,11 +318,11 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// - `streamId` must not reference a null stream. /// - `streamId` must not reference a canceled stream. /// - `msg.sender` must be the sender. - /// - `amount` must be greater than zero and must not exceed the refundable amount. + /// - `refundAmount` must be greater than zero and must not exceed the refundable amount. /// /// @param streamId The ID of the stream to refund from. - /// @param amount The amount to refund, in units of the ERC-20 asset's decimals. - function refundFromStream(uint256 streamId, uint128 amount) external; + /// @param refundAmount The amount to refund, denoted in 18 decimals. + function refundFromStream(uint256 streamId, uint128 refundAmount) external; /// @notice Restarts the stream with the provided rate per second. /// diff --git a/src/libraries/Errors.sol b/src/libraries/Errors.sol index 4d442a66..71aa1eaa 100644 --- a/src/libraries/Errors.sol +++ b/src/libraries/Errors.sol @@ -20,7 +20,7 @@ library Errors { /// @notice Thrown when trying to create multiple streams and the number of senders, recipients and rates per second /// does not match. error SablierV2OpenEnded_CreateArrayCountsNotEqual( - uint256 sendersCount, uint256 recipientsCount, uint256 ratesPerSecondCount + uint256 recipientsCount, uint256 sendersCount, uint256 ratesPerSecondCount ); /// @notice Thrown when trying to set the rate per second of a stream to zero. @@ -32,9 +32,9 @@ library Errors { /// @notice Thrown when trying to create a OpenEnded stream with a zero deposit amount. error SablierV2OpenEnded_DepositAmountZero(); - /// @notice Thrown when trying to deposit on multiple streams and the number of stream ids does + /// @notice Thrown when trying to deposit on multiple streams and the number of stream IDs does /// not match the number of deposit amounts. - error SablierV2OpenEnded_DepositArrayCountsNotEqual(uint256 streamIdsCount, uint256 amountsCount); + error SablierV2OpenEnded_DepositArrayCountsNotEqual(uint256 streamIdsCount, uint256 depositAmountsCount); /// @notice Thrown when trying to create a stream with an asset with no decimals. error SablierV2OpenEnded_InvalidAssetDecimals(IERC20 asset); @@ -46,7 +46,7 @@ library Errors { error SablierV2OpenEnded_Null(uint256 streamId); /// @notice Thrown when trying to refund an amount greater than the refundable amount. - error SablierV2OpenEnded_Overrefund(uint256 streamId, uint128 amount, uint128 refundableAmount); + error SablierV2OpenEnded_Overrefund(uint256 streamId, uint128 refundAmount, uint128 refundableAmount); /// @notice Thrown when trying to create a OpenEnded stream with the recipient as the zero address. error SablierV2OpenEnded_RecipientZeroAddress(); diff --git a/test/integration/deposit/deposit.t.sol b/test/integration/deposit/deposit.t.sol index 4a9cf5a0..6578cd40 100644 --- a/test/integration/deposit/deposit.t.sol +++ b/test/integration/deposit/deposit.t.sol @@ -57,7 +57,12 @@ contract Deposit_Integration_Test is Integration_Test { }); vm.expectEmit({ emitter: address(openEnded) }); - emit DepositOpenEndedStream({ streamId: streamId, funder: users.sender, asset: asset, amount: DEPOSIT_AMOUNT }); + emit DepositOpenEndedStream({ + streamId: streamId, + funder: users.sender, + asset: asset, + depositAmount: DEPOSIT_AMOUNT + }); expectCallToTransferFrom({ asset: asset, diff --git a/test/integration/refund-from-stream/refundFromStream.t.sol b/test/integration/refund-from-stream/refundFromStream.t.sol index 3c559660..ecca343c 100644 --- a/test/integration/refund-from-stream/refundFromStream.t.sol +++ b/test/integration/refund-from-stream/refundFromStream.t.sol @@ -24,12 +24,12 @@ contract RefundFromStream_Integration_Test is Integration_Test { function test_RevertGiven_Null() external whenNotDelegateCalled { expectRevertNull(); - openEnded.refundFromStream({ streamId: nullStreamId, amount: REFUND_AMOUNT }); + openEnded.refundFromStream({ streamId: nullStreamId, refundAmount: REFUND_AMOUNT }); } function test_RevertGiven_Canceled() external whenNotDelegateCalled givenNotNull { expectRevertCanceled(); - openEnded.refundFromStream({ streamId: defaultStreamId, amount: REFUND_AMOUNT }); + openEnded.refundFromStream({ streamId: defaultStreamId, refundAmount: REFUND_AMOUNT }); } function test_RevertWhen_CallerUnauthorized_Recipient() @@ -43,7 +43,7 @@ contract RefundFromStream_Integration_Test is Integration_Test { vm.expectRevert( abi.encodeWithSelector(Errors.SablierV2OpenEnded_Unauthorized.selector, defaultStreamId, users.recipient) ); - openEnded.refundFromStream({ streamId: defaultStreamId, amount: REFUND_AMOUNT }); + openEnded.refundFromStream({ streamId: defaultStreamId, refundAmount: REFUND_AMOUNT }); } function test_RevertWhen_CallerUnauthorized_MaliciousThirdParty() @@ -57,7 +57,7 @@ contract RefundFromStream_Integration_Test is Integration_Test { vm.expectRevert( abi.encodeWithSelector(Errors.SablierV2OpenEnded_Unauthorized.selector, defaultStreamId, users.eve) ); - openEnded.refundFromStream({ streamId: defaultStreamId, amount: REFUND_AMOUNT }); + openEnded.refundFromStream({ streamId: defaultStreamId, refundAmount: REFUND_AMOUNT }); } function test_RevertWhen_RefundAmountZero() @@ -68,7 +68,7 @@ contract RefundFromStream_Integration_Test is Integration_Test { whenCallerAuthorized { vm.expectRevert(Errors.SablierV2OpenEnded_RefundAmountZero.selector); - openEnded.refundFromStream({ streamId: defaultStreamId, amount: 0 }); + openEnded.refundFromStream({ streamId: defaultStreamId, refundAmount: 0 }); } function test_RevertWhen_Overrefund() @@ -87,7 +87,7 @@ contract RefundFromStream_Integration_Test is Integration_Test { DEPOSIT_AMOUNT - ONE_MONTH_STREAMED_AMOUNT ) ); - openEnded.refundFromStream({ streamId: defaultStreamId, amount: DEPOSIT_AMOUNT }); + openEnded.refundFromStream({ streamId: defaultStreamId, refundAmount: DEPOSIT_AMOUNT }); } function test_RefundFromStream_AssetNot18Decimals() @@ -129,10 +129,15 @@ contract RefundFromStream_Integration_Test is Integration_Test { }); vm.expectEmit({ emitter: address(openEnded) }); - emit RefundFromOpenEndedStream({ streamId: streamId, sender: users.sender, asset: asset, amount: REFUND_AMOUNT }); + emit RefundFromOpenEndedStream({ + streamId: streamId, + sender: users.sender, + asset: asset, + refundAmount: REFUND_AMOUNT + }); expectCallToTransfer({ asset: asset, to: users.sender, amount: normalizeTransferAmount(streamId, REFUND_AMOUNT) }); - openEnded.refundFromStream({ streamId: streamId, amount: REFUND_AMOUNT }); + openEnded.refundFromStream({ streamId: streamId, refundAmount: REFUND_AMOUNT }); uint128 actualStreamBalance = openEnded.getBalance(streamId); uint128 expectedStreamBalance = DEPOSIT_AMOUNT - REFUND_AMOUNT; diff --git a/test/integration/withdraw-multiple/withdrawMultiple.t.sol b/test/integration/withdraw-multiple/withdrawMultiple.t.sol index 42d497d0..7459020f 100644 --- a/test/integration/withdraw-multiple/withdrawMultiple.t.sol +++ b/test/integration/withdraw-multiple/withdrawMultiple.t.sol @@ -236,14 +236,14 @@ contract WithdrawMultiple_Integration_Concrete_Test is Integration_Test { streamId: defaultStreamIds[0], to: users.recipient, asset: dai, - amount: WITHDRAW_AMOUNT + withdrawAmount: WITHDRAW_AMOUNT }); vm.expectEmit({ emitter: address(openEnded) }); emit WithdrawFromOpenEndedStream({ streamId: defaultStreamIds[1], to: users.recipient, asset: dai, - amount: WITHDRAW_AMOUNT + withdrawAmount: WITHDRAW_AMOUNT }); openEnded.withdrawMultiple({ streamIds: defaultStreamIds, times: times }); diff --git a/test/integration/withdraw/withdraw.t.sol b/test/integration/withdraw/withdraw.t.sol index dc483c8d..91a92bdf 100644 --- a/test/integration/withdraw/withdraw.t.sol +++ b/test/integration/withdraw/withdraw.t.sol @@ -236,7 +236,7 @@ contract Withdraw_Integration_Test is Integration_Test { streamId: streamId, to: users.recipient, asset: asset, - amount: WITHDRAW_AMOUNT + withdrawAmount: WITHDRAW_AMOUNT }); expectCallToTransfer({ diff --git a/test/invariant/handlers/OpenEndedHandler.sol b/test/invariant/handlers/OpenEndedHandler.sol index 822935cb..b0aed418 100644 --- a/test/invariant/handlers/OpenEndedHandler.sol +++ b/test/invariant/handlers/OpenEndedHandler.sol @@ -125,7 +125,7 @@ contract OpenEndedHandler is BaseHandler { asset.approve({ spender: address(openEnded), value: depositAmount }); // Deposit into the stream. - openEnded.deposit({ streamId: currentStreamId, amount: depositAmount }); + openEnded.deposit({ streamId: currentStreamId, depositAmount: depositAmount }); // Store the deposited amount. openEndedStore.updateStreamDepositedAmountsSum(depositAmount); diff --git a/test/utils/Events.sol b/test/utils/Events.sol index 026e1891..2d70e7a0 100644 --- a/test/utils/Events.sol +++ b/test/utils/Events.sol @@ -33,11 +33,11 @@ abstract contract Events { ); event DepositOpenEndedStream( - uint256 indexed streamId, address indexed funder, IERC20 indexed asset, uint128 amount + uint256 indexed streamId, address indexed funder, IERC20 indexed asset, uint128 depositAmount ); event RefundFromOpenEndedStream( - uint256 indexed streamId, address indexed sender, IERC20 indexed asset, uint128 amount + uint256 indexed streamId, address indexed sender, IERC20 indexed asset, uint128 refundAmount ); event RestartOpenEndedStream( @@ -45,6 +45,6 @@ abstract contract Events { ); event WithdrawFromOpenEndedStream( - uint256 indexed streamId, address indexed to, IERC20 indexed asset, uint128 amount + uint256 indexed streamId, address indexed to, IERC20 indexed asset, uint128 withdrawAmount ); } From 53f7b3aab3a8d449e981575fbcade652be5b25c2 Mon Sep 17 00:00:00 2001 From: andreivladbrg Date: Mon, 22 Apr 2024 16:34:47 +0300 Subject: [PATCH 09/21] docs: improve readability for streamId requirements test: say "given" for balance zero tests --- src/interfaces/ISablierV2OpenEnded.sol | 15 +++++---------- .../withdraw-multiple/withdrawMultiple.t.sol | 1 + .../withdraw-multiple/withdrawMultiple.tree | 6 +++--- test/integration/withdraw/withdraw.t.sol | 6 +++++- test/integration/withdraw/withdraw.tree | 4 ++-- test/utils/Modifiers.sol | 6 +++++- 6 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/interfaces/ISablierV2OpenEnded.sol b/src/interfaces/ISablierV2OpenEnded.sol index 4579078e..84538ad1 100644 --- a/src/interfaces/ISablierV2OpenEnded.sol +++ b/src/interfaces/ISablierV2OpenEnded.sol @@ -152,8 +152,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// /// Requiremenets: /// - Must not be delegate called. - /// - `streamId` must not reference a null stream. - /// - `streamId` must not reference a canceled stream. + /// - `streamId` must not reference a null stream or a canceled stream. /// - `msg.sender` must be the stream's sender. /// - `newRatePerSecond` must be greater than zero and not equal to the current rate per second. /// @@ -167,8 +166,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// /// Requirements: /// - Must not be delegate called. - /// - `streamId` must not reference a null stream. - /// - `streamId` must not reference a canceled stream. + /// - `streamId` must not reference a null stream or a canceled stream. /// - `msg.sender` must be the stream's sender. /// /// @param streamId The ID of the stream to cancel. @@ -289,8 +287,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// /// Requirements: /// - Must not be delegate called. - /// - `streamId` must not reference a null stream. - /// - `streamId` must not reference a canceled stream. + /// - `streamId` must not reference a null stream or a canceled stream. /// - `depositAmount` must be greater than zero. /// /// @param streamId The ID of the stream to deposit on. @@ -315,8 +312,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// /// Requirements: /// - Must not be delegate called. - /// - `streamId` must not reference a null stream. - /// - `streamId` must not reference a canceled stream. + /// - `streamId` must not reference a null stream or a canceled stream. /// - `msg.sender` must be the sender. /// - `refundAmount` must be greater than zero and must not exceed the refundable amount. /// @@ -330,8 +326,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// /// Requirements: /// - Must not be delegate called. - // - `streamId` must not reference a null stream. - /// - `streamId` must reference a canceled stream. + // - `streamId` must not reference a null stream or a canceled stream. /// - `msg.sender` must be the stream's sender. /// - `ratePerSecond` must be greater than zero. /// diff --git a/test/integration/withdraw-multiple/withdrawMultiple.t.sol b/test/integration/withdraw-multiple/withdrawMultiple.t.sol index 7459020f..957e2f24 100644 --- a/test/integration/withdraw-multiple/withdrawMultiple.t.sol +++ b/test/integration/withdraw-multiple/withdrawMultiple.t.sol @@ -220,6 +220,7 @@ contract WithdrawMultiple_Integration_Concrete_Test is Integration_Test { givenNotCanceled whenWithdrawalTimeGreaterThanLastUpdate whenWithdrawalTimeNotInTheFuture + givenBalanceNotZero { defaultDeposit(); defaultDeposit(defaultStreamIds[1]); diff --git a/test/integration/withdraw-multiple/withdrawMultiple.tree b/test/integration/withdraw-multiple/withdrawMultiple.tree index 5e60a542..c1fccd8a 100644 --- a/test/integration/withdraw-multiple/withdrawMultiple.tree +++ b/test/integration/withdraw-multiple/withdrawMultiple.tree @@ -28,11 +28,11 @@ withdrawMultiple.t.sol ├── when some withdrawal times are in the future │ └── it should revert └── when none withdrawal times are in the future - ├── when all balances are zero + ├── given all balances are zero │ └── it should revert - ├── when some balances are zero + ├── given some balances are zero │ └── it should revert - └── when all balances are greater than zero + └── given all balances are greater than zero ├── it should make the withdrawals ├── it should update the times └── it should emit multiple {WithdrawFromOpenEndedStream} events \ No newline at end of file diff --git a/test/integration/withdraw/withdraw.t.sol b/test/integration/withdraw/withdraw.t.sol index 91a92bdf..842d1d68 100644 --- a/test/integration/withdraw/withdraw.t.sol +++ b/test/integration/withdraw/withdraw.t.sol @@ -120,7 +120,7 @@ contract Withdraw_Integration_Test is Integration_Test { openEnded.withdraw({ streamId: defaultStreamId, to: users.recipient, time: futureTime }); } - function test_RevertWhen_BalanceZero() + function test_RevertGiven_BalanceZero() external whenNotDelegateCalled givenNotNull @@ -147,6 +147,7 @@ contract Withdraw_Integration_Test is Integration_Test { whenWithdrawalAddressIsRecipient whenWithdrawalTimeGreaterThanLastUpdate whenWithdrawalTimeNotInTheFuture + givenBalanceNotZero { openEnded.withdraw({ streamId: defaultStreamId, to: users.recipient, time: WITHDRAW_TIME }); @@ -168,6 +169,7 @@ contract Withdraw_Integration_Test is Integration_Test { whenWithdrawalAddressIsRecipient whenWithdrawalTimeGreaterThanLastUpdate whenWithdrawalTimeNotInTheFuture + givenBalanceNotZero { address unknownCaller = address(0xCAFE); resetPrank({ msgSender: unknownCaller }); @@ -192,6 +194,7 @@ contract Withdraw_Integration_Test is Integration_Test { whenWithdrawalAddressIsRecipient whenWithdrawalTimeGreaterThanLastUpdate whenWithdrawalTimeNotInTheFuture + givenBalanceNotZero whenCallerRecipient { // Set the timestamp to 1 month ago to create the stream with the same `lastTimeUpdate` as `defaultStreamId`. @@ -212,6 +215,7 @@ contract Withdraw_Integration_Test is Integration_Test { whenWithdrawalAddressIsRecipient whenWithdrawalTimeGreaterThanLastUpdate whenWithdrawalTimeNotInTheFuture + givenBalanceNotZero whenCallerRecipient { _test_Withdraw(defaultStreamId, dai); diff --git a/test/integration/withdraw/withdraw.tree b/test/integration/withdraw/withdraw.tree index 4a262b41..019148d6 100644 --- a/test/integration/withdraw/withdraw.tree +++ b/test/integration/withdraw/withdraw.tree @@ -23,9 +23,9 @@ withdraw.t.sol ├── when the withdrawal time is in the future │ └── it should revert └── when the withdrawal time is not in the future - ├── when the balance is zero + ├── given the balance is zero │ └── it should revert - └── when the balance is not zero + └── given the balance is not zero ├── when the caller is not the recipient │ ├── when the caller is the sender │ │ └── it should make the withdrawal diff --git a/test/utils/Modifiers.sol b/test/utils/Modifiers.sol index ea2230fb..b2e5525c 100644 --- a/test/utils/Modifiers.sol +++ b/test/utils/Modifiers.sol @@ -94,7 +94,7 @@ abstract contract Modifiers { WITHDRAW //////////////////////////////////////////////////////////////////////////*/ - modifier whenToNonZeroAddress() { + modifier givenBalanceNotZero() { _; } @@ -102,6 +102,10 @@ abstract contract Modifiers { _; } + modifier whenToNonZeroAddress() { + _; + } + modifier whenWithdrawalAddressIsRecipient() { _; } From 0d0576a83ce854f95c15cd01b03405b093ba604b Mon Sep 17 00:00:00 2001 From: andreivladbrg Date: Mon, 22 Apr 2024 17:40:10 +0300 Subject: [PATCH 10/21] refactor: change order of array counts in custom error test: createMultiple function --- src/SablierV2OpenEnded.sol | 4 +- src/libraries/Errors.sol | 2 +- .../create-multiple/createMultiple.t.sol | 138 ++++++++++++++++++ .../create-multiple/createMultiple.tree | 15 ++ 4 files changed, 156 insertions(+), 3 deletions(-) create mode 100644 test/integration/create-multiple/createMultiple.t.sol create mode 100644 test/integration/create-multiple/createMultiple.tree diff --git a/src/SablierV2OpenEnded.sol b/src/SablierV2OpenEnded.sol index f51e955f..508cae7b 100644 --- a/src/SablierV2OpenEnded.sol +++ b/src/SablierV2OpenEnded.sol @@ -181,8 +181,8 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope // Check: count of `senders`, `recipients`, `ratesPerSecond` matches. if (recipientsCount != sendersCount || recipientsCount != ratesPerSecondCount) { - revert Errors.SablierV2OpenEnded_CreateArrayCountsNotEqual( - sendersCount, recipientsCount, ratesPerSecondCount + revert Errors.SablierV2OpenEnded_CreateMultipleArrayCountsNotEqual( + recipientsCount, sendersCount, ratesPerSecondCount ); } diff --git a/src/libraries/Errors.sol b/src/libraries/Errors.sol index 71aa1eaa..27cb9e5c 100644 --- a/src/libraries/Errors.sol +++ b/src/libraries/Errors.sol @@ -19,7 +19,7 @@ library Errors { /// @notice Thrown when trying to create multiple streams and the number of senders, recipients and rates per second /// does not match. - error SablierV2OpenEnded_CreateArrayCountsNotEqual( + error SablierV2OpenEnded_CreateMultipleArrayCountsNotEqual( uint256 recipientsCount, uint256 sendersCount, uint256 ratesPerSecondCount ); diff --git a/test/integration/create-multiple/createMultiple.t.sol b/test/integration/create-multiple/createMultiple.t.sol new file mode 100644 index 00000000..6b5eec34 --- /dev/null +++ b/test/integration/create-multiple/createMultiple.t.sol @@ -0,0 +1,138 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.22; + +import { ISablierV2OpenEnded } from "src/interfaces/ISablierV2OpenEnded.sol"; +import { Errors } from "src/libraries/Errors.sol"; +import { OpenEnded } from "src/types/DataTypes.sol"; + +import { Integration_Test } from "../Integration.t.sol"; + +contract CreateMultiple_Integration_Test is Integration_Test { + address[] internal defaultRecipients; + address[] internal defaultSenders; + uint128[] internal defaultRatesPerSecond; + + function setUp() public override { + Integration_Test.setUp(); + + defaultRecipients.push(users.recipient); + defaultRecipients.push(users.recipient); + defaultSenders.push(users.sender); + defaultSenders.push(users.sender); + defaultRatesPerSecond.push(RATE_PER_SECOND); + defaultRatesPerSecond.push(RATE_PER_SECOND); + } + + function test_RevertWhen_DelegateCall() external { + bytes memory callData = abi.encodeCall( + ISablierV2OpenEnded.createMultiple, (defaultRecipients, defaultSenders, defaultRatesPerSecond, dai) + ); + _test_RevertWhen_DelegateCall(callData); + } + + modifier whenArrayCountsNotEqual() { + _; + } + + function test_RevertWhen_RecipientsCountNotEqual() external whenNotDelegateCalled whenArrayCountsNotEqual { + address[] memory recipients = new address[](1); + + vm.expectRevert( + abi.encodeWithSelector( + Errors.SablierV2OpenEnded_CreateMultipleArrayCountsNotEqual.selector, + recipients.length, + defaultSenders.length, + defaultRatesPerSecond.length + ) + ); + openEnded.createMultiple(recipients, defaultSenders, defaultRatesPerSecond, dai); + } + + function test_RevertWhen_SendersCountNotEqual() external whenNotDelegateCalled whenArrayCountsNotEqual { + address[] memory senders = new address[](1); + + vm.expectRevert( + abi.encodeWithSelector( + Errors.SablierV2OpenEnded_CreateMultipleArrayCountsNotEqual.selector, + defaultRecipients.length, + senders.length, + defaultRatesPerSecond.length + ) + ); + openEnded.createMultiple(defaultRecipients, senders, defaultRatesPerSecond, dai); + } + + function test_RevertWhen_RatesPerSecondCountNotEqual() external whenNotDelegateCalled whenArrayCountsNotEqual { + uint128[] memory ratesPerSecond = new uint128[](1); + + vm.expectRevert( + abi.encodeWithSelector( + Errors.SablierV2OpenEnded_CreateMultipleArrayCountsNotEqual.selector, + defaultRecipients.length, + defaultSenders.length, + ratesPerSecond.length + ) + ); + openEnded.createMultiple(defaultRecipients, defaultSenders, ratesPerSecond, dai); + } + + modifier whenArrayCountsEqual() { + _; + } + + function test_CreateMultiple() external whenNotDelegateCalled whenArrayCountsEqual { + uint256 beforeNextStreamId = openEnded.nextStreamId(); + + vm.expectEmit({ emitter: address(openEnded) }); + emit CreateOpenEndedStream({ + streamId: beforeNextStreamId, + sender: users.sender, + recipient: users.recipient, + ratePerSecond: RATE_PER_SECOND, + asset: dai, + lastTimeUpdate: uint40(block.timestamp) + }); + vm.expectEmit({ emitter: address(openEnded) }); + emit CreateOpenEndedStream({ + streamId: beforeNextStreamId + 1, + sender: users.sender, + recipient: users.recipient, + ratePerSecond: RATE_PER_SECOND, + asset: dai, + lastTimeUpdate: uint40(block.timestamp) + }); + + uint256[] memory streamIds = + openEnded.createMultiple(defaultRecipients, defaultSenders, defaultRatesPerSecond, dai); + + uint256 afterNextStreamId = openEnded.nextStreamId(); + + assertEq(streamIds[0], beforeNextStreamId, "streamIds[0] != beforeNextStreamId"); + assertEq(streamIds[1], beforeNextStreamId + 1, "streamIds[1] != beforeNextStreamId + 1"); + + assertEq(streamIds.length, defaultRecipients.length, "streamIds.length != defaultRecipients.length"); + assertEq( + beforeNextStreamId + defaultRecipients.length, + afterNextStreamId, + "afterNextStreamId != beforeNextStreamId + defaultRecipients.length" + ); + + OpenEnded.Stream memory expectedStream = OpenEnded.Stream({ + ratePerSecond: RATE_PER_SECOND, + asset: dai, + assetDecimals: 18, + balance: 0, + lastTimeUpdate: uint40(block.timestamp), + isCanceled: false, + isStream: true, + recipient: users.recipient, + sender: users.sender + }); + + OpenEnded.Stream memory actualStream = openEnded.getStream(streamIds[0]); + assertEq(actualStream, expectedStream); + + actualStream = openEnded.getStream(streamIds[1]); + assertEq(actualStream, expectedStream); + } +} diff --git a/test/integration/create-multiple/createMultiple.tree b/test/integration/create-multiple/createMultiple.tree new file mode 100644 index 00000000..fda1e5ac --- /dev/null +++ b/test/integration/create-multiple/createMultiple.tree @@ -0,0 +1,15 @@ +createMultiple.t.sol +├── when delegate called +│ └── it should revert +└── when not delegate called + ├── when arrays counts are not equal + │ ├── when the recipients array is not equal + │ │ └── it should revert + │ ├── when the senders array is not equal + │ │ └── it should revert + │ └── when the rates per second array is not equal + │ └── it should revert + └── when arrays counts are equal + ├── it should create multiple streams + ├── it should bump the next stream id multiple times + └── it should emit multiple {CreateOpenEndedStream} events \ No newline at end of file From 6cca0e60a8728dd3ed0a41270d511aba3b7a62af Mon Sep 17 00:00:00 2001 From: andreivladbrg Date: Wed, 24 Apr 2024 15:58:00 +0300 Subject: [PATCH 11/21] test: createAndDepositMultiple function --- src/SablierV2OpenEnded.sol | 2 +- .../createAndDepositMultiple.t.sol | 127 ++++++++++++++++++ .../createAndDepositMultiple.tree | 13 ++ .../create-multiple/createMultiple.t.sol | 8 -- .../create-multiple/createMultiple.tree | 4 +- test/utils/Modifiers.sol | 12 ++ 6 files changed, 155 insertions(+), 11 deletions(-) create mode 100644 test/integration/create-and-deposit-multiple/createAndDepositMultiple.t.sol create mode 100644 test/integration/create-and-deposit-multiple/createAndDepositMultiple.tree diff --git a/src/SablierV2OpenEnded.sol b/src/SablierV2OpenEnded.sol index 508cae7b..81afd8dd 100644 --- a/src/SablierV2OpenEnded.sol +++ b/src/SablierV2OpenEnded.sol @@ -179,7 +179,7 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope uint256 sendersCount = senders.length; uint256 ratesPerSecondCount = ratesPerSecond.length; - // Check: count of `senders`, `recipients`, `ratesPerSecond` matches. + // Check: count of `senders`, `recipients` and `ratesPerSecond` matches. if (recipientsCount != sendersCount || recipientsCount != ratesPerSecondCount) { revert Errors.SablierV2OpenEnded_CreateMultipleArrayCountsNotEqual( recipientsCount, sendersCount, ratesPerSecondCount diff --git a/test/integration/create-and-deposit-multiple/createAndDepositMultiple.t.sol b/test/integration/create-and-deposit-multiple/createAndDepositMultiple.t.sol new file mode 100644 index 00000000..7c473d30 --- /dev/null +++ b/test/integration/create-and-deposit-multiple/createAndDepositMultiple.t.sol @@ -0,0 +1,127 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.22; + +import { ISablierV2OpenEnded } from "src/interfaces/ISablierV2OpenEnded.sol"; +import { Errors } from "src/libraries/Errors.sol"; +import { OpenEnded } from "src/types/DataTypes.sol"; + +import { Integration_Test } from "../Integration.t.sol"; + +contract CreateMultiple_Integration_Test is Integration_Test { + address[] internal defaultRecipients; + address[] internal defaultSenders; + uint128[] internal defaultRatesPerSecond; + uint128[] internal defaultDepositAmounts; + + function setUp() public override { + Integration_Test.setUp(); + + defaultRecipients.push(users.recipient); + defaultRecipients.push(users.recipient); + defaultSenders.push(users.sender); + defaultSenders.push(users.sender); + defaultRatesPerSecond.push(RATE_PER_SECOND); + defaultRatesPerSecond.push(RATE_PER_SECOND); + defaultDepositAmounts.push(DEPOSIT_AMOUNT); + defaultDepositAmounts.push(DEPOSIT_AMOUNT); + } + + function test_RevertWhen_DelegateCall() external { + bytes memory callData = abi.encodeCall( + ISablierV2OpenEnded.createAndDepositMultiple, + (defaultRecipients, defaultSenders, defaultRatesPerSecond, dai, defaultDepositAmounts) + ); + _test_RevertWhen_DelegateCall(callData); + } + + function test_RevertWhen_DepositAmountsArrayIsNotEqual() external whenNotDelegateCalled whenArrayCountsNotEqual { + uint128[] memory depositAmounts = new uint128[](0); + + vm.expectRevert( + abi.encodeWithSelector( + Errors.SablierV2OpenEnded_DepositArrayCountsNotEqual.selector, + defaultRecipients.length, + depositAmounts.length + ) + ); + openEnded.createAndDepositMultiple( + defaultRecipients, defaultSenders, defaultRatesPerSecond, dai, depositAmounts + ); + } + + function test_CreateAndDepositMultiple() external whenNotDelegateCalled whenArrayCountsEqual { + uint256 beforeNextStreamId = openEnded.nextStreamId(); + + vm.expectEmit({ emitter: address(openEnded) }); + emit CreateOpenEndedStream({ + streamId: beforeNextStreamId, + sender: users.sender, + recipient: users.recipient, + ratePerSecond: RATE_PER_SECOND, + asset: dai, + lastTimeUpdate: uint40(block.timestamp) + }); + vm.expectEmit({ emitter: address(openEnded) }); + emit CreateOpenEndedStream({ + streamId: beforeNextStreamId + 1, + sender: users.sender, + recipient: users.recipient, + ratePerSecond: RATE_PER_SECOND, + asset: dai, + lastTimeUpdate: uint40(block.timestamp) + }); + + vm.expectEmit({ emitter: address(openEnded) }); + emit DepositOpenEndedStream({ + streamId: beforeNextStreamId, + funder: users.sender, + asset: dai, + depositAmount: DEPOSIT_AMOUNT + }); + + vm.expectEmit({ emitter: address(openEnded) }); + emit DepositOpenEndedStream({ + streamId: beforeNextStreamId + 1, + funder: users.sender, + asset: dai, + depositAmount: DEPOSIT_AMOUNT + }); + + expectCallToTransferFrom({ asset: dai, from: users.sender, to: address(openEnded), amount: DEPOSIT_AMOUNT }); + expectCallToTransferFrom({ asset: dai, from: users.sender, to: address(openEnded), amount: DEPOSIT_AMOUNT }); + + uint256[] memory streamIds = openEnded.createAndDepositMultiple( + defaultRecipients, defaultSenders, defaultRatesPerSecond, dai, defaultDepositAmounts + ); + + uint256 afterNextStreamId = openEnded.nextStreamId(); + + assertEq(streamIds[0], beforeNextStreamId, "streamIds[0] != beforeNextStreamId"); + assertEq(streamIds[1], beforeNextStreamId + 1, "streamIds[1] != beforeNextStreamId + 1"); + + assertEq(streamIds.length, defaultRecipients.length, "streamIds.length != defaultRecipients.length"); + assertEq( + beforeNextStreamId + defaultRecipients.length, + afterNextStreamId, + "afterNextStreamId != beforeNextStreamId + defaultRecipients.length" + ); + + OpenEnded.Stream memory expectedStream = OpenEnded.Stream({ + ratePerSecond: RATE_PER_SECOND, + asset: dai, + assetDecimals: 18, + balance: DEPOSIT_AMOUNT, + lastTimeUpdate: uint40(block.timestamp), + isCanceled: false, + isStream: true, + recipient: users.recipient, + sender: users.sender + }); + + OpenEnded.Stream memory actualStream = openEnded.getStream(streamIds[0]); + assertEq(actualStream, expectedStream); + + actualStream = openEnded.getStream(streamIds[1]); + assertEq(actualStream, expectedStream); + } +} diff --git a/test/integration/create-and-deposit-multiple/createAndDepositMultiple.tree b/test/integration/create-and-deposit-multiple/createAndDepositMultiple.tree new file mode 100644 index 00000000..f9679821 --- /dev/null +++ b/test/integration/create-and-deposit-multiple/createAndDepositMultiple.tree @@ -0,0 +1,13 @@ +createMultiple.t.sol +├── when delegate called +│ └── it should revert +└── when not delegate called + ├── when array counts are not equal + │ └── when the deposit amounts array is not equal + │ └── it should revert + └── when array counts are equal + ├── it should create multiple streams + ├── it should update the stream balance + ├── it should perform the ERC-20 transfer + ├── it should bump the next stream id multiple times + └── it should emit multiple {CreateOpenEndedStream}, {Transfer} and {DepositOpenEndedStream} events \ No newline at end of file diff --git a/test/integration/create-multiple/createMultiple.t.sol b/test/integration/create-multiple/createMultiple.t.sol index 6b5eec34..e1927335 100644 --- a/test/integration/create-multiple/createMultiple.t.sol +++ b/test/integration/create-multiple/createMultiple.t.sol @@ -30,10 +30,6 @@ contract CreateMultiple_Integration_Test is Integration_Test { _test_RevertWhen_DelegateCall(callData); } - modifier whenArrayCountsNotEqual() { - _; - } - function test_RevertWhen_RecipientsCountNotEqual() external whenNotDelegateCalled whenArrayCountsNotEqual { address[] memory recipients = new address[](1); @@ -76,10 +72,6 @@ contract CreateMultiple_Integration_Test is Integration_Test { openEnded.createMultiple(defaultRecipients, defaultSenders, ratesPerSecond, dai); } - modifier whenArrayCountsEqual() { - _; - } - function test_CreateMultiple() external whenNotDelegateCalled whenArrayCountsEqual { uint256 beforeNextStreamId = openEnded.nextStreamId(); diff --git a/test/integration/create-multiple/createMultiple.tree b/test/integration/create-multiple/createMultiple.tree index fda1e5ac..4f48b4b3 100644 --- a/test/integration/create-multiple/createMultiple.tree +++ b/test/integration/create-multiple/createMultiple.tree @@ -2,14 +2,14 @@ createMultiple.t.sol ├── when delegate called │ └── it should revert └── when not delegate called - ├── when arrays counts are not equal + ├── when array counts are not equal │ ├── when the recipients array is not equal │ │ └── it should revert │ ├── when the senders array is not equal │ │ └── it should revert │ └── when the rates per second array is not equal │ └── it should revert - └── when arrays counts are equal + └── when array counts are equal ├── it should create multiple streams ├── it should bump the next stream id multiple times └── it should emit multiple {CreateOpenEndedStream} events \ No newline at end of file diff --git a/test/utils/Modifiers.sol b/test/utils/Modifiers.sol index b2e5525c..ba8eac9e 100644 --- a/test/utils/Modifiers.sol +++ b/test/utils/Modifiers.sol @@ -62,6 +62,18 @@ abstract contract Modifiers { _; } + /*////////////////////////////////////////////////////////////////////////// + CREATE-MULTIPLE + //////////////////////////////////////////////////////////////////////////*/ + + modifier whenArrayCountsNotEqual() { + _; + } + + modifier whenArrayCountsEqual() { + _; + } + /*////////////////////////////////////////////////////////////////////////// DEPOSIT //////////////////////////////////////////////////////////////////////////*/ From e15c02c07273661b5c11370d2e82cc73f9988b33 Mon Sep 17 00:00:00 2001 From: smol-ninja Date: Mon, 29 Apr 2024 13:08:39 +0100 Subject: [PATCH 12/21] Refactoring open ended (#52) * perf: optimize modifiers * refactor: rename streamDebt to streamDebtOf * fix: add override * style: solhint-disable no-console * chore: use return variable in streamDebtOf * test: update streamDebt files --------- Co-authored-by: andreivladbrg --- script/Base.s.sol | 3 +- src/SablierV2OpenEnded.sol | 88 ++++++++++++++++--- src/abstracts/SablierV2OpenEndedState.sol | 26 +++--- src/interfaces/ISablierV2OpenEnded.sol | 18 ++-- .../streamDebtOf.t.sol} | 18 ++-- .../streamDebtOf.tree} | 2 +- 6 files changed, 111 insertions(+), 44 deletions(-) rename test/integration/{stream-debt/streamDebt.t.sol => stream-debt-of/streamDebtOf.t.sol} (50%) rename test/integration/{stream-debt/streamDebt.tree => stream-debt-of/streamDebtOf.tree} (96%) diff --git a/script/Base.s.sol b/script/Base.s.sol index a64b9c2b..3b551b38 100644 --- a/script/Base.s.sol +++ b/script/Base.s.sol @@ -1,4 +1,5 @@ // SPDX-License-Identifier: GPL-3.0-or-later +// solhint-disable no-console pragma solidity >=0.8.22; import { Strings } from "@openzeppelin/contracts/utils/Strings.sol"; @@ -57,7 +58,7 @@ abstract contract BaseScript is Script { string memory json = vm.readFile("package.json"); string memory version = json.readString(".version"); string memory create2Salt = string.concat("ChainID ", chainId, ", Version ", version); - console2.log("The CREATE2 salt is \"%s\"", create2Salt); + console2.log("The CREATE2 salt is %s", create2Salt); return bytes32(abi.encodePacked(create2Salt)); } } diff --git a/src/SablierV2OpenEnded.sol b/src/SablierV2OpenEnded.sol index 81afd8dd..7f1ec665 100644 --- a/src/SablierV2OpenEnded.sol +++ b/src/SablierV2OpenEnded.sol @@ -28,6 +28,7 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope view override notCanceled(streamId) + notNull(streamId) returns (uint128 refundableAmount) { refundableAmount = _refundableAmountOf(streamId, uint40(block.timestamp)); @@ -42,25 +43,40 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope view override notCanceled(streamId) + notNull(streamId) returns (uint128 refundableAmount) { refundableAmount = _refundableAmountOf(streamId, time); } /// @inheritdoc ISablierV2OpenEnded - function streamDebt(uint256 streamId) external view notCanceled(streamId) returns (uint128 debt) { + function streamDebtOf(uint256 streamId) + external + view + override + notCanceled(streamId) + notNull(streamId) + returns (uint128 debt) + { uint128 balance = _streams[streamId].balance; uint128 streamedAmount = _streamedAmountOf(streamId, uint40(block.timestamp)); - if (balance >= streamedAmount) { + if (balance < streamedAmount) { + debt = streamedAmount - balance; + } else { return 0; } - - debt = streamedAmount - balance; } /// @inheritdoc ISablierV2OpenEnded - function streamedAmountOf(uint256 streamId) external view notCanceled(streamId) returns (uint128 streamedAmount) { + function streamedAmountOf(uint256 streamId) + external + view + override + notCanceled(streamId) + notNull(streamId) + returns (uint128 streamedAmount) + { streamedAmount = _streamedAmountOf(streamId, uint40(block.timestamp)); } @@ -71,7 +87,9 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope ) external view + override notCanceled(streamId) + notNull(streamId) returns (uint128 streamedAmount) { streamedAmount = _streamedAmountOf(streamId, time); @@ -81,7 +99,9 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope function withdrawableAmountOf(uint256 streamId) external view + override notCanceled(streamId) + notNull(streamId) returns (uint128 withdrawableAmount) { withdrawableAmount = _withdrawableAmountOf(streamId, uint40(block.timestamp)); @@ -94,7 +114,9 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope ) external view + override notCanceled(streamId) + notNull(streamId) returns (uint128 withdrawableAmount) { withdrawableAmount = _withdrawableAmountOf(streamId, time); @@ -110,8 +132,10 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope uint128 newRatePerSecond ) external + override noDelegateCall notCanceled(streamId) + notNull(streamId) onlySender(streamId) { // Effects and Interactions: adjust the stream. @@ -119,7 +143,14 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope } /// @inheritdoc ISablierV2OpenEnded - function cancel(uint256 streamId) public noDelegateCall notCanceled(streamId) onlySender(streamId) { + function cancel(uint256 streamId) + public + override + noDelegateCall + notCanceled(streamId) + notNull(streamId) + onlySender(streamId) + { _cancel(streamId); } @@ -141,6 +172,7 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope IERC20 asset ) external + override returns (uint256 streamId) { // Checks, Effects and Interactions: create the stream. @@ -156,6 +188,7 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope uint128 depositAmount ) external + override returns (uint256 streamId) { // Checks, Effects and Interactions: create the stream. @@ -173,6 +206,7 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope IERC20 asset ) public + override returns (uint256[] memory streamIds) { uint256 recipientsCount = recipients.length; @@ -202,6 +236,7 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope uint128[] calldata depositAmounts ) external + override returns (uint256[] memory streamIds) { streamIds = new uint256[](recipients.length); @@ -211,13 +246,22 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope } /// @inheritdoc ISablierV2OpenEnded - function deposit(uint256 streamId, uint128 depositAmount) external noDelegateCall notCanceled(streamId) { + function deposit( + uint256 streamId, + uint128 depositAmount + ) + external + override + noDelegateCall + notCanceled(streamId) + notNull(streamId) + { // Checks, Effects and Interactions: deposit on stream. _deposit(streamId, depositAmount); } /// @inheritdoc ISablierV2OpenEnded - function depositMultiple(uint256[] memory streamIds, uint128[] calldata amounts) public noDelegateCall { + function depositMultiple(uint256[] memory streamIds, uint128[] calldata amounts) public override noDelegateCall { uint256 streamIdsCount = streamIds.length; uint256 depositAmountsCount = amounts.length; @@ -238,13 +282,20 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope } /// @inheritdoc ISablierV2OpenEnded - function restartStream(uint256 streamId, uint128 ratePerSecond) external { + function restartStream(uint256 streamId, uint128 ratePerSecond) external override { // Checks, Effects and Interactions: restart the stream. _restartStream(streamId, ratePerSecond); } /// @inheritdoc ISablierV2OpenEnded - function restartStreamAndDeposit(uint256 streamId, uint128 ratePerSecond, uint128 depositAmount) external { + function restartStreamAndDeposit( + uint256 streamId, + uint128 ratePerSecond, + uint128 depositAmount + ) + external + override + { // Checks, Effects and Interactions: restart the stream. _restartStream(streamId, ratePerSecond); @@ -258,8 +309,10 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope uint128 refundAmount ) external + override noDelegateCall notCanceled(streamId) + notNull(streamId) onlySender(streamId) { // Checks, Effects and Interactions: make the refund. @@ -267,7 +320,7 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope } /// @inheritdoc ISablierV2OpenEnded - function withdraw(uint256 streamId, address to, uint40 time) external { + function withdraw(uint256 streamId, address to, uint40 time) external override { // Checks, Effects and Interactions: make the withdrawal. _withdraw(streamId, to, time); } @@ -289,7 +342,7 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope } /// @inheritdoc ISablierV2OpenEnded - function withdrawMax(uint256 streamId, address to) external { + function withdrawMax(uint256 streamId, address to) external override { // Checks, Effects and Interactions: make the withdrawal. _withdraw(streamId, to, uint40(block.timestamp)); } @@ -632,7 +685,16 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope } /// @dev See the documentation for the user-facing functions that call this internal function. - function _withdraw(uint256 streamId, address to, uint40 time) internal noDelegateCall notCanceled(streamId) { + function _withdraw( + uint256 streamId, + address to, + uint40 time + ) + internal + noDelegateCall + notCanceled(streamId) + notNull(streamId) + { // Check: the withdrawal address is not zero. if (to == address(0)) { revert Errors.SablierV2OpenEnded_WithdrawToZeroAddress(); diff --git a/src/abstracts/SablierV2OpenEndedState.sol b/src/abstracts/SablierV2OpenEndedState.sol index d1b0e0b2..6464fcf9 100644 --- a/src/abstracts/SablierV2OpenEndedState.sol +++ b/src/abstracts/SablierV2OpenEndedState.sol @@ -34,7 +34,7 @@ abstract contract SablierV2OpenEndedState is ISablierV2OpenEndedState { /// @dev Checks that `streamId` does not reference a canceled stream. modifier notCanceled(uint256 streamId) { - if (isCanceled(streamId)) { + if (_streams[streamId].isCanceled) { revert Errors.SablierV2OpenEnded_StreamCanceled(streamId); } _; @@ -50,7 +50,7 @@ abstract contract SablierV2OpenEndedState is ISablierV2OpenEndedState { /// @dev Checks the `msg.sender` is the stream's sender. modifier onlySender(uint256 streamId) { - if (!_isCallerStreamSender(streamId)) { + if (msg.sender != _streams[streamId].sender) { revert Errors.SablierV2OpenEnded_Unauthorized(streamId, msg.sender); } _; @@ -109,12 +109,18 @@ abstract contract SablierV2OpenEndedState is ISablierV2OpenEndedState { } /// @inheritdoc ISablierV2OpenEndedState - function getSender(uint256 streamId) external view notNull(streamId) returns (address sender) { + function getSender(uint256 streamId) external view override notNull(streamId) returns (address sender) { sender = _streams[streamId].sender; } /// @inheritdoc ISablierV2OpenEndedState - function getStream(uint256 streamId) external view notNull(streamId) returns (OpenEnded.Stream memory stream) { + function getStream(uint256 streamId) + external + view + override + notNull(streamId) + returns (OpenEnded.Stream memory stream) + { stream = _streams[streamId]; } @@ -124,17 +130,7 @@ abstract contract SablierV2OpenEndedState is ISablierV2OpenEndedState { } /// @inheritdoc ISablierV2OpenEndedState - function isStream(uint256 streamId) public view returns (bool result) { + function isStream(uint256 streamId) public view override returns (bool result) { result = _streams[streamId].isStream; } - - /*////////////////////////////////////////////////////////////////////////// - INTERNAL CONSTANT FUNCTIONS - //////////////////////////////////////////////////////////////////////////*/ - - /// @notice Checks whether `msg.sender` is the stream's sender. - /// @param streamId The stream ID for the query. - function _isCallerStreamSender(uint256 streamId) internal view returns (bool) { - return msg.sender == _streams[streamId].sender; - } } diff --git a/src/interfaces/ISablierV2OpenEnded.sol b/src/interfaces/ISablierV2OpenEnded.sol index 84538ad1..1035c4bd 100644 --- a/src/interfaces/ISablierV2OpenEnded.sol +++ b/src/interfaces/ISablierV2OpenEnded.sol @@ -99,42 +99,44 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { //////////////////////////////////////////////////////////////////////////*/ /// @notice Calculates the amount that the sender can refund from stream, denoted in 18 decimals. - /// @dev Reverts if `streamId` references a canceled stream. + /// @dev Reverts if `streamId` references a canceled or a null stream. /// @param streamId The stream ID for the query. + /// @return refundableAmount The amount that the sender can refund. function refundableAmountOf(uint256 streamId) external view returns (uint128 refundableAmount); /// @notice Calculates the amount that the sender can refund from stream at `time`, denoted in 18 decimals. - /// @dev Reverts if `streamId` references a canceled stream. + /// @dev Reverts if `streamId` references a canceled or a null stream. /// @param streamId The stream ID for the query. /// @param time The Unix timestamp for the streamed amount calculation. + /// @return refundableAmount The amount that the sender can refund. function refundableAmountOf(uint256 streamId, uint40 time) external view returns (uint128 refundableAmount); /// @notice Calculates the amount that the sender owes on the stream, i.e. if more assets have been streamed than /// its balance, denoted in 18 decimals. If there is no debt, it will return zero. - /// @dev Reverts if `streamId` references a canceled stream. + /// @dev Reverts if `streamId` references a canceled or a null stream. /// @param streamId The stream ID for the query. - function streamDebt(uint256 streamId) external view returns (uint128 debt); + function streamDebtOf(uint256 streamId) external view returns (uint128 debt); /// @notice Calculates the amount streamed to the recipient from the last time update to the current time, /// denoted in 18 decimals. - /// @dev Reverts if `streamId` references a canceled stream. + /// @dev Reverts if `streamId` references a canceled or a null stream. /// @param streamId The stream ID for the query. function streamedAmountOf(uint256 streamId) external view returns (uint128 streamedAmount); /// @notice Calculates the amount streamed to the recipient from the last time update to `time` passed as parameter, /// denoted in 18 decimals. - /// @dev Reverts if `streamId` references a canceled stream. + /// @dev Reverts if `streamId` references a canceled or a null stream. /// @param streamId The stream ID for the query. /// @param time The Unix timestamp for the streamed amount calculation. function streamedAmountOf(uint256 streamId, uint40 time) external view returns (uint128 streamedAmount); /// @notice Calculates the amount that the recipient can withdraw from the stream, denoted in 18 decimals. - /// @dev Reverts if `streamId` references a canceled stream. + /// @dev Reverts if `streamId` references a canceled or a null stream. /// @param streamId The stream ID for the query. function withdrawableAmountOf(uint256 streamId) external view returns (uint128 withdrawableAmount); /// @notice Calculates the amount that the recipient can withdraw from the stream at `time`, denoted in 18 decimals. - /// @dev Reverts if `streamId` references a canceled stream. + /// @dev Reverts if `streamId` references a canceled or a null stream. /// @param streamId The stream ID for the query. /// @param time The Unix timestamp for the streamed amount calculation. function withdrawableAmountOf(uint256 streamId, uint40 time) external view returns (uint128 withdrawableAmount); diff --git a/test/integration/stream-debt/streamDebt.t.sol b/test/integration/stream-debt-of/streamDebtOf.t.sol similarity index 50% rename from test/integration/stream-debt/streamDebt.t.sol rename to test/integration/stream-debt-of/streamDebtOf.t.sol index d4a28a7b..a0ff6bd3 100644 --- a/test/integration/stream-debt/streamDebt.t.sol +++ b/test/integration/stream-debt-of/streamDebtOf.t.sol @@ -1,6 +1,7 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity >=0.8.22; +import { console2 } from "forge-std/src/console2.sol"; import { Integration_Test } from "../Integration.t.sol"; contract StreamDebt_Integration_Test is Integration_Test { @@ -10,23 +11,28 @@ contract StreamDebt_Integration_Test is Integration_Test { function test_RevertGiven_Null() external { expectRevertNull(); - openEnded.streamDebt(nullStreamId); + openEnded.streamDebtOf(nullStreamId); } function test_RevertGiven_Canceled() external givenNotNull { expectRevertCanceled(); - openEnded.streamDebt(defaultStreamId); + openEnded.streamDebtOf(defaultStreamId); } - function test_StreamDebt_BalanceGreaterThanOrEqualStreamedAmount() external givenNotNull givenNotCanceled { + function test_StreamDebtOf_BalanceGreaterThanOrEqualStreamedAmount() external givenNotNull givenNotCanceled { defaultDeposit(); - uint128 streamDebt = openEnded.streamDebt(defaultStreamId); + uint128 streamDebt = openEnded.streamDebtOf(defaultStreamId); + assertEq(streamDebt, 0, "stream debt"); } - function test_StreamDebt() external givenNotNull givenNotCanceled { + function test_streamDebtOf() external givenNotNull givenNotCanceled { vm.warp({ newTimestamp: WARP_ONE_MONTH }); - uint128 streamDebt = openEnded.streamDebt(defaultStreamId); + uint128 streamDebt = openEnded.streamDebtOf(defaultStreamId); + + console2.log("streamedAmountOf %s", openEnded.streamedAmountOf(defaultStreamId)); + console2.log("balance %s", openEnded.getBalance(defaultStreamId)); + assertEq(streamDebt, ONE_MONTH_STREAMED_AMOUNT, "stream debt"); } } diff --git a/test/integration/stream-debt/streamDebt.tree b/test/integration/stream-debt-of/streamDebtOf.tree similarity index 96% rename from test/integration/stream-debt/streamDebt.tree rename to test/integration/stream-debt-of/streamDebtOf.tree index 18102d58..db21f29e 100644 --- a/test/integration/stream-debt/streamDebt.tree +++ b/test/integration/stream-debt-of/streamDebtOf.tree @@ -1,4 +1,4 @@ -streamDebt.t.sol +streamDebtOf.t.sol ├── given the id references a null stream │ └── it should revert └── given the id does not reference a null stream From 6d3c8ac9fb94bcb0dffe790b3d941332ebf7f6a4 Mon Sep 17 00:00:00 2001 From: smol-ninja Date: Mon, 29 Apr 2024 13:17:02 +0100 Subject: [PATCH 13/21] test: refactor streamDebtOf --- test/integration/stream-debt-of/streamDebtOf.t.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/stream-debt-of/streamDebtOf.t.sol b/test/integration/stream-debt-of/streamDebtOf.t.sol index a0ff6bd3..7b0c1abe 100644 --- a/test/integration/stream-debt-of/streamDebtOf.t.sol +++ b/test/integration/stream-debt-of/streamDebtOf.t.sol @@ -4,7 +4,7 @@ pragma solidity >=0.8.22; import { console2 } from "forge-std/src/console2.sol"; import { Integration_Test } from "../Integration.t.sol"; -contract StreamDebt_Integration_Test is Integration_Test { +contract StreamDebtOf_Integration_Test is Integration_Test { function setUp() public override { Integration_Test.setUp(); } From a5c38d87a4dbc4718ff8781842265c142096dc52 Mon Sep 17 00:00:00 2001 From: andreivladbrg Date: Mon, 29 Apr 2024 15:54:19 +0300 Subject: [PATCH 14/21] test: remove unneeded console log --- test/integration/stream-debt-of/streamDebtOf.t.sol | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/integration/stream-debt-of/streamDebtOf.t.sol b/test/integration/stream-debt-of/streamDebtOf.t.sol index 7b0c1abe..b38aa937 100644 --- a/test/integration/stream-debt-of/streamDebtOf.t.sol +++ b/test/integration/stream-debt-of/streamDebtOf.t.sol @@ -1,7 +1,6 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity >=0.8.22; -import { console2 } from "forge-std/src/console2.sol"; import { Integration_Test } from "../Integration.t.sol"; contract StreamDebtOf_Integration_Test is Integration_Test { @@ -30,9 +29,6 @@ contract StreamDebtOf_Integration_Test is Integration_Test { vm.warp({ newTimestamp: WARP_ONE_MONTH }); uint128 streamDebt = openEnded.streamDebtOf(defaultStreamId); - console2.log("streamedAmountOf %s", openEnded.streamedAmountOf(defaultStreamId)); - console2.log("balance %s", openEnded.getBalance(defaultStreamId)); - assertEq(streamDebt, ONE_MONTH_STREAMED_AMOUNT, "stream debt"); } } From b8b2130bd8338a10d745fb618b20ca08aef11e0d Mon Sep 17 00:00:00 2001 From: andreivladbrg Date: Mon, 29 Apr 2024 16:34:43 +0300 Subject: [PATCH 15/21] refactor: say "amount" in function paramaters instead of explicit names --- src/SablierV2OpenEnded.sol | 51 ++++++++----------- src/interfaces/ISablierV2OpenEnded.sol | 30 +++++------ .../refund-from-stream/refundFromStream.t.sol | 14 ++--- test/invariant/handlers/OpenEndedHandler.sol | 2 +- 4 files changed, 45 insertions(+), 52 deletions(-) diff --git a/src/SablierV2OpenEnded.sol b/src/SablierV2OpenEnded.sol index 7f1ec665..36c48786 100644 --- a/src/SablierV2OpenEnded.sol +++ b/src/SablierV2OpenEnded.sol @@ -185,7 +185,7 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope address recipient, uint128 ratePerSecond, IERC20 asset, - uint128 depositAmount + uint128 amount ) external override @@ -195,7 +195,7 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope streamId = _create(sender, recipient, ratePerSecond, asset); // Checks, Effects and Interactions: deposit on stream. - _deposit(streamId, depositAmount); + _deposit(streamId, amount); } /// @inheritdoc ISablierV2OpenEnded @@ -233,7 +233,7 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope address[] calldata senders, uint128[] calldata ratesPerSecond, IERC20 asset, - uint128[] calldata depositAmounts + uint128[] calldata amounts ) external override @@ -242,7 +242,7 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope streamIds = new uint256[](recipients.length); streamIds = createMultiple(recipients, senders, ratesPerSecond, asset); - depositMultiple(streamIds, depositAmounts); + depositMultiple(streamIds, amounts); } /// @inheritdoc ISablierV2OpenEnded @@ -263,11 +263,11 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope /// @inheritdoc ISablierV2OpenEnded function depositMultiple(uint256[] memory streamIds, uint128[] calldata amounts) public override noDelegateCall { uint256 streamIdsCount = streamIds.length; - uint256 depositAmountsCount = amounts.length; + uint256 amountsCount = amounts.length; // Check: count of `streamIds` matches count of `amounts`. - if (streamIdsCount != depositAmountsCount) { - revert Errors.SablierV2OpenEnded_DepositArrayCountsNotEqual(streamIdsCount, depositAmountsCount); + if (streamIdsCount != amountsCount) { + revert Errors.SablierV2OpenEnded_DepositArrayCountsNotEqual(streamIdsCount, amountsCount); } for (uint256 i = 0; i < streamIdsCount; ++i) { @@ -288,25 +288,18 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope } /// @inheritdoc ISablierV2OpenEnded - function restartStreamAndDeposit( - uint256 streamId, - uint128 ratePerSecond, - uint128 depositAmount - ) - external - override - { + function restartStreamAndDeposit(uint256 streamId, uint128 ratePerSecond, uint128 amount) external override { // Checks, Effects and Interactions: restart the stream. _restartStream(streamId, ratePerSecond); // Checks, Effects and Interactions: deposit on stream. - _deposit(streamId, depositAmount); + _deposit(streamId, amount); } /// @inheritdoc ISablierV2OpenEnded function refundFromStream( uint256 streamId, - uint128 refundAmount + uint128 amount ) external override @@ -316,7 +309,7 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope onlySender(streamId) { // Checks, Effects and Interactions: make the refund. - _refundFromStream(streamId, refundAmount); + _refundFromStream(streamId, amount); } /// @inheritdoc ISablierV2OpenEnded @@ -590,26 +583,26 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope } /// @dev See the documentation for the user-facing functions that call this internal function. - function _deposit(uint256 streamId, uint128 depositAmount) internal { + function _deposit(uint256 streamId, uint128 amount) internal { // Check: the deposit amount is not zero. - if (depositAmount == 0) { + if (amount == 0) { revert Errors.SablierV2OpenEnded_DepositAmountZero(); } // Effect: update the stream balance. - _streams[streamId].balance += depositAmount; + _streams[streamId].balance += amount; // Retrieve the ERC-20 asset from storage. IERC20 asset = _streams[streamId].asset; // Calculate the transfer amount. - uint128 transferAmount = _calculateTransferAmount(streamId, depositAmount); + uint128 transferAmount = _calculateTransferAmount(streamId, amount); // Interaction: transfer the deposit amount. asset.safeTransferFrom(msg.sender, address(this), transferAmount); // Log the deposit. - emit ISablierV2OpenEnded.DepositOpenEndedStream(streamId, msg.sender, asset, depositAmount); + emit ISablierV2OpenEnded.DepositOpenEndedStream(streamId, msg.sender, asset, amount); } /// @dev Helper function to update the `balance` and to perform the ERC-20 transfer. @@ -625,25 +618,25 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope } /// @dev See the documentation for the user-facing functions that call this internal function. - function _refundFromStream(uint256 streamId, uint128 refundAmount) internal { + function _refundFromStream(uint256 streamId, uint128 amount) internal { address sender = _streams[streamId].sender; uint128 refundableAmount = _refundableAmountOf(streamId, uint40(block.timestamp)); // Check: the amount is not zero. - if (refundAmount == 0) { + if (amount == 0) { revert Errors.SablierV2OpenEnded_RefundAmountZero(); } // Check: the withdraw amount is not greater than the refundable amount. - if (refundAmount > refundableAmount) { - revert Errors.SablierV2OpenEnded_Overrefund(streamId, refundAmount, refundableAmount); + if (amount > refundableAmount) { + revert Errors.SablierV2OpenEnded_Overrefund(streamId, amount, refundableAmount); } // Effects and interactions: update the `balance` and perform the ERC-20 transfer. - _extractFromStream(streamId, sender, refundAmount); + _extractFromStream(streamId, sender, amount); // Log the refund. - emit ISablierV2OpenEnded.RefundFromOpenEndedStream(streamId, sender, _streams[streamId].asset, refundAmount); + emit ISablierV2OpenEnded.RefundFromOpenEndedStream(streamId, sender, _streams[streamId].asset, amount); } /// @dev See the documentation for the user-facing functions that call this internal function. diff --git a/src/interfaces/ISablierV2OpenEnded.sol b/src/interfaces/ISablierV2OpenEnded.sol index 1035c4bd..93090052 100644 --- a/src/interfaces/ISablierV2OpenEnded.sol +++ b/src/interfaces/ISablierV2OpenEnded.sol @@ -224,39 +224,39 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// have to be the same as `msg.sender`. /// @param ratePerSecond The amount of assets that is increasing by every second, denoted in 18 decimals. /// @param asset The contract address of the ERC-20 asset used for streaming. - /// @param depositAmount The amount deposited in the stream. + /// @param amount The amount deposited in the stream. /// @return streamId The ID of the newly created stream. function createAndDeposit( address recipient, address sender, uint128 ratePerSecond, IERC20 asset, - uint128 depositAmount + uint128 amount ) external returns (uint256 streamId); /// @notice Creates multiple open-ended streams with the `block.timestamp` as the time reference and with - /// `depositAmounts` balances. + /// `amounts` balances. /// /// @dev Emits multiple {CreateOpenEndedStream}, {Transfer} and {DepositOpenEndedStream} events. /// /// Requirements: /// - All requirements from {create} must be met for each stream. - /// - There must be an equal number of `recipients`, `senders`, `ratesPerSecond` and `depositAmounts`. + /// - There must be an equal number of `recipients`, `senders`, `ratesPerSecond` and `amounts`. /// /// @param recipients The addresses receiving the assets. /// @param senders The addresses streaming the assets, with the ability to adjust and cancel the stream. /// @param ratesPerSecond The amounts of assets that are increasing by every second, denoted in 18 decimals. /// @param asset The contract address of the ERC-20 asset used for streaming. - /// @param depositAmounts The amounts deposited in the streams. + /// @param amounts The amounts deposited in the streams. /// @return streamIds The IDs of the newly created streams. function createAndDepositMultiple( address[] calldata recipients, address[] calldata senders, uint128[] calldata ratesPerSecond, IERC20 asset, - uint128[] calldata depositAmounts + uint128[] calldata amounts ) external returns (uint256[] memory streamIds); @@ -293,8 +293,8 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// - `depositAmount` must be greater than zero. /// /// @param streamId The ID of the stream to deposit on. - /// @param depositAmount The amount deposited in the stream, denoted in 18 decimals. - function deposit(uint256 streamId, uint128 depositAmount) external; + /// @param amount The amount deposited in the stream, denoted in 18 decimals. + function deposit(uint256 streamId, uint128 amount) external; /// @notice Deposits assets in multiple streams. /// @@ -305,8 +305,8 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// - There must be an equal number of `streamIds` and `depositAmount`. /// /// @param streamIds The ids of the streams to deposit on. - /// @param depositAmounts The amount of assets to be deposited, denoted in 18 decimals. - function depositMultiple(uint256[] calldata streamIds, uint128[] calldata depositAmounts) external; + /// @param amounts The amount of assets to be deposited, denoted in 18 decimals. + function depositMultiple(uint256[] calldata streamIds, uint128[] calldata amounts) external; /// @notice Refunds the provided amount of assets from the stream to the sender's address. /// @@ -316,11 +316,11 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// - Must not be delegate called. /// - `streamId` must not reference a null stream or a canceled stream. /// - `msg.sender` must be the sender. - /// - `refundAmount` must be greater than zero and must not exceed the refundable amount. + /// - `amount` must be greater than zero and must not exceed the refundable amount. /// /// @param streamId The ID of the stream to refund from. - /// @param refundAmount The amount to refund, denoted in 18 decimals. - function refundFromStream(uint256 streamId, uint128 refundAmount) external; + /// @param amount The amount to refund, denoted in 18 decimals. + function refundFromStream(uint256 streamId, uint128 amount) external; /// @notice Restarts the stream with the provided rate per second. /// @@ -347,8 +347,8 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// /// @param streamId The ID of the stream to restart. /// @param ratePerSecond The amount of assets that is increasing by every second, denoted in 18 decimals. - /// @param depositAmount The amount deposited in the stream. - function restartStreamAndDeposit(uint256 streamId, uint128 ratePerSecond, uint128 depositAmount) external; + /// @param amount The amount deposited in the stream. + function restartStreamAndDeposit(uint256 streamId, uint128 ratePerSecond, uint128 amount) external; /// @notice Withdraws the amount of assets calculated based on time reference, from the stream /// to the provided `to` address. diff --git a/test/integration/refund-from-stream/refundFromStream.t.sol b/test/integration/refund-from-stream/refundFromStream.t.sol index ecca343c..ef3c1b0b 100644 --- a/test/integration/refund-from-stream/refundFromStream.t.sol +++ b/test/integration/refund-from-stream/refundFromStream.t.sol @@ -24,12 +24,12 @@ contract RefundFromStream_Integration_Test is Integration_Test { function test_RevertGiven_Null() external whenNotDelegateCalled { expectRevertNull(); - openEnded.refundFromStream({ streamId: nullStreamId, refundAmount: REFUND_AMOUNT }); + openEnded.refundFromStream({ streamId: nullStreamId, amount: REFUND_AMOUNT }); } function test_RevertGiven_Canceled() external whenNotDelegateCalled givenNotNull { expectRevertCanceled(); - openEnded.refundFromStream({ streamId: defaultStreamId, refundAmount: REFUND_AMOUNT }); + openEnded.refundFromStream({ streamId: defaultStreamId, amount: REFUND_AMOUNT }); } function test_RevertWhen_CallerUnauthorized_Recipient() @@ -43,7 +43,7 @@ contract RefundFromStream_Integration_Test is Integration_Test { vm.expectRevert( abi.encodeWithSelector(Errors.SablierV2OpenEnded_Unauthorized.selector, defaultStreamId, users.recipient) ); - openEnded.refundFromStream({ streamId: defaultStreamId, refundAmount: REFUND_AMOUNT }); + openEnded.refundFromStream({ streamId: defaultStreamId, amount: REFUND_AMOUNT }); } function test_RevertWhen_CallerUnauthorized_MaliciousThirdParty() @@ -57,7 +57,7 @@ contract RefundFromStream_Integration_Test is Integration_Test { vm.expectRevert( abi.encodeWithSelector(Errors.SablierV2OpenEnded_Unauthorized.selector, defaultStreamId, users.eve) ); - openEnded.refundFromStream({ streamId: defaultStreamId, refundAmount: REFUND_AMOUNT }); + openEnded.refundFromStream({ streamId: defaultStreamId, amount: REFUND_AMOUNT }); } function test_RevertWhen_RefundAmountZero() @@ -68,7 +68,7 @@ contract RefundFromStream_Integration_Test is Integration_Test { whenCallerAuthorized { vm.expectRevert(Errors.SablierV2OpenEnded_RefundAmountZero.selector); - openEnded.refundFromStream({ streamId: defaultStreamId, refundAmount: 0 }); + openEnded.refundFromStream({ streamId: defaultStreamId, amount: 0 }); } function test_RevertWhen_Overrefund() @@ -87,7 +87,7 @@ contract RefundFromStream_Integration_Test is Integration_Test { DEPOSIT_AMOUNT - ONE_MONTH_STREAMED_AMOUNT ) ); - openEnded.refundFromStream({ streamId: defaultStreamId, refundAmount: DEPOSIT_AMOUNT }); + openEnded.refundFromStream({ streamId: defaultStreamId, amount: DEPOSIT_AMOUNT }); } function test_RefundFromStream_AssetNot18Decimals() @@ -137,7 +137,7 @@ contract RefundFromStream_Integration_Test is Integration_Test { }); expectCallToTransfer({ asset: asset, to: users.sender, amount: normalizeTransferAmount(streamId, REFUND_AMOUNT) }); - openEnded.refundFromStream({ streamId: streamId, refundAmount: REFUND_AMOUNT }); + openEnded.refundFromStream({ streamId: streamId, amount: REFUND_AMOUNT }); uint128 actualStreamBalance = openEnded.getBalance(streamId); uint128 expectedStreamBalance = DEPOSIT_AMOUNT - REFUND_AMOUNT; diff --git a/test/invariant/handlers/OpenEndedHandler.sol b/test/invariant/handlers/OpenEndedHandler.sol index b0aed418..822935cb 100644 --- a/test/invariant/handlers/OpenEndedHandler.sol +++ b/test/invariant/handlers/OpenEndedHandler.sol @@ -125,7 +125,7 @@ contract OpenEndedHandler is BaseHandler { asset.approve({ spender: address(openEnded), value: depositAmount }); // Deposit into the stream. - openEnded.deposit({ streamId: currentStreamId, depositAmount: depositAmount }); + openEnded.deposit({ streamId: currentStreamId, amount: depositAmount }); // Store the deposited amount. openEndedStore.updateStreamDepositedAmountsSum(depositAmount); From 7dd64ce542dc3e3ad197b6cc441060a3a58be911 Mon Sep 17 00:00:00 2001 From: andreivladbrg Date: Mon, 29 Apr 2024 16:36:50 +0300 Subject: [PATCH 16/21] test: correct the contract name test: remove unneeded delegate call test --- .../createAndDepositMultiple.t.sol | 11 +-------- .../createAndDepositMultiple.tree | 23 ++++++++----------- 2 files changed, 11 insertions(+), 23 deletions(-) diff --git a/test/integration/create-and-deposit-multiple/createAndDepositMultiple.t.sol b/test/integration/create-and-deposit-multiple/createAndDepositMultiple.t.sol index 7c473d30..b3937ef3 100644 --- a/test/integration/create-and-deposit-multiple/createAndDepositMultiple.t.sol +++ b/test/integration/create-and-deposit-multiple/createAndDepositMultiple.t.sol @@ -1,13 +1,12 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity >=0.8.22; -import { ISablierV2OpenEnded } from "src/interfaces/ISablierV2OpenEnded.sol"; import { Errors } from "src/libraries/Errors.sol"; import { OpenEnded } from "src/types/DataTypes.sol"; import { Integration_Test } from "../Integration.t.sol"; -contract CreateMultiple_Integration_Test is Integration_Test { +contract CreateAndDepositMultiple_Integration_Test is Integration_Test { address[] internal defaultRecipients; address[] internal defaultSenders; uint128[] internal defaultRatesPerSecond; @@ -26,14 +25,6 @@ contract CreateMultiple_Integration_Test is Integration_Test { defaultDepositAmounts.push(DEPOSIT_AMOUNT); } - function test_RevertWhen_DelegateCall() external { - bytes memory callData = abi.encodeCall( - ISablierV2OpenEnded.createAndDepositMultiple, - (defaultRecipients, defaultSenders, defaultRatesPerSecond, dai, defaultDepositAmounts) - ); - _test_RevertWhen_DelegateCall(callData); - } - function test_RevertWhen_DepositAmountsArrayIsNotEqual() external whenNotDelegateCalled whenArrayCountsNotEqual { uint128[] memory depositAmounts = new uint128[](0); diff --git a/test/integration/create-and-deposit-multiple/createAndDepositMultiple.tree b/test/integration/create-and-deposit-multiple/createAndDepositMultiple.tree index f9679821..78df65f8 100644 --- a/test/integration/create-and-deposit-multiple/createAndDepositMultiple.tree +++ b/test/integration/create-and-deposit-multiple/createAndDepositMultiple.tree @@ -1,13 +1,10 @@ -createMultiple.t.sol -├── when delegate called -│ └── it should revert -└── when not delegate called - ├── when array counts are not equal - │ └── when the deposit amounts array is not equal - │ └── it should revert - └── when array counts are equal - ├── it should create multiple streams - ├── it should update the stream balance - ├── it should perform the ERC-20 transfer - ├── it should bump the next stream id multiple times - └── it should emit multiple {CreateOpenEndedStream}, {Transfer} and {DepositOpenEndedStream} events \ No newline at end of file +createAndDepositMultiple.t.sol +├── when array counts are not equal +│ └── when the deposit amounts array is not equal +│ └── it should revert +└── when array counts are equal + ├── it should create multiple streams + ├── it should update the stream balance + ├── it should perform the ERC-20 transfer + ├── it should bump the next stream id multiple times + └── it should emit multiple {CreateOpenEndedStream}, {Transfer} and {DepositOpenEndedStream} events \ No newline at end of file From 1e120e72302266e77b9c2e77c1909c118a9772a6 Mon Sep 17 00:00:00 2001 From: andreivladbrg Date: Mon, 29 Apr 2024 16:41:52 +0300 Subject: [PATCH 17/21] perf: optimize createAndDepositMultiple --- src/SablierV2OpenEnded.sol | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/SablierV2OpenEnded.sol b/src/SablierV2OpenEnded.sol index 36c48786..ca66d30f 100644 --- a/src/SablierV2OpenEnded.sol +++ b/src/SablierV2OpenEnded.sol @@ -242,7 +242,16 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope streamIds = new uint256[](recipients.length); streamIds = createMultiple(recipients, senders, ratesPerSecond, asset); - depositMultiple(streamIds, amounts); + uint256 streamIdsCount = streamIds.length; + if (streamIdsCount != amounts.length) { + revert Errors.SablierV2OpenEnded_DepositArrayCountsNotEqual(streamIdsCount, amounts.length); + } + + // Deposit on each stream. + for (uint256 i = 0; i < streamIdsCount; ++i) { + // Checks, Effects and Interactions: deposit on stream. + _deposit(streamIds[i], amounts[i]); + } } /// @inheritdoc ISablierV2OpenEnded From cfa028ac1c6a4bc83c1553c651949be45401c5d3 Mon Sep 17 00:00:00 2001 From: andreivladbrg Date: Mon, 29 Apr 2024 16:44:35 +0300 Subject: [PATCH 18/21] test: remove unneeded delegatecall test in cancelMultiple --- .../cancel-multiple/cancelMultiple.t.sol | 6 --- .../cancel-multiple/cancelMultiple.tree | 47 +++++++++---------- 2 files changed, 22 insertions(+), 31 deletions(-) diff --git a/test/integration/cancel-multiple/cancelMultiple.t.sol b/test/integration/cancel-multiple/cancelMultiple.t.sol index 0e07b221..2484ddde 100644 --- a/test/integration/cancel-multiple/cancelMultiple.t.sol +++ b/test/integration/cancel-multiple/cancelMultiple.t.sol @@ -1,7 +1,6 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity >=0.8.22; -import { ISablierV2OpenEnded } from "src/interfaces/ISablierV2OpenEnded.sol"; import { Errors } from "src/libraries/Errors.sol"; import { Integration_Test } from "../Integration.t.sol"; @@ -13,11 +12,6 @@ contract CancelMultiple_Integration_Concrete_Test is Integration_Test { vm.warp({ newTimestamp: WARP_ONE_MONTH }); } - function test_RevertWhen_DelegateCall() external { - bytes memory callData = abi.encodeCall(ISablierV2OpenEnded.cancelMultiple, (defaultStreamIds)); - _test_RevertWhen_DelegateCall(callData); - } - function test_CancelMultiple_ArrayCountZero() external whenNotDelegateCalled { uint256[] memory streamIds = new uint256[](0); openEnded.cancelMultiple(streamIds); diff --git a/test/integration/cancel-multiple/cancelMultiple.tree b/test/integration/cancel-multiple/cancelMultiple.tree index e5718f7c..7ccdbccb 100644 --- a/test/integration/cancel-multiple/cancelMultiple.tree +++ b/test/integration/cancel-multiple/cancelMultiple.tree @@ -1,26 +1,23 @@ cancelMultiple.t.sol -├── when delegate called -│ └── it should revert -└── when not delegate called - ├── when the array count is zero - │ └── it should do nothing - └── when the array count is not zero - ├── given the stream IDs array references only null streams - │ └── it should revert - ├── given the stream IDs array references some null streams - │ └── it should revert - └── given the stream IDs array references only streams that are not null - ├── when the caller is unauthorized for all streams - │ ├── when the caller is a malicious third party - │ │ └── it should revert - │ └── when the caller is the 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 the recipient - │ └── it should revert - └── when the caller is authorized for all streams - ├── it should cancel the streams - ├── it should update the rate per second for each stream - └── it should emit {CancelOpenEndedStream} events +├── when the array count is zero +│ └── it should do nothing +└── when the array count is not zero + ├── given the stream IDs array references only null streams + │ └── it should revert + ├── given the stream IDs array references some null streams + │ └── it should revert + └── given the stream IDs array references only streams that are not null + ├── when the caller is unauthorized for all streams + │ ├── when the caller is a malicious third party + │ │ └── it should revert + │ └── when the caller is the 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 the recipient + │ └── it should revert + └── when the caller is authorized for all streams + ├── it should cancel the streams + ├── it should update the rate per second for each stream + └── it should emit {CancelOpenEndedStream} events From 3569fed2d556138d8de2e17b61845fae97db7ee6 Mon Sep 17 00:00:00 2001 From: smol-ninja Date: Wed, 1 May 2024 20:00:22 +0100 Subject: [PATCH 19/21] test: refactoring relates changes --- src/SablierV2OpenEnded.sol | 4 +-- src/interfaces/ISablierV2OpenEnded.sol | 20 +++++++------- src/libraries/Errors.sol | 26 +++++++++---------- test/integration/Integration.t.sol | 17 +++++++++--- .../adjustRatePerSecond.t.sol | 2 +- test/integration/cancel/cancel.t.sol | 2 +- .../createAndDepositMultiple.t.sol | 14 ---------- .../create-multiple/createMultiple.t.sol | 15 ++--------- test/integration/create/create.t.sol | 2 +- test/integration/deposit/deposit.t.sol | 2 +- .../refund-from-stream/refundFromStream.t.sol | 2 +- .../restart-stream/restartStream.t.sol | 2 +- .../withdraw-multiple/withdrawMultiple.t.sol | 2 +- test/integration/withdraw/withdraw.t.sol | 2 +- 14 files changed, 50 insertions(+), 62 deletions(-) diff --git a/src/SablierV2OpenEnded.sol b/src/SablierV2OpenEnded.sol index ca66d30f..67195382 100644 --- a/src/SablierV2OpenEnded.sol +++ b/src/SablierV2OpenEnded.sol @@ -257,7 +257,7 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope /// @inheritdoc ISablierV2OpenEnded function deposit( uint256 streamId, - uint128 depositAmount + uint128 amount ) external override @@ -266,7 +266,7 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope notNull(streamId) { // Checks, Effects and Interactions: deposit on stream. - _deposit(streamId, depositAmount); + _deposit(streamId, amount); } /// @inheritdoc ISablierV2OpenEnded diff --git a/src/interfaces/ISablierV2OpenEnded.sol b/src/interfaces/ISablierV2OpenEnded.sol index 93090052..924e0830 100644 --- a/src/interfaces/ISablierV2OpenEnded.sol +++ b/src/interfaces/ISablierV2OpenEnded.sol @@ -151,6 +151,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// /// Notes: /// - The streamed assets, until the adjustment moment, must be transferred to the recipient. + /// - This function updates stream's `lastTimeUpdate` to the current block timestamp. /// /// Requiremenets: /// - Must not be delegate called. @@ -211,12 +212,12 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { returns (uint256 streamId); /// @notice Creates a new open-ended stream with the `block.timestamp` as the time reference - /// and with `depositAmount` balance. + /// and with `amount` balance. /// /// @dev Emits a {CreateOpenEndedStream}, {Transfer} and {DepositOpenEndedStream} events. /// /// Requirements: - /// - `depositAmount` must be greater than zero. + /// - `amount` must be greater than zero. /// - Refer to the requirements in {create}. /// /// @param recipient The address receiving the assets. @@ -243,7 +244,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// /// Requirements: /// - All requirements from {create} must be met for each stream. - /// - There must be an equal number of `recipients`, `senders`, `ratesPerSecond` and `amounts`. + /// - `recipients`, `senders`, `ratesPerSecond` and `amounts` arrays must be of equal length. /// /// @param recipients The addresses receiving the assets. /// @param senders The addresses streaming the assets, with the ability to adjust and cancel the stream. @@ -267,7 +268,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// @dev Emits multiple {CreateOpenEndedStream} events. /// /// Requirements: - /// - There must be an equal number of `recipients`, `senders` and `ratesPerSecond`. + /// - `recipients`, `senders` and `ratesPerSecond` arrays must be of equal length. /// - All requirements from {create} must be met for each stream. /// /// @param recipients The addresses receiving the assets. @@ -290,7 +291,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// Requirements: /// - Must not be delegate called. /// - `streamId` must not reference a null stream or a canceled stream. - /// - `depositAmount` must be greater than zero. + /// - `amount` must be greater than zero. /// /// @param streamId The ID of the stream to deposit on. /// @param amount The amount deposited in the stream, denoted in 18 decimals. @@ -302,7 +303,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// /// Requirements: /// - All requirements from {deposit} must be met for each stream. - /// - There must be an equal number of `streamIds` and `depositAmount`. + /// - `streamIds` and `amounts` arrays must be of equal length. /// /// @param streamIds The ids of the streams to deposit on. /// @param amounts The amount of assets to be deposited, denoted in 18 decimals. @@ -325,6 +326,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// @notice Restarts the stream with the provided rate per second. /// /// @dev Emits a {RestartOpenEndedStream} event. + /// - This function updates stream's `lastTimeUpdate` to the current block timestamp. /// /// Requirements: /// - Must not be delegate called. @@ -336,13 +338,13 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// @param ratePerSecond The amount of assets that is increasing by every second, denoted in 18 decimals. function restartStream(uint256 streamId, uint128 ratePerSecond) external; - /// @notice Restarts the stream with the provided rate per second, and deposits `depositAmount` in the stream + /// @notice Restarts the stream with the provided rate per second, and deposits `amount` in the stream /// balance. /// /// @dev Emits a {RestartOpenEndedStream}, {Transfer} and {DepositOpenEndedStream} event. /// /// Requirements: - /// - `depositAmount` must be greater than zero. + /// - `amount` must be greater than zero. /// - Refer to the requirements in {restartStream}. /// /// @param streamId The ID of the stream to restart. @@ -385,7 +387,7 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState { /// /// Requirements: /// - Must not be delegate called. - /// - There must be an equal number of `streamIds` and `times`. + /// - `streamIds` and `times` arrays must be of equal length. /// - Each stream ID in the array must not reference a null stream. /// - Each time in the array must be greater than the last time update and must not exceed `block.timestamp`. /// diff --git a/src/libraries/Errors.sol b/src/libraries/Errors.sol index 27cb9e5c..a6cdde02 100644 --- a/src/libraries/Errors.sol +++ b/src/libraries/Errors.sol @@ -23,12 +23,6 @@ library Errors { uint256 recipientsCount, uint256 sendersCount, uint256 ratesPerSecondCount ); - /// @notice Thrown when trying to set the rate per second of a stream to zero. - error SablierV2OpenEnded_RatePerSecondZero(); - - /// @notice Thrown when trying to change the rate per second with the same rate per second. - error SablierV2OpenEnded_RatePerSecondNotDifferent(uint128 ratePerSecond); - /// @notice Thrown when trying to create a OpenEnded stream with a zero deposit amount. error SablierV2OpenEnded_DepositAmountZero(); @@ -48,6 +42,12 @@ library Errors { /// @notice Thrown when trying to refund an amount greater than the refundable amount. error SablierV2OpenEnded_Overrefund(uint256 streamId, uint128 refundAmount, uint128 refundableAmount); + /// @notice Thrown when trying to change the rate per second with the same rate per second. + error SablierV2OpenEnded_RatePerSecondNotDifferent(uint128 ratePerSecond); + + /// @notice Thrown when trying to set the rate per second of a stream to zero. + error SablierV2OpenEnded_RatePerSecondZero(); + /// @notice Thrown when trying to create a OpenEnded stream with the recipient as the zero address. error SablierV2OpenEnded_RecipientZeroAddress(); @@ -69,19 +69,19 @@ library Errors { /// @notice Thrown when trying to withdraw to an address other than the recipient's. error SablierV2OpenEnded_WithdrawalAddressNotRecipient(uint256 streamId, address caller, address to); - /// @notice Thrown when trying to withdraw from multiple streams and the number of stream IDs does - /// not match the number of withdraw times. - error SablierV2OpenEnded_WithdrawMultipleArrayCountsNotEqual(uint256 streamIdCount, uint256 timesCount); - /// @notice Thrown when trying to withdraw assets with a withdrawal time in the future. error SablierV2OpenEnded_WithdrawalTimeInTheFuture(uint40 time, uint256 currentTime); /// @notice Thrown when trying to withdraw assets with a withdrawal time not greater than `lastTimeUpdate`. error SablierV2OpenEnded_WithdrawalTimeNotGreaterThanLastUpdate(uint40 time, uint40 lastUpdate); - /// @notice Thrown when trying to withdraw to the zero address. - error SablierV2OpenEnded_WithdrawToZeroAddress(); - /// @notice Thrown when trying to withdraw but the stream balance is zero. error SablierV2OpenEnded_WithdrawBalanceZero(uint256 streamId); + + /// @notice Thrown when trying to withdraw from multiple streams and the number of stream IDs does + /// not match the number of withdraw times. + error SablierV2OpenEnded_WithdrawMultipleArrayCountsNotEqual(uint256 streamIdCount, uint256 timesCount); + + /// @notice Thrown when trying to withdraw to the zero address. + error SablierV2OpenEnded_WithdrawToZeroAddress(); } diff --git a/test/integration/Integration.t.sol b/test/integration/Integration.t.sol index dd776fac..58e88a55 100644 --- a/test/integration/Integration.t.sol +++ b/test/integration/Integration.t.sol @@ -8,15 +8,28 @@ import { Errors } from "src/libraries/Errors.sol"; import { Base_Test } from "../Base.t.sol"; abstract contract Integration_Test is Base_Test { + uint128[] internal defaultDepositAmounts; + uint128[] internal defaultRatesPerSecond; + address[] internal defaultRecipients; + address[] internal defaultSenders; uint256 internal defaultStreamId; uint256[] internal defaultStreamIds; + uint256 internal nullStreamId = 420; function setUp() public virtual override { Base_Test.setUp(); defaultStreamId = createDefaultStream(); + defaultStreamIds.push(defaultStreamId); defaultStreamIds.push(createDefaultStream()); + + for (uint256 i; i < 2; ++i) { + defaultRecipients.push(users.recipient); + defaultSenders.push(users.sender); + defaultRatesPerSecond.push(RATE_PER_SECOND); + defaultDepositAmounts.push(DEPOSIT_AMOUNT); + } } /*////////////////////////////////////////////////////////////////////////// @@ -48,14 +61,12 @@ abstract contract Integration_Test is Base_Test { COMMON //////////////////////////////////////////////////////////////////////////*/ - function _test_RevertWhen_DelegateCall(bytes memory callData) internal { + function expectRevertDueToDelegateCall(bytes memory callData) internal { (bool success, bytes memory returnData) = address(openEnded).delegatecall(callData); assertFalse(success, "delegatecall success"); assertEq(returnData, abi.encodeWithSelector(Errors.DelegateCall.selector), "delegatecall return data"); } - uint256 internal nullStreamId = 420; - function expectRevertNull() internal { vm.expectRevert(abi.encodeWithSelector(Errors.SablierV2OpenEnded_Null.selector, nullStreamId)); } diff --git a/test/integration/adjust-rate-per-second/adjustRatePerSecond.t.sol b/test/integration/adjust-rate-per-second/adjustRatePerSecond.t.sol index 319811e1..9baaf77e 100644 --- a/test/integration/adjust-rate-per-second/adjustRatePerSecond.t.sol +++ b/test/integration/adjust-rate-per-second/adjustRatePerSecond.t.sol @@ -16,7 +16,7 @@ contract adjustRatePerSecond_Integration_Test is Integration_Test { function test_RevertWhen_DelegateCall() external { bytes memory callData = abi.encodeCall(ISablierV2OpenEnded.adjustRatePerSecond, (defaultStreamId, RATE_PER_SECOND)); - _test_RevertWhen_DelegateCall(callData); + expectRevertDueToDelegateCall(callData); } function test_RevertGiven_Null() external whenNotDelegateCalled { diff --git a/test/integration/cancel/cancel.t.sol b/test/integration/cancel/cancel.t.sol index b1398904..bc209e22 100644 --- a/test/integration/cancel/cancel.t.sol +++ b/test/integration/cancel/cancel.t.sol @@ -17,7 +17,7 @@ contract Cancel_Integration_Test is Integration_Test { function test_RevertWhen_DelegateCall() external { bytes memory callData = abi.encodeCall(ISablierV2OpenEnded.cancel, (defaultStreamId)); - _test_RevertWhen_DelegateCall(callData); + expectRevertDueToDelegateCall(callData); } function test_RevertGiven_Null() external whenNotDelegateCalled { diff --git a/test/integration/create-and-deposit-multiple/createAndDepositMultiple.t.sol b/test/integration/create-and-deposit-multiple/createAndDepositMultiple.t.sol index b3937ef3..215ec324 100644 --- a/test/integration/create-and-deposit-multiple/createAndDepositMultiple.t.sol +++ b/test/integration/create-and-deposit-multiple/createAndDepositMultiple.t.sol @@ -7,22 +7,8 @@ import { OpenEnded } from "src/types/DataTypes.sol"; import { Integration_Test } from "../Integration.t.sol"; contract CreateAndDepositMultiple_Integration_Test is Integration_Test { - address[] internal defaultRecipients; - address[] internal defaultSenders; - uint128[] internal defaultRatesPerSecond; - uint128[] internal defaultDepositAmounts; - function setUp() public override { Integration_Test.setUp(); - - defaultRecipients.push(users.recipient); - defaultRecipients.push(users.recipient); - defaultSenders.push(users.sender); - defaultSenders.push(users.sender); - defaultRatesPerSecond.push(RATE_PER_SECOND); - defaultRatesPerSecond.push(RATE_PER_SECOND); - defaultDepositAmounts.push(DEPOSIT_AMOUNT); - defaultDepositAmounts.push(DEPOSIT_AMOUNT); } function test_RevertWhen_DepositAmountsArrayIsNotEqual() external whenNotDelegateCalled whenArrayCountsNotEqual { diff --git a/test/integration/create-multiple/createMultiple.t.sol b/test/integration/create-multiple/createMultiple.t.sol index e1927335..154103ac 100644 --- a/test/integration/create-multiple/createMultiple.t.sol +++ b/test/integration/create-multiple/createMultiple.t.sol @@ -8,26 +8,15 @@ import { OpenEnded } from "src/types/DataTypes.sol"; import { Integration_Test } from "../Integration.t.sol"; contract CreateMultiple_Integration_Test is Integration_Test { - address[] internal defaultRecipients; - address[] internal defaultSenders; - uint128[] internal defaultRatesPerSecond; - function setUp() public override { Integration_Test.setUp(); - - defaultRecipients.push(users.recipient); - defaultRecipients.push(users.recipient); - defaultSenders.push(users.sender); - defaultSenders.push(users.sender); - defaultRatesPerSecond.push(RATE_PER_SECOND); - defaultRatesPerSecond.push(RATE_PER_SECOND); } function test_RevertWhen_DelegateCall() external { bytes memory callData = abi.encodeCall( ISablierV2OpenEnded.createMultiple, (defaultRecipients, defaultSenders, defaultRatesPerSecond, dai) ); - _test_RevertWhen_DelegateCall(callData); + expectRevertDueToDelegateCall(callData); } function test_RevertWhen_RecipientsCountNotEqual() external whenNotDelegateCalled whenArrayCountsNotEqual { @@ -58,7 +47,7 @@ contract CreateMultiple_Integration_Test is Integration_Test { openEnded.createMultiple(defaultRecipients, senders, defaultRatesPerSecond, dai); } - function test_RevertWhen_RatesPerSecondCountNotEqual() external whenNotDelegateCalled whenArrayCountsNotEqual { + function test_RevertWhen_RatePerSecondCountNotEqual() external whenNotDelegateCalled whenArrayCountsNotEqual { uint128[] memory ratesPerSecond = new uint128[](1); vm.expectRevert( diff --git a/test/integration/create/create.t.sol b/test/integration/create/create.t.sol index c79c68e5..ce6ba201 100644 --- a/test/integration/create/create.t.sol +++ b/test/integration/create/create.t.sol @@ -17,7 +17,7 @@ contract Create_Integration_Test is Integration_Test { function test_RevertWhen_DelegateCall() external { bytes memory callData = abi.encodeCall(ISablierV2OpenEnded.create, (users.sender, users.recipient, RATE_PER_SECOND, dai)); - _test_RevertWhen_DelegateCall(callData); + expectRevertDueToDelegateCall(callData); } function test_RevertWhen_SenderZeroAddress() external whenNotDelegateCalled { diff --git a/test/integration/deposit/deposit.t.sol b/test/integration/deposit/deposit.t.sol index 6578cd40..f8461346 100644 --- a/test/integration/deposit/deposit.t.sol +++ b/test/integration/deposit/deposit.t.sol @@ -15,7 +15,7 @@ contract Deposit_Integration_Test is Integration_Test { function test_RevertWhen_DelegateCall() external { bytes memory callData = abi.encodeCall(ISablierV2OpenEnded.deposit, (defaultStreamId, DEPOSIT_AMOUNT)); - _test_RevertWhen_DelegateCall(callData); + expectRevertDueToDelegateCall(callData); } function test_RevertGiven_Null() external whenNotDelegateCalled { diff --git a/test/integration/refund-from-stream/refundFromStream.t.sol b/test/integration/refund-from-stream/refundFromStream.t.sol index ef3c1b0b..f802696d 100644 --- a/test/integration/refund-from-stream/refundFromStream.t.sol +++ b/test/integration/refund-from-stream/refundFromStream.t.sol @@ -19,7 +19,7 @@ contract RefundFromStream_Integration_Test is Integration_Test { function test_RevertWhen_DelegateCall() external { bytes memory callData = abi.encodeCall(ISablierV2OpenEnded.refundFromStream, (defaultStreamId, REFUND_AMOUNT)); - _test_RevertWhen_DelegateCall(callData); + expectRevertDueToDelegateCall(callData); } function test_RevertGiven_Null() external whenNotDelegateCalled { diff --git a/test/integration/restart-stream/restartStream.t.sol b/test/integration/restart-stream/restartStream.t.sol index 8e61b22d..7c29e173 100644 --- a/test/integration/restart-stream/restartStream.t.sol +++ b/test/integration/restart-stream/restartStream.t.sol @@ -15,7 +15,7 @@ contract RestartStream_Integration_Test is Integration_Test { function test_RevertWhen_DelegateCall() external { bytes memory callData = abi.encodeCall(ISablierV2OpenEnded.restartStream, (defaultStreamId, RATE_PER_SECOND)); - _test_RevertWhen_DelegateCall(callData); + expectRevertDueToDelegateCall(callData); } function test_RevertGiven_Null() external whenNotDelegateCalled { diff --git a/test/integration/withdraw-multiple/withdrawMultiple.t.sol b/test/integration/withdraw-multiple/withdrawMultiple.t.sol index 957e2f24..d04148c9 100644 --- a/test/integration/withdraw-multiple/withdrawMultiple.t.sol +++ b/test/integration/withdraw-multiple/withdrawMultiple.t.sol @@ -19,7 +19,7 @@ contract WithdrawMultiple_Integration_Concrete_Test is Integration_Test { function test_RevertWhen_DelegateCall() external { bytes memory callData = abi.encodeCall(ISablierV2OpenEnded.withdrawMultiple, (defaultStreamIds, times)); - _test_RevertWhen_DelegateCall(callData); + expectRevertDueToDelegateCall(callData); } function test_RevertWhen_ArrayCountsNotEqual() external whenNotDelegateCalled { diff --git a/test/integration/withdraw/withdraw.t.sol b/test/integration/withdraw/withdraw.t.sol index 842d1d68..31ef035f 100644 --- a/test/integration/withdraw/withdraw.t.sol +++ b/test/integration/withdraw/withdraw.t.sol @@ -20,7 +20,7 @@ contract Withdraw_Integration_Test is Integration_Test { function test_RevertWhen_DelegateCall() external { bytes memory callData = abi.encodeCall(ISablierV2OpenEnded.withdraw, (defaultStreamId, users.recipient, WITHDRAW_TIME)); - _test_RevertWhen_DelegateCall(callData); + expectRevertDueToDelegateCall(callData); } function test_RevertGiven_Null() external whenNotDelegateCalled { From 043149f6a497d75cfe099f79a6a9ff535464da34 Mon Sep 17 00:00:00 2001 From: andreivladbrg Date: Tue, 7 May 2024 23:50:33 +0300 Subject: [PATCH 20/21] test: remove unneeded tree branches --- .../createAndDepositMultiple.tree | 3 +-- .../create-multiple/createMultiple.t.sol | 8 ------ .../create-multiple/createMultiple.tree | 25 ++++++++----------- 3 files changed, 12 insertions(+), 24 deletions(-) diff --git a/test/integration/create-and-deposit-multiple/createAndDepositMultiple.tree b/test/integration/create-and-deposit-multiple/createAndDepositMultiple.tree index 78df65f8..d4e32197 100644 --- a/test/integration/create-and-deposit-multiple/createAndDepositMultiple.tree +++ b/test/integration/create-and-deposit-multiple/createAndDepositMultiple.tree @@ -1,7 +1,6 @@ createAndDepositMultiple.t.sol ├── when array counts are not equal -│ └── when the deposit amounts array is not equal -│ └── it should revert +│ └── it should revert └── when array counts are equal ├── it should create multiple streams ├── it should update the stream balance diff --git a/test/integration/create-multiple/createMultiple.t.sol b/test/integration/create-multiple/createMultiple.t.sol index 154103ac..849fd8db 100644 --- a/test/integration/create-multiple/createMultiple.t.sol +++ b/test/integration/create-multiple/createMultiple.t.sol @@ -1,7 +1,6 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity >=0.8.22; -import { ISablierV2OpenEnded } from "src/interfaces/ISablierV2OpenEnded.sol"; import { Errors } from "src/libraries/Errors.sol"; import { OpenEnded } from "src/types/DataTypes.sol"; @@ -12,13 +11,6 @@ contract CreateMultiple_Integration_Test is Integration_Test { Integration_Test.setUp(); } - function test_RevertWhen_DelegateCall() external { - bytes memory callData = abi.encodeCall( - ISablierV2OpenEnded.createMultiple, (defaultRecipients, defaultSenders, defaultRatesPerSecond, dai) - ); - expectRevertDueToDelegateCall(callData); - } - function test_RevertWhen_RecipientsCountNotEqual() external whenNotDelegateCalled whenArrayCountsNotEqual { address[] memory recipients = new address[](1); diff --git a/test/integration/create-multiple/createMultiple.tree b/test/integration/create-multiple/createMultiple.tree index 4f48b4b3..51718611 100644 --- a/test/integration/create-multiple/createMultiple.tree +++ b/test/integration/create-multiple/createMultiple.tree @@ -1,15 +1,12 @@ createMultiple.t.sol -├── when delegate called -│ └── it should revert -└── when not delegate called - ├── when array counts are not equal - │ ├── when the recipients array is not equal - │ │ └── it should revert - │ ├── when the senders array is not equal - │ │ └── it should revert - │ └── when the rates per second array is not equal - │ └── it should revert - └── when array counts are equal - ├── it should create multiple streams - ├── it should bump the next stream id multiple times - └── it should emit multiple {CreateOpenEndedStream} events \ No newline at end of file +├── when array counts are not equal +│ ├── when the recipients array is not equal +│ │ └── it should revert +│ ├── when the senders array is not equal +│ │ └── it should revert +│ └── when the rates per second array is not equal +│ └── it should revert +└── when array counts are equal + ├── it should create multiple streams + ├── it should bump the next stream id multiple times + └── it should emit multiple {CreateOpenEndedStream} events \ No newline at end of file From 183181e4b6b73e7fbd795251e99a2b95c5686d72 Mon Sep 17 00:00:00 2001 From: andreivladbrg Date: Wed, 8 May 2024 15:29:58 +0300 Subject: [PATCH 21/21] test: add modifiers in test_CancelMultiple --- test/integration/cancel-multiple/cancelMultiple.t.sol | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/integration/cancel-multiple/cancelMultiple.t.sol b/test/integration/cancel-multiple/cancelMultiple.t.sol index 2484ddde..743ec0bd 100644 --- a/test/integration/cancel-multiple/cancelMultiple.t.sol +++ b/test/integration/cancel-multiple/cancelMultiple.t.sol @@ -108,7 +108,13 @@ contract CancelMultiple_Integration_Concrete_Test is Integration_Test { openEnded.cancelMultiple(defaultStreamIds); } - function test_CancelMultiple() external { + function test_CancelMultiple() + external + whenNotDelegateCalled + whenArrayCountNotZero + givenNotNull + whenCallerUnauthorized + { openEnded.cancelMultiple(defaultStreamIds); assertTrue(openEnded.isCanceled(defaultStreamIds[0]));