-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat(SDKConnectV2): RPC Message Handling #19823
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
base: ts/sdk-connect-v2/resume-and-reconnect
Are you sure you want to change the base?
feat(SDKConnectV2): RPC Message Handling #19823
Conversation
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. |
getProviderState: any; | ||
}) => | ||
getRpcMethodMiddleware({ | ||
hostname: middlewareHostname, |
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.
should this be the origin value? previously connection.origin
?
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.
Sadly I don't think we can do that. A bunch of old code relies on the middlewareHostname
to have this very specific format for routing. It's kinda silly..
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.
More context: a bunch of routing and UI code relies on this prefix
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.
Hmm... Well I've made this change to setupBridge and I don't think I saw any issues come out of it. Does this seem right or wrong?
public async handleConnectDeeplink(url: string): Promise<void> { | ||
public handleConnectDeeplink: (url: string) => Promise<void> | undefined; | ||
|
||
private async _handleConnectDeeplink(url: string): Promise<void> { |
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.
Bug: Throttling and Loading State Issues
The handleConnectDeeplink
method, now throttled, returns undefined
instead of a Promise<void>
during the throttle period, which breaks callers expecting to await its result. Also, showLoading
is now called after parseConnectionRequest
, potentially leading to unbalanced loading indicator states if parsing fails before showLoading
is invoked.
|
Description
This PR is part of the larger initiative to integrate the new Mobile Wallet Protocol (MWP). It completes core logic by integrating multichain RPC message handling between the dApp and the wallet.
Project Roadmap:
This PR:
MultichainSubscriptionManager
for MMSDK V2 connections, enabling multi-chain capabilities for the SDK.BackgroundBridge
to:handleConnectDeeplink
: Implements throttling as a defensive measure to prevent issues caused by rapid, repeated calls to the deep link handler.Changelog
CHANGELOG entry: null
Related issues
Fixes:
Manual testing steps
No manual testing is needed. This PR introduces headless core logic that is not yet fully connected to the UI. Comprehensive manual testing will be performed in a subsequent PR once the end-to-end flow is complete.
Screenshots/Recordings
Before
N/A (Logic change only)
After
N/A (Logic change only)
Pre-merge author checklist
Pre-merge reviewer checklist