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

V0.47.x ibc #2889

Closed
wants to merge 74 commits into from
Closed

V0.47.x ibc #2889

wants to merge 74 commits into from

Conversation

faddat
Copy link
Contributor

@faddat faddat commented Dec 6, 2022

Description

Our weekly software workshop was on SDK 47.

Seems we've got a solid branch going!

Please welcome @RusAkh and @GNaD13 -- I think these are their first commits on ibc-go.

Commit Message / Changelog Entry

This PR attempts to ONLY upgrade ibc-go to cosmos-sdk v0.47.x


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 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.
  • Wrote unit and integration tests.
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/).
  • Added relevant godoc comments.
  • Provide a commit message to be used for the changelog entry in the PR description for review.
  • Re-reviewed Files changed in the Github PR explorer.
  • Review Codecov Report in the comment section below once CI passes.

@faddat faddat requested a review from tmsdkeys as a code owner December 8, 2022 13:01
@colin-axner
Copy link
Contributor

Hi, would it be possible to consolidate effort? How does this pr differ from #2672? I'd prefer to go with #2672 only because I've already reviewed that and contributed fixes. Would it be possible to open pr's to that pr to fix any remaining tests? Once we get the tests passing we can merge into main before waiting for a final v0.47 release

Small note, ibc-go does not need pulsar for it's proto files

@faddat
Copy link
Contributor Author

faddat commented Dec 8, 2022

What we can do is begin to work from the now updated #2672. It hadn't been touched in a long time, so we began work here.

I think that it's very important that you please consider what an outside observer would have seem of #2672 --which is nothing, literally, over a 2 week period of time until yesterday, and yesterday, when the request was made, there were no new commits on the other branch.

We can absolutely collaborate, and I'm absolutely requesting that the ibc-go team communicate so that we can collaborate.

I asked our team to continue work on this branch because when we were asked to stop work on this branch, the most recent commit on the branch was

fe2f2c2

and the branch looked solidly abandoned, and that was our experience with 46. Our assumption was that everyone else was already at full capacity, we were confused why asked to stop work, as getting the full cosmos stack singing in unison is a key strategic objective.

What we (and other multi-chain validators and maintainers) need is the full cosmos stack stable on a fresh release of the SDK. The 46 series is much, much pain and it seems that 46.6 will soon join every other release of the 46 series on the pile of unusable 46 releases, so this is a really important objective for us.

Next, I must gently remind members of the ibc-go team that we actively promoted the use of SDK 46. That has proven to be a significiant error on our part, leaving us with two choices:

  • do like most and recommend / ship with tendermint 34 + sdk 45
  • work really hard to ensure that sdk 47 goes well

For the purpose of reference, I'll also go ahead and define the

full cosmos stack:

  • tendermint
  • cosmos-sdk
  • ibc-go
  • interchain-accounts-demo
  • interchain security
  • CosmWasm
  • iavl
  • cosmos-db
  • ics23
  • ledger-cosmos-go
  • liquidity-staking-module

All of those items, and likely more -- need to work together smoothly.

Our Objectives

On forks

With recent updates, and with Dragbonberry, it was made clear that having 20+ microforks of the SDK is an active danger. At Notional, we're rejecting chains like Rebus for validation, due to their use of a forked ibc-go library. One of our big hopes with v0.47.x is that we can begin to converge the most significant fork -- osmosis-labs/cosmos-sdk back with the main branch.

Conclusion

Thus, I am going to close this PR, and we will begin several new branches (one per team member working on this) and we'll gladly contribute to the branch in question.

We attend the cosmos-sdk and tendermint community calls, and gain a ton of insight from them. Does ibc-go have such a call?

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.

9 participants