-
Notifications
You must be signed in to change notification settings - Fork 249
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
Add LogIndex and TxIndex into logs/event response body #862
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #862 +/- ##
==========================================
+ Coverage 60.62% 60.70% +0.08%
==========================================
Files 215 215
Lines 23099 23125 +26
==========================================
+ Hits 14003 14038 +35
+ Misses 7947 7935 -12
- Partials 1149 1152 +3 ☔ View full report in Codecov by Sentry. |
* feat: add thorclient * refactor: remove roundTripper * refactor: change null check * clean: remove commented code * feat: add account revision and pending tx * fix: add licence headers and fix linter issue * refactor: rename package * refactor: change revision type to string * refactor: rename GetLogs and GetTransfers to FilterEvents and FilterTransfers * refactor: change FilterEvents and FilterTransactions request type to EventFilter * Adding common.EventWrapper to handle channel errors * tweak * update rawclient + update account tests * tidy up names * update tests * pr comments * adding raw tx * Tidy up method names and calls * options client * tweaks * pr comments * Update thorclient/common/common.go Co-authored-by: libotony <[email protected]> * pr comments * Adding Subscriptions * Pr comments * adjust func orders * pr comments * changing subscribe to use the channel close vs multiple channels * adding go-doc * no error after unsubscribe * pr comments * checking status code is 2xx * fix: change FilterTransfers argument --------- Co-authored-by: otherview <[email protected]> Co-authored-by: libotony <[email protected]>
* Show all issues on lint * fix lint
* fix(docker): using AWS docker repo for trivy * fix(docker): using AWS docker repo for trivy
* ehancement: create a cache for block based subscriptions * minor: change function names for subscriptions * test: add unit test for message cache * chore: add license headers * refactor: fix up error handling * fix: remove bad test * fix: PR comments * fix: PR comments - remove block cache * refactor(subscriptions): store structs in cache, not bytes * fix(license): add license header * chore(subscriptions): revert unit test changes * enhancement: resolve pr comments to use simplelru * enhancement: resolve pr comments - use id as key
api/doc/thor.yaml
Outdated
nullable: true | ||
example: 1 | ||
logIndex: | ||
descrption: The index of the log in the receipt's outputs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to highlight it's an overall index among all clauses.
@@ -504,7 +507,7 @@ func (w *Writer) Write(b *block.Block, receipts tx.Receipts) error { | |||
|
|||
if err := w.exec( | |||
query, | |||
newSequence(blockNum, eventCount), | |||
newSequence(blockNum, uint32(txIndex), eventCount), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eventCount
and transferCount
needs to be reset for every receipt/tx.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And the logic of insert blockID
into ref needs to be updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eventCount
andtransferCount
needs to be reset for every receipt/tx.
Are we changing the indexes logic? Before the count
was not reset to 0 every receipt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think by adding txIndex
we are trying to align with ethereum's filter API, so we need to change the concept of logIndex
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
blockID insertion logic updated
@paologalligit possible to trigger CI? You may have to rebase with target branch. I think @libotony had issues on his mainDB branch, not sure how he solved |
6edc877
to
b74f714
Compare
Here, #868 (comment)
|
@paologalligit I think the |
* Refactor thor node * thorchain allows insertion of blocks * remove thorNode, added testchain * clean up + comments * adding license headers * adding templating tests for thorclient * Remove test event hacks * remove types * removed chain_builder + added logdb to testchain * pr comments * Update test/testchain/chain.go Co-authored-by: libotony <[email protected]> --------- Co-authored-by: libotony <[email protected]>
* chore(docs): update spec for validator nodes * chore(docs): update cores * chore(docs): remove public node stuff
fbca57e
to
297ffd5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm !
@paologalligit Appended a commit a2a74f8 |
* Thor client (#818) * feat: add thorclient * refactor: remove roundTripper * refactor: change null check * clean: remove commented code * feat: add account revision and pending tx * fix: add licence headers and fix linter issue * refactor: rename package * refactor: change revision type to string * refactor: rename GetLogs and GetTransfers to FilterEvents and FilterTransfers * refactor: change FilterEvents and FilterTransactions request type to EventFilter * Adding common.EventWrapper to handle channel errors * tweak * update rawclient + update account tests * tidy up names * update tests * pr comments * adding raw tx * Tidy up method names and calls * options client * tweaks * pr comments * Update thorclient/common/common.go Co-authored-by: libotony <[email protected]> * pr comments * Adding Subscriptions * Pr comments * adjust func orders * pr comments * changing subscribe to use the channel close vs multiple channels * adding go-doc * no error after unsubscribe * pr comments * checking status code is 2xx * fix: change FilterTransfers argument --------- Co-authored-by: otherview <[email protected]> Co-authored-by: libotony <[email protected]> * Show all issues on lint (#869) * Show all issues on lint * fix lint * fix(docker): using AWS docker repo for trivy (#872) * fix(docker): using AWS docker repo for trivy * fix(docker): using AWS docker repo for trivy * Darren/feat/add subscription cache (#866) * ehancement: create a cache for block based subscriptions * minor: change function names for subscriptions * test: add unit test for message cache * chore: add license headers * refactor: fix up error handling * fix: remove bad test * fix: PR comments * fix: PR comments - remove block cache * refactor(subscriptions): store structs in cache, not bytes * fix(license): add license header * chore(subscriptions): revert unit test changes * enhancement: resolve pr comments to use simplelru * enhancement: resolve pr comments - use id as key * Add additional block tests (#863) * enhancement(logging): leverage trace level (#873) * Add testchain package (#844) * Refactor thor node * thorchain allows insertion of blocks * remove thorNode, added testchain * clean up + comments * adding license headers * adding templating tests for thorclient * Remove test event hacks * remove types * removed chain_builder + added logdb to testchain * pr comments * Update test/testchain/chain.go Co-authored-by: libotony <[email protected]> --------- Co-authored-by: libotony <[email protected]> * chore(docs): update spec for validator nodes (#875) * chore(docs): update spec for validator nodes * chore(docs): update cores * chore(docs): remove public node stuff * Darren/logdb remove leading zeros (#865) * feat: add new txIndex column to event meta response * test: add convert event test * feat: make txLog and txIndex as optional return params * chore: update swagger with new event optional data * feat: save logIndex in sequence * feat: tweaked bits in sequence * refactor: rename optional log meta field * refactor: comments, yaml and txIndex counts * rebase to master * fix: remove stale struct * add txIndex to returned logdb query * reset to 0 eventCount and transferCount each receipt and write blockId only once * fix lint * rephrase logIndex description in yaml file * refactor: use filter.Option instead of eventFilter.Option * move includeIndexes to api --------- Co-authored-by: otherview <[email protected]> Co-authored-by: libotony <[email protected]> Co-authored-by: Darren Kelly <[email protected]> Co-authored-by: Makis Christou <[email protected]>
Description
This PR adds
logIndex
andtxIndex
fields intologs/event
response body.GH Issue
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Checklist: