Make TransactionExtension tuple of tuple transparent for implication (#7028)

Currently `(A, B, C)` and `((A, B), C)` change the order of implications
in the transaction extension pipeline. This order is not accessible in
the metadata, because the metadata is just a vector of transaction
extension, the nested structure is not visible.

This PR make the implementation for tuple of `TransactionExtension`
better for tuple of tuple. `(A, B, C)` and `((A, B), C)` don't change
the implication for the validation A.

This is a breaking change but only when using the trait
`TransactionExtension` the code implementing the trait is not breaking
(surprising rust behavior but fine).

---------

Co-authored-by: command-bot <>
Co-authored-by: Bastian Köcher <[email protected]>
(cherry picked from commit b5a5ac4)

Author: 2 people

prdoc/pr_7028.prdoc

@@ -0,0 +1,25 @@
+title: 'Fix implication order in implementation of `TransactionExtension` for tuple'
+doc:
+- audience:
+  - Runtime Dev
+  - Runtime User
+  description: |-
+    Before this PR, the implications were different in the pipeline `(A, B, C)` and `((A, B), C)`.
+    This PR fixes this behavior and make nested tuple transparant, the implication order of tuple of
+    tuple is now the same as in a single tuple.
+
+    For runtime users this mean that the implication can be breaking depending on the pipeline used
+    in the runtime.
+
+    For runtime developers this breaks usage of `TransactionExtension::validate`.
+    When calling `TransactionExtension::validate` the implication must now implement `Implication`
+    trait, you can use `TxBaseImplication` to wrap the type and use it as the base implication.
+    E.g. instead of `&(extension_version, call),` you can write `&TxBaseImplication((extension_version, call))`.
+
+crates:
+- name: sp-runtime
+  bump: major
+- name: pallet-skip-feeless-payment
+  bump: major
+- name: frame-system
+  bump: major

substrate/frame/system/src/extensions/check_non_zero_sender.rs

@@ -86,7 +86,7 @@ mod tests {
 	use crate::mock::{new_test_ext, Test, CALL};
 	use frame_support::{assert_ok, dispatch::DispatchInfo};
 	use sp_runtime::{
-		traits::{AsTransactionAuthorizedOrigin, DispatchTransaction},
+		traits::{AsTransactionAuthorizedOrigin, DispatchTransaction, TxBaseImplication},
 		transaction_validity::{TransactionSource::External, TransactionValidityError},
 	};
 
@@ -118,7 +118,7 @@ mod tests {
 			let info = DispatchInfo::default();
 			let len = 0_usize;
 			let (_, _, origin) = CheckNonZeroSender::<Test>::new()
-				.validate(None.into(), CALL, &info, len, (), CALL, External)
+				.validate(None.into(), CALL, &info, len, (), &TxBaseImplication(CALL), External)
 				.unwrap();
 			assert!(!origin.is_transaction_authorized());
 		})

substrate/frame/system/src/extensions/check_nonce.rs

@@ -186,7 +186,7 @@ mod tests {
 		assert_ok, assert_storage_noop, dispatch::GetDispatchInfo, traits::OriginTrait,
 	};
 	use sp_runtime::{
-		traits::{AsTransactionAuthorizedOrigin, DispatchTransaction},
+		traits::{AsTransactionAuthorizedOrigin, DispatchTransaction, TxBaseImplication},
 		transaction_validity::TransactionSource::External,
 	};
 
@@ -335,7 +335,7 @@ mod tests {
 			let info = DispatchInfo::default();
 			let len = 0_usize;
 			let (_, val, origin) = CheckNonce::<Test>(1u64.into())
-				.validate(None.into(), CALL, &info, len, (), CALL, External)
+				.validate(None.into(), CALL, &info, len, (), &TxBaseImplication(CALL), External)
 				.unwrap();
 			assert!(!origin.is_transaction_authorized());
 			assert_ok!(CheckNonce::<Test>(1u64.into()).prepare(val, &origin, CALL, &info, len));
@@ -359,7 +359,7 @@ mod tests {
 			let len = 0_usize;
 			// run the validation step
 			let (_, val, origin) = CheckNonce::<Test>(1u64.into())
-				.validate(Some(1).into(), CALL, &info, len, (), CALL, External)
+				.validate(Some(1).into(), CALL, &info, len, (), &TxBaseImplication(CALL), External)
 				.unwrap();
 			// mutate `AccountData` for the caller
 			crate::Account::<Test>::mutate(1, |info| {

substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs

@@ -46,7 +46,8 @@ use frame_support::{
 use scale_info::{StaticTypeInfo, TypeInfo};
 use sp_runtime::{
 	traits::{
-		DispatchInfoOf, DispatchOriginOf, PostDispatchInfoOf, TransactionExtension, ValidateResult,
+		DispatchInfoOf, DispatchOriginOf, Implication, PostDispatchInfoOf, TransactionExtension,
+		ValidateResult,
 	},
 	transaction_validity::TransactionValidityError,
 };
@@ -147,7 +148,7 @@ where
 		info: &DispatchInfoOf<T::RuntimeCall>,
 		len: usize,
 		self_implicit: S::Implicit,
-		inherited_implication: &impl Encode,
+		inherited_implication: &impl Implication,
 		source: TransactionSource,
 	) -> ValidateResult<Self::Val, T::RuntimeCall> {
 		if call.is_feeless(&origin) {

substrate/primitives/runtime/src/traits/mod.rs

@@ -54,7 +54,8 @@ use std::str::FromStr;
 
 pub mod transaction_extension;
 pub use transaction_extension::{
-	DispatchTransaction, TransactionExtension, TransactionExtensionMetadata, ValidateResult,
+	DispatchTransaction, Implication, ImplicationParts, TransactionExtension,
+	TransactionExtensionMetadata, TxBaseImplication, ValidateResult,
 };
 
 /// A lazy value.

substrate/primitives/runtime/src/traits/transaction_extension/dispatch_transaction.rs

@@ -111,7 +111,7 @@ where
 			info,
 			len,
 			self.implicit()?,
-			&(extension_version, call),
+			&TxBaseImplication((extension_version, call)),
 			source,
 		) {
 			// After validation, some origin must have been authorized.