diff --git a/prdoc/pr_7028.prdoc b/prdoc/pr_7028.prdoc
new file mode 100644
index 0000000000000000000000000000000000000000..ead918fc2e0077ef4972075705e1e0cc10447cc4
--- /dev/null
+++ b/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
diff --git a/substrate/frame/system/src/extensions/check_non_zero_sender.rs b/substrate/frame/system/src/extensions/check_non_zero_sender.rs
index 577e2b324fca5b981b58dd794aad28c7d67cdc3b..978eebaf3dace5542991573c942f222015148d51 100644
--- a/substrate/frame/system/src/extensions/check_non_zero_sender.rs
+++ b/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());
})
diff --git a/substrate/frame/system/src/extensions/check_nonce.rs b/substrate/frame/system/src/extensions/check_nonce.rs
index 004ec08a26f26f1b480d11f232a532bc4d8d269c..bc19a09e06a9e0d567441d066218b69377401c01 100644
--- a/substrate/frame/system/src/extensions/check_nonce.rs
+++ b/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| {
diff --git a/substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs b/substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs
index dd907f6fcbb74eacfeced2a25222122177b37532..5ba1d1297679eb6cc6b0d905a70a98d57b99a563 100644
--- a/substrate/frame/transaction-payment/skip-feeless-payment/src/lib.rs
+++ b/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) {
diff --git a/substrate/primitives/runtime/src/traits/mod.rs b/substrate/primitives/runtime/src/traits/mod.rs
index cfcc3e5a354d6d3fbf513f0ad09e6480c1c1a349..d371152dc40a48b774588af588a4ec160249426a 100644
--- a/substrate/primitives/runtime/src/traits/mod.rs
+++ b/substrate/primitives/runtime/src/traits/mod.rs
@@ -55,7 +55,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.
diff --git a/substrate/primitives/runtime/src/traits/transaction_extension/dispatch_transaction.rs b/substrate/primitives/runtime/src/traits/transaction_extension/dispatch_transaction.rs
index 28030d12fc9f3c390eddb492c2c88c37a5a2ed77..1fbaab0d45ac13bac0192383dcfcb7c942346ba7 100644
--- a/substrate/primitives/runtime/src/traits/transaction_extension/dispatch_transaction.rs
+++ b/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.
diff --git a/substrate/primitives/runtime/src/traits/transaction_extension/mod.rs b/substrate/primitives/runtime/src/traits/transaction_extension/mod.rs
index f8c5dc6a724eb9e6f61935eff675af1e3096eb01..27f33acb69cce1d53b6dc4058772ba0303b44e21 100644
--- a/substrate/primitives/runtime/src/traits/transaction_extension/mod.rs
+++ b/substrate/primitives/runtime/src/traits/transaction_extension/mod.rs
@@ -43,6 +43,72 @@ mod dispatch_transaction;
pub use as_transaction_extension::AsTransactionExtension;
pub use dispatch_transaction::DispatchTransaction;
+/// Provides `Sealed` trait.
+mod private {
+ /// Special trait that prevents the implementation of some traits outside of this crate.
+ pub trait Sealed {}
+}
+
+/// The base implication in a transaction.
+///
+/// This struct is used to represent the base implication in the transaction, that is
+/// the implication not part of any transaction extensions. It usually comprises of the call and
+/// the transaction extension version.
+///
+/// The concept of implication in the transaction extension pipeline is explained in the trait
+/// documentation: [`TransactionExtension`].
+#[derive(Encode)]
+pub struct TxBaseImplication<T>(pub T);
+
+impl<T: Encode> Implication for TxBaseImplication<T> {
+ fn parts(&self) -> ImplicationParts<&impl Encode, &impl Encode, &impl Encode> {
+ ImplicationParts { base: self, explicit: &(), implicit: &() }
+ }
+}
+
+impl<T> private::Sealed for TxBaseImplication<T> {}
+
+/// The implication in a transaction.
+///
+/// The concept of implication in the transaction extension pipeline is explained in the trait
+/// documentation: [`TransactionExtension`].
+#[derive(Encode)]
+pub struct ImplicationParts<Base, Explicit, Implicit> {
+ /// The base implication, that is implication not part of any transaction extension, usually
+ /// the call and the transaction extension version.
+ pub base: Base,
+ /// The explicit implication in transaction extensions.
+ pub explicit: Explicit,
+ /// The implicit implication in transaction extensions.
+ pub implicit: Implicit,
+}
+
+impl<Base: Encode, Explicit: Encode, Implicit: Encode> Implication
+ for ImplicationParts<Base, Explicit, Implicit>
+{
+ fn parts(&self) -> ImplicationParts<&impl Encode, &impl Encode, &impl Encode> {
+ ImplicationParts { base: &self.base, explicit: &self.explicit, implicit: &self.implicit }
+ }
+}
+
+impl<Base, Explicit, Implicit> private::Sealed for ImplicationParts<Base, Explicit, Implicit> {}
+
+/// Interface of implications in the transaction extension pipeline.
+///
+/// Implications can be encoded, this is useful for checking signature on the implications.
+/// Implications can be split into parts, this allow to destructure and restructure the
+/// implications, this is useful for nested pipeline.
+///
+/// This trait is sealed, consider using [`TxBaseImplication`] and [`ImplicationParts`]
+/// implementations.
+///
+/// The concept of implication in the transaction extension pipeline is explained in the trait
+/// documentation: [`TransactionExtension`].
+pub trait Implication: Encode + private::Sealed {
+ /// Destructure the implication into its parts.
+ fn parts(&self) -> ImplicationParts<&impl Encode, &impl Encode, &impl Encode>;
+}
+
/// Shortcut for the result value of the `validate` function.
pub type ValidateResult<Val, Call> =
Result<(ValidTransaction, Val, DispatchOriginOf<Call>), TransactionValidityError>;
@@ -244,7 +310,7 @@ pub trait TransactionExtension<Call: Dispatchable>:
info: &DispatchInfoOf<Call>,
len: usize,
self_implicit: Self::Implicit,
- inherited_implication: &impl Encode,
+ inherited_implication: &impl Implication,
source: TransactionSource,
) -> ValidateResult<Self::Val, Call>;
@@ -499,7 +565,7 @@ impl<Call: Dispatchable> TransactionExtension<Call> for Tuple {
info: &DispatchInfoOf<Call>,
len: usize,
self_implicit: Self::Implicit,
- inherited_implication: &impl Encode,
+ inherited_implication: &impl Implication,
source: TransactionSource,
) -> Result<
(ValidTransaction, Self::Val, <Call as Dispatchable>::RuntimeOrigin),
@@ -510,23 +576,20 @@ impl<Call: Dispatchable> TransactionExtension<Call> for Tuple {
let following_explicit_implications = for_tuples!( ( #( &self.Tuple ),* ) );
let following_implicit_implications = self_implicit;
+ let implication_parts = inherited_implication.parts();
+
for_tuples!(#(
// Implication of this pipeline element not relevant for later items, so we pop it.
let (_item, following_explicit_implications) = following_explicit_implications.pop_front();
let (item_implicit, following_implicit_implications) = following_implicit_implications.pop_front();
let (item_valid, item_val, origin) = {
- let implications = (
- // The first is the implications born of the fact we return the mutated
- // origin.
- inherited_implication,
- // This is the explicitly made implication born of the fact the new origin is
- // passed into the next items in this pipeline-tuple.
- &following_explicit_implications,
- // This is the implicitly made implication born of the fact the new origin is
- // passed into the next items in this pipeline-tuple.
- &following_implicit_implications,
- );
- Tuple.validate(origin, call, info, len, item_implicit, &implications, source)?
+ Tuple.validate(origin, call, info, len, item_implicit,
+ &ImplicationParts {
+ base: implication_parts.base,
+ explicit: (&following_explicit_implications, implication_parts.explicit),
+ implicit: (&following_implicit_implications, implication_parts.implicit),
+ },
+ source)?
};
let valid = valid.combine_with(item_valid);
let val = val.push_back(item_val);
@@ -620,7 +683,7 @@ impl<Call: Dispatchable> TransactionExtension<Call> for () {
_info: &DispatchInfoOf<Call>,
_len: usize,
_self_implicit: Self::Implicit,
- _inherited_implication: &impl Encode,
+ _inherited_implication: &impl Implication,
_source: TransactionSource,
) -> Result<
(ValidTransaction, (), <Call as Dispatchable>::RuntimeOrigin),
@@ -639,3 +702,168 @@ impl<Call: Dispatchable> TransactionExtension<Call> for () {
Ok(())
}
}
+
+#[cfg(test)]
+mod test {
+ use super::*;
+
+ #[test]
+ fn test_implications_on_nested_structure() {
+ use scale_info::TypeInfo;
+ use std::cell::RefCell;
+
+ #[derive(Clone, Debug, Eq, PartialEq, Encode, Decode, TypeInfo)]
+ struct MockExtension {
+ also_implicit: u8,
+ explicit: u8,
+ }
+
+ const CALL_IMPLICIT: u8 = 23;
+
+ thread_local! {
+ static COUNTER: RefCell<u8> = RefCell::new(1);
+ }
+
+ impl TransactionExtension<()> for MockExtension {
+ const IDENTIFIER: &'static str = "MockExtension";
+ type Implicit = u8;
+ fn implicit(&self) -> Result<Self::Implicit, TransactionValidityError> {
+ Ok(self.also_implicit)
+ }
+ type Val = ();
+ type Pre = ();
+ fn weight(&self, _call: &()) -> Weight {
+ Weight::zero()
+ }
+ fn prepare(
+ self,
+ _val: Self::Val,
+ _origin: &DispatchOriginOf<()>,
+ _call: &(),
+ _info: &DispatchInfoOf<()>,
+ _len: usize,
+ ) -> Result<Self::Pre, TransactionValidityError> {
+ Ok(())
+ }
+ fn validate(
+ &self,
+ origin: DispatchOriginOf<()>,
+ _call: &(),
+ _info: &DispatchInfoOf<()>,
+ _len: usize,
+ self_implicit: Self::Implicit,
+ inherited_implication: &impl Implication,
+ _source: TransactionSource,
+ ) -> ValidateResult<Self::Val, ()> {
+ COUNTER.with(|c| {
+ let mut counter = c.borrow_mut();
+
+ assert_eq!(self_implicit, *counter);
+ assert_eq!(
+ self,
+ &MockExtension { also_implicit: *counter, explicit: *counter + 1 }
+ );
+
+ // Implications must be call then 1 to 22 then 1 to 22 odd.
+ let mut assert_implications = Vec::new();
+ assert_implications.push(CALL_IMPLICIT);
+ for i in *counter + 2..23 {
+ assert_implications.push(i);
+ }
+ for i in *counter + 2..23 {
+ if i % 2 == 1 {
+ assert_implications.push(i);
+ }
+ }
+ assert_eq!(inherited_implication.encode(), assert_implications);
+
+ *counter += 2;
+ });
+ Ok((ValidTransaction::default(), (), origin))
+ }
+ fn post_dispatch_details(
+ _pre: Self::Pre,
+ _info: &DispatchInfoOf<()>,
+ _post_info: &PostDispatchInfoOf<()>,
+ _len: usize,
+ _result: &DispatchResult,
+ ) -> Result<Weight, TransactionValidityError> {
+ Ok(Weight::zero())
+ }
+ }
+
+ // Test for one nested structure
+
+ let ext = (
+ MockExtension { also_implicit: 1, explicit: 2 },
+ MockExtension { also_implicit: 3, explicit: 4 },
+ (
+ MockExtension { also_implicit: 5, explicit: 6 },
+ MockExtension { also_implicit: 7, explicit: 8 },
+ (
+ MockExtension { also_implicit: 9, explicit: 10 },
+ MockExtension { also_implicit: 11, explicit: 12 },
+ ),
+ MockExtension { also_implicit: 13, explicit: 14 },
+ MockExtension { also_implicit: 15, explicit: 16 },
+ ),
+ MockExtension { also_implicit: 17, explicit: 18 },
+ (MockExtension { also_implicit: 19, explicit: 20 },),
+ MockExtension { also_implicit: 21, explicit: 22 },
+ );
+
+ let implicit = ext.implicit().unwrap();
+
+ let res = ext
+ .validate(
+ (),
+ &(),
+ &DispatchInfoOf::<()>::default(),
+ 0,
+ implicit,
+ &TxBaseImplication(CALL_IMPLICIT),
+ TransactionSource::Local,
+ )
+ .expect("valid");
+
+ assert_eq!(res.0, ValidTransaction::default());
+
+ // Test for another nested structure
+
+ COUNTER.with(|c| {
+ *c.borrow_mut() = 1;
+ });
+
+ let ext = (
+ MockExtension { also_implicit: 1, explicit: 2 },
+ MockExtension { also_implicit: 3, explicit: 4 },
+ MockExtension { also_implicit: 5, explicit: 6 },
+ MockExtension { also_implicit: 7, explicit: 8 },
+ MockExtension { also_implicit: 9, explicit: 10 },
+ MockExtension { also_implicit: 11, explicit: 12 },
+ (
+ MockExtension { also_implicit: 13, explicit: 14 },
+ MockExtension { also_implicit: 15, explicit: 16 },
+ MockExtension { also_implicit: 17, explicit: 18 },
+ MockExtension { also_implicit: 19, explicit: 20 },
+ MockExtension { also_implicit: 21, explicit: 22 },
+ ),
+ );
+
+ let implicit = ext.implicit().unwrap();
+
+ let res = ext
+ .validate(
+ (),
+ &(),
+ &DispatchInfoOf::<()>::default(),
+ 0,
+ implicit,
+ &TxBaseImplication(CALL_IMPLICIT),
+ TransactionSource::Local,
+ )
+ .expect("valid");
+
+ assert_eq!(res.0, ValidTransaction::default());
+ }
+}