From 487ac63072820df61018cf0234618f39a37cc7d9 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Fri, 5 Aug 2022 10:00:53 +0300 Subject: [PATCH] Unprofitable message delivery tx metric (#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 --- .../relays/messages/src/message_lane_loop.rs | 6 +- .../messages/src/message_race_delivery.rs | 7 +- bridges/relays/messages/src/metrics.rs | 23 +++++- .../src/relay_strategy/altruistic_strategy.rs | 32 +++++++- .../relay_strategy/enforcement_strategy.rs | 6 +- .../src/relay_strategy/mix_strategy.rs | 14 ++++ .../relays/messages/src/relay_strategy/mod.rs | 19 ++++- .../src/relay_strategy/rational_strategy.rs | 73 ++++++++++++++----- 8 files changed, 151 insertions(+), 29 deletions(-) diff --git a/bridges/relays/messages/src/message_lane_loop.rs b/bridges/relays/messages/src/message_lane_loop.rs index 84473f0f27b23..2e700deb9a6bf 100644 --- a/bridges/relays/messages/src/message_lane_loop.rs +++ b/bridges/relays/messages/src/message_lane_loop.rs @@ -271,7 +271,11 @@ pub async fn run( relay_utils::relay_loop(source_client, target_client) .reconnect_delay(params.reconnect_delay) .with_metrics(metrics_params) - .loop_metric(MessageLaneLoopMetrics::new(Some(&metrics_prefix::

(¶ms.lane)))?)? + .loop_metric(MessageLaneLoopMetrics::new( + Some(&metrics_prefix::

(¶ms.lane)), + P::SOURCE_NAME, + P::TARGET_NAME, + )?)? .expose() .await? .run(metrics_prefix::

(¶ms.lane), move |source_client, target_client, metrics| { diff --git a/bridges/relays/messages/src/message_race_delivery.rs b/bridges/relays/messages/src/message_race_delivery.rs index 4484d4b7994c4..85f6d955e7df0 100644 --- a/bridges/relays/messages/src/message_race_delivery.rs +++ b/bridges/relays/messages/src/message_race_delivery.rs @@ -56,7 +56,7 @@ pub async fn run( 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( latest_confirmed_nonces_at_source: VecDeque::new(), target_nonces: None, strategy: BasicStrategy::new(), + metrics_msg, }, ) .await @@ -255,6 +256,8 @@ struct MessageDeliveryStrategy target_nonces: Option>, /// Basic delivery strategy. strategy: MessageDeliveryStrategyBase

, + /// Message lane metrics. + metrics_msg: Option, } type MessageDeliveryStrategyBase

= 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 { diff --git a/bridges/relays/messages/src/metrics.rs b/bridges/relays/messages/src/metrics.rs index 4decb7e092e71..f5cc8831c0286 100644 --- a/bridges/relays/messages/src/metrics.rs +++ b/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, + /// Count of unprofitable message delivery transactions that we have submitted so far. + unprofitable_delivery_transactions: Counter, } impl MessageLaneLoopMetrics { /// Create and register messages loop metrics. - pub fn new(prefix: Option<&str>) -> Result { + pub fn new( + prefix: Option<&str>, + source_name: &str, + target_name: &str, + ) -> Result { 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,6 +140,11 @@ 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 { @@ -134,6 +152,7 @@ impl Metric for MessageLaneLoopMetrics { 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(()) } } diff --git a/bridges/relays/messages/src/relay_strategy/altruistic_strategy.rs b/bridges/relays/messages/src/relay_strategy/altruistic_strategy.rs index d6fec7f1297b3..b66129117873e 100644 --- a/bridges/relays/messages/src/relay_strategy/altruistic_strategy.rs +++ b/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

, >( &mut self, - _reference: &mut RelayReference, + reference: &mut RelayReference, ) -> 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

, + TargetClient: MessageLaneTargetClient

, + >( + &self, + reference: &RelayReference, + ) { + 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(); + } + } + } } diff --git a/bridges/relays/messages/src/relay_strategy/enforcement_strategy.rs b/bridges/relays/messages/src/relay_strategy/enforcement_strategy.rs index 1e9ef5bdbf818..5d231462e86d9 100644 --- a/bridges/relays/messages/src/relay_strategy/enforcement_strategy.rs +++ b/bridges/relays/messages/src/relay_strategy/enforcement_strategy.rs @@ -65,6 +65,7 @@ impl EnforcementStrategy { 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 EnforcementStrategy { ); } - 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 } diff --git a/bridges/relays/messages/src/relay_strategy/mix_strategy.rs b/bridges/relays/messages/src/relay_strategy/mix_strategy.rs index 4ac7fe1d0ed06..d00c27196787c 100644 --- a/bridges/relays/messages/src/relay_strategy/mix_strategy.rs +++ b/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

, + TargetClient: MessageLaneTargetClient

, + >( + &self, + reference: &RelayReference, + ) { + match self.relayer_mode { + RelayerMode::Altruistic => AltruisticStrategy.final_decision(reference).await, + RelayerMode::Rational => RationalStrategy.final_decision(reference).await, + } + } } diff --git a/bridges/relays/messages/src/relay_strategy/mod.rs b/bridges/relays/messages/src/relay_strategy/mod.rs index d902bd93e5cf9..da615f34e8659 100644 --- a/bridges/relays/messages/src/relay_strategy/mod.rs +++ b/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, ) -> bool; + + /// Notification that the following maximal nonce has been selected for the delivery. + async fn final_decision< + P: MessageLane, + SourceClient: MessageLaneSourceClient

, + TargetClient: MessageLaneTargetClient

, + >( + &self, + reference: &RelayReference, + ); } /// 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, /// 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, /// Source queue. pub nonces_queue: SourceRangesQueue< P::SourceHeaderHash, diff --git a/bridges/relays/messages/src/relay_strategy/rational_strategy.rs b/bridges/relays/messages/src/relay_strategy/rational_strategy.rs index fd0a1ffafc8b9..f45f01725c631 100644 --- a/bridges/relays/messages/src/relay_strategy/rational_strategy.rs +++ b/bridges/relays/messages/src/relay_strategy/rational_strategy.rs @@ -44,37 +44,23 @@ impl RelayStrategy for RationalStrategy { &mut self, reference: &mut RelayReference, ) -> 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

, + TargetClient: MessageLaneTargetClient

, + >( + &self, + _reference: &RelayReference, + ) { + // 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 { + /// 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

, + TargetClient: MessageLaneTargetClient

, +>( + reference: &RelayReference, +) -> Result, 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 }) }