Feat/add type signed data v4 with salt #400
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new component that implements Typed Data V4 signing with an added salt parameter to mitigate a known bug. Key changes include:
- The addition of the signTypedDataV4WithSaltComponent in src/index.js.
- A new UI component in src/components/signatures/signTypedDataV4-sign-with-salt.js that provides signing and verification controls.
- Exporting the new component in src/components/signatures/index.js.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/index.js | Imported and invoked the new signTypedDataV4WithSaltComponent. |
| src/components/signatures/signTypedDataV4-sign-with-salt.js | Added a new component for signing/verification with salt. |
| src/components/signatures/index.js | Exported the new component. |
| signTypedDataV4WithSalt.onclick = async () => { | ||
| const msgParams = { | ||
| domain: { | ||
| chainId: globalContext.chainIdInt.toString(), |
There was a problem hiding this comment.
The chainId is converted to a string during signing, but in verification it is used as a number (line 163). Consider using the same data type for chainId in both places to prevent potential verification issues.
| chainId: globalContext.chainIdInt.toString(), | |
| chainId: globalContext.chainIdInt, |
Copilot uses AI. Check for mistakes.
| name: 'Ether Mail', | ||
| verifyingContract: '0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC', | ||
| version: '1', | ||
| salt: 'test', |
There was a problem hiding this comment.
The salt parameter is hard-coded as 'test'. If this is only intended for development, consider adding a clear comment or mechanism to supply a proper salt value for production use.
Copilot uses AI. Check for mistakes.
This PR has added sign Typed data v4 signature with domain salt to prevent bug MetaMask/metamask-extension#30473 happen in metamask extension and mobile.