fix: make domain.name optional in v2 to legacy EIP-712 conversion#283
Open
lcastillo-ledger wants to merge 1 commit intomainfrom
Open
fix: make domain.name optional in v2 to legacy EIP-712 conversion#283lcastillo-ledger wants to merge 1 commit intomainfrom
lcastillo-ledger wants to merge 1 commit intomainfrom
Conversation
Some EIP-712 contracts (e.g. Safe) do not include a `name` field in their EIP712Domain. The v2 to legacy EIP-712 converter was requiring `domain.name` to be present and failing with "Missing domain name" when it was absent. Make `dapp_name` fall back to `metadata.owner` when `domain.name` is not set, so descriptors for contracts with a minimal EIP712Domain (e.g. only chainId + verifyingContract) can still be converted. Made-with: Cursor
5 tasks
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
1 participant
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.
Summary
domain.nameoptional when converting v2 ERC-7730 descriptors to the legacy Ledger EIP-712 formatdomain.nameis absent, fall back tometadata.ownerfor the legacy descriptor's display nameEIP712Domainonly contains[chainId, verifyingContract](nonamefield)Context
Safe smart contracts (v1.3.0+) define their EIP-712 domain separator with only
chainIdandverifyingContract— thenamefield is not part of the domain. The v2 to legacy EIP-712 converter was unconditionally requiringdomain.nameand erroring with "Missing domain name" when it was absent, making it impossible to convert descriptors for these contracts.The
dapp_namefield in the legacy format is a display-only label (not part of the EIP-712 domain hash), so falling back tometadata.owneris both safe and semantically appropriate.Test plan
domain.nameset → should usedomain.nameas beforedomainor withdomain.nameabsent → should fall back tometadata.ownerinstead of failingMade with Cursor