-
Notifications
You must be signed in to change notification settings - Fork 40
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
ERC-20 token transaction list #2659
ERC-20 token transaction list #2659
Conversation
53d050a
to
55c0f9f
Compare
4d6c3fa
to
29e29d0
Compare
044bbfb
to
d70617c
Compare
d70617c
to
73bf62b
Compare
150539e
to
ac1c1fe
Compare
1 similar comment
ac1c1fe
to
d3714c0
Compare
cb97e51
to
0ca9bfd
Compare
0ca9bfd
to
9d17043
Compare
1 similar comment
|
||
regularTransactions = responses | ||
.flat() | ||
.filter(({ tx }) => +tx.amount > 0); // Remove transaction with 0 value (e.g.: fee payments) |
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 thought we agreed to only remove transactions that are duplicate (only if another (actual) transaction with the same hash exists) and not filter them by whether they have 0 value.
@CedrikNikita @peronczyk
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.
Yes, you are right. The hash suppose to be the same for two equal transactions that we got from different requests.
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.
Yes. And also we had quite extensive conversation with the team members and Sotiris that for the ERC-20 tokens we need to hide transactions that are for the fee payments (0 value).
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.
The goal was to exclude the duplicates from txlist
request by rewriting them with information from tokentx
for the transactions with the same hash.
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.
Exactly, we should hide fee payments transactions
. But filtering out all 0 value transactions
removes more than just the fee payment transactions
.
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.
Fee payments are not duplicates - those are regular transactions with separate transaction hashes and value equal to "0". Here's example object that describes such a transaction:

I'm not sure what you are @CedrikNikita referring to by tokentx
. Can you explain?
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.
For this list I propose to filter also out all fee payments transactions (what are the other 0 values?). We need to be able to show such information in origin transaction details page and/or aescan.
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 changed the logic so only when loading all account transactions the duplicated items with amount 0 are removed. Can you guys see if its OK now in your opinion?
@CedrikNikita @martinkaintas
I am ok |
861979a
to
aa07c61
Compare
aa07c61
to
62fef54
Compare
62fef54
to
b515043
Compare
b515043
to
e39956a
Compare
e39956a
to
88399b7
Compare
Yes, I also don't like this. But this was agreed with Sotiris. We were choosing between the |
88399b7
to
abb67ad
Compare
Closes: #2590