Reduce the number of macros used for SignedExtensions

Reduce the number of macros used for SignedExtensions Signed-off-by: Serban Iorga <serban@parity.io>
2026-06-09 21:21:11 +00:00 · 2022-07-25 11:27:35 +03:00
parent 057fd6cab2
commit 5d9bd1d0b5
7 changed files with 403 additions and 406 deletions
@@ -18,6 +18,9 @@

 #![cfg_attr(not(feature = "std"), no_std)]

+use bp_runtime::FilterCall;
+use sp_runtime::transaction_validity::TransactionValidity;
+
 pub mod messages;
 pub mod messages_api;
 pub mod messages_benchmarking;
@@ -26,3 +29,167 @@ pub mod parachains_benchmarking;

 #[cfg(feature = "integrity-test")]
 pub mod integrity;
+
+/// A duplication of the `FilterCall` trait.
+///
+/// We need this trait in order to be able to implement it for the messages pallet,
+/// since the implementation is done outside of the pallet crate.
+pub trait BridgeRuntimeFilterCall<Call> {
+	/// Checks if a runtime call is valid.
+	fn validate(call: &Call) -> TransactionValidity;
+}
+
+impl<Call, T, I> BridgeRuntimeFilterCall<Call> for pallet_bridge_grandpa::Pallet<T, I>
+where
+	pallet_bridge_grandpa::Pallet<T, I>: FilterCall<Call>,
+{
+	fn validate(call: &Call) -> TransactionValidity {
+		<pallet_bridge_grandpa::Pallet<T, I> as FilterCall<Call>>::validate(call)
+	}
+}
+
+impl<Call, T, I> BridgeRuntimeFilterCall<Call> for pallet_bridge_parachains::Pallet<T, I>
+where
+	pallet_bridge_parachains::Pallet<T, I>: FilterCall<Call>,
+{
+	fn validate(call: &Call) -> TransactionValidity {
+		<pallet_bridge_parachains::Pallet<T, I> as FilterCall<Call>>::validate(call)
+	}
+}
+
+/// Declares a runtime-specific `BridgeRejectObsoleteHeadersAndMessages` signed extension.
+///
+/// ## Example
+///
+/// ```nocompile
+/// generate_bridge_reject_obsolete_headers_and_messages!{
+///     Call, AccountId
+///     BridgeRialtoGrandpa, BridgeWestendGrandpa,
+///     BridgeRialtoParachains
+/// }
+/// ```
+///
+/// The goal of this extension is to avoid "mining" transactions that provide outdated bridged
+/// headers and messages. Without that extension, even honest relayers may lose their funds if
+/// there are multiple relays running and submitting the same information.
+#[macro_export]
+macro_rules! generate_bridge_reject_obsolete_headers_and_messages {
+	($call:ty, $account_id:ty, $($filter_call:ty),*) => {
+		#[derive(Clone, codec::Decode, codec::Encode, Eq, PartialEq, frame_support::RuntimeDebug, scale_info::TypeInfo)]
+		pub struct BridgeRejectObsoleteHeadersAndMessages;
+		impl sp_runtime::traits::SignedExtension for BridgeRejectObsoleteHeadersAndMessages {
+			const IDENTIFIER: &'static str = "BridgeRejectObsoleteHeadersAndMessages";
+			type AccountId = $account_id;
+			type Call = $call;
+			type AdditionalSigned = ();
+			type Pre = ();
+
+			fn additional_signed(&self) -> sp_std::result::Result<
+				(),
+				sp_runtime::transaction_validity::TransactionValidityError,
+			> {
+				Ok(())
+			}
+
+			fn validate(
+				&self,
+				_who: &Self::AccountId,
+				call: &Self::Call,
+				_info: &sp_runtime::traits::DispatchInfoOf<Self::Call>,
+				_len: usize,
+			) -> sp_runtime::transaction_validity::TransactionValidity {
+				let valid = sp_runtime::transaction_validity::ValidTransaction::default();
+				$(
+					let valid = valid
+						.combine_with(<$filter_call as $crate::BridgeRuntimeFilterCall<$call>>::validate(call)?);
+				)*
+				Ok(valid)
+			}
+
+			fn pre_dispatch(
+				self,
+				who: &Self::AccountId,
+				call: &Self::Call,
+				info: &sp_runtime::traits::DispatchInfoOf<Self::Call>,
+				len: usize,
+			) -> Result<Self::Pre, sp_runtime::transaction_validity::TransactionValidityError> {
+				self.validate(who, call, info, len).map(drop)
+			}
+		}
+	};
+}
+
+#[cfg(test)]
+mod tests {
+	use crate::BridgeRuntimeFilterCall;
+	use frame_support::{assert_err, assert_ok};
+	use sp_runtime::{
+		traits::SignedExtension,
+		transaction_validity::{InvalidTransaction, TransactionValidity, ValidTransaction},
+	};
+
+	pub struct MockCall {
+		data: u32,
+	}
+
+	impl sp_runtime::traits::Dispatchable for MockCall {
+		type Origin = ();
+		type Config = ();
+		type Info = ();
+		type PostInfo = ();
+
+		fn dispatch(
+			self,
+			_origin: Self::Origin,
+		) -> sp_runtime::DispatchResultWithInfo<Self::PostInfo> {
+			unimplemented!()
+		}
+	}
+
+	struct FirstFilterCall;
+	impl BridgeRuntimeFilterCall<MockCall> for FirstFilterCall {
+		fn validate(call: &MockCall) -> TransactionValidity {
+			if call.data <= 1 {
+				return InvalidTransaction::Custom(1).into()
+			}
+
+			Ok(ValidTransaction { priority: 1, ..Default::default() })
+		}
+	}
+
+	struct SecondFilterCall;
+	impl BridgeRuntimeFilterCall<MockCall> for SecondFilterCall {
+		fn validate(call: &MockCall) -> TransactionValidity {
+			if call.data <= 2 {
+				return InvalidTransaction::Custom(2).into()
+			}
+
+			Ok(ValidTransaction { priority: 2, ..Default::default() })
+		}
+	}
+
+	#[test]
+	fn test() {
+		generate_bridge_reject_obsolete_headers_and_messages!(
+			MockCall,
+			(),
+			FirstFilterCall,
+			SecondFilterCall
+		);
+
+		assert_err!(
+			BridgeRejectObsoleteHeadersAndMessages.validate(&(), &MockCall { data: 1 }, &(), 0),
+			InvalidTransaction::Custom(1)
+		);
+
+		assert_err!(
+			BridgeRejectObsoleteHeadersAndMessages.validate(&(), &MockCall { data: 2 }, &(), 0),
+			InvalidTransaction::Custom(2)
+		);
+
+		assert_ok!(
+			BridgeRejectObsoleteHeadersAndMessages.validate(&(), &MockCall { data: 3 }, &(), 0),
+			ValidTransaction { priority: 3, ..Default::default() }
+		)
+	}
+}
@@ -14,136 +14,100 @@
 // You should have received a copy of the GNU General Public License
 // along with Parity Bridges Common.  If not, see <http://www.gnu.org/licenses/>.

-/// Declares a runtime-specific `BridgeRejectObsoleteMessages` and
-/// `BridgeRejectObsoleteMessageConfirmations` signed extensions.
-///
-/// ## Example
-///
-/// ```nocompile
-/// bridge_runtime_common::declare_bridge_reject_obsolete_messages!{
-///     Runtime,
-///     Call::BridgeRialtoMessages => WithRialtoMessagesInstance,
-///     Call::BridgeRialtoParachainMessages => WithRialtoParachainMessagesInstance,
-/// }
-/// ```
-///
-/// The goal of this extension is to avoid "mining" messages delivery and delivery confirmation
-/// transactions, that are delivering outdated messages/confirmations. Without that extension,
+use crate::{
+	messages::{
+		source::FromBridgedChainMessagesDeliveryProof, target::FromBridgedChainMessagesProof,
+	},
+	BridgeRuntimeFilterCall,
+};
+use frame_support::{dispatch::CallableCallFor, traits::IsSubType};
+use pallet_bridge_messages::{Config, Pallet};
+use sp_runtime::transaction_validity::TransactionValidity;
+
+/// Validate messages in order to avoid "mining" messages delivery and delivery confirmation
+/// transactions, that are delivering outdated messages/confirmations. Without this validation,
 /// even honest relayers may lose their funds if there are multiple relays running and submitting
 /// the same messages/confirmations.
-#[macro_export]
-macro_rules! declare_bridge_reject_obsolete_messages {
-	($runtime:ident, $($call:path => $instance:ty),*) => {
-		/// Transaction-with-obsolete-messages check that will reject transaction if
-		/// it submits obsolete messages/confirmations.
-		#[derive(Clone, codec::Decode, codec::Encode, Eq, PartialEq, frame_support::RuntimeDebug, scale_info::TypeInfo)]
-		pub struct BridgeRejectObsoleteMessages;
+impl<
+		BridgedHeaderHash,
+		SourceHeaderChain: bp_messages::target_chain::SourceHeaderChain<
+			<T as Config<I>>::InboundMessageFee,
+			MessagesProof = FromBridgedChainMessagesProof<BridgedHeaderHash>,
+		>,
+		TargetHeaderChain: bp_messages::source_chain::TargetHeaderChain<
+			<T as Config<I>>::OutboundPayload,
+			<T as frame_system::Config>::AccountId,
+			MessagesDeliveryProof = FromBridgedChainMessagesDeliveryProof<BridgedHeaderHash>,
+		>,
+		Call: IsSubType<CallableCallFor<Pallet<T, I>, T>>,
+		T: frame_system::Config<Call = Call>
+			+ Config<I, SourceHeaderChain = SourceHeaderChain, TargetHeaderChain = TargetHeaderChain>,
+		I: 'static,
+	> BridgeRuntimeFilterCall<Call> for Pallet<T, I>
+{
+	fn validate(call: &Call) -> TransactionValidity {
+		match call.is_sub_type() {
+			Some(pallet_bridge_messages::Call::<T, I>::receive_messages_proof {
+				ref proof,
+				..
+			}) => {
+				let inbound_lane_data =
+					pallet_bridge_messages::InboundLanes::<T, I>::get(&proof.lane);
+				if proof.nonces_end <= inbound_lane_data.last_delivered_nonce() {
+					log::trace!(
+						target: pallet_bridge_messages::LOG_TARGET,
+						"Rejecting obsolete messages delivery transaction: \
+                            lane {:?}, bundled {:?}, best {:?}",
+						proof.lane,
+						proof.nonces_end,
+						inbound_lane_data.last_delivered_nonce(),
+					);

-		impl sp_runtime::traits::SignedExtension for BridgeRejectObsoleteMessages {
-			const IDENTIFIER: &'static str = "BridgeRejectObsoleteMessages";
-			type AccountId = <$runtime as frame_system::Config>::AccountId;
-			type Call = <$runtime as frame_system::Config>::Call;
-			type AdditionalSigned = ();
-			type Pre = ();
-
-			fn additional_signed(&self) -> sp_std::result::Result<
-				(),
-				sp_runtime::transaction_validity::TransactionValidityError,
-			> {
-				Ok(())
-			}
-
-			fn validate(
-				&self,
-				_who: &Self::AccountId,
-				call: &Self::Call,
-				_info: &sp_runtime::traits::DispatchInfoOf<Self::Call>,
-				_len: usize,
-			) -> sp_runtime::transaction_validity::TransactionValidity {
-				match *call {
-					$(
-						$call(pallet_bridge_messages::Call::<$runtime, $instance>::receive_messages_proof {
-							ref proof,
-							..
-						}) => {
-							let nonces_end = proof.nonces_end;
-
-							let inbound_lane_data = pallet_bridge_messages::InboundLanes::<$runtime, $instance>::get(&proof.lane);
-							if proof.nonces_end <= inbound_lane_data.last_delivered_nonce() {
-								log::trace!(
-									target: pallet_bridge_messages::LOG_TARGET,
-									"Rejecting obsolete messages delivery transaction: lane {:?}, bundled {:?}, best {:?}",
-									proof.lane,
-									proof.nonces_end,
-									inbound_lane_data.last_delivered_nonce(),
-								);
-
-								return sp_runtime::transaction_validity::InvalidTransaction::Stale.into();
-							}
-
-							Ok(sp_runtime::transaction_validity::ValidTransaction::default())
-						},
-						$call(pallet_bridge_messages::Call::<$runtime, $instance>::receive_messages_delivery_proof {
-							ref proof,
-							ref relayers_state,
-							..
-						}) => {
-							let latest_delivered_nonce = relayers_state.last_delivered_nonce;
-
-							let outbound_lane_data = pallet_bridge_messages::OutboundLanes::<$runtime, $instance>::get(&proof.lane);
-							if latest_delivered_nonce <= outbound_lane_data.latest_received_nonce {
-								log::trace!(
-									target: pallet_bridge_messages::LOG_TARGET,
-									"Rejecting obsolete messages confirmation transaction: lane {:?}, bundled {:?}, best {:?}",
-									proof.lane,
-									latest_delivered_nonce,
-									outbound_lane_data.latest_received_nonce,
-								);
-
-								return sp_runtime::transaction_validity::InvalidTransaction::Stale.into();
-							}
-
-							Ok(sp_runtime::transaction_validity::ValidTransaction::default())
-						}
-					)*
-					_ => Ok(sp_runtime::transaction_validity::ValidTransaction::default()),
+					return sp_runtime::transaction_validity::InvalidTransaction::Stale.into()
 				}
-			}
+			},
+			Some(pallet_bridge_messages::Call::<T, I>::receive_messages_delivery_proof {
+				ref proof,
+				ref relayers_state,
+				..
+			}) => {
+				let latest_delivered_nonce = relayers_state.last_delivered_nonce;

-			fn pre_dispatch(
-				self,
-				who: &Self::AccountId,
-				call: &Self::Call,
-				info: &sp_runtime::traits::DispatchInfoOf<Self::Call>,
-				len: usize,
-			) -> Result<Self::Pre, sp_runtime::transaction_validity::TransactionValidityError> {
-				self.validate(who, call, info, len).map(drop)
-			}
+				let outbound_lane_data =
+					pallet_bridge_messages::OutboundLanes::<T, I>::get(&proof.lane);
+				if latest_delivered_nonce <= outbound_lane_data.latest_received_nonce {
+					log::trace!(
+						target: pallet_bridge_messages::LOG_TARGET,
+						"Rejecting obsolete messages confirmation transaction: \
+                            lane {:?}, bundled {:?}, best {:?}",
+						proof.lane,
+						latest_delivered_nonce,
+						outbound_lane_data.latest_received_nonce,
+					);

-			fn post_dispatch(
-				_maybe_pre: Option<Self::Pre>,
-				_info: &sp_runtime::traits::DispatchInfoOf<Self::Call>,
-				_post_info: &sp_runtime::traits::PostDispatchInfoOf<Self::Call>,
-				_len: usize,
-				_result: &sp_runtime::DispatchResult,
-			) -> Result<(), sp_runtime::transaction_validity::TransactionValidityError> {
-				Ok(())
-			}
+					return sp_runtime::transaction_validity::InvalidTransaction::Stale.into()
+				}
+			},
+			_ => {},
 		}
-	};
+
+		Ok(sp_runtime::transaction_validity::ValidTransaction::default())
+	}
 }

 #[cfg(test)]
 mod tests {
 	use bp_messages::UnrewardedRelayersState;
-	use frame_support::weights::{DispatchClass, DispatchInfo, Pays};
 	use millau_runtime::{
-		bridge_runtime_common::messages::{
-			source::FromBridgedChainMessagesDeliveryProof, target::FromBridgedChainMessagesProof,
+		bridge_runtime_common::{
+			messages::{
+				source::FromBridgedChainMessagesDeliveryProof,
+				target::FromBridgedChainMessagesProof,
+			},
+			BridgeRuntimeFilterCall,
 		},
-		BridgeRejectObsoleteMessages, Call, Runtime, WithRialtoMessagesInstance,
+		Call, Runtime, WithRialtoMessagesInstance,
 	};
-	use sp_runtime::traits::SignedExtension;

 	fn deliver_message_10() {
 		pallet_bridge_messages::InboundLanes::<Runtime, WithRialtoMessagesInstance>::insert(
@@ -156,13 +120,9 @@ mod tests {
 		nonces_start: bp_messages::MessageNonce,
 		nonces_end: bp_messages::MessageNonce,
 	) -> bool {
-		BridgeRejectObsoleteMessages
-			.validate(
-				&[0u8; 32].into(),
-				&Call::BridgeRialtoMessages(pallet_bridge_messages::Call::<
-					Runtime,
-					WithRialtoMessagesInstance,
-				>::receive_messages_proof {
+		pallet_bridge_messages::Pallet::<Runtime, WithRialtoMessagesInstance>::validate(
+			&Call::BridgeRialtoMessages(
+				pallet_bridge_messages::Call::<Runtime, ()>::receive_messages_proof {
 					relayer_id_at_bridged_chain: [0u8; 32].into(),
 					messages_count: (nonces_end - nonces_start + 1) as u32,
 					dispatch_weight: 0,
@@ -173,11 +133,10 @@ mod tests {
 						nonces_start,
 						nonces_end,
 					},
-				}),
-				&DispatchInfo { weight: 0, class: DispatchClass::Operational, pays_fee: Pays::Yes },
-				0,
-			)
-			.is_ok()
+				},
+			),
+		)
+		.is_ok()
 	}

 	#[test]
@@ -222,27 +181,23 @@ mod tests {
 	}

 	fn validate_message_confirmation(last_delivered_nonce: bp_messages::MessageNonce) -> bool {
-		BridgeRejectObsoleteMessages
-			.validate(
-				&[0u8; 32].into(),
-				&Call::BridgeRialtoMessages(pallet_bridge_messages::Call::<
-					Runtime,
-					WithRialtoMessagesInstance,
-				>::receive_messages_delivery_proof {
-					proof: FromBridgedChainMessagesDeliveryProof {
-						bridged_header_hash: Default::default(),
-						storage_proof: Vec::new(),
-						lane: [0, 0, 0, 0],
-					},
-					relayers_state: UnrewardedRelayersState {
-						last_delivered_nonce,
-						..Default::default()
-					},
-				}),
-				&DispatchInfo { weight: 0, class: DispatchClass::Operational, pays_fee: Pays::Yes },
-				0,
-			)
-			.is_ok()
+		pallet_bridge_messages::Pallet::<Runtime, WithRialtoMessagesInstance>::validate(
+			&Call::BridgeRialtoMessages(pallet_bridge_messages::Call::<
+				Runtime,
+				WithRialtoMessagesInstance,
+			>::receive_messages_delivery_proof {
+				proof: FromBridgedChainMessagesDeliveryProof {
+					bridged_header_hash: Default::default(),
+					storage_proof: Vec::new(),
+					lane: [0, 0, 0, 0],
+				},
+				relayers_state: UnrewardedRelayersState {
+					last_delivered_nonce,
+					..Default::default()
+				},
+			}),
+		)
+		.is_ok()
 	}

 	#[test]