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

Ethereum improvements and internal transactions #7805

Merged
merged 24 commits into from
Apr 5, 2023
Merged

Ethereum improvements and internal transactions #7805

merged 24 commits into from
Apr 5, 2023

Conversation

tomasklim
Copy link
Member

@tomasklim tomasklim commented Mar 8, 2023

Description

  • use ETH method name
  • fix showing fee twice when transaction is failed
  • show internal txs in transaction history together with other transfers
  • internal txs counted in day header
  • fix invalid transaction of only internal txs
  • fix fiat value in transaction detail modal
  • show fiat values everytime, remove unnecessary Show Fiat button Improve transaction detail amounts #7109
  • removes total inputs/outputs Improve transaction detail amounts #7109
  • adds symbol operators to amounts Improve transaction detail amounts #7109
  • hides fee for receive txs Improve transaction detail amounts #7109
  • fix custom fee inputs in bump fee section
  • sort tokens by $ balance, $ price, symbol length, alphabetically
  • adds support for ERC20, ERC721, ERC1155
  • hides tx amount in transaction list if tx is failed
  • inputs/outputs section redesign
  • token address attribute changed to contract to work with blockbook
  • show NFT (text) in transaction list and detail
  • single row transaction item amount cancelled for tokens and internal transactions
  • added new tx type contract
  • added internal transactions to export txs and fix exporting of targets when tokens are available
  • migration - ETH network txs are removed from remember wallet and forced to refetch again! INFORM USER

Related Issue

closes #7368
fixes https://satoshilabs.slack.com/archives/C03SRP8PSS2/p1676543488631379
#7109

Screenshots:

Inputs/Outputs:

  • difference with design
    • ETH amount under address (to fit)
    • address not expand on hover (goes out of window + problem with discreet mode)
    • added copy button
    • not sure about inputs/outputs (we dont know all of internal txs and tokens as we filter them in lower layer)
    • showing "Analyze in blockbook" everytime because of 🔝
      Screenshot 2023-03-22 at 17 54 57

Amount:

  • signs
  • fix prices
    Screenshot 2023-03-22 at 17 58 00

Transaction list

  • the 0.02.. ETH transfer is internal tx

Screenshot 2023-03-20 at 12 42 18

Tokens section:

  • sorted

Screenshot 2023-03-20 at 12 45 43

- BTC inputs outputs

Screenshot 2023-03-20 at 13 32 05

Contract transaction

Screenshot 2023-03-22 at 18 05 06

Follow-up

@tomasklim tomasklim requested a review from sime March 8, 2023 18:27
@tomasklim tomasklim force-pushed the feat/eth branch 5 times, most recently from c9f993d to b12f5f3 Compare March 13, 2023 16:27
@tomasklim tomasklim force-pushed the feat/eth branch 2 times, most recently from 54255ab to 6f7b569 Compare March 20, 2023 11:34
@tomasklim tomasklim marked this pull request as ready for review March 20, 2023 12:06
@tomasklim tomasklim force-pushed the feat/eth branch 2 times, most recently from 86e88b1 to 8297a73 Compare March 22, 2023 16:52
@tomasklim tomasklim requested a review from a team as a code owner March 22, 2023 16:52
@tomasklim tomasklim force-pushed the feat/eth branch 2 times, most recently from e0134f3 to cfc65c5 Compare March 27, 2023 08:46
Copy link
Contributor

@dahaca dahaca left a comment

Choose a reason for hiding this comment

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

A migration is needed to make sure that internalTransfers is []. If a devce had been remembered, the app would otherwise crash.

I haven't tested some things like exports or NFTs, will have another review round tomorrow! 🙌

packages/blockchain-link-types/src/common.ts Outdated Show resolved Hide resolved
packages/blockchain-link-utils/src/blockbook.ts Outdated Show resolved Hide resolved
packages/blockchain-link-utils/src/blockbook.ts Outdated Show resolved Hide resolved
packages/suite/src/components/suite/FormattedNftAmount.tsx Outdated Show resolved Hide resolved
suite-common/wallet-utils/src/exportTransactions.ts Outdated Show resolved Hide resolved
@tomasklim tomasklim requested a review from szymonlesisz as a code owner April 3, 2023 12:48
@tomasklim tomasklim force-pushed the feat/eth branch 2 times, most recently from e2b760e to 50e97ed Compare April 4, 2023 08:11
@tomasklim tomasklim requested a review from dahaca April 5, 2023 08:54
@matejkriz matejkriz added the release Will be included in the upcoming release. Needs to be backported to the release branch. label Apr 5, 2023
Copy link
Contributor

@dahaca dahaca left a comment

Choose a reason for hiding this comment

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

✅✅✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release Will be included in the upcoming release. Needs to be backported to the release branch.
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

ETH internal transactions and token transfers
3 participants