rewrite network code to use notifications_protocol APIs from Substrate #788
Conversation
rphmeier
added
the
A3-in_progress
gavofyork
added
the
B0-silent
rphmeier
added
A0-please_review
| .filter_map(|msg| match msg.0 { | ||
| crate::legacy::gossip::GossipMessage::Statement(s) => future::ready(Some(s.signed_statement)), | ||
| _ => future::ready(None), |
There was a problem hiding this comment.
Probably making this async and removing future::ready would be a bit more tidier
There was a problem hiding this comment.
The issue I ran into when trying this suggestion is that the async move {} block isn't Unpin, which select requires. So we have to pin_mut!(checked_messages).
|
|
||
| // import all statements pending on this candidate | ||
| let (mut statements, _traces) = if let GenericStatement::Candidate(_) = statement.statement { | ||
| deferred_statements.take_deferred(&c_hash) |
There was a problem hiding this comment.
Maybe .take_deferred(&c_hash).0 and remove the unused _traces variable?
There was a problem hiding this comment.
TBH I find this (pattern matching rather than field access) to be more idiomatic Rust.
| } | ||
|
|
||
| impl ParachainNetwork for Service { | ||
| type Error = future::Either<mpsc::SendError, oneshot::Canceled>; |
There was a problem hiding this comment.
Here and below: probably instead of Either RouterError could be used? It seems to include the variants of this Either.
There was a problem hiding this comment.
I guess so, although the Either formulation is more minimal.
This API is much better for interfacing with the rest of the validation pipeline, which is futures-based.
I skipped implementing
PoVBlockrequests, because the idea is to do that by gossip after #742 is in.Old network code has been placed in the
legacysubmodule. It will be removed in a follow-up that plugs in the new network code in place of the old.