-
Notifications
You must be signed in to change notification settings - Fork 20.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RPC] incorrect logIndex in logs received from eth_getfilterchanges #1226
Comments
Thanks for that find 👍 |
@obscuren I don't see the Index modified anywhere in core/state/log.go or core/filters.go. Is there somewhere else events might be processed where this is set? If not, I suspect this is simply not being tracked and defaults to 0 as a uint. |
This might have been accidentally removed when the log messages were re-written. It should be done prior to storing the receipts to the database in the |
@debris Can you provide some example RPC calls to create the logs? |
Using geth javascript console
Getting multiple log fires into a single block:
I see this output (a log i patched in):
Would you expect logIndex to increment as "log index" indicates? If not, what behaviour would you expect? |
@obscuren I finally triggered some outputs where Index (last column) is set to something besides 0, but it doesn't make much sense at first glance:
Another spot:
And another:
|
@debris @frozeman Can you describe how the ordering should be? Is it simply 0...N for all logs in that block sorted first by transaction index? i.e, if transaction 0 contains 2 logs and transaction 1 contains 1 log and transaction 3 contains 1 log, then the logs would be ordered as such: logindex txindex |
@obscuren could you have a look, as i don't know even what the log index is anyway. if one transactions generated two logs, then the index would be 0 and 1. So its basically the index of the logs in a transaction receipt (which can only belong to one transaction) This number would only make really sense when we have |
@frozeman if neither you nor @debris know what this is used for, I would nominate to remove it. As a simple counter 0...N of the collection it's pretty useless. However, if it is used to indicate the order of the logs being created within each transaction, then we can be explicit about ordering without relying on the collection being properly sorted. cc: @wanderer |
I don't fully agree. If the log index specifies the index position in the receipt log array, then this information helps to identify and match logs within a transaction receipt. As well as helps to understand at which order logs were caused by a transaction. |
This is the correct ordering according to the current version of wiki. As far as I know |
My vote would be for log index to be the order of the logs within each transaction. For most transactions that trigger only a single log, this value would be 0. When more than 1 log is triggered in a single transaction, it would increment by 1. Combined with the TxIndex, the order of logs in a block should always be consistent when sorted by the key set (TxIndex, LogIndex). This change may be "harder" but I believe to be more semantically correct. |
I agree log index should be the index of the log within a transaction. So agree to what you described @tgerring Because this will make specially sense if we have |
develop HEAD log index is not correct:
|
👍 But i think this belongs into its own issue. (But it internally might be connected) |
I see that the problem is still not resolved. Any update on this? |
See #1545 Two transactions:
First transaction creates 1 log, second transaction creates 2 (multi sig). Both logs are mined in the same block.
|
:1 but array start with 0, so the log index should start with 0 too, so you can map it to the right log for example. |
Ah is see. Actual the log index should be a representation of the index in the receipts array IMO, otherwise i'm not sure what this log index is supposed to be used for. (rather than mapping it to the right log in a receipt) Or is there any other use for knowing that log x came after log y? I can see it be useful, but then you can't map logs anymore back to its receipts position. |
It was wrong. I'm updating the comment now |
Ok, you changed the receipt position. So what is now the correct index? with what are we going? logIndex == pos in receipt |
I thought we had discussed above that logIndex was the log position within the receipt, so as to distinguish multiple logs firing as the result of the same transaction |
core/state: Set log index. Closes #1226
) * go.mod: update golang.org/x/crypto to fix a Go 1.14 race rejection Cherry-pick conflicts: go.mod go.sum * p2p/discv5: fix test on go 1.14 (ethereum#20724) Cherry-picked to celo-blockchain * travis, appveyor, build, Dockerfile: bump Go to 1.14.2 (ethereum#20913) * travis, appveyor, build, Dockerfile: bump Go to 1.14.2 * travis, appveyor: force GO111MODULE=on for every build Cherry-pick conflicts: .travis.yml appveyor.yml build/checksums.txt * Update CircleCI and dockerfiles to use Go 1.14 * Revert ios build CI config to work with Go 1.13 * Update golang download links to 1.14.12 Co-authored-by: Péter Szilágyi <peterke@gmail.com> Co-authored-by: Felix Lange <fjl@twurst.com> Co-authored-by: Péter Szilágyi <peterke@gmail.com> Co-authored-by: Felix Lange <fjl@twurst.com>
…ereum#1226) * eth: fix a rare datarace on CHT challenge reply / shutdown * trie: check childrens' existence concurrently for snap heal * eth/protocols/snap: fix problems due to idle-but-busy peers * eth/filters: change filter block to be by-ref (ethereum#26054) This PR changes the block field in the filter to be a pointer, to disambiguate between empty hash and no hash * rpc: handle wrong HTTP batch response length (ethereum#26064) * eth/protocols/snap: throttle trie heal requests when peers DoS us (ethereum#25666) * eth/protocols/snap: throttle trie heal requests when peers DoS us * eth/protocols/snap: lower heal throttle log to debug Co-authored-by: Martin Holst Swende <martin@swende.se> * eth/protocols/snap: fix comment Co-authored-by: Martin Holst Swende <martin@swende.se> Co-authored-by: Péter Szilágyi <peterke@gmail.com> Co-authored-by: Martin Holst Swende <martin@swende.se> Co-authored-by: Jordan Krage <jmank88@gmail.com>
Currently it's always 0.
Steps to reproduce:
transactionIndex
is > 0,logIndex
is0
The text was updated successfully, but these errors were encountered: