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

[E2E] Create stableswap and cl pools #4175

Merged

Conversation

phamminh0811
Copy link
Contributor

Closes: #3742

What is the purpose of the change

This pull request add CreateStableSwapPool function to e2e test and also create stableswap and CL pools to setup test

Testing and Verifying

This change is already covered by existing tests.

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes? (yes / no)
  • Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? (yes / no)
  • How is the feature or change documented? (not applicable / specification (x/<module>/spec/) / Osmosis docs repo / not documented)

@phamminh0811 phamminh0811 changed the title Minh/create stableswap and cl pools [E2E] Create stableswap and cl pools Jan 31, 2023
tests/e2e/configurer/chain/commands.go Outdated Show resolved Hide resolved
tests/e2e/configurer/upgrade.go Outdated Show resolved Hide resolved
tests/e2e/configurer/upgrade.go Outdated Show resolved Hide resolved
@pysel pysel added the V:state/compatible/no_backport State machine compatible PR, depends on prior breaks label Feb 1, 2023
@pysel pysel marked this pull request as draft February 1, 2023 14:33
@pysel
Copy link
Member

pysel commented Feb 1, 2023

There is no need to commit rate_limiter.wasm, this file is generated when running e2e, @phamminh0811, could you please remove it?

Copy link
Member

@pysel pysel left a comment

Choose a reason for hiding this comment

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

LGTM when comments addressed. Please, reset rate_limiter.wasm to the commit of development:

git checkout <latest main commit hash> -- <path to rate limiter>

tests/e2e/scripts/stablePool.json Outdated Show resolved Hide resolved
@phamminh0811
Copy link
Contributor Author

LGTM when comments addressed. Please, reset rate_limiter.wasm to the commit of development:

git checkout <latest main commit hash> -- <path to rate limiter>

seems like this wasm file is deleted from main branch since this commit #4208
should we reset them?

@pysel
Copy link
Member

pysel commented Feb 3, 2023

LGTM when comments addressed. Please, reset rate_limiter.wasm to the commit of development:

git checkout <latest main commit hash> -- <path to rate limiter>

seems like this wasm file is deleted from main branch since this commit #4208 should we reset them?

I see, SG, I think then you may merge main to this PR and the issue is solved! Lmk when this is ready!

tests/e2e/scripts/stablePool.json Outdated Show resolved Hide resolved
tests/e2e/configurer/chain/commands.go Outdated Show resolved Hide resolved
@phamminh0811 phamminh0811 marked this pull request as ready for review February 6, 2023 08:19
Copy link
Member

@pysel pysel left a comment

Choose a reason for hiding this comment

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

LGTM! The only thing is rate_limiter.wasm file. Can you please pull changes from main, I think it should be gone then?

tests/e2e/e2e_test.go Outdated Show resolved Hide resolved
tests/e2e/e2e_test.go Outdated Show resolved Hide resolved
@@ -35,4 +35,7 @@ var (
// creation in case more pools are added to genesis
// in the future
PreUpgradePoolId uint64 = 2
// Stableswap pool is created after balance pool,
// so the pool id should be 3.
PreUpgradeStableSwapPoolId uint64 = 3
Copy link
Member

Choose a reason for hiding this comment

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

This has changed since #4384

Let's make sure that we are not hard-coding this value. Instead, let's get the resulting pool id from CreateStableSwapPool and persist it until we reach TestStableSwapPostUpgrade

Can you please make a similar update for PreUpgradePoolId?

Copy link
Member

Choose a reason for hiding this comment

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

We do something similar with wallet creation where we set it pre-upgrade here:

config.StrideMigrateWallet = chainANode.CreateWalletAndFund(config.StrideMigrateWallet, []string{
fmt.Sprintf("%d%s", amountOfEachTokenToLP, initialization.StOsmoDenom),
fmt.Sprintf("%d%s", amountOfEachTokenToLP, initialization.StJunoDenom),
fmt.Sprintf("%d%s", amountOfEachTokenToLP, initialization.StStarsDenom),
})

and then use here in tests:

node.BankSend(token, initialization.ValidatorWalletName, config.StrideMigrateWallet)

We should have similar logic for pool ids because now we create pools with ids 1 to 833 in genesis. So your stabelswap pool number is likely to be 835 or so

@p0mvn
Copy link
Member

p0mvn commented Feb 27, 2023

Hi @phamminh0811 . Do you have any updates here?

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 nice work

@p0mvn
Copy link
Member

p0mvn commented Mar 6, 2023

Hi @phamminh0811 . Can you please resolve conflicts here? Let's get this in

@phamminh0811
Copy link
Contributor Author

hi @p0mvn , I have resolved those conflicts, please check it

@phamminh0811 phamminh0811 requested a review from p0mvn March 10, 2023 06:40
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.

Thank you

go.work.sum Outdated Show resolved Hide resolved
@p0mvn p0mvn added V:state/compatible/backport State machine compatible PR, should be backported A:backport/v15.x backport patches to v15.x branch and removed V:state/compatible/no_backport State machine compatible PR, depends on prior breaks labels Mar 11, 2023
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

@p0mvn p0mvn merged commit c99200b into osmosis-labs:main Mar 13, 2023
mergify bot pushed a commit that referenced this pull request Mar 13, 2023
* Create stableswap pools and CL pools

* Refactor code to make sure e2e test upgrading run well

* Update tests/e2e/configurer/chain/commands.go

delete redundant

Co-authored-by: Roman <[email protected]>

* Remove CL pool and change stable token denom

* renome debug bin

* Remove rate limiter

* resolve compile osmosis amd64 for linux error

* resolve conflict

* Update stablepool

Co-authored-by: Ruslan Akhtariev <[email protected]>

* update create function name

Co-authored-by: Ruslan Akhtariev <[email protected]>

* rename create stableswap function

* add rate_limiter.wasm

* Update whitespace error

* add some logic to execute a swap

* update wasm file

* Add stableswap pool test to e2e

* Change TestStableSwapPostUpgrade to try swapping the pool created in post upgrade

* avoid hard code on e2e test

* fix conflict

* Delete go.work.sum

---------

Co-authored-by: Roman <[email protected]>
Co-authored-by: Ruslan Akhtariev <[email protected]>
(cherry picked from commit c99200b)

# Conflicts:
#	tests/e2e/e2e_test.go
p0mvn added a commit that referenced this pull request Mar 13, 2023
pysel added a commit that referenced this pull request Mar 14, 2023
* Create stableswap pools and CL pools

* Refactor code to make sure e2e test upgrading run well

* Update tests/e2e/configurer/chain/commands.go

delete redundant

Co-authored-by: Roman <[email protected]>

* Remove CL pool and change stable token denom

* renome debug bin

* Remove rate limiter

* resolve compile osmosis amd64 for linux error

* resolve conflict

* Update stablepool

Co-authored-by: Ruslan Akhtariev <[email protected]>

* update create function name

Co-authored-by: Ruslan Akhtariev <[email protected]>

* rename create stableswap function

* add rate_limiter.wasm

* Update whitespace error

* add some logic to execute a swap

* update wasm file

* Add stableswap pool test to e2e

* Change TestStableSwapPostUpgrade to try swapping the pool created in post upgrade

* avoid hard code on e2e test

* fix conflict

* Delete go.work.sum

---------

Co-authored-by: Roman <[email protected]>
Co-authored-by: Ruslan Akhtariev <[email protected]>
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 V:state/compatible/backport State machine compatible PR, should be backported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create stableswap and CL pools pre upgrade in v15
3 participants