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

Lbp one weight change #1260

Draft
wants to merge 30 commits into
base: main
Choose a base branch
from
Draft

Lbp one weight change #1260

wants to merge 30 commits into from

Conversation

mkflow27
Copy link

@mkflow27 mkflow27 commented Jan 27, 2025

See below for the original text.

This version is in the same spirit, keeping things simple for aggregators by limiting the scope to token sales, and essentially fixing all parameters on deployment: the sale has a fixed start and end time, with a single weight change schedule. Liquidity can only be added before the start, and only removed after the end. Swaps can only occur during the sale.

There is a flag indicating whether the project token can be sold "back" into the pool. If not, it is a "one way" distribution. Everything is immutable. The only things that can change unpredictably are the actual balances (due to swaps), and the swap fee (which can be changed by the owner).

I temporarily removed the LBPLib (used for better error messages). I'll put that back when we have a fixed design and I'm writing the tests.

Goal of the draft:

  • I would like to get feedback on the approach to the feature implementations. Once they are agreed upon I will move on to cleaning up the tests.

Description

This pr adds changes to the LBP. Namely:

  • Move weight change setting to the constructor
  • Introduce bootstrapToken which allows for the pool creator to determine if the bootstrap token can be sold&bought or only bought.
  • introduce allowRemovalOnlyAfterWeightChange which allows for the pool creator to determine if removing liquidity is allowed during pool lifecycle or only after weight change has ended
  • Allow swaps to only be possible during & after weight change

Overall the goal of this pr is to deliver the following feature requests:

  • Support only 1 weight change, defined on construction, this would make it much easier for aggregators to know the weight, without the need to pool for events. The start time can be at a point in the future
  • Remove swaps enabled, swaps are turned on during the weight change, otherwise they are off.
  • A flag, if set, the liquidity can only be removed once the weight change has ended
  • A flag if set, the token can only be bought and not sold

Type of change

  • Bug fix
  • New feature
  • Breaking change
  • Dependency changes
  • Code refactor / cleanup
  • Optimization: [ ] gas / [ ] bytecode
  • Documentation or wording changes
  • Other

Checklist:

  • The diff is legible and has no extraneous changes
  • Complex code has been commented, including external interfaces
  • Tests have 100% code coverage
  • The base branch is either main, or there's a description of how to merge

Issue Resolution

@jubeira jubeira requested review from jubeira and EndymionJkb and removed request for jubeira January 27, 2025 20:45
Copy link
Contributor

@jubeira jubeira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @mkflow27, thanks for putting this together!

Looks good overall; here's a first pass with some comments, mostly small things where there's room for improvement and some questions.

Besides that, the main question I have is whether we want all pools to be like this (i.e. a single weight update at construction time vs configurable weight updates). If we want this to be more flexible and have some pools with a single update and some pools with configurable updates, then we might need to add a flag and rethink a few things.

Same for disabling swaps. Do we just want them enabled after the update starts and that's it?

pkg/pool-weighted/contracts/lbp/LBPool.sol Outdated Show resolved Hide resolved
@@ -37,7 +44,6 @@ contract LBPool is WeightedPool, Ownable2Step, BaseHooks {
uint32 endTime;
uint64 startWeight0;
uint64 endWeight0;
bool swapEnabled;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danielmkm @mkflow27 do we want all pools to be like this, or was this meant to be an optional feature?

pkg/pool-weighted/contracts/lbp/LBPool.sol Outdated Show resolved Hide resolved
pkg/pool-weighted/contracts/lbp/LBPool.sol Outdated Show resolved Hide resolved
pkg/pool-weighted/contracts/lbp/LBPool.sol Outdated Show resolved Hide resolved
pkg/pool-weighted/contracts/lbp/LBPool.sol Outdated Show resolved Hide resolved
pkg/pool-weighted/contracts/lbp/LBPool.sol Outdated Show resolved Hide resolved
pkg/pool-weighted/contracts/lbp/LBPool.sol Outdated Show resolved Hide resolved
pkg/pool-weighted/contracts/lbp/LBPool.sol Outdated Show resolved Hide resolved
Copy link
Collaborator

@EndymionJkb EndymionJkb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm proposing some structural changes. Let me know what you think.

pkg/pool-weighted/contracts/lbp/LBPoolFactory.sol Outdated Show resolved Hide resolved
pkg/pool-weighted/contracts/lbp/LBPool.sol Outdated Show resolved Hide resolved
pkg/pool-weighted/contracts/lbp/LBPool.sol Outdated Show resolved Hide resolved
pkg/pool-weighted/contracts/lbp/LBPool.sol Show resolved Hide resolved
@jubeira jubeira force-pushed the lbp-one-weight-change branch from 55fd9da to 481490b Compare January 29, 2025 15:38
@EndymionJkb EndymionJkb mentioned this pull request Jan 29, 2025
12 tasks
@EndymionJkb EndymionJkb self-assigned this Jan 31, 2025
@joaobrunoah
Copy link
Contributor

Should we use the Balancer Contract Registry to identify a trusted router in the LBP?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these tests deleted?

Copy link
Author

@mkflow27 mkflow27 Feb 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initially did not have enough time to refactor the typescript tests. So I removed them with the intention of adding them during my work on this pr ( I think )

@EndymionJkb EndymionJkb requested a review from jubeira February 6, 2025 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants