feat: add unlock amounts functionality #1075

andreivladbrg · 2024-11-04T16:32:43Z

Since we have the invariant: $\text{cliff time} = 0 \implies \text{cliff unlock amount} = 0$, we would have these 6 possible scenarios for the streamed amount function:

Scenario	Cliff Time	Start Unlock Amount	Cliff Unlock Amount	Shape
1	No	No	No	Simple Linear
2	Yes	No	No	Constant, then linear (see below)
3	No	Yes	No	Unlock Linear
4	Yes	Yes	No	Initial amount, constant, then linear (see below)
5	Yes	No	Yes	Constant, then unlock with linear function
6	Yes	Yes	Yes	Unlock Cliff

Plot graphs with docs code

Scenario 2

Code to update the docs shapes with:

<FunctionPlot
  options={{
    data: [
      { fn: "0", range: [0, 25], color: "#f77423" },
      { fn: "(4/3) * (x - 25)", range: [25, 100], color: "#f77423" },
      {
        points: [
          [0, 0],
          [25, 0],
        ],
        fnType: "points",
        graphType: "polyline",
        color: "#f77423",
      },
    ],
  }}
/>

Scenario 4

Code to update the docs shapes with:

<FunctionPlot
  options={{
    data: [
      { fn: "25", range: [0, 50], color: "#f77423" }, // Constant at y=25 from x=0 to x=50
      { fn: "1.5 * (x - 50) + 25", range: [50, 100], color: "#f77423" }, // Linear rise with slope 1.5 starting from y=25 at x=50
      {
        points: [
          [0, 25],
          [50, 25],
        ],
        fnType: "points",
        graphType: "polyline",
        color: "#f77423",
      },
    ],
  }}
/>

Note: Since the mathematical function has changed in SablierLockupLinear._calculateStreamedAmount, in order to achieve the previous cliff linear shape, it is required to pass an unlockAmount.cliff > 0; otherwise, the second scenario shape will be achieved. This is also the reason I needed to change the tests from warping to WARP_26_PERCENT instead of CLIFF_TIME, as many tests were failing. The cliff amount has a remainder of 2534 due to a different "streamable" range.

As you will see in the code, I am referring to the range that calculates the elapsed time as "streamable," which might be confusing since the entire function is called "streamed amount." However, we will eventually change this terminology to "vesting," so don’t mind the current terminology if you find it unclear.

Below you can see an useful diagram to vizualize while reviewing the new function.

New function flow

flowchart TD
    start_time_check([Is startTime >= current block timestamp?]):::check
    return_zero([Return 0]):::return
    end_time_check([Is current block timestamp >= endTime?]):::check
    return_deposited([Return depositedAmount]):::return
    cliff_time_check([Is cliffTime set and cliffTime > current block timestamp?]):::check
    return_start_unlock([Return start unlock amount]):::return
    unlock_sum_check([Is unlockAmountsSum >= depositedAmount?]):::check
    elapsed_calculation_check([Calculate elapsedTime and streamableDuration]):::calculation
    etp_calculation([Calculate elapsedTimePercentage]):::calculation
    streamable_amount_calc([Calculate streamableAmount]):::calculation
    streamed_amount_calc([Calculate streamedAmount]):::calculation
    final_check([Is streamedAmount > depositedAmount?]):::check
    return_withdrawn([Return withdrawn amount]):::return
    return_streamed([Return streamedAmount]):::return

    start_time_check -- "Yes" --> return_zero
    start_time_check -- "No" --> end_time_check
    end_time_check -- "Yes" --> return_deposited
    end_time_check -- "No" --> cliff_time_check
    cliff_time_check -- "Yes" --> return_start_unlock
    cliff_time_check -- "No" --> unlock_sum_check
    unlock_sum_check -- "Yes" --> return_deposited
    unlock_sum_check -- "No" --> elapsed_calculation_check

    elapsed_calculation_check --> etp_calculation
    etp_calculation --> streamable_amount_calc
    streamable_amount_calc --> streamed_amount_calc
    streamed_amount_calc --> final_check
    final_check -- "Yes" --> return_withdrawn
    final_check -- "No" --> return_streamed

    classDef check fill:#FFC107,stroke:#333,stroke-width:2px;
    classDef return fill:#4CAF50,stroke:#333,stroke-width:2px;
    classDef calculation fill:#03A9F4,stroke:#333,stroke-width:2px;

andreivladbrg · 2024-11-05T23:21:41Z

@smol-ninja the PR is ready for your review, I have updated the OP with some useful comments, please get first through them before reviewing,

andreivladbrg · 2024-11-08T23:20:46Z

@smol-ninja just rebased from refactor/singleton-contract. in case i have missed something, i have created a recovery branch.

Note: the size of the contract is at its limit, adding new things to the contract would require us to decrease the optimizer runs.

Contract	Size (B)	Margin (B)
SablierLockup	24,450	126

smol-ninja

Nice work on the PR. Reviewed the src and left my comments below.

src/core/SablierLockup.sol

src/core/libraries/Helpers.sol

src/core/interfaces/ISablierLockup.sol

src/core/libraries/VestingMath.sol

smol-ninja · 2024-11-09T20:53:41Z

src/periphery/SablierMerkleLL.sol

@@ -36,6 +36,9 @@ contract SablierMerkleLL is
    /// @inheritdoc ISablierMerkleLL
    MerkleLL.Schedule public override schedule;

+    /// @inheritdoc ISablierMerkleLL
+    LockupLinear.UnlockAmounts public override unlockAmounts;


Instead of creating is as a separate variable, how about merging it into schedule. The name schedule is very generic and can include amounts as well.

hmm, not sure what to say, will think more about this

i would like to keep the current version to have two different structs 1. for time related variables 2. amount related amounts

schedule name is more suited for time related variables IMO

andreivladbrg · 2024-11-13T16:36:28Z

@smol-ninja I have addressed all the src feedback, and I have finally rebased from test PRs—it was a mess… but we’re here. I hope I didn’t miss anything while rebasing.

andreivladbrg · 2024-11-13T16:38:09Z

src/core/SablierLockup.sol

@@ -408,36 +433,47 @@ contract SablierLockup is ISablierLockup, SablierLockupBase {
        });
    }

+    struct CreateLLParams {


temporarily added this due to stack too deep error

will try to think of a better option, but couldn't find one yet (tried scoping {} blocks of code, tried a different function for the event, tried splitting the logic, and didn't work unfortunetely)

refactor: some polishes docs: update explanatory comments test: add new test branches in create function test: streamed amount in lockup linear test: fix common streamed amount tests test: remove unneeded branches build: update deps test: fix tranches default function feat: include unlock amounts in StreamLL docs: fix some comments test: improve fuzz streamed amount tests test: update fork with unlock amounts test: fix last failing tests docs: last polishes

andreivladbrg marked this pull request as draft November 4, 2024 16:32

andreivladbrg force-pushed the feat/unlock-amounts branch from 3e0cf63 to 9d7e555 Compare November 4, 2024 16:37

andreivladbrg marked this pull request as ready for review November 5, 2024 23:19

This was linked to issues Nov 5, 2024

Instant unlock + more flexible cliff in LockupLinear #1043

Open

Upgrade to PRBMath v4.1.0 #1067

Open

smol-ninja self-requested a review November 6, 2024 10:31

andreivladbrg force-pushed the feat/unlock-amounts branch 2 times, most recently from b65cb8f to d4d8187 Compare November 8, 2024 23:14

andreivladbrg changed the base branch from staging to refactor/singleton-contract November 8, 2024 23:16

This comment was marked as resolved.

Sign in to view

smol-ninja requested changes Nov 9, 2024

View reviewed changes

andreivladbrg force-pushed the refactor/singleton-contract branch from 151dbdd to 57d7def Compare November 11, 2024 09:16

smol-ninja force-pushed the refactor/singleton-contract branch from 57d7def to 8ca1274 Compare November 11, 2024 10:01

smol-ninja force-pushed the feat/unlock-amounts branch from d4d8187 to d8865fd Compare November 11, 2024 10:07

smol-ninja force-pushed the refactor/singleton-contract branch from 8ca1274 to 9050f98 Compare November 11, 2024 10:33

andreivladbrg force-pushed the feat/unlock-amounts branch from d8865fd to 8e1364a Compare November 13, 2024 16:02

andreivladbrg changed the base branch from refactor/singleton-contract to avb/test-singleton-review November 13, 2024 16:02

andreivladbrg force-pushed the feat/unlock-amounts branch 2 times, most recently from 237cadb to e98d613 Compare November 13, 2024 16:32

andreivladbrg commented Nov 13, 2024

View reviewed changes

andreivladbrg force-pushed the feat/unlock-amounts branch from e98d613 to 290eb76 Compare November 13, 2024 17:39

andreivladbrg force-pushed the avb/test-singleton-review branch 3 times, most recently from 62d26a3 to 20e40ba Compare November 13, 2024 22:23

Base automatically changed from avb/test-singleton-review to staging November 13, 2024 22:23

andreivladbrg force-pushed the feat/unlock-amounts branch 2 times, most recently from 267b7fd to 4b8a424 Compare November 13, 2024 22:32

andreivladbrg force-pushed the feat/unlock-amounts branch from 4b8a424 to 5d48595 Compare November 13, 2024 22:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add unlock amounts functionality #1075

feat: add unlock amounts functionality #1075

andreivladbrg commented Nov 4, 2024 •

edited

Loading

andreivladbrg commented Nov 5, 2024

andreivladbrg commented Nov 8, 2024

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

smol-ninja left a comment

smol-ninja Nov 9, 2024

andreivladbrg Nov 11, 2024 •

edited

Loading

andreivladbrg Nov 13, 2024 •

edited

Loading

andreivladbrg commented Nov 13, 2024

andreivladbrg Nov 13, 2024

feat: add unlock amounts functionality #1075

Are you sure you want to change the base?

feat: add unlock amounts functionality #1075

Conversation

andreivladbrg commented Nov 4, 2024 • edited Loading

Scenario 2

Code to update the docs shapes with:

Scenario 4

Code to update the docs shapes with:

andreivladbrg commented Nov 5, 2024

andreivladbrg commented Nov 8, 2024

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

smol-ninja left a comment

Choose a reason for hiding this comment

smol-ninja Nov 9, 2024

Choose a reason for hiding this comment

andreivladbrg Nov 11, 2024 • edited Loading

Choose a reason for hiding this comment

andreivladbrg Nov 13, 2024 • edited Loading

Choose a reason for hiding this comment

andreivladbrg commented Nov 13, 2024

andreivladbrg Nov 13, 2024

Choose a reason for hiding this comment

andreivladbrg commented Nov 4, 2024 •

edited

Loading

andreivladbrg Nov 11, 2024 •

edited

Loading

andreivladbrg Nov 13, 2024 •

edited

Loading