From 8e245212dddb126944457c0f0bdc57e2510a3ef0 Mon Sep 17 00:00:00 2001
From: Bruno Barbieri <1247834+brunobar79@users.noreply.github.com>
Date: Tue, 10 Sep 2024 20:57:55 +0300
Subject: [PATCH] Fix wrong messages during MWP Flow (#6094)

* add logging for pkey errors

* fix keychain error handling

* checksum address before keychain lookup

* remove invalid chainId param from tx

* Update src/keychain/index.ts

* Update src/keychain/index.ts

Co-authored-by: Jin <jin.chung17@gmail.com>

* Explicitly return UserCanceled error for code strings 10 and 13

* Remove returning -3 explicit error code

* Use enum values instead of primitives for error codes for readability

---------

Co-authored-by: Jin <jin.chung17@gmail.com>
(cherry picked from commit 82dca430d0e092dea431c9eff0fec214f2c7dcdc)
---
 src/keychain/index.ts                | 24 ++++++++-----
 src/model/wallet.ts                  | 52 +++++++++++++++++++++-------
 src/screens/SignTransactionSheet.tsx |  3 +-
 src/utils/ethereumUtils.ts           |  2 ++
 4 files changed, 59 insertions(+), 22 deletions(-)

diff --git a/src/keychain/index.ts b/src/keychain/index.ts
index 0ef3d78771a..782d3f06a8d 100644
--- a/src/keychain/index.ts
+++ b/src/keychain/index.ts
@@ -115,6 +115,7 @@ export async function get(key: string, options: KeychainOptions = {}): Promise<R
             error: e.toString(),
           },
         });
+
         switch (e.toString()) {
           /*
            * Can happen if the user initially had biometrics enabled, installed
@@ -170,14 +171,21 @@ export async function get(key: string, options: KeychainOptions = {}): Promise<R
             return _get(attempts + 1);
           }
           default: {
-            logger.error(new RainbowError(`[keychain]: _get() handled unknown error`), {
-              message: e.toString(),
-            });
-
-            return {
-              value: undefined,
-              error: ErrorType.Unknown,
-            };
+            // Avoid logging user cancelled operations
+            if (e.toString().includes('code: 10') || e.toString().includes('code: 13')) {
+              return {
+                value: undefined,
+                error: ErrorType.UserCanceled,
+              };
+            } else {
+              logger.error(new RainbowError(`[keychain]: _get() handled unknown error`), {
+                message: e.toString(),
+              });
+              return {
+                value: undefined,
+                error: ErrorType.Unknown,
+              };
+            }
           }
         }
       }
diff --git a/src/model/wallet.ts b/src/model/wallet.ts
index d35c72f8b48..c4bc99c6518 100644
--- a/src/model/wallet.ts
+++ b/src/model/wallet.ts
@@ -289,7 +289,10 @@ export const loadWallet = async <S extends Screen>({
     privateKey = await loadPrivateKey(addressToUse, isHardwareWallet);
   }
 
-  if (privateKey === -1 || privateKey === -2) {
+  // kc.ErrorType.UserCanceled means the user cancelled, so we don't wanna do anything
+  // kc.ErrorType.NotAuthenticated means the user is not authenticated (maybe removed biometrics).
+  //    In this case we show an alert inside loadPrivateKey
+  if (privateKey === kc.ErrorType.UserCanceled || privateKey === kc.ErrorType.NotAuthenticated) {
     return null;
   }
   if (isHardwareWalletKey(privateKey)) {
@@ -536,7 +539,10 @@ export const oldLoadSeedPhrase = async (): Promise<null | EthereumWalletSeed> =>
 
 export const loadAddress = (): Promise<null | EthereumAddress> => keychain.loadString(addressKey) as Promise<string | null>;
 
-export const loadPrivateKey = async (address: EthereumAddress, hardware: boolean): Promise<null | EthereumPrivateKey | -1 | -2> => {
+export const loadPrivateKey = async (
+  address: EthereumAddress,
+  hardware: boolean
+): Promise<null | EthereumPrivateKey | kc.ErrorType.UserCanceled | kc.ErrorType.NotAuthenticated> => {
   try {
     const isSeedPhraseMigrated = await keychain.loadString(oldSeedPhraseMigratedKey);
 
@@ -550,8 +556,8 @@ export const loadPrivateKey = async (address: EthereumAddress, hardware: boolean
 
     if (!privateKey) {
       const privateKeyData = await getKeyForWallet(address, hardware);
-      if (privateKeyData === -1) {
-        return -1;
+      if (privateKeyData === kc.ErrorType.UserCanceled || privateKeyData === kc.ErrorType.NotAuthenticated) {
+        return privateKeyData;
       }
       privateKey = privateKeyData?.privateKey ?? null;
     }
@@ -911,9 +917,12 @@ export const saveKeyForWallet = async (
  * @desc Gets wallet keys for the given address depending wallet type
  * @param address The wallet address.
  * @param hardware If the wallet is a hardware wallet.
- * @return null | PrivateKeyData | -1
+ * @return null | PrivateKeyData | kc.ErrorType.UserCanceled | kc.ErrorType.NotAuthenticated
  */
-export const getKeyForWallet = async (address: EthereumAddress, hardware: boolean): Promise<null | PrivateKeyData | -1> => {
+export const getKeyForWallet = async (
+  address: EthereumAddress,
+  hardware: boolean
+): Promise<null | PrivateKeyData | kc.ErrorType.UserCanceled | kc.ErrorType.NotAuthenticated> => {
   if (hardware) {
     return await getHardwareKey(address);
   } else {
@@ -971,9 +980,11 @@ export const saveHardwareKey = async (
 /**
  * @desc Gets wallet private key for a given address.
  * @param address The wallet address.
- * @return null | PrivateKeyData | -1
+ * @return null | PrivateKeyData | kc.ErrorType.UserCanceled | kc.ErrorType.NotAuthenticated
  */
-export const getPrivateKey = async (address: EthereumAddress): Promise<null | PrivateKeyData | -1> => {
+export const getPrivateKey = async (
+  address: EthereumAddress
+): Promise<null | PrivateKeyData | kc.ErrorType.UserCanceled | kc.ErrorType.NotAuthenticated> => {
   try {
     const key = `${address}_${privateKeyKey}`;
     const options = { authenticationPrompt };
@@ -984,11 +995,26 @@ export const getPrivateKey = async (address: EthereumAddress): Promise<null | Pr
       androidEncryptionPin,
     });
 
-    if (error === -2) {
-      Alert.alert(lang.t('wallet.authenticate.alert.error'), lang.t('wallet.authenticate.alert.current_authentication_not_secure_enough'));
-      return null;
+    switch (error) {
+      case kc.ErrorType.UserCanceled:
+        // User Cancelled - We want to bubble up this error code. No need to track it.
+        return kc.ErrorType.UserCanceled;
+      case kc.ErrorType.NotAuthenticated:
+        // Alert the user and bubble up the error code.
+        Alert.alert(
+          lang.t('wallet.authenticate.alert.error'),
+          lang.t('wallet.authenticate.alert.current_authentication_not_secure_enough')
+        );
+        return kc.ErrorType.NotAuthenticated;
+      case kc.ErrorType.Unavailable:
+        // This means we couldn't find any matches for this key.
+        logger.error(new RainbowError('KC unavailable for PKEY lookup'), { error });
+        break;
+      default:
+        // This is an unknown error
+        logger.error(new RainbowError('KC unknown error for PKEY lookup'), { error });
+        break;
     }
-
     return pkey || null;
   } catch (error) {
     logger.error(new RainbowError('[wallet]: Error in getPrivateKey'), { error });
@@ -1043,7 +1069,7 @@ export const getSeedPhrase = async (
       androidEncryptionPin,
     });
 
-    if (error === -2) {
+    if (error === kc.ErrorType.NotAuthenticated) {
       Alert.alert(lang.t('wallet.authenticate.alert.error'), lang.t('wallet.authenticate.alert.current_authentication_not_secure_enough'));
       return null;
     }
diff --git a/src/screens/SignTransactionSheet.tsx b/src/screens/SignTransactionSheet.tsx
index 7fd728f1b3a..a558fc97067 100644
--- a/src/screens/SignTransactionSheet.tsx
+++ b/src/screens/SignTransactionSheet.tsx
@@ -72,6 +72,7 @@ import { useNonceForDisplay } from '@/hooks/useNonceForDisplay';
 import { useProviderSetup } from '@/hooks/useProviderSetup';
 import { useTransactionSubmission } from '@/hooks/useSubmitTransaction';
 import { useConfirmTransaction } from '@/hooks/useConfirmTransaction';
+import { toChecksumAddress } from 'ethereumjs-util';
 
 type SignTransactionSheetParams = {
   transactionDetails: RequestData;
@@ -332,7 +333,7 @@ export const SignTransactionSheet = () => {
         screen: SCREEN_FOR_REQUEST_SOURCE[source],
         operation: TimeToSignOperation.KeychainRead,
       })({
-        address: accountInfo.address,
+        address: toChecksumAddress(accountInfo.address),
         provider: providerToUse,
         timeTracking: {
           screen: SCREEN_FOR_REQUEST_SOURCE[source],
diff --git a/src/utils/ethereumUtils.ts b/src/utils/ethereumUtils.ts
index 9d96405b93f..3ce5b2ae24d 100644
--- a/src/utils/ethereumUtils.ts
+++ b/src/utils/ethereumUtils.ts
@@ -501,6 +501,8 @@ const calculateL1FeeOptimism = async (tx: RainbowTransaction, provider: Provider
       newTx.nonce = Number(await provider.getTransactionCount(newTx.from));
     }
 
+    // @ts-expect-error operand should be optional
+    delete newTx?.chainId;
     // @ts-expect-error operand should be optional
     delete newTx?.from;
     // @ts-expect-error gas is not in type RainbowTransaction