diff --git a/benchmark/Flow.Gas.t.sol b/benchmark/Flow.Gas.t.sol index 6e96a60a..62fdb90c 100644 --- a/benchmark/Flow.Gas.t.sol +++ b/benchmark/Flow.Gas.t.sol @@ -33,7 +33,7 @@ contract Flow_Gas_Test is Integration_Test { // Create the file if it doesn't exist, otherwise overwrite it. vm.writeFile({ path: benchmarkResultsFile, - data: string.concat("# Benchmarks using 6-decimal asset \n\n", "| Function | Gas Usage |\n", "| --- | --- |\n") + data: string.concat("# Benchmarks using 6-decimal token \n\n", "| Function | Gas Usage |\n", "| --- | --- |\n") }); } @@ -74,19 +74,18 @@ contract Flow_Gas_Test is Integration_Test { // {flow.void} computeGas("void", abi.encodeCall(flow.void, (streamId))); - // {flow.withdrawAt} (on an insolvent stream) on an incremented stream ID. + // {flow.withdraw} (on an insolvent stream) on an incremented stream ID. computeGas( - "withdrawAt (insolvent stream)", - abi.encodeCall(flow.withdrawAt, (++streamId, users.recipient, getBlockTimestamp())) + "withdraw (insolvent stream)", + abi.encodeCall(flow.withdraw, (++streamId, users.recipient, getBlockTimestamp())) ); // Deposit amount on an incremented stream ID to make stream solvent. deposit(++streamId, flow.uncoveredDebtOf(streamId) + DEPOSIT_AMOUNT_6D); - // {flow.withdrawAt} (on a solvent stream). + // {flow.withdraw} (on a solvent stream). computeGas( - "withdrawAt (solvent stream)", - abi.encodeCall(flow.withdrawAt, (streamId, users.recipient, getBlockTimestamp())) + "withdraw (solvent stream)", abi.encodeCall(flow.withdraw, (streamId, users.recipient, getBlockTimestamp())) ); // {flow.withdrawMax} on an incremented stream ID. diff --git a/benchmark/results/SablierFlow.md b/benchmark/results/SablierFlow.md index 671cea56..1c25e065 100644 --- a/benchmark/results/SablierFlow.md +++ b/benchmark/results/SablierFlow.md @@ -1,15 +1,15 @@ -# Benchmarks using 6-decimal asset +# Benchmarks using 6-decimal token -| Function | Gas Usage | -| ------------------------------- | --------- | -| `adjustRatePerSecond` | 43788 | -| `create` | 113491 | -| `deposit` | 24832 | -| `depositViaBroker` | 21529 | -| `pause` | 9089 | -| `refund` | 11136 | -| `restart` | 6513 | -| `void` | 7963 | -| `withdrawAt (insolvent stream)` | 53210 | -| `withdrawAt (solvent stream)` | 17692 | -| `withdrawMax` | 49012 | +| Function | Gas Usage | +| ----------------------------- | --------- | +| `adjustRatePerSecond` | 43788 | +| `create` | 113491 | +| `deposit` | 24832 | +| `depositViaBroker` | 21529 | +| `pause` | 9089 | +| `refund` | 11136 | +| `restart` | 6513 | +| `void` | 7963 | +| `withdraw (insolvent stream)` | 53210 | +| `withdraw (solvent stream)` | 17692 | +| `withdrawMax` | 49012 | diff --git a/diagrams.md b/diagrams.md index 25511252..761f3b12 100644 --- a/diagrams.md +++ b/diagrams.md @@ -65,7 +65,7 @@ stateDiagram-v2 1. The arrows point to the status on which the function can be called 2. The "update" comments refer only to the internal state -3. `st` is always updated to `block.timestamp`, expect for `withdrawAt` +3. `st` is always updated to `block.timestamp`, expect for `withdraw` 4. Red lines refers to the function that are doing an ERC-20 transfer ```mermaid diff --git a/foundry.toml b/foundry.toml index 04a6bce9..eb0d1e7c 100644 --- a/foundry.toml +++ b/foundry.toml @@ -20,7 +20,7 @@ [profile.default.fuzz] max_test_rejects = 1_000_000 # Number of times `vm.assume` can fail - runs = 5000 + runs = 20 [profile.default.invariant] call_override = false # Override unsafe external calls to perform reentrancy checks diff --git a/package.json b/package.json index 7841125a..85e7c6b7 100644 --- a/package.json +++ b/package.json @@ -30,11 +30,11 @@ "LICENSE-GPL.md" ], "keywords": [ - "asset-distribution", - "asset-streaming", + "token-distribution", + "token-streaming", "blockchain", "crypto", - "cryptoasset-streaming", + "cryptotoken-streaming", "ethereum", "forge", "foundry", diff --git a/src/SablierFlow.sol b/src/SablierFlow.sol index 1344dcc9..d967d079 100644 --- a/src/SablierFlow.sol +++ b/src/SablierFlow.sol @@ -50,7 +50,7 @@ contract SablierFlow is /// @inheritdoc ISablierFlow function coveredDebtOf(uint256 streamId) external view override notNull(streamId) returns (uint128 coveredDebt) { - coveredDebt = _coveredDebtOf({ streamId: streamId, time: uint40(block.timestamp) }); + coveredDebt = _coveredDebtOf(streamId); } /// @inheritdoc ISablierFlow @@ -69,7 +69,7 @@ contract SablierFlow is return 0; } - (uint128 ongoingDebt,) = _ongoingDebtOf(streamId, uint40(block.timestamp)); + (uint128 ongoingDebt,) = _ongoingDebtOf(streamId); uint128 snapshotDebt = _streams[streamId].snapshotDebt; uint128 totalDebt = snapshotDebt + ongoingDebt; @@ -98,7 +98,7 @@ contract SablierFlow is /// @inheritdoc ISablierFlow function ongoingDebtOf(uint256 streamId) external view override notNull(streamId) returns (uint128 ongoingDebt) { - (ongoingDebt,) = _ongoingDebtOf(streamId, uint40(block.timestamp)); + (ongoingDebt,) = _ongoingDebtOf(streamId); } /// @inheritdoc ISablierFlow @@ -109,7 +109,7 @@ contract SablierFlow is notNull(streamId) returns (uint128 refundableAmount) { - refundableAmount = _refundableAmountOf({ streamId: streamId, time: uint40(block.timestamp) }); + refundableAmount = _refundableAmountOf(streamId); } /// @inheritdoc ISablierFlow @@ -143,7 +143,7 @@ contract SablierFlow is /// @inheritdoc ISablierFlow function totalDebtOf(uint256 streamId) external view override notNull(streamId) returns (uint128 totalDebt) { - totalDebt = _totalDebtOf(streamId, uint40(block.timestamp)); + totalDebt = _totalDebtOf(streamId); } /// @inheritdoc ISablierFlow @@ -165,7 +165,7 @@ contract SablierFlow is notNull(streamId) returns (uint128 withdrawableAmount) { - withdrawableAmount = _coveredDebtOf(streamId, uint40(block.timestamp)); + withdrawableAmount = _coveredDebtOf(streamId); } /*////////////////////////////////////////////////////////////////////////// @@ -405,33 +405,19 @@ contract SablierFlow is } /// @inheritdoc ISablierFlow - function withdrawAt( + function withdraw( uint256 streamId, address to, - uint40 time + uint128 amount ) external override noDelegateCall notNull(streamId) updateMetadata(streamId) - returns (uint128 withdrawAmount) { - // Retrieve the snapshot time from storage. - uint40 snapshotTime = _streams[streamId].snapshotTime; - - // Check: the time reference is greater than `snapshotTime`. - if (time < snapshotTime) { - revert Errors.SablierFlow_WithdrawTimeLessThanSnapshotTime(streamId, snapshotTime, time); - } - - // Check: the withdrawal time is not in the future. - if (time > uint40(block.timestamp)) { - revert Errors.SablierFlow_WithdrawalTimeInTheFuture(streamId, time, block.timestamp); - } - // Checks, Effects, and Interactions: make the withdrawal. - withdrawAmount = _withdrawAt(streamId, to, time); + _withdraw(streamId, to, amount); } /// @inheritdoc ISablierFlow @@ -446,8 +432,8 @@ contract SablierFlow is updateMetadata(streamId) returns (uint128 withdrawAmount) { - // Checks, Effects, and Interactions: make the withdrawal. - withdrawAmount = _withdrawAt(streamId, to, uint40(block.timestamp)); + withdrawAmount = _coveredDebtOf(streamId); + _withdraw(streamId, to, withdrawAmount); } /*////////////////////////////////////////////////////////////////////////// @@ -455,7 +441,7 @@ contract SablierFlow is //////////////////////////////////////////////////////////////////////////*/ /// @dev Calculates the amount of covered debt by the stream balance. - function _coveredDebtOf(uint256 streamId, uint40 time) internal view returns (uint128) { + function _coveredDebtOf(uint256 streamId) internal view returns (uint128) { uint128 balance = _streams[streamId].balance; // If the balance is zero, return zero. @@ -463,7 +449,7 @@ contract SablierFlow is return 0; } - uint128 totalDebt = _totalDebtOf(streamId, time); + uint128 totalDebt = _totalDebtOf(streamId); // If the stream balance is less than or equal to the total debt, return the stream balance. if (balance < totalDebt) { @@ -473,33 +459,26 @@ contract SablierFlow is return totalDebt; } - /// @dev When token decimal is less than 18, the `_ongoingDebtOf` function can be non-injective with respect to its - /// input parameter `time` due to the denormalization. This results into a time range [t, t+δ] during which it + /// @dev When token decimal is less than 18, the `_ongoingDebtOf` function can be non-injective with respect + /// `block.timestamp` due to the denormalization. This results into a time range [t, t+δ] during which it /// would produce the same value. Thus, to minimise any loss to the users, this function returns a corrected time /// which is the lower bound, t in the range [t, t+δ]. The corrected time is the nearest Unix timestamp that /// produces the smallest remainder greater than the minimum transferable value. This corrected time is then stored /// as the snapshot time. /// /// @param streamId The ID of the stream. - /// @param time The time to calculate the ongoing debt. /// /// @return ongoingDebt The denormalized ongoing debt accrued since the last snapshot. Return 0 if the stream is - /// paused or `time` is less than the snapshot time. - /// @return correctedTime The corrected time derived from the denormalized ongoing debt. Return `time` if + /// paused or `block.timestamp` is less than the snapshot time. + /// @return correctedTime The corrected time derived from the denormalized ongoing debt. Return `block.timestamp` if /// the stream is paused or is less than the snapshot time. - function _ongoingDebtOf( - uint256 streamId, - uint40 time - ) - internal - view - returns (uint128 ongoingDebt, uint40 correctedTime) - { + function _ongoingDebtOf(uint256 streamId) internal view returns (uint128 ongoingDebt, uint40 correctedTime) { + uint40 blockTimestamp = uint40(block.timestamp); uint40 snapshotTime = _streams[streamId].snapshotTime; - // Check: if the stream is paused or the `time` is less than the `snapshotTime`. - if (_streams[streamId].isPaused || time <= snapshotTime) { - return (0, time); + // Check: if the stream is paused or the `block.timestamp` is less than the `snapshotTime`. + if (_streams[streamId].isPaused || blockTimestamp <= snapshotTime) { + return (0, blockTimestamp); } uint128 elapsedTime; @@ -507,7 +486,7 @@ contract SablierFlow is // Safe to use unchecked because subtraction cannot underflow. unchecked { // Calculate time elapsed since the last snapshot. - elapsedTime = time - snapshotTime; + elapsedTime = blockTimestamp - snapshotTime; } uint128 ratePerSecond = _streams[streamId].ratePerSecond.unwrap(); @@ -516,9 +495,9 @@ contract SablierFlow is // Calculate the ongoing debt accrued by multiplying the elapsed time by the rate per second. uint128 normalizedOngoingDebt = elapsedTime * ratePerSecond; - // If the token decimals are 18, return the normalized ongoing debt and the time. + // If the token decimals are 18, return the normalized ongoing debt and the `block.timestamp`. if (tokenDecimals == 18) { - return (normalizedOngoingDebt, time); + return (normalizedOngoingDebt, blockTimestamp); } // Safe to use unchecked because we use {SafeCast}. @@ -534,20 +513,25 @@ contract SablierFlow is correctedTime = uint40(snapshotTime + renormalizedOngoingDebt / ratePerSecond); } + // There is an edge case where the corrected time is equal to the snapshot time. + if (correctedTime == snapshotTime) { + ongoingDebt = 0; + } + return (ongoingDebt, correctedTime); } /// @dev Calculates the refundable amount. - function _refundableAmountOf(uint256 streamId, uint40 time) internal view returns (uint128) { - return _streams[streamId].balance - _coveredDebtOf(streamId, time); + function _refundableAmountOf(uint256 streamId) internal view returns (uint128) { + return _streams[streamId].balance - _coveredDebtOf(streamId); } /// @notice Calculates the total debt at the provided time. /// @dev The total debt is the sum of the snapshot debt and the ongoing debt. This value is independent of the /// stream's balance. - function _totalDebtOf(uint256 streamId, uint40 time) internal view returns (uint128) { + function _totalDebtOf(uint256 streamId) internal view returns (uint128) { // Calculate the ongoing debt streamed since last snapshot. - (uint128 ongoingDebt,) = _ongoingDebtOf(streamId, time); + (uint128 ongoingDebt,) = _ongoingDebtOf(streamId); // Calculate the total debt. return _streams[streamId].snapshotDebt + ongoingDebt; @@ -557,7 +541,7 @@ contract SablierFlow is function _uncoveredDebtOf(uint256 streamId) internal view returns (uint128) { uint128 balance = _streams[streamId].balance; - uint128 totalDebt = _totalDebtOf(streamId, uint40(block.timestamp)); + uint128 totalDebt = _totalDebtOf(streamId); if (balance < totalDebt) { return totalDebt - balance; @@ -585,7 +569,7 @@ contract SablierFlow is } // Calculate the ongoing debt and corrected time. - (uint128 ongoingDebt, uint40 correctedTime) = _ongoingDebtOf(streamId, uint40(block.timestamp)); + (uint128 ongoingDebt, uint40 correctedTime) = _ongoingDebtOf(streamId); // Effect: update the snapshot debt. _streams[streamId].snapshotDebt += ongoingDebt; @@ -704,7 +688,7 @@ contract SablierFlow is /// @dev See the documentation for the user-facing functions that call this internal function. function _pause(uint256 streamId) internal { // Effect: update the snapshot debt. - (uint128 ongoingDebt,) = _ongoingDebtOf(streamId, uint40(block.timestamp)); + (uint128 ongoingDebt,) = _ongoingDebtOf(streamId); _streams[streamId].snapshotDebt += ongoingDebt; // Effect: set the rate per second to zero. @@ -730,7 +714,7 @@ contract SablierFlow is } // Calculate the refundable amount. - uint128 refundableAmount = _refundableAmountOf({ streamId: streamId, time: uint40(block.timestamp) }); + uint128 refundableAmount = _refundableAmountOf(streamId); // Check: the refund amount is not greater than the refundable amount. if (amount > refundableAmount) { @@ -820,7 +804,12 @@ contract SablierFlow is } /// @dev See the documentation for the user-facing functions that call this internal function. - function _withdrawAt(uint256 streamId, address to, uint40 time) internal returns (uint128 withdrawAmount) { + function _withdraw(uint256 streamId, address to, uint128 amount) internal { + // Check: the withdraw amount is not zero. + if (amount == 0) { + revert Errors.SablierFlow_WithdrawAmountZero(streamId); + } + // Check: the withdrawal address is not zero. if (to == address(0)) { revert Errors.SablierFlow_WithdrawToZeroAddress(streamId); @@ -834,46 +823,62 @@ contract SablierFlow is uint128 balance = _streams[streamId].balance; - // Check: the stream balance is not zero. - if (balance == 0) { - revert Errors.SablierFlow_WithdrawNoFundsAvailable(streamId); - } + // Calculate the ongoing debt streamed since last snapshot. + (uint128 ongoingDebt, uint40 correctedTime) = _ongoingDebtOf(streamId); - (uint128 ongoingDebt, uint40 correctedTime) = _ongoingDebtOf(streamId, time); + // Calculate the total debt. uint128 totalDebt = _streams[streamId].snapshotDebt + ongoingDebt; - // If there is debt, the withdraw amount is the balance, and the snapshot debt is updated so that we - // don't lose track of the debt. - if (totalDebt > balance) { - withdrawAmount = balance; + uint128 withdrawableAmount; + + // If the stream balance is less than or equal to the total debt, the withdrawable amount is the balance. + if (balance < totalDebt) { + withdrawableAmount = balance; + } + // Otherwise, if stream balance is greater than the total debt, the withdrawable amount is the sum of ongoing + // debt and snapshot debt. + else { + withdrawableAmount = totalDebt; + } + // Check: the withdraw amount is not greater than the withdrawable amount. + if (amount > withdrawableAmount) { + revert Errors.SablierFlow_Overdraw(streamId, amount, withdrawableAmount); + } + + // Safe to use unchecked, the balance cannot be less than `amount` at this point. + unchecked { + // Effect: update the stream balance. + _streams[streamId].balance -= amount; + } + + // If there is debt, the snapshot debt is updated so that we don't lose track of the debt. + if (amount < totalDebt) { // Safe to use unchecked because subtraction cannot underflow. unchecked { // Effect: update the snapshot debt. - _streams[streamId].snapshotDebt = totalDebt - balance; + _streams[streamId].snapshotDebt = totalDebt - amount; } } - // Otherwise, recipient can withdraw the full amount, and the snapshot debt must be set to zero. - else { - withdrawAmount = totalDebt; - + // Otherwise, the snapshot debt must be set to zero. + else if (amount == totalDebt) { // Effect: set the snapshot debt to zero. _streams[streamId].snapshotDebt = 0; } + // Maybe we should add a safety check wether the snapshot debt is not the same as it previously was? + // Effect: update the stream time. _streams[streamId].snapshotTime = correctedTime; - // Load the variables in memory. + // Load the variable in memory. IERC20 token = _streams[streamId].token; UD60x18 protocolFee = protocolFee[token]; - uint128 totalWithdrawAmount = withdrawAmount; uint128 feeAmount; if (protocolFee > ZERO) { // Calculate the protocol fee amount and the net withdraw amount. - (feeAmount, withdrawAmount) = - Helpers.calculateAmountsFromFee({ totalAmount: totalWithdrawAmount, fee: protocolFee }); + (feeAmount, amount) = Helpers.calculateAmountsFromFee({ totalAmount: amount, fee: protocolFee }); // Safe to use unchecked because addition cannot overflow. unchecked { @@ -882,11 +887,8 @@ contract SablierFlow is } } - // Effect: update the stream balance. - _streams[streamId].balance -= totalWithdrawAmount; - // Interaction: perform the ERC-20 transfer. - token.safeTransfer({ to: to, value: withdrawAmount }); + token.safeTransfer({ to: to, value: amount }); // Log the withdrawal. emit ISablierFlow.WithdrawFromFlowStream({ @@ -895,7 +897,7 @@ contract SablierFlow is token: token, caller: msg.sender, protocolFeeAmount: feeAmount, - withdrawAmount: withdrawAmount, + withdrawAmount: amount, snapshotTime: correctedTime }); } diff --git a/src/interfaces/ISablierFlow.sol b/src/interfaces/ISablierFlow.sol index cdf3268c..99c3d235 100644 --- a/src/interfaces/ISablierFlow.sol +++ b/src/interfaces/ISablierFlow.sol @@ -440,26 +440,24 @@ interface ISablierFlow is /// - `streamId` must not reference a null stream. /// - `to` must not be the zero address. /// - `to` must be the recipient if `msg.sender` is not the stream's recipient. - /// - `time` must be greater than the stream's `snapshotTime` and must not be in the future. + /// - `amount` must be greater than zero and must not exceed the withdrawable amount. /// - The stream balance must be greater than zero. /// /// @param streamId The ID of the stream to withdraw from. /// @param to The address receiving the withdrawn tokens. - /// @param time The Unix timestamp up to which ongoing debt is calculated from the snapshot time. - /// - /// @return withdrawAmount The amount transferred to the recipient, denoted in token's decimals. - function withdrawAt(uint256 streamId, address to, uint40 time) external returns (uint128 withdrawAmount); + /// @param amount The amount to withdraw, denoted in units of the token's decimals. + function withdraw(uint256 streamId, address to, uint128 amount) external; /// @notice Withdraws the entire withdrawable amount from the stream to the provided address `to`. /// /// @dev Emits a {Transfer} and {WithdrawFromFlowStream} event. /// /// Notes: - /// - It uses the value returned by {withdrawAt} with the current block timestamp. - /// - Refer to the notes in {withdrawAt}. + /// - It uses the value returned by {withdraw} with the current block timestamp. + /// - Refer to the notes in {withdraw}. /// /// Requirements: - /// - Refer to the requirements in {withdrawAt}. + /// - Refer to the requirements in {withdraw}. /// /// @param streamId The ID of the stream to withdraw from. /// @param to The address receiving the withdrawn tokens. diff --git a/src/libraries/Errors.sol b/src/libraries/Errors.sol index 4470901d..ec48243c 100644 --- a/src/libraries/Errors.sol +++ b/src/libraries/Errors.sol @@ -42,6 +42,9 @@ library Errors { /// @notice Thrown when the ID references a null stream. error SablierFlow_Null(uint256 streamId); + /// @notice Thrown when trying to withdraw an amount greater than the withdrawable amount. + error SablierFlow_Overdraw(uint256 streamId, uint128 amount, uint128 withdrawableAmount); + /// @notice Thrown when trying to change the rate per second with the same rate per second. error SablierFlow_RatePerSecondNotDifferent(uint256 streamId, UD21x18 ratePerSecond); @@ -75,15 +78,8 @@ library Errors { /// @notice Thrown when trying to withdraw to an address other than the recipient's. error SablierFlow_WithdrawalAddressNotRecipient(uint256 streamId, address caller, address to); - /// @notice Thrown when trying to withdraw tokens with a withdrawal time in the future. - error SablierFlow_WithdrawalTimeInTheFuture(uint256 streamId, uint40 time, uint256 currentTime); - - /// @notice Thrown when trying to withdraw but the stream no funds available. - error SablierFlow_WithdrawNoFundsAvailable(uint256 streamId); - - /// @notice Thrown when trying to withdraw tokens with a withdrawal time not greater than or equal to - /// `snapshotTime`. - error SablierFlow_WithdrawTimeLessThanSnapshotTime(uint256 streamId, uint40 snapshotTime, uint40 withdrawTime); + /// @notice Thrown when trying to withdraw zero tokens from a stream. + error SablierFlow_WithdrawAmountZero(uint256 streamId); /// @notice Thrown when trying to withdraw to the zero address. error SablierFlow_WithdrawToZeroAddress(uint256 streamId); diff --git a/src/libraries/Helpers.sol b/src/libraries/Helpers.sol index 5c252dc9..3b916b83 100644 --- a/src/libraries/Helpers.sol +++ b/src/libraries/Helpers.sol @@ -21,6 +21,9 @@ library Helpers { // Calculate the fee amount based on the fee percentage. feeAmount = ud(totalAmount).mul(fee).intoUint128(); + // Assert that the total amount is strictly greater than the fee amount. + assert(totalAmount > feeAmount); + // Calculate the net amount after subtracting the fee from the total amount. netAmount = totalAmount - feeAmount; } diff --git a/test/fork/Flow.t.sol b/test/fork/Flow.t.sol index 9fcdcdc2..fcbf4a87 100644 --- a/test/fork/Flow.t.sol +++ b/test/fork/Flow.t.sol @@ -23,7 +23,7 @@ contract Flow_Fork_Test is Fork_Test { refund, restart, void, - withdrawAt + withdraw } /// @dev A struct to hold the fuzzed parameters to be used during fork tests. @@ -37,7 +37,7 @@ contract Flow_Fork_Test is Fork_Test { // Amounts uint128 depositAmount; uint128 refundAmount; - uint40 withdrawAtTime; + uint128 withdrawAmount; } /// @dev A struct to hold the actual and expected values, this prevents stack overflow. @@ -145,7 +145,7 @@ contract Flow_Fork_Test is Fork_Test { params.ratePerSecond, params.depositAmount, params.refundAmount, - params.withdrawAtTime + params.withdrawAmount ); } } @@ -156,14 +156,14 @@ contract Flow_Fork_Test is Fork_Test { /// @param ratePerSecond The rate per second. /// @param depositAmount The deposit amount. /// @param refundAmount The refund amount. - /// @param withdrawAtTime The time to withdraw at. + /// @param withdrawAmount The withdraw amount. function _executeFunc( FlowFunc flowFunc, uint256 streamId, UD21x18 ratePerSecond, uint128 depositAmount, uint128 refundAmount, - uint40 withdrawAtTime + uint128 withdrawAmount ) private { @@ -179,8 +179,8 @@ contract Flow_Fork_Test is Fork_Test { _test_Restart(streamId, ratePerSecond); } else if (flowFunc == FlowFunc.void) { _test_Void(streamId); - } else if (flowFunc == FlowFunc.withdrawAt) { - _test_WithdrawAt(streamId, withdrawAtTime); + } else if (flowFunc == FlowFunc.withdraw) { + _test_Withdraw(streamId, withdrawAmount); } } @@ -569,41 +569,34 @@ contract Flow_Fork_Test is Fork_Test { } /*////////////////////////////////////////////////////////////////////////// - WITHDRAW-AT + WITHDRAW //////////////////////////////////////////////////////////////////////////*/ - function _test_WithdrawAt(uint256 streamId, uint40 withdrawTime) private { - withdrawTime = boundUint40( - uint40(uint256(keccak256(abi.encodePacked(withdrawTime, streamId)))), - flow.getSnapshotTime(streamId), - getBlockTimestamp() - ); - + function _test_Withdraw(uint256 streamId, uint128 withdrawAmount) private { uint8 tokenDecimals = flow.getTokenDecimals(streamId); - uint128 ratePerSecond = flow.getRatePerSecond(streamId).unwrap(); uint128 streamBalance = flow.getBalance(streamId); if (streamBalance == 0) { depositOnStream(streamId, getDefaultDepositAmount(tokenDecimals)); streamBalance = flow.getBalance(streamId); } - uint128 ongoingDebt = - getDenormalizedAmount(ratePerSecond * (withdrawTime - flow.getSnapshotTime(streamId)), tokenDecimals); - uint128 totalDebt = flow.getSnapshotDebt(streamId) + ongoingDebt; - uint128 withdrawAmount = streamBalance < totalDebt ? streamBalance : totalDebt; - - (, address caller,) = vm.readCallers(); - address recipient = flow.getRecipient(streamId); + withdrawAmount = boundUint128( + uint128(uint256(keccak256(abi.encodePacked(withdrawAmount, streamId)))), + 1, + flow.withdrawableAmountOf(streamId) + ); uint256 initialTokenBalance = token.balanceOf(address(flow)); + uint128 totalDebt = flow.totalDebtOf(streamId); // Compute the snapshot time that will be stored post withdraw. - if (!flow.isPaused(streamId) && withdrawTime > flow.getSnapshotTime(streamId)) { - vars.expectedSnapshotTime = - uint40(getNormalizedAmount(ongoingDebt, tokenDecimals) / ratePerSecond + flow.getSnapshotTime(streamId)); - } else { - vars.expectedSnapshotTime = withdrawTime; - } + vars.expectedSnapshotTime = uint40( + getNormalizedAmount(flow.ongoingDebtOf(streamId), tokenDecimals) / flow.getRatePerSecond(streamId).unwrap() + + flow.getSnapshotTime(streamId) + ); + + (, address caller,) = vm.readCallers(); + address recipient = flow.getRecipient(streamId); // Expect the relevant events to be emitted. vm.expectEmit({ emitter: address(token) }); @@ -624,28 +617,25 @@ contract Flow_Fork_Test is Fork_Test { emit MetadataUpdate({ _tokenId: streamId }); // Withdraw the tokens. - flow.withdrawAt(streamId, recipient, withdrawTime); + flow.withdraw(streamId, recipient, withdrawAmount); // It should update snapshot time. vars.actualSnapshotTime = flow.getSnapshotTime(streamId); - assertEq(vars.actualSnapshotTime, vars.expectedSnapshotTime, "WithdrawAt: snapshot time"); + assertEq(vars.actualSnapshotTime, vars.expectedSnapshotTime, "Withdraw: snapshot time"); // It should decrease the total debt by withdrawn amount. - vars.actualTotalDebt = flow.getSnapshotDebt(streamId) - + getDenormalizedAmount( - ratePerSecond * (vars.expectedSnapshotTime - flow.getSnapshotTime(streamId)), tokenDecimals - ); + vars.actualTotalDebt = flow.totalDebtOf(streamId); vars.expectedTotalDebt = totalDebt - withdrawAmount; - assertEq(vars.actualTotalDebt, vars.expectedTotalDebt, "WithdrawAt: total debt"); + assertEq(vars.actualTotalDebt, vars.expectedTotalDebt, "Withdraw: total debt"); // It should reduce the stream balance by the withdrawn amount. vars.actualStreamBalance = flow.getBalance(streamId); vars.expectedStreamBalance = streamBalance - withdrawAmount; - assertEq(vars.actualStreamBalance, vars.expectedStreamBalance, "WithdrawAt: stream balance"); + assertEq(vars.actualStreamBalance, vars.expectedStreamBalance, "Withdraw: stream balance"); // It should reduce the token balance of stream. vars.actualTokenBalance = token.balanceOf(address(flow)); vars.expectedTokenBalance = initialTokenBalance - withdrawAmount; - assertEq(vars.actualTokenBalance, vars.expectedTokenBalance, "WithdrawAt: token balance"); + assertEq(vars.actualTokenBalance, vars.expectedTokenBalance, "Withdraw: token balance"); } } diff --git a/test/integration/concrete/batch/batch.t.sol b/test/integration/concrete/batch/batch.t.sol index d7c21081..d3ab250c 100644 --- a/test/integration/concrete/batch/batch.t.sol +++ b/test/integration/concrete/batch/batch.t.sol @@ -333,8 +333,8 @@ contract Batch_Integration_Concrete_Test is Integration_Test { // The calls declared as bytes bytes[] memory calls = new bytes[](2); - calls[0] = abi.encodeCall(flow.withdrawAt, (defaultStreamIds[0], users.recipient, WITHDRAW_TIME)); - calls[1] = abi.encodeCall(flow.withdrawAt, (defaultStreamIds[1], users.recipient, WITHDRAW_TIME)); + calls[0] = abi.encodeCall(flow.withdraw, (defaultStreamIds[0], users.recipient, WITHDRAW_TIME)); + calls[1] = abi.encodeCall(flow.withdraw, (defaultStreamIds[1], users.recipient, WITHDRAW_TIME)); // It should emit 2 {Transfer}, 2 {WithdrawFromFlowStream} and 2 {MetadataUpdated} events. diff --git a/test/integration/concrete/collect-protocol-revenue/collectProtocolRevenue.t.sol b/test/integration/concrete/collect-protocol-revenue/collectProtocolRevenue.t.sol index 9797bcd4..e8c2f669 100644 --- a/test/integration/concrete/collect-protocol-revenue/collectProtocolRevenue.t.sol +++ b/test/integration/concrete/collect-protocol-revenue/collectProtocolRevenue.t.sol @@ -37,7 +37,7 @@ contract CollectProtocolRevenue_Integration_Concrete_Test is Integration_Test { function test_GivenProtocolRevenueNotZero() external whenCallerAdmin { // Withdraw to generate protocol revenue. - flow.withdrawAt({ streamId: streamIdWithProtocolFee, to: users.recipient, time: WITHDRAW_TIME }); + flow.withdraw({ streamId: streamIdWithProtocolFee, to: users.recipient, amount: WITHDRAW_AMOUNT_6D }); // It should transfer protocol revenue to provided address. expectCallToTransfer({ token: tokenWithProtocolFee, to: users.admin, amount: PROTOCOL_FEE_AMOUNT_6D }); diff --git a/test/integration/concrete/deposit-via-broker/depositViaBroker.t.sol b/test/integration/concrete/deposit-via-broker/depositViaBroker.t.sol index 327f709a..80c4ec17 100644 --- a/test/integration/concrete/deposit-via-broker/depositViaBroker.t.sol +++ b/test/integration/concrete/deposit-via-broker/depositViaBroker.t.sol @@ -55,7 +55,7 @@ contract DepositViaBroker_Integration_Concrete_Test is Integration_Test { whenBrokerFeeNotGreaterThanMaxFee whenBrokerAddressNotZero { - vm.expectRevert(abi.encodeWithSelector(Errors.SablierFlow_DepositAmountZero.selector, defaultStreamId)); + vm.expectRevert(); flow.depositViaBroker(defaultStreamId, 0, defaultBroker); } diff --git a/test/integration/concrete/withdraw-at/withdrawAt.tree b/test/integration/concrete/withdraw-at/withdrawAt.tree deleted file mode 100644 index 10e9bb33..00000000 --- a/test/integration/concrete/withdraw-at/withdrawAt.tree +++ /dev/null @@ -1,42 +0,0 @@ -WithdrawAt_Integration_Concrete_Test -├── when delegate call -│ └── it should revert -└── when no delegate call - ├── given null - │ └── it should revert - └── given not null - ├── when time less than snapshot time - │ └── it should revert - ├── when time greater than current time - │ └── it should revert - └── when time between snapshot time and current time - ├── when withdrawal address zero - │ └── it should revert - └── when withdrawal address not zero - ├── when withdrawal address not owner - │ ├── when caller sender - │ │ └── it should revert - │ ├── when caller unknown - │ │ └── it should revert - │ └── when caller recipient - │ └── it should withdraw - └── when withdrawal address owner - ├── given balance zero - │ └── it should revert - └── given balance not zero - ├── when total debt exceeds balance - │ └── it should withdraw the balance - └── when total debt not exceed balance - ├── given protocol fee not zero - │ ├── it should update the protocol revenue - │ └── it should withdraw the net amount - └── given protocol fee zero - ├── given token has 18 decimals - │ └── it should make the withdrawal - └── given token not have 18 decimals - ├── it should withdraw the withdrawable amount - ├── it should decrease the total debt by withdrawn value - ├── it should reduce the stream balance by the withdrawn amount - ├── it should update snapshot time - ├── it should emit 1 {Transfer}, 1 {WithdrawFromFlowStream} and 1 {MetadataUpdated} events - └── it should return the withdrawn amount \ No newline at end of file diff --git a/test/integration/concrete/withdraw-max/withdrawMax.t.sol b/test/integration/concrete/withdraw-max/withdrawMax.t.sol index 2dc10c58..a7f8469c 100644 --- a/test/integration/concrete/withdraw-max/withdrawMax.t.sol +++ b/test/integration/concrete/withdraw-max/withdrawMax.t.sol @@ -60,7 +60,7 @@ contract WithdrawMax_Integration_Concrete_Test is Integration_Test { // It should perform the ERC-20 transfer. expectCallToTransfer({ token: usdc, to: users.recipient, amount: withdrawAmount }); - uint128 actualWithdrawAmount = flow.withdrawMax(defaultStreamId, users.recipient); + flow.withdrawMax(defaultStreamId, users.recipient); // It should update the stream balance. uint128 actualStreamBalance = flow.getBalance(defaultStreamId); @@ -74,8 +74,5 @@ contract WithdrawMax_Integration_Concrete_Test is Integration_Test { // It should update snapshot time. uint128 actualSnapshotTime = flow.getSnapshotTime(defaultStreamId); assertEq(actualSnapshotTime, getBlockTimestamp(), "snapshot time"); - - // Assert that the withdraw amounts match. - assertEq(actualWithdrawAmount, withdrawAmount); } } diff --git a/test/integration/concrete/withdraw-at/withdrawAt.t.sol b/test/integration/concrete/withdraw/withdraw.sol similarity index 55% rename from test/integration/concrete/withdraw-at/withdrawAt.t.sol rename to test/integration/concrete/withdraw/withdraw.sol index 8a847193..eebbf9b0 100644 --- a/test/integration/concrete/withdraw-at/withdrawAt.t.sol +++ b/test/integration/concrete/withdraw/withdraw.sol @@ -2,78 +2,46 @@ pragma solidity >=0.8.22; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; -import { ZERO } from "@prb/math/src/UD60x18.sol"; import { Errors } from "src/libraries/Errors.sol"; import { Integration_Test } from "../../Integration.t.sol"; -contract WithdrawAt_Integration_Concrete_Test is Integration_Test { +contract Withdraw_Integration_Concrete_Test is Integration_Test { function setUp() public override { Integration_Test.setUp(); + depositToDefaultStream(); + // Set recipient as the caller for this test. resetPrank({ msgSender: users.recipient }); } function test_RevertWhen_DelegateCall() external { - bytes memory callData = abi.encodeCall(flow.withdrawAt, (defaultStreamId, users.recipient, WITHDRAW_TIME)); + bytes memory callData = abi.encodeCall(flow.withdraw, (defaultStreamId, users.recipient, WITHDRAW_TIME)); expectRevert_DelegateCall(callData); } function test_RevertGiven_Null() external whenNoDelegateCall { - bytes memory callData = abi.encodeCall(flow.withdrawAt, (nullStreamId, users.recipient, WITHDRAW_TIME)); + bytes memory callData = abi.encodeCall(flow.withdraw, (nullStreamId, users.recipient, WITHDRAW_TIME)); expectRevert_Null(callData); } - function test_RevertWhen_TimeLessThanSnapshotTime() external whenNoDelegateCall givenNotNull { - // Update the snapshot time and warp the current block timestamp to it. - updateSnapshotTimeAndWarp(defaultStreamId); - - uint40 snapshotTime = flow.getSnapshotTime(defaultStreamId); - - vm.expectRevert( - abi.encodeWithSelector( - Errors.SablierFlow_WithdrawTimeLessThanSnapshotTime.selector, - defaultStreamId, - snapshotTime, - WITHDRAW_TIME - ) - ); - flow.withdrawAt({ streamId: defaultStreamId, to: users.recipient, time: WITHDRAW_TIME }); - } - - function test_RevertWhen_TimeGreaterThanCurrentTime() external whenNoDelegateCall givenNotNull { - vm.expectRevert( - abi.encodeWithSelector( - Errors.SablierFlow_WithdrawalTimeInTheFuture.selector, - defaultStreamId, - getBlockTimestamp() + 1, - getBlockTimestamp() - ) - ); - flow.withdrawAt({ streamId: defaultStreamId, to: users.recipient, time: getBlockTimestamp() + 1 }); + function test_RevertWhen_AmountZero() external whenNoDelegateCall givenNotNull { + vm.expectRevert(abi.encodeWithSelector(Errors.SablierFlow_WithdrawAmountZero.selector, defaultStreamId)); + flow.withdraw({ streamId: defaultStreamId, to: users.recipient, amount: 0 }); } - modifier whenTimeBetweenSnapshotTimeAndCurrentTime() { - _; - } - - function test_RevertWhen_WithdrawalAddressZero() - external - whenNoDelegateCall - givenNotNull - whenTimeBetweenSnapshotTimeAndCurrentTime - { + function test_RevertWhen_WithdrawalAddressZero() external whenNoDelegateCall givenNotNull whenAmountNotZero { vm.expectRevert(abi.encodeWithSelector(Errors.SablierFlow_WithdrawToZeroAddress.selector, defaultStreamId)); - flow.withdrawAt({ streamId: defaultStreamId, to: address(0), time: WITHDRAW_TIME }); + flow.withdraw({ streamId: defaultStreamId, to: address(0), amount: WITHDRAW_AMOUNT_6D }); } function test_RevertWhen_CallerSender() external whenNoDelegateCall givenNotNull - whenTimeBetweenSnapshotTimeAndCurrentTime + whenAmountNotZero whenWithdrawalAddressNotZero whenWithdrawalAddressNotOwner { @@ -84,14 +52,14 @@ contract WithdrawAt_Integration_Concrete_Test is Integration_Test { Errors.SablierFlow_WithdrawalAddressNotRecipient.selector, defaultStreamId, users.sender, users.sender ) ); - flow.withdrawAt({ streamId: defaultStreamId, to: users.sender, time: WITHDRAW_TIME }); + flow.withdraw({ streamId: defaultStreamId, to: users.sender, amount: WITHDRAW_AMOUNT_6D }); } function test_RevertWhen_CallerUnknown() external whenNoDelegateCall givenNotNull - whenTimeBetweenSnapshotTimeAndCurrentTime + whenAmountNotZero whenWithdrawalAddressNotZero whenWithdrawalAddressNotOwner { @@ -102,90 +70,95 @@ contract WithdrawAt_Integration_Concrete_Test is Integration_Test { Errors.SablierFlow_WithdrawalAddressNotRecipient.selector, defaultStreamId, users.eve, users.eve ) ); - flow.withdrawAt({ streamId: defaultStreamId, to: users.eve, time: WITHDRAW_TIME }); + flow.withdraw({ streamId: defaultStreamId, to: users.eve, amount: WITHDRAW_AMOUNT_6D }); } function test_WhenCallerRecipient() external whenNoDelegateCall givenNotNull - whenTimeBetweenSnapshotTimeAndCurrentTime + whenAmountNotZero whenWithdrawalAddressNotZero whenWithdrawalAddressNotOwner - givenBalanceNotZero { // It should withdraw. _test_Withdraw({ streamId: defaultStreamId, to: users.eve, depositAmount: DEPOSIT_AMOUNT_6D, + feeAmount: 0, withdrawAmount: WITHDRAW_AMOUNT_6D }); } - function test_RevertGiven_BalanceZero() + function test_RevertGiven_StreamHasUncoveredDebt() external whenNoDelegateCall givenNotNull - whenTimeBetweenSnapshotTimeAndCurrentTime + whenAmountNotZero whenWithdrawalAddressNotZero - whenWithdrawalAddressIsOwner + whenWithdrawalAddressOwner + whenAmountOverdraws { - // Go back to the starting point. - vm.warp({ newTimestamp: MAY_1_2024 }); + // Warp to the moment when stream accumulates uncovered debt. + vm.warp({ newTimestamp: flow.depletionTimeOf(defaultStreamId) }); - // Create a new stream with a deposit of 0. - uint256 streamId = createDefaultStream(); - - vm.warp({ newTimestamp: WARP_ONE_MONTH }); - - vm.expectRevert(abi.encodeWithSelector(Errors.SablierFlow_WithdrawNoFundsAvailable.selector, streamId)); - flow.withdrawAt({ streamId: streamId, to: users.recipient, time: WITHDRAW_TIME }); + uint128 overdrawAmount = flow.getBalance(defaultStreamId) + 1; + vm.expectRevert( + abi.encodeWithSelector( + Errors.SablierFlow_Overdraw.selector, defaultStreamId, overdrawAmount, overdrawAmount - 1 + ) + ); + flow.withdraw({ streamId: defaultStreamId, to: users.recipient, amount: overdrawAmount }); } - function test_WhenTotalDebtExceedsBalance() + function test_RevertGiven_StreamHasNoUncoveredDebt() external whenNoDelegateCall givenNotNull - whenTimeBetweenSnapshotTimeAndCurrentTime + whenAmountNotZero whenWithdrawalAddressNotZero - whenWithdrawalAddressIsOwner - givenBalanceNotZero + whenWithdrawalAddressOwner + whenAmountOverdraws { - // Go back to the starting point. - vm.warp({ newTimestamp: MAY_1_2024 }); - - resetPrank({ msgSender: users.sender }); - - uint128 chickenfeed = 50e6; - - // Create a new stream with a much smaller deposit. - uint256 streamId = createDefaultStream(); - deposit(streamId, chickenfeed); - - // Simulate the one month of streaming. - vm.warp({ newTimestamp: WARP_ONE_MONTH }); - - // Make recipient the caller for subsequent tests. - resetPrank({ msgSender: users.recipient }); - - // It should withdraw the balance. - _test_Withdraw({ streamId: streamId, to: users.recipient, depositAmount: chickenfeed, withdrawAmount: 50e6 }); + uint128 overdrawAmount = flow.withdrawableAmountOf(defaultStreamId) + 1; + vm.expectRevert( + abi.encodeWithSelector( + Errors.SablierFlow_Overdraw.selector, defaultStreamId, overdrawAmount, overdrawAmount - 1 + ) + ); + flow.withdraw({ streamId: defaultStreamId, to: users.recipient, amount: overdrawAmount }); } - modifier whenTotalDebtNotExceedBalance() { - _; + function test_WhenAmountNotEqualTotalDebt() + external + whenNoDelegateCall + givenNotNull + whenAmountNotZero + whenWithdrawalAddressNotZero + whenWithdrawalAddressOwner + whenAmountNotOverdraw + { + // It should update snapshot debt + // It should make the withdrawal + _test_Withdraw({ + streamId: defaultStreamId, + to: users.recipient, + depositAmount: DEPOSIT_AMOUNT_6D, + feeAmount: 0, + withdrawAmount: flow.totalDebtOf(defaultStreamId) - 1 + }); } function test_GivenProtocolFeeNotZero() external whenNoDelegateCall givenNotNull - whenTimeBetweenSnapshotTimeAndCurrentTime + whenAmountNotZero whenWithdrawalAddressNotZero - whenWithdrawalAddressIsOwner - givenBalanceNotZero - whenTotalDebtNotExceedBalance + whenWithdrawalAddressOwner + whenAmountNotOverdraw + whenAmountEqualTotalDebt { // Go back to the starting point. vm.warp({ newTimestamp: MAY_1_2024 }); @@ -199,14 +172,15 @@ contract WithdrawAt_Integration_Concrete_Test is Integration_Test { // Simulate the one month of streaming. vm.warp({ newTimestamp: WARP_ONE_MONTH }); - // Make recipient the caller for subsequent tests. + // Make recipient the caller test. resetPrank({ msgSender: users.recipient }); - // It should withdraw the total debt. + // It should make the withdrawal. _test_Withdraw({ streamId: streamId, to: users.recipient, depositAmount: DEPOSIT_AMOUNT_6D, + feeAmount: PROTOCOL_FEE_AMOUNT_6D, withdrawAmount: WITHDRAW_AMOUNT_6D }); } @@ -215,25 +189,40 @@ contract WithdrawAt_Integration_Concrete_Test is Integration_Test { external whenNoDelegateCall givenNotNull - whenTimeBetweenSnapshotTimeAndCurrentTime + whenAmountNotZero whenWithdrawalAddressNotZero whenWithdrawalAddressNotOwner - givenBalanceNotZero - whenTotalDebtNotExceedBalance + whenAmountEqualTotalDebt givenProtocolFeeZero { - // it should make the withdrawal + // Go back to the starting point. + vm.warp({ newTimestamp: MAY_1_2024 }); + + // Create the stream and make a deposit. + uint256 streamId = createDefaultStream(dai); + deposit(streamId, DEPOSIT_AMOUNT_18D); + + // Simulate the one month of streaming. + vm.warp({ newTimestamp: WARP_ONE_MONTH }); + + // It should withdraw the total debt. + _test_Withdraw({ + streamId: streamId, + to: users.recipient, + depositAmount: DEPOSIT_AMOUNT_18D, + feeAmount: 0, + withdrawAmount: WITHDRAW_AMOUNT_18D + }); } function test_GivenTokenNotHave18Decimals() external whenNoDelegateCall givenNotNull - whenTimeBetweenSnapshotTimeAndCurrentTime + whenAmountNotZero whenWithdrawalAddressNotZero - whenWithdrawalAddressIsOwner - givenBalanceNotZero - whenTotalDebtNotExceedBalance + whenWithdrawalAddressOwner + whenAmountEqualTotalDebt givenProtocolFeeZero { // It should withdraw the total debt. @@ -241,25 +230,28 @@ contract WithdrawAt_Integration_Concrete_Test is Integration_Test { streamId: defaultStreamId, to: users.recipient, depositAmount: DEPOSIT_AMOUNT_6D, + feeAmount: 0, withdrawAmount: WITHDRAW_AMOUNT_6D }); } - function _test_Withdraw(uint256 streamId, address to, uint128 depositAmount, uint128 withdrawAmount) private { + function _test_Withdraw( + uint256 streamId, + address to, + uint128 depositAmount, + uint128 feeAmount, + uint128 withdrawAmount + ) + private + { IERC20 token = flow.getToken(streamId); - uint128 previousFullTotalDebt = flow.totalDebtOf(streamId); - uint128 expectedProtocolRevenue = flow.protocolRevenue(token); + uint128 previousTotalDebt = flow.totalDebtOf(streamId); - uint128 feeAmount = 0; - if (flow.protocolFee(token) > ZERO) { - feeAmount = PROTOCOL_FEE_AMOUNT_6D; - withdrawAmount -= feeAmount; - expectedProtocolRevenue += feeAmount; - } + uint128 expectedProtocolRevenue = flow.protocolRevenue(token) + feeAmount; // It should emit 1 {Transfer}, 1 {WithdrawFromFlowStream} and 1 {MetadataUpdated} events. vm.expectEmit({ emitter: address(token) }); - emit IERC20.Transfer({ from: address(flow), to: to, value: withdrawAmount }); + emit IERC20.Transfer({ from: address(flow), to: to, value: withdrawAmount - feeAmount }); vm.expectEmit({ emitter: address(flow) }); emit WithdrawFromFlowStream({ @@ -268,39 +260,36 @@ contract WithdrawAt_Integration_Concrete_Test is Integration_Test { token: token, caller: users.recipient, protocolFeeAmount: feeAmount, - withdrawAmount: withdrawAmount, - snapshotTime: WITHDRAW_TIME + withdrawAmount: withdrawAmount - feeAmount, + snapshotTime: getBlockTimestamp() }); vm.expectEmit({ emitter: address(flow) }); emit MetadataUpdate({ _tokenId: streamId }); // It should perform the ERC-20 transfer. - expectCallToTransfer({ token: token, to: to, amount: withdrawAmount }); + expectCallToTransfer({ token: token, to: to, amount: withdrawAmount - feeAmount }); uint256 initialTokenBalance = token.balanceOf(address(flow)); - uint128 actualWithdrawAmount = flow.withdrawAt({ streamId: streamId, to: to, time: WITHDRAW_TIME }); + flow.withdraw({ streamId: streamId, to: to, amount: withdrawAmount }); // Assert the protocol revenue. assertEq(flow.protocolRevenue(token), expectedProtocolRevenue, "protocol revenue"); // It should update snapshot time. - assertEq(flow.getSnapshotTime(streamId), WITHDRAW_TIME, "snapshot time"); + assertEq(flow.getSnapshotTime(streamId), getBlockTimestamp(), "snapshot time"); // It should decrease the total debt by the withdrawn value and fee amount. - uint128 expectedFullTotalDebt = previousFullTotalDebt - withdrawAmount - feeAmount; - assertEq(flow.totalDebtOf(streamId), expectedFullTotalDebt, "total debt"); + uint128 expectedTotalDebt = previousTotalDebt - withdrawAmount; + assertEq(flow.totalDebtOf(streamId), expectedTotalDebt, "total debt"); // It should reduce the stream balance by the withdrawn value and fee amount. - uint128 expectedStreamBalance = depositAmount - withdrawAmount - feeAmount; + uint128 expectedStreamBalance = depositAmount - withdrawAmount; assertEq(flow.getBalance(streamId), expectedStreamBalance, "stream balance"); // It should reduce the token balance of stream. - uint256 expectedTokenBalance = initialTokenBalance - withdrawAmount; + uint256 expectedTokenBalance = initialTokenBalance - withdrawAmount + feeAmount; assertEq(token.balanceOf(address(flow)), expectedTokenBalance, "token balance"); - - // Assert that the returned value equals the net withdrawn value. - assertEq(actualWithdrawAmount, withdrawAmount); } } diff --git a/test/integration/concrete/withdraw/withdraw.tree b/test/integration/concrete/withdraw/withdraw.tree new file mode 100644 index 00000000..5a7ad44c --- /dev/null +++ b/test/integration/concrete/withdraw/withdraw.tree @@ -0,0 +1,44 @@ +Withdraw_Integration_Concrete_Test +├── when delegate call +│ └── it should revert +└── when no delegate call + ├── given null + │ └── it should revert + └── given not null + ├── when amount zero + │ └── it should revert + └── when amount not zero + ├── when withdrawal address zero + │ └── it should revert + └── when withdrawal address not zero + ├── when withdrawal address not owner + │ ├── when caller sender + │ │ └── it should revert + │ ├── when caller unknown + │ │ └── it should revert + │ └── when caller recipient + │ └── it should withdraw + └── when withdrawal address owner + ├── when amount overdraws + │ ├── given stream has uncovered debt + │ │ └── it should revert + │ └── given stream has no uncovered debt + │ └── it should revert + └── when amount not overdraw + ├── when amount not equal total debt + │ ├── it should update snapshot debt + │ └── it should make the withdrawal + └── when amount equal total debt + ├── given protocol fee not zero + │ ├── it should update the protocol revenue + │ └── it should withdraw the net amount + └── given protocol fee zero + ├── given token has 18 decimals + │ └── it should make the withdrawal + └── given token not have 18 decimals + ├── it should withdraw the withdrawable amount + ├── it should reduce the stream balance by the withdrawn amount + ├── it should update snapshot debt to zero + ├── it should update snapshot time + ├── it should emit 1 {Transfer}, 1 {WithdrawFromFlowStream} and 1 {MetadataUpdated} events + └── it should return the withdrawn amount \ No newline at end of file diff --git a/test/integration/fuzz/Fuzz.t.sol b/test/integration/fuzz/Fuzz.t.sol index c5027ff8..87ad26c3 100644 --- a/test/integration/fuzz/Fuzz.t.sol +++ b/test/integration/fuzz/Fuzz.t.sol @@ -69,7 +69,7 @@ abstract contract Shared_Integration_Fuzz_Test is Integration_Test { } /// @dev Helper function to return the address of either recipient or operator depending on the value of `timeJump`. - /// This function is used to prank the caller in {withdrawAt}, {withdrawMax} and {void} calls. + /// This function is used to prank the caller in {withdraw}, {withdrawMax} and {void} calls. function useRecipientOrOperator(uint256 streamId, uint40 timeJump) internal returns (address) { if (timeJump % 2 != 0) { return users.recipient; diff --git a/test/integration/fuzz/withdrawAt.t.sol b/test/integration/fuzz/withdraw.t.sol similarity index 51% rename from test/integration/fuzz/withdrawAt.t.sol rename to test/integration/fuzz/withdraw.t.sol index c254ab81..9a9260e0 100644 --- a/test/integration/fuzz/withdrawAt.t.sol +++ b/test/integration/fuzz/withdraw.t.sol @@ -3,100 +3,10 @@ pragma solidity >=0.8.22; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { ud, UD60x18, ZERO } from "@prb/math/src/UD60x18.sol"; - +import { console2 } from "forge-std/src/console2.sol"; import { Shared_Integration_Fuzz_Test } from "./Fuzz.t.sol"; -contract WithdrawAt_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { - struct Vars { - uint40 actualSnapshotTime; - uint256 actualStreamBalance; - uint256 actualTokenBalance; - uint128 actualTotalDebt; - uint128 expectedProtocolRevenue; - uint40 expectedSnapshotTime; - uint128 expectedStreamBalance; - uint256 expectedTokenBalance; - uint128 expectedTotalDebt; - uint128 ongoingDebt; - uint128 streamBalance; - uint256 tokenBalance; - uint128 totalDebt; - uint128 withdrawAmount; - } - - Vars internal vars; - - /// @dev It should withdraw 0 amount from a stream. - function testFuzz_Paused_Withdraw( - address caller, - uint256 streamId, - uint40 timeJump, - uint40 withdrawTime, - uint8 decimals - ) - external - whenNoDelegateCall - givenNotNull - givenProtocolFeeZero - { - vm.assume(caller != address(0)); - - (streamId,,) = useFuzzedStreamOrCreate(streamId, decimals); - - // Pause the stream. - flow.pause(streamId); - - // Bound the time jump to provide a realistic time frame. - timeJump = boundUint40(timeJump, 1 seconds, 100 weeks); - - uint40 warpTimestamp = getBlockTimestamp() + timeJump; - - // Simulate the passage of time. - vm.warp({ newTimestamp: warpTimestamp }); - - // Bound the withdraw time between the allowed range. - withdrawTime = boundUint40(withdrawTime, MAY_1_2024, warpTimestamp); - - // Ensure no value is transferred. - vm.expectEmit({ emitter: address(token) }); - emit IERC20.Transfer({ from: address(flow), to: users.recipient, value: 0 }); - - vm.expectEmit({ emitter: address(flow) }); - emit WithdrawFromFlowStream({ - streamId: streamId, - to: users.recipient, - token: token, - caller: caller, - protocolFeeAmount: 0, - withdrawAmount: 0, - snapshotTime: withdrawTime - }); - - vm.expectEmit({ emitter: address(flow) }); - emit MetadataUpdate({ _tokenId: streamId }); - - vars.expectedTotalDebt = flow.totalDebtOf(streamId); - vars.expectedStreamBalance = flow.getBalance(streamId); - vars.expectedTokenBalance = token.balanceOf(address(flow)); - - // Change prank to caller and withdraw the tokens. - resetPrank(caller); - flow.withdrawAt(streamId, users.recipient, withdrawTime); - - // Assert that all states are unchanged except for snapshotTime. - vars.actualSnapshotTime = flow.getSnapshotTime(streamId); - assertEq(vars.actualSnapshotTime, withdrawTime, "snapshot time"); - - vars.actualTotalDebt = flow.totalDebtOf(streamId); - assertEq(vars.actualTotalDebt, vars.expectedTotalDebt, "total debt"); - - vars.actualStreamBalance = flow.getBalance(streamId); - assertEq(vars.actualStreamBalance, vars.expectedStreamBalance, "stream balance"); - - vars.actualTokenBalance = token.balanceOf(address(flow)); - assertEq(vars.actualTokenBalance, vars.expectedTokenBalance, "token balance"); - } - +contract Withdraw_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { /// @dev Checklist: /// - It should withdraw token from a stream. /// - It should emit the following events: {Transfer}, {MetadataUpdate}, {WithdrawFromFlowStream} @@ -105,14 +15,14 @@ contract WithdrawAt_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { /// - Only two values for caller (stream owner and approved operator). /// - Multiple non-zero values for to address. /// - Multiple streams to withdraw from, each with different token decimals and rps. - /// - Multiple values for withdraw time in the range (snapshotTime, currentTime). It could also be before or after + /// - Multiple values for withdraw amount, in the range (1, withdrawablemAmount). It could also be before or after /// depletion time. /// - Multiple points in time. function testFuzz_WithdrawalAddressNotOwner( address to, uint256 streamId, uint40 timeJump, - uint40 withdrawTime, + uint128 withdrawAmount, uint8 decimals ) external @@ -130,7 +40,7 @@ contract WithdrawAt_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { resetPrank({ msgSender: caller }); // Withdraw the tokens. - _test_WithdrawAt(caller, to, streamId, timeJump, withdrawTime); + _test_Withdraw(caller, to, streamId, timeJump, withdrawAmount); } /// @dev Checklist: @@ -142,7 +52,7 @@ contract WithdrawAt_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { /// - Multiple non-zero values for callers. /// - Multiple non-zero values for protocol fee not exceeding max allowed. /// - Multiple streams to withdraw from, each with different token decimals and rps. - /// - Multiple values for withdraw time in the range (snapshotTime, currentTime). It could also be before or after + /// - Multiple values for withdraw amount, in the range (1, withdrawablemAmount). It could also be before or after /// depletion time. /// - Multiple points in time. function testFuzz_ProtocolFeeNotZero( @@ -150,14 +60,14 @@ contract WithdrawAt_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { UD60x18 protocolFee, uint256 streamId, uint40 timeJump, - uint40 withdrawTime, + uint128 withdrawAmount, uint8 decimals ) external whenNoDelegateCall givenNotNull givenNotPaused - whenWithdrawalAddressIsOwner + whenWithdrawalAddressOwner { vm.assume(caller != address(0)); @@ -171,7 +81,7 @@ contract WithdrawAt_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { // Prank the caller and withdraw the tokens. resetPrank(caller); - _test_WithdrawAt(caller, users.recipient, streamId, timeJump, withdrawTime); + _test_Withdraw(caller, users.recipient, streamId, timeJump, withdrawAmount); } /// @dev Checklist: @@ -181,22 +91,22 @@ contract WithdrawAt_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { /// Given enough runs, all of the following scenarios should be fuzzed: /// - Multiple non-zero values for callers. /// - Multiple streams to withdraw from, each with different token decimals and rps. - /// - Multiple values for withdraw time in the range (snapshotTime, currentTime). It could also be before or - /// after + /// - Multiple values for withdraw amount, in the range (1, withdrawablemAmount). It could also be before or after + /// depletion time. /// depletion time. /// - Multiple points in time. - function testFuzz_WithdrawAt( + function testFuzz_Withdraw( address caller, uint256 streamId, uint40 timeJump, - uint40 withdrawTime, + uint128 withdrawAmount, uint8 decimals ) external whenNoDelegateCall givenNotNull givenNotPaused - whenWithdrawalAddressIsOwner + whenWithdrawalAddressOwner givenProtocolFeeZero { vm.assume(caller != address(0)); @@ -205,57 +115,74 @@ contract WithdrawAt_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { // Prank the caller and withdraw the tokens. resetPrank(caller); - _test_WithdrawAt(caller, users.recipient, streamId, timeJump, withdrawTime); + _test_Withdraw(caller, users.recipient, streamId, timeJump, withdrawAmount); } - // Shared private function. - function _test_WithdrawAt( + /// @dev A struct to hold the variables used in the test below, this prevents stack error. + struct Vars { + uint256 previousTokenBalance; + uint128 previousOngoingDebt; + uint128 previousTotalDebt; + uint128 previousStreamBalance; + uint128 expectedProtocolRevenue; + uint128 feeAmount; + uint128 actualProtocolRevenue; + uint40 actualSnapshotTime; + uint40 expectedSnapshotTime; + uint128 actualTotalDebt; + uint128 expectedTotalDebt; + uint128 actualStreamBalance; + uint128 expectedStreamBalance; + uint256 actualTokenBalance; + uint256 expectedTokenBalance; + } + + Vars internal vars; + + /// @dev Shared private function. + function _test_Withdraw( address caller, address to, uint256 streamId, uint40 timeJump, - uint40 withdrawTime + uint128 withdrawAmount ) private { // Bound the time jump to provide a realistic time frame. timeJump = boundUint40(timeJump, 1 seconds, 100 weeks); - uint40 warpTimestamp = getBlockTimestamp() + boundUint40(timeJump, 1 seconds, 100 weeks); - // Simulate the passage of time. - vm.warp({ newTimestamp: warpTimestamp }); + vm.warp({ newTimestamp: getBlockTimestamp() + timeJump }); + + // If the withdrawable amount is still zero, warp closely to depletion time. + if (flow.withdrawableAmountOf(streamId) == 0) { + vm.warp({ newTimestamp: flow.depletionTimeOf(streamId) - 1 }); + } - // Bound the withdraw time between the allowed range. - withdrawTime = boundUint40(withdrawTime, MAY_1_2024, warpTimestamp); + // Bound the withdraw amount between the allowed range. + withdrawAmount = boundUint128(withdrawAmount, 1, flow.withdrawableAmountOf(streamId)); - vars.tokenBalance = token.balanceOf(address(flow)); - vars.ongoingDebt = getDenormalizedAmount({ - amount: flow.getRatePerSecond(streamId).unwrap() * (withdrawTime - flow.getSnapshotTime(streamId)), - decimals: flow.getTokenDecimals(streamId) - }); - vars.totalDebt = flow.getSnapshotDebt(streamId) + vars.ongoingDebt; - vars.streamBalance = flow.getBalance(streamId); - vars.withdrawAmount = vars.streamBalance < vars.totalDebt ? vars.streamBalance : vars.totalDebt; + vars.previousTokenBalance = token.balanceOf(address(flow)); + vars.previousOngoingDebt = flow.totalDebtOf(streamId); + vars.previousTotalDebt = flow.getSnapshotDebt(streamId) + vars.previousOngoingDebt; + vars.previousStreamBalance = flow.getBalance(streamId); vars.expectedProtocolRevenue = flow.protocolRevenue(token); - - uint128 feeAmount; if (flow.protocolFee(token) > ZERO) { - feeAmount = uint128(ud(vars.withdrawAmount).mul(flow.protocolFee(token)).unwrap()); - vars.withdrawAmount -= feeAmount; - vars.expectedProtocolRevenue += feeAmount; + vars.feeAmount = ud(withdrawAmount).mul(flow.protocolFee(token)).intoUint128(); + vars.expectedProtocolRevenue += vars.feeAmount; } // Compute the snapshot time that will be stored post withdraw. vars.expectedSnapshotTime = uint40( - getNormalizedAmount(vars.ongoingDebt, flow.getTokenDecimals(streamId)) + getNormalizedAmount(vars.previousOngoingDebt, flow.getTokenDecimals(streamId)) / flow.getRatePerSecond(streamId).unwrap() + flow.getSnapshotTime(streamId) ); // Expect the relevant events to be emitted. vm.expectEmit({ emitter: address(token) }); - emit IERC20.Transfer({ from: address(flow), to: to, value: vars.withdrawAmount }); + emit IERC20.Transfer({ from: address(flow), to: to, value: withdrawAmount - vars.feeAmount }); vm.expectEmit({ emitter: address(flow) }); emit WithdrawFromFlowStream({ @@ -263,8 +190,8 @@ contract WithdrawAt_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { to: to, token: token, caller: caller, - protocolFeeAmount: feeAmount, - withdrawAmount: vars.withdrawAmount, + protocolFeeAmount: vars.feeAmount, + withdrawAmount: withdrawAmount - vars.feeAmount, snapshotTime: vars.expectedSnapshotTime }); @@ -272,31 +199,29 @@ contract WithdrawAt_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { emit MetadataUpdate({ _tokenId: streamId }); // Withdraw the tokens. - flow.withdrawAt(streamId, to, withdrawTime); + flow.withdraw(streamId, to, withdrawAmount); // Assert the protocol revenue. - assertEq(flow.protocolRevenue(token), vars.expectedProtocolRevenue, "protocol revenue"); - - uint40 snapshotTime = flow.getSnapshotTime(streamId); + vars.actualProtocolRevenue = flow.protocolRevenue(token); + assertEq(vars.actualProtocolRevenue, vars.expectedProtocolRevenue, "protocol revenue"); // It should update snapshot time. - assertEq(snapshotTime, vars.expectedSnapshotTime, "snapshot time"); + vars.actualSnapshotTime = flow.getSnapshotTime(streamId); + assertEq(vars.actualSnapshotTime, vars.expectedSnapshotTime, "snapshot time"); - // It should decrease the full total debt by withdrawn amount and fee amount. - vars.actualTotalDebt = flow.getSnapshotDebt(streamId) - + getDenormalizedAmount({ - amount: flow.getRatePerSecond(streamId).unwrap() * (vars.expectedSnapshotTime - snapshotTime), - decimals: flow.getTokenDecimals(streamId) - }); - vars.expectedTotalDebt = vars.totalDebt - vars.withdrawAmount - feeAmount; + // It should decrease the full total debt by withdrawn amount. + vars.actualTotalDebt = flow.totalDebtOf(streamId); + vars.expectedTotalDebt = vars.previousTotalDebt - withdrawAmount; assertEq(vars.actualTotalDebt, vars.expectedTotalDebt, "total debt"); - // It should reduce the stream balance by the withdrawn amount and fee amount. - vars.expectedStreamBalance = vars.streamBalance - vars.withdrawAmount - feeAmount; - assertEq(flow.getBalance(streamId), vars.expectedStreamBalance, "stream balance"); + // It should reduce the stream balance by the withdrawn amount. + vars.actualStreamBalance = flow.getBalance(streamId); + vars.expectedStreamBalance = vars.previousStreamBalance - withdrawAmount; + assertEq(vars.actualStreamBalance, vars.expectedStreamBalance, "stream balance"); - // It should reduce the token balance of stream by net withdrawn amount. - vars.expectedTokenBalance = vars.tokenBalance - vars.withdrawAmount; - assertEq(token.balanceOf(address(flow)), vars.expectedTokenBalance, "token balance"); + // It should reduce the token balance of stream by the net withdrawn amount. + vars.actualTokenBalance = token.balanceOf(address(flow)); + vars.expectedTokenBalance = vars.previousTokenBalance - withdrawAmount + vars.feeAmount; + assertEq(vars.actualTokenBalance, vars.expectedTokenBalance, "token balance"); } } diff --git a/test/integration/fuzz/withdrawMax.t.sol b/test/integration/fuzz/withdrawMax.t.sol index d24174aa..3546126e 100644 --- a/test/integration/fuzz/withdrawMax.t.sol +++ b/test/integration/fuzz/withdrawMax.t.sol @@ -112,7 +112,7 @@ contract WithdrawMax_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { whenNoDelegateCall givenNotNull givenNotPaused - whenWithdrawalAddressIsOwner + whenWithdrawalAddressOwner { vm.assume(caller != address(0)); diff --git a/test/invariant/Flow.t.sol b/test/invariant/Flow.t.sol index 5994c192..9c56230b 100644 --- a/test/invariant/Flow.t.sol +++ b/test/invariant/Flow.t.sol @@ -220,7 +220,7 @@ contract Flow_Invariant_Test is Base_Test { uint256 lastStreamId = flowStore.lastStreamId(); for (uint256 i = 0; i < lastStreamId; ++i) { uint256 streamId = flowStore.streamIds(i); - if (flow.getRatePerSecond(streamId).unwrap() != 0 && flowHandler.calls("withdrawAt") == 0) { + if (flow.getRatePerSecond(streamId).unwrap() != 0 && flowHandler.calls("withdraw") == 0) { assertGe( flow.totalDebtOf(streamId), flowHandler.previousTotalDebtOf(streamId), diff --git a/test/invariant/handlers/FlowHandler.sol b/test/invariant/handlers/FlowHandler.sol index 2a24b2ab..79d84e89 100644 --- a/test/invariant/handlers/FlowHandler.sol +++ b/test/invariant/handlers/FlowHandler.sol @@ -234,14 +234,14 @@ contract FlowHandler is BaseHandler { flow.void(currentStreamId); } - function withdrawAt( + function withdraw( uint256 timeJumpSeed, uint256 streamIndexSeed, address to, - uint40 time + uint128 amount ) external - instrument("withdrawAt") + instrument("withdraw") useFuzzedStream(streamIndexSeed) useFuzzedStreamRecipient adjustTimestamp(timeJumpSeed) @@ -253,8 +253,8 @@ contract FlowHandler is BaseHandler { // Check if there is anything to withdraw. vm.assume(flow.coveredDebtOf(currentStreamId) > 0); - // Bound the time so that it is between snapshot time and current time. - time = uint40(_bound(time, flow.getSnapshotTime(currentStreamId), getBlockTimestamp())); + // Bound the withdraw amount so that it is less than maximum wihtdrawable amount. + amount = boundUint128(amount, 1, flow.withdrawableAmountOf(currentStreamId)); // There is an edge case when the sender is the same as the recipient. In this scenario, the withdrawal // address must be set to the recipient. @@ -265,19 +265,8 @@ contract FlowHandler is BaseHandler { uint128 initialBalance = flow.getBalance(currentStreamId); - // We need to calculate the total debt at the time of withdrawal. Otherwise the modifier updates the mappings - // with `block.timestamp` as the time reference. - uint128 totalDebt = flow.getSnapshotDebt(currentStreamId) - + getDenormalizedAmount({ - amount: flow.getRatePerSecond(currentStreamId).unwrap() * (time - flow.getSnapshotTime(currentStreamId)), - decimals: flow.getTokenDecimals(currentStreamId) - }); - uint128 uncoveredDebt = initialBalance < totalDebt ? totalDebt - initialBalance : 0; - previousTotalDebtOf[currentStreamId] = totalDebt; - previousUncoveredDebtOf[currentStreamId] = uncoveredDebt; - // Withdraw from the stream. - flow.withdrawAt({ streamId: currentStreamId, to: to, time: time }); + flow.withdraw({ streamId: currentStreamId, to: to, amount: amount }); uint128 amountWithdrawn = initialBalance - flow.getBalance(currentStreamId); diff --git a/test/utils/Modifiers.sol b/test/utils/Modifiers.sol index f06806b3..c9148c53 100644 --- a/test/utils/Modifiers.sol +++ b/test/utils/Modifiers.sol @@ -119,14 +119,30 @@ abstract contract Modifiers { } /*////////////////////////////////////////////////////////////////////////// - WITHDRAW-AT + WITHDRAW //////////////////////////////////////////////////////////////////////////*/ modifier givenProtocolFeeZero() { _; } - modifier whenWithdrawalAddressIsOwner() { + modifier whenAmountEqualTotalDebt() { + _; + } + + modifier whenAmountNotOverdraw() { + _; + } + + modifier whenAmountNotZero() { + _; + } + + modifier whenAmountOverdraws() { + _; + } + + modifier whenWithdrawalAddressOwner() { _; }