👑 Private keys import and accounts backup (#3089)

### About this PR This PR will contain all changes for features: - [x] allow importing private keys in the keyring service (initial scope of this PR) - [x] import private keys with plain text (#3119) - [x] import private keys with JSON (#3177) - [x] backup account with mnemonic and private key export (#3252) --- ## Initial scope Resolves #3070 ### What Handle importing wallets with private keys in the keyring service: * save pk imported accounts in the encrypted vault * handle serialization and deserialization of pk imported wallets * handle signing messages with both keyrings and pk wallets * handle communication between service and redux store about pk wallets * fix types and field names in the redux store Cleanup and add more tests: - move existing keyring integration tests to the right folder - write more unit tests to cover private key imported wallets - cleanup some duplicated tests - add util functions to the test's `factories` - add function to mock local storage ### Testing - make sure unit tests and E2E are passing - test manually adding accounts and signing transactions Latest build: [extension-builds-3089](https://github.com/tahowallet/extension/suites/14105826655/artifacts/789186059) (as of Thu, 06 Jul 2023 12:56:07 GMT).
tahowallet · Jul 7, 2023 · 713acec · 713acec
2 parents ab9bd8e + 0a13649
commit 713acec
Show file tree

Hide file tree

Showing 126 changed files with 6,501 additions and 2,683 deletions.
diff --git a/.env.defaults b/.env.defaults
@@ -32,4 +32,4 @@ SUPPORT_SWAP_QUOTE_REFRESH=false
 SUPPORT_ACHIEVEMENTS_BANNER=false
 SUPPORT_NFT_SEND=false
 USE_MAINNET_FORK=false
-ENABLE_UPDATED_DAPP_CONNECTIONS=false
+ENABLE_UPDATED_DAPP_CONNECTIONS=false
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -92,7 +92,13 @@ module.exports = {
         ],
       },
     ],
-    "@typescript-eslint/no-unused-vars": "error",
+    "@typescript-eslint/no-unused-vars": [
+      "error",
+      {
+        argsIgnorePattern: "^_",
+        varsIgnorePattern: "^_",
+      },
+    ],
     "no-unused-vars": "off",
   },
   ignorePatterns: [

diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS
@@ -1,8 +1,8 @@
-# Any changes to keyring code deserve extra scrutiny to prevent key
+# Any changes to internal-signer service code deserve extra scrutiny to prevent key
 # exfiltration and general "roll your own crypto" mistakes. Newer
-# contributions to keyring code should be assumed insecure, requiring
+# contributions to internal-signer code should be assumed insecure, requiring
 # agreement across the team to merge.
-/background/services/keyring/* @tahowallet/extension-security-auditors
+/background/services/internal-signer/* @tahowallet/extension-security-auditors
 # Any changes to dependencies deserve extra scrutiny to help prevent supply
 # chain attacks
 yarn.lock @tahowallet/extension-dependency-auditors

diff --git a/README.md b/README.md
@@ -243,7 +243,7 @@ services (in the API package) and the interface and browser notifications:
 │                 │                                                    - On-chain prices        ┃     │      └─────────────────┘
 │                 │                                                                             ┃     │
 │                 │                                                                             ┃     │      ┌────────────────┐
-│                 │                                                   Keyring                   ┃     │      │                │
+│                 │                                                   Internal Signer           ┃     │      │                │
 │                 ├──────list accounts, sign tx, sign message───────▶ - Native  ────────────────╋─────┼──────▶   Extension    │
 │                 │                                                   - Remote                  ┃     │      │  Storage API   │
 │      ┌──────────┴──────────┐                                                                  ┃     │      │                │

diff --git a/background/lib/posthog.ts b/background/lib/posthog.ts
@@ -12,6 +12,8 @@ export enum AnalyticsEvent {
   NEW_ACCOUNT_TO_TRACK = "Address added to tracking on network",
   CUSTOM_CHAIN_ADDED = "Custom chain added",
   DAPP_CONNECTED = "Dapp Connected",
+  VAULT_MIGRATION = "Migrate to newer vault version",
+  VAULT_MIGRATION_FAILED = "Vault version migration failed",
 }
 
 export enum OneTimeAnalyticsEvent {

diff --git a/background/lib/token-lists.ts b/background/lib/token-lists.ts
@@ -22,7 +22,7 @@ import { DeepWriteable } from "../types"
 const cleanTokenListResponse = (json: any, url: string) => {
   if (url.includes("api-polygon-tokens.polygon.technology")) {
     if (typeof json === "object" && json !== null && "tags" in json) {
-      const { tags, ...cleanedJson } = json
+      const { tags: _, ...cleanedJson } = json
       return cleanedJson
     }
   }

diff --git a/background/lib/utils/index.ts b/background/lib/utils/index.ts
@@ -108,14 +108,14 @@ export function gweiToWei(value: number | bigint): bigint {
   return BigInt(utils.parseUnits(value.toString(), "gwei").toString())
 }
 
-export function convertToEth(value: string | number | bigint): string {
+export function convertToEth(value: bigint): string {
   if (value && value >= 1) {
     return utils.formatUnits(BigInt(value))
   }
   return ""
 }
 
-export function weiToGwei(value: string | number | bigint): string {
+export function weiToGwei(value: bigint): string {
   if (value && value >= 1) {
     return truncateDecimalAmount(utils.formatUnits(BigInt(value), "gwei"), 2)
   }

diff --git a/background/lib/validate/prices.ts b/background/lib/validate/prices.ts
@@ -27,7 +27,7 @@ export const coingeckoPriceSchema: JSONSchemaType<CoingeckoPriceData> = {
     additionalProperties: { type: "number", nullable: true },
     nullable: true,
   },
-}
+} as const
 
 export type CoingeckoPriceData = {
   [coinId: string]:

diff --git a/background/main.ts b/background/main.ts
@@ -23,7 +23,7 @@ import {
   EnrichmentService,
   IndexingService,
   InternalEthereumProviderService,
-  KeyringService,
+  InternalSignerService,
   NameService,
   PreferenceService,
   ProviderBridgeService,
@@ -38,7 +38,7 @@ import {
   getNoopService,
 } from "./services"
 
-import { HexString, KeyringTypes, NormalizedEVMAddress } from "./types"
+import { HexString, NormalizedEVMAddress } from "./types"
 import { SignedTransaction } from "./networks"
 import { AccountBalance, AddressOnNetwork, NameOnNetwork } from "./accounts"
 import { Eligible } from "./services/doggo/types"
@@ -65,12 +65,12 @@ import {
   setReferrerStats,
 } from "./redux-slices/claim"
 import {
-  emitter as keyringSliceEmitter,
-  keyringLocked,
-  keyringUnlocked,
-  updateKeyrings,
+  emitter as internalSignerSliceEmitter,
+  internalSignerLocked,
+  internalSignerUnlocked,
+  updateInternalSigners,
   setKeyringToVerify,
-} from "./redux-slices/keyrings"
+} from "./redux-slices/internal-signer"
 import { blockSeen, setEVMNetworks } from "./redux-slices/networks"
 import {
   initializationLoadingTimeHitLimit,
@@ -83,6 +83,7 @@ import {
   toggleCollectAnalytics,
   setShowAnalyticsNotification,
   setSelectedNetwork,
+  setAutoLockInterval,
   setShownDismissableItems,
   dismissableItemMarkedAsShown,
 } from "./redux-slices/ui"
@@ -191,6 +192,10 @@ import {
   isBuiltInNetworkBaseAsset,
   isSameAsset,
 } from "./redux-slices/utils/asset-utils"
+import {
+  SignerImportMetadata,
+  SignerInternalTypes,
+} from "./services/internal-signer"
 import { getPricePoint, getTokenPrices } from "./lib/prices"
 import { DismissableItem } from "./services/preferences"
 
@@ -298,8 +303,16 @@ export default class Main extends BaseService<never> {
 
   static create: ServiceCreatorFunction<never, Main, []> = async () => {
     const preferenceService = PreferenceService.create()
-    const keyringService = KeyringService.create()
-    const chainService = ChainService.create(preferenceService, keyringService)
+    const analyticsService = AnalyticsService.create(preferenceService)
+
+    const internalSignerService = InternalSignerService.create(
+      preferenceService,
+      analyticsService
+    )
+    const chainService = ChainService.create(
+      preferenceService,
+      internalSignerService
+    )
     const indexingService = IndexingService.create(
       preferenceService,
       chainService
@@ -323,13 +336,11 @@ export default class Main extends BaseService<never> {
     const ledgerService = LedgerService.create()
 
     const signingService = SigningService.create(
-      keyringService,
+      internalSignerService,
       ledgerService,
       chainService
     )
 
-    const analyticsService = AnalyticsService.create(preferenceService)
-
     const nftsService = NFTsService.create(chainService)
 
     const abilitiesService = AbilitiesService.create(
@@ -380,7 +391,7 @@ export default class Main extends BaseService<never> {
       await chainService,
       await enrichmentService,
       await indexingService,
-      await keyringService,
+      await internalSignerService,
       await nameService,
       await internalEthereumProviderService,
       await providerBridgeService,
@@ -418,11 +429,11 @@ export default class Main extends BaseService<never> {
      */
     private indexingService: IndexingService,
     /**
-     * A promise to the keyring service, which stores key material, derives
-     * accounts, and signs messagees and transactions. The promise will be
+     * A promise to the internal signer service, which stores key material, derives
+     * accounts, and signs messages and transactions. The promise will be
      * resolved when the service is initialized.
      */
-    private keyringService: KeyringService,
+    private internalSignerService: InternalSignerService,
     /**
      * A promise to the name service, responsible for resolving names to
      * addresses and content.
@@ -533,7 +544,7 @@ export default class Main extends BaseService<never> {
       this.chainService.startService(),
       this.indexingService.startService(),
       this.enrichmentService.startService(),
-      this.keyringService.startService(),
+      this.internalSignerService.startService(),
       this.nameService.startService(),
       this.internalEthereumProviderService.startService(),
       this.providerBridgeService.startService(),
@@ -556,7 +567,7 @@ export default class Main extends BaseService<never> {
       this.chainService.stopService(),
       this.indexingService.stopService(),
       this.enrichmentService.stopService(),
-      this.keyringService.stopService(),
+      this.internalSignerService.stopService(),
       this.nameService.stopService(),
       this.internalEthereumProviderService.stopService(),
       this.providerBridgeService.stopService(),
@@ -576,7 +587,7 @@ export default class Main extends BaseService<never> {
 
   async initializeRedux(): Promise<void> {
     this.connectIndexingService()
-    this.connectKeyringService()
+    this.connectInternalSignerService()
     this.connectNameService()
     this.connectInternalEthereumProviderService()
     this.connectProviderBridgeService()
@@ -1071,7 +1082,7 @@ export default class Main extends BaseService<never> {
   }
 
   async connectSigningService(): Promise<void> {
-    this.keyringService.emitter.on("address", (address) =>
+    this.internalSignerService.emitter.on("address", (address) =>
       this.signingService.addTrackedAddress(address, "keyring")
     )
 
@@ -1110,12 +1121,12 @@ export default class Main extends BaseService<never> {
     })
   }
 
-  async connectKeyringService(): Promise<void> {
-    this.keyringService.emitter.on("keyrings", (keyrings) => {
-      this.store.dispatch(updateKeyrings(keyrings))
+  async connectInternalSignerService(): Promise<void> {
+    this.internalSignerService.emitter.on("internalSigners", (signers) => {
+      this.store.dispatch(updateInternalSigners(signers))
     })
 
-    this.keyringService.emitter.on("address", async (address) => {
+    this.internalSignerService.emitter.on("address", async (address) => {
       const trackedNetworks = await this.chainService.getTrackedNetworks()
       trackedNetworks.forEach((network) => {
         // Mark as loading and wire things up.
@@ -1134,48 +1145,41 @@ export default class Main extends BaseService<never> {
       })
     })
 
-    this.keyringService.emitter.on("locked", async (isLocked) => {
+    this.internalSignerService.emitter.on("locked", async (isLocked) => {
       if (isLocked) {
-        this.store.dispatch(keyringLocked())
+        this.store.dispatch(internalSignerLocked())
       } else {
-        this.store.dispatch(keyringUnlocked())
+        this.store.dispatch(internalSignerUnlocked())
       }
     })
 
-    keyringSliceEmitter.on("createPassword", async (password) => {
-      await this.keyringService.unlock(password, true)
+    internalSignerSliceEmitter.on("createPassword", async (password) => {
+      await this.internalSignerService.unlock(password, true)
     })
 
-    keyringSliceEmitter.on("lockKeyrings", async () => {
-      await this.keyringService.lock()
+    internalSignerSliceEmitter.on("lockInternalSigners", async () => {
+      await this.internalSignerService.lock()
     })
 
-    keyringSliceEmitter.on("deriveAddress", async (keyringID) => {
+    internalSignerSliceEmitter.on("deriveAddress", async (keyringID) => {
       await this.signingService.deriveAddress({
         type: "keyring",
         keyringID,
       })
     })
 
-    keyringSliceEmitter.on("generateNewKeyring", async (path) => {
+    internalSignerSliceEmitter.on("generateNewKeyring", async (path) => {
       // TODO move unlocking to a reasonable place in the initialization flow
       const generated: {
         id: string
         mnemonic: string[]
-      } = await this.keyringService.generateNewKeyring(
-        KeyringTypes.mnemonicBIP39S256,
+      } = await this.internalSignerService.generateNewKeyring(
+        SignerInternalTypes.mnemonicBIP39S256,
         path
       )
 
       this.store.dispatch(setKeyringToVerify(generated))
     })
-
-    keyringSliceEmitter.on(
-      "importKeyring",
-      async ({ mnemonic, path, source }) => {
-        await this.keyringService.importKeyring(mnemonic, source, path)
-      }
-    )
   }
 
   async connectInternalEthereumProviderService(): Promise<void> {
@@ -1502,6 +1506,14 @@ export default class Main extends BaseService<never> {
       }
     )
 
+    this.preferenceService.emitter.on(
+      "updateAutoLockInterval",
+      async (newTimerValue) => {
+        await this.internalSignerService.updateAutoLockInterval()
+        this.store.dispatch(setAutoLockInterval(newTimerValue))
+      }
+    )
+
     this.preferenceService.emitter.on(
       "initializeShownDismissableItems",
       async (dismissableItems) => {
@@ -1670,8 +1682,20 @@ export default class Main extends BaseService<never> {
     })
   }
 
-  async unlockKeyrings(password: string): Promise<boolean> {
-    return this.keyringService.unlock(password)
+  async unlockInternalSigners(password: string): Promise<boolean> {
+    return this.internalSignerService.unlock(password)
+  }
+
+  async exportMnemonic(address: HexString): Promise<string | null> {
+    return this.internalSignerService.exportMnemonic(address)
+  }
+
+  async exportPrivateKey(address: HexString): Promise<string | null> {
+    return this.internalSignerService.exportPrivateKey(address)
+  }
+
+  async importSigner(signerRaw: SignerImportMetadata): Promise<string | null> {
+    return this.internalSignerService.importSigner(signerRaw)
   }
 
   async getActivityDetails(txHash: string): Promise<ActivityDetail[]> {
@@ -1713,7 +1737,7 @@ export default class Main extends BaseService<never> {
                 This event is fired when any address on a network is added to the tracked list. 
                 
                 Note: this does not track recovery phrase(ish) import! But when an address is used 
-                on a network for the first time (read-only or recovery phrase/ledger/keyring).
+                on a network for the first time (read-only or recovery phrase/ledger/keyring/private key).
                 `,
         }
       )
@@ -1773,6 +1797,10 @@ export default class Main extends BaseService<never> {
         this.analyticsService.sendAnalyticsEvent(event)
       }
     })
+
+    uiSliceEmitter.on("updateAutoLockInterval", async (newTimerValue) => {
+      await this.preferenceService.updateAutoLockInterval(newTimerValue)
+    })
   }
 
   async updateAssetMetadata(

diff --git a/background/package.json b/background/package.json
@@ -39,7 +39,7 @@
     "@ledgerhq/hw-transport": "^6.20.0",
     "@ledgerhq/hw-transport-webusb": "^6.20.0",
     "@redux-devtools/remote": "^0.7.4",
-    "@tallyho/hd-keyring": "0.4.0",
+    "@tallyho/hd-keyring": "0.5.0",
     "@tallyho/provider-bridge-shared": "0.0.1",
     "@tallyho/window-provider": "0.0.1",
     "@types/w3c-web-usb": "^1.0.5",
@@ -49,7 +49,9 @@
     "@walletconnect/utils": "^2.1.4",
     "ajv": "^8.6.2",
     "ajv-formats": "^2.1.0",
+    "argon2-browser": "^1.18.0",
     "assert": "^2.0.0",
+    "base64-loader": "^1.0.0",
     "bnc-sdk": "^3.4.1",
     "dayjs": "^1.10.7",
     "dexie": "^3.0.3",
@@ -64,6 +66,7 @@
   },
   "devDependencies": {
     "@reduxjs/toolkit": "^1.6.1",
+    "@types/argon2-browser": "^1.18.1",
     "@types/sinon": "^10.0.12",
     "@types/uuid": "^8.3.4",
     "@types/webextension-polyfill": "^0.8.0",