Skip to content

Commit

Permalink
refactor: address feedback (#1079)
Browse files Browse the repository at this point in the history 
* refactor: address feedback

* refactor: move calculate segments/tranches in Helpers

refactor: revert constant in library change
refactor: re introduce _checkCliffAndEndTime function

* refactor: polish code

* chore: remove unneeded import

* add stream id in _create

* use ISablierLockupBase.isTransferable

---------

Co-authored-by: smol-ninja <shubhamy2015@gmail.com>
  • Loading branch information
@andreivladbrg @smol-ninja
andreivladbrg and smol-ninja committed Nov 8, 2024
1 parent 8464af3 commit b4d9ea6
Show file tree
Hide file tree
Showing 12 changed files with 310 additions and 292 deletions.
6 changes: 3 additions & 3 deletions src/core/LockupNFTDescriptor.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";

/*
██╗ ██████╗ ██████╗██╗ ██╗██╗ ██╗██████╗ ███╗ ██╗███████╗████████╗
Expand Down Expand Up @@ -49,7 +49,7 @@ contract LockupNFTDescriptor is ILockupNFTDescriptor {
uint128 depositedAmount;
bool isTransferable;
string json;
ISablierLockupBase lockup;
ISablierLockup lockup;
string lockupModel;
string lockupStringified;
bytes returnData;
Expand All @@ -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));
Expand Down
227 changes: 122 additions & 105 deletions src/core/SablierLockup.sol
View file

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion src/core/abstracts/SablierLockupBase.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -205,7 +206,7 @@ abstract contract SablierLockupBase is

/// @inheritdoc ISablierLockupBase
function streamedAmountOf(uint256 streamId)
public
external
view
override
notNull(streamId)
Expand Down
57 changes: 25 additions & 32 deletions src/core/interfaces/ISablierLockup.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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.
Expand All @@ -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,
Expand All @@ -64,6 +63,7 @@ interface ISablierLockup is ISablierLockupBase {
bool cancelable,
bool transferable,
Lockup.Timestamps timestamps,
uint40 cliffTime,
address broker
);

Expand All @@ -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(
Expand All @@ -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
);

/*//////////////////////////////////////////////////////////////////////////
Expand All @@ -103,22 +102,16 @@ 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.
Expand All @@ -138,12 +131,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);
Expand Down Expand Up @@ -177,12 +170,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);
Expand All @@ -200,10 +193,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.
Expand Down Expand Up @@ -233,18 +226,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);
Expand All @@ -262,10 +255,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.
Expand Down
10 changes: 2 additions & 8 deletions src/core/libraries/Errors.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading

0 comments on commit b4d9ea6

Please sign in to comment.