improve personal sign compatibility (#730)

rainbow-me · Jul 8, 2023 · b87870f · b87870f
1 parent bb8a919
commit b87870f
Show file tree

Hide file tree

Showing 7 changed files with 61 additions and 43 deletions.
diff --git a/e2e/helpers.ts b/e2e/helpers.ts
@@ -404,3 +404,24 @@ export async function delayTime(
       return await delay(5000);
   }
 }
+
+export async function awaitTextChange(
+  id: string,
+  text: string,
+  driver: WebDriver,
+) {
+  try {
+    const element = await findElementById({
+      id: id,
+      driver,
+    });
+
+    await driver.wait(until.elementTextIs(element, text), waitUntilTime);
+  } catch (error) {
+    console.error(
+      `Error occurred while awaiting text change for element with ID '${id}':`,
+      error,
+    );
+    throw error;
+  }
+}
diff --git a/e2e/serial/1_swapFlow1.test.ts b/e2e/serial/1_swapFlow1.test.ts
@@ -9,6 +9,7 @@ import { WebDriver } from 'selenium-webdriver';
 import { afterAll, beforeAll, expect, it } from 'vitest';
 
 import {
+  delay,
   delayTime,
   doNotFindElementByTestId,
   fillPrivateKey,
@@ -968,9 +969,14 @@ it('should be able to execute swap', async () => {
   await delayTime('medium');
   await findElementByTestIdAndClick({ id: 'swap-review-execute', driver });
   await delayTime('very-long');
+  // Adding delay to make sure the provider gets the balance after the swap
+  // Because CI is slow so this triggers a race condition most of the time.
+  await delay(5000);
   const ethBalanceAfterSwap = await provider.getBalance(
     TEST_VARIABLES.SEED_WALLET.ADDRESS,
   );
+  console.log('ethBalanceBeforeSwap', ethBalanceBeforeSwap.toString());
+  console.log('ethBalanceAfterSwap', ethBalanceAfterSwap.toString());
   const balanceDifference = subtract(
     ethBalanceBeforeSwap.toString(),
     ethBalanceAfterSwap.toString(),

diff --git a/e2e/serial/5_dappInteractionFlow.test.ts b/e2e/serial/5_dappInteractionFlow.test.ts
@@ -1,9 +1,11 @@
 import 'chromedriver';
 import 'geckodriver';
+import { getAddress } from '@ethersproject/address';
 import { WebDriver } from 'selenium-webdriver';
 import { afterAll, beforeAll, describe, expect, it } from 'vitest';
 
 import {
+  awaitTextChange,
   delayTime,
   fillPrivateKey,
   findElementById,
@@ -126,7 +128,9 @@ describe('App interactions flow', () => {
     expect(accounts).toBeTruthy();
 
     const connectedAddress = await accounts.getText();
-    expect(connectedAddress).toBe(TEST_VARIABLES.SEED_WALLET.ADDRESS);
+    expect(getAddress(connectedAddress)).toBe(
+      getAddress(TEST_VARIABLES.SEED_WALLET.ADDRESS),
+    );
   });
 
   it('should be able to complete a personal sign', async () => {
@@ -543,12 +547,7 @@ describe('App interactions flow', () => {
     await driver.switchTo().window(dappHandler);
     await delayTime('very-long');
 
-    const confirmation = await findElementById({
-      id: 'collectiblesStatus',
-      driver,
-    });
-    const confrimationText = await confirmation.getText();
-    expect(confrimationText).toBe('Deployed');
+    await awaitTextChange('collectiblesStatus', 'Deployed', driver);
   });
 
   it('should be able to mint a collectible', async () => {
@@ -574,13 +573,7 @@ describe('App interactions flow', () => {
 
     await driver.switchTo().window(dappHandler);
     await delayTime('very-long');
-
-    const confirmation = await findElementById({
-      id: 'collectiblesStatus',
-      driver,
-    });
-    const confrimationText = await confirmation.getText();
-    expect(confrimationText).toBe('Mint completed');
+    await awaitTextChange('collectiblesStatus', 'Mint completed', driver);
   });
 
   it('should be able to approve a collectible', async () => {
@@ -607,12 +600,7 @@ describe('App interactions flow', () => {
     await driver.switchTo().window(dappHandler);
     await delayTime('very-long');
 
-    const confirmation = await findElementById({
-      id: 'collectiblesStatus',
-      driver,
-    });
-    const confrimationText = await confirmation.getText();
-    expect(confrimationText).toBe('Approve completed');
+    await awaitTextChange('collectiblesStatus', 'Approve completed', driver);
   });
 
   it('should be able to set approval for all for a collectible', async () => {
@@ -639,12 +627,11 @@ describe('App interactions flow', () => {
     await driver.switchTo().window(dappHandler);
     await delayTime('very-long');
 
-    const confirmation = await findElementById({
-      id: 'collectiblesStatus',
+    await awaitTextChange(
+      'collectiblesStatus',
+      'Set Approval For All completed',
       driver,
-    });
-    const confrimationText = await confirmation.getText();
-    expect(confrimationText).toBe('Set Approval For All completed');
+    );
   });
 
   it('should be able to revoke approval for a collectible', async () => {
@@ -671,12 +658,7 @@ describe('App interactions flow', () => {
     await driver.switchTo().window(dappHandler);
     await delayTime('very-long');
 
-    const confirmation = await findElementById({
-      id: 'collectiblesStatus',
-      driver,
-    });
-    const confrimationText = await confirmation.getText();
-    expect(confrimationText).toBe('Revoke completed');
+    await awaitTextChange('collectiblesStatus', 'Revoke completed', driver);
   });
 
   it('should be able to transfer a collectible', async () => {
@@ -702,12 +684,10 @@ describe('App interactions flow', () => {
 
     await driver.switchTo().window(dappHandler);
     await delayTime('very-long');
-
-    const confirmation = await findElementById({
-      id: 'collectiblesStatus',
+    await awaitTextChange(
+      'collectiblesStatus',
+      'Transfer From completed',
       driver,
-    });
-    const confrimationText = await confirmation.getText();
-    expect(confrimationText).toBe('Transfer From completed');
+    );
   });
 });
diff --git a/src/core/utils/signMessages.tsx b/src/core/utils/signMessages.tsx
@@ -19,11 +19,19 @@ export const getSigningRequestDisplayDetails = (
     }
     case 'personal_sign': {
       let message = payload?.params?.[0] as string;
-      const address = payload?.params?.[1] as Address;
+      let address = payload?.params?.[1] as Address;
+      // While this is technically incorrect
+      // we'll support anyways for compatibility purposes
+      // since other wallets do it
+      if (isAddress(message)) {
+        message = payload?.params?.[1] as string;
+        address = payload?.params?.[0] as Address;
+      }
+
       try {
         const strippedMessage = isHexString(message)
           ? message.slice(2)
-          : message;
+          : `${Buffer.from(message, 'utf8').toString('hex')}`; // Some dapps send the message as a utf8 string
         const buffer = Buffer.from(strippedMessage, 'hex');
         message = buffer.length === 32 ? message : buffer.toString('utf8');
       } catch (error) {

diff --git a/src/entries/background/handlers/handleProviderRequest.ts b/src/entries/background/handlers/handleProviderRequest.ts
@@ -129,7 +129,9 @@ export const handleProviderRequest = ({
           break;
         }
         case 'eth_accounts': {
-          response = activeSession ? [activeSession.address] : [];
+          response = activeSession
+            ? [activeSession.address?.toLowerCase()]
+            : [];
           break;
         }
         case 'eth_sendTransaction':
@@ -220,7 +222,7 @@ export const handleProviderRequest = ({
         }
         case 'eth_requestAccounts': {
           if (activeSession) {
-            response = [activeSession.address];
+            response = [activeSession.address?.toLowerCase()];
             break;
           }
           const { address, chainId } = (await messengerProviderRequest(
@@ -238,7 +240,7 @@ export const handleProviderRequest = ({
             chainId,
             url,
           });
-          response = [address];
+          response = [address?.toLowerCase()];
           break;
         }
         case 'eth_blockNumber': {

diff --git a/src/entries/popup/pages/messages/ApproveAppRequest.tsx b/src/entries/popup/pages/messages/ApproveAppRequest.tsx
@@ -45,8 +45,8 @@ export const ApproveAppRequest = () => {
         Number(pendingRequest?.meta?.sender?.tab?.id)?.toString()
       ];
     if (pendingRequests.length <= 1 && notificationWindow?.id) {
+      notificationWindow?.id && chrome.windows.remove(notificationWindow?.id);
       setTimeout(() => {
-        notificationWindow?.id && chrome.windows.remove(notificationWindow?.id);
         navigate(ROUTES.HOME);
       }, 50);
     }

diff --git a/src/entries/popup/pages/messages/SignMessage/SignMessageInfo.tsx b/src/entries/popup/pages/messages/SignMessage/SignMessageInfo.tsx
@@ -83,6 +83,7 @@ export const SignMessageInfo = ({ request }: SignMessageProps) => {
               <Box
                 style={{
                   wordBreak: 'break-all',
+                  whiteSpace: 'pre-wrap',
                 }}
               >
                 <Text