fix: eip712 domains not being valid for gen 5 devices - #624
Merged
Merged
Conversation
Contributor
Author
|
@metamaskbot publish-preview |
|
Preview builds have been published. See these instructions (from the Expand for full list of packages and versions. |
hmalik88
approved these changes
Aug 20, 2026
montelaidev
added a commit
that referenced
this pull request
Sep 21, 2026
<!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? Are there any issues or other links reviewers should consult to understand this pull request better? For instance: * Fixes #12345 * See: #67890 --> **Description** EIP-712 signing fails on Ledger Gen5 devices with "the signature doesn't match the right address" when the payload omits the EIP712Domain type (common — ethers-style dapps infer it from the domain object keys). The missing type gets defaulted to an empty list, so both the device and local verification hash an empty domain struct instead of the real domain values. Gen5 devices handle this undefined case differently than older devices, causing the signature check to fail. Fix: derive the EIP712Domain type from the domain object keys before signing and verification. Payloads that already declare it are untouched — zero behavior change for well-formed input. <!-- Are there any examples of this change being used in another repository? When considering changes to the MetaMask module template, it's strongly preferred that the change be experimented with in another repository first. This gives reviewers a better sense of how the change works, making it less likely the change will need to be reverted or adjusted later. --> <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes EIP-712 normalization and signature recovery for payloads missing EIP712Domain; well-formed payloads are untouched, but incorrect derivation could affect permit/NFT signing on hardware wallets. > > **Overview** > Fixes **Ledger Gen5** failures when dapps send EIP-712 payloads with a populated `domain` but no `types.EIP712Domain` (common ethers-style shape). Without this, `@metamask/eth-sig-util` sanitization treats the domain type as empty, so the host hashes a zero-field domain while the device behaves differently—leading to **“The signature doesn't match the right address”**. > > Adds **`withDerivedEip712Domain`**, which builds `EIP712Domain` from canonical domain keys (in contract order, skipping null/undefined and non-standard keys) when the type is missing or empty; payloads that already declare it are returned unchanged. **`LedgerKeyring.signTypedData`** runs this normalization before `sanitizeData`, sends the derived types to the device, and verifies with the same normalized data. The helper is **re-exported** from the package entry; unit and keyring tests cover NFT permit-style payloads and on-chain domain separator alignment. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit c99a1a1. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
EIP-712 signing fails on Ledger Gen5 devices with "the signature doesn't match the right address" when the payload omits the EIP712Domain type (common — ethers-style dapps infer it from the domain object keys).
The missing type gets defaulted to an empty list, so both the device and local verification hash an empty domain struct instead of the real domain values. Gen5 devices handle this undefined case differently than older devices, causing the signature check to fail.
Fix: derive the EIP712Domain type from the domain object keys before signing and verification. Payloads that already declare it are untouched — zero behavior change for well-formed input.
Note
Medium Risk
Changes EIP-712 normalization and signature recovery for payloads missing EIP712Domain; well-formed payloads are untouched, but incorrect derivation could affect permit/NFT signing on hardware wallets.
Overview
Fixes Ledger Gen5 failures when dapps send EIP-712 payloads with a populated
domainbut notypes.EIP712Domain(common ethers-style shape). Without this,@metamask/eth-sig-utilsanitization treats the domain type as empty, so the host hashes a zero-field domain while the device behaves differently—leading to “The signature doesn't match the right address”.Adds
withDerivedEip712Domain, which buildsEIP712Domainfrom canonical domain keys (in contract order, skipping null/undefined and non-standard keys) when the type is missing or empty; payloads that already declare it are returned unchanged.LedgerKeyring.signTypedDataruns this normalization beforesanitizeData, sends the derived types to the device, and verifies with the same normalized data. The helper is re-exported from the package entry; unit and keyring tests cover NFT permit-style payloads and on-chain domain separator alignment.Reviewed by Cursor Bugbot for commit c99a1a1. Bugbot is set up for automated code reviews on this repo. Configure here.