From b32603251dc3267a006875a2ee9e2ac5f932780f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Mon, 23 Mar 2020 15:30:27 +0100 Subject: [PATCH 01/15] WiP --- bin/node/runtime/src/lib.rs | 7 +++-- frame/executive/src/lib.rs | 12 ++++---- .../runtime/src/generic/checked_extrinsic.rs | 7 +++-- primitives/runtime/src/testing.rs | 3 +- primitives/runtime/src/traits.rs | 8 ++++-- .../runtime/src/transaction_validity.rs | 28 +++++++++++++++++++ .../transaction-pool/src/runtime_api.rs | 18 ++++++++++-- 7 files changed, 68 insertions(+), 15 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 58728a507bce2..04a9b860dc485 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -35,7 +35,7 @@ use sp_runtime::{ impl_opaque_keys, generic, create_runtime_str, }; use sp_runtime::curve::PiecewiseLinear; -use sp_runtime::transaction_validity::TransactionValidity; +use sp_runtime::transaction_validity::{TransactionValidity, TransactionSource}; use sp_runtime::traits::{ self, BlakeTwo256, Block as BlockT, StaticLookup, SaturatedConversion, ConvertInto, OpaqueKeys, @@ -735,7 +735,10 @@ impl_runtime_apis! { } impl sp_transaction_pool::runtime_api::TaggedTransactionQueue for Runtime { - fn validate_transaction(tx: ::Extrinsic) -> TransactionValidity { + fn validate_transaction( + tx: ::Extrinsic, + source: TransactionSource, + ) -> TransactionValidity { Executive::validate_transaction(tx) } } diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 1b3867cf6e01d..b404ca3d092f6 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -59,12 +59,14 @@ //! # pub type Balances = u64; //! # pub type AllModules = u64; //! # pub enum Runtime {}; -//! # use sp_runtime::transaction_validity::{TransactionValidity, UnknownTransaction}; +//! # use sp_runtime::transaction_validity::{ +//! # TransactionValidity, UnknownTransaction, TransactionSource, +//! # }; //! # use sp_runtime::traits::ValidateUnsigned; //! # impl ValidateUnsigned for Runtime { //! # type Call = (); //! # -//! # fn validate_unsigned(_call: &Self::Call) -> TransactionValidity { +//! # fn validate_unsigned(_call: &Self::Call, _origin: TransactionSource) -> TransactionValidity { //! # UnknownTransaction::NoUnsignedValidator.into() //! # } //! # } @@ -82,7 +84,7 @@ use sp_runtime::{ self, Header, Zero, One, Checkable, Applyable, CheckEqual, OnFinalize, OnInitialize, NumberFor, Block as BlockT, OffchainWorker, Dispatchable, Saturating, OnRuntimeUpgrade, }, - transaction_validity::TransactionValidity, + transaction_validity::{TransactionValidity, TransactionSource}, }; use sp_runtime::traits::ValidateUnsigned; use codec::{Codec, Encode}; @@ -345,12 +347,12 @@ where /// side-effects; it merely checks whether the transaction would panic if it were included or not. /// /// Changes made to storage should be discarded. - pub fn validate_transaction(uxt: Block::Extrinsic) -> TransactionValidity { + pub fn validate_transaction(uxt: Block::Extrinsic, source: TransactionSource) -> TransactionValidity { let encoded_len = uxt.using_encoded(|d| d.len()); let xt = uxt.check(&Default::default())?; let dispatch_info = xt.get_dispatch_info(); - xt.validate::(dispatch_info, encoded_len) + xt.validate::(source, dispatch_info, encoded_len) } /// Start an offchain worker and generate extrinsics. diff --git a/primitives/runtime/src/generic/checked_extrinsic.rs b/primitives/runtime/src/generic/checked_extrinsic.rs index 25a8274354ae2..aa30b861269c6 100644 --- a/primitives/runtime/src/generic/checked_extrinsic.rs +++ b/primitives/runtime/src/generic/checked_extrinsic.rs @@ -21,7 +21,7 @@ use crate::traits::{ self, Member, MaybeDisplay, SignedExtension, Dispatchable, }; use crate::traits::ValidateUnsigned; -use crate::transaction_validity::TransactionValidity; +use crate::transaction_validity::{TransactionValidity, TransactionSource}; /// Definition of something that the external world might want to say; its /// existence implies that it has been checked and is good, particularly with @@ -50,6 +50,9 @@ where fn validate>( &self, + // TODO [ToDr] should source be passed to `SignedExtension`s? + // Perhaps a change for 2.0 to avoid breaking too much APIs? + source: TransactionSource, info: Self::DispatchInfo, len: usize, ) -> TransactionValidity { @@ -57,7 +60,7 @@ where Extra::validate(extra, id, &self.function, info.clone(), len) } else { let valid = Extra::validate_unsigned(&self.function, info, len)?; - let unsigned_validation = U::validate_unsigned(&self.function)?; + let unsigned_validation = U::validate_unsigned(source, &self.function)?; Ok(valid.combine_with(unsigned_validation)) } } diff --git a/primitives/runtime/src/testing.rs b/primitives/runtime/src/testing.rs index 2439070054211..b3139828c1d11 100644 --- a/primitives/runtime/src/testing.rs +++ b/primitives/runtime/src/testing.rs @@ -27,7 +27,7 @@ use crate::traits::ValidateUnsigned; use crate::{generic, KeyTypeId, ApplyExtrinsicResult}; pub use sp_core::{H256, sr25519}; use sp_core::{crypto::{CryptoType, Dummy, key_types, Public}, U256}; -use crate::transaction_validity::{TransactionValidity, TransactionValidityError}; +use crate::transaction_validity::{TransactionValidity, TransactionValidityError, TransactionSource}; /// Authority Id #[derive(Default, PartialEq, Eq, Clone, Encode, Decode, Debug, Hash, Serialize, Deserialize, PartialOrd, Ord)] @@ -357,6 +357,7 @@ impl Applyable for TestXt where /// Checks to see if this is a valid *transaction*. It returns information on it if so. fn validate>( &self, + _source: TransactionSource, _info: Self::DispatchInfo, _len: usize, ) -> TransactionValidity { diff --git a/primitives/runtime/src/traits.rs b/primitives/runtime/src/traits.rs index 81b7733319bf8..dba729d05f9ce 100644 --- a/primitives/runtime/src/traits.rs +++ b/primitives/runtime/src/traits.rs @@ -28,7 +28,8 @@ use serde::{Serialize, Deserialize, de::DeserializeOwned}; use sp_core::{self, Hasher, TypeId, RuntimeDebug}; use crate::codec::{Codec, Encode, Decode}; use crate::transaction_validity::{ - ValidTransaction, TransactionValidity, TransactionValidityError, UnknownTransaction, + ValidTransaction, TransactionSource, TransactionValidity, TransactionValidityError, + UnknownTransaction, }; use crate::generic::{Digest, DigestItem}; pub use sp_arithmetic::traits::{ @@ -896,6 +897,7 @@ pub trait Applyable: Sized + Send + Sync { /// Checks to see if this is a valid *transaction*. It returns information on it if so. fn validate>( &self, + source: TransactionSource, info: Self::DispatchInfo, len: usize, ) -> TransactionValidity; @@ -942,7 +944,7 @@ pub trait ValidateUnsigned { /// /// Changes made to storage WILL be persisted if the call returns `Ok`. fn pre_dispatch(call: &Self::Call) -> Result<(), TransactionValidityError> { - Self::validate_unsigned(call) + Self::validate_unsigned(TransactionSource::InBlock, call) .map(|_| ()) .map_err(Into::into) } @@ -953,7 +955,7 @@ pub trait ValidateUnsigned { /// whether the transaction would panic if it were included or not. /// /// Changes made to storage should be discarded by caller. - fn validate_unsigned(call: &Self::Call) -> TransactionValidity; + fn validate_unsigned(source: TransactionSource, call: &Self::Call) -> TransactionValidity; } /// Opaque data type that may be destructured into a series of raw byte slices (which represent diff --git a/primitives/runtime/src/transaction_validity.rs b/primitives/runtime/src/transaction_validity.rs index 6dfb80b6d7e45..9f7f39697d5c2 100644 --- a/primitives/runtime/src/transaction_validity.rs +++ b/primitives/runtime/src/transaction_validity.rs @@ -161,6 +161,34 @@ impl Into for UnknownTransaction { } } +/// The origin of the transaction. +/// +/// Depending on the source we might apply different validation schemes. +/// For instance we can disallow specific kinds of transactions if they were not produced +/// by our local node (for instance off-chain workers). +pub enum TransactionSource { + /// Transaction is already included in block. + /// + /// This means that we can't really tell where the transaction is coming from, + /// since it's already in the received block. Note that the custom validator + /// using either `Local` or `External` should most likely just allow `InBlock` + /// transactions as well. + InBlock, + + /// Transaction is coming from a local source. + /// + /// This means that the transaction was produced either internally by the node + /// (for instance an Off-Chain Worker, or an Off-Chain Call) opposed + /// to being received over the network. + Local, + + /// Transaction has been received externally. + /// + /// This means the transaction has been received from (usually) "untrusted" source, + /// for instance received over the network or RPC. + External, +} + /// Information concerning a valid transaction. #[derive(Clone, PartialEq, Eq, Encode, Decode, RuntimeDebug)] pub struct ValidTransaction { diff --git a/primitives/transaction-pool/src/runtime_api.rs b/primitives/transaction-pool/src/runtime_api.rs index e30ce7c8289da..cc1c62ec075bf 100644 --- a/primitives/transaction-pool/src/runtime_api.rs +++ b/primitives/transaction-pool/src/runtime_api.rs @@ -16,13 +16,27 @@ //! Tagged Transaction Queue Runtime API. -use sp_runtime::transaction_validity::TransactionValidity; +use sp_runtime::transaction_validity::{TransactionValidity, TransactionSource}; use sp_runtime::traits::Block as BlockT; sp_api::decl_runtime_apis! { /// The `TaggedTransactionQueue` api trait for interfering with the transaction queue. + #[api_version(2)] pub trait TaggedTransactionQueue { - /// Validate the given transaction. + /// Validate the transaction. + #[changed_in(2)] fn validate_transaction(tx: ::Extrinsic) -> TransactionValidity; + + /// Validate the transaction. + /// + /// This method is invoked by the transaction pool to learn details about given transaction. + /// The implementation should make sure to verify the correctness of the transaction + /// against current state. + /// Note that this call may be performed by the pool multiple times and transactions + /// might be verified in any possible order. + fn validate_transaction( + tx: ::Extrinsic, + source: TransactionSource, + ) -> TransactionValidity; } } From daf9a8ec1b98cc0fb8790a3dd19b536bf4168637 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Mon, 23 Mar 2020 16:39:02 +0100 Subject: [PATCH 02/15] Support source in the runtime API. --- bin/node-template/runtime/src/lib.rs | 11 +++++---- bin/node/runtime/src/lib.rs | 4 ++-- client/transaction-pool/src/api.rs | 20 ++++++++++++---- frame/example-offchain-worker/src/lib.rs | 9 +++++-- frame/executive/src/lib.rs | 24 ++++++++++++++----- frame/im-online/src/benchmarking.rs | 3 ++- frame/im-online/src/lib.rs | 7 ++++-- .../procedural/src/construct_runtime/mod.rs | 7 +++--- frame/support/src/unsigned.rs | 19 +++++++++++---- .../runtime/src/transaction_validity.rs | 1 + .../transaction-pool/src/runtime_api.rs | 2 +- test-utils/runtime/src/lib.rs | 11 +++++++-- 12 files changed, 86 insertions(+), 32 deletions(-) diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index 0414759a5ad86..94f033fd8f58e 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -11,8 +11,8 @@ include!(concat!(env!("OUT_DIR"), "/wasm_binary.rs")); use sp_std::prelude::*; use sp_core::OpaqueMetadata; use sp_runtime::{ - ApplyExtrinsicResult, transaction_validity::TransactionValidity, generic, create_runtime_str, - impl_opaque_keys, MultiSignature, + ApplyExtrinsicResult, generic, create_runtime_str, impl_opaque_keys, MultiSignature, + transaction_validity::{TransactionValidity, TransactionSource}, }; use sp_runtime::traits::{ BlakeTwo256, Block as BlockT, IdentityLookup, Verify, ConvertInto, IdentifyAccount @@ -318,8 +318,11 @@ impl_runtime_apis! { } impl sp_transaction_pool::runtime_api::TaggedTransactionQueue for Runtime { - fn validate_transaction(tx: ::Extrinsic) -> TransactionValidity { - Executive::validate_transaction(tx) + fn validate_transaction( + source: TransactionSource, + tx: ::Extrinsic, + ) -> TransactionValidity { + Executive::validate_transaction(source, tx) } } diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 04a9b860dc485..d587eaf0117cb 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -736,10 +736,10 @@ impl_runtime_apis! { impl sp_transaction_pool::runtime_api::TaggedTransactionQueue for Runtime { fn validate_transaction( - tx: ::Extrinsic, source: TransactionSource, + tx: ::Extrinsic, ) -> TransactionValidity { - Executive::validate_transaction(tx) + Executive::validate_transaction(source, tx) } } diff --git a/client/transaction-pool/src/api.rs b/client/transaction-pool/src/api.rs index a8888baa4839b..c942f60577f25 100644 --- a/client/transaction-pool/src/api.rs +++ b/client/transaction-pool/src/api.rs @@ -29,10 +29,10 @@ use sc_client_api::{ }; use sp_runtime::{ generic::BlockId, traits::{self, Block as BlockT, BlockIdTo, Header as HeaderT, Hash as HashT}, - transaction_validity::TransactionValidity, + transaction_validity::{TransactionValidity, TransactionSource}, }; use sp_transaction_pool::runtime_api::TaggedTransactionQueue; -use sp_api::ProvideRuntimeApi; +use sp_api::{ProvideRuntimeApi, ApiExt}; use crate::error::{self, Error}; @@ -88,8 +88,20 @@ impl sc_transaction_graph::ChainApi for FullChainApi, _>( + &at, |v| v >= 2, + ) + .unwrap_or_default(); + let res = if has_v2 { + // TODO [ToDr] source + runtime_api.validate_transaction(&at, TransactionSource::External, uxt) + } else { + #[allow(deprecated)] // old validate_transaction + runtime_api.validate_transaction_before_version_2(&at, uxt) + }; + let res = res.map_err(|e| Error::RuntimeApi(format!("{:?}", e))); if let Err(e) = tx.send(res) { log::warn!("Unable to send a validate transaction result: {:?}", e); } diff --git a/frame/example-offchain-worker/src/lib.rs b/frame/example-offchain-worker/src/lib.rs index 5a417fa0782f3..e64b3dfa7757c 100644 --- a/frame/example-offchain-worker/src/lib.rs +++ b/frame/example-offchain-worker/src/lib.rs @@ -51,7 +51,9 @@ use sp_core::crypto::KeyTypeId; use sp_runtime::{ offchain::{http, Duration, storage::StorageValueRef}, traits::Zero, - transaction_validity::{InvalidTransaction, ValidTransaction, TransactionValidity}, + transaction_validity::{ + InvalidTransaction, ValidTransaction, TransactionValidity, TransactionSource, + }, }; use sp_std::{vec, vec::Vec}; use lite_json::json::JsonValue; @@ -509,7 +511,10 @@ impl frame_support::unsigned::ValidateUnsigned for Module { /// By default unsigned transactions are disallowed, but implementing the validator /// here we make sure that some particular calls (the ones produced by offchain worker) /// are being whitelisted and marked as valid. - fn validate_unsigned(call: &Self::Call) -> TransactionValidity { + fn validate_unsigned( + _source: TransactionSource, + call: &Self::Call, + ) -> TransactionValidity { // Firstly let's check that we call the right function. if let Call::submit_price_unsigned(block_number, new_price) = call { // Now let's check if the transaction has any chance to succeed. diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index b404ca3d092f6..7fb587659c6b1 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -60,13 +60,16 @@ //! # pub type AllModules = u64; //! # pub enum Runtime {}; //! # use sp_runtime::transaction_validity::{ -//! # TransactionValidity, UnknownTransaction, TransactionSource, -//! # }; +//! # TransactionValidity, UnknownTransaction, TransactionSource, +//! # }; //! # use sp_runtime::traits::ValidateUnsigned; //! # impl ValidateUnsigned for Runtime { //! # type Call = (); //! # -//! # fn validate_unsigned(_call: &Self::Call, _origin: TransactionSource) -> TransactionValidity { +//! # fn validate_unsigned( +//! # _source: TransactionSource, +//! # _call: &Self::Call, +//! # ) -> TransactionValidity { //! # UnknownTransaction::NoUnsignedValidator.into() //! # } //! # } @@ -347,7 +350,10 @@ where /// side-effects; it merely checks whether the transaction would panic if it were included or not. /// /// Changes made to storage should be discarded. - pub fn validate_transaction(uxt: Block::Extrinsic, source: TransactionSource) -> TransactionValidity { + pub fn validate_transaction( + source: TransactionSource, + uxt: Block::Extrinsic, + ) -> TransactionValidity { let encoded_len = uxt.using_encoded(|d| d.len()); let xt = uxt.check(&Default::default())?; @@ -520,7 +526,10 @@ mod tests { Ok(()) } - fn validate_unsigned(call: &Self::Call) -> TransactionValidity { + fn validate_unsigned( + _source: TransactionSource, + call: &Self::Call, + ) -> TransactionValidity { match call { Call::Balances(BalancesCall::set_balance(_, _, _)) => Ok(Default::default()), _ => UnknownTransaction::NoUnsignedValidator.into(), @@ -734,7 +743,10 @@ mod tests { let mut t = new_test_ext(1); t.execute_with(|| { - assert_eq!(Executive::validate_transaction(xt.clone()), Ok(Default::default())); + assert_eq!( + Executive::validate_transaction(TransactionSource::InBlock, xt.clone()), + Ok(Default::default()) + ); assert_eq!(Executive::apply_extrinsic(xt), Ok(Err(DispatchError::BadOrigin))); }); } diff --git a/frame/im-online/src/benchmarking.rs b/frame/im-online/src/benchmarking.rs index 1269328634894..973bd0c36147c 100644 --- a/frame/im-online/src/benchmarking.rs +++ b/frame/im-online/src/benchmarking.rs @@ -24,6 +24,7 @@ use frame_system::RawOrigin; use frame_benchmarking::benchmarks; use sp_core::offchain::{OpaquePeerId, OpaqueMultiaddr}; use sp_runtime::traits::{ValidateUnsigned, Zero}; +use sp_runtime::transaction_validity::TransactionSource; use crate::Module as ImOnline; @@ -72,7 +73,7 @@ benchmarks! { let (input_heartbeat, signature) = create_heartbeat::(k, e)?; let call = Call::heartbeat(input_heartbeat, signature); }: { - ImOnline::::validate_unsigned(&call)?; + ImOnline::::validate_unsigned(TransactionSource::InBlock, &call)?; } } diff --git a/frame/im-online/src/lib.rs b/frame/im-online/src/lib.rs index 861c57e5b61bc..a164c379fba77 100644 --- a/frame/im-online/src/lib.rs +++ b/frame/im-online/src/lib.rs @@ -82,7 +82,7 @@ use sp_runtime::{ RuntimeDebug, traits::{Convert, Member, Saturating, AtLeast32Bit}, Perbill, PerThing, transaction_validity::{ - TransactionValidity, ValidTransaction, InvalidTransaction, + TransactionValidity, ValidTransaction, InvalidTransaction, TransactionSource, TransactionPriority, }, }; @@ -624,7 +624,10 @@ impl pallet_session::OneSessionHandler for Module { impl frame_support::unsigned::ValidateUnsigned for Module { type Call = Call; - fn validate_unsigned(call: &Self::Call) -> TransactionValidity { + fn validate_unsigned( + _source: TransactionSource, + call: &Self::Call, + ) -> TransactionValidity { if let Call::heartbeat(heartbeat, signature) = call { if >::is_online(heartbeat.authority_index) { // we already received a heartbeat for this authority diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index 942a47a533e5f..1c366d0348699 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -89,7 +89,7 @@ fn construct_runtime_parsed(definition: RuntimeDefinition) -> Result Result( diff --git a/frame/support/src/unsigned.rs b/frame/support/src/unsigned.rs index 4289e4e474f53..a4274c8172c5d 100644 --- a/frame/support/src/unsigned.rs +++ b/frame/support/src/unsigned.rs @@ -18,7 +18,7 @@ pub use crate::sp_runtime::traits::ValidateUnsigned; #[doc(hidden)] pub use crate::sp_runtime::transaction_validity::{ - TransactionValidity, UnknownTransaction, TransactionValidityError, + TransactionValidity, UnknownTransaction, TransactionValidityError, TransactionSource, }; @@ -34,7 +34,10 @@ pub use crate::sp_runtime::transaction_validity::{ /// # impl frame_support::unsigned::ValidateUnsigned for Module { /// # type Call = Call; /// # -/// # fn validate_unsigned(call: &Self::Call) -> frame_support::unsigned::TransactionValidity { +/// # fn validate_unsigned( +/// # _source: frame_support::transaction_validity::TransactionSource, +/// # _call: &Self::Call, +/// # ) -> frame_support::unsigned::TransactionValidity { /// # unimplemented!(); /// # } /// # } @@ -78,10 +81,13 @@ macro_rules! impl_outer_validate_unsigned { } } - fn validate_unsigned(call: &Self::Call) -> $crate::unsigned::TransactionValidity { + fn validate_unsigned( + source: $crate::unsigned::TransactionSource, + call: &Self::Call, + ) -> $crate::unsigned::TransactionValidity { #[allow(unreachable_patterns)] match call { - $( Call::$module(inner_call) => $module::validate_unsigned(inner_call), )* + $( Call::$module(inner_call) => $module::validate_unsigned(source, inner_call), )* _ => $crate::unsigned::UnknownTransaction::NoUnsignedValidator.into(), } } @@ -110,7 +116,10 @@ mod test_partial_and_full_call { impl super::super::ValidateUnsigned for Module { type Call = Call; - fn validate_unsigned(_call: &Self::Call) -> super::super::TransactionValidity { + fn validate_unsigned( + _source: super::super::TransactionSource, + _call: &Self::Call + ) -> super::super::TransactionValidity { unimplemented!(); } } diff --git a/primitives/runtime/src/transaction_validity.rs b/primitives/runtime/src/transaction_validity.rs index 9f7f39697d5c2..7c3709521f27f 100644 --- a/primitives/runtime/src/transaction_validity.rs +++ b/primitives/runtime/src/transaction_validity.rs @@ -166,6 +166,7 @@ impl Into for UnknownTransaction { /// Depending on the source we might apply different validation schemes. /// For instance we can disallow specific kinds of transactions if they were not produced /// by our local node (for instance off-chain workers). +#[derive(Clone, PartialEq, Eq, Encode, Decode, RuntimeDebug)] pub enum TransactionSource { /// Transaction is already included in block. /// diff --git a/primitives/transaction-pool/src/runtime_api.rs b/primitives/transaction-pool/src/runtime_api.rs index cc1c62ec075bf..fa2e51653b284 100644 --- a/primitives/transaction-pool/src/runtime_api.rs +++ b/primitives/transaction-pool/src/runtime_api.rs @@ -35,8 +35,8 @@ sp_api::decl_runtime_apis! { /// Note that this call may be performed by the pool multiple times and transactions /// might be verified in any possible order. fn validate_transaction( - tx: ::Extrinsic, source: TransactionSource, + tx: ::Extrinsic, ) -> TransactionValidity; } } diff --git a/test-utils/runtime/src/lib.rs b/test-utils/runtime/src/lib.rs index 59955cce48693..120dd502cdcd6 100644 --- a/test-utils/runtime/src/lib.rs +++ b/test-utils/runtime/src/lib.rs @@ -36,6 +36,7 @@ use sp_runtime::{ ApplyExtrinsicResult, create_runtime_str, Perbill, impl_opaque_keys, transaction_validity::{ TransactionValidity, ValidTransaction, TransactionValidityError, InvalidTransaction, + TransactionSource, }, traits::{ BlindCheckable, BlakeTwo256, Block as BlockT, Extrinsic as ExtrinsicT, @@ -492,7 +493,10 @@ cfg_if! { } impl sp_transaction_pool::runtime_api::TaggedTransactionQueue for Runtime { - fn validate_transaction(utx: ::Extrinsic) -> TransactionValidity { + fn validate_transaction( + source: TransactionSource, + utx: ::Extrinsic, + ) -> TransactionValidity { if let Extrinsic::IncludeData(data) = utx { return Ok(ValidTransaction { priority: data.len() as u64, @@ -679,7 +683,10 @@ cfg_if! { } impl sp_transaction_pool::runtime_api::TaggedTransactionQueue for Runtime { - fn validate_transaction(utx: ::Extrinsic) -> TransactionValidity { + fn validate_transaction( + source: TransactionSource, + utx: ::Extrinsic, + ) -> TransactionValidity { if let Extrinsic::IncludeData(data) = utx { return Ok(ValidTransaction{ priority: data.len() as u64, From 9bc6589ff9c8a2c37eb392ba32ad7d2bd2fa25f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Mon, 23 Mar 2020 17:43:31 +0100 Subject: [PATCH 03/15] Finish implementation in txpool. --- client/rpc/src/author/mod.rs | 14 ++++-- client/service/src/lib.rs | 3 +- client/service/test/src/lib.rs | 3 +- .../transaction-pool/graph/src/base_pool.rs | 4 ++ client/transaction-pool/graph/src/pool.rs | 48 ++++++++++++++----- .../graph/src/validated_pool.rs | 4 +- client/transaction-pool/src/api.rs | 7 +-- client/transaction-pool/src/lib.rs | 19 ++++++-- client/transaction-pool/src/revalidation.rs | 5 +- .../runtime/src/transaction_validity.rs | 2 +- primitives/transaction-pool/src/lib.rs | 2 +- primitives/transaction-pool/src/pool.rs | 9 +++- test-utils/runtime/src/lib.rs | 4 +- .../runtime/transaction-pool/src/lib.rs | 2 + 14 files changed, 92 insertions(+), 34 deletions(-) diff --git a/client/rpc/src/author/mod.rs b/client/rpc/src/author/mod.rs index 80a3a4349ed82..a3f23e8e1437a 100644 --- a/client/rpc/src/author/mod.rs +++ b/client/rpc/src/author/mod.rs @@ -37,7 +37,7 @@ use sp_core::{Bytes, traits::BareCryptoStorePtr}; use sp_api::ProvideRuntimeApi; use sp_runtime::generic; use sp_transaction_pool::{ - TransactionPool, InPoolTransaction, TransactionStatus, + TransactionPool, InPoolTransaction, TransactionStatus, TransactionSource, BlockHash, TxHash, TransactionFor, error::IntoPoolError, }; use sp_session::SessionKeys; @@ -75,6 +75,14 @@ impl Author { } } + +/// Currently we treat all RPC transactions as externals. +/// +/// Possibly in the future we could allow opt-in for special treatment +/// of such transactions, so that the block authors can inject +/// some unique transactions via RPC and have them included in the pool. +const TX_SOURCE: TransactionSource = TransactionSource::External; + impl AuthorApi, BlockHash

> for Author where P: TransactionPool + Sync + Send + 'static, @@ -127,7 +135,7 @@ impl AuthorApi, BlockHash

> for Author }; let best_block_hash = self.client.info().best_hash; Box::new(self.pool - .submit_one(&generic::BlockId::hash(best_block_hash), xt) + .submit_one(&generic::BlockId::hash(best_block_hash), TX_SOURCE, xt) .compat() .map_err(|e| e.into_pool_error() .map(Into::into) @@ -173,7 +181,7 @@ impl AuthorApi, BlockHash

> for Author .map_err(error::Error::from)?; Ok( self.pool - .submit_and_watch(&generic::BlockId::hash(best_block_hash), dxt) + .submit_and_watch(&generic::BlockId::hash(best_block_hash), TX_SOURCE, dxt) .map_err(|e| e.into_pool_error() .map(error::Error::from) .unwrap_or_else(|e| error::Error::Verification(Box::new(e)).into()) diff --git a/client/service/src/lib.rs b/client/service/src/lib.rs index 0a7d5ff103f3c..6d0c93fbf54d7 100644 --- a/client/service/src/lib.rs +++ b/client/service/src/lib.rs @@ -621,7 +621,8 @@ where match Decode::decode(&mut &encoded[..]) { Ok(uxt) => { let best_block_id = BlockId::hash(self.client.info().best_hash); - let import_future = self.pool.submit_one(&best_block_id, uxt); + let source = sp_transaction_pool::TransactionSource::External; + let import_future = self.pool.submit_one(&best_block_id, source, uxt); let import_future = import_future .map(move |import_result| { match import_result { diff --git a/client/service/test/src/lib.rs b/client/service/test/src/lib.rs index 537d44647701e..3e1eda3795217 100644 --- a/client/service/test/src/lib.rs +++ b/client/service/test/src/lib.rs @@ -482,9 +482,10 @@ pub fn sync( let first_user_data = &network.full_nodes[0].2; let best_block = BlockId::number(first_service.get().client().chain_info().best_number); let extrinsic = extrinsic_factory(&first_service.get(), first_user_data); + let source = sp_transaction_pool::TransactionSource::External; futures::executor::block_on( - first_service.get().transaction_pool().submit_one(&best_block, extrinsic) + first_service.get().transaction_pool().submit_one(&best_block, source, extrinsic) ).expect("failed to submit extrinsic"); network.run_until_all_full( diff --git a/client/transaction-pool/graph/src/base_pool.rs b/client/transaction-pool/graph/src/base_pool.rs index 33b7a51f9415b..45fc9a8e837aa 100644 --- a/client/transaction-pool/graph/src/base_pool.rs +++ b/client/transaction-pool/graph/src/base_pool.rs @@ -33,6 +33,7 @@ use sp_runtime::transaction_validity::{ TransactionTag as Tag, TransactionLongevity as Longevity, TransactionPriority as Priority, + TransactionSource as Source, }; use sp_transaction_pool::{error, PoolStatus, InPoolTransaction}; @@ -102,6 +103,8 @@ pub struct Transaction { pub provides: Vec, /// Should that transaction be propagated. pub propagate: bool, + /// Origin of that transaction. + pub source: Source, } impl AsRef for Transaction { @@ -155,6 +158,7 @@ impl Transaction { bytes: self.bytes.clone(), hash: self.hash.clone(), priority: self.priority.clone(), + source: self.source, valid_till: self.valid_till.clone(), requires: self.requires.clone(), provides: self.provides.clone(), diff --git a/client/transaction-pool/graph/src/pool.rs b/client/transaction-pool/graph/src/pool.rs index f0bf17dcb8dd2..7e1fc397fa042 100644 --- a/client/transaction-pool/graph/src/pool.rs +++ b/client/transaction-pool/graph/src/pool.rs @@ -31,7 +31,9 @@ use futures::{ use sp_runtime::{ generic::BlockId, traits::{self, SaturatedConversion}, - transaction_validity::{TransactionValidity, TransactionTag as Tag, TransactionValidityError}, + transaction_validity::{ + TransactionValidity, TransactionTag as Tag, TransactionValidityError, TransactionSource, + }, }; use sp_transaction_pool::error; use wasm_timer::Instant; @@ -76,6 +78,7 @@ pub trait ChainApi: Send + Sync { fn validate_transaction( &self, at: &BlockId, + source: TransactionSource, uxt: ExtrinsicFor, ) -> Self::ValidationFuture; @@ -144,12 +147,17 @@ impl Pool { } /// Imports a bunch of unverified extrinsics to the pool - pub async fn submit_at(&self, at: &BlockId, xts: T, force: bool) - -> Result, B::Error>>, B::Error> - where - T: IntoIterator> + pub async fn submit_at( + &self, + at: &BlockId, + source: TransactionSource, + xts: T, + force: bool, + ) -> Result, B::Error>>, B::Error> where + T: IntoIterator>, { let validated_pool = self.validated_pool.clone(); + let xts = xts.into_iter().map(|xt| (source, xt)); self.verify(at, xts, force) .map(move |validated_transactions| validated_transactions .map(|validated_transactions| validated_pool.submit(validated_transactions @@ -162,9 +170,10 @@ impl Pool { pub async fn submit_one( &self, at: &BlockId, + source: TransactionSource, xt: ExtrinsicFor, ) -> Result, B::Error> { - self.submit_at(at, std::iter::once(xt), false) + self.submit_at(at, source, std::iter::once(xt), false) .map(|import_result| import_result.and_then(|mut import_result| import_result .pop() .expect("One extrinsic passed; one result returned; qed") @@ -176,10 +185,13 @@ impl Pool { pub async fn submit_and_watch( &self, at: &BlockId, + source: TransactionSource, xt: ExtrinsicFor, ) -> Result, BlockHash>, B::Error> { let block_number = self.resolve_block_number(at)?; - let (_, tx) = self.verify_one(at, block_number, xt, false).await; + let (_, tx) = self.verify_one( + at, block_number, source, xt, false + ).await; self.validated_pool.submit_and_watch(tx) } @@ -249,7 +261,7 @@ impl Pool { // to get validity info and tags that the extrinsic provides. None => { let validity = self.validated_pool.api() - .validate_transaction(parent, extrinsic.clone()) + .validate_transaction(parent, TransactionSource::InBlock, extrinsic.clone()) .await; if let Ok(Ok(validity)) = validity { @@ -303,8 +315,12 @@ impl Pool { // Try to re-validate pruned transactions since some of them might be still valid. // note that `known_imported_hashes` will be rejected here due to temporary ban. - let pruned_hashes = prune_status.pruned.iter().map(|tx| tx.hash.clone()).collect::>(); - let pruned_transactions = prune_status.pruned.into_iter().map(|tx| tx.data.clone()); + let pruned_hashes = prune_status.pruned + .iter() + .map(|tx| tx.hash.clone()).collect::>(); + let pruned_transactions = prune_status.pruned + .into_iter() + .map(|tx| (tx.source, tx.data.clone())); let reverified_transactions = self.verify(at, pruned_transactions, false).await?; @@ -335,7 +351,7 @@ impl Pool { async fn verify( &self, at: &BlockId, - xts: impl IntoIterator>, + xts: impl IntoIterator)>, force: bool, ) -> Result, ValidatedTransactionFor>, B::Error> { // we need a block number to compute tx validity @@ -345,7 +361,7 @@ impl Pool { for (hash, validated_tx) in futures::future::join_all( xts.into_iter() - .map(|xt| self.verify_one(at, block_number, xt, force)) + .map(|(source, xt)| self.verify_one(at, block_number, source, xt, force)) ) .await { @@ -360,6 +376,7 @@ impl Pool { &self, block_id: &BlockId, block_number: NumberFor, + source: TransactionSource, xt: ExtrinsicFor, force: bool, ) -> (ExHash, ValidatedTransactionFor) { @@ -371,7 +388,11 @@ impl Pool { ) } - let validation_result = self.validated_pool.api().validate_transaction(block_id, xt.clone()).await; + let validation_result = self.validated_pool.api().validate_transaction( + block_id, + source, + xt.clone(), + ).await; let status = match validation_result { Ok(status) => status, @@ -386,6 +407,7 @@ impl Pool { ValidatedTransaction::valid_at( block_number.saturated_into::(), hash.clone(), + source, xt, bytes, validity, diff --git a/client/transaction-pool/graph/src/validated_pool.rs b/client/transaction-pool/graph/src/validated_pool.rs index 3a6573d9bd6c9..b63e56e48101e 100644 --- a/client/transaction-pool/graph/src/validated_pool.rs +++ b/client/transaction-pool/graph/src/validated_pool.rs @@ -32,7 +32,7 @@ use parking_lot::{Mutex, RwLock}; use sp_runtime::{ generic::BlockId, traits::{self, SaturatedConversion}, - transaction_validity::{TransactionTag as Tag, ValidTransaction}, + transaction_validity::{TransactionTag as Tag, ValidTransaction, TransactionSource}, }; use sp_transaction_pool::{error, PoolStatus}; use wasm_timer::Instant; @@ -58,6 +58,7 @@ impl ValidatedTransaction { pub fn valid_at( at: u64, hash: Hash, + source: TransactionSource, data: Ex, bytes: usize, validity: ValidTransaction, @@ -66,6 +67,7 @@ impl ValidatedTransaction { data, bytes, hash, + source, priority: validity.priority, requires: validity.requires, provides: validity.provides, diff --git a/client/transaction-pool/src/api.rs b/client/transaction-pool/src/api.rs index c942f60577f25..bf104c8d78191 100644 --- a/client/transaction-pool/src/api.rs +++ b/client/transaction-pool/src/api.rs @@ -81,6 +81,7 @@ impl sc_transaction_graph::ChainApi for FullChainApi, + source: TransactionSource, uxt: sc_transaction_graph::ExtrinsicFor, ) -> Self::ValidationFuture { let (tx, rx) = oneshot::channel(); @@ -95,8 +96,7 @@ impl sc_transaction_graph::ChainApi for FullChainApi sc_transaction_graph::ChainApi for LightChainApi, + source: TransactionSource, uxt: sc_transaction_graph::ExtrinsicFor, ) -> Self::ValidationFuture { let header_hash = self.client.expect_block_hash_from_id(at); @@ -186,7 +187,7 @@ impl sc_transaction_graph::ChainApi for LightChainApi TransactionPool for BasicPool fn submit_at( &self, at: &BlockId, + source: TransactionSource, xts: Vec>, ) -> PoolFuture, Self::Error>>, Self::Error> { let pool = self.pool.clone(); let at = *at; async move { - pool.submit_at(&at, xts, false).await + pool.submit_at(&at, source, xts, false).await }.boxed() } fn submit_one( &self, at: &BlockId, + source: TransactionSource, xt: TransactionFor, ) -> PoolFuture, Self::Error> { let pool = self.pool.clone(); let at = *at; async move { - pool.submit_one(&at, xt).await + pool.submit_one(&at, source, xt).await }.boxed() } fn submit_and_watch( &self, at: &BlockId, + source: TransactionSource, xt: TransactionFor, ) -> PoolFuture>, Self::Error> { let at = *at; let pool = self.pool.clone(); async move { - pool.submit_and_watch(&at, xt) + pool.submit_and_watch(&at, source, xt) .map(|result| result.map(|watcher| Box::new(watcher.into_stream()) as _)) .await }.boxed() @@ -437,7 +441,14 @@ impl MaintainedTransactionPool for BasicPool resubmit_transactions.extend(block_transactions); } - if let Err(e) = pool.submit_at(&id, resubmit_transactions, true).await { + if let Err(e) = pool.submit_at( + &id, + // These transactions are coming from retracted blocks, we should + // simply consider them external. + TransactionSource::External, + resubmit_transactions, + true + ).await { log::debug!( target: "txpool", "[{:?}] Error re-submitting transactions: {:?}", id, e diff --git a/client/transaction-pool/src/revalidation.rs b/client/transaction-pool/src/revalidation.rs index 4b3f9955ca7a9..dd06213a1a43d 100644 --- a/client/transaction-pool/src/revalidation.rs +++ b/client/transaction-pool/src/revalidation.rs @@ -78,7 +78,7 @@ async fn batch_revalidate( None => continue, }; - match api.validate_transaction(&BlockId::Number(at), ext.data.clone()).await { + match api.validate_transaction(&BlockId::Number(at), ext.source, ext.data.clone()).await { Ok(Err(TransactionValidityError::Invalid(err))) => { log::debug!(target: "txpool", "[{:?}]: Revalidation: invalid {:?}", ext_hash, err); invalid_hashes.push(ext_hash); @@ -94,6 +94,7 @@ async fn batch_revalidate( ValidatedTransaction::valid_at( at.saturated_into::(), ext_hash, + ext.source, ext.data.clone(), api.hash_and_length(&ext.data).1, validity, @@ -343,4 +344,4 @@ mod tests { // number of ready assert_eq!(pool.validated_pool().status().ready, 1); } -} \ No newline at end of file +} diff --git a/primitives/runtime/src/transaction_validity.rs b/primitives/runtime/src/transaction_validity.rs index 7c3709521f27f..bd339938c29af 100644 --- a/primitives/runtime/src/transaction_validity.rs +++ b/primitives/runtime/src/transaction_validity.rs @@ -166,7 +166,7 @@ impl Into for UnknownTransaction { /// Depending on the source we might apply different validation schemes. /// For instance we can disallow specific kinds of transactions if they were not produced /// by our local node (for instance off-chain workers). -#[derive(Clone, PartialEq, Eq, Encode, Decode, RuntimeDebug)] +#[derive(Copy, Clone, PartialEq, Eq, Encode, Decode, RuntimeDebug, parity_util_mem::MallocSizeOf)] pub enum TransactionSource { /// Transaction is already included in block. /// diff --git a/primitives/transaction-pool/src/lib.rs b/primitives/transaction-pool/src/lib.rs index 9abbcfbdf28aa..e4498bd024880 100644 --- a/primitives/transaction-pool/src/lib.rs +++ b/primitives/transaction-pool/src/lib.rs @@ -29,5 +29,5 @@ mod pool; pub use pool::*; pub use sp_runtime::transaction_validity::{ - TransactionLongevity, TransactionPriority, TransactionTag, + TransactionLongevity, TransactionPriority, TransactionTag, TransactionSource, }; diff --git a/primitives/transaction-pool/src/pool.rs b/primitives/transaction-pool/src/pool.rs index cad06679647e4..2c9d0e33e8b52 100644 --- a/primitives/transaction-pool/src/pool.rs +++ b/primitives/transaction-pool/src/pool.rs @@ -31,7 +31,7 @@ use sp_runtime::{ generic::BlockId, traits::{Block as BlockT, Member, NumberFor}, transaction_validity::{ - TransactionLongevity, TransactionPriority, TransactionTag, + TransactionLongevity, TransactionPriority, TransactionTag, TransactionSource, }, }; @@ -192,6 +192,7 @@ pub trait TransactionPool: Send + Sync { fn submit_at( &self, at: &BlockId, + source: TransactionSource, xts: Vec>, ) -> PoolFuture, Self::Error>>, Self::Error>; @@ -199,6 +200,7 @@ pub trait TransactionPool: Send + Sync { fn submit_one( &self, at: &BlockId, + source: TransactionSource, xt: TransactionFor, ) -> PoolFuture, Self::Error>; @@ -206,6 +208,7 @@ pub trait TransactionPool: Send + Sync { fn submit_and_watch( &self, at: &BlockId, + source: TransactionSource, xt: TransactionFor, ) -> PoolFuture>, Self::Error>; @@ -299,7 +302,9 @@ impl OffchainSubmitTransaction for TPool { extrinsic ); - let result = futures::executor::block_on(self.submit_one(&at, extrinsic)); + let result = futures::executor::block_on(self.submit_one( + &at, TransactionSource::Local, extrinsic, + )); result.map(|_| ()) .map_err(|e| log::warn!( diff --git a/test-utils/runtime/src/lib.rs b/test-utils/runtime/src/lib.rs index 120dd502cdcd6..f3efb4bea7c76 100644 --- a/test-utils/runtime/src/lib.rs +++ b/test-utils/runtime/src/lib.rs @@ -494,7 +494,7 @@ cfg_if! { impl sp_transaction_pool::runtime_api::TaggedTransactionQueue for Runtime { fn validate_transaction( - source: TransactionSource, + _source: TransactionSource, utx: ::Extrinsic, ) -> TransactionValidity { if let Extrinsic::IncludeData(data) = utx { @@ -684,7 +684,7 @@ cfg_if! { impl sp_transaction_pool::runtime_api::TaggedTransactionQueue for Runtime { fn validate_transaction( - source: TransactionSource, + _source: TransactionSource, utx: ::Extrinsic, ) -> TransactionValidity { if let Extrinsic::IncludeData(data) = utx { diff --git a/test-utils/runtime/transaction-pool/src/lib.rs b/test-utils/runtime/transaction-pool/src/lib.rs index 8cd4e58954b73..432c9e520d1b1 100644 --- a/test-utils/runtime/transaction-pool/src/lib.rs +++ b/test-utils/runtime/transaction-pool/src/lib.rs @@ -25,6 +25,7 @@ use sp_runtime::{ traits::{BlakeTwo256, Hash as HashT}, transaction_validity::{ TransactionValidity, ValidTransaction, TransactionValidityError, InvalidTransaction, + TransactionSource, }, }; use std::collections::{HashSet, HashMap}; @@ -180,6 +181,7 @@ impl sc_transaction_graph::ChainApi for TestApi { fn validate_transaction( &self, _at: &BlockId, + _source: TransactionSource, uxt: sc_transaction_graph::ExtrinsicFor, ) -> Self::ValidationFuture { self.validation_requests.write().push(uxt.clone()); From 01b13afc5f16c6134e8f8363f423a6997ef9bdd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Mon, 23 Mar 2020 17:46:34 +0100 Subject: [PATCH 04/15] Fix warning. --- frame/support/src/unsigned.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/frame/support/src/unsigned.rs b/frame/support/src/unsigned.rs index a4274c8172c5d..305138583aff6 100644 --- a/frame/support/src/unsigned.rs +++ b/frame/support/src/unsigned.rs @@ -82,6 +82,7 @@ macro_rules! impl_outer_validate_unsigned { } fn validate_unsigned( + #[allow(unused_variables)] source: $crate::unsigned::TransactionSource, call: &Self::Call, ) -> $crate::unsigned::TransactionValidity { From cf975fce8ef75d61127730cb97160557098d2055 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Tue, 24 Mar 2020 11:36:40 +0100 Subject: [PATCH 05/15] Fix tests. --- bin/node/executor/tests/submit_transaction.rs | 5 +- .../basic-authorship/src/basic_authorship.rs | 12 +- client/consensus/manual-seal/src/lib.rs | 14 ++- client/offchain/src/lib.rs | 3 +- client/service/src/lib.rs | 9 +- .../transaction-pool/graph/src/base_pool.rs | 40 ++++++- client/transaction-pool/graph/src/future.rs | 2 + client/transaction-pool/graph/src/pool.rs | 50 ++++---- client/transaction-pool/graph/src/ready.rs | 4 + client/transaction-pool/graph/src/rotator.rs | 3 + client/transaction-pool/src/revalidation.rs | 6 +- client/transaction-pool/src/testing/pool.rs | 111 +++++++++++------- utils/frame/rpc/system/src/lib.rs | 5 +- 13 files changed, 175 insertions(+), 89 deletions(-) diff --git a/bin/node/executor/tests/submit_transaction.rs b/bin/node/executor/tests/submit_transaction.rs index 1a92aeca6ba77..5e5be5bade6a4 100644 --- a/bin/node/executor/tests/submit_transaction.rs +++ b/bin/node/executor/tests/submit_transaction.rs @@ -138,7 +138,7 @@ fn should_submit_signed_twice_from_the_same_account() { fn submitted_transaction_should_be_valid() { use codec::Encode; use frame_support::storage::StorageMap; - use sp_runtime::transaction_validity::ValidTransaction; + use sp_runtime::transaction_validity::{ValidTransaction, TransactionSource}; use sp_runtime::traits::StaticLookup; let mut t = new_test_ext(COMPACT_CODE, false); @@ -163,6 +163,7 @@ fn submitted_transaction_should_be_valid() { let tx0 = state.read().transactions[0].clone(); let mut t = new_test_ext(COMPACT_CODE, false); t.execute_with(|| { + let source = TransactionSource::External; let extrinsic = UncheckedExtrinsic::decode(&mut &*tx0).unwrap(); // add balance to the account let author = extrinsic.signature.clone().unwrap().0; @@ -172,7 +173,7 @@ fn submitted_transaction_should_be_valid() { >::insert(&address, account); // check validity - let res = Executive::validate_transaction(extrinsic); + let res = Executive::validate_transaction(source, extrinsic); assert_eq!(res.unwrap(), ValidTransaction { priority: 2_411_002_000_000, diff --git a/client/basic-authorship/src/basic_authorship.rs b/client/basic-authorship/src/basic_authorship.rs index 986d797ca736b..e70f3b846418a 100644 --- a/client/basic-authorship/src/basic_authorship.rs +++ b/client/basic-authorship/src/basic_authorship.rs @@ -315,13 +315,15 @@ mod tests { prelude::*, runtime::{Extrinsic, Transfer}, }; - use sp_transaction_pool::{ChainEvent, MaintainedTransactionPool}; + use sp_transaction_pool::{ChainEvent, MaintainedTransactionPool, TransactionSource}; use sc_transaction_pool::{BasicPool, FullChainApi}; use sp_api::Core; use backend::Backend; use sp_blockchain::HeaderBackend; use sp_runtime::traits::NumberFor; + const SOURCE: TransactionSource = TransactionSource::External; + fn extrinsic(nonce: u64) -> Extrinsic { Transfer { amount: Default::default(), @@ -338,7 +340,7 @@ mod tests { id: BlockId::Number(block_number.into()), retracted: vec![], is_new_best: true, - header: header, + header, } } @@ -351,7 +353,7 @@ mod tests { ); futures::executor::block_on( - txpool.submit_at(&BlockId::number(0), vec![extrinsic(0), extrinsic(1)]) + txpool.submit_at(&BlockId::number(0), SOURCE, vec![extrinsic(0), extrinsic(1)]) ).unwrap(); futures::executor::block_on( @@ -403,7 +405,7 @@ mod tests { let block_id = BlockId::Hash(genesis_hash); futures::executor::block_on( - txpool.submit_at(&BlockId::number(0), vec![extrinsic(0)]), + txpool.submit_at(&BlockId::number(0), SOURCE, vec![extrinsic(0)]), ).unwrap(); futures::executor::block_on( @@ -454,7 +456,7 @@ mod tests { ); futures::executor::block_on( - txpool.submit_at(&BlockId::number(0), vec![ + txpool.submit_at(&BlockId::number(0), SOURCE, vec![ extrinsic(0), extrinsic(1), Transfer { diff --git a/client/consensus/manual-seal/src/lib.rs b/client/consensus/manual-seal/src/lib.rs index 36d12a0046744..f3a0ca887fd00 100644 --- a/client/consensus/manual-seal/src/lib.rs +++ b/client/consensus/manual-seal/src/lib.rs @@ -224,7 +224,7 @@ mod tests { txpool::Options, }; use substrate_test_runtime_transaction_pool::{TestApi, uxt}; - use sp_transaction_pool::{TransactionPool, MaintainedTransactionPool}; + use sp_transaction_pool::{TransactionPool, MaintainedTransactionPool, TransactionSource}; use sp_runtime::generic::BlockId; use sp_blockchain::HeaderBackend; use sp_consensus::ImportedAux; @@ -236,6 +236,8 @@ mod tests { Arc::new(TestApi::empty()) } + const SOURCE: TransactionSource = TransactionSource::External; + #[tokio::test] async fn instant_seal() { let builder = TestClientBuilder::new(); @@ -278,7 +280,7 @@ mod tests { rt.block_on(future); }); // submit a transaction to pool. - let result = pool.submit_one(&BlockId::Number(0), uxt(Alice, 0)).await; + let result = pool.submit_one(&BlockId::Number(0), SOURCE, uxt(Alice, 0)).await; // assert that it was successfully imported assert!(result.is_ok()); // assert that the background task returns ok @@ -330,7 +332,7 @@ mod tests { rt.block_on(future); }); // submit a transaction to pool. - let result = pool.submit_one(&BlockId::Number(0), uxt(Alice, 0)).await; + let result = pool.submit_one(&BlockId::Number(0), SOURCE, uxt(Alice, 0)).await; // assert that it was successfully imported assert!(result.is_ok()); let (tx, rx) = futures::channel::oneshot::channel(); @@ -399,7 +401,7 @@ mod tests { rt.block_on(future); }); // submit a transaction to pool. - let result = pool.submit_one(&BlockId::Number(0), uxt(Alice, 0)).await; + let result = pool.submit_one(&BlockId::Number(0), SOURCE, uxt(Alice, 0)).await; // assert that it was successfully imported assert!(result.is_ok()); @@ -430,7 +432,7 @@ mod tests { ); // assert that there's a new block in the db. assert!(backend.blockchain().header(BlockId::Number(0)).unwrap().is_some()); - assert!(pool.submit_one(&BlockId::Number(1), uxt(Alice, 1)).await.is_ok()); + assert!(pool.submit_one(&BlockId::Number(1), SOURCE, uxt(Alice, 1)).await.is_ok()); pool.maintain(sp_transaction_pool::ChainEvent::NewBlock { id: BlockId::Number(1), @@ -453,7 +455,7 @@ mod tests { assert!(backend.blockchain().header(BlockId::Number(1)).unwrap().is_some()); pool_api.increment_nonce(Alice.into()); - assert!(pool.submit_one(&BlockId::Number(2), uxt(Alice, 2)).await.is_ok()); + assert!(pool.submit_one(&BlockId::Number(2), SOURCE, uxt(Alice, 2)).await.is_ok()); let (tx2, rx2) = futures::channel::oneshot::channel(); assert!(sink.send(EngineCommand::SealNewBlock { parent_hash: Some(created_block.hash), diff --git a/client/offchain/src/lib.rs b/client/offchain/src/lib.rs index 27a7f508459ba..94850e3fd3461 100644 --- a/client/offchain/src/lib.rs +++ b/client/offchain/src/lib.rs @@ -191,7 +191,8 @@ mod tests { at: &BlockId, extrinsic: ::Extrinsic, ) -> Result<(), ()> { - futures::executor::block_on(self.0.submit_one(&at, extrinsic)) + let source = sp_transaction_pool::TransactionSource::Local; + futures::executor::block_on(self.0.submit_one(&at, source, extrinsic)) .map(|_| ()) .map_err(|_| ()) } diff --git a/client/service/src/lib.rs b/client/service/src/lib.rs index 6d0c93fbf54d7..d6912df393eea 100644 --- a/client/service/src/lib.rs +++ b/client/service/src/lib.rs @@ -671,6 +671,7 @@ mod tests { Default::default(), Arc::new(FullChainApi::new(client.clone())), ).0); + let source = sp_runtime::transaction_validity::TransactionSource::External; let best = longest_chain.best_chain().unwrap(); let transaction = Transfer { amount: 5, @@ -678,8 +679,12 @@ mod tests { from: AccountKeyring::Alice.into(), to: Default::default(), }.into_signed_tx(); - block_on(pool.submit_one(&BlockId::hash(best.hash()), transaction.clone())).unwrap(); - block_on(pool.submit_one(&BlockId::hash(best.hash()), Extrinsic::IncludeData(vec![1]))).unwrap(); + block_on(pool.submit_one( + &BlockId::hash(best.hash()), source, transaction.clone()), + ).unwrap(); + block_on(pool.submit_one( + &BlockId::hash(best.hash()), source, Extrinsic::IncludeData(vec![1])), + ).unwrap(); assert_eq!(pool.status().ready, 2); // when diff --git a/client/transaction-pool/graph/src/base_pool.rs b/client/transaction-pool/graph/src/base_pool.rs index 45fc9a8e837aa..8416757ec1fb6 100644 --- a/client/transaction-pool/graph/src/base_pool.rs +++ b/client/transaction-pool/graph/src/base_pool.rs @@ -189,6 +189,7 @@ impl fmt::Debug for Transaction where write!(fmt, "valid_till: {:?}, ", &self.valid_till)?; write!(fmt, "bytes: {:?}, ", &self.bytes)?; write!(fmt, "propagate: {:?}, ", &self.propagate)?; + write!(fmt, "source: {:?}, ", &self.source)?; write!(fmt, "requires: [")?; print_tags(fmt, &self.requires)?; write!(fmt, "], provides: [")?; @@ -560,6 +561,7 @@ mod tests { requires: vec![], provides: vec![vec![1]], propagate: true, + source: Source::External, }).unwrap(); // then @@ -582,6 +584,7 @@ mod tests { requires: vec![], provides: vec![vec![1]], propagate: true, + source: Source::External, }).unwrap(); pool.import(Transaction { data: vec![1u8], @@ -592,6 +595,7 @@ mod tests { requires: vec![], provides: vec![vec![1]], propagate: true, + source: Source::External, }).unwrap_err(); // then @@ -615,6 +619,7 @@ mod tests { requires: vec![vec![0]], provides: vec![vec![1]], propagate: true, + source: Source::External, }).unwrap(); assert_eq!(pool.ready().count(), 0); assert_eq!(pool.ready.len(), 0); @@ -627,6 +632,7 @@ mod tests { requires: vec![], provides: vec![vec![0]], propagate: true, + source: Source::External, }).unwrap(); // then @@ -649,6 +655,7 @@ mod tests { requires: vec![vec![0]], provides: vec![vec![1]], propagate: true, + source: Source::External, }).unwrap(); pool.import(Transaction { data: vec![3u8], @@ -659,6 +666,7 @@ mod tests { requires: vec![vec![2]], provides: vec![], propagate: true, + source: Source::External, }).unwrap(); pool.import(Transaction { data: vec![2u8], @@ -669,6 +677,7 @@ mod tests { requires: vec![vec![1]], provides: vec![vec![3], vec![2]], propagate: true, + source: Source::External, }).unwrap(); pool.import(Transaction { data: vec![4u8], @@ -679,6 +688,7 @@ mod tests { requires: vec![vec![3], vec![4]], provides: vec![], propagate: true, + source: Source::External, }).unwrap(); assert_eq!(pool.ready().count(), 0); assert_eq!(pool.ready.len(), 0); @@ -692,6 +702,7 @@ mod tests { requires: vec![], provides: vec![vec![0], vec![4]], propagate: true, + source: Source::External, }).unwrap(); // then @@ -724,6 +735,7 @@ mod tests { requires: vec![vec![0]], provides: vec![vec![1]], propagate: true, + source: Source::External, }).unwrap(); pool.import(Transaction { data: vec![3u8], @@ -734,6 +746,7 @@ mod tests { requires: vec![vec![1]], provides: vec![vec![2]], propagate: true, + source: Source::External, }).unwrap(); assert_eq!(pool.ready().count(), 0); assert_eq!(pool.ready.len(), 0); @@ -748,6 +761,7 @@ mod tests { requires: vec![vec![2]], provides: vec![vec![0]], propagate: true, + source: Source::External, }).unwrap(); // then @@ -768,6 +782,7 @@ mod tests { requires: vec![], provides: vec![vec![0]], propagate: true, + source: Source::External, }).unwrap(); let mut it = pool.ready().into_iter().map(|tx| tx.data[0]); assert_eq!(it.next(), Some(4)); @@ -796,6 +811,7 @@ mod tests { requires: vec![vec![0]], provides: vec![vec![1]], propagate: true, + source: Source::External, }).unwrap(); pool.import(Transaction { data: vec![3u8], @@ -806,6 +822,7 @@ mod tests { requires: vec![vec![1]], provides: vec![vec![2]], propagate: true, + source: Source::External, }).unwrap(); assert_eq!(pool.ready().count(), 0); assert_eq!(pool.ready.len(), 0); @@ -820,6 +837,7 @@ mod tests { requires: vec![vec![2]], provides: vec![vec![0]], propagate: true, + source: Source::External, }).unwrap(); // then @@ -840,6 +858,7 @@ mod tests { requires: vec![], provides: vec![vec![0]], propagate: true, + source: Source::External, }).unwrap_err(); let mut it = pool.ready().into_iter().map(|tx| tx.data[0]); assert_eq!(it.next(), None); @@ -863,6 +882,7 @@ mod tests { requires: vec![], provides: vec![vec![0], vec![4]], propagate: true, + source: Source::External, }).expect("import 1 should be ok"); pool.import(Transaction { data: vec![3u8; 1024], @@ -873,6 +893,7 @@ mod tests { requires: vec![], provides: vec![vec![2], vec![7]], propagate: true, + source: Source::External, }).expect("import 2 should be ok"); assert!(parity_util_mem::malloc_size(&pool) > 5000); @@ -891,6 +912,7 @@ mod tests { requires: vec![], provides: vec![vec![0], vec![4]], propagate: true, + source: Source::External, }).unwrap(); pool.import(Transaction { data: vec![1u8], @@ -901,6 +923,7 @@ mod tests { requires: vec![vec![0]], provides: vec![vec![1]], propagate: true, + source: Source::External, }).unwrap(); pool.import(Transaction { data: vec![3u8], @@ -911,6 +934,7 @@ mod tests { requires: vec![vec![2]], provides: vec![], propagate: true, + source: Source::External, }).unwrap(); pool.import(Transaction { data: vec![2u8], @@ -921,6 +945,7 @@ mod tests { requires: vec![vec![1]], provides: vec![vec![3], vec![2]], propagate: true, + source: Source::External, }).unwrap(); pool.import(Transaction { data: vec![4u8], @@ -931,6 +956,7 @@ mod tests { requires: vec![vec![3], vec![4]], provides: vec![], propagate: true, + source: Source::External, }).unwrap(); // future pool.import(Transaction { @@ -942,6 +968,7 @@ mod tests { requires: vec![vec![11]], provides: vec![], propagate: true, + source: Source::External, }).unwrap(); assert_eq!(pool.ready().count(), 5); assert_eq!(pool.future.len(), 1); @@ -968,6 +995,7 @@ mod tests { requires: vec![vec![0]], provides: vec![vec![100]], propagate: true, + source: Source::External, }).unwrap(); // ready pool.import(Transaction { @@ -979,6 +1007,7 @@ mod tests { requires: vec![], provides: vec![vec![1]], propagate: true, + source: Source::External, }).unwrap(); pool.import(Transaction { data: vec![2u8], @@ -989,6 +1018,7 @@ mod tests { requires: vec![vec![2]], provides: vec![vec![3]], propagate: true, + source: Source::External, }).unwrap(); pool.import(Transaction { data: vec![3u8], @@ -999,6 +1029,7 @@ mod tests { requires: vec![vec![1]], provides: vec![vec![2]], propagate: true, + source: Source::External, }).unwrap(); pool.import(Transaction { data: vec![4u8], @@ -1009,6 +1040,7 @@ mod tests { requires: vec![vec![3], vec![2]], provides: vec![vec![4]], propagate: true, + source: Source::External, }).unwrap(); assert_eq!(pool.ready().count(), 4); @@ -1044,9 +1076,10 @@ mod tests { requires: vec![vec![3], vec![2]], provides: vec![vec![4]], propagate: true, + source: Source::External, }), "Transaction { \ -hash: 4, priority: 1000, valid_till: 64, bytes: 1, propagate: true, \ +hash: 4, priority: 1000, valid_till: 64, bytes: 1, propagate: true, source: External, \ requires: [03,02], provides: [04], data: [4]}".to_owned() ); } @@ -1062,6 +1095,7 @@ requires: [03,02], provides: [04], data: [4]}".to_owned() requires: vec![vec![3], vec![2]], provides: vec![vec![4]], propagate: true, + source: Source::External, }.is_propagable(), true); assert_eq!(Transaction { @@ -1073,6 +1107,7 @@ requires: [03,02], provides: [04], data: [4]}".to_owned() requires: vec![vec![3], vec![2]], provides: vec![vec![4]], propagate: false, + source: Source::External, }.is_propagable(), false); } @@ -1094,6 +1129,7 @@ requires: [03,02], provides: [04], data: [4]}".to_owned() requires: vec![vec![0]], provides: vec![], propagate: true, + source: Source::External, }); if let Err(error::Error::RejectedFutureTransaction) = err { @@ -1117,6 +1153,7 @@ requires: [03,02], provides: [04], data: [4]}".to_owned() requires: vec![vec![0]], provides: vec![], propagate: true, + source: Source::External, }).unwrap(); // then @@ -1146,6 +1183,7 @@ requires: [03,02], provides: [04], data: [4]}".to_owned() requires: vec![vec![0]], provides: vec![], propagate: true, + source: Source::External, }).unwrap(); flag diff --git a/client/transaction-pool/graph/src/future.rs b/client/transaction-pool/graph/src/future.rs index a84a5fbe689c3..76181c837f988 100644 --- a/client/transaction-pool/graph/src/future.rs +++ b/client/transaction-pool/graph/src/future.rs @@ -249,6 +249,7 @@ impl FutureTransactions { #[cfg(test)] mod tests { use super::*; + use sp_runtime::transaction_validity::TransactionSource; #[test] fn can_track_heap_size() { @@ -263,6 +264,7 @@ mod tests { requires: vec![vec![1], vec![2]], provides: vec![vec![3], vec![4]], propagate: true, + source: TransactionSource::External, }.into(), missing_tags: vec![vec![1u8], vec![2u8]].into_iter().collect(), imported_at: std::time::Instant::now(), diff --git a/client/transaction-pool/graph/src/pool.rs b/client/transaction-pool/graph/src/pool.rs index 7e1fc397fa042..a7e4ab554d501 100644 --- a/client/transaction-pool/graph/src/pool.rs +++ b/client/transaction-pool/graph/src/pool.rs @@ -444,7 +444,7 @@ mod tests { use futures::executor::block_on; use super::*; use sp_transaction_pool::TransactionStatus; - use sp_runtime::transaction_validity::{ValidTransaction, InvalidTransaction}; + use sp_runtime::transaction_validity::{ValidTransaction, InvalidTransaction, TransactionSource}; use codec::Encode; use substrate_test_runtime::{Block, Extrinsic, Transfer, H256, AccountId}; use assert_matches::assert_matches; @@ -452,6 +452,7 @@ mod tests { use crate::base_pool::Limit; const INVALID_NONCE: u64 = 254; + const SOURCE: TransactionSource = TransactionSource::External; #[derive(Clone, Debug, Default)] struct TestApi { @@ -472,6 +473,7 @@ mod tests { fn validate_transaction( &self, at: &BlockId, + _source: TransactionSource, uxt: ExtrinsicFor, ) -> Self::ValidationFuture { let hash = self.hash_and_length(&uxt).0; @@ -563,7 +565,7 @@ mod tests { let pool = pool(); // when - let hash = block_on(pool.submit_one(&BlockId::Number(0), uxt(Transfer { + let hash = block_on(pool.submit_one(&BlockId::Number(0), SOURCE, uxt(Transfer { from: AccountId::from_h256(H256::from_low_u64_be(1)), to: AccountId::from_h256(H256::from_low_u64_be(2)), amount: 5, @@ -587,7 +589,7 @@ mod tests { // when pool.validated_pool.rotator().ban(&Instant::now(), vec![pool.hash_of(&uxt)]); - let res = block_on(pool.submit_one(&BlockId::Number(0), uxt)); + let res = block_on(pool.submit_one(&BlockId::Number(0), SOURCE, uxt)); assert_eq!(pool.validated_pool().status().ready, 0); assert_eq!(pool.validated_pool().status().future, 0); @@ -603,20 +605,20 @@ mod tests { let stream = pool.validated_pool().import_notification_stream(); // when - let _hash = block_on(pool.submit_one(&BlockId::Number(0), uxt(Transfer { + let _hash = block_on(pool.submit_one(&BlockId::Number(0), SOURCE, uxt(Transfer { from: AccountId::from_h256(H256::from_low_u64_be(1)), to: AccountId::from_h256(H256::from_low_u64_be(2)), amount: 5, nonce: 0, }))).unwrap(); - let _hash = block_on(pool.submit_one(&BlockId::Number(0), uxt(Transfer { + let _hash = block_on(pool.submit_one(&BlockId::Number(0), SOURCE, uxt(Transfer { from: AccountId::from_h256(H256::from_low_u64_be(1)), to: AccountId::from_h256(H256::from_low_u64_be(2)), amount: 5, nonce: 1, }))).unwrap(); // future doesn't count - let _hash = block_on(pool.submit_one(&BlockId::Number(0), uxt(Transfer { + let _hash = block_on(pool.submit_one(&BlockId::Number(0), SOURCE, uxt(Transfer { from: AccountId::from_h256(H256::from_low_u64_be(1)), to: AccountId::from_h256(H256::from_low_u64_be(2)), amount: 5, @@ -639,19 +641,19 @@ mod tests { fn should_clear_stale_transactions() { // given let pool = pool(); - let hash1 = block_on(pool.submit_one(&BlockId::Number(0), uxt(Transfer { + let hash1 = block_on(pool.submit_one(&BlockId::Number(0), SOURCE, uxt(Transfer { from: AccountId::from_h256(H256::from_low_u64_be(1)), to: AccountId::from_h256(H256::from_low_u64_be(2)), amount: 5, nonce: 0, }))).unwrap(); - let hash2 = block_on(pool.submit_one(&BlockId::Number(0), uxt(Transfer { + let hash2 = block_on(pool.submit_one(&BlockId::Number(0), SOURCE, uxt(Transfer { from: AccountId::from_h256(H256::from_low_u64_be(1)), to: AccountId::from_h256(H256::from_low_u64_be(2)), amount: 5, nonce: 1, }))).unwrap(); - let hash3 = block_on(pool.submit_one(&BlockId::Number(0), uxt(Transfer { + let hash3 = block_on(pool.submit_one(&BlockId::Number(0), SOURCE, uxt(Transfer { from: AccountId::from_h256(H256::from_low_u64_be(1)), to: AccountId::from_h256(H256::from_low_u64_be(2)), amount: 5, @@ -675,7 +677,7 @@ mod tests { fn should_ban_mined_transactions() { // given let pool = pool(); - let hash1 = block_on(pool.submit_one(&BlockId::Number(0), uxt(Transfer { + let hash1 = block_on(pool.submit_one(&BlockId::Number(0), SOURCE, uxt(Transfer { from: AccountId::from_h256(H256::from_low_u64_be(1)), to: AccountId::from_h256(H256::from_low_u64_be(2)), amount: 5, @@ -702,7 +704,7 @@ mod tests { ..Default::default() }, TestApi::default().into()); - let hash1 = block_on(pool.submit_one(&BlockId::Number(0), uxt(Transfer { + let hash1 = block_on(pool.submit_one(&BlockId::Number(0), SOURCE, uxt(Transfer { from: AccountId::from_h256(H256::from_low_u64_be(1)), to: AccountId::from_h256(H256::from_low_u64_be(2)), amount: 5, @@ -711,7 +713,7 @@ mod tests { assert_eq!(pool.validated_pool().status().future, 1); // when - let hash2 = block_on(pool.submit_one(&BlockId::Number(0), uxt(Transfer { + let hash2 = block_on(pool.submit_one(&BlockId::Number(0), SOURCE, uxt(Transfer { from: AccountId::from_h256(H256::from_low_u64_be(2)), to: AccountId::from_h256(H256::from_low_u64_be(2)), amount: 5, @@ -738,7 +740,7 @@ mod tests { }, TestApi::default().into()); // when - block_on(pool.submit_one(&BlockId::Number(0), uxt(Transfer { + block_on(pool.submit_one(&BlockId::Number(0), SOURCE, uxt(Transfer { from: AccountId::from_h256(H256::from_low_u64_be(1)), to: AccountId::from_h256(H256::from_low_u64_be(2)), amount: 5, @@ -756,7 +758,7 @@ mod tests { let pool = pool(); // when - let err = block_on(pool.submit_one(&BlockId::Number(0), uxt(Transfer { + let err = block_on(pool.submit_one(&BlockId::Number(0), SOURCE, uxt(Transfer { from: AccountId::from_h256(H256::from_low_u64_be(1)), to: AccountId::from_h256(H256::from_low_u64_be(2)), amount: 5, @@ -776,7 +778,7 @@ mod tests { fn should_trigger_ready_and_finalized() { // given let pool = pool(); - let watcher = block_on(pool.submit_and_watch(&BlockId::Number(0), uxt(Transfer { + let watcher = block_on(pool.submit_and_watch(&BlockId::Number(0), SOURCE, uxt(Transfer { from: AccountId::from_h256(H256::from_low_u64_be(1)), to: AccountId::from_h256(H256::from_low_u64_be(2)), amount: 5, @@ -800,7 +802,7 @@ mod tests { fn should_trigger_ready_and_finalized_when_pruning_via_hash() { // given let pool = pool(); - let watcher = block_on(pool.submit_and_watch(&BlockId::Number(0), uxt(Transfer { + let watcher = block_on(pool.submit_and_watch(&BlockId::Number(0), SOURCE, uxt(Transfer { from: AccountId::from_h256(H256::from_low_u64_be(1)), to: AccountId::from_h256(H256::from_low_u64_be(2)), amount: 5, @@ -824,7 +826,7 @@ mod tests { fn should_trigger_future_and_ready_after_promoted() { // given let pool = pool(); - let watcher = block_on(pool.submit_and_watch(&BlockId::Number(0), uxt(Transfer { + let watcher = block_on(pool.submit_and_watch(&BlockId::Number(0), SOURCE, uxt(Transfer { from: AccountId::from_h256(H256::from_low_u64_be(1)), to: AccountId::from_h256(H256::from_low_u64_be(2)), amount: 5, @@ -834,7 +836,7 @@ mod tests { assert_eq!(pool.validated_pool().status().future, 1); // when - block_on(pool.submit_one(&BlockId::Number(0), uxt(Transfer { + block_on(pool.submit_one(&BlockId::Number(0), SOURCE, uxt(Transfer { from: AccountId::from_h256(H256::from_low_u64_be(1)), to: AccountId::from_h256(H256::from_low_u64_be(2)), amount: 5, @@ -858,7 +860,7 @@ mod tests { amount: 5, nonce: 0, }); - let watcher = block_on(pool.submit_and_watch(&BlockId::Number(0), uxt)).unwrap(); + let watcher = block_on(pool.submit_and_watch(&BlockId::Number(0), SOURCE, uxt)).unwrap(); assert_eq!(pool.validated_pool().status().ready, 1); // when @@ -882,7 +884,7 @@ mod tests { amount: 5, nonce: 0, }); - let watcher = block_on(pool.submit_and_watch(&BlockId::Number(0), uxt)).unwrap(); + let watcher = block_on(pool.submit_and_watch(&BlockId::Number(0), SOURCE, uxt)).unwrap(); assert_eq!(pool.validated_pool().status().ready, 1); // when @@ -917,7 +919,7 @@ mod tests { amount: 5, nonce: 0, }); - let watcher = block_on(pool.submit_and_watch(&BlockId::Number(0), xt)).unwrap(); + let watcher = block_on(pool.submit_and_watch(&BlockId::Number(0), SOURCE, xt)).unwrap(); assert_eq!(pool.validated_pool().status().ready, 1); // when @@ -927,7 +929,7 @@ mod tests { amount: 4, nonce: 1, }); - block_on(pool.submit_one(&BlockId::Number(1), xt)).unwrap(); + block_on(pool.submit_one(&BlockId::Number(1), SOURCE, xt)).unwrap(); assert_eq!(pool.validated_pool().status().ready, 1); // then @@ -956,7 +958,7 @@ mod tests { // This transaction should go to future, since we use `nonce: 1` let pool2 = pool.clone(); std::thread::spawn(move || { - block_on(pool2.submit_one(&BlockId::Number(0), xt)).unwrap(); + block_on(pool2.submit_one(&BlockId::Number(0), SOURCE, xt)).unwrap(); ready.send(()).unwrap(); }); @@ -970,7 +972,7 @@ mod tests { }); // The tag the above transaction provides (TestApi is using just nonce as u8) let provides = vec![0_u8]; - block_on(pool.submit_one(&BlockId::Number(0), xt)).unwrap(); + block_on(pool.submit_one(&BlockId::Number(0), SOURCE, xt)).unwrap(); assert_eq!(pool.validated_pool().status().ready, 1); // Now block import happens before the second transaction is able to finish verification. diff --git a/client/transaction-pool/graph/src/ready.rs b/client/transaction-pool/graph/src/ready.rs index 23f0d49a93071..c856535a61651 100644 --- a/client/transaction-pool/graph/src/ready.rs +++ b/client/transaction-pool/graph/src/ready.rs @@ -545,6 +545,7 @@ fn remove_item(vec: &mut Vec, item: &T) { #[cfg(test)] mod tests { use super::*; + use sp_runtime::transaction_validity::TransactionSource as Source; fn tx(id: u8) -> Transaction> { Transaction { @@ -556,6 +557,7 @@ mod tests { requires: vec![vec![1], vec![2]], provides: vec![vec![3], vec![4]], propagate: true, + source: Source::External, } } @@ -656,6 +658,7 @@ mod tests { requires: vec![tx1.provides[0].clone()], provides: vec![], propagate: true, + source: Source::External, }; // when @@ -688,6 +691,7 @@ mod tests { requires: vec![], provides: vec![], propagate: true, + source: Source::External, }; import(&mut ready, tx).unwrap(); diff --git a/client/transaction-pool/graph/src/rotator.rs b/client/transaction-pool/graph/src/rotator.rs index 55a9230522e2f..be96174d1d916 100644 --- a/client/transaction-pool/graph/src/rotator.rs +++ b/client/transaction-pool/graph/src/rotator.rs @@ -100,6 +100,7 @@ impl PoolRotator { #[cfg(test)] mod tests { use super::*; + use sp_runtime::transaction_validity::TransactionSource; type Hash = u64; type Ex = (); @@ -122,6 +123,7 @@ mod tests { requires: vec![], provides: vec![], propagate: true, + source: TransactionSource::External, }; (hash, tx) @@ -188,6 +190,7 @@ mod tests { requires: vec![], provides: vec![], propagate: true, + source: TransactionSource::External, } } diff --git a/client/transaction-pool/src/revalidation.rs b/client/transaction-pool/src/revalidation.rs index dd06213a1a43d..ee0aa1ab2d6f5 100644 --- a/client/transaction-pool/src/revalidation.rs +++ b/client/transaction-pool/src/revalidation.rs @@ -313,9 +313,9 @@ where #[cfg(test)] mod tests { - use super::*; use sc_transaction_graph::Pool; + use sp_transaction_pool::TransactionSource; use substrate_test_runtime_transaction_pool::{TestApi, uxt}; use futures::executor::block_on; use substrate_test_runtime_client::{ @@ -335,7 +335,9 @@ mod tests { let queue = Arc::new(RevalidationQueue::new(api.clone(), pool.clone())); let uxt = uxt(Alice, 0); - let uxt_hash = block_on(pool.submit_one(&BlockId::number(0), uxt.clone())).expect("Should be valid"); + let uxt_hash = block_on( + pool.submit_one(&BlockId::number(0), TransactionSource::External, uxt.clone()) + ).expect("Should be valid"); block_on(queue.revalidate_later(0, vec![uxt_hash])); diff --git a/client/transaction-pool/src/testing/pool.rs b/client/transaction-pool/src/testing/pool.rs index 27cb6c62a5327..b797e79173e82 100644 --- a/client/transaction-pool/src/testing/pool.rs +++ b/client/transaction-pool/src/testing/pool.rs @@ -20,7 +20,7 @@ use futures::executor::block_on; use txpool::{self, Pool}; use sp_runtime::{ generic::BlockId, - transaction_validity::ValidTransaction, + transaction_validity::{ValidTransaction, TransactionSource}, }; use substrate_test_runtime_client::{ runtime::{Block, Hash, Index, Header, Extrinsic}, @@ -52,10 +52,12 @@ fn header(number: u64) -> Header { } } +const SOURCE: TransactionSource = TransactionSource::External; + #[test] fn submission_should_work() { let pool = pool(); - block_on(pool.submit_one(&BlockId::number(0), uxt(Alice, 209))).unwrap(); + block_on(pool.submit_one(&BlockId::number(0), SOURCE, uxt(Alice, 209))).unwrap(); let pending: Vec<_> = pool.validated_pool().ready().map(|a| a.data.transfer().nonce).collect(); assert_eq!(pending, vec![209]); @@ -64,8 +66,8 @@ fn submission_should_work() { #[test] fn multiple_submission_should_work() { let pool = pool(); - block_on(pool.submit_one(&BlockId::number(0), uxt(Alice, 209))).unwrap(); - block_on(pool.submit_one(&BlockId::number(0), uxt(Alice, 210))).unwrap(); + block_on(pool.submit_one(&BlockId::number(0), SOURCE, uxt(Alice, 209))).unwrap(); + block_on(pool.submit_one(&BlockId::number(0), SOURCE, uxt(Alice, 210))).unwrap(); let pending: Vec<_> = pool.validated_pool().ready().map(|a| a.data.transfer().nonce).collect(); assert_eq!(pending, vec![209, 210]); @@ -74,7 +76,7 @@ fn multiple_submission_should_work() { #[test] fn early_nonce_should_be_culled() { let pool = pool(); - block_on(pool.submit_one(&BlockId::number(0), uxt(Alice, 208))).unwrap(); + block_on(pool.submit_one(&BlockId::number(0), SOURCE, uxt(Alice, 208))).unwrap(); let pending: Vec<_> = pool.validated_pool().ready().map(|a| a.data.transfer().nonce).collect(); assert_eq!(pending, Vec::::new()); @@ -84,11 +86,11 @@ fn early_nonce_should_be_culled() { fn late_nonce_should_be_queued() { let pool = pool(); - block_on(pool.submit_one(&BlockId::number(0), uxt(Alice, 210))).unwrap(); + block_on(pool.submit_one(&BlockId::number(0), SOURCE, uxt(Alice, 210))).unwrap(); let pending: Vec<_> = pool.validated_pool().ready().map(|a| a.data.transfer().nonce).collect(); assert_eq!(pending, Vec::::new()); - block_on(pool.submit_one(&BlockId::number(0), uxt(Alice, 209))).unwrap(); + block_on(pool.submit_one(&BlockId::number(0), SOURCE, uxt(Alice, 209))).unwrap(); let pending: Vec<_> = pool.validated_pool().ready().map(|a| a.data.transfer().nonce).collect(); assert_eq!(pending, vec![209, 210]); } @@ -96,8 +98,8 @@ fn late_nonce_should_be_queued() { #[test] fn prune_tags_should_work() { let pool = pool(); - let hash209 = block_on(pool.submit_one(&BlockId::number(0), uxt(Alice, 209))).unwrap(); - block_on(pool.submit_one(&BlockId::number(0), uxt(Alice, 210))).unwrap(); + let hash209 = block_on(pool.submit_one(&BlockId::number(0), SOURCE, uxt(Alice, 209))).unwrap(); + block_on(pool.submit_one(&BlockId::number(0), SOURCE, uxt(Alice, 210))).unwrap(); let pending: Vec<_> = pool.validated_pool().ready().map(|a| a.data.transfer().nonce).collect(); assert_eq!(pending, vec![209, 210]); @@ -118,16 +120,16 @@ fn prune_tags_should_work() { fn should_ban_invalid_transactions() { let pool = pool(); let uxt = uxt(Alice, 209); - let hash = block_on(pool.submit_one(&BlockId::number(0), uxt.clone())).unwrap(); + let hash = block_on(pool.submit_one(&BlockId::number(0), SOURCE, uxt.clone())).unwrap(); pool.validated_pool().remove_invalid(&[hash]); - block_on(pool.submit_one(&BlockId::number(0), uxt.clone())).unwrap_err(); + block_on(pool.submit_one(&BlockId::number(0), SOURCE, uxt.clone())).unwrap_err(); // when let pending: Vec<_> = pool.validated_pool().ready().map(|a| a.data.transfer().nonce).collect(); assert_eq!(pending, Vec::::new()); // then - block_on(pool.submit_one(&BlockId::number(0), uxt.clone())).unwrap_err(); + block_on(pool.submit_one(&BlockId::number(0), SOURCE, uxt.clone())).unwrap_err(); } #[test] @@ -138,7 +140,7 @@ fn should_correctly_prune_transactions_providing_more_than_one_tag() { })); let pool = Pool::new(Default::default(), api.clone()); let xt = uxt(Alice, 209); - block_on(pool.submit_one(&BlockId::number(0), xt.clone())).expect("1. Imported"); + block_on(pool.submit_one(&BlockId::number(0), SOURCE, xt.clone())).expect("1. Imported"); assert_eq!(pool.validated_pool().status().ready, 1); // remove the transaction that just got imported. @@ -151,7 +153,7 @@ fn should_correctly_prune_transactions_providing_more_than_one_tag() { // so now let's insert another transaction that also provides the 155 api.increment_nonce(Alice.into()); let xt = uxt(Alice, 211); - block_on(pool.submit_one(&BlockId::number(2), xt.clone())).expect("2. Imported"); + block_on(pool.submit_one(&BlockId::number(2), SOURCE, xt.clone())).expect("2. Imported"); assert_eq!(pool.validated_pool().status().ready, 1); assert_eq!(pool.validated_pool().status().future, 1); let pending: Vec<_> = pool.validated_pool().ready().map(|a| a.data.transfer().nonce).collect(); @@ -189,7 +191,7 @@ fn should_prune_old_during_maintenance() { let (pool, _guard) = maintained_pool(); - block_on(pool.submit_one(&BlockId::number(0), xt.clone())).expect("1. Imported"); + block_on(pool.submit_one(&BlockId::number(0), SOURCE, xt.clone())).expect("1. Imported"); assert_eq!(pool.status().ready, 1); pool.api.push_block(1, vec![xt.clone()]); @@ -204,8 +206,8 @@ fn should_revalidate_during_maintenance() { let xt2 = uxt(Alice, 210); let (pool, _guard) = maintained_pool(); - block_on(pool.submit_one(&BlockId::number(0), xt1.clone())).expect("1. Imported"); - block_on(pool.submit_one(&BlockId::number(0), xt2.clone())).expect("2. Imported"); + block_on(pool.submit_one(&BlockId::number(0), SOURCE, xt1.clone())).expect("1. Imported"); + block_on(pool.submit_one(&BlockId::number(0), SOURCE, xt2.clone())).expect("2. Imported"); assert_eq!(pool.status().ready, 2); assert_eq!(pool.api.validation_requests().len(), 2); @@ -229,7 +231,7 @@ fn should_resubmit_from_retracted_during_maintenance() { let (pool, _guard) = maintained_pool(); - block_on(pool.submit_one(&BlockId::number(0), xt.clone())).expect("1. Imported"); + block_on(pool.submit_one(&BlockId::number(0), SOURCE, xt.clone())).expect("1. Imported"); assert_eq!(pool.status().ready, 1); pool.api.push_block(1, vec![]); @@ -248,7 +250,7 @@ fn should_not_retain_invalid_hashes_from_retracted() { let (pool, _guard) = maintained_pool(); - block_on(pool.submit_one(&BlockId::number(0), xt.clone())).expect("1. Imported"); + block_on(pool.submit_one(&BlockId::number(0), SOURCE, xt.clone())).expect("1. Imported"); assert_eq!(pool.status().ready, 1); pool.api.push_block(1, vec![]); @@ -274,7 +276,7 @@ fn should_revalidate_transaction_multiple_times() { let (pool, _guard) = maintained_pool(); - block_on(pool.submit_one(&BlockId::number(0), xt.clone())).expect("1. Imported"); + block_on(pool.submit_one(&BlockId::number(0), SOURCE, xt.clone())).expect("1. Imported"); assert_eq!(pool.status().ready, 1); pool.api.push_block(1, vec![xt.clone()]); @@ -283,7 +285,7 @@ fn should_revalidate_transaction_multiple_times() { block_on(pool.maintain(block_event(1))); block_on(futures_timer::Delay::new(BACKGROUND_REVALIDATION_INTERVAL*2)); - block_on(pool.submit_one(&BlockId::number(0), xt.clone())).expect("1. Imported"); + block_on(pool.submit_one(&BlockId::number(0), SOURCE, xt.clone())).expect("1. Imported"); assert_eq!(pool.status().ready, 1); pool.api.push_block(2, vec![]); @@ -304,8 +306,8 @@ fn should_revalidate_across_many_blocks() { let (pool, _guard) = maintained_pool(); - block_on(pool.submit_one(&BlockId::number(1), xt1.clone())).expect("1. Imported"); - block_on(pool.submit_one(&BlockId::number(1), xt2.clone())).expect("1. Imported"); + block_on(pool.submit_one(&BlockId::number(1), SOURCE, xt1.clone())).expect("1. Imported"); + block_on(pool.submit_one(&BlockId::number(1), SOURCE, xt2.clone())).expect("1. Imported"); assert_eq!(pool.status().ready, 2); pool.api.push_block(1, vec![]); @@ -313,7 +315,7 @@ fn should_revalidate_across_many_blocks() { block_on(futures_timer::Delay::new(BACKGROUND_REVALIDATION_INTERVAL*2)); - block_on(pool.submit_one(&BlockId::number(2), xt3.clone())).expect("1. Imported"); + block_on(pool.submit_one(&BlockId::number(2), SOURCE, xt3.clone())).expect("1. Imported"); assert_eq!(pool.status().ready, 3); pool.api.push_block(2, vec![xt1.clone()]); @@ -336,15 +338,25 @@ fn should_push_watchers_during_maintaince() { let (pool, _guard) = maintained_pool(); let tx0 = alice_uxt(0); - let watcher0 = block_on(pool.submit_and_watch(&BlockId::Number(0), tx0.clone())).unwrap(); + let watcher0 = block_on( + pool.submit_and_watch(&BlockId::Number(0), SOURCE, tx0.clone()) + ).unwrap(); let tx1 = alice_uxt(1); - let watcher1 = block_on(pool.submit_and_watch(&BlockId::Number(0), tx1.clone())).unwrap(); + let watcher1 = block_on( + pool.submit_and_watch(&BlockId::Number(0), SOURCE, tx1.clone()) + ).unwrap(); let tx2 = alice_uxt(2); - let watcher2 = block_on(pool.submit_and_watch(&BlockId::Number(0), tx2.clone())).unwrap(); + let watcher2 = block_on( + pool.submit_and_watch(&BlockId::Number(0), SOURCE, tx2.clone()) + ).unwrap(); let tx3 = alice_uxt(3); - let watcher3 = block_on(pool.submit_and_watch(&BlockId::Number(0), tx3.clone())).unwrap(); + let watcher3 = block_on( + pool.submit_and_watch(&BlockId::Number(0), SOURCE, tx3.clone()) + ).unwrap(); let tx4 = alice_uxt(4); - let watcher4 = block_on(pool.submit_and_watch(&BlockId::Number(0), tx4.clone())).unwrap(); + let watcher4 = block_on( + pool.submit_and_watch(&BlockId::Number(0), SOURCE, tx4.clone()) + ).unwrap(); assert_eq!(pool.status().ready, 5); // when @@ -397,10 +409,10 @@ fn should_push_watchers_during_maintaince() { #[test] fn can_track_heap_size() { let (pool, _guard) = maintained_pool(); - block_on(pool.submit_one(&BlockId::number(0), uxt(Alice, 209))).expect("1. Imported"); - block_on(pool.submit_one(&BlockId::number(0), uxt(Alice, 210))).expect("1. Imported"); - block_on(pool.submit_one(&BlockId::number(0), uxt(Alice, 211))).expect("1. Imported"); - block_on(pool.submit_one(&BlockId::number(0), uxt(Alice, 212))).expect("1. Imported"); + block_on(pool.submit_one(&BlockId::number(0), SOURCE, uxt(Alice, 209))).expect("1. Imported"); + block_on(pool.submit_one(&BlockId::number(0), SOURCE, uxt(Alice, 210))).expect("1. Imported"); + block_on(pool.submit_one(&BlockId::number(0), SOURCE, uxt(Alice, 211))).expect("1. Imported"); + block_on(pool.submit_one(&BlockId::number(0), SOURCE, uxt(Alice, 212))).expect("1. Imported"); assert!(parity_util_mem::malloc_size(&pool) > 3000); } @@ -411,7 +423,9 @@ fn finalization() { let api = TestApi::with_alice_nonce(209); api.push_block(1, vec![]); let (pool, _background) = BasicPool::new(Default::default(), api.into()); - let watcher = block_on(pool.submit_and_watch(&BlockId::number(1), xt.clone())).expect("1. Imported"); + let watcher = block_on( + pool.submit_and_watch(&BlockId::number(1), SOURCE, xt.clone()) + ).expect("1. Imported"); pool.api.push_block(2, vec![xt.clone()]); let header = pool.api.chain().read().header_by_number.get(&2).cloned().unwrap(); @@ -461,7 +475,9 @@ fn fork_aware_finalization() { // block B1 { - let watcher = block_on(pool.submit_and_watch(&BlockId::number(1), from_alice.clone())).expect("1. Imported"); + let watcher = block_on( + pool.submit_and_watch(&BlockId::number(1), SOURCE, from_alice.clone()) + ).expect("1. Imported"); let header = pool.api.push_block(2, vec![from_alice.clone()]); canon_watchers.push((watcher, header.hash())); assert_eq!(pool.status().ready, 1); @@ -482,8 +498,9 @@ fn fork_aware_finalization() { // block C2 { let header = pool.api.push_fork_block_with_parent(b1, vec![from_dave.clone()]); - from_dave_watcher = block_on(pool.submit_and_watch(&BlockId::number(1), from_dave.clone())) - .expect("1. Imported"); + from_dave_watcher = block_on( + pool.submit_and_watch(&BlockId::number(1), SOURCE, from_dave.clone()) + ).expect("1. Imported"); assert_eq!(pool.status().ready, 1); let event = ChainEvent::NewBlock { id: BlockId::Hash(header.hash()), @@ -498,7 +515,9 @@ fn fork_aware_finalization() { // block D2 { - from_bob_watcher = block_on(pool.submit_and_watch(&BlockId::number(1), from_bob.clone())).expect("1. Imported"); + from_bob_watcher = block_on( + pool.submit_and_watch(&BlockId::number(1), SOURCE, from_bob.clone()) + ).expect("1. Imported"); assert_eq!(pool.status().ready, 1); let header = pool.api.push_fork_block_with_parent(c2, vec![from_bob.clone()]); @@ -515,7 +534,9 @@ fn fork_aware_finalization() { // block C1 { - let watcher = block_on(pool.submit_and_watch(&BlockId::number(1), from_charlie.clone())).expect("1.Imported"); + let watcher = block_on( + pool.submit_and_watch(&BlockId::number(1), SOURCE, from_charlie.clone()) + ).expect("1.Imported"); assert_eq!(pool.status().ready, 1); let header = pool.api.push_block(3, vec![from_charlie.clone()]); @@ -535,7 +556,9 @@ fn fork_aware_finalization() { // block D1 { let xt = uxt(Eve, 0); - let w = block_on(pool.submit_and_watch(&BlockId::number(1), xt.clone())).expect("1. Imported"); + let w = block_on( + pool.submit_and_watch(&BlockId::number(1), SOURCE, xt.clone()) + ).expect("1. Imported"); assert_eq!(pool.status().ready, 3); let header = pool.api.push_block(4, vec![xt.clone()]); canon_watchers.push((w, header.hash())); @@ -607,7 +630,7 @@ fn fork_aware_finalization() { fn ready_set_should_not_resolve_before_block_update() { let (pool, _guard) = maintained_pool(); let xt1 = uxt(Alice, 209); - block_on(pool.submit_one(&BlockId::number(1), xt1.clone())).expect("1. Imported"); + block_on(pool.submit_one(&BlockId::number(1), SOURCE, xt1.clone())).expect("1. Imported"); assert!(pool.ready_at(1).now_or_never().is_none()); } @@ -619,7 +642,7 @@ fn ready_set_should_resolve_after_block_update() { let xt1 = uxt(Alice, 209); - block_on(pool.submit_one(&BlockId::number(1), xt1.clone())).expect("1. Imported"); + block_on(pool.submit_one(&BlockId::number(1), SOURCE, xt1.clone())).expect("1. Imported"); block_on(pool.maintain(block_event(1))); assert!(pool.ready_at(1).now_or_never().is_some()); @@ -632,7 +655,7 @@ fn ready_set_should_eventually_resolve_when_block_update_arrives() { let xt1 = uxt(Alice, 209); - block_on(pool.submit_one(&BlockId::number(1), xt1.clone())).expect("1. Imported"); + block_on(pool.submit_one(&BlockId::number(1), SOURCE, xt1.clone())).expect("1. Imported"); let noop_waker = futures::task::noop_waker(); let mut context = futures::task::Context::from_waker(&noop_waker); @@ -653,4 +676,4 @@ fn ready_set_should_eventually_resolve_when_block_update_arrives() { assert_eq!(data.len(), 1); } } -} \ No newline at end of file +} diff --git a/utils/frame/rpc/system/src/lib.rs b/utils/frame/rpc/system/src/lib.rs index e6214c73d6ea1..c73ddfe93efa0 100644 --- a/utils/frame/rpc/system/src/lib.rs +++ b/utils/frame/rpc/system/src/lib.rs @@ -239,6 +239,7 @@ mod tests { BasicPool::new(Default::default(), Arc::new(FullChainApi::new(client.clone()))).0 ); + let source = sp_runtime::transaction_validity::TransactionSource::External; let new_transaction = |nonce: u64| { let t = Transfer { from: AccountKeyring::Alice.into(), @@ -250,9 +251,9 @@ mod tests { }; // Populate the pool let ext0 = new_transaction(0); - block_on(pool.submit_one(&BlockId::number(0), ext0)).unwrap(); + block_on(pool.submit_one(&BlockId::number(0), source, ext0)).unwrap(); let ext1 = new_transaction(1); - block_on(pool.submit_one(&BlockId::number(0), ext1)).unwrap(); + block_on(pool.submit_one(&BlockId::number(0), source, ext1)).unwrap(); let accounts = FullSystem::new(client, pool); From f9ec1d9de9e71d70c668d7a8594d5842162cbb81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Tue, 24 Mar 2020 11:50:22 +0100 Subject: [PATCH 06/15] Apply suggestions from code review Co-Authored-By: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Co-Authored-By: Nikolay Volf --- client/transaction-pool/graph/src/base_pool.rs | 2 +- frame/executive/src/lib.rs | 4 ++-- frame/support/procedural/src/construct_runtime/mod.rs | 2 +- primitives/runtime/src/transaction_validity.rs | 6 +++--- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/client/transaction-pool/graph/src/base_pool.rs b/client/transaction-pool/graph/src/base_pool.rs index 45fc9a8e837aa..1140506f3f538 100644 --- a/client/transaction-pool/graph/src/base_pool.rs +++ b/client/transaction-pool/graph/src/base_pool.rs @@ -103,7 +103,7 @@ pub struct Transaction { pub provides: Vec, /// Should that transaction be propagated. pub propagate: bool, - /// Origin of that transaction. + /// Source of that transaction. pub source: Source, } diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 7fb587659c6b1..f18ca9993b828 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -60,7 +60,7 @@ //! # pub type AllModules = u64; //! # pub enum Runtime {}; //! # use sp_runtime::transaction_validity::{ -//! # TransactionValidity, UnknownTransaction, TransactionSource, +//! # TransactionValidity, UnknownTransaction, TransactionSource, //! # }; //! # use sp_runtime::traits::ValidateUnsigned; //! # impl ValidateUnsigned for Runtime { @@ -745,7 +745,7 @@ mod tests { t.execute_with(|| { assert_eq!( Executive::validate_transaction(TransactionSource::InBlock, xt.clone()), - Ok(Default::default()) + Ok(Default::default()), ); assert_eq!(Executive::apply_extrinsic(xt), Ok(Err(DispatchError::BadOrigin))); }); diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index 1c366d0348699..b74a27e7ba936 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -121,7 +121,7 @@ fn construct_runtime_parsed(definition: RuntimeDefinition) -> Result( diff --git a/primitives/runtime/src/transaction_validity.rs b/primitives/runtime/src/transaction_validity.rs index bd339938c29af..bf7734c7f3e48 100644 --- a/primitives/runtime/src/transaction_validity.rs +++ b/primitives/runtime/src/transaction_validity.rs @@ -161,7 +161,7 @@ impl Into for UnknownTransaction { } } -/// The origin of the transaction. +/// The source of the transaction. /// /// Depending on the source we might apply different validation schemes. /// For instance we can disallow specific kinds of transactions if they were not produced @@ -178,8 +178,8 @@ pub enum TransactionSource { /// Transaction is coming from a local source. /// - /// This means that the transaction was produced either internally by the node - /// (for instance an Off-Chain Worker, or an Off-Chain Call) opposed + /// This means that the transaction was produced internally by the node + /// (for instance an Off-Chain Worker, or an Off-Chain Call), as opposed /// to being received over the network. Local, From 3a8343fb8d40b3c06a8cb371061898022f3a50a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Tue, 24 Mar 2020 11:59:13 +0100 Subject: [PATCH 07/15] Extra changes. --- primitives/runtime/src/generic/checked_extrinsic.rs | 2 +- primitives/runtime/src/transaction_validity.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/primitives/runtime/src/generic/checked_extrinsic.rs b/primitives/runtime/src/generic/checked_extrinsic.rs index aa30b861269c6..911a1131bb5fc 100644 --- a/primitives/runtime/src/generic/checked_extrinsic.rs +++ b/primitives/runtime/src/generic/checked_extrinsic.rs @@ -50,7 +50,7 @@ where fn validate>( &self, - // TODO [ToDr] should source be passed to `SignedExtension`s? + // TODO [#5006;ToDr] should source be passed to `SignedExtension`s? // Perhaps a change for 2.0 to avoid breaking too much APIs? source: TransactionSource, info: Self::DispatchInfo, diff --git a/primitives/runtime/src/transaction_validity.rs b/primitives/runtime/src/transaction_validity.rs index bf7734c7f3e48..78f724b4d22aa 100644 --- a/primitives/runtime/src/transaction_validity.rs +++ b/primitives/runtime/src/transaction_validity.rs @@ -171,7 +171,7 @@ pub enum TransactionSource { /// Transaction is already included in block. /// /// This means that we can't really tell where the transaction is coming from, - /// since it's already in the received block. Note that the custom validator + /// since it's already in the received block. Note that the custom validation logic /// using either `Local` or `External` should most likely just allow `InBlock` /// transactions as well. InBlock, From 353fc361a388739c776ae03ca6f2c82d5689e040 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Tue, 24 Mar 2020 12:43:44 +0100 Subject: [PATCH 08/15] Fix test and benches. --- client/rpc/src/state/tests.rs | 2 +- client/transaction-pool/graph/benches/basics.rs | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/client/rpc/src/state/tests.rs b/client/rpc/src/state/tests.rs index 75ce4ed9c3c4d..c7b5f88215ec3 100644 --- a/client/rpc/src/state/tests.rs +++ b/client/rpc/src/state/tests.rs @@ -403,7 +403,7 @@ fn should_return_runtime_version() { let result = "{\"specName\":\"test\",\"implName\":\"parity-test\",\"authoringVersion\":1,\ \"specVersion\":2,\"implVersion\":2,\"apis\":[[\"0xdf6acb689907609b\",2],\ - [\"0x37e397fc7c91f5e4\",1],[\"0xd2bc9897eed08f15\",1],[\"0x40fe3ad401f8959a\",4],\ + [\"0x37e397fc7c91f5e4\",1],[\"0xd2bc9897eed08f15\",2],[\"0x40fe3ad401f8959a\",4],\ [\"0xc6e9a76309f39b09\",1],[\"0xdd718d5cc53262d4\",1],[\"0xcbca25e39f142387\",1],\ [\"0xf78b278be53f454c\",2],[\"0xab3c0572291feb8b\",1],[\"0xbc9d89904f5b923f\",1]]}"; diff --git a/client/transaction-pool/graph/benches/basics.rs b/client/transaction-pool/graph/benches/basics.rs index 6f5d39f09f5c5..1b995c4b47d8f 100644 --- a/client/transaction-pool/graph/benches/basics.rs +++ b/client/transaction-pool/graph/benches/basics.rs @@ -23,7 +23,7 @@ use codec::Encode; use substrate_test_runtime::{Block, Extrinsic, Transfer, H256, AccountId}; use sp_runtime::{ generic::BlockId, - transaction_validity::{TransactionValidity, TransactionTag as Tag}, + transaction_validity::{TransactionValidity, TransactionTag as Tag, TransactionSource}, }; use sp_core::blake2_256; @@ -121,6 +121,7 @@ fn uxt(transfer: Transfer) -> Extrinsic { } fn bench_configured(pool: Pool, number: u64) { + let source = TransactionSource::External; let mut futures = Vec::new(); let mut tags = Vec::new(); @@ -133,7 +134,7 @@ fn bench_configured(pool: Pool, number: u64) { }); tags.push(to_tag(nonce, AccountId::from_h256(H256::from_low_u64_be(1)))); - futures.push(pool.submit_one(&BlockId::Number(1), xt)); + futures.push(pool.submit_one(&BlockId::Number(1), source, xt)); } let res = block_on(futures::future::join_all(futures.into_iter())); From 93016476691ec08a053f17dde6e6966033dc9f30 Mon Sep 17 00:00:00 2001 From: NikVolf Date: Tue, 24 Mar 2020 23:05:45 +0300 Subject: [PATCH 09/15] fix test --- client/transaction-pool/graph/src/base_pool.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/transaction-pool/graph/src/base_pool.rs b/client/transaction-pool/graph/src/base_pool.rs index 5402ad443c50e..783a36f7381ee 100644 --- a/client/transaction-pool/graph/src/base_pool.rs +++ b/client/transaction-pool/graph/src/base_pool.rs @@ -1079,7 +1079,7 @@ mod tests { source: Source::External, }), "Transaction { \ -hash: 4, priority: 1000, valid_till: 64, bytes: 1, propagate: true, source: External, \ +hash: 4, priority: 1000, valid_till: 64, bytes: 1, propagate: true, source: TransactionSource::External, \ requires: [03,02], provides: [04], data: [4]}".to_owned() ); } From 74850df89505a9d0825a439dd04c33f639f80aaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Tue, 24 Mar 2020 21:00:46 +0100 Subject: [PATCH 10/15] Fix test & benches again. --- client/transaction-pool/graph/benches/basics.rs | 7 +++++-- client/transaction-pool/graph/src/base_pool.rs | 4 ++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/client/transaction-pool/graph/benches/basics.rs b/client/transaction-pool/graph/benches/basics.rs index 1b995c4b47d8f..23b4dba348806 100644 --- a/client/transaction-pool/graph/benches/basics.rs +++ b/client/transaction-pool/graph/benches/basics.rs @@ -18,12 +18,14 @@ use criterion::{criterion_group, criterion_main, Criterion}; use futures::{future::{ready, Ready}, executor::block_on}; use sc_transaction_graph::*; -use sp_runtime::transaction_validity::{ValidTransaction, InvalidTransaction}; use codec::Encode; use substrate_test_runtime::{Block, Extrinsic, Transfer, H256, AccountId}; use sp_runtime::{ generic::BlockId, - transaction_validity::{TransactionValidity, TransactionTag as Tag, TransactionSource}, + transaction_validity::{ + ValidTransaction, InvalidTransaction, TransactionValidity, TransactionTag as Tag, + TransactionSource, + }, }; use sp_core::blake2_256; @@ -55,6 +57,7 @@ impl ChainApi for TestApi { fn validate_transaction( &self, at: &BlockId, + _source: TransactionSource, uxt: ExtrinsicFor, ) -> Self::ValidationFuture { let nonce = uxt.transfer().nonce; diff --git a/client/transaction-pool/graph/src/base_pool.rs b/client/transaction-pool/graph/src/base_pool.rs index 5402ad443c50e..38151e9bfd23b 100644 --- a/client/transaction-pool/graph/src/base_pool.rs +++ b/client/transaction-pool/graph/src/base_pool.rs @@ -1079,8 +1079,8 @@ mod tests { source: Source::External, }), "Transaction { \ -hash: 4, priority: 1000, valid_till: 64, bytes: 1, propagate: true, source: External, \ -requires: [03,02], provides: [04], data: [4]}".to_owned() +hash: 4, priority: 1000, valid_till: 64, bytes: 1, propagate: true, \ +source: TransactionSource::External, requires: [03,02], provides: [04], data: [4]}".to_owned() ); } From 6aa7402a88cf41d788a61ed3c418804a73950090 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 25 Mar 2020 08:05:06 +0100 Subject: [PATCH 11/15] Fix tests. --- client/transaction-pool/src/testing/pool.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/transaction-pool/src/testing/pool.rs b/client/transaction-pool/src/testing/pool.rs index 504bcfdcffd89..7dbb8e6158e26 100644 --- a/client/transaction-pool/src/testing/pool.rs +++ b/client/transaction-pool/src/testing/pool.rs @@ -704,7 +704,7 @@ fn should_not_accept_old_signatures() { let xt = Extrinsic::Transfer { transfer, signature: old_singature, exhaust_resources_when_not_first: false }; assert_matches::assert_matches!( - block_on(pool.submit_one(&BlockId::number(0), xt.clone())), + block_on(pool.submit_one(&BlockId::number(0), SOURCE, xt.clone())), Err(error::Error::Pool( sp_transaction_pool::error::Error::InvalidTransaction(InvalidTransaction::BadProof) )), From 2efbf7574bf39234ae3af60d8db3331430b2d543 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 25 Mar 2020 08:06:24 +0100 Subject: [PATCH 12/15] Update bumpalo --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 76fabe80eb1be..b57b40370740a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -472,9 +472,9 @@ dependencies = [ [[package]] name = "bumpalo" -version = "3.2.0" +version = "3.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1f359dc14ff8911330a51ef78022d376f25ed00248912803b58f00cb1c27f742" +checksum = "12ae9db68ad7fac5fe51304d20f016c911539251075a214f8e663babefa35187" [[package]] name = "byte-slice-cast" From ee73a620f19e00817dc909852491996cc2901c80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 25 Mar 2020 09:46:24 +0100 Subject: [PATCH 13/15] Fix doc test. --- frame/support/src/unsigned.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/frame/support/src/unsigned.rs b/frame/support/src/unsigned.rs index 305138583aff6..3bc6f692affc2 100644 --- a/frame/support/src/unsigned.rs +++ b/frame/support/src/unsigned.rs @@ -34,10 +34,8 @@ pub use crate::sp_runtime::transaction_validity::{ /// # impl frame_support::unsigned::ValidateUnsigned for Module { /// # type Call = Call; /// # -/// # fn validate_unsigned( -/// # _source: frame_support::transaction_validity::TransactionSource, -/// # _call: &Self::Call, -/// # ) -> frame_support::unsigned::TransactionValidity { +/// # fn validate_unsigned(_source: frame_support::unsigned::TransactionSource, _call: &Self::Call) +/// -> frame_support::unsigned::TransactionValidity { /// # unimplemented!(); /// # } /// # } From 934cf9ca443e3274f1c1fc718092bfce1cecaa95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 25 Mar 2020 12:05:30 +0100 Subject: [PATCH 14/15] Fix doctest. --- frame/executive/src/lib.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 80a1b591de9da..7bfdcd6e5b2ed 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -66,10 +66,7 @@ //! # impl ValidateUnsigned for Runtime { //! # type Call = (); //! # -//! # fn validate_unsigned( -//! # _source: TransactionSource, -//! # _call: &Self::Call, -//! # ) -> TransactionValidity { +//! # fn validate_unsigned(_source: TransactionSource, _call: &Self::Call) -> TransactionValidity { //! # UnknownTransaction::NoUnsignedValidator.into() //! # } //! # } From 44575132d6678eafbad7e3265282110a0ad16eea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Wed, 25 Mar 2020 12:35:50 +0100 Subject: [PATCH 15/15] Fix doctest. --- frame/executive/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/executive/src/lib.rs b/frame/executive/src/lib.rs index 7bfdcd6e5b2ed..d30b66e0837df 100644 --- a/frame/executive/src/lib.rs +++ b/frame/executive/src/lib.rs @@ -60,7 +60,7 @@ //! # pub type AllModules = u64; //! # pub enum Runtime {}; //! # use sp_runtime::transaction_validity::{ -//! # TransactionValidity, UnknownTransaction, TransactionSource, +//! TransactionValidity, UnknownTransaction, TransactionSource, //! # }; //! # use sp_runtime::traits::ValidateUnsigned; //! # impl ValidateUnsigned for Runtime {