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

chore!: v22 Namespace & Upgrade Handler #993

Merged
merged 10 commits into from
Apr 21, 2024
Merged

chore!: v22 Namespace & Upgrade Handler #993

merged 10 commits into from
Apr 21, 2024

Conversation

dimiandre
Copy link
Member

  1. v22 upgrade handler
  2. v22 namespace
  3. version bumps - including ibc-go

@dimiandre
Copy link
Member Author

seems tests are broken because of
strangelove-ventures/interchaintest#1043

thanks @faddat for pointing out

@Reecepbcups
Copy link
Contributor

Reecepbcups commented Apr 9, 2024

Comet had a minor state breaking change (types). Can use github.com/cometbft/cometbft => github.com/cometbft/cometbft v0.37.2 in the interchaintest/ go.mod as well since interchaintest does not depend on actual cometbft, but rather just the types and func signatures

@dimiandre
Copy link
Member Author

Comet had a minor state breaking change (types). Can use github.com/cometbft/cometbft => github.com/cometbft/cometbft v0.37.2 in the interchaintest/ go.mod as well since interchaintest does not depend on actual cometbft, but rather just the types and func signatures

thanks reece! seems to do the trick!

@kopeboy
Copy link

kopeboy commented Apr 10, 2024

Are the version bumps needed for this specific chain upgrade or can we do them later, eg. with the help of a new maintainer from Dev Dept's Blockchain engineer RFP? (ofc help by anyone in the community is always appreciated and could be retroactively compensated).

I see a lot of errors (in 8 out of 9 x/modules + in app/apptesting/test_suite.go and app/test_helpers.go, etc.) are caused by the missing functions RequestBeginBlock & RequestEndBlock in the new cometbft. We are bumping it from 0.37.2 to 0.38.4, but they were removed starting from v0.38.0, and I'm a noob but it doesn't seem required:
image

EDIT: I had this comment ready but I posted it late 😅 Happy that you already figured the issue out and that I'm learning to review PRs here 😇

@dimiandre
Copy link
Member Author

Are the version bumps needed for this specific chain upgrade or can we do them later, eg. with the help of a new maintainer from Dev Dept's Blockchain engineer RFP? (ofc help by anyone in the community is always appreciated and could be retroactively compensated).

I see a lot of errors (in 8 out of 9 x/modules + in app/apptesting/test_suite.go and app/test_helpers.go, etc.) are caused by the missing functions RequestBeginBlock & RequestEndBlock in the new cometbft. We are bumping it from 0.37.2 to 0.38.4, but they were removed starting from v0.38.0, and I'm a noob but it doesn't seem required: image

EDIT: I had this comment ready but I posted it late 😅 Happy that you already figured the issue out and that I'm learning to review PRs here 😇

version bumps here are minor, and needed to solve the security issues we had with ibc-go.
When you tidy go deps the versions get minor-bumped automatically

Also ibc-go v7.4 is state breaking comparing v7.3 that's why i'm cutting the v22 release

@faddat
Copy link
Contributor

faddat commented Apr 11, 2024

Every time Juno is upgraded, best practices are:

  • use the latest major golang release
  • use the latest release of EVERY cosmos ecosystem package

The reason for this is that there are frequently security improvements, and performance improvements. I can't think of a time when it was a bad thing to use the latest dependencies. It is reasonable to use cosmos-sdk and ibc-go as a guide for what's freshest.

@dimiandre tidying doesn't update things automatically. It takes its configuration from the contents of go.mod and prepares a new go.sum file from go.mod

Copy link
Contributor

@faddat faddat left a comment

Choose a reason for hiding this comment

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

This doesn't update some ecosystem libraries. I've done the updates, you'd just need to merge the PR below:

#994

x/tokenfactory/keeper/keeper.go Dismissed Show dismissed Hide dismissed
* update the juno update

* tidy ict

* remove toolchain line

* bump go

* bump cometbft-db

* update pfm and ibc-hooks

* tidy interchaintest

* lock exp in interchaintest

* bump cometbft-db

* tidy

* tidy ict
@dimiandre dimiandre added the review me Review me for merge label Apr 21, 2024
@dimiandre dimiandre merged commit 32568db into main Apr 21, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review me Review me for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants