[stable2407] Backport #7028

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

@@ -93,7 +93,15 @@ where
 mod tests {
 	use super::*;
 	use crate::mock::{new_test_ext, Test, CALL};
+<<<<<<< HEAD
 	use frame_support::{assert_noop, assert_ok};
+=======
+	use frame_support::{assert_ok, dispatch::DispatchInfo};
+	use sp_runtime::{
+		traits::{AsTransactionAuthorizedOrigin, DispatchTransaction, TxBaseImplication},
+		transaction_validity::{TransactionSource::External, TransactionValidityError},
+	};
+>>>>>>> b5a5ac4 (Make `TransactionExtension` tuple of tuple transparent for implication (#7028))
 
 	#[test]
 	fn zero_account_ban_works() {
@@ -104,7 +112,30 @@ mod tests {
 				CheckNonZeroSender::<Test>::new().validate(&0, CALL, &info, len),
 				InvalidTransaction::BadSigner
 			);
+<<<<<<< HEAD
 			assert_ok!(CheckNonZeroSender::<Test>::new().validate(&1, CALL, &info, len));
+=======
+			assert_ok!(CheckNonZeroSender::<Test>::new().validate_only(
+				Some(1).into(),
+				CALL,
+				&info,
+				len,
+				External,
+				0,
+			));
+		})
+	}
+
+	#[test]
+	fn unsigned_origin_works() {
+		new_test_ext().execute_with(|| {
+			let info = DispatchInfo::default();
+			let len = 0_usize;
+			let (_, _, origin) = CheckNonZeroSender::<Test>::new()
+				.validate(None.into(), CALL, &info, len, (), &TxBaseImplication(CALL), External)
+				.unwrap();
+			assert!(!origin.is_transaction_authorized());
+>>>>>>> b5a5ac4 (Make `TransactionExtension` tuple of tuple transparent for implication (#7028))
 		})
 	}
 }

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

@@ -133,8 +133,19 @@ where
 #[cfg(test)]
 mod tests {
 	use super::*;
+<<<<<<< HEAD
 	use crate::mock::{new_test_ext, Test, CALL};
 	use frame_support::{assert_noop, assert_ok};
+=======
+	use crate::mock::{new_test_ext, RuntimeCall, Test, CALL};
+	use frame_support::{
+		assert_ok, assert_storage_noop, dispatch::GetDispatchInfo, traits::OriginTrait,
+	};
+	use sp_runtime::{
+		traits::{AsTransactionAuthorizedOrigin, DispatchTransaction, TxBaseImplication},
+		transaction_validity::TransactionSource::External,
+	};
+>>>>>>> b5a5ac4 (Make `TransactionExtension` tuple of tuple transparent for implication (#7028))
 
 	#[test]
 	fn signed_ext_check_nonce_works() {
@@ -198,6 +209,7 @@ mod tests {
 			let info = DispatchInfo::default();
 			let len = 0_usize;
 			// Both providers and sufficients zero
+<<<<<<< HEAD
 			assert_noop!(
 				CheckNonce::<Test>(1u64.into()).validate(&1, CALL, &info, len),
 				InvalidTransaction::Payment
@@ -212,6 +224,141 @@ mod tests {
 			// Non-zero sufficients
 			assert_ok!(CheckNonce::<Test>(1u64.into()).validate(&3, CALL, &info, len));
 			assert_ok!(CheckNonce::<Test>(1u64.into()).pre_dispatch(&3, CALL, &info, len));
+=======
+			assert_storage_noop!({
+				assert_eq!(
+					CheckNonce::<Test>(1u64.into())
+						.validate_only(Some(1).into(), CALL, &info, len, External, 0)
+						.unwrap_err(),
+					TransactionValidityError::Invalid(InvalidTransaction::Payment)
+				);
+				assert_eq!(
+					CheckNonce::<Test>(1u64.into())
+						.validate_and_prepare(Some(1).into(), CALL, &info, len, 0)
+						.unwrap_err(),
+					TransactionValidityError::Invalid(InvalidTransaction::Payment)
+				);
+			});
+			// Non-zero providers
+			assert_ok!(CheckNonce::<Test>(1u64.into()).validate_only(
+				Some(2).into(),
+				CALL,
+				&info,
+				len,
+				External,
+				0,
+			));
+			assert_ok!(CheckNonce::<Test>(1u64.into()).validate_and_prepare(
+				Some(2).into(),
+				CALL,
+				&info,
+				len,
+				0,
+			));
+			// Non-zero sufficients
+			assert_ok!(CheckNonce::<Test>(1u64.into()).validate_only(
+				Some(3).into(),
+				CALL,
+				&info,
+				len,
+				External,
+				0,
+			));
+			assert_ok!(CheckNonce::<Test>(1u64.into()).validate_and_prepare(
+				Some(3).into(),
+				CALL,
+				&info,
+				len,
+				0,
+			));
+		})
+	}
+
+	#[test]
+	fn unsigned_check_nonce_works() {
+		new_test_ext().execute_with(|| {
+			let info = DispatchInfo::default();
+			let len = 0_usize;
+			let (_, val, origin) = CheckNonce::<Test>(1u64.into())
+				.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));
+		})
+	}
+
+	#[test]
+	fn check_nonce_preserves_account_data() {
+		new_test_ext().execute_with(|| {
+			crate::Account::<Test>::insert(
+				1,
+				crate::AccountInfo {
+					nonce: 1u64.into(),
+					consumers: 0,
+					providers: 1,
+					sufficients: 0,
+					data: 0,
+				},
+			);
+			let info = DispatchInfo::default();
+			let len = 0_usize;
+			// run the validation step
+			let (_, val, origin) = CheckNonce::<Test>(1u64.into())
+				.validate(Some(1).into(), CALL, &info, len, (), &TxBaseImplication(CALL), External)
+				.unwrap();
+			// mutate `AccountData` for the caller
+			crate::Account::<Test>::mutate(1, |info| {
+				info.data = 42;
+			});
+			// run the preparation step
+			assert_ok!(CheckNonce::<Test>(1u64.into()).prepare(val, &origin, CALL, &info, len));
+			// only the nonce should be altered by the preparation step
+			let expected_info = crate::AccountInfo {
+				nonce: 2u64.into(),
+				consumers: 0,
+				providers: 1,
+				sufficients: 0,
+				data: 42,
+			};
+			assert_eq!(crate::Account::<Test>::get(1), expected_info);
+		})
+	}
+
+	#[test]
+	fn check_nonce_skipped_and_refund_for_other_origins() {
+		new_test_ext().execute_with(|| {
+			let ext = CheckNonce::<Test>(1u64.into());
+
+			let mut info = CALL.get_dispatch_info();
+			info.extension_weight = ext.weight(CALL);
+
+			// Ensure we test the refund.
+			assert!(info.extension_weight != Weight::zero());
+
+			let len = CALL.encoded_size();
+
+			let origin = crate::RawOrigin::Root.into();
+			let (pre, origin) = ext.validate_and_prepare(origin, CALL, &info, len, 0).unwrap();
+
+			assert!(origin.as_system_ref().unwrap().is_root());
+
+			let pd_res = Ok(());
+			let mut post_info = frame_support::dispatch::PostDispatchInfo {
+				actual_weight: Some(info.total_weight()),
+				pays_fee: Default::default(),
+			};
+
+			<CheckNonce<Test> as TransactionExtension<RuntimeCall>>::post_dispatch(
+				pre,
+				&info,
+				&mut post_info,
+				len,
+				&pd_res,
+			)
+			.unwrap();
+
+			assert_eq!(post_info.actual_weight, Some(info.call_weight));
+>>>>>>> b5a5ac4 (Make `TransactionExtension` tuple of tuple transparent for implication (#7028))
 		})
 	}
 }

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

@@ -43,8 +43,16 @@ use frame_support::{
 };
 use scale_info::{StaticTypeInfo, TypeInfo};
 use sp_runtime::{
+<<<<<<< HEAD
 	traits::{DispatchInfoOf, PostDispatchInfoOf, SignedExtension},
 	transaction_validity::{TransactionValidity, TransactionValidityError, ValidTransaction},
+=======
+	traits::{
+		DispatchInfoOf, DispatchOriginOf, Implication, PostDispatchInfoOf, TransactionExtension,
+		ValidateResult,
+	},
+	transaction_validity::TransactionValidityError,
+>>>>>>> b5a5ac4 (Make `TransactionExtension` tuple of tuple transparent for implication (#7028))
 };
 
 #[cfg(test)]
@@ -129,9 +137,18 @@ where
 		call: &Self::Call,
 		info: &DispatchInfoOf<Self::Call>,
 		len: usize,
+<<<<<<< HEAD
 	) -> TransactionValidity {
 		if call.is_feeless(&<T as frame_system::Config>::RuntimeOrigin::signed(who.clone())) {
 			Ok(ValidTransaction::default())
+=======
+		self_implicit: S::Implicit,
+		inherited_implication: &impl Implication,
+		source: TransactionSource,
+	) -> ValidateResult<Self::Val, T::RuntimeCall> {
+		if call.is_feeless(&origin) {
+			Ok((Default::default(), Skip(origin.caller().clone()), origin))
+>>>>>>> b5a5ac4 (Make `TransactionExtension` tuple of tuple transparent for implication (#7028))
 		} else {
 			self.0.validate(who, call, info, len)
 		}

substrate/primitives/runtime/src/traits.rs

@@ -52,6 +52,15 @@ use std::fmt::Display;
 #[cfg(feature = "std")]
 use std::str::FromStr;
 
+<<<<<<< HEAD:substrate/primitives/runtime/src/traits.rs
+=======
+pub mod transaction_extension;
+pub use transaction_extension::{
+	DispatchTransaction, Implication, ImplicationParts, TransactionExtension,
+	TransactionExtensionMetadata, TxBaseImplication, ValidateResult,
+};
+
+>>>>>>> b5a5ac4 (Make `TransactionExtension` tuple of tuple transparent for implication (#7028)):substrate/primitives/runtime/src/traits/mod.rs
 /// A lazy value.
 pub trait Lazy<T: ?Sized> {
 	/// Get a reference to the underlying value.