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

Add function for retrieving token recipient senders #6398

Merged
merged 7 commits into from
Jul 9, 2020

Conversation

dylanjw
Copy link
Contributor

@dylanjw dylanjw commented Apr 8, 2020

ref: #6395

Description

Add a function get_token_recipient_senders to dashboard.utils. It will retrieve and filter event logs for erc20 token transfers. Always returns a list.

Refers/Fixes

#6395

Testing

Let me know if you want tests written. I was unsure how to deal with the set up of test blockchain state. Here is a gist with a few assertions against some addresses I set up on rinkeby: https://gist.github.com/dylanjw/f3c0d876aee170b71a61858a322d24ed

@gitcoinbot gitcoinbot mentioned this pull request Apr 8, 2020
@dylanjw dylanjw force-pushed the token_provinenance branch 3 times, most recently from 91faf08 to 99a44cd Compare April 8, 2020 07:46
@codecov
Copy link

codecov bot commented Apr 8, 2020

Codecov Report

Merging #6398 into master will increase coverage by 0.04%.
The diff coverage is 90.47%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6398      +/-   ##
==========================================
+ Coverage   26.23%   26.27%   +0.04%     
==========================================
  Files         300      300              
  Lines       29323    29343      +20     
  Branches     4317     4319       +2     
==========================================
+ Hits         7693     7711      +18     
- Misses      21360    21361       +1     
- Partials      270      271       +1     
Impacted Files Coverage Δ
app/dashboard/utils.py 42.22% <90.47%> (+1.74%) ⬆️

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 1aa584d...7daa36c. Read the comment docs.

@danlipert
Copy link
Contributor

@dylanjw thanks for the PR! Generally, for testing, we either would like to see unit tests (mocking API responses usually works fine) or just describe how you tested or show your test cases like with your gist 👍

def get_token_recipient_senders(recipient_address, token_addres):
w3 = Web3(HTTPProvider(settings.WEB3_HTTP_PROVIDER))
contract = w3.eth.contract(
address=token_address,
Copy link
Contributor

Choose a reason for hiding this comment

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

token_address is not defined

if balance == 0:
return []

transfers = contract.events.Transfer.getLogs(
Copy link
Contributor

Choose a reason for hiding this comment

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

AttributeError: type object 'Transfer' has no attribute 'getLogs'

Copy link
Contributor

Choose a reason for hiding this comment

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

what version of web3py are you running? im running 4.6.0, which is the version in the requirements/base.txt

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, I wrote this using the latest web3. Ill get this fixed later today.

@dylanjw dylanjw force-pushed the token_provinenance branch 3 times, most recently from fc05982 to 0ca458c Compare April 10, 2020 07:01
@dylanjw dylanjw force-pushed the token_provinenance branch from 0ca458c to 8e2cdd4 Compare April 10, 2020 07:02
@dylanjw dylanjw force-pushed the token_provinenance branch from 8e2cdd4 to eabfd8e Compare April 10, 2020 07:06
balance = contract.functions.balanceOf(recipient_address).call()

if balance == 0:
return []
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I probably should take this out. If a user receives tokens, and then transfers them out; this function will return [] as if they never received them. I intended to create a fast exit path, but to determine if a address has ever received tokens, I dont think I can avoid retrieving the token transfer event logs.

@dylanjw
Copy link
Contributor Author

dylanjw commented Apr 10, 2020

@owocki Ive updated the function to work with web3, 4.6, and added a test. Let me know if you need any other changes.

@dylanjw dylanjw requested a review from owocki April 10, 2020 17:33
@dylanjw
Copy link
Contributor Author

dylanjw commented Apr 18, 2020

@owocki Are you planning to upgrade to web3 >=5 soon? I have time to help if that is on the todo list.

@owocki
Copy link
Contributor

owocki commented Apr 20, 2020

defer to @danlipert and @gitcoinco/engineers on that

@thelostone-mc
Copy link
Member

@dylanjw #6366

@danlipert danlipert merged commit e612086 into gitcoinco:master Jul 9, 2020
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.

4 participants