Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Use single map and remove_all for EventTopics #4566

Merged
merged 1 commit into from
Jan 8, 2020
Merged

Use single map and remove_all for EventTopics #4566

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 6 additions & 10 deletions frame/system/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,18 +405,14 @@ decl_storage! {
/// Mapping between a topic (represented by T::Hash) and a vector of indexes
/// of events in the `<Events<T>>` list.
///
/// 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.
EventTopics get(fn event_topics): double_map hasher(blake2_256) (), blake2_256(T::Hash)
=> Vec<(T::BlockNumber, EventIndex)>;
EventTopics get(fn event_topics): map T::Hash => Vec<(T::BlockNumber, EventIndex)>;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could reduce the complexity of the hasher used on the key here.

I don't quite understand if this is something worth changing.

}
add_extra_genesis {
config(changes_trie_config): Option<ChangesTrieConfiguration>;
Expand Down Expand Up @@ -583,7 +579,7 @@ impl<T: Trait> Module<T> {
let block_no = Self::block_number();
for topic in topics {
// The same applies here.
if <EventTopics<T>>::append(&(), topic, &[(block_no, event_idx)]).is_err() {
if <EventTopics<T>>::append(topic, &[(block_no, event_idx)]).is_err() {
return;
}
}
Expand Down Expand Up @@ -647,7 +643,7 @@ impl<T: Trait> Module<T> {
<ExtrinsicsRoot<T>>::put(txs_root);
<Events<T>>::kill();
EventCount::kill();
<EventTopics<T>>::remove_prefix(&());
<EventTopics<T>>::remove_all();
}

/// Remove temporary "environment" entries in storage.
Expand Down Expand Up @@ -1302,15 +1298,15 @@ mod tests {
// 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]),
System::event_topics(&topics[0]),
vec![(BLOCK_NUMBER, 0), (BLOCK_NUMBER, 1)],
);
assert_eq!(
System::event_topics(&(), &topics[1]),
System::event_topics(&topics[1]),
vec![(BLOCK_NUMBER, 0), (BLOCK_NUMBER, 2)],
);
assert_eq!(
System::event_topics(&(), &topics[2]),
System::event_topics(&topics[2]),
vec![(BLOCK_NUMBER, 0)],
);
});
Expand Down