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

deps(go): use cosmos/ibc-go v6.3.0 #9341

Merged
merged 6 commits into from
May 14, 2024
Merged

deps(go): use cosmos/ibc-go v6.3.0 #9341

merged 6 commits into from
May 14, 2024

Conversation

LuqiPan
Copy link
Contributor

@LuqiPan LuqiPan commented May 8, 2024

closes: #9205
refs: #9205

Description

This PR updates ibc-go in response to GHSA-j496-crgh-34mx

TODO

  • upgrade our runbook with a note on async channel version negotiation

Security Considerations

This PR addresses GHSA-j496-crgh-34mx

Scaling Considerations

N/A

Documentation Considerations

N/A

Testing Considerations

Relying on our CI/CD to verify things as this is a simple dependency version bump

Upgrade Considerations

N/A

@LuqiPan LuqiPan added the force:integration Force integration tests to run on PR label May 8, 2024
Copy link

cloudflare-pages bot commented May 8, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: b2a90dd
Status: ✅  Deploy successful!
Preview URL: https://a6dbfe3b.agoric-sdk.pages.dev
Branch Preview URL: https://9205-update-ibc-go.agoric-sdk.pages.dev

View logs

@@ -192,9 +192,6 @@ replace (
// Pick up an IAVL race fix.
github.com/cosmos/iavl => github.com/cosmos/iavl v0.19.7

// Async version negotiation
github.com/cosmos/ibc-go/v6 => github.com/agoric-labs/ibc-go/v6 v6.2.1-alpha.agoric.3
Copy link
Member

Choose a reason for hiding this comment

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

@michaelfig should confirm, but looking at the diff of v6.2.1-alpha.agoric.3 from upstream v6.2.1, I don't think we can move away from our fork. And there are hopefully tests in agoric-sdk that will fail if we try.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think we can move away from our fork. And there are hopefully tests in agoric-sdk that will fail if we try.

@LuqiPan's changes in this PR remove the async version negotiation feature detection between our ibc-go fork and agoric-sdk, which allows the tests to pass again while preventing agoric-sdk from using the AsyncVersion code.

In a meeting some time ago, we had agreed not to use that semi-automatic feature detection, and instead would document in the interchain stack upgrade runbook the conditions under which we could set AsyncVersions = true, namely by performing some investigation into the ibc-go implementation we rely on before landing PRs like this one.

Copy link
Member

@michaelfig michaelfig left a comment

Choose a reason for hiding this comment

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

Blocking until the interchain stack upgrade runbook describes the work needed to upgrade AsyncVersions. I'd be happy to help you document this, @LuqiPan.

@LuqiPan
Copy link
Contributor Author

LuqiPan commented May 10, 2024

Created #9358 as reference issue, I am updating this PR and interchain stack upgrade runbook to reference this issue.

@LuqiPan LuqiPan added the automerge:squash Automatically squash merge label May 10, 2024
@LuqiPan
Copy link
Contributor Author

LuqiPan commented May 10, 2024

Updated this PR and https://github.com/Agoric/agoric-sdk/wiki/Upgrading-the-Interchain-Stack (at the bottom) with #9358, should be ready for another look

@LuqiPan LuqiPan marked this pull request as ready for review May 10, 2024 17:20
@LuqiPan LuqiPan requested a review from michaelfig May 10, 2024 17:20
golang/cosmos/go.mod Outdated Show resolved Hide resolved
@LuqiPan LuqiPan requested a review from michaelfig May 14, 2024 21:24
@mergify mergify bot merged commit 6b5e706 into master May 14, 2024
63 checks passed
@mergify mergify bot deleted the 9205-update-ibc-go branch May 14, 2024 22:36
@aj-agoric
Copy link

Moved back to code complete since bot should not have moved this to "done-mn validated"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:squash Automatically squash merge force:integration Force integration tests to run on PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade Ibc-go to address ASA-2024-007
4 participants