From b67f34c57ba161a25d0f83ac221a91a73f09bc86 Mon Sep 17 00:00:00 2001 From: Andrei Vlad Birgaoanu <99738872+andreivladbrg@users.noreply.github.com> Date: Fri, 13 Sep 2024 17:59:56 +0300 Subject: [PATCH] refactor: `amount` param in `_withdraw` (#232) * refactor: change the time param in withdraw with an amount test: update tests accordingly * docs: fix * perf: remove redudant assert * Shub's feedback * test: remove redundant lines in withdraw handler test: change token symbols * undeeded imports * test: add getRenormalizedAmount in Utils.sol test: assertions in withdrawMax fuzz test * test: set lower bound for timejump to 0 in fuzz and invariant * remove comment * test: lower bound for rps * refactor: remove corrected time logic (#236) * chore: polish * chore: polish --------- Co-authored-by: smol-ninja Co-authored-by: gavriliumircea <48255669+gavriliumircea@users.noreply.github.com> --- benchmark/Flow.Gas.t.sol | 20 +- benchmark/results/SablierFlow.md | 28 +- diagrams.md | 2 +- package.json | 1 + src/SablierFlow.sol | 178 ++++------- src/interfaces/ISablierFlow.sol | 28 +- src/libraries/Errors.sol | 14 +- test/Base.t.sol | 6 +- test/fork/Flow.t.sol | 82 ++--- test/integration/Integration.t.sol | 3 +- test/integration/concrete/batch/batch.t.sol | 21 +- .../collectProtocolRevenue.t.sol | 2 +- .../deposit-via-broker/depositViaBroker.t.sol | 1 + .../concrete/withdraw-at/withdrawAt.tree | 42 --- .../concrete/withdraw-max/withdrawMax.t.sol | 15 +- .../withdraw.t.sol} | 230 +++++++------ .../concrete/withdraw/withdraw.tree | 44 +++ test/integration/fuzz/Fuzz.t.sol | 2 +- .../fuzz/adjustRatePerSecond.t.sol | 16 +- test/integration/fuzz/coveredDebtOf.t.sol | 37 ++- test/integration/fuzz/depletionTimeOf.t.sol | 2 +- test/integration/fuzz/deposit.t.sol | 2 +- test/integration/fuzz/ongoingDebtOf.t.sol | 17 +- test/integration/fuzz/pause.t.sol | 6 +- .../integration/fuzz/refundableAmountOf.t.sol | 32 +- test/integration/fuzz/restart.t.sol | 2 +- test/integration/fuzz/totalDebtOf.t.sol | 15 +- test/integration/fuzz/uncoveredDebtOf.t.sol | 4 +- test/integration/fuzz/withdraw.t.sol | 224 +++++++++++++ test/integration/fuzz/withdrawAt.t.sol | 302 ------------------ test/integration/fuzz/withdrawMax.t.sol | 93 ++---- test/invariant/Flow.t.sol | 2 +- test/invariant/handlers/BaseHandler.sol | 2 +- test/invariant/handlers/FlowHandler.sol | 35 +- test/utils/Events.sol | 3 +- test/utils/Modifiers.sol | 20 +- test/utils/Utils.sol | 2 +- 37 files changed, 666 insertions(+), 869 deletions(-) delete mode 100644 test/integration/concrete/withdraw-at/withdrawAt.tree rename test/integration/concrete/{withdraw-at/withdrawAt.t.sol => withdraw/withdraw.t.sol} (55%) create mode 100644 test/integration/concrete/withdraw/withdraw.tree create mode 100644 test/integration/fuzz/withdraw.t.sol delete mode 100644 test/integration/fuzz/withdrawAt.t.sol diff --git a/benchmark/Flow.Gas.t.sol b/benchmark/Flow.Gas.t.sol index 6e96a60a..9247f0be 100644 --- a/benchmark/Flow.Gas.t.sol +++ b/benchmark/Flow.Gas.t.sol @@ -1,6 +1,8 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity >=0.8.22; +import { ud21x18 } from "@prb/math/src/UD21x18.sol"; + import { Integration_Test } from "./../test/integration/Integration.t.sol"; /// @notice A contract to benchmark Flow functions. @@ -33,7 +35,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") }); } @@ -43,7 +45,10 @@ contract Flow_Gas_Test is Integration_Test { function testGas_Implementations() external { // {flow.adjustRatePerSecond} - computeGas("adjustRatePerSecond", abi.encodeCall(flow.adjustRatePerSecond, (streamId, RATE_PER_SECOND + 1))); + computeGas( + "adjustRatePerSecond", + abi.encodeCall(flow.adjustRatePerSecond, (streamId, ud21x18(RATE_PER_SECOND_U128 + 1))) + ); // {flow.create} computeGas( @@ -74,19 +79,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, WITHDRAW_AMOUNT_6D)) ); // 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, WITHDRAW_AMOUNT_6D)) ); // {flow.withdrawMax} on an incremented stream ID. diff --git a/benchmark/results/SablierFlow.md b/benchmark/results/SablierFlow.md index 671cea56..ed704cff 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` | 44069 | +| `create` | 113829 | +| `deposit` | 25068 | +| `depositViaBroker` | 21858 | +| `pause` | 9371 | +| `refund` | 11514 | +| `restart` | 6456 | +| `void` | 8543 | +| `withdraw (insolvent stream)` | 56632 | +| `withdraw (solvent stream)` | 39046 | +| `withdrawMax` | 51468 | diff --git a/diagrams.md b/diagrams.md index 25511252..26c2c6b3 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`, except for `withdraw` 4. Red lines refers to the function that are doing an ERC-20 transfer ```mermaid diff --git a/package.json b/package.json index 7841125a..a5dc1e6f 100644 --- a/package.json +++ b/package.json @@ -35,6 +35,7 @@ "blockchain", "crypto", "cryptoasset-streaming", + "cryptotoken-streaming", "ethereum", "forge", "foundry", diff --git a/src/SablierFlow.sol b/src/SablierFlow.sol index 0198e982..50b899a1 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); } /*////////////////////////////////////////////////////////////////////////// @@ -383,33 +383,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 @@ -424,8 +410,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); } /*////////////////////////////////////////////////////////////////////////// @@ -433,7 +419,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. @@ -441,7 +427,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) { @@ -451,33 +437,15 @@ 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 - /// 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 - /// the stream is paused or is less than the snapshot time. - function _ongoingDebtOf( - uint256 streamId, - uint40 time - ) - internal - view - returns (uint128 ongoingDebt, uint40 correctedTime) - { + /// @dev Calculates the ongoing debt accrued since last snapshot. Return 0 if the stream is paused or + /// `block.timestamp` is less than or equal to snapshot time. + function _ongoingDebtOf(uint256 streamId) internal view returns (uint128 ongoingDebt) { + 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; } uint128 elapsedTime; @@ -485,18 +453,17 @@ 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(); uint8 tokenDecimals = _streams[streamId].tokenDecimals; // Calculate the ongoing debt accrued by multiplying the elapsed time by the rate per second. - uint128 normalizedOngoingDebt = elapsedTime * ratePerSecond; + uint128 normalizedOngoingDebt = elapsedTime * _streams[streamId].ratePerSecond.unwrap(); - // 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; } // Safe to use unchecked because we use {SafeCast}. @@ -504,34 +471,20 @@ contract SablierFlow is uint8 factor = 18 - tokenDecimals; // Since debt is denoted in token decimals, denormalize the amount. ongoingDebt = (normalizedOngoingDebt / (10 ** factor)).toUint128(); - - // Renormalize the ongoing debt for the calculation of corrected time. - uint128 renormalizedOngoingDebt = (ongoingDebt * (10 ** factor)).toUint128(); - - // Return 0 if renormalized ongoing debt is less than `ratePerSecond`. This eliminates the risk of leaking - // funds when renormalized debt is less than rate per second. - if (renormalizedOngoingDebt < ratePerSecond) { - return (0, snapshotTime); - } - - // Derive the corrected time from the renormalized ongoing debt. - correctedTime = uint40(snapshotTime + renormalizedOngoingDebt / ratePerSecond); } - - 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; @@ -541,7 +494,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; @@ -568,14 +521,14 @@ contract SablierFlow is revert Errors.SablierFlow_RatePerSecondNotDifferent(streamId, newRatePerSecond); } - // Calculate the ongoing debt and corrected time. - (uint128 ongoingDebt, uint40 correctedTime) = _ongoingDebtOf(streamId, uint40(block.timestamp)); + // Calculate the ongoing debt. + uint128 ongoingDebt = _ongoingDebtOf(streamId); // Effect: update the snapshot debt. _streams[streamId].snapshotDebt += ongoingDebt; // Effect: update the snapshot time. - _streams[streamId].snapshotTime = correctedTime; + _streams[streamId].snapshotTime = uint40(block.timestamp); // Effect: set the new rate per second. _streams[streamId].ratePerSecond = newRatePerSecond; @@ -688,7 +641,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. @@ -714,7 +667,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) { @@ -804,7 +757,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); @@ -816,48 +774,46 @@ contract SablierFlow is revert Errors.SablierFlow_WithdrawalAddressNotRecipient({ streamId: streamId, caller: msg.sender, to: to }); } + // Calculate the total debt. + uint128 totalDebt = _totalDebtOf(streamId); + + // Calculate the withdrawable amount. uint128 balance = _streams[streamId].balance; + uint128 withdrawableAmount; - // Check: the stream balance is not zero. - if (balance == 0) { - revert Errors.SablierFlow_WithdrawNoFundsAvailable(streamId); + if (balance < totalDebt) { + // If the stream balance is less than the total debt, the withdrawable amount is the balance. + withdrawableAmount = balance; + } else { + // Otherwise, the withdrawable amount is the total debt. + withdrawableAmount = totalDebt; } - (uint128 ongoingDebt, uint40 correctedTime) = _ongoingDebtOf(streamId, time); - 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; - - // Safe to use unchecked because subtraction cannot underflow. - unchecked { - // Effect: update the snapshot debt. - _streams[streamId].snapshotDebt = totalDebt - balance; - } + // Check: the withdraw amount is not greater than the withdrawable amount. + if (amount > withdrawableAmount) { + revert Errors.SablierFlow_Overdraw(streamId, amount, withdrawableAmount); } - // Otherwise, recipient can withdraw the full amount, and the snapshot debt must be set to zero. - else { - withdrawAmount = totalDebt; - // Effect: set the snapshot debt to zero. - _streams[streamId].snapshotDebt = 0; + // Safe to use unchecked, the balance or total debt cannot be less than `amount` at this point. + unchecked { + // Effect: update the stream balance. + _streams[streamId].balance -= amount; + + // Effect: update the snapshot debt. + _streams[streamId].snapshotDebt = totalDebt - amount; } // Effect: update the stream time. - _streams[streamId].snapshotTime = correctedTime; + _streams[streamId].snapshotTime = uint40(block.timestamp); // Load the variables 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 { @@ -866,11 +822,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({ @@ -879,8 +832,7 @@ contract SablierFlow is token: token, caller: msg.sender, protocolFeeAmount: feeAmount, - withdrawAmount: withdrawAmount, - snapshotTime: correctedTime + withdrawAmount: amount }); } } diff --git a/src/interfaces/ISablierFlow.sol b/src/interfaces/ISablierFlow.sol index c5df7837..bc6d7700 100644 --- a/src/interfaces/ISablierFlow.sol +++ b/src/interfaces/ISablierFlow.sol @@ -97,15 +97,13 @@ interface ISablierFlow is /// decimals. /// @param withdrawAmount The amount withdrawn to the recipient after subtracting the protocol fee, denoted in /// token's decimals. - /// @param snapshotTime The Unix timestamp representing the updated snapshot time. event WithdrawFromFlowStream( uint256 indexed streamId, address indexed to, IERC20 indexed token, address caller, uint128 protocolFeeAmount, - uint128 withdrawAmount, - uint40 snapshotTime + uint128 withdrawAmount ); /*////////////////////////////////////////////////////////////////////////// @@ -386,18 +384,12 @@ interface ISablierFlow is /// @param streamId The ID of the stream to void. function void(uint256 streamId) external; - /// @notice Withdraws to the `to` address the amount calculated based on the `time` reference and the snapshot time. + /// @notice Withdraws the provided amount of tokens from the stream to the `to` address. /// /// @dev Emits a {Transfer} and {WithdrawFromFlowStream} event. /// /// Notes: - /// - It sets the snapshot time to the `time` specified. - /// - If stream balance is less than the total debt at `time`: - /// - It withdraws the full balance. - /// - It sets the snapshot debt to the total debt minus the stream balance. - /// - If stream balance is greater than the total debt at `time`: - /// - It withdraws the total debt at `time`. - /// - It sets the snapshot debt to zero. + /// - It sets the snapshot time to the `block.timestamp` function. /// - If the protocol fee is enabled for the streaming token, the amount withdrawn is adjusted by the protocol fee. /// /// Requirements: @@ -405,26 +397,22 @@ 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. - /// - The stream balance must be greater than zero. + /// - `amount` must be greater than zero and must not exceed the withdrawable amount. /// /// @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 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}. + /// - 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/test/Base.t.sol b/test/Base.t.sol index 47a8fe2f..dd0a25f5 100644 --- a/test/Base.t.sol +++ b/test/Base.t.sol @@ -80,14 +80,14 @@ abstract contract Base_Test is Assertions, Events, Modifiers, Test, Utils { function createAndLabelTokens() internal { // Deploy the tokens. tokenWithoutDecimals = createToken("Token without Decimals", "TWD", 0); - tokenWithProtocolFee = createToken("Token with Protocol Fee", "APF", 6); + tokenWithProtocolFee = createToken("Token with Protocol Fee", "TPF", 6); dai = createToken("Dai stablecoin", "DAI", 18); usdc = createToken("USD Coin", "USDC", 6); usdt = new ERC20MissingReturn("Tether", "USDT", 6); // Label the tokens. - vm.label(address(tokenWithoutDecimals), "AWD"); - vm.label(address(tokenWithProtocolFee), "APF"); + vm.label(address(tokenWithoutDecimals), "TWD"); + vm.label(address(tokenWithProtocolFee), "TPF"); vm.label(address(dai), "DAI"); vm.label(address(usdc), "USDC"); vm.label(address(usdt), "USDT"); diff --git a/test/fork/Flow.t.sol b/test/fork/Flow.t.sol index 9fcdcdc2..34500642 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); } } @@ -249,14 +249,9 @@ contract Flow_Fork_Test is Fork_Test { uint128 beforeSnapshotAmount = flow.getSnapshotDebt(streamId); uint128 totalDebt = flow.totalDebtOf(streamId); uint128 ongoingDebt = flow.ongoingDebtOf(streamId); - if (ongoingDebt != 0) { - vars.expectedSnapshotTime = uint40( - flow.getSnapshotTime(streamId) - + getNormalizedAmount(ongoingDebt, flow.getTokenDecimals(streamId)) / oldRatePerSecond.unwrap() - ); - } else { - vars.expectedSnapshotTime = getBlockTimestamp(); - } + + // Compute the snapshot time that will be stored post withdraw. + vars.expectedSnapshotTime = getBlockTimestamp(); // It should emit 1 {AdjustFlowStream}, 1 {MetadataUpdate} events. vm.expectEmit({ emitter: address(flow) }); @@ -569,41 +564,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; - } + uint128 ongoingDebtNormalized = getNormalizedAmount(flow.ongoingDebtOf(streamId), tokenDecimals); + uint128 ratePerSecond = flow.getRatePerSecond(streamId).unwrap(); + uint40 snapshotTime = flow.getSnapshotTime(streamId); + + vars.expectedSnapshotTime = getBlockTimestamp(); + + (, address caller,) = vm.readCallers(); + address recipient = flow.getRecipient(streamId); // Expect the relevant events to be emitted. vm.expectEmit({ emitter: address(token) }); @@ -616,36 +604,32 @@ contract Flow_Fork_Test is Fork_Test { token: token, caller: caller, protocolFeeAmount: 0, - withdrawAmount: withdrawAmount, - snapshotTime: vars.expectedSnapshotTime + withdrawAmount: withdrawAmount }); vm.expectEmit({ emitter: address(flow) }); 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/Integration.t.sol b/test/integration/Integration.t.sol index 1839cb4d..1b462a77 100644 --- a/test/integration/Integration.t.sol +++ b/test/integration/Integration.t.sol @@ -138,8 +138,7 @@ abstract contract Integration_Test is Base_Test { // Restores the rate per second. flow.adjustRatePerSecond(streamId, ratePerSecond); - // Warp to make block.timestamp match the snapshot time. This is necessary as the correct snapshot time can - // deviate from the block.timestamp due to precision loss. + // Warp to the snapshot time. vm.warp({ newTimestamp: flow.getSnapshotTime(streamId) }); } diff --git a/test/integration/concrete/batch/batch.t.sol b/test/integration/concrete/batch/batch.t.sol index d7c21081..1e1aa5d2 100644 --- a/test/integration/concrete/batch/batch.t.sol +++ b/test/integration/concrete/batch/batch.t.sol @@ -29,13 +29,10 @@ contract Batch_Integration_Concrete_Test is Integration_Test { function test_RevertWhen_CustomError() external { // The calls declared as bytes bytes[] memory calls = new bytes[](1); - calls[0] = abi.encodeCall(flow.withdrawMax, (1, users.sender)); + calls[0] = abi.encodeCall(flow.withdrawMax, (1, users.recipient)); bytes memory expectedRevertData = abi.encodeWithSelector( - Errors.BatchError.selector, - abi.encodeWithSelector( - Errors.SablierFlow_WithdrawalAddressNotRecipient.selector, 1, users.sender, users.sender - ) + Errors.BatchError.selector, abi.encodeWithSelector(Errors.SablierFlow_WithdrawAmountZero.selector, 1) ); vm.expectRevert(expectedRevertData); @@ -333,8 +330,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_AMOUNT_6D)); + calls[1] = abi.encodeCall(flow.withdraw, (defaultStreamIds[1], users.recipient, WITHDRAW_AMOUNT_6D)); // It should emit 2 {Transfer}, 2 {WithdrawFromFlowStream} and 2 {MetadataUpdated} events. @@ -346,11 +343,10 @@ contract Batch_Integration_Concrete_Test is Integration_Test { emit WithdrawFromFlowStream({ streamId: defaultStreamIds[0], to: users.recipient, - token: IERC20(address(usdc)), + token: usdc, caller: users.sender, protocolFeeAmount: 0, - withdrawAmount: WITHDRAW_AMOUNT_6D, - snapshotTime: WITHDRAW_TIME + withdrawAmount: WITHDRAW_AMOUNT_6D }); vm.expectEmit({ emitter: address(flow) }); @@ -364,11 +360,10 @@ contract Batch_Integration_Concrete_Test is Integration_Test { emit WithdrawFromFlowStream({ streamId: defaultStreamIds[1], to: users.recipient, - token: IERC20(address(usdc)), + token: usdc, protocolFeeAmount: 0, caller: users.sender, - withdrawAmount: WITHDRAW_AMOUNT_6D, - snapshotTime: WITHDRAW_TIME + withdrawAmount: WITHDRAW_AMOUNT_6D }); vm.expectEmit({ emitter: address(flow) }); 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..ca29de85 100644 --- a/test/integration/concrete/deposit-via-broker/depositViaBroker.t.sol +++ b/test/integration/concrete/deposit-via-broker/depositViaBroker.t.sol @@ -56,6 +56,7 @@ contract DepositViaBroker_Integration_Concrete_Test is Integration_Test { whenBrokerAddressNotZero { vm.expectRevert(abi.encodeWithSelector(Errors.SablierFlow_DepositAmountZero.selector, defaultStreamId)); + 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..18f83fca 100644 --- a/test/integration/concrete/withdraw-max/withdrawMax.t.sol +++ b/test/integration/concrete/withdraw-max/withdrawMax.t.sol @@ -37,11 +37,11 @@ contract WithdrawMax_Integration_Concrete_Test is Integration_Test { } function _test_WithdrawMax() private { - uint128 withdrawAmount = ONE_MONTH_DEBT_6D; + uint128 expectedWithdrawAmount = ONE_MONTH_DEBT_6D; // It should emit 1 {Transfer}, 1 {WithdrawFromFlowStream} and 1 {MetadataUpdated} events. vm.expectEmit({ emitter: address(usdc) }); - emit IERC20.Transfer({ from: address(flow), to: users.recipient, value: withdrawAmount }); + emit IERC20.Transfer({ from: address(flow), to: users.recipient, value: expectedWithdrawAmount }); vm.expectEmit({ emitter: address(flow) }); emit WithdrawFromFlowStream({ @@ -50,17 +50,16 @@ contract WithdrawMax_Integration_Concrete_Test is Integration_Test { token: IERC20(address(usdc)), caller: users.sender, protocolFeeAmount: 0, - withdrawAmount: withdrawAmount, - snapshotTime: getBlockTimestamp() + withdrawAmount: expectedWithdrawAmount }); vm.expectEmit({ emitter: address(flow) }); emit MetadataUpdate({ _tokenId: defaultStreamId }); // It should perform the ERC-20 transfer. - expectCallToTransfer({ token: usdc, to: users.recipient, amount: withdrawAmount }); + expectCallToTransfer({ token: usdc, to: users.recipient, amount: expectedWithdrawAmount }); - uint128 actualWithdrawAmount = flow.withdrawMax(defaultStreamId, users.recipient); + uint128 actualWithdrawnAmount = flow.withdrawMax(defaultStreamId, users.recipient); // It should update the stream balance. uint128 actualStreamBalance = flow.getBalance(defaultStreamId); @@ -75,7 +74,7 @@ contract WithdrawMax_Integration_Concrete_Test is Integration_Test { uint128 actualSnapshotTime = flow.getSnapshotTime(defaultStreamId); assertEq(actualSnapshotTime, getBlockTimestamp(), "snapshot time"); - // Assert that the withdraw amounts match. - assertEq(actualWithdrawAmount, withdrawAmount); + // It should return the actual withdrawn amount. + assertEq(actualWithdrawnAmount, expectedWithdrawAmount, "withdrawn amount"); } } diff --git a/test/integration/concrete/withdraw-at/withdrawAt.t.sol b/test/integration/concrete/withdraw/withdraw.t.sol similarity index 55% rename from test/integration/concrete/withdraw-at/withdrawAt.t.sol rename to test/integration/concrete/withdraw/withdraw.t.sol index 8a847193..2a554b4c 100644 --- a/test/integration/concrete/withdraw-at/withdrawAt.t.sol +++ b/test/integration/concrete/withdraw/withdraw.t.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,35 @@ contract WithdrawAt_Integration_Concrete_Test is Integration_Test { token: token, caller: users.recipient, protocolFeeAmount: feeAmount, - withdrawAmount: withdrawAmount, - snapshotTime: WITHDRAW_TIME + withdrawAmount: withdrawAmount - feeAmount }); 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/adjustRatePerSecond.t.sol b/test/integration/fuzz/adjustRatePerSecond.t.sol index 0f3f74d9..c7ab0b29 100644 --- a/test/integration/fuzz/adjustRatePerSecond.t.sol +++ b/test/integration/fuzz/adjustRatePerSecond.t.sol @@ -32,16 +32,22 @@ contract AdjustRatePerSecond_Integration_Fuzz_Test is Shared_Integration_Fuzz_Te flow.pause(streamId); // Bound the time jump to provide a realistic time frame. - timeJump = boundUint40(timeJump, 1 seconds, 100 weeks); + timeJump = boundUint40(timeJump, 0 seconds, 100 weeks); // Simulate the passage of time. vm.warp({ newTimestamp: getBlockTimestamp() + timeJump }); + uint128 previousTotalDebt = flow.totalDebtOf(streamId); + // Expect the relevant error. vm.expectRevert(abi.encodeWithSelector(Errors.SablierFlow_StreamPaused.selector, streamId)); // Adjust the rate per second. flow.adjustRatePerSecond(streamId, newRatePerSecond); + + assertEq(flow.ongoingDebtOf(streamId), 0, "ongoing debt"); + + assertEq(previousTotalDebt, flow.totalDebtOf(streamId), "rate per second"); } /// @dev Checklist: @@ -68,11 +74,13 @@ contract AdjustRatePerSecond_Integration_Fuzz_Test is Shared_Integration_Fuzz_Te newRatePerSecond = ud21x18(boundUint128(newRatePerSecond.unwrap(), 1, UINT128_MAX)); // Bound the time jump to provide a realistic time frame. - timeJump = boundUint40(timeJump, 1 seconds, 100 weeks); + timeJump = boundUint40(timeJump, 0 seconds, 100 weeks); // Simulate the passage of time. vm.warp({ newTimestamp: getBlockTimestamp() + timeJump }); + uint128 previousTotalDebt = flow.totalDebtOf(streamId); + UD21x18 currentRatePerSecond = flow.getRatePerSecond(streamId); if (newRatePerSecond.unwrap() == currentRatePerSecond.unwrap()) { // Expect the relevant error. @@ -97,5 +105,9 @@ contract AdjustRatePerSecond_Integration_Fuzz_Test is Shared_Integration_Fuzz_Te // Adjust the rate per second. flow.adjustRatePerSecond(streamId, newRatePerSecond); + + assertEq(flow.ongoingDebtOf(streamId), 0, "ongoing debt"); + + assertEq(previousTotalDebt, flow.totalDebtOf(streamId), "rate per second"); } } diff --git a/test/integration/fuzz/coveredDebtOf.t.sol b/test/integration/fuzz/coveredDebtOf.t.sol index 07908776..e26e8644 100644 --- a/test/integration/fuzz/coveredDebtOf.t.sol +++ b/test/integration/fuzz/coveredDebtOf.t.sol @@ -9,16 +9,21 @@ contract CoveredDebtOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { /// Given enough runs, all of the following scenarios should be fuzzed: /// - Multiple paused streams, each with different token decimals and rps. /// - Multiple points in time prior to depletion period. - function testFuzz_PreDepletion_Paused(uint256 streamId, uint40 timeJump, uint8 decimals) external givenNotNull { + function testFuzz_PreDepletion_Paused( + uint256 streamId, + uint40 warpTimestamp, + uint8 decimals + ) + external + givenNotNull + { (streamId,,) = useFuzzedStreamOrCreate(streamId, decimals); - uint40 depletionPeriod = flow.depletionTimeOf(streamId); - // Bound the time jump so that it is less than the depletion timestamp. - timeJump = boundUint40(timeJump, getBlockTimestamp(), depletionPeriod - 1); + warpTimestamp = boundUint40(warpTimestamp, getBlockTimestamp(), flow.depletionTimeOf(streamId) - 1); // Simulate the passage of time. - vm.warp({ newTimestamp: timeJump }); + vm.warp({ newTimestamp: warpTimestamp }); uint128 expectedCoveredDebt = flow.coveredDebtOf(streamId); @@ -26,7 +31,7 @@ contract CoveredDebtOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { flow.pause(streamId); // Simulate the passage of time. - vm.warp({ newTimestamp: uint256(getBlockTimestamp()) + uint256(timeJump) }); + vm.warp({ newTimestamp: boundUint40(warpTimestamp, getBlockTimestamp() + 1, UINT40_MAX) }); // Assert that the covered debt did not change. uint128 actualCoveredDebt = flow.coveredDebtOf(streamId); @@ -40,7 +45,7 @@ contract CoveredDebtOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { /// - Multiple points in time prior to depletion period. function testFuzz_PreDepletion( uint256 streamId, - uint40 timeJump, + uint40 warpTimestamp, uint8 decimals ) external @@ -49,18 +54,17 @@ contract CoveredDebtOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { { (streamId, decimals,) = useFuzzedStreamOrCreate(streamId, decimals); - uint40 depletionPeriod = flow.depletionTimeOf(streamId); - // Bound the time jump so that it is less than the depletion timestamp. - timeJump = boundUint40(timeJump, getBlockTimestamp(), depletionPeriod - 1); + warpTimestamp = boundUint40(warpTimestamp, getBlockTimestamp(), flow.depletionTimeOf(streamId) - 1); // Simulate the passage of time. - vm.warp({ newTimestamp: timeJump }); + vm.warp({ newTimestamp: warpTimestamp }); + + uint128 ratePerSecond = flow.getRatePerSecond(streamId).unwrap(); // Assert that the covered debt equals the ongoing debt. uint128 actualCoveredDebt = flow.coveredDebtOf(streamId); - uint128 expectedCoveredDebt = - getDenormalizedAmount(flow.getRatePerSecond(streamId).unwrap() * (timeJump - MAY_1_2024), decimals); + uint128 expectedCoveredDebt = getDenormalizedAmount(ratePerSecond * (warpTimestamp - MAY_1_2024), decimals); assertEq(actualCoveredDebt, expectedCoveredDebt); } @@ -70,15 +74,14 @@ contract CoveredDebtOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { /// Given enough runs, all of the following scenarios should be fuzzed: /// - Multiple streams, each with different token decimals and rps. /// - Multiple points in time post depletion period. - function testFuzz_PostDepletion(uint256 streamId, uint40 timeJump, uint8 decimals) external givenNotNull { + function testFuzz_PostDepletion(uint256 streamId, uint40 warpTimestamp, uint8 decimals) external givenNotNull { (streamId,, depositedAmount) = useFuzzedStreamOrCreate(streamId, decimals); // Bound the time jump so it is greater than depletion timestamp. - uint40 depletionPeriod = flow.depletionTimeOf(streamId); - timeJump = boundUint40(timeJump, depletionPeriod + 1, UINT40_MAX); + warpTimestamp = boundUint40(warpTimestamp, flow.depletionTimeOf(streamId) + 1, UINT40_MAX); // Simulate the passage of time. - vm.warp({ newTimestamp: timeJump }); + vm.warp({ newTimestamp: warpTimestamp }); // Assert that the covered debt equals the stream balance. uint128 actualCoveredDebt = flow.coveredDebtOf(streamId); diff --git a/test/integration/fuzz/depletionTimeOf.t.sol b/test/integration/fuzz/depletionTimeOf.t.sol index 9a28bcc5..acc47770 100644 --- a/test/integration/fuzz/depletionTimeOf.t.sol +++ b/test/integration/fuzz/depletionTimeOf.t.sol @@ -28,7 +28,7 @@ contract DepletionTimeOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { ); // Bound the time jump to provide a realistic time frame. - timeJump = boundUint40(timeJump, 1 seconds, 100 weeks); + timeJump = boundUint40(timeJump, 0 seconds, 100 weeks); // Simulate the passage of time. vm.warp({ newTimestamp: getBlockTimestamp() + timeJump }); diff --git a/test/integration/fuzz/deposit.t.sol b/test/integration/fuzz/deposit.t.sol index f54c5aa5..b2cbfdd2 100644 --- a/test/integration/fuzz/deposit.t.sol +++ b/test/integration/fuzz/deposit.t.sol @@ -38,7 +38,7 @@ contract Deposit_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { depositAmount = boundDepositAmount(depositAmount, initialStreamBalance, decimals); // Bound the time jump to provide a realistic time frame. - timeJump = boundUint40(timeJump, 1 seconds, 100 weeks); + timeJump = boundUint40(timeJump, 0 seconds, 100 weeks); // Change prank to caller and deal some tokens to him. deal({ token: address(token), to: caller, give: depositAmount }); diff --git a/test/integration/fuzz/ongoingDebtOf.t.sol b/test/integration/fuzz/ongoingDebtOf.t.sol index 4841b357..87f60bdb 100644 --- a/test/integration/fuzz/ongoingDebtOf.t.sol +++ b/test/integration/fuzz/ongoingDebtOf.t.sol @@ -13,7 +13,7 @@ contract OngoingDebtOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { (streamId,,) = useFuzzedStreamOrCreate(streamId, decimals); // Bound the time jump to provide a realistic time frame. - timeJump = boundUint40(timeJump, 1 seconds, 100 weeks); + timeJump = boundUint40(timeJump, 0 seconds, 100 weeks); // Simulate the passage of time. vm.warp({ newTimestamp: getBlockTimestamp() + timeJump }); @@ -47,7 +47,7 @@ contract OngoingDebtOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { (streamId,,) = useFuzzedStreamOrCreate(streamId, decimals); // Bound the time jump to provide a realistic time frame. - timeJump = boundUint40(timeJump, 1 seconds, 100 weeks); + timeJump = boundUint40(timeJump, 0 seconds, 100 weeks); // Simulate the passage of time. vm.warp({ newTimestamp: getBlockTimestamp() + timeJump }); @@ -80,17 +80,16 @@ contract OngoingDebtOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { updateSnapshotTimeAndWarp(streamId); // Bound the time jump to provide a realistic time frame. - timeJump = boundUint40(timeJump, 1 seconds, 100 weeks); - - uint40 warpTimestamp = getBlockTimestamp() + timeJump; + timeJump = boundUint40(timeJump, 0 seconds, 100 weeks); // Simulate the passage of time. - vm.warp({ newTimestamp: warpTimestamp }); + vm.warp({ newTimestamp: getBlockTimestamp() + timeJump }); + + uint128 ratePerSecond = flow.getRatePerSecond(streamId).unwrap(); - // Assert that total debt is zero. + // Assert that the ongoing debt equals the expected value. uint128 actualOngoingDebt = flow.ongoingDebtOf(streamId); - uint128 expectedOngoingDebt = - getDenormalizedAmount(flow.getRatePerSecond(streamId).unwrap() * (warpTimestamp - MAY_1_2024), decimals); + uint128 expectedOngoingDebt = getDenormalizedAmount(ratePerSecond * timeJump, decimals); assertEq(actualOngoingDebt, expectedOngoingDebt, "ongoing debt"); } } diff --git a/test/integration/fuzz/pause.t.sol b/test/integration/fuzz/pause.t.sol index 2febbaa4..bf952fe7 100644 --- a/test/integration/fuzz/pause.t.sol +++ b/test/integration/fuzz/pause.t.sol @@ -26,7 +26,7 @@ contract Pause_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { flow.pause(streamId); // Bound the time jump to provide a realistic time frame. - timeJump = boundUint40(timeJump, 1 seconds, 100 weeks); + timeJump = boundUint40(timeJump, 0 seconds, 100 weeks); // Simulate the passage of time. vm.warp({ newTimestamp: getBlockTimestamp() + timeJump }); @@ -59,7 +59,7 @@ contract Pause_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { (streamId,,) = useFuzzedStreamOrCreate(streamId, decimals); // Bound the time jump to provide a realistic time frame. - timeJump = boundUint40(timeJump, 1 seconds, 100 weeks); + timeJump = boundUint40(timeJump, 0 seconds, 100 weeks); // Simulate the passage of time. vm.warp({ newTimestamp: getBlockTimestamp() + timeJump }); @@ -82,6 +82,8 @@ contract Pause_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { // Assert that the stream is paused. assertTrue(flow.isPaused(streamId), "paused"); + assertEq(flow.ongoingDebtOf(streamId), 0, "ongoing debt"); + // Assert that the rate per second is 0. assertEq(flow.getRatePerSecond(streamId), 0, "rate per second"); } diff --git a/test/integration/fuzz/refundableAmountOf.t.sol b/test/integration/fuzz/refundableAmountOf.t.sol index 43bf7b7a..9fb8eb47 100644 --- a/test/integration/fuzz/refundableAmountOf.t.sol +++ b/test/integration/fuzz/refundableAmountOf.t.sol @@ -9,7 +9,14 @@ contract RefundableAmountOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Tes /// Given enough runs, all of the following scenarios should be fuzzed: /// - Multiple paused streams, each with different token decimals and rps. /// - Multiple points in time prior to depletion period. - function testFuzz_PreDepletion_Paused(uint256 streamId, uint40 timeJump, uint8 decimals) external givenNotNull { + function testFuzz_PreDepletion_Paused( + uint256 streamId, + uint40 warpTimestamp, + uint8 decimals + ) + external + givenNotNull + { (streamId,, depositedAmount) = useFuzzedStreamOrCreate(streamId, decimals); uint40 depletionPeriod = flow.depletionTimeOf(streamId); @@ -20,10 +27,10 @@ contract RefundableAmountOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Tes uint128 previousStreamBalance = flow.getBalance(streamId); // Bound the time jump so that it is less than the depletion timestamp. - timeJump = boundUint40(timeJump, getBlockTimestamp(), depletionPeriod - 1); + warpTimestamp = boundUint40(warpTimestamp, getBlockTimestamp(), depletionPeriod - 1); // Simulate the passage of time. - vm.warp({ newTimestamp: timeJump }); + vm.warp({ newTimestamp: warpTimestamp }); // Assert that the refundable amount equals the stream balance before the time warp. uint128 actualRefundableAmount = flow.refundableAmountOf(streamId); @@ -40,7 +47,7 @@ contract RefundableAmountOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Tes /// - Multiple points in time prior to depletion period. function testFuzz_PreDepletion( uint256 streamId, - uint40 timeJump, + uint40 warpTimestamp, uint8 decimals ) external @@ -50,16 +57,17 @@ contract RefundableAmountOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Tes (streamId, decimals, depositedAmount) = useFuzzedStreamOrCreate(streamId, decimals); // Bound the time jump so that it is less than the depletion timestamp. - uint40 depletionPeriod = flow.depletionTimeOf(streamId); - timeJump = boundUint40(timeJump, getBlockTimestamp(), depletionPeriod - 1); + warpTimestamp = boundUint40(warpTimestamp, getBlockTimestamp(), flow.depletionTimeOf(streamId) - 1); // Simulate the passage of time. - vm.warp({ newTimestamp: timeJump }); + vm.warp({ newTimestamp: warpTimestamp }); + + uint128 ratePerSecond = flow.getRatePerSecond(streamId).unwrap(); // Assert that the refundable amount same as the deposited amount minus streamed amount. uint128 actualRefundableAmount = flow.refundableAmountOf(streamId); - uint128 expectedRefundableAmount = depositedAmount - - getDenormalizedAmount(flow.getRatePerSecond(streamId).unwrap() * (timeJump - MAY_1_2024), decimals); + uint128 expectedRefundableAmount = + depositedAmount - getDenormalizedAmount(ratePerSecond * (warpTimestamp - MAY_1_2024), decimals); assertEq(actualRefundableAmount, expectedRefundableAmount); } @@ -68,15 +76,15 @@ contract RefundableAmountOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Tes /// Given enough runs, all of the following scenarios should be fuzzed: /// - Multiple streams, each with different token decimals and rps. /// - Multiple points in time post depletion period. - function testFuzz_PostDepletion(uint256 streamId, uint40 timeJump, uint8 decimals) external givenNotNull { + function testFuzz_PostDepletion(uint256 streamId, uint40 warpTimestamp, uint8 decimals) external givenNotNull { (streamId,,) = useFuzzedStreamOrCreate(streamId, decimals); // Bound the time jump so it is greater than depletion timestamp. uint40 depletionPeriod = flow.depletionTimeOf(streamId); - timeJump = boundUint40(timeJump, depletionPeriod + 1, UINT40_MAX); + warpTimestamp = boundUint40(warpTimestamp, depletionPeriod + 1, UINT40_MAX); // Simulate the passage of time. - vm.warp({ newTimestamp: timeJump }); + vm.warp({ newTimestamp: warpTimestamp }); // Assert that the refundable amount is zero. uint128 actualRefundableAmount = flow.refundableAmountOf(streamId); diff --git a/test/integration/fuzz/restart.t.sol b/test/integration/fuzz/restart.t.sol index 36bd9084..9f0b19ba 100644 --- a/test/integration/fuzz/restart.t.sol +++ b/test/integration/fuzz/restart.t.sol @@ -33,7 +33,7 @@ contract Restart_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { flow.pause(streamId); // Bound the time jump to provide a realistic time frame. - timeJump = boundUint40(timeJump, 1 seconds, 100 weeks); + timeJump = boundUint40(timeJump, 0 seconds, 100 weeks); uint40 warpTimestamp = getBlockTimestamp() + timeJump; diff --git a/test/integration/fuzz/totalDebtOf.t.sol b/test/integration/fuzz/totalDebtOf.t.sol index 29bd765d..aee079e7 100644 --- a/test/integration/fuzz/totalDebtOf.t.sol +++ b/test/integration/fuzz/totalDebtOf.t.sol @@ -13,7 +13,7 @@ contract TotalDebtOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { (streamId,,) = useFuzzedStreamOrCreate(streamId, decimals); // Bound the time jump to provide a realistic time frame. - timeJump = boundUint40(timeJump, 1 seconds, 100 weeks); + timeJump = boundUint40(timeJump, 0 seconds, 100 weeks); // Simulate the passage of time. vm.warp({ newTimestamp: getBlockTimestamp() + timeJump }); @@ -48,17 +48,16 @@ contract TotalDebtOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { (streamId, decimals,) = useFuzzedStreamOrCreate(streamId, decimals); // Bound the time jump to provide a realistic time frame. - timeJump = boundUint40(timeJump, 1 seconds, 100 weeks); - - uint40 warpTimestamp = getBlockTimestamp() + timeJump; + timeJump = boundUint40(timeJump, 0 seconds, 100 weeks); // Simulate the passage of time. - vm.warp({ newTimestamp: warpTimestamp }); + vm.warp({ newTimestamp: getBlockTimestamp() + timeJump }); - // Assert that total debt is zero. + uint128 ratePerSecond = flow.getRatePerSecond(streamId).unwrap(); + + // Assert that total debt is the ongoing debt. uint128 actualTotalDebt = flow.totalDebtOf(streamId); - uint128 expectedTotalDebt = - getDenormalizedAmount(flow.getRatePerSecond(streamId).unwrap() * (warpTimestamp - MAY_1_2024), decimals); + uint128 expectedTotalDebt = getDenormalizedAmount(ratePerSecond * timeJump, decimals); assertEq(actualTotalDebt, expectedTotalDebt, "total debt"); } } diff --git a/test/integration/fuzz/uncoveredDebtOf.t.sol b/test/integration/fuzz/uncoveredDebtOf.t.sol index 1210e2a2..88b8abc7 100644 --- a/test/integration/fuzz/uncoveredDebtOf.t.sol +++ b/test/integration/fuzz/uncoveredDebtOf.t.sol @@ -13,7 +13,7 @@ contract UncoveredDebtOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { (streamId,,) = useFuzzedStreamOrCreate(streamId, decimals); // Bound the time jump to provide a realistic time frame. - timeJump = boundUint40(timeJump, 1 seconds, 100 weeks); + timeJump = boundUint40(timeJump, 0 seconds, 100 weeks); // Simulate the passage of time. vm.warp({ newTimestamp: getBlockTimestamp() + timeJump }); @@ -54,7 +54,7 @@ contract UncoveredDebtOf_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { uint40 depletionTime = flow.depletionTimeOf(streamId); // Bound the time jump to provide a realistic time frame. - timeJump = boundUint40(timeJump, 1 seconds, 100 weeks); + timeJump = boundUint40(timeJump, 0 seconds, 100 weeks); uint40 warpTimestamp = getBlockTimestamp() + timeJump; diff --git a/test/integration/fuzz/withdraw.t.sol b/test/integration/fuzz/withdraw.t.sol new file mode 100644 index 00000000..a56f1813 --- /dev/null +++ b/test/integration/fuzz/withdraw.t.sol @@ -0,0 +1,224 @@ +// SPDX-License-Identifier: UNLICENSED +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 { Shared_Integration_Fuzz_Test } from "./Fuzz.t.sol"; + +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} + /// + /// Given enough runs, all of the following scenarios should be fuzzed: + /// - 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 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, + uint128 withdrawAmount, + uint8 decimals + ) + external + whenNoDelegateCall + givenNotNull + givenNotPaused + givenProtocolFeeZero + { + vm.assume(to != address(0) && to != address(flow)); + + (streamId,,) = useFuzzedStreamOrCreate(streamId, decimals); + + // Prank to either recipient or operator. + address caller = useRecipientOrOperator(streamId, timeJump); + resetPrank({ msgSender: caller }); + + // Withdraw the tokens. + _test_Withdraw(caller, to, streamId, timeJump, withdrawAmount); + } + + /// @dev Checklist: + /// - It should increase protocol revenue for the token. + /// - It should withdraw token amount after deducting protocol fee from the stream. + /// - It should emit the following events: {Transfer}, {MetadataUpdate}, {WithdrawFromFlowStream} + /// + /// Given enough runs, all of the following scenarios should be fuzzed: + /// - 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 amount, in the range (1, withdrawablemAmount). It could also be before or after + /// depletion time. + /// - Multiple points in time. + function testFuzz_ProtocolFeeNotZero( + address caller, + UD60x18 protocolFee, + uint256 streamId, + uint40 timeJump, + uint128 withdrawAmount, + uint8 decimals + ) + external + whenNoDelegateCall + givenNotNull + givenNotPaused + whenWithdrawalAddressOwner + { + vm.assume(caller != address(0)); + + protocolFee = bound(protocolFee, ZERO, MAX_FEE); + + (streamId,,) = useFuzzedStreamOrCreate(streamId, decimals); + + // Set protocol fee. + resetPrank(users.admin); + flow.setProtocolFee(token, protocolFee); + + // Prank the caller and withdraw the tokens. + resetPrank(caller); + _test_Withdraw(caller, users.recipient, streamId, timeJump, withdrawAmount); + } + + /// @dev Checklist: + /// - It should withdraw token from a stream. + /// - It should emit the following events: {Transfer}, {MetadataUpdate}, {WithdrawFromFlowStream} + /// + /// 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 amount, in the range (1, withdrawablemAmount). It could also be before or after + /// depletion time. + /// depletion time. + /// - Multiple points in time. + function testFuzz_Withdraw( + address caller, + uint256 streamId, + uint40 timeJump, + uint128 withdrawAmount, + uint8 decimals + ) + external + whenNoDelegateCall + givenNotNull + givenNotPaused + whenWithdrawalAddressOwner + givenProtocolFeeZero + { + vm.assume(caller != address(0)); + + (streamId,,) = useFuzzedStreamOrCreate(streamId, decimals); + + // Prank the caller and withdraw the tokens. + resetPrank(caller); + _test_Withdraw(caller, users.recipient, streamId, timeJump, withdrawAmount); + } + + /// @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, + uint128 withdrawAmount + ) + private + { + // Bound the time jump to provide a realistic time frame. + timeJump = boundUint40(timeJump, 0 seconds, 100 weeks); + + // Simulate the passage of time. + 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 amount between the allowed range. + withdrawAmount = boundUint128(withdrawAmount, 1, flow.withdrawableAmountOf(streamId)); + + 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); + if (flow.protocolFee(token) > ZERO) { + 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 = getBlockTimestamp(); + + // Expect the relevant events to be emitted. + vm.expectEmit({ emitter: address(token) }); + emit IERC20.Transfer({ from: address(flow), to: to, value: withdrawAmount - vars.feeAmount }); + + vm.expectEmit({ emitter: address(flow) }); + emit WithdrawFromFlowStream({ + streamId: streamId, + to: to, + token: token, + caller: caller, + protocolFeeAmount: vars.feeAmount, + withdrawAmount: withdrawAmount - vars.feeAmount + }); + + vm.expectEmit({ emitter: address(flow) }); + emit MetadataUpdate({ _tokenId: streamId }); + + // Withdraw the tokens. + flow.withdraw(streamId, to, withdrawAmount); + + assertEq(flow.ongoingDebtOf(streamId), 0, "ongoing debt"); + + // Assert the protocol revenue. + vars.actualProtocolRevenue = flow.protocolRevenue(token); + assertEq(vars.actualProtocolRevenue, vars.expectedProtocolRevenue, "protocol revenue"); + + // It should update snapshot time. + vars.actualSnapshotTime = flow.getSnapshotTime(streamId); + assertEq(vars.actualSnapshotTime, vars.expectedSnapshotTime, "snapshot time"); + + // 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. + 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 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/withdrawAt.t.sol b/test/integration/fuzz/withdrawAt.t.sol deleted file mode 100644 index c254ab81..00000000 --- a/test/integration/fuzz/withdrawAt.t.sol +++ /dev/null @@ -1,302 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -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 { 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"); - } - - /// @dev Checklist: - /// - It should withdraw token from a stream. - /// - It should emit the following events: {Transfer}, {MetadataUpdate}, {WithdrawFromFlowStream} - /// - /// Given enough runs, all of the following scenarios should be fuzzed: - /// - 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 - /// depletion time. - /// - Multiple points in time. - function testFuzz_WithdrawalAddressNotOwner( - address to, - uint256 streamId, - uint40 timeJump, - uint40 withdrawTime, - uint8 decimals - ) - external - whenNoDelegateCall - givenNotNull - givenNotPaused - givenProtocolFeeZero - { - vm.assume(to != address(0) && to != address(flow)); - - (streamId,,) = useFuzzedStreamOrCreate(streamId, decimals); - - // Prank to either recipient or operator. - address caller = useRecipientOrOperator(streamId, timeJump); - resetPrank({ msgSender: caller }); - - // Withdraw the tokens. - _test_WithdrawAt(caller, to, streamId, timeJump, withdrawTime); - } - - /// @dev Checklist: - /// - It should increase protocol revenue for the token. - /// - It should withdraw token amount after deducting protocol fee from the stream. - /// - It should emit the following events: {Transfer}, {MetadataUpdate}, {WithdrawFromFlowStream} - /// - /// Given enough runs, all of the following scenarios should be fuzzed: - /// - 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 - /// depletion time. - /// - Multiple points in time. - function testFuzz_ProtocolFeeNotZero( - address caller, - UD60x18 protocolFee, - uint256 streamId, - uint40 timeJump, - uint40 withdrawTime, - uint8 decimals - ) - external - whenNoDelegateCall - givenNotNull - givenNotPaused - whenWithdrawalAddressIsOwner - { - vm.assume(caller != address(0)); - - protocolFee = bound(protocolFee, ZERO, MAX_FEE); - - (streamId,,) = useFuzzedStreamOrCreate(streamId, decimals); - - // Set protocol fee. - resetPrank(users.admin); - flow.setProtocolFee(token, protocolFee); - - // Prank the caller and withdraw the tokens. - resetPrank(caller); - _test_WithdrawAt(caller, users.recipient, streamId, timeJump, withdrawTime); - } - - /// @dev Checklist: - /// - It should withdraw token from a stream. - /// - It should emit the following events: {Transfer}, {MetadataUpdate}, {WithdrawFromFlowStream} - /// - /// 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 - /// depletion time. - /// - Multiple points in time. - function testFuzz_WithdrawAt( - address caller, - uint256 streamId, - uint40 timeJump, - uint40 withdrawTime, - uint8 decimals - ) - external - whenNoDelegateCall - givenNotNull - givenNotPaused - whenWithdrawalAddressIsOwner - givenProtocolFeeZero - { - vm.assume(caller != address(0)); - - (streamId,,) = useFuzzedStreamOrCreate(streamId, decimals); - - // Prank the caller and withdraw the tokens. - resetPrank(caller); - _test_WithdrawAt(caller, users.recipient, streamId, timeJump, withdrawTime); - } - - // Shared private function. - function _test_WithdrawAt( - address caller, - address to, - uint256 streamId, - uint40 timeJump, - uint40 withdrawTime - ) - 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 }); - - // Bound the withdraw time between the allowed range. - withdrawTime = boundUint40(withdrawTime, MAY_1_2024, warpTimestamp); - - 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.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; - } - - // Compute the snapshot time that will be stored post withdraw. - vars.expectedSnapshotTime = uint40( - getNormalizedAmount(vars.ongoingDebt, 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 }); - - vm.expectEmit({ emitter: address(flow) }); - emit WithdrawFromFlowStream({ - streamId: streamId, - to: to, - token: token, - caller: caller, - protocolFeeAmount: feeAmount, - withdrawAmount: vars.withdrawAmount, - snapshotTime: vars.expectedSnapshotTime - }); - - vm.expectEmit({ emitter: address(flow) }); - emit MetadataUpdate({ _tokenId: streamId }); - - // Withdraw the tokens. - flow.withdrawAt(streamId, to, withdrawTime); - - // Assert the protocol revenue. - assertEq(flow.protocolRevenue(token), vars.expectedProtocolRevenue, "protocol revenue"); - - uint40 snapshotTime = flow.getSnapshotTime(streamId); - - // It should update snapshot time. - assertEq(snapshotTime, 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; - 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 token balance of stream by net withdrawn amount. - vars.expectedTokenBalance = vars.tokenBalance - vars.withdrawAmount; - assertEq(token.balanceOf(address(flow)), vars.expectedTokenBalance, "token balance"); - } -} diff --git a/test/integration/fuzz/withdrawMax.t.sol b/test/integration/fuzz/withdrawMax.t.sol index d24174aa..24ae7577 100644 --- a/test/integration/fuzz/withdrawMax.t.sol +++ b/test/integration/fuzz/withdrawMax.t.sol @@ -6,56 +6,6 @@ import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { Shared_Integration_Fuzz_Test } from "./Fuzz.t.sol"; contract WithdrawMax_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { - /// @dev It should withdraw 0 amount from a stream. - function testFuzz_Paused( - address caller, - uint256 streamId, - uint40 timeJump, - uint8 decimals - ) - external - whenNoDelegateCall - givenNotNull - { - 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); - - // Simulate the passage of time. - vm.warp({ newTimestamp: getBlockTimestamp() + timeJump }); - - // Ensure no value is transferred. - vm.expectEmit({ emitter: address(token) }); - emit IERC20.Transfer({ from: address(flow), to: users.recipient, value: 0 }); - - uint128 expectedTotalDebt = flow.totalDebtOf(streamId); - uint128 expectedStreamBalance = flow.getBalance(streamId); - uint256 expectedTokenBalance = token.balanceOf(address(flow)); - - // Prank the caller and withdraw the tokens. - resetPrank(caller); - flow.withdrawMax(streamId, users.recipient); - - // Assert that all states are unchanged except for snapshotTime. - uint128 actualSnapshotTime = flow.getSnapshotTime(streamId); - assertEq(actualSnapshotTime, getBlockTimestamp(), "snapshot time"); - - uint128 actualTotalDebt = flow.totalDebtOf(streamId); - assertEq(actualTotalDebt, expectedTotalDebt, "total debt"); - - uint128 actualStreamBalance = flow.getBalance(streamId); - assertEq(actualStreamBalance, expectedStreamBalance, "stream balance"); - - uint256 actualTokenBalance = token.balanceOf(address(flow)); - assertEq(actualTokenBalance, expectedTokenBalance, "token balance"); - } - /// @dev Checklist: /// - It should withdraw the max covered debt from a stream. /// - It should emit the following events: {Transfer}, {MetadataUpdate}, {WithdrawFromFlowStream} @@ -81,7 +31,7 @@ contract WithdrawMax_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { (streamId,,) = useFuzzedStreamOrCreate(streamId, decimals); // Bound the time jump to provide a realistic time frame. - timeJump = boundUint40(timeJump, 1 seconds, 100 weeks); + timeJump = boundUint40(timeJump, 0 seconds, 100 weeks); // Simulate the passage of time. vm.warp({ newTimestamp: getBlockTimestamp() + timeJump }); @@ -112,14 +62,14 @@ contract WithdrawMax_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { whenNoDelegateCall givenNotNull givenNotPaused - whenWithdrawalAddressIsOwner + whenWithdrawalAddressOwner { vm.assume(caller != address(0)); (streamId,,) = useFuzzedStreamOrCreate(streamId, decimals); // Bound the time jump to provide a realistic time frame. - timeJump = boundUint40(timeJump, 1 seconds, 100 weeks); + timeJump = boundUint40(timeJump, 0 seconds, 100 weeks); // Simulate the passage of time. vm.warp({ newTimestamp: getBlockTimestamp() + timeJump }); @@ -131,20 +81,18 @@ contract WithdrawMax_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { // Shared private function. function _test_WithdrawMax(address caller, address withdrawTo, uint256 streamId) private { + // If the withdrawable amount is still zero, warp closely to depletion time. + if (flow.withdrawableAmountOf(streamId) == 0) { + vm.warp({ newTimestamp: flow.depletionTimeOf(streamId) - 1 }); + } + uint128 totalDebt = flow.totalDebtOf(streamId); uint256 tokenBalance = token.balanceOf(address(flow)); uint128 streamBalance = flow.getBalance(streamId); - uint128 withdrawAmount = flow.coveredDebtOf(streamId); - uint40 expectedCorrectedAmount = uint40( - getNormalizedAmount(flow.ongoingDebtOf(streamId), flow.getTokenDecimals(streamId)) - / flow.getRatePerSecond(streamId).unwrap() + flow.getSnapshotTime(streamId) - ); + uint128 withdrawAmount = flow.withdrawableAmountOf(streamId); - uint128 residualDebt = getDenormalizedAmount({ - amount: flow.getRatePerSecond(streamId).unwrap() * (getBlockTimestamp() - expectedCorrectedAmount), - decimals: flow.getTokenDecimals(streamId) - }); + uint40 expectedSnapshotTime = getBlockTimestamp(); // Expect the relevant events to be emitted. vm.expectEmit({ emitter: address(token) }); @@ -157,8 +105,7 @@ contract WithdrawMax_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { token: token, caller: caller, protocolFeeAmount: 0, - withdrawAmount: withdrawAmount, - snapshotTime: expectedCorrectedAmount + withdrawAmount: withdrawAmount }); vm.expectEmit({ emitter: address(flow) }); @@ -167,27 +114,29 @@ contract WithdrawMax_Integration_Fuzz_Test is Shared_Integration_Fuzz_Test { // Withdraw the tokens. flow.withdrawMax(streamId, withdrawTo); + assertEq(flow.ongoingDebtOf(streamId), 0, "ongoing debt"); + // It should update snapshot time. - assertEq(flow.getSnapshotTime(streamId), expectedCorrectedAmount, "snapshot time"); + assertEq(flow.getSnapshotTime(streamId), expectedSnapshotTime, "snapshot time"); // It should decrease the total debt by the withdrawn value. uint128 actualTotalDebt = flow.totalDebtOf(streamId); - uint128 expectedTotalDebt = totalDebt - withdrawAmount + residualDebt; + uint128 expectedTotalDebt = totalDebt - withdrawAmount; assertEq(actualTotalDebt, expectedTotalDebt, "total debt"); + // It should reduce the stream balance by the withdrawn amount. + uint128 actualStreamBalance = flow.getBalance(streamId); + uint128 expectedStreamBalance = streamBalance - withdrawAmount; + assertEq(actualStreamBalance, expectedStreamBalance, "stream balance"); + // Assert that snapshot time is updated correctly. - assertEq(flow.getSnapshotTime(streamId), expectedCorrectedAmount, "snapshot time"); + assertEq(flow.getSnapshotTime(streamId), expectedSnapshotTime, "snapshot time"); // Assert that total debt equals snapshot debt and ongoing debt assertEq( flow.totalDebtOf(streamId), flow.getSnapshotDebt(streamId) + flow.ongoingDebtOf(streamId), "snapshot debt" ); - // It should reduce the stream balance by the withdrawn amount. - uint128 actualStreamBalance = flow.getBalance(streamId); - uint128 expectedStreamBalance = streamBalance - withdrawAmount; - assertEq(actualStreamBalance, expectedStreamBalance, "stream balance"); - // It should reduce the token balance of stream. uint256 actualTokenBalance = token.balanceOf(address(flow)); uint256 expectedTokenBalance = tokenBalance - withdrawAmount; 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/BaseHandler.sol b/test/invariant/handlers/BaseHandler.sol index d127aa7a..22231c38 100644 --- a/test/invariant/handlers/BaseHandler.sol +++ b/test/invariant/handlers/BaseHandler.sol @@ -46,7 +46,7 @@ abstract contract BaseHandler is StdCheats, Utils { /// @dev Simulates the passage of time. The time jump is upper bounded so that streams don't settle too quickly. /// @param timeJumpSeed A fuzzed value needed for generating random time warps. modifier adjustTimestamp(uint256 timeJumpSeed) { - uint256 timeJump = _bound(timeJumpSeed, 2 minutes, 40 days); + uint256 timeJump = _bound(timeJumpSeed, 0 seconds, 40 days); vm.warp(getBlockTimestamp() + timeJump); _; } diff --git a/test/invariant/handlers/FlowHandler.sol b/test/invariant/handlers/FlowHandler.sol index 2a24b2ab..0a084076 100644 --- a/test/invariant/handlers/FlowHandler.sol +++ b/test/invariant/handlers/FlowHandler.sol @@ -3,7 +3,6 @@ pragma solidity >=0.8.22; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { ud21x18, UD21x18 } from "@prb/math/src/UD21x18.sol"; -import { ud, UD60x18, UNIT, ZERO } from "@prb/math/src/UD60x18.sol"; import { ISablierFlow } from "src/interfaces/ISablierFlow.sol"; @@ -234,14 +233,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 +252,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. @@ -263,30 +262,10 @@ contract FlowHandler is BaseHandler { to = currentRecipient; } - 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 }); - - uint128 amountWithdrawn = initialBalance - flow.getBalance(currentStreamId); - - UD60x18 protocolFee = flow.protocolFee(flow.getToken(currentStreamId)); - if (protocolFee > ZERO) { - amountWithdrawn -= uint128((ud(amountWithdrawn).mul(UNIT - protocolFee)).unwrap()); - } + flow.withdraw({ streamId: currentStreamId, to: to, amount: amount }); // Update the withdrawn amount. - flowStore.updateStreamWithdrawnAmountsSum(currentStreamId, amountWithdrawn); + flowStore.updateStreamWithdrawnAmountsSum(currentStreamId, amount); } } diff --git a/test/utils/Events.sol b/test/utils/Events.sol index cd7e8e1c..08cc0b84 100644 --- a/test/utils/Events.sol +++ b/test/utils/Events.sol @@ -76,7 +76,6 @@ abstract contract Events { IERC20 indexed token, address caller, uint128 protocolFeeAmount, - uint128 withdrawAmount, - uint40 snapshotTime + uint128 withdrawAmount ); } 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() { _; } diff --git a/test/utils/Utils.sol b/test/utils/Utils.sol index 6fa025e6..d4ae41e7 100644 --- a/test/utils/Utils.sol +++ b/test/utils/Utils.sol @@ -32,7 +32,7 @@ abstract contract Utils is CommonBase, Constants, PRBMathUtils { /// @dev Bounds the rate per second between a realistic range. function boundRatePerSecond(UD21x18 ratePerSecond) internal pure returns (UD21x18) { - return ud21x18(boundUint128(ratePerSecond.unwrap(), 0.0000001e18, 10e18)); + return ud21x18(boundUint128(ratePerSecond.unwrap(), 0.0000000001e18, 10e18)); } /// @dev Bounds a `uint128` number.