feat: merge all lockups into a single contract

[smol-ninja]

Closes #1074.

Since the work is split into two PRs. The order of merging would be:

1. test: refactor all tests according to singleton contracts #1077
2. feat: merge all lockups into a single contract #1069

• Add Model enum
• Rename SablierLockup to SablierLockupBase and SablierLockupOne to SablierLockup
• Emit separate events in each lockup create function
• Include endTime in CreateWithTimestamps struct
• Refactor calculateStreamedAmountForSegments to disallow unnecessary inputs
• Set Stream using JSON in create functions
• Update Helpers: split it into two libraries
• Update Errors
• Refactor calculation function in LD for readability
• Update inherited components
• Update shell scripts
• Update precompiles
• Update deployment scripts
• Update periphery
• Run a deployment on Sepolia. etherscan
• Include public libraries in artifacts
• prefix error names for libraries to SablierHelpers and SablierVestingMath

[smol-ninja]

@andreivladbrg let me know if you get a chance to review this approach. For the records, this is just to demonstrate the idea and does not confirm the actual changes that should be made.

[andreivladbrg]

@smol-ninja at your request, I am going to leave here some of the suggestion/things I would change/improve:

---

I would rename the current SablierLockup contract to SablierLockupBase and the SablierLockupOne to SablierLockup to match what we do in flow.

---

I believe we should have different create events emitted (similarly to claim on airstream). So that the unique
params related to the model used is emitted.

---

Add endTime in the Lockup.CreateWithTimestamps struct, allowing all create function signatures to follow this pattern: function create(CommonParams calldata params, UniqueStorage memory unique) external override returns (uint256 streamId);. This change would require us to check that the timestamp of the last segment/tranche matches the endTime.

Move this logic in _calculateStreamedAmount from individual LD/LL/LT functions (since it is shared logic).

uint256 blockTimestamp = block.timestamp;

if (_streams[streamId].startTime <= blockTimestamp) {
    return 0;
}

if (block.timestamp >= _streams[streamId].endTime) {
    return _streams[streamId].amounts.deposited;
}

---

I would move Helpers.checkAndCalculateBrokerFee check in _create.

---

There is no need to pass the entire stream struct into calculateStreamedAmountForSegments.

---

We can keep the current approach to updating storage for the stream struct, as I find it more readable:

_streams[streamId] = Lockup.Stream({
    amounts: Lockup.Amounts({ deposited: createAmounts.deposit, refunded: 0, withdrawn: 0 }),
    asset: params.asset,
    endTime: params.timestamps.end,
    isCancelable: params.cancelable,
    isDepleted: false,
    isStream: true,
    isTransferable: params.transferable,
    sender: params.sender,
    startTime: params.timestamps.start,
    wasCanceled: false
});

---

I am not a fan of the Family enum name, I would rather chose something like StreamingModel,
StreamType,LockupModel or LockupType.

---

The Helpers library becomes a long one and maybe hard to follow, what if we going to have two libraries:

1. Calculation library: for all the calculation functions
2. Checks (validations)

For the first one, we can place all calculation logic for all models in one place.

Click to see what I mean

function _calculateStreamedAmount(uint256 streamId) internal view override returns (uint128 streamedAmount) {
    // If the end time is not in the future, return the deposited amount.
    uint256 blockTimestamp = block.timestamp;

    if (_streams[streamId].startTime <= blockTimestamp) {
        return 0;
    }

    if (block.timestamp >= _streams[streamId].endTime) {
        return _streams[streamId].amounts.deposited;
    }

    Family family = _streams[streamId].family;

    if (family == Lockup.Family.LOCKUP_LINEAR) {
        streamedAmount = Calculation.linearStream(params);
    } else if (family == Lockup.Family.LOCKUP_DYNAMIC) {
        streamedAmount = Calculation.calculateStreamedAmountForSegments(params);
    } else if (family == Lockup.Family.LOCKUP_TRANCHED) {
        streamedAmount = Calculation.sumOfPassedTranches(params);
    }
}

A major benefit of this is that we can create a mock for this library independently and isolate tests for calculation logic, which is arguably the most fragile part of the code.

---

If possible, and assuming no significant downsides, I would divide the dynamic calculation function into two parts:

1. sum of passed segments
   (function calculatePassedSegments(uint128[] memory amounts, uint40[] memory timestamps) internal pure returns (uint128))
   so that we can use it for both LOCKUP_DYNAMIC and LOCKUP_TRANCHED models.
2. current segment streamed amount

[smol-ninja]

I would rename the current SablierLockup contract to SablierLockupBase and the SablierLockupOne to SablierLockup to match what we do in flow.

Agree. SablierLockupOne was just a placeholder name to avoid conflict with existing SablierLockup contract while writing that down.

I believe we should have different create events emitted (similarly to claim on airstream). So that the unique
params related to the model used is emitted.

Unique params related to the model are still emitted through separate events. But your suggestion sounds good to me. If it increase the contract size by a good margin, would it be OK to have shared event and a separate lockup specific event?

Add endTime in the Lockup.CreateWithTimestamps struct, allowing all create function signatures to follow this pattern: function create(CommonParams calldata params, UniqueStorage memory unique) external override returns (uint256 streamId

Sounds good. Will experiment with this idea.

I would move Helpers.checkAndCalculateBrokerFee check in _create

Why? This would increase contract size.

There is no need to pass the entire stream struct into calculateStreamedAmountForSegments

Sounds good.

We can keep the current approach to updating storage for the stream struct

Agree.

I am not a fan of the Family enum name, I would rather chose something like StreamingModel,
StreamType,LockupModel or LockupType.

"Fan" is a personal choice. My rationale is that (1) its a single word (2) Inspired from Family of Curves which means each curves in the family share the same formula which is indeed a case here. Therefore I find Family more suitable for this. This is also keyword that can be remembered easily.

The Helpers library becomes a long one and maybe hard to follow

May be it is not hard to follow. We have bigger files than than in our repos. To increase readability, I can put categories in it.

A major benefit of this is that we can create a mock for this library independently and isolate tests for calculation logic, which is arguably the most fragile part of the code

Sorry, I do not understand it. Can you please re-explain?

If possible, and assuming no significant downsides, I would divide the dynamic calculation function

Sounds good. Will try this approach.

[PaulRBerg]

I would suggest Model or DistributionModel (not StreamingModel because LockupTranched is not streaming) because this is how we have been referring to the Linear, Dynamic, and Tranched categories all along. Just search for "model" in the src folder, and you will find it used in the NFT SVG and many explanatory comments.

[smol-ninja]

because LockupTranched is not streaming

Why isn't LockupTranched categorised as streaming? If a stream is defined based on a per-second unlocks then even Lockup Dynamic is not streaming even though LD can be used to create per-second streams but then LT can also be used to create per-second streams by setting tranche duration to 1s.

IMO, streaming should be defined as the unlocking of tokens over a period of time (referring to multiple cadences discussion). So, as long as there are multiple unlocks periodically, it can be considered a stream.

[PaulRBerg]

We discussed the streaming terminology here: #708

[smol-ninja]

Sounds great. Thanks for pointing me to the other discussion. I am in for LockupModel/DistributionModel.

[andreivladbrg]

re terminology , i like the simple Model

---

Unique params related to the model are still emitted through separate events. But your suggestion sounds good to me. If it increase the contract size by a good margin, would it be OK to have shared event and a separate lockup specific event?

hmm, didn't see them, i talking about: cliff for LL, segments for LD, and tranches for LT

I would move Helpers.checkAndCalculateBrokerFee check in _create
Why? This would increase contract size

on contrary, this would decrease the size as there would only one time this function called (in the common _create). the version i reviewed was like this:

Click to see code

function _createLD(Params memory params) internal returns(uint256) {
   Helpers.checkAndCalculateBrokerFee(params);
   // --snip--
   _create(params);
}
function _createLL(Params memory params) internal returns(uint256) {
   Helpers.checkAndCalculateBrokerFee(params);
   // --snip--
   _create(params);
}
function _createLD(Params memory params) internal returns(uint256) {
   Helpers.checkAndCalculateBrokerFee(params);
   // --snip--
   _create(params);
}
function _create(Params memory params) internal returns(uint256) {
   // --snip--
}

I am suggesting:

function _createLD(Params memory params) internal returns(uint256) {
   // --snip--
   _create(params);
}
function _createLL(Params memory params) internal returns(uint256) {
   // --snip--
   _create(params);
}
function _createLD(Params memory params) internal returns(uint256) {
   // --snip--
   _create(params);
}
function _create(Params memory params) internal returns(uint256) {
   Helpers.checkAndCalculateBrokerFee(params);
}

---

Sorry, I do not understand it. Can you please re-explain?

My idea was to divide the logic as follows:

1. Conditional checks (e.g. if the start is in the future, return 0; if the end is in the past, return the deposit) within the contract itself.
2. Calculation logic isolated in a library (e.g. calculation of: elapsed time, elapsed percentage, streamed amount, etc.)

This structure would improve organization and allow testing to be divided between the two parts.

flowchart TD
    subgraph SmartContractSystem
        direction TB
        A[Contract A]
        B(Library Calculation Logic)
        C(Mock Library for Testing)
        A -->|Check: if X then Y, if W then Z| ACheck[Conditional Checks]
    end
    
    %% Contract A structure
    
    %% A -->|Calls| B[Library Calculation Logic]

    %% Testing Structure
    subgraph Testing
        direction TB
        T[Test Calculation Logic]
        T1[Test Conditional Checks]
        T1 --> |Uses| ACheck
        T -->|Uses| C
    end

    %% Explanation
    ACheck -->|If Conditions Met| B
    C -.->|Mock Contract| B

Loading

[smol-ninja]

I would move Helpers.checkAndCalculateBrokerFee check in _create

Its not possible to do so. The reason is that the returned value of checkAndCalculateBrokerFee: createAmounts is used in the lockup specific check functions. If you have a better solution, please let me know or feel free to push a commit directly.

---

@andreivladbrg I have included all your suggestions including the library split. They were all good. I have also updated the core source contracts. Feel free to review the src/core directory as the contract work is done from my side. You can also see whats done and whats remaining in the PR description.

The size of the SablierLockup contract is 24,284 at 1000 runs which is 292 less than the allowed.

[smol-ninja]

@andreivladbrg this PR is ready for your review. A separate PR is opened to implement the tests and the remaining changes (#1077). Please see the description in both the PRs to see what they cover.

The CI will succeed in #1077.

[andreivladbrg]

Great work!
Left some comments which were tentatively addressed in the upcoming PR created on top of this.