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

Wallet doesn't correctly show amount of tokens to be transferred when unstaking from Rocketpool or bridging from Polygon #25755

Closed
urbenlegend opened this issue Oct 1, 2022 · 5 comments · Fixed by brave/brave-core#21796
Assignees
Labels
feature/web3/wallet/core feature/web3/wallet/simulation Transaction and message simulations feature/web3/wallet Integrating Ethereum+ wallet support OS/Desktop priority/P4 Planned work. We expect to get to it "soon". QA Pass-Win64 QA/Yes release-notes/include

Comments

@urbenlegend
Copy link

urbenlegend commented Oct 1, 2022

Description

When attempting to unstake from Rocketpool (converting rETH to ETH), the wallet doesn't correctly show amount of tokens to be transferred. For example, if you try to unstake 0.3 rETH, Brave Wallet will display a confirmation dialog that shows the total amount to be transferred as 0 ETH + <gas fees> instead of 0.3 rETH + <gas fees>.

The same issue happens when bridging Ethereum from Polygon to the Ethereum Mainnet.

Steps to Reproduce

Rocketpool

  1. Add rETH token to Brave Wallet's custom assets using the official address here https://etherscan.io/token/0xae78736cd615f374d3085123a210448e74fc6393
  2. Stake some ETH into Rocketpool and get rETH.
  3. Attempt to unstake that rETH.
  4. Notice that the Brave Wallet confirmation dialog shows 0 ETH + <gas> to be transferred, instead of the correct amount of rETH.

Polygon Bridge

  1. Get some wrapped Ethereum on the Polygon Network.
  2. Use Polygon Bridge to transfer that ethereum over to mainnet: https://wallet.polygon.technology/bridge
  3. Notice that the Brave Wallet confirmation dialog shows 0 MATIC + <MATIC gas> instead of the correct amount of wrapped ETH.

Actual result:

Screenshot from 2022-10-01 00-31-47
Screenshot from 2022-10-01 00-32-18
Screenshot from 2022-10-01 09-51-26
Screenshot from 2022-10-01 09-51-15

Expected result:

Second screenshot should show 0.3 rETH + 0.00270736 ETH under Total instead of 0 ETH + 0.00270736 ETH.
Fourth screenshot should show 0.0567 WETH + 0.00126862 MATIC under Total instead of 0 MATIC + 0.00126882 MATIC

Reproduces how often:

Every time

Desktop Brave version:

1.44.101 Chromium: 106.0.5249.65 (Official Build) (64-bit)

Android Device details:

  • Install type (ARM, x86): ARM
  • Device type (Phone, Tablet, Phablet): Pixel 5
  • Android version: 13

Version/Channel Information:

  • Can you reproduce this issue with the current release? Yes
  • Can you reproduce this issue with the beta channel? Not tested
  • Can you reproduce this issue with the nightly channel? Not tested

Other Additional Information:

  • Does the issue resolve itself when disabling Brave Shields? No
  • Does the issue resolve itself when disabling Brave Rewards? No
  • Is the issue reproducible on the latest version of Chrome? N/A

Miscellaneous Information:

@urbenlegend urbenlegend added OS/Android Fixes related to Android browser functionality OS/Desktop labels Oct 1, 2022
@urbenlegend urbenlegend changed the title Wallet doesn't correctly show amount of tokens to be transferred when unstaking from Rocketpool Wallet doesn't correctly show amount of tokens to be transferred when unstaking from Rocketpool or bridging from Polygon Oct 1, 2022
@onyb onyb self-assigned this Oct 2, 2022
@onyb onyb added the feature/web3/wallet Integrating Ethereum+ wallet support label Oct 2, 2022
@onyb
Copy link
Member

onyb commented Oct 2, 2022

Thanks for reporting this issue. What you're describing is essentially a problem in all EVM wallets called "blind-signing", which means the parameters of the transaction are unknown to the user. The reason why we display "0 ETH" is because we cannot determine if the asset being spent is "rETH", nor can we infer the associated amount.

Fortunately, we have a way to solve this now, thanks to brave/brave-core#12797 that was recently merged. We're rolling this out in the next release with DEX transactions. For the rest, we'll need to support them on a case-by-case basis - Rocketpool and Polygon bridge are both good candidates.

We're additionally exploring options like transaction simulations that would fix this for all kinds of transactions.

@urbenlegend
Copy link
Author

urbenlegend commented Oct 2, 2022

@onyb Thanks for the info. Glad there's a path towards the fix. Hope to see it land soon!

@onyb
Copy link
Member

onyb commented Oct 2, 2022

Great question! Etherscan is able to do it because the transaction has already been processed by the network, and is therefore aware of all the events emitted by the transaction.

In case of Brave Wallet (or any other EVM wallet to speak of), the transaction is unsigned and is simply a sequence of raw bytes. In order to figure out the precise parameters, we need to be able to parse these raw bytes and infer the details - this is what we've started doing with Swaps (already available on Beta - brave/brave-core#14745).

By the way, when I mentioned "transaction simulations", it was to mimic execution of the transaction, without actually broadcasting it, and capturing the events in the process.

@urbenlegend
Copy link
Author

Yes, I totally realized how silly my question was a split second after I commented and deleted the question from my comment but not before you beat me to the punch and answered it. Thanks for the great explanation though!

Is that raw byte sequence available for all unsigned transactions? You mentioned swaps, which I am assuming is the swapping functionality built into the Wallet, but what about things like Rocketpool and Polygon Bridge, or even external swaps like Matcha or Uniswap? Are those byte sequences similarly parseable?

@srirambv
Copy link
Contributor

Verified as part of #24271

@srirambv srirambv added QA Pass-Win64 and removed OS/Android Fixes related to Android browser functionality labels Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/web3/wallet/core feature/web3/wallet/simulation Transaction and message simulations feature/web3/wallet Integrating Ethereum+ wallet support OS/Desktop priority/P4 Planned work. We expect to get to it "soon". QA Pass-Win64 QA/Yes release-notes/include
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants