Allow Proposal requesting from any DA node #3619
Conversation
| @@ -420,7 +420,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> ConsensusTaskSt | |||
| } | |||
|
|
|||
| if let Err(e) = self.consensus.write().await.update_high_qc(qc.clone()) { | |||
| tracing::error!("{e:?}"); | |||
| tracing::trace!("{e:?}"); | |||
There was a problem hiding this comment.
This was intentional. This is a useless log and noisy as heck.
| ) | ||
| .await; | ||
| } | ||
| { |
There was a problem hiding this comment.
These dangling handles on the consensus type bit me while I was developing this. Multi-line accesses/mutations are in their own scope to ensure that lower calls do not block waiting for a lock that'll never be available. I just wrapped it so that way the consensus handles in this file go out of scope. It's on the roadmap to refactor a lot of this during the dependency refactor rollout.
There was a problem hiding this comment.
I think there is a critical missing piece, the leader is the only one that stores the proposal to disc. This means if all nodes were to restart and the high QC points to a proposal where the leader was not in DA we will be stuck permanently. We need the DA nodes to store (storage.append_proposal) the proposal message as well. Also the request should go to the leader of the view as well as the DA committee.
The restart tests work here because everyone is on the DA committee so the leader can always respond.
Closes #3603
This PR:
Lets the requester of a proposal broadcast to the entire DA committee to receive a proposal on a first-come, first-served basis.
This PR does not:
Key places to review: