extension
extension copied to clipboard
tahowallet
Tally Ledger Sign screen and Ledger display show token contract as destination address
Discord Discussion Link
https://discord.com/channels/808358975287722045/875438141610270740/971607078143672320
What browsers are you seeing the problem on?
Chrome, Brave
What were you trying to do?
Wanting to verify my 'send to' address of the transaction
What did not work?
In between the 1st and the 2nd sign screen of the Tally Ho Ledger sign UI the To address changes from the destination address to the token contract address. The token contact address is then also displayed on the ledger.
Below for a GTC transfer.
- D46Ba = the address I'm sending to
- 8163f = the GTC token contract
First screen
Second screen
Version
Community Edition v0.13.2
Relevant log output
No response
This is correct behavior! But perhaps not clear behavior.
The transfer screen is meant to show you who you're sending the GTC to (the token recipient).
The Ledger screen shows you who the transaction data is sent to. The transaction data in this case is sent to the token contract, which then triggers the transfer of the tokens to the token recipient.
@Shadowfiend yeah and it's different than in MM. When I'm setting up a send ERC20 transaction in MM it shows me the token recipient on my Ledger UI. So I guess that could be confusing.
When I'm setting up a send ERC20 transaction in MM it shows me the token recipient on my Ledger UI. So I guess that could be confusing.
I wonder if this is the actual bug? We know we're using WebUSB and MM isn't, but we could also be sending the tx differently?
It also might be different per asset the Ledger hardware "knows" :grimacing:
Ah, if ledger is interpreting the ERC20 interaction rather than showing raw contract info then we may well be displaying the wrong thing here, and that would be a bug.
Adding a side by side comparison here
https://docs.google.com/document/d/1bh6EcDfgevXl9hiCyDmrv7EI67WMjL5L/edit?usp=sharing&ouid=116739263929417862114&rtpof=true&sd=true
It looks like the To displayed on the ledger matches what our UI shows in those screenshots/photos. That is correct behavior.
We do need to adapt the other fields to align to what the Ledger UI is displaying. I think we'll have to wait for RFB 2 implementation to do that cleanly.
Yeah I think everything is matching on the Tally Ho UI & Ledger UI side. So it's more a decision, rather than a bug.
Where I expect confusion could come:
- Ledger icw MM shows the recipient address on the Ledger UI
- Ledger icw Tally Ho shows the token contract address on the Ledger UI
Secondly, what's nice about Ledger icw MM is that it displays the token amount & qty, where Ledger icw Tally Ho displays ETH 0 currently --> I imagine this is something we'd also want eventually so maybe I split it from here and make it a feature request?
Ahh I see! I anchored hard to the initial screenshots in the description and misunderstood here, sorry. Yeah, so to display the ERC20 recipient on the Ledger, I believe we need to support the Ledger ERC20 module, which we don't do yet. When we do that, we'll also want our UI to display that correctly. More work definitely needed there.
I don't know the timeline, but should we update the UI a bit?
Currently it makes it look like I'm sending tokens to the contract address.
We could change 'To' here to 'Token Contract' address, add the recipient address on this page as well, and add the note that the Ledger UI will display the token contract address?
cc @VladUXUI
I'd rather change both together to keep them as similar as possible... in an ideal world this screen shows the same thing as the device.
