Clarifying docs on event storage by ChrisChinchilla · Pull Request #4952 · argotorg/solidity

ChrisChinchilla · 2018-09-12T13:13:43Z

Checklist

All tests are passing
README / documentation was extended, if necessary
Used meaningful commit messages

Description

Closes #544 in an attempt to make docs clearer around how event data is stored.

ChrisChinchilla · 2018-09-12T13:42:29Z

My remaining question from the original issue discussion is around retrieval. And there are a lot of SO threads on the same topic, so I'd love to get this clear.

I'd like to include an example of a topic structure and then update the JavaScript example to show retrieving an actual (non-hashed) value, and/or link to an example in the web3.js docs if one exists.

Around that example I will add something like what @chriseth said:

The low-level effect of calling an event is that a so-called log is created. Indexed arguments are stored in the "topic" field of a log. This field is put inside a bloom filter (a special kind of database index) which in turn means that it is rather fast to search for the value if you know it, but it is impossible to retrieve the value. Non-indexed argumenst are stored in the data part of the log which can be retrieved later. In addition to that, strings and bytes are stored by hash inside the topic field.

So that people understand what's happening behind the scenes.

codecov · 2018-09-12T13:58:04Z

Codecov Report

Merging #4952 into develop will decrease coverage by 0.11%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #4952      +/-   ##
===========================================
- Coverage    88.02%   87.91%   -0.12%     
===========================================
  Files          314      314              
  Lines        31782    31657     -125     
  Branches      3748     3730      -18     
===========================================
- Hits         27976    27830     -146     
- Misses        2537     2569      +32     
+ Partials      1269     1258      -11

Flag	Coverage Δ
#all	`87.91% <ø> (-0.12%)`	⬇️
#syntax	`28.51% <ø> (-0.27%)`	⬇️

mancze

There are still places which are a little confusing. I think that there is not enough emphasis that indexed does not mean "store and additionally index" but "do not store, but use as index". I've marked places I think are ambiguous.

mancze · 2018-09-13T13:37:24Z


-Events are
-inheritable members of contracts. When they are called, they cause the
+Events are inheritable members of contracts. When you call them, they cause the


...they cause the arguments to be stored in the transaction's log...

I think there should be clear statement right at this place, that indexed arguments are stored in another structure (topics) than non-indexed arguments. The wording should clearly state that storage of the arguments depends on their indexed flag. I believe typical assumption about indexes is that they actually duplicate stored data in index structures, not that data are removed from the actual storage and moved into index structure which is the only place they exist.

I would rephrase to:

...they cause the log entry to be stored in the transaction's log...

And later how are arguments stored within log entry.

mancze · 2018-09-13T13:40:14Z

+in the blockchain. These logs are associated with the address of the contract,
+are incorporated into the blockchain, and stay there as long as a block is
+accessible (forever as of the Frontier and Homestead releases, but this might
+change with Serenity). Log and event data is not accessible from within


...Log and event data is not accessible from within...

Event data are part of the log entry, conjunction "and" seems a little confusing (like event data and log are two separate things). Perhaps:

...Log with its event data is not accessible from within...

mancze · 2018-09-13T13:47:25Z

-Keccak-256 hash of it is stored as topic instead. This is because a topic
-can only hold a single word (32 bytes).
+Up to three parameters can receive the attribute ``indexed`` which stores the
+respective arguments in a special data structure known as :ref:`"topics" <events_topics>`. If you use arrays (including ``string`` and ``bytes``) as


...which stores the respective arguments in a special data structure known as "topics".

Emphasize that they are stored in topics exclusively:

...which stores the respective arguments in a special data structure known as "topics" instead of actual event data.

mancze · 2018-09-13T13:52:15Z


-All non-indexed arguments will be :ref:`ABI-encoded <ABI>` into the data part of the log.
+All non-indexed arguments are :ref:`ABI-encoded <ABI>` into the data part of
+the log.


Finally put a link to abi-spec.html#events. It's very nicely explained there. Including pros and cons of the indexed arguments and the consequences. I actually just read it because I didn't know about it.

For more details how are event calls stored please see :ref:ABI log entry specification.

ChrisChinchilla · 2018-09-14T08:44:52Z

@mancze I clarified a couple of things, and actually, your comments helped me understand even more. I didn't add the link to abi-spec.html#events as you suggested, as it's already added a few lines above.

Do you think the examples need updating, or are they OK as is?

mancze · 2018-09-14T11:29:51Z

@ChrisChinchilla Example seem fine, yet in its JavaScript part there is this part:

// result will contain various information
// including the arguments given to the Deposit
// call.

I would prefer to use "non-indexed arguments" and possibly add "and topics".

In addition one might wonder what various information actually is. It would be nice to have actual log output placed as a comment (leaving out irrelevant properties).

ChrisChinchilla · 2018-09-17T12:27:47Z

OK @mancze I added some of your comments from above, hopefully, it enough. I was conscious of this doc including too much web3 code as that can get out of date. If you like this I'll get it reviewed and hopefully merged.

chriseth · 2018-09-17T16:19:02Z

+that the log actually exists inside the blockchain.
+
+.. note::
+  You have to supply block headers because the contract can only see the last


This is just a minor note for the last sentence in the paragraph, I don't think it should stand out that much.

chriseth · 2018-09-17T16:19:20Z

+  You have to supply block headers because the contract can only see the last
+  256 block hashes.
+
+You can add the attribute ``indexed`` to up to three parameters which instead adds them


instead of what?

chriseth · 2018-09-17T16:20:05Z

    }

-The use in the JavaScript API would be as follows:
+Then use in the JavaScript API is as follows:


chriseth · 2018-09-17T16:21:40Z

I think this should also clarify better what can be done with topics and what can not be done with topics. @ChrisChinchilla please ask Andrei to clarify.

ChrisChinchilla · 2018-09-20T06:34:53Z

OK @chriseth agreed. What is Andrei's GitHub/Gitter handle?

chriseth · 2018-09-20T10:04:45Z

@gumb0

gumb0 · 2018-09-20T10:45:36Z

Basically the logs can be searched by their topics and you can receive notifications about the new logs with chosen topics, not sure what else I can tell.

chriseth · 2018-09-20T18:54:00Z

@gumb0 basically the question is whether it is impossible to retrieve the topic (for example because only its hash is stored on because it is only stored in a bloom filter together with other values) or ont.

gumb0 · 2018-09-21T09:07:53Z

It is possible, topics are stored, bloom filters are just additional data structure to make filtering fast.
See https://github.com/ethereum/wiki/wiki/JSON-RPC#returns-42 - eth_getFilterChanges returns array of topics.

ChrisChinchilla · 2018-09-21T10:33:13Z

Thanks @gumb0 I think i'm starting to understand, but any chance you can link a code example maybe? Might help me fully understand :)

gumb0 · 2018-09-21T10:55:18Z

@ChrisChinchilla I don't have any dapp code to show, but I have this link that explains a lot about events I think https://blog.qtum.org/how-solidity-events-are-implemented-diving-into-the-ethereum-vm-part-6-30e07b3037b9

ChrisChinchilla · 2018-09-24T11:35:55Z

@gumb0 I tried to incorporate the sections I thought were relevant of that (excellent) post into the docs to cover, mostly regarding filtering with a topic. It ended up being a small change, but could you see if what I added makes sense to you or not? And anything else you found that you think is worth mentioning?

chriseth · 2018-09-24T13:05:02Z

+because the contract can only see the last 256 block hashes.
+
+You can add the attribute ``indexed`` to up to three parameters which adds them
+to a special data structure known as :ref:`"topics" <events_topics>`. If you use


As @mancze says, I think we should at least mention something like " instead of the data part of the log" here.

chriseth · 2018-09-24T13:05:47Z

+
+For example, the code below uses the JSON RPC ``eth_getFilterChanges``
+`method <https://github.com/ethereum/wiki/wiki/JSON-RPC#returns-42>`_ to filter
+logs that match a topic with a certain address value:


I think this is still too cryptic. Also, it should not use ... unless it is clear that those are not part of the actual string.

OK…

I see … used in a lot of place in the solidity docs, are you saying that all those instances are wrong, or is there a subtle difference of when to use it? It's fairly standard syntax to use, but I admit, I have never used it in the middle of a string before. But, other examples in the docs do. A placeholder might be a more readable solution.

I need a little help here, what do you mean by 'cryptic'? Maybe to add an example of logs the filter returns?

the example does not use json-rpc. It is just a snippet of the payload of a json-rpc request. It misses the function name and other things. If we do this, we should explain json-rpc a bit more, but I think it would be better to use web3.js instead.

Why not provide the full string? Also, I don't think we have used the single-character unicode ... before in these docs.

Here's the relevant part of web3.js docs I believe https://web3js.readthedocs.io/en/1.0/web3-eth-subscribe.html#subscribe-logs

OK all, try this change. I'm still learning a lot here, so I hope it's at least partially right.

@chriseth regarding '…' vs '...' (😆), I used to work in print, so have aways used the 'correct one out of habit, but I'm really not bothered about using it. For unicode reasons I will stick to the 3 dots from now on.

axic · 2018-09-26T17:38:00Z

 - `Javascript documentation <https://github.com/ethereum/wiki/wiki/JavaScript-API#contract-events>`_
 - `Example usage of events <https://github.com/debris/smart-exchange/blob/master/lib/contracts/SmartExchange.sol>`_
 - `How to access them in js <https://github.com/debris/smart-exchange/blob/master/lib/exchange_transactions.js>`_
+- `How Solidity Events Are Implemented (blog post) <https://blog.qtum.org/how-solidity-events-are-implemented-diving-into-the-ethereum-vm-part-6-30e07b3037b9>`_


Do we want to link to a different blockchain?

Also do these sources actually explain anything beyond this documentation? If so, don't we want to include that detail?

@axic It was talking about Ethereum, but sure, I get the point. I just thought it was really good. Maybe instead I'll remove it and add what's missing in our docs over time.

axic · 2018-09-26T17:39:26Z


 Offset ``g`` points to the start of the content of the array ``["one", "two", "three"]`` which is line 10 (320 bytes); thus ``g = 0x0000000000000000000000000000000000000000000000000000000000000140``.

+.. _events_topics:


I think this should be named abi_events.

axic · 2018-09-26T17:41:03Z

-which in turn can be used to "call" JavaScript callbacks in the user interface
-of a dapp, which listen for these events.
+Events allow you to use the EVM logging facilities to "call" JavaScript
+callbacks in the user interface of a dapp, which listen for these events.


I think this is better:

Solidity events provide a nice abstraction over the EVM's logging facilities. Applications can subscribe and listen to these events through the RPC interface of an Ethereum client.

ChrisChinchilla · 2018-10-02T09:46:56Z

@axic Other changes also made.

@gumb0

Updates from comments Clarify code comments and add an event output example Clarification from review Updated with information from @gumb0 Add clarifier Updates from review Remove link Update example code

ChrisChinchilla · 2018-10-05T09:26:14Z

Rebased and squashed

chriseth

There are still some small things to fix, but I'm merging now to end this PR's misery ;)

ChrisChinchilla · 2018-10-05T13:46:24Z

😆 @chriseth Sure, we are an 'agile' project :)

hmijail · 2018-12-19T07:21:53Z

After struggling for a good while to understand the "indexed" thing, some random answer in Stack Overflow mentioned bloom filters and suddenly everything made sense. Came here to suggest this to should be mentioned in the docs, and found that it seemed to be in its way... but got lost?

To summarize: I'm currently learning Solidity, and the events/logging docs are still pretty obtuse/abstract IMO. Maybe mentioning bloom filters would not help a huge percentage of the population, but I see here lots of different wordings which would be big improvements anyway.

ChrisChinchilla · 2019-02-27T11:06:12Z

Thanks @hmijail I will try to get back to re-reviewing this, but in the meantime, if you have anything to add that you think will improve the current docs, please create a PR and we can have some further discussion in that :)

ChrisChinchilla mentioned this pull request Sep 12, 2018

Documentation not clear how event data is stored #544

Closed

mancze reviewed Sep 13, 2018

View reviewed changes

chriseth reviewed Sep 17, 2018

View reviewed changes

chriseth suggested changes Sep 17, 2018

View reviewed changes

axic added the documentation 📖 label Sep 19, 2018

chriseth reviewed Sep 24, 2018

View reviewed changes

axic reviewed Sep 26, 2018

View reviewed changes

Update docs on event data and logs

973d91e

Updates from comments Clarify code comments and add an event output example Clarification from review Updated with information from @gumb0 Add clarifier Updates from review Remove link Update example code

ChrisChinchilla force-pushed the docs-544-event-data branch from e71f6e7 to 973d91e Compare October 5, 2018 09:26

chriseth approved these changes Oct 5, 2018

View reviewed changes

chriseth merged commit 44c1293 into develop Oct 5, 2018

axic deleted the docs-544-event-data branch November 22, 2018 00:29


		Offset ``g`` points to the start of the content of the array ``["one", "two", "three"]`` which is line 10 (320 bytes); thus ``g = 0x0000000000000000000000000000000000000000000000000000000000000140``.

		.. _events_topics:

Conversation

ChrisChinchilla commented Sep 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Description

Uh oh!

ChrisChinchilla commented Sep 12, 2018

Uh oh!

codecov Bot commented Sep 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mancze left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChrisChinchilla commented Sep 14, 2018

Uh oh!

mancze commented Sep 14, 2018

Uh oh!

ChrisChinchilla commented Sep 17, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chriseth commented Sep 17, 2018

Uh oh!

ChrisChinchilla commented Sep 20, 2018

Uh oh!

chriseth commented Sep 20, 2018

Uh oh!

gumb0 commented Sep 20, 2018

Uh oh!

chriseth commented Sep 20, 2018

Uh oh!

gumb0 commented Sep 21, 2018

Uh oh!

ChrisChinchilla commented Sep 21, 2018

Uh oh!

gumb0 commented Sep 21, 2018

Uh oh!

ChrisChinchilla commented Sep 24, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChrisChinchilla commented Oct 2, 2018

Uh oh!

ChrisChinchilla commented Oct 5, 2018

Uh oh!

ChrisChinchilla commented Sep 12, 2018 •

edited

Loading

codecov Bot commented Sep 12, 2018 •

edited

Loading

mancze left a comment •

edited

Loading

hmijail commented Dec 19, 2018 •

edited

Loading