Skip to content

fix: integration with default transport #1357

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.

Sign up for GitHub

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

Merged
elribonazo merged 1 commit into from
Oct 3, 2025
Merged

fix: integration with default transport #1357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 40 additions & 12 deletions packages/sdk-multichain/src/multichain/transports/default/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,34 +3,63 @@ import type { CaipAccountId } from '@metamask/utils';
import type { ExtendedTransport, RPCAPI, Scope, SessionData } from 'src/domain';
import { addValidAccounts, getOptionalScopes, getValidAccounts } from 'src/multichain/utils';

const DEFAULT_REQUEST_TIMEOUT = 60 * 1000;

@adonesky1 adonesky1 Oct 2, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the basis for a 60 second timeout? Can you explain why we're adding this now? What is it fixing or protecting against specifically?

@elribonazo elribonazo Oct 3, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was agreed with @chakra-guy, @adonesky1... while it does not hurt to leave the RPC calls without a timeout, at the same time, if we don't add one the requests could be running forever and we think it is more secure to just expire the RPC requests that are not answered in reasonable time.

If u wanted to establish a connection or do a RPC call, 60 seconds will be enough, same logic can be found in Mobile Wallet protocol itself.


export class DefaultTransport implements ExtendedTransport {
#notificationCallbacks: Set<(data: unknown) => void> = new Set();

@adonesky1 adonesky1 Oct 2, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of tracking these callbacks?

@adonesky1 adonesky1 Oct 2, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is it solving for us on this pass?

@elribonazo elribonazo Oct 3, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is an excellent point @adonesky1

In Recent changes we moved all the Wallet related business logic into the Transport layer. By business logic we mean, handling connections, fetching sessions and doing session upgrade.

Before

In the past, the main export in the Multichain SDK had all that logic to manage session upgrades on connection or session restore, but combined with the Multichain api provider it led to unexpected wallet_getSession requests and bad user experience as the user requires one step on MWP to get the personal sign.

Ex: The user requests personal_sign. The Multichain api client will then trigger a wallet_getSession request. The personal sign message will only be received if the user manually goes to Mobile app and then back to the Dapp.

After

src/multichain/transports/mwp and src/multichain/transports/default, contain the 2 main transports that we need and add the same business logic on connection requests and invoke methods.

MWP and Default transport are responsible of handling wallet related business logic like upgrading the sessions on connection, reconnection, coming back to live from background, etc.

This also adds separation of concerns in the project, all that is wallet related remains contained inside the transports and the main class is just responsible of managing which flow we need to go into, really simple...

  • Multichain SDK is no longer responsible of handling wallet rpc requests or session upgrades
  • Transports do all this logic
  • Multichain SDK is just responsible of knowing which transport to plug into at any point in time and just iniciating the connection with the transport itself.
  • Transports, compared to before are now stronger and resolve connections only when the business logic is solved. EX, a Connection request only solves once we complete the connection with multichain-api itself + fetch a valid wallet_getSession or create a new one with wallet_createSession.

Answer

To the question of "Why we need notificationCallbacks" is because we are triggering this events from the Multichain DefaultTransport itself and we wanna be able to send notifications from our transport but also keep the multichain api transport notifications intact and working, thats why that is needed.
Kind of extending the events comming from the api client basically, does all the cleaning on disconnect too, so we should be good to go

#requestId = 0;
#transport: Transport = getDefaultTransport();
#defaultRequestOptions = {
timeout: DEFAULT_REQUEST_TIMEOUT,
};

#notifyCallbacks(data: unknown) {
for (const cb of this.#notificationCallbacks) {
try {
cb(data);
} catch (err) {
console.log('[WindowPostMessageTransport] notifyCallbacks error:', err);
}
}
}

async connect(options?: { scopes: Scope[]; caipAccountIds: CaipAccountId[] }): Promise<void> {
await this.#transport.connect();

//Get wallet session
const sessionRequest = await this.request({ method: 'wallet_getSession' });
const sessionRequest = await this.request({ method: 'wallet_getSession' }, this.#defaultRequestOptions);
let walletSession = sessionRequest.result as SessionData;
if (walletSession && options) {
const currentScopes = Object.keys(walletSession?.sessionScopes ?? {}) as Scope[];
const proposedScopes = options?.scopes ?? [];
const isSameScopes = currentScopes.every((scope) => proposedScopes.includes(scope)) && proposedScopes.every((scope) => currentScopes.includes(scope));
if (!isSameScopes) {
await this.request({ method: 'wallet_revokeSession', params: walletSession });
await this.request({ method: 'wallet_revokeSession', params: walletSession }, this.#defaultRequestOptions);
const optionalScopes = addValidAccounts(getOptionalScopes(options?.scopes ?? []), getValidAccounts(options?.caipAccountIds ?? []));
const sessionRequest: CreateSessionParams<RPCAPI> = { optionalScopes };
const response = await this.request({ method: 'wallet_createSession', params: sessionRequest });
const response = await this.request({ method: 'wallet_createSession', params: sessionRequest }, this.#defaultRequestOptions);
if (response.error) {
throw new Error(response.error.message);
}
walletSession = response.result as SessionData;
}
} else {
} else if (!walletSession) {
const optionalScopes = addValidAccounts(getOptionalScopes(options?.scopes ?? []), getValidAccounts(options?.caipAccountIds ?? []));
const sessionRequest: CreateSessionParams<RPCAPI> = { optionalScopes };
await this.request({ method: 'wallet_createSession', params: sessionRequest });
const response = await this.request({ method: 'wallet_createSession', params: sessionRequest }, this.#defaultRequestOptions);
if (response.error) {
throw new Error(response.error.message);
}
walletSession = response.result as SessionData;
}
this.#notifyCallbacks({
method: 'wallet_sessionChanged',
params: walletSession,
});
}

async disconnect(): Promise<void> {
this.#notificationCallbacks.clear();
return this.#transport.disconnect();
}

Expand All @@ -39,15 +68,14 @@ export class DefaultTransport implements ExtendedTransport {
}

async request<TRequest extends TransportRequest, TResponse extends TransportResponse>(request: TRequest, options?: { timeout?: number }) {
const requestWithId = {
...request,
jsonrpc: '2.0',
id: `${this.#requestId++}`,
};
return this.#transport.request(requestWithId, options) as Promise<TResponse>;
return this.#transport.request(request, options) as Promise<TResponse>;
}

onNotification(callback: (data: unknown) => void) {
return this.#transport.onNotification(callback);
this.#transport.onNotification(callback);
this.#notificationCallbacks.add(callback);
return () => {
this.#notificationCallbacks.delete(callback);
};
}
}
12 changes: 5 additions & 7 deletions packages/sdk-multichain/src/session.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,18 +98,17 @@ function testSuite<T extends MultichainOptions>({ platform, createSDK, options:
if (platform === 'web') {
t.expect(mockedData.mockDefaultTransport.request).toHaveBeenCalledWith(
t.expect.objectContaining({
jsonrpc: '2.0',
method: 'wallet_getSession',
}),

undefined,
{ timeout: 60 * 1000 },
);
t.expect(mockedData.mockDefaultTransport.request).toHaveBeenCalledWith(
t.expect.objectContaining({
method: 'wallet_revokeSession',
params: mockSessionData,
}),
undefined,
{ timeout: 60 * 1000 },
);

t.expect(mockedData.mockDefaultTransport.request).toHaveBeenCalledWith(
Expand All @@ -119,7 +118,7 @@ function testSuite<T extends MultichainOptions>({ platform, createSDK, options:
optionalScopes: mockedSessionUpgradeData.sessionScopes,
},
}),
undefined,
{ timeout: 60 * 1000 },
);
} else {
t.expect(mockedData.mockDappClient.sendRequest).toHaveBeenCalledWith(
Expand Down Expand Up @@ -168,10 +167,9 @@ function testSuite<T extends MultichainOptions>({ platform, createSDK, options:
if (platform === 'web') {
t.expect(mockedData.mockDefaultTransport.request).toHaveBeenCalledWith(
t.expect.objectContaining({
jsonrpc: '2.0',
method: 'wallet_getSession',
}),
undefined,
{ timeout: 60 * 1000 },
);
t.expect(mockedData.mockDefaultTransport.request).toHaveBeenCalledWith(
t.expect.objectContaining({
Expand All @@ -180,7 +178,7 @@ function testSuite<T extends MultichainOptions>({ platform, createSDK, options:
optionalScopes: mockSessionData.sessionScopes,
},
}),
undefined,
{ timeout: 60 * 1000 },
);
} else {
t.expect(mockedData.mockDappClient.sendRequest).toHaveBeenCalledWith(
Expand Down
2 changes: 1 addition & 1 deletion packages/sdk-multichain/src/ui/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ t.describe('ModalFactory', () => {
await constructorArgs.onClose();

// Multichain SDK is what will close the modal instead
t.expect(mockModal.unmount).not.toHaveBeenCalled();
t.expect(mockModal.unmount).toHaveBeenCalled();
});

t.it('should handle desktop onboarding correctly', async () => {
Expand Down
Loading