-
Notifications
You must be signed in to change notification settings - Fork 607
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
Upgrade IAVL and SDK with RAM improvements and bug fixes for v6.4 #907
Conversation
Codecov Report
@@ Coverage Diff @@
## v6.x #907 +/- ##
=======================================
Coverage 19.06% 19.06%
=======================================
Files 167 167
Lines 23684 23684
=======================================
Hits 4516 4516
Misses 18387 18387
Partials 781 781 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM once two minor comments are in!
CHANGELOG.md
Outdated
|
||
### SDK fork updates | ||
|
||
- [sdk-#114](https://github.com/osmosis-labs/cosmos-sdk/pull/114) [commit](https://github.com/osmosis-labs/cosmos-sdk/pull/114/commits/7ec05684856bf87b868f67d05459d64931ac599d) upgrading iavl with ram optimizations during migration and extra logs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah sorry I didn't click through to the PR before, didn't realize its the bug fix. So then we just need to update this log message.
- [sdk-#114](https://github.com/osmosis-labs/cosmos-sdk/pull/114) [commit](https://github.com/osmosis-labs/cosmos-sdk/pull/114/commits/7ec05684856bf87b868f67d05459d64931ac599d) upgrading iavl with ram optimizations during migration and extra logs | |
- [sdk-#114](https://github.com/osmosis-labs/cosmos-sdk/pull/114) upgrading iavl with ram optimizations during migration and extra logs | |
- [commit](https://github.com/osmosis-labs/cosmos-sdk/pull/114/commits/7ec05684856bf87b868f67d05459d64931ac599d) Fix the bug when a node crashes during commit, and runs into non-logical commit issues. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see what you meant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
Syncing now....
Closes: #XXX
Description
The main focus of this upgrade is stability improvements with the IAVL patch of v0.44.3x-osmo-v5.1. The big points are:
The bugs people were seeing for nodes being saved to a different hash, etc. happen after restoring the node post-crash during commit (the crash could be caused by something else). The bug on restart stems from the long-standing issue that SDK commits are actually not-atomic. This manifested in a failure to flush the latest height when an error during commit occurs, causing the state invalidities on restart. We mitigated the non-atomicity problem by flushing the metadata about the stores that successfully committed a new height. As a result, on restart, the SDK now knows which stores/modules are already on the newly committed height.
For contributor use:
docs/
) or specification (x/<module>/spec/
)Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorer