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

ERC-20 token transaction list #2659

Merged
merged 1 commit into from Feb 13, 2024

Conversation

peronczyk
Copy link
Collaborator

@peronczyk peronczyk commented Jan 25, 2024

Closes: #2590

@peronczyk peronczyk force-pushed the feat/erc-20-token-transaction-list branch from 53d050a to 55c0f9f Compare January 25, 2024 11:46
@peronczyk peronczyk force-pushed the feat/erc-20-token-transaction-list branch 2 times, most recently from 044bbfb to d70617c Compare January 25, 2024 12:55
Copy link

@peronczyk peronczyk linked an issue Jan 25, 2024 that may be closed by this pull request
@peronczyk peronczyk self-assigned this Jan 25, 2024
@peronczyk peronczyk linked an issue Jan 25, 2024 that may be closed by this pull request
src/protocols/BaseProtocolAdapter.ts Outdated Show resolved Hide resolved
src/composables/assetDetails.ts Show resolved Hide resolved
src/composables/latestTransactionList.ts Show resolved Hide resolved
src/composables/latestTransactionList.ts Show resolved Hide resolved
src/popup/components/AccountDetailsTransactionsBase.vue Outdated Show resolved Hide resolved
src/popup/components/TransactionList.vue Show resolved Hide resolved
src/popup/components/TransactionList.vue Outdated Show resolved Hide resolved
src/popup/pages/Assets/AssetDetailsTransactions.vue Outdated Show resolved Hide resolved
src/protocols/ethereum/composables/ethBaseFee.ts Outdated Show resolved Hide resolved
src/types/index.ts Outdated Show resolved Hide resolved
@peronczyk peronczyk force-pushed the feat/erc-20-token-transaction-list branch from d70617c to 73bf62b Compare January 26, 2024 14:04
Copy link

@CedrikNikita
Copy link
Collaborator

CedrikNikita commented Jan 29, 2024

tokenTransactions
Yes, some random transactions that was made a seconds ago appear at the top after i scroll to the bottom and then up again.

.env Outdated Show resolved Hide resolved
@peronczyk peronczyk force-pushed the feat/erc-20-token-transaction-list branch 2 times, most recently from 150539e to ac1c1fe Compare January 30, 2024 10:50
Copy link

1 similar comment
Copy link

@peronczyk peronczyk force-pushed the feat/erc-20-token-transaction-list branch from ac1c1fe to d3714c0 Compare January 31, 2024 12:51
Copy link

@peronczyk peronczyk force-pushed the feat/erc-20-token-transaction-list branch 2 times, most recently from cb97e51 to 0ca9bfd Compare February 2, 2024 15:51
Copy link

github-actions bot commented Feb 2, 2024

@peronczyk peronczyk force-pushed the feat/erc-20-token-transaction-list branch from 0ca9bfd to 9d17043 Compare February 2, 2024 16:02
Copy link

github-actions bot commented Feb 2, 2024

1 similar comment
Copy link

github-actions bot commented Feb 2, 2024

@CedrikNikita
Copy link
Collaborator

CedrikNikita commented Feb 5, 2024

Allow token transaction amount is shown without decimal shifting.
image


regularTransactions = responses
.flat()
.filter(({ tx }) => +tx.amount > 0); // Remove transaction with 0 value (e.g.: fee payments)
Copy link
Collaborator

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

Copy link
Collaborator

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.

Copy link
Collaborator Author

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).

Copy link
Collaborator

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.

Copy link
Collaborator

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.

Copy link
Collaborator Author

@peronczyk peronczyk Feb 6, 2024

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:

image

I'm not sure what you are @CedrikNikita referring to by tokentx. Can you explain?

Copy link
Collaborator

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.

Copy link
Collaborator Author

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

@smaroudasunicorn
Copy link
Collaborator

I am ok

@peronczyk peronczyk force-pushed the feat/erc-20-token-transaction-list branch from 861979a to aa07c61 Compare February 8, 2024 15:18
Copy link

github-actions bot commented Feb 8, 2024

@peronczyk peronczyk force-pushed the feat/erc-20-token-transaction-list branch from aa07c61 to 62fef54 Compare February 9, 2024 11:18
Copy link

github-actions bot commented Feb 9, 2024

Copy link

@CedrikNikita
Copy link
Collaborator

CedrikNikita commented Feb 13, 2024

downloading token transactions

Token transactions are not being loaded, if transactions was long time ago.

@peronczyk peronczyk force-pushed the feat/erc-20-token-transaction-list branch from b515043 to e39956a Compare February 13, 2024 08:15
Copy link

@peronczyk peronczyk force-pushed the feat/erc-20-token-transaction-list branch from e39956a to 88399b7 Compare February 13, 2024 08:51
@peronczyk
Copy link
Collaborator Author

peronczyk commented Feb 13, 2024

downloading token transactions
Token transactions are not being loaded, if transactions was long time ago.

Yes, I also don't like this. But this was agreed with Sotiris. We were choosing between the /txs and /activities endpoint. Both has some downsides, like not having incoming transactions or being not able to fetch old transactions. Sotiris suggested that we should use the activities because it was used previously. We are waiting with this one for the backend developers: aeternity/ae_mdw#1678

@peronczyk peronczyk force-pushed the feat/erc-20-token-transaction-list branch from 88399b7 to abb67ad Compare February 13, 2024 09:51
Copy link

@peronczyk peronczyk merged commit f6698a1 into feat/eth-support Feb 13, 2024
2 checks passed
@peronczyk peronczyk deleted the feat/erc-20-token-transaction-list branch February 13, 2024 11:24
@peronczyk peronczyk linked an issue Feb 16, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants