From 41a7db195bb5e07e4dd6431eede77e0b8aaab277 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Wed, 2 Mar 2022 17:36:26 +0300 Subject: [PATCH] Override conversion rate when computing message fee (#1261) * override conversion rate when message is sent * spelling + fmt * add --conversion-rate-override cli option * try to read conversion rate from cmd output * fix output * fmt --- .../bin-substrate/src/cli/estimate_fee.rs | 107 ++++++++++++++++-- .../bin-substrate/src/cli/send_message.rs | 18 ++- .../bin-substrate/src/cli/swap_tokens.rs | 33 +++++- .../relays/lib-substrate-relay/src/helpers.rs | 27 ++++- 4 files changed, 167 insertions(+), 18 deletions(-) diff --git a/bridges/relays/bin-substrate/src/cli/estimate_fee.rs b/bridges/relays/bin-substrate/src/cli/estimate_fee.rs index ffa566902bed..375c0aafc81b 100644 --- a/bridges/relays/bin-substrate/src/cli/estimate_fee.rs +++ b/bridges/relays/bin-substrate/src/cli/estimate_fee.rs @@ -24,9 +24,10 @@ 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; /// Estimate Delivery & Dispatch Fee command. -#[derive(StructOpt, Debug, PartialEq, Eq)] +#[derive(StructOpt, Debug, PartialEq)] pub struct EstimateFee { /// A bridge instance to encode call for. #[structopt(possible_values = FullBridge::VARIANTS, case_insensitive = true)] @@ -36,15 +37,44 @@ pub struct EstimateFee { /// Hex-encoded id of lane that will be delivering the message. #[structopt(long, default_value = "00000000")] lane: HexLaneId, + /// A way to override conversion rate between bridge tokens. + /// + /// If not specified, conversion rate from runtime storage is used. It may be obsolete and + /// your message won't be relayed. + #[structopt(long)] + conversion_rate_override: Option, /// Payload to send over the bridge. #[structopt(flatten)] payload: crate::cli::encode_message::MessagePayload, } +/// A way to override conversion rate between bridge tokens. +#[derive(Debug, Clone, Copy, PartialEq)] +pub enum ConversionRateOverride { + /// The actual conversion rate is computed in the same way how rate metric works. + Metric, + /// The actual conversion rate is specified explicitly. + Explicit(f64), +} + +impl std::str::FromStr for ConversionRateOverride { + type Err = String; + + fn from_str(s: &str) -> Result { + if s.to_lowercase() == "metric" { + return Ok(ConversionRateOverride::Metric) + } + + f64::from_str(s) + .map(ConversionRateOverride::Explicit) + .map_err(|e| format!("Failed to parse '{:?}'. Expected 'metric' or explicit value", e)) + } +} + impl EstimateFee { /// Run the command. pub async fn run(self) -> anyhow::Result<()> { - let Self { source, bridge, lane, payload } = self; + let Self { source, bridge, lane, conversion_rate_override, payload } = self; select_full_bridge!(bridge, { let source_client = source.to_client::().await?; @@ -52,8 +82,9 @@ impl EstimateFee { let payload = Source::encode_message(payload).map_err(|e| anyhow::format_err!("{:?}", e))?; - let fee: BalanceOf = estimate_message_delivery_and_dispatch_fee( + let fee = estimate_message_delivery_and_dispatch_fee::( &source_client, + conversion_rate_override, ESTIMATE_MESSAGE_FEE_METHOD, lane, payload, @@ -67,13 +98,70 @@ impl EstimateFee { } } -pub(crate) async fn estimate_message_delivery_and_dispatch_fee( - client: &relay_substrate_client::Client, +pub(crate) async fn estimate_message_delivery_and_dispatch_fee< + Source: Chain, + Target: Chain, + P: Clone + Encode, +>( + client: &relay_substrate_client::Client, + conversion_rate_override: Option, + estimate_fee_method: &str, + lane: bp_messages::LaneId, + payload: P, +) -> anyhow::Result> { + // actual conversion rate CAN be lesser than the rate stored in the runtime. So we may try to + // pay lesser fee for the message delivery. But in this case, message may be rejected by the + // lane. So we MUST use the larger of two fees - one computed with stored fee and the one + // computed with actual fee. + + let conversion_rate_override = match ( + conversion_rate_override, + Source::TOKEN_ID, + Target::TOKEN_ID, + ) { + (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()); + 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?, + ); + log::info!(target: "bridge", "Conversion rate override: {:?} (from metric)", conversion_rate_override.to_float()); + Some(conversion_rate_override) + }, + _ => None, + }; + + Ok(std::cmp::max( + do_estimate_message_delivery_and_dispatch_fee( + client, + estimate_fee_method, + lane, + payload.clone(), + None, + ) + .await?, + do_estimate_message_delivery_and_dispatch_fee( + client, + estimate_fee_method, + lane, + payload.clone(), + conversion_rate_override, + ) + .await?, + )) +} + +/// Estimate message delivery and dispatch fee with given conversion rate override. +async fn do_estimate_message_delivery_and_dispatch_fee( + client: &relay_substrate_client::Client, estimate_fee_method: &str, lane: bp_messages::LaneId, payload: P, -) -> anyhow::Result { - let conversion_rate_override: Option = None; + conversion_rate_override: Option, +) -> anyhow::Result> { let encoded_response = client .state_call( estimate_fee_method.into(), @@ -81,7 +169,7 @@ pub(crate) async fn estimate_message_delivery_and_dispatch_fee = Decode::decode(&mut &encoded_response.0[..]) + let decoded_response: Option> = Decode::decode(&mut &encoded_response.0[..]) .map_err(relay_substrate_client::Error::ResponseParseFailed)?; let fee = decoded_response.ok_or_else(|| { anyhow::format_err!("Unable to decode fee from: {:?}", HexBytes(encoded_response.to_vec())) @@ -106,6 +194,8 @@ mod tests { "rialto-to-millau", "--source-port", "1234", + "--conversion-rate-override", + "42.5", "call", "--sender", &alice, @@ -122,6 +212,7 @@ mod tests { EstimateFee { bridge: FullBridge::RialtoToMillau, lane: HexLaneId([0, 0, 0, 0]), + conversion_rate_override: Some(ConversionRateOverride::Explicit(42.5)), source: SourceConnectionParams { source_host: "127.0.0.1".into(), source_port: 1234, diff --git a/bridges/relays/bin-substrate/src/cli/send_message.rs b/bridges/relays/bin-substrate/src/cli/send_message.rs index 42b2886b7cd5..b1f9ac7a42ac 100644 --- a/bridges/relays/bin-substrate/src/cli/send_message.rs +++ b/bridges/relays/bin-substrate/src/cli/send_message.rs @@ -17,12 +17,12 @@ use crate::cli::{ bridge::FullBridge, encode_call::{self, CliEncodeCall}, - estimate_fee::estimate_message_delivery_and_dispatch_fee, + estimate_fee::{estimate_message_delivery_and_dispatch_fee, ConversionRateOverride}, Balance, ExplicitOrMaximal, HexBytes, HexLaneId, Origins, SourceConnectionParams, SourceSigningParams, TargetConnectionParams, TargetSigningParams, }; use bp_message_dispatch::{CallOrigin, MessagePayload}; -use bp_runtime::{BalanceOf, Chain as _}; +use bp_runtime::Chain as _; use codec::Encode; use frame_support::weights::Weight; use relay_substrate_client::{Chain, SignParam, TransactionSignScheme, UnsignedTransaction}; @@ -66,6 +66,12 @@ pub struct SendMessage { /// Hex-encoded lane id. Defaults to `00000000`. #[structopt(long, default_value = "00000000")] lane: HexLaneId, + /// A way to override conversion rate between bridge tokens. + /// + /// If not specified, conversion rate from runtime storage is used. It may be obsolete and + /// your message won't be relayed. + #[structopt(long)] + conversion_rate_override: Option, /// Where dispatch fee is paid? #[structopt( long, @@ -169,11 +175,13 @@ impl SendMessage { let source_sign = self.source_sign.to_keypair::()?; let lane = self.lane.clone().into(); + let conversion_rate_override = self.conversion_rate_override; let fee = match self.fee { Some(fee) => fee, None => Balance( - estimate_message_delivery_and_dispatch_fee::, _, _>( + estimate_message_delivery_and_dispatch_fee::( &source_client, + conversion_rate_override, ESTIMATE_MESSAGE_FEE_METHOD, lane, payload.clone(), @@ -317,6 +325,8 @@ mod tests { "1234", "--source-signer", "//Alice", + "--conversion-rate-override", + "0.75", "remark", "--remark-payload", "1234", @@ -354,6 +364,8 @@ mod tests { "Target", "--target-signer", "//Bob", + "--conversion-rate-override", + "metric", "remark", "--remark-payload", "1234", diff --git a/bridges/relays/bin-substrate/src/cli/swap_tokens.rs b/bridges/relays/bin-substrate/src/cli/swap_tokens.rs index c4b80ec66b9c..b93b96daa238 100644 --- a/bridges/relays/bin-substrate/src/cli/swap_tokens.rs +++ b/bridges/relays/bin-substrate/src/cli/swap_tokens.rs @@ -36,8 +36,8 @@ use sp_core::{blake2_256, storage::StorageKey, Bytes, Pair, U256}; use sp_runtime::traits::{Convert, Header as HeaderT}; use crate::cli::{ - Balance, CliChain, SourceConnectionParams, SourceSigningParams, TargetConnectionParams, - TargetSigningParams, + estimate_fee::ConversionRateOverride, Balance, CliChain, SourceConnectionParams, + SourceSigningParams, TargetConnectionParams, TargetSigningParams, }; /// Swap tokens. @@ -65,6 +65,12 @@ pub struct SwapTokens { /// Target chain balance that target signer wants to swap. #[structopt(long)] target_balance: Balance, + /// A way to override conversion rate between bridge tokens. + /// + /// If not specified, conversion rate from runtime storage is used. It may be obsolete and + /// your message won't be relayed. + #[structopt(long)] + conversion_rate_override: Option, } /// Token swap type. @@ -133,6 +139,7 @@ impl SwapTokens { let source_sign = self.source_sign.to_keypair::()?; let target_client = self.target.to_client::().await?; let target_sign = self.target_sign.to_keypair::()?; + let conversion_rate_override = self.conversion_rate_override; // names of variables in this function are matching names used by the // `pallet-bridge-token-swap` @@ -198,9 +205,14 @@ impl SwapTokens { // prepare `create_swap` call let target_public_at_bridged_chain: AccountPublicOf = target_sign.public().into(); - let swap_delivery_and_dispatch_fee: BalanceOf = - crate::cli::estimate_fee::estimate_message_delivery_and_dispatch_fee( + let swap_delivery_and_dispatch_fee = + crate::cli::estimate_fee::estimate_message_delivery_and_dispatch_fee::< + Source, + Target, + _, + >( &source_client, + conversion_rate_override.clone(), ESTIMATE_SOURCE_TO_TARGET_MESSAGE_FEE_METHOD, SOURCE_TO_TARGET_LANE_ID, bp_message_dispatch::MessagePayload { @@ -356,9 +368,14 @@ impl SwapTokens { dispatch_fee_payment: bp_runtime::messages::DispatchFeePayment::AtSourceChain, call: claim_swap_call.encode(), }; - let claim_swap_delivery_and_dispatch_fee: BalanceOf = - crate::cli::estimate_fee::estimate_message_delivery_and_dispatch_fee( + let claim_swap_delivery_and_dispatch_fee = + crate::cli::estimate_fee::estimate_message_delivery_and_dispatch_fee::< + Target, + Source, + _, + >( &target_client, + conversion_rate_override.clone(), ESTIMATE_TARGET_TO_SOURCE_MESSAGE_FEE_METHOD, TARGET_TO_SOURCE_LANE_ID, claim_swap_message.clone(), @@ -753,6 +770,7 @@ mod tests { swap_type: TokenSwapType::NoLock, source_balance: Balance(8000000000), target_balance: Balance(9000000000), + conversion_rate_override: None, } ); } @@ -778,6 +796,8 @@ mod tests { "//Bob", "--target-balance", "9000000000", + "--conversion-rate-override", + "metric", "lock-until-block", "--blocks-before-expire", "1", @@ -827,6 +847,7 @@ mod tests { }, source_balance: Balance(8000000000), target_balance: Balance(9000000000), + conversion_rate_override: Some(ConversionRateOverride::Metric), } ); } diff --git a/bridges/relays/lib-substrate-relay/src/helpers.rs b/bridges/relays/lib-substrate-relay/src/helpers.rs index 6f30a5d468ee..1a9abce67439 100644 --- a/bridges/relays/lib-substrate-relay/src/helpers.rs +++ b/bridges/relays/lib-substrate-relay/src/helpers.rs @@ -16,7 +16,7 @@ //! Substrate relay helpers -use relay_utils::metrics::{FloatJsonValueMetric, PrometheusError}; +use relay_utils::metrics::{FloatJsonValueMetric, PrometheusError, StandaloneMetric}; /// Creates standalone token price metric. pub fn token_price_metric(token_id: &str) -> Result { @@ -27,3 +27,28 @@ pub fn token_price_metric(token_id: &str) -> Result 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 source_token_value = *source_token_metric.shared_value_ref().read().await; + let target_token_value = *target_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), + _ => Err(anyhow::format_err!( + "Failed to compute conversion rate from {} to {}", + target_token_id, + source_token_id, + )), + } +}