From 7ca0ab70881f26f926daa36f2c2c4a07a1c6455b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Fri, 16 Aug 2019 11:24:02 +0200 Subject: [PATCH 1/3] Verify signature and session index during apply phase of im-online. --- core/sr-primitives/src/traits.rs | 7 ++++++- srml/im-online/src/lib.rs | 10 ++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/core/sr-primitives/src/traits.rs b/core/sr-primitives/src/traits.rs index 5fc0cb5c5761b..f5aeab23b5eab 100644 --- a/core/sr-primitives/src/traits.rs +++ b/core/sr-primitives/src/traits.rs @@ -1095,7 +1095,12 @@ pub trait RuntimeApiInfo { const VERSION: u32; } -/// Something that can validate unsigned extrinsics. +/// Something that can validate unsigned extrinsics for the transaction pool. +/// +/// Note that any checks done here are only used for determining the validity of +/// 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. pub trait ValidateUnsigned { /// The call to validate type Call; diff --git a/srml/im-online/src/lib.rs b/srml/im-online/src/lib.rs index 37e9882b4ceea..29811f8f926bf 100644 --- a/srml/im-online/src/lib.rs +++ b/srml/im-online/src/lib.rs @@ -77,7 +77,7 @@ use rstd::prelude::*; use session::SessionIndex; use sr_io::Printable; use srml_support::{ - StorageValue, decl_module, decl_event, decl_storage, StorageDoubleMap, print, + StorageValue, decl_module, decl_event, decl_storage, StorageDoubleMap, print, ensure }; use system::ensure_none; use app_crypto::RuntimeAppPublic; @@ -206,11 +206,12 @@ decl_module! { fn heartbeat( origin, heartbeat: Heartbeat, - _signature: AuthoritySignature + signature: AuthoritySignature ) { ensure_none(origin)?; let current_session = >::current_index(); + ensure!(current_session == heartbeat.session_index, "Outdated hearbeat received."); let exists = ::exists( ¤t_session, &heartbeat.authority_index @@ -218,6 +219,11 @@ decl_module! { let keys = Keys::get(); let public = keys.get(heartbeat.authority_index as usize); if let (true, Some(public)) = (!exists, public) { + let signature_valid = heartbeat.using_encoded(|encoded_heartbeat| { + public.verify(&encoded_heartbeat, &signature) + }); + ensure!(signature_valid, "Invalid hearbeat signature."); + Self::deposit_event(Event::HeartbeatReceived(public.clone())); let network_state = heartbeat.network_state.encode(); From 70a97f41f45044615c739e55bdf9098672afe7ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Fri, 16 Aug 2019 11:57:26 +0200 Subject: [PATCH 2/3] Bump impl_version. --- node/runtime/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index d3eced8f61caa..166711fa893eb 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -81,7 +81,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { // implementation changes and behavior does not, then leave spec_version as // is and increment impl_version. spec_version: 143, - impl_version: 143, + impl_version: 144, apis: RUNTIME_API_VERSIONS, }; From 03ca259b40fb8ac2333041e91e6986571926b25b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Fri, 16 Aug 2019 12:33:17 +0200 Subject: [PATCH 3/3] Add docs to SignedExtension --- core/sr-primitives/src/traits.rs | 36 +++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/core/sr-primitives/src/traits.rs b/core/sr-primitives/src/traits.rs index f5aeab23b5eab..abbbef3a5b260 100644 --- a/core/sr-primitives/src/traits.rs +++ b/core/sr-primitives/src/traits.rs @@ -874,6 +874,14 @@ pub trait SignedExtension: fn additional_signed(&self) -> Result; /// Validate a signed transaction for the transaction queue. + /// + /// 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 transaction, + /// that can pay for it's execution and quickly eliminate ones + /// that are stale or incorrect. + /// + /// Make sure to perform the same checks in `pre_dispatch` function. fn validate( &self, _who: &Self::AccountId, @@ -885,6 +893,13 @@ pub trait SignedExtension: } /// Do any pre-flight stuff for a signed transaction. + /// + /// Note this function by default delegates to `validate`, so that + /// all checks performed for the transaction queue are also performed during + /// the dispatch phase (applying the extrinsic). + /// + /// If you ever override this function, you need to make sure to always + /// perform the same validation as in `validate`. fn pre_dispatch( self, who: &Self::AccountId, @@ -895,9 +910,17 @@ pub trait SignedExtension: self.validate(who, call, info, len).map(|_| Self::Pre::default()) } - /// 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. + /// 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, + /// and quickly eliminate ones that are stale or incorrect. + /// + /// Make sure to perform the same checks in `pre_dispatch_unsigned` function. fn validate_unsigned( _call: &Self::Call, _info: DispatchInfo, @@ -905,6 +928,13 @@ pub trait SignedExtension: ) -> Result { Ok(Default::default()) } /// Do any pre-flight stuff for a unsigned transaction. + /// + /// Note this function by default delegates to `validate_unsigned`, so that + /// all checks performed for the transaction queue are also performed during + /// the dispatch phase (applying the extrinsic). + /// + /// If you ever override this function, you need to make sure to always + /// perform the same validation as in `validate_unsigned`. fn pre_dispatch_unsigned( call: &Self::Call, info: DispatchInfo,