Unprofitable message delivery tx metric (paritytech#1536)

* unprofitable message delivery tx metric

* proper impl

* send Rialto -> Millau messages using XCM pallet

* use altruistic relays in Rialto <> Millau bridge

* add unprofitable transactions dashboard

* fix + logging

* fix test

bridges/relays/messages/src/message_lane_loop.rs

@@ -271,7 +271,11 @@ pub async fn run<P: MessageLane, Strategy: RelayStrategy>(
 	relay_utils::relay_loop(source_client, target_client)
 		.reconnect_delay(params.reconnect_delay)
 		.with_metrics(metrics_params)
-		.loop_metric(MessageLaneLoopMetrics::new(Some(&metrics_prefix::<P>(&params.lane)))?)?
+		.loop_metric(MessageLaneLoopMetrics::new(
+			Some(&metrics_prefix::<P>(&params.lane)),
+			P::SOURCE_NAME,
+			P::TARGET_NAME,
+		)?)?
 		.expose()
 		.await?
 		.run(metrics_prefix::<P>(&params.lane), move |source_client, target_client, metrics| {

bridges/relays/messages/src/message_race_delivery.rs

@@ -56,7 +56,7 @@ pub async fn run<P: MessageLane, Strategy: RelayStrategy>(
 		source_state_updates,
 		MessageDeliveryRaceTarget {
 			client: target_client.clone(),
-			metrics_msg,
+			metrics_msg: metrics_msg.clone(),
 			_phantom: Default::default(),
 		},
 		target_state_updates,
@@ -74,6 +74,7 @@ pub async fn run<P: MessageLane, Strategy: RelayStrategy>(
 			latest_confirmed_nonces_at_source: VecDeque::new(),
 			target_nonces: None,
 			strategy: BasicStrategy::new(),
+			metrics_msg,
 		},
 	)
 	.await
@@ -255,6 +256,8 @@ struct MessageDeliveryStrategy<P: MessageLane, Strategy: RelayStrategy, SC, TC>
 	target_nonces: Option<TargetClientNonces<DeliveryRaceTargetNoncesData>>,
 	/// Basic delivery strategy.
 	strategy: MessageDeliveryStrategyBase<P>,
+	/// Message lane metrics.
+	metrics_msg: Option<MessageLaneLoopMetrics>,
 }
 
 type MessageDeliveryStrategyBase<P> = BasicStrategy<
@@ -519,6 +522,7 @@ where
 			lane_target_client: lane_target_client.clone(),
 			nonces_queue: source_queue.clone(),
 			nonces_queue_range: 0..maximal_source_queue_index + 1,
+			metrics: self.metrics_msg.clone(),
 		};
 
 		let mut strategy = EnforcementStrategy::new(self.relay_strategy.clone());
@@ -631,6 +635,7 @@ mod tests {
 			latest_confirmed_nonces_at_source: vec![(header_id(1), 19)].into_iter().collect(),
 			lane_source_client: TestSourceClient::default(),
 			lane_target_client: TestTargetClient::default(),
+			metrics_msg: None,
 			target_nonces: Some(TargetClientNonces {
 				latest_nonce: 19,
 				nonces_data: DeliveryRaceTargetNoncesData {

bridges/relays/messages/src/metrics.rs

@@ -24,7 +24,7 @@ use crate::{
 use bp_messages::MessageNonce;
 use finality_relay::SyncLoopMetrics;
 use relay_utils::metrics::{
-	metric_name, register, GaugeVec, Metric, Opts, PrometheusError, Registry, U64,
+	metric_name, register, Counter, GaugeVec, Metric, Opts, PrometheusError, Registry, U64,
 };
 
 /// Message lane relay metrics.
@@ -39,11 +39,17 @@ pub struct MessageLaneLoopMetrics {
 	/// Lane state nonces: "source_latest_generated", "source_latest_confirmed",
 	/// "target_latest_received", "target_latest_confirmed".
 	lane_state_nonces: GaugeVec<U64>,
+	/// Count of unprofitable message delivery transactions that we have submitted so far.
+	unprofitable_delivery_transactions: Counter<U64>,
 }
 
 impl MessageLaneLoopMetrics {
 	/// Create and register messages loop metrics.
-	pub fn new(prefix: Option<&str>) -> Result<Self, PrometheusError> {
+	pub fn new(
+		prefix: Option<&str>,
+		source_name: &str,
+		target_name: &str,
+	) -> Result<Self, PrometheusError> {
 		Ok(MessageLaneLoopMetrics {
 			source_to_target_finality_metrics: SyncLoopMetrics::new(
 				prefix,
@@ -59,6 +65,13 @@ impl MessageLaneLoopMetrics {
 				Opts::new(metric_name(prefix, "lane_state_nonces"), "Nonces of the lane state"),
 				&["type"],
 			)?,
+			unprofitable_delivery_transactions: Counter::new(
+				metric_name(prefix, "unprofitable_delivery_transactions"),
+				format!(
+					"Count of unprofitable message delivery transactions from {} to {}",
+					source_name, target_name
+				),
+			)?,
 		})
 	}
 
@@ -127,13 +140,19 @@ impl MessageLaneLoopMetrics {
 			.with_label_values(&["target_latest_confirmed"])
 			.set(target_latest_confirmed_nonce);
 	}
+
+	/// Note unprofitable delivery transaction.
+	pub fn note_unprofitable_delivery_transactions(&self) {
+		self.unprofitable_delivery_transactions.inc()
+	}
 }
 
 impl Metric for MessageLaneLoopMetrics {
 	fn register(&self, registry: &Registry) -> Result<(), PrometheusError> {
 		self.source_to_target_finality_metrics.register(registry)?;
 		self.target_to_source_finality_metrics.register(registry)?;
 		register(self.lane_state_nonces.clone(), registry)?;
+		register(self.unprofitable_delivery_transactions.clone(), registry)?;
 		Ok(())
 	}
 }

bridges/relays/messages/src/relay_strategy/altruistic_strategy.rs

@@ -23,7 +23,7 @@ use crate::{
 	message_lane_loop::{
 		SourceClient as MessageLaneSourceClient, TargetClient as MessageLaneTargetClient,
 	},
-	relay_strategy::{RelayReference, RelayStrategy},
+	relay_strategy::{RationalStrategy, RelayReference, RelayStrategy},
 };
 
 /// The relayer doesn't care about rewards.
@@ -38,8 +38,36 @@ impl RelayStrategy for AltruisticStrategy {
 		TargetClient: MessageLaneTargetClient<P>,
 	>(
 		&mut self,
-		_reference: &mut RelayReference<P, SourceClient, TargetClient>,
+		reference: &mut RelayReference<P, SourceClient, TargetClient>,
 	) -> bool {
+		// we don't care about costs and rewards, but we want to report unprofitable transactions
+		// => let rational strategy fill required fields
+		let _ = RationalStrategy.decide(reference).await;
 		true
 	}
+
+	async fn final_decision<
+		P: MessageLane,
+		SourceClient: MessageLaneSourceClient<P>,
+		TargetClient: MessageLaneTargetClient<P>,
+	>(
+		&self,
+		reference: &RelayReference<P, SourceClient, TargetClient>,
+	) {
+		if let Some(ref metrics) = reference.metrics {
+			if reference.total_cost > reference.total_reward {
+				log::debug!(
+					target: "bridge",
+					"The relayer has submitted unprofitable {} -> {} message delivery trabsaction with {} messages: total cost = {:?}, total reward = {:?}",
+					P::SOURCE_NAME,
+					P::TARGET_NAME,
+					reference.index + 1,
+					reference.total_cost,
+					reference.total_reward,
+				);
+
+				metrics.note_unprofitable_delivery_transactions();
+			}
+		}
+	}
 }

bridges/relays/messages/src/relay_strategy/enforcement_strategy.rs

@@ -65,6 +65,7 @@ impl<Strategy: RelayStrategy> EnforcementStrategy<Strategy> {
 		let mut relay_reference = RelayReference {
 			lane_source_client: reference.lane_source_client.clone(),
 			lane_target_client: reference.lane_target_client.clone(),
+			metrics: reference.metrics.clone(),
 
 			selected_reward: P::SourceChainBalance::zero(),
 			selected_cost: P::SourceChainBalance::zero(),
@@ -211,7 +212,10 @@ impl<Strategy: RelayStrategy> EnforcementStrategy<Strategy> {
 				);
 			}
 
-			Some(hard_selected_begin_nonce + hard_selected_count as MessageNonce - 1)
+			let selected_max_nonce =
+				hard_selected_begin_nonce + hard_selected_count as MessageNonce - 1;
+			self.strategy.final_decision(&relay_reference).await;
+			Some(selected_max_nonce)
 		} else {
 			None
 		}

bridges/relays/messages/src/relay_strategy/mix_strategy.rs

@@ -55,4 +55,18 @@ impl RelayStrategy for MixStrategy {
 			RelayerMode::Rational => RationalStrategy.decide(reference).await,
 		}
 	}
+
+	async fn final_decision<
+		P: MessageLane,
+		SourceClient: MessageLaneSourceClient<P>,
+		TargetClient: MessageLaneTargetClient<P>,
+	>(
+		&self,
+		reference: &RelayReference<P, SourceClient, TargetClient>,
+	) {
+		match self.relayer_mode {
+			RelayerMode::Altruistic => AltruisticStrategy.final_decision(reference).await,
+			RelayerMode::Rational => RationalStrategy.final_decision(reference).await,
+		}
+	}
 }

bridges/relays/messages/src/relay_strategy/mod.rs

@@ -16,11 +16,9 @@
 
 //! Relayer strategy
 
-use std::ops::Range;
-
 use async_trait::async_trait;
-
 use bp_messages::{MessageNonce, Weight};
+use std::ops::Range;
 
 use crate::{
 	message_lane::MessageLane,
@@ -29,6 +27,7 @@ use crate::{
 		TargetClient as MessageLaneTargetClient,
 	},
 	message_race_strategy::SourceRangesQueue,
+	metrics::MessageLaneLoopMetrics,
 };
 
 pub(crate) use self::enforcement_strategy::*;
@@ -55,6 +54,16 @@ pub trait RelayStrategy: 'static + Clone + Send + Sync {
 		&mut self,
 		reference: &mut RelayReference<P, SourceClient, TargetClient>,
 	) -> bool;
+
+	/// Notification that the following maximal nonce has been selected for the delivery.
+	async fn final_decision<
+		P: MessageLane,
+		SourceClient: MessageLaneSourceClient<P>,
+		TargetClient: MessageLaneTargetClient<P>,
+	>(
+		&self,
+		reference: &RelayReference<P, SourceClient, TargetClient>,
+	);
 }
 
 /// Reference data for participating in relay
@@ -67,6 +76,8 @@ pub struct RelayReference<
 	pub lane_source_client: SourceClient,
 	/// The client that is connected to the message lane target node.
 	pub lane_target_client: TargetClient,
+	/// Metrics reference.
+	pub metrics: Option<MessageLaneLoopMetrics>,
 	/// Current block reward summary
 	pub selected_reward: P::SourceChainBalance,
 	/// Current block cost summary
@@ -112,6 +123,8 @@ pub struct RelayMessagesBatchReference<
 	pub lane_source_client: SourceClient,
 	/// The client that is connected to the message lane target node.
 	pub lane_target_client: TargetClient,
+	/// Metrics reference.
+	pub metrics: Option<MessageLaneLoopMetrics>,
 	/// Source queue.
 	pub nonces_queue: SourceRangesQueue<
 		P::SourceHeaderHash,

bridges/relays/messages/src/relay_strategy/rational_strategy.rs

@@ -44,37 +44,23 @@ impl RelayStrategy for RationalStrategy {
 		&mut self,
 		reference: &mut RelayReference<P, SourceClient, TargetClient>,
 	) -> bool {
-		// technically, multiple confirmations will be delivered in a single transaction,
-		// meaning less loses for relayer. But here we don't know the final relayer yet, so
-		// we're adding a separate transaction for every message. Normally, this cost is covered
-		// by the message sender. Probably reconsider this?
-		let confirmation_transaction_cost =
-			reference.lane_source_client.estimate_confirmation_transaction().await;
-
-		let delivery_transaction_cost = match reference
-			.lane_target_client
-			.estimate_delivery_transaction_in_source_tokens(
-				reference.hard_selected_begin_nonce..=
-					(reference.hard_selected_begin_nonce + reference.index as MessageNonce),
-				reference.selected_prepaid_nonces,
-				reference.selected_unpaid_weight,
-				reference.selected_size as u32,
-			)
-			.await
-		{
-			Ok(v) => v,
+		let total_cost = match estimate_messages_delivery_cost(reference).await {
+			Ok(total_cost) => total_cost,
 			Err(err) => {
 				log::debug!(
 					target: "bridge",
 					"Failed to estimate delivery transaction cost: {:?}. No nonces selected for delivery",
 					err,
 				);
+
 				return false
 			},
 		};
 
 		// if it is the first message that makes reward less than cost, let's log it
 		// if this message makes batch profitable again, let's log it
+		let MessagesDeliveryCost { confirmation_transaction_cost, delivery_transaction_cost } =
+			total_cost;
 		let is_total_reward_less_than_cost = reference.total_reward < reference.total_cost;
 		let prev_total_cost = reference.total_cost;
 		let prev_total_reward = reference.total_reward;
@@ -119,4 +105,53 @@ impl RelayStrategy for RationalStrategy {
 
 		false
 	}
+
+	async fn final_decision<
+		P: MessageLane,
+		SourceClient: MessageLaneSourceClient<P>,
+		TargetClient: MessageLaneTargetClient<P>,
+	>(
+		&self,
+		_reference: &RelayReference<P, SourceClient, TargetClient>,
+	) {
+		// rational relayer would never submit unprofitable transactions, so we don't need to do
+		// anything here
+	}
+}
+
+/// Total cost of mesage delivery and confirmation.
+struct MessagesDeliveryCost<SourceChainBalance> {
+	/// Cost of message delivery transaction.
+	pub delivery_transaction_cost: SourceChainBalance,
+	/// Cost of confirmation delivery transaction.
+	pub confirmation_transaction_cost: SourceChainBalance,
+}
+
+/// Returns cost of message delivery and confirmation delivery transactions
+async fn estimate_messages_delivery_cost<
+	P: MessageLane,
+	SourceClient: MessageLaneSourceClient<P>,
+	TargetClient: MessageLaneTargetClient<P>,
+>(
+	reference: &RelayReference<P, SourceClient, TargetClient>,
+) -> Result<MessagesDeliveryCost<P::SourceChainBalance>, TargetClient::Error> {
+	// technically, multiple confirmations will be delivered in a single transaction,
+	// meaning less loses for relayer. But here we don't know the final relayer yet, so
+	// we're adding a separate transaction for every message. Normally, this cost is covered
+	// by the message sender. Probably reconsider this?
+	let confirmation_transaction_cost =
+		reference.lane_source_client.estimate_confirmation_transaction().await;
+
+	let delivery_transaction_cost = reference
+		.lane_target_client
+		.estimate_delivery_transaction_in_source_tokens(
+			reference.hard_selected_begin_nonce..=
+				(reference.hard_selected_begin_nonce + reference.index as MessageNonce),
+			reference.selected_prepaid_nonces,
+			reference.selected_unpaid_weight,
+			reference.selected_size as u32,
+		)
+		.await?;
+
+	Ok(MessagesDeliveryCost { confirmation_transaction_cost, delivery_transaction_cost })
 }