Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
"@lavamoat/preinstall-always-fail": "^2.0.0",
"@metamask/api-specs": "^0.14.0",
"@metamask/auto-changelog": "^3.4.3",
"@metamask/connect-multichain": "^0.1.0",
"@metamask/eslint-config": "^12.2.0",
"@metamask/eslint-config-nodejs": "^12.1.0",
"@metamask/eslint-config-typescript": "^12.1.0",
Expand Down Expand Up @@ -94,7 +95,8 @@
"react-scripts>react-app-polyfill>core-js": false,
"@solana/web3.js>bigint-buffer": false,
"@solana/web3.js>rpc-websockets>bufferutil": false,
"@solana/web3.js>rpc-websockets>utf-8-validate": false
"@solana/web3.js>rpc-websockets>utf-8-validate": false,
"@metamask/connect-multichain>@metamask/mobile-wallet-protocol-core>centrifuge>protobufjs": false
}
}
}
14 changes: 13 additions & 1 deletion src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import {
} from './helpers/MethodInvocationHelpers';
import { generateSolanaMethodExamples } from './helpers/solana-method-signatures';
import { useSDK } from './sdk';
import { WINDOW_POST_MESSAGE_ID } from './sdk/SDK';
import { MM_CONNECT_ID, WINDOW_POST_MESSAGE_ID } from './sdk/SDK';

function App() {
const [caipAccountIds, setCaipAccountIds] = useState<string[]>(['']);
Expand Down Expand Up @@ -857,6 +857,18 @@ function App() {
>
Auto Connect via postMessage
</button>
<button
onClick={() => {
connect(MM_CONNECT_ID).catch((error) => {
console.error('Auto-connect via MM Connect failed:', error);
});
}}
disabled={isExternallyConnectableConnected}
data-testid="auto-connect-mm-connect-button"
id="auto-connect-mm-connect-button"
>
Auto Connect via MM Connect
</button>
</div>
</section>
<section>
Expand Down
6 changes: 6 additions & 0 deletions src/sdk/SDK.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
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';

export const MM_CONNECT_ID = 'mm-connect';
export const WINDOW_POST_MESSAGE_ID = 'window.postMessage';
export const METAMASK_PROD_CHROME_ID = 'nkbihfbeogaeaoehlefnkodbefgpgknn';

Expand Down Expand Up @@ -113,6 +115,10 @@ export class SDK {
if (extensionId === WINDOW_POST_MESSAGE_ID) {
this.#provider = new MetaMaskMultichainWindowPostMessageProvider();
connected = await this.#provider.connect();
} else if (extensionId === MM_CONNECT_ID) {
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
111 changes: 111 additions & 0 deletions src/sdk/providers/MetaMaskConnectProvider.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
import type { MultichainCore, Scope } from '@metamask/connect-multichain';
import { createMetamaskConnect } from '@metamask/connect-multichain';
import type { CaipAccountId } from '@metamask/utils';

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

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

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

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

#walletSession: unknown = { sessionScopes: {} };

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) => {
if (notification.method === 'wallet_sessionChanged') {
this.#walletSession = notification.params;
}
this.#notifyCallbacks(notification);
},
},
ui: {
preferDesktop: false,
preferExtension: false,
},
});

await this.#mmConnect?.connect(['eip155:1'], []);
Copy link

Choose a reason for hiding this comment

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

Bug: Duplicate Connection Logic in Provider Initialization

The line await this.#mmConnect?.connect(['eip155:1'], []); in the connect() method hardcodes the initial connection with scope 'eip155:1'. This duplicates the connection logic that should only occur in the request() method when handling 'wallet_createSession'. This causes the connection to be established twice: once with the hardcoded scope during provider initialization, and again with the actual user-provided scopes when createSession is called. The hardcoded connection call should be removed from the connect() method.

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: Session Initialization Conflict

The connect() method prematurely calls mmConnect.connect with hardcoded ['eip155:1'] and empty accounts. This creates an unwanted initial session that conflicts with the wallet_createSession handler's role in establishing user-specified sessions, potentially leading to redundant connections or unexpected behavior.

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: Redundant Connection Interferes with Multi-Chain Support

The connect() method prematurely calls mmConnect.connect with hardcoded eip155:1 and empty accounts. This creates a redundant connection that conflicts with the wallet_createSession handler, which later connects with the actual user-requested scopes, potentially leading to an incorrect initial state and hindering multi-chain support.

Fix in Cursor Fix in Web


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().catch(console.error);
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') {
return this.#walletSession;
}
if (request.method === 'wallet_revokeSession') {
this.disconnect();
}
if (request.method === 'wallet_createSession') {
const requestedScopes = Object.keys({
...request.params.optionalScopes,
...request.params.requiredScopes,
}) as unknown as Scope[];

const requestedAccounts = new Set<CaipAccountId>();
Object.values(request.params.optionalScopes).forEach((scopeObject) => {
const { accounts } = scopeObject as { accounts: CaipAccountId[] };
accounts.forEach((account) => requestedAccounts.add(account));
});
await this.#mmConnect?.connect(
requestedScopes,
Array.from(requestedAccounts),
);
return this.#walletSession;
}
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