From dbe821c37f9db20919034d96920d29922e79447b Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Fri, 3 May 2019 18:16:35 +0200 Subject: [PATCH 01/18] Sketch of indexed events. --- srml/system/src/lib.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index e3d77438f9c27..d6e6d8742153c 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -213,6 +213,8 @@ pub struct EventRecord { pub phase: Phase, /// The event itself. pub event: E, + + // TODO: Add list of topics here. } decl_event!( @@ -314,6 +316,9 @@ decl_storage! { Digest get(digest): T::Digest; /// Events deposited for the current block. Events get(events): Vec>; + /// Mapping between a topic (represented by T::Hash) and a vector of indices + /// of events in the `>` list. + EventTopics get(event_topics): map T::Hash => Vec; // u32 = EventIndex } add_extra_genesis { config(changes_trie_config): Option; @@ -372,6 +377,21 @@ pub fn ensure_inherent(o: OuterOrigin) -> Result<(), &'s } impl Module { + pub fn deposit_event_indexed(topics: &[T::Hash], event: T::Event) { + // FIXME: Inefficient + let event_idx = Self::events().len() as u32; + + Self::deposit_event(event); + + for topic in topics { + >::mutate(topic, |event_indices| { + // We want to use something like `StorageValue::append` here to avoid + // the same problem we used to have with >. + event_indices.push(event_idx); + }); + } + } + /// Gets the index of extrinsic that is currently executing. pub fn extrinsic_index() -> Option { storage::unhashed::get(well_known_keys::EXTRINSIC_INDEX) @@ -397,6 +417,8 @@ impl Module { >::put(txs_root); >::put(Self::calculate_random()); >::kill(); + + // TODO: >::kill(); } /// Remove temporary "environment" entries in storage. From fcd7f917ccbdeba137e90c7bd9571741bc711290 Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Mon, 6 May 2019 12:55:53 +0200 Subject: [PATCH 02/18] Get EventIndex by holding another variable. --- srml/system/src/lib.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index d6e6d8742153c..3c71badc291f0 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -190,6 +190,9 @@ decl_module! { if >::append(&events).is_err() { let [event] = events; >::put(vec![event]); + >::put(1); + } else { + >::mutate(|event_count| *event_count += 1); } } } @@ -292,6 +295,8 @@ fn hash69 + Default>() -> T { h } +type EventIndex = u32; + decl_storage! { trait Store for Module as System { /// Extrinsics nonce for accounts. @@ -316,9 +321,11 @@ decl_storage! { Digest get(digest): T::Digest; /// Events deposited for the current block. Events get(events): Vec>; + /// The number of events in the `Events` list. + EventCount get(event_count): EventIndex; /// Mapping between a topic (represented by T::Hash) and a vector of indices /// of events in the `>` list. - EventTopics get(event_topics): map T::Hash => Vec; // u32 = EventIndex + EventTopics get(event_topics): map T::Hash => Vec; } add_extra_genesis { config(changes_trie_config): Option; @@ -378,8 +385,7 @@ pub fn ensure_inherent(o: OuterOrigin) -> Result<(), &'s impl Module { pub fn deposit_event_indexed(topics: &[T::Hash], event: T::Event) { - // FIXME: Inefficient - let event_idx = Self::events().len() as u32; + let event_idx = Self::event_count(); Self::deposit_event(event); @@ -417,6 +423,7 @@ impl Module { >::put(txs_root); >::put(Self::calculate_random()); >::kill(); + >::kill(); // TODO: >::kill(); } From 00d354cc30ac352b7a11fa72d12388d5b7a1ce6b Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Mon, 6 May 2019 12:56:06 +0200 Subject: [PATCH 03/18] Add some docs. --- srml/system/src/lib.rs | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index 3c71badc291f0..70359c0e1756f 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -191,8 +191,21 @@ decl_module! { let [event] = events; >::put(vec![event]); >::put(1); + + // TODO: this has an unpleasant effect: from now on `>` + // refers to indexes of wrong events and also might refer to events + // which don't exist in `EventTopics`. } else { - >::mutate(|event_count| *event_count += 1); + >::mutate(|event_count| { + event_count.checked_add(1).expect( + "if `append` returns `Err` in case if we are trying to insert more than + `u32::max_value` events; + if we in this branch then there are less than `u32::max_value` events; + `checked_add(1)` must succeed in this case; + qed + " + ) + }); } } } @@ -295,6 +308,10 @@ fn hash69 + Default>() -> T { h } +/// This type alias represents an index of an event. +/// +/// We use `u32` here because this index is used as index for `Events` +/// which can't contain more than `u32::max_value()` items. type EventIndex = u32; decl_storage! { @@ -449,7 +466,13 @@ impl Module { digest.push(item); } - // > stays to be inspected by the client. + // The following fields + // + // - > + // - > + // - > + // + // stay to be inspected by the client and will be cleared by `Self::initialize`. ::new(number, extrinsics_root, storage_root, parent_hash, digest) } From 94491eb9b8dcce6fb62542c165cd6632d8c83e55 Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Mon, 6 May 2019 13:28:51 +0200 Subject: [PATCH 04/18] Use DoubleMap to store reverse topic index --- srml/system/src/lib.rs | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index 70359c0e1756f..cef9008272e48 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -80,7 +80,10 @@ use primitives::traits::{self, CheckEqual, SimpleArithmetic, SimpleBitOps, Zero, Hash, Member, MaybeDisplay, EnsureOrigin, Digest as DigestT, As, CurrentHeight, BlockNumberToHash, MaybeSerializeDebugButNotDeserialize, MaybeSerializeDebug, StaticLookup}; use substrate_primitives::storage::well_known_keys; -use srml_support::{storage, StorageValue, StorageMap, Parameter, decl_module, decl_event, decl_storage}; +use srml_support::{ + storage, decl_module, decl_event, decl_storage, StorageDoubleMap, StorageValue, + StorageMap, Parameter, +}; use safe_mix::TripletMix; use parity_codec::{Encode, Decode}; @@ -342,7 +345,11 @@ decl_storage! { EventCount get(event_count): EventIndex; /// Mapping between a topic (represented by T::Hash) and a vector of indices /// of events in the `>` list. - EventTopics get(event_topics): map T::Hash => Vec; + /// + /// The key serves no purpose. This field is declared as double_map just for convenience + /// of using `remove_prefix`. + EventTopics get(event_topics): double_map hasher(blake2_256) (), blake2_256(T::Hash) + => Vec; } add_extra_genesis { config(changes_trie_config): Option; @@ -402,16 +409,20 @@ pub fn ensure_inherent(o: OuterOrigin) -> Result<(), &'s impl Module { pub fn deposit_event_indexed(topics: &[T::Hash], event: T::Event) { + // TODO: What are we going to do with duplicates in `topics`? + let event_idx = Self::event_count(); Self::deposit_event(event); for topic in topics { - >::mutate(topic, |event_indices| { - // We want to use something like `StorageValue::append` here to avoid - // the same problem we used to have with >. - event_indices.push(event_idx); - }); + let mut event_indices = >::get(&(), topic); + + // We want to use something like `StorageValue::append` here to avoid + // the same problem we used to have with >. + event_indices.push(event_idx); + + >::insert(&(), topic, event_indices); } } @@ -441,8 +452,7 @@ impl Module { >::put(Self::calculate_random()); >::kill(); >::kill(); - - // TODO: >::kill(); + >::remove_prefix(&()); } /// Remove temporary "environment" entries in storage. From a271f9306fae345a73172e83e00ea75242852721 Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Mon, 6 May 2019 16:38:07 +0200 Subject: [PATCH 05/18] Implement StorageDoubleMap::append --- srml/support/src/lib.rs | 31 +++++++++++++++++++ srml/support/src/storage/mod.rs | 28 +++++++++++++++++ .../support/src/storage/unhashed/generator.rs | 20 ++++++++++++ 3 files changed, 79 insertions(+) diff --git a/srml/support/src/lib.rs b/srml/support/src/lib.rs index 78edc361d5e67..5dafd83257642 100644 --- a/srml/support/src/lib.rs +++ b/srml/support/src/lib.rs @@ -216,6 +216,7 @@ mod tests { pub DataDM config(test_config) build(|_| vec![(15u32, 16u32, 42u64)]): double_map hasher(twox_64_concat) u32, blake2_256(u32) => u64; pub GenericDataDM: double_map T::BlockNumber, twox_128(T::BlockNumber) => T::BlockNumber; pub GenericData2DM: double_map T::BlockNumber, twox_256(T::BlockNumber) => Option; + pub AppendableDM: double_map u32, blake2_256(T::BlockNumber) => Vec; } } @@ -345,6 +346,21 @@ mod tests { assert_eq!(DoubleMap::get(key1, key2+1), 0u64); assert_eq!(DoubleMap::get(key1+1, key2), 4u64); assert_eq!(DoubleMap::get(key1+1, key2+1), 4u64); + + }); + } + + #[test] + fn double_map_append_should_work() { + with_externalities(&mut new_test_ext(), || { + type DoubleMap = AppendableDM; + + let key1 = 17u32; + let key2 = 18u32; + + DoubleMap::insert(key1, key2, vec![1]); + DoubleMap::append(key1, key2, &[2, 3]); + assert_eq!(DoubleMap::get(key1, key2), vec![1, 2, 3]); }); } @@ -431,6 +447,21 @@ mod tests { ), documentation: DecodeDifferent::Encode(&[]), }, + StorageFunctionMetadata { + name: DecodeDifferent::Encode("AppendableDM"), + modifier: StorageFunctionModifier::Default, + ty: StorageFunctionType::DoubleMap{ + hasher: StorageHasher::Blake2_256, + key1: DecodeDifferent::Encode("u32"), + key2: DecodeDifferent::Encode("T::BlockNumber"), + value: DecodeDifferent::Encode("Vec"), + key2_hasher: DecodeDifferent::Encode("blake2_256"), + }, + default: DecodeDifferent::Encode( + DefaultByteGetter(&__GetByteStructGenericData2DM(PhantomData::)) + ), + documentation: DecodeDifferent::Encode(&[]), + }, ]) }; diff --git a/srml/support/src/storage/mod.rs b/srml/support/src/storage/mod.rs index fb23383c9d113..426e480f0c262 100644 --- a/srml/support/src/storage/mod.rs +++ b/srml/support/src/storage/mod.rs @@ -401,6 +401,20 @@ pub trait StorageDoubleMap { KArg1: Borrow, KArg2: Borrow, F: FnOnce(&mut Self::Query) -> R; + + /// Append the given items to the value under the key specified. + /// + /// `V` is required to implement `codec::EncodeAppend`. + fn append( + k1: KArg1, + k2: KArg2, + items: &[I], + ) -> Result<(), &'static str> + where + KArg1: Borrow, + KArg2: Borrow, + I: codec::Encode, + V: EncodeAppend; } impl StorageDoubleMap for U @@ -453,6 +467,20 @@ where { U::mutate(k1.borrow(), k2.borrow(), f, &RuntimeStorage) } + + fn append( + k1: KArg1, + k2: KArg2, + items: &[I], + ) -> Result<(), &'static str> + where + KArg1: Borrow, + KArg2: Borrow, + I: codec::Encode, + V: EncodeAppend, + { + U::append(k1.borrow(), k2.borrow(), items, &RuntimeStorage) + } } /// child storage NOTE could replace unhashed by having only one kind of storage (root being null storage diff --git a/srml/support/src/storage/unhashed/generator.rs b/srml/support/src/storage/unhashed/generator.rs index 25592dce6d944..7e9e5a21993f4 100644 --- a/srml/support/src/storage/unhashed/generator.rs +++ b/srml/support/src/storage/unhashed/generator.rs @@ -150,4 +150,24 @@ pub trait StorageDoubleMap /// Mutate the value under a key. fn mutate R, S: UnhashedStorage>(k1: &K1, k2: &K2, f: F, storage: &S) -> R; + + /// Append the given items to the value under the key specified. + fn append( + k1: &K1, + k2: &K2, + items: &[I], + storage: &S, + ) -> Result<(), &'static str> + where + I: codec::Encode, + V: codec::EncodeAppend, + { + let key = Self::key_for(k1, k2); + let new_val = ::append( + storage.get_raw(&key).unwrap_or_default(), + items, + ).ok_or_else(|| "Could not append given item")?; + storage.put_raw(&key, &new_val); + Ok(()) + } } From cb3eecf03084ad60accdba13de18601bfc84752d Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Mon, 6 May 2019 16:54:43 +0200 Subject: [PATCH 06/18] Use append for EventTopics. --- srml/system/src/lib.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index cef9008272e48..12d211f63147f 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -410,19 +410,15 @@ pub fn ensure_inherent(o: OuterOrigin) -> Result<(), &'s impl Module { pub fn deposit_event_indexed(topics: &[T::Hash], event: T::Event) { // TODO: What are we going to do with duplicates in `topics`? - let event_idx = Self::event_count(); Self::deposit_event(event); for topic in topics { - let mut event_indices = >::get(&(), topic); - - // We want to use something like `StorageValue::append` here to avoid - // the same problem we used to have with >. - event_indices.push(event_idx); - - >::insert(&(), topic, event_indices); + if >::append(&(), topic, &[event_idx]).is_err() { + // Overwrite + >::insert(&(), topic, vec![event_idx]); + } } } From 1eaf378aa34ccc48b17f47af0bf8de8e8a5718d0 Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Mon, 6 May 2019 17:51:15 +0200 Subject: [PATCH 07/18] Refactor. --- srml/system/src/lib.rs | 77 ++++++++++++++++++++++-------------------- 1 file changed, 41 insertions(+), 36 deletions(-) diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index 12d211f63147f..76ce927b4a69f 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -182,34 +182,7 @@ decl_module! { pub struct Module for enum Call where origin: T::Origin { /// Deposits an event into this block's event record. pub fn deposit_event(event: T::Event) { - let extrinsic_index = Self::extrinsic_index(); - let phase = extrinsic_index.map_or(Phase::Finalization, |c| Phase::ApplyExtrinsic(c)); - let event = EventRecord { phase, event }; - - // Appending can only fail if `Events` can not be decoded or - // when we try to insert more than `u32::max_value()` events. - // If one of these conditions is met, we just insert the new event. - let events = [event]; - if >::append(&events).is_err() { - let [event] = events; - >::put(vec![event]); - >::put(1); - - // TODO: this has an unpleasant effect: from now on `>` - // refers to indexes of wrong events and also might refer to events - // which don't exist in `EventTopics`. - } else { - >::mutate(|event_count| { - event_count.checked_add(1).expect( - "if `append` returns `Err` in case if we are trying to insert more than - `u32::max_value` events; - if we in this branch then there are less than `u32::max_value` events; - `checked_add(1)` must succeed in this case; - qed - " - ) - }); - } + Self::deposit_event_indexed(&[], event); } } } @@ -408,17 +381,49 @@ pub fn ensure_inherent(o: OuterOrigin) -> Result<(), &'s } impl Module { + /// Deposit an event with the given topics. pub fn deposit_event_indexed(topics: &[T::Hash], event: T::Event) { - // TODO: What are we going to do with duplicates in `topics`? - let event_idx = Self::event_count(); - - Self::deposit_event(event); + let extrinsic_index = Self::extrinsic_index(); + let phase = extrinsic_index.map_or(Phase::Finalization, |c| Phase::ApplyExtrinsic(c)); + let event = EventRecord { phase, event }; + + let event_idx = match >::mutate(|event_count| + event_count + .checked_add(1) + .map(|new_count| { + let old_count = *event_count; + // Update the event_count storage item and return the old_count as + // the result of this mutation. + *event_count = new_count; + old_count + }) + ) { + // We've reached the maximum number of events at this block, just + // don't do anything and leave the event_count unaltered. + None => return, + Some(old_count) => old_count, + }; + + // Appending can only fail if `Events` can not be decoded or + // when we try to insert more than `u32::max_value()` events. + // + // We perform early return if we've reached the maximum capacity of the event list, + // so `Events` seems to be corrupted. Also, this has happened after the start of execution + // (since the event list is cleared at the block initialization). + // The most sensible thing to do here is to just ignore this event and wait until the + // new block. + if >::append(&[event]).is_err() { + // TODO(reviewers): debug_assert!(false) is tempting here, since we want to be safe in + // the production version but it would be nice to catch these kind of issues at the + // development time. We even have -Cdebug-assertion=y on CI and it seems + // we are building wasm with this flag enabled. + // + // However, there no prior precedents of using it. + return; + } for topic in topics { - if >::append(&(), topic, &[event_idx]).is_err() { - // Overwrite - >::insert(&(), topic, vec![event_idx]); - } + >::append(&(), topic, &[event_idx]); } } From 48ed3b108603ce7d19037b377bf03f645ddaedd9 Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Mon, 6 May 2019 18:01:11 +0200 Subject: [PATCH 08/18] Avoid `mutate` --- srml/system/src/lib.rs | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index 76ce927b4a69f..fd7c0ed5304cf 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -387,21 +387,17 @@ impl Module { let phase = extrinsic_index.map_or(Phase::Finalization, |c| Phase::ApplyExtrinsic(c)); let event = EventRecord { phase, event }; - let event_idx = match >::mutate(|event_count| - event_count - .checked_add(1) - .map(|new_count| { - let old_count = *event_count; - // Update the event_count storage item and return the old_count as - // the result of this mutation. - *event_count = new_count; - old_count - }) - ) { - // We've reached the maximum number of events at this block, just - // don't do anything and leave the event_count unaltered. - None => return, - Some(old_count) => old_count, + // Index of the to be added event. + let event_idx = { + let old_event_count = >::get(); + let new_event_count = match old_event_count.checked_add(1) { + // We've reached the maximum number of events at this block, just + // don't do anything and leave the event_count unaltered. + None => return, + Some(nc) => nc, + }; + >::put(new_event_count); + old_event_count }; // Appending can only fail if `Events` can not be decoded or @@ -423,7 +419,10 @@ impl Module { } for topic in topics { - >::append(&(), topic, &[event_idx]); + // The same applies here. + if >::append(&(), topic, &[event_idx]).is_err() { + return; + } } } From c95455f12a43b9f6b6a9671aac02d8cd146f8e98 Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Mon, 6 May 2019 18:04:03 +0200 Subject: [PATCH 09/18] Docs. --- srml/system/src/lib.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index fd7c0ed5304cf..05ec71593b6d5 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -316,11 +316,11 @@ decl_storage! { Events get(events): Vec>; /// The number of events in the `Events` list. EventCount get(event_count): EventIndex; - /// Mapping between a topic (represented by T::Hash) and a vector of indices + /// Mapping between a topic (represented by T::Hash) and a vector of indexes /// of events in the `>` list. /// - /// The key serves no purpose. This field is declared as double_map just for convenience - /// of using `remove_prefix`. + /// The first key serves no purpose. This field is declared as double_map just + /// for convenience of using `remove_prefix`. EventTopics get(event_topics): double_map hasher(blake2_256) (), blake2_256(T::Hash) => Vec; } @@ -381,7 +381,11 @@ pub fn ensure_inherent(o: OuterOrigin) -> Result<(), &'s } impl Module { - /// Deposit an event with the given topics. + /// Deposits an event into this block's event record adding this event + /// to the corresponding topic indexes. + /// + /// This will update storage entries that correpond to the specified topics. + /// It is expected that light-clients could subscribe to this topics. pub fn deposit_event_indexed(topics: &[T::Hash], event: T::Event) { let extrinsic_index = Self::extrinsic_index(); let phase = extrinsic_index.map_or(Phase::Finalization, |c| Phase::ApplyExtrinsic(c)); From aa2c29bd308702bc125bb0021bcc2d622a452170 Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Mon, 6 May 2019 18:19:01 +0200 Subject: [PATCH 10/18] Add topics to EventRecord --- srml/system/src/lib.rs | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index 05ec71593b6d5..1b74e5971ca46 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -200,13 +200,13 @@ pub enum Phase { /// Record of an event happening. #[derive(Encode, Decode)] #[cfg_attr(feature = "std", derive(Serialize, PartialEq, Eq, Clone, Debug))] -pub struct EventRecord { +pub struct EventRecord { /// The phase of the block it happened in. pub phase: Phase, /// The event itself. pub event: E, - - // TODO: Add list of topics here. + /// The list of the topics this event has. + pub topics: Vec, } decl_event!( @@ -313,7 +313,7 @@ decl_storage! { /// Digest of the current block, also part of the block header. Digest get(digest): T::Digest; /// Events deposited for the current block. - Events get(events): Vec>; + Events get(events): Vec>; /// The number of events in the `Events` list. EventCount get(event_count): EventIndex; /// Mapping between a topic (represented by T::Hash) and a vector of indexes @@ -389,7 +389,11 @@ impl Module { pub fn deposit_event_indexed(topics: &[T::Hash], event: T::Event) { let extrinsic_index = Self::extrinsic_index(); let phase = extrinsic_index.map_or(Phase::Finalization, |c| Phase::ApplyExtrinsic(c)); - let event = EventRecord { phase, event }; + let event = EventRecord { + phase, + event, + topics: topics.iter().cloned().collect::>(), + }; // Index of the to be added event. let event_idx = { @@ -675,7 +679,13 @@ mod tests { System::finalize(); assert_eq!( System::events(), - vec![EventRecord { phase: Phase::Finalization, event: 1u16 }] + vec![ + EventRecord { + phase: Phase::Finalization, + event: 1u16, + topics: vec![], + } + ] ); System::initialize(&2, &[0u8; 32].into(), &[0u8; 32].into()); @@ -686,10 +696,10 @@ mod tests { System::deposit_event(3u16); System::finalize(); assert_eq!(System::events(), vec![ - EventRecord { phase: Phase::ApplyExtrinsic(0), event: 42u16 }, - EventRecord { phase: Phase::ApplyExtrinsic(0), event: 100u16 }, - EventRecord { phase: Phase::ApplyExtrinsic(1), event: 101u16 }, - EventRecord { phase: Phase::Finalization, event: 3u16 } + EventRecord { phase: Phase::ApplyExtrinsic(0), event: 42u16, topics: vec![] }, + EventRecord { phase: Phase::ApplyExtrinsic(0), event: 100u16, topics: vec![] }, + EventRecord { phase: Phase::ApplyExtrinsic(1), event: 101u16, topics: vec![] }, + EventRecord { phase: Phase::Finalization, event: 3u16, topics: vec![] } ]); }); } From a614a66b34bdf8a1cf41104a76ee99b66e129474 Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Mon, 6 May 2019 18:42:17 +0200 Subject: [PATCH 11/18] Update tests. --- node/executor/src/lib.rs | 45 ++++++++++++++++++++++++------------- srml/contract/src/tests.rs | 38 ++++++++++++++++++++++--------- srml/council/src/motions.rs | 30 ++++++++++++++++--------- srml/grandpa/src/tests.rs | 2 ++ 4 files changed, 80 insertions(+), 35 deletions(-) diff --git a/node/executor/src/lib.rs b/node/executor/src/lib.rs index 9fe26123ff43d..23c0f826d733d 100644 --- a/node/executor/src/lib.rs +++ b/node/executor/src/lib.rs @@ -488,7 +488,8 @@ mod tests { assert_eq!(System::events(), vec![ EventRecord { phase: Phase::ApplyExtrinsic(0), - event: Event::system(system::Event::ExtrinsicSuccess) + event: Event::system(system::Event::ExtrinsicSuccess), + topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(1), @@ -497,23 +498,28 @@ mod tests { bob().into(), 69, 0 - )) + )), + topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(1), - event: Event::system(system::Event::ExtrinsicSuccess) + event: Event::system(system::Event::ExtrinsicSuccess), + topics: vec![], }, EventRecord { phase: Phase::Finalization, - event: Event::treasury(treasury::RawEvent::Spending(0)) + event: Event::treasury(treasury::RawEvent::Spending(0)), + topics: vec![], }, EventRecord { phase: Phase::Finalization, - event: Event::treasury(treasury::RawEvent::Burnt(0)) + event: Event::treasury(treasury::RawEvent::Burnt(0)), + topics: vec![], }, EventRecord { phase: Phase::Finalization, - event: Event::treasury(treasury::RawEvent::Rollover(0)) + event: Event::treasury(treasury::RawEvent::Rollover(0)), + topics: vec![], }, ]); }); @@ -535,7 +541,8 @@ mod tests { assert_eq!(System::events(), vec![ EventRecord { phase: Phase::ApplyExtrinsic(0), - event: Event::system(system::Event::ExtrinsicSuccess) + event: Event::system(system::Event::ExtrinsicSuccess), + topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(1), @@ -546,11 +553,13 @@ mod tests { 5, 0 ) - ) + ), + topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(1), - event: Event::system(system::Event::ExtrinsicSuccess) + event: Event::system(system::Event::ExtrinsicSuccess), + topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(2), @@ -561,27 +570,33 @@ mod tests { 15, 0 ) - ) + ), + topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(2), - event: Event::system(system::Event::ExtrinsicSuccess) + event: Event::system(system::Event::ExtrinsicSuccess), + topics: vec![], }, EventRecord { phase: Phase::Finalization, - event: Event::session(session::RawEvent::NewSession(1)) + event: Event::session(session::RawEvent::NewSession(1)), + topics: vec![], }, EventRecord { phase: Phase::Finalization, - event: Event::treasury(treasury::RawEvent::Spending(0)) + event: Event::treasury(treasury::RawEvent::Spending(0)), + topics: vec![], }, EventRecord { phase: Phase::Finalization, - event: Event::treasury(treasury::RawEvent::Burnt(0)) + event: Event::treasury(treasury::RawEvent::Burnt(0)), + topics: vec![], }, EventRecord { phase: Phase::Finalization, - event: Event::treasury(treasury::RawEvent::Rollover(0)) + event: Event::treasury(treasury::RawEvent::Rollover(0)), + topics: vec![], }, ]); }); diff --git a/srml/contract/src/tests.rs b/srml/contract/src/tests.rs index e18a99b8327bb..5bff4f301e734 100644 --- a/srml/contract/src/tests.rs +++ b/srml/contract/src/tests.rs @@ -363,28 +363,34 @@ fn instantiate_and_call_and_deposit_event() { EventRecord { phase: Phase::ApplyExtrinsic(0), event: MetaEvent::balances(balances::RawEvent::NewAccount(1, 1_000_000)), + topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(0), event: MetaEvent::contract(RawEvent::CodeStored(HASH_RETURN_FROM_START_FN.into())), + topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(0), event: MetaEvent::balances( balances::RawEvent::NewAccount(BOB, 100) - ) + ), + topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(0), - event: MetaEvent::contract(RawEvent::Transfer(ALICE, BOB, 100)) + event: MetaEvent::contract(RawEvent::Transfer(ALICE, BOB, 100)), + topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(0), - event: MetaEvent::contract(RawEvent::Contract(BOB, vec![1, 2, 3, 4])) + event: MetaEvent::contract(RawEvent::Contract(BOB, vec![1, 2, 3, 4])), + topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(0), - event: MetaEvent::contract(RawEvent::Instantiated(ALICE, BOB)) + event: MetaEvent::contract(RawEvent::Instantiated(ALICE, BOB)), + topics: vec![], } ]); @@ -434,10 +440,12 @@ fn dispatch_call() { EventRecord { phase: Phase::ApplyExtrinsic(0), event: MetaEvent::balances(balances::RawEvent::NewAccount(1, 1_000_000)), + topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(0), event: MetaEvent::contract(RawEvent::CodeStored(HASH_DISPATCH_CALL.into())), + topics: vec![], }, ]); @@ -461,24 +469,29 @@ fn dispatch_call() { EventRecord { phase: Phase::ApplyExtrinsic(0), event: MetaEvent::balances(balances::RawEvent::NewAccount(1, 1_000_000)), + topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(0), event: MetaEvent::contract(RawEvent::CodeStored(HASH_DISPATCH_CALL.into())), + topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(0), event: MetaEvent::balances( balances::RawEvent::NewAccount(BOB, 100) - ) + ), + topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(0), - event: MetaEvent::contract(RawEvent::Transfer(ALICE, BOB, 100)) + event: MetaEvent::contract(RawEvent::Transfer(ALICE, BOB, 100)), + topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(0), - event: MetaEvent::contract(RawEvent::Instantiated(ALICE, BOB)) + event: MetaEvent::contract(RawEvent::Instantiated(ALICE, BOB)), + topics: vec![], }, // Dispatching the call. @@ -486,19 +499,22 @@ fn dispatch_call() { phase: Phase::ApplyExtrinsic(0), event: MetaEvent::balances( balances::RawEvent::NewAccount(CHARLIE, 50) - ) + ), + topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(0), event: MetaEvent::balances( balances::RawEvent::Transfer(BOB, CHARLIE, 50, 0) - ) + ), + topics: vec![], }, // Event emited as a result of dispatch. EventRecord { phase: Phase::ApplyExtrinsic(0), - event: MetaEvent::contract(RawEvent::Dispatched(BOB, true)) + event: MetaEvent::contract(RawEvent::Dispatched(BOB, true)), + topics: vec![], } ]); }, @@ -645,10 +661,12 @@ fn set_rent_hash_and_code() { EventRecord { phase: Phase::ApplyExtrinsic(0), event: MetaEvent::balances(balances::RawEvent::NewAccount(1, 1_000_000)), + topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(0), event: MetaEvent::contract(RawEvent::CodeStored(HASH_SET_RENT.into())), + topics: vec![], }, ]); } diff --git a/srml/council/src/motions.rs b/srml/council/src/motions.rs index 393ebce4f890d..e560fadb9c5a4 100644 --- a/srml/council/src/motions.rs +++ b/srml/council/src/motions.rs @@ -234,7 +234,8 @@ mod tests { assert_eq!(System::events(), vec![ EventRecord { phase: Phase::ApplyExtrinsic(0), - event: OuterEvent::motions(RawEvent::Proposed(1, 0, hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), 3)) + event: OuterEvent::motions(RawEvent::Proposed(1, 0, hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), 3)), + topics: vec![], } ]); }); @@ -287,11 +288,13 @@ mod tests { assert_eq!(System::events(), vec![ EventRecord { phase: Phase::ApplyExtrinsic(0), - event: OuterEvent::motions(RawEvent::Proposed(1, 0, hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), 2)) + event: OuterEvent::motions(RawEvent::Proposed(1, 0, hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), 2)), + topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(0), - event: OuterEvent::motions(RawEvent::Voted(1, hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), false, 0, 1)) + event: OuterEvent::motions(RawEvent::Voted(1, hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), false, 0, 1)), + topics: vec![], } ]); }); @@ -309,15 +312,18 @@ mod tests { assert_eq!(System::events(), vec![ EventRecord { phase: Phase::ApplyExtrinsic(0), - event: OuterEvent::motions(RawEvent::Proposed(1, 0, hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), 3)) + event: OuterEvent::motions(RawEvent::Proposed(1, 0, hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), 3)), + topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(0), - event: OuterEvent::motions(RawEvent::Voted(2, hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), false, 1, 1)) + event: OuterEvent::motions(RawEvent::Voted(2, hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), false, 1, 1)), + topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(0), - event: OuterEvent::motions(RawEvent::Disapproved(hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into())) + event: OuterEvent::motions(RawEvent::Disapproved(hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into())), + topics: vec![], } ]); }); @@ -335,19 +341,23 @@ mod tests { assert_eq!(System::events(), vec![ EventRecord { phase: Phase::ApplyExtrinsic(0), - event: OuterEvent::motions(RawEvent::Proposed(1, 0, hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), 2)) + event: OuterEvent::motions(RawEvent::Proposed(1, 0, hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), 2)), + topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(0), - event: OuterEvent::motions(RawEvent::Voted(2, hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), true, 2, 0)) + event: OuterEvent::motions(RawEvent::Voted(2, hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), true, 2, 0)), + topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(0), - event: OuterEvent::motions(RawEvent::Approved(hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into())) + event: OuterEvent::motions(RawEvent::Approved(hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into())), + topics: vec![], }, EventRecord { phase: Phase::ApplyExtrinsic(0), - event: OuterEvent::motions(RawEvent::Executed(hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), false)) + event: OuterEvent::motions(RawEvent::Executed(hex!["cd0b662a49f004093b80600415cf4126399af0d27ed6c185abeb1469c17eb5bf"].into(), false)), + topics: vec![], } ]); }); diff --git a/srml/grandpa/src/tests.rs b/srml/grandpa/src/tests.rs index 3050b6a572e0d..560766f8c62ad 100644 --- a/srml/grandpa/src/tests.rs +++ b/srml/grandpa/src/tests.rs @@ -47,6 +47,7 @@ fn authorities_change_logged() { EventRecord { phase: Phase::Finalization, event: RawEvent::NewAuthorities(vec![(4, 1), (5, 1), (6, 1)]).into(), + topics: vec![], }, ]); }); @@ -77,6 +78,7 @@ fn authorities_change_logged_after_delay() { EventRecord { phase: Phase::Finalization, event: RawEvent::NewAuthorities(vec![(4, 1), (5, 1), (6, 1)]).into(), + topics: vec![], }, ]); }); From 98035b7c88d5984df7575280ab8a53bdb6bb7baa Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Mon, 6 May 2019 18:42:26 +0200 Subject: [PATCH 12/18] Rebuild. --- core/test-runtime/wasm/Cargo.lock | 2 +- node-template/runtime/wasm/Cargo.lock | 2 +- node/runtime/wasm/Cargo.lock | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/test-runtime/wasm/Cargo.lock b/core/test-runtime/wasm/Cargo.lock index c6510ab383669..559db5bc253e4 100644 --- a/core/test-runtime/wasm/Cargo.lock +++ b/core/test-runtime/wasm/Cargo.lock @@ -2457,7 +2457,7 @@ dependencies = [ "sr-version 1.0.0", "substrate-inherents 1.0.0", "substrate-primitives 1.0.0", - "tokio 0.1.16 (registry+https://github.com/rust-lang/crates.io-index)", + "tokio-timer 0.2.10 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] diff --git a/node-template/runtime/wasm/Cargo.lock b/node-template/runtime/wasm/Cargo.lock index 221aa72729797..c3d4aa27a3ccb 100644 --- a/node-template/runtime/wasm/Cargo.lock +++ b/node-template/runtime/wasm/Cargo.lock @@ -2599,7 +2599,7 @@ dependencies = [ "sr-version 1.0.0", "substrate-inherents 1.0.0", "substrate-primitives 1.0.0", - "tokio 0.1.16 (registry+https://github.com/rust-lang/crates.io-index)", + "tokio-timer 0.2.10 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] diff --git a/node/runtime/wasm/Cargo.lock b/node/runtime/wasm/Cargo.lock index c5f7018fcc7b5..abdb866d2c5e7 100644 --- a/node/runtime/wasm/Cargo.lock +++ b/node/runtime/wasm/Cargo.lock @@ -2734,7 +2734,7 @@ dependencies = [ "sr-version 1.0.0", "substrate-inherents 1.0.0", "substrate-primitives 1.0.0", - "tokio 0.1.16 (registry+https://github.com/rust-lang/crates.io-index)", + "tokio-timer 0.2.10 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] From e92e39d7b16233e8a64233739350946b40ba8f65 Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Mon, 6 May 2019 19:41:11 +0200 Subject: [PATCH 13/18] Bump version. --- node/runtime/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index 65eb3b8f800c0..dbd0e09e20bd4 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -58,8 +58,8 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("node"), impl_name: create_runtime_str!("substrate-node"), authoring_version: 10, - spec_version: 69, - impl_version: 70, + spec_version: 70, + impl_version: 71, apis: RUNTIME_API_VERSIONS, }; From 28d5520a3950f8bd7af6d58d5afe7ae6ac52a566 Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Wed, 8 May 2019 16:52:58 +0200 Subject: [PATCH 14/18] Event topics test. --- srml/system/src/lib.rs | 58 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index 1b74e5971ca46..0452b99ae1893 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -703,4 +703,62 @@ mod tests { ]); }); } + + #[test] + fn deposit_event_topics() { + with_externalities(&mut new_test_ext(), || { + System::initialize(&1, &[0u8; 32].into(), &[0u8; 32].into()); + System::note_finished_extrinsics(); + + let topics = vec![ + H256::repeat_byte(1), + H256::repeat_byte(2), + H256::repeat_byte(3), + ]; + + // We deposit a few events with different sets of topics. + System::deposit_event_indexed(&topics[0..3], 1u16); + System::deposit_event_indexed(&topics[0..1], 2u16); + System::deposit_event_indexed(&topics[1..2], 3u16); + + System::finalize(); + + // Check that topics are reflected in the event record. + assert_eq!( + System::events(), + vec![ + EventRecord { + phase: Phase::Finalization, + event: 1u16, + topics: topics[0..3].to_vec(), + }, + EventRecord { + phase: Phase::Finalization, + event: 2u16, + topics: topics[0..1].to_vec(), + }, + EventRecord { + phase: Phase::Finalization, + event: 3u16, + topics: topics[1..2].to_vec(), + } + ] + ); + + // Check that the topic-events mapping reflects the deposited topics. + // Note that these are indexes of the events. + assert_eq!( + System::event_topics(&(), &topics[0]), + vec![0, 1], + ); + assert_eq!( + System::event_topics(&(), &topics[1]), + vec![0, 2], + ); + assert_eq!( + System::event_topics(&(), &topics[2]), + vec![0], + ); + }); + } } From 5f9ca06de6471b48e6ea84ebf2e3cb9c48cff122 Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Thu, 9 May 2019 17:00:53 +0200 Subject: [PATCH 15/18] Mix in BlockNumber to distinguish updates --- srml/system/src/lib.rs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index df223e225292e..23f55d8c03920 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -322,8 +322,20 @@ decl_storage! { /// /// The first key serves no purpose. This field is declared as double_map just /// for convenience of using `remove_prefix`. + /// + /// All topic vectors have deterministic storage locations depending on the topic. This + /// allows light-clients to leverage the changes trie storage tracking mechanism and + /// in case of changes fetch the list of events of interest. + /// + /// The value has the type `(T::BlockNumber, EventIndex)` because if we used only just + /// the `EventIndex` then in case if the topic has the same contents on the next block + /// no notification will be triggered thus the event might be lost. + /// + /// Possibly, we can improve it by using something like: + /// `Option<(BlockNumber, Vec)>`, however in this case we won't be able to use + /// `EventTopics::append`. EventTopics get(event_topics): double_map hasher(blake2_256) (), blake2_256(T::Hash) - => Vec; + => Vec<(T::BlockNumber, EventIndex)>; } add_extra_genesis { config(changes_trie_config): Option; @@ -427,9 +439,10 @@ impl Module { return; } + let block_no = Self::block_number(); for topic in topics { // The same applies here. - if >::append(&(), topic, &[event_idx]).is_err() { + if >::append(&(), topic, &[(block_no, event_idx)]).is_err() { return; } } From 24b944f871843acac07d1da0a4417bae01036200 Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Fri, 10 May 2019 15:43:15 +0200 Subject: [PATCH 16/18] Fix srml-system test. --- srml/system/src/lib.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index 23f55d8c03920..d38c1d9eed70b 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -755,7 +755,9 @@ mod tests { #[test] fn deposit_event_topics() { with_externalities(&mut new_test_ext(), || { - System::initialize(&1, &[0u8; 32].into(), &[0u8; 32].into()); + const BLOCK_NUMBER: u64 = 1; + + System::initialize(&BLOCK_NUMBER, &[0u8; 32].into(), &[0u8; 32].into()); System::note_finished_extrinsics(); let topics = vec![ @@ -797,15 +799,15 @@ mod tests { // Note that these are indexes of the events. assert_eq!( System::event_topics(&(), &topics[0]), - vec![0, 1], + vec![(BLOCK_NUMBER, 0), (BLOCK_NUMBER, 1)], ); assert_eq!( System::event_topics(&(), &topics[1]), - vec![0, 2], + vec![(BLOCK_NUMBER, 0), (BLOCK_NUMBER, 2)], ); assert_eq!( System::event_topics(&(), &topics[2]), - vec![0], + vec![(BLOCK_NUMBER, 0)], ); }); } From 40b296ec8ee55dfdeeb3081f969fd1cd7ea9f5b4 Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Fri, 10 May 2019 17:23:06 +0200 Subject: [PATCH 17/18] Post merge fixes. --- node/executor/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/node/executor/src/lib.rs b/node/executor/src/lib.rs index 3a40e0c423bad..c6903529c4cd1 100644 --- a/node/executor/src/lib.rs +++ b/node/executor/src/lib.rs @@ -582,7 +582,7 @@ mod tests { }, EventRecord { phase: Phase::Finalization, - event: Event::treasury(treasury::RawEvent::Spending(0)) + event: Event::treasury(treasury::RawEvent::Spending(0)), topics: vec![], }, EventRecord { @@ -597,7 +597,8 @@ mod tests { }, EventRecord { phase: Phase::Finalization, - event: Event::session(session::RawEvent::NewSession(1)) + event: Event::session(session::RawEvent::NewSession(1)), + topics: vec![], }, ]); }); From d6bfa29983e9ca8523c2afb9f5e842f53c19bb1e Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Mon, 13 May 2019 12:02:55 +0200 Subject: [PATCH 18/18] Comments/TODO. --- srml/system/src/lib.rs | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index a1e5195fd1ef8..b009785011fb6 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -322,6 +322,12 @@ decl_storage! { Events get(events): Vec>; /// The number of events in the `Events` list. EventCount get(event_count): EventIndex; + + // TODO: https://github.com/paritytech/substrate/issues/2553 + // Possibly, we can improve it by using something like: + // `Option<(BlockNumber, Vec)>`, however in this case we won't be able to use + // `EventTopics::append`. + /// Mapping between a topic (represented by T::Hash) and a vector of indexes /// of events in the `>` list. /// @@ -335,10 +341,6 @@ decl_storage! { /// The value has the type `(T::BlockNumber, EventIndex)` because if we used only just /// the `EventIndex` then in case if the topic has the same contents on the next block /// no notification will be triggered thus the event might be lost. - /// - /// Possibly, we can improve it by using something like: - /// `Option<(BlockNumber, Vec)>`, however in this case we won't be able to use - /// `EventTopics::append`. EventTopics get(event_topics): double_map hasher(blake2_256) (), blake2_256(T::Hash) => Vec<(T::BlockNumber, EventIndex)>; } @@ -432,15 +434,9 @@ impl Module { // We perform early return if we've reached the maximum capacity of the event list, // so `Events` seems to be corrupted. Also, this has happened after the start of execution // (since the event list is cleared at the block initialization). - // The most sensible thing to do here is to just ignore this event and wait until the - // new block. if >::append(&[event]).is_err() { - // TODO(reviewers): debug_assert!(false) is tempting here, since we want to be safe in - // the production version but it would be nice to catch these kind of issues at the - // development time. We even have -Cdebug-assertion=y on CI and it seems - // we are building wasm with this flag enabled. - // - // However, there no prior precedents of using it. + // The most sensible thing to do here is to just ignore this event and wait until the + // new block. return; }