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

Notice of possible breaking change for dapps relying on the result of EIP-712 signatures #178

Open
danjm opened this issue Mar 20, 2023 · 0 comments

Comments

@danjm
Copy link
Contributor

danjm commented Mar 20, 2023

In as soon as 4 weeks time, MetaMask will release a fix to our EIP-712 signing code for ledger devices, and this has the possibility of breaking dapps which explicitly relied on our raw signature return values.

As per the Ethereum Yellow Paper, the recovery identifier v - the last byte of the signature - should be either 27 (0x1b) or 28 (0x1c). However, since Metamask started supporting signatures for EIP-712 message, we have (incorrectly) returned signatures with a v value of either 0 or 1. While our signing logic is correct, and messages have been signed with the correct keys, the signature as returned to your dapp would have ended with the incorrect byte. To further illustrate the problem, some projects have had to explicitly work around this by explicitly checking the v value on the returned signature and if it matches 0 or 1, adding 27 to it.

A fix to this issue was made in a PR to our ledger keyring repo last summer: https://github.com/MetaMask/eth-ledger-bridge-keyring/pull/152/files This fix will be included in an upcoming release. With this fix, the last byte of the signature returned to your dapp, when a user with a Ledger account signs an EIP-712 message, will change.

For example, given the exact same inputs and signed with the exact same account, a signature that previously was returned as this 0x72d4e38a0e582e09a620fd38e236fe687a1ec782206b56d576f579c026a7e5b946759735981cd0c3efb02d36df28bb2feedfec3d90e408efc93f45b894946e3200 would now be returned as 0x72d4e38a0e582e09a620fd38e236fe687a1ec782206b56d576f579c026a7e5b946759735981cd0c3efb02d36df28bb2feedfec3d90e408efc93f45b894946e321b

Note that recovery will not be impacted: if you could recover an address from any given signature before this change, you will still be able to recover an address after this change.

But this could break functionality of your dapp, for Ledger users, if you relied on the explicit signature hex string returned by MetaMask. In such a case, you may need to either migrate saved data or add logic that checks the last two bytes for either scenario (v being 0 or 1, OR v being 27 or 28).

This change will be released as early as 4 weeks from this posting.

@danjm danjm pinned this issue Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant