fixed mess with conversion rates (#1338)

Author: svyatonik

bridges/relays/bin-substrate/src/cli/estimate_fee.rs

@@ -24,7 +24,7 @@ use relay_substrate_client::Chain;
 use sp_runtime::FixedU128;
 use structopt::StructOpt;
 use strum::VariantNames;
-use substrate_relay_helper::helpers::target_to_source_conversion_rate;
+use substrate_relay_helper::helpers::tokens_conversion_rate_from_metrics;
 
 /// Estimate Delivery & Dispatch Fee command.
 #[derive(StructOpt, Debug, PartialEq)]
@@ -98,6 +98,8 @@ impl EstimateFee {
 	}
 }
 
+/// The caller may provide target to source tokens conversion rate override to use in fee
+/// computation.
 pub(crate) async fn estimate_message_delivery_and_dispatch_fee<
 	Source: Chain,
 	Target: Chain,
@@ -121,14 +123,14 @@ pub(crate) async fn estimate_message_delivery_and_dispatch_fee<
 	) {
 		(Some(ConversionRateOverride::Explicit(v)), _, _) => {
 			let conversion_rate_override = FixedU128::from_float(v);
-			log::info!(target: "bridge", "Conversion rate override: {:?} (explicit)", conversion_rate_override.to_float());
+			log::info!(target: "bridge", "{} -> {} conversion rate override: {:?} (explicit)", Target::NAME, Source::NAME, conversion_rate_override.to_float());
 			Some(conversion_rate_override)
 		},
 		(Some(ConversionRateOverride::Metric), Some(source_token_id), Some(target_token_id)) => {
 			let conversion_rate_override = FixedU128::from_float(
-				target_to_source_conversion_rate(source_token_id, target_token_id).await?,
+				tokens_conversion_rate_from_metrics(target_token_id, source_token_id).await?,
 			);
-			log::info!(target: "bridge", "Conversion rate override: {:?} (from metric)", conversion_rate_override.to_float());
+			log::info!(target: "bridge", "{} -> {} conversion rate override: {:?} (from metric)", Target::NAME, Source::NAME, conversion_rate_override.to_float());
 			Some(conversion_rate_override)
 		},
 		_ => None,

bridges/relays/bin-substrate/src/cli/relay_headers_and_messages.rs

@@ -403,13 +403,13 @@ impl RelayHeadersAndMessages {
 						.as_ref()
 						.ok_or_else(format_err)?
 						.shared_value_ref(),
-					left_to_right_metrics
-						.source_to_base_conversion_rate
+					right_to_left_metrics
+						.target_to_base_conversion_rate
 						.as_ref()
 						.ok_or_else(format_err)?
 						.shared_value_ref(),
-					left_to_right_metrics
-						.target_to_base_conversion_rate
+					right_to_left_metrics
+						.source_to_base_conversion_rate
 						.as_ref()
 						.ok_or_else(format_err)?
 						.shared_value_ref(),

bridges/relays/lib-substrate-relay/src/conversion_rate_update.rs

@@ -106,7 +106,7 @@ async fn maybe_select_new_conversion_rate(
 	let left_to_base_conversion_rate = (*left_to_base_conversion_rate.read().await)?;
 	let right_to_base_conversion_rate = (*right_to_base_conversion_rate.read().await)?;
 	let actual_left_to_right_conversion_rate =
-		right_to_base_conversion_rate / left_to_base_conversion_rate;
+		left_to_base_conversion_rate / right_to_base_conversion_rate;
 
 	let rate_difference =
 		(actual_left_to_right_conversion_rate - left_to_right_stored_conversion_rate).abs();
@@ -229,15 +229,27 @@ mod tests {
 
 	#[test]
 	fn transaction_is_submitted_when_difference_is_above_threshold() {
+		let left_to_right_stored_conversion_rate = 1.0;
+		let left_to_base_conversion_rate = 18f64;
+		let right_to_base_conversion_rate = 180f64;
+
+		assert!(left_to_base_conversion_rate < right_to_base_conversion_rate);
+
 		assert_eq!(
 			test_maybe_select_new_conversion_rate(
 				TransactionStatus::Idle,
-				Some(1.0),
-				Some(1.0),
-				Some(1.03),
+				Some(left_to_right_stored_conversion_rate),
+				Some(left_to_base_conversion_rate),
+				Some(right_to_base_conversion_rate),
 				0.02
 			),
-			(Some((1.0, 1.03)), TransactionStatus::Idle),
+			(
+				Some((
+					left_to_right_stored_conversion_rate,
+					left_to_base_conversion_rate / right_to_base_conversion_rate,
+				)),
+				TransactionStatus::Idle
+			),
 		);
 	}
 }

bridges/relays/lib-substrate-relay/src/helpers.rs

@@ -29,26 +29,78 @@ pub fn token_price_metric(token_id: &str) -> Result<FloatJsonValueMetric, Promet
 }
 
 /// Compute conversion rate between two tokens immediately, without spawning any metrics.
-pub async fn target_to_source_conversion_rate(
-	source_token_id: &str,
-	target_token_id: &str,
+///
+/// Returned rate may be used in expression: `from_tokens * rate -> to_tokens`.
+pub async fn tokens_conversion_rate_from_metrics(
+	from_token_id: &str,
+	to_token_id: &str,
 ) -> anyhow::Result<f64> {
-	let source_token_metric = token_price_metric(source_token_id)?;
-	source_token_metric.update().await;
-	let target_token_metric = token_price_metric(target_token_id)?;
-	target_token_metric.update().await;
+	let from_token_metric = token_price_metric(from_token_id)?;
+	from_token_metric.update().await;
+	let to_token_metric = token_price_metric(to_token_id)?;
+	to_token_metric.update().await;
 
-	let source_token_value = *source_token_metric.shared_value_ref().read().await;
-	let target_token_value = *target_token_metric.shared_value_ref().read().await;
+	let from_token_value = *from_token_metric.shared_value_ref().read().await;
+	let to_token_value = *to_token_metric.shared_value_ref().read().await;
 	// `FloatJsonValueMetric` guarantees that the value is positive && normal, so no additional
 	// checks required here
-	match (source_token_value, target_token_value) {
-		(Some(source_token_value), Some(target_token_value)) =>
-			Ok(target_token_value / source_token_value),
+	match (from_token_value, to_token_value) {
+		(Some(from_token_value), Some(to_token_value)) =>
+			Ok(tokens_conversion_rate(from_token_value, to_token_value)),
 		_ => Err(anyhow::format_err!(
 			"Failed to compute conversion rate from {} to {}",
-			target_token_id,
-			source_token_id,
+			from_token_id,
+			to_token_id,
 		)),
 	}
 }
+
+/// Compute conversion rate between two tokens, given token prices.
+///
+/// Returned rate may be used in expression: `from_tokens * rate -> to_tokens`.
+///
+/// Both prices are assumed to be normal and non-negative.
+pub fn tokens_conversion_rate(from_token_value: f64, to_token_value: f64) -> f64 {
+	from_token_value / to_token_value
+}
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+
+	#[test]
+	fn rialto_to_millau_conversion_rate_is_correct() {
+		let rialto_price = 18.18;
+		let millau_price = 136.35;
+		assert!(rialto_price < millau_price);
+
+		let conversion_rate = tokens_conversion_rate(rialto_price, millau_price);
+		let rialto_amount = 100.0;
+		let millau_amount = rialto_amount * conversion_rate;
+		assert!(
+			rialto_amount > millau_amount,
+			"{} RLT * {} = {} MLU",
+			rialto_amount,
+			conversion_rate,
+			millau_amount,
+		);
+	}
+
+	#[test]
+	fn millau_to_rialto_conversion_rate_is_correct() {
+		let rialto_price = 18.18;
+		let millau_price = 136.35;
+		assert!(rialto_price < millau_price);
+
+		let conversion_rate = tokens_conversion_rate(millau_price, rialto_price);
+		let millau_amount = 100.0;
+		let rialto_amount = millau_amount * conversion_rate;
+		assert!(
+			rialto_amount > millau_amount,
+			"{} MLU * {} = {} RLT",
+			millau_amount,
+			conversion_rate,
+			rialto_amount,
+		);
+	}
+}

bridges/relays/lib-substrate-relay/src/messages_metrics.rs

@@ -16,7 +16,7 @@
 
 //! Tools for supporting message lanes between two Substrate-based chains.
 
-use crate::messages_lane::SubstrateMessageLane;
+use crate::{helpers::tokens_conversion_rate, messages_lane::SubstrateMessageLane};
 
 use codec::Decode;
 use frame_system::AccountInfo;
@@ -119,19 +119,11 @@ impl<SC: Chain, TC: Chain> StandaloneMessagesMetrics<SC, TC> {
 
 	/// Return conversion rate from target to source tokens.
 	pub async fn target_to_source_conversion_rate(&self) -> Option<f64> {
-		Self::compute_target_to_source_conversion_rate(
-			*self.target_to_base_conversion_rate.as_ref()?.shared_value_ref().read().await,
-			*self.source_to_base_conversion_rate.as_ref()?.shared_value_ref().read().await,
-		)
-	}
-
-	/// Return conversion rate from target to source tokens, given conversion rates from
-	/// target/source tokens to some base token.
-	fn compute_target_to_source_conversion_rate(
-		target_to_base_conversion_rate: Option<f64>,
-		source_to_base_conversion_rate: Option<f64>,
-	) -> Option<f64> {
-		Some(source_to_base_conversion_rate? / target_to_base_conversion_rate?)
+		let from_token_value =
+			(*self.target_to_base_conversion_rate.as_ref()?.shared_value_ref().read().await)?;
+		let to_token_value =
+			(*self.source_to_base_conversion_rate.as_ref()?.shared_value_ref().read().await)?;
+		Some(tokens_conversion_rate(from_token_value, to_token_value))
 	}
 }
 
@@ -379,14 +371,6 @@ mod tests {
 	use frame_support::storage::generator::StorageValue;
 	use sp_core::storage::StorageKey;
 
-	#[async_std::test]
-	async fn target_to_source_conversion_rate_works() {
-		assert_eq!(
-			StandaloneMessagesMetrics::<relay_rococo_client::Rococo, relay_wococo_client::Wococo>::compute_target_to_source_conversion_rate(Some(183.15), Some(12.32)),
-			Some(12.32 / 183.15),
-		);
-	}
-
 	#[test]
 	fn token_decimals_used_properly() {
 		let plancks = 425_000_000_000;