Conversation
rphmeier
added
A8-looksgood
and removed
A0-please_review
|
Could the 3/4s rule be repurposed as a voting rule as well? I'd also prefer the interface if the header-backend were passed in by reference to the function, rather than requiring implicit shared ownership of the backend. An |
andresilva
added
A3-in_progress
|
Going to rejig the PR to instead do this with |
| N: Network<Block> + 'static, | ||
| N::In: 'static, | ||
| SC: SelectChain<Block> + 'static, | ||
| VR: VotingRule<Block>, |
There was a problem hiding this comment.
Should this be a trait object? I don’t think this will be a bottleneck, so if there will be more than one compiled into a binary, I think this should be a trait object instead.
There was a problem hiding this comment.
We need to guarantee it is Clone somewhere else, with a trait object we can't enforce that bound.
andresilva
added
A0-please_review
|
I think that I've addressed all the previous comments and moved the 3/4 unfinalized chain voting logic to a |
| /// returned value **must** be an ancestor of the given `current_target`, | ||
| /// this also means that a variant must be maintained throughout the | ||
| /// execution of voting rules wherein `current_target <= best_target`. | ||
| fn restrict_vote( |
There was a problem hiding this comment.
the generic parameter could also be on the function here, although it allows for a little less specialization (that we probably would not make use of)
There was a problem hiding this comment.
We use VotingRule as a trait object so we can't have a generic here.
|
|
||
| match self.select_chain.finality_target(block, None) { | ||
| Ok(Some(mut best_hash)) => { | ||
| Ok(Some(best_hash)) => { |
There was a problem hiding this comment.
I guess in this context I don't understand the purpose of finality_target anymore.
There was a problem hiding this comment.
I agree, we already had a discussion about SelectChain on #2869 (comment). Since we now have voting rules to implement the vote target logic what do you think about renaming finality_target in SelectChain to best_containing? In this case SelectChain is only responsible for implementing some custom notion of "best".
|
@rphmeier still ok with this? |
5 participants
This PR adds support for plugging arbitrary voting rules to restrict GRANDPA votes. A
VotingRuletrait is introduced that allows restricting a GRANDPA vote based on what was the initial best finality target and the current finality target.The GRANDPA environment uses
SelectChainto find an initial finality target candidate, additionally the GRANDPA environment restricts the vote to be on 3/4 of the unfinalized chain and furthermore the vote is limited by any pending authority set change. After these mandatory voting rules, the environment uses the givenVotingRuleto further restrict the vote if necessary.The node is updated to use a voting rule to always vote behind the best block. I also added a composite voting rule and builder that allows combining multiple voting rules. This might be useful in polkadot to implement voting rules that take into account data availability.