Skip to content
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

Improve event decoding #575

Merged
merged 4 commits into from
May 31, 2020
Merged

Improve event decoding #575

merged 4 commits into from
May 31, 2020

Conversation

iamdefinitelyahuman
Copy link
Member

What I did

Store event ABI data and decode events on a per-deployment basis, to avoid issues when different contracts have events with the same signature but different indexed elements.

Fixes #511

How I did it

  • In network/events.py, event ABIs for specific addresses are held in _deployment_topics. When decoding an event, if the originating address has an event ABI, it is used. If not, the global event ABI (_topics) is used.
  • When updating _topics, if an ABI already exists it is only replaced if the previous record had no indexed values and the new one has indexed values. So far, every occurrence of this bug that has been reported has been a result of no indexing, when the existing record does use indexing. Upstream in eth_event, providing an indexed ABI will now decode an unindexed event log.

How to verify it

Run the tests. I added a test case for this specific issue.

@codecov-commenter
Copy link

codecov-commenter commented May 30, 2020

Codecov Report

Merging #575 into master will increase coverage by 0.07%.
The diff coverage is 86.95%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #575      +/-   ##
==========================================
+ Coverage   87.87%   87.95%   +0.07%     
==========================================
  Files          56       56              
  Lines        6335     6392      +57     
  Branches     1401     1418      +17     
==========================================
+ Hits         5567     5622      +55     
  Misses        531      531              
- Partials      237      239       +2     
Impacted Files Coverage Δ
brownie/network/transaction.py 85.52% <77.77%> (-0.66%) ⬇️
brownie/network/event.py 94.28% <96.77%> (+1.97%) ⬆️
brownie/network/contract.py 90.95% <100.00%> (+0.01%) ⬆️
brownie/convert/normalize.py 94.93% <0.00%> (+6.32%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6babef0...f98d86b. Read the comment docs.

Copy link
Collaborator

@matnad matnad left a comment

Choose a reason for hiding this comment

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

lgtm

@iamdefinitelyahuman iamdefinitelyahuman merged commit 0b4d5ac into master May 31, 2020
@iamdefinitelyahuman iamdefinitelyahuman deleted the fix-event-decoding branch May 31, 2020 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Event topics are not unique
3 participants