test: polish invariants

README.md

@@ -157,26 +157,24 @@ Currently, it's not possible to address this precision problem entirely.
    `flow.collectProtocolRevenue` handlers. In real life, someone can transfer tokens to the contract or admin can
    withdraw revenue from the contract.
 
-3. for any stream, if $ud > 0 \implies cd = bal$
+3. snapshot time should never decrease
 
-4. if $rps \gt 0$ and no deposits are made $\implies \frac{d(ud)}{dt} \ge 0$
+4. for any stream, if $ud > 0 \implies cd = bal$
 
-5. if $rps \gt 0$, and no withdraw is made $\implies \frac{d(td)}{dt} \ge 0$
+5. if $rps \gt 0$ and no deposits are made $\implies \frac{d(ud)}{dt} \ge 0$
 
-6. for any stream, sum of deposited amounts $\ge$ sum of withdrawn amounts + sum of refunded
+6. if $rps \gt 0$, and no withdraw is made $\implies \frac{d(td)}{dt} \ge 0$
 
-7. sum of all deposited amounts $\ge$ sum of all withdrawn amounts + sum of all refunded
+7. for any stream, sum of deposited amounts $\ge$ sum of withdrawn amounts + sum of refunded
 
-8. next stream id = current stream id + 1
+8. sum of all deposited amounts $\ge$ sum of all withdrawn amounts + sum of all refunded
 
-9. if $ud = 0$ and $isPaused = true \implies cd = sa$
+9. next stream id = current stream id + 1
 
-10. if $ud = 0$ and $isPaused = false \implies cd = sa + oa$
+10. if $ud = 0$ \implies cd = td$
 
 11. $bal = ra + cd$
 
 12. if $isPaused = true \implies rps = 0$
 
 13. if $isVoided = true \implies isPaused = true$, $ra = 0$ and $ud = 0$
-
-14. snapshot time should never decrease

src/SablierFlow.sol

@@ -825,6 +825,9 @@ contract SablierFlow is
         // Interaction: perform the ERC-20 transfer.
         token.safeTransfer({ to: to, value: amount });
 
+        // Protocol Invariant: the difference in total debt should be equal to the difference in the stream balance.
+        assert(totalDebt - _totalDebtOf(streamId) == balance - _streams[streamId].balance);
+
         // Log the withdrawal.
         emit ISablierFlow.WithdrawFromFlowStream({
             streamId: streamId,

test/fork/Flow.t.sol

@@ -584,10 +584,6 @@ contract Flow_Fork_Test is Fork_Test {
         uint256 initialTokenBalance = token.balanceOf(address(flow));
         uint128 totalDebt = flow.totalDebtOf(streamId);
 
-        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();

test/invariant/Flow.t.sol

@@ -131,11 +131,11 @@ contract Flow_Invariant_Test is Base_Test {
     }
 
     /// @dev If rps > 0, and no additional deposits are made, then the uncovered debt should never decrease.
-    function invariant_UncoveredDebtGt0_RpsGt0_UncoveredDebtIncrease() external view {
+    function invariant_RpsGt0_UncoveredDebtGt0_UncoveredDebtIncrease() external view {
         uint256 lastStreamId = flowStore.lastStreamId();
         for (uint256 i = 0; i < lastStreamId; ++i) {
             uint256 streamId = flowStore.streamIds(i);
-            if (flow.getRatePerSecond(streamId).unwrap() > 0 && flowHandler.calls("deposit") == 0) {
+            if (flow.getRatePerSecond(streamId).unwrap() > 0 && flowHandler.calls(streamId, "deposit") == 0) {
                 assertGe(
                     flow.uncoveredDebtOf(streamId),
                     flowHandler.previousUncoveredDebtOf(streamId),
@@ -145,6 +145,21 @@ contract Flow_Invariant_Test is Base_Test {
         }
     }
 
+    /// @dev If rps > 0, no withdraw is made, the total debt should always increase.
+    function invariant_RpsGt0_TotalDebtAlwaysIncreases() external view {
+        uint256 lastStreamId = flowStore.lastStreamId();
+        for (uint256 i = 0; i < lastStreamId; ++i) {
+            uint256 streamId = flowStore.streamIds(i);
+            if (flow.getRatePerSecond(streamId).unwrap() != 0 && flowHandler.calls(streamId, "withdraw") == 0) {
+                assertGe(
+                    flow.totalDebtOf(streamId),
+                    flowHandler.previousTotalDebtOf(streamId),
+                    "Invariant violation: total debt should be monotonically increasing"
+                );
+            }
+        }
+    }
+
     /// @dev For any stream, the sum of all deposited amounts should always be greater than or equal to the sum of all
     /// withdrawn and refunded amounts.
     function invariant_InflowGeOutflow() external view {
@@ -183,48 +198,17 @@ contract Flow_Invariant_Test is Base_Test {
         }
     }
 
-    /// @dev If there is no uncovered debt and the stream is paused, the covered debt should always be equal to
-    /// the snapshot debt.
-    function invariant_NoUncoveredDebt_StreamedPaused_CoveredDebtEqSnapshotAmount() external view {
+    /// @dev If there is no uncovered debt, the covered debt should always be equal to
+    /// the total debt.
+    function invariant_NoUncoveredDebt_StreamedPaused_CoveredDebtEqTotalDebt() external view {
         uint256 lastStreamId = flowStore.lastStreamId();
         for (uint256 i = 0; i < lastStreamId; ++i) {
             uint256 streamId = flowStore.streamIds(i);
-            if (flow.isPaused(streamId) && flow.uncoveredDebtOf(streamId) == 0) {
+            if (flow.uncoveredDebtOf(streamId) == 0) {
                 assertEq(
                     flow.coveredDebtOf(streamId),
-                    flow.getSnapshotDebt(streamId),
-                    "Invariant violation: paused stream covered debt != snapshot debt"
-                );
-            }
-        }
-    }
-
-    /// @dev If there is no uncovered debt and the stream is not paused, the covered debt should always be equal
-    /// to the sum of snapshot debt and ongoing debt.
-    function invariant_NoDebt_CoveredDebtEqOngoingDebtPlusSnapshotAmount() external view {
-        uint256 lastStreamId = flowStore.lastStreamId();
-        for (uint256 i = 0; i < lastStreamId; ++i) {
-            uint256 streamId = flowStore.streamIds(i);
-            if (!flow.isPaused(streamId) && flow.uncoveredDebtOf(streamId) == 0) {
-                assertEq(
-                    flow.coveredDebtOf(streamId),
-                    flow.ongoingDebtOf(streamId) + flow.getSnapshotDebt(streamId),
-                    "Invariant violation: covered debt != ongoing debt + snapshot debt"
-                );
-            }
-        }
-    }
-
-    /// @dev If rps > 0, no withdraw is made, the total debt should always increase.
-    function invariant_RpsGt0_TotalDebtAlwaysIncreases() external view {
-        uint256 lastStreamId = flowStore.lastStreamId();
-        for (uint256 i = 0; i < lastStreamId; ++i) {
-            uint256 streamId = flowStore.streamIds(i);
-            if (flow.getRatePerSecond(streamId).unwrap() != 0 && flowHandler.calls("withdraw") == 0) {
-                assertGe(
                     flow.totalDebtOf(streamId),
-                    flowHandler.previousTotalDebtOf(streamId),
-                    "Invariant violation: total debt should be monotonically increasing"
+                    "Invariant violation: paused stream covered debt != snapshot debt"
                 );
             }
         }

test/invariant/handlers/BaseHandler.sol

@@ -17,8 +17,8 @@ abstract contract BaseHandler is StdCheats, Utils {
     /// @dev Maximum number of streams that can be created during an invariant campaign.
     uint256 internal constant MAX_STREAM_COUNT = 100;
 
-    /// @dev Maps function names to the number of times they have been called.
-    mapping(string func => uint256 calls) public calls;
+    /// @dev Maps function names and the number of times they have been called by the stream ID.
+    mapping(uint256 streamId => mapping(string func => uint256 calls)) public calls;
 
     /// @dev The total number of calls made to this contract.
     uint256 public totalCalls;
@@ -53,8 +53,8 @@ abstract contract BaseHandler is StdCheats, Utils {
     }
 
     /// @dev Records a function call for instrumentation purposes.
-    modifier instrument(string memory functionName) {
-        calls[functionName]++;
+    modifier instrument(uint256 streamId, string memory functionName) {
+        calls[streamId][functionName]++;
         totalCalls++;
         _;
     }

test/invariant/handlers/FlowCreateHandler.sol

@@ -60,6 +60,7 @@ contract FlowCreateHandler is BaseHandler {
 
     /// @dev Struct to prevent stack too deep error.
     struct CreateParams {
+        uint128 depositAmount;
         uint256 timeJump;
         uint256 tokenIndex;
         address sender;
@@ -70,10 +71,10 @@ contract FlowCreateHandler is BaseHandler {
 
     function create(CreateParams memory params)
         public
-        instrument("create")
         checkUsers(params)
         useFuzzedToken(params.tokenIndex)
         adjustTimestamp(params.timeJump)
+        instrument(flow.nextStreamId(), "create")
     {
         vm.assume(flowStore.lastStreamId() < MAX_STREAM_COUNT);
 
@@ -88,15 +89,12 @@ contract FlowCreateHandler is BaseHandler {
         flowStore.pushStreamId(streamId, params.sender, params.recipient);
     }
 
-    function createAndDeposit(
-        CreateParams memory params,
-        uint128 depositAmount
-    )
+    function createAndDeposit(CreateParams memory params)
         public
-        instrument("createAndDeposit")
         checkUsers(params)
         useFuzzedToken(params.tokenIndex)
         adjustTimestamp(params.timeJump)
+        instrument(flow.nextStreamId(), "createAndDeposit")
     {
         vm.assume(flowStore.lastStreamId() < MAX_STREAM_COUNT);
 
@@ -106,7 +104,7 @@ contract FlowCreateHandler is BaseHandler {
         uint128 upperBound = getDenormalizedAmount(1_000_000e18, decimals);
 
         // Make sure the deposit amount is non-zero and less than values that could cause an overflow.
-        vm.assume(depositAmount >= 100 && depositAmount <= upperBound);
+        vm.assume(params.depositAmount >= 100 && params.depositAmount <= upperBound);
 
         // Use a realistic range for the rate per second.
         vm.assume(params.ratePerSecond.unwrap() >= 0.0000000001e18 && params.ratePerSecond.unwrap() <= 10e18);
@@ -115,21 +113,26 @@ contract FlowCreateHandler is BaseHandler {
         deal({
             token: address(currentToken),
             to: params.sender,
-            give: currentToken.balanceOf(params.sender) + depositAmount
+            give: currentToken.balanceOf(params.sender) + params.depositAmount
         });
 
         // Approve {SablierFlow} to spend the tokens.
-        currentToken.approve({ spender: address(flow), value: depositAmount });
+        currentToken.approve({ spender: address(flow), value: params.depositAmount });
 
         // Create the stream.
         uint256 streamId = flow.createAndDeposit(
-            params.sender, params.recipient, params.ratePerSecond, currentToken, params.transferable, depositAmount
+            params.sender,
+            params.recipient,
+            params.ratePerSecond,
+            currentToken,
+            params.transferable,
+            params.depositAmount
         );
 
         // Store the stream id.
         flowStore.pushStreamId(streamId, params.sender, params.recipient);
 
         // Store the deposited amount.
-        flowStore.updateStreamDepositedAmountsSum(streamId, depositAmount);
+        flowStore.updateStreamDepositedAmountsSum(streamId, params.depositAmount);
     }
 }

test/invariant/handlers/FlowHandler.sol

@@ -2,7 +2,7 @@
 pragma solidity >=0.8.22;
 
 import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.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";
 
@@ -79,11 +79,11 @@ contract FlowHandler is BaseHandler {
         UD21x18 newRatePerSecond
     )
         external
-        instrument("adjustRatePerSecond")
         useFuzzedStream(streamIndex)
         useFuzzedStreamSender
         adjustTimestamp(timeJump)
         updateFlowHandlerStates
+        instrument(currentStreamId, "adjustRatePerSecond")
     {
         // Only non paused streams can have their rate per second adjusted.
         vm.assume(!flow.isPaused(currentStreamId));
@@ -104,11 +104,11 @@ contract FlowHandler is BaseHandler {
         uint128 depositAmount
     )
         external
-        instrument("deposit")
         useFuzzedStream(streamIndex)
         useFuzzedStreamSender
         adjustTimestamp(timeJump)
         updateFlowHandlerStates
+        instrument(currentStreamId, "deposit")
     {
         // Voided streams cannot be deposited on.
         vm.assume(!flow.isVoided(currentStreamId));
@@ -135,18 +135,18 @@ contract FlowHandler is BaseHandler {
     }
 
     /// @dev A function that does nothing but warp the time into the future.
-    function passTime(uint256 timeJump) external instrument("passTime") adjustTimestamp(timeJump) { }
+    function passTime(uint256 timeJump) external adjustTimestamp(timeJump) { }
 
     function pause(
         uint256 timeJump,
         uint256 streamIndex
     )
         external
-        instrument("pause")
         useFuzzedStream(streamIndex)
         useFuzzedStreamSender
         adjustTimestamp(timeJump)
         updateFlowHandlerStates
+        instrument(currentStreamId, "pause")
     {
         // Paused streams cannot be paused again.
         vm.assume(!flow.isPaused(currentStreamId));
@@ -161,11 +161,11 @@ contract FlowHandler is BaseHandler {
         uint128 refundAmount
     )
         external
-        instrument("refund")
         useFuzzedStream(streamIndex)
         useFuzzedStreamSender
         adjustTimestamp(timeJump)
         updateFlowHandlerStates
+        instrument(currentStreamId, "refund")
     {
         // Voided streams cannot be refunded.
         vm.assume(!flow.isVoided(currentStreamId));
@@ -191,11 +191,11 @@ contract FlowHandler is BaseHandler {
         UD21x18 ratePerSecond
     )
         external
-        instrument("restart")
         useFuzzedStream(streamIndex)
         useFuzzedStreamSender
         adjustTimestamp(timeJump)
         updateFlowHandlerStates
+        instrument(currentStreamId, "restart")
     {
         // Voided streams cannot be restarted.
         vm.assume(!flow.isVoided(currentStreamId));
@@ -215,11 +215,11 @@ contract FlowHandler is BaseHandler {
         uint256 streamIndex
     )
         external
-        instrument("void")
         useFuzzedStream(streamIndex)
         useFuzzedStreamRecipient
         adjustTimestamp(timeJump)
         updateFlowHandlerStates
+        instrument(currentStreamId, "void")
     {
         // Voided streams cannot be voided again.
         vm.assume(!flow.isVoided(currentStreamId));
@@ -238,11 +238,11 @@ contract FlowHandler is BaseHandler {
         uint128 amount
     )
         external
-        instrument("withdraw")
         useFuzzedStream(streamIndex)
         useFuzzedStreamRecipient
         adjustTimestamp(timeJump)
         updateFlowHandlerStates
+        instrument(currentStreamId, "withdraw")
     {
         // The protocol doesn't allow the withdrawal address to be the zero address.
         vm.assume(to != address(0));

test/utils/Utils.sol

@@ -22,7 +22,6 @@ abstract contract Utils is CommonBase, Constants, PRBMathUtils {
         returns (uint128 depositAmount)
     {
         uint128 maxDepositAmount = (UINT128_MAX - balance);
-
         if (decimals < 18) {
             maxDepositAmount = maxDepositAmount / uint128(10 ** (18 - decimals));
         }