diff --git a/core/sr-primitives/src/generic/checked_extrinsic.rs b/core/sr-primitives/src/generic/checked_extrinsic.rs index 510a7ac8c85c9..3fc7711ccc451 100644 --- a/core/sr-primitives/src/generic/checked_extrinsic.rs +++ b/core/sr-primitives/src/generic/checked_extrinsic.rs @@ -18,8 +18,10 @@ //! stage. use crate::traits::{ - self, Member, MaybeDisplay, SignedExtension, Dispatchable, ValidateUnsigned, + self, Member, MaybeDisplay, SignedExtension, Dispatchable, }; +#[allow(deprecated)] +use crate::traits::ValidateUnsigned; use crate::weights::{GetDispatchInfo, DispatchInfo}; use crate::transaction_validity::TransactionValidity; @@ -51,6 +53,7 @@ where self.signed.as_ref().map(|x| &x.0) } + #[allow(deprecated)] // Allow ValidateUnsigned fn validate>( &self, info: DispatchInfo, @@ -60,11 +63,13 @@ where Extra::validate(extra, id, &self.function, info, len) } else { let valid = Extra::validate_unsigned(&self.function, info, len)?; - Ok(valid.combine_with(U::validate_unsigned(&self.function)?)) + let unsigned_validation = U::validate_unsigned(&self.function)?; + Ok(valid.combine_with(unsigned_validation)) } } - fn apply( + #[allow(deprecated)] // Allow ValidateUnsigned + fn apply>( self, info: DispatchInfo, len: usize, @@ -74,6 +79,7 @@ where (Some(id), pre) } else { let pre = Extra::pre_dispatch_unsigned(&self.function, info, len)?; + U::pre_dispatch(&self.function)?; (None, pre) }; let res = self.function.dispatch(Origin::from(maybe_who)); diff --git a/core/sr-primitives/src/testing.rs b/core/sr-primitives/src/testing.rs index d60e58bab1a5c..b1bd94a5461b1 100644 --- a/core/sr-primitives/src/testing.rs +++ b/core/sr-primitives/src/testing.rs @@ -20,9 +20,11 @@ use serde::{Serialize, Serializer, Deserialize, de::Error as DeError, Deserializ use std::{fmt::Debug, ops::Deref, fmt, cell::RefCell}; use crate::codec::{Codec, Encode, Decode}; use crate::traits::{ - self, Checkable, Applyable, BlakeTwo256, OpaqueKeys, ValidateUnsigned, + self, Checkable, Applyable, BlakeTwo256, OpaqueKeys, SignedExtension, Dispatchable, }; +#[allow(deprecated)] +use crate::traits::ValidateUnsigned; use crate::{generic, KeyTypeId, ApplyResult}; use crate::weights::{GetDispatchInfo, DispatchInfo}; pub use primitives::{H256, sr25519}; @@ -323,6 +325,7 @@ impl Applyable for TestXt where fn sender(&self) -> Option<&Self::AccountId> { self.0.as_ref().map(|x| &x.0) } /// Checks to see if this is a valid *transaction*. It returns information on it if so. + #[allow(deprecated)] // Allow ValidateUnsigned fn validate>( &self, _info: DispatchInfo, @@ -333,7 +336,8 @@ impl Applyable for TestXt where /// Executes all necessary logic needed prior to dispatch and deconstructs into function call, /// index and sender. - fn apply( + #[allow(deprecated)] // Allow ValidateUnsigned + fn apply>( self, info: DispatchInfo, len: usize, diff --git a/core/sr-primitives/src/traits.rs b/core/sr-primitives/src/traits.rs index ecaf6a78fcfe4..195fca26fad19 100644 --- a/core/sr-primitives/src/traits.rs +++ b/core/sr-primitives/src/traits.rs @@ -755,9 +755,6 @@ pub trait SignedExtension: Codec + Debug + Sync + Send + Clone + Eq + PartialEq /// Validate an unsigned transaction for the transaction queue. /// - /// Normally the default implementation is fine since `ValidateUnsigned` - /// is a better way of recognising and validating unsigned transactions. - /// /// This function can be called frequently by the transaction queue, /// to obtain transaction validity against current state. /// It should perform all checks that determine a valid unsigned transaction, @@ -889,6 +886,7 @@ pub trait Applyable: Sized + Send + Sync { fn sender(&self) -> Option<&Self::AccountId>; /// Checks to see if this is a valid *transaction*. It returns information on it if so. + #[allow(deprecated)] // Allow ValidateUnsigned fn validate>( &self, info: DispatchInfo, @@ -897,7 +895,8 @@ pub trait Applyable: Sized + Send + Sync { /// Executes all necessary logic needed prior to dispatch and deconstructs into function call, /// index and sender. - fn apply( + #[allow(deprecated)] // Allow ValidateUnsigned + fn apply>( self, info: DispatchInfo, len: usize, @@ -966,10 +965,27 @@ pub trait RuntimeApiInfo { /// the transaction for the transaction pool. /// During block execution phase one need to perform the same checks anyway, /// since this function is not being called. +#[deprecated(note = "Use SignedExtensions instead.")] pub trait ValidateUnsigned { /// The call to validate type Call; + /// Validate the call right before dispatch. + /// + /// This method should be used to prevent transactions already in the pool + /// (i.e. passing `validate_unsigned`) from being included in blocks + /// in case we know they now became invalid. + /// + /// By default it's a good idea to call `validate_unsigned` from within + /// this function again to make sure we never include an invalid transaction. + /// + /// Changes made to storage WILL be persisted if the call returns `Ok`. + fn pre_dispatch(call: &Self::Call) -> Result<(), crate::ApplyError> { + Self::validate_unsigned(call) + .map(|_| ()) + .map_err(Into::into) + } + /// Return the validity of the call /// /// This doesn't execute any side-effects; it merely checks diff --git a/srml/executive/src/lib.rs b/srml/executive/src/lib.rs index 927a7fef25581..146e0ebcadcd8 100644 --- a/srml/executive/src/lib.rs +++ b/srml/executive/src/lib.rs @@ -60,7 +60,9 @@ //! # pub type AllModules = u64; //! # pub enum Runtime {}; //! # use sr_primitives::transaction_validity::{TransactionValidity, UnknownTransaction}; +//! # #[allow(deprecated)] //! # use sr_primitives::traits::ValidateUnsigned; +//! # #[allow(deprecated)] //! # impl ValidateUnsigned for Runtime { //! # type Call = (); //! # @@ -79,10 +81,12 @@ use sr_primitives::{ generic::Digest, ApplyResult, weights::GetDispatchInfo, traits::{ self, Header, Zero, One, Checkable, Applyable, CheckEqual, OnFinalize, OnInitialize, - NumberFor, Block as BlockT, OffchainWorker, ValidateUnsigned, Dispatchable + NumberFor, Block as BlockT, OffchainWorker, Dispatchable, }, transaction_validity::TransactionValidity, }; +#[allow(deprecated)] +use sr_primitives::traits::ValidateUnsigned; use codec::{Codec, Encode}; use system::{extrinsics_root, DigestOf}; @@ -100,6 +104,7 @@ pub struct Executive( PhantomData<(System, Block, Context, UnsignedValidator, AllModules)> ); +#[allow(deprecated)] // Allow ValidateUnsigned, remove the attribute when the trait is removed. impl< System: system::Trait, Block: traits::Block, @@ -119,6 +124,7 @@ where } } +#[allow(deprecated)] // Allow ValidateUnsigned, remove the attribute when the trait is removed. impl< System: system::Trait, Block: traits::Block, @@ -242,7 +248,7 @@ where // Decode parameters and dispatch let dispatch_info = xt.get_dispatch_info(); - let r = Applyable::apply(xt, dispatch_info, encoded_len)?; + let r = Applyable::apply::(xt, dispatch_info, encoded_len)?; >::note_applied_extrinsic(&r, encoded_len as u32); @@ -326,7 +332,6 @@ mod tests { } } - // Workaround for https://github.com/rust-lang/rust/issues/26925 . Remove when sorted. #[derive(Clone, Eq, PartialEq)] pub struct Runtime; parameter_types! { @@ -382,9 +387,14 @@ mod tests { type FeeMultiplierUpdate = (); } + #[allow(deprecated)] impl ValidateUnsigned for Runtime { type Call = Call; + fn pre_dispatch(_call: &Self::Call) -> Result<(), ApplyError> { + Ok(()) + } + fn validate_unsigned(call: &Self::Call) -> TransactionValidity { match call { Call::Balances(BalancesCall::set_balance(_, _, _)) => Ok(Default::default()), diff --git a/srml/im-online/src/lib.rs b/srml/im-online/src/lib.rs index fa970f4997891..49280f93b28db 100644 --- a/srml/im-online/src/lib.rs +++ b/srml/im-online/src/lib.rs @@ -88,7 +88,7 @@ use sr_staking_primitives::{ offence::{ReportOffence, Offence, Kind}, }; use support::{ - decl_module, decl_event, decl_storage, print, ensure, Parameter, debug + decl_module, decl_event, decl_storage, print, Parameter, debug }; use system::ensure_none; use system::offchain::SubmitUnsignedTransaction; @@ -243,24 +243,20 @@ decl_module! { fn heartbeat( origin, heartbeat: Heartbeat, - signature: ::Signature + // since signature verification is done in `validate_unsigned` + // we can skip doing it here again. + _signature: ::Signature ) { ensure_none(origin)?; let current_session = >::current_index(); - ensure!(current_session == heartbeat.session_index, "Outdated heartbeat received."); let exists = ::exists( ¤t_session, &heartbeat.authority_index ); let keys = Keys::::get(); - let maybe_public = keys.get(heartbeat.authority_index as usize); - if let (false, Some(public)) = (exists, maybe_public) { - let signature_valid = heartbeat.using_encoded(|encoded_heartbeat| { - public.verify(&encoded_heartbeat, &signature) - }); - ensure!(signature_valid, "Invalid heartbeat signature."); - + let public = keys.get(heartbeat.authority_index as usize); + if let (false, Some(public)) = (exists, public) { Self::deposit_event(Event::::HeartbeatReceived(public.clone())); let network_state = heartbeat.network_state.encode(); @@ -545,6 +541,7 @@ impl session::OneSessionHandler for Module { } } +#[allow(deprecated)] impl support::unsigned::ValidateUnsigned for Module { type Call = Call; diff --git a/srml/im-online/src/tests.rs b/srml/im-online/src/tests.rs index 57f2e4008b61c..b744d7c57d7e3 100644 --- a/srml/im-online/src/tests.rs +++ b/srml/im-online/src/tests.rs @@ -103,6 +103,9 @@ fn heartbeat( authority_index: u32, id: UintAuthorityId, ) -> dispatch::Result { + #[allow(deprecated)] + use support::unsigned::ValidateUnsigned; + let heartbeat = Heartbeat { block_number, network_state: OpaqueNetworkState { @@ -114,6 +117,8 @@ fn heartbeat( }; let signature = id.sign(&heartbeat.encode()).unwrap(); + #[allow(deprecated)] // Allow ValidateUnsigned + ImOnline::pre_dispatch(&crate::Call::heartbeat(heartbeat.clone(), signature.clone()))?; ImOnline::heartbeat( Origin::system(system::RawOrigin::None), heartbeat, @@ -170,8 +175,8 @@ fn late_heartbeat_should_fail() { assert_eq!(Session::validators(), vec![1, 2, 3]); // when - assert_noop!(heartbeat(1, 3, 0, 1.into()), "Outdated heartbeat received."); - assert_noop!(heartbeat(1, 1, 0, 1.into()), "Outdated heartbeat received."); + assert_noop!(heartbeat(1, 3, 0, 1.into()), "Transaction is outdated"); + assert_noop!(heartbeat(1, 1, 0, 1.into()), "Transaction is outdated"); }); } diff --git a/srml/support/src/unsigned.rs b/srml/support/src/unsigned.rs index 4d2ceddd79f4a..282f3ac8ae9a8 100644 --- a/srml/support/src/unsigned.rs +++ b/srml/support/src/unsigned.rs @@ -15,6 +15,7 @@ // along with Substrate. If not, see . #[doc(hidden)] +#[allow(deprecated)] pub use crate::sr_primitives::traits::ValidateUnsigned; #[doc(hidden)] pub use crate::sr_primitives::transaction_validity::{TransactionValidity, UnknownTransaction}; @@ -65,9 +66,20 @@ macro_rules! impl_outer_validate_unsigned { $( $module:ident )* } ) => { + #[allow(deprecated)] // Allow ValidateUnsigned impl $crate::unsigned::ValidateUnsigned for $runtime { type Call = Call; + fn pre_dispatch(call: &Self::Call) -> Result<(), $crate::unsigned::ApplyError> { + #[allow(unreachable_patterns)] + match call { + $( Call::$module(inner_call) => $module::pre_dispatch(inner_call), )* + // pre-dispatch should not stop inherent extrinsics, validation should prevent + // including arbitrary (non-inherent) extrinsics to blocks. + _ => Ok(()), + } + } + fn validate_unsigned(call: &Self::Call) -> $crate::unsigned::TransactionValidity { #[allow(unreachable_patterns)] match call { @@ -97,6 +109,7 @@ mod test_partial_and_full_call { pub mod timestamp { pub struct Module; + #[allow(deprecated)] // Allow ValidateUnsigned impl super::super::ValidateUnsigned for Module { type Call = Call;