From e281b907a90337e6cb0101b6d4d8c9718143e78b Mon Sep 17 00:00:00 2001 From: Omridan159 Date: Tue, 2 Jul 2024 10:30:23 +0400 Subject: [PATCH 1/2] chore: reorder accounts in ETH_REQUESTACCOUNTS response to prioritize selectedAddress in the 'AndroidService' --- .../SDKConnect/AndroidSDK/AndroidService.ts | 44 ++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/app/core/SDKConnect/AndroidSDK/AndroidService.ts b/app/core/SDKConnect/AndroidSDK/AndroidService.ts index c53a1ab7428..b57c2f6ae93 100644 --- a/app/core/SDKConnect/AndroidSDK/AndroidService.ts +++ b/app/core/SDKConnect/AndroidSDK/AndroidService.ts @@ -476,10 +476,52 @@ export default class AndroidService extends EventEmitter2 { // eslint-disable-next-line @typescript-eslint/no-explicit-any async sendMessage(message: any, forceRedirect?: boolean) { const id = message?.data?.id; - this.communicationClient.sendMessage(JSON.stringify(message)); let rpcMethod = this.rpcQueueManager.getId(id); + const isConnectionResponse = rpcMethod === RPC_METHODS.ETH_REQUESTACCOUNTS; + + if (isConnectionResponse) { + const preferencesController = ( + Engine.context as { + PreferencesController: PreferencesController; + } + ).PreferencesController; + + const selectedAddress = + preferencesController.state.selectedAddress.toLowerCase(); + + const lowercaseAccounts = (message.data.result as string[]).map( + (a: string) => a.toLowerCase(), + ); + + const isPartOfConnectedAddresses = + lowercaseAccounts.includes(selectedAddress); + + if (isPartOfConnectedAddresses) { + // Remove the selectedAddress from the lowercaseAccounts if it exists + const remainingAccounts = lowercaseAccounts.filter( + (account) => account !== selectedAddress, + ); + + // Create the reorderedAccounts array with selectedAddress as the first element + const reorderedAccounts: string[] = [ + selectedAddress, + ...remainingAccounts, + ]; + + message = { + ...message, + data: { + ...message.data, + result: reorderedAccounts, + }, + }; + } + } + + this.communicationClient.sendMessage(JSON.stringify(message)); DevLogger.log(`AndroidService::sendMessage method=${rpcMethod}`, message); + // handle multichain rpc call responses separately const chainRPCs = this.batchRPCManager.getById(id); if (chainRPCs) { From 0d51cbb901575123dadb9502a237315a16d74bfa Mon Sep 17 00:00:00 2001 From: Omridan159 Date: Tue, 2 Jul 2024 11:15:13 +0400 Subject: [PATCH 2/2] chore: add unit tests --- .../SDKConnect/AndroidSDK/AndroidService.ts | 132 ++---------------- .../AndroidService/sendMessage.test.ts | 131 +++++++++++++++++ .../AndroidSDK/AndroidService/sendMessage.ts | 128 +++++++++++++++++ 3 files changed, 270 insertions(+), 121 deletions(-) create mode 100644 app/core/SDKConnect/AndroidSDK/AndroidService/sendMessage.test.ts create mode 100644 app/core/SDKConnect/AndroidSDK/AndroidService/sendMessage.ts diff --git a/app/core/SDKConnect/AndroidSDK/AndroidService.ts b/app/core/SDKConnect/AndroidSDK/AndroidService.ts index b57c2f6ae93..8ebaa583977 100644 --- a/app/core/SDKConnect/AndroidSDK/AndroidService.ts +++ b/app/core/SDKConnect/AndroidSDK/AndroidService.ts @@ -28,27 +28,23 @@ import { PermissionController } from '@metamask/permission-controller'; import { PreferencesController } from '@metamask/preferences-controller'; import { PROTOCOLS } from '../../../constants/deeplinks'; import BatchRPCManager from '../BatchRPCManager'; -import { - DEFAULT_SESSION_TIMEOUT_MS, - METHODS_TO_DELAY, - RPC_METHODS, -} from '../SDKConnectConstants'; -import getDefaultBridgeParams from './getDefaultBridgeParams'; -import handleBatchRpcResponse from '../handlers/handleBatchRpcResponse'; +import { DEFAULT_SESSION_TIMEOUT_MS } from '../SDKConnectConstants'; import handleCustomRpcCalls from '../handlers/handleCustomRpcCalls'; import DevLogger from '../utils/DevLogger'; import AndroidSDKEventHandler from './AndroidNativeSDKEventHandler'; +import sendMessage from './AndroidService/sendMessage'; import { DappClient, DappConnections } from './dapp-sdk-types'; +import getDefaultBridgeParams from './getDefaultBridgeParams'; export default class AndroidService extends EventEmitter2 { - private communicationClient = NativeModules.CommunicationClient; - private connections: DappConnections = {}; - private rpcQueueManager = new RPCQueueManager(); - private bridgeByClientId: { [clientId: string]: BackgroundBridge } = {}; - private eventHandler: AndroidSDKEventHandler; - private batchRPCManager: BatchRPCManager = new BatchRPCManager('android'); + public communicationClient = NativeModules.CommunicationClient; + public connections: DappConnections = {}; + public rpcQueueManager = new RPCQueueManager(); + public bridgeByClientId: { [clientId: string]: BackgroundBridge } = {}; + public eventHandler: AndroidSDKEventHandler; + public batchRPCManager: BatchRPCManager = new BatchRPCManager('android'); // To keep track in order to get the associated bridge to handle batch rpc calls - private currentClientId?: string; + public currentClientId?: string; constructor() { super(); @@ -475,112 +471,6 @@ export default class AndroidService extends EventEmitter2 { // TODO: Replace "any" with type // eslint-disable-next-line @typescript-eslint/no-explicit-any async sendMessage(message: any, forceRedirect?: boolean) { - const id = message?.data?.id; - let rpcMethod = this.rpcQueueManager.getId(id); - const isConnectionResponse = rpcMethod === RPC_METHODS.ETH_REQUESTACCOUNTS; - - if (isConnectionResponse) { - const preferencesController = ( - Engine.context as { - PreferencesController: PreferencesController; - } - ).PreferencesController; - - const selectedAddress = - preferencesController.state.selectedAddress.toLowerCase(); - - const lowercaseAccounts = (message.data.result as string[]).map( - (a: string) => a.toLowerCase(), - ); - - const isPartOfConnectedAddresses = - lowercaseAccounts.includes(selectedAddress); - - if (isPartOfConnectedAddresses) { - // Remove the selectedAddress from the lowercaseAccounts if it exists - const remainingAccounts = lowercaseAccounts.filter( - (account) => account !== selectedAddress, - ); - - // Create the reorderedAccounts array with selectedAddress as the first element - const reorderedAccounts: string[] = [ - selectedAddress, - ...remainingAccounts, - ]; - - message = { - ...message, - data: { - ...message.data, - result: reorderedAccounts, - }, - }; - } - } - - this.communicationClient.sendMessage(JSON.stringify(message)); - - DevLogger.log(`AndroidService::sendMessage method=${rpcMethod}`, message); - - // handle multichain rpc call responses separately - const chainRPCs = this.batchRPCManager.getById(id); - if (chainRPCs) { - const isLastRpcOrError = await handleBatchRpcResponse({ - chainRpcs: chainRPCs, - msg: message, - backgroundBridge: this.bridgeByClientId[this.currentClientId ?? ''], - batchRPCManager: this.batchRPCManager, - sendMessage: ({ msg }) => this.sendMessage(msg), - }); - DevLogger.log( - `AndroidService::sendMessage isLastRpc=${isLastRpcOrError}`, - chainRPCs, - ); - - if (!isLastRpcOrError) { - DevLogger.log( - `AndroidService::sendMessage NOT last rpc --- skip goBack()`, - chainRPCs, - ); - this.rpcQueueManager.remove(id); - // Only continue processing the message and goback if all rpcs in the batch have been handled - return; - } - - // Always set the method to metamask_batch otherwise it may not have been set correctly because of the batch rpc flow. - rpcMethod = RPC_METHODS.METAMASK_BATCH; - DevLogger.log( - `AndroidService::sendMessage chainRPCs=${chainRPCs} COMPLETED!`, - ); - } - - this.rpcQueueManager.remove(id); - - if (!rpcMethod && forceRedirect !== true) { - DevLogger.log( - `AndroidService::sendMessage no rpc method --- rpcMethod=${rpcMethod} forceRedirect=${forceRedirect} --- skip goBack()`, - ); - return; - } - - try { - if (METHODS_TO_DELAY[rpcMethod]) { - // Add delay to see the feedback modal - await wait(1000); - } - - if (!this.rpcQueueManager.isEmpty()) { - DevLogger.log( - `AndroidService::sendMessage NOT empty --- skip goBack()`, - this.rpcQueueManager.get(), - ); - return; - } - - DevLogger.log(`AndroidService::sendMessage empty --- goBack()`); - Minimizer.goBack(); - } catch (error) { - Logger.log(error, `AndroidService:: error waiting for empty rpc queue`); - } + return sendMessage(this, message, forceRedirect); } } diff --git a/app/core/SDKConnect/AndroidSDK/AndroidService/sendMessage.test.ts b/app/core/SDKConnect/AndroidSDK/AndroidService/sendMessage.test.ts new file mode 100644 index 00000000000..647b57884eb --- /dev/null +++ b/app/core/SDKConnect/AndroidSDK/AndroidService/sendMessage.test.ts @@ -0,0 +1,131 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import Logger from '../../../../util/Logger'; +import Engine from '../../../Engine'; +import { Minimizer } from '../../../NativeModules'; +import { RPC_METHODS } from '../../SDKConnectConstants'; +import handleBatchRpcResponse from '../../handlers/handleBatchRpcResponse'; +import { wait } from '../../utils/wait.util'; +import AndroidService from '../AndroidService'; +import sendMessage from './sendMessage'; + +jest.mock('../../../Engine'); +jest.mock('../../../NativeModules', () => ({ + Minimizer: { + goBack: jest.fn(), + }, +})); +jest.mock('../../../../util/Logger'); +jest.mock('../../utils/wait.util', () => ({ + wait: jest.fn().mockResolvedValue(undefined), +})); +jest.mock('@metamask/preferences-controller'); +jest.mock('../AndroidService'); +jest.mock('../../handlers/handleBatchRpcResponse', () => jest.fn()); +jest.mock('../../utils/DevLogger'); + +describe('sendMessage', () => { + let instance: jest.Mocked; + let message: any; + + const mockGetId = jest.fn(); + const mockRemove = jest.fn(); + const mockIsEmpty = jest.fn().mockReturnValue(true); + const mockGet = jest.fn(); + const mockSendMessage = jest.fn(); + const mockGetById = jest.fn(); + + beforeEach(() => { + jest.clearAllMocks(); + + instance = { + rpcQueueManager: { + getId: mockGetId, + remove: mockRemove, + isEmpty: mockIsEmpty, + get: mockGet, + }, + communicationClient: { + sendMessage: mockSendMessage, + }, + batchRPCManager: { + getById: mockGetById, + }, + bridgeByClientId: {}, + currentClientId: 'test-client-id', + } as unknown as jest.Mocked; + + message = { + data: { + id: 'test-id', + result: ['0x1', '0x2'], + }, + }; + + (Engine.context as any) = { + PreferencesController: { + state: { + selectedAddress: '0x1', + }, + }, + }; + }); + + it('should send message with reordered accounts if selectedAddress is in result', async () => { + mockGetId.mockReturnValue(RPC_METHODS.ETH_REQUESTACCOUNTS); + + await sendMessage(instance, message); + + expect(mockSendMessage).toHaveBeenCalledWith( + JSON.stringify({ + ...message, + data: { + ...message.data, + result: ['0x1', '0x2'], + }, + }), + ); + }); + + it('should send message without reordering if selectedAddress is not in result', async () => { + (Engine.context as any).PreferencesController.state.selectedAddress = '0x3'; + + mockGetId.mockReturnValue(RPC_METHODS.ETH_REQUESTACCOUNTS); + + await sendMessage(instance, message); + + expect(mockSendMessage).toHaveBeenCalledWith(JSON.stringify(message)); + }); + + it('should handle multichain rpc call responses separately', async () => { + mockGetId.mockReturnValue('someMethod'); + mockGetById.mockReturnValue(['rpc1', 'rpc2']); + (handleBatchRpcResponse as jest.Mock).mockResolvedValue(true); + + await sendMessage(instance, message); + + expect(handleBatchRpcResponse).toHaveBeenCalled(); + expect(mockRemove).toHaveBeenCalledWith('test-id'); + expect(mockSendMessage).toHaveBeenCalledWith(JSON.stringify(message)); + }); + + it('should not call goBack if rpcQueueManager is not empty', async () => { + mockGetId.mockReturnValue('someMethod'); + mockIsEmpty.mockReturnValue(false); + + await sendMessage(instance, message); + + expect(Minimizer.goBack).not.toHaveBeenCalled(); + }); + + it('should handle error when waiting for empty rpc queue', async () => { + mockGetId.mockReturnValue('someMethod'); + (wait as jest.Mock).mockRejectedValue(new Error('test error')); + + await sendMessage(instance, message); + + expect(Logger.log).toHaveBeenCalledWith( + expect.any(Error), + `AndroidService:: error waiting for empty rpc queue`, + ); + }); +}); diff --git a/app/core/SDKConnect/AndroidSDK/AndroidService/sendMessage.ts b/app/core/SDKConnect/AndroidSDK/AndroidService/sendMessage.ts new file mode 100644 index 00000000000..b795c789d0b --- /dev/null +++ b/app/core/SDKConnect/AndroidSDK/AndroidService/sendMessage.ts @@ -0,0 +1,128 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import Engine from '../../../Engine'; +import { Minimizer } from '../../../NativeModules'; +import Logger from '../../../../util/Logger'; +import { wait } from '../../utils/wait.util'; +import { PreferencesController } from '@metamask/preferences-controller'; +import AndroidService from '../AndroidService'; +import { METHODS_TO_DELAY, RPC_METHODS } from '../../SDKConnectConstants'; +import handleBatchRpcResponse from '../../handlers/handleBatchRpcResponse'; +import DevLogger from '../../utils/DevLogger'; + +async function sendMessage( + instance: AndroidService, + message: any, + forceRedirect?: boolean, +) { + const id = message?.data?.id; + let rpcMethod = instance.rpcQueueManager.getId(id); + + const isConnectionResponse = rpcMethod === RPC_METHODS.ETH_REQUESTACCOUNTS; + + if (isConnectionResponse) { + const preferencesController = ( + Engine.context as { + PreferencesController: PreferencesController; + } + ).PreferencesController; + + const selectedAddress = + preferencesController.state.selectedAddress.toLowerCase(); + + const lowercaseAccounts = (message.data.result as string[]).map( + (a: string) => a.toLowerCase(), + ); + + const isPartOfConnectedAddresses = + lowercaseAccounts.includes(selectedAddress); + + if (isPartOfConnectedAddresses) { + // Remove the selectedAddress from the lowercaseAccounts if it exists + const remainingAccounts = lowercaseAccounts.filter( + (account) => account !== selectedAddress, + ); + + // Create the reorderedAccounts array with selectedAddress as the first element + const reorderedAccounts: string[] = [ + selectedAddress, + ...remainingAccounts, + ]; + + message = { + ...message, + data: { + ...message.data, + result: reorderedAccounts, + }, + }; + } + } + + instance.communicationClient.sendMessage(JSON.stringify(message)); + + DevLogger.log(`AndroidService::sendMessage method=${rpcMethod}`, message); + + // handle multichain rpc call responses separately + const chainRPCs = instance.batchRPCManager.getById(id); + if (chainRPCs) { + const isLastRpcOrError = await handleBatchRpcResponse({ + chainRpcs: chainRPCs, + msg: message, + backgroundBridge: + instance.bridgeByClientId[instance.currentClientId ?? ''], + batchRPCManager: instance.batchRPCManager, + sendMessage: ({ msg }) => instance.sendMessage(msg), + }); + DevLogger.log( + `AndroidService::sendMessage isLastRpc=${isLastRpcOrError}`, + chainRPCs, + ); + + if (!isLastRpcOrError) { + DevLogger.log( + `AndroidService::sendMessage NOT last rpc --- skip goBack()`, + chainRPCs, + ); + instance.rpcQueueManager.remove(id); + // Only continue processing the message and goback if all rpcs in the batch have been handled + return; + } + + // Always set the method to metamask_batch otherwise it may not have been set correctly because of the batch rpc flow. + rpcMethod = RPC_METHODS.METAMASK_BATCH; + DevLogger.log( + `AndroidService::sendMessage chainRPCs=${chainRPCs} COMPLETED!`, + ); + } + + instance.rpcQueueManager.remove(id); + + if (!rpcMethod && forceRedirect !== true) { + DevLogger.log( + `AndroidService::sendMessage no rpc method --- rpcMethod=${rpcMethod} forceRedirect=${forceRedirect} --- skip goBack()`, + ); + return; + } + + try { + if (METHODS_TO_DELAY[rpcMethod]) { + // Add delay to see the feedback modal + await wait(1000); + } + + if (!instance.rpcQueueManager.isEmpty()) { + DevLogger.log( + `AndroidService::sendMessage NOT empty --- skip goBack()`, + instance.rpcQueueManager.get(), + ); + return; + } + + DevLogger.log(`AndroidService::sendMessage empty --- goBack()`); + Minimizer.goBack(); + } catch (error) { + Logger.log(error, `AndroidService:: error waiting for empty rpc queue`); + } +} + +export default sendMessage;