refactor: address feedback

src/core/LockupNFTDescriptor.sol

@@ -7,12 +7,12 @@ import { IERC721Metadata } from "@openzeppelin/contracts/token/ERC721/extensions
 import { Base64 } from "@openzeppelin/contracts/utils/Base64.sol";
 import { Strings } from "@openzeppelin/contracts/utils/Strings.sol";
 import { ILockupNFTDescriptor } from "./interfaces/ILockupNFTDescriptor.sol";
+import { ISablierLockup } from "./interfaces/ISablierLockup.sol";
 import { ISablierLockupBase } from "./interfaces/ISablierLockupBase.sol";
 import { Errors } from "./libraries/Errors.sol";
 import { NFTSVG } from "./libraries/NFTSVG.sol";
 import { SVGElements } from "./libraries/SVGElements.sol";
 import { Lockup } from "./types/DataTypes.sol";
-
 /*
 
 ██╗      ██████╗  ██████╗██╗  ██╗██╗   ██╗██████╗     ███╗   ██╗███████╗████████╗
@@ -49,7 +49,7 @@ contract LockupNFTDescriptor is ILockupNFTDescriptor {
         uint128 depositedAmount;
         bool isTransferable;
         string json;
-        ISablierLockupBase lockup;
+        ISablierLockup lockup;
         string lockupModel;
         string lockupStringified;
         bytes returnData;
@@ -64,7 +64,7 @@ contract LockupNFTDescriptor is ILockupNFTDescriptor {
         TokenURIVars memory vars;
 
         // Load the contracts.
-        vars.lockup = ISablierLockupBase(address(lockup));
+        vars.lockup = ISablierLockup(address(lockup));
         vars.lockupModel = mapSymbol(lockup);
         vars.lockupStringified = address(lockup).toHexString();
         vars.asset = address(vars.lockup.getAsset(streamId));
@@ -99,7 +99,7 @@ contract LockupNFTDescriptor is ILockupNFTDescriptor {
 
         // Performs a low-level call to handle older deployments that miss the `isTransferable` function.
         (vars.success, vars.returnData) =
-            address(vars.lockup).staticcall(abi.encodeCall(ISablierLockupBase.isTransferable, (streamId)));
+            address(vars.lockup).staticcall(abi.encodeCall(vars.lockup.isTransferable, (streamId)));
 
         // When the call has failed, the stream NFT is assumed to be transferable.
         vars.isTransferable = vars.success ? abi.decode(vars.returnData, (bool)) : true;

src/core/abstracts/SablierLockupBase.sol

@@ -7,6 +7,7 @@ import { ERC721 } from "@openzeppelin/contracts/token/ERC721/ERC721.sol";
 import { IERC721Metadata } from "@openzeppelin/contracts/token/ERC721/extensions/IERC721Metadata.sol";
 import { IERC165 } from "@openzeppelin/contracts/utils/introspection/IERC165.sol";
 import { UD60x18 } from "@prb/math/src/UD60x18.sol";
+
 import { ILockupNFTDescriptor } from "./../interfaces/ILockupNFTDescriptor.sol";
 import { ISablierLockupBase } from "./../interfaces/ISablierLockupBase.sol";
 import { ISablierLockupRecipient } from "./../interfaces/ISablierLockupRecipient.sol";
@@ -31,7 +32,8 @@ abstract contract SablierLockupBase is
                                   STATE VARIABLES
     //////////////////////////////////////////////////////////////////////////*/
 
-    /// @inheritdoc ISablierLockupBase
+    /// @dev The maximum broker fee that can be charged by the broker, denoted as a fixed-point number where
+    /// 1e18 is 100%.
     UD60x18 public constant override MAX_BROKER_FEE = UD60x18.wrap(0.1e18);
 
     /// @inheritdoc ISablierLockupBase
@@ -205,7 +207,7 @@ abstract contract SablierLockupBase is
 
     /// @inheritdoc ISablierLockupBase
     function streamedAmountOf(uint256 streamId)
-        public
+        external
         view
         override
         notNull(streamId)

src/core/interfaces/ISablierLockup.sol

@@ -23,8 +23,7 @@ interface ISablierLockup is ISablierLockupBase {
     /// @param asset The contract address of the ERC-20 asset to be distributed.
     /// @param cancelable Boolean indicating whether the stream is cancelable or not.
     /// @param transferable Boolean indicating whether the stream NFT is transferable or not.
-    /// @param timestamps Struct encapsulating (i) the stream's start time, (ii) cliff time, and (iii) end time, all as
-    /// Unix timestamps.
+    /// @param timestamps Struct encapsulating (i) the stream's start time and (ii) end time, all as Unix timestamps.
     /// @param broker The address of the broker who has helped create the stream, e.g. a front-end website.
     /// @param segments The segments the protocol uses to compose the dynamic distribution function.
     event CreateLockupDynamicStream(
@@ -37,8 +36,8 @@ interface ISablierLockup is ISablierLockupBase {
         bool cancelable,
         bool transferable,
         Lockup.Timestamps timestamps,
-        address broker,
-        LockupDynamic.Segment[] segments
+        LockupDynamic.Segment[] segments,
+        address broker
     );
 
     /// @notice Emitted when a stream is created using Lockup linear model.
@@ -51,8 +50,8 @@ interface ISablierLockup is ISablierLockupBase {
     /// @param asset The contract address of the ERC-20 asset to be distributed.
     /// @param cancelable Boolean indicating whether the stream is cancelable or not.
     /// @param transferable Boolean indicating whether the stream NFT is transferable or not.
-    /// @param timestamps Struct encapsulating (i) the stream's start time, (ii) cliff time, and (iii) end time, all as
-    /// Unix timestamps.
+    /// @param timestamps Struct encapsulating (i) the stream's start time and (ii) end time, all as Unix timestamps.
+    /// @param cliffTime The Unix timestamp for the cliff period's end. A value of zero means there is no cliff.
     /// @param broker The address of the broker who has helped create the stream, e.g. a front-end website.
     event CreateLockupLinearStream(
         uint256 streamId,
@@ -64,6 +63,7 @@ interface ISablierLockup is ISablierLockupBase {
         bool cancelable,
         bool transferable,
         Lockup.Timestamps timestamps,
+        uint40 cliffTime,
         address broker
     );
 
@@ -77,8 +77,7 @@ interface ISablierLockup is ISablierLockupBase {
     /// @param asset The contract address of the ERC-20 asset to be distributed.
     /// @param cancelable Boolean indicating whether the stream is cancelable or not.
     /// @param transferable Boolean indicating whether the stream NFT is transferable or not.
-    /// @param timestamps Struct encapsulating (i) the stream's start time, (ii) cliff time, and (iii) end time, all as
-    /// Unix timestamps.
+    /// @param timestamps Struct encapsulating (i) the stream's start time and (ii) end time, both as Unix timestamps.
     /// @param broker The address of the broker who has helped create the stream, e.g. a front-end website.
     /// @param tranches The tranches the protocol uses to compose the tranched distribution function.
     event CreateLockupTranchedStream(
@@ -91,8 +90,8 @@ interface ISablierLockup is ISablierLockupBase {
         bool cancelable,
         bool transferable,
         Lockup.Timestamps timestamps,
-        address broker,
-        LockupTranched.Tranche[] tranches
+        LockupTranched.Tranche[] tranches,
+        address broker
     );
 
     /*//////////////////////////////////////////////////////////////////////////
@@ -103,22 +102,17 @@ interface ISablierLockup is ISablierLockupBase {
     /// @dev This is initialized at construction time and cannot be changed later.
     function MAX_COUNT() external view returns (uint256);
 
-    /// @notice Retrieves the stream's cliff timestamp, which is a Unix timestamp. A zero value means no cliff.
-    /// @dev Reverts if `streamId` references a null stream.
+    /// @notice Retrieves the stream's cliff time, which is a Unix timestamp.  A value of zero means there
+    /// is no cliff.
+    /// @dev Reverts if `streamId` references a null stream or a non Lockup Linear stream.
     /// @param streamId The stream ID for the query.
-    function getCliffTime(uint256 streamId) external view returns (uint40 cliff);
+    function getCliffTime(uint256 streamId) external view returns (uint40 cliffTime);
 
     /// @notice Retrieves the segments used to compose the dynamic distribution function.
     /// @dev Reverts if `streamId` references a null stream or a non Lockup Dynamic stream.
     /// @param streamId The stream ID for the query.
     function getSegments(uint256 streamId) external view returns (LockupDynamic.Segment[] memory segments);
 
-    /// @notice Retrieves the stream's start time, cliff time and end time.
-    /// @dev Reverts if `streamId` references a null stream.
-    /// @param streamId The stream ID for the query.
-    /// @return timestamps See the documentation in {DataTypes}.
-    function getTimestamps(uint256 streamId) external view returns (Lockup.Timestamps memory timestamps);
-
     /// @notice Retrieves the tranches used to compose the tranched distribution function.
     /// @dev Reverts if `streamId` references a null stream or a non Lockup Tranched stream.
     /// @param streamId The stream ID for the query.
@@ -138,12 +132,12 @@ interface ISablierLockup is ISablierLockupBase {
     /// - All requirements in {createWithTimestampsLD} must be met for the calculated parameters.
     ///
     /// @param params Struct encapsulating the function parameters, which are documented in {DataTypes}.
-    /// @param segments Segments with durations used to compose the dynamic distribution function. Timestamps are
-    /// calculated by starting from `block.timestamp` and adding each duration to the previous timestamp.
+    /// @param segmentsWithDuration Segments with durations used to compose the dynamic distribution function. Timestamps
+    /// are calculated by starting from `block.timestamp` and adding each duration to the previous timestamp.
     /// @return streamId The ID of the newly created stream.
     function createWithDurationsLD(
         Lockup.CreateWithDurations calldata params,
-        LockupDynamic.SegmentWithDuration[] calldata segments
+        LockupDynamic.SegmentWithDuration[] calldata segmentsWithDuration
     )
         external
         returns (uint256 streamId);
@@ -177,12 +171,12 @@ interface ISablierLockup is ISablierLockupBase {
     /// - All requirements in {createWithTimestampsLT} must be met for the calculated parameters.
     ///
     /// @param params Struct encapsulating the function parameters, which are documented in {DataTypes}.
-    /// @param tranches Tranches with durations used to compose the tranched distribution function. Timestamps are
-    /// calculated by starting from `block.timestamp` and adding each duration to the previous timestamp.
+    /// @param tranchesWithDuration Tranches with durations used to compose the tranched distribution function.
+    /// Timestamps are calculated by starting from `block.timestamp` and adding each duration to the previous timestamp.
     /// @return streamId The ID of the newly created stream.
     function createWithDurationsLT(
         Lockup.CreateWithDurations calldata params,
-        LockupTranched.TrancheWithDuration[] calldata tranches
+        LockupTranched.TrancheWithDuration[] calldata tranchesWithDuration
     )
         external
         returns (uint256 streamId);
@@ -200,10 +194,10 @@ interface ISablierLockup is ISablierLockupBase {
     /// - Must not be delegate called.
     /// - `params.totalAmount` must be greater than zero.
     /// - If set, `params.broker.fee` must not be greater than `MAX_BROKER_FEE`.
-    /// - `params.startTime` must be greater than zero and less than the first segment's timestamp.
+    /// - `params.timestamps.start` must be greater than zero and less than the first segment's timestamp.
     /// - `segments` must have at least one segment, but not more than `MAX_COUNT`.
     /// - The segment timestamps must be arranged in ascending order.
-    /// - `params.endTime` must be equal to the last segment's timestamp.
+    /// - `params.timestamps.end` must be equal to the last segment's timestamp.
     /// - The sum of the segment amounts must equal the deposit amount.
     /// - `params.recipient` must not be the zero address.
     /// - `params.sender` must not be the zero address.
@@ -233,18 +227,18 @@ interface ISablierLockup is ISablierLockupBase {
     /// - `params.totalAmount` must be greater than zero.
     /// - If set, `params.broker.fee` must not be greater than `MAX_BROKER_FEE`.
     /// - `params.timestamps.start` must be greater than zero and less than `params.timestamps.end`.
-    /// - If set, `cliff` must be greater than `params.timestamps.start` and less than
+    /// - If set, `cliffTime` must be greater than `params.timestamps.start` and less than
     /// `params.timestamps.end`.
     /// - `params.recipient` must not be the zero address.
     /// - `params.sender` must not be the zero address.
     /// - `msg.sender` must have allowed this contract to spend at least `params.totalAmount` assets.
     ///
     /// @param params Struct encapsulating the function parameters, which are documented in {DataTypes}.
-    /// @param cliff The Unix timestamp for the cliff period's end. A value of zero means there is no cliff.
+    /// @param cliffTime The Unix timestamp for the cliff period's end. A value of zero means there is no cliff.
     /// @return streamId The ID of the newly created stream.
     function createWithTimestampsLL(
         Lockup.CreateWithTimestamps calldata params,
-        uint40 cliff
+        uint40 cliffTime
     )
         external
         returns (uint256 streamId);
@@ -262,10 +256,10 @@ interface ISablierLockup is ISablierLockupBase {
     /// - Must not be delegate called.
     /// - `params.totalAmount` must be greater than zero.
     /// - If set, `params.broker.fee` must not be greater than `MAX_BROKER_FEE`.
-    /// - `params.startTime` must be greater than zero and less than the first tranche's timestamp.
+    /// - `params.timestamps.start` must be greater than zero and less than the first tranche's timestamp.
     /// - `tranches` must have at least one tranche, but not more than `MAX_COUNT`.
     /// - The tranche timestamps must be arranged in ascending order.
-    /// - `params.endTime` must be equal to the last tranche's timestamp.
+    /// - `params.timestamps.end` must be equal to the last tranche's timestamp.
     /// - The sum of the tranche amounts must equal the deposit amount.
     /// - `params.recipient` must not be the zero address.
     /// - `params.sender` must not be the zero address.

src/core/libraries/Errors.sol

@@ -109,14 +109,8 @@ library Errors {
     /// @notice Thrown when trying to create a tranched stream with end time not equal to the last tranche's timestamp.
     error SablierLockup_EndTimeNotEqualToLastTrancheTimestamp(uint40 endTime, uint40 lastTrancheTimestamp);
 
-    /// @notice Thrown when calling a function on a stream without Dynamic distribution.
-    error SablierLockup_NotDynamicDistribution(Lockup.Model lockupModel);
-
-    /// @notice Thrown when calling a function on a stream without Linear distribution.
-    error SablierLockup_NotLinearDistribution(Lockup.Model lockupModel);
-
-    /// @notice Thrown when calling a function on a stream without Tranched distribution.
-    error SablierLockup_NotTranchedDistribution(Lockup.Model lockupModel);
+    /// @notice Thrown when a function is called on a stream that does not use the expected Lockup model.
+    error SablierLockup_NotExpectedModel(Lockup.Model actualLockupModel, Lockup.Model expectedLockupModel);
 
     /// @notice Thrown when trying to create a dynamic stream with more segments than the maximum allowed.
     error SablierLockup_SegmentCountTooHigh(uint256 count);