Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
4 changes: 4 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -102,5 +102,9 @@
"@solana/web3.js>rpc-websockets>bufferutil": false,
"@solana/web3.js>rpc-websockets>utf-8-validate": false
}
},
"dependencies": {
"@metamask/connect": "^0.1.0",
"@metamask/connect-multichain": "^0.1.0"
}
}
5 changes: 5 additions & 0 deletions src/sdk/SDK.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { CaipAccountId, CaipChainId, Json } from '@metamask/utils';
import { parseCaipChainId, parseCaipAccountId } from '@metamask/utils';

import MetaMaskConnectProvider from './providers/MetaMaskConnectProvider';
import type MetaMaskMultichainBaseProvider from './providers/MetaMaskMultichainBaseProvider';
import MetaMaskMultichainExternallyConnectableProvider from './providers/MetaMaskMultichainExternallyConnectableProvider';
import MetaMaskMultichainWindowPostMessageProvider from './providers/MetaMaskMultichainWindowPostMessageProvider';
Expand Down Expand Up @@ -113,6 +114,10 @@ export class SDK {
if (extensionId === WINDOW_POST_MESSAGE_ID) {
this.#provider = new MetaMaskMultichainWindowPostMessageProvider();
connected = await this.#provider.connect();
} else if (extensionId === 'connect') {
this.#provider =
new MetaMaskConnectProvider() as unknown as MetaMaskMultichainBaseProvider;
connected = await this.#provider.connect();
} else {
this.#provider = new MetaMaskMultichainExternallyConnectableProvider();
connected = await this.#provider.connect(extensionId);
Expand Down
91 changes: 91 additions & 0 deletions src/sdk/providers/MetaMaskConnectProvider.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import type { MultichainCore } from '@metamask/connect-multichain';
import { createMetamaskConnect } from '@metamask/connect-multichain';

import type { Provider } from './Provider';

type NotificationCallback = (notification: any) => void;

class MetaMaskConnectProvider implements Provider {
#mmConnect: MultichainCore | null;

#notificationCallbacks: Set<NotificationCallback> = new Set();

constructor() {
this.#mmConnect = null;
this.#notificationCallbacks = new Set();
}

async connect(): Promise<boolean> {
if (this.#mmConnect) {
this.disconnect();
}

this.#mmConnect = await createMetamaskConnect({
dapp: {
name: 'MultichainTest Dapp',
url: 'https://metamask.github.io/test-dapp-multichain/latest/',
},
transport: {
onNotification: (notification: any) => {
this.#notifyCallbacks(notification);
},
},
});


return true;
Copy link

Choose a reason for hiding this comment

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

Bug: Metamask Connection Initialization Failure

The connect() method calls createMetamaskConnect() but doesn't assign its result to this.#mmConnect. This leaves this.#mmConnect as null, causing isConnected() to always return false, disconnect() to be ineffective, and request() calls (like wallet_invokeMethod) to fail.

Fix in Cursor Fix in Web

}
Copy link

Choose a reason for hiding this comment

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

Bug: Connect Method Ignores Metamask Errors

The connect() method always returns true, even if createMetamaskConnect() fails or throws an error. This masks actual connection failures and provides incorrect status information to callers.

Fix in Cursor Fix in Web


disconnect(): void {
if (this.#mmConnect !== null) {
this.#mmConnect.disconnect();
this.#mmConnect = null;
}
}

isConnected(): boolean {
return Boolean(this.#mmConnect);
}

async request(request: { method: string; params: any }): Promise<any> {
if (request.method === 'wallet_invokeMethod') {
return this.#mmConnect?.invokeMethod(request.params);
}
if (request.method === 'wallet_getSession') {
// handle this locally
}
if (request.method === 'wallet_revokeSession') {
// noop?
}
if (request.method === 'wallet_createSession') {
await this.#mmConnect?.connect(['eip155:1'], []);
Copy link
Member Author

Choose a reason for hiding this comment

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

this probably isn't right. Should only be called in the connect method

}
return Promise.resolve(null);
}

onNotification(callback: NotificationCallback): void {
this.#notificationCallbacks.add(callback);
}

removeNotificationListener(callback: NotificationCallback): void {
this.#notificationCallbacks.delete(callback);
}

removeAllNotificationListeners() {
this.#notificationCallbacks.forEach(
this.removeNotificationListener.bind(this),
);
}
Copy link

Choose a reason for hiding this comment

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

Bug: Concurrent Modification in Notification Callback Removal

The removeAllNotificationListeners() method modifies the #notificationCallbacks Set while iterating over it using forEach. This causes unpredictable iteration behavior as elements are deleted during traversal. The Set should be cleared directly instead: this.#notificationCallbacks.clear();

Fix in Cursor Fix in Web


#notifyCallbacks(notification: any): void {
this.#notificationCallbacks.forEach((callback) => {
try {
callback(notification);
} catch (error) {
console.error('Error in notification callback:', error);
}
});
}
}

export default MetaMaskConnectProvider;
Loading
Loading