-
-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add connection id to simple deeplinks #63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
packages/connect-multichain/src/multichain/rpc/requestRouter.ts
Outdated
Show resolved
Hide resolved
packages/connect-multichain/src/multichain/transports/mwp/index.ts
Outdated
Show resolved
Hide resolved
ffmcgee725
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some feedback, and also we should address the broken unit tests
Co-authored-by: ffmcgee <[email protected]>
Co-authored-by: ffmcgee <[email protected]>
…link' into jl/add-session-id-to-simple-deeplink
|
no changelog because this is not developer or user facing |
| const [activeSession] = await sessionStore.list(); | ||
| return activeSession; | ||
| } catch (error){ | ||
| // TODO: verify if this try catch is necessary |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have we verified this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not yet. I looked at the code already, doesn't looks like this can throw, but not going to remove it out of abundance of safety
Explanation
When deeplinking the user into the wallet for non-initial connection cases, the wallet has no idea which connection/session the user is opening the wallet from. In the case that the connection the dapp is using no longer exists on the wallet side, the user will sit in the wallet indefinitely waiting for a request that will never arrive.
This PR fixes that by including the connection id to the previously plain
metamask://connect/mwpdeeplink. It is nowmetamask://connect/mwp?id=CONN_ID_HERE. This will allow the wallet to prompt the user with an error toast if that connection no longer exists on the wallet side.References
See: MetaMask/metamask-mobile#23700
Checklist