This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Merged
Conversation
pepyakin
added
A0-please_review
DemiMarie-temp
suggested changes
May 13, 2019
| let event_idx = { | ||
| let old_event_count = <EventCount<T>>::get(); | ||
| let new_event_count = match old_event_count.checked_add(1) { | ||
| // We've reached the maximum number of events at this block, just |
There was a problem hiding this comment.
This means that event delivery is unreliable, which could lead to starvation or deadlock in some protocols.
Contributor
Author
There was a problem hiding this comment.
Ah, this is more related to #2491 or even before that, to #2282.
I believe this code is a mere defending practice and will not be triggered. Here is my reasoning: wasm32 is a 32-bit platform. It can have at most 4GB of memory, that is, 2^32 bytes. Considering an element to be u8, by the time we reach this number of elements the memory will already be full.
If you are still concerned by this issue, I think it would be best to move this discussion into a new issue (or at least to the now closed #2282)
pepyakin
force-pushed
the
ser-contract-topic-events
branch
from
e23224e to
fe64849
Compare
pepyakin
removed
the
A3-in_progress
MTDK1
pushed a commit
to bdevux/substrate
that referenced
this pull request
Jul 10, 2019
* Introduce an IndexedEvent * Plumb topics through the Ext interface. * Add topics to ext_deposit_event * Charging for events. * Check the number of topics. * Check for duplicate topics. * Bump API version. * Move derive(*Eq) under test. * Use sorting for finding duplicates.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR builds on top of #2491 and adds an ability for a contract to supply a list of topics along with an event.
ext_deposit_eventnow gets two paramters to supply a buffer with a topic list:topics_ptrandtopics_len. There are some restrictions on this list.Vec<TopicOf<T>>vector.max_event_topics(otherwise, the function traps).There is one exception for 1) - passing
0to thetopics_lenparameter is treated as an empty topics vector. In this case, the value oftopics_ptris ignored. The deduplication check is implemented as two loops: this is ok, as long asmax_event_topicsis small, and doesn't require additional memory.This PR also introduces some fixed gas fee per topic, which is checked before recording the event.
The rest is almost the same, all events are recorded along with their topics and if they made it to the finalization phase, then all the events are deposited with respective topics.