Change types and add modifier to getter (#296)

* fix: use uint256 for scaledOngoingDebt

fix: add notNull in isTransferable getter
fix: use uint256 in depletionTimeOf

* test: remove undeeded down casting

* fix: use ">" in depletionTimeOf

refactor: use uint256 for snapshot debt

* address last review comments

* test: lower the rps bound range

benchmark/Flow.Gas.t.sol

@@ -89,7 +89,7 @@ contract Flow_Gas_Test is Integration_Test {
         );
 
         // Deposit amount on an incremented stream ID to make stream solvent.
-        deposit(++streamId, flow.uncoveredDebtOf(streamId) + DEPOSIT_AMOUNT_6D);
+        deposit(++streamId, uint128(flow.uncoveredDebtOf(streamId)) + DEPOSIT_AMOUNT_6D);
 
         // {flow.withdraw} (on a solvent stream).
         computeGas(

benchmark/results/SablierFlow.md

@@ -2,15 +2,15 @@
 
 | Function                      | Gas Usage |
 | ----------------------------- | --------- |
-| `adjustRatePerSecond`         | 44009     |
-| `create`                      | 113794    |
-| `deposit`                     | 30152     |
-| `depositViaBroker`            | 22142     |
-| `pause`                       | 9340      |
-| `refund`                      | 11671     |
-| `restart`                     | 7106      |
-| `void (solvent stream)`       | 10389     |
-| `void (insolvent stream)`     | 36795     |
-| `withdraw (insolvent stream)` | 57634     |
-| `withdraw (solvent stream)`   | 40047     |
-| `withdrawMax`                 | 52200     |
+| `adjustRatePerSecond`         | 43628     |
+| `create`                      | 113659    |
+| `deposit`                     | 30035     |
+| `depositViaBroker`            | 21953     |
+| `pause`                       | 8983      |
+| `refund`                      | 11534     |
+| `restart`                     | 7031      |
+| `void (solvent stream)`       | 9517      |
+| `void (insolvent stream)`     | 36241     |
+| `withdraw (insolvent stream)` | 57034     |
+| `withdraw (solvent stream)`   | 39502     |
+| `withdrawMax`                 | 51379     |

src/SablierFlow.sol

@@ -60,7 +60,7 @@ contract SablierFlow is
         override
         notNull(streamId)
         notPaused(streamId)
-        returns (uint40 depletionTime)
+        returns (uint256 depletionTime)
     {
         uint128 balance = _streams[streamId].balance;
 
@@ -69,15 +69,15 @@ contract SablierFlow is
             return 0;
         }
 
-        uint128 snapshotDebt = _streams[streamId].snapshotDebt;
+        uint256 snapshotDebt = _streams[streamId].snapshotDebt;
 
         // If the stream has uncovered debt, return zero.
-        if (snapshotDebt + _ongoingDebtOf(streamId) >= balance) {
+        if (snapshotDebt + _ongoingDebtOf(streamId) > balance) {
             return 0;
         }
 
-        uint8 tokenDecimals = _streams[streamId].tokenDecimals;
-        uint128 solvencyAmount;
+        uint256 tokenDecimals = _streams[streamId].tokenDecimals;
+        uint256 solvencyAmount;
 
         // Depletion time is defined as the UNIX timestamp beyond which the total debt exceeds stream balance.
         // So we calculate it by solving: debt at depletion time = stream balance + 1. This ensures that we find the
@@ -87,16 +87,16 @@ contract SablierFlow is
             if (tokenDecimals == 18) {
                 solvencyAmount = (balance - snapshotDebt + 1);
             } else {
-                uint128 scaleFactor = (10 ** (18 - tokenDecimals)).toUint128();
+                uint256 scaleFactor = (10 ** (18 - tokenDecimals));
                 solvencyAmount = (balance - snapshotDebt + 1) * scaleFactor;
             }
             uint256 solvencyPeriod = solvencyAmount / _streams[streamId].ratePerSecond.unwrap();
-            return _streams[streamId].snapshotTime + solvencyPeriod.toUint40();
+            return _streams[streamId].snapshotTime + solvencyPeriod;
         }
     }
 
     /// @inheritdoc ISablierFlow
-    function ongoingDebtOf(uint256 streamId) external view override notNull(streamId) returns (uint128 ongoingDebt) {
+    function ongoingDebtOf(uint256 streamId) external view override notNull(streamId) returns (uint256 ongoingDebt) {
         ongoingDebt = _ongoingDebtOf(streamId);
     }
 
@@ -141,7 +141,7 @@ contract SablierFlow is
     }
 
     /// @inheritdoc ISablierFlow
-    function totalDebtOf(uint256 streamId) external view override notNull(streamId) returns (uint128 totalDebt) {
+    function totalDebtOf(uint256 streamId) external view override notNull(streamId) returns (uint256 totalDebt) {
         totalDebt = _totalDebtOf(streamId);
     }
 
@@ -151,7 +151,7 @@ contract SablierFlow is
         view
         override
         notNull(streamId)
-        returns (uint128 uncoveredDebt)
+        returns (uint256 uncoveredDebt)
     {
         uncoveredDebt = _uncoveredDebtOf(streamId);
     }
@@ -437,41 +437,42 @@ contract SablierFlow is
             return 0;
         }
 
-        uint128 totalDebt = _totalDebtOf(streamId);
+        uint256 totalDebt = _totalDebtOf(streamId);
 
         // If the stream balance is less than or equal to the total debt, return the stream balance.
         if (balance < totalDebt) {
             return balance;
         }
 
-        return totalDebt;
+        // At this point, the total debt fits within `uint128`, as it is less than or equal to the balance.
+        return totalDebt.toUint128();
     }
 
     /// @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) {
+    function _ongoingDebtOf(uint256 streamId) internal view returns (uint256 ongoingDebt) {
         uint40 blockTimestamp = uint40(block.timestamp);
         uint40 snapshotTime = _streams[streamId].snapshotTime;
 
-        uint128 ratePerSecond = _streams[streamId].ratePerSecond.unwrap();
+        uint256 ratePerSecond = _streams[streamId].ratePerSecond.unwrap();
 
         // Check:if the rate per second is zero or the `block.timestamp` is less than the `snapshotTime`.
         if (ratePerSecond == 0 || blockTimestamp <= snapshotTime) {
             return 0;
         }
 
-        uint128 elapsedTime;
+        uint256 elapsedTime;
 
         // Safe to use unchecked because subtraction cannot underflow.
         unchecked {
             // Calculate time elapsed since the last snapshot.
             elapsedTime = blockTimestamp - snapshotTime;
         }
 
-        uint8 tokenDecimals = _streams[streamId].tokenDecimals;
-
         // Calculate the ongoing debt accrued by multiplying the elapsed time by the rate per second.
-        uint128 scaledOngoingDebt = elapsedTime * ratePerSecond;
+        uint256 scaledOngoingDebt = elapsedTime * ratePerSecond;
+
+        uint8 tokenDecimals = _streams[streamId].tokenDecimals;
 
         // If the token decimals are 18, return the scaled ongoing debt and the `block.timestamp`.
         if (tokenDecimals == 18) {
@@ -480,7 +481,7 @@ contract SablierFlow is
 
         // Safe to use unchecked because we use {SafeCast}.
         unchecked {
-            uint128 scaleFactor = (10 ** (18 - tokenDecimals)).toUint128();
+            uint256 scaleFactor = 10 ** (18 - tokenDecimals);
             // Since debt is denoted in token decimals, descale the amount.
             ongoingDebt = scaledOngoingDebt / scaleFactor;
         }
@@ -494,16 +495,16 @@ contract SablierFlow is
     /// @notice Calculates the total debt.
     /// @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) internal view returns (uint128) {
+    function _totalDebtOf(uint256 streamId) internal view returns (uint256) {
         // Calculate the total debt.
         return _streams[streamId].snapshotDebt + _ongoingDebtOf(streamId);
     }
 
     /// @dev Calculates the uncovered debt.
-    function _uncoveredDebtOf(uint256 streamId) internal view returns (uint128) {
+    function _uncoveredDebtOf(uint256 streamId) internal view returns (uint256) {
         uint128 balance = _streams[streamId].balance;
 
-        uint128 totalDebt = _totalDebtOf(streamId);
+        uint256 totalDebt = _totalDebtOf(streamId);
 
         if (balance < totalDebt) {
             return totalDebt - balance;
@@ -523,7 +524,7 @@ contract SablierFlow is
             revert Errors.SablierFlow_RatePerSecondNotDifferent(streamId, newRatePerSecond);
         }
 
-        uint128 ongoingDebt = _ongoingDebtOf(streamId);
+        uint256 ongoingDebt = _ongoingDebtOf(streamId);
 
         // Update the snapshot debt only if the stream has ongoing debt.
         if (ongoingDebt > 0) {
@@ -709,11 +710,11 @@ contract SablierFlow is
             revert Errors.SablierFlow_Unauthorized({ streamId: streamId, caller: msg.sender });
         }
 
-        uint128 debtToWriteOff = _uncoveredDebtOf(streamId);
+        uint256 debtToWriteOff = _uncoveredDebtOf(streamId);
 
         // If the stream is solvent, update the total debt normally.
         if (debtToWriteOff == 0) {
-            uint128 ongoingDebt = _ongoingDebtOf(streamId);
+            uint256 ongoingDebt = _ongoingDebtOf(streamId);
             if (ongoingDebt > 0) {
                 // Effect: Update the snapshot debt by adding the ongoing debt.
                 _streams[streamId].snapshotDebt += ongoingDebt;
@@ -764,7 +765,7 @@ contract SablierFlow is
         }
 
         // Calculate the total debt.
-        uint128 totalDebt = _totalDebtOf(streamId);
+        uint256 totalDebt = _totalDebtOf(streamId);
 
         // Calculate the withdrawable amount.
         uint128 balance = _streams[streamId].balance;
@@ -775,7 +776,7 @@ contract SablierFlow is
             withdrawableAmount = balance;
         } else {
             // Otherwise, the withdrawable amount is the total debt.
-            withdrawableAmount = totalDebt;
+            withdrawableAmount = totalDebt.toUint128();
         }
 
         // Check: the withdraw amount is not greater than the withdrawable amount.

src/abstracts/SablierFlowBase.sol

@@ -139,7 +139,7 @@ abstract contract SablierFlowBase is
         view
         override
         notNull(streamId)
-        returns (uint128 snapshotDebt)
+        returns (uint256 snapshotDebt)
     {
         snapshotDebt = _streams[streamId].snapshotDebt;
     }
@@ -181,7 +181,7 @@ abstract contract SablierFlowBase is
     }
 
     /// @inheritdoc ISablierFlowBase
-    function isTransferable(uint256 streamId) external view override returns (bool result) {
+    function isTransferable(uint256 streamId) external view override notNull(streamId) returns (bool result) {
         result = _streams[streamId].isTransferable;
     }
 

src/interfaces/ISablierFlow.sol

@@ -24,7 +24,7 @@ interface ISablierFlow is
     /// @param newRatePerSecond The new rate per second, denoted as a fixed-point number where 1e18 is 1 token
     /// per second.
     event AdjustFlowStream(
-        uint256 indexed streamId, uint128 totalDebt, UD21x18 oldRatePerSecond, UD21x18 newRatePerSecond
+        uint256 indexed streamId, uint256 totalDebt, UD21x18 oldRatePerSecond, UD21x18 newRatePerSecond
     );
 
     /// @notice Emitted when a Flow stream is created.
@@ -56,7 +56,7 @@ interface ISablierFlow is
     /// @param recipient The address of the stream's recipient.
     /// @param totalDebt The amount of tokens owed by the sender to the recipient, denoted in token's decimals.
     event PauseFlowStream(
-        uint256 indexed streamId, address indexed sender, address indexed recipient, uint128 totalDebt
+        uint256 indexed streamId, address indexed sender, address indexed recipient, uint256 totalDebt
     );
 
     /// @notice Emitted when a sender is refunded from a stream.
@@ -84,8 +84,8 @@ interface ISablierFlow is
         address indexed sender,
         address indexed recipient,
         address caller,
-        uint128 newTotalDebt,
-        uint128 writtenOffDebt
+        uint256 newTotalDebt,
+        uint256 writtenOffDebt
     );
 
     /// @notice Emitted when tokens are withdrawn from a stream by a recipient or an approved operator.
@@ -119,12 +119,12 @@ interface ISablierFlow is
     /// there already is uncovered debt, it returns zero.
     /// @dev Reverts if `streamId` references a paused or a null stream.
     /// @param streamId The stream ID for the query.
-    function depletionTimeOf(uint256 streamId) external view returns (uint40 depletionTime);
+    function depletionTimeOf(uint256 streamId) external view returns (uint256 depletionTime);
 
     /// @notice Returns the amount of debt accrued since the snapshot time until now, denoted in token's decimals.
     /// @dev Reverts if `streamId` references a null stream.
     /// @param streamId The stream ID for the query.
-    function ongoingDebtOf(uint256 streamId) external view returns (uint128 ongoingDebt);
+    function ongoingDebtOf(uint256 streamId) external view returns (uint256 ongoingDebt);
 
     /// @notice Returns the amount that the sender can be refunded from the stream, denoted in token's decimals.
     /// @dev Reverts if `streamId` references a null stream.
@@ -139,12 +139,12 @@ interface ISablierFlow is
     /// @notice Returns the total amount owed by the sender to the recipient, denoted in token's decimals.
     /// @dev Reverts if `streamId` references a null stream.
     /// @param streamId The stream ID for the query.
-    function totalDebtOf(uint256 streamId) external view returns (uint128 totalDebt);
+    function totalDebtOf(uint256 streamId) external view returns (uint256 totalDebt);
 
     /// @notice Returns the amount of debt not covered by the stream balance, denoted in token's decimals.
     /// @dev Reverts if `streamId` references a null stream.
     /// @param streamId The stream ID for the query.
-    function uncoveredDebtOf(uint256 streamId) external view returns (uint128 uncoveredDebt);
+    function uncoveredDebtOf(uint256 streamId) external view returns (uint256 uncoveredDebt);
 
     /// @notice Calculates the amount that the recipient can withdraw from the stream, denoted in token decimals. This
     /// is an alias for `coveredDebtOf`.

src/interfaces/ISablierFlowBase.sol

@@ -87,7 +87,7 @@ interface ISablierFlowBase is
     /// @notice Retrieves the snapshot debt of the stream, denoted in token's decimals.
     /// @dev Reverts if `streamId` references a null stream.
     /// @param streamId The stream ID for the query.
-    function getSnapshotDebt(uint256 streamId) external view returns (uint128 snapshotDebt);
+    function getSnapshotDebt(uint256 streamId) external view returns (uint256 snapshotDebt);
 
     /// @notice Retrieves the snapshot time of the stream, which is a Unix timestamp.
     /// @dev Reverts if `streamId` references a null stream.

src/types/DataTypes.sol

@@ -72,6 +72,6 @@ library Flow {
         IERC20 token;
         uint8 tokenDecimals;
         // slot 3
-        uint128 snapshotDebt;
+        uint256 snapshotDebt;
     }
 }

tests/fork/Flow.t.sol

@@ -49,20 +49,20 @@ contract Flow_Fork_Test is Fork_Test {
         uint256 actualAggregateAmount;
         UD21x18 actualRatePerSecond;
         uint40 actualSnapshotTime;
-        uint128 actualSnapshotDebt;
+        uint256 actualSnapshotDebt;
         uint128 actualStreamBalance;
         uint256 actualStreamId;
         uint256 actualTokenBalance;
-        uint128 actualTotalDebt;
+        uint256 actualTotalDebt;
         // Expected values.
         uint256 expectedAggregateAmount;
         UD21x18 expectedRatePerSecond;
         uint40 expectedSnapshotTime;
-        uint128 expectedSnapshotDebt;
+        uint256 expectedSnapshotDebt;
         uint128 expectedStreamBalance;
         uint256 expectedStreamId;
         uint256 expectedTokenBalance;
-        uint128 expectedTotalDebt;
+        uint256 expectedTotalDebt;
     }
 
     /*//////////////////////////////////////////////////////////////////////////
@@ -251,9 +251,9 @@ contract Flow_Fork_Test is Fork_Test {
             newRatePerSecond = ud21x18(newRatePerSecond.unwrap() + 1);
         }
 
-        uint128 beforeSnapshotAmount = flow.getSnapshotDebt(streamId);
-        uint128 totalDebt = flow.totalDebtOf(streamId);
-        uint128 ongoingDebt = flow.ongoingDebtOf(streamId);
+        uint256 beforeSnapshotAmount = flow.getSnapshotDebt(streamId);
+        uint256 totalDebt = flow.totalDebtOf(streamId);
+        uint256 ongoingDebt = flow.ongoingDebtOf(streamId);
 
         // Compute the snapshot time that will be stored post withdraw.
         vars.expectedSnapshotTime = getBlockTimestamp();
@@ -443,7 +443,7 @@ contract Flow_Fork_Test is Fork_Test {
         // If the refundable amount less than 1, deposit some funds.
         if (flow.refundableAmountOf(streamId) <= 1) {
             uint128 depositAmount =
-                flow.uncoveredDebtOf(streamId) + getDefaultDepositAmount(flow.getTokenDecimals(streamId));
+                uint128(flow.uncoveredDebtOf(streamId)) + getDefaultDepositAmount(flow.getTokenDecimals(streamId));
             depositOnStream(streamId, depositAmount);
         }
 
@@ -527,8 +527,8 @@ contract Flow_Fork_Test is Fork_Test {
         // Make sure the requirements are respected.
         address sender = flow.getSender(streamId);
         address recipient = flow.getRecipient(streamId);
-        uint128 uncoveredDebt = flow.uncoveredDebtOf(streamId);
-        uint128 expectedTotalDebt;
+        uint256 uncoveredDebt = flow.uncoveredDebtOf(streamId);
+        uint256 expectedTotalDebt;
 
         resetPrank({ msgSender: sender });
 
@@ -583,7 +583,7 @@ contract Flow_Fork_Test is Fork_Test {
         );
 
         uint256 initialTokenBalance = token.balanceOf(address(flow));
-        uint128 totalDebt = flow.totalDebtOf(streamId);
+        uint256 totalDebt = flow.totalDebtOf(streamId);
 
         vars.expectedSnapshotTime =
             withdrawAmount <= flow.getSnapshotDebt(streamId) ? flow.getSnapshotTime(streamId) : getBlockTimestamp();

tests/integration/concrete/adjust-rate-per-second/adjustRatePerSecond.t.sol

@@ -95,7 +95,7 @@ contract AdjustRatePerSecond_Integration_Concrete_Test is Integration_Test {
         uint40 expectedSnapshotTime = getBlockTimestamp() - ONE_MONTH;
         assertEq(actualSnapshotTime, expectedSnapshotTime, "snapshot time");
 
-        uint128 actualSnapshotDebt = flow.getSnapshotDebt(defaultStreamId);
+        uint256 actualSnapshotDebt = flow.getSnapshotDebt(defaultStreamId);
         uint128 expectedSnapshotDebt = 0;
         assertEq(actualSnapshotDebt, expectedSnapshotDebt, "snapshot debt");