fix: error when re-using exising id on permissionController (#9309)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

This PR fixes an issue with incorrect use of the permissionController
which doesn't allow re-using similar ID to create a permission request.

If a connection sends multiple request accounts, it would endup
rejecting the connection and all further requests until the app is
restarted.

Because the permissionController doesn't allow passing metadata, the
workaround was to use the id field to link back to metadata but it
eventually created the issue described above.

Future version of the permissionController have been patched from
MetaMask/core#4179

## **Related issues**

Fixes: #9308

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

app/components/Views/AccountConnect/AccountConnect.tsx

@@ -67,6 +67,7 @@ import {
 import AccountConnectMultiSelector from './AccountConnectMultiSelector';
 import AccountConnectSingle from './AccountConnectSingle';
 import AccountConnectSingleSelector from './AccountConnectSingleSelector';
+import DevLogger from '../../../core/SDKConnect/utils/DevLogger';
 const createStyles = () =>
   StyleSheet.create({
     fullScreenModal: {
@@ -109,7 +110,9 @@ const AccountConnect = (props: AccountConnectProps) => {
       : AvatarAccountType.JazzIcon,
   );
 
-  const { id: channelId, origin: metadataOrigin } = hostInfo.metadata as {
+  // on inappBrowser: hostname
+  // on sdk or walletconnect: channelId
+  const { origin: channelIdOrHostname } = hostInfo.metadata as {
     id: string;
     origin: string;
   };
@@ -120,7 +123,9 @@ const AccountConnect = (props: AccountConnectProps) => {
   const [hostname, setHostname] = useState<string>(origin);
 
   const urlWithProtocol = prefixUrlWithProtocol(hostname);
-  const sdkConnection = SDKConnect.getInstance().getConnection({ channelId });
+  const sdkConnection = SDKConnect.getInstance().getConnection({
+    channelId: channelIdOrHostname,
+  });
   // Last wallet connect session metadata
   const wc2Metadata = useSelector((state: RootState) => state.sdk.wc2Metadata);
 
@@ -175,21 +180,24 @@ const AccountConnect = (props: AccountConnectProps) => {
     // walletconnect channelId format: 1713357238460272
     // sdk channelId format: uuid
     // inappbrowser channelId format: app.uniswap.io
-
+    DevLogger.log(
+      `AccountConnect::loadHostname hostname=${hostname} origin=${origin} metadataOrigin=${channelIdOrHostname}`,
+      sdkConnection,
+    );
     // check if channelId contains dot, it comes from in-app browser and we can use it.
-    if (channelId.indexOf('.') !== -1) {
+    if (channelIdOrHostname.indexOf('.') !== -1) {
       return origin;
     }
 
     if (sdkConnection) {
       const _hostname = (
-        sdkConnection?.originatorInfo?.url ?? metadataOrigin
+        sdkConnection?.originatorInfo?.url ?? channelIdOrHostname
       ).replace(AppConstants.MM_SDK.SDK_REMOTE_ORIGIN, '');
       return _hostname;
     }
 
-    return wc2Metadata?.url ?? channelId;
-  }, [channelId, metadataOrigin, sdkConnection, origin, wc2Metadata]);
+    return wc2Metadata?.url ?? channelIdOrHostname;
+  }, [hostname, channelIdOrHostname, sdkConnection, origin, wc2Metadata]);
 
   // Retrieve hostname info based on channelId
   useEffect(() => {
@@ -212,10 +220,10 @@ const AccountConnect = (props: AccountConnectProps) => {
   const cancelPermissionRequest = useCallback(
     (requestId) => {
       Engine.context.PermissionController.rejectPermissionsRequest(requestId);
-      if (channelId && accountsLength === 0) {
+      if (channelIdOrHostname && accountsLength === 0) {
         // Remove Potential SDK connection
         SDKConnect.getInstance().removeChannel({
-          channelId,
+          channelId: channelIdOrHostname,
           sendTerminate: true,
         });
       }
@@ -228,7 +236,7 @@ const AccountConnect = (props: AccountConnectProps) => {
     [
       Engine.context.PermissionController,
       accountsLength,
-      channelId,
+      channelIdOrHostname,
       trackEvent,
     ],
   );
@@ -288,7 +296,7 @@ const AccountConnect = (props: AccountConnectProps) => {
       ...hostInfo,
       metadata: {
         ...hostInfo.metadata,
-        origin: metadataOrigin,
+        origin: channelIdOrHostname,
       },
       approvedAccounts: selectedAccounts,
     };
@@ -351,7 +359,7 @@ const AccountConnect = (props: AccountConnectProps) => {
     Engine.context.PermissionController,
     toastRef,
     accountsLength,
-    metadataOrigin,
+    channelIdOrHostname,
     triggerDappViewedEvent,
     trackEvent,
   ]);

app/core/RPCMethods/RPCMethodMiddleware.ts

@@ -120,25 +120,16 @@ export const checkActiveAccountAndChainId = async ({
       hostname,
       formattedAddress,
     });
-    const validHostname = hostname?.replace(
-      AppConstants.MM_SDK.SDK_REMOTE_ORIGIN,
-      '',
-    );
 
     const permissionsController = (
       Engine.context as { PermissionController: PermissionController<any, any> }
     ).PermissionController;
     DevLogger.log(
-      `checkActiveAccountAndChainId channelId=${channelId} isWalletConnect=${isWalletConnect} validHostname=${validHostname}`,
+      `checkActiveAccountAndChainId channelId=${channelId} isWalletConnect=${isWalletConnect} hostname=${hostname}`,
       permissionsController.state,
     );
 
-    let accounts: string[] = [];
-    if (isWalletConnect) {
-      accounts = await getPermittedAccounts(validHostname);
-    } else {
-      accounts = (await getPermittedAccounts(channelId ?? validHostname)) ?? [];
-    }
+    const accounts = (await getPermittedAccounts(channelId ?? hostname)) ?? [];
 
     const normalizedAccounts = accounts.map(safeToChecksumAddress);
 
@@ -294,22 +285,18 @@ export const getRpcMethodMiddleware = ({
   injectHomePageScripts,
   // For analytics
   analytics,
-}: RPCMethodsMiddleParameters) =>
+}: RPCMethodsMiddleParameters) => {
+  // Make sure to always have the correct origin
+  hostname = hostname.replace(AppConstants.MM_SDK.SDK_REMOTE_ORIGIN, '');
+  DevLogger.log(
+    `getRpcMethodMiddleware hostname=${hostname} channelId=${channelId}`,
+  );
   // all user facing RPC calls not implemented by the provider
-  createAsyncMiddleware(async (req: any, res: any, next: any) => {
+  return createAsyncMiddleware(async (req: any, res: any, next: any) => {
     // Used by eth_accounts and eth_coinbase RPCs.
     const getEthAccounts = async () => {
-      let accounts: string[] = [];
-      const validHostname = hostname.replace(
-        AppConstants.MM_SDK.SDK_REMOTE_ORIGIN,
-        '',
-      );
-      if (isMMSDK) {
-        accounts =
-          (await getPermittedAccounts(channelId ?? validHostname)) ?? [];
-      } else {
-        accounts = await getPermittedAccounts(validHostname);
-      }
+      const accounts: string[] =
+        (await getPermittedAccounts(channelId ?? hostname)) ?? [];
       res.result = accounts;
     };
 
@@ -388,7 +375,7 @@ export const getRpcMethodMiddleware = ({
               getPermissionsForOrigin:
                 Engine.context.PermissionController.getPermissions.bind(
                   Engine.context.PermissionController,
-                  hostname,
+                  channelId ?? hostname,
                 ),
             },
           );
@@ -398,14 +385,6 @@ export const getRpcMethodMiddleware = ({
         }),
       wallet_requestPermissions: async () =>
         new Promise<any>((resolve, reject) => {
-          let requestId: string | undefined;
-          if (isMMSDK) {
-            // Extract id from hostname
-            requestId = hostname.replace(
-              AppConstants.MM_SDK.SDK_REMOTE_ORIGIN,
-              '',
-            );
-          }
           requestPermissionsHandler
             .implementation(
               req,
@@ -421,9 +400,8 @@ export const getRpcMethodMiddleware = ({
                 requestPermissionsForOrigin:
                   Engine.context.PermissionController.requestPermissions.bind(
                     Engine.context.PermissionController,
-                    { origin: requestId ?? hostname },
+                    { origin: channelId ?? hostname },
                     req.params[0],
-                    { id: requestId },
                   ),
               },
             )
@@ -486,43 +464,21 @@ export const getRpcMethodMiddleware = ({
       },
       eth_requestAccounts: async () => {
         const { params } = req;
-        const validHostname = hostname.replace(
-          AppConstants.MM_SDK.SDK_REMOTE_ORIGIN,
-          '',
-        );
         const permittedAccounts = await getPermittedAccounts(
-          channelId ?? validHostname,
+          channelId ?? hostname,
         );
 
         if (!params?.force && permittedAccounts.length) {
           res.result = permittedAccounts;
         } else {
           try {
             checkTabActive();
-            const currentPerm =
-              Engine.context.PermissionController.getPermissions(
-                channelId ?? validHostname,
-              );
-            const accountPerm =
-              Engine.context.PermissionController.getPermission(
-                channelId ?? validHostname,
-                'eth_accounts',
-              );
-            DevLogger.log(
-              `eth_requestAccounts currentPerm ${channelId ?? validHostname}`,
-              currentPerm,
-              accountPerm,
-            );
             await Engine.context.PermissionController.requestPermissions(
-              { origin: channelId ?? validHostname },
+              { origin: channelId ?? hostname },
               { eth_accounts: {} },
-              {
-                id: channelId ?? validHostname,
-                preserveExistingPermissions: true,
-              },
             );
             DevLogger.log(`eth_requestAccounts requestPermissions`);
-            const acc = await getPermittedAccounts(hostname);
+            const acc = await getPermittedAccounts(channelId ?? hostname);
             DevLogger.log(`eth_requestAccounts getPermittedAccounts`, acc);
             res.result = acc;
           } catch (error) {
@@ -541,16 +497,6 @@ export const getRpcMethodMiddleware = ({
       eth_sendTransaction: async () => {
         checkTabActive();
 
-        if (isMMSDK) {
-          // Append origin to the request so it can be parsed in UI TransactionHeader
-          DevLogger.log(
-            `SDK Transaction detected --- custom hostname -- ${hostname} --> ${
-              AppConstants.MM_SDK.SDK_REMOTE_ORIGIN + url.current
-            }`,
-          );
-          hostname = AppConstants.MM_SDK.SDK_REMOTE_ORIGIN + url.current;
-        }
-
         return RPCMethods.eth_sendTransaction({
           hostname,
           req,
@@ -658,6 +604,7 @@ export const getRpcMethodMiddleware = ({
           isWalletConnect,
         });
 
+        DevLogger.log(`personal_sign`, params, pageMeta, hostname);
         PPOMUtil.validateRequest(req);
 
         const rawSig = await SignatureController.newUnsignedPersonalMessage({
@@ -890,16 +837,7 @@ export const getRpcMethodMiddleware = ({
        */
       metamask_getProviderState: async () => {
         let accounts: string[] = [];
-        const validHostname = hostname.replace(
-          AppConstants.MM_SDK.SDK_REMOTE_ORIGIN,
-          '',
-        );
-        if (isMMSDK) {
-          accounts =
-            (await getPermittedAccounts(channelId ?? validHostname)) ?? [];
-        } else {
-          accounts = await getPermittedAccounts(validHostname);
-        }
+        accounts = (await getPermittedAccounts(channelId ?? hostname)) ?? [];
         res.result = {
           ...getProviderState(),
           accounts,
@@ -971,4 +909,5 @@ export const getRpcMethodMiddleware = ({
       throw e;
     }
   });
+};
 export default getRpcMethodMiddleware;

app/core/SDKConnect/AndroidSDK/AndroidService.ts

@@ -256,7 +256,6 @@ export default class AndroidService extends EventEmitter2 {
     return permissionsController.requestPermissions(
       { origin: channelId },
       { eth_accounts: {} },
-      { id: channelId },
     );
   }
 

app/core/SDKConnect/ConnectionManagement/reconnect.ts

@@ -91,11 +91,11 @@ async function reconnect({
     //   instance.removeChannel(channelId, true);
     // }
 
-    // instance condition should not happen keeping it for debug purpose.
-    console.warn(`Priotity to deeplink - overwrite previous connection`);
-    instance.removeChannel({ channelId, sendTerminate: true });
+    // issue can happen during dev because bundle takes too long to load via metro.
+    // should not happen but keeping it for reference / debug purpose.
+    console.warn(`BUNDLE WARNING: Already connecting --- Priotity to deeplink`);
+    // instance.removeChannel({ channelId, sendTerminate: true });
   }
-
   if (!instance.state.connections[channelId]) {
     interruptReason = 'no connection';
   }
@@ -130,7 +130,9 @@ async function reconnect({
     }
   }
 
-  DevLogger.log(`SDKConnect::reconnect - starting reconnection`);
+  DevLogger.log(
+    `SDKConnect::reconnect - starting reconnection channel=${channelId}`,
+  );
 
   const connection = instance.state.connections[channelId];
   instance.state.connecting[channelId] = true;

app/core/SDKConnect/SDKDeeplinkProtocol/DeeplinkProtocolService.ts

@@ -277,7 +277,6 @@ export default class DeeplinkProtocolService {
     return permissionsController.requestPermissions(
       { origin: channelId },
       { eth_accounts: {} },
-      { id: channelId },
     );
   }