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

Validate block hash and signatures #808

Closed
wants to merge 44 commits into from

Conversation

JekaMas
Copy link
Contributor

@JekaMas JekaMas commented Oct 4, 2018

It's a first iteration fixing our consensus and voting.

When I had added proposed block and block fragments validation, the consensus crushed, panicked and crushed. So I had to make many changes until it started work better.

Main changes:

  • fixed double choosing a proposer in each round
  • fixed votingSystem algorithm
  • fixed bug with starting validator.Start twice, that was caused incorrect voting, a part of Merkle proof errors and all duplicate vote errors.
  • force some wait after proposal block sent to prevent a data race. If proposer announced a block it switches to PreVote stage that only 100ms, at the same time all other validators have to wait up too ProposalTimeout (>400ms), so it can happen that the proposer will fail with VoteTimeout constantly
  • validating a proposal block and block fragments: restored a correct block sign, checking block number and hash, checking validator's round and chainID
  • removed data race between receiving a proposal announce and block fragments
  • removed a data race related to events.Post
  • check voters addresses in voting_table while checking for duplicated votes
  • refactored client/core/voting_table_test.go to remove shared state between tests and data races
  • fixed stateDB panic when more than one restore revision was called
  • In client/knode/validator/validator.go val.round is always 0: init -> val.round = 0, newRoundState -> if val.round != 0 -> val.round++. It causes that a part of init doesn't work and votingSystem always gets round=0 (causes duplicate votes). So added a proper round change and inits.

Cons:

  • crush-recovery doesn't work fine.

Closes #787 #91

Next steps are in #837

@JekaMas JekaMas added the WIP Work in progress label Oct 4, 2018
@JekaMas JekaMas force-pushed the issue/validate-block-hash-and-signatures-#787 branch from c43e6a4 to 9530a60 Compare October 8, 2018 17:08
@JekaMas JekaMas force-pushed the issue/validate-block-hash-and-signatures-#787 branch from 9b83557 to f911d5b Compare October 9, 2018 07:56
@JekaMas JekaMas force-pushed the issue/validate-block-hash-and-signatures-#787 branch 4 times, most recently from c850bc0 to bfc25ef Compare October 10, 2018 15:44
@JekaMas JekaMas force-pushed the issue/validate-block-hash-and-signatures-#787 branch from 838e083 to 1afca6a Compare October 19, 2018 07:11
And I restart the validator
And I wait for my node to be synced
Then My node should be a validator
# Scenario:
Copy link
Member

Choose a reason for hiding this comment

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

Why did you remove this?

@JekaMas JekaMas force-pushed the issue/validate-block-hash-and-signatures-#787 branch from 112910f to 3959f0c Compare October 19, 2018 08:20
@@ -37,11 +35,12 @@ func WaitMined(ctx context.Context, backend Backend, txHash common.Hash) (*types
} else {
logger.Trace("Transaction not yet mined")
}
// Wait for the next round.

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we could check ctx timeout and break the func being a loop

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The check is a few lines below the code fragment :)

})

if revertedIndex == -1 {
panic(fmt.Errorf("revision id %v cannot be reverted. found the revision at index %d. latest revision index %d",
Copy link
Contributor

Choose a reason for hiding this comment

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

why not our normal log.Crit with context

votes map[common.Hash]*Vote // non-nil votes
nilVotes map[common.Hash]*Vote // nil votes
counter map[common.Hash]int // map[block.Hash]count
isVoted map[common.Address]struct{} // map[voterAddress]count
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure if you need this, the votes could be replaced by AddressVote?

@JekaMas JekaMas force-pushed the issue/validate-block-hash-and-signatures-#787 branch 2 times, most recently from 89f12ae to 5687ad6 Compare October 19, 2018 12:07
@JekaMas JekaMas force-pushed the issue/validate-block-hash-and-signatures-#787 branch from 5687ad6 to 4748217 Compare October 22, 2018 07:24
@JekaMas JekaMas force-pushed the issue/validate-block-hash-and-signatures-#787 branch from 4748217 to 5aeaa84 Compare October 22, 2018 10:15
@JekaMas JekaMas force-pushed the issue/validate-block-hash-and-signatures-#787 branch from f5721c7 to 94cc480 Compare October 22, 2018 17:41
@yourheropaul
Copy link
Member

yourheropaul commented Oct 23, 2018

On the local cluster, I get errors like

WARN [10-22|12:36:11.066] Majority of validators pre-voted nil
WARN [10-22|12:36:11.292] got a proposal block add error           err="expected proposed block round 33, got 44" block="\n\tProposal(969333d451f6cc2a35a298d94bc5d425b04e2eb90a877de797133
3ffaed6acf7)\n\tBlock Number:\t\t69\n\tRound:\t  \t\t\t44\n\tLocked Block:\t\t0000000000000000000000000000000000000000000000000000000000000000\n\tLocked Round:\t\t0\n\tV:        \t\t\t0x2
8\n\tR:        \t\t\t0xbabea431231c85c8c130f6661c727c7895168fc2e1fe491557385e88c82842ac\n\tS:        \t\t\t0x3a2119e7492f8e8f6168c66e2d39b15801f097443fc89d00c5917ab661be3b63\n\tHex:
\t\t\tf88a452c80a00000000000000000000000000000000000000000000000000000000000000000e201a0000000000000000000000000000000000000000000000000000000000000000028a0babea431231c85c8c130f6661c727c7
895168fc2e1fe491557385e88c82842aca03a2119e7492f8e8f6168c66e2d39b15801f097443fc89d00c5917ab661be3b63\n"
WARN [10-22|12:36:12.678] Proposal's block is nil, voting nil      block=true

@JekaMas JekaMas force-pushed the issue/validate-block-hash-and-signatures-#787 branch from dbde51c to 9b8dc01 Compare October 23, 2018 16:57
@JekaMas JekaMas force-pushed the issue/validate-block-hash-and-signatures-#787 branch from 9b8dc01 to 62c3cd6 Compare October 23, 2018 16:59
@yourheropaul yourheropaul added the WIP Work in progress label Oct 26, 2018
@JekaMas JekaMas force-pushed the issue/validate-block-hash-and-signatures-#787 branch 3 times, most recently from 61d90f1 to abddd3d Compare October 29, 2018 09:42
@JekaMas JekaMas force-pushed the issue/validate-block-hash-and-signatures-#787 branch from abddd3d to 3573d76 Compare October 29, 2018 10:04
@rgeraldes rgeraldes closed this May 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validate block hash and signatures
5 participants