Propagate Substrate#4284 to Polkadot #680
Conversation
| inner: Arc<MessageValidator<dyn ChainContext>>, | ||
| // Note: this is always `Some` in real code and `None` in tests. | ||
| service: Option<Arc<PolkadotNetworkService>>, | ||
| // Note: this is always `Some` in real code and `None` in tests. |
There was a problem hiding this comment.
This really isn't great, but I couldn't come up with a better solution.
There was a problem hiding this comment.
Hide it under a #[cfg(not(test))] flag perhaps?
|
I'm having trouble compiling this branch after the last master merge. Would need to wait for Polkadot to be up-to-date with Substrate master to try again. In the meanwhile, I think this branch compiles but I'm not 100% sure. |
network/src/lib.rs
Outdated
| use futures::prelude::*; | ||
| <<<<<<< HEAD | ||
| use futures03::{compat::Stream01CompatExt, FutureExt, StreamExt, TryFutureExt}; | ||
| ======= |
There was a problem hiding this comment.
This should probably not be here..
| service.clone(), | ||
| executor, | ||
| POLKADOT_ENGINE_ID, | ||
| gossip_side |
There was a problem hiding this comment.
| gossip_side | |
| gossip_side, |
| service.clone(), | ||
| executor, | ||
| POLKADOT_ENGINE_ID, | ||
| gossip_side |
There was a problem hiding this comment.
| gossip_side | |
| gossip_side, |
| service.clone(), | ||
| executor, | ||
| POLKADOT_ENGINE_ID, | ||
| gossip_side |
There was a problem hiding this comment.
| gossip_side | |
| gossip_side, |
| service.clone(), | ||
| executor, | ||
| POLKADOT_ENGINE_ID, | ||
| gossip_side |
There was a problem hiding this comment.
| gossip_side | |
| gossip_side, |
| // NOTE: since RegisteredMessageValidator is meant to be a type-safe proof | ||
| // that we've actually done the registration, this should be the only way | ||
| // to construct it outside of tests. | ||
| pub fn register_validator<C: ChainContext + 'static>( |
There was a problem hiding this comment.
This function was always a bit awkward; I'd guess that we'd want to refactor things so that the network crate isn't responsible for instantiating all of these gossip validators. That would fix your "always Some in practice, None in tests" issue from below.
There was a problem hiding this comment.
Not a blocker for this PR, but probably how we want to do things in the future
|
Closed in favor of #695 |
4 participants
paritytech/substrate#4284
Changes:
RegisteredGossipValidatornow implements theNetworkServicetrait, rather thansc_network::NetworkService.AvailabilityNetworkShimandValidationNetworknow use only aRegisteredGossipValidatorrather thansc_network::NetworkService.executorparameter when creating aRegisteredGossipValidator.