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

Rebase LinearPool #1955

Merged
merged 23 commits into from
Oct 28, 2022
Merged

Rebase LinearPool #1955

merged 23 commits into from
Oct 28, 2022

Conversation

EndymionJkb
Copy link
Collaborator

Description

With the infrastructure in place (NewBasePool in pool-utils, used by ManagedPool), make LinearPool also use NewBasePool.

It required minor changes to internal functions in NewBasePool: initialization required another argument, and we need to override _getMinimumBpt. I put in a check to not mint 0 tokens (and emit an event with 0); could remove if we don't mind that. (Not an optimization concern, since it's only called once; I mainly thought the event might be confusing.)

Type of change

  • Bug fix
  • New feature
  • Breaking change
  • Dependency changes
  • Code refactor / cleanup
  • Documentation or wording changes
  • Other

Checklist:

  • The diff is legible and has no extraneous changes
  • Complex code has been commented, including external interfaces
  • Tests are included for all code paths - same tests
  • The base branch is either master, or there's a description of how to merge

Issue Resolution

Advances #1953

Copy link
Contributor

@nventuro nventuro left a comment

Choose a reason for hiding this comment

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

I love that this PR touches no tests.

pkg/pool-linear/contracts/LinearPool.sol Show resolved Hide resolved
pkg/pool-linear/contracts/LinearPool.sol Outdated Show resolved Hide resolved
pkg/pool-linear/contracts/LinearPool.sol Outdated Show resolved Hide resolved
pkg/pool-linear/contracts/LinearPool.sol Show resolved Hide resolved
pkg/pool-utils/contracts/NewBasePool.sol Outdated Show resolved Hide resolved
@EndymionJkb EndymionJkb requested a review from nventuro October 28, 2022 20:45
@nventuro nventuro merged commit e791860 into master Oct 28, 2022
@nventuro nventuro deleted the rebase-linear branch October 28, 2022 21:58
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.

2 participants