fix(driver): decide in a future round if this is possible#1
fix(driver): decide in a future round if this is possible#1
Conversation
* Currently, it fails when the decision is reached with votes * It works when the decision is reached with a commit certificate
* A bit less atrocious solution for the multiplex method
| &mut self, | ||
| new_threshold: VKOutput<ValueId<Ctx>>, | ||
| threshold_round: Round, | ||
| vote_value: Option<ValueId<Ctx>>, |
There was a problem hiding this comment.
I think that is because of Threshold::Value(id), which receives a ValueId
| // Check for a concurrent PrecommitValue(v) output | ||
| if let Some(v) = vote_value { | ||
| if self.vote_keeper.is_threshold_met( | ||
| &r, | ||
| VoteType::Precommit, | ||
| Threshold::Value(v.clone()), | ||
| ) { | ||
| if let Some(proposal) = | ||
| self.valid_proposal_for_round_and_value(threshold_round, v) | ||
| { | ||
| return ( | ||
| threshold_round, | ||
| RoundInput::ProposalAndPrecommitValue(proposal.message.clone()), | ||
| ); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I know the point was to not modify the vote keeper, but I am wondering if it would not be simpler to make this change there instead?
There was a problem hiding this comment.
Not sure if you mean the same thing but I would rather go with the approach where instead of having the mux re-check is_threshold_met after getting a SkipRound, I'd prefer we change apply_vote to return all thresholds crossed by a vote. That way both SkipRound and PrecommitValue would come back from the votekeeper directly, and the driver can just pick the highest-priority one without special-casing it here.
There was a problem hiding this comment.
I am creating another version with this change in the vote keeper. I am just wondering if this wouldn't break other stuff...
There was a problem hiding this comment.
I'd prefer we change apply_vote to return all thresholds crossed by a vote
I like this a idea too.
3 participants
When a process receives a
Precommitfrom roundrand can produce bothSkipRound(r)andPrecommitValue(v), it opts for the first - therefore postponing the decision. This is problematic for two reasons:ProposedValue(v, validity)when starting roundrPrecommit, aCommitcertificate for(r, v)is received, the decision is immediateThis PR is an attempt to solve this undesired behaviour without making changes in the vote keeper logic.