Refactoring open ended

script/Base.s.sol

@@ -1,4 +1,5 @@
 // SPDX-License-Identifier: GPL-3.0-or-later
+// solhint-disable no-console
 pragma solidity >=0.8.22;
 
 import { Strings } from "@openzeppelin/contracts/utils/Strings.sol";
@@ -57,7 +58,7 @@ abstract contract BaseScript is Script {
         string memory json = vm.readFile("package.json");
         string memory version = json.readString(".version");
         string memory create2Salt = string.concat("ChainID ", chainId, ", Version ", version);
-        console2.log("The CREATE2 salt is \"%s\"", create2Salt);
+        console2.log("The CREATE2 salt is %s", create2Salt);
         return bytes32(abi.encodePacked(create2Salt));
     }
 }

src/SablierV2OpenEnded.sol

@@ -28,6 +28,7 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope
         view
         override
         notCanceled(streamId)
+        notNull(streamId)
         returns (uint128 refundableAmount)
     {
         refundableAmount = _refundableAmountOf(streamId, uint40(block.timestamp));
@@ -42,25 +43,40 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope
         view
         override
         notCanceled(streamId)
+        notNull(streamId)
         returns (uint128 refundableAmount)
     {
         refundableAmount = _refundableAmountOf(streamId, time);
     }
 
     /// @inheritdoc ISablierV2OpenEnded
-    function streamDebt(uint256 streamId) external view notCanceled(streamId) returns (uint128 debt) {
+    function streamDebtOf(uint256 streamId)
+        external
+        view
+        override
+        notCanceled(streamId)
+        notNull(streamId)
+        returns (uint128 debt)
+    {
         uint128 balance = _streams[streamId].balance;
         uint128 streamedAmount = _streamedAmountOf(streamId, uint40(block.timestamp));
 
-        if (balance >= streamedAmount) {
-            return 0;
+        if (balance < streamedAmount) {
+            return streamedAmount - balance;
         }
 
-        debt = streamedAmount - balance;
+        return 0;
     }
 
     /// @inheritdoc ISablierV2OpenEnded
-    function streamedAmountOf(uint256 streamId) external view notCanceled(streamId) returns (uint128 streamedAmount) {
+    function streamedAmountOf(uint256 streamId)
+        external
+        view
+        override
+        notCanceled(streamId)
+        notNull(streamId)
+        returns (uint128 streamedAmount)
+    {
         streamedAmount = _streamedAmountOf(streamId, uint40(block.timestamp));
     }
 
@@ -71,7 +87,9 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope
     )
         external
         view
+        override
         notCanceled(streamId)
+        notNull(streamId)
         returns (uint128 streamedAmount)
     {
         streamedAmount = _streamedAmountOf(streamId, time);
@@ -81,7 +99,9 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope
     function withdrawableAmountOf(uint256 streamId)
         external
         view
+        override
         notCanceled(streamId)
+        notNull(streamId)
         returns (uint128 withdrawableAmount)
     {
         withdrawableAmount = _withdrawableAmountOf(streamId, uint40(block.timestamp));
@@ -94,7 +114,9 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope
     )
         external
         view
+        override
         notCanceled(streamId)
+        notNull(streamId)
         returns (uint128 withdrawableAmount)
     {
         withdrawableAmount = _withdrawableAmountOf(streamId, time);
@@ -110,16 +132,25 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope
         uint128 newRatePerSecond
     )
         external
+        override
         noDelegateCall
         notCanceled(streamId)
+        notNull(streamId)
         onlySender(streamId)
     {
         // Effects and Interactions: adjust the stream.
         _adjustRatePerSecond(streamId, newRatePerSecond);
     }
 
     /// @inheritdoc ISablierV2OpenEnded
-    function cancel(uint256 streamId) public noDelegateCall notCanceled(streamId) onlySender(streamId) {
+    function cancel(uint256 streamId)
+        public
+        override
+        noDelegateCall
+        notCanceled(streamId)
+        notNull(streamId)
+        onlySender(streamId)
+    {
         _cancel(streamId);
     }
 
@@ -141,6 +172,7 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope
         IERC20 asset
     )
         external
+        override
         returns (uint256 streamId)
     {
         // Checks, Effects and Interactions: create the stream.
@@ -156,6 +188,7 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope
         uint128 depositAmount
     )
         external
+        override
         returns (uint256 streamId)
     {
         // Checks, Effects and Interactions: create the stream.
@@ -173,6 +206,7 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope
         IERC20 asset
     )
         public
+        override
         returns (uint256[] memory streamIds)
     {
         uint256 recipientsCount = recipients.length;
@@ -202,6 +236,7 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope
         uint128[] calldata depositAmounts
     )
         external
+        override
         returns (uint256[] memory streamIds)
     {
         streamIds = new uint256[](recipients.length);
@@ -211,13 +246,22 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope
     }
 
     /// @inheritdoc ISablierV2OpenEnded
-    function deposit(uint256 streamId, uint128 depositAmount) external noDelegateCall notCanceled(streamId) {
+    function deposit(
+        uint256 streamId,
+        uint128 depositAmount
+    )
+        external
+        override
+        noDelegateCall
+        notCanceled(streamId)
+        notNull(streamId)
+    {
         // Checks, Effects and Interactions: deposit on stream.
         _deposit(streamId, depositAmount);
     }
 
     /// @inheritdoc ISablierV2OpenEnded
-    function depositMultiple(uint256[] memory streamIds, uint128[] calldata amounts) public noDelegateCall {
+    function depositMultiple(uint256[] memory streamIds, uint128[] calldata amounts) public override noDelegateCall {
         uint256 streamIdsCount = streamIds.length;
         uint256 depositAmountsCount = amounts.length;
 
@@ -238,13 +282,20 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope
     }
 
     /// @inheritdoc ISablierV2OpenEnded
-    function restartStream(uint256 streamId, uint128 ratePerSecond) external {
+    function restartStream(uint256 streamId, uint128 ratePerSecond) external override {
         // Checks, Effects and Interactions: restart the stream.
         _restartStream(streamId, ratePerSecond);
     }
 
     /// @inheritdoc ISablierV2OpenEnded
-    function restartStreamAndDeposit(uint256 streamId, uint128 ratePerSecond, uint128 depositAmount) external {
+    function restartStreamAndDeposit(
+        uint256 streamId,
+        uint128 ratePerSecond,
+        uint128 depositAmount
+    )
+        external
+        override
+    {
         // Checks, Effects and Interactions: restart the stream.
         _restartStream(streamId, ratePerSecond);
 
@@ -258,16 +309,18 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope
         uint128 refundAmount
     )
         external
+        override
         noDelegateCall
         notCanceled(streamId)
+        notNull(streamId)
         onlySender(streamId)
     {
         // Checks, Effects and Interactions: make the refund.
         _refundFromStream(streamId, refundAmount);
     }
 
     /// @inheritdoc ISablierV2OpenEnded
-    function withdraw(uint256 streamId, address to, uint40 time) external {
+    function withdraw(uint256 streamId, address to, uint40 time) external override {
         // Checks, Effects and Interactions: make the withdrawal.
         _withdraw(streamId, to, time);
     }
@@ -289,7 +342,7 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope
     }
 
     /// @inheritdoc ISablierV2OpenEnded
-    function withdrawMax(uint256 streamId, address to) external {
+    function withdrawMax(uint256 streamId, address to) external override {
         // Checks, Effects and Interactions: make the withdrawal.
         _withdraw(streamId, to, uint40(block.timestamp));
     }
@@ -632,7 +685,16 @@ contract SablierV2OpenEnded is ISablierV2OpenEnded, NoDelegateCall, SablierV2Ope
     }
 
     /// @dev See the documentation for the user-facing functions that call this internal function.
-    function _withdraw(uint256 streamId, address to, uint40 time) internal noDelegateCall notCanceled(streamId) {
+    function _withdraw(
+        uint256 streamId,
+        address to,
+        uint40 time
+    )
+        internal
+        noDelegateCall
+        notCanceled(streamId)
+        notNull(streamId)
+    {
         // Check: the withdrawal address is not zero.
         if (to == address(0)) {
             revert Errors.SablierV2OpenEnded_WithdrawToZeroAddress();

src/abstracts/SablierV2OpenEndedState.sol

@@ -34,7 +34,7 @@ abstract contract SablierV2OpenEndedState is ISablierV2OpenEndedState {
 
     /// @dev Checks that `streamId` does not reference a canceled stream.
     modifier notCanceled(uint256 streamId) {
-        if (isCanceled(streamId)) {
+        if (_streams[streamId].isCanceled) {
             revert Errors.SablierV2OpenEnded_StreamCanceled(streamId);
         }
         _;
@@ -50,7 +50,7 @@ abstract contract SablierV2OpenEndedState is ISablierV2OpenEndedState {
 
     /// @dev Checks the `msg.sender` is the stream's sender.
     modifier onlySender(uint256 streamId) {
-        if (!_isCallerStreamSender(streamId)) {
+        if (msg.sender != _streams[streamId].sender) {
             revert Errors.SablierV2OpenEnded_Unauthorized(streamId, msg.sender);
         }
         _;
@@ -109,12 +109,18 @@ abstract contract SablierV2OpenEndedState is ISablierV2OpenEndedState {
     }
 
     /// @inheritdoc ISablierV2OpenEndedState
-    function getSender(uint256 streamId) external view notNull(streamId) returns (address sender) {
+    function getSender(uint256 streamId) external view override notNull(streamId) returns (address sender) {
         sender = _streams[streamId].sender;
     }
 
     /// @inheritdoc ISablierV2OpenEndedState
-    function getStream(uint256 streamId) external view notNull(streamId) returns (OpenEnded.Stream memory stream) {
+    function getStream(uint256 streamId)
+        external
+        view
+        override
+        notNull(streamId)
+        returns (OpenEnded.Stream memory stream)
+    {
         stream = _streams[streamId];
     }
 
@@ -124,17 +130,7 @@ abstract contract SablierV2OpenEndedState is ISablierV2OpenEndedState {
     }
 
     /// @inheritdoc ISablierV2OpenEndedState
-    function isStream(uint256 streamId) public view returns (bool result) {
+    function isStream(uint256 streamId) public view override returns (bool result) {
         result = _streams[streamId].isStream;
     }
-
-    /*//////////////////////////////////////////////////////////////////////////
-                            INTERNAL CONSTANT FUNCTIONS
-    //////////////////////////////////////////////////////////////////////////*/
-
-    /// @notice Checks whether `msg.sender` is the stream's sender.
-    /// @param streamId The stream ID for the query.
-    function _isCallerStreamSender(uint256 streamId) internal view returns (bool) {
-        return msg.sender == _streams[streamId].sender;
-    }
 }

src/interfaces/ISablierV2OpenEnded.sol

@@ -99,42 +99,44 @@ interface ISablierV2OpenEnded is ISablierV2OpenEndedState {
     //////////////////////////////////////////////////////////////////////////*/
 
     /// @notice Calculates the amount that the sender can refund from stream, denoted in 18 decimals.
-    /// @dev Reverts if `streamId` references a canceled stream.
+    /// @dev Reverts if `streamId` references a canceled or a null stream.
     /// @param streamId The stream ID for the query.
+    /// @return refundableAmount The amount that the sender can refund.
     function refundableAmountOf(uint256 streamId) external view returns (uint128 refundableAmount);
 
     /// @notice Calculates the amount that the sender can refund from stream at `time`, denoted in 18 decimals.
-    /// @dev Reverts if `streamId` references a canceled stream.
+    /// @dev Reverts if `streamId` references a canceled or a null stream.
     /// @param streamId The stream ID for the query.
     /// @param time The Unix timestamp for the streamed amount calculation.
+    /// @return refundableAmount The amount that the sender can refund.
     function refundableAmountOf(uint256 streamId, uint40 time) external view returns (uint128 refundableAmount);
 
     /// @notice Calculates the amount that the sender owes on the stream, i.e. if more assets have been streamed than
     /// its balance, denoted in 18 decimals. If there is no debt, it will return zero.
-    /// @dev Reverts if `streamId` references a canceled stream.
+    /// @dev Reverts if `streamId` references a canceled or a null stream.
     /// @param streamId The stream ID for the query.
-    function streamDebt(uint256 streamId) external view returns (uint128 debt);
+    function streamDebtOf(uint256 streamId) external view returns (uint128 debt);
 
     /// @notice Calculates the amount streamed to the recipient from the last time update to the current time,
     /// denoted in 18 decimals.
-    /// @dev Reverts if `streamId` references a canceled stream.
+    /// @dev Reverts if `streamId` references a canceled or a null stream.
     /// @param streamId The stream ID for the query.
     function streamedAmountOf(uint256 streamId) external view returns (uint128 streamedAmount);
 
     /// @notice Calculates the amount streamed to the recipient from the last time update to `time` passed as parameter,
     /// denoted in 18 decimals.
-    /// @dev Reverts if `streamId` references a canceled stream.
+    /// @dev Reverts if `streamId` references a canceled or a null stream.
     /// @param streamId The stream ID for the query.
     /// @param time The Unix timestamp for the streamed amount calculation.
     function streamedAmountOf(uint256 streamId, uint40 time) external view returns (uint128 streamedAmount);
 
     /// @notice Calculates the amount that the recipient can withdraw from the stream, denoted in 18 decimals.
-    /// @dev Reverts if `streamId` references a canceled stream.
+    /// @dev Reverts if `streamId` references a canceled or a null stream.
     /// @param streamId The stream ID for the query.
     function withdrawableAmountOf(uint256 streamId) external view returns (uint128 withdrawableAmount);
 
     /// @notice Calculates the amount that the recipient can withdraw from the stream at `time`, denoted in 18 decimals.
-    /// @dev Reverts if `streamId` references a canceled stream.
+    /// @dev Reverts if `streamId` references a canceled or a null stream.
     /// @param streamId The stream ID for the query.
     /// @param time The Unix timestamp for the streamed amount calculation.
     function withdrawableAmountOf(uint256 streamId, uint40 time) external view returns (uint128 withdrawableAmount);

test/integration/stream-debt/streamDebt.t.sol

@@ -10,23 +10,23 @@ contract StreamDebt_Integration_Test is Integration_Test {
 
     function test_RevertGiven_Null() external {
         expectRevertNull();
-        openEnded.streamDebt(nullStreamId);
+        openEnded.streamDebtOf(nullStreamId);
     }
 
     function test_RevertGiven_Canceled() external givenNotNull {
         expectRevertCanceled();
-        openEnded.streamDebt(defaultStreamId);
+        openEnded.streamDebtOf(defaultStreamId);
     }
 
     function test_StreamDebt_BalanceGreaterThanOrEqualStreamedAmount() external givenNotNull givenNotCanceled {
         defaultDeposit();
-        uint128 streamDebt = openEnded.streamDebt(defaultStreamId);
+        uint128 streamDebt = openEnded.streamDebtOf(defaultStreamId);
         assertEq(streamDebt, 0, "stream debt");
     }
 
-    function test_StreamDebt() external givenNotNull givenNotCanceled {
+    function test_streamDebtOf() external givenNotNull givenNotCanceled {
         vm.warp({ newTimestamp: WARP_ONE_MONTH });
-        uint128 streamDebt = openEnded.streamDebt(defaultStreamId);
+        uint128 streamDebt = openEnded.streamDebtOf(defaultStreamId);
         assertEq(streamDebt, ONE_MONTH_STREAMED_AMOUNT, "stream debt");
     }
 }