Finschia / finschia-sdk

A framework for building blockchains based Finschia Mainnet that is forked from cosmos-sdk
Apache License 2.0
61 stars 31 forks source link

fix: app-hash mismatch if upgrade migration commit is interrupted(backport cosmos/cosmos-sdk#13530) #1310

Closed jaeseung-bae closed 8 months ago

jaeseung-bae commented 8 months ago

Description

Motivation and context

How has this been tested?

Screenshots (if appropriate):

Checklist:

CLAassistant commented 8 months ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

:white_check_mark: jaeseung-bae
:x: mmsqe
You have signed the CLA already but the status is still pending? Let us recheck it.

codecov[bot] commented 8 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 70.42%. Comparing base (e5709f3) to head (ab6defa). Report is 1 commits behind head on main.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/Finschia/finschia-sdk/pull/1310/graphs/tree.svg?width=650&height=150&src=pr&token=m16qfzIPO7&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Finschia)](https://app.codecov.io/gh/Finschia/finschia-sdk/pull/1310?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Finschia) ```diff @@ Coverage Diff @@ ## main #1310 +/- ## ======================================= Coverage 70.41% 70.42% ======================================= Files 643 643 Lines 54759 54765 +6 ======================================= + Hits 38560 38566 +6 Misses 14024 14024 Partials 2175 2175 ``` | [Files](https://app.codecov.io/gh/Finschia/finschia-sdk/pull/1310?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Finschia) | Coverage Δ | | |---|---|---| | [store/rootmulti/store.go](https://app.codecov.io/gh/Finschia/finschia-sdk/pull/1310?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Finschia#diff-c3RvcmUvcm9vdG11bHRpL3N0b3JlLmdv) | `74.59% <100.00%> (+0.27%)` | :arrow_up: |
jaeseung-bae commented 8 months ago

Is it a breaking change? Theoretically, yes it is, but the consequence would be chain halt. In my opinion, we may backport the fix. I'd like to get opinions from other reviewers.

Thanks for the comment. I was a little bit confused with whether it's breaking change or not.

jaeseung-bae commented 8 months ago

Is it a breaking change? Theoretically, yes it is, but the consequence would be chain halt. In my opinion, we may backport the fix. I'd like to get opinions from other reviewers.

Thanks for the comment. I was a little bit confused with whether it's breaking change or not.

After I re-check, I concluded that it's not a consensus breaking change.

For reviewers, please feel free to comment any opinions on this.

ulbqb commented 8 months ago

I think this issue is about replaying blocks. Consensus wouldn't be related.