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

Implement Randomized leader election based on the VRF #1841

Merged
merged 13 commits into from
Jan 28, 2020

Conversation

HoOngEe
Copy link
Contributor

@HoOngEe HoOngEe commented Nov 4, 2019

It resolves #1897

@kseo kseo requested a review from majecty November 4, 2019 03:14
@HoOngEe HoOngEe force-pushed the VRF-election branch 2 times, most recently from f016c4b to bffb073 Compare November 4, 2019 11:02
core/src/consensus/sortition/binom_cdf.rs Outdated Show resolved Hide resolved
core/src/consensus/sortition/lot.rs Outdated Show resolved Hide resolved
@majecty majecty added the do-not-merge Do not merge (for mergify.io) label Nov 5, 2019
@HoOngEe HoOngEe force-pushed the VRF-election branch 8 times, most recently from c3eac7f to c85cdf9 Compare November 7, 2019 02:58
@HoOngEe HoOngEe requested a review from hyunsikjeong November 7, 2019 02:59
@HoOngEe HoOngEe force-pushed the VRF-election branch 7 times, most recently from baac827 to a310782 Compare November 12, 2019 06:24
@majecty
Copy link
Contributor

majecty commented Nov 12, 2019

PriorityMessage part and Account provider part look good to me.
Let's split them as a new PR and merge them!

@HoOngEe HoOngEe force-pushed the VRF-election branch 2 times, most recently from cf7dfef to b9d3b52 Compare November 18, 2019 11:34
@HoOngEe HoOngEe removed the request for review from hyunsikjeong November 22, 2019 06:45
@HoOngEe HoOngEe force-pushed the VRF-election branch 4 times, most recently from 7add5db to 8767752 Compare November 27, 2019 08:57
@HoOngEe HoOngEe force-pushed the VRF-election branch 7 times, most recently from 82f315a to 09cf2ff Compare January 20, 2020 09:24
@majecty
Copy link
Contributor

majecty commented Jan 21, 2020

Please fix this commit message.

Change the possible tendermint states
TO accept multiple proposals and wait their imports, A new product type is required.

TO -> To, wait -> wait for

Also, what do you mean by the term "product type"?

@@ -708,7 +703,7 @@ impl Worker {
// need to reset vote
self.broadcast_state(
vote_step,
self.proposal.block_hash(),
self.votes.get_highest_proposal_summary(self.current_sortition_round()),
Copy link
Contributor

Choose a reason for hiding this comment

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

I had a concern about this line. The concern is how to handle the highest proposal block if the block was invalid. There are many strategies to make a better situation.

However, we can think of a situation like this. A proposer is elected by the VRF. The proposer chooses not to create a block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If validators are elected as eligible proposers but they send invalid proposal block then all validators may agree on Nil. I think this is natural behavior.

vrf_seed_info: Box::new(current_seed),
}
} else {
cwarn!(ENGINE, "Seal request for an old view");
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use cdebug! here. Also, let's change the log message.
This else case is executed when there is a proposal that has a higher score.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modified the message

core/src/consensus/tendermint/network.rs Outdated Show resolved Hide resolved
core/src/consensus/tendermint/worker.rs Outdated Show resolved Hide resolved
finalized_view_of_current_block: Option<View>,
}

fn load_with_version<T: Decodable>(db: &dyn KeyValueDB) -> Option<T> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, it is a fantastic way to load data.

core/src/consensus/tendermint/worker.rs Show resolved Hide resolved
@HoOngEe HoOngEe force-pushed the VRF-election branch 2 times, most recently from 7ecb713 to c644eab Compare January 23, 2020 09:27
@HoOngEe
Copy link
Contributor Author

HoOngEe commented Jan 23, 2020

I changed the commit message on the commit Change the possible tendermint states

To accept multiple proposals and wait for their imports, I introduced
a new type for TendermintState::Propose.
In randomized leader election environment, CodeChain doesn't need
to manage weight values. Previously, it is utilized to guarantee
stake-proportional leader election but now the property comes from
the nature of the probability model.
@HoOngEe HoOngEe merged commit 5b74d99 into CodeChain-io:VRF-election Jan 28, 2020
@HoOngEe HoOngEe deleted the VRF-election branch January 28, 2020 03:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge Do not merge (for mergify.io)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants