Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LockupTranched: a new Lockup contract with sharply delineated chunks #787

Closed
3 of 4 tasks
PaulRBerg opened this issue Jan 9, 2024 · 8 comments
Closed
3 of 4 tasks
Assignees
Labels
effort: epic Multi-stage task that may require multiple PRs. priority: 0 Do this first before everything else. This is critical and nothing works without this. type: feature New feature or request. work: complicated Sense-analyze-respond. The relationship between cause and effect requires analysis or expertise.

Comments

@PaulRBerg
Copy link
Member

PaulRBerg commented Jan 9, 2024

As discussed here: https://github.com/sablier-labs/company-discussions/discussions/19

Tasks:

  • Build a PoC implementation
  • Benchmark the PoC against LockupDynamic:
    • Compare the creation of Unlock in Steps streams, e.g. this
    • Compare withdrawals from standard streams with one segment, i.e., how much more gas efficient will the withdraw function in LockupTranched be
  • Decide if LockupTranched is worth it
  • Proceed with writing tests for it (using Bulloak)

Note: we will keep using the same terminology, i.e., streams and streamedAmountOf. Related: #708 (comment)

@PaulRBerg PaulRBerg added type: feature New feature or request. effort: epic Multi-stage task that may require multiple PRs. priority: 0 Do this first before everything else. This is critical and nothing works without this. work: complicated Sense-analyze-respond. The relationship between cause and effect requires analysis or expertise. labels Jan 9, 2024
@andreivladbrg
Copy link
Member

Just wanted to let you know that I've implemented the POC and did benchmarks (see README file). You can see the work here:
https://github.com/andreivladbrg/v2-tranched/

Now it is up to decide if it worth implementing here.

@PaulRBerg
Copy link
Member Author

Awesome, thanks @andreivladbrg, will review later.

@PaulRBerg
Copy link
Member Author

PaulRBerg commented Jan 23, 2024

Kudos, @andreivladbrg, for the benchmark you have prepared here.

I think it is as clear as day that we should move forward with LockupTranched.

  • For 1-year vesting, it saves (i) up to 300K gas at creation time and (ii) up to 20k gas per withdrawal
  • For 2-year vesting, it saves (i) up to 600K gas at creation time and (ii) up to 25k gas per withdrawal
  • For 4-year vesting, it saves (i) up to 1.2M gas at creation time and (ii) up to 36k gas per withdrawal

These gas optimizations are due to LT making it possible to have half as many segments as LD. But LT is more efficient even when the number of segments is equal.

Screenshot 2024-01-23 at 11 14 33 AM

@razgraf could you give us the green light?

@razgraf
Copy link
Member

razgraf commented Jan 23, 2024

Sounds good!

@PaulRBerg
Copy link
Member Author

Great, thanks.

@andreivladbrg, you can move forward with implementing LockupTranched properly. The contract should be added in V2 Core.

Note: we should first merge #798

@andreivladbrg
Copy link
Member

@andreivladbrg, you can move forward with implementing LockupTranched properly. The contract should be added in V2 Core.

I will, but currently I am working on making a huge refactor for core, that it would be very beneficial as the Lockup contracts are increasing

https://github.com/andreivladbrg/refactor-core/

saying it now, it may take a while

@PaulRBerg
Copy link
Member Author

That's fine but please create a discussion/ proposal prior to a PR

@andreivladbrg
Copy link
Member

34fee6b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: epic Multi-stage task that may require multiple PRs. priority: 0 Do this first before everything else. This is critical and nothing works without this. type: feature New feature or request. work: complicated Sense-analyze-respond. The relationship between cause and effect requires analysis or expertise.
Projects
None yet
Development

No branches or pull requests

3 participants