test: use equality in the withdraw multiple test and in invariant (#320)

* test: use equality in the withdraw multiple test

test: use only withdrawMax

* test: use equality in invariant

* docs: update security assumption

* build: rename test to tests in script

* shub feedback

* update invariant and comment

* test: remove private function

---------

Co-authored-by: smol-ninja <[email protected]>

SECURITY.md

@@ -57,10 +57,10 @@ it must adhere to the following assumptions:
   trust that the sender will not abuse the refund function to reclaim tokens.
 - The `depletionTimeOf` function depends on the stream's rate per second. Therefore, any change in the rate per second
   will result in a new depletion time.
-- As explained in the [Technical Documentation](https://github.com/sablier-labs/flow/blob/main/TECHNICAL-DOC.md),
-  recipients cannot withdraw the exact amount of debt streamed due to precision errors. This discrepancy is minor, with
-  the maximum potential loss (in USDC) being just $0.01 per withdraw. Typically, this loss ranges from 0 to 1 unit of
-  the token (in its native decimal format), depending on the timing of the recipient's withdrawal.
+- As explained in the [Technical Documentation](https://github.com/sablier-labs/flow/blob/main/TECHNICAL-DOC.md), there
+  could be a minor discrepancy between the actual streamed amount and the expected amount. This is due to `rps` being an
+  18-decimal number, while users provide the amount per interval in the UI. If `rps` had infinite decimals, this
+  discrepancy would not occur.
 
 ### Rewards
 

TECHNICAL-DOC.md

@@ -93,8 +93,7 @@ can only withdraw the available balance.
 
 16. if $isVoided = true \implies isPaused = true$ and $ud = 0$
 
-17. if $isVoided = false \implies \text{expected amount streamed} \ge td + \text{amount withdrawn}$ and
-    $\text{expected amount streamed} - (td + \text{amount withdrawn}) \le 10$
+17. if $isVoided = false \implies \text{expected amount streamed} = td + \text{amount withdrawn}$
 
 ## Limitation
 

package.json

@@ -63,8 +63,8 @@
     "clean": "rm -rf artifacts broadcast cache docs out out-optimized out-svg",
     "lint": "bun run lint:sol && bun run prettier:check",
     "lint:fix": "bun run lint:sol:fix && forge fmt",
-    "lint:sol": "forge fmt --check && bun solhint \"{benchmark,precompiles,script,src,test}/**/*.sol\"",
-    "lint:sol:fix": "bun solhint \"{benchmark,precompiles,script,src,test}/**/*.sol\" --fix --noPrompt",
+    "lint:sol": "forge fmt --check && bun solhint \"{benchmark,precompiles,script,src,tests}/**/*.sol\"",
+    "lint:sol:fix": "bun solhint \"{benchmark,precompiles,script,src,tests}/**/*.sol\" --fix --noPrompt",
     "prepack": "bun install && bash ./shell/prepare-artifacts.sh",
     "prepare": "husky",
     "prettier:check": "prettier --check \"**/*.{json,md,svg,yml}\"",

tests/integration/fuzz/create.t.sol

@@ -3,7 +3,7 @@ pragma solidity >=0.8.22;
 
 import { IERC4906 } from "@openzeppelin/contracts/interfaces/IERC4906.sol";
 import { IERC721 } from "@openzeppelin/contracts/token/ERC721/IERC721.sol";
-import { ud21x18, UD21x18 } from "@prb/math/src/UD21x18.sol";
+import { UD21x18 } from "@prb/math/src/UD21x18.sol";
 
 import { ISablierFlow } from "src/interfaces/ISablierFlow.sol";
 

tests/integration/fuzz/restart.t.sol

@@ -2,7 +2,7 @@
 pragma solidity >=0.8.22;
 
 import { IERC4906 } from "@openzeppelin/contracts/interfaces/IERC4906.sol";
-import { ud21x18, UD21x18 } from "@prb/math/src/UD21x18.sol";
+import { UD21x18 } from "@prb/math/src/UD21x18.sol";
 
 import { ISablierFlow } from "src/interfaces/ISablierFlow.sol";
 

tests/integration/fuzz/withdrawMultiple.t.sol

@@ -3,20 +3,18 @@ pragma solidity >=0.8.22;
 
 import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import { ud21x18 } from "@prb/math/src/UD21x18.sol";
-import { ISablierFlow } from "src/interfaces/ISablierFlow.sol";
 
 import { Shared_Integration_Fuzz_Test } from "./Fuzz.t.sol";
 
-contract WithdrawMultiple_Delay_Fuzz_Test is Shared_Integration_Fuzz_Test {
+contract WithdrawMaxMultiple_NoDelay_Fuzz_Test is Shared_Integration_Fuzz_Test {
     /// @dev Checklist:
     /// - It should test multiple withdrawals from the stream using `withdrawMax`.
-    /// - It should assert that the actual withdrawn amount is less than the desired amount.
-    /// - It should check that stream delay and deviation are within acceptable limits for realistic values of rps.
+    /// - It should assert that the actual withdrawn amount is equal to the desired amount.
     ///
     /// Given enough runs, all of the following scenarios should be fuzzed for USDC:
     /// - Multiple values for realistic rps.
     /// - Multiple withdrawal counts on the same stream at multiple points in time.
-    function testFuzz_WithdrawMultiple_Usdc_SmallDelay(uint128 rps, uint256 withdrawCount, uint40 timeJump) external {
+    function testFuzz_WithdrawMaxMultiple_Usdc_NoDelay(uint128 rps, uint256 withdrawCount, uint40 timeJump) external {
         rps = boundRatePerSecond(ud21x18(rps)).unwrap();
 
         IERC20 token = createToken(DECIMALS);
@@ -50,72 +48,25 @@ contract WithdrawMultiple_Delay_Fuzz_Test is Shared_Integration_Fuzz_Test {
         // Calculate the desired amount.
         uint256 desiredTotalWithdrawnAmount = (rps * totalStreamPeriod) / SCALE_FACTOR;
 
-        // Calculate the deviation.
-        uint256 deviationAmount = desiredTotalWithdrawnAmount - actualTotalWithdrawnAmount;
-
-        // Calculate the stream delay.
-        uint256 streamDelay = (deviationAmount * SCALE_FACTOR) / rps;
-
-        // Assert that the stream delay is within 5 second for the given fuzzed rps.
-        assertLe(streamDelay, 5 seconds);
-
-        // Assert that the deviation is less than 0.01e6 USDC.
-        assertLe(deviationAmount, 0.01e6);
-
-        // Assert that actual withdrawn amount is always less than the desired amount.
-        assertLe(actualTotalWithdrawnAmount, desiredTotalWithdrawnAmount);
+        // Assert that actual sum of withdrawn amount equals the total desired amount.
+        assertEq(actualTotalWithdrawnAmount, desiredTotalWithdrawnAmount);
     }
 
     /// @dev Checklist:
     /// - It should test multiple withdrawals from the stream using `withdrawMax`.
-    /// - It should assert that the actual withdrawn amount is always less than the desired amount.
-    ///
-    /// Given enough runs, all of the following scenarios should be fuzzed:
-    /// - Multiple values for decimals
-    /// - Multiple values for wide ranged rps.
-    /// - Multiple withdrawal counts on the same stream at multiple points in time.
-    function testFuzz_WithdrawMaxMultiple_RpsWideRange(
-        uint128 rps,
-        uint256 withdrawCount,
-        uint40 timeJump,
-        uint8 decimals
-    )
-        external
-    {
-        _test_WithdrawMultiple(rps, withdrawCount, timeJump, decimals, ISablierFlow.withdrawMax.selector, 0);
-    }
-
-    /// @dev Checklist:
-    /// - It should test multiple withdrawals from the stream using `withdraw`.
-    /// - It should assert that the actual withdrawn amount is always less than the desired amount.
+    /// - It should assert that the actual withdrawn amount is equal to the desired amount.
     ///
     /// Given enough runs, all of the following scenarios should be fuzzed:
     /// - Multiple values for decimals
     /// - Multiple values for wide ranged rps.
-    /// - Multiple amounts to withdraw.
     /// - Multiple withdrawal counts on the same stream at multiple points in time.
-    function testFuzz_WithdrawMultiple_RpsWideRange(
+    function testFuzz_WithdrawMaxMultiple_RpsWideRange_NoDelay(
         uint128 rps,
-        uint128 withdrawAmount,
         uint256 withdrawCount,
         uint40 timeJump,
         uint8 decimals
     )
         external
-    {
-        _test_WithdrawMultiple(rps, withdrawCount, timeJump, decimals, ISablierFlow.withdraw.selector, withdrawAmount);
-    }
-
-    // Private helper function.
-    function _test_WithdrawMultiple(
-        uint128 rps,
-        uint256 withdrawCount,
-        uint40 timeJump,
-        uint8 decimals,
-        bytes4 selector,
-        uint128 withdrawAmount
-    )
-        private
     {
         decimals = boundUint8(decimals, 0, 18);
         IERC20 token = createToken(decimals);
@@ -147,16 +98,7 @@ contract WithdrawMultiple_Delay_Fuzz_Test is Shared_Integration_Fuzz_Test {
             timeJump = boundUint40(timeJump, 1 hours, 1 days);
             vm.warp({ newTimestamp: getBlockTimestamp() + timeJump });
 
-            // Withdraw the tokens based on the selector value.
-            // ISablierFlow.withdraw
-            if (selector == ISablierFlow.withdraw.selector) {
-                withdrawAmount = boundUint128(withdrawAmount, 1, flow.withdrawableAmountOf(streamId));
-                flow.withdraw(streamId, users.recipient, withdrawAmount);
-            }
-            // ISablierFlow.withdrawMax
-            else if (selector == ISablierFlow.withdrawMax.selector) {
-                (withdrawAmount,) = flow.withdrawMax(streamId, users.recipient);
-            }
+            (uint128 withdrawAmount,) = flow.withdrawMax(streamId, users.recipient);
 
             // Update the actual total amount withdrawn.
             actualTotalWithdrawnAmount += withdrawAmount;
@@ -165,7 +107,7 @@ contract WithdrawMultiple_Delay_Fuzz_Test is Shared_Integration_Fuzz_Test {
         uint40 totalStreamPeriod = getBlockTimestamp() - timeBeforeFirstWithdraw;
         uint256 desiredTotalWithdrawnAmount = getDescaledAmount(rps * totalStreamPeriod, decimals);
 
-        // Assert that actual withdrawn amount is always less than the desired amount.
-        assertLe(actualTotalWithdrawnAmount, desiredTotalWithdrawnAmount);
+        // Assert that actual sum of withdrawn amounts equal the desired amount.
+        assertEq(actualTotalWithdrawnAmount, desiredTotalWithdrawnAmount);
     }
 }

tests/invariant/Flow.t.sol

@@ -332,8 +332,8 @@ contract Flow_Invariant_Test is Base_Test {
         }
     }
 
-    /// @dev For non-voided streams, the expected streamed amount should be greater than or equal to the sum of total
-    /// debt and withdrawn amount. And, the difference between the two should not exceed 10 mvt.
+    /// @dev For non-voided streams, the expected streamed amount should equal to the sum of withdrawn
+    /// amount and total debt.
     function invariant_TotalStreamedEqTotalDebtPlusWithdrawn() external view {
         uint256 lastStreamId = flowStore.lastStreamId();
         for (uint256 i = 0; i < lastStreamId; ++i) {
@@ -345,16 +345,10 @@ contract Flow_Invariant_Test is Base_Test {
                     calculateExpectedStreamedAmount(flowStore.streamIds(i), flow.getTokenDecimals(streamId));
                 uint256 actualTotalStreamed = flow.totalDebtOf(streamId) + flowStore.withdrawnAmounts(streamId);
 
-                assertGe(
+                assertEq(
                     expectedTotalStreamed,
                     actualTotalStreamed,
-                    "Invariant violation: expected streamed amount >= total debt + withdrawn amount"
-                );
-
-                assertLe(
-                    expectedTotalStreamed - actualTotalStreamed,
-                    10,
-                    "Invariant violation: expected streamed amount - total debt + withdrawn amount <= 10"
+                    "Invariant violation: expected total streamed amount != total debt + withdrawn amount"
                 );
             }
         }