Inconsistent signature notation with `personal_sign` and Ledger

[nlordell]

Describe the bug

This is quite a minor issue but I noticed that the signatures produced from personal_sign when using a MetaMask account and a Ledger HW wallet account are inconsistent.

Steps to reproduce

In order to reproduce this, simply sign with a MetaMask account and a Ledger account, and see that the notation used for the signature recovery value (a.k.a. v-value) are inconsistent:

1. Use personal_sign to sign a message with a MetaMask account
   • notice that the signature has v ∊ {27, 28}
2. Use personal_sign to sign a message with an imported Ledger account
   • notice that the signature has v ∊ {0, 1}

Sample code

const [account] = await window.ethereum.request({ method: "eth_requestAccounts" });
console.log(await window.ethereum.request({
  method: "personal_sign",
  params: [account, "MetaMask rocks!"],
}));

Expected behavior

Expected the signature notation to be the same. Specifically v ∊ {27, 28} as:

• That is what the "normal" MetaMask accounts do
• That is what the Ethereum JSON RPC example suggests is correct for the related eth_sign method
• That is the notation used for transactions signatures
• That is how Ethereum nodes behave (e.g. Geth)

Browser details:

• OS: Fedora 33
• Hardware Wallet: Ledger Nano S Firmware version 1.6.1
• Browser: Firefox 87
• MetaMask Version: 9.3.0

Additional context

I believe that the fix is very easy. It looks like the Ledger keyring mistakenly converts from v ∊ {27, 28} to v ∊ {0, 1} notation: /~https://github.com/MetaMask/eth-ledger-bridge-keyring/blob/d525991401df398f288270f83e2c44b6804c76ee/index.js#L248

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity in the last 90 days. It will be closed in 45 days if there is no further activity. The MetaMask team intends on reviewing this issue before close, and removing the stale label if it is still a bug. We welcome new comments on this issue. We do not intend on closing issues if they report bugs that are still reproducible. Thank you for your contributions.

[nlordell]

Out of curiosity, has this issue been addressed?

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity in the last 90 days. It will be closed in 45 days if there is no further activity. The MetaMask team intends on reviewing this issue before close, and removing the stale label if it is still a bug. We welcome new comments on this issue. We do not intend on closing issues if they report bugs that are still reproducible. Thank you for your contributions.

[nlordell]

I believe this is still an issue...

[plasmacorral]

@nlordell Thank you for taking the time to share this issue with us, your patience, and diligent follow-up. I expect this was resolved in MetaMask/eth-ledger-bridge-keyring#152 and will be closing your report. If you find a way to reproduce it, please open a fresh issue and make sure to mention this issue.