Push DRB result to Membership #2625
Conversation
pls148
requested review from
philippecamacho,
ImJeremyHe,
sveitser,
jbearer,
tbro and
imabdulbasit
as code owners
pls148
force-pushed
the
ps/drb-result
branch
2 times, most recently
from
0df1805 to
6ee5cc2
Compare
hotshot-types/src/drb.rs
Outdated
| @@ -81,18 +81,22 @@ pub fn compute_drb_result<TYPES: NodeType>(drb_seed_input: DrbSeedInput) -> DrbR | |||
| /// The DRB result is the output of a spawned `compute_drb_result` call. | |||
| #[must_use] | |||
| pub fn leader<TYPES: NodeType>( | |||
There was a problem hiding this comment.
is this function actually used anywhere?
I feel like this one should be removed; it'll happen in the Membership implementation and I believe the logic should happen via that. I think I also have this function re-implemented in #2638.
There was a problem hiding this comment.
removed for this one, i added it back in the testing PR just so there's something to work with but will rebase once #2638 merges
hotshot-types/src/traits/election.rs
Outdated
|
|
||
| /// Called to notify the Membership when a new DRB result has been calculated. | ||
| /// Observes the same semantics as add_epoch_root | ||
| async fn add_drb_result(&mut self, _epoch: TYPES::Epoch, _drb_result: DrbResult) {} |
There was a problem hiding this comment.
Can we remove the default no-op implementation here (also for add_epoch_root)?
|
|
||
| (sk.clone(), leader) | ||
| } | ||
|
|
There was a problem hiding this comment.
do we need this extra logic? I think previously key_pair_for_id used the fact that all keys were generated from pre-determined seeds (which made sense in the context of a test)
I'm just a bit worried about complicating the logic here any more, since I don't think we need to pass around the keys like this in the integration tests
There was a problem hiding this comment.
This partially gets us to the point where we can have some DRB tests working, as we frequently make guesses about which node hsould be the leader based on the leader being the VIEW-th one of the eligible set. By making it look things up, we can get SOME (but not all) of the tests working with a StaticCommitteeWithDrb Membership implementation.
pls148
force-pushed
the
ps/drb-result
branch
from
6ee5cc2 to
4f86d70
Compare
pls148
force-pushed
the
ps/drb-result
branch
from
4f86d70 to
5515098
Compare
pls148
force-pushed
the
ps/drb-result
branch
from
5515098 to
f7d38ef
Compare
Closes #<ISSUE_NUMBER>
This PR:
This PR does not:
Key places to review: