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.

Sign up for GitHub

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

Jump to bottom

chore: v16 e2e init image updates #5902

Merged
merged 3 commits into from
Jul 28, 2023
Merged

chore: v16 e2e init image updates #5902

merged 3 commits into from
Jul 28, 2023

Conversation

czarcas7ic
Copy link
Member

@czarcas7ic czarcas7ic commented Jul 28, 2023 •
edited
Loading

Closes: #XXX

What is the purpose of the change

We are creating canonical CL pools for a list of gamm pools in the v17 upgrade handler. In order to test if these pools properly get created / linked / SFS, we must modify the v16 init image to create the pools (similar to how we have done it in the past for the stride pool and the v16 CL pool)

Testing and Verifying

Tested against the following PR

#5895

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes?
  • Changelog entry added to Unreleased section of CHANGELOG.md?

Where is the change documented?

  • Specification (x/{module}/README.md)
  • Osmosis documentation site
  • Code comments?
  • N/A

@czarcas7ic czarcas7ic added V:state/compatible/no_backport State machine compatible PR, depends on prior breaks A:no-changelog labels Jul 28, 2023
@github-actions
Copy link
Contributor

Important Notice

This PR includes modifications to the tests/e2e/initialization module.
Please follow the instructions below:

  1. Backport these changes to the previous Osmosis version's branch.
  2. Run the script inside a Docker container to update genesis and configs for pre-upgrade Osmosis.
  3. Merge the backported changes.
  4. The image will be built and uploaded to Docker Hub here.
  5. Grab the latest image and update it in the PR to the main branch replacing the previousVersionInitTag in the osmosis/tests/e2e/containers/config.go

Please let us know if you need any help.

@czarcas7ic czarcas7ic marked this pull request as ready for review July 28, 2023 02:08
@mattverse mattverse changed the title chore: v16 init image updates chore: v16 e2e init image updates Jul 28, 2023
mattverse
mattverse approved these changes Jul 28, 2023
View reviewed changes
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.

Nice, utACK

tests/e2e/initialization/v17.go
Comment on lines +14 to +19
ISTIBCDenom = "ibc/92BE0717F4678905E53F4E45B2DED18BC0CB97BF1F8B6A25AFEDF3D5A879B4D5"
CMSTIBCDenom = "ibc/23CA6C8D1AB2145DD13EB1E089A2E3F960DC298B468CCE034E19E5A78B61136E"
WBTCIBCDenom = "ibc/D1542AA8762DB13087D8364F3EA6509FD6F009A34F00426AF9E4F9FA85CBBF1F"
DOTIBCDenom = "ibc/3FF92D26B407FD61AE95D975712A7C319CDE28DE4D80BDC9978D935932B991D7"
CROIBCDenom = "ibc/E6931F78057F7CC5DA0FD6CEF82FF39373A6E0452BF1FD76910B93292CF356C1"
AKTIBCDenom = "ibc/1480B8FD20AD5FCAE81EA87584D269547DD4D436843C1D20F15E00EB64743EF4"
Copy link
Member

Choose a reason for hiding this comment

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

Manually confirmed that each IBC denom corresponds to the correct denom name

tests/e2e/initialization/v17.go Outdated
},
{
BaseAsset: CMSTIBCDenom,
SpreadFactor: sdk.MustNewDecFromStr("0.0001"), // Normally 0.0002, but is not authorized
Copy link
Member

Choose a reason for hiding this comment

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

just out of curiosity and for context, what do you mean here by not authorized thus having 0.0001 spread factor?

Copy link
Member Author

Choose a reason for hiding this comment

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

We have authorized spread factors as a param and 0.0002 is not one of them

tests/e2e/initialization/config.go
upgradeBaseDenoms := fmt.Sprintf("%s%s,", DaiBalanceA, DaiDenom)
n := len(AssetPairs)
for i, assetPair := range AssetPairs {
if assetPair.BaseAsset == "uion" {
Copy link
Member

Choose a reason for hiding this comment

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

why do we pass for uion?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is already registered in another step. If I removed it from that step and added it here, it would have been more difficult to unwind this change (this change is only needed for v17 and can be removed after)

@czarcas7ic czarcas7ic merged commit f34d07f into v16.x Jul 28, 2023
@czarcas7ic czarcas7ic deleted the adam/v16-init-image-updates branch July 28, 2023 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattverse mattverse mattverse approved these changes

Assignees
No one assigned
Labels
A:no-changelog V:state/compatible/no_backport State machine compatible PR, depends on prior breaks
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@czarcas7ic @mattverse