Use real conversion rate in greedy relayer strategy (#1035)

* use real conversion rate in greedy relayer strategy

* only accept positive, normal numbers in FloatJsonValueMetric

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,

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,

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,

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,

bridges/relays/bin-substrate/src/messages_lane.rs

@@ -201,6 +201,15 @@ pub struct StandaloneMessagesMetrics {
 	pub source_to_base_conversion_rate: Option<F64SharedRef>,
 }
 
+impl StandaloneMessagesMetrics {
+	/// Return conversion rate from target to source tokens.
+	pub async fn target_to_source_conversion_rate(&self) -> Option<f64> {
+		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<P: SubstrateMessageLane>(
 	metrics_prefix: Option<String>,

bridges/relays/bin-substrate/src/messages_target.rs

@@ -18,7 +18,7 @@
 //! runtime that implements `<BridgedChainName>HeaderApi` to allow bridging with
 //! <BridgedName> 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<SC: Chain, TC: Chain, P: SubstrateMessageLane
 	lane: P,
 	lane_id: LaneId,
 	instance: ChainId,
+	metric_values: StandaloneMessagesMetrics,
 	source_to_target_headers_relay: Option<OnDemandHeadersRelay<SC>>,
 	_phantom: PhantomData<I>,
 }
@@ -64,6 +65,7 @@ impl<SC: Chain, TC: Chain, P: SubstrateMessageLane, I> SubstrateMessagesTarget<S
 		lane: P,
 		lane_id: LaneId,
 		instance: ChainId,
+		metric_values: StandaloneMessagesMetrics,
 		source_to_target_headers_relay: Option<OnDemandHeadersRelay<SC>>,
 	) -> Self {
 		SubstrateMessagesTarget {
@@ -71,6 +73,7 @@ impl<SC: Chain, TC: Chain, P: SubstrateMessageLane, I> SubstrateMessagesTarget<S
 			lane,
 			lane_id,
 			instance,
+			metric_values,
 			source_to_target_headers_relay,
 			_phantom: Default::default(),
 		}
@@ -84,6 +87,7 @@ impl<SC: Chain, TC: Chain, P: SubstrateMessageLane, I> 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<MessageNonce>,
 		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::<SC, TC>(
-			FixedU128::one(),
+	) -> Result<P::SourceChainBalance, SubstrateError> {
+		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::<SC, TC>(
+			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()),
-		)
+		))
 	}
 }
 

bridges/relays/messages/src/message_lane_loop.rs

@@ -212,7 +212,7 @@ pub trait TargetClient<P: MessageLane>: RelayClient {
 		nonces: RangeInclusive<MessageNonce>,
 		total_dispatch_weight: Weight,
 		total_size: u32,
-	) -> P::SourceChainBalance;
+	) -> Result<P::SourceChainBalance, Self::Error>;
 }
 
 /// State of the client.
@@ -775,10 +775,12 @@ pub(crate) mod tests {
 			nonces: RangeInclusive<MessageNonce>,
 			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<TestSourceChainBalance, TestError> {
+			Ok(
+				BASE_MESSAGE_DELIVERY_TRANSACTION_COST * (nonces.end() - nonces.start() + 1)
+					+ total_dispatch_weight
+					+ total_size as TestSourceChainBalance,
+			)
 		}
 	}
 

bridges/relays/messages/src/message_race_delivery.rs

@@ -654,7 +654,15 @@ async fn select_nonces_for_delivery_transaction<P: MessageLane>(
 						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

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<f64, String
 		.first()
 		.and_then(|v| v.as_f64())
 		.ok_or_else(|| format!("Missing required value from response: {:?}", response,))?;
+	if !selected_value.is_normal() || selected_value < 0.0 {
+		return Err(format!(
+			"Failed to parse float value {:?} from response. It is assumed to be positive and normal",
+			selected_value,
+		));
+	}
 
 	Ok(selected_value)
 }
@@ -129,4 +138,19 @@ mod tests {
 			Ok(433.05),
 		);
 	}
+
+	#[test]
+	fn parse_service_response_rejects_negative_numbers() {
+		assert!(parse_service_response("$.kusama.usd", r#"{"kusama":{"usd":-433.05}}"#).is_err());
+	}
+
+	#[test]
+	fn parse_service_response_rejects_zero_numbers() {
+		assert!(parse_service_response("$.kusama.usd", r#"{"kusama":{"usd":0.0}}"#).is_err());
+	}
+
+	#[test]
+	fn parse_service_response_rejects_nan() {
+		assert!(parse_service_response("$.kusama.usd", r#"{"kusama":{"usd":NaN}}"#).is_err());
+	}
 }