Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify SyncingEngine::new() a bit #5396

Merged
13 changes: 13 additions & 0 deletions prdoc/pr_5396.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: Simplify `SyncingEngine::new()`

doc:
- audience: Node Dev
description: |
Tiny changes to simplify the internal implemenation of API `SyncingEngine::new()`.
liuchengxu marked this conversation as resolved.
Show resolved Hide resolved

crates:
- name: sc-network-sync
bump: none
67 changes: 27 additions & 40 deletions substrate/client/network/sync/src/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,8 +326,7 @@ where
if net_config.network_config.max_blocks_per_request > MAX_BLOCKS_IN_RESPONSE as u32 {
log::info!(
target: LOG_TARGET,
"clamping maximum blocks per request to {}",
MAX_BLOCKS_IN_RESPONSE,
"clamping maximum blocks per request to {MAX_BLOCKS_IN_RESPONSE}",
);
MAX_BLOCKS_IN_RESPONSE as u32
} else {
Expand All @@ -348,12 +347,7 @@ where
imp_p.insert(reserved.peer_id);
}
for config in net_config.notification_protocols() {
let peer_ids = config
.set_config()
.reserved_nodes
.iter()
.map(|info| info.peer_id)
.collect::<Vec<PeerId>>();
let peer_ids = config.set_config().reserved_nodes.iter().map(|info| info.peer_id);
imp_p.extend(peer_ids);
}

Expand Down Expand Up @@ -387,18 +381,16 @@ where
total.saturating_sub(net_config.network_config.default_peers_set_num_full) as usize
};

let info = client.info();

let (block_announce_config, notification_service) =
Self::get_block_announce_proto_config::<N>(
protocol_id,
fork_id,
roles,
client.info().best_number,
client.info().best_hash,
client
.block_hash(Zero::zero())
.ok()
.flatten()
.expect("Genesis block exists; qed"),
info.best_number,
info.best_hash,
info.genesis_hash,
&net_config.network_config.default_peers_set,
network_metrics,
Arc::clone(&peer_store_handle),
Expand All @@ -424,11 +416,6 @@ where
let (tx, service_rx) = tracing_unbounded("mpsc_chain_sync", 100_000);
let num_connected = Arc::new(AtomicUsize::new(0));
let is_major_syncing = Arc::new(AtomicBool::new(false));
let genesis_hash = client
.block_hash(0u32.into())
.ok()
.flatten()
.expect("Genesis block exists; qed");

// `default_peers_set.in_peers` contains an unspecified amount of light peers so the number
// of full inbound peers must be calculated from the total full peer count
Expand Down Expand Up @@ -457,7 +444,7 @@ where
num_connected: num_connected.clone(),
is_major_syncing: is_major_syncing.clone(),
service_rx,
genesis_hash,
genesis_hash: info.genesis_hash,
important_peers,
default_peers_set_no_slot_connected_peers: HashSet::new(),
warp_sync_target_block_header_rx_fused,
Expand Down Expand Up @@ -565,7 +552,7 @@ where
"Received block announce from disconnected peer {peer_id}",
);
debug_assert!(false);
return
return;
},
};
peer.known_blocks.insert(hash);
Expand All @@ -590,17 +577,17 @@ where
Ok(Some(header)) => header,
Ok(None) => {
log::warn!(target: LOG_TARGET, "Trying to announce unknown block: {hash}");
return
return;
},
Err(e) => {
log::warn!(target: LOG_TARGET, "Error reading block header {hash}: {e}");
return
return;
},
};

// don't announce genesis block since it will be ignored
if header.number().is_zero() {
return
return;
}

let is_best = self.client.info().best_hash == hash;
Expand Down Expand Up @@ -662,7 +649,7 @@ where
target: LOG_TARGET,
"Terminating `SyncingEngine` due to fatal error: {e:?}.",
);
return
return;
}
}
}
Expand Down Expand Up @@ -885,12 +872,12 @@ where
target: LOG_TARGET,
"received notification from {peer} who had been earlier refused by `SyncingEngine`",
);
return
return;
}

let Ok(announce) = BlockAnnounce::decode(&mut notification.as_ref()) else {
log::warn!(target: LOG_TARGET, "failed to decode block announce");
return
return;
};

self.push_block_announce_validation(peer, announce);
Expand Down Expand Up @@ -920,7 +907,7 @@ where
fn on_sync_peer_disconnected(&mut self, peer_id: PeerId) {
let Some(info) = self.peers.remove(&peer_id) else {
log::debug!(target: LOG_TARGET, "{peer_id} does not exist in `SyncingEngine`");
return
return;
};

if self.important_peers.contains(&peer_id) {
Expand Down Expand Up @@ -990,7 +977,7 @@ where
);
}

return Err(true)
return Err(true);
}

Ok(handshake)
Expand Down Expand Up @@ -1025,7 +1012,7 @@ where
"Called `validate_connection()` with already connected peer {peer_id}",
);
debug_assert!(false);
return Err(false)
return Err(false);
}

let no_slot_peer = self.default_peers_set_no_slot_peers.contains(&peer_id);
Expand All @@ -1038,7 +1025,7 @@ where
this_peer_reserved_slot
{
log::debug!(target: LOG_TARGET, "Too many full nodes, rejecting {peer_id}");
return Err(false)
return Err(false);
}

// make sure to accept no more than `--in-peers` many full nodes
Expand All @@ -1048,7 +1035,7 @@ where
self.num_in_peers == self.max_in_peers
{
log::debug!(target: LOG_TARGET, "All inbound slots have been consumed, rejecting {peer_id}");
return Err(false)
return Err(false);
}

// make sure that all slots are not occupied by light peers
Expand All @@ -1059,7 +1046,7 @@ where
(self.peers.len() - self.strategy.num_peers()) >= self.default_peers_set_num_light
{
log::debug!(target: LOG_TARGET, "Too many light nodes, rejecting {peer_id}");
return Err(false)
return Err(false);
}

Ok(handshake)
Expand Down Expand Up @@ -1116,7 +1103,7 @@ where
if !self.peers.contains_key(&peer_id) {
trace!(target: LOG_TARGET, "Cannot send block request to unknown peer {peer_id}");
debug_assert!(false);
return
return;
}

let downloader = self.block_downloader.clone();
Expand All @@ -1138,7 +1125,7 @@ where
if !self.peers.contains_key(&peer_id) {
trace!(target: LOG_TARGET, "Cannot send state request to unknown peer {peer_id}");
debug_assert!(false);
return
return;
}

let (tx, rx) = oneshot::channel();
Expand Down Expand Up @@ -1173,7 +1160,7 @@ where
if !self.peers.contains_key(&peer_id) {
trace!(target: LOG_TARGET, "Cannot send warp proof request to unknown peer {peer_id}");
debug_assert!(false);
return
return;
}

let (tx, rx) = oneshot::channel();
Expand Down Expand Up @@ -1236,7 +1223,7 @@ where
peer_id,
self.block_announce_protocol_name.clone(),
);
return
return;
},
Err(BlockResponseError::ExtractionFailed(e)) => {
debug!(
Expand All @@ -1246,7 +1233,7 @@ where
e
);
self.network_service.report_peer(peer_id, rep::BAD_MESSAGE);
return
return;
},
}
},
Expand All @@ -1263,7 +1250,7 @@ where
peer_id,
self.block_announce_protocol_name.clone(),
);
return
return;
},
};

Expand Down
Loading