Repository: paritytech/substrate
File: client/network/src/protocol.rs (Rust)

Code before:
			if status.roles.is_light() {
				debug!(target: "sync", "Peer {} is unable to serve light requests", who);
				self.peerset_handle.report_peer(who, rep::BAD_ROLE);
				self.behaviour.disconnect_peer(&who, HARDCODED_PEERSETS_SYNC);
				return Err(())
			}

			// we don't interested in peers that are far behind us
			let self_best_block = self.chain.info().best_number;
			let blocks_difference = self_best_block
				.checked_sub(&status.best_number)
				.unwrap_or_else(Zero::zero)
				.saturated_into::<u64>();
			if blocks_difference > LIGHT_MAXIMAL_BLOCKS_DIFFERENCE {
				debug!(target: "sync", "Peer {} is far behind us and will unable to serve light requests", who);
				self.peerset_handle.report_peer(who, rep::PEER_BEHIND_US_LIGHT);
				self.behaviour.disconnect_peer(&who, HARDCODED_PEERSETS_SYNC);
				return Err(())
			}
		}

		if status.roles.is_full() && self.sync.num_peers() >= self.default_peers_set_num_full {
			debug!(target: "sync", "Too many full nodes, rejecting {}", who);
			self.behaviour.disconnect_peer(&who, HARDCODED_PEERSETS_SYNC);
			return Err(())
		} else if status.roles.is_light() &&
			(self.peers.len() - self.sync.num_peers()) < self.default_peers_set_num_light
		{
			// Make sure that not all slots are occupied by light clients.
			debug!(target: "sync", "Too many light nodes, rejecting {}", who);
			self.behaviour.disconnect_peer(&who, HARDCODED_PEERSETS_SYNC);
			return Err(())
		}

		let peer = Peer {
			info: PeerInfo {
				roles: status.roles,
				best_hash: status.best_hash,
				best_number: status.best_number,
			},
			request: None,
			known_blocks: LruHashSet::new(
				NonZeroUsize::new(MAX_KNOWN_BLOCKS).expect("Constant is nonzero"),
			),
		};

		let req = if peer.info.roles.is_full() {
			match self.sync.new_peer(who, peer.info.best_hash, peer.info.best_number) {
				Ok(req) => req,
				Err(sync::BadPeer(id, repu)) => {
					self.behaviour.disconnect_peer(&id, HARDCODED_PEERSETS_SYNC);

Review comment (bug):
This else is redundant, please get rid of it. I love that you used `roles.is_light()` here, although it is actually always `!roles.is_full()`, but having it made it easier to read the code. Why are we not worried here about overflow?

Code after:
			if status.roles.is_light() {
				debug!(target: "sync", "Peer {} is unable to serve light requests", who);
				self.peerset_handle.report_peer(who, rep::BAD_ROLE);
				self.behaviour.disconnect_peer(&who, HARDCODED_PEERSETS_SYNC);
				return Err(())
			}

			// we don't interested in peers that are far behind us
			let self_best_block = self.chain.info().best_number;
			let blocks_difference = self_best_block
				.checked_sub(&status.best_number)
				.unwrap_or_else(Zero::zero)
				.saturated_into::<u64>();
			if blocks_difference > LIGHT_MAXIMAL_BLOCKS_DIFFERENCE {
				debug!(target: "sync", "Peer {} is far behind us and will unable to serve light requests", who);
				self.peerset_handle.report_peer(who, rep::PEER_BEHIND_US_LIGHT);
				self.behaviour.disconnect_peer(&who, HARDCODED_PEERSETS_SYNC);
				return Err(())
			}
		}

		if status.roles.is_full() && self.sync.num_peers() >= self.default_peers_set_num_full {
			debug!(target: "sync", "Too many full nodes, rejecting {}", who);
			self.behaviour.disconnect_peer(&who, HARDCODED_PEERSETS_SYNC);
			return Err(())
		}

		if status.roles.is_light() &&
			(self.peers.len() - self.sync.num_peers()) < self.default_peers_set_num_light
		{
			// Make sure that not all slots are occupied by light clients.
			debug!(target: "sync", "Too many light nodes, rejecting {}", who);
			self.behaviour.disconnect_peer(&who, HARDCODED_PEERSETS_SYNC);
			return Err(())
		}

		let peer = Peer {
			info: PeerInfo {
				roles: status.roles,
				best_hash: status.best_hash,
				best_number: status.best_number,
			},
			request: None,
			known_blocks: LruHashSet::new(
				NonZeroUsize::new(MAX_KNOWN_BLOCKS).expect("Constant is nonzero"),
			),
		};

		let req = if peer.info.roles.is_full() {
			match self.sync.new_peer(who, peer.info.best_hash, peer.info.best_number) {
				Ok(req) => req,