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

Update iavl with WorkingHash fix #1484

Merged
merged 1 commit into from
May 9, 2024
Merged

Update iavl with WorkingHash fix #1484

merged 1 commit into from
May 9, 2024

Conversation

roy-dydx
Copy link
Contributor

@roy-dydx roy-dydx commented May 9, 2024

Changelist

WorkingHash was computed incorrectly for trees that are initialized with an InitialVersion. This happens when a new store is introduced in an upgrade. If the store is empty, the hash ends up being the same anyways so the bug only manifests when the store has values at the upgrade height.

See dydxprotocol/iavl@1c8b8e7.

Test Plan

[Describe how this PR was tested (if applicable)]

Author/Reviewer Checklist

  • If this PR has changes that result in a different app state given the same prior state and transaction list, manually add the state-breaking label.
  • If the PR has breaking postgres changes to the indexer add the indexer-postgres-breaking label.
  • If this PR isn't state-breaking but has changes that modify behavior in PrepareProposal or ProcessProposal, manually add the label proposal-breaking.
  • If this PR is one of many that implement a specific feature, manually label them all feature:[feature-name].
  • If you wish to for mergify-bot to automatically create a PR to backport your change to a release branch, manually add the label backport/[branch-name].
  • Manually add any of the following labels: refactor, chore, bug.

protocol/go.mod Outdated
@@ -432,7 +432,7 @@ replace (
github.com/cometbft/cometbft => github.com/dydxprotocol/cometbft v0.38.6-0.20240426214049-c8beeeada40a
// Use dYdX fork of Cosmos SDK
github.com/cosmos/cosmos-sdk => github.com/dydxprotocol/cosmos-sdk v0.50.6-0.20240326192503-dd116391188d
github.com/cosmos/iavl => github.com/dydxprotocol/iavl v1.1.1-0.20240408175732-0fca9d69cbc4
github.com/cosmos/iavl => github.com/dydxprotocol/iavl v1.1.1-0.20240509160433-4cb59900bfb2
Copy link
Contributor

@teddyding teddyding May 9, 2024

Choose a reason for hiding this comment

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

Question on the underlying iavl fork commit: instead of allowing roy/* should we specify roy/v1.x.x-fix instead to not push future test commits

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't matter. Rule only applies for the branch itself.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, we don't plan to continue managing the our iavl fork right? We are just using this commit for the WorkingHash fix

@vincentwschau
Copy link
Contributor

vincentwschau commented May 9, 2024

The commit dydxprotocol/iavl@4cb59900bfb2 is failing tests. I don't think this is expected.

@roy-dydx roy-dydx merged commit 711ae63 into main May 9, 2024
16 checks passed
@roy-dydx roy-dydx deleted the roy/workinghash branch May 9, 2024 16:31
@roy-dydx
Copy link
Contributor Author

roy-dydx commented May 9, 2024

@Mergifyio backport release/protocol/v5.x

Copy link

mergify bot commented May 9, 2024

backport release/protocol/v5.x

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request May 9, 2024
roy-dydx added a commit that referenced this pull request May 9, 2024
(cherry picked from commit 711ae63)

Co-authored-by: roy-dydx <133032749+roy-dydx@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants