status-im / status-desktop

Status Desktop client made in Nim & QML
https://status.app
Mozilla Public License 2.0
301 stars 79 forks source link

Fix(dapps): Fixing transactions flows for WC and BC #16767

Closed alexjba closed 2 hours ago

alexjba commented 3 days ago

What does the PR do

Closes: #16747 #16004 #16116 #16755

What all these issues have in common is that the gas to be used by the transaction was hardcoded to 21k. To fix this we're computing the gas for each transaction if it's not being supplied by the dapp.

The full fix includes:

  1. Fixing the address matching - Some dApps (https://yearn.fi/v3/) will use a different case. To fix this we'll use the wallet address case when the transaction is parsed. https://github.com/status-im/status-desktop/commit/f60c3321ce5ffa196489ba1eee32c0c546f56c74
  2. Laggy sign request modal - This was caused by the blocking network calls to compute the fees and the estimated time. To fix this the calls have been moved in the threadpool.
  3. Refused transactions - In the UI we were estimating a max fee using 21k as gas value for all transactions. This resulted in a low fee and we couldn't properly check if the user has enough ETH to complete the transactions. As a result we could initiate transactions where the fees were higher than the max ETH balance, result in the transaction refusal.
  4. Blocked transactions - Since we've been using 21k as gas, all transactions with a gas higher than this could result in being blocked.
  5. Since now we're computing the fees in an asynchronous manner I've noticed the loading state for the fees text is incorrect. The text behind the loading animation was visible. This has been fixed by updating the AnimatedText to allow a custom target property to animate. The fees component need to animate customText property and not the text property.

There are some updates to the fees computation required by the new dapps architecture. Since the fees requests are now async I've reused the SubscriptionBroker mechanism used in other parts of the app to manage the request periodicity and bundling when the same request is needed by two subscribers (WC or BC).

This PR moves the fees computation from SignRequestPlugin to the TransactionFeesBroker - component used to manage the periodic backend calls and the TransactionFeesSubscriber - component that aggregates the backend responses and computes the fees. The TransactionFeesSubscriber will receive data from 3 different requests (requestSuggestedFees, requestGasEstimate, requestEstimatedTime). This data is aggregated and transformed in a ready to be consumed format for the UI.

New user stories:

Given the dApp requests a transaction without gas information
And the session request modal is open
When the user brings Status in focus
Then the gas is requested from status-go
And the fees are updated periodically (each 5 seconds)
Given the dApp requests a transaction with gas information
And the session request modal is open
When the user brings Status in focus
Then the estimated gas is not requested from status-go
And the fees are updated periodically using the provided gas
Given the dApp requests a transaction
And the session request modal is open
When Status in not in focus
Then the fees computation task is not running
Given the session request modal is opened
And the user is asked to approve a transaction
When the user approves the transaction
Then the fees computed by Status are used in the transaction
// Fees data: maxFeePerGas, maxPriorityFeePerGas, gasPrice
// The reasoning for this is that we're updating these fields
Given the dApp requests a transaction with gas information
And the session request modal is open
Then the fees data is not shown in the raw transaction description

Affected areas

Wallet Connect, Browser Connect

Architecture compliance

Screenshot of functionality (including design for comparison)

Uniswap tx - User has enough for fees

https://github.com/user-attachments/assets/982bc1ba-4a3a-4360-b844-03e5b5f345e2

Rarible tx - custom gas. A comparison with Coinbase fees computation. The user does not have enough for fees

https://github.com/user-attachments/assets/2b7bb3dd-46b6-4b5e-bf7d-7c22b2b8c180

yearn.fy - Withdraw flow

https://github.com/user-attachments/assets/8e8059db-a22b-4dc7-ab0e-2db1313eb7cb

yearn.fy - Deposit flow

https://github.com/user-attachments/assets/5509fbc4-aeee-4b2c-927b-6cd4251e7b68

status-im-auto commented 3 days ago

Jenkins Builds

Click to see older builds (7) | :grey_question: | Commit | :hash: | Finished (UTC) | Duration | Platform | Result | |-|-|-|-|-|-|-| | :heavy_check_mark: | b340de63 | [#1](https://ci.status.im/job/status-desktop/job/prs/job/macos/job/aarch64/job/package/job/PR-16767/1/) | 2024-11-18 14:26:11 | ~7 min | `macos/aarch64` | [:apple:`dmg`](https://status-im-desktop-prs.ams3.cdn.digitaloceanspaces.com/StatusIm-Desktop-241118-141820-b340de-pr16767-aarch64.dmg) | | :heavy_check_mark: | b340de63 | [#1](https://ci.status.im/job/status-desktop/job/prs/job/linux/job/x86_64/job/tests-nim/job/PR-16767/1/) | 2024-11-18 14:26:20 | ~8 min | `tests/nim` | [:page_facing_up:`log`](https://ci.status.im/job/status-desktop/job/prs/job/linux/job/x86_64/job/tests-nim/job/PR-16767/1//consoleText) | | :heavy_check_mark: | b340de63 | [#1](https://ci.status.im/job/status-desktop/job/prs/job/linux/job/x86_64/job/tests-ui/job/PR-16767/1/) | 2024-11-18 14:31:24 | ~13 min | `tests/ui` | [:page_facing_up:`log`](https://ci.status.im/job/status-desktop/job/prs/job/linux/job/x86_64/job/tests-ui/job/PR-16767/1//consoleText) | | :heavy_check_mark: | b340de63 | [#1](https://ci.status.im/job/status-desktop/job/prs/job/macos/job/x86_64/job/package/job/PR-16767/1/) | 2024-11-18 14:33:20 | ~14 min | `macos/x86_64` | [:apple:`dmg`](https://status-im-desktop-prs.ams3.cdn.digitaloceanspaces.com/StatusIm-Desktop-241118-141820-b340de-pr16767-x86_64.dmg) | | :heavy_check_mark: | b340de63 | [#1](https://ci.status.im/job/status-desktop/job/prs/job/linux/job/x86_64/job/package-nix/job/PR-16767/1/) | 2024-11-18 14:34:40 | ~16 min | `linux-nix/x86_64` | [:package:`tgz`](https://status-im-desktop-prs.ams3.cdn.digitaloceanspaces.com/StatusIm-Desktop-241118-141814-b340de-pr16767-x86_64.nix.tar.gz) | | :heavy_check_mark: | b340de63 | [#1](https://ci.status.im/job/status-desktop/job/prs/job/linux/job/x86_64/job/package/job/PR-16767/1/) | 2024-11-18 14:42:07 | ~23 min | `linux/x86_64` | [:package:`tgz`](https://status-im-desktop-prs.ams3.cdn.digitaloceanspaces.com/StatusIm-Desktop-241118-141814-b340de-pr16767-x86_64.tar.gz) | | :heavy_check_mark: | b340de63 | [#1](https://ci.status.im/job/status-desktop/job/prs/job/windows/job/x86_64/job/package/job/PR-16767/1/) | 2024-11-18 14:46:56 | ~28 min | `windows/x86_64` | [:cd:`exe`](https://status-im-desktop-prs.ams3.cdn.digitaloceanspaces.com/StatusIm-Desktop-241118-141821-b340de-pr16767-x86_64.exe) |
:grey_question: Commit :hash: Finished (UTC) Duration Platform Result
:heavy_check_mark: f42f3427 #2 2024-11-18 16:04:25 ~4 min macos/aarch64 :apple:dmg
:heavy_check_mark: f42f3427 #2 2024-11-18 16:08:37 ~8 min tests/nim :page_facing_up:log
:heavy_check_mark: f42f3427 #2 2024-11-18 16:12:26 ~12 min tests/ui :page_facing_up:log
:heavy_check_mark: f42f3427 #2 2024-11-18 16:12:53 ~13 min macos/x86_64 :apple:dmg
:heavy_check_mark: f42f3427 #2 2024-11-18 16:13:39 ~13 min linux-nix/x86_64 :package:tgz
:heavy_check_mark: f42f3427 #2 2024-11-18 16:20:39 ~20 min linux/x86_64 :package:tgz
:heavy_check_mark: f42f3427 #2 2024-11-18 16:25:06 ~25 min windows/x86_64 :cd:exe
:heavy_check_mark: f42f3427 #3 2024-11-20 14:23:28 ~6 min macos/aarch64 :apple:dmg
:heavy_check_mark: f42f3427 #3 2024-11-20 14:24:19 ~7 min tests/nim :page_facing_up:log
:heavy_check_mark: f42f3427 #3 2024-11-20 14:29:47 ~12 min macos/x86_64 :apple:dmg
:heavy_check_mark: f42f3427 #3 2024-11-20 14:31:32 ~14 min linux-nix/x86_64 :package:tgz
:heavy_check_mark: f42f3427 #3 2024-11-20 14:31:53 ~15 min tests/ui :page_facing_up:log
:heavy_check_mark: f42f3427 #3 2024-11-20 14:38:19 ~21 min linux/x86_64 :package:tgz
:heavy_check_mark: f42f3427 #3 2024-11-20 14:43:09 ~26 min windows/x86_64 :cd:exe
:x: f42f3427 #4 2024-11-20 16:12:28 ~1 min macos/aarch64 :page_facing_up:log
:x: f42f3427 #4 2024-11-20 16:13:10 ~2 min linux-nix/x86_64 :page_facing_up:log
:heavy_check_mark: f42f3427 #4 2024-11-20 16:19:37 ~8 min tests/nim :page_facing_up:log
:heavy_check_mark: f42f3427 #4 2024-11-20 16:24:47 ~13 min tests/ui :page_facing_up:log
:heavy_check_mark: f42f3427 #4 2024-11-20 16:26:40 ~15 min macos/x86_64 :apple:dmg
:heavy_check_mark: f42f3427 #4 2024-11-20 16:32:54 ~22 min linux/x86_64 :package:tgz
:heavy_check_mark: f42f3427 #4 2024-11-20 16:36:24 ~25 min windows/x86_64 :cd:exe
:heavy_check_mark: ff6faf8d #5 2024-11-22 07:58:44 ~6 min macos/aarch64 :apple:dmg
:heavy_check_mark: ff6faf8d #5 2024-11-22 08:05:11 ~13 min tests/nim :page_facing_up:log
:heavy_check_mark: ff6faf8d #5 2024-11-22 08:06:34 ~14 min macos/x86_64 :apple:dmg
:heavy_check_mark: ff6faf8d #5 2024-11-22 08:06:51 ~14 min tests/ui :page_facing_up:log
:heavy_check_mark: ff6faf8d #5 2024-11-22 08:07:09 ~15 min linux-nix/x86_64 :package:tgz
:heavy_check_mark: ff6faf8d #5 2024-11-22 08:08:57 ~17 min linux/x86_64 :package:tgz
:heavy_check_mark: ff6faf8d #5 2024-11-22 08:18:02 ~26 min windows/x86_64 :cd:exe
virginiabalducci commented 2 days ago

Wallet connect transactions look good. Browser connect transaction had an issue which was fixed here https://github.com/status-im/status-desktop/pull/16771 There are some transaction type on BC that are pending testing because they have not been developed yet