add dispute metrics, some chores (#3842)

* rename: MsgFilter -> MessageInterceptor

* feat: add dispute metrics

* fixup

* test fixins

* fix metrics

* dummysubsystem export and trait fn fix

* chore: fmt

* undo unwanted changes

* foo

* pfmt

* fixup

* fixup

* revert

* some more

* Update node/malus/Cargo.toml

Co-authored-by: Andronik Ordian <write@reusable.software>

* Update node/core/dispute-coordinator/src/metrics.rs

Co-authored-by: Andronik Ordian <write@reusable.software>

* Update node/core/dispute-coordinator/src/metrics.rs

Co-authored-by: Andronik Ordian <write@reusable.software>

* Update node/core/dispute-coordinator/src/metrics.rs

Co-authored-by: Andronik Ordian <write@reusable.software>

* add license header

* fix lockfile

* new with opts

* fmt

* Update node/core/dispute-coordinator/src/metrics.rs

* feature gate

Co-authored-by: Andronik Ordian <write@reusable.software>

polkadot/node/malus/src/lib.rs

@@ -24,38 +24,52 @@ use polkadot_node_subsystem::*;
 pub use polkadot_node_subsystem::{messages::AllMessages, overseer, FromOverseer};
 use std::{future::Future, pin::Pin};
 
+#[cfg(test)]
+mod tests;
+
 /// Filter incoming and outgoing messages.
-pub trait MsgFilter: Send + Sync + Clone + 'static {
+pub trait MessageInterceptor<Sender>: Send + Sync + Clone + 'static
+where
+	Sender: overseer::SubsystemSender<Self::Message> + Clone + 'static,
+{
 	/// The message type the original subsystem handles incoming.
 	type Message: Send + 'static;
 
 	/// Filter messages that are to be received by
 	/// the subsystem.
-	fn filter_in(&self, msg: FromOverseer<Self::Message>) -> Option<FromOverseer<Self::Message>> {
+	///
+	/// For non-trivial cases, the `sender` can be used to send
+	/// multiple messages after doing some additional processing.
+	fn intercept_incoming(
+		&self,
+		_sender: &mut Sender,
+		msg: FromOverseer<Self::Message>,
+	) -> Option<FromOverseer<Self::Message>> {
 		Some(msg)
 	}
 
 	/// Modify outgoing messages.
-	fn filter_out(&self, msg: AllMessages) -> Option<AllMessages> {
+	fn intercept_outgoing(&self, msg: AllMessages) -> Option<AllMessages> {
 		Some(msg)
 	}
 }
 
 /// A sender with the outgoing messages filtered.
 #[derive(Clone)]
-pub struct FilteredSender<Sender, Fil> {
+pub struct InterceptedSender<Sender, Fil> {
 	inner: Sender,
 	message_filter: Fil,
 }
 
 #[async_trait::async_trait]
-impl<Sender, Fil> overseer::SubsystemSender<AllMessages> for FilteredSender<Sender, Fil>
+impl<Sender, Fil> overseer::SubsystemSender<AllMessages> for InterceptedSender<Sender, Fil>
 where
-	Sender: overseer::SubsystemSender<AllMessages>,
-	Fil: MsgFilter,
+	Sender: overseer::SubsystemSender<AllMessages>
+		+ overseer::SubsystemSender<<Fil as MessageInterceptor<Sender>>::Message>,
+	Fil: MessageInterceptor<Sender>,
 {
 	async fn send_message(&mut self, msg: AllMessages) {
-		if let Some(msg) = self.message_filter.filter_out(msg) {
+		if let Some(msg) = self.message_filter.intercept_outgoing(msg) {
 			self.inner.send_message(msg).await;
 		}
 	}
@@ -71,26 +85,39 @@ where
 	}
 
 	fn send_unbounded_message(&mut self, msg: AllMessages) {
-		if let Some(msg) = self.message_filter.filter_out(msg) {
+		if let Some(msg) = self.message_filter.intercept_outgoing(msg) {
 			self.inner.send_unbounded_message(msg);
 		}
 	}
 }
 
 /// A subsystem context, that filters the outgoing messages.
-pub struct FilteredContext<Context: overseer::SubsystemContext + SubsystemContext, Fil: MsgFilter> {
-	inner: Context,
-	message_filter: Fil,
-	sender: FilteredSender<<Context as overseer::SubsystemContext>::Sender, Fil>,
-}
-
-impl<Context, Fil> FilteredContext<Context, Fil>
+pub struct InterceptedContext<Context, Fil>
 where
 	Context: overseer::SubsystemContext + SubsystemContext,
-	Fil: MsgFilter<Message = <Context as overseer::SubsystemContext>::Message>,
+	Fil: MessageInterceptor<<Context as overseer::SubsystemContext>::Sender>,
+	<Context as overseer::SubsystemContext>::Sender: overseer::SubsystemSender<
+		<Fil as MessageInterceptor<<Context as overseer::SubsystemContext>::Sender>>::Message,
+	>,
+{
+	inner: Context,
+	message_filter: Fil,
+	sender: InterceptedSender<<Context as overseer::SubsystemContext>::Sender, Fil>,
+}
+
+impl<Context, Fil> InterceptedContext<Context, Fil>
+where
+	Context: overseer::SubsystemContext + SubsystemContext,
+	Fil: MessageInterceptor<
+		<Context as overseer::SubsystemContext>::Sender,
+		Message = <Context as overseer::SubsystemContext>::Message,
+	>,
+	<Context as overseer::SubsystemContext>::Sender: overseer::SubsystemSender<
+		<Fil as MessageInterceptor<<Context as overseer::SubsystemContext>::Sender>>::Message,
+	>,
 {
 	pub fn new(mut inner: Context, message_filter: Fil) -> Self {
-		let sender = FilteredSender::<<Context as overseer::SubsystemContext>::Sender, Fil> {
+		let sender = InterceptedSender::<<Context as overseer::SubsystemContext>::Sender, Fil> {
 			inner: inner.sender().clone(),
 			message_filter: message_filter.clone(),
 		};
@@ -99,15 +126,21 @@ where
 }
 
 #[async_trait::async_trait]
-impl<Context, Fil> overseer::SubsystemContext for FilteredContext<Context, Fil>
+impl<Context, Fil> overseer::SubsystemContext for InterceptedContext<Context, Fil>
 where
 	Context: overseer::SubsystemContext + SubsystemContext,
-	Fil: MsgFilter<Message = <Context as overseer::SubsystemContext>::Message>,
+	Fil: MessageInterceptor<
+		<Context as overseer::SubsystemContext>::Sender,
+		Message = <Context as overseer::SubsystemContext>::Message,
+	>,
 	<Context as overseer::SubsystemContext>::AllMessages:
 		From<<Context as overseer::SubsystemContext>::Message>,
+	<Context as overseer::SubsystemContext>::Sender: overseer::SubsystemSender<
+		<Fil as MessageInterceptor<<Context as overseer::SubsystemContext>::Sender>>::Message,
+	>,
 {
 	type Message = <Context as overseer::SubsystemContext>::Message;
-	type Sender = FilteredSender<<Context as overseer::SubsystemContext>::Sender, Fil>;
+	type Sender = InterceptedSender<<Context as overseer::SubsystemContext>::Sender, Fil>;
 	type Error = <Context as overseer::SubsystemContext>::Error;
 	type AllMessages = <Context as overseer::SubsystemContext>::AllMessages;
 	type Signal = <Context as overseer::SubsystemContext>::Signal;
@@ -117,7 +150,9 @@ where
 			match self.inner.try_recv().await? {
 				None => return Ok(None),
 				Some(msg) =>
-					if let Some(msg) = self.message_filter.filter_in(msg) {
+					if let Some(msg) =
+						self.message_filter.intercept_incoming(self.inner.sender(), msg)
+					{
 						return Ok(Some(msg))
 					},
 			}
@@ -127,7 +162,7 @@ where
 	async fn recv(&mut self) -> SubsystemResult<FromOverseer<Self::Message>> {
 		loop {
 			let msg = self.inner.recv().await?;
-			if let Some(msg) = self.message_filter.filter_in(msg) {
+			if let Some(msg) = self.message_filter.intercept_incoming(self.inner.sender(), msg) {
 				return Ok(msg)
 			}
 		}
@@ -155,27 +190,33 @@ where
 }
 
 /// A subsystem to which incoming and outgoing filters are applied.
-pub struct FilteredSubsystem<Sub, Fil> {
-	subsystem: Sub,
-	message_filter: Fil,
+pub struct InterceptedSubsystem<Sub, Interceptor> {
+	pub subsystem: Sub,
+	pub message_interceptor: Interceptor,
 }
 
-impl<Sub, Fil> FilteredSubsystem<Sub, Fil> {
-	pub fn new(subsystem: Sub, message_filter: Fil) -> Self {
-		Self { subsystem, message_filter }
+impl<Sub, Interceptor> InterceptedSubsystem<Sub, Interceptor> {
+	pub fn new(subsystem: Sub, message_interceptor: Interceptor) -> Self {
+		Self { subsystem, message_interceptor }
 	}
 }
 
-impl<Context, Sub, Fil> overseer::Subsystem<Context, SubsystemError> for FilteredSubsystem<Sub, Fil>
+impl<Context, Sub, Interceptor> overseer::Subsystem<Context, SubsystemError> for InterceptedSubsystem<Sub, Interceptor>
 where
 	Context: overseer::SubsystemContext + SubsystemContext + Sync + Send,
-	Sub: overseer::Subsystem<FilteredContext<Context, Fil>, SubsystemError>,
-	FilteredContext<Context, Fil>: overseer::SubsystemContext + SubsystemContext,
-	Fil: MsgFilter<Message = <Context as overseer::SubsystemContext>::Message>,
+	Sub: overseer::Subsystem<InterceptedContext<Context, Interceptor>, SubsystemError>,
+	InterceptedContext<Context, Interceptor>: overseer::SubsystemContext + SubsystemContext,
+	Interceptor: MessageInterceptor<
+		<Context as overseer::SubsystemContext>::Sender,
+		Message = <Context as overseer::SubsystemContext>::Message,
+	>,
+	<Context as overseer::SubsystemContext>::Sender: overseer::SubsystemSender<
+		<Interceptor as MessageInterceptor<<Context as overseer::SubsystemContext>::Sender>>::Message,
+	>,
 {
 	fn start(self, ctx: Context) -> SpawnedSubsystem {
-		let ctx = FilteredContext::new(ctx, self.message_filter);
-		overseer::Subsystem::<FilteredContext<Context, Fil>, SubsystemError>::start(
+		let ctx = InterceptedContext::new(ctx, self.message_interceptor);
+		overseer::Subsystem::<InterceptedContext<Context, Interceptor>, SubsystemError>::start(
 			self.subsystem,
 			ctx,
 		)

polkadot/node/malus/src/tests.rs

@@ -0,0 +1,90 @@
+// Copyright 2021 Parity Technologies (UK) Ltd.
+// This file is part of Polkadot.
+
+// Polkadot is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Polkadot is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
+
+use super::*;
+
+use polkadot_node_subsystem_test_helpers::*;
+
+use polkadot_node_subsystem::{
+	messages::{AllMessages, AvailabilityStoreMessage},
+	overseer::{gen::TimeoutExt, Subsystem},
+	DummySubsystem,
+};
+
+#[derive(Clone, Debug)]
+struct BlackHoleInterceptor;
+
+impl<Sender> MessageInterceptor<Sender> for BlackHoleInterceptor
+where
+	Sender: overseer::SubsystemSender<AllMessages>
+		+ overseer::SubsystemSender<AvailabilityStoreMessage>
+		+ Clone
+		+ 'static,
+{
+	type Message = AvailabilityStoreMessage;
+	fn intercept_incoming(
+		&self,
+		_sender: &mut Sender,
+		msg: FromOverseer<Self::Message>,
+	) -> Option<FromOverseer<Self::Message>> {
+		match msg {
+			FromOverseer::Communication { msg: _msg } => None,
+			// to conclude the test cleanly
+			sig => Some(sig),
+		}
+	}
+}
+
+async fn overseer_send<T: Into<AllMessages>>(overseer: &mut TestSubsystemContextHandle<T>, msg: T) {
+	overseer.send(FromOverseer::Communication { msg }).await;
+}
+
+#[test]
+fn integrity_test() {
+	let pool = sp_core::testing::TaskExecutor::new();
+	let (context, mut overseer) = make_subsystem_context(pool);
+
+	let sub = DummySubsystem;
+
+	let sub_intercepted = InterceptedSubsystem::new(sub, BlackHoleInterceptor);
+
+	// Try to send a message we know is going to be filtered.
+	let test_fut = async move {
+		let (tx, rx) = futures::channel::oneshot::channel();
+		overseer_send(
+			&mut overseer,
+			AvailabilityStoreMessage::QueryChunk(Default::default(), 0.into(), tx),
+		)
+		.await;
+		let _ = rx.timeout(std::time::Duration::from_millis(100)).await.unwrap();
+		overseer
+	};
+	let subsystem = async move {
+		sub_intercepted.start(context).future.await.unwrap();
+	};
+
+	futures::pin_mut!(test_fut);
+	futures::pin_mut!(subsystem);
+
+	futures::executor::block_on(futures::future::join(
+		async move {
+			let mut overseer = test_fut.await;
+			overseer.send(FromOverseer::Signal(OverseerSignal::Conclude)).await;
+		},
+		subsystem,
+	))
+	.1;
+}

polkadot/node/malus/src/variant-a.rs

@@ -27,7 +27,7 @@ use polkadot_cli::{
 	create_default_subsystems,
 	service::{
 		AuthorityDiscoveryApi, AuxStore, BabeApi, Block, Error, HeaderBackend, Overseer,
-		OverseerGen, OverseerGenArgs, OverseerHandle, ParachainHost, ProvideRuntimeApi, SpawnNamed,
+		OverseerGen, OverseerGenArgs, ParachainHost, ProvideRuntimeApi, SpawnNamed,
 	},
 	Cli,
 };
@@ -35,11 +35,15 @@ use polkadot_cli::{
 // Import extra types relevant to the particular
 // subsystem.
 use polkadot_node_core_candidate_validation::CandidateValidationSubsystem;
-use polkadot_node_subsystem::messages::CandidateValidationMessage;
+use polkadot_node_subsystem::{
+	messages::{AllMessages, CandidateValidationMessage},
+	overseer::{self, OverseerHandle},
+	FromOverseer,
+};
 
-// Filter wrapping related types.
 use malus::*;
 
+// Filter wrapping related types.
 use std::sync::{
 	atomic::{AtomicUsize, Ordering},
 	Arc,
@@ -51,17 +55,27 @@ use structopt::StructOpt;
 #[derive(Clone, Default, Debug)]
 struct Skippy(Arc<AtomicUsize>);
 
-impl MsgFilter for Skippy {
+impl<Sender> MessageInterceptor<Sender> for Skippy
+where
+	Sender: overseer::SubsystemSender<AllMessages>
+		+ overseer::SubsystemSender<CandidateValidationMessage>
+		+ Clone
+		+ 'static,
+{
 	type Message = CandidateValidationMessage;
 
-	fn filter_in(&self, msg: FromOverseer<Self::Message>) -> Option<FromOverseer<Self::Message>> {
+	fn intercept_incoming(
+		&self,
+		_sender: &mut Sender,
+		msg: FromOverseer<Self::Message>,
+	) -> Option<FromOverseer<Self::Message>> {
 		if self.0.fetch_add(1, Ordering::Relaxed) % 2 == 0 {
 			Some(msg)
 		} else {
 			None
 		}
 	}
-	fn filter_out(&self, msg: AllMessages) -> Option<AllMessages> {
+	fn intercept_outgoing(&self, msg: AllMessages) -> Option<AllMessages> {
 		Some(msg)
 	}
 }
@@ -88,7 +102,7 @@ impl OverseerGen for BehaveMaleficient {
 		let all_subsystems = create_default_subsystems(args)?.replace_candidate_validation(
 			// create the filtered subsystem
 			|orig: CandidateValidationSubsystem| {
-				FilteredSubsystem::new(
+				InterceptedSubsystem::new(
 					CandidateValidationSubsystem::with_config(
 						candidate_validation_config,
 						orig.metrics,