Add weight of refund extension post_dispatch to the weights of messages pallet (paritytech#2089)

* add weight of refund extension post_dispatch to the weights of messages pallet

* fixed tests and removed TODO

* verify runtime overhead weight in integrity tests

* add integrity tests to rialto-parachain runtime

* refactor weights a bit

* refund ext is disabled for Rialto <> Millau bridge

* Update bin/runtime-common/src/integrity.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update bin/runtime-common/src/integrity.rs

Co-authored-by: Adrian Catangiu <[email protected]>

* Update modules/relayers/src/weights_ext.rs

Co-authored-by: Adrian Catangiu <[email protected]>

---------

Co-authored-by: Adrian Catangiu <[email protected]>

Author: svyatonik

bridges/bin/millau/runtime/src/lib.rs

@@ -599,7 +599,7 @@ generate_bridge_reject_obsolete_headers_and_messages! {
 
 bp_runtime::generate_static_str_provider!(BridgeRefundRialtoPara2000Lane0Msgs);
 /// Signed extension that refunds relayers that are delivering messages from the Rialto parachain.
-pub type PriorityBoostPerMessage = ConstU64<699_683_285>;
+pub type PriorityBoostPerMessage = ConstU64<324_316_715>;
 pub type BridgeRefundRialtoParachainMessages = RefundBridgedParachainMessages<
 	Runtime,
 	RefundableParachain<WithRialtoParachainsInstance, RialtoParachainId>,

bridges/bin/millau/runtime/src/rialto_messages.rs

@@ -26,6 +26,7 @@ use bridge_runtime_common::{
 	messages_xcm_extension::{XcmBlobHauler, XcmBlobHaulerAdapter},
 };
 use frame_support::{parameter_types, weights::Weight, RuntimeDebug};
+use pallet_bridge_relayers::WeightInfoExt as _;
 use xcm::latest::prelude::*;
 use xcm_builder::HaulBlobExporter;
 
@@ -139,6 +140,14 @@ impl pallet_bridge_messages::WeightInfoExt for crate::weights::RialtoMessagesWei
 	fn expected_extra_storage_proof_size() -> u32 {
 		bp_rialto::EXTRA_STORAGE_PROOF_SIZE
 	}
+
+	fn receive_messages_proof_overhead_from_runtime() -> Weight {
+		pallet_bridge_relayers::weights::BridgeWeight::<Runtime>::receive_messages_proof_overhead_from_runtime()
+	}
+
+	fn receive_messages_delivery_proof_overhead_from_runtime() -> Weight {
+		pallet_bridge_relayers::weights::BridgeWeight::<Runtime>::receive_messages_delivery_proof_overhead_from_runtime()
+	}
 }
 
 #[cfg(test)]
@@ -161,6 +170,7 @@ mod tests {
 			bp_rialto::EXTRA_STORAGE_PROOF_SIZE,
 			bp_millau::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX,
 			bp_millau::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX,
+			false,
 		);
 	}
 

bridges/bin/millau/runtime/src/rialto_parachain_messages.rs

@@ -28,6 +28,7 @@ use bridge_runtime_common::{
 	messages_xcm_extension::{XcmBlobHauler, XcmBlobHaulerAdapter},
 };
 use frame_support::{parameter_types, weights::Weight, RuntimeDebug};
+use pallet_bridge_relayers::WeightInfoExt as _;
 use xcm::latest::prelude::*;
 use xcm_builder::HaulBlobExporter;
 
@@ -142,6 +143,14 @@ impl pallet_bridge_messages::WeightInfoExt
 	fn expected_extra_storage_proof_size() -> u32 {
 		bp_rialto_parachain::EXTRA_STORAGE_PROOF_SIZE
 	}
+
+	fn receive_messages_proof_overhead_from_runtime() -> Weight {
+		pallet_bridge_relayers::weights::BridgeWeight::<Runtime>::receive_messages_proof_overhead_from_runtime()
+	}
+
+	fn receive_messages_delivery_proof_overhead_from_runtime() -> Weight {
+		pallet_bridge_relayers::weights::BridgeWeight::<Runtime>::receive_messages_delivery_proof_overhead_from_runtime()
+	}
 }
 
 #[cfg(test)]
@@ -167,6 +176,7 @@ mod tests {
 			bp_rialto_parachain::EXTRA_STORAGE_PROOF_SIZE,
 			bp_millau::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX,
 			bp_millau::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX,
+			true,
 		);
 	}
 

bridges/bin/rialto-parachain/runtime/src/millau_messages.rs

@@ -154,6 +154,7 @@ mod tests {
 			bp_millau::EXTRA_STORAGE_PROOF_SIZE,
 			bp_rialto_parachain::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX,
 			bp_rialto_parachain::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX,
+			false,
 		);
 	}
 

bridges/bin/rialto/runtime/src/millau_messages.rs

@@ -154,6 +154,7 @@ mod tests {
 			bp_millau::EXTRA_STORAGE_PROOF_SIZE,
 			bp_rialto::MAX_UNREWARDED_RELAYERS_IN_CONFIRMATION_TX,
 			bp_rialto::MAX_UNCONFIRMED_MESSAGES_IN_CONFIRMATION_TX,
+			false,
 		);
 	}
 

bridges/bin/runtime-common/src/integrity.rs

@@ -24,8 +24,9 @@ use crate::{messages, messages::MessageBridge};
 use bp_messages::{InboundLaneData, MessageNonce};
 use bp_runtime::{Chain, ChainId};
 use codec::Encode;
-use frame_support::{storage::generator::StorageValue, traits::Get};
+use frame_support::{storage::generator::StorageValue, traits::Get, weights::Weight};
 use frame_system::limits;
+use pallet_bridge_messages::WeightInfoExt as _;
 use sp_runtime::traits::SignedExtension;
 
 /// Macro that ensures that the runtime configuration and chain primitives crate are sharing
@@ -289,15 +290,25 @@ where
 }
 
 /// Check that the message lane weights are correct.
-pub fn check_message_lane_weights<C: Chain, T: frame_system::Config>(
+pub fn check_message_lane_weights<
+	C: Chain,
+	T: frame_system::Config + pallet_bridge_messages::Config,
+>(
 	bridged_chain_extra_storage_proof_size: u32,
 	this_chain_max_unrewarded_relayers: MessageNonce,
 	this_chain_max_unconfirmed_messages: MessageNonce,
+	// whether `RefundBridgedParachainMessages` extension is deployed at runtime and is used for
+	// refunding this bridge transactions?
+	//
+	// in other words: pass true for all known production chains
+	runtime_includes_refund_extension: bool,
 ) {
-	type Weights<T> = pallet_bridge_messages::weights::BridgeWeight<T>;
+	type Weights<T> = <T as pallet_bridge_messages::Config>::WeightInfo;
 
+	// check basic weight assumptions
 	pallet_bridge_messages::ensure_weights_are_correct::<Weights<T>>();
 
+	// check that weights allow us to receive messages
 	let max_incoming_message_proof_size = bridged_chain_extra_storage_proof_size
 		.saturating_add(messages::target::maximal_incoming_message_size(C::max_extrinsic_size()));
 	pallet_bridge_messages::ensure_able_to_receive_message::<Weights<T>>(
@@ -307,6 +318,7 @@ pub fn check_message_lane_weights<C: Chain, T: frame_system::Config>(
 		messages::target::maximal_incoming_message_dispatch_weight(C::max_extrinsic_weight()),
 	);
 
+	// check that weights allow us to receive delivery confirmations
 	let max_incoming_inbound_lane_data_proof_size =
 		InboundLaneData::<()>::encoded_size_hint_u32(this_chain_max_unrewarded_relayers as _);
 	pallet_bridge_messages::ensure_able_to_receive_confirmation::<Weights<T>>(
@@ -316,6 +328,20 @@ pub fn check_message_lane_weights<C: Chain, T: frame_system::Config>(
 		this_chain_max_unrewarded_relayers,
 		this_chain_max_unconfirmed_messages,
 	);
+
+	// check that extra weights of delivery/confirmation transactions include the weight
+	// of `RefundBridgedParachainMessages` operations. This signed extension assumes the worst case
+	// (i.e. slashing if delivery transaction was invalid) and refunds some weight if
+	// assumption was wrong (i.e. if we did refund instead of slashing). This check
+	// ensures the extension will not refund weight when it doesn't need to (i.e. if pallet
+	// weights do not account weights of refund extension).
+	if runtime_includes_refund_extension {
+		assert_ne!(Weights::<T>::receive_messages_proof_overhead_from_runtime(), Weight::zero());
+		assert_ne!(
+			Weights::<T>::receive_messages_delivery_proof_overhead_from_runtime(),
+			Weight::zero()
+		);
+	}
 }
 
 /// Check that the `AdditionalSigned` type of a wrapped runtime is the same as the one of the

bridges/bin/runtime-common/src/refund_relayer_extension.rs

@@ -40,7 +40,9 @@ use pallet_bridge_parachains::{
 	BoundedBridgeGrandpaConfig, CallSubType as ParachainsCallSubType, Config as ParachainsConfig,
 	RelayBlockNumber, SubmitParachainHeadsHelper, SubmitParachainHeadsInfo,
 };
-use pallet_bridge_relayers::{Config as RelayersConfig, Pallet as RelayersPallet};
+use pallet_bridge_relayers::{
+	Config as RelayersConfig, Pallet as RelayersPallet, WeightInfoExt as _,
+};
 use pallet_transaction_payment::{Config as TransactionPaymentConfig, OnChargeTransaction};
 use pallet_utility::{Call as UtilityCall, Config as UtilityConfig, Pallet as UtilityPallet};
 use scale_info::TypeInfo;
@@ -445,11 +447,19 @@ where
 
 		// decrease post-dispatch weight/size using extra weight/size that we know now
 		let post_info_len = len.saturating_sub(extra_size as usize);
-		let mut post_info = *post_info;
-		post_info.actual_weight =
-			Some(post_info.actual_weight.unwrap_or(info.weight).saturating_sub(extra_weight));
+		let mut post_info_weight =
+			post_info.actual_weight.unwrap_or(info.weight).saturating_sub(extra_weight);
+
+		// let's also replace the weight of slashing relayer with the weight of rewarding relayer
+		if call_info.is_receive_messages_proof_call() {
+			post_info_weight = post_info_weight.saturating_sub(
+				<Runtime as RelayersConfig>::WeightInfo::extra_weight_of_successful_receive_messages_proof_call(),
+			);
+		}
 
 		// compute the relayer refund
+		let mut post_info = *post_info;
+		post_info.actual_weight = Some(post_info_weight);
 		let refund = Refund::compute_refund(info, &post_info, post_info_len, tip);
 
 		// we can finally reward relayer
@@ -1052,7 +1062,20 @@ mod tests {
 		assert_eq!(post_dispatch_result, Ok(()));
 	}
 
-	fn expected_reward() -> ThisChainBalance {
+	fn expected_delivery_reward() -> ThisChainBalance {
+		let mut post_dispatch_info = post_dispatch_info();
+		let extra_weight = <TestRuntime as RelayersConfig>::WeightInfo::extra_weight_of_successful_receive_messages_proof_call();
+		post_dispatch_info.actual_weight =
+			Some(dispatch_info().weight.saturating_sub(extra_weight));
+		pallet_transaction_payment::Pallet::<TestRuntime>::compute_actual_fee(
+			1024,
+			&dispatch_info(),
+			&post_dispatch_info,
+			Zero::zero(),
+		)
+	}
+
+	fn expected_confirmation_reward() -> ThisChainBalance {
 		pallet_transaction_payment::Pallet::<TestRuntime>::compute_actual_fee(
 			1024,
 			&dispatch_info(),
@@ -1449,7 +1472,7 @@ mod tests {
 
 			// without any size/weight refund: we expect regular reward
 			let pre_dispatch_data = all_finality_pre_dispatch_data();
-			let regular_reward = expected_reward();
+			let regular_reward = expected_delivery_reward();
 			run_post_dispatch(Some(pre_dispatch_data), Ok(()));
 			assert_eq!(
 				RelayersPallet::<TestRuntime>::relayer_reward(
@@ -1496,7 +1519,7 @@ mod tests {
 					relayer_account_at_this_chain(),
 					MsgProofsRewardsAccount::get()
 				),
-				Some(expected_reward()),
+				Some(expected_delivery_reward()),
 			);
 
 			run_post_dispatch(Some(all_finality_confirmation_pre_dispatch_data()), Ok(()));
@@ -1505,7 +1528,7 @@ mod tests {
 					relayer_account_at_this_chain(),
 					MsgDeliveryProofsRewardsAccount::get()
 				),
-				Some(expected_reward()),
+				Some(expected_confirmation_reward()),
 			);
 		});
 	}
@@ -1521,7 +1544,7 @@ mod tests {
 					relayer_account_at_this_chain(),
 					MsgProofsRewardsAccount::get()
 				),
-				Some(expected_reward()),
+				Some(expected_delivery_reward()),
 			);
 
 			run_post_dispatch(Some(parachain_finality_confirmation_pre_dispatch_data()), Ok(()));
@@ -1530,7 +1553,7 @@ mod tests {
 					relayer_account_at_this_chain(),
 					MsgDeliveryProofsRewardsAccount::get()
 				),
-				Some(expected_reward()),
+				Some(expected_confirmation_reward()),
 			);
 		});
 	}
@@ -1546,7 +1569,7 @@ mod tests {
 					relayer_account_at_this_chain(),
 					MsgProofsRewardsAccount::get()
 				),
-				Some(expected_reward()),
+				Some(expected_delivery_reward()),
 			);
 
 			run_post_dispatch(Some(confirmation_pre_dispatch_data()), Ok(()));
@@ -1555,7 +1578,7 @@ mod tests {
 					relayer_account_at_this_chain(),
 					MsgDeliveryProofsRewardsAccount::get()
 				),
-				Some(expected_reward()),
+				Some(expected_confirmation_reward()),
 			);
 		});
 	}

bridges/modules/messages/src/weights_ext.rs

@@ -266,6 +266,27 @@ pub trait WeightInfoExt: WeightInfo {
 	/// this value, we're going to charge relayer for that.
 	fn expected_extra_storage_proof_size() -> u32;
 
+	// Our configuration assumes that the runtime has special signed extensions used to:
+	//
+	// 1) reject obsolete delivery and confirmation transactions;
+	//
+	// 2) refund transaction cost to relayer and register his rewards.
+	//
+	// The checks in (1) are trivial, so its computation weight may be ignored. And we only touch
+	// storage values that are read during the call. So we may ignore the weight of this check.
+	//
+	// However, during (2) we read and update storage values of other pallets
+	// (`pallet-bridge-relayers` and balances/assets pallet). So we need to add this weight to the
+	// weight of our call. Hence two following methods.
+
+	/// Extra weight that is added to the `receive_messages_proof` call weight by signed extensions
+	/// that are declared at runtime level.
+	fn receive_messages_proof_overhead_from_runtime() -> Weight;
+
+	/// Extra weight that is added to the `receive_messages_delivery_proof` call weight by signed
+	/// extensions that are declared at runtime level.
+	fn receive_messages_delivery_proof_overhead_from_runtime() -> Weight;
+
 	// Functions that are directly mapped to extrinsics weights.
 
 	/// Weight of message delivery extrinsic.
@@ -276,6 +297,8 @@ pub trait WeightInfoExt: WeightInfo {
 	) -> Weight {
 		// basic components of extrinsic weight
 		let transaction_overhead = Self::receive_messages_proof_overhead();
+		let transaction_overhead_from_runtime =
+			Self::receive_messages_proof_overhead_from_runtime();
 		let outbound_state_delivery_weight =
 			Self::receive_messages_proof_outbound_lane_state_overhead();
 		let messages_delivery_weight =
@@ -292,6 +315,7 @@ pub trait WeightInfoExt: WeightInfo {
 		);
 
 		transaction_overhead
+			.saturating_add(transaction_overhead_from_runtime)
 			.saturating_add(outbound_state_delivery_weight)
 			.saturating_add(messages_delivery_weight)
 			.saturating_add(messages_dispatch_weight)
@@ -305,6 +329,8 @@ pub trait WeightInfoExt: WeightInfo {
 	) -> Weight {
 		// basic components of extrinsic weight
 		let transaction_overhead = Self::receive_messages_delivery_proof_overhead();
+		let transaction_overhead_from_runtime =
+			Self::receive_messages_delivery_proof_overhead_from_runtime();
 		let messages_overhead =
 			Self::receive_messages_delivery_proof_messages_overhead(relayers_state.total_messages);
 		let relayers_overhead = Self::receive_messages_delivery_proof_relayers_overhead(
@@ -319,6 +345,7 @@ pub trait WeightInfoExt: WeightInfo {
 		);
 
 		transaction_overhead
+			.saturating_add(transaction_overhead_from_runtime)
 			.saturating_add(messages_overhead)
 			.saturating_add(relayers_overhead)
 			.saturating_add(proof_size_overhead)
@@ -424,12 +451,28 @@ impl WeightInfoExt for () {
 	fn expected_extra_storage_proof_size() -> u32 {
 		EXTRA_STORAGE_PROOF_SIZE
 	}
+
+	fn receive_messages_proof_overhead_from_runtime() -> Weight {
+		Weight::zero()
+	}
+
+	fn receive_messages_delivery_proof_overhead_from_runtime() -> Weight {
+		Weight::zero()
+	}
 }
 
 impl<T: frame_system::Config> WeightInfoExt for crate::weights::BridgeWeight<T> {
 	fn expected_extra_storage_proof_size() -> u32 {
 		EXTRA_STORAGE_PROOF_SIZE
 	}
+
+	fn receive_messages_proof_overhead_from_runtime() -> Weight {
+		Weight::zero()
+	}
+
+	fn receive_messages_delivery_proof_overhead_from_runtime() -> Weight {
+		Weight::zero()
+	}
 }
 
 #[cfg(test)]