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

Combine all Lockups into a singleton contract #1074

Closed
smol-ninja opened this issue Oct 31, 2024 · 5 comments
Closed

Combine all Lockups into a singleton contract #1074

smol-ninja opened this issue Oct 31, 2024 · 5 comments
Assignees
Labels
effort: epic Multi-stage task that may require multiple PRs. priority: 1 This is important. It should be dealt with shortly. type: feature New feature or request. work: complex Probe-sense-respond. The relationship between cause and effect can only be perceived in retrospect.

Comments

@smol-ninja
Copy link
Member

Originally discussed in #1064

The task is to write a singleton contract that can represent all lockup streams and see if its viable.

@smol-ninja smol-ninja added effort: epic Multi-stage task that may require multiple PRs. work: complex Probe-sense-respond. The relationship between cause and effect can only be perceived in retrospect. type: refactor Change that neither fixes a bug nor adds a feature. priority: 1 This is important. It should be dealt with shortly. labels Oct 31, 2024
@smol-ninja smol-ninja self-assigned this Oct 31, 2024
@smol-ninja smol-ninja added type: feature New feature or request. and removed type: refactor Change that neither fixes a bug nor adds a feature. labels Oct 31, 2024
@smol-ninja
Copy link
Member Author

smol-ninja commented Nov 1, 2024

Question to @sablier-labs/solidity:

Should be have separate MAX_COUNT for segments and tranches (status quo); or should we use the same value for both?

If you look at their values, they are very close to each other. Therefore, my recommendation is to use the same value for both. The common MAX_COUNT would be defined as the maximum value that does not let either of the two exceeds the block gas limit.

@PaulRBerg
Copy link
Member

Makes sense to use the same value.

@andreivladbrg
Copy link
Member

i also agree that the same value for both is better

@smol-ninja
Copy link
Member Author

smol-ninja commented Nov 4, 2024

Test deployment of Lockup contract:

  1. Lockup deployment
  2. VestingMath library
  3. Helpers library

Remarks:

  • Foundry automatically deploys and links libraries without have to explicitly specify them. Warp output.
  • Etherscan verification requires --libraries arguments (foundry book). Warp output.
  • Once verified, Etherscan shows the library code on the contract page.

@smol-ninja
Copy link
Member Author

smol-ninja commented Nov 4, 2024

Also, I don't know the answer but the createWithDuration on the singleton contract is cheaper than deployed Lockup Linear contract (keeping all parameters same).

  1. 174,000 on singleton
  2. 205,120 on deployed LockupLinear

Seems like calls to public libraries ain’t affecting the gas usage.

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: 1 This is important. It should be dealt with shortly. type: feature New feature or request. work: complex Probe-sense-respond. The relationship between cause and effect can only be perceived in retrospect.
Projects
None yet
Development

No branches or pull requests

3 participants