Override conversion rate when computing message fee (paritytech#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

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,24 +37,54 @@ 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<ConversionRateOverride>,
 	/// 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<Self, Self::Err> {
+		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::<Source>().await?;
 			let lane = lane.into();
 			let payload =
 				Source::encode_message(payload).map_err(|e| anyhow::format_err!("{:?}", e))?;
 
-			let fee: BalanceOf<Source> = estimate_message_delivery_and_dispatch_fee(
+			let fee = estimate_message_delivery_and_dispatch_fee::<Source, Target, _>(
 				&source_client,
+				conversion_rate_override,
 				ESTIMATE_MESSAGE_FEE_METHOD,
 				lane,
 				payload,
@@ -67,21 +98,78 @@ impl EstimateFee {
 	}
 }
 
-pub(crate) async fn estimate_message_delivery_and_dispatch_fee<Fee: Decode, C: Chain, P: Encode>(
-	client: &relay_substrate_client::Client<C>,
+pub(crate) async fn estimate_message_delivery_and_dispatch_fee<
+	Source: Chain,
+	Target: Chain,
+	P: Clone + Encode,
+>(
+	client: &relay_substrate_client::Client<Source>,
+	conversion_rate_override: Option<ConversionRateOverride>,
+	estimate_fee_method: &str,
+	lane: bp_messages::LaneId,
+	payload: P,
+) -> anyhow::Result<BalanceOf<Source>> {
+	// 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<Source: Chain, P: Encode>(
+	client: &relay_substrate_client::Client<Source>,
 	estimate_fee_method: &str,
 	lane: bp_messages::LaneId,
 	payload: P,
-) -> anyhow::Result<Fee> {
-	let conversion_rate_override: Option<FixedU128> = None;
+	conversion_rate_override: Option<FixedU128>,
+) -> anyhow::Result<BalanceOf<Source>> {
 	let encoded_response = client
 		.state_call(
 			estimate_fee_method.into(),
 			(lane, payload, conversion_rate_override).encode().into(),
 			None,
 		)
 		.await?;
-	let decoded_response: Option<Fee> = Decode::decode(&mut &encoded_response.0[..])
+	let decoded_response: Option<BalanceOf<Source>> = 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,

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<ConversionRateOverride>,
 	/// Where dispatch fee is paid?
 	#[structopt(
 		long,
@@ -169,11 +175,13 @@ impl SendMessage {
 			let source_sign = self.source_sign.to_keypair::<Source>()?;
 
 			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::<BalanceOf<Source>, _, _>(
+					estimate_message_delivery_and_dispatch_fee::<Source, Target, _>(
 						&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",

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<ConversionRateOverride>,
 }
 
 /// Token swap type.
@@ -133,6 +139,7 @@ impl SwapTokens {
 			let source_sign = self.source_sign.to_keypair::<Target>()?;
 			let target_client = self.target.to_client::<Target>().await?;
 			let target_sign = self.target_sign.to_keypair::<Target>()?;
+			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> =
 				target_sign.public().into();
-			let swap_delivery_and_dispatch_fee: BalanceOf<Source> =
-				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<Target> =
-					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),
 			}
 		);
 	}

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<FloatJsonValueMetric, PrometheusError> {
@@ -27,3 +27,28 @@ pub fn token_price_metric(token_id: &str) -> Result<FloatJsonValueMetric, Promet
 		format!("Rate used to convert from {} to some BASE tokens", token_id.to_uppercase()),
 	)
 }
+
+/// 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,
+) -> 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 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,
+		)),
+	}
+}