From c3af86cee45d98dd5f97fb2de419ba4e25ac14b7 Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Mon, 20 Jan 2020 12:57:49 +0100 Subject: [PATCH 01/18] Move DummySpecialization to sc-network (#4680) --- client/network/src/protocol/specialization.rs | 26 ++++++++++++++++++ client/network/test/src/lib.rs | 27 +------------------ 2 files changed, 27 insertions(+), 26 deletions(-) diff --git a/client/network/src/protocol/specialization.rs b/client/network/src/protocol/specialization.rs index 9b1452160e535..af6d5f7a239cd 100644 --- a/client/network/src/protocol/specialization.rs +++ b/client/network/src/protocol/specialization.rs @@ -57,6 +57,32 @@ pub trait NetworkSpecialization: Send + Sync + 'static { fn on_block_imported(&mut self, _ctx: &mut dyn Context, _hash: B::Hash, _header: &B::Header) { } } +/// A specialization that does nothing. +#[derive(Clone)] +pub struct DummySpecialization; + +impl NetworkSpecialization for DummySpecialization { + fn status(&self) -> Vec { + vec![] + } + + fn on_connect( + &mut self, + _ctx: &mut dyn Context, + _peer_id: PeerId, + _status: crate::message::Status + ) {} + + fn on_disconnect(&mut self, _ctx: &mut dyn Context, _peer_id: PeerId) {} + + fn on_message( + &mut self, + _ctx: &mut dyn Context, + _peer_id: PeerId, + _message: Vec, + ) {} +} + /// Construct a simple protocol that is composed of several sub protocols. /// Each "sub protocol" needs to implement `Specialization` and needs to provide a `new()` function. /// For more fine grained implementations, this macro is not usable. diff --git a/client/network/test/src/lib.rs b/client/network/test/src/lib.rs index f1b7fa478c6c8..1b13e83343e58 100644 --- a/client/network/test/src/lib.rs +++ b/client/network/test/src/lib.rs @@ -62,6 +62,7 @@ use substrate_test_runtime_client::{self, AccountKeyring}; pub use substrate_test_runtime_client::runtime::{Block, Extrinsic, Hash, Transfer}; pub use substrate_test_runtime_client::{TestClient, TestClientBuilder, TestClientBuilderExt}; +pub use sc_network::specialization::DummySpecialization; type AuthorityId = sp_consensus_babe::AuthorityId; @@ -101,32 +102,6 @@ impl Verifier for PassThroughVerifier { } } -/// The test specialization. -#[derive(Clone)] -pub struct DummySpecialization; - -impl NetworkSpecialization for DummySpecialization { - fn status(&self) -> Vec { - vec![] - } - - fn on_connect( - &mut self, - _ctx: &mut dyn Context, - _peer_id: PeerId, - _status: sc_network::message::Status - ) {} - - fn on_disconnect(&mut self, _ctx: &mut dyn Context, _peer_id: PeerId) {} - - fn on_message( - &mut self, - _ctx: &mut dyn Context, - _peer_id: PeerId, - _message: Vec, - ) {} -} - pub type PeersFullClient = sc_client::Client; pub type PeersLightClient = From 561bd727489501cfb24d29020062b3a5d4fdcca9 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 20 Jan 2020 15:58:39 +0100 Subject: [PATCH 02/18] keep nominations after getting kicked with zero slash (#4681) * keep nominations after getting kicked with zero slash * rename next_key to maybe_next_key Co-Authored-By: Gavin Wood Co-authored-by: Gavin Wood --- frame/staking/src/lib.rs | 4 +- frame/staking/src/migration.rs | 58 +++++++++++++++++++-- frame/staking/src/slashing.rs | 27 +++++++--- frame/staking/src/tests.rs | 93 +++++++++++++++++++++++++++++++++- 4 files changed, 169 insertions(+), 13 deletions(-) diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index 326a01599034c..c547545c389a5 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -1482,11 +1482,11 @@ impl Module { let Nominations { submitted_in, mut targets, suppressed: _ } = nominations; // Filter out nomination targets which were nominated before the most recent - // slashing span. + // non-zero slash. targets.retain(|stash| { ::SlashingSpans::get(&stash).map_or( true, - |spans| submitted_in >= spans.last_start(), + |spans| submitted_in >= spans.last_nonzero_slash(), ) }); diff --git a/frame/staking/src/migration.rs b/frame/staking/src/migration.rs index bb020b0fc0e5f..6cb472375a48d 100644 --- a/frame/staking/src/migration.rs +++ b/frame/staking/src/migration.rs @@ -20,12 +20,14 @@ pub type VersionNumber = u32; // the current expected version of the storage -pub const CURRENT_VERSION: VersionNumber = 1; +pub const CURRENT_VERSION: VersionNumber = 2; +/// The inner logic of migrations. #[cfg(any(test, feature = "migrate"))] -mod inner { +pub mod inner { use crate::{Store, Module, Trait}; - use frame_support::{StorageLinkedMap, StorageValue}; + use frame_support::{StorageLinkedMap, StoragePrefixedMap, StorageValue}; + use codec::{Encode, Decode}; use sp_std::vec::Vec; use super::{CURRENT_VERSION, VersionNumber}; @@ -60,6 +62,55 @@ mod inner { frame_support::print("Finished migrating Staking storage to v1."); } + // migrate storage from v1 to v2: adds another field to the `SlashingSpans` + // struct. + pub fn to_v2(version: &mut VersionNumber) { + use crate::{EraIndex, slashing::SpanIndex}; + #[derive(Decode)] + struct V1SlashingSpans { + span_index: SpanIndex, + last_start: EraIndex, + prior: Vec, + } + + #[derive(Encode)] + struct V2SlashingSpans { + span_index: SpanIndex, + last_start: EraIndex, + last_nonzero_slash: EraIndex, + prior: Vec, + } + + if *version != 1 { return } + *version += 1; + + let prefix = as Store>::SlashingSpans::final_prefix(); + let mut current_key = prefix.to_vec(); + loop { + let maybe_next_key = sp_io::storage::next_key(¤t_key[..]) + .filter(|v| v.starts_with(&prefix[..])); + + match maybe_next_key { + Some(next_key) => { + let maybe_spans = sp_io::storage::get(&next_key[..]) + .and_then(|v| V1SlashingSpans::decode(&mut &v[..]).ok()); + if let Some(spans) = maybe_spans { + let new_val = V2SlashingSpans { + span_index: spans.span_index, + last_start: spans.last_start, + last_nonzero_slash: spans.last_start, + prior: spans.prior, + }.encode(); + + sp_io::storage::set(&next_key[..], &new_val[..]); + } + current_key = next_key; + } + None => break, + } + } + } + pub(super) fn perform_migrations() { as Store>::StorageVersion::mutate(|version| { if *version < MIN_SUPPORTED_VERSION { @@ -72,6 +123,7 @@ mod inner { if *version == CURRENT_VERSION { return } to_v1::(version); + to_v2::(version); }); } } diff --git a/frame/staking/src/slashing.rs b/frame/staking/src/slashing.rs index 7322b9a1d3118..df36b1c763c4b 100644 --- a/frame/staking/src/slashing.rs +++ b/frame/staking/src/slashing.rs @@ -90,7 +90,9 @@ pub struct SlashingSpans { span_index: SpanIndex, // the start era of the most recent (ongoing) slashing span. last_start: EraIndex, - // all prior slashing spans start indices, in reverse order (most recent first) + // the last era at which a non-zero slash occurred. + last_nonzero_slash: EraIndex, + // all prior slashing spans' start indices, in reverse order (most recent first) // encoded as offsets relative to the slashing span after it. prior: Vec, } @@ -102,6 +104,10 @@ impl SlashingSpans { SlashingSpans { span_index: 0, last_start: window_start, + // initialize to zero, as this structure is lazily created until + // the first slash is applied. setting equal to `window_start` would + // put a time limit on nominations. + last_nonzero_slash: 0, prior: Vec::new(), } } @@ -136,9 +142,9 @@ impl SlashingSpans { sp_std::iter::once(last).chain(prior) } - /// Yields the era index where the last (current) slashing span started. - pub(crate) fn last_start(&self) -> EraIndex { - self.last_start + /// Yields the era index where the most recent non-zero slash occurred. + pub(crate) fn last_nonzero_slash(&self) -> EraIndex { + self.last_nonzero_slash } // prune the slashing spans against a window, whose start era index is given. @@ -457,8 +463,12 @@ impl<'a, T: 'a + Trait> InspectingSpans<'a, T> { self.dirty = self.spans.end_span(now) || self.dirty; } - fn add_slash(&mut self, amount: BalanceOf) { + // add some value to the slash of the staker. + // invariant: the staker is being slashed for non-zero value here + // although `amount` may be zero, as it is only a difference. + fn add_slash(&mut self, amount: BalanceOf, slash_era: EraIndex) { *self.slash_of += amount; + self.spans.last_nonzero_slash = sp_std::cmp::max(self.spans.last_nonzero_slash, slash_era); } // find the span index of the given era, if covered. @@ -489,7 +499,7 @@ impl<'a, T: 'a + Trait> InspectingSpans<'a, T> { let reward = REWARD_F1 * (self.reward_proportion * slash).saturating_sub(span_record.paid_out); - self.add_slash(difference); + self.add_slash(difference, slash_era); changed = true; reward @@ -681,6 +691,7 @@ mod tests { let spans = SlashingSpans { span_index: 0, last_start: 1000, + last_nonzero_slash: 0, prior: Vec::new(), }; @@ -695,6 +706,7 @@ mod tests { let spans = SlashingSpans { span_index: 10, last_start: 1000, + last_nonzero_slash: 0, prior: vec![10, 9, 8, 10], }; @@ -715,6 +727,7 @@ mod tests { let mut spans = SlashingSpans { span_index: 10, last_start: 1000, + last_nonzero_slash: 0, prior: vec![10, 9, 8, 10], }; @@ -768,6 +781,7 @@ mod tests { let mut spans = SlashingSpans { span_index: 10, last_start: 1000, + last_nonzero_slash: 0, prior: vec![10, 9, 8, 10], }; assert_eq!(spans.prune(2000), Some((6, 10))); @@ -784,6 +798,7 @@ mod tests { let mut spans = SlashingSpans { span_index: 1, last_start: 10, + last_nonzero_slash: 0, prior: Vec::new(), }; diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index c290c6a8581d4..bb1fe32025ebc 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -18,12 +18,13 @@ use super::*; use mock::*; +use codec::Encode; use sp_runtime::{assert_eq_error_rate, traits::{OnInitialize, BadOrigin}}; use sp_staking::offence::OffenceDetails; use frame_support::{ assert_ok, assert_noop, traits::{Currency, ReservableCurrency}, - dispatch::DispatchError, + dispatch::DispatchError, StorageMap, }; use substrate_test_utils::assert_eq_uvec; @@ -2710,7 +2711,95 @@ fn slash_kicks_validators_not_nominators() { // and make sure that the vote will be ignored even if the validator // re-registers. - let last_slash = ::SlashingSpans::get(&11).unwrap().last_start(); + let last_slash = ::SlashingSpans::get(&11).unwrap().last_nonzero_slash(); assert!(nominations.submitted_in < last_slash); }); } + +#[test] +fn migration_v2() { + ExtBuilder::default().build().execute_with(|| { + use crate::{EraIndex, slashing::SpanIndex}; + + #[derive(Encode)] + struct V1SlashingSpans { + span_index: SpanIndex, + last_start: EraIndex, + prior: Vec, + } + + // inject old-style values directly into storage. + let set = |stash, spans: V1SlashingSpans| { + let key = ::SlashingSpans::hashed_key_for(stash); + sp_io::storage::set(&key, &spans.encode()); + }; + + let spans_11 = V1SlashingSpans { + span_index: 10, + last_start: 1, + prior: vec![0], + }; + + let spans_21 = V1SlashingSpans { + span_index: 1, + last_start: 5, + prior: vec![], + }; + + set(11, spans_11); + set(21, spans_21); + + ::StorageVersion::put(1); + + // perform migration. + crate::migration::inner::to_v2::(&mut 1); + + assert_eq!( + ::SlashingSpans::get(&11).unwrap().last_nonzero_slash(), + 1, + ); + + assert_eq!( + ::SlashingSpans::get(&21).unwrap().last_nonzero_slash(), + 5, + ); + }); +} + +#[test] +fn zero_slash_keeps_nominators() { + ExtBuilder::default().build().execute_with(|| { + start_era(1); + + assert_eq!(Balances::free_balance(&11), 1000); + + let exposure = Staking::stakers(&11); + assert_eq!(Balances::free_balance(&101), 2000); + + on_offence_now( + &[ + OffenceDetails { + offender: (11, exposure.clone()), + reporters: vec![], + }, + ], + &[Perbill::from_percent(0)], + ); + + assert_eq!(Balances::free_balance(&11), 1000); + assert_eq!(Balances::free_balance(&101), 2000); + + // This is the best way to check that the validator was chilled; `get` will + // return default value. + for (stash, _) in ::Validators::enumerate() { + assert!(stash != 11); + } + + let nominations = ::Nominators::get(&101).unwrap(); + + // and make sure that the vote will not be ignored, because the slash was + // zero. + let last_slash = ::SlashingSpans::get(&11).unwrap().last_nonzero_slash(); + assert!(nominations.submitted_in >= last_slash); + }); +} From a90c72e6d7088d0473670bf74253c65791411589 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 20 Jan 2020 16:00:43 +0100 Subject: [PATCH 03/18] deprecate chain_status field of network handshake (#4675) * deprecate chain_status field of network handshake * Update client/network/src/protocol/message.rs remove unneeded whitespace. Co-Authored-By: Pierre Krieger Co-authored-by: Pierre Krieger --- client/network/src/protocol.rs | 2 +- client/network/src/protocol/message.rs | 58 +++++++++++++++++++++++++- 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/client/network/src/protocol.rs b/client/network/src/protocol.rs index 2aa29ea2793e3..6914ea9efe138 100644 --- a/client/network/src/protocol.rs +++ b/client/network/src/protocol.rs @@ -74,7 +74,7 @@ const MAX_KNOWN_BLOCKS: usize = 1024; // ~32kb per peer + LruHashSet overhead const MAX_KNOWN_EXTRINSICS: usize = 4096; // ~128kb per peer + overhead /// Current protocol version. -pub(crate) const CURRENT_VERSION: u32 = 5; +pub(crate) const CURRENT_VERSION: u32 = 6; /// Lowest version we support pub(crate) const MIN_VERSION: u32 = 3; diff --git a/client/network/src/protocol/message.rs b/client/network/src/protocol/message.rs index 30f0c34175aa5..ef7d550de6cbe 100644 --- a/client/network/src/protocol/message.rs +++ b/client/network/src/protocol/message.rs @@ -252,7 +252,29 @@ pub mod generic { } /// Status sent on connection. + // TODO https://github.com/paritytech/substrate/issues/4674: replace the `Status` + // struct with this one, after waiting a few releases beyond `NetworkSpecialization`'s + // removal (https://github.com/paritytech/substrate/pull/4665) + // + // and set MIN_VERSION to 6. #[derive(Debug, PartialEq, Eq, Clone, Encode, Decode)] + pub struct CompactStatus { + /// Protocol version. + pub version: u32, + /// Minimum supported version. + pub min_supported_version: u32, + /// Supported roles. + pub roles: Roles, + /// Best block number. + pub best_number: Number, + /// Best block hash. + pub best_hash: Hash, + /// Genesis block hash. + pub genesis_hash: Hash, + } + + /// Status sent on connection. + #[derive(Debug, PartialEq, Eq, Clone, Encode)] pub struct Status { /// Protocol version. pub version: u32, @@ -266,10 +288,44 @@ pub mod generic { pub best_hash: Hash, /// Genesis block hash. pub genesis_hash: Hash, - /// Chain-specific status. + /// DEPRECATED. Chain-specific status. pub chain_status: Vec, } + impl Decode for Status { + fn decode(value: &mut I) -> Result { + const LAST_CHAIN_STATUS_VERSION: u32 = 5; + let compact = CompactStatus::decode(value)?; + let chain_status = match >::decode(value) { + Ok(v) => v, + Err(e) => if compact.version <= LAST_CHAIN_STATUS_VERSION { + return Err(e) + } else { + Vec::new() + } + }; + + let CompactStatus { + version, + min_supported_version, + roles, + best_number, + best_hash, + genesis_hash, + } = compact; + + Ok(Status { + version, + min_supported_version, + roles, + best_number, + best_hash, + genesis_hash, + chain_status, + }) + } + } + /// Request block data from a peer. #[derive(Debug, PartialEq, Eq, Clone, Encode, Decode)] pub struct BlockRequest { From a083572ebfb4b85864c6016447b1481cc701b96b Mon Sep 17 00:00:00 2001 From: Max Inden Date: Mon, 20 Jan 2020 17:25:56 +0100 Subject: [PATCH 04/18] client/finality-grandpa/communication: Add doc comment for PeerReport (#4684) --- client/finality-grandpa/src/communication/gossip.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/client/finality-grandpa/src/communication/gossip.rs b/client/finality-grandpa/src/communication/gossip.rs index ec74393d80ff1..7b21c1d0797d3 100644 --- a/client/finality-grandpa/src/communication/gossip.rs +++ b/client/finality-grandpa/src/communication/gossip.rs @@ -1445,6 +1445,7 @@ impl sc_network_gossip::Validator for GossipValidator Date: Tue, 21 Jan 2020 00:26:19 +0800 Subject: [PATCH 05/18] ci: increase retention for logs of tests to 144 hours (#4677) * ci: increase retention for logs of tests to 144 hours * change to days --- .gitlab-ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 05834f11d1944..b98e859182bc3 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -146,7 +146,7 @@ test-linux-stable: &test-linux - awk '/^warning:/,/^$/ { print }' output.log > ${CI_COMMIT_SHORT_SHA}_warnings.log artifacts: name: $CI_COMMIT_SHORT_SHA - expire_in: 24 hrs + expire_in: 3 days paths: - ${CI_COMMIT_SHORT_SHA}_warnings.log @@ -210,7 +210,7 @@ test-linux-stable-int: artifacts: name: $CI_COMMIT_SHORT_SHA when: on_failure - expire_in: 24 hrs + expire_in: 3 days paths: - ${CI_COMMIT_SHORT_SHA}_int_failure.log From 1f9d09d597a250e11f8f742b2d6d2cc9ae911c6c Mon Sep 17 00:00:00 2001 From: thiolliere Date: Mon, 20 Jan 2020 17:26:53 +0100 Subject: [PATCH 06/18] Pallet session new API (#4609) * Initial work * Fix most things * fix test * fix old comment * migration * fix * remove useless stuff * fix * less spaghetti implementation * fix initial session * fix --- bin/node/runtime/src/lib.rs | 3 +- frame/authority-discovery/src/lib.rs | 10 +-- frame/babe/src/mock.rs | 3 +- frame/im-online/src/mock.rs | 22 ++--- frame/session/src/historical.rs | 129 ++++++++++++--------------- frame/session/src/lib.rs | 84 ++++++++--------- frame/session/src/mock.rs | 25 +++--- frame/staking/src/lib.rs | 63 +++++++------ frame/staking/src/mock.rs | 3 +- frame/staking/src/tests.rs | 12 +-- 10 files changed, 159 insertions(+), 195 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index d769be7b505d5..e158cbe2cbb88 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -236,14 +236,13 @@ parameter_types! { } impl pallet_session::Trait for Runtime { - type OnSessionEnding = Staking; + type SessionManager = Staking; type SessionHandler = ::KeyTypeIdProviders; type ShouldEndSession = Babe; type Event = Event; type Keys = SessionKeys; type ValidatorId = ::AccountId; type ValidatorIdOf = pallet_staking::StashOf; - type SelectInitialValidators = Staking; type DisabledValidatorsThreshold = DisabledValidatorsThreshold; } diff --git a/frame/authority-discovery/src/lib.rs b/frame/authority-discovery/src/lib.rs index b3911859f4787..c427043397075 100644 --- a/frame/authority-discovery/src/lib.rs +++ b/frame/authority-discovery/src/lib.rs @@ -109,26 +109,18 @@ mod tests { pub struct Test; impl Trait for Test {} - pub struct TestOnSessionEnding; - impl pallet_session::OnSessionEnding for TestOnSessionEnding { - fn on_session_ending(_: SessionIndex, _: SessionIndex) -> Option> { - None - } - } - parameter_types! { pub const DisabledValidatorsThreshold: Perbill = Perbill::from_percent(33); } impl pallet_session::Trait for Test { - type OnSessionEnding = TestOnSessionEnding; + type SessionManager = (); type Keys = UintAuthorityId; type ShouldEndSession = pallet_session::PeriodicSessions; type SessionHandler = TestSessionHandler; type Event = (); type ValidatorId = AuthorityId; type ValidatorIdOf = ConvertInto; - type SelectInitialValidators = (); type DisabledValidatorsThreshold = DisabledValidatorsThreshold; } diff --git a/frame/babe/src/mock.rs b/frame/babe/src/mock.rs index 3f0c42a6cb9c6..e65f305dc4de2 100644 --- a/frame/babe/src/mock.rs +++ b/frame/babe/src/mock.rs @@ -79,9 +79,8 @@ impl pallet_session::Trait for Test { type ValidatorId = ::AccountId; type ShouldEndSession = Babe; type SessionHandler = (Babe,Babe,); - type OnSessionEnding = (); + type SessionManager = (); type ValidatorIdOf = (); - type SelectInitialValidators = (); type Keys = MockSessionKeys; type DisabledValidatorsThreshold = DisabledValidatorsThreshold; } diff --git a/frame/im-online/src/mock.rs b/frame/im-online/src/mock.rs index 387ff47c09fa5..5c428c38582ff 100644 --- a/frame/im-online/src/mock.rs +++ b/frame/im-online/src/mock.rs @@ -43,28 +43,25 @@ thread_local! { pub static VALIDATORS: RefCell>> = RefCell::new(Some(vec![1, 2, 3])); } -pub struct TestOnSessionEnding; -impl pallet_session::OnSessionEnding for TestOnSessionEnding { - fn on_session_ending(_ending_index: SessionIndex, _will_apply_at: SessionIndex) - -> Option> - { +pub struct TestSessionManager; +impl pallet_session::SessionManager for TestSessionManager { + fn new_session(_new_index: SessionIndex) -> Option> { VALIDATORS.with(|l| l.borrow_mut().take()) } + fn end_session(_: SessionIndex) {} } -impl pallet_session::historical::OnSessionEnding for TestOnSessionEnding { - fn on_session_ending(_ending_index: SessionIndex, _will_apply_at: SessionIndex) - -> Option<(Vec, Vec<(u64, u64)>)> - { +impl pallet_session::historical::SessionManager for TestSessionManager { + fn new_session(_new_index: SessionIndex) -> Option> { VALIDATORS.with(|l| l .borrow_mut() .take() .map(|validators| { - let full_identification = validators.iter().map(|v| (*v, *v)).collect(); - (validators, full_identification) + validators.iter().map(|v| (*v, *v)).collect() }) ) } + fn end_session(_: SessionIndex) {} } /// An extrinsic type used for tests. @@ -131,13 +128,12 @@ parameter_types! { impl pallet_session::Trait for Runtime { type ShouldEndSession = pallet_session::PeriodicSessions; - type OnSessionEnding = pallet_session::historical::NoteHistoricalRoot; + type SessionManager = pallet_session::historical::NoteHistoricalRoot; type SessionHandler = (ImOnline, ); type ValidatorId = u64; type ValidatorIdOf = ConvertInto; type Keys = UintAuthorityId; type Event = (); - type SelectInitialValidators = (); type DisabledValidatorsThreshold = DisabledValidatorsThreshold; } diff --git a/frame/session/src/historical.rs b/frame/session/src/historical.rs index 939e6133e8520..e67a2ae1b0eab 100644 --- a/frame/session/src/historical.rs +++ b/frame/session/src/historical.rs @@ -48,8 +48,7 @@ pub trait Trait: super::Trait { /// validator, since they may be outdated by the time this is queried from a /// historical trie. /// - /// This mapping is expected to remain stable in between calls to - /// `Self::OnSessionEnding::on_session_ending` which return new validators. + /// It must return the identification for the current session index. type FullIdentificationOf: Convert>; } @@ -57,16 +56,19 @@ decl_storage! { trait Store for Module as Session { /// Mapping from historical session indices to session-data root hash and validator count. HistoricalSessions get(fn historical_root): map SessionIndex => Option<(T::Hash, ValidatorCount)>; - /// Queued full identifications for queued sessions whose validators have become obsolete. - CachedObsolete get(fn cached_obsolete): map SessionIndex - => Option>; /// The range of historical sessions we store. [first, last) StoredRange: Option<(SessionIndex, SessionIndex)>; + /// Deprecated. + CachedObsolete: map SessionIndex => Option>; } } decl_module! { - pub struct Module for enum Call where origin: T::Origin { } + pub struct Module for enum Call where origin: T::Origin { + fn on_initialize(_n: T::BlockNumber) { + CachedObsolete::::remove_all(); + } + } } impl Module { @@ -97,52 +99,52 @@ impl Module { } } -/// Specialization of the crate-level `OnSessionEnding` which returns the old -/// set of full identification when changing the validator set. -pub trait OnSessionEnding: crate::OnSessionEnding { - /// If there was a validator set change, its returns the set of new validators along with the - /// old validators and their full identifications. - fn on_session_ending(ending: SessionIndex, will_apply_at: SessionIndex) - -> Option<(Vec, Vec<(ValidatorId, FullIdentification)>)>; +/// Specialization of the crate-level `SessionManager` which returns the set of full identification +/// when creating a new session. +pub trait SessionManager: crate::SessionManager { + /// If there was a validator set change, its returns the set of new validators along with their + /// full identifications. + fn new_session(new_index: SessionIndex) -> Option>; + fn end_session(end_index: SessionIndex); } -/// An `OnSessionEnding` implementation that wraps an inner `I` and also +/// An `SessionManager` implementation that wraps an inner `I` and also /// sets the historical trie root of the ending session. pub struct NoteHistoricalRoot(sp_std::marker::PhantomData<(T, I)>); -impl crate::OnSessionEnding for NoteHistoricalRoot - where I: OnSessionEnding +impl crate::SessionManager for NoteHistoricalRoot + where I: SessionManager { - fn on_session_ending(ending: SessionIndex, applied_at: SessionIndex) -> Option> { + fn new_session(new_index: SessionIndex) -> Option> { StoredRange::mutate(|range| { - range.get_or_insert_with(|| (ending, ending)).1 = ending + 1; + range.get_or_insert_with(|| (new_index, new_index)).1 = new_index + 1; }); - // do all of this _before_ calling the other `on_session_ending` impl - // so that we have e.g. correct exposures from the _current_. + let new_validators_and_id = >::new_session(new_index); + let new_validators = new_validators_and_id.as_ref().map(|new_validators| { + new_validators.iter().map(|(v, _id)| v.clone()).collect() + }); - let count = >::validators().len() as u32; - match ProvingTrie::::generate_for(ending) { - Ok(trie) => >::insert(ending, &(trie.root, count)), - Err(reason) => { - print("Failed to generate historical ancestry-inclusion proof."); - print(reason); + if let Some(new_validators) = new_validators_and_id { + let count = new_validators.len() as u32; + match ProvingTrie::::generate_for(new_validators) { + Ok(trie) => >::insert(new_index, &(trie.root, count)), + Err(reason) => { + print("Failed to generate historical ancestry-inclusion proof."); + print(reason); + } + }; + } else { + let previous_index = new_index.saturating_sub(1); + if let Some(previous_session) = >::get(previous_index) { + >::insert(new_index, previous_session); } - }; - - // trie has been generated for this session, so it's no longer queued. - >::remove(&ending); - - let (new_validators, old_exposures) = >::on_session_ending(ending, applied_at)?; - - // every session from `ending+1 .. applied_at` now has obsolete `FullIdentification` - // now that a new validator election has occurred. - // we cache these in the trie until those sessions themselves end. - for obsolete in (ending + 1) .. applied_at { - >::insert(obsolete, &old_exposures); } - Some(new_validators) + new_validators + } + fn end_session(end_index: SessionIndex) { + >::end_session(end_index) } } @@ -158,15 +160,14 @@ pub struct ProvingTrie { } impl ProvingTrie { - fn generate_for(now: SessionIndex) -> Result { + fn generate_for(validators: I) -> Result + where I: IntoIterator + { let mut db = MemoryDB::default(); let mut root = Default::default(); - fn build(root: &mut T::Hash, db: &mut MemoryDB>, validators: I) - -> Result<(), &'static str> - where I: IntoIterator)> { - let mut trie = TrieDBMut::new(db, root); + let mut trie = TrieDBMut::new(&mut db, &mut root); for (i, (validator, full_id)) in validators.into_iter().enumerate() { let i = i as u32; let keys = match >::load_keys(&validator) { @@ -174,11 +175,7 @@ impl ProvingTrie { Some(k) => k, }; - let full_id = full_id.or_else(|| T::FullIdentificationOf::convert(validator.clone())); - let full_id = match full_id { - None => return Err("no full identification for a current validator"), - Some(full) => (validator, full), - }; + let full_id = (validator, full_id); // map each key to the owner index. for key_id in T::Keys::key_ids() { @@ -194,17 +191,6 @@ impl ProvingTrie { let _ = i.using_encoded(|k| full_id.using_encoded(|v| trie.insert(k, v))) .map_err(|_| "failed to insert into trie")?; } - - Ok(()) - } - - // if the current session's full identifications are obsolete but cached, - // use those. - if let Some(obsolete) = >::get(&now) { - build::(&mut root, &mut db, obsolete.into_iter().map(|(v, f)| (v, Some(f))))? - } else { - let validators = >::validators(); - build::(&mut root, &mut db, validators.into_iter().map(|v| (v, None)))? } Ok(ProvingTrie { @@ -281,7 +267,12 @@ impl> frame_support::traits::KeyOwnerProofSystem<(KeyTy fn prove(key: (KeyTypeId, D)) -> Option { let session = >::current_index(); - let trie = ProvingTrie::::generate_for(session).ok()?; + let validators = >::validators().into_iter() + .filter_map(|validator| { + T::FullIdentificationOf::convert(validator.clone()) + .map(|full_id| (validator, full_id)) + }); + let trie = ProvingTrie::::generate_for(validators).ok()?; let (id, data) = key; @@ -348,13 +339,9 @@ mod tests { set_next_validators(vec![1, 2, 4]); force_new_session(); - assert!(Historical::cached_obsolete(&(proof.session + 1)).is_none()); - System::set_block_number(2); Session::on_initialize(2); - assert!(Historical::cached_obsolete(&(proof.session + 1)).is_some()); - assert!(Historical::historical_root(proof.session).is_some()); assert!(Session::current_index() > proof.session); @@ -366,15 +353,13 @@ mod tests { force_new_session(); System::set_block_number(3); Session::on_initialize(3); - - assert!(Historical::cached_obsolete(&(proof.session + 1)).is_none()); }); } #[test] fn prune_up_to_works() { new_test_ext().execute_with(|| { - for i in 1..101u64 { + for i in 1..99u64 { set_next_validators(vec![i]); force_new_session(); @@ -385,7 +370,7 @@ mod tests { assert_eq!(StoredRange::get(), Some((0, 100))); - for i in 1..100 { + for i in 0..100 { assert!(Historical::historical_root(i).is_some()) } @@ -405,7 +390,7 @@ mod tests { Historical::prune_up_to(100); assert_eq!(StoredRange::get(), None); - for i in 101..201u64 { + for i in 99..199u64 { set_next_validators(vec![i]); force_new_session(); @@ -416,14 +401,14 @@ mod tests { assert_eq!(StoredRange::get(), Some((100, 200))); - for i in 101..200 { + for i in 100..200 { assert!(Historical::historical_root(i).is_some()) } Historical::prune_up_to(9999); assert_eq!(StoredRange::get(), None); - for i in 101..200 { + for i in 100..200 { assert!(Historical::historical_root(i).is_none()) } }); diff --git a/frame/session/src/lib.rs b/frame/session/src/lib.rs index 2200221af42fe..098b5330779ef 100644 --- a/frame/session/src/lib.rs +++ b/frame/session/src/lib.rs @@ -162,29 +162,30 @@ impl< } } -/// An event handler for when the session is ending. -/// TODO [slashing] consider renaming to OnSessionStarting -pub trait OnSessionEnding { - /// Handle the fact that the session is ending, and optionally provide the new validator set. +/// A trait for managing creation of new validator set. +pub trait SessionManager { + /// Plan a new session, and optionally provide the new validator set. /// /// Even if the validator-set is the same as before, if any underlying economic /// conditions have changed (i.e. stake-weights), the new validator set must be returned. /// This is necessary for consensus engines making use of the session module to /// issue a validator-set change so misbehavior can be provably associated with the new /// economic conditions as opposed to the old. + /// The returned validator set, if any, will not be applied until `new_index`. + /// `new_index` is strictly greater than from previous call. /// - /// `ending_index` is the index of the currently ending session. - /// The returned validator set, if any, will not be applied until `will_apply_at`. - /// `will_apply_at` is guaranteed to be at least `ending_index + 1`, since session indices don't - /// repeat, but it could be some time after in case we are staging authority set changes. - fn on_session_ending( - ending_index: SessionIndex, - will_apply_at: SessionIndex - ) -> Option>; + /// The first session start at index 0. + fn new_session(new_index: SessionIndex) -> Option>; + /// End the session. + /// + /// Because the session pallet can queue validator set the ending session can be lower than the + /// last new session index. + fn end_session(end_index: SessionIndex); } -impl OnSessionEnding for () { - fn on_session_ending(_: SessionIndex, _: SessionIndex) -> Option> { None } +impl SessionManager for () { + fn new_session(_: SessionIndex) -> Option> { None } + fn end_session(_: SessionIndex) {} } /// Handler for session lifecycle events. @@ -214,7 +215,7 @@ pub trait SessionHandler { /// A notification for end of the session. /// - /// Note it is triggered before any `OnSessionEnding` handlers, + /// Note it is triggered before any `SessionManager::end_session` handlers, /// so we can still affect the validator set. fn on_before_session_ending() {} @@ -248,7 +249,7 @@ pub trait OneSessionHandler: BoundToRuntimeAppPublic { /// A notification for end of the session. /// - /// Note it is triggered before any `OnSessionEnding` handlers, + /// Note it is triggered before any `SessionManager::end_session` handlers, /// so we can still affect the validator set. fn on_before_session_ending() {} @@ -318,21 +319,6 @@ impl SessionHandler for TestSessionHandler { fn on_disabled(_: usize) {} } -/// Handler for selecting the genesis validator set. -pub trait SelectInitialValidators { - /// Returns the initial validator set. If `None` is returned - /// all accounts that have session keys set in the genesis block - /// will be validators. - fn select_initial_validators() -> Option>; -} - -/// Implementation of `SelectInitialValidators` that does nothing. -impl SelectInitialValidators for () { - fn select_initial_validators() -> Option> { - None - } -} - impl ValidatorRegistration for Module { fn is_registered(id: &T::ValidatorId) -> bool { Self::load_keys(id).is_some() @@ -352,8 +338,8 @@ pub trait Trait: frame_system::Trait { /// Indicator for when to end the session. type ShouldEndSession: ShouldEndSession; - /// Handler for when a session is about to end. - type OnSessionEnding: OnSessionEnding; + /// Handler for managing new session. + type SessionManager: SessionManager; /// Handler when a session has changed. type SessionHandler: SessionHandler; @@ -366,9 +352,6 @@ pub trait Trait: frame_system::Trait { /// After the threshold is reached `disabled` method starts to return true, /// which in combination with `pallet_staking` forces a new era. type DisabledValidatorsThreshold: Get; - - /// Select initial validators. - type SelectInitialValidators: SelectInitialValidators; } const DEDUP_KEY_PREFIX: &[u8] = b":session:keys"; @@ -435,12 +418,19 @@ decl_storage! { .expect("genesis config must not contain duplicates; qed"); } - let initial_validators = T::SelectInitialValidators::select_initial_validators() - .unwrap_or_else(|| config.keys.iter().map(|(ref v, _)| v.clone()).collect()); + let initial_validators_0 = T::SessionManager::new_session(0) + .unwrap_or_else(|| { + frame_support::print("No initial validator provided by `SessionManager`, use \ + session config keys to generate initial validator set."); + config.keys.iter().map(|(ref v, _)| v.clone()).collect() + }); + assert!(!initial_validators_0.is_empty(), "Empty validator set for session 0 in genesis block!"); - assert!(!initial_validators.is_empty(), "Empty validator set in genesis block!"); + let initial_validators_1 = T::SessionManager::new_session(1) + .unwrap_or_else(|| initial_validators_0.clone()); + assert!(!initial_validators_1.is_empty(), "Empty validator set for session 1 in genesis block!"); - let queued_keys: Vec<_> = initial_validators + let queued_keys: Vec<_> = initial_validators_1 .iter() .cloned() .map(|v| ( @@ -452,7 +442,7 @@ decl_storage! { // Tell everyone about the genesis session keys T::SessionHandler::on_genesis_session::(&queued_keys); - >::put(initial_validators); + >::put(initial_validators_0); >::put(queued_keys); }); } @@ -545,10 +535,14 @@ impl Module { DisabledValidators::take(); } - let applied_at = session_index + 2; + T::SessionManager::end_session(session_index); + + // Increment session index. + let session_index = session_index + 1; + CurrentIndex::put(session_index); // Get next validator set. - let maybe_next_validators = T::OnSessionEnding::on_session_ending(session_index, applied_at); + let maybe_next_validators = T::SessionManager::new_session(session_index + 1); let (next_validators, next_identities_changed) = if let Some(validators) = maybe_next_validators { @@ -560,10 +554,6 @@ impl Module { (>::get(), false) }; - // Increment session index. - let session_index = session_index + 1; - CurrentIndex::put(session_index); - // Queue next session keys. let (queued_amalgamated, next_changed) = { // until we are certain there has been a change, iterate the prior diff --git a/frame/session/src/mock.rs b/frame/session/src/mock.rs index 7fe9cd01f43bf..ff84743a61596 100644 --- a/frame/session/src/mock.rs +++ b/frame/session/src/mock.rs @@ -88,9 +88,10 @@ impl SessionHandler for TestSessionHandler { } } -pub struct TestOnSessionEnding; -impl OnSessionEnding for TestOnSessionEnding { - fn on_session_ending(_: SessionIndex, _: SessionIndex) -> Option> { +pub struct TestSessionManager; +impl SessionManager for TestSessionManager { + fn end_session(_: SessionIndex) {} + fn new_session(_: SessionIndex) -> Option> { if !TEST_SESSION_CHANGED.with(|l| *l.borrow()) { VALIDATORS.with(|v| { let mut v = v.borrow_mut(); @@ -108,14 +109,13 @@ impl OnSessionEnding for TestOnSessionEnding { } #[cfg(feature = "historical")] -impl crate::historical::OnSessionEnding for TestOnSessionEnding { - fn on_session_ending(ending_index: SessionIndex, will_apply_at: SessionIndex) - -> Option<(Vec, Vec<(u64, u64)>)> +impl crate::historical::SessionManager for TestSessionManager { + fn end_session(_: SessionIndex) {} + fn new_session(new_index: SessionIndex) + -> Option> { - let pair_with_ids = |vals: &[u64]| vals.iter().map(|&v| (v, v)).collect::>(); - >::on_session_ending(ending_index, will_apply_at) - .map(|vals| (pair_with_ids(&vals), vals)) - .map(|(ids, vals)| (vals, ids)) + >::new_session(new_index) + .map(|vals| vals.into_iter().map(|val| (val, val)).collect()) } } @@ -190,15 +190,14 @@ parameter_types! { impl Trait for Test { type ShouldEndSession = TestShouldEndSession; #[cfg(feature = "historical")] - type OnSessionEnding = crate::historical::NoteHistoricalRoot; + type SessionManager = crate::historical::NoteHistoricalRoot; #[cfg(not(feature = "historical"))] - type OnSessionEnding = TestOnSessionEnding; + type SessionManager = TestSessionManager; type SessionHandler = TestSessionHandler; type ValidatorId = u64; type ValidatorIdOf = ConvertInto; type Keys = MockSessionKeys; type Event = (); - type SelectInitialValidators = (); type DisabledValidatorsThreshold = DisabledValidatorsThreshold; } diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index c547545c389a5..7f1ad37b32cf6 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -265,7 +265,7 @@ use frame_support::{ WithdrawReasons, OnUnbalanced, Imbalance, Get, Time } }; -use pallet_session::{historical::OnSessionEnding, SelectInitialValidators}; +use pallet_session::historical::SessionManager; use sp_runtime::{ Perbill, RuntimeDebug, @@ -575,8 +575,7 @@ impl SessionInterface<::AccountId> for T whe FullIdentificationOf = ExposureOf, >, T::SessionHandler: pallet_session::SessionHandler<::AccountId>, - T::OnSessionEnding: pallet_session::OnSessionEnding<::AccountId>, - T::SelectInitialValidators: pallet_session::SelectInitialValidators<::AccountId>, + T::SessionManager: pallet_session::SessionManager<::AccountId>, T::ValidatorIdOf: Convert<::AccountId, Option<::AccountId>> { fn disable_validator(validator: &::AccountId) -> Result { @@ -1346,11 +1345,8 @@ impl Module { imbalance } - /// Session has just ended. Provide the validator set for the next session if it's an era-end, along - /// with the exposure of the prior validator set. - fn new_session(session_index: SessionIndex) - -> Option<(Vec, Vec<(T::AccountId, Exposure>)>)> - { + /// Session has just ended. Provide the validator set for the next session if it's an era-end. + fn new_session(session_index: SessionIndex) -> Option> { let era_length = session_index.checked_sub(Self::current_era_start_session_index()).unwrap_or(0); match ForceEra::get() { Forcing::ForceNew => ForceEra::kill(), @@ -1358,12 +1354,17 @@ impl Module { Forcing::NotForcing if era_length >= T::SessionsPerEra::get() => (), _ => return None, } - let validators = T::SessionInterface::validators(); - let prior = validators.into_iter() - .map(|v| { let e = Self::stakers(&v); (v, e) }) - .collect(); - Self::new_era(session_index).map(move |new| (new, prior)) + Self::new_era(session_index) + } + + /// Initialise the first session (and consequently the first era) + fn initial_session() -> Option> { + // note: `CurrentEraStart` is set in `on_finalize` of the first block because now is not + // available yet. + CurrentEraStartSessionIndex::put(0); + BondedEras::mutate(|bonded| bonded.push((0, 0))); + Self::select_validators().1 } /// The era has changed - enact new staking set. @@ -1646,19 +1647,30 @@ impl Module { } } -impl pallet_session::OnSessionEnding for Module { - fn on_session_ending(_ending: SessionIndex, start_session: SessionIndex) -> Option> { +impl pallet_session::SessionManager for Module { + fn new_session(new_index: SessionIndex) -> Option> { Self::ensure_storage_upgraded(); - Self::new_session(start_session - 1).map(|(new, _old)| new) + if new_index == 0 { + return Self::initial_session(); + } + Self::new_session(new_index - 1) } + fn end_session(_end_index: SessionIndex) {} } -impl OnSessionEnding>> for Module { - fn on_session_ending(_ending: SessionIndex, start_session: SessionIndex) - -> Option<(Vec, Vec<(T::AccountId, Exposure>)>)> +impl SessionManager>> for Module { + fn new_session(new_index: SessionIndex) + -> Option>)>> { - Self::ensure_storage_upgraded(); - Self::new_session(start_session - 1) + >::new_session(new_index).map(|validators| { + validators.into_iter().map(|v| { + let exposure = >::get(&v); + (v, exposure) + }).collect() + }) + } + fn end_session(end_index: SessionIndex) { + >::end_session(end_index) } } @@ -1707,12 +1719,6 @@ impl Convert> } } -impl SelectInitialValidators for Module { - fn select_initial_validators() -> Option> { - >::select_validators().1 - } -} - /// This is intended to be used with `FilterHistoricalOffences`. impl OnOffenceHandler> for Module where T: pallet_session::Trait::AccountId>, @@ -1721,8 +1727,7 @@ impl OnOffenceHandler, >, T::SessionHandler: pallet_session::SessionHandler<::AccountId>, - T::OnSessionEnding: pallet_session::OnSessionEnding<::AccountId>, - T::SelectInitialValidators: pallet_session::SelectInitialValidators<::AccountId>, + T::SessionManager: pallet_session::SessionManager<::AccountId>, T::ValidatorIdOf: Convert<::AccountId, Option<::AccountId>> { fn on_offence( diff --git a/frame/staking/src/mock.rs b/frame/staking/src/mock.rs index 3c238b56ed317..8aa20c19c6e29 100644 --- a/frame/staking/src/mock.rs +++ b/frame/staking/src/mock.rs @@ -162,14 +162,13 @@ parameter_types! { pub const DisabledValidatorsThreshold: Perbill = Perbill::from_percent(25); } impl pallet_session::Trait for Test { - type OnSessionEnding = pallet_session::historical::NoteHistoricalRoot; + type SessionManager = pallet_session::historical::NoteHistoricalRoot; type Keys = UintAuthorityId; type ShouldEndSession = pallet_session::PeriodicSessions; type SessionHandler = TestSessionHandler; type Event = (); type ValidatorId = AccountId; type ValidatorIdOf = crate::StashOf; - type SelectInitialValidators = Staking; type DisabledValidatorsThreshold = DisabledValidatorsThreshold; } diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index bb1fe32025ebc..43a2d0079164f 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -365,7 +365,7 @@ fn less_than_needed_candidates_works() { #[test] fn no_candidate_emergency_condition() { ExtBuilder::default() - .minimum_validator_count(10) + .minimum_validator_count(1) .validator_count(15) .num_validators(4) .validator_pool(true) @@ -374,21 +374,21 @@ fn no_candidate_emergency_condition() { .execute_with(|| { // initial validators assert_eq_uvec!(validator_controllers(), vec![10, 20, 30, 40]); + let prefs = ValidatorPrefs { commission: Perbill::one() }; + ::Validators::insert(11, prefs.clone()); // set the minimum validator count. ::MinimumValidatorCount::put(10); - ::ValidatorCount::put(15); - assert_eq!(Staking::validator_count(), 15); let _ = Staking::chill(Origin::signed(10)); // trigger era - System::set_block_number(1); - Session::on_initialize(System::block_number()); + start_era(1); // Previous ones are elected. chill is invalidates. TODO: #2494 assert_eq_uvec!(validator_controllers(), vec![10, 20, 30, 40]); - assert_eq!(Staking::current_elected().len(), 0); + // Though the validator preferences has been removed. + assert!(Staking::validators(11) != prefs); }); } From 5cd952bbf6405a165d0399fd3e5004961ace6819 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 20 Jan 2020 17:28:27 +0100 Subject: [PATCH 07/18] Only support ECDSA compressed public keys (#4667) Some fixes after: https://github.com/paritytech/substrate/pull/4502 This removes the unwanted `expect`s from `MultiSigner`. Instead we convert from full to compressed in `TryFrom` and can return an error on invalid input. --- primitives/core/src/ecdsa.rs | 120 +++++++++++-------------------- primitives/runtime/src/lib.rs | 6 +- primitives/runtime/src/traits.rs | 4 +- 3 files changed, 46 insertions(+), 84 deletions(-) diff --git a/primitives/core/src/ecdsa.rs b/primitives/core/src/ecdsa.rs index afd75dae99522..99db0e6b2d1a0 100644 --- a/primitives/core/src/ecdsa.rs +++ b/primitives/core/src/ecdsa.rs @@ -46,14 +46,9 @@ use secp256k1::{PublicKey, SecretKey}; #[cfg(feature = "full_crypto")] type Seed = [u8; 32]; -/// The ECDSA public key. +/// The ECDSA compressed public key. #[derive(Clone, Encode, Decode)] -pub enum Public { - /// A full raw ECDSA public key. - Full([u8; 64]), - /// A compressed ECDSA public key. - Compressed([u8; 33]), -} +pub struct Public([u8; 33]); impl PartialOrd for Public { fn partial_cmp(&self, other: &Self) -> Option { @@ -90,42 +85,12 @@ pub enum PublicError { } impl Public { - /// A new instance from the given 64-byte `data`. - /// - /// NOTE: No checking goes on to ensure this is a real public key. Only use it if - /// you are certain that the array actually is a pubkey. GIGO! - pub fn from_raw(data: [u8; 64]) -> Self { - Self::Full(data) - } - - /// A new instance from the given 65-byte `data`. + /// A new instance from the given 33-byte `data`. /// /// NOTE: No checking goes on to ensure this is a real public key. Only use it if /// you are certain that the array actually is a pubkey. GIGO! - pub fn from_full(data: [u8; 65]) -> Self { - let raw_key = &data[1..]; - let mut key = [0u8; 64]; - key.copy_from_slice(raw_key); - Self::Full(key) - } - - /// Return in compressed format. - /// - /// Returns an error if `self` is an invalid full public key. - pub fn as_compressed(&self) -> Result<[u8; 33], ()> { - match self { - Self::Full(d) => secp256k1::PublicKey::parse_slice(d, None) - .map(|k| k.serialize_compressed()) - .map_err(|_| ()), - Self::Compressed(d) => Ok(*d), - } - } - - /// Convert `Self` into a compressed public key. - /// - /// Returns an error if `self` is an invalid full public key. - pub fn into_compressed(self) -> Result { - self.as_compressed().map(Self::Compressed) + pub fn from_raw(data: [u8; 33]) -> Self { + Self(data) } } @@ -135,15 +100,9 @@ impl TraitPublic for Public { /// NOTE: No checking goes on to ensure this is a real public key. Only use it if /// you are certain that the array actually is a pubkey. GIGO! fn from_slice(data: &[u8]) -> Self { - if data.len() == 33 { - let mut r = [0u8; 33]; - r.copy_from_slice(data); - Self::Compressed(r) - } else { - let mut r = [0u8; 64]; - r.copy_from_slice(data); - Self::Full(r) - } + let mut r = [0u8; 33]; + r.copy_from_slice(data); + Self(r) } } @@ -151,25 +110,19 @@ impl Derive for Public {} impl Default for Public { fn default() -> Self { - Public::Full([0u8; 64]) + Public([0u8; 33]) } } impl AsRef<[u8]> for Public { fn as_ref(&self) -> &[u8] { - match self { - Self::Full(d) => &d[..], - Self::Compressed(d) => &d[..], - } + &self.0[..] } } impl AsMut<[u8]> for Public { fn as_mut(&mut self) -> &mut [u8] { - match self { - Self::Full(d) => &mut d[..], - Self::Compressed(d) => &mut d[..], - } + &mut self.0[..] } } @@ -177,10 +130,13 @@ impl sp_std::convert::TryFrom<&[u8]> for Public { type Error = (); fn try_from(data: &[u8]) -> Result { - if data.len() == 33 || data.len() == 64 { + if data.len() == 33 { Ok(Self::from_slice(data)) } else { - Err(()) + secp256k1::PublicKey::parse_slice(data, None) + .map(|k| k.serialize_compressed()) + .map(Self) + .map_err(|_| ()) } } } @@ -192,9 +148,9 @@ impl From for Public { } } -impl UncheckedFrom<[u8; 64]> for Public { - fn unchecked_from(x: [u8; 64]) -> Self { - Public::Full(x) +impl UncheckedFrom<[u8; 33]> for Public { + fn unchecked_from(x: [u8; 33]) -> Self { + Public(x) } } @@ -354,8 +310,9 @@ impl Signature { pub fn recover>(&self, message: M) -> Option { let message = secp256k1::Message::parse(&blake2_256(message.as_ref())); let sig: (_, _) = self.try_into().ok()?; - secp256k1::recover(&message, &sig.0, &sig.1).ok() - .map(|recovered| Public::from_full(recovered.serialize())) + secp256k1::recover(&message, &sig.0, &sig.1) + .ok() + .map(|recovered| Public(recovered.serialize_compressed())) } } @@ -476,7 +433,7 @@ impl TraitPair for Pair { /// Get the public key. fn public(&self) -> Public { - Public::from_full(self.public.serialize()) + Public(self.public.serialize_compressed()) } /// Sign a message. @@ -490,9 +447,7 @@ impl TraitPair for Pair { let message = secp256k1::Message::parse(&blake2_256(message.as_ref())); let sig: (_, _) = match sig.try_into() { Ok(x) => x, _ => return false }; match secp256k1::recover(&message, &sig.0, &sig.1) { - Ok(actual) => pubkey.as_compressed() - .map(|p| &p[..] == &actual.serialize_compressed()[..]) - .unwrap_or(false), + Ok(actual) => &pubkey.0[..] == &actual.serialize_compressed()[..], _ => false, } } @@ -587,9 +542,12 @@ mod test { &hex!("9d61b19deffd5a60ba844af492ec2cc44449c5697b326919703bac031cae7f60") ); let public = pair.public(); - assert_eq!(public, Public::from_raw( - hex!("8db55b05db86c0b1786ca49f095d76344c9e6056b2f02701a7e7f3c20aabfd913ebbe148dd17c56551a52952371071a6c604b3f3abe8f2c8fa742158ea6dd7d4") - )); + assert_eq!( + public, + Public::try_from( + &hex!("8db55b05db86c0b1786ca49f095d76344c9e6056b2f02701a7e7f3c20aabfd913ebbe148dd17c56551a52952371071a6c604b3f3abe8f2c8fa742158ea6dd7d4")[..], + ).unwrap(), + ); let message = b""; let signature = hex!("3dde91174bd9359027be59a428b8146513df80a2a3c7eda2194f64de04a69ab97b753169e94db6ffd50921a2668a48b94ca11e3d32c1ff19cfe88890aa7e8f3c00"); let signature = Signature::from_raw(signature); @@ -604,9 +562,12 @@ mod test { None ).unwrap(); let public = pair.public(); - assert_eq!(public, Public::from_raw( - hex!("8db55b05db86c0b1786ca49f095d76344c9e6056b2f02701a7e7f3c20aabfd913ebbe148dd17c56551a52952371071a6c604b3f3abe8f2c8fa742158ea6dd7d4") - )); + assert_eq!( + public, + Public::try_from( + &hex!("8db55b05db86c0b1786ca49f095d76344c9e6056b2f02701a7e7f3c20aabfd913ebbe148dd17c56551a52952371071a6c604b3f3abe8f2c8fa742158ea6dd7d4")[..], + ).unwrap(), + ); let message = b""; let signature = hex!("3dde91174bd9359027be59a428b8146513df80a2a3c7eda2194f64de04a69ab97b753169e94db6ffd50921a2668a48b94ca11e3d32c1ff19cfe88890aa7e8f3c00"); let signature = Signature::from_raw(signature); @@ -628,9 +589,12 @@ mod test { fn seeded_pair_should_work() { let pair = Pair::from_seed(b"12345678901234567890123456789012"); let public = pair.public(); - assert_eq!(public, Public::from_raw( - hex!("5676109c54b9a16d271abeb4954316a40a32bcce023ac14c8e26e958aa68fba995840f3de562156558efbfdac3f16af0065e5f66795f4dd8262a228ef8c6d813") - )); + assert_eq!( + public, + Public::try_from( + &hex!("5676109c54b9a16d271abeb4954316a40a32bcce023ac14c8e26e958aa68fba995840f3de562156558efbfdac3f16af0065e5f66795f4dd8262a228ef8c6d813")[..], + ).unwrap(), + ); let message = hex!("2f8c6129d816cf51c374bc7f08c3e63ed156cf78aefb4a6550d97b87997977ee00000000000000000200d75a980182b10ab7d54bfed3c964073a0ee172f3daa62325af021a68f707511a4500000000000000"); let signature = pair.sign(&message[..]); println!("Correct signature: {:?}", signature); diff --git a/primitives/runtime/src/lib.rs b/primitives/runtime/src/lib.rs index 6033f221a1f5f..46930c35e8e8d 100644 --- a/primitives/runtime/src/lib.rs +++ b/primitives/runtime/src/lib.rs @@ -238,9 +238,7 @@ impl traits::IdentifyAccount for MultiSigner { match self { MultiSigner::Ed25519(who) => <[u8; 32]>::from(who).into(), MultiSigner::Sr25519(who) => <[u8; 32]>::from(who).into(), - MultiSigner::Ecdsa(who) => sp_io::hashing::blake2_256( - &who.as_compressed().expect("`who` is a valid `ECDSA` public key; qed")[..], - ).into(), + MultiSigner::Ecdsa(who) => sp_io::hashing::blake2_256(&who.as_ref()[..]).into(), } } } @@ -724,7 +722,7 @@ mod tests { let multi_signer = MultiSigner::from(pair.public()); assert!(multi_sig.verify(msg, &multi_signer.into_account())); - let multi_signer = MultiSigner::from(pair.public().into_compressed().unwrap()); + let multi_signer = MultiSigner::from(pair.public()); assert!(multi_sig.verify(msg, &multi_signer.into_account())); } } diff --git a/primitives/runtime/src/traits.rs b/primitives/runtime/src/traits.rs index c02856d20d917..2547ce1072185 100644 --- a/primitives/runtime/src/traits.rs +++ b/primitives/runtime/src/traits.rs @@ -102,7 +102,7 @@ impl Verify for sp_core::ecdsa::Signature { self.as_ref(), &sp_io::hashing::blake2_256(msg.get()), ) { - Ok(pubkey) => signer.as_compressed().map(|s| &s[..] == &pubkey[..]).unwrap_or(false), + Ok(pubkey) => &signer.as_ref()[..] == &pubkey[..], _ => false, } } @@ -1357,6 +1357,6 @@ mod tests { assert!(ecdsa::Pair::verify(&signature, msg, &pair.public())); assert!(signature.verify(msg, &pair.public())); - assert!(signature.verify(msg, &pair.public().into_compressed().unwrap())); + assert!(signature.verify(msg, &pair.public())); } } From cfbb24cbdbc84410624a84daec55bfe178c7549f Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Mon, 20 Jan 2020 19:52:02 +0100 Subject: [PATCH 08/18] Call enable_all() when building tokio runtime (#4690) --- bin/node/cli/src/cli.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/bin/node/cli/src/cli.rs b/bin/node/cli/src/cli.rs index dcc5d39dbb7e7..8156cd766cc79 100644 --- a/bin/node/cli/src/cli.rs +++ b/bin/node/cli/src/cli.rs @@ -108,6 +108,7 @@ pub fn run(args: I, exit: E, version: sc_cli::VersionInfo) -> error::Re let runtime = RuntimeBuilder::new() .thread_name("main-tokio-") .threaded_scheduler() + .enable_all() .build() .map_err(|e| format!("{:?}", e))?; match config.roles { From cb9c1818f198b946b402652d173bbd76e8fbe7d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 21 Jan 2020 12:33:28 +0100 Subject: [PATCH 09/18] Make debug builds more usable (#4683) * Make debug builds more usable This pr makes debug builds more usable in terms of `cargo run -- --dev`. 1. `--dev` activates `--execution native`, iff `--execution` is not given or no sub `--execution-*` is given. 2. It was probably a mistake to compile WASM in debug for a debug build. So, we now build the WASM binary always as `release` (if not requested differently by the user). So, we trade compilation time for a better debug experience. * Make sure we only overwrite default values * Make it work * Apply suggestion --- Cargo.lock | 2 +- bin/node-template/runtime/build.rs | 2 +- bin/node/cli/src/cli.rs | 7 +++- bin/node/runtime/build.rs | 2 +- client/cli/src/execution_strategy.rs | 24 +++++++++++- client/cli/src/lib.rs | 39 ++++++++++++++----- client/cli/src/params.rs | 11 +++--- client/executor/runtime-test/build.rs | 2 +- .../runtime-interface/test-wasm/build.rs | 2 +- test-utils/runtime/build.rs | 2 +- utils/wasm-builder/Cargo.toml | 2 +- utils/wasm-builder/src/wasm_project.rs | 2 +- 12 files changed, 73 insertions(+), 24 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 630e372dff82e..8030a4cd13ad2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6973,7 +6973,7 @@ version = "2.0.0" [[package]] name = "substrate-wasm-builder" -version = "1.0.8" +version = "1.0.9" dependencies = [ "atty 0.2.13 (registry+https://github.com/rust-lang/crates.io-index)", "build-helper 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/bin/node-template/runtime/build.rs b/bin/node-template/runtime/build.rs index ed0e28ec0d140..dc7e949d809f8 100644 --- a/bin/node-template/runtime/build.rs +++ b/bin/node-template/runtime/build.rs @@ -19,7 +19,7 @@ use wasm_builder_runner::{build_current_project_with_rustflags, WasmBuilderSourc fn main() { build_current_project_with_rustflags( "wasm_binary.rs", - WasmBuilderSource::Crates("1.0.8"), + WasmBuilderSource::Crates("1.0.9"), // This instructs LLD to export __heap_base as a global variable, which is used by the // external memory allocator. "-Clink-arg=--export=__heap_base", diff --git a/bin/node/cli/src/cli.rs b/bin/node/cli/src/cli.rs index 8156cd766cc79..6a2e02efb6209 100644 --- a/bin/node/cli/src/cli.rs +++ b/bin/node/cli/src/cli.rs @@ -141,7 +141,12 @@ pub fn run(args: I, exit: E, version: sc_cli::VersionInfo) -> error::Re &version, None, )?; - sc_cli::fill_import_params(&mut config, &cli_args.import_params, ServiceRoles::FULL)?; + sc_cli::fill_import_params( + &mut config, + &cli_args.import_params, + ServiceRoles::FULL, + cli_args.shared_params.dev, + )?; match ChainSpec::from(config.chain_spec.id()) { Some(ref c) if c == &ChainSpec::Development || c == &ChainSpec::LocalTestnet => {}, diff --git a/bin/node/runtime/build.rs b/bin/node/runtime/build.rs index 1ed2fa43e685c..9c81ea6f38b8d 100644 --- a/bin/node/runtime/build.rs +++ b/bin/node/runtime/build.rs @@ -21,7 +21,7 @@ fn main() { "wasm_binary.rs", WasmBuilderSource::CratesOrPath { path: "../../../utils/wasm-builder", - version: "1.0.8", + version: "1.0.9", }, // This instructs LLD to export __heap_base as a global variable, which is used by the // external memory allocator. diff --git a/client/cli/src/execution_strategy.rs b/client/cli/src/execution_strategy.rs index fe353da80ddef..888d7b6c4a096 100644 --- a/client/cli/src/execution_strategy.rs +++ b/client/cli/src/execution_strategy.rs @@ -20,7 +20,7 @@ use structopt::clap::arg_enum; arg_enum! { /// How to execute blocks - #[derive(Debug, Clone, Copy)] + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ExecutionStrategy { // Execute with native build (if available, WebAssembly otherwise). Native, @@ -33,3 +33,25 @@ arg_enum! { } } +impl ExecutionStrategy { + /// Returns the variant as `'&static str`. + pub fn as_str(&self) -> &'static str { + match self { + Self::Native => "Native", + Self::Wasm => "Wasm", + Self::Both => "Both", + Self::NativeElseWasm => "NativeElseWasm", + } + } +} + +/// Default value for the `--execution-syncing` parameter. +pub const DEFAULT_EXECUTION_SYNCING: ExecutionStrategy = ExecutionStrategy::NativeElseWasm; +/// Default value for the `--execution-import-block` parameter. +pub const DEFAULT_EXECUTION_IMPORT_BLOCK: ExecutionStrategy = ExecutionStrategy::NativeElseWasm; +/// Default value for the `--execution-block-construction` parameter. +pub const DEFAULT_EXECUTION_BLOCK_CONSTRUCTION: ExecutionStrategy = ExecutionStrategy::Wasm; +/// Default value for the `--execution-offchain-worker` parameter. +pub const DEFAULT_EXECUTION_OFFCHAIN_WORKER: ExecutionStrategy = ExecutionStrategy::Native; +/// Default value for the `--execution-other` parameter. +pub const DEFAULT_EXECUTION_OTHER: ExecutionStrategy = ExecutionStrategy::Native; diff --git a/client/cli/src/lib.rs b/client/cli/src/lib.rs index 52a3fc46ec726..fc5025952860b 100644 --- a/client/cli/src/lib.rs +++ b/client/cli/src/lib.rs @@ -40,6 +40,7 @@ use sc_network::{ }, }; use sp_core::H256; +use execution_strategy::*; use std::{ io::{Write, Read, Seek, Cursor, stdin, stdout, ErrorKind}, iter, fmt::Debug, fs::{self, File}, @@ -556,7 +557,12 @@ impl<'a> ParseAndPrepareImport<'a> { self.version, None, )?; - fill_import_params(&mut config, &self.params.import_params, sc_service::Roles::FULL)?; + fill_import_params( + &mut config, + &self.params.import_params, + sc_service::Roles::FULL, + self.params.shared_params.dev, + )?; let file: Box = match self.params.input { Some(filename) => Box::new(File::open(filename)?), @@ -620,7 +626,12 @@ impl<'a> CheckBlock<'a> { self.version, None, )?; - fill_import_params(&mut config, &self.params.import_params, sc_service::Roles::FULL)?; + fill_import_params( + &mut config, + &self.params.import_params, + sc_service::Roles::FULL, + self.params.shared_params.dev, + )?; fill_config_keystore_in_memory(&mut config)?; let input = if self.params.input.starts_with("0x") { &self.params.input[2..] } else { &self.params.input[..] }; @@ -904,6 +915,7 @@ pub fn fill_import_params( config: &mut Configuration, cli: &ImportParams, role: sc_service::Roles, + is_dev: bool, ) -> error::Result<()> where C: Default, @@ -943,13 +955,22 @@ pub fn fill_import_params( config.wasm_method = cli.wasm_method.into(); let exec = &cli.execution_strategies; - let exec_all_or = |strat: ExecutionStrategy| exec.execution.unwrap_or(strat).into(); + let exec_all_or = |strat: ExecutionStrategy, default: ExecutionStrategy| { + exec.execution.unwrap_or(if strat == default && is_dev { + ExecutionStrategy::Native + } else { + strat + }).into() + }; + config.execution_strategies = ExecutionStrategies { - syncing: exec_all_or(exec.execution_syncing), - importing: exec_all_or(exec.execution_import_block), - block_construction: exec_all_or(exec.execution_block_construction), - offchain_worker: exec_all_or(exec.execution_offchain_worker), - other: exec_all_or(exec.execution_other), + syncing: exec_all_or(exec.execution_syncing, DEFAULT_EXECUTION_SYNCING), + importing: exec_all_or(exec.execution_import_block, DEFAULT_EXECUTION_IMPORT_BLOCK), + block_construction: + exec_all_or(exec.execution_block_construction, DEFAULT_EXECUTION_BLOCK_CONSTRUCTION), + offchain_worker: + exec_all_or(exec.execution_offchain_worker, DEFAULT_EXECUTION_OFFCHAIN_WORKER), + other: exec_all_or(exec.execution_other, DEFAULT_EXECUTION_OTHER), }; Ok(()) } @@ -987,7 +1008,7 @@ where sc_service::Roles::FULL }; - fill_import_params(&mut config, &cli.import_params, role)?; + fill_import_params(&mut config, &cli.import_params, role, is_dev)?; config.impl_name = impl_name; config.impl_commit = version.commit; diff --git a/client/cli/src/params.rs b/client/cli/src/params.rs index 07fbe9b73885c..57b90c2e73d5c 100644 --- a/client/cli/src/params.rs +++ b/client/cli/src/params.rs @@ -18,6 +18,7 @@ use crate::traits::GetSharedParams; use std::{str::FromStr, path::PathBuf}; use structopt::{StructOpt, StructOptInternal, clap::{arg_enum, App, AppSettings, SubCommand, Arg}}; +use crate::execution_strategy::*; pub use crate::execution_strategy::ExecutionStrategy; @@ -321,7 +322,7 @@ pub struct ExecutionStrategies { value_name = "STRATEGY", possible_values = &ExecutionStrategy::variants(), case_insensitive = true, - default_value = "NativeElseWasm" + default_value = DEFAULT_EXECUTION_SYNCING.as_str(), )] pub execution_syncing: ExecutionStrategy, @@ -331,7 +332,7 @@ pub struct ExecutionStrategies { value_name = "STRATEGY", possible_values = &ExecutionStrategy::variants(), case_insensitive = true, - default_value = "NativeElseWasm" + default_value = DEFAULT_EXECUTION_IMPORT_BLOCK.as_str(), )] pub execution_import_block: ExecutionStrategy, @@ -341,7 +342,7 @@ pub struct ExecutionStrategies { value_name = "STRATEGY", possible_values = &ExecutionStrategy::variants(), case_insensitive = true, - default_value = "Wasm" + default_value = DEFAULT_EXECUTION_BLOCK_CONSTRUCTION.as_str(), )] pub execution_block_construction: ExecutionStrategy, @@ -351,7 +352,7 @@ pub struct ExecutionStrategies { value_name = "STRATEGY", possible_values = &ExecutionStrategy::variants(), case_insensitive = true, - default_value = "Native" + default_value = DEFAULT_EXECUTION_OFFCHAIN_WORKER.as_str(), )] pub execution_offchain_worker: ExecutionStrategy, @@ -361,7 +362,7 @@ pub struct ExecutionStrategies { value_name = "STRATEGY", possible_values = &ExecutionStrategy::variants(), case_insensitive = true, - default_value = "Native" + default_value = DEFAULT_EXECUTION_OTHER.as_str(), )] pub execution_other: ExecutionStrategy, diff --git a/client/executor/runtime-test/build.rs b/client/executor/runtime-test/build.rs index 1ed2fa43e685c..9c81ea6f38b8d 100644 --- a/client/executor/runtime-test/build.rs +++ b/client/executor/runtime-test/build.rs @@ -21,7 +21,7 @@ fn main() { "wasm_binary.rs", WasmBuilderSource::CratesOrPath { path: "../../../utils/wasm-builder", - version: "1.0.8", + version: "1.0.9", }, // This instructs LLD to export __heap_base as a global variable, which is used by the // external memory allocator. diff --git a/primitives/runtime-interface/test-wasm/build.rs b/primitives/runtime-interface/test-wasm/build.rs index 8e1d17e821195..9c81ea6f38b8d 100644 --- a/primitives/runtime-interface/test-wasm/build.rs +++ b/primitives/runtime-interface/test-wasm/build.rs @@ -21,7 +21,7 @@ fn main() { "wasm_binary.rs", WasmBuilderSource::CratesOrPath { path: "../../../utils/wasm-builder", - version: "1.0.6", + version: "1.0.9", }, // This instructs LLD to export __heap_base as a global variable, which is used by the // external memory allocator. diff --git a/test-utils/runtime/build.rs b/test-utils/runtime/build.rs index a90270d85d55f..8a9ee642c4fee 100644 --- a/test-utils/runtime/build.rs +++ b/test-utils/runtime/build.rs @@ -21,7 +21,7 @@ fn main() { "wasm_binary.rs", WasmBuilderSource::CratesOrPath { path: "../../utils/wasm-builder", - version: "1.0.8", + version: "1.0.9", }, // Note that we set the stack-size to 1MB explicitly even though it is set // to this value by default. This is because some of our tests (`restoration_of_globals`) diff --git a/utils/wasm-builder/Cargo.toml b/utils/wasm-builder/Cargo.toml index 66d94c32ab330..5941a39ffb6a6 100644 --- a/utils/wasm-builder/Cargo.toml +++ b/utils/wasm-builder/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "substrate-wasm-builder" -version = "1.0.8" +version = "1.0.9" authors = ["Parity Technologies "] description = "Utility for building WASM binaries" edition = "2018" diff --git a/utils/wasm-builder/src/wasm_project.rs b/utils/wasm-builder/src/wasm_project.rs index a028d00930a4e..c9b573ff1940f 100644 --- a/utils/wasm-builder/src/wasm_project.rs +++ b/utils/wasm-builder/src/wasm_project.rs @@ -357,7 +357,7 @@ fn is_release_build() -> bool { ), } } else { - !build_helper::debug() + true } } From f52ea97fb6074598e64989711f9b882418d087c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Tue, 21 Jan 2020 11:45:36 +0000 Subject: [PATCH 10/18] grandpa: reduce allocations when verifying multiple messages (#4693) --- .../finality-grandpa/src/communication/mod.rs | 66 +++++++++++++++++-- client/finality-grandpa/src/justification.rs | 4 +- 2 files changed, 62 insertions(+), 8 deletions(-) diff --git a/client/finality-grandpa/src/communication/mod.rs b/client/finality-grandpa/src/communication/mod.rs index 7723047d1b423..5077d435e8550 100644 --- a/client/finality-grandpa/src/communication/mod.rs +++ b/client/finality-grandpa/src/communication/mod.rs @@ -600,8 +600,28 @@ impl> Clone for NetworkBridge { } } -pub(crate) fn localized_payload(round: RoundNumber, set_id: SetIdNumber, message: &E) -> Vec { - (message, round, set_id).encode() +/// Encode round message localized to a given round and set id. +pub(crate) fn localized_payload( + round: RoundNumber, + set_id: SetIdNumber, + message: &E, +) -> Vec { + let mut buf = Vec::new(); + localized_payload_with_buffer(round, set_id, message, &mut buf); + buf +} + +/// Encode round message localized to a given round and set id using the given +/// buffer. The given buffer will be cleared and the resulting encoded payload +/// will always be written to the start of the buffer. +pub(crate) fn localized_payload_with_buffer( + round: RoundNumber, + set_id: SetIdNumber, + message: &E, + buf: &mut Vec, +) { + buf.clear(); + (message, round, set_id).encode_to(buf) } /// Type-safe wrapper around a round number. @@ -612,17 +632,41 @@ pub struct Round(pub RoundNumber); #[derive(Debug, Clone, Copy, Eq, PartialEq, PartialOrd, Ord, Encode, Decode)] pub struct SetId(pub SetIdNumber); -// check a message. +/// Check a message signature by encoding the message as a localized payload and +/// verifying the provided signature using the expected authority id. pub(crate) fn check_message_sig( message: &Message, id: &AuthorityId, signature: &AuthoritySignature, round: RoundNumber, set_id: SetIdNumber, +) -> Result<(), ()> { + check_message_sig_with_buffer::( + message, + id, + signature, + round, + set_id, + &mut Vec::new(), + ) +} + +/// Check a message signature by encoding the message as a localized payload and +/// verifying the provided signature using the expected authority id. +/// The encoding necessary to verify the signature will be done using the given +/// buffer, the original content of the buffer will be cleared. +pub(crate) fn check_message_sig_with_buffer( + message: &Message, + id: &AuthorityId, + signature: &AuthoritySignature, + round: RoundNumber, + set_id: SetIdNumber, + buf: &mut Vec, ) -> Result<(), ()> { let as_public = id.clone(); - let encoded_raw = localized_payload(round, set_id, message); - if AuthorityPair::verify(signature, &encoded_raw, &as_public) { + localized_payload_with_buffer(round, set_id, message, buf); + + if AuthorityPair::verify(signature, buf, &as_public) { Ok(()) } else { debug!(target: "afg", "Bad signature on message from {:?}", id); @@ -752,6 +796,7 @@ fn check_compact_commit( } // check signatures on all contained precommits. + let mut buf = Vec::new(); for (i, (precommit, &(ref sig, ref id))) in msg.precommits.iter() .zip(&msg.auth_data) .enumerate() @@ -759,12 +804,13 @@ fn check_compact_commit( use crate::communication::gossip::Misbehavior; use finality_grandpa::Message as GrandpaMessage; - if let Err(()) = check_message_sig::( + if let Err(()) = check_message_sig_with_buffer::( &GrandpaMessage::Precommit(precommit.clone()), id, sig, round.0, set_id.0, + &mut buf, ) { debug!(target: "afg", "Bad commit message signature {}", id); telemetry!(CONSENSUS_DEBUG; "afg.bad_commit_msg_signature"; "id" => ?id); @@ -836,6 +882,7 @@ fn check_catch_up( round: RoundNumber, set_id: SetIdNumber, mut signatures_checked: usize, + buf: &mut Vec, ) -> Result where B: BlockT, I: Iterator, &'a AuthorityId, &'a AuthoritySignature)>, @@ -845,12 +892,13 @@ fn check_catch_up( for (msg, id, sig) in messages { signatures_checked += 1; - if let Err(()) = check_message_sig::( + if let Err(()) = check_message_sig_with_buffer::( &msg, id, sig, round, set_id, + buf, ) { debug!(target: "afg", "Bad catch up message signature {}", id); telemetry!(CONSENSUS_DEBUG; "afg.bad_catch_up_msg_signature"; "id" => ?id); @@ -866,6 +914,8 @@ fn check_catch_up( Ok(signatures_checked) } + let mut buf = Vec::new(); + // check signatures on all contained prevotes. let signatures_checked = check_signatures::( msg.prevotes.iter().map(|vote| { @@ -874,6 +924,7 @@ fn check_catch_up( msg.round_number, set_id.0, 0, + &mut buf, )?; // check signatures on all contained precommits. @@ -884,6 +935,7 @@ fn check_catch_up( msg.round_number, set_id.0, signatures_checked, + &mut buf, )?; Ok(()) diff --git a/client/finality-grandpa/src/justification.rs b/client/finality-grandpa/src/justification.rs index 308056725f67b..ad96956454fd7 100644 --- a/client/finality-grandpa/src/justification.rs +++ b/client/finality-grandpa/src/justification.rs @@ -132,14 +132,16 @@ impl GrandpaJustification { } } + let mut buf = Vec::new(); let mut visited_hashes = HashSet::new(); for signed in self.commit.precommits.iter() { - if let Err(_) = communication::check_message_sig::( + if let Err(_) = communication::check_message_sig_with_buffer::( &finality_grandpa::Message::Precommit(signed.precommit.clone()), &signed.id, &signed.signature, self.round, set_id, + &mut buf, ) { return Err(ClientError::BadJustification( "invalid signature for precommit in grandpa justification".to_string()).into()); From 6ee1244e2d018333746d82131222308e0d802e6a Mon Sep 17 00:00:00 2001 From: Pierre Krieger Date: Tue, 21 Jan 2020 13:06:15 +0100 Subject: [PATCH 11/18] Pass an executor through the Configuration (#4688) * Pass an executor through the Configuration * Make tasks_executor mandatory * Fix tests --- bin/node-template/src/cli.rs | 6 +++++- bin/node/cli/src/cli.rs | 6 +++++- client/service/src/builder.rs | 6 +++++- client/service/src/config.rs | 6 ++++-- client/service/src/error.rs | 3 +++ client/service/src/lib.rs | 25 ++++--------------------- client/service/test/src/lib.rs | 21 +++++++++++++++++++-- utils/browser/src/lib.rs | 3 +++ 8 files changed, 48 insertions(+), 28 deletions(-) diff --git a/bin/node-template/src/cli.rs b/bin/node-template/src/cli.rs index 44764e5c9db41..fcfd330816cd1 100644 --- a/bin/node-template/src/cli.rs +++ b/bin/node-template/src/cli.rs @@ -18,7 +18,7 @@ pub fn run(args: I, exit: E, version: VersionInfo) -> error::Result<()> type Config = Configuration<(), T>; match parse_and_prepare::(&version, "substrate-node", args) { ParseAndPrepare::Run(cmd) => cmd.run(load_spec, exit, - |exit, _cli_args, _custom_args, config: Config<_>| { + |exit, _cli_args, _custom_args, mut config: Config<_>| { info!("{}", version.name); info!(" version {}", config.full_version()); info!(" by {}, 2017, 2018", version.author); @@ -26,6 +26,10 @@ pub fn run(args: I, exit: E, version: VersionInfo) -> error::Result<()> info!("Node name: {}", config.name); info!("Roles: {}", display_role(&config)); let runtime = Runtime::new().map_err(|e| format!("{:?}", e))?; + config.tasks_executor = { + let runtime_handle = runtime.handle().clone(); + Some(Box::new(move |fut| { runtime_handle.spawn(fut); })) + }; match config.roles { ServiceRoles::LIGHT => run_until_exit( runtime, diff --git a/bin/node/cli/src/cli.rs b/bin/node/cli/src/cli.rs index 6a2e02efb6209..5ade700513e53 100644 --- a/bin/node/cli/src/cli.rs +++ b/bin/node/cli/src/cli.rs @@ -98,7 +98,7 @@ pub fn run(args: I, exit: E, version: sc_cli::VersionInfo) -> error::Re match parse_and_prepare::(&version, "substrate-node", args) { ParseAndPrepare::Run(cmd) => cmd.run(load_spec, exit, - |exit, _cli_args, _custom_args, config: Config<_, _>| { + |exit, _cli_args, _custom_args, mut config: Config<_, _>| { info!("{}", version.name); info!(" version {}", config.full_version()); info!(" by Parity Technologies, 2017-2019"); @@ -111,6 +111,10 @@ pub fn run(args: I, exit: E, version: sc_cli::VersionInfo) -> error::Re .enable_all() .build() .map_err(|e| format!("{:?}", e))?; + config.tasks_executor = { + let runtime_handle = runtime.handle().clone(); + Some(Box::new(move |fut| { runtime_handle.spawn(fut); })) + }; match config.roles { ServiceRoles::LIGHT => run_until_exit( runtime, diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index 5449b00b061a3..044798701c6e1 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -1147,7 +1147,11 @@ ServiceBuilder< essential_failed_rx, to_spawn_tx, to_spawn_rx, - to_poll: Vec::new(), + tasks_executor: if let Some(exec) = config.tasks_executor { + exec + } else { + return Err(Error::TasksExecutorRequired); + }, rpc_handlers, _rpc: rpc, _telemetry: telemetry, diff --git a/client/service/src/config.rs b/client/service/src/config.rs index 75c626821479d..8a145dec165b4 100644 --- a/client/service/src/config.rs +++ b/client/service/src/config.rs @@ -21,7 +21,7 @@ pub use sc_client_db::{kvdb::KeyValueDB, PruningMode}; pub use sc_network::config::{ExtTransport, NetworkConfiguration, Roles}; pub use sc_executor::WasmExecutionMethod; -use std::{path::{PathBuf, Path}, net::SocketAddr, sync::Arc}; +use std::{future::Future, path::{PathBuf, Path}, pin::Pin, net::SocketAddr, sync::Arc}; pub use sc_transaction_pool::txpool::Options as TransactionPoolOptions; use sc_chain_spec::{ChainSpec, RuntimeGenesis, Extension, NoExtension}; use sp_core::crypto::Protected; @@ -29,7 +29,6 @@ use target_info::Target; use sc_telemetry::TelemetryEndpoints; /// Service configuration. -#[derive(Clone)] pub struct Configuration { /// Implementation name pub impl_name: &'static str, @@ -39,6 +38,8 @@ pub struct Configuration { pub impl_commit: &'static str, /// Node roles. pub roles: Roles, + /// How to spawn background tasks. Mandatory, otherwise creating a `Service` will error. + pub tasks_executor: Option + Send>>) + Send>>, /// Extrinsic pool configuration. pub transaction_pool: TransactionPoolOptions, /// Network configuration. @@ -160,6 +161,7 @@ impl Configuration where config_dir: config_dir.clone(), name: Default::default(), roles: Roles::FULL, + tasks_executor: None, transaction_pool: Default::default(), network: Default::default(), keystore: KeystoreConfig::None, diff --git a/client/service/src/error.rs b/client/service/src/error.rs index 6516b1c62c6d6..14b03d7e95de7 100644 --- a/client/service/src/error.rs +++ b/client/service/src/error.rs @@ -40,6 +40,9 @@ pub enum Error { /// Best chain selection strategy is missing. #[display(fmt="Best chain selection strategy (SelectChain) is not provided.")] SelectChainRequired, + /// Tasks executor is missing. + #[display(fmt="Tasks executor hasn't been provided.")] + TasksExecutorRequired, /// Other error. Other(String), } diff --git a/client/service/src/lib.rs b/client/service/src/lib.rs index 87327d0967583..c1b87e4491904 100644 --- a/client/service/src/lib.rs +++ b/client/service/src/lib.rs @@ -38,7 +38,7 @@ use parking_lot::Mutex; use sc_client::Client; use exit_future::Signal; use futures::{ - Future, FutureExt, Stream, StreamExt, TryFutureExt, + Future, FutureExt, Stream, StreamExt, future::select, channel::mpsc, compat::*, sink::SinkExt, @@ -95,10 +95,8 @@ pub struct Service { to_spawn_tx: mpsc::UnboundedSender + Send>>>, /// Receiver for futures that must be spawned as background tasks. to_spawn_rx: mpsc::UnboundedReceiver + Send>>>, - /// List of futures to poll from `poll`. - /// If spawning a background task is not possible, we instead push the task into this `Vec`. - /// The elements must then be polled manually. - to_poll: Vec + Send>>>, + /// How to spawn background tasks. + tasks_executor: Box + Send>>) + Send>, rpc_handlers: sc_rpc_server::RpcHandler, _rpc: Box, _telemetry: Option, @@ -322,22 +320,7 @@ impl Future for } while let Poll::Ready(Some(task_to_spawn)) = Pin::new(&mut this.to_spawn_rx).poll_next(cx) { - // TODO: Update to tokio 0.2 when libp2p get switched to std futures (#4383) - let executor = tokio_executor::DefaultExecutor::current(); - use futures01::future::Executor; - if let Err(err) = executor.execute(task_to_spawn.unit_error().compat()) { - debug!( - target: "service", - "Failed to spawn background task: {:?}; falling back to manual polling", - err - ); - this.to_poll.push(Box::pin(err.into_future().compat().map(drop))); - } - } - - // Polling all the `to_poll` futures. - while let Some(pos) = this.to_poll.iter_mut().position(|t| Pin::new(t).poll(cx).is_ready()) { - let _ = this.to_poll.remove(pos); + (this.tasks_executor)(task_to_spawn); } // The service future never ends. diff --git a/client/service/test/src/lib.rs b/client/service/test/src/lib.rs index dd6395e9c6228..961c8d98ffd09 100644 --- a/client/service/test/src/lib.rs +++ b/client/service/test/src/lib.rs @@ -19,9 +19,11 @@ use std::iter; use std::sync::{Arc, Mutex, MutexGuard}; use std::net::Ipv4Addr; +use std::pin::Pin; use std::time::Duration; use log::info; use futures01::{Future, Stream, Poll}; +use futures::{FutureExt as _, TryFutureExt as _}; use tempfile::TempDir; use tokio::{runtime::Runtime, prelude::FutureExt}; use tokio::timer::Interval; @@ -131,6 +133,7 @@ fn node_config ( index: usize, spec: &ChainSpec, role: Roles, + tasks_executor: Box + Send>>) + Send>, key_seed: Option, base_port: u16, root: &TempDir, @@ -172,6 +175,7 @@ fn node_config ( impl_version: "0.1", impl_commit: "", roles: role, + tasks_executor: Some(tasks_executor), transaction_pool: Default::default(), network: network_config, keystore: KeystoreConfig::Path { @@ -251,10 +255,15 @@ impl TestNet where let executor = self.runtime.executor(); for (key, authority) in authorities { + let tasks_executor = { + let executor = executor.clone(); + Box::new(move |fut: Pin + Send>>| executor.spawn(fut.unit_error().compat())) + }; let node_config = node_config( self.nodes, &self.chain_spec, Roles::AUTHORITY, + tasks_executor, Some(key), self.base_port, &temp, @@ -270,7 +279,11 @@ impl TestNet where } for full in full { - let node_config = node_config(self.nodes, &self.chain_spec, Roles::FULL, None, self.base_port, &temp); + let tasks_executor = { + let executor = executor.clone(); + Box::new(move |fut: Pin + Send>>| executor.spawn(fut.unit_error().compat())) + }; + let node_config = node_config(self.nodes, &self.chain_spec, Roles::FULL, tasks_executor, None, self.base_port, &temp); let addr = node_config.network.listen_addresses.iter().next().unwrap().clone(); let (service, user_data) = full(node_config).expect("Error creating test node service"); let service = SyncService::from(service); @@ -282,7 +295,11 @@ impl TestNet where } for light in light { - let node_config = node_config(self.nodes, &self.chain_spec, Roles::LIGHT, None, self.base_port, &temp); + let tasks_executor = { + let executor = executor.clone(); + Box::new(move |fut: Pin + Send>>| executor.spawn(fut.unit_error().compat())) + }; + let node_config = node_config(self.nodes, &self.chain_spec, Roles::LIGHT, tasks_executor, None, self.base_port, &temp); let addr = node_config.network.listen_addresses.iter().next().unwrap().clone(); let service = SyncService::from(light(node_config).expect("Error creating test node service")); diff --git a/utils/browser/src/lib.rs b/utils/browser/src/lib.rs index 0dbde57182766..7b7fda45839ee 100644 --- a/utils/browser/src/lib.rs +++ b/utils/browser/src/lib.rs @@ -52,6 +52,9 @@ where allow_private_ipv4: true, enable_mdns: false, }; + config.tasks_executor = Some(Box::new(move |fut| { + wasm_bindgen_futures::spawn_local(fut) + })); config.telemetry_external_transport = Some(transport); config.roles = Roles::LIGHT; config.name = format!("{} (Browser)", name); From 14720148ee5a07911fbba8c91ffd9f4490bded68 Mon Sep 17 00:00:00 2001 From: Hero Bird Date: Tue, 21 Jan 2020 13:06:54 +0100 Subject: [PATCH 12/18] contracts: New contract events + unconfusions (#4685) * contracts: during execution -> contract trapped during execution This message confused many people so we are improving it to make clear what happened. * contracts: rename Event::Contract -> Event::ContractExecution * contracts: fix tests after ContractExecution renaming * contracts: Add Evicted and Restored events * fix doc comment * wrap to not go over (soft) 100 column line limit * add event deposit for eventual eviction upon pay_rent * contracts: adjust tests for the new events * emit Evicted event immediately and add tombstone flag bool --- frame/contracts/src/exec.rs | 2 +- frame/contracts/src/lib.rs | 30 ++++++++- frame/contracts/src/rent.rs | 6 +- frame/contracts/src/tests.rs | 98 +++++++++++++++++++++++++++-- frame/contracts/src/wasm/mod.rs | 6 +- frame/contracts/src/wasm/runtime.rs | 2 +- 6 files changed, 131 insertions(+), 13 deletions(-) diff --git a/frame/contracts/src/exec.rs b/frame/contracts/src/exec.rs index 2c77173135077..9d786c320b5a9 100644 --- a/frame/contracts/src/exec.rs +++ b/frame/contracts/src/exec.rs @@ -782,7 +782,7 @@ where fn deposit_event(&mut self, topics: Vec, data: Vec) { self.ctx.deferred.push(DeferredAction::DepositEvent { topics, - event: RawEvent::Contract(self.ctx.self_account.clone(), data), + event: RawEvent::ContractExecution(self.ctx.self_account.clone(), data), }); } diff --git a/frame/contracts/src/lib.rs b/frame/contracts/src/lib.rs index df8da8866020d..40ce86518a5e4 100644 --- a/frame/contracts/src/lib.rs +++ b/frame/contracts/src/lib.rs @@ -786,7 +786,12 @@ impl Module { rent_allowance, delta, } => { - let _result = Self::restore_to(donor, dest, code_hash, rent_allowance, delta); + let result = Self::restore_to( + donor.clone(), dest.clone(), code_hash.clone(), rent_allowance.clone(), delta + ); + Self::deposit_event( + RawEvent::Restored(donor, dest, code_hash, rent_allowance, result.is_ok()) + ); } } }); @@ -896,6 +901,25 @@ decl_event! { /// Contract deployed by address at the specified address. Instantiated(AccountId, AccountId), + /// Contract has been evicted and is now in tombstone state. + /// + /// # Params + /// + /// - `contract`: `AccountId`: The account ID of the evicted contract. + /// - `tombstone`: `bool`: True if the evicted contract left behind a tombstone. + Evicted(AccountId, bool), + + /// Restoration for a contract has been initiated. + /// + /// # Params + /// + /// - `donor`: `AccountId`: Account ID of the restoring contract + /// - `dest`: `AccountId`: Account ID of the restored contract + /// - `code_hash`: `Hash`: Code hash of the restored contract + /// - `rent_allowance: `Balance`: Rent allowance of the restored contract + /// - `success`: `bool`: True if the restoration was successful + Restored(AccountId, AccountId, Hash, Balance, bool), + /// Code with the specified hash has been stored. CodeStored(Hash), @@ -906,8 +930,8 @@ decl_event! { /// successful execution or not. Dispatched(AccountId, bool), - /// An event from contract of account. - Contract(AccountId, Vec), + /// An event deposited upon execution of a contract from the account. + ContractExecution(AccountId, Vec), } } diff --git a/frame/contracts/src/rent.rs b/frame/contracts/src/rent.rs index 59c8b02d199cf..508511da4cbe3 100644 --- a/frame/contracts/src/rent.rs +++ b/frame/contracts/src/rent.rs @@ -14,7 +14,8 @@ // You should have received a copy of the GNU General Public License // along with Substrate. If not, see . -use crate::{BalanceOf, ContractInfo, ContractInfoOf, TombstoneContractInfo, Trait, AliveContractInfo}; +use crate::{Module, RawEvent, BalanceOf, ContractInfo, ContractInfoOf, TombstoneContractInfo, + Trait, AliveContractInfo}; use sp_runtime::traits::{Bounded, CheckedDiv, CheckedMul, Saturating, Zero, SaturatedConversion}; use frame_support::traits::{Currency, ExistenceRequirement, Get, WithdrawReason, OnUnbalanced}; @@ -101,6 +102,7 @@ fn try_evict_or_and_pay_rent( // The contract cannot afford to leave a tombstone, so remove the contract info altogether. >::remove(account); child::kill_storage(&contract.trie_id, contract.child_trie_unique_id()); + >::deposit_event(RawEvent::Evicted(account.clone(), false)); return (RentOutcome::Evicted, None); } @@ -160,6 +162,8 @@ fn try_evict_or_and_pay_rent( child::kill_storage(&contract.trie_id, contract.child_trie_unique_id()); + >::deposit_event(RawEvent::Evicted(account.clone(), true)); + return (RentOutcome::Evicted, Some(tombstone_info)); } diff --git a/frame/contracts/src/tests.rs b/frame/contracts/src/tests.rs index 91679a9216a16..9a2ef36bb86f0 100644 --- a/frame/contracts/src/tests.rs +++ b/frame/contracts/src/tests.rs @@ -453,7 +453,7 @@ fn instantiate_and_call_and_deposit_event() { }, EventRecord { phase: Phase::ApplyExtrinsic(0), - event: MetaEvent::contract(RawEvent::Contract(BOB, vec![1, 2, 3, 4])), + event: MetaEvent::contract(RawEvent::ContractExecution(BOB, vec![1, 2, 3, 4])), topics: vec![], }, EventRecord { @@ -650,7 +650,7 @@ fn dispatch_call_not_dispatched_after_top_level_transaction_failure() { 100_000, vec![], ), - "during execution" + "contract trapped during execution" ); assert_eq!(System::events(), vec![ EventRecord { @@ -1139,8 +1139,16 @@ fn call_removed_contract() { Contract::call(Origin::signed(ALICE), BOB, 0, 100_000, call::null()), "contract has been evicted" ); + // Calling a contract that is about to evict shall emit an event. + assert_eq!(System::events(), vec![ + EventRecord { + phase: Phase::ApplyExtrinsic(0), + event: MetaEvent::contract(RawEvent::Evicted(BOB, true)), + topics: vec![], + }, + ]); - // Subsequent contract calls should also fail. + // Subsequent contract calls should also fail. assert_err!( Contract::call(Origin::signed(ALICE), BOB, 0, 100_000, call::null()), "contract has been evicted" @@ -1367,6 +1375,9 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage: // Advance 4 blocks, to the 5th. initialize_block(5); + /// Preserve `BOB`'s code hash for later introspection. + let bob_code_hash = ContractInfoOf::::get(BOB).unwrap() + .get_alive().unwrap().code_hash; // Call `BOB`, which makes it pay rent. Since the rent allowance is set to 0 // we expect that it will get removed leaving tombstone. assert_err!( @@ -1374,6 +1385,15 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage: "contract has been evicted" ); assert!(ContractInfoOf::::get(BOB).unwrap().get_tombstone().is_some()); + assert_eq!(System::events(), vec![ + EventRecord { + phase: Phase::ApplyExtrinsic(0), + event: MetaEvent::contract( + RawEvent::Evicted(BOB.clone(), true) + ), + topics: vec![], + }, + ]); /// Create another account with the address `DJANGO` with `CODE_RESTORATION`. /// @@ -1416,6 +1436,60 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage: assert_eq!(django_contract.storage_size, 16); assert_eq!(django_contract.trie_id, django_trie_id); assert_eq!(django_contract.deduct_block, System::block_number()); + match (test_different_storage, test_restore_to_with_dirty_storage) { + (true, false) => { + assert_eq!(System::events(), vec![ + EventRecord { + phase: Phase::ApplyExtrinsic(0), + event: MetaEvent::contract( + RawEvent::Restored(DJANGO, BOB, bob_code_hash, 50, false) + ), + topics: vec![], + }, + ]); + } + (_, true) => { + assert_eq!(System::events(), vec![ + EventRecord { + phase: Phase::ApplyExtrinsic(0), + event: MetaEvent::contract(RawEvent::Evicted(BOB, true)), + topics: vec![], + }, + EventRecord { + phase: Phase::ApplyExtrinsic(0), + event: MetaEvent::balances(pallet_balances::RawEvent::NewAccount(CHARLIE, 1_000_000)), + topics: vec![], + }, + EventRecord { + phase: Phase::ApplyExtrinsic(0), + event: MetaEvent::balances(pallet_balances::RawEvent::NewAccount(DJANGO, 30_000)), + topics: vec![], + }, + EventRecord { + phase: Phase::ApplyExtrinsic(0), + event: MetaEvent::contract(RawEvent::Transfer(CHARLIE, DJANGO, 30_000)), + topics: vec![], + }, + EventRecord { + phase: Phase::ApplyExtrinsic(0), + event: MetaEvent::contract(RawEvent::Instantiated(CHARLIE, DJANGO)), + topics: vec![], + }, + EventRecord { + phase: Phase::ApplyExtrinsic(0), + event: MetaEvent::contract(RawEvent::Restored( + DJANGO, + BOB, + bob_code_hash, + 50, + false, + )), + topics: vec![], + }, + ]); + } + _ => unreachable!(), + } } else { // Here we expect that the restoration is succeeded. Check that the restoration // contract `DJANGO` ceased to exist and that `BOB` returned back. @@ -1427,6 +1501,20 @@ fn restoration(test_different_storage: bool, test_restore_to_with_dirty_storage: assert_eq!(bob_contract.trie_id, django_trie_id); assert_eq!(bob_contract.deduct_block, System::block_number()); assert!(ContractInfoOf::::get(DJANGO).is_none()); + assert_eq!(System::events(), vec![ + EventRecord { + phase: Phase::ApplyExtrinsic(0), + event: MetaEvent::balances(balances::RawEvent::ReapedAccount(DJANGO, 0)), + topics: vec![], + }, + EventRecord { + phase: Phase::ApplyExtrinsic(0), + event: MetaEvent::contract( + RawEvent::Restored(DJANGO, BOB, bob_contract.code_hash, 50, true) + ), + topics: vec![], + }, + ]); } }); } @@ -1533,7 +1621,7 @@ fn storage_max_value_limit() { 100_000, Encode::encode(&(self::MaxValueSize::get() + 1)), ), - "during execution" + "contract trapped during execution" ); }); } @@ -2056,7 +2144,7 @@ fn cannot_self_destruct_while_live() { 100_000, vec![0], ), - "during execution" + "contract trapped during execution" ); // Check that BOB is still alive. diff --git a/frame/contracts/src/wasm/mod.rs b/frame/contracts/src/wasm/mod.rs index 8e02eb482bb8a..60402cf3a09cf 100644 --- a/frame/contracts/src/wasm/mod.rs +++ b/frame/contracts/src/wasm/mod.rs @@ -1430,7 +1430,9 @@ mod tests { MockExt::default(), &mut gas_meter ), - Err(ExecError { reason: DispatchError::Other("during execution"), buffer: _ }) + Err(ExecError { + reason: DispatchError::Other("contract trapped during execution"), buffer: _ + }) ); } @@ -1472,7 +1474,7 @@ mod tests { MockExt::default(), &mut gas_meter ), - Err(ExecError { reason: DispatchError::Other("during execution"), buffer: _ }) + Err(ExecError { reason: DispatchError::Other("contract trapped during execution"), buffer: _ }) ); } diff --git a/frame/contracts/src/wasm/runtime.rs b/frame/contracts/src/wasm/runtime.rs index 81b0809f82b02..75751b6d359a3 100644 --- a/frame/contracts/src/wasm/runtime.rs +++ b/frame/contracts/src/wasm/runtime.rs @@ -109,7 +109,7 @@ pub(crate) fn to_execution_result( Err(ExecError { reason: "validation error".into(), buffer: runtime.scratch_buf }), // Any other kind of a trap should result in a failure. Err(sp_sandbox::Error::Execution) | Err(sp_sandbox::Error::OutOfBounds) => - Err(ExecError { reason: "during execution".into(), buffer: runtime.scratch_buf }), + Err(ExecError { reason: "contract trapped during execution".into(), buffer: runtime.scratch_buf }), } } From ef970571ccd87d973a6a359bf709a357303161dc Mon Sep 17 00:00:00 2001 From: Nikolay Volf Date: Tue, 21 Jan 2020 08:01:55 -0800 Subject: [PATCH 13/18] fix docs deadlinks (#4698) --- client/consensus/slots/src/lib.rs | 2 +- client/src/lib.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/client/consensus/slots/src/lib.rs b/client/consensus/slots/src/lib.rs index 3aa243af72b06..c6185d5d307b9 100644 --- a/client/consensus/slots/src/lib.rs +++ b/client/consensus/slots/src/lib.rs @@ -45,7 +45,7 @@ use parking_lot::Mutex; /// The changes that need to applied to the storage to create the state for a block. /// -/// See [`state_machine::StorageChanges`] for more information. +/// See [`sp_state_machine::StorageChanges`] for more information. pub type StorageChanges = sp_state_machine::StorageChanges, NumberFor>; diff --git a/client/src/lib.rs b/client/src/lib.rs index db2d4785a2c98..8aa71c5ec5c88 100644 --- a/client/src/lib.rs +++ b/client/src/lib.rs @@ -20,20 +20,20 @@ //! parts: //! //! - A database containing the blocks and chain state, generally referred to as -//! the [`Backend`](backend::Backend). +//! the [`Backend`](sc_client_api::backend::Backend). //! - A runtime environment, generally referred to as the [`Executor`](CallExecutor). //! //! # Initialization //! //! Creating a [`Client`] is done by calling the `new` method and passing to it a -//! [`Backend`](backend::Backend) and an [`Executor`](CallExecutor). +//! [`Backend`](sc_client_api::backend::Backend) and an [`Executor`](CallExecutor). //! //! The former is typically provided by the `sc-client-db` crate. //! //! The latter typically requires passing one of: //! //! - A [`LocalCallExecutor`] running the runtime locally. -//! - A [`RemoteCallExecutor`](light::call_executor::RemoteCallExecutor) that will ask a +//! - A [`RemoteCallExecutor`](light::call_executor::RemoteCallRequest) that will ask a //! third-party to perform the executions. //! - A [`RemoteOrLocalCallExecutor`](light::call_executor::RemoteOrLocalCallExecutor), combination //! of the two. From 4b2f70f8476a8c4092df952e549090268f936d1d Mon Sep 17 00:00:00 2001 From: Nikolay Volf Date: Tue, 21 Jan 2020 13:10:26 -0800 Subject: [PATCH 14/18] remove license preamble from node-template (#4699) --- bin/node-template/runtime/build.rs | 16 ---------------- bin/node-template/src/main.rs | 2 -- 2 files changed, 18 deletions(-) diff --git a/bin/node-template/runtime/build.rs b/bin/node-template/runtime/build.rs index dc7e949d809f8..8bdf7584bb81a 100644 --- a/bin/node-template/runtime/build.rs +++ b/bin/node-template/runtime/build.rs @@ -1,19 +1,3 @@ -// Copyright 2019-2020 Parity Technologies (UK) Ltd. -// This file is part of Substrate. - -// Substrate is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// Substrate is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with Substrate. If not, see . - use wasm_builder_runner::{build_current_project_with_rustflags, WasmBuilderSource}; fn main() { diff --git a/bin/node-template/src/main.rs b/bin/node-template/src/main.rs index 2942bb68a37aa..ea64bc1413642 100644 --- a/bin/node-template/src/main.rs +++ b/bin/node-template/src/main.rs @@ -1,7 +1,5 @@ //! Substrate Node Template CLI library. - #![warn(missing_docs)] -#![warn(unused_extern_crates)] mod chain_spec; #[macro_use] From cf020addf0e2d9d5fdaa7a34aa83b380f49471bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Tue, 21 Jan 2020 21:14:44 +0000 Subject: [PATCH 15/18] grandpa: filter some telemetry events on larger voter sets (#4700) --- .../finality-grandpa/src/communication/mod.rs | 65 +++++++++++-------- 1 file changed, 37 insertions(+), 28 deletions(-) diff --git a/client/finality-grandpa/src/communication/mod.rs b/client/finality-grandpa/src/communication/mod.rs index 5077d435e8550..18cb14c739636 100644 --- a/client/finality-grandpa/src/communication/mod.rs +++ b/client/finality-grandpa/src/communication/mod.rs @@ -106,6 +106,11 @@ mod benefit { pub(super) const PER_EQUIVOCATION: i32 = 10; } +/// If the voter set is larger than this value some telemetry events are not +/// sent to avoid increasing usage resource on the node and flooding the +/// telemetry server (e.g. received votes, received commits.) +const TELEMETRY_VOTERS_LIMIT: usize = 10; + /// A handle to the network. /// /// Something that provides both the capabilities needed for the `gossip_network::Network` trait as @@ -308,29 +313,31 @@ impl> NetworkBridge { return Ok(None); } - match &msg.message.message { - PrimaryPropose(propose) => { - telemetry!(CONSENSUS_INFO; "afg.received_propose"; - "voter" => ?format!("{}", msg.message.id), - "target_number" => ?propose.target_number, - "target_hash" => ?propose.target_hash, - ); - }, - Prevote(prevote) => { - telemetry!(CONSENSUS_INFO; "afg.received_prevote"; - "voter" => ?format!("{}", msg.message.id), - "target_number" => ?prevote.target_number, - "target_hash" => ?prevote.target_hash, - ); - }, - Precommit(precommit) => { - telemetry!(CONSENSUS_INFO; "afg.received_precommit"; - "voter" => ?format!("{}", msg.message.id), - "target_number" => ?precommit.target_number, - "target_hash" => ?precommit.target_hash, - ); - }, - }; + if voters.len() <= TELEMETRY_VOTERS_LIMIT { + match &msg.message.message { + PrimaryPropose(propose) => { + telemetry!(CONSENSUS_INFO; "afg.received_propose"; + "voter" => ?format!("{}", msg.message.id), + "target_number" => ?propose.target_number, + "target_hash" => ?propose.target_hash, + ); + }, + Prevote(prevote) => { + telemetry!(CONSENSUS_INFO; "afg.received_prevote"; + "voter" => ?format!("{}", msg.message.id), + "target_number" => ?prevote.target_number, + "target_hash" => ?prevote.target_hash, + ); + }, + Precommit(precommit) => { + telemetry!(CONSENSUS_INFO; "afg.received_precommit"; + "voter" => ?format!("{}", msg.message.id), + "target_number" => ?precommit.target_number, + "target_hash" => ?precommit.target_hash, + ); + }, + }; + } Ok(Some(msg.message)) } @@ -474,11 +481,13 @@ fn incoming_global( format!("{}", a) }).collect(); - telemetry!(CONSENSUS_INFO; "afg.received_commit"; - "contains_precommits_signed_by" => ?precommits_signed_by, - "target_number" => ?msg.message.target_number.clone(), - "target_hash" => ?msg.message.target_hash.clone(), - ); + if voters.len() <= TELEMETRY_VOTERS_LIMIT { + telemetry!(CONSENSUS_INFO; "afg.received_commit"; + "contains_precommits_signed_by" => ?precommits_signed_by, + "target_number" => ?msg.message.target_number.clone(), + "target_hash" => ?msg.message.target_hash.clone(), + ); + } if let Err(cost) = check_compact_commit::( &msg.message, From ef578cd5102e73dcca8ee902555467fed1031792 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Wed, 22 Jan 2020 12:17:52 +0100 Subject: [PATCH 16/18] Support `u128`/`i128` in runtime interface (#4703) * Support `u128`/`i128` in runtime interface This implements support for `u128`/`i128` as parameters/return value in runtime interfaces. As we can not pass them as identity, as for the other primitives types, we pass them as an pointer to an `[u8; 16]` array. * Remove some unsafe code usage --- primitives/runtime-interface/src/impls.rs | 52 +++++++++++++++++++ primitives/runtime-interface/src/lib.rs | 3 ++ .../runtime-interface/test-wasm/src/lib.rs | 20 +++++++ primitives/runtime-interface/test/src/lib.rs | 5 ++ 4 files changed, 80 insertions(+) diff --git a/primitives/runtime-interface/src/impls.rs b/primitives/runtime-interface/src/impls.rs index 3cd114268bbd3..cde8e60eea350 100644 --- a/primitives/runtime-interface/src/impls.rs +++ b/primitives/runtime-interface/src/impls.rs @@ -471,3 +471,55 @@ impl IntoFFIValue for Pointer { Ok(self.into()) } } + +/// Implement the traits for `u128`/`i128` +macro_rules! for_u128_i128 { + ($type:ty) => { + /// `u128`/`i128` is passed as `u32`. + /// + /// The `u32` is a pointer to an `[u8; 16]` array. + impl RIType for $type { + type FFIType = u32; + } + + #[cfg(not(feature = "std"))] + impl IntoFFIValue for $type { + type Owned = (); + + fn into_ffi_value(&self) -> WrappedFFIValue { + unsafe { (mem::transmute::<&Self, *const u8>(self) as u32).into() } + } + } + + #[cfg(not(feature = "std"))] + impl FromFFIValue for $type { + fn from_ffi_value(arg: u32) -> $type { + <$type>::from_le_bytes(<[u8; mem::size_of::<$type>()]>::from_ffi_value(arg)) + } + } + + #[cfg(feature = "std")] + impl FromFFIValue for $type { + type SelfInstance = $type; + + fn from_ffi_value(context: &mut dyn FunctionContext, arg: u32) -> Result<$type> { + let data = context.read_memory(Pointer::new(arg), mem::size_of::<$type>() as u32)?; + let mut res = [0u8; mem::size_of::<$type>()]; + res.copy_from_slice(&data); + Ok(<$type>::from_le_bytes(res)) + } + } + + #[cfg(feature = "std")] + impl IntoFFIValue for $type { + fn into_ffi_value(self, context: &mut dyn FunctionContext) -> Result { + let addr = context.allocate_memory(mem::size_of::<$type>() as u32)?; + context.write_memory(addr, &self.to_le_bytes())?; + Ok(addr.into()) + } + } + } +} + +for_u128_i128!(u128); +for_u128_i128!(i128); diff --git a/primitives/runtime-interface/src/lib.rs b/primitives/runtime-interface/src/lib.rs index 30d74ad2db399..609f4f600b784 100644 --- a/primitives/runtime-interface/src/lib.rs +++ b/primitives/runtime-interface/src/lib.rs @@ -81,10 +81,12 @@ //! | `u16` | `u16` | `Identity` | //! | `u32` | `u32` | `Identity` | //! | `u64` | `u64` | `Identity` | +//! | `i128` | `u32` | `v.as_ptr()` (pointer to a 16 byte array) | //! | `i8` | `i8` | `Identity` | //! | `i16` | `i16` | `Identity` | //! | `i32` | `i32` | `Identity` | //! | `i64` | `i64` | `Identity` | +//! | `u128` | `u32` | `v.as_ptr()` (pointer to a 16 byte array) | //! | `bool` | `u8` | `if v { 1 } else { 0 }` | //! | `&str` | `u64` | v.len() 32bit << 32 | v.as_ptr() 32bit | //! | `&[u8]` | `u64` | v.len() 32bit << 32 | v.as_ptr() 32bit | @@ -93,6 +95,7 @@ //! | `&[T] where T: Encode` | `u64` | `let e = v.encode();`

e.len() 32bit << 32 | e.as_ptr() 32bit | //! | `[u8; N]` | `u32` | `v.as_ptr()` | //! | `*const T` | `u32` | `Identity` | +//! | `Option` | `u64` | `let e = v.encode();`

e.len() 32bit << 32 | e.as_ptr() 32bit | //! | [`T where T: PassBy`](pass_by::Inner) | Depends on inner | Depends on inner | //! | [`T where T: PassBy`](pass_by::Codec) | `u64`| v.len() 32bit << 32 | v.as_ptr() 32bit | //! diff --git a/primitives/runtime-interface/test-wasm/src/lib.rs b/primitives/runtime-interface/test-wasm/src/lib.rs index a7e7e3e983a97..67fbfdcfec602 100644 --- a/primitives/runtime-interface/test-wasm/src/lib.rs +++ b/primitives/runtime-interface/test-wasm/src/lib.rs @@ -82,6 +82,16 @@ pub trait TestApi { fn overwrite_native_function_implementation() -> bool { false } + + /// Gets an `u128` and returns this value + fn get_and_return_u128(val: u128) -> u128 { + val + } + + /// Gets an `i128` and returns this value + fn get_and_return_i128(val: i128) -> i128 { + val + } } /// Two random external functions from the old runtime interface. @@ -191,4 +201,14 @@ wasm_export_functions! { assert!(test_api::overwrite_native_function_implementation()); } + + fn test_u128_i128_as_parameter_and_return_value() { + for val in &[u128::max_value(), 1u128, 5000u128, u64::max_value() as u128] { + assert_eq!(*val, test_api::get_and_return_u128(*val)); + } + + for val in &[i128::max_value(), i128::min_value(), 1i128, 5000i128, u64::max_value() as i128] { + assert_eq!(*val, test_api::get_and_return_i128(*val)); + } + } } diff --git a/primitives/runtime-interface/test/src/lib.rs b/primitives/runtime-interface/test/src/lib.rs index b209e1f71ce55..48c120b2c9fd1 100644 --- a/primitives/runtime-interface/test/src/lib.rs +++ b/primitives/runtime-interface/test/src/lib.rs @@ -108,3 +108,8 @@ fn test_invalid_utf8_data_should_return_an_error() { fn test_overwrite_native_function_implementation() { call_wasm_method::("test_overwrite_native_function_implementation"); } + +#[test] +fn test_u128_i128_as_parameter_and_return_value() { + call_wasm_method::("test_u128_i128_as_parameter_and_return_value"); +} From 2cc177251d600482953a91d92d0f66c55a0385d9 Mon Sep 17 00:00:00 2001 From: Max Inden Date: Wed, 22 Jan 2020 14:49:28 +0100 Subject: [PATCH 17/18] client/authority-discovery/Cargo.toml: Update dependency (#4706) * client/authority-discovery/Cargo.toml: Update dependency * client/authority-discovery: Pass packet payload and addresses as slice Starting with Bytes 0.5 `Vec` does not implement `Buf`, but `&[T]` does. --- Cargo.lock | 25 ++++++++++++++++++++++++- client/authority-discovery/Cargo.toml | 2 +- client/authority-discovery/src/lib.rs | 10 +++++----- 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8030a4cd13ad2..04a7a33595364 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4500,6 +4500,15 @@ dependencies = [ "prost-derive 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "prost" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "bytes 0.5.3 (registry+https://github.com/rust-lang/crates.io-index)", + "prost-derive 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "prost-build" version = "0.5.0" @@ -4529,6 +4538,18 @@ dependencies = [ "syn 0.15.44 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "prost-derive" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "anyhow 1.0.25 (registry+https://github.com/rust-lang/crates.io-index)", + "itertools 0.8.2 (registry+https://github.com/rust-lang/crates.io-index)", + "proc-macro2 1.0.6 (registry+https://github.com/rust-lang/crates.io-index)", + "quote 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)", + "syn 1.0.11 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "prost-types" version = "0.5.0" @@ -5012,7 +5033,7 @@ dependencies = [ "libp2p 0.14.0-alpha.1 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", "parity-scale-codec 1.1.2 (registry+https://github.com/rust-lang/crates.io-index)", - "prost 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)", + "prost 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)", "prost-build 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)", "quickcheck 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)", "rand 0.7.2 (registry+https://github.com/rust-lang/crates.io-index)", @@ -8590,8 +8611,10 @@ dependencies = [ "checksum proc-macro2 0.4.30 (registry+https://github.com/rust-lang/crates.io-index)" = "cf3d2011ab5c909338f7887f4fc896d35932e29146c12c8d01da6b22a80ba759" "checksum proc-macro2 1.0.6 (registry+https://github.com/rust-lang/crates.io-index)" = "9c9e470a8dc4aeae2dee2f335e8f533e2d4b347e1434e5671afc49b054592f27" "checksum prost 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)" = "96d14b1c185652833d24aaad41c5832b0be5616a590227c1fbff57c616754b23" +"checksum prost 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)" = "ce49aefe0a6144a45de32927c77bd2859a5f7677b55f220ae5b744e87389c212" "checksum prost-build 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)" = "eb788126ea840817128183f8f603dce02cb7aea25c2a0b764359d8e20010702e" "checksum prost-derive 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)" = "5e7dc378b94ac374644181a2247cebf59a6ec1c88b49ac77f3a94b86b79d0e11" +"checksum prost-derive 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)" = "537aa19b95acde10a12fec4301466386f757403de4cd4e5b4fa78fb5ecb18f72" "checksum prost-types 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)" = "1de482a366941c8d56d19b650fac09ca08508f2a696119ee7513ad590c8bac6f" "checksum protobuf 2.8.1 (registry+https://github.com/rust-lang/crates.io-index)" = "40361836defdd5871ff7e84096c6f6444af7fc157f8ef1789f54f147687caa20" "checksum pwasm-utils 0.12.0 (registry+https://github.com/rust-lang/crates.io-index)" = "4f7a12f176deee919f4ba55326ee17491c8b707d0987aed822682c821b660192" diff --git a/client/authority-discovery/Cargo.toml b/client/authority-discovery/Cargo.toml index 7381e5add19a1..25c3f161d4018 100644 --- a/client/authority-discovery/Cargo.toml +++ b/client/authority-discovery/Cargo.toml @@ -16,7 +16,7 @@ futures = "0.3.1" futures-timer = "2.0" libp2p = { version = "0.14.0-alpha.1", default-features = false, features = ["secp256k1", "libp2p-websocket"] } log = "0.4.8" -prost = "0.5.0" +prost = "0.6.1" rand = "0.7.2" sc-client-api = { version = "2.0.0", path = "../api" } sc-keystore = { version = "2.0.0", path = "../keystore" } diff --git a/client/authority-discovery/src/lib.rs b/client/authority-discovery/src/lib.rs index ee2b655b925a6..6260ac9a85b12 100644 --- a/client/authority-discovery/src/lib.rs +++ b/client/authority-discovery/src/lib.rs @@ -315,10 +315,10 @@ where let remote_addresses: Vec = values.into_iter() .map(|(_k, v)| { - let schema::SignedAuthorityAddresses { - signature, - addresses, - } = schema::SignedAuthorityAddresses::decode(v).map_err(Error::DecodingProto)?; + let schema::SignedAuthorityAddresses { signature, addresses } = + schema::SignedAuthorityAddresses::decode(v.as_slice()) + .map_err(Error::DecodingProto)?; + let signature = AuthoritySignature::decode(&mut &signature[..]) .map_err(Error::EncodingDecodingScale)?; @@ -326,7 +326,7 @@ where return Err(Error::VerifyingDhtPayload); } - let addresses: Vec = schema::AuthorityAddresses::decode(addresses) + let addresses = schema::AuthorityAddresses::decode(addresses.as_slice()) .map(|a| a.addresses) .map_err(Error::DecodingProto)? .into_iter() From 6c3b86d16ad19751fbbab86e369883b5425c72df Mon Sep 17 00:00:00 2001 From: Nikolay Volf Date: Wed, 22 Jan 2020 05:53:40 -0800 Subject: [PATCH 18/18] More cleanups in node-template (#4705) * remove and reformat * remove some more * commit Cargo.lock --- Cargo.lock | 4 -- bin/node-template/Cargo.toml | 4 -- bin/node-template/src/chain_spec.rs | 62 +++++++++++++++-------------- 3 files changed, 33 insertions(+), 37 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 04a7a33595364..eadf251f759ee 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3221,8 +3221,6 @@ dependencies = [ "futures 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", "node-template-runtime 2.0.0", - "parity-scale-codec 1.1.2 (registry+https://github.com/rust-lang/crates.io-index)", - "parking_lot 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)", "sc-basic-authorship 0.8.0", "sc-cli 0.8.0", "sc-client 0.8.0", @@ -3237,12 +3235,10 @@ dependencies = [ "sp-core 2.0.0", "sp-finality-grandpa 2.0.0", "sp-inherents 2.0.0", - "sp-io 2.0.0", "sp-runtime 2.0.0", "sp-transaction-pool 2.0.0", "substrate-build-script-utils 2.0.0", "tokio 0.2.4 (registry+https://github.com/rust-lang/crates.io-index)", - "trie-root 0.15.2 (registry+https://github.com/rust-lang/crates.io-index)", "vergen 3.0.4 (registry+https://github.com/rust-lang/crates.io-index)", ] diff --git a/bin/node-template/Cargo.toml b/bin/node-template/Cargo.toml index f956bbb22193a..cf3d7509f45d3 100644 --- a/bin/node-template/Cargo.toml +++ b/bin/node-template/Cargo.toml @@ -14,10 +14,6 @@ futures = "0.3.1" ctrlc = { version = "3.1.3", features = ["termination"] } log = "0.4.8" tokio = { version = "0.2", features = ["rt-threaded"] } -parking_lot = "0.9.0" -codec = { package = "parity-scale-codec", version = "1.0.0" } -trie-root = "0.15.2" -sp-io = { version = "2.0.0", path = "../../primitives/io" } sc-cli = { version = "0.8.0", path = "../../client/cli" } sp-core = { version = "2.0.0", path = "../../primitives/core" } sc-executor = { version = "0.8", path = "../../client/executor" } diff --git a/bin/node-template/src/chain_spec.rs b/bin/node-template/src/chain_spec.rs index fae9feaf5113b..6afb67547bd6c 100644 --- a/bin/node-template/src/chain_spec.rs +++ b/bin/node-template/src/chain_spec.rs @@ -56,17 +56,19 @@ impl Alternative { Alternative::Development => ChainSpec::from_genesis( "Development", "dev", - || testnet_genesis(vec![ - get_authority_keys_from_seed("Alice"), - ], - get_account_id_from_seed::("Alice"), - vec![ + || testnet_genesis( + vec![ + get_authority_keys_from_seed("Alice"), + ], get_account_id_from_seed::("Alice"), - get_account_id_from_seed::("Bob"), - get_account_id_from_seed::("Alice//stash"), - get_account_id_from_seed::("Bob//stash"), - ], - true), + vec![ + get_account_id_from_seed::("Alice"), + get_account_id_from_seed::("Bob"), + get_account_id_from_seed::("Alice//stash"), + get_account_id_from_seed::("Bob//stash"), + ], + true, + ), vec![], None, None, @@ -76,26 +78,28 @@ impl Alternative { Alternative::LocalTestnet => ChainSpec::from_genesis( "Local Testnet", "local_testnet", - || testnet_genesis(vec![ - get_authority_keys_from_seed("Alice"), - get_authority_keys_from_seed("Bob"), - ], - get_account_id_from_seed::("Alice"), - vec![ + || testnet_genesis( + vec![ + get_authority_keys_from_seed("Alice"), + get_authority_keys_from_seed("Bob"), + ], get_account_id_from_seed::("Alice"), - get_account_id_from_seed::("Bob"), - get_account_id_from_seed::("Charlie"), - get_account_id_from_seed::("Dave"), - get_account_id_from_seed::("Eve"), - get_account_id_from_seed::("Ferdie"), - get_account_id_from_seed::("Alice//stash"), - get_account_id_from_seed::("Bob//stash"), - get_account_id_from_seed::("Charlie//stash"), - get_account_id_from_seed::("Dave//stash"), - get_account_id_from_seed::("Eve//stash"), - get_account_id_from_seed::("Ferdie//stash"), - ], - true), + vec![ + get_account_id_from_seed::("Alice"), + get_account_id_from_seed::("Bob"), + get_account_id_from_seed::("Charlie"), + get_account_id_from_seed::("Dave"), + get_account_id_from_seed::("Eve"), + get_account_id_from_seed::("Ferdie"), + get_account_id_from_seed::("Alice//stash"), + get_account_id_from_seed::("Bob//stash"), + get_account_id_from_seed::("Charlie//stash"), + get_account_id_from_seed::("Dave//stash"), + get_account_id_from_seed::("Eve//stash"), + get_account_id_from_seed::("Ferdie//stash"), + ], + true, + ), vec![], None, None,