Skip to content

Commit

Permalink
[Tx ext stage 2: 1/4] Add TransactionSource as argument in `Transac…
Browse files Browse the repository at this point in the history
…tionExtension::validate` (#6323)

## Meta 

This PR is part of 4 PR:
* #6323
* #6324
* #6325
* #6326

## Description

One goal of transaction extension is to get rid or unsigned
transactions.
But unsigned transaction validation has access to the
`TransactionSource`.

The source is used for unsigned transactions that the node trust and
don't want to pay upfront.
Instead of using transaction source we could do: the transaction is
valid if it is signed by the block author, conceptually it should work,
but it doesn't look so easy.

This PR add `TransactionSource` to the validate function for transaction
extensions
  • Loading branch information
gui1117 authored Nov 13, 2024
1 parent 5aeaa66 commit 8e3d929
Show file tree
Hide file tree
Showing 27 changed files with 217 additions and 65 deletions.
13 changes: 9 additions & 4 deletions bridges/bin/runtime-common/src/extensions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@ macro_rules! generate_bridge_reject_obsolete_headers_and_messages {
_len: usize,
_self_implicit: Self::Implicit,
_inherited_implication: &impl codec::Encode,
_source: sp_runtime::transaction_validity::TransactionSource,
) -> Result<
(
sp_runtime::transaction_validity::ValidTransaction,
Expand Down Expand Up @@ -390,7 +391,9 @@ mod tests {
parameter_types, AsSystemOriginSigner, AsTransactionAuthorizedOrigin, ConstU64,
DispatchTransaction, Header as _, TransactionExtension,
},
transaction_validity::{InvalidTransaction, TransactionValidity, ValidTransaction},
transaction_validity::{
InvalidTransaction, TransactionSource::External, TransactionValidity, ValidTransaction,
},
DispatchError,
};

Expand Down Expand Up @@ -610,7 +613,8 @@ mod tests {
42u64.into(),
&MockCall { data: 1 },
&(),
0
0,
External,
),
InvalidTransaction::Custom(1)
);
Expand All @@ -629,7 +633,8 @@ mod tests {
42u64.into(),
&MockCall { data: 2 },
&(),
0
0,
External,
),
InvalidTransaction::Custom(2)
);
Expand All @@ -645,7 +650,7 @@ mod tests {

assert_eq!(
BridgeRejectObsoleteHeadersAndMessages
.validate_only(42u64.into(), &MockCall { data: 3 }, &(), 0)
.validate_only(42u64.into(), &MockCall { data: 3 }, &(), 0, External)
.unwrap()
.0,
ValidTransaction { priority: 3, ..Default::default() },
Expand Down
9 changes: 8 additions & 1 deletion bridges/modules/relayers/src/extension/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use bp_runtime::{Chain, RangeInclusiveExt, StaticStrProvider};
use codec::{Decode, Encode};
use frame_support::{
dispatch::{DispatchInfo, PostDispatchInfo},
pallet_prelude::TransactionSource,
weights::Weight,
CloneNoBound, DefaultNoBound, EqNoBound, PartialEqNoBound, RuntimeDebugNoBound,
};
Expand Down Expand Up @@ -304,6 +305,7 @@ where
_len: usize,
_self_implicit: Self::Implicit,
_inherited_implication: &impl Encode,
_source: TransactionSource,
) -> ValidateResult<Self::Val, R::RuntimeCall> {
// Prepare relevant data for `prepare`
let parsed_call = match C::parse_and_check_for_obsolete_call(call)? {
Expand Down Expand Up @@ -463,7 +465,9 @@ mod tests {
use pallet_utility::Call as UtilityCall;
use sp_runtime::{
traits::{ConstU64, DispatchTransaction, Header as HeaderT},
transaction_validity::{InvalidTransaction, TransactionValidity, ValidTransaction},
transaction_validity::{
InvalidTransaction, TransactionSource::External, TransactionValidity, ValidTransaction,
},
DispatchError,
};

Expand Down Expand Up @@ -1076,6 +1080,7 @@ mod tests {
&call,
&DispatchInfo::default(),
0,
External,
)
.map(|t| t.0)
}
Expand All @@ -1088,6 +1093,7 @@ mod tests {
&call,
&DispatchInfo::default(),
0,
External,
)
.map(|t| t.0)
}
Expand All @@ -1100,6 +1106,7 @@ mod tests {
&call,
&DispatchInfo::default(),
0,
External,
)
.map(|t| t.0)
}
Expand Down
11 changes: 7 additions & 4 deletions polkadot/runtime/common/src/claims.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ use sp_runtime::{
Dispatchable, TransactionExtension, Zero,
},
transaction_validity::{
InvalidTransaction, TransactionValidity, TransactionValidityError, ValidTransaction,
InvalidTransaction, TransactionSource, TransactionValidity, TransactionValidityError,
ValidTransaction,
},
RuntimeDebug,
};
Expand Down Expand Up @@ -663,6 +664,7 @@ where
_len: usize,
_self_implicit: Self::Implicit,
_inherited_implication: &impl Encode,
_source: TransactionSource,
) -> Result<
(ValidTransaction, Self::Val, <T::RuntimeCall as Dispatchable>::RuntimeOrigin),
TransactionValidityError,
Expand Down Expand Up @@ -716,6 +718,7 @@ mod tests {
use super::*;
use hex_literal::hex;
use secp_utils::*;
use sp_runtime::transaction_validity::TransactionSource::External;

use codec::Encode;
// The testing primitives are very useful for avoiding having to work with signatures
Expand Down Expand Up @@ -1075,7 +1078,7 @@ mod tests {
});
let di = c.get_dispatch_info();
assert_eq!(di.pays_fee, Pays::No);
let r = p.validate_only(Some(42).into(), &c, &di, 20);
let r = p.validate_only(Some(42).into(), &c, &di, 20, External);
assert_eq!(r.unwrap().0, ValidTransaction::default());
});
}
Expand All @@ -1088,13 +1091,13 @@ mod tests {
statement: StatementKind::Regular.to_text().to_vec(),
});
let di = c.get_dispatch_info();
let r = p.validate_only(Some(42).into(), &c, &di, 20);
let r = p.validate_only(Some(42).into(), &c, &di, 20, External);
assert!(r.is_err());
let c = RuntimeCall::Claims(ClaimsCall::attest {
statement: StatementKind::Saft.to_text().to_vec(),
});
let di = c.get_dispatch_info();
let r = p.validate_only(Some(69).into(), &c, &di, 20);
let r = p.validate_only(Some(69).into(), &c, &di, 20, External);
assert!(r.is_err());
});
}
Expand Down
32 changes: 32 additions & 0 deletions prdoc/pr_6323.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
title: add `TransactionSource` to `TransactionExtension::validate`
doc:
- audience: Runtime Dev
description: |
Add a the source of the extrinsic as an argument in `TransactionExtension::validate`.
The transaction source can be useful for transactions that should only be valid if it comes from the node. For example from offchain worker.
To update the current code. The transaction source can simply be ignored: `_source: TransactionSource`


crates:
- name: sp-runtime
bump: major
- name: bridge-runtime-common
bump: patch
- name: frame-system
bump: patch
- name: pallet-transaction-payment
bump: patch
- name: polkadot-runtime-common
bump: patch
- name: pallet-sudo
bump: patch
- name: pallet-verify-signature
bump: patch
- name: pallet-asset-tx-payment
bump: patch
- name: pallet-bridge-relayers
bump: patch
- name: pallet-asset-conversion-tx-payment
bump: patch
- name: pallet-skip-feeless-payment
bump: patch
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
use core::{fmt, marker::PhantomData};

use codec::{Decode, Encode};
use frame_support::{traits::OriginTrait, Parameter};
use frame_support::{pallet_prelude::TransactionSource, traits::OriginTrait, Parameter};
use scale_info::TypeInfo;
use sp_runtime::{
impl_tx_ext_default,
Expand Down Expand Up @@ -94,6 +94,7 @@ where
_len: usize,
_self_implicit: Self::Implicit,
inherited_implication: &impl codec::Encode,
_source: TransactionSource,
) -> ValidateResult<Self::Val, T::RuntimeCall> {
// If the extension is inactive, just move on in the pipeline.
let Some(auth) = &self.inner else {
Expand Down
2 changes: 2 additions & 0 deletions substrate/frame/examples/basic/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ use codec::{Decode, Encode};
use core::marker::PhantomData;
use frame_support::{
dispatch::{ClassifyDispatch, DispatchClass, DispatchResult, Pays, PaysFee, WeighData},
pallet_prelude::TransactionSource,
traits::IsSubType,
weights::Weight,
};
Expand Down Expand Up @@ -508,6 +509,7 @@ where
len: usize,
_self_implicit: Self::Implicit,
_inherited_implication: &impl Encode,
_source: TransactionSource,
) -> ValidateResult<Self::Val, <T as frame_system::Config>::RuntimeCall> {
// if the transaction is too big, just drop it.
if len > 200 {
Expand Down
5 changes: 3 additions & 2 deletions substrate/frame/examples/basic/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use sp_core::H256;
// or public keys. `u64` is used as the `AccountId` and no `Signature`s are required.
use sp_runtime::{
traits::{BlakeTwo256, DispatchTransaction, IdentityLookup},
transaction_validity::TransactionSource::External,
BuildStorage,
};
// Reexport crate as its pallet name for construct_runtime.
Expand Down Expand Up @@ -146,15 +147,15 @@ fn signed_ext_watch_dummy_works() {

assert_eq!(
WatchDummy::<Test>(PhantomData)
.validate_only(Some(1).into(), &call, &info, 150)
.validate_only(Some(1).into(), &call, &info, 150, External)
.unwrap()
.0
.priority,
u64::MAX,
);
assert_eq!(
WatchDummy::<Test>(PhantomData)
.validate_only(Some(1).into(), &call, &info, 250)
.validate_only(Some(1).into(), &call, &info, 250, External)
.unwrap_err(),
InvalidTransaction::ExhaustsResources.into(),
);
Expand Down
3 changes: 2 additions & 1 deletion substrate/frame/sudo/src/extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
use crate::{Config, Key};
use codec::{Decode, Encode};
use core::{fmt, marker::PhantomData};
use frame_support::{dispatch::DispatchInfo, ensure};
use frame_support::{dispatch::DispatchInfo, ensure, pallet_prelude::TransactionSource};
use scale_info::TypeInfo;
use sp_runtime::{
impl_tx_ext_default,
Expand Down Expand Up @@ -94,6 +94,7 @@ where
_len: usize,
_self_implicit: Self::Implicit,
_inherited_implication: &impl Encode,
_source: TransactionSource,
) -> Result<
(
ValidTransaction,
Expand Down
11 changes: 9 additions & 2 deletions substrate/frame/system/src/extensions/check_mortality.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

use crate::{pallet_prelude::BlockNumberFor, BlockHash, Config, Pallet};
use codec::{Decode, Encode};
use frame_support::pallet_prelude::TransactionSource;
use scale_info::TypeInfo;
use sp_runtime::{
generic::Era,
Expand Down Expand Up @@ -91,6 +92,7 @@ impl<T: Config + Send + Sync> TransactionExtension<T::RuntimeCall> for CheckMort
_len: usize,
_self_implicit: Self::Implicit,
_inherited_implication: &impl Encode,
_source: TransactionSource,
) -> ValidateResult<Self::Val, T::RuntimeCall> {
let current_u64 = <Pallet<T>>::block_number().saturated_into::<u64>();
let valid_till = self.0.death(current_u64);
Expand All @@ -115,7 +117,9 @@ mod tests {
weights::Weight,
};
use sp_core::H256;
use sp_runtime::traits::DispatchTransaction;
use sp_runtime::{
traits::DispatchTransaction, transaction_validity::TransactionSource::External,
};

#[test]
fn signed_ext_check_era_should_work() {
Expand Down Expand Up @@ -151,7 +155,10 @@ mod tests {
<BlockHash<Test>>::insert(16, H256::repeat_byte(1));

assert_eq!(
ext.validate_only(Some(1).into(), CALL, &normal, len).unwrap().0.longevity,
ext.validate_only(Some(1).into(), CALL, &normal, len, External)
.unwrap()
.0
.longevity,
15
);
})
Expand Down
12 changes: 7 additions & 5 deletions substrate/frame/system/src/extensions/check_non_zero_sender.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
use crate::Config;
use codec::{Decode, Encode};
use core::marker::PhantomData;
use frame_support::{traits::OriginTrait, DefaultNoBound};
use frame_support::{pallet_prelude::TransactionSource, traits::OriginTrait, DefaultNoBound};
use scale_info::TypeInfo;
use sp_runtime::{
impl_tx_ext_default,
Expand Down Expand Up @@ -68,6 +68,7 @@ impl<T: Config + Send + Sync> TransactionExtension<T::RuntimeCall> for CheckNonZ
_len: usize,
_self_implicit: Self::Implicit,
_inherited_implication: &impl Encode,
_source: TransactionSource,
) -> sp_runtime::traits::ValidateResult<Self::Val, T::RuntimeCall> {
if let Some(who) = origin.as_signer() {
if who.using_encoded(|d| d.iter().all(|x| *x == 0)) {
Expand All @@ -86,7 +87,7 @@ mod tests {
use frame_support::{assert_ok, dispatch::DispatchInfo};
use sp_runtime::{
traits::{AsTransactionAuthorizedOrigin, DispatchTransaction},
transaction_validity::TransactionValidityError,
transaction_validity::{TransactionSource::External, TransactionValidityError},
};

#[test]
Expand All @@ -96,15 +97,16 @@ mod tests {
let len = 0_usize;
assert_eq!(
CheckNonZeroSender::<Test>::new()
.validate_only(Some(0).into(), CALL, &info, len)
.validate_only(Some(0).into(), CALL, &info, len, External)
.unwrap_err(),
TransactionValidityError::from(InvalidTransaction::BadSigner)
);
assert_ok!(CheckNonZeroSender::<Test>::new().validate_only(
Some(1).into(),
CALL,
&info,
len
len,
External,
));
})
}
Expand All @@ -115,7 +117,7 @@ mod tests {
let info = DispatchInfo::default();
let len = 0_usize;
let (_, _, origin) = CheckNonZeroSender::<Test>::new()
.validate(None.into(), CALL, &info, len, (), CALL)
.validate(None.into(), CALL, &info, len, (), CALL, External)
.unwrap();
assert!(!origin.is_transaction_authorized());
})
Expand Down
Loading

0 comments on commit 8e3d929

Please sign in to comment.