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

go-kosu: Use MultiStore #141

Merged
merged 4 commits into from Jun 27, 2019
Merged

go-kosu: Use MultiStore #141

merged 4 commits into from Jun 27, 2019

Conversation

@gchaincl
Copy link
Contributor

gchaincl commented Jun 26, 2019

Overview

Use MultiStore engine from cosmos-sdk

Description

Replaces the state management using the right tool for the job +1
Some other minor changes are included here.

@hrharder

This comment has been minimized.

Copy link
Member

hrharder commented Jun 26, 2019

Awesome! I will test this out today @gchaincl. Yesterday i was able to recreate the issue in the test-net with nodes not getting the same AppHash for blocks (resulting in consensus failure) so hopefully this resolves that. Did you get to trying it out in the docker-compose testnet yet?

gchaincl added 2 commits Jun 26, 2019
Copy link
Member

hrharder left a comment

Merge whenever you feel is ready @gchaincl. i will try to spend some time investigating the rebalance issue

@hrharder

This comment has been minimized.

Copy link
Member

hrharder commented Jun 27, 2019

I believe I have fixed the rebalance issue (see fix/go-kosu/rebalance-issue)

Was able to get it to go up to ~10 in round number before I turned it off.

I believe the issue was that all 4 rebalance transactions were getting executed instead of one, the reason being DeliverTx assumed all Tx's that passed CheckTx are still valid, even though they may not be. So by running CheckTx again before modifying the state, we can make sure the transaction from the mempool is still valid.

@gchaincl I also set the initial round info in the Store during my debugging, and I'm not sure if that contributed to the fix, but I left it.

Feel free to merge my branch in to this, or just add the additional checkTx call here.

@gchaincl

This comment has been minimized.

Copy link
Contributor Author

gchaincl commented Jun 27, 2019

Great news! I remember I was checking it twice but then I removed it cause I thought it was duplicated. Thanks for finding the problem!

@gchaincl gchaincl merged commit 3ae0d43 into master Jun 27, 2019
1 check passed
1 check passed
continuous-integration/drone/pr Build is passing
Details
@gchaincl gchaincl deleted the go-kosu/multi-store branch Jun 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.