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

fix: fix ibc error #654

Merged
merged 2 commits into from
Nov 23, 2021
Merged

fix: fix ibc error #654

merged 2 commits into from
Nov 23, 2021

Conversation

aleem1314
Copy link
Contributor

@aleem1314 aleem1314 commented Nov 20, 2021 •
edited by zube bot
Loading

Description

Closes: #653

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title
  • added ! to the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • followed the guidelines for building modules
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • included comments for documenting Go code
  • updated the relevant documentation or specification
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed ! in the type prefix if API or client breaking change
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage
  • manually tested (if applicable)

@aleem1314 aleem1314 requested a review from anilcse November 20, 2021 06:55
@codecov
Copy link

codecov bot commented Nov 20, 2021 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (release/v2.0.x@4803705). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head afd8c56 differs from pull request most recent head 1bae0f8. Consider uploading reports for the commit 1bae0f8 to get more accurate results

@@                Coverage Diff                @@
##             release/v2.0.x     #654   +/-   ##
=================================================
  Coverage                  ?   75.59%           
=================================================
  Files                     ?      106           
  Lines                     ?    14143           
  Branches                  ?        0           
=================================================
  Hits                      ?    10691           
  Misses                    ?     2828           
  Partials                  ?      624
Flag Coverage Δ
experimental-codecov 75.59% <0.00%> (?)
stable-codecov 67.18% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

@aleem1314 aleem1314 changed the title fix: add ibc migration patch fix: fix ibc error Nov 20, 2021
@ryanchristo ryanchristo added the backport/v2.0.x backport to release/v2.0.x branch label Nov 20, 2021
robert-zaremba
robert-zaremba reviewed Nov 21, 2021
View reviewed changes
app/app.go
app.AddPatch(3038452, func(ctx sdk.Context) error {
app.IBCKeeper.ConnectionKeeper.SetParams(ctx, ibcconnectiontypes.DefaultParams())
return nil
})
Copy link
Collaborator

Choose a reason for hiding this comment

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

so, this is a hack to get consensus breaking change without doing a SoftwareUpgrade proposal?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, correct

anilcse
anilcse approved these changes Nov 23, 2021
View reviewed changes
Copy link
Contributor

@anilcse anilcse left a comment

Choose a reason for hiding this comment

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

tACK

@aleem1314 aleem1314 marked this pull request as ready for review November 23, 2021 06:40
clevinson
clevinson approved these changes Nov 23, 2021
View reviewed changes
Copy link
Member

@clevinson clevinson left a comment

Choose a reason for hiding this comment

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

Approving as is for final testing of v2.1.0-rc1. We will need to update the upgrade height before publishing final release.

@clevinson clevinson enabled auto-merge (squash) November 23, 2021 16:39
@clevinson clevinson merged commit 446a7cf into release/v2.0.x Nov 23, 2021
@clevinson clevinson deleted the aleem/v2.0-ibc-fix branch November 23, 2021 16:45
@ryanchristo
Copy link
Member

Should this be included in master as well?

@robert-zaremba
Copy link
Collaborator

No, the params are stored in the state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robert-zaremba robert-zaremba robert-zaremba left review comments

@anilcse anilcse anilcse approved these changes

@clevinson clevinson clevinson approved these changes

Assignees
No one assigned
Labels
backport/v2.0.x backport to release/v2.0.x branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@aleem1314 @ryanchristo @robert-zaremba @clevinson @anilcse