-
Notifications
You must be signed in to change notification settings - Fork 48
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
[DRB] - Verify the DRB result from the computation one epoch in advance #3948
base: main
Are you sure you want to change the base?
Conversation
TYPES: NodeType, | ||
I: NodeImplementation<TYPES>, | ||
V: Versions, | ||
>( | ||
epoch_number: TYPES::Epoch, | ||
task_state: &mut QuorumVoteTaskState<TYPES, I, V>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels a bit over-coupled to make this take the whole task_state when all we need is drb_computation and consensus, but those also don't really exist in any other place that makes sense. Might make it make understanding what the function can mutate more obvious though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like we also need the shared consensus
field of task_state
in this function to get the results
table, so I prefer to keep the argument as is. But let me know if you think there's a better way though!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't think of a better way to pass the arguments, I'd say leave it as is. Otherwise everything looks good to me!
Closes #3858.
Note: This PR is based on the lr/double-quorum branch corresponding to #3922, which if merged first, this PR will be merged directly to
main
.This PR:
DrbComputations
intoDrbSeedsAndResults
andDrbComputation
.Consensus
but keeps the latter at the same place.quorum_vote/drb_computations.rs
totypes/src/drb.rs
, and some to thequorum_vote/handlers.rs
.e - 1
.e
.e - 1
.utils.rs
.This PR does not:
Key places to review:
quorum_proposal/handlers.rs
.quorum_vote/handlers.rs
.types/src/drb.rs
.lr/double-quorum
andmain
.)