fix(keychain): fix private key missing error (#5277)

### Description

As the title. I've done a detailed writeup
[here](https://linear.app/valora/issue/RET-1067/error-while-swapping-private-key-not-found#comment-3729279a).

### Test plan

As a user who knows their pincode, I should always be able to perform a
transaction and see my recovery phrase.

### Related issues

- Fixes RET-1067

### Backwards compatibility

Y

### Network scalability

Y

src/identity/actions.ts

@@ -33,6 +33,7 @@ export enum Actions {
   FETCH_ADDRESS_VERIFICATION_STATUS = 'IDENTITY/FETCH_ADDRESS_VERIFICATION_STATUS',
   ADDRESS_VERIFICATION_STATUS_RECEIVED = 'IDENTITY/ADDRESS_VERIFICATION_STATUS_RECEIVED',
   CONTACTS_SAVED = 'IDENTITY/CONTACTS_SAVED',
+  STORED_PASSWORD_REFRESHED = 'IDENTITY/STORED_PASSWORD_REFRESHED',
 }
 
 export interface SetHasSeenVerificationNux {
@@ -149,6 +150,10 @@ interface ContactsSavedAction {
   hash: string
 }
 
+interface StoredPasswordRefreshedAction {
+  type: Actions.STORED_PASSWORD_REFRESHED
+}
+
 export type ActionTypes =
   | SetHasSeenVerificationNux
   | UpdateE164PhoneNumberAddressesAction
@@ -170,6 +175,7 @@ export type ActionTypes =
   | FetchAddressVerificationAction
   | AddressVerificationStatusReceivedAction
   | ContactsSavedAction
+  | StoredPasswordRefreshedAction
 
 export const setHasSeenVerificationNux = (status: boolean): SetHasSeenVerificationNux => ({
   type: Actions.SET_SEEN_VERIFICATION_NUX,
@@ -333,3 +339,7 @@ export const contactsSaved = (hash: string): ContactsSavedAction => ({
   type: Actions.CONTACTS_SAVED,
   hash,
 })
+
+export const storedPasswordRefreshed = (): StoredPasswordRefreshedAction => ({
+  type: Actions.STORED_PASSWORD_REFRESHED,
+})

src/identity/reducer.ts

@@ -90,6 +90,7 @@ interface State {
   // Mapping of address to verification status; undefined entries represent a loading state
   addressToVerificationStatus: AddressToVerificationStatus
   lastSavedContactsHash: string | null
+  shouldRefreshStoredPasswordHash: boolean
 }
 
 const initialState: State = {
@@ -109,6 +110,7 @@ const initialState: State = {
   secureSendPhoneNumberMapping: {},
   addressToVerificationStatus: {},
   lastSavedContactsHash: null,
+  shouldRefreshStoredPasswordHash: false,
 }
 
 export const reducer = (
@@ -289,6 +291,11 @@ export const reducer = (
         ...state,
         lastSavedContactsHash: action.hash,
       }
+    case Actions.STORED_PASSWORD_REFRESHED:
+      return {
+        ...state,
+        shouldRefreshStoredPasswordHash: false,
+      }
     default:
       return state
   }

src/identity/selectors.ts

@@ -36,3 +36,6 @@ export const identifierToE164NumberSelector = createSelector(
 
 export const lastSavedContactsHashSelector = (state: RootState) =>
   state.identity.lastSavedContactsHash
+
+export const shouldRefreshStoredPasswordHashSelector = (state: RootState) =>
+  state.identity.shouldRefreshStoredPasswordHash

src/pincode/authentication.test.ts

@@ -6,13 +6,16 @@ import { PincodeType } from 'src/account/reducer'
 import { pincodeTypeSelector } from 'src/account/selectors'
 import { AuthenticationEvents } from 'src/analytics/Events'
 import ValoraAnalytics from 'src/analytics/ValoraAnalytics'
+import { storedPasswordRefreshed } from 'src/identity/actions'
 import { navigate, navigateBack } from 'src/navigator/NavigationService'
 import {
   CANCELLED_PIN_INPUT,
+  checkPin,
   DEFAULT_CACHE_ACCOUNT,
   getPasswordSaga,
   getPincode,
   getPincodeWithBiometry,
+  passwordHashStorageKey,
   PinBlocklist,
   removeStoredPin,
   retrieveOrGeneratePepper,
@@ -23,15 +26,18 @@ import {
   clearPasswordCaches,
   getCachedPepper,
   getCachedPin,
+  setCachedPasswordHash,
   setCachedPepper,
   setCachedPin,
 } from 'src/pincode/PasswordCache'
 import { store } from 'src/redux/store'
 import { ensureError } from 'src/utils/ensureError'
 import Logger from 'src/utils/Logger'
+import { getWalletAsync } from 'src/web3/contracts'
 import { getMockStoreData } from 'test/utils'
 import { mockAccount } from 'test/values'
 
+jest.mock('src/web3/contracts')
 jest.unmock('src/pincode/authentication')
 jest.mock('src/redux/store', () => ({ store: { getState: jest.fn() } }))
 jest.mock('react-native-securerandom', () => ({
@@ -54,6 +60,7 @@ const mockPin = '111555'
 const mockedKeychain = jest.mocked(Keychain)
 const mockStore = jest.mocked(store)
 mockStore.getState.mockImplementation(getMockStoreData)
+mockStore.dispatch = jest.fn()
 const mockedNavigate = navigate as jest.Mock
 
 const expectPincodeEntered = () => {
@@ -363,6 +370,9 @@ describe(updatePin, () => {
       }
       return Promise.resolve(false)
     })
+    jest.mocked(getWalletAsync).mockResolvedValue({
+      updateAccount: jest.fn().mockResolvedValue(true),
+    } as any)
   })
 
   it('should update the cached pin, stored password, and store mnemonic', async () => {
@@ -533,3 +543,108 @@ describe(PinBlocklist, () => {
     })
   })
 })
+
+describe(checkPin, () => {
+  const expectedPassword = mockPepper.password + mockPin
+  const expectedPasswordHash = 'd9bb2d77ec27dc8bf4269a6241daaa0388e8908518458f6ce0314380d11411cd' // sha256 of expectedPassword
+  const mockUnlockAccount = jest.fn().mockImplementation((_account, password, _duration) => {
+    if (password === expectedPassword) {
+      return Promise.resolve(true)
+    }
+    return Promise.resolve(false)
+  })
+
+  beforeEach(() => {
+    jest.clearAllMocks()
+    clearPasswordCaches()
+    setCachedPepper(DEFAULT_CACHE_ACCOUNT, mockPepper.password)
+    setCachedPasswordHash(mockAccount, expectedPasswordHash)
+
+    jest.mocked(getWalletAsync).mockResolvedValue({
+      unlockAccount: mockUnlockAccount,
+    } as any)
+  })
+
+  it('returns true if the pin unlocks the account, and refresh the stored password even if a stored password exists', async () => {
+    mockStore.getState.mockImplementationOnce(() =>
+      getMockStoreData({ identity: { shouldRefreshStoredPasswordHash: true } })
+    )
+
+    const result = await checkPin(mockPin, mockAccount)
+
+    expect(result).toBe(true)
+    expect(mockUnlockAccount).toHaveBeenCalledWith(mockAccount, expectedPassword, 600)
+    expect(mockedKeychain.setGenericPassword).toHaveBeenCalledTimes(1)
+    expect(mockedKeychain.setGenericPassword).toHaveBeenCalledWith(
+      'CELO',
+      expectedPasswordHash,
+      expect.objectContaining({
+        service: passwordHashStorageKey(mockAccount),
+      })
+    )
+    expect(mockStore.dispatch).toHaveBeenCalledWith(storedPasswordRefreshed())
+  })
+
+  it('returns true if the pin unlocks the account, and stored the password if it does not exist', async () => {
+    setCachedPasswordHash(mockAccount, '') // no cached password hash
+    mockedKeychain.getGenericPassword.mockResolvedValueOnce(false) // no stored password hash
+    mockStore.getState.mockImplementationOnce(() =>
+      getMockStoreData({ identity: { shouldRefreshStoredPasswordHash: false } })
+    )
+
+    const result = await checkPin(mockPin, mockAccount)
+
+    expect(result).toBe(true)
+    expect(mockUnlockAccount).toHaveBeenCalledWith(mockAccount, expectedPassword, 600)
+    expect(mockedKeychain.setGenericPassword).toHaveBeenCalledTimes(1)
+    expect(mockedKeychain.setGenericPassword).toHaveBeenCalledWith(
+      'CELO',
+      expectedPasswordHash,
+      expect.objectContaining({
+        service: passwordHashStorageKey(mockAccount),
+      })
+    )
+    expect(mockStore.dispatch).toHaveBeenCalledWith(storedPasswordRefreshed())
+  })
+
+  it('returns true if the pin matches the stored password, without unlocking the account or updating the keychain', async () => {
+    mockStore.getState.mockImplementationOnce(() =>
+      getMockStoreData({ identity: { shouldRefreshStoredPasswordHash: false } })
+    )
+
+    const result = await checkPin(mockPin, mockAccount)
+
+    expect(result).toBe(true)
+    expect(mockUnlockAccount).not.toHaveBeenCalled()
+    expect(mockedKeychain.setGenericPassword).not.toHaveBeenCalled()
+    expect(mockStore.dispatch).not.toHaveBeenCalled()
+  })
+
+  it('returns false if the pin does not match the stored password', async () => {
+    mockStore.getState.mockImplementationOnce(() =>
+      getMockStoreData({ identity: { shouldRefreshStoredPasswordHash: false } })
+    )
+
+    const result = await checkPin('143826', mockAccount) // incorrect pin
+
+    expect(result).toBe(false)
+    expect(mockUnlockAccount).not.toHaveBeenCalled()
+    expect(mockedKeychain.setGenericPassword).not.toHaveBeenCalled()
+    expect(mockStore.dispatch).not.toHaveBeenCalled()
+  })
+
+  it('returns false if the pin does not unlock the wallet', async () => {
+    const incorrectPin = '143826'
+    const incorrectPassword = mockPepper.password + incorrectPin
+    mockStore.getState.mockImplementationOnce(() =>
+      getMockStoreData({ identity: { shouldRefreshStoredPasswordHash: true } })
+    )
+
+    const result = await checkPin('143826', mockAccount) // incorrect pin
+
+    expect(result).toBe(false)
+    expect(mockUnlockAccount).toHaveBeenCalledWith(mockAccount, incorrectPassword, 600)
+    expect(mockedKeychain.setGenericPassword).not.toHaveBeenCalled()
+    expect(mockStore.dispatch).not.toHaveBeenCalled()
+  })
+})

src/pincode/authentication.ts

@@ -17,6 +17,8 @@ import ValoraAnalytics from 'src/analytics/ValoraAnalytics'
 import { ErrorMessages } from 'src/app/ErrorMessages'
 import { getStoredMnemonic, storeMnemonic } from 'src/backup/utils'
 import i18n from 'src/i18n'
+import { storedPasswordRefreshed } from 'src/identity/actions'
+import { shouldRefreshStoredPasswordHashSelector } from 'src/identity/selectors'
 import { navigate, navigateBack } from 'src/navigator/NavigationService'
 import { Screens } from 'src/navigator/Screens'
 import {
@@ -140,6 +142,7 @@ export async function retrieveOrGeneratePepper(account = DEFAULT_CACHE_ACCOUNT)
   if (!getCachedPepper(account)) {
     let storedPepper = await retrieveStoredItem(STORAGE_KEYS.PEPPER)
     if (!storedPepper) {
+      Logger.debug(TAG, 'No stored pepper, generating new pepper and storing it to the keychain')
       const randomBytes = await generateSecureRandom(PEPPER_LENGTH)
       const pepper = Buffer.from(randomBytes).toString('hex')
       await storeItem({ key: STORAGE_KEYS.PEPPER, value: pepper })
@@ -395,15 +398,19 @@ export async function requestPincodeInput(
 
 // Confirm pin is correct by checking it against the stored password hash
 export async function checkPin(pin: string, account: string) {
+  const shouldRefreshStoredPasswordHash = shouldRefreshStoredPasswordHashSelector(store.getState())
+
   const hashForPin = await getPasswordHashForPin(pin)
   const correctHash = await retrievePasswordHash(account)
 
-  if (!correctHash) {
-    Logger.warn(`${TAG}@checkPin`, 'No password hash stored. Checking with rpcWallet instead.')
+  if (!correctHash || shouldRefreshStoredPasswordHash) {
+    Logger.warn(`${TAG}@checkPin`, 'Validating pin without stored password hash')
     const password = await getPasswordForPin(pin)
     const unlocked = await ensureCorrectPassword(password, account)
     if (unlocked) {
       await storePasswordHash(hashForPin, account)
+      store.dispatch(storedPasswordRefreshed())
+
       return true
     }
     return false

src/redux/migrations.ts

@@ -1744,4 +1744,11 @@ export const migrations = {
       pointsConfigStatus: 'idle',
     },
   }),
+  208: (state: any) => ({
+    ...state,
+    identity: {
+      ...state.identity,
+      shouldRefreshStoredPasswordHash: true,
+    },
+  }),
 }

src/redux/store.test.ts

@@ -98,7 +98,7 @@ describe('store state', () => {
       {
         "_persist": {
           "rehydrated": true,
-          "version": 207,
+          "version": 208,
         },
         "account": {
           "acceptedTerms": false,
@@ -253,6 +253,7 @@ describe('store state', () => {
           },
           "lastSavedContactsHash": null,
           "secureSendPhoneNumberMapping": {},
+          "shouldRefreshStoredPasswordHash": true,
           "walletToAccountAddress": {},
         },
         "imports": {

src/redux/store.ts

@@ -21,7 +21,7 @@ const persistConfig: PersistConfig<ReducersRootState> = {
   key: 'root',
   // default is -1, increment as we make migrations
   // See https://github.com/valora-inc/wallet/tree/main/WALLET.md#redux-state-migration
-  version: 207,
+  version: 208,
   keyPrefix: `reduxStore-`, // the redux-persist default is `persist:` which doesn't work with some file systems.
   storage: FSStorage(),
   blacklist: ['networkInfo', 'alert', 'imports', 'keylessBackup', 'jumpstart'],

src/web3/contracts.test.ts

@@ -44,7 +44,7 @@ describe('getViemWallet', () => {
         .provide([
           [select(walletAddressSelector), '0x123'],
           [call(listStoredAccounts), [{ address: '0x123', createdAt: date }]],
-          [call(getPasswordSaga, '0x123', false, true), 'password'],
+          [call(getPasswordSaga, '0x123', true, false), 'password'],
           [
             call(getStoredPrivateKey, { address: '0x123', createdAt: new Date() }, 'password'),
             null,
@@ -61,7 +61,7 @@ describe('getViemWallet', () => {
       .provide([
         [select(walletAddressSelector), '0x123'],
         [call(listStoredAccounts), [{ address: '0x123', createdAt: date }]],
-        [call(getPasswordSaga, '0x123', false, true), 'password'],
+        [call(getPasswordSaga, '0x123', true, false), 'password'],
         [
           call(getStoredPrivateKey, { address: '0x123', createdAt: new Date() }, 'password'),
           'password',

src/web3/contracts.ts

@@ -135,7 +135,7 @@ export function* getViemWallet(chain: Chain) {
   if (!account) {
     throw new Error(`Account ${walletAddress} not found in Keychain`)
   }
-  const password = yield* call(getPasswordSaga, walletAddress, false, true)
+  const password = yield* call(getPasswordSaga, walletAddress, true, false)
   const privateKey = yield* call(getStoredPrivateKey, account, password)
   if (!privateKey) {
     throw new Error(`Private key not found for account ${walletAddress}`)

test/RootStateSchema.json

@@ -4716,6 +4716,9 @@
                 "secureSendPhoneNumberMapping": {
                     "$ref": "#/definitions/SecureSendPhoneNumberMapping"
                 },
+                "shouldRefreshStoredPasswordHash": {
+                    "type": "boolean"
+                },
                 "walletToAccountAddress": {
                     "$ref": "#/definitions/WalletToAccountAddressType"
                 }
@@ -4732,6 +4735,7 @@
                 "importContactsProgress",
                 "lastSavedContactsHash",
                 "secureSendPhoneNumberMapping",
+                "shouldRefreshStoredPasswordHash",
                 "walletToAccountAddress"
             ],
             "type": "object"

test/schemas.ts

@@ -3249,6 +3249,18 @@ export const v207Schema = {
   },
 }
 
+export const v208Schema = {
+  ...v207Schema,
+  _persist: {
+    ...v207Schema._persist,
+    version: 208,
+  },
+  identity: {
+    ...v207Schema.identity,
+    shouldRefreshStoredPasswordHash: true,
+  },
+}
+
 export function getLatestSchema(): Partial<RootState> {
-  return v207Schema as Partial<RootState>
+  return v208Schema as Partial<RootState>
 }