Skip to content

feat: upgrade ibc-go to Cosmos SDK v0.54 + CometBFT v1.0.1 #8358

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

fcecin
Copy link

@fcecin fcecin commented May 7, 2025

Description

This PR upgrades ibc-go to Cosmos SDK v0.54, which is based on CometBFT v1.0.1.

NOTE: This PR/branch is a WIP. It is flagged "ready for review" so it will run the e2e tests, but it should not be merged. This is not actually ready for review.

This branch is currently depending to an unreleased version of the Cosmos SDK that was upgraded to CometBFT v1.0.1 and that is passing all its tests. These ibc-go dependencies are all at the same commit 74993f0a47e5. This PR will be fixed to use an actual SDK release when it is available.

This branch is also using a fork of interchaintest that is upgraded to both CometBFT v1.0.1 and the upgraded ibc-go root module (which interchaintest depends on); before this PR is finalized, this interchaintest fork should be retired and the code changes should be ported to the intechaintest fork that ibc-go was already using or the original interchaintest repo.


Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against the correct branch (see CONTRIBUTING.md).
  • Linked to GitHub issue with discussion and accepted design, OR link to spec that describes this work.
  • Code follows the module structure standards and Go style guide.
  • Wrote unit and integration tests.
  • Updated relevant documentation (docs/).
  • Added relevant godoc comments.
  • Provide a conventional commit message to follow the repository standards.
  • Include a descriptive changelog entry when appropriate. This may be left to the discretion of the PR reviewers. (e.g. chores should be omitted from changelog)
  • Re-reviewed Files changed in the GitHub PR explorer.
  • Review SonarCloud Report in the comment section below once CI passes.

fcecin added 3 commits May 6, 2025 21:42
this commit does not port the ./e2e module, since it depends on
interchaintest, which in turns depends on the ibc-go root module.

this commit includes a port of the root module, which can then be
imported by the upgraded interchaintest.
this commit updates the interchaintest dependency to a temporary
commit of the fcecin/interchaintest fork which contains fixes
for comet v1.

this is a WIP as e2e tests are failing
@fcecin fcecin marked this pull request as ready for review May 7, 2025 19:52
@faddat
Copy link
Contributor

faddat commented May 12, 2025

Thank you!

@faddat
Copy link
Contributor

faddat commented May 12, 2025

Any idea why the tests are failing?

Also, I'd be really happy to fix these CI settings, so might as well apply for a gig at ICL.

@gjermundgaraba
Copy link
Contributor

Most of the non-e2e related issues should hopefully be resolved in #8365

@faddat
Copy link
Contributor

faddat commented May 13, 2025

I might use this branch as a dev platform for a bit, I'll give feedback as I do that.

@faddat
Copy link
Contributor

faddat commented May 13, 2025

Here's the repo with the result - it's literally just this Simapp extracted. I'm liking it as a template because it addresses the core "why" of users of the SDK: IBC.

If wasmvm were cleaned up, then cosmwasm would be shipped in there too, but for now, I think it's fair to say that this branch represents a minimal cosmos chain that just might contain fixes to issues like p2p storms, and will usable in production in the near future.

So, we've got to build a chain that's going to see enormous tx volume, and see what happens.

This template is also suited to making micro-currencies. It's as minimal as possible, so there's a smaller attack surface area, so it is what I would recommend to teams new to using the cosmos-sdk, instead of offering cosmwasm, which we already more or less know presents a number of issues (though does have a lot of compelling value)

I might actually contact Limechain, to see if they'd like to record a video on the changes in performance. We got that very tightly nailed down when we made a network of raspberry Pi devices.

Basically if I were noble, etc... I'd look at this as "the thing to audit when looking for issues that sit below our application", so that they can distinguish between issues in the underlying stack and issues in their application code.

We should try to ship a quarter-solana on a network of raspberry Pi's. When we are doing that, we are hitting what I think is the theoretical limit of the stack.

Anyhow when you peel back the skin some, what you find is a lot of stuff like this:

And it's little stuff like that (and probably some big things too) that is holding back db perf generally.

This is kind of like how I ended up fixated on wasmvm:

While it isn't omnipresent, it's a very, very common dependency for a lot of valuable applications, so the oddities surrounding how wasmvm works aren't really okay.

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.

3 participants