narwhal
narwhal copied to clipboard
Make network interface info optional
Fixes Issue#539
- I would expect a change to the .proto, to make it explicit to callers they do not have to provide this information, is that possible?
Proto3 fields are optional by default (i.e. generated code for existing PrimaryAddresses proto message). They recently added back the optional
keyword but its used for field presence for basic types. I think convention is to not put the explicit optional
keyword in the proto, but I don't mind adding it or a comment if thats best for our clients.
Is there a reasonable cutoff point where we can push an
Option
futher into the backend (ideally until theCommittee
) ?
Refactored the backend with optional primary addresses.
So I do think we want to make things optional, but I think we actually want to make the worker addresses optional over the primary's. We should always know concretely who the validators are and at least 1 address for them but we should be completely removing all the worker information out of the committee structure.
Ok I did read the linked issue and I see the intent....maybe we instead want to completely remove all network info from the committee structure?
How do we fix this? The simplest way I can think of is to modify the ReliableNetwork trait (at least), and modify the broadcast function there so that we wait, before the actual sending, until we have at least (2f+1) addresses in the Committee.
@huitseeker this is what I plan on doing. First thing I thought about doing was passing the Committee
into the ReliableNetwork
broadcast method instead of a list of addresses so that we could calculate if the appropriate total stake has been reached but realized that this would have to be special cased for both the WorkerNetwork
and the PrimaryNetwork
. So I think it makes sense to remove the default trait impl for reliable broadcast and then implement the trait in both the worker & primary case separately but with a shared trait interface that looks like this
async fn broadcast(
&mut self,
committee: Committee,
worker_cache: SharedWorkerCache,
message: &Self::Message,
) -> Vec<CancelOnDropHandler<MessageResult>>
The difference would be the worker impl would retrieve the worker addresses and the primary impl would retrieve the primary addresses but they would both retry the same way until we have atleast (2f+1) addresses.
The worker broadcast seems like it will work fine however the issue I am seeing for the primary broadcast is that without a SharedCommittee
I am not sure how we can properly retry until we have (2f+1) addresses as the committee that is passed in will never change. Do we want to switch to using a SharedCommittee or do we have to wait for a NewEpoch or a UpdateCommittee signal?
Let's re-open this in https://github.com/MystenLabs/sui if it's still relevant