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

migrate stXXX/XXX constant product pool -> stableswap pool #4384

Merged
merged 22 commits into from
Feb 23, 2023

Conversation

asalzmann
Copy link
Contributor

@asalzmann asalzmann commented Feb 22, 2023

What is the purpose of the change

Migrate pools 833, 817, and 810 from constant product -> stable swap.

Brief Changelog

  • add upgrade handler code
  • test migration function
  • test basic LP functionality (swap, withdraw after migration)

Testing and Verifying

This change added tests and can be verified as follows:

  • Added a unittest TestMigrateBalancerToStablePools to verify liquidity stays constant before/after the migration
  • Added e2e tests
    • setup in CreatePreUpgradeState by LPing to the 3 pools
    • add TestMigrateBalancerToStable e2e test which swaps in and exits each pool

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes? no
  • Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? yes
  • How is the feature or change documented? CHANGELOG.md

@github-actions github-actions bot added C:app-wiring Changes to the app folder C:x/gamm Changes, features and bugs related to the gamm module. labels Feb 22, 2023
app/upgrades/v15/upgrade_test.go Outdated Show resolved Hide resolved
app/upgrades/v15/upgrade_test.go Outdated Show resolved Hide resolved
app/upgrades/v15/constants.go Show resolved Hide resolved
app/upgrades/v15/upgrades.go Show resolved Hide resolved
app/upgrades/v15/upgrades.go Show resolved Hide resolved
app/upgrades/v15/upgrades.go Outdated Show resolved Hide resolved
@p0mvn
Copy link
Member

p0mvn commented Feb 22, 2023

Please merge main into this branch to make sure that #4396 is included.

This is the final step needed for e2e testing the migrations during the upgrade.

We created Stride balancer pools via genesis in v14 in #4393

Now, when the upgrade runs, the pools will be found and the newly implemented migration run against them

@p0mvn p0mvn added the V:state/breaking State machine breaking PR label Feb 22, 2023
Copy link
Member

@mattverse mattverse left a comment

Choose a reason for hiding this comment

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

LGTM other than the suggested comments and e2e passing

app/upgrades/v15/upgrades.go Show resolved Hide resolved
app/upgrades/v15/upgrade_test.go Outdated Show resolved Hide resolved
x/gamm/keeper/pool.go Outdated Show resolved Hide resolved
tests/e2e/e2e_test.go Outdated Show resolved Hide resolved
// join the pool
shareOutAmount := sdk.NewInt(10000000)
tokenInMaxs := sdk.NewCoins(sdk.NewCoin("foo", sdk.NewInt(5000000)), sdk.NewCoin("bar", sdk.NewInt(5000000)))
_, _, err = suite.App.GAMMKeeper.JoinPoolNoSwap(suite.Ctx, testAccount, poolID, shareOutAmount, tokenInMaxs)
Copy link
Member

@p0mvn p0mvn Feb 23, 2023

Choose a reason for hiding this comment

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

So this function has sharesOut as one of its returns, we could use these to then use for ExitPool

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice, used this below

Copy link
Member

@p0mvn p0mvn left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you @asalzmann

@p0mvn p0mvn added the A:backport/v15.x backport patches to v15.x branch label Feb 23, 2023
@p0mvn p0mvn merged commit 0355d27 into osmosis-labs:main Feb 23, 2023
mergify bot pushed a commit that referenced this pull request Feb 23, 2023
* migrate stXXX/XXX constant product pool -> stableswap pool

* update upgrade unittest

* send LP tokens to lpWallet

* update tests

* add join, exit unittest

* add checks per comments

* one more comment

* lp pre-upgrade

* clean up and TODOs

* fix e2e

* add query

* rm TestMigrateBalancerToStable, update ExitPool in upgrade_test.go

* rm TestMigrateBalancerToStable

* update changelog

* fix e2e

* more assertions for TestMigrateBalancerToStablePools

* lint

---------

Co-authored-by: Roman <roman@osmosis.team>
(cherry picked from commit 0355d27)

# Conflicts:
#	tests/e2e/configurer/chain/queries.go
p0mvn added a commit that referenced this pull request Feb 23, 2023
@github-actions github-actions bot mentioned this pull request Mar 1, 2024
@github-actions github-actions bot mentioned this pull request Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:backport/v15.x backport patches to v15.x branch C:app-wiring Changes to the app folder C:x/gamm Changes, features and bugs related to the gamm module. V:state/breaking State machine breaking PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants