Add a linear fee multiplier to ensure safety margins (#127)

* Add fee multiplier

* fmt

* fmt

* Update test mock

* Swap order for parameters to calculate_fee

---------

Co-authored-by: claravanstaden <Cats 4 life!>

bridges/snowbridge/pallets/inbound-queue/src/mock.rs

@@ -188,7 +188,8 @@ parameter_types! {
 	pub Parameters: PricingParameters<u128> = PricingParameters {
 		exchange_rate: FixedU128::from_rational(1, 400),
 		fee_per_gas: gwei(20),
-		rewards: Rewards { local: DOT, remote: meth(1) }
+		rewards: Rewards { local: DOT, remote: meth(1) },
+		multiplier: FixedU128::from_rational(1, 1),
 	};
 }
 

bridges/snowbridge/pallets/outbound-queue/runtime-api/src/lib.rs

@@ -3,18 +3,21 @@
 #![cfg_attr(not(feature = "std"), no_std)]
 
 use frame_support::traits::tokens::Balance as BalanceT;
-use snowbridge_core::outbound::Message;
+use snowbridge_core::{
+	outbound::{Command, Fee},
+	PricingParameters,
+};
 use snowbridge_outbound_queue_merkle_tree::MerkleProof;
 
 sp_api::decl_runtime_apis! {
 	pub trait OutboundQueueApi<Balance> where Balance: BalanceT
 	{
 		/// Generate a merkle proof for a committed message identified by `leaf_index`.
 		/// The merkle root is stored in the block header as a
-		/// `\[`sp_runtime::generic::DigestItem::Other`\]`
+		/// `sp_runtime::generic::DigestItem::Other`
 		fn prove_message(leaf_index: u64) -> Option<MerkleProof>;
 
-		/// Calculate the delivery fee for `message`
-		fn calculate_fee(message: Message) -> Option<Balance>;
+		/// Calculate the delivery fee for `command`
+		fn calculate_fee(command: Command, parameters: Option<PricingParameters<Balance>>) -> Fee<Balance>;
 	}
 }

bridges/snowbridge/pallets/outbound-queue/src/api.rs

@@ -4,8 +4,12 @@
 
 use crate::{Config, MessageLeaves};
 use frame_support::storage::StorageStreamIter;
-use snowbridge_core::outbound::{Message, SendMessage};
+use snowbridge_core::{
+	outbound::{Command, Fee, GasMeter},
+	PricingParameters,
+};
 use snowbridge_outbound_queue_merkle_tree::{merkle_proof, MerkleProof};
+use sp_core::Get;
 
 pub fn prove_message<T>(leaf_index: u64) -> Option<MerkleProof>
 where
@@ -19,12 +23,14 @@ where
 	Some(proof)
 }
 
-pub fn calculate_fee<T>(message: Message) -> Option<T::Balance>
+pub fn calculate_fee<T>(
+	command: Command,
+	parameters: Option<PricingParameters<T::Balance>>,
+) -> Fee<T::Balance>
 where
 	T: Config,
 {
-	match crate::Pallet::<T>::validate(&message) {
-		Ok((_, fees)) => Some(fees.total()),
-		_ => None,
-	}
+	let gas_used_at_most = T::GasMeter::maximum_gas_used_at_most(&command);
+	let parameters = parameters.unwrap_or(T::PricingParameters::get());
+	crate::Pallet::<T>::calculate_fee(gas_used_at_most, parameters)
 }

bridges/snowbridge/pallets/outbound-queue/src/lib.rs

@@ -47,24 +47,37 @@
 //! consume on Ethereum. Using this upper bound, a final fee can be calculated.
 //!
 //! The fee calculation also requires the following parameters:
-//! * ETH/DOT exchange rate
-//! * Ether fee per unit of gas
+//! * Average ETH/DOT exchange rate over some period
+//! * Max fee per unit of gas that bridge is willing to refund relayers for
 //!
 //! By design, it is expected that governance should manually update these
 //! parameters every few weeks using the `set_pricing_parameters` extrinsic in the
 //! system pallet.
 //!
+//! This is an interim measure. Once ETH/DOT liquidity pools are available in the Polkadot network,
+//! we'll use them as a source of pricing info, subject to certain safeguards.
+//!
 //! ## Fee Computation Function
 //!
 //! ```text
 //! LocalFee(Message) = WeightToFee(ProcessMessageWeight(Message))
-//! RemoteFee(Message) = MaxGasRequired(Message) * FeePerGas + Reward
-//! Fee(Message) = LocalFee(Message) + (RemoteFee(Message) / Ratio("ETH/DOT"))
+//! RemoteFee(Message) = MaxGasRequired(Message) * Params.MaxFeePerGas + Params.Reward
+//! RemoteFeeAdjusted(Message) = Params.Multiplier * (RemoteFee(Message) / Params.Ratio("ETH/DOT"))
+//! Fee(Message) = LocalFee(Message) + Params.Multiplier * RemoteFeeAdjusted(Message)
 //! ```
 //!
-//! By design, the computed fee is always going to conservative, to cover worst-case
-//! costs of dispatch on Ethereum. In future iterations of the design, we will optimize
-//! this, or provide a mechanism to asynchronously refund a portion of collected fees.
+//! By design, the computed fee includes a safety factor (the `Multiplier`) to cover
+//! unfavourable fluctuations in the ETH/DOT exchange rate.
+//!
+//! ## Fee Settlement
+//!
+//! On the remote side, in the gateway contract, the relayer accrues
+//!
+//! ```text
+//! Min(GasPrice, Message.MaxFeePerGas) * GasUsed() + Message.Reward
+//! ```
+//! Or in plain english, relayers are refunded for gas consumption, using a
+//! price that is a minimum of the actual gas price, or `Message.MaxFeePerGas``.
 //!
 //! # Extrinsics
 //!
@@ -106,7 +119,7 @@ pub use snowbridge_outbound_queue_merkle_tree::MerkleProof;
 use sp_core::{H256, U256};
 use sp_runtime::{
 	traits::{CheckedDiv, Hash},
-	DigestItem,
+	DigestItem, Saturating,
 };
 use sp_std::prelude::*;
 pub use types::{CommittedMessage, ProcessMessageOriginOf};
@@ -366,8 +379,9 @@ pub mod pallet {
 			// downcast to u128
 			let fee: u128 = fee.try_into().defensive_unwrap_or(u128::MAX);
 
-			// convert to local currency
+			// multiply by multiplier and convert to local currency
 			let fee = FixedU128::from_inner(fee)
+				.saturating_mul(params.multiplier)
 				.checked_div(&params.exchange_rate)
 				.expect("exchange rate is not zero; qed")
 				.into_inner();

bridges/snowbridge/pallets/outbound-queue/src/mock.rs

@@ -87,7 +87,8 @@ parameter_types! {
 	pub Parameters: PricingParameters<u128> = PricingParameters {
 		exchange_rate: FixedU128::from_rational(1, 400),
 		fee_per_gas: gwei(20),
-		rewards: Rewards { local: DOT, remote: meth(1) }
+		rewards: Rewards { local: DOT, remote: meth(1) },
+		multiplier: FixedU128::from_rational(4, 3),
 	};
 }
 

bridges/snowbridge/pallets/outbound-queue/src/test.rs

@@ -268,42 +268,48 @@ fn encode_digest_item() {
 }
 
 #[test]
-fn validate_messages_with_fees() {
+fn test_calculate_fees_with_unit_multiplier() {
 	new_tester().execute_with(|| {
-		let message = mock_message(1000);
-		let (_, fee) = OutboundQueue::validate(&message).unwrap();
+		let gas_used: u64 = 250000;
+		let price_params: PricingParameters<<Test as Config>::Balance> = PricingParameters {
+			exchange_rate: FixedU128::from_rational(1, 400),
+			fee_per_gas: 10000_u32.into(),
+			rewards: Rewards { local: 1_u32.into(), remote: 1_u32.into() },
+			multiplier: FixedU128::from_rational(1, 1),
+		};
+		let fee = OutboundQueue::calculate_fee(gas_used, price_params);
 		assert_eq!(fee.local, 698000000);
-		assert_eq!(fee.remote, 2680000000000);
+		assert_eq!(fee.remote, 1000000);
 	});
 }
 
 #[test]
-fn test_calculate_fees() {
+fn test_calculate_fees_with_multiplier() {
 	new_tester().execute_with(|| {
 		let gas_used: u64 = 250000;
-		let illegal_price_params: PricingParameters<<Test as Config>::Balance> =
-			PricingParameters {
-				exchange_rate: FixedU128::from_rational(1, 400),
-				fee_per_gas: 10000_u32.into(),
-				rewards: Rewards { local: 1_u32.into(), remote: 1_u32.into() },
-			};
-		let fee = OutboundQueue::calculate_fee(gas_used, illegal_price_params);
+		let price_params: PricingParameters<<Test as Config>::Balance> = PricingParameters {
+			exchange_rate: FixedU128::from_rational(1, 400),
+			fee_per_gas: 10000_u32.into(),
+			rewards: Rewards { local: 1_u32.into(), remote: 1_u32.into() },
+			multiplier: FixedU128::from_rational(4, 3),
+		};
+		let fee = OutboundQueue::calculate_fee(gas_used, price_params);
 		assert_eq!(fee.local, 698000000);
-		assert_eq!(fee.remote, 1000000);
+		assert_eq!(fee.remote, 1333333);
 	});
 }
 
 #[test]
 fn test_calculate_fees_with_valid_exchange_rate_but_remote_fee_calculated_as_zero() {
 	new_tester().execute_with(|| {
 		let gas_used: u64 = 250000;
-		let illegal_price_params: PricingParameters<<Test as Config>::Balance> =
-			PricingParameters {
-				exchange_rate: FixedU128::from_rational(1, 1),
-				fee_per_gas: 1_u32.into(),
-				rewards: Rewards { local: 1_u32.into(), remote: 1_u32.into() },
-			};
-		let fee = OutboundQueue::calculate_fee(gas_used, illegal_price_params.clone());
+		let price_params: PricingParameters<<Test as Config>::Balance> = PricingParameters {
+			exchange_rate: FixedU128::from_rational(1, 1),
+			fee_per_gas: 1_u32.into(),
+			rewards: Rewards { local: 1_u32.into(), remote: 1_u32.into() },
+			multiplier: FixedU128::from_rational(1, 1),
+		};
+		let fee = OutboundQueue::calculate_fee(gas_used, price_params.clone());
 		assert_eq!(fee.local, 698000000);
 		// Though none zero pricing params the remote fee calculated here is invalid
 		// which should be avoided

bridges/snowbridge/pallets/system/src/lib.rs

@@ -159,6 +159,7 @@ pub mod pallet {
 		type DefaultPricingParameters: Get<PricingParametersOf<Self>>;
 
 		/// Cost of delivering a message from Ethereum
+		#[pallet::constant]
 		type InboundDeliveryCost: Get<BalanceOf<Self>>;
 
 		type WeightInfo: WeightInfo;
@@ -334,6 +335,7 @@ pub mod pallet {
 			let command = Command::SetPricingParameters {
 				exchange_rate: params.exchange_rate.into(),
 				delivery_cost: T::InboundDeliveryCost::get().saturated_into::<u128>(),
+				multiplier: params.multiplier.into(),
 			};
 			Self::send(PRIMARY_GOVERNANCE_CHANNEL, command, PaysFee::<T>::No)?;
 

bridges/snowbridge/pallets/system/src/mock.rs

@@ -202,7 +202,8 @@ parameter_types! {
 	pub Parameters: PricingParameters<u128> = PricingParameters {
 		exchange_rate: FixedU128::from_rational(1, 400),
 		fee_per_gas: gwei(20),
-		rewards: Rewards { local: DOT, remote: meth(1) }
+		rewards: Rewards { local: DOT, remote: meth(1) },
+		multiplier: FixedU128::from_rational(4, 3)
 	};
 	pub const InboundDeliveryCost: u128 = 1_000_000_000;
 

bridges/snowbridge/primitives/core/src/outbound.rs

@@ -136,6 +136,8 @@ mod v1 {
 			exchange_rate: UD60x18,
 			// Cost of delivering a message from Ethereum to BridgeHub, in ROC/KSM/DOT
 			delivery_cost: u128,
+			// Fee multiplier
+			multiplier: UD60x18,
 		},
 	}
 
@@ -203,10 +205,11 @@ mod v1 {
 					Token::Uint(U256::from(*transfer_asset_xcm)),
 					Token::Uint(*register_token),
 				])]),
-				Command::SetPricingParameters { exchange_rate, delivery_cost } =>
+				Command::SetPricingParameters { exchange_rate, delivery_cost, multiplier } =>
 					ethabi::encode(&[Token::Tuple(vec![
 						Token::Uint(exchange_rate.clone().into_inner()),
 						Token::Uint(U256::from(*delivery_cost)),
+						Token::Uint(multiplier.clone().into_inner()),
 					])]),
 			}
 		}
@@ -273,7 +276,8 @@ mod v1 {
 	}
 }
 
-#[cfg_attr(feature = "std", derive(PartialEq, Debug))]
+#[derive(Clone, Encode, Decode, RuntimeDebug, TypeInfo)]
+#[cfg_attr(feature = "std", derive(PartialEq))]
 /// Fee for delivering message
 pub struct Fee<Balance>
 where
@@ -346,12 +350,13 @@ pub trait GasMeter {
 	/// the command within the message
 	const MAXIMUM_BASE_GAS: u64;
 
+	/// Total gas consumed at most, including verification & dispatch
 	fn maximum_gas_used_at_most(command: &Command) -> u64 {
 		Self::MAXIMUM_BASE_GAS + Self::maximum_dispatch_gas_used_at_most(command)
 	}
 
-	/// Measures the maximum amount of gas a command payload will require to dispatch, AFTER
-	/// validation & verification.
+	/// Measures the maximum amount of gas a command payload will require to *dispatch*, NOT
+	/// including validation & verification.
 	fn maximum_dispatch_gas_used_at_most(command: &Command) -> u64;
 }
 

bridges/snowbridge/primitives/core/src/pricing.rs

@@ -13,6 +13,8 @@ pub struct PricingParameters<Balance> {
 	pub rewards: Rewards<Balance>,
 	/// Ether (wei) fee per gas unit
 	pub fee_per_gas: U256,
+	/// Fee multiplier
+	pub multiplier: FixedU128,
 }
 
 #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug, MaxEncodedLen, TypeInfo)]
@@ -43,6 +45,9 @@ where
 		if self.rewards.remote.is_zero() {
 			return Err(InvalidPricingParameters)
 		}
+		if self.multiplier == FixedU128::zero() {
+			return Err(InvalidPricingParameters)
+		}
 		Ok(())
 	}
 }

cumulus/parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/lib.rs

@@ -38,7 +38,9 @@ pub mod xcm_config;
 use cumulus_pallet_parachain_system::RelayNumberMonotonicallyIncreases;
 use snowbridge_beacon_primitives::{Fork, ForkVersions};
 use snowbridge_core::{
-	gwei, meth, outbound::Message, AgentId, AllowSiblingsOnly, PricingParameters, Rewards,
+	gwei, meth,
+	outbound::{Command, Fee},
+	AgentId, AllowSiblingsOnly, PricingParameters, Rewards,
 };
 use snowbridge_router_primitives::inbound::MessageToXcm;
 use sp_api::impl_runtime_apis;
@@ -503,7 +505,8 @@ parameter_types! {
 	pub Parameters: PricingParameters<u128> = PricingParameters {
 		exchange_rate: FixedU128::from_rational(1, 400),
 		fee_per_gas: gwei(20),
-		rewards: Rewards { local: 1 * UNITS, remote: meth(1) }
+		rewards: Rewards { local: 1 * UNITS, remote: meth(1) },
+		multiplier: FixedU128::from_rational(1, 1),
 	};
 }
 
@@ -1022,8 +1025,8 @@ impl_runtime_apis! {
 			snowbridge_pallet_outbound_queue::api::prove_message::<Runtime>(leaf_index)
 		}
 
-		fn calculate_fee(message: Message) -> Option<Balance> {
-			snowbridge_pallet_outbound_queue::api::calculate_fee::<Runtime>(message)
+		fn calculate_fee(command: Command, parameters: Option<PricingParameters<Balance>>) -> Fee<Balance> {
+			snowbridge_pallet_outbound_queue::api::calculate_fee::<Runtime>(command, parameters)
 		}
 	}