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

Backport/for v5.0.7 #999

Closed
wants to merge 3 commits into from
Closed

Conversation

yaruwangway
Copy link
Contributor

Closes: #879

Description

Backport:


For contributor use:

  • 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.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@codecov
Copy link

codecov bot commented Sep 29, 2021

Codecov Report

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

@@                Coverage Diff                @@
##             release/v5.0.x     #999   +/-   ##
=================================================
  Coverage                  ?   11.59%           
=================================================
  Files                     ?       11           
  Lines                     ?      966           
  Branches                  ?        0           
=================================================
  Hits                      ?      112           
  Misses                    ?      849           
  Partials                  ?        5           

@joe-bowman
Copy link

joe-bowman commented Sep 29, 2021

This is not compatible with existing cosmoshub-4 versions - most likely the antehandler change I suspect - previous versions are accepting multiple IBC handlers.

This should minimally be 5.1.0, not 5.0.7.

(for reference, I got an apphash mismatch about 4 blocks after restarting the chain with this version).

cosmos_1  | panic: Failed to process committed block (7834928:62C9DAC073167CA743731C12B70856B60326CF0399532524C28FDB3969EBF3EB): wrong Block.Header.AppHash.  Expected 1207E15A20758286DCC6F1A48E50B47B273BC186BA4D58954D87CE2EE4DD4064, got 94087B9A5B3ADC7E78A9B9FCC63DB66244E61C211BD877B4E7268EE9F5B16A66

okwme
okwme approved these changes Sep 29, 2021
@okwme okwme mentioned this pull request Sep 29, 2021
@yaruwangway
Copy link
Contributor Author

this might be due to the newest release of v5.0.6 is not on the branch of v5.0.x. I think we need fix branch first and rebase this branch in this pull request. (not so sure this will solve the app hash mismatch)

@joe-bowman
Copy link

joe-bowman commented Sep 30, 2021 via email

@yaruwangway
Copy link
Contributor Author

It is the antehandler change; v5.0.6 + ask v0.42.10 (with iavl fixes) works a treat (I'm running it in prod as of last night)

On Thu, 30 Sep 2021, 10:46 yaruwangway, @.***> wrote: this might be due to the newest release of v5.0.6 is not on the branch of v5.0.x. I think we need fix branch first and rebase this branch in this pull request. (not so sure this will solve the app hash mismatch) — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#999 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABA7QYAVL3RHY7V6SIBS65LUEQWVTANCNFSM5FADCTCA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

Hi @joe-bowman, thanks! Releasing v5.1.0 makes sense!

@okwme
Copy link
Contributor

okwme commented Oct 7, 2021

closing this in light of releasing v5.0.7 without ante-handler changes and putting them in v5.0.8

@okwme okwme closed this Oct 7, 2021
@tac0turtle tac0turtle deleted the backport/for_v5.0.7 branch December 14, 2021 09:45
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