From 833c4744c22fd2d09764961af760c6993e0474a9 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Thu, 3 Mar 2022 14:46:07 +0300 Subject: [PATCH] fixed mess with conversion rates (#1338) --- .../bin-substrate/src/cli/estimate_fee.rs | 10 ++- .../src/cli/relay_headers_and_messages.rs | 8 +- .../src/conversion_rate_update.rs | 22 +++-- .../relays/lib-substrate-relay/src/helpers.rs | 80 +++++++++++++++---- .../src/messages_metrics.rs | 28 ++----- 5 files changed, 99 insertions(+), 49 deletions(-) diff --git a/bridges/relays/bin-substrate/src/cli/estimate_fee.rs b/bridges/relays/bin-substrate/src/cli/estimate_fee.rs index 375c0aafc81b7..1f3e560fc0d03 100644 --- a/bridges/relays/bin-substrate/src/cli/estimate_fee.rs +++ b/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, diff --git a/bridges/relays/bin-substrate/src/cli/relay_headers_and_messages.rs b/bridges/relays/bin-substrate/src/cli/relay_headers_and_messages.rs index 47336000338f8..86b6da39778db 100644 --- a/bridges/relays/bin-substrate/src/cli/relay_headers_and_messages.rs +++ b/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(), diff --git a/bridges/relays/lib-substrate-relay/src/conversion_rate_update.rs b/bridges/relays/lib-substrate-relay/src/conversion_rate_update.rs index 93458457d34c9..482e6e4c86166 100644 --- a/bridges/relays/lib-substrate-relay/src/conversion_rate_update.rs +++ b/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 + ), ); } } diff --git a/bridges/relays/lib-substrate-relay/src/helpers.rs b/bridges/relays/lib-substrate-relay/src/helpers.rs index 1a9abce67439b..80359b1c2a980 100644 --- a/bridges/relays/lib-substrate-relay/src/helpers.rs +++ b/bridges/relays/lib-substrate-relay/src/helpers.rs @@ -29,26 +29,78 @@ pub fn token_price_metric(token_id: &str) -> Result to_tokens`. +pub async fn tokens_conversion_rate_from_metrics( + from_token_id: &str, + to_token_id: &str, ) -> anyhow::Result { - 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, + ); + } +} diff --git a/bridges/relays/lib-substrate-relay/src/messages_metrics.rs b/bridges/relays/lib-substrate-relay/src/messages_metrics.rs index 9e303cebc1d7b..b165892fda1dc 100644 --- a/bridges/relays/lib-substrate-relay/src/messages_metrics.rs +++ b/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 StandaloneMessagesMetrics { /// Return conversion rate from target to source tokens. pub async fn target_to_source_conversion_rate(&self) -> Option { - 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, - source_to_base_conversion_rate: Option, - ) -> Option { - 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::::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;