fix: isPermitSignatureRequest logic. Do not display as Permit if spender is not present

[digiwand]

Description

If primaryType either:

• Permit
• PermitBatch
• PermitBatchTransferFrom
• PermitSingle
• PermitTransferFrom

and it does not have a "spender" hex address, do not render the signature as a Permit. Without a spender, we will fallback to an EIP-712 signature.

Note: The check for isPermitSignatureRequest is not sophisticated. This will be revisited with the upcoming Signatures API work.

Related issues

Fixes: #27385

Manual testing steps

await window.ethereum.request({ "method": "eth_signTypedData_v4", "params": [
  "0xE56E7847Fd3661A9B7c86aAF1DaEa08D9DA5750e", 
    {
            domain: {
              name: 'Test Domain',
              version: '1',
              chainId: 11155111,
              verifyingContract: '0xC36442b4a4522E871399CD717aBDD847Ab11FE88',
            },
                    types: {
          Person: [
            { name: "name", type: "string" },
            { name: "wallet", type: "address" },
          ],
          Permit: [
            { name: "from", type: "Person" },
            { name: "to", type: "Person" },
            { name: "contents", type: "string" },
          ],
        },
        primaryType: "Permit",
        message: {
          from: {
            name: "Cow",
            wallet: "0xCD2a3d9F938E13CD947Ec05AbC7FE734Df8DD826",
          },
          to: {
            name: "Bob",
            wallet: "0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB",
          },
          contents: "Hello, Bob!",
        },
  }
], });

Screenshots/Recordings

Before

After

Pre-merge author checklist

• I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards.
• I've completed the PR template to the best of my ability
• I’ve included tests if applicable
• I’ve documented my code using JSDoc format if applicable
• I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Pre-merge reviewer checklist

• I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
• I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

[github-actions]

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

[metamaskbot]

Builds ready [7cd71ee]

• builds: chrome, firefox
• builds (beta): chrome
• builds (flask): chrome, firefox
• builds (MMI): chrome, firefox
• builds (test): chrome, firefox
• builds (test-flask): chrome, firefox
• build viz: Build System
• mv3: Background Module Init Stats
• mv3: UI Init Stats
• mv3: Module Load Stats
• mv3: Bundle Size Stats
• mv2: E2e Actions Stats
• code coverage: Report
• storybook: Storybook
• typescript migration: Dashboard
• all artifacts
• bundle viz:

  • background: 0, 1, 2, 3, 4, 5, 6
  • common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
  • content-script: 0
  • offscreen: 0
  • ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1809 ± 126 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	413	2550	1737	395	190
		domContentLoaded	1566	2518	1775	249	119
		load	1574	2550	1809	263	126
		domInteractive	14	258	57	53	25

Bundle size diffs [🚨 Warning! Bundle size has increased!]

• background: 0 Bytes (0.00%)
• ui: 0 Bytes (0.00%)
• common: 236 Bytes (0.00%)

[sonarcloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[metamaskbot]

Builds ready [876f27d]

• builds: chrome, firefox
• builds (beta): chrome
• builds (flask): chrome, firefox
• builds (MMI): chrome, firefox
• builds (test): chrome, firefox
• builds (test-flask): chrome, firefox
• build viz: Build System
• mv3: Background Module Init Stats
• mv3: UI Init Stats
• mv3: Module Load Stats
• mv3: Bundle Size Stats
• mv2: E2e Actions Stats
• code coverage: Report
• storybook: Storybook
• typescript migration: Dashboard
• all artifacts
• bundle viz:

  • background: 0, 1, 2, 3, 4, 5, 6
  • common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
  • content-script: 0
  • offscreen: 0
  • ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1732 ± 76 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	253	2134	1644	355	170
		domContentLoaded	1463	2116	1700	155	74
		load	1470	2136	1732	158	76
		domInteractive	17	195	50	38	18

Bundle size diffs [🚨 Warning! Bundle size has increased!]

• background: 0 Bytes (0.00%)
• ui: 0 Bytes (0.00%)
• common: 153 Bytes (0.00%)

[jpuri]

Hey @digiwand : I think this task requires more detailed discussion as there can be many different validations for different signature. In case we are not able to validate a signature - should we reject it or display as simple typed sign or some other approach.

[jpuri]

We should re-evaluate the approach and discuss this more.

[digiwand]

Hi @jpuri, I agree we should discuss how to approach this further. In the meantime, I think we should still enforce the spender exists for displaying Permit since spender is required for EIP-2612, Permit2, and EIP-4494 (NFT)(not finalized ERC), and without it, our UI is misleading.

should we reject it or display as simple typed sign or some other approach

I think we should persist the behavior of our old signatures and not reject the signatures in this case. As the signature authors design their own EIP-712 struct, any arbitrary string could be a valid primaryType. This should mean a message with primaryType === "Permit" without a spender, or another required Permit spec, can still be a valid EIP-712.

We could possibly warn developers if we detect that they may be trying to create a EIP-2612 signature and the required fields are invalid or missing.

We have explicit support for a minor subset of Permit primaryTypes as mentioned in the description above, along with Order primaryTypes not mentioned. We will revisit support for additional types with the upcoming Signature API work

[digiwand]

per discussion to reevaulate UI/UX closing ticket