From b572116aeacf3e7f9964d85d790cd7e28e9ffef1 Mon Sep 17 00:00:00 2001 From: Adrian Catangiu <adrian@parity.io> Date: Tue, 4 Oct 2022 14:34:54 +0300 Subject: [PATCH] client/beefy: small code improvements (#12414) * client/beefy: remove bounds on type definitions * client/beefy: remove gossip protocol legacy name * client/beefy: simplify justification request response engine Signed-off-by: Adrian Catangiu <adrian@parity.io> --- .../client/beefy/src/communication/mod.rs | 5 --- .../outgoing_requests_engine.rs | 34 ++++++++----------- substrate/client/beefy/src/lib.rs | 16 ++------- 3 files changed, 17 insertions(+), 38 deletions(-) diff --git a/substrate/client/beefy/src/communication/mod.rs b/substrate/client/beefy/src/communication/mod.rs index 93646677c0e..91798d4ae0d 100644 --- a/substrate/client/beefy/src/communication/mod.rs +++ b/substrate/client/beefy/src/communication/mod.rs @@ -33,9 +33,6 @@ pub(crate) mod beefy_protocol_name { /// BEEFY justifications protocol name suffix. const JUSTIFICATIONS_NAME: &str = "/beefy/justifications/1"; - /// Old names for the gossip protocol, used for backward compatibility. - pub(super) const LEGACY_NAMES: [&str; 1] = ["/paritytech/beefy/1"]; - /// Name of the votes gossip protocol used by BEEFY. /// /// Must be registered towards the networking in order for BEEFY voter to properly function. @@ -73,9 +70,7 @@ pub fn beefy_peers_set_config( ) -> sc_network_common::config::NonDefaultSetConfig { let mut cfg = sc_network_common::config::NonDefaultSetConfig::new(gossip_protocol_name, 1024 * 1024); - cfg.allow_non_reserved(25, 25); - cfg.add_fallback_names(beefy_protocol_name::LEGACY_NAMES.iter().map(|&n| n.into()).collect()); cfg } diff --git a/substrate/client/beefy/src/communication/request_response/outgoing_requests_engine.rs b/substrate/client/beefy/src/communication/request_response/outgoing_requests_engine.rs index e22958e19cd..c4d3c926190 100644 --- a/substrate/client/beefy/src/communication/request_response/outgoing_requests_engine.rs +++ b/substrate/client/beefy/src/communication/request_response/outgoing_requests_engine.rs @@ -20,10 +20,7 @@ use beefy_primitives::{crypto::AuthorityId, BeefyApi, ValidatorSet}; use codec::Encode; -use futures::{ - channel::{oneshot, oneshot::Canceled}, - stream::{self, StreamExt}, -}; +use futures::channel::{oneshot, oneshot::Canceled}; use log::{debug, error, warn}; use parking_lot::Mutex; use sc_network::{PeerId, ProtocolName}; @@ -50,8 +47,8 @@ type Response = Result<Vec<u8>, RequestFailure>; type ResponseReceiver = oneshot::Receiver<Response>; enum State<B: Block> { - Idle(stream::Pending<Result<Response, Canceled>>), - AwaitingResponse(PeerId, NumberFor<B>, stream::Once<ResponseReceiver>), + Idle, + AwaitingResponse(PeerId, NumberFor<B>, ResponseReceiver), } pub struct OnDemandJustificationsEngine<B: Block, R> { @@ -83,7 +80,7 @@ where protocol_name, live_peers, peers_cache: VecDeque::new(), - state: State::Idle(stream::pending()), + state: State::Idle, } } @@ -118,15 +115,14 @@ where IfDisconnected::ImmediateError, ); - self.state = State::AwaitingResponse(peer, block, stream::once(rx)); + self.state = State::AwaitingResponse(peer, block, rx); } /// If no other request is in progress, start new justification request for `block`. pub fn request(&mut self, block: NumberFor<B>) { // ignore new requests while there's already one pending - match &self.state { - State::AwaitingResponse(_, _, _) => return, - State::Idle(_) => (), + if matches!(self.state, State::AwaitingResponse(_, _, _)) { + return } self.reset_peers_cache_for_block(block); @@ -148,7 +144,7 @@ where "🥩 cancel pending request for justification #{:?}", number ); - self.state = State::Idle(stream::pending()); + self.state = State::Idle; }, _ => (), } @@ -194,19 +190,19 @@ where pub async fn next(&mut self) -> Option<BeefyVersionedFinalityProof<B>> { let (peer, block, resp) = match &mut self.state { - State::Idle(pending) => { - let _ = pending.next().await; - // This never happens since 'stream::pending' never generates any items. + State::Idle => { + futures::pending!(); + // Doesn't happen as 'futures::pending!()' is an 'await' barrier that never passes. return None }, State::AwaitingResponse(peer, block, receiver) => { - let resp = receiver.next().await?; + let resp = receiver.await; (*peer, *block, resp) }, }; - // We received the awaited response. Our 'stream::once()' receiver will never generate any - // other response, meaning we're done with current state. Move the engine to `State::Idle`. - self.state = State::Idle(stream::pending()); + // We received the awaited response. Our 'receiver' will never generate any other response, + // meaning we're done with current state. Move the engine to `State::Idle`. + self.state = State::Idle; let block_id = BlockId::number(block); let validator_set = self diff --git a/substrate/client/beefy/src/lib.rs b/substrate/client/beefy/src/lib.rs index 7407f101e99..760fc753b18 100644 --- a/substrate/client/beefy/src/lib.rs +++ b/substrate/client/beefy/src/lib.rs @@ -153,11 +153,7 @@ where } /// BEEFY gadget network parameters. -pub struct BeefyNetworkParams<B, N> -where - B: Block, - N: GossipNetwork<B> + NetworkRequest + SyncOracle + Send + Sync + 'static, -{ +pub struct BeefyNetworkParams<B: Block, N> { /// Network implementing gossip, requests and sync-oracle. pub network: Arc<N>, /// Chain specific BEEFY gossip protocol name. See @@ -171,15 +167,7 @@ where } /// BEEFY gadget initialization parameters. -pub struct BeefyParams<B, BE, C, N, R> -where - B: Block, - BE: Backend<B>, - C: Client<B, BE>, - R: ProvideRuntimeApi<B>, - R::Api: BeefyApi<B> + MmrApi<B, MmrRootHash>, - N: GossipNetwork<B> + NetworkRequest + SyncOracle + Send + Sync + 'static, -{ +pub struct BeefyParams<B: Block, BE, C, N, R> { /// BEEFY client pub client: Arc<C>, /// Client Backend -- GitLab