Skip to content

consensus: remove logic to unlock block on 2/3 prevote for nil#6954

Merged
williambanfield merged 40 commits intowb/proposer-based-timestampsfrom
wb/issue-6849
Sep 24, 2021
Merged

consensus: remove logic to unlock block on 2/3 prevote for nil#6954
williambanfield merged 40 commits intowb/proposer-based-timestampsfrom
wb/issue-6849

Conversation

@williambanfield
Copy link
Contributor

@williambanfield williambanfield commented Sep 16, 2021

What does this change do?

This change removes the logic for a validator to 'unlock' its locked block if it sees > 2/3 prevotes for nil in a round. This condition is not in the algorithm description in the Tendermint paper.

This logic was present in both enterPrecommit and addVote. The logic in addVote has been updated to only unlock on receipt of 2/3 prevotes for non-nil values and the logic in enterPrecommit has been completely removed.

This change also updates the tests to add a new test for the unlock condition. As a cosmetic fixup, the test file was also updated to match the comments with the current names of the Lock suite of tests. Finally, the Relock test was updated to use the lock channels to ensure the unlock/lock logic remained correct despite this change. The test for the old behavior was removed.

Notes

  • The logic for a validator to prevote its own locked block regardless of proposal is maintained during this change. It will be updated in a future pull request.

closes: #6849

@williambanfield
Copy link
Contributor Author

cc @zeu5

Copy link
Contributor

@cmwaters cmwaters left a comment

Choose a reason for hiding this comment

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

Looks mostly good. I have a few questions / comments.

@Alex-duzhichao
Copy link

Alex-duzhichao commented Dec 1, 2021

@williambanfield I have also found that the implementation of tendermint is different from the algorithm description in tendermint paper. That's confusing. So thank you for your changes!

Another problem I can't understand is the SkipTimeoutCommit parameter which also haven't been explained in tendermint paper. Why can't we enter a new height straightly but have to wait TimeoutCommit ? Can you give me some inspiration about SkipTimeoutCommit ? Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants