From fd39d3519ee2bb66032dd8b0aa35a62b1a1aa115 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Thu, 1 Jul 2021 09:26:54 +0300 Subject: [PATCH] Use real conversion rate in greedy relayer strategy (#1035) * use real conversion rate in greedy relayer strategy * only accept positive, normal numbers in FloatJsonValueMetric --- .../src/chains/millau_messages_to_rialto.rs | 3 +- .../src/chains/rialto_messages_to_millau.rs | 3 +- .../src/chains/rococo_messages_to_wococo.rs | 3 +- .../src/chains/wococo_messages_to_rococo.rs | 3 +- .../relays/bin-substrate/src/messages_lane.rs | 9 ++++++ .../bin-substrate/src/messages_target.rs | 28 ++++++++++++++----- .../relays/messages/src/message_lane_loop.rs | 12 ++++---- .../messages/src/message_race_delivery.rs | 10 ++++++- .../utils/src/metrics/float_json_value.rs | 24 ++++++++++++++++ 9 files changed, 78 insertions(+), 17 deletions(-) diff --git a/bridges/relays/bin-substrate/src/chains/millau_messages_to_rialto.rs b/bridges/relays/bin-substrate/src/chains/millau_messages_to_rialto.rs index 77f1adbd37..21a5f6fd27 100644 --- a/bridges/relays/bin-substrate/src/chains/millau_messages_to_rialto.rs +++ b/bridges/relays/bin-substrate/src/chains/millau_messages_to_rialto.rs @@ -171,7 +171,7 @@ pub async fn run( max_messages_weight_in_single_batch, ); - let (metrics_params, _) = add_standalone_metrics( + let (metrics_params, metrics_values) = add_standalone_metrics( Some(messages_relay::message_lane_loop::metrics_prefix::< MillauMessagesToRialto, >(&lane_id)), @@ -206,6 +206,7 @@ pub async fn run( lane, lane_id, MILLAU_CHAIN_ID, + metrics_values, params.source_to_target_headers_relay, ), metrics_params, diff --git a/bridges/relays/bin-substrate/src/chains/rialto_messages_to_millau.rs b/bridges/relays/bin-substrate/src/chains/rialto_messages_to_millau.rs index 8a82456cdf..1c0fc0a52e 100644 --- a/bridges/relays/bin-substrate/src/chains/rialto_messages_to_millau.rs +++ b/bridges/relays/bin-substrate/src/chains/rialto_messages_to_millau.rs @@ -170,7 +170,7 @@ pub async fn run( max_messages_weight_in_single_batch, ); - let (metrics_params, _) = add_standalone_metrics( + let (metrics_params, metrics_values) = add_standalone_metrics( Some(messages_relay::message_lane_loop::metrics_prefix::< RialtoMessagesToMillau, >(&lane_id)), @@ -205,6 +205,7 @@ pub async fn run( lane, lane_id, RIALTO_CHAIN_ID, + metrics_values, params.source_to_target_headers_relay, ), metrics_params, diff --git a/bridges/relays/bin-substrate/src/chains/rococo_messages_to_wococo.rs b/bridges/relays/bin-substrate/src/chains/rococo_messages_to_wococo.rs index 1e30f97f49..99bcf53c8a 100644 --- a/bridges/relays/bin-substrate/src/chains/rococo_messages_to_wococo.rs +++ b/bridges/relays/bin-substrate/src/chains/rococo_messages_to_wococo.rs @@ -185,7 +185,7 @@ pub async fn run( max_messages_weight_in_single_batch, ); - let (metrics_params, _) = add_standalone_metrics( + let (metrics_params, metrics_values) = add_standalone_metrics( Some(messages_relay::message_lane_loop::metrics_prefix::< RococoMessagesToWococo, >(&lane_id)), @@ -220,6 +220,7 @@ pub async fn run( lane, lane_id, ROCOCO_CHAIN_ID, + metrics_values, params.source_to_target_headers_relay, ), metrics_params, diff --git a/bridges/relays/bin-substrate/src/chains/wococo_messages_to_rococo.rs b/bridges/relays/bin-substrate/src/chains/wococo_messages_to_rococo.rs index 04f9e1f559..b9fcd7ac3a 100644 --- a/bridges/relays/bin-substrate/src/chains/wococo_messages_to_rococo.rs +++ b/bridges/relays/bin-substrate/src/chains/wococo_messages_to_rococo.rs @@ -185,7 +185,7 @@ pub async fn run( max_messages_weight_in_single_batch, ); - let (metrics_params, _) = add_standalone_metrics( + let (metrics_params, metrics_values) = add_standalone_metrics( Some(messages_relay::message_lane_loop::metrics_prefix::< WococoMessagesToRococo, >(&lane_id)), @@ -220,6 +220,7 @@ pub async fn run( lane, lane_id, WOCOCO_CHAIN_ID, + metrics_values, params.source_to_target_headers_relay, ), metrics_params, diff --git a/bridges/relays/bin-substrate/src/messages_lane.rs b/bridges/relays/bin-substrate/src/messages_lane.rs index aed208a8ad..b0119b12f0 100644 --- a/bridges/relays/bin-substrate/src/messages_lane.rs +++ b/bridges/relays/bin-substrate/src/messages_lane.rs @@ -201,6 +201,15 @@ pub struct StandaloneMessagesMetrics { pub source_to_base_conversion_rate: Option, } +impl StandaloneMessagesMetrics { + /// Return conversion rate from target to source tokens. + pub async fn target_to_source_conversion_rate(&self) -> Option { + let target_to_base_conversion_rate = (*self.target_to_base_conversion_rate.as_ref()?.read().await)?; + let source_to_base_conversion_rate = (*self.source_to_base_conversion_rate.as_ref()?.read().await)?; + Some(target_to_base_conversion_rate / source_to_base_conversion_rate) + } +} + /// Add general standalone metrics for the message lane relay loop. pub fn add_standalone_metrics( metrics_prefix: Option, diff --git a/bridges/relays/bin-substrate/src/messages_target.rs b/bridges/relays/bin-substrate/src/messages_target.rs index 71f51c1e05..5abcd88b85 100644 --- a/bridges/relays/bin-substrate/src/messages_target.rs +++ b/bridges/relays/bin-substrate/src/messages_target.rs @@ -18,7 +18,7 @@ //! runtime that implements `HeaderApi` to allow bridging with //! chain. -use crate::messages_lane::SubstrateMessageLane; +use crate::messages_lane::{StandaloneMessagesMetrics, SubstrateMessageLane}; use crate::messages_source::{read_client_state, SubstrateMessagesProof}; use crate::on_demand_headers::OnDemandHeadersRelay; @@ -34,7 +34,7 @@ use messages_relay::{ message_lane::{SourceHeaderIdOf, TargetHeaderIdOf}, message_lane_loop::{TargetClient, TargetClientState}, }; -use num_traits::{Bounded, One, Zero}; +use num_traits::{Bounded, Zero}; use relay_substrate_client::{Chain, Client, Error as SubstrateError, HashOf}; use relay_utils::{relay_loop::Client as RelayClient, BlockNumberBase, HeaderId}; use sp_core::Bytes; @@ -53,6 +53,7 @@ pub struct SubstrateMessagesTarget>, _phantom: PhantomData, } @@ -64,6 +65,7 @@ impl SubstrateMessagesTarget>, ) -> Self { SubstrateMessagesTarget { @@ -71,6 +73,7 @@ impl SubstrateMessagesTarget Clone for SubstrateMessag lane: self.lane.clone(), lane_id: self.lane_id, instance: self.instance, + metric_values: self.metric_values.clone(), source_to_target_headers_relay: self.source_to_target_headers_relay.clone(), _phantom: Default::default(), } @@ -239,10 +243,20 @@ where nonces: RangeInclusive, total_dispatch_weight: Weight, total_size: u32, - ) -> P::SourceChainBalance { - // TODO: use actual rate (https://github.com/paritytech/parity-bridges-common/issues/997) - convert_target_tokens_to_source_tokens::( - FixedU128::one(), + ) -> Result { + let conversion_rate = self + .metric_values + .target_to_source_conversion_rate() + .await + .ok_or_else(|| { + SubstrateError::Custom(format!( + "Failed to compute conversion rate from {} to {}", + TC::NAME, + SC::NAME, + )) + })?; + Ok(convert_target_tokens_to_source_tokens::( + FixedU128::from_float(conversion_rate), self.client .estimate_extrinsic_fee(self.lane.make_messages_delivery_transaction( Zero::zero(), @@ -252,7 +266,7 @@ where )) .await .unwrap_or_else(|_| TC::Balance::max_value()), - ) + )) } } diff --git a/bridges/relays/messages/src/message_lane_loop.rs b/bridges/relays/messages/src/message_lane_loop.rs index 7ad5952603..c70fc532e9 100644 --- a/bridges/relays/messages/src/message_lane_loop.rs +++ b/bridges/relays/messages/src/message_lane_loop.rs @@ -212,7 +212,7 @@ pub trait TargetClient: RelayClient { nonces: RangeInclusive, total_dispatch_weight: Weight, total_size: u32, - ) -> P::SourceChainBalance; + ) -> Result; } /// State of the client. @@ -775,10 +775,12 @@ pub(crate) mod tests { nonces: RangeInclusive, total_dispatch_weight: Weight, total_size: u32, - ) -> TestSourceChainBalance { - BASE_MESSAGE_DELIVERY_TRANSACTION_COST * (nonces.end() - nonces.start() + 1) - + total_dispatch_weight - + total_size as TestSourceChainBalance + ) -> Result { + Ok( + BASE_MESSAGE_DELIVERY_TRANSACTION_COST * (nonces.end() - nonces.start() + 1) + + total_dispatch_weight + + total_size as TestSourceChainBalance, + ) } } diff --git a/bridges/relays/messages/src/message_race_delivery.rs b/bridges/relays/messages/src/message_race_delivery.rs index bde09af706..a03a671c70 100644 --- a/bridges/relays/messages/src/message_race_delivery.rs +++ b/bridges/relays/messages/src/message_race_delivery.rs @@ -654,7 +654,15 @@ async fn select_nonces_for_delivery_transaction( new_selected_unpaid_weight, new_selected_size as u32, ) - .await; + .await + .map_err(|err| { + log::debug!( + target: "bridge", + "Failed to estimate delivery transaction cost: {:?}. No nonces selected for delivery", + err, + ); + }) + .ok()?; // 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 diff --git a/bridges/relays/utils/src/metrics/float_json_value.rs b/bridges/relays/utils/src/metrics/float_json_value.rs index 2a18df2aac..c610ac04dc 100644 --- a/bridges/relays/utils/src/metrics/float_json_value.rs +++ b/bridges/relays/utils/src/metrics/float_json_value.rs @@ -24,6 +24,9 @@ use std::time::Duration; const UPDATE_INTERVAL: Duration = Duration::from_secs(60); /// Metric that represents float value received from HTTP service as float gauge. +/// +/// The float value returned by the service is assumed to be normal (`f64::is_normal` +/// should return `true`) and strictly positive. #[derive(Debug, Clone)] pub struct FloatJsonValueMetric { url: String, @@ -114,6 +117,12 @@ fn parse_service_response(json_path: &str, response: &str) -> Result