bugfix: fix dapp compat issues #1021
Conversation
| @@ -61,6 +61,10 @@ import DrawerStatusTracker from '../../../core/DrawerStatusTracker'; | |||
| const { HOMEPAGE_URL } = AppConstants; | |||
| const HOMEPAGE_HOST = 'home.metamask.io'; | |||
|
|
|||
| function resemblesAddress(string) { | |||
There was a problem hiding this comment.
If we can't use isValidAddress from ethereumjs-util here, can you move it to app/util/address.js?
There was a problem hiding this comment.
I wasn't sure if using isValidAddress was sufficient for all cases and defaulted to the logic the extension uses, which is where resemblesAddress came from. I will move this to the address util file for sure.
| try { | ||
| data = JSON.parse(payload.params[1]); | ||
| } catch (e) { | ||
| throw new Error('Data must be passed as a valid JSON string.'); |
There was a problem hiding this comment.
I think if you throw here it won't get back to the dapp context.
You should do Promise.reject({ error: error.message, jsonrpc: payload.jsonrpc, id: payload.id });
There was a problem hiding this comment.
Throwing here does get back to the dapp context (the thrown error is caught and shuffled as the RPC response up the chain.) I verified this only in a dapp context using Dan's test page, but I'm happy to explicitly reject if that's preferred.
There was a problem hiding this comment.
I left this as-is for now to stay in-line with the way we handle invalid data above.
|
|
||
| // eslint-disable-next-line eqeqeq | ||
| if (chainId && chainId != activeChainId) { | ||
| throw new Error(`Provided chainId (${chainId}) must match the active chainId (${activeChainId})`); |
There was a problem hiding this comment.
same as previous comment
brunobar79
added
the
needs-qa
|
There is an issue with LedgerDEX When I navigate to LedgerDEX the page keeps refreshing and after signing to log in nothing happens; seen here = https://recordit.co/FtQBLcV1bv |
Description
This pull request fixes dapp compatibility issues.
isApprovedshould returnfalseby default.eth_personalSignshould ducktype address parameter.eth_signTypedData_v3should verifychainId.Checklist
Issue
References #916
Resolves #915
Resolves #951