refactor: say "recipientAmount" instead of sum in _withdrawableAmount…

…Of function test: be more precise for caller test branches test: remove unneeded test for pause function docs: correct invariants
sablier-labs · andreivladbrg · May 24, 2024 · May 17, 2024 · May 18, 2024 · May 20, 2024
commit 886c5d20d422585cdbe40b048613ec4613acae9b
diff --git a/README.md b/README.md
@@ -84,7 +84,7 @@ The refundable amount is calculated by subtracting the streamed amount from the
 | streamedAmount     | sa           |
 | debt               | debt         |
 | withdrawableAmount | wa           |
-| refundableAmount   | ra           |
+| refundableAmount   | rfa          |
 
 ### Issues:
 
@@ -147,11 +147,9 @@ _wa ≤ bal_
 
 _if(debt = 0) then wa = sa + ra_
 
-_if(debt > 0 && ra <= bal) then wa = ra_
+_if(debt = 0 && isPaused = true) then wa = ra_
 
-_if(debt > 0 && ra > bal) then wa = bal_
-
-_ra ≤ sa(from create time to now)_
+_if(debt > 0) then wa = bal_
 
 _bal = sum of deposits - sum of withdrawals_
 

diff --git a/src/SablierV2OpenEnded.sol b/src/SablierV2OpenEnded.sol
@@ -379,13 +379,13 @@ contract SablierV2OpenEnded is
         // Calculate the streamed amount since last update.
         uint128 streamedAmount = _streamedAmountOf(streamId, time);
 
-        uint128 sum = streamedAmount + remainingAmount;
+        uint128 recipientAmount = streamedAmount + remainingAmount;
 
         // If there has been streamed more than how much is available, return the stream balance.
-        if (sum > balance) {
+        if (recipientAmount > balance) {
             return balance;
         } else {
-            return sum;
+            return recipientAmount;
         }
     }
 

diff --git a/test/integration/adjust-rate-per-second/adjustRatePerSecond.t.sol b/test/integration/adjust-rate-per-second/adjustRatePerSecond.t.sol
@@ -32,7 +32,7 @@ contract AdjustRatePerSecond_Integration_Test is Integration_Test {
         whenNotDelegateCalled
         givenNotNull
         givenNotPaused
-        whenCallerUnauthorized
+        whenCallerIsNotTheSender
     {
         resetPrank({ msgSender: users.recipient });
         vm.expectRevert(
@@ -46,7 +46,7 @@ contract AdjustRatePerSecond_Integration_Test is Integration_Test {
         whenNotDelegateCalled
         givenNotNull
         givenNotPaused
-        whenCallerUnauthorized
+        whenCallerIsNotTheSender
     {
         resetPrank({ msgSender: users.eve });
         vm.expectRevert(
@@ -60,7 +60,7 @@ contract AdjustRatePerSecond_Integration_Test is Integration_Test {
         whenNotDelegateCalled
         givenNotNull
         givenNotPaused
-        whenCallerAuthorized
+        whenCallerIsTheSender
     {
         vm.expectRevert(Errors.SablierV2OpenEnded_RatePerSecondZero.selector);
         openEnded.adjustRatePerSecond({ streamId: defaultStreamId, newRatePerSecond: 0 });
@@ -71,7 +71,7 @@ contract AdjustRatePerSecond_Integration_Test is Integration_Test {
         whenNotDelegateCalled
         givenNotNull
         givenNotPaused
-        whenCallerAuthorized
+        whenCallerIsTheSender
         whenRatePerSecondNonZero
     {
         vm.expectRevert(
@@ -85,7 +85,7 @@ contract AdjustRatePerSecond_Integration_Test is Integration_Test {
         whenNotDelegateCalled
         givenNotNull
         givenNotPaused
-        whenCallerAuthorized
+        whenCallerIsTheSender
         whenRatePerSecondNonZero
         whenRatePerSecondNotDifferent
     {
@@ -131,7 +131,7 @@ contract AdjustRatePerSecond_Integration_Test is Integration_Test {
         whenNotDelegateCalled
         givenNotNull
         givenNotPaused
-        whenCallerAuthorized
+        whenCallerIsTheSender
     {
         openEnded.deposit(defaultStreamId, DEPOSIT_AMOUNT);
         vm.warp({ newTimestamp: WARP_ONE_MONTH });

diff --git a/test/integration/adjust-rate-per-second/adjustRatePerSecond.tree b/test/integration/adjust-rate-per-second/adjustRatePerSecond.tree
@@ -8,12 +8,12 @@ adjustRatePerSecond.t.sol
       ├── given the id references a paused stream
       │  └── it should revert
       └── given the id does not reference a paused stream
-          ├── when the caller is unauthorized
+          ├── when the caller is not the sender
           │  ├── when the caller is the recipient
           │  │  └── it should revert
           │  └── when the caller is a malicious third party
           │     └── it should revert
-          └── when the caller is authorized
+          └── when the caller is the sender
               ├── when the provided rate per second is zero
               │  └── it should revert
               └── when the provided rate per second is not zero

diff --git a/test/integration/pause/pause.t.sol b/test/integration/pause/pause.t.sol
@@ -28,12 +28,12 @@ contract Pause_Integration_Test is Integration_Test {
         openEnded.pause(defaultStreamId);
     }
 
-    function test_RevertWhen_CallerUnauthorized_Recipient()
+    function test_RevertWhen_CallerRecipient()
         external
         whenNotDelegateCalled
         givenNotNull
         givenNotPaused
-        whenCallerUnauthorized
+        whenCallerIsNotTheSender
     {
         resetPrank({ msgSender: users.recipient });
         vm.expectRevert(
@@ -42,12 +42,12 @@ contract Pause_Integration_Test is Integration_Test {
         openEnded.pause(defaultStreamId);
     }
 
-    function test_RevertWhen_CallerUnauthorized_MaliciousThirdParty()
+    function test_RevertWhen_CallerMaliciousThirdParty()
         external
         whenNotDelegateCalled
         givenNotNull
         givenNotPaused
-        whenCallerUnauthorized
+        whenCallerIsNotTheSender
     {
         resetPrank({ msgSender: users.eve });
         vm.expectRevert(
@@ -56,12 +56,12 @@ contract Pause_Integration_Test is Integration_Test {
         openEnded.pause(defaultStreamId);
     }
 
-    function test_Pause_WithdrawableAmountZero()
+    function test_Pause_StreamHasDebt()
         external
         whenNotDelegateCalled
         givenNotNull
         givenNotPaused
-        whenCallerAuthorized
+        whenCallerIsTheSender
     {
         assertEq(openEnded.refundableAmountOf(defaultStreamId), 0, "refundable amount before pause");
         assertEq(openEnded.withdrawableAmountOf(defaultStreamId), 0, "withdrawable amount before pause");
@@ -81,40 +81,12 @@ contract Pause_Integration_Test is Integration_Test {
         assertEq(actualStreamBalance, 0, "stream balance");
     }
 
-    function test_Pause_RefundableAmountZero()
-        external
-        whenNotDelegateCalled
-        givenNotNull
-        givenNotPaused
-        whenCallerAuthorized
-        givenWithdrawableAmountNotZero
-    {
-        openEnded.deposit(defaultStreamId, WITHDRAW_AMOUNT);
-
-        assertEq(openEnded.getBalance(defaultStreamId), WITHDRAW_AMOUNT, "balance before");
-        assertEq(openEnded.withdrawableAmountOf(defaultStreamId), WITHDRAW_AMOUNT, "withdrawable amount before pause");
-
-        openEnded.pause(defaultStreamId);
-
-        assertTrue(openEnded.isPaused(defaultStreamId), "is paused");
-
-        uint128 actualRatePerSecond = openEnded.getRatePerSecond(defaultStreamId);
-        assertEq(actualRatePerSecond, 0, "rate per second");
-
-        uint128 actualRemainingAmount = openEnded.getRemainingAmount(defaultStreamId);
-        uint128 expectedRemainingAmount = ONE_MONTH_STREAMED_AMOUNT;
-        assertEq(actualRemainingAmount, expectedRemainingAmount, "remaining amount");
-
-        uint128 actualStreamBalance = openEnded.getBalance(defaultStreamId);
-        assertEq(actualStreamBalance, WITHDRAW_AMOUNT, "stream balance");
-    }
-
     function test_Pause()
         external
         whenNotDelegateCalled
         givenNotNull
         givenNotPaused
-        whenCallerAuthorized
+        whenCallerIsTheSender
         givenWithdrawableAmountNotZero
         givenRefundableAmountNotZero
     {

diff --git a/test/integration/pause/pause.tree b/test/integration/pause/pause.tree
@@ -8,23 +8,19 @@ pause.t.sol
       ├── given the id references a paused stream
       │  └── it should revert
       └── given the id does not reference a paused stream
-          ├── when the caller is unauthorized
+          ├── when the caller is not the sender
           │  ├── when the caller is the recipient
           │  │  └── it should revert
           │  └── when the caller is a malicious third party
           │     └── it should revert
-          └── when the caller is authorized
-             ├── given the withdrawable amount is zero
+          └── when the caller is the sender
+             ├── given the stream has debt
              │  ├── it should pause the stream
-             │  └── it should set the rate per second to zero
-             └── given the withdrawable amount is not zero
-                ├── given the refundable amount is zero
-                │  ├── it should pause the stream
-                │  ├── it should set the rate per second to zero
-                │  └── it should update the remaining amount
-                └── given thet refundable amount is not zero
-                   ├── it should pause the stream
-                   ├── it should set the rate per second to zero
-                   ├── it should update the remaining amount
-                   ├── it should emit a {PauseOpenEndedStream} event
-                   └── it should emit a {MetadataUpdate} event
+             │  ├── it should set the rate per second to zero
+             │  └── it should update the remaining amount
+             └── given the stream does not have debt
+                ├── it should pause the stream
+                ├── it should set the rate per second to zero
+                ├── it should update the remaining amount
+                ├── it should emit a {PauseOpenEndedStream} event
+                └── it should emit a {MetadataUpdate} event
diff --git a/test/integration/refund-from-stream/refundFromStream.t.sol b/test/integration/refund-from-stream/refundFromStream.t.sol
@@ -31,7 +31,7 @@ contract RefundFromStream_Integration_Test is Integration_Test {
         external
         whenNotDelegateCalled
         givenNotNull
-        whenCallerUnauthorized
+        whenCallerIsNotTheSender
     {
         resetPrank({ msgSender: users.recipient });
         vm.expectRevert(
@@ -44,7 +44,7 @@ contract RefundFromStream_Integration_Test is Integration_Test {
         external
         whenNotDelegateCalled
         givenNotNull
-        whenCallerUnauthorized
+        whenCallerIsNotTheSender
     {
         resetPrank({ msgSender: users.eve });
         vm.expectRevert(
@@ -53,7 +53,7 @@ contract RefundFromStream_Integration_Test is Integration_Test {
         openEnded.refundFromStream({ streamId: defaultStreamId, amount: REFUND_AMOUNT });
     }
 
-    function test_RevertWhen_RefundAmountZero() external whenNotDelegateCalled givenNotNull whenCallerAuthorized {
+    function test_RevertWhen_RefundAmountZero() external whenNotDelegateCalled givenNotNull whenCallerIsTheSender {
         vm.expectRevert(Errors.SablierV2OpenEnded_RefundAmountZero.selector);
         openEnded.refundFromStream({ streamId: defaultStreamId, amount: 0 });
     }
@@ -62,7 +62,7 @@ contract RefundFromStream_Integration_Test is Integration_Test {
         external
         whenNotDelegateCalled
         givenNotNull
-        whenCallerAuthorized
+        whenCallerIsTheSender
         whenRefundAmountNotZero
     {
         vm.expectRevert(
@@ -76,7 +76,7 @@ contract RefundFromStream_Integration_Test is Integration_Test {
         openEnded.refundFromStream({ streamId: defaultStreamId, amount: DEPOSIT_AMOUNT });
     }
 
-    function test_RefundFromStream_PausedStream() external whenNotDelegateCalled givenNotNull whenCallerAuthorized {
+    function test_RefundFromStream_PausedStream() external whenNotDelegateCalled givenNotNull whenCallerIsTheSender {
         openEnded.pause(defaultStreamId);
 
         expectCallToTransfer({ asset: dai, to: users.sender, amount: REFUND_AMOUNT });
@@ -91,7 +91,7 @@ contract RefundFromStream_Integration_Test is Integration_Test {
         external
         whenNotDelegateCalled
         givenNotNull
-        whenCallerAuthorized
+        whenCallerIsTheSender
         whenRefundAmountNotZero
         whenNoOverrefund
     {
@@ -108,7 +108,7 @@ contract RefundFromStream_Integration_Test is Integration_Test {
         external
         whenNotDelegateCalled
         givenNotNull
-        whenCallerAuthorized
+        whenCallerIsTheSender
         whenRefundAmountNotZero
         whenNoOverrefund
     {

diff --git a/test/integration/refund-from-stream/refundFromStream.tree b/test/integration/refund-from-stream/refundFromStream.tree
@@ -5,12 +5,12 @@ refundFromStream.t.sol
    ├── given the id references a null stream
    │  └── it should revert
    └── given the id does not reference a null stream
-      ├── when the caller is unauthorized
+      ├── when the caller is not the sender
       │  ├── when the caller is the recipient
       │  │  └── it should revert
       │  └── when the caller is a malicious third party
       │     └── it should revert
-      └── when the caller is authorized
+      └── when the caller is the sender
          ├── when the refund amount is zero
          │  └── it should revert
          └── when the refund amount is not zero

diff --git a/test/integration/restart-stream/restartStream.t.sol b/test/integration/restart-stream/restartStream.t.sol
@@ -34,7 +34,7 @@ contract RestartStream_Integration_Test is Integration_Test {
         whenNotDelegateCalled
         givenNotNull
         givenPaused
-        whenCallerUnauthorized
+        whenCallerIsNotTheSender
     {
         resetPrank({ msgSender: users.recipient });
         vm.expectRevert(
@@ -48,7 +48,7 @@ contract RestartStream_Integration_Test is Integration_Test {
         whenNotDelegateCalled
         givenNotNull
         givenPaused
-        whenCallerUnauthorized
+        whenCallerIsNotTheSender
     {
         resetPrank({ msgSender: users.eve });
         vm.expectRevert(
@@ -62,7 +62,7 @@ contract RestartStream_Integration_Test is Integration_Test {
         whenNotDelegateCalled
         givenNotNull
         givenPaused
-        whenCallerAuthorized
+        whenCallerIsTheSender
     {
         vm.expectRevert(Errors.SablierV2OpenEnded_RatePerSecondZero.selector);
         openEnded.restartStream({ streamId: defaultStreamId, ratePerSecond: 0 });
@@ -73,7 +73,7 @@ contract RestartStream_Integration_Test is Integration_Test {
         whenNotDelegateCalled
         givenNotNull
         givenPaused
-        whenCallerAuthorized
+        whenCallerIsTheSender
         whenRatePerSecondNonZero
     {
         vm.expectEmit({ emitter: address(openEnded) });

diff --git a/test/integration/restart-stream/restartStream.tree b/test/integration/restart-stream/restartStream.tree
@@ -8,12 +8,12 @@ restartStream.t.sol
       ├── given the id does not references a paused stream
       │  └── it should revert
       └── given the id reference a paused stream
-         ├── when the caller is unauthorized
+         ├── when the caller is not the sender
          │  ├── when the caller is the sender
          │  │  └── it should revert
          │  └── when the caller is a malicious third party
          │     └── it should revert
-         └── when the caller is authorized
+         └── when the caller is the sender
             ├── when the provided rate per second is zero
             │  └── it should revert
             └── when the provided rate per second is not zero

diff --git a/test/invariant/handlers/OpenEndedCreateHandler.sol b/test/invariant/handlers/OpenEndedCreateHandler.sol
@@ -70,7 +70,7 @@ contract OpenEndedCreateHandler is BaseHandler {
             openEnded.create(params.sender, params.recipient, params.ratePerSecond, asset, params.isTransferable);
 
         // Store the stream id.
-        openEndedStore.pushStreamId(streamId, params.sender, params.recipient, openEnded.getLastTimeUpdate(streamId));
+        openEndedStore.pushStreamId(streamId, params.sender, params.recipient);
     }
 
     function createAndDeposit(
@@ -104,7 +104,7 @@ contract OpenEndedCreateHandler is BaseHandler {
         );
 
         // Store the stream id.
-        openEndedStore.pushStreamId(streamId, params.sender, params.recipient, openEnded.getLastTimeUpdate(streamId));
+        openEndedStore.pushStreamId(streamId, params.sender, params.recipient);
 
         // Store the deposited amount.
         openEndedStore.updateStreamDepositedAmountsSum(streamId, depositAmount);