Do some cleanups (#14608)

* Do some cleanups

Found them while looking over the code.

* More

* Fix
This commit is contained in:
Bastian Köcher
2023-07-21 23:54:01 +02:00
committed by GitHub
parent 98304ee957
commit b16976053e
9 changed files with 39 additions and 45 deletions
@@ -87,7 +87,7 @@
use ahash::{AHashMap, AHashSet};
use log::{debug, trace};
use parity_scale_codec::{Decode, Encode};
use parity_scale_codec::{Decode, DecodeAll, Encode};
use prometheus_endpoint::{register, CounterVec, Opts, PrometheusError, Registry, U64};
use rand::seq::SliceRandom;
use sc_network::{PeerId, ReputationChange};
@@ -546,9 +546,8 @@ impl<N: Ord> Peers<N> {
who: &PeerId,
update: NeighborPacket<N>,
) -> Result<Option<&View<N>>, Misbehavior> {
let peer = match self.inner.get_mut(who) {
None => return Ok(None),
Some(p) => p,
let Some(peer) = self.inner.get_mut(who) else {
return Ok(None)
};
let invalid_change = peer.view.set_id > update.set_id ||
@@ -1040,9 +1039,8 @@ impl<Block: BlockT> Inner<Block> {
request: CatchUpRequestMessage,
set_state: &environment::SharedVoterSetState<Block>,
) -> (Option<GossipMessage<Block>>, Action<Block::Hash>) {
let local_view = match self.local_view {
None => return (None, Action::Discard(Misbehavior::OutOfScopeMessage.cost())),
Some(ref view) => view,
let Some(local_view) = &self.local_view else {
return (None, Action::Discard(Misbehavior::OutOfScopeMessage.cost()))
};
if request.set_id != local_view.set_id {
@@ -1175,10 +1173,8 @@ impl<Block: BlockT> Inner<Block> {
Err(misbehavior) => (misbehavior.cost(), None),
};
let (catch_up, report) = match update_res {
Ok(_) => self.try_catch_up(who),
_ => (None, None),
};
let (catch_up, report) =
if update_res.is_ok() { self.try_catch_up(who) } else { (None, None) };
let neighbor_topics = topics.unwrap_or_default();
@@ -1435,7 +1431,7 @@ impl<Block: BlockT> GossipValidator<Block> {
let message_name;
let action = {
match GossipMessage::<Block>::decode(&mut data) {
match GossipMessage::<Block>::decode_all(&mut data) {
Ok(GossipMessage::Vote(ref message)) => {
message_name = Some("vote");
self.inner.write().validate_round_message(who, message)
@@ -1599,9 +1595,8 @@ impl<Block: BlockT> sc_network_gossip::Validator<Block> for GossipValidator<Bloc
// if the topic is not something we're keeping at the moment,
// do not send.
let (maybe_round, set_id) = match inner.live_topics.topic_info(topic) {
None => return false,
Some(x) => x,
let Some((maybe_round, set_id)) = inner.live_topics.topic_info(topic) else {
return false
};
if let MessageIntent::Broadcast = intent {
@@ -1622,12 +1617,11 @@ impl<Block: BlockT> sc_network_gossip::Validator<Block> for GossipValidator<Bloc
}
// global message.
let local_view = match inner.local_view {
Some(ref v) => v,
None => return false, // cannot evaluate until we have a local view.
let Some(local_view) = &inner.local_view else {
return false // cannot evaluate until we have a local view.
};
match GossipMessage::<Block>::decode(&mut data) {
match GossipMessage::<Block>::decode_all(&mut data) {
Err(_) => false,
Ok(GossipMessage::Commit(full)) => {
// we only broadcast commit messages if they're for the same
@@ -1658,13 +1652,12 @@ impl<Block: BlockT> sc_network_gossip::Validator<Block> for GossipValidator<Bloc
Some((None, _)) => {},
};
let local_view = match inner.local_view {
Some(ref v) => v,
None => return true, // no local view means we can't evaluate or hold any topic.
let Some(local_view) = &inner.local_view else {
return true // no local view means we can't evaluate or hold any topic.
};
// global messages -- only keep the best commit.
match GossipMessage::<Block>::decode(&mut data) {
match GossipMessage::<Block>::decode_all(&mut data) {
Err(_) => true,
Ok(GossipMessage::Commit(full)) => match local_view.last_commit {
Some((number, round, set_id)) =>
@@ -45,7 +45,7 @@ use finality_grandpa::{
voter_set::VoterSet,
Message::{Precommit, Prevote, PrimaryPropose},
};
use parity_scale_codec::{Decode, Encode};
use parity_scale_codec::{Decode, DecodeAll, Encode};
use sc_network::{NetworkBlock, NetworkSyncForkRequest, ReputationChange};
use sc_network_gossip::{GossipEngine, Network as GossipNetwork};
use sc_telemetry::{telemetry, TelemetryHandle, CONSENSUS_DEBUG, CONSENSUS_INFO};
@@ -352,7 +352,7 @@ impl<B: BlockT, N: Network<B>, S: Syncing<B>> NetworkBridge<B, N, S> {
let telemetry = self.telemetry.clone();
let incoming =
self.gossip_engine.lock().messages_for(topic).filter_map(move |notification| {
let decoded = GossipMessage::<B>::decode(&mut &notification.message[..]);
let decoded = GossipMessage::<B>::decode_all(&mut &notification.message[..]);
match decoded {
Err(ref e) => {
@@ -651,7 +651,7 @@ fn incoming_global<B: BlockT>(
.messages_for(topic)
.filter_map(|notification| {
// this could be optimized by decoding piecewise.
let decoded = GossipMessage::<B>::decode(&mut &notification.message[..]);
let decoded = GossipMessage::<B>::decode_all(&mut &notification.message[..]);
if let Err(ref e) = decoded {
trace!(
target: LOG_TARGET,
@@ -108,7 +108,7 @@ impl<B: BlockT> Stream for NeighborPacketWorker<B> {
//
// Note: In case poll_unpin is called after the resetted delay fires again, this
// will drop one tick. Deemed as very unlikely and also not critical.
while let Poll::Ready(()) = this.delay.poll_unpin(cx) {}
while this.delay.poll_unpin(cx).is_ready() {}
if let Some((ref to, ref packet)) = this.last {
return Poll::Ready(Some((to.clone(), GossipMessage::<B>::from(packet.clone()))))
@@ -23,7 +23,7 @@ use std::{
};
use finality_grandpa::{voter_set::VoterSet, Error as GrandpaError};
use parity_scale_codec::{Decode, Encode};
use parity_scale_codec::{Decode, DecodeAll, Encode};
use sp_blockchain::{Error as ClientError, HeaderBackend};
use sp_consensus_grandpa::AuthorityId;
use sp_runtime::traits::{Block as BlockT, Header as HeaderT, NumberFor};
@@ -136,7 +136,7 @@ impl<Block: BlockT> GrandpaJustification<Block> {
where
NumberFor<Block>: finality_grandpa::BlockNumberOps,
{
let justification = GrandpaJustification::<Block>::decode(&mut &*encoded)
let justification = GrandpaJustification::<Block>::decode_all(&mut &*encoded)
.map_err(|_| ClientError::JustificationDecode)?;
if (
@@ -16,7 +16,7 @@
//! Utilities for generating and verifying GRANDPA warp sync proofs.
use sp_runtime::codec::{self, Decode, Encode};
use parity_scale_codec::{Decode, DecodeAll, Encode};
use crate::{
best_justification, find_scheduled_change, AuthoritySetChanges, AuthoritySetHardFork,
@@ -38,7 +38,7 @@ use std::{collections::HashMap, sync::Arc};
pub enum Error {
/// Decoding error.
#[error("Failed to decode block hash: {0}.")]
DecodeScale(#[from] codec::Error),
DecodeScale(#[from] parity_scale_codec::Error),
/// Client backend error.
#[error("{0}")]
Client(#[from] sp_blockchain::Error),
@@ -137,7 +137,7 @@ impl<Block: BlockT> WarpSyncProof<Block> {
.and_then(|just| just.into_justification(GRANDPA_ENGINE_ID))
.ok_or_else(|| Error::MissingData)?;
let justification = GrandpaJustification::<Block>::decode(&mut &justification[..])?;
let justification = GrandpaJustification::<Block>::decode_all(&mut &justification[..])?;
let proof = WarpSyncFragment { header: header.clone(), justification };
let proof_size = proof.encoded_size();
@@ -291,7 +291,7 @@ where
authorities: AuthorityList,
) -> Result<VerificationResult<Block>, Box<dyn std::error::Error + Send + Sync>> {
let EncodedProof(proof) = proof;
let proof = WarpSyncProof::<Block>::decode(&mut proof.as_slice())
let proof = WarpSyncProof::<Block>::decode_all(&mut proof.as_slice())
.map_err(|e| format!("Proof decoding error: {:?}", e))?;
let last_header = proof
.proofs
@@ -318,8 +318,9 @@ where
#[cfg(test)]
mod tests {
use super::{codec::Encode, WarpSyncProof};
use super::WarpSyncProof;
use crate::{AuthoritySetChanges, GrandpaJustification};
use parity_scale_codec::Encode;
use rand::prelude::*;
use sc_block_builder::BlockBuilderProvider;
use sp_blockchain::HeaderBackend;