feat: full deeplink protocol (#992)

* feat: wip * feat: send channel config with wallet key * feat: log errors * feat: erorr management and dont send wallet_init until values persisted * feat: cleanup resume * feat: cleanup * feat: logs * feat: cleanup * feat: remove unused tests * fix: unit tests
MetaMask · Aug 21, 2024 · ecc1598 · ecc1598
1 parent 04ff2b3
commit ecc1598
Show file tree

Hide file tree

Showing 8 changed files with 93 additions and 109 deletions.
diff --git a/...ication-layer/src/services/RemoteCommunication/MessageHandlers/handleWalletInitMessage.ts b/...ication-layer/src/services/RemoteCommunication/MessageHandlers/handleWalletInitMessage.ts
@@ -3,7 +3,7 @@ import { CommunicationLayerMessage } from '../../../types/CommunicationLayerMess
 import { EventType } from '../../../types/EventType';
 import { logger } from '../../../utils/logger';
 
-export function handleWalletInitMessage(
+export async function handleWalletInitMessage(
   instance: RemoteCommunication,
   message: CommunicationLayerMessage,
 ) {
@@ -19,32 +19,36 @@ export function handleWalletInitMessage(
       'chainId' in data &&
       'walletKey' in data
     ) {
-      // Persist channel config
-      const { channelConfig } = instance.state;
-      logger.RemoteCommunication(
-        `WALLET_INIT: channelConfig`,
-        JSON.stringify(channelConfig, null, 2),
-      );
+      try {
+        // Persist channel config
+        const { channelConfig } = instance.state;
+        logger.RemoteCommunication(
+          `WALLET_INIT: channelConfig`,
+          JSON.stringify(channelConfig, null, 2),
+        );
 
-      if (channelConfig) {
-        const accounts = data.accounts as string[];
-        const chainId = data.chainId as string;
-        const walletKey = data.walletKey as string;
+        if (channelConfig) {
+          const accounts = data.accounts as string[];
+          const chainId = data.chainId as string;
+          const walletKey = data.walletKey as string;
 
-        instance.state.storageManager?.persistChannelConfig({
-          ...channelConfig,
-          otherKey: walletKey,
-          relayPersistence: true,
-        });
+          await instance.state.storageManager?.persistChannelConfig({
+            ...channelConfig,
+            otherKey: walletKey,
+            relayPersistence: true,
+          });
 
-        instance.state.storageManager?.persistAccounts(accounts);
-        instance.state.storageManager?.persistChainId(chainId);
-      }
+          await instance.state.storageManager?.persistAccounts(accounts);
+          await instance.state.storageManager?.persistChainId(chainId);
+        }
 
-      instance.emit(EventType.WALLET_INIT, {
-        accounts: data.accounts,
-        chainId: data.chainId,
-      });
+        instance.emit(EventType.WALLET_INIT, {
+          accounts: data.accounts,
+          chainId: data.chainId,
+        });
+      } catch (error) {
+        console.error('RemoteCommunication::on "wallet_init" -- error', error);
+      }
     } else {
       console.error(
         'RemoteCommunication::on "wallet_init" -- invalid data format',

diff --git a/packages/sdk-communication-layer/src/services/SocketService/ConnectionManager/resume.test.ts b/packages/sdk-communication-layer/src/services/SocketService/ConnectionManager/resume.test.ts
@@ -1,6 +1,5 @@
 import { SocketService } from '../../../SocketService';
 import { EventType } from '../../../types/EventType';
-import { MessageType } from '../../../types/MessageType';
 import { logger } from '../../../utils/logger';
 import { resume } from './resume';
 
@@ -36,7 +35,7 @@ describe('resume', () => {
           start: mockStart,
         },
       },
-      remote: { state: {} },
+      remote: { state: {}, hasRelayPersistence: jest.fn() },
       sendMessage: mockSendMessage,
     } as unknown as SocketService;
   });
@@ -76,25 +75,7 @@ describe('resume', () => {
 
     resume(instance);
 
-    expect(mockSendMessage).toHaveBeenCalledWith({ type: MessageType.READY });
-  });
-
-  it('should not send READY message if an originator, but initiate key exchange', () => {
-    instance.state.isOriginator = true;
-
-    mockAreKeysExchanged.mockReturnValue(true);
-
-    resume(instance);
-
-    expect(mockSendMessage).not.toHaveBeenCalled();
-  });
-
-  it('should start key exchange if keys are not exchanged and not an originator', () => {
-    mockAreKeysExchanged.mockReturnValue(false);
-
-    resume(instance);
-
-    expect(mockStart).toHaveBeenCalledWith({ isOriginator: false });
+    expect(mockEmit).toHaveBeenCalled();
   });
 
   it('should update manualDisconnect and resumed state after resuming', () => {

diff --git a/packages/sdk-communication-layer/src/services/SocketService/ConnectionManager/resume.ts b/packages/sdk-communication-layer/src/services/SocketService/ConnectionManager/resume.ts
@@ -14,19 +14,24 @@ import { handleJoinChannelResults } from './handleJoinChannelResult';
  *
  * @param instance The current instance of the SocketService.
  */
-export function resume(instance: SocketService) {
+export function resume(instance: SocketService): void {
   const { state, remote } = instance;
   const { socket, channelId, context, keyExchange, isOriginator } = state;
   const { isOriginator: remoteIsOriginator } = remote.state;
 
   logger.SocketService(
-    `[SocketService: resume()] context=${context} connected=${
+    `[SocketService: resume()] channelId=${channelId} context=${context} connected=${
       socket?.connected
     } manualDisconnect=${state.manualDisconnect} resumed=${
       state.resumed
     } keysExchanged=${keyExchange?.areKeysExchanged()}`,
   );
 
+  if (!channelId) {
+    logger.SocketService(`[SocketService: resume()] channelId is not defined`);
+    throw new Error('ChannelId is not defined');
+  }
+
   if (socket?.connected) {
     logger.SocketService(`[SocketService: resume()] already connected.`);
     socket.emit(MessageType.PING, {
@@ -35,6 +40,15 @@ export function resume(instance: SocketService) {
       context: 'on_channel_config',
       message: '',
     });
+
+    if (!remote.hasRelayPersistence() && !keyExchange?.areKeysExchanged()) {
+      // Always try to recover key exchange from both side (wallet / dapp)
+      if (isOriginator) {
+        instance.sendMessage({ type: MessageType.READY });
+      } else {
+        keyExchange?.start({ isOriginator: false });
+      }
+    }
   } else {
     socket?.connect();
 
@@ -51,7 +65,11 @@ export function resume(instance: SocketService) {
       },
       async (
         error: string | null,
-        result?: { ready: boolean; persistence?: boolean; walletKey?: string },
+        result?: {
+          ready: boolean;
+          persistence?: boolean;
+          walletKey?: string;
+        },
       ) => {
         try {
           await handleJoinChannelResults(instance, error, result);
@@ -62,17 +80,6 @@ export function resume(instance: SocketService) {
     );
   }
 
-  // Always try to recover key exchange from both side (wallet / dapp)
-  if (keyExchange?.areKeysExchanged()) {
-    if (!isOriginator) {
-      instance.sendMessage({ type: MessageType.READY });
-    }
-  } else if (!isOriginator) {
-    keyExchange?.start({
-      isOriginator: isOriginator ?? false,
-    });
-  }
-
   state.manualDisconnect = false;
   state.resumed = true;
 }
diff --git a/...s/sdk-communication-layer/src/services/SocketService/MessageHandlers/handleSendMessage.ts b/...s/sdk-communication-layer/src/services/SocketService/MessageHandlers/handleSendMessage.ts
@@ -41,6 +41,9 @@ export function handleSendMessage(
   message: CommunicationLayerMessage,
 ) {
   if (!instance.state.channelId) {
+    logger.SocketService(
+      `handleSendMessage: no channelId - Create a channel first`,
+    );
     throw new Error('Create a channel first');
   }
 

diff --git a/packages/sdk-socket-server-next/src/protocol/handleMessage.ts b/packages/sdk-socket-server-next/src/protocol/handleMessage.ts
@@ -134,11 +134,16 @@ export const handleMessage = async ({
       );
 
       // broadcast that the channel supports relayPersistence
-      socket.broadcast
-        .to(channelId)
-        .emit(`config-${channelId}`, { persistence: true });
+      socket.broadcast.to(channelId).emit(`config-${channelId}`, {
+        persistence: true,
+        walletKey: channelConfig.walletKey,
+      });
+
       // also inform current client
-      socket.emit(`config-${channelId}`, { persistence: true });
+      socket.emit(`config-${channelId}`, {
+        persistence: true,
+        walletKey: channelConfig.walletKey,
+      });
     }
     return;
   } catch (error) {

diff --git a/packages/sdk/src/services/RemoteCommunicationPostMessageStream/onMessage.ts b/packages/sdk/src/services/RemoteCommunicationPostMessageStream/onMessage.ts
@@ -29,9 +29,7 @@ export function onMessage(
     }
 
     if (!message?.name) {
-      logger(
-        `[RCPMS: onMessage()] ignore message without name message=${message}`,
-      );
+      logger(`[RCPMS: onMessage()] ignore message without name`, message);
       return;
     }
 

diff --git a/packages/sdk/src/services/RemoteCommunicationPostMessageStream/write.test.ts b/packages/sdk/src/services/RemoteCommunicationPostMessageStream/write.test.ts
@@ -27,6 +27,7 @@ describe('write function', () => {
   const mockGetKeyInfo = jest.fn();
   const mockIsSecure = jest.fn();
   const mockOpenDeeplink = jest.fn();
+  const mockIsMetaMaskInstalled = jest.fn();
   const isProviderConnected = jest.fn();
 
   let mockRemoteCommunicationPostMessageStream = {
@@ -44,6 +45,7 @@ describe('write function', () => {
         isSecure: mockIsSecure,
         isMobileWeb: mockIsMobileWeb,
         openDeeplink: mockOpenDeeplink,
+        isMetaMaskInstalled: mockIsMetaMaskInstalled,
       },
       debug: false,
     },
@@ -57,13 +59,15 @@ describe('write function', () => {
     callback = jest.fn();
 
     mockIsMobileWeb.mockReturnValue(false);
+    mockIsMetaMaskInstalled.mockReturnValue(true);
 
     mockEthereum.mockReturnValue({
       isConnected: isProviderConnected,
     });
 
     mockExtractMethod.mockReturnValue({
       method: 'metamask_getProviderState',
+      data: { data: {} },
     });
 
     mockRemoteCommunicationPostMessageStream = {
@@ -81,6 +85,7 @@ describe('write function', () => {
           isSecure: mockIsSecure,
           openDeeplink: mockOpenDeeplink,
           isMobileWeb: mockIsMobileWeb,
+          isMetaMaskInstalled: mockIsMetaMaskInstalled,
         },
         debug: false,
       },
@@ -125,21 +130,6 @@ describe('write function', () => {
       mockIsMobileWeb.mockReturnValue(false);
     });
 
-    it('should call the callback and not send a message if neither socketConnected nor ready', async () => {
-      mockIsConnected.mockReturnValue(false);
-      mockIsReady.mockReturnValue(false);
-
-      await write(
-        mockRemoteCommunicationPostMessageStream,
-        { jsonrpc: '2.0', method: 'some_method' },
-        'utf8',
-        callback,
-      );
-
-      expect(callback).toHaveBeenCalledWith(new Error('disconnected'));
-      expect(mockSendMessage).not.toHaveBeenCalled();
-    });
-
     it('should warn if ready is true but socketConnected is false', async () => {
       mockIsReady.mockReturnValue(true);
       mockIsConnected.mockReturnValue(false);
@@ -154,13 +144,14 @@ describe('write function', () => {
       );
 
       expect(consoleWarnSpy).toHaveBeenCalledWith(
-        `[RCPMS: _write()] invalid socket status -- shouldn't happen`,
+        '[RCPMS: write()] activeDeeplinkProtocol=undefined',
       );
     });
 
     it('should debug log if both ready and socketConnected are true', async () => {
       mockIsReady.mockReturnValue(true);
       mockIsConnected.mockReturnValue(true);
+      mockGetChannelId.mockReturnValue('some_channel_id');
 
       await write(
         mockRemoteCommunicationPostMessageStream,
@@ -170,7 +161,10 @@ describe('write function', () => {
       );
 
       expect(spyLogger).toHaveBeenCalledWith(
-        `[RCPMS: _write()] method metamask_getProviderState doesn't need redirect.`,
+        expect.stringContaining(
+          "[RCPMS: write()] method='metamask_getProviderState' isRemoteReady=true",
+        ),
+        expect.anything(),
       );
     });
   });
@@ -182,10 +176,10 @@ describe('write function', () => {
       mockIsAuthorized.mockReturnValue(true);
       mockIsMobileWeb.mockReturnValue(false);
       mockIsSecure.mockReturnValue(true);
+      mockGetChannelId.mockReturnValue('some_channel_id');
     });
 
     it('should redirect if method exists in METHODS_TO_REDIRECT', async () => {
-      mockGetChannelId.mockReturnValue('some_channel_id');
       mockExtractMethod.mockReturnValue({
         method: Object.keys(METHODS_TO_REDIRECT)[0],
       });
@@ -198,15 +192,16 @@ describe('write function', () => {
       );
 
       expect(mockOpenDeeplink).toHaveBeenCalledWith(
-        'https://metamask.app.link/connect?channelId=some_channel_id&pubkey=&comm=socket&t=d&v=2',
-        'metamask://connect?channelId=some_channel_id&pubkey=&comm=socket&t=d&v=2',
+        expect.stringContaining(
+          'https://metamask.app.link/connect?channelId=some_channel_id',
+        ),
+        expect.stringContaining('metamask://connect?channelId=some_channel_id'),
         '_self',
       );
     });
 
     it('should create a deeplink if remote is paused', async () => {
       mockIsPaused.mockReturnValue(true);
-      mockGetChannelId.mockReturnValue('some_channel_id');
 
       await write(
         mockRemoteCommunicationPostMessageStream,
@@ -216,8 +211,12 @@ describe('write function', () => {
       );
 
       expect(mockOpenDeeplink).toHaveBeenCalledWith(
-        'https://metamask.app.link/connect?redirect=true&channelId=some_channel_id&pubkey=&comm=socket&t=d&v=2',
-        'metamask://connect?redirect=true&channelId=some_channel_id&pubkey=&comm=socket&t=d&v=2',
+        expect.stringContaining(
+          'https://metamask.app.link/connect?redirect=true&channelId=some_channel_id',
+        ),
+        expect.stringContaining(
+          'metamask://connect?redirect=true&channelId=some_channel_id',
+        ),
         '_self',
       );
     });
@@ -228,6 +227,7 @@ describe('write function', () => {
       mockIsReady.mockReturnValue(true);
       mockIsConnected.mockReturnValue(true);
       mockIsMobileWeb.mockReturnValue(false);
+      mockGetChannelId.mockReturnValue('some_channel_id');
 
       await write(
         mockRemoteCommunicationPostMessageStream,
-Original file line number
+Diff line change
@@ Expand Up / @@ -29,9 +29,7 @@ export function onMessage( @@
         }
         if (!message?.name) {
-          logger(
-            `[RCPMS: onMessage()] ignore message without name message=${message}`,
-          );
+          logger(`[RCPMS: onMessage()] ignore message without name`, message);
           return;
         }
@@ Expand Down @@