Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

signTypedData presents uint as hexadecimal in MetaMask UI #1193

Closed
PierreJeanjacquot opened this issue Dec 9, 2020 · 6 comments
Closed

signTypedData presents uint as hexadecimal in MetaMask UI #1193

PierreJeanjacquot opened this issue Dec 9, 2020 · 6 comments
Labels
enhancement New feature or improvement. fixed/complete This Bug is fixed or Enhancement is complete and published.

Comments

@PierreJeanjacquot
Copy link

Hi @ricmoo thank you for the EIP712 integration,

I'm currently looking to move my projects to Signer.signTypedData, everything I need works as expected, except I'm facing a presentational issue when signing data containing uint type with MetaMask.

  • uint,...,uint256 are presented to MetaMask's users in hexadecimal format.
  • uint are displayed human readable when calling Web3Provider.send('eth_signTypedData_v4', args).

The issue seems to be only presentational as the resulting signature is the same in both cases.

Here is a demo of the issue: https://codesandbox.io/s/gracious-raman-id9cg?file=/src/index.js

@ricmoo ricmoo added investigate Under investigation and may be a bug. on-deck This Enhancement or Bug is currently being worked on. labels Dec 14, 2020
@ricmoo
Copy link
Member

ricmoo commented Dec 14, 2020

The JSON-RPC definition of signTypedData is a bit loose in explaining what the payload should look like for values. Currently, I convert everything to a value compatible with results (e.g. quantity for numbers, which is a hex string without any leading zeros). I can make the payload generator return decimal numbers easy enough, but that should be something I would expect the client (i.e. MetaMask) to handle for display logic.

I'll ping them for their opinions and maybe they have better docs as to input value formats. :)

@pcowgill
Copy link

Hi @ricmoo, what would you say is the rough timeline for signTypedData leaving the experimental phase in ethers, triggering the renaming from _signTypedData to signTypedData? Thanks!

@ricmoo
Copy link
Member

ricmoo commented Jan 18, 2021

I am not really sure. I think the API is stable. I could probably move it out of experimental any time, but was wondering if I shouldn’t wait for v6 this summer. I’ll be starting that fork in February, then I’ll have a better idea of how long it might take. But I already have my wish list figured out for it. :)

Changing the formatting in metamask is likely quite simple. I want to talk to the MM folks first though, because I think that is something they should handle, in general. The payload should not be implying anything about presentation and it sounds like it is right now. I don’t mind making ethers use decimal and MM also doing it, but MM should be doing it too. :)

I don’t mind it getting done in both places. :)

@ricmoo ricmoo added enhancement New feature or improvement. and removed on-deck This Enhancement or Bug is currently being worked on. labels Feb 25, 2021
@ricmoo
Copy link
Member

ricmoo commented Feb 25, 2021

I've verified that MetaMask does this and made a small change (locally) that should expose numbers as decimal.

It's fine if they also replicate this functionality, which I'll suggest to them too.

@ricmoo
Copy link
Member

ricmoo commented Mar 8, 2021

This should be fixed in 5.0.32. Try it out and let me know. :)

@ricmoo ricmoo added fixed/complete This Bug is fixed or Enhancement is complete and published. and removed investigate Under investigation and may be a bug. labels Mar 8, 2021
@PierreJeanjacquot
Copy link
Author

Looks great, thank you @ricmoo! 🎉

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or improvement. fixed/complete This Bug is fixed or Enhancement is complete and published.
Projects
None yet
Development

No branches or pull requests

3 participants