In general Pair trait is not strictly required to be Send/Sync/Clone.
Secrets throughout the codebase are constructed and destroyed asap (as should be).
In some situation these bounds could even be detrimental; some components may require the secrets to not be shared across threads (in practice silently moved to another stack without cleaning up the previous location) or cloned (e.g. in cases where the backend doesn't implement something like zeroize to clean-up memory).
Furthermore, secrets provided by some backends may not allow for these bounds. For example the upcoming Bandersnatch ring-vrf secret is not Sync. So here there is not much space for options wrt this trait bound.
The only place where these bounds are currently vacuously required is in AURA code.
Here some components are generic over the key type and these components are required to be Send.
However the Pair is not part of the components struct instance and the generic is only used as a bound to a particular key type and thus authority id type (i.e. these structs contain a PhantomData<Pair>).
The issue is easily addressed by using PhantomData<fn() -> Pair> instead of PhantomData<Pair>.
PhantomData<fn() -> Pair> is both Send and Sync (regardless of Pair bounds).
That is, the types with PhantomData doesn't own a Pair, but instead they can potentially produce Pairs (this is the big difference).
The PR also contains trivial cleanups and removal of other bounds where possible
davxy
commented
1 year ago
In general
Pairtrait is not strictly required to beSend/Sync/Clone.Secrets throughout the codebase are constructed and destroyed asap (as should be).
In some situation these bounds could even be detrimental; some components may require the secrets to not be shared across threads (in practice silently moved to another stack without cleaning up the previous location) or cloned (e.g. in cases where the backend doesn't implement something like
zeroizeto clean-up memory).Furthermore, secrets provided by some backends may not allow for these bounds. For example the upcoming Bandersnatch
ring-vrfsecret is notSync. So here there is not much space for options wrt this trait bound.The only place where these bounds are currently vacuously required is in AURA code.
Here some components are generic over the key type and these components are required to be
Send. However thePairis not part of the components struct instance and the generic is only used as a bound to a particular key type and thus authority id type (i.e. these structs contain aPhantomData<Pair>).The issue is easily addressed by using
PhantomData<fn() -> Pair>instead ofPhantomData<Pair>.PhantomData<fn() -> Pair>is bothSendandSync(regardless ofPairbounds).That is, the types with
PhantomDatadoesn't own aPair, but instead they can potentially producePairs (this is the big difference).The PR also contains trivial cleanups and removal of other bounds where possible
cumulus companion: https://github.com/paritytech/cumulus/pull/2941
required by https://github.com/paritytech/substrate/pull/14412
Nomicon ref (not published yet): https://github.com/rust-lang/nomicon/pull/411