refactor/reputation: unify the values used (#2462)

* refactor/reputation: unify the values used

* chore/rep: rename Annoy* to Cost*, make duplicate message Cost*Repeated

* fix/reputation: lost and found, convert at the boundary to substrate

* refactor/rep: move conversion to base reputation one level down, left conversions

* fix/rep: order of magnitude adjustments

Thanks pierre!

* remove spaces

* chore/rep: give rationale for order of magnitude

* refactor/rep: move UnifiedReputationChange to separate file

* fix/rep: order of magnitudes correction
This commit is contained in:
Bernhard Schuster
2021-02-17 17:18:13 +01:00
committed by GitHub
parent 62c5896592
commit 1e2161258b
16 changed files with 132 additions and 73 deletions
@@ -41,19 +41,19 @@ use polkadot_node_subsystem::{
};
use polkadot_node_subsystem_util::metrics::{self, prometheus};
use polkadot_node_network_protocol::{
PeerId, View, v1 as protocol_v1, ReputationChange as Rep,
PeerId, View, v1 as protocol_v1, UnifiedReputationChange as Rep,
};
const LOG_TARGET: &str = "approval_distribution";
const COST_UNEXPECTED_MESSAGE: Rep = Rep::new(-100, "Peer sent an out-of-view assignment or approval");
const COST_DUPLICATE_MESSAGE: Rep = Rep::new(-100, "Peer sent identical messages");
const COST_ASSIGNMENT_TOO_FAR_IN_THE_FUTURE: Rep = Rep::new(-30, "The vote was valid but too far in the future");
const COST_INVALID_MESSAGE: Rep = Rep::new(-1000, "The vote was bad");
const BENEFIT_VALID_MESSAGE: Rep = Rep::new(10, "Peer sent a valid message");
const BENEFIT_VALID_MESSAGE_FIRST: Rep = Rep::new(15, "Valid message with new information");
const COST_UNEXPECTED_MESSAGE: Rep = Rep::CostMinor("Peer sent an out-of-view assignment or approval");
const COST_DUPLICATE_MESSAGE: Rep = Rep::CostMinorRepeated("Peer sent identical messages");
const COST_ASSIGNMENT_TOO_FAR_IN_THE_FUTURE: Rep = Rep::CostMinor("The vote was valid but too far in the future");
const COST_INVALID_MESSAGE: Rep = Rep::CostMajor("The vote was bad");
const BENEFIT_VALID_MESSAGE: Rep = Rep::BenefitMinor("Peer sent a valid message");
const BENEFIT_VALID_MESSAGE_FIRST: Rep = Rep::BenefitMinorFirst("Valid message with new information");
/// The Approval Distribution subsystem.
pub struct ApprovalDistribution {
@@ -32,7 +32,7 @@ use sp_keystore::{CryptoStore, SyncCryptoStorePtr};
use polkadot_erasure_coding::branch_hash;
use polkadot_node_network_protocol::{
v1 as protocol_v1, PeerId, ReputationChange as Rep, View, OurView,
v1 as protocol_v1, PeerId, View, OurView, UnifiedReputationChange as Rep,
};
use polkadot_node_subsystem_util::metrics::{self, prometheus};
use polkadot_primitives::v1::{
@@ -95,11 +95,11 @@ enum Error {
type Result<T> = std::result::Result<T, Error>;
const COST_MERKLE_PROOF_INVALID: Rep = Rep::new(-100, "Merkle proof was invalid");
const COST_NOT_A_LIVE_CANDIDATE: Rep = Rep::new(-51, "Candidate is not live");
const COST_PEER_DUPLICATE_MESSAGE: Rep = Rep::new(-500, "Peer sent identical messages");
const BENEFIT_VALID_MESSAGE_FIRST: Rep = Rep::new(15, "Valid message with new information");
const BENEFIT_VALID_MESSAGE: Rep = Rep::new(10, "Valid message");
const COST_MERKLE_PROOF_INVALID: Rep = Rep::CostMinor("Merkle proof was invalid");
const COST_NOT_A_LIVE_CANDIDATE: Rep = Rep::CostMinor("Candidate is not live");
const COST_PEER_DUPLICATE_MESSAGE: Rep = Rep::CostMajorRepeated("Peer sent identical messages");
const BENEFIT_VALID_MESSAGE_FIRST: Rep = Rep::BenefitMinorFirst("Valid message with new information");
const BENEFIT_VALID_MESSAGE: Rep = Rep::BenefitMinor("Valid message");
/// Checked signed availability bitfield that is distributed
/// to other peers.
@@ -43,7 +43,7 @@ use polkadot_subsystem::{
},
};
use polkadot_node_network_protocol::{
peer_set::PeerSet, v1 as protocol_v1, PeerId, ReputationChange as Rep, RequestId,
peer_set::PeerSet, v1 as protocol_v1, PeerId, RequestId, UnifiedReputationChange as Rep,
};
use polkadot_node_subsystem_util::{
Timeout, TimeoutExt,
@@ -57,8 +57,8 @@ mod tests;
const LOG_TARGET: &str = "availability_recovery";
const COST_MERKLE_PROOF_INVALID: Rep = Rep::new(-100, "Merkle proof was invalid");
const COST_UNEXPECTED_CHUNK: Rep = Rep::new(-100, "Peer has sent an unexpected chunk");
const COST_MERKLE_PROOF_INVALID: Rep = Rep::CostMinor("Merkle proof was invalid");
const COST_UNEXPECTED_CHUNK: Rep = Rep::CostMinor("Peer has sent an unexpected chunk");
// How many parallel requests interaction should have going at once.
const N_PARALLEL: usize = 50;
@@ -32,23 +32,16 @@ use polkadot_subsystem::{
};
use polkadot_node_subsystem_util::metrics::{self, prometheus};
use polkadot_primitives::v1::{Hash, SignedAvailabilityBitfield, SigningContext, ValidatorId};
use polkadot_node_network_protocol::{v1 as protocol_v1, PeerId, View, ReputationChange, OurView};
use polkadot_node_network_protocol::{v1 as protocol_v1, PeerId, View, UnifiedReputationChange as Rep, OurView};
use std::collections::{HashMap, HashSet};
const COST_SIGNATURE_INVALID: ReputationChange =
ReputationChange::new(-100, "Bitfield signature invalid");
const COST_VALIDATOR_INDEX_INVALID: ReputationChange =
ReputationChange::new(-100, "Bitfield validator index invalid");
const COST_MISSING_PEER_SESSION_KEY: ReputationChange =
ReputationChange::new(-133, "Missing peer session key");
const COST_NOT_IN_VIEW: ReputationChange =
ReputationChange::new(-51, "Not interested in that parent hash");
const COST_PEER_DUPLICATE_MESSAGE: ReputationChange =
ReputationChange::new(-500, "Peer sent the same message multiple times");
const BENEFIT_VALID_MESSAGE_FIRST: ReputationChange =
ReputationChange::new(15, "Valid message with new information");
const BENEFIT_VALID_MESSAGE: ReputationChange =
ReputationChange::new(10, "Valid message");
const COST_SIGNATURE_INVALID: Rep = Rep::CostMajor("Bitfield signature invalid");
const COST_VALIDATOR_INDEX_INVALID: Rep = Rep::CostMajor("Bitfield validator index invalid");
const COST_MISSING_PEER_SESSION_KEY: Rep = Rep::CostMinor("Missing peer session key");
const COST_NOT_IN_VIEW: Rep = Rep::CostMinor("Not interested in that parent hash");
const COST_PEER_DUPLICATE_MESSAGE: Rep = Rep::CostMinorRepeated("Peer sent the same message multiple times");
const BENEFIT_VALID_MESSAGE_FIRST: Rep = Rep::BenefitMinorFirst("Valid message with new information");
const BENEFIT_VALID_MESSAGE: Rep = Rep::BenefitMinor("Valid message");
/// Checked signed availability bitfield that is distributed
/// to other peers.
@@ -236,7 +229,7 @@ impl BitfieldDistribution {
async fn modify_reputation<Context>(
ctx: &mut Context,
peer: PeerId,
rep: ReputationChange,
rep: Rep,
)
where
Context: SubsystemContext<Message = BitfieldDistributionMessage>,
+2 -2
View File
@@ -19,7 +19,7 @@ use futures::channel::mpsc;
use parity_scale_codec::Decode;
use polkadot_node_network_protocol::{
peer_set::PeerSet, v1 as protocol_v1, PeerId, ReputationChange,
peer_set::PeerSet, v1 as protocol_v1, PeerId, UnifiedReputationChange,
};
use polkadot_primitives::v1::{AuthorityDiscoveryId, BlockNumber};
use polkadot_subsystem::messages::{AllMessages, NetworkBridgeMessage};
@@ -55,7 +55,7 @@ pub(crate) enum Action {
},
/// Report a peer to the network implementation (decreasing/increasing its reputation).
ReportPeer(PeerId, ReputationChange),
ReportPeer(PeerId, UnifiedReputationChange),
/// A subsystem updates us on the relay chain leaves we consider active.
///
+5 -5
View File
@@ -34,7 +34,7 @@ use polkadot_subsystem::messages::{
};
use polkadot_primitives::v1::{Hash, BlockNumber};
use polkadot_node_network_protocol::{
ReputationChange, PeerId, peer_set::PeerSet, View, v1 as protocol_v1, OurView,
PeerId, peer_set::PeerSet, View, v1 as protocol_v1, OurView, UnifiedReputationChange as Rep,
};
/// Peer set infos for network initialization.
@@ -72,10 +72,10 @@ pub use multiplexer::RequestMultiplexer;
const MAX_VIEW_HEADS: usize = 5;
const MALFORMED_MESSAGE_COST: ReputationChange = ReputationChange::new(-500, "Malformed Network-bridge message");
const UNCONNECTED_PEERSET_COST: ReputationChange = ReputationChange::new(-50, "Message sent to un-connected peer-set");
const MALFORMED_VIEW_COST: ReputationChange = ReputationChange::new(-500, "Malformed view");
const EMPTY_VIEW_COST: ReputationChange = ReputationChange::new(-500, "Peer sent us an empty view");
const MALFORMED_MESSAGE_COST: Rep = Rep::CostMajor("Malformed Network-bridge message");
const UNCONNECTED_PEERSET_COST: Rep = Rep::CostMinor("Message sent to un-connected peer-set");
const MALFORMED_VIEW_COST: Rep = Rep::CostMajor("Malformed view");
const EMPTY_VIEW_COST: Rep = Rep::CostMajor("Peer sent us an empty view");
// network bridge log target
const LOG_TARGET: &'static str = "network_bridge";
+4 -4
View File
@@ -29,7 +29,7 @@ use sc_network::{NetworkService, IfDisconnected};
use polkadot_node_network_protocol::{
peer_set::PeerSet,
request_response::{OutgoingRequest, Requests},
PeerId, ReputationChange,
PeerId, UnifiedReputationChange as Rep,
};
use polkadot_primitives::v1::{Block, Hash};
use polkadot_subsystem::{SubsystemError, SubsystemResult};
@@ -86,7 +86,7 @@ where
#[derive(Debug, PartialEq)]
pub enum NetworkAction {
/// Note a change in reputation for a peer.
ReputationChange(PeerId, ReputationChange),
ReputationChange(PeerId, Rep),
/// Write a notification to a given peer on the given peer-set.
WriteNotification(PeerId, PeerSet, Vec<u8>),
}
@@ -111,7 +111,7 @@ pub trait Network: Send + 'static {
fn report_peer(
&mut self,
who: PeerId,
cost_benefit: ReputationChange,
cost_benefit: Rep,
) -> BoxFuture<SubsystemResult<()>> {
async move {
self.action_sink()
@@ -167,7 +167,7 @@ impl Network for Arc<NetworkService<Block, Hash>> {
cost_benefit,
peer
);
self.0.report_peer(peer, cost_benefit)
self.0.report_peer(peer, cost_benefit.into_base_rep())
}
NetworkAction::WriteNotification(peer, peer_set, message) => self
.0
@@ -32,7 +32,7 @@ use polkadot_subsystem::{
},
};
use polkadot_node_network_protocol::{
PeerId, ReputationChange as Rep,
PeerId, UnifiedReputationChange as Rep,
};
use polkadot_primitives::v1::CollatorId;
use polkadot_node_subsystem_util::{
@@ -36,17 +36,17 @@ use polkadot_subsystem::{
},
};
use polkadot_node_network_protocol::{
v1 as protocol_v1, View, OurView, PeerId, ReputationChange as Rep, RequestId,
v1 as protocol_v1, View, OurView, PeerId, RequestId, UnifiedReputationChange as Rep,
};
use polkadot_node_subsystem_util::{TimeoutExt as _, metrics::{self, prometheus}};
use polkadot_node_primitives::{Statement, SignedFullStatement};
use super::{modify_reputation, LOG_TARGET, Result};
const COST_UNEXPECTED_MESSAGE: Rep = Rep::new(-10, "An unexpected message");
const COST_REQUEST_TIMED_OUT: Rep = Rep::new(-20, "A collation request has timed out");
const COST_REPORT_BAD: Rep = Rep::new(-50, "A collator was reported by another subsystem");
const BENEFIT_NOTIFY_GOOD: Rep = Rep::new(50, "A collator was noted good by another subsystem");
const COST_UNEXPECTED_MESSAGE: Rep = Rep::CostMinor("An unexpected message");
const COST_REQUEST_TIMED_OUT: Rep = Rep::CostMinor("A collation request has timed out");
const COST_REPORT_BAD: Rep = Rep::CostMajor("A collator was reported by another subsystem");
const BENEFIT_NOTIFY_GOOD: Rep = Rep::BenefitMinor("A collator was noted good by another subsystem");
#[derive(Clone, Default)]
pub struct Metrics(Option<MetricsInner>);
@@ -40,7 +40,7 @@ use polkadot_node_subsystem_util::{
metrics::{self, prometheus},
};
use polkadot_node_network_protocol::{
peer_set::PeerSet, v1 as protocol_v1, ReputationChange as Rep, PeerId, OurView,
peer_set::PeerSet, v1 as protocol_v1, PeerId, OurView, UnifiedReputationChange as Rep,
};
use futures::prelude::*;
@@ -54,13 +54,13 @@ mod error;
#[cfg(test)]
mod tests;
const COST_APPARENT_FLOOD: Rep = Rep::new(-500, "Peer appears to be flooding us with PoV requests");
const COST_UNEXPECTED_POV: Rep = Rep::new(-500, "Peer sent us an unexpected PoV");
const COST_APPARENT_FLOOD: Rep = Rep::CostMajor("Peer appears to be flooding us with PoV requests");
const COST_UNEXPECTED_POV: Rep = Rep::CostMajor("Peer sent us an unexpected PoV");
const COST_AWAITED_NOT_IN_VIEW: Rep
= Rep::new(-100, "Peer claims to be awaiting something outside of its view");
= Rep::CostMinor("Peer claims to be awaiting something outside of its view");
const BENEFIT_FRESH_POV: Rep = Rep::new(25, "Peer supplied us with an awaited PoV");
const BENEFIT_LATE_POV: Rep = Rep::new(10, "Peer supplied us with an awaited PoV, \
const BENEFIT_FRESH_POV: Rep = Rep::BenefitMinorFirst("Peer supplied us with an awaited PoV");
const BENEFIT_LATE_POV: Rep = Rep::BenefitMinor("Peer supplied us with an awaited PoV, \
but was not the first to do so");
const LOG_TARGET: &str = "pov_distribution";
+4 -2
View File
@@ -23,12 +23,14 @@ use polkadot_primitives::v1::{Hash, BlockNumber};
use parity_scale_codec::{Encode, Decode};
use std::{fmt, collections::HashMap};
pub use sc_network::{ReputationChange, PeerId};
pub use sc_network::PeerId;
#[doc(hidden)]
pub use polkadot_node_jaeger::JaegerSpan;
#[doc(hidden)]
pub use std::sync::Arc;
mod reputation;
pub use self::reputation::{ReputationChange, UnifiedReputationChange};
/// Peer-sets and protocols used for parachains.
pub mod peer_set;
@@ -372,7 +374,7 @@ pub mod v1 {
}
impl std::fmt::Debug for CompressedPoV {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "CompressedPoV({} bytes)", self.0.len())
}
}
@@ -0,0 +1,65 @@
pub use sc_network::ReputationChange;
/// Unified annoyance cost and good behavior benefits.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
#[allow(missing_docs)]
pub enum UnifiedReputationChange {
CostMajor(&'static str),
CostMinor(&'static str),
CostMajorRepeated(&'static str),
CostMinorRepeated(&'static str),
Malicious(&'static str),
BenefitMinorFirst(&'static str),
BenefitMinor(&'static str),
BenefitMajorFirst(&'static str),
BenefitMajor(&'static str),
}
impl UnifiedReputationChange {
/// Obtain the cost or benefit associated with
/// the enum variant.
///
/// Order of magnitude rationale:
///
/// * the peerset will not connect to a peer whose reputation is below a fixed value
/// * `max(2% *$rep, 1)` is the delta of convergence towards a reputation of 0
///
/// The whole range of an `i32` should be used, so order of magnitude of
/// something malicious should be `1<<20` (give or take).
const fn cost_or_benefit(&self) -> i32 {
match self {
Self::CostMinor(_) => -100_000,
Self::CostMajor(_) => -300_000,
Self::CostMinorRepeated(_) => -200_000,
Self::CostMajorRepeated(_) => -600_000,
Self::Malicious(_) => -1_000_000,
Self::BenefitMajorFirst(_) => 300_000,
Self::BenefitMajor(_) => 200_000,
Self::BenefitMinorFirst(_) => 15_000,
Self::BenefitMinor(_) => 10_000,
}
}
/// Extract the static description.
pub const fn description(&self) -> &'static str {
match self {
Self::CostMinor(description) => description,
Self::CostMajor(description) => description,
Self::CostMinorRepeated(description) => description,
Self::CostMajorRepeated(description) => description,
Self::Malicious(description) => description,
Self::BenefitMajorFirst(description) => description,
Self::BenefitMajor(description) => description,
Self::BenefitMinorFirst(description) => description,
Self::BenefitMinor(description) => description,
}
}
/// Convert into a base reputation as used with substrate.
pub const fn into_base_rep(self) -> ReputationChange {
ReputationChange::new(
self.cost_or_benefit(),
self.description()
)
}
}
@@ -10,9 +10,9 @@ futures = "0.3.12"
tracing = "0.1.22"
tracing-futures = "0.2.4"
polkadot-primitives = { path = "../../../primitives" }
node-primitives = { package = "polkadot-node-primitives", path = "../../primitives" }
sp-staking = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
polkadot-subsystem = { package = "polkadot-node-subsystem", path = "../../subsystem" }
polkadot-node-primitives = { path = "../../primitives" }
polkadot-node-subsystem-util = { path = "../../subsystem-util" }
polkadot-node-network-protocol = { path = "../../network/protocol" }
arrayvec = "0.5.2"
@@ -31,12 +31,12 @@ use polkadot_subsystem::{
},
};
use polkadot_node_subsystem_util::metrics::{self, prometheus};
use node_primitives::SignedFullStatement;
use polkadot_node_primitives::{SignedFullStatement};
use polkadot_primitives::v1::{
Hash, CompactStatement, ValidatorIndex, ValidatorId, SigningContext, ValidatorSignature, CandidateHash,
};
use polkadot_node_network_protocol::{
v1 as protocol_v1, View, PeerId, ReputationChange as Rep, OurView,
v1 as protocol_v1, View, PeerId, OurView, UnifiedReputationChange as Rep,
};
use futures::prelude::*;
@@ -45,14 +45,13 @@ use indexmap::IndexSet;
use std::collections::{HashMap, HashSet};
const COST_UNEXPECTED_STATEMENT: Rep = Rep::new(-100, "Unexpected Statement");
const COST_INVALID_SIGNATURE: Rep = Rep::new(-500, "Invalid Statement Signature");
const COST_DUPLICATE_STATEMENT: Rep = Rep::new(-250, "Statement sent more than once by peer");
const COST_APPARENT_FLOOD: Rep = Rep::new(-1000, "Peer appears to be flooding us with statements");
const COST_UNEXPECTED_STATEMENT: Rep = Rep::CostMinor("Unexpected Statement");
const COST_INVALID_SIGNATURE: Rep = Rep::CostMajor("Invalid Statement Signature");
const COST_DUPLICATE_STATEMENT: Rep = Rep::CostMajorRepeated("Statement sent more than once by peer");
const COST_APPARENT_FLOOD: Rep = Rep::Malicious("Peer appears to be flooding us with statements");
const BENEFIT_VALID_STATEMENT: Rep = Rep::new(5, "Peer provided a valid statement");
const BENEFIT_VALID_STATEMENT_FIRST: Rep = Rep::new(
25,
const BENEFIT_VALID_STATEMENT: Rep = Rep::BenefitMajor("Peer provided a valid statement");
const BENEFIT_VALID_STATEMENT_FIRST: Rep = Rep::BenefitMajorFirst(
"Peer was the first to provide a valid statement",
);
@@ -1077,7 +1076,7 @@ mod tests {
use std::sync::Arc;
use sp_keyring::Sr25519Keyring;
use sp_application_crypto::AppKey;
use node_primitives::Statement;
use polkadot_node_primitives::Statement;
use polkadot_primitives::v1::CommittedCandidateReceipt;
use assert_matches::assert_matches;
use futures::executor::{self, block_on};
+2 -2
View File
@@ -2028,7 +2028,7 @@ mod tests {
use polkadot_primitives::v1::{BlockData, CollatorPair, PoV, CandidateHash};
use polkadot_subsystem::{messages::RuntimeApiRequest, messages::NetworkBridgeEvent, JaegerSpan};
use polkadot_node_primitives::{CollationResult, CollationGenerationConfig};
use polkadot_node_network_protocol::{PeerId, ReputationChange};
use polkadot_node_network_protocol::{PeerId, UnifiedReputationChange};
use polkadot_node_subsystem_util::metered;
use sp_core::crypto::Pair as _;
@@ -2754,7 +2754,7 @@ mod tests {
}
fn test_network_bridge_msg() -> NetworkBridgeMessage {
NetworkBridgeMessage::ReportPeer(PeerId::random(), ReputationChange::new(42, ""))
NetworkBridgeMessage::ReportPeer(PeerId::random(), UnifiedReputationChange::BenefitMinor(""))
}
fn test_approval_distribution_msg() -> ApprovalDistributionMessage {
+2 -2
View File
@@ -25,7 +25,7 @@
use futures::channel::{mpsc, oneshot};
use thiserror::Error;
use polkadot_node_network_protocol::{
peer_set::PeerSet, v1 as protocol_v1, ReputationChange, PeerId,
peer_set::PeerSet, v1 as protocol_v1, UnifiedReputationChange, PeerId,
request_response::{Requests, request::IncomingRequest, v1 as req_res_v1},
};
use polkadot_node_primitives::{
@@ -217,7 +217,7 @@ impl CollatorProtocolMessage {
#[derive(Debug)]
pub enum NetworkBridgeMessage {
/// Report a peer for their actions.
ReportPeer(PeerId, ReputationChange),
ReportPeer(PeerId, UnifiedReputationChange),
/// Send a message to one or more peers on the validation peer-set.
SendValidationMessage(Vec<PeerId>, protocol_v1::ValidationProtocol),