From c7c818705d90a8248432c22778043a7b4fbf41e8 Mon Sep 17 00:00:00 2001 From: James Wilson Date: Wed, 12 Jan 2022 18:05:10 +0000 Subject: [PATCH 1/3] Use Derivative to skip bounds on T when they aren't necessary, and remove unnecessary bounds on Config --- Cargo.toml | 1 + src/client.rs | 4 +- src/config.rs | 11 ++- src/events.rs | 23 +++--- src/extrinsic/extra.rs | 176 +++++++++++++++++++++++++++-------------- src/storage.rs | 11 ++- 6 files changed, 149 insertions(+), 77 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 3b2070bec9..fc0110a298 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,6 +39,7 @@ sp-runtime = { git = "https://github.com/paritytech/substrate/", branch = "maste sp-version = { package = "sp-version", git = "https://github.com/paritytech/substrate/", branch = "master" } frame-metadata = "14.0.0" +derivative = "2.2.0" [dev-dependencies] sp-arithmetic = { git = "https://github.com/paritytech/substrate/", branch = "master", default-features = false } diff --git a/src/client.rs b/src/client.rs index 19782e25ff..0ae001cf18 100644 --- a/src/client.rs +++ b/src/client.rs @@ -40,6 +40,7 @@ use crate::{ Config, Metadata, }; +use derivative::Derivative; use std::sync::Arc; /// ClientBuilder for constructing a Client. @@ -111,7 +112,8 @@ impl ClientBuilder { } /// Client to interface with a substrate node. -#[derive(Clone)] +#[derive(Derivative)] +#[derivative(Clone(bound = ""))] pub struct Client { rpc: Rpc, genesis_hash: T::Hash, diff --git a/src/config.rs b/src/config.rs index 429a3e4075..1258660a25 100644 --- a/src/config.rs +++ b/src/config.rs @@ -32,7 +32,10 @@ use sp_runtime::traits::{ }; /// Runtime types. -pub trait Config: Clone + Default + Sized + Send + Sync + 'static { +// Note: the 'static bound isn't strictly required, but currently deriving TypeInfo +// automatically applies a 'static bound to all generic types (including this one), +// and so until that is resolved, we'll keep the (easy to satisfy) constraint here. +pub trait Config: 'static { /// Account index (aka nonce) type. This stores the number of previous /// transactions associated with a sender account. type Index: Parameter + Member + Default + AtLeast32Bit + Copy + scale_info::TypeInfo; @@ -83,8 +86,10 @@ pub trait Parameter: Codec + EncodeLike + Clone + Eq + Debug {} impl Parameter for T where T: Codec + EncodeLike + Clone + Eq + Debug {} /// Default set of commonly used types by Substrate runtimes. -#[derive(Clone, Debug, Default, Eq, PartialEq)] -pub struct DefaultConfig; +// Note: We only use this at the type level, so instances of it should not be constructed, +// hence the private () type contained within. +#[derive(Debug)] +pub struct DefaultConfig(()); impl Config for DefaultConfig { type Index = u32; diff --git a/src/events.rs b/src/events.rs index 15b74dca6f..17ad0583e5 100644 --- a/src/events.rs +++ b/src/events.rs @@ -14,16 +14,6 @@ // You should have received a copy of the GNU General Public License // along with subxt. If not, see . -use codec::{ - Codec, - Compact, - Decode, - Encode, - Error as CodecError, - Input, -}; -use std::marker::PhantomData; - use crate::{ metadata::{ EventMetadata, @@ -35,11 +25,21 @@ use crate::{ Metadata, Phase, }; +use codec::{ + Codec, + Compact, + Decode, + Encode, + Error as CodecError, + Input, +}; +use derivative::Derivative; use scale_info::{ TypeDef, TypeDefPrimitive, }; use sp_core::Bytes; +use std::marker::PhantomData; /// Raw bytes for an Event #[derive(Debug)] @@ -69,7 +69,8 @@ impl RawEvent { } /// Events decoder. -#[derive(Debug, Clone)] +#[derive(Derivative)] +#[derivative(Clone(bound = ""), Debug(bound = ""))] pub struct EventsDecoder { metadata: Metadata, marker: PhantomData, diff --git a/src/extrinsic/extra.rs b/src/extrinsic/extra.rs index 832d5658e0..27676ec3e9 100644 --- a/src/extrinsic/extra.rs +++ b/src/extrinsic/extra.rs @@ -18,10 +18,7 @@ use codec::{ Decode, Encode, }; -use core::{ - fmt::Debug, - marker::PhantomData, -}; +use derivative::Derivative; use scale_info::TypeInfo; use sp_runtime::{ generic::Era, @@ -47,20 +44,45 @@ use crate::Config; /// This is modified from the substrate version to allow passing in of the version, which is /// returned via `additional_signed()`. +/// A version of [`std::marker::PhantomData`] that is also Send and Sync (which is fine +/// because regardless of the generic param, it is always possible to Send + Sync this). +#[derive(Derivative, Encode, Decode, TypeInfo)] +#[derivative( + Clone(bound = ""), + PartialEq(bound = ""), + Debug(bound = ""), + Eq(bound = ""), + Default(bound = "") +)] +#[scale_info(skip_type_params(T))] +pub struct PhantomDataSendSync(core::marker::PhantomData); + +unsafe impl Send for PhantomDataSendSync {} +unsafe impl Sync for PhantomDataSendSync {} + +impl PhantomDataSendSync { + pub(crate) fn new() -> Self { + Self(core::marker::PhantomData) + } +} + /// Ensure the runtime version registered in the transaction is the same as at present. -#[derive(Encode, Decode, Clone, Eq, PartialEq, Debug, TypeInfo)] +#[derive(Derivative, Encode, Decode, TypeInfo)] +#[derivative( + Clone(bound = ""), + PartialEq(bound = ""), + Debug(bound = ""), + Eq(bound = "") +)] #[scale_info(skip_type_params(T))] pub struct CheckSpecVersion( - pub PhantomData, + pub PhantomDataSendSync, /// Local version to be used for `AdditionalSigned` #[codec(skip)] pub u32, ); -impl SignedExtension for CheckSpecVersion -where - T: Config + Clone + Debug + Eq + Send + Sync, -{ +impl SignedExtension for CheckSpecVersion { const IDENTIFIER: &'static str = "CheckSpecVersion"; type AccountId = T::AccountId; type Call = (); @@ -88,19 +110,22 @@ where /// /// This is modified from the substrate version to allow passing in of the version, which is /// returned via `additional_signed()`. -#[derive(Encode, Decode, Clone, Eq, PartialEq, Debug, TypeInfo)] +#[derive(Derivative, Encode, Decode, TypeInfo)] +#[derivative( + Clone(bound = ""), + PartialEq(bound = ""), + Debug(bound = ""), + Eq(bound = "") +)] #[scale_info(skip_type_params(T))] pub struct CheckTxVersion( - pub PhantomData, + pub PhantomDataSendSync, /// Local version to be used for `AdditionalSigned` #[codec(skip)] pub u32, ); -impl SignedExtension for CheckTxVersion -where - T: Config + Clone + Debug + Eq + Send + Sync, -{ +impl SignedExtension for CheckTxVersion { const IDENTIFIER: &'static str = "CheckTxVersion"; type AccountId = T::AccountId; type Call = (); @@ -128,19 +153,22 @@ where /// /// This is modified from the substrate version to allow passing in of the genesis hash, which is /// returned via `additional_signed()`. -#[derive(Encode, Decode, Clone, Eq, PartialEq, Debug, TypeInfo)] +#[derive(Derivative, Encode, Decode, TypeInfo)] +#[derivative( + Clone(bound = ""), + PartialEq(bound = ""), + Debug(bound = ""), + Eq(bound = "") +)] #[scale_info(skip_type_params(T))] pub struct CheckGenesis( - pub PhantomData, + pub PhantomDataSendSync, /// Local genesis hash to be used for `AdditionalSigned` #[codec(skip)] pub T::Hash, ); -impl SignedExtension for CheckGenesis -where - T: Config + Clone + Debug + Eq + Send + Sync, -{ +impl SignedExtension for CheckGenesis { const IDENTIFIER: &'static str = "CheckGenesis"; type AccountId = T::AccountId; type Call = (); @@ -169,20 +197,23 @@ where /// This is modified from the substrate version to allow passing in of the genesis hash, which is /// returned via `additional_signed()`. It assumes therefore `Era::Immortal` (The transaction is /// valid forever) -#[derive(Encode, Decode, Clone, Eq, PartialEq, Debug, TypeInfo)] +#[derive(Derivative, Encode, Decode, TypeInfo)] +#[derivative( + Clone(bound = ""), + PartialEq(bound = ""), + Debug(bound = ""), + Eq(bound = "") +)] #[scale_info(skip_type_params(T))] pub struct CheckMortality( /// The default structure for the Extra encoding - pub (Era, PhantomData), + pub (Era, PhantomDataSendSync), /// Local genesis hash to be used for `AdditionalSigned` #[codec(skip)] pub T::Hash, ); -impl SignedExtension for CheckMortality -where - T: Config + Clone + Debug + Eq + Send + Sync, -{ +impl SignedExtension for CheckMortality { const IDENTIFIER: &'static str = "CheckMortality"; type AccountId = T::AccountId; type Call = (); @@ -205,14 +236,17 @@ where } /// Nonce check and increment to give replay protection for transactions. -#[derive(Encode, Decode, Clone, Eq, PartialEq, Debug, TypeInfo)] +#[derive(Derivative, Encode, Decode, TypeInfo)] +#[derivative( + Clone(bound = ""), + PartialEq(bound = ""), + Debug(bound = ""), + Eq(bound = "") +)] #[scale_info(skip_type_params(T))] pub struct CheckNonce(#[codec(compact)] pub T::Index); -impl SignedExtension for CheckNonce -where - T: Config + Clone + Debug + Eq + Send + Sync, -{ +impl SignedExtension for CheckNonce { const IDENTIFIER: &'static str = "CheckNonce"; type AccountId = T::AccountId; type Call = (); @@ -235,14 +269,17 @@ where } /// Resource limit check. -#[derive(Encode, Decode, Clone, Eq, PartialEq, Debug, TypeInfo)] +#[derive(Derivative, Encode, Decode, TypeInfo)] +#[derivative( + Clone(bound = ""), + PartialEq(bound = ""), + Debug(bound = ""), + Eq(bound = "") +)] #[scale_info(skip_type_params(T))] -pub struct CheckWeight(pub PhantomData); +pub struct CheckWeight(pub PhantomDataSendSync); -impl SignedExtension for CheckWeight -where - T: Config + Clone + Debug + Eq + Send + Sync, -{ +impl SignedExtension for CheckWeight { const IDENTIFIER: &'static str = "CheckWeight"; type AccountId = T::AccountId; type Call = (); @@ -266,17 +303,21 @@ where /// Require the transactor pay for themselves and maybe include a tip to gain additional priority /// in the queue. -#[derive(Encode, Decode, Clone, Debug, Default, Eq, PartialEq, TypeInfo)] +#[derive(Derivative, Encode, Decode, TypeInfo)] +#[derivative( + Clone(bound = ""), + PartialEq(bound = ""), + Debug(bound = ""), + Eq(bound = ""), + Default(bound = "") +)] #[scale_info(skip_type_params(T))] pub struct ChargeTransactionPayment( #[codec(compact)] u128, - pub PhantomData, + pub PhantomDataSendSync, ); -impl SignedExtension for ChargeTransactionPayment -where - T: Config + Clone + Debug + Eq + Send + Sync, -{ +impl SignedExtension for ChargeTransactionPayment { const IDENTIFIER: &'static str = "ChargeTransactionPayment"; type AccountId = T::AccountId; type Call = (); @@ -300,7 +341,14 @@ where /// Require the transactor pay for themselves and maybe include a tip to gain additional priority /// in the queue. -#[derive(Encode, Decode, Clone, Default, Eq, PartialEq, Debug, TypeInfo)] +#[derive(Derivative, Encode, Decode, TypeInfo)] +#[derivative( + Clone(bound = ""), + PartialEq(bound = ""), + Debug(bound = ""), + Eq(bound = ""), + Default(bound = "") +)] #[scale_info(skip_type_params(T))] pub struct ChargeAssetTxPayment { /// The tip for the block author. @@ -309,13 +357,10 @@ pub struct ChargeAssetTxPayment { /// The asset with which to pay the tip. pub asset_id: Option, /// Marker for unused type parameter. - pub marker: PhantomData, + pub marker: PhantomDataSendSync, } -impl SignedExtension for ChargeAssetTxPayment -where - T: Config + Clone + Debug + Eq + Send + Sync, -{ +impl SignedExtension for ChargeAssetTxPayment { const IDENTIFIER: &'static str = "ChargeAssetTxPayment"; type AccountId = T::AccountId; type Call = (); @@ -358,19 +403,25 @@ pub trait SignedExtra: SignedExtension { } /// Default `SignedExtra` for substrate runtimes. -#[derive(Encode, Decode, Clone, Eq, PartialEq, Debug, TypeInfo)] +#[derive(Derivative, Encode, Decode, TypeInfo)] +#[derivative( + Clone(bound = ""), + PartialEq(bound = ""), + Debug(bound = ""), + Eq(bound = "") +)] #[scale_info(skip_type_params(T))] pub struct DefaultExtraWithTxPayment { spec_version: u32, tx_version: u32, nonce: T::Index, genesis_hash: T::Hash, - marker: PhantomData, + marker: PhantomDataSendSync, } impl SignedExtra for DefaultExtraWithTxPayment where - T: Config + Clone + Debug + Eq + Send + Sync, + T: Config, X: SignedExtension + Default, { type Extra = ( @@ -396,18 +447,21 @@ where tx_version, nonce, genesis_hash, - marker: PhantomData, + marker: PhantomDataSendSync::new(), } } fn extra(&self) -> Self::Extra { ( - CheckSpecVersion(PhantomData, self.spec_version), - CheckTxVersion(PhantomData, self.tx_version), - CheckGenesis(PhantomData, self.genesis_hash), - CheckMortality((Era::Immortal, PhantomData), self.genesis_hash), + CheckSpecVersion(PhantomDataSendSync::new(), self.spec_version), + CheckTxVersion(PhantomDataSendSync::new(), self.tx_version), + CheckGenesis(PhantomDataSendSync::new(), self.genesis_hash), + CheckMortality( + (Era::Immortal, PhantomDataSendSync::new()), + self.genesis_hash, + ), CheckNonce(self.nonce), - CheckWeight(PhantomData), + CheckWeight(PhantomDataSendSync::new()), X::default(), ) } @@ -416,7 +470,7 @@ where impl + Default> SignedExtension for DefaultExtraWithTxPayment where - T: Config + Clone + Debug + Eq + Send + Sync, + T: Config, X: SignedExtension, { const IDENTIFIER: &'static str = "DefaultExtra"; diff --git a/src/storage.rs b/src/storage.rs index b7be297abc..5b617a218c 100644 --- a/src/storage.rs +++ b/src/storage.rs @@ -132,13 +132,22 @@ impl StorageMapKey { } /// Client for querying runtime storage. -#[derive(Clone)] pub struct StorageClient<'a, T: Config> { rpc: &'a Rpc, metadata: &'a Metadata, iter_page_size: u32, } +impl<'a, T: Config> Clone for StorageClient<'a, T> { + fn clone(&self) -> Self { + Self { + rpc: self.rpc, + metadata: self.metadata, + iter_page_size: self.iter_page_size, + } + } +} + impl<'a, T: Config> StorageClient<'a, T> { /// Create a new [`StorageClient`] pub fn new(rpc: &'a Rpc, metadata: &'a Metadata, iter_page_size: u32) -> Self { From a15a26e75f2abf7375f1c60f6ad4c6a6ce8e0d26 Mon Sep 17 00:00:00 2001 From: James Wilson Date: Wed, 12 Jan 2022 18:36:23 +0000 Subject: [PATCH 2/3] loosen a couple more derive bounds --- src/config.rs | 7 +++---- src/lib.rs | 10 +++++++++- src/transaction.rs | 22 +++++++++++++--------- 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/config.rs b/src/config.rs index 1258660a25..288888dcdf 100644 --- a/src/config.rs +++ b/src/config.rs @@ -86,10 +86,9 @@ pub trait Parameter: Codec + EncodeLike + Clone + Eq + Debug {} impl Parameter for T where T: Codec + EncodeLike + Clone + Eq + Debug {} /// Default set of commonly used types by Substrate runtimes. -// Note: We only use this at the type level, so instances of it should not be constructed, -// hence the private () type contained within. -#[derive(Debug)] -pub struct DefaultConfig(()); +// Note: We only use this at the type level, so it should be impossible to +// create an instance of it. +pub enum DefaultConfig {} impl Config for DefaultConfig { type Index = u32; diff --git a/src/lib.rs b/src/lib.rs index 0d8db57693..5b78c89cc4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -57,6 +57,7 @@ use core::{ fmt::Debug, marker::PhantomData, }; +use derivative::Derivative; mod client; mod config; @@ -180,7 +181,14 @@ pub enum Phase { /// /// [`WrapperKeepOpaque`] stores the type only in its opaque format, aka as a `Vec`. To /// access the real type `T` [`Self::try_decode`] needs to be used. -#[derive(Debug, Eq, PartialEq, Default, Clone, Decode, Encode)] +#[derive(Derivative, Encode, Decode)] +#[derivative( + Debug(bound = ""), + Clone(bound = ""), + PartialEq(bound = ""), + Eq(bound = ""), + Default(bound = "") +)] pub struct WrapperKeepOpaque { data: Vec, _phantom: PhantomData, diff --git a/src/transaction.rs b/src/transaction.rs index b38b632bb7..d1396a1f5d 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -16,11 +16,6 @@ use std::task::Poll; -use sp_core::storage::StorageKey; -use sp_runtime::traits::Hash; -pub use sp_runtime::traits::SignedExtension; -pub use sp_version::RuntimeVersion; - use crate::{ client::Client, error::{ @@ -32,6 +27,7 @@ use crate::{ Config, Phase, }; +use derivative::Derivative; use futures::{ Stream, StreamExt, @@ -40,10 +36,15 @@ use jsonrpsee::core::{ client::Subscription as RpcSubscription, Error as RpcError, }; +use sp_core::storage::StorageKey; +use sp_runtime::traits::Hash; +pub use sp_runtime::traits::SignedExtension; +pub use sp_version::RuntimeVersion; /// This struct represents a subscription to the progress of some transaction, and is /// returned from [`crate::SubmittableExtrinsic::sign_and_submit_then_watch()`]. -#[derive(Debug)] +#[derive(Derivative)] +#[derivative(Debug(bound = ""))] pub struct TransactionProgress<'client, T: Config> { sub: Option>>, ext_hash: T::Hash, @@ -261,7 +262,8 @@ impl<'client, T: Config> Stream for TransactionProgress<'client, T> { /// finalized. The `FinalityTimeout` event will be emitted when the block did not reach finality /// within 512 blocks. This either indicates that finality is not available for your chain, /// or that finality gadget is lagging behind. -#[derive(Debug)] +#[derive(Derivative)] +#[derivative(Debug(bound = ""))] pub enum TransactionStatus<'client, T: Config> { /// The transaction is part of the "future" queue. Future, @@ -310,7 +312,8 @@ impl<'client, T: Config> TransactionStatus<'client, T> { } /// This struct represents a transaction that has made it into a block. -#[derive(Debug)] +#[derive(Derivative)] +#[derivative(Debug(bound = ""))] pub struct TransactionInBlock<'client, T: Config> { block_hash: T::Hash, ext_hash: T::Hash, @@ -416,7 +419,8 @@ impl<'client, T: Config> TransactionInBlock<'client, T> { /// This represents the events related to our transaction. /// We can iterate over the events, or look for a specific one. -#[derive(Debug)] +#[derive(Derivative)] +#[derivative(Debug(bound = ""))] pub struct TransactionEvents { block_hash: T::Hash, ext_hash: T::Hash, From 58675def4bd150198feea5061517feeb91f3196e Mon Sep 17 00:00:00 2001 From: James Wilson Date: Thu, 13 Jan 2022 11:08:12 +0000 Subject: [PATCH 3/3] Use PhantomDataSendSync to avoid accidentally removing Send+Sync bounds --- src/events.rs | 11 ++++------- src/extrinsic/extra.rs | 23 +---------------------- src/lib.rs | 33 +++++++++++++++++++++++++++------ 3 files changed, 32 insertions(+), 35 deletions(-) diff --git a/src/events.rs b/src/events.rs index 17ad0583e5..b114997772 100644 --- a/src/events.rs +++ b/src/events.rs @@ -23,6 +23,7 @@ use crate::{ Error, Event, Metadata, + PhantomDataSendSync, Phase, }; use codec::{ @@ -39,7 +40,6 @@ use scale_info::{ TypeDefPrimitive, }; use sp_core::Bytes; -use std::marker::PhantomData; /// Raw bytes for an Event #[derive(Debug)] @@ -71,15 +71,12 @@ impl RawEvent { /// Events decoder. #[derive(Derivative)] #[derivative(Clone(bound = ""), Debug(bound = ""))] -pub struct EventsDecoder { +pub struct EventsDecoder { metadata: Metadata, - marker: PhantomData, + marker: PhantomDataSendSync, } -impl EventsDecoder -where - T: Config, -{ +impl EventsDecoder { /// Creates a new `EventsDecoder`. pub fn new(metadata: Metadata) -> Self { Self { diff --git a/src/extrinsic/extra.rs b/src/extrinsic/extra.rs index 27676ec3e9..7a456e4a5c 100644 --- a/src/extrinsic/extra.rs +++ b/src/extrinsic/extra.rs @@ -14,6 +14,7 @@ // You should have received a copy of the GNU General Public License // along with subxt. If not, see . +use crate::PhantomDataSendSync; use codec::{ Decode, Encode, @@ -44,28 +45,6 @@ use crate::Config; /// This is modified from the substrate version to allow passing in of the version, which is /// returned via `additional_signed()`. -/// A version of [`std::marker::PhantomData`] that is also Send and Sync (which is fine -/// because regardless of the generic param, it is always possible to Send + Sync this). -#[derive(Derivative, Encode, Decode, TypeInfo)] -#[derivative( - Clone(bound = ""), - PartialEq(bound = ""), - Debug(bound = ""), - Eq(bound = ""), - Default(bound = "") -)] -#[scale_info(skip_type_params(T))] -pub struct PhantomDataSendSync(core::marker::PhantomData); - -unsafe impl Send for PhantomDataSendSync {} -unsafe impl Sync for PhantomDataSendSync {} - -impl PhantomDataSendSync { - pub(crate) fn new() -> Self { - Self(core::marker::PhantomData) - } -} - /// Ensure the runtime version registered in the transaction is the same as at present. #[derive(Derivative, Encode, Decode, TypeInfo)] #[derivative( diff --git a/src/lib.rs b/src/lib.rs index 5b78c89cc4..405c43f79c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -53,10 +53,7 @@ use codec::{ DecodeAll, Encode, }; -use core::{ - fmt::Debug, - marker::PhantomData, -}; +use core::fmt::Debug; use derivative::Derivative; mod client; @@ -191,7 +188,7 @@ pub enum Phase { )] pub struct WrapperKeepOpaque { data: Vec, - _phantom: PhantomData, + _phantom: PhantomDataSendSync, } impl WrapperKeepOpaque { @@ -216,7 +213,31 @@ impl WrapperKeepOpaque { pub fn from_encoded(data: Vec) -> Self { Self { data, - _phantom: PhantomData, + _phantom: PhantomDataSendSync::new(), } } } + +/// A version of [`std::marker::PhantomData`] that is also Send and Sync (which is fine +/// because regardless of the generic param, it is always possible to Send + Sync this +/// 0 size type). +#[derive(Derivative, Encode, Decode, scale_info::TypeInfo)] +#[derivative( + Clone(bound = ""), + PartialEq(bound = ""), + Debug(bound = ""), + Eq(bound = ""), + Default(bound = "") +)] +#[scale_info(skip_type_params(T))] +#[doc(hidden)] +pub struct PhantomDataSendSync(core::marker::PhantomData); + +impl PhantomDataSendSync { + pub(crate) fn new() -> Self { + Self(core::marker::PhantomData) + } +} + +unsafe impl Send for PhantomDataSendSync {} +unsafe impl Sync for PhantomDataSendSync {}