From d40ccb9208b895d9b8c9afa1dfd85f0eb8b7a282 Mon Sep 17 00:00:00 2001 From: sonytooo Date: Wed, 12 Aug 2026 13:19:04 +0300 Subject: [PATCH 01/15] feat: accounts sync payload and keystore export/import Adds the ground work for syncing accounts between the Ambire mobile app and the browser extension over animated QR codes. - libs/accountsSync: the transported payload (existing types only), its serialization and a strict parser for the scanned bytes - keystore: exportForSync collects the selected keys, the seeds they were derived from and the password wrapped main key, all still encrypted; importFromSync unwraps the other device's main key with its password and re-encrypts everything with the local one - libs/keystore: decryptMainKeyWithSecret, the inverse of encryptMainKeyWithSecret, now also used by the GCM unlock path - addSeed can preserve the id a seed had on the other device, so synced keys keep pointing to the seed they were derived from Fixes a bug where the isReadyToStoreKeys setter added the queued internal and external keys in parallel; both persist the same storage entry, so one of the two sets was lost. They are now added sequentially, together with the seeds queued by a sync that happened before the device password was set. Co-Authored-By: Claude Opus 5 (1M context) --- src/controllers/keystore/keystore.test.ts | 164 +++++++++++++++ src/controllers/keystore/keystore.ts | 230 +++++++++++++++++---- src/libs/accountsSync/accountsSync.test.ts | 182 ++++++++++++++++ src/libs/accountsSync/accountsSync.ts | 119 +++++++++++ src/libs/keystore/keystore.ts | 29 +++ 5 files changed, 688 insertions(+), 36 deletions(-) create mode 100644 src/libs/accountsSync/accountsSync.test.ts create mode 100644 src/libs/accountsSync/accountsSync.ts diff --git a/src/controllers/keystore/keystore.test.ts b/src/controllers/keystore/keystore.test.ts index 99acdc0c03..bbd9e6e899 100644 --- a/src/controllers/keystore/keystore.test.ts +++ b/src/controllers/keystore/keystore.test.ts @@ -542,3 +542,167 @@ describe('import/export with pub key test', () => { ) }) }) + +describe('accounts sync between two devices', () => { + const EXTERNAL_ADDR = '0xd8dA6BF26964aF9D7eEd9e03E53415D37aA96045' + const exportingPass = 'exportingDevicePass' + const importingPass = 'importingDevicePass' + + let exportingKeystore: IKeystoreController + let importingKeystore: IKeystoreController + let exportedSeedId: string + + const uiCtrl = new UiController({ uiManager }) + + const createKeystore = () => + new KeystoreController( + 'default', + new StorageController(produceMemoryStore()), + keystoreSigners, + uiCtrl + ) + + const buildPayload = (keyAddrs: string[]) => + exportingKeystore + .exportForSync(keyAddrs) + .then((exported) => ({ v: 1 as const, accounts: [], ...exported })) + + beforeEach(async () => { + exportingKeystore = createKeystore() + await exportingKeystore.addSecret('password', exportingPass, '', false) + await exportingKeystore.unlockWithSecret('password', exportingPass) + + await exportingKeystore.addTempSeed({ + seed: process.env.SEED, + hdPathTemplate: BIP44_STANDARD_DERIVATION_TEMPLATE + }) + await exportingKeystore.persistTempSeed() + exportedSeedId = exportingKeystore.seeds[0]!.id + + await exportingKeystore.addKeys([ + { + addr: keyPublicAddress, + label: 'Key 1', + type: 'internal', + privateKey: privKey, + dedicatedToOneSA: false, + meta: { createdAt: new Date().getTime(), fromSeedId: exportedSeedId } + } + ]) + await exportingKeystore.addKeysExternallyStored([ + { + addr: EXTERNAL_ADDR, + label: 'Ledger Key 1', + type: 'ledger', + dedicatedToOneSA: false, + meta: { + deviceId: '1', + deviceModel: 'nanoX', + hdPathTemplate: BIP44_STANDARD_DERIVATION_TEMPLATE, + index: 1, + createdAt: new Date().getTime() + } + } + ]) + + importingKeystore = createKeystore() + }) + + test('exports the selected keys, their seed and the password wrapped main key', async () => { + const exported = await exportingKeystore.exportForSync([keyPublicAddress]) + + expect(exported.secret.id).toBe('password') + expect(exported.secret.aesEncrypted.cipherType).toBe('AES-GCM') + // The private key must leave the device encrypted, exactly as it is stored + expect(exported.keys).toHaveLength(1) + expect(exported.keys[0]!.addr).toBe(keyPublicAddress) + expect(exported.keys[0]!.privKey).toMatchObject({ cipherType: 'AES-GCM' }) + expect(JSON.stringify(exported)).not.toContain(privKey) + // Only the seed the exported key was derived from + expect(exported.seeds.map((s) => s.id)).toEqual([exportedSeedId]) + }) + + test('does not export keys that were not selected', async () => { + const exported = await exportingKeystore.exportForSync([EXTERNAL_ADDR]) + + expect(exported.keys.map((k) => k.addr)).toEqual([EXTERNAL_ADDR]) + expect(exported.seeds).toHaveLength(0) + }) + + describe('Negative cases', () => { + suppressConsoleBeforeEach() + + test('refuses to export from a device without a password', async () => { + const biometricsOnlyKeystore = createKeystore() + await biometricsOnlyKeystore.addSecret('biometrics', 'biometricsSecret', '', true) + + await expect(biometricsOnlyKeystore.exportForSync([keyPublicAddress])).rejects.toThrow( + 'Set a password for this device before syncing your accounts.' + ) + }) + + test('does not import anything when the password of the other device is wrong', async () => { + await importingKeystore.addSecret('password', importingPass, '', true) + const payload = await buildPayload([keyPublicAddress]) + + await expect(importingKeystore.importFromSync(payload, 'wrongPass')).rejects.toThrow( + 'Incorrect password. Please try again.' + ) + expect(importingKeystore.keys).toHaveLength(0) + expect(importingKeystore.seeds).toHaveLength(0) + }) + }) + + test('imports keys and seeds into a device that already has a password', async () => { + await importingKeystore.addSecret('password', importingPass, '', true) + const payload = await buildPayload([keyPublicAddress, EXTERNAL_ADDR]) + + await importingKeystore.importFromSync(payload, exportingPass) + + expect(importingKeystore.keys).toHaveLength(2) + expect(importingKeystore.keys).toContainEqual( + expect.objectContaining({ addr: EXTERNAL_ADDR, type: 'ledger', isExternallyStored: true }) + ) + // The key is re-encrypted with the importing device's main key, so it can sign + const signer = await importingKeystore.getSigner(keyPublicAddress, 'internal') + expect(signer.key.addr).toBe(keyPublicAddress) + + // The seed comes along and the key keeps pointing to it + expect(importingKeystore.seeds.map((s) => s.id)).toEqual([exportedSeedId]) + expect(importingKeystore.keys.find((k) => k.type === 'internal')?.meta.fromSeedId).toBe( + exportedSeedId + ) + expect((await importingKeystore.getSavedSeed(exportedSeedId)).seed).toBe(process.env.SEED) + }) + + test('imports before the device password is set (onboarding) and stores everything once it is', async () => { + const payload = await buildPayload([keyPublicAddress, EXTERNAL_ADDR]) + + await importingKeystore.importFromSync(payload, exportingPass) + + // Nothing can be stored yet, as there is no main key to encrypt with + expect(importingKeystore.keys).toHaveLength(0) + expect(importingKeystore.seeds).toHaveLength(0) + + await importingKeystore.addSecret('password', importingPass, '', true) + + expect(importingKeystore.keys).toHaveLength(2) + expect(importingKeystore.seeds.map((s) => s.id)).toEqual([exportedSeedId]) + expect(importingKeystore.keys.find((k) => k.type === 'internal')?.meta.fromSeedId).toBe( + exportedSeedId + ) + const signer = await importingKeystore.getSigner(keyPublicAddress, 'internal') + expect(signer.key.addr).toBe(keyPublicAddress) + }) + + test('syncing the same accounts twice does not duplicate keys or seeds', async () => { + await importingKeystore.addSecret('password', importingPass, '', true) + const payload = await buildPayload([keyPublicAddress, EXTERNAL_ADDR]) + + await importingKeystore.importFromSync(payload, exportingPass) + await importingKeystore.importFromSync(payload, exportingPass) + + expect(importingKeystore.keys).toHaveLength(2) + expect(importingKeystore.seeds).toHaveLength(1) + }) +}) diff --git a/src/controllers/keystore/keystore.ts b/src/controllers/keystore/keystore.ts index 1f3564d62d..57f4df9578 100644 --- a/src/controllers/keystore/keystore.ts +++ b/src/controllers/keystore/keystore.ts @@ -10,6 +10,7 @@ import { computeAddress, concat, getBytes, hexlify, keccak256, Mnemonic, Wallet import { CIPHER, CIPHER_OLD, + decryptMainKeyWithSecret, decryptWithKey, deriveSecret, encryptMainKeyWithSecret, @@ -31,6 +32,7 @@ import { Account } from '../../interfaces/account' import { IEventEmitterRegistryController, Statuses } from '../../interfaces/eventEmitter' import { KeyIterator } from '../../interfaces/keyIterator' import { + AESGCMEncrypted, ExternalKey, IKeystoreController, InternalKey, @@ -50,6 +52,7 @@ import { import { Platform } from '../../interfaces/platform' import { IStorageController } from '../../interfaces/storage' import { IUiController } from '../../interfaces/ui' +import { AccountsSyncPayload } from '../../libs/accountsSync/accountsSync' import { EntropyGenerator } from '../../libs/entropyGenerator/entropyGenerator' import { getDefaultKeyLabel } from '../../libs/keys/keys' import { ScryptAdapter } from '../../libs/scrypt/scryptAdapter' @@ -119,6 +122,8 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl #externalKeysToAddOnKeystoreReady: ReadyToAddKeys['external'] = [] + #seedsToAddOnKeystoreReady: (KeystoreTempSeed & { id?: StoredKeystoreSeed['id'] })[] = [] + keyStoreUid: string | null #isReadyToStoreKeys: boolean = false @@ -214,11 +219,13 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl set isReadyToStoreKeys(val) { this.#isReadyToStoreKeys = val - if (val && this.#internalKeysToAddOnKeystoreReady.length) { - void this.#addKeys(this.#internalKeysToAddOnKeystoreReady) - } - if (val && this.#externalKeysToAddOnKeystoreReady.length) { - void this.#addKeysExternallyStored(this.#externalKeysToAddOnKeystoreReady) + const hasQueuedData = + !!this.#seedsToAddOnKeystoreReady.length || + !!this.#internalKeysToAddOnKeystoreReady.length || + !!this.#externalKeysToAddOnKeystoreReady.length + + if (val && hasQueuedData) { + void this.#addQueuedKeysAndSeeds() } } @@ -381,26 +388,23 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl throw new Error('keystore: invalid gcm secret cipher type') } - const keyFromSecret = await crypto.subtle.importKey( - 'raw', - // use 256 bits (first 32 bytes) - secretKey.slice(0, 32), - { name: CIPHER }, - false, - ['encrypt', 'decrypt'] - ) + this.#mainKey = await this.#decryptMainKeyWithSecret(secretKey, secretEntry.aesEncrypted) + + this.errorMessage = '' + } - let decrypted: ArrayBuffer + /** + * Decrypts a main key wrapped with the provided secret, translating a wrong + * secret into the user facing "Incorrect password" error. Used both when + * unlocking this device and when importing keys synced from another device + * (where the main key of the other device gets unwrapped with its password). + */ + async #decryptMainKeyWithSecret( + secretKey: Uint8Array, + aesEncrypted: AESGCMEncrypted + ): Promise { try { - decrypted = await crypto.subtle.decrypt( - { - name: CIPHER, - iv: new Uint8Array(getBytes(secretEntry.aesEncrypted.iv)), - tagLength: 128 - }, - keyFromSecret, - new Uint8Array(getBytes(secretEntry.aesEncrypted.ciphertext)) - ) + return await decryptMainKeyWithSecret(secretKey, aesEncrypted) } catch (error: any) { // Either wrong password or corrupted/tampered ciphertext if (error?.name === 'OperationError') { @@ -424,16 +428,6 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl error instanceof Error ? error : new Error('keystore: unexpected error during GCM unlock') }) } - - this.#mainKey = await crypto.subtle.importKey( - 'raw', - decrypted.slice(0, 32), - { name: CIPHER }, - true, - ['encrypt', 'decrypt'] - ) - - this.errorMessage = '' } async #findStoredSeed(seed: string, seedPassphrase?: string | null) { @@ -722,7 +716,18 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl this.emitUpdate() } - async #addSeed({ seed, seedPassphrase, hdPathTemplate, notBackedUp }: KeystoreTempSeed) { + /** + * Adds a seed to the keystore and returns the id it is stored with. An `id` can be + * passed to preserve the id a seed already had on another device (accounts sync), + * so that the synced keys' `meta.fromSeedId` keeps pointing to it. + */ + async #addSeed({ + seed, + seedPassphrase, + hdPathTemplate, + notBackedUp, + id + }: KeystoreTempSeed & { id?: StoredKeystoreSeed['id'] }): Promise { await this.initialLoadPromise if (this.#mainKey === null) @@ -742,14 +747,15 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl } const existingEntry = await this.#findStoredSeed(seed, seedPassphrase) - if (existingEntry) return + if (existingEntry) return existingEntry.id const entropy = extractEntropyFromSeed(seed) const label = `Recovery Phrase ${this.#keystoreSeeds.length + 1}` + const isIdTaken = !!id && this.#keystoreSeeds.some((s) => s.id === id) const newEntry: StoredKeystoreSeed = { - id: generateUuid(), + id: isIdTaken || !id ? generateUuid() : id, label, seed: await encryptWithKey(this.#mainKey, entropy), seedPassphrase: seedPassphrase @@ -764,12 +770,47 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl await this.#storage.set('keystoreSeeds', this.#keystoreSeeds) this.emitUpdate() + + return newEntry.id } async addSeed(keystoreSeed: KeystoreTempSeed) { await this.withStatus('addSeed', () => this.#addSeed(keystoreSeed), true) } + /** + * Adds the seeds and keys that were queued while the keystore was not ready to store + * them yet (e.g. accounts synced from another device during onboarding, where the + * device password is set afterwards). + * + * Everything runs sequentially, because internal and external keys are persisted + * under the same storage key and would otherwise overwrite each other. + */ + async #addQueuedKeysAndSeeds() { + const seedsToAdd = this.#seedsToAddOnKeystoreReady + const internalKeysToAdd = this.#internalKeysToAddOnKeystoreReady + const externalKeysToAdd = this.#externalKeysToAddOnKeystoreReady + this.#seedsToAddOnKeystoreReady = [] + this.#internalKeysToAddOnKeystoreReady = [] + this.#externalKeysToAddOnKeystoreReady = [] + + try { + // Seeds first, so the keys can keep pointing to the seed they were derived from + for (const seed of seedsToAdd) { + await this.#addSeed(seed) + } + await this.#addKeys(internalKeysToAdd) + await this.#addKeysExternallyStored(externalKeysToAdd) + } catch (error: any) { + this.emitError({ + level: 'major', + message: + 'Something went wrong when saving your keys. Please try again or contact support if the problem persists.', + error: error instanceof Error ? error : new Error('keystore: failed to add queued keys') + }) + } + } + async #updateSeed({ id, label, @@ -1161,6 +1202,123 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl await this.addKeys([keyToAdd]) } + /** + * Collects everything another Ambire product needs in order to take over the given + * keys: the keys themselves and the seeds they were derived from (still encrypted + * with this device's main key), plus this device's main key wrapped with its password. + * Nothing gets decrypted here, so this works even when the keystore is locked. + */ + async exportForSync( + keyAddrs: Key['addr'][] + ): Promise> { + await this.initialLoadPromise + + const secret = this.#keystoreSecrets.find((s) => s.id === 'password') + if (!secret) + throw new EmittableError({ + level: 'expected', + message: 'Set a password for this device before syncing your accounts.', + error: new Error('keystore: no password secret to sync with') + }) + + if (secret.aesEncrypted.cipherType !== CIPHER) + throw new EmittableError({ + level: 'major', + message: + 'Something went wrong when preparing your accounts for syncing. Please unlock the app again or contact support if the problem persists.', + error: new Error('keystore: password secret not migrated to GCM yet') + }) + + const keys = this.#keystoreKeys.filter((key) => keyAddrs.includes(key.addr)) + const seedIds = new Set(keys.map((key) => key.meta?.fromSeedId).filter(Boolean)) + const seeds = this.#keystoreSeeds.filter((seed) => seedIds.has(seed.id)) + + return { secret, keys, seeds } + } + + /** + * Takes over the keys and seeds exported by another Ambire product. The password of + * that other device unwraps its main key, which is used only to decrypt the synced + * data in memory - everything is then re-encrypted with this device's main key. + * + * Intentionally not wrapped in `withStatus` and lets errors propagate, because the + * MainController orchestrates the whole migration (and must not add the accounts if + * this fails). + */ + async importFromSync(payload: AccountsSyncPayload, password: string) { + await this.initialLoadPromise + + const { secret, keys, seeds } = payload + if (secret.aesEncrypted.cipherType !== CIPHER) + throw new Error('keystore: synced main key is not encrypted with GCM') + + const secretKey = await deriveSecret(this.#scryptAdapter, password, secret.scryptParams.salt) + // The exporting device's main key. Kept in this scope only, never persisted. + const exportedMainKey = await this.#decryptMainKeyWithSecret(secretKey, secret.aesEncrypted) + + // Seeds first, so the keys below can be linked to the seed they were derived from + const syncedSeedIds: { [exportedSeedId: string]: StoredKeystoreSeed['id'] } = {} + for (const seed of seeds) { + const entropy = await decryptWithKey(exportedMainKey, seed.seed) + const seedPassphrase = seed.seedPassphrase + ? new TextDecoder().decode(await decryptWithKey(exportedMainKey, seed.seedPassphrase)) + : null + + syncedSeedIds[seed.id] = await this.#storeSyncedSeed({ + id: seed.id, + seed: reconstructSeedFromEntropy(entropy, seedPassphrase), + seedPassphrase, + hdPathTemplate: seed.hdPathTemplate, + notBackedUp: seed.notBackedUp + }) + } + + const internalKeys: ReadyToAddKeys['internal'] = [] + const externalKeys: ReadyToAddKeys['external'] = [] + + for (const key of keys) { + if (key.type !== 'internal') { + externalKeys.push(key) + continue + } + + const { fromSeedId, ...restMeta } = key.meta + // Drop the reference if the seed didn't come along, so it doesn't dangle + const syncedFromSeedId = fromSeedId ? syncedSeedIds[fromSeedId] : undefined + + internalKeys.push({ + addr: key.addr, + type: 'internal', + label: key.label, + dedicatedToOneSA: key.dedicatedToOneSA, + privateKey: hexlify(await decryptWithKey(exportedMainKey, key.privKey)), + meta: syncedFromSeedId ? { ...restMeta, fromSeedId: syncedFromSeedId } : restMeta + }) + } + + // Both queue themselves until the keystore is ready to store keys, which is what + // makes syncing before the device password is set (onboarding) work + await this.#addKeys(internalKeys) + await this.#addKeysExternallyStored(externalKeys) + } + + /** + * Stores a synced seed, or queues it if the device password is not set yet + * (onboarding). The keystore holds no seeds in that case, so the requested id is + * guaranteed to be the one the seed ends up stored with. + */ + async #storeSyncedSeed( + seed: KeystoreTempSeed & { id: StoredKeystoreSeed['id'] } + ): Promise { + if (!this.isReadyToStoreKeys || !this.#mainKey) { + this.#seedsToAddOnKeystoreReady.push(seed) + + return seed.id + } + + return this.#addSeed(seed) + } + async getSigner(keyAddress: Key['addr'], keyType: Key['type']): Promise { await this.initialLoadPromise const keys = this.#keystoreKeys diff --git a/src/libs/accountsSync/accountsSync.test.ts b/src/libs/accountsSync/accountsSync.test.ts new file mode 100644 index 0000000000..40315db94a --- /dev/null +++ b/src/libs/accountsSync/accountsSync.test.ts @@ -0,0 +1,182 @@ +import { getBytes } from 'ethers' + +import { CIPHER } from '@/libs/keystore/keystore' + +import { + ACCOUNTS_SYNC_PAYLOAD_VERSION, + AccountsSyncPayload, + parseAccountsSyncPayload, + serializeAccountsSyncPayload +} from './accountsSync' + +const ACCOUNT_ADDR = '0x8DC9b3e1F5b0Dc9F6b2e0d3D0Ba0A5a32B0E7C4B' +const KEY_ADDR = '0x085f8A348f6fBc6F8d8FC3f1e427473436506D65' +const EXTERNAL_KEY_ADDR = '0x1A2C3802A9eC12725678dAF23DbFD13134e5893A' + +const gcmPayload = (byteLength: number) => ({ + cipherType: CIPHER as 'AES-GCM', + ciphertext: `0x${'ab'.repeat(byteLength)}`, + iv: `0x${'cd'.repeat(12)}` +}) + +const buildPayload = (): AccountsSyncPayload => ({ + v: ACCOUNTS_SYNC_PAYLOAD_VERSION, + secret: { + id: 'password', + scryptParams: { salt: `0x${'ef'.repeat(32)}`, N: 131072, r: 8, p: 1, dkLen: 64 }, + aesEncrypted: gcmPayload(48) + }, + accounts: [ + { + addr: ACCOUNT_ADDR, + associatedKeys: [KEY_ADDR], + initialPrivileges: [ + [KEY_ADDR, '0x0000000000000000000000000000000000000000000000000000000000000002'] + ], + creation: { + factoryAddr: '0xa8202f888b9b2dFA5Ceb2204865018133F6F179A', + bytecode: `0x${'60'.repeat(120)}`, + salt: `0x${'00'.repeat(32)}` + }, + preferences: { label: 'Account 1', pfp: ACCOUNT_ADDR } + } + ], + keys: [ + { + addr: KEY_ADDR, + type: 'internal', + label: 'Key 1', + dedicatedToOneSA: true, + meta: { createdAt: 1755000000000, fromSeedId: 'seed-1' }, + privKey: gcmPayload(48) + }, + { + addr: EXTERNAL_KEY_ADDR, + type: 'ledger', + label: 'Ledger Key 1', + dedicatedToOneSA: false, + meta: { + deviceId: 'device-1', + deviceModel: 'nanoX', + hdPathTemplate: "m/44'/60'/0'/0/", + index: 0, + createdAt: 1755000000000 + }, + privKey: null + } + ], + seeds: [ + { + id: 'seed-1', + label: 'Recovery Phrase 1', + hdPathTemplate: "m/44'/60'/0'/0/", + seed: gcmPayload(32), + seedPassphrase: null + } + ] +}) + +const serializeAndParse = (payload: any) => + parseAccountsSyncPayload(getBytes(serializeAccountsSyncPayload(payload))) + +describe('accountsSync payload', () => { + it('round-trips a payload with internal keys, external keys and seeds', () => { + const payload = buildPayload() + + expect(serializeAndParse(payload)).toEqual(payload) + }) + + it('rejects data that is not a sync payload', () => { + expect(() => parseAccountsSyncPayload(new Uint8Array([1, 2, 3]))).toThrow( + 'not a valid sync payload' + ) + }) + + it('rejects an unsupported payload version', () => { + expect(() => serializeAndParse({ ...buildPayload(), v: 2 })).toThrow( + 'unsupported payload version 2' + ) + }) + + it('rejects a payload without the password protected main key', () => { + const withBiometricsSecret = buildPayload() + withBiometricsSecret.secret.id = 'biometrics' + + expect(() => serializeAndParse(withBiometricsSecret)).toThrow( + 'missing the password protected main key' + ) + }) + + it('rejects invalid scrypt params', () => { + const payload: any = buildPayload() + delete payload.secret.scryptParams.N + + expect(() => serializeAndParse(payload)).toThrow('invalid scrypt params') + }) + + it('rejects a main key that is not AES-GCM encrypted', () => { + const payload: any = buildPayload() + payload.secret.aesEncrypted = { + cipherType: 'aes-128-ctr', + ciphertext: '0xabab', + iv: '0xcdcd', + mac: '0xefef' + } + + // `tryParseGcmPayload` rejects a known but unsupported cipher itself + expect(() => serializeAndParse(payload)).toThrow('unsupported payload cipherType') + }) + + it('rejects a payload without accounts', () => { + expect(() => serializeAndParse({ ...buildPayload(), accounts: [] })).toThrow( + 'no accounts in the payload' + ) + }) + + it('rejects an account with an invalid address', () => { + const payload: any = buildPayload() + payload.accounts[0].addr = '0xnot-an-address' + + expect(() => serializeAndParse(payload)).toThrow('invalid account addr') + }) + + it('rejects an internal key that is not AES-GCM encrypted', () => { + const payload: any = buildPayload() + payload.keys[0].privKey = '0xdeadbeef' + + expect(() => serializeAndParse(payload)).toThrow( + `key ${KEY_ADDR} is not encrypted with AES-GCM` + ) + }) + + it('rejects an external key that carries a private key', () => { + const payload: any = buildPayload() + payload.keys[1].privKey = gcmPayload(48) + + expect(() => serializeAndParse(payload)).toThrow( + `external key ${EXTERNAL_KEY_ADDR} has a privKey` + ) + }) + + it('rejects a seed that is not AES-GCM encrypted', () => { + const payload: any = buildPayload() + payload.seeds[0].seed = '0xdeadbeef' + + expect(() => serializeAndParse(payload)).toThrow('seed seed-1 is not encrypted with AES-GCM') + }) + + it('rejects a seed passphrase that is not AES-GCM encrypted', () => { + const payload: any = buildPayload() + payload.seeds[0].seedPassphrase = '0xdeadbeef' + + expect(() => serializeAndParse(payload)).toThrow( + 'seed passphrase seed-1 is not encrypted with AES-GCM' + ) + }) + + it('accepts a payload with no keys and no seeds (view-only accounts)', () => { + const viewOnlyPayload = { ...buildPayload(), keys: [], seeds: [] } + + expect(serializeAndParse(viewOnlyPayload)).toEqual(viewOnlyPayload) + }) +}) diff --git a/src/libs/accountsSync/accountsSync.ts b/src/libs/accountsSync/accountsSync.ts new file mode 100644 index 0000000000..ec61bdb0f1 --- /dev/null +++ b/src/libs/accountsSync/accountsSync.ts @@ -0,0 +1,119 @@ +import { hexlify, isAddress, toUtf8Bytes, toUtf8String } from 'ethers' + +import { Account } from '../../interfaces/account' +import { + MainKeyEncryptedWithSecret, + StoredKey, + StoredKeystoreSeed +} from '../../interfaces/keystore' +import { CIPHER, tryParseGcmPayload } from '../keystore/keystore' + +/** + * The UR type used to transport the accounts sync payload over animated QR codes. + */ +export const ACCOUNTS_SYNC_UR_TYPE = 'ambire-account-sync' + +export const ACCOUNTS_SYNC_PAYLOAD_VERSION = 1 + +/** + * Everything needed to move accounts (and the keys controlling them) from one + * Ambire product to another. Sensitive data travels exactly as it is stored: + * private keys and seeds stay encrypted with the exporting device's main key, + * which itself travels wrapped with the exporting device's password (`secret`). + * The importing device unwraps the main key with that password, decrypts and + * re-encrypts everything with its own main key. + */ +export type AccountsSyncPayload = { + v: typeof ACCOUNTS_SYNC_PAYLOAD_VERSION + secret: MainKeyEncryptedWithSecret + accounts: Account[] + keys: StoredKey[] + seeds: StoredKeystoreSeed[] +} + +const requireGcmPayload = (payload: any, what: string) => { + if (!tryParseGcmPayload(payload)) + throw new Error(`accountsSync: ${what} is not encrypted with ${CIPHER}`) +} + +const validateSecret = (secret: any) => { + if (!secret || secret.id !== 'password') + throw new Error('accountsSync: missing the password protected main key') + + const { salt, N, r, p, dkLen } = secret.scryptParams || {} + const hasValidScryptParams = + typeof salt === 'string' && + typeof N === 'number' && + typeof r === 'number' && + typeof p === 'number' && + typeof dkLen === 'number' + if (!hasValidScryptParams) throw new Error('accountsSync: invalid scrypt params') + + requireGcmPayload(secret.aesEncrypted, 'the main key') +} + +const validateAccounts = (accounts: any) => { + if (!Array.isArray(accounts) || !accounts.length) + throw new Error('accountsSync: no accounts in the payload') + + accounts.forEach((account) => { + if (!account || !isAddress(account.addr)) throw new Error('accountsSync: invalid account addr') + if (!Array.isArray(account.associatedKeys)) + throw new Error('accountsSync: invalid account associatedKeys') + if (!account.preferences?.label) throw new Error('accountsSync: invalid account preferences') + }) +} + +const validateKeys = (keys: any) => { + if (!Array.isArray(keys)) throw new Error('accountsSync: invalid keys') + + keys.forEach((key) => { + if (!key || !isAddress(key.addr)) throw new Error('accountsSync: invalid key addr') + if (typeof key.type !== 'string') throw new Error('accountsSync: invalid key type') + + if (key.type === 'internal') return requireGcmPayload(key.privKey, `key ${key.addr}`) + // External keys are stored on the hardware device, so there is nothing to encrypt + if (key.privKey !== null) + throw new Error(`accountsSync: external key ${key.addr} has a privKey`) + }) +} + +const validateSeeds = (seeds: any) => { + if (!Array.isArray(seeds)) throw new Error('accountsSync: invalid seeds') + + seeds.forEach((seed) => { + if (!seed?.id) throw new Error('accountsSync: invalid seed id') + requireGcmPayload(seed.seed, `seed ${seed.id}`) + if (seed.seedPassphrase) requireGcmPayload(seed.seedPassphrase, `seed passphrase ${seed.id}`) + }) +} + +/** + * Serializes the payload to the hex encoded bytes carried by the animated QR codes. + */ +export const serializeAccountsSyncPayload = (payload: AccountsSyncPayload): string => + hexlify(toUtf8Bytes(JSON.stringify(payload))) + +/** + * Parses and validates the bytes assembled from the scanned animated QR codes. + * Throws if the payload is incomplete, tampered with or produced by an + * incompatible (newer) app version. + */ +export const parseAccountsSyncPayload = (bytes: Uint8Array): AccountsSyncPayload => { + let payload: any + try { + payload = JSON.parse(toUtf8String(bytes)) + } catch { + throw new Error('accountsSync: the scanned data is not a valid sync payload') + } + + if (payload?.v !== ACCOUNTS_SYNC_PAYLOAD_VERSION) + throw new Error(`accountsSync: unsupported payload version ${payload?.v}`) + + validateSecret(payload.secret) + validateAccounts(payload.accounts) + validateKeys(payload.keys) + validateSeeds(payload.seeds) + + return payload +} diff --git a/src/libs/keystore/keystore.ts b/src/libs/keystore/keystore.ts index b70953b96e..02c16e8acb 100644 --- a/src/libs/keystore/keystore.ts +++ b/src/libs/keystore/keystore.ts @@ -100,6 +100,35 @@ export const encryptMainKeyWithSecret = async ( return encryptWithKey(importedSecretKey, exportedMainKeyUint8Array) } +/** + * The counterpart of `encryptMainKeyWithSecret` - decrypts a main key that was wrapped with a secret. + * Throws an `OperationError` if the secret is wrong (or the ciphertext was tampered with). + */ +export const decryptMainKeyWithSecret = async ( + secretKey: Uint8Array, + aesEncrypted: AESGCMEncrypted +): Promise => { + const importedSecretKey = await crypto.subtle.importKey( + 'raw', + // use 256 bits (first 32 bytes) + secretKey.slice(0, 32), + { name: CIPHER }, + false, + ['encrypt', 'decrypt'] + ) + + const decrypted = await crypto.subtle.decrypt( + { name: CIPHER, iv: new Uint8Array(getBytes(aesEncrypted.iv)), tagLength: 128 }, + importedSecretKey, + new Uint8Array(getBytes(aesEncrypted.ciphertext)) + ) + + return crypto.subtle.importKey('raw', decrypted.slice(0, 32), { name: CIPHER }, true, [ + 'encrypt', + 'decrypt' + ]) +} + /** * As the type is string | AESGCMEncrypted, we need to check if it's a GCM payload or a legacy string payload */ From 8d62f88c12a53b7ddf602e4fdb184244062c6a3d Mon Sep 17 00:00:00 2001 From: sonytooo Date: Wed, 12 Aug 2026 13:26:27 +0300 Subject: [PATCH 02/15] feat: orchestrate the accounts sync from the main controller - accounts: getAccountsForSync returns the records of the selected accounts - main: exportAccountsForSync collects them together with the keys controlling them and sends the payload to the UI over the one time data channel, so it never lands in persisted state - main: importAccountsFromSync parses the scanned data, hands the keys to the keystore and only then adds the accounts, so a wrong password of the other device leaves nothing behind Co-Authored-By: Claude Opus 5 (1M context) --- src/controllers/accounts/accounts.ts | 9 ++ src/controllers/main/main.ts | 67 ++++++++ src/controllers/main/mainAccountsSync.test.ts | 149 ++++++++++++++++++ src/interfaces/main.ts | 4 +- 4 files changed, 228 insertions(+), 1 deletion(-) create mode 100644 src/controllers/main/mainAccountsSync.test.ts diff --git a/src/controllers/accounts/accounts.ts b/src/controllers/accounts/accounts.ts index 193c37c8c9..a5fbdf826d 100644 --- a/src/controllers/accounts/accounts.ts +++ b/src/controllers/accounts/accounts.ts @@ -406,6 +406,15 @@ export class AccountsController extends EventEmitter implements IAccountsControl return this.accountStates[addr]?.[chainId.toString()] } + /** + * The account records another Ambire product needs in order to take over the given + * accounts (accounts sync). Preferences travel along, so that the accounts look the + * same on both devices. + */ + getAccountsForSync(addrs: Account['addr'][]): Account[] { + return this.accounts.filter((account) => addrs.includes(account.addr)) + } + resetAccountsNewlyAddedState() { this.accounts = this.accounts.map((a) => ({ ...a, newlyAdded: false })) this.emitUpdate() diff --git a/src/controllers/main/main.ts b/src/controllers/main/main.ts index 4403580e46..c690134785 100644 --- a/src/controllers/main/main.ts +++ b/src/controllers/main/main.ts @@ -1,4 +1,5 @@ import { ethErrors } from 'eth-rpc-errors' +import { getBytes } from 'ethers' import EmittableError from '@/classes/EmittableError' import { AMBIRE_ACCOUNT_FACTORY } from '@/consts/deploy' @@ -98,6 +99,12 @@ import { SubmittedAccountOp } from '@/libs/accountOp/submittedAccountOp' import { AccountOpStatus } from '@/libs/accountOp/types' +import { + ACCOUNTS_SYNC_PAYLOAD_VERSION, + AccountsSyncPayload, + parseAccountsSyncPayload, + serializeAccountsSyncPayload +} from '@/libs/accountsSync/accountsSync' import { HumanizerMeta } from '@/libs/humanizer/interfaces' import { KeyIterator } from '@/libs/keyIterator/keyIterator' import { getAccountKeysCount } from '@/libs/keys/keys' @@ -1656,6 +1663,66 @@ export class MainController extends EventEmitter implements IMainController { await this.withStatus('updateAccounts', async () => this.#updateAccounts(accountsUpdate)) } + /** + * Prepares the selected accounts and the keys controlling them for the other Ambire + * product and sends the result to the UI, which displays it as animated QR codes. + * Everything sensitive leaves this device encrypted, see `keystore.exportForSync`. + */ + async exportAccountsForSync(addrs: Account['addr'][]) { + await this.withStatus('exportAccountsForSync', async () => { + const accounts = this.accounts.getAccountsForSync(addrs) + + if (!accounts.length) + throw new EmittableError({ + level: 'expected', + message: 'Select at least one account to sync.', + error: new Error('main: no accounts to sync') + }) + + const keyAddrs = Array.from(new Set(accounts.flatMap((account) => account.associatedKeys))) + const { secret, keys, seeds } = await this.keystore.exportForSync(keyAddrs) + + this.ui.message.sendUiMessage({ + accountsSyncPayload: serializeAccountsSyncPayload({ + v: ACCOUNTS_SYNC_PAYLOAD_VERSION, + secret, + accounts, + keys, + seeds + }) + }) + }) + } + + /** + * Takes over the accounts and keys scanned from the other Ambire product's QR codes. + * `payload` is the hex encoded data assembled from the scanned codes and `password` + * is the device password of the product that exported them. + */ + async importAccountsFromSync({ payload, password }: { payload: string; password: string }) { + await this.withStatus('importAccountsFromSync', async () => { + let parsedPayload: AccountsSyncPayload + try { + parsedPayload = parseAccountsSyncPayload(getBytes(payload)) + } catch (error: any) { + throw new EmittableError({ + level: 'expected', + message: + 'The scanned QR codes do not contain Ambire accounts, or not all of them were scanned. Please try again.', + error: error instanceof Error ? error : new Error('main: invalid accounts sync payload') + }) + } + + // The accounts are added only if the keys made it in, so that the user doesn't + // end up with accounts they cannot sign with + await this.keystore.importFromSync(parsedPayload, password) + await this.#updateAccounts({ + accountsToAdd: parsedPayload.accounts, + accountAddressesToRemove: [] + }) + }) + } + async reloadSelectedAccount(options?: { chainIds?: bigint[] maxDataAgeMs?: number diff --git a/src/controllers/main/mainAccountsSync.test.ts b/src/controllers/main/mainAccountsSync.test.ts new file mode 100644 index 0000000000..6b9839c62c --- /dev/null +++ b/src/controllers/main/mainAccountsSync.test.ts @@ -0,0 +1,149 @@ +import { Wallet } from 'ethers' + +import { describe, expect, jest, test } from '@jest/globals' + +import { makeMainController } from '../../../test/helpers/mainController' +import { suppressConsoleBeforeEach } from '../../../test/helpers/console' +import { DEFAULT_ACCOUNT_LABEL } from '../../consts/account' +import { MainController } from './main' + +const EXPORTING_PASS = 'exportingDevicePass' +const IMPORTING_PASS = 'importingDevicePass' + +const firstWallet = Wallet.createRandom() +const secondWallet = Wallet.createRandom() + +const toAccount = (addr: string, label: string) => ({ + addr, + associatedKeys: [addr], + initialPrivileges: [], + creation: null, + preferences: { label, pfp: addr } +}) + +const accounts = [ + toAccount(firstWallet.address, 'Account 1'), + toAccount(secondWallet.address, DEFAULT_ACCOUNT_LABEL) +] + +const makeExportingDevice = async () => { + const { mainCtrl } = await makeMainController(async (storageCtrl) => { + await storageCtrl.set('accounts', accounts) + await storageCtrl.set('selectedAccount', accounts[0]!.addr) + }) + + await mainCtrl.keystore.addSecret('password', EXPORTING_PASS, '', true) + await mainCtrl.keystore.addKeys( + [firstWallet, secondWallet].map((wallet, i) => ({ + addr: wallet.address, + label: `Key ${i + 1}`, + type: 'internal' as const, + privateKey: wallet.privateKey, + dedicatedToOneSA: false, + meta: { createdAt: new Date().getTime() } + })) + ) + + return mainCtrl +} + +const makeImportingDevice = async ({ withPassword }: { withPassword: boolean }) => { + const { mainCtrl } = await makeMainController() + + if (withPassword) await mainCtrl.keystore.addSecret('password', IMPORTING_PASS, '', true) + + return mainCtrl +} + +const exportPayload = async (mainCtrl: MainController, addrs: string[]) => { + const sendUiMessage = jest.spyOn(mainCtrl.ui.message, 'sendUiMessage') + + await mainCtrl.exportAccountsForSync(addrs) + + const { accountsSyncPayload } = (sendUiMessage.mock.calls[0]?.[0] || {}) as { + accountsSyncPayload?: string + } + sendUiMessage.mockRestore() + + return accountsSyncPayload as string +} + +describe('MainController accounts sync', () => { + test('exports only the selected accounts and imports them on the other device', async () => { + const exportingDevice = await makeExportingDevice() + const payload = await exportPayload(exportingDevice, [accounts[0]!.addr]) + + const importingDevice = await makeImportingDevice({ withPassword: true }) + await importingDevice.importAccountsFromSync({ payload, password: EXPORTING_PASS }) + + expect(importingDevice.accounts.accounts.map((a) => a.addr)).toEqual([accounts[0]!.addr]) + // Preferences travel along, so the account looks the same on both devices + expect(importingDevice.accounts.accounts[0]!.preferences.label).toBe('Account 1') + // The key is re-encrypted with the importing device's main key, so it can sign + const signer = await importingDevice.keystore.getSigner(accounts[0]!.addr, 'internal') + expect(signer.key.addr).toBe(accounts[0]!.addr) + }) + + test('imports accounts scanned before the device password was set (onboarding)', async () => { + const exportingDevice = await makeExportingDevice() + const payload = await exportPayload( + exportingDevice, + accounts.map((a) => a.addr) + ) + + const importingDevice = await makeImportingDevice({ withPassword: false }) + await importingDevice.importAccountsFromSync({ payload, password: EXPORTING_PASS }) + + // The accounts are already there, the keys wait for a main key to be encrypted with + expect(importingDevice.accounts.accounts).toHaveLength(2) + expect(importingDevice.keystore.keys).toHaveLength(0) + + await importingDevice.keystore.addSecret('password', IMPORTING_PASS, '', true) + + expect(importingDevice.keystore.keys.map((k) => k.addr)).toEqual(accounts.map((a) => a.addr)) + }) + + describe('Negative cases', () => { + suppressConsoleBeforeEach() + + test('adds no accounts when the password of the other device is wrong', async () => { + const exportingDevice = await makeExportingDevice() + const payload = await exportPayload(exportingDevice, [accounts[0]!.addr]) + + const importingDevice = await makeImportingDevice({ withPassword: true }) + await importingDevice.importAccountsFromSync({ payload, password: 'wrongPass' }) + + expect(importingDevice.emittedErrors.at(-1)?.message).toBe( + 'Incorrect password. Please try again.' + ) + expect(importingDevice.accounts.accounts).toHaveLength(0) + expect(importingDevice.keystore.keys).toHaveLength(0) + }) + + test('adds no accounts when the scanned data is not a sync payload', async () => { + const importingDevice = await makeImportingDevice({ withPassword: true }) + + await importingDevice.importAccountsFromSync({ + payload: '0x010203', + password: IMPORTING_PASS + }) + + expect(importingDevice.emittedErrors.at(-1)?.message).toContain( + 'do not contain Ambire accounts' + ) + expect(importingDevice.accounts.accounts).toHaveLength(0) + }) + + test('exports nothing when no account is selected', async () => { + const exportingDevice = await makeExportingDevice() + const sendUiMessage = jest.spyOn(exportingDevice.ui.message, 'sendUiMessage') + + await exportingDevice.exportAccountsForSync([]) + + expect(exportingDevice.emittedErrors.at(-1)?.message).toBe( + 'Select at least one account to sync.' + ) + expect(sendUiMessage).not.toHaveBeenCalled() + }) + }) +}) diff --git a/src/interfaces/main.ts b/src/interfaces/main.ts index 5e35cdd895..23c376e40f 100644 --- a/src/interfaces/main.ts +++ b/src/interfaces/main.ts @@ -14,5 +14,7 @@ export const STATUS_WRAPPED_METHODS = { handleAccountPickerInitNfc: 'INITIAL', importSmartAccountFromDefaultSeed: 'INITIAL', selectAccount: 'INITIAL', - accountPickerSetInitParamsFromNewSeed: 'INITIAL' + accountPickerSetInitParamsFromNewSeed: 'INITIAL', + exportAccountsForSync: 'INITIAL', + importAccountsFromSync: 'INITIAL' } as const From 38aa6ac37956a09bd60df8970dc268899447c0b2 Mon Sep 17 00:00:00 2001 From: sonytooo Date: Wed, 12 Aug 2026 14:03:35 +0300 Subject: [PATCH 03/15] feat: reply to the UI request that triggered an accounts sync step The UI awaits the export payload and the import result through the existing one time data channel (requestId), instead of watching a transient status, which mobile can collapse before the UI renders it. Co-Authored-By: Claude Opus 5 (1M context) --- src/controllers/main/main.ts | 55 ++++++++++++++----- src/controllers/main/mainAccountsSync.test.ts | 18 +++--- 2 files changed, 53 insertions(+), 20 deletions(-) diff --git a/src/controllers/main/main.ts b/src/controllers/main/main.ts index c690134785..ea4f4c8362 100644 --- a/src/controllers/main/main.ts +++ b/src/controllers/main/main.ts @@ -1663,13 +1663,41 @@ export class MainController extends EventEmitter implements IMainController { await this.withStatus('updateAccounts', async () => this.#updateAccounts(accountsUpdate)) } + /** + * Runs an accounts sync step and replies to the UI request that triggered it, so the + * UI can await the result instead of watching a transient status. + */ + async #withSyncResponse( + callName: 'exportAccountsForSync' | 'importAccountsFromSync', + requestId: string | undefined, + fn: () => Promise + ) { + await this.withStatus(callName, async () => { + try { + const res = await fn() + + if (requestId) this.ui.message.sendUiMessage({ requestId, ok: true, res }) + } catch (error: any) { + // Rethrown, so that `withStatus` emits (and reports) the error as usual + if (requestId) + this.ui.message.sendUiMessage({ + requestId, + ok: false, + error: error?.message || `${callName} failed` + }) + + throw error + } + }) + } + /** * Prepares the selected accounts and the keys controlling them for the other Ambire - * product and sends the result to the UI, which displays it as animated QR codes. + * product and returns the payload, which the UI displays as animated QR codes. * Everything sensitive leaves this device encrypted, see `keystore.exportForSync`. */ - async exportAccountsForSync(addrs: Account['addr'][]) { - await this.withStatus('exportAccountsForSync', async () => { + async exportAccountsForSync(addrs: Account['addr'][], requestId?: string) { + await this.#withSyncResponse('exportAccountsForSync', requestId, async () => { const accounts = this.accounts.getAccountsForSync(addrs) if (!accounts.length) @@ -1682,14 +1710,12 @@ export class MainController extends EventEmitter implements IMainController { const keyAddrs = Array.from(new Set(accounts.flatMap((account) => account.associatedKeys))) const { secret, keys, seeds } = await this.keystore.exportForSync(keyAddrs) - this.ui.message.sendUiMessage({ - accountsSyncPayload: serializeAccountsSyncPayload({ - v: ACCOUNTS_SYNC_PAYLOAD_VERSION, - secret, - accounts, - keys, - seeds - }) + return serializeAccountsSyncPayload({ + v: ACCOUNTS_SYNC_PAYLOAD_VERSION, + secret, + accounts, + keys, + seeds }) }) } @@ -1699,8 +1725,11 @@ export class MainController extends EventEmitter implements IMainController { * `payload` is the hex encoded data assembled from the scanned codes and `password` * is the device password of the product that exported them. */ - async importAccountsFromSync({ payload, password }: { payload: string; password: string }) { - await this.withStatus('importAccountsFromSync', async () => { + async importAccountsFromSync( + { payload, password }: { payload: string; password: string }, + requestId?: string + ) { + await this.#withSyncResponse('importAccountsFromSync', requestId, async () => { let parsedPayload: AccountsSyncPayload try { parsedPayload = parseAccountsSyncPayload(getBytes(payload)) diff --git a/src/controllers/main/mainAccountsSync.test.ts b/src/controllers/main/mainAccountsSync.test.ts index 6b9839c62c..ad45d4b7f4 100644 --- a/src/controllers/main/mainAccountsSync.test.ts +++ b/src/controllers/main/mainAccountsSync.test.ts @@ -58,14 +58,14 @@ const makeImportingDevice = async ({ withPassword }: { withPassword: boolean }) const exportPayload = async (mainCtrl: MainController, addrs: string[]) => { const sendUiMessage = jest.spyOn(mainCtrl.ui.message, 'sendUiMessage') - await mainCtrl.exportAccountsForSync(addrs) + await mainCtrl.exportAccountsForSync(addrs, 'request-1') - const { accountsSyncPayload } = (sendUiMessage.mock.calls[0]?.[0] || {}) as { - accountsSyncPayload?: string - } + const response = (sendUiMessage.mock.calls[0]?.[0] || {}) as { ok?: boolean; res?: string } sendUiMessage.mockRestore() - return accountsSyncPayload as string + expect(response.ok).toBe(true) + + return response.res as string } describe('MainController accounts sync', () => { @@ -138,12 +138,16 @@ describe('MainController accounts sync', () => { const exportingDevice = await makeExportingDevice() const sendUiMessage = jest.spyOn(exportingDevice.ui.message, 'sendUiMessage') - await exportingDevice.exportAccountsForSync([]) + await exportingDevice.exportAccountsForSync([], 'request-1') expect(exportingDevice.emittedErrors.at(-1)?.message).toBe( 'Select at least one account to sync.' ) - expect(sendUiMessage).not.toHaveBeenCalled() + expect(sendUiMessage).toHaveBeenCalledWith({ + requestId: 'request-1', + ok: false, + error: 'Select at least one account to sync.' + }) }) }) }) From cb3a6c1bac54a77ed02f931a63ed96c0a146cebf Mon Sep 17 00:00:00 2001 From: sonytooo Date: Wed, 12 Aug 2026 23:23:09 +0300 Subject: [PATCH 04/15] test: cover the remaining accounts sync cases and harden the export - accounts with no keys, accounts controlled by a hardware wallet, re-syncing the same accounts and syncing keys whose seed the device already has - exportForSync refuses to export keys or seeds that are still on the old encryption, so it fails on the device that can explain it instead of on the other one - lock() drops the queued keys and seeds, as they hold decrypted material while waiting for the device password during onboarding Co-Authored-By: Claude Opus 5 (1M context) --- src/controllers/keystore/keystore.test.ts | 20 ++++++ src/controllers/keystore/keystore.ts | 24 +++++++ src/controllers/main/mainAccountsSync.test.ts | 69 ++++++++++++++++++- 3 files changed, 110 insertions(+), 3 deletions(-) diff --git a/src/controllers/keystore/keystore.test.ts b/src/controllers/keystore/keystore.test.ts index bbd9e6e899..79678f3900 100644 --- a/src/controllers/keystore/keystore.test.ts +++ b/src/controllers/keystore/keystore.test.ts @@ -695,6 +695,26 @@ describe('accounts sync between two devices', () => { expect(signer.key.addr).toBe(keyPublicAddress) }) + test('links the synced keys to a seed the device already has', async () => { + await importingKeystore.addSecret('password', importingPass, '', true) + // The very same recovery phrase was already imported on this device, under an id of + // its own, so the synced keys have to be linked to that one + await importingKeystore.addTempSeed({ + seed: process.env.SEED, + hdPathTemplate: BIP44_STANDARD_DERIVATION_TEMPLATE + }) + await importingKeystore.persistTempSeed() + const alreadyStoredSeedId = importingKeystore.seeds[0]!.id + + await importingKeystore.importFromSync(await buildPayload([keyPublicAddress]), exportingPass) + + expect(importingKeystore.seeds).toHaveLength(1) + expect(importingKeystore.seeds[0]!.id).toBe(alreadyStoredSeedId) + expect(importingKeystore.keys.find((k) => k.type === 'internal')?.meta.fromSeedId).toBe( + alreadyStoredSeedId + ) + }) + test('syncing the same accounts twice does not duplicate keys or seeds', async () => { await importingKeystore.addSecret('password', importingPass, '', true) const payload = await buildPayload([keyPublicAddress, EXTERNAL_ADDR]) diff --git a/src/controllers/keystore/keystore.ts b/src/controllers/keystore/keystore.ts index 57f4df9578..5e15cf81c4 100644 --- a/src/controllers/keystore/keystore.ts +++ b/src/controllers/keystore/keystore.ts @@ -38,6 +38,7 @@ import { InternalKey, Key, KeyPreferences, + KeystoreEncryptedPayload, KeystoreSeed, KeystoreSignerInterface, KeystoreSignerType, @@ -201,6 +202,10 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl lock() { this.#mainKey = null if (this.#tempSeed) this.deleteTempSeed(false) + // Anything still queued holds decrypted keys and seed phrases (a sync that happened + // before the device password was set), so it must not outlive a lock + this.#seedsToAddOnKeystoreReady = [] + this.#internalKeysToAddOnKeystoreReady = [] this.emitUpdate() } @@ -1233,6 +1238,25 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl const seedIds = new Set(keys.map((key) => key.meta?.fromSeedId).filter(Boolean)) const seeds = this.#keystoreSeeds.filter((seed) => seedIds.has(seed.id)) + // Keys and seeds are migrated to GCM on unlock, so this should never happen. If it + // does, fail here rather than on the other device, which would reject the payload + const isEncryptedWithGCM = (payload: KeystoreEncryptedPayload) => + typeof payload !== 'string' && payload?.cipherType === CIPHER + const hasNotMigratedPayload = + keys.some((key) => key.type === 'internal' && !isEncryptedWithGCM(key.privKey)) || + seeds.some( + (seed) => + !isEncryptedWithGCM(seed.seed) || + (!!seed.seedPassphrase && !isEncryptedWithGCM(seed.seedPassphrase)) + ) + + if (hasNotMigratedPayload) + throw new EmittableError({ + level: 'major', + message: 'Something went wrong when preparing your accounts for syncing. Please try again.', + error: new Error('keystore: keys or seeds not migrated to GCM yet') + }) + return { secret, keys, seeds } } diff --git a/src/controllers/main/mainAccountsSync.test.ts b/src/controllers/main/mainAccountsSync.test.ts index ad45d4b7f4..b7646dd637 100644 --- a/src/controllers/main/mainAccountsSync.test.ts +++ b/src/controllers/main/mainAccountsSync.test.ts @@ -5,6 +5,7 @@ import { describe, expect, jest, test } from '@jest/globals' import { makeMainController } from '../../../test/helpers/mainController' import { suppressConsoleBeforeEach } from '../../../test/helpers/console' import { DEFAULT_ACCOUNT_LABEL } from '../../consts/account' +import { BIP44_STANDARD_DERIVATION_TEMPLATE } from '../../consts/derivation' import { MainController } from './main' const EXPORTING_PASS = 'exportingDevicePass' @@ -21,9 +22,14 @@ const toAccount = (addr: string, label: string) => ({ preferences: { label, pfp: addr } }) +const VIEW_ONLY_ADDR = '0xd8dA6BF26964aF9D7eEd9e03E53415D37aA96045' +const LEDGER_ADDR = '0x1A2C3802A9eC12725678dAF23DbFD13134e5893A' + const accounts = [ toAccount(firstWallet.address, 'Account 1'), - toAccount(secondWallet.address, DEFAULT_ACCOUNT_LABEL) + toAccount(secondWallet.address, DEFAULT_ACCOUNT_LABEL), + toAccount(VIEW_ONLY_ADDR, 'Watched account'), + toAccount(LEDGER_ADDR, 'Ledger account') ] const makeExportingDevice = async () => { @@ -43,6 +49,21 @@ const makeExportingDevice = async () => { meta: { createdAt: new Date().getTime() } })) ) + await mainCtrl.keystore.addKeysExternallyStored([ + { + addr: LEDGER_ADDR, + label: 'Ledger Key 1', + type: 'ledger', + dedicatedToOneSA: false, + meta: { + deviceId: '1', + deviceModel: 'nanoX', + hdPathTemplate: BIP44_STANDARD_DERIVATION_TEMPLATE, + index: 0, + createdAt: new Date().getTime() + } + } + ]) return mainCtrl } @@ -95,12 +116,54 @@ describe('MainController accounts sync', () => { await importingDevice.importAccountsFromSync({ payload, password: EXPORTING_PASS }) // The accounts are already there, the keys wait for a main key to be encrypted with - expect(importingDevice.accounts.accounts).toHaveLength(2) + expect(importingDevice.accounts.accounts).toHaveLength(accounts.length) expect(importingDevice.keystore.keys).toHaveLength(0) await importingDevice.keystore.addSecret('password', IMPORTING_PASS, '', true) - expect(importingDevice.keystore.keys.map((k) => k.addr)).toEqual(accounts.map((a) => a.addr)) + // Every account that has a key on the other device can sign on this one as well + expect(importingDevice.keystore.keys.map((k) => k.addr)).toEqual([ + firstWallet.address, + secondWallet.address, + LEDGER_ADDR + ]) + }) + + test('syncs an account that has no keys at all', async () => { + const exportingDevice = await makeExportingDevice() + const payload = await exportPayload(exportingDevice, [VIEW_ONLY_ADDR]) + + const importingDevice = await makeImportingDevice({ withPassword: true }) + await importingDevice.importAccountsFromSync({ payload, password: EXPORTING_PASS }) + + expect(importingDevice.accounts.accounts.map((a) => a.addr)).toEqual([VIEW_ONLY_ADDR]) + // It stays a watched account on this device too + expect(importingDevice.keystore.keys).toHaveLength(0) + }) + + test('syncs an account controlled by a hardware wallet, without a private key to move', async () => { + const exportingDevice = await makeExportingDevice() + const payload = await exportPayload(exportingDevice, [LEDGER_ADDR]) + + const importingDevice = await makeImportingDevice({ withPassword: true }) + await importingDevice.importAccountsFromSync({ payload, password: EXPORTING_PASS }) + + expect(importingDevice.accounts.accounts.map((a) => a.addr)).toEqual([LEDGER_ADDR]) + expect(importingDevice.keystore.keys).toEqual([ + expect.objectContaining({ addr: LEDGER_ADDR, type: 'ledger', isExternallyStored: true }) + ]) + }) + + test('does not duplicate an account the other device already has', async () => { + const exportingDevice = await makeExportingDevice() + const payload = await exportPayload(exportingDevice, [accounts[0]!.addr]) + + const importingDevice = await makeImportingDevice({ withPassword: true }) + await importingDevice.importAccountsFromSync({ payload, password: EXPORTING_PASS }) + await importingDevice.importAccountsFromSync({ payload, password: EXPORTING_PASS }) + + expect(importingDevice.accounts.accounts).toHaveLength(1) + expect(importingDevice.keystore.keys).toHaveLength(1) }) describe('Negative cases', () => { From f1d68040b4ba5c56658dcb5bd849e989a2a6bf7e Mon Sep 17 00:00:00 2001 From: sonytooo Date: Fri, 14 Aug 2026 12:27:13 +0300 Subject: [PATCH 05/15] optionally export seeds --- package-lock.json | 4 ++-- src/controllers/keystore/keystore.test.ts | 17 +++++++++++++++++ src/controllers/keystore/keystore.ts | 9 +++++++-- src/controllers/main/main.ts | 11 +++++++++-- src/controllers/main/mainAccountsSync.test.ts | 4 ++-- 5 files changed, 37 insertions(+), 8 deletions(-) diff --git a/package-lock.json b/package-lock.json index c2bfa2e689..05e7ddfdce 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "ambire-common", - "version": "2.107.1", + "version": "2.107.3", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "ambire-common", - "version": "2.107.1", + "version": "2.107.3", "dependencies": { "@ambire/signature-validator": "^1.5.0", "@corpus-core/colibri-stateless": "^1.1.30", diff --git a/src/controllers/keystore/keystore.test.ts b/src/controllers/keystore/keystore.test.ts index 79678f3900..6407f845d6 100644 --- a/src/controllers/keystore/keystore.test.ts +++ b/src/controllers/keystore/keystore.test.ts @@ -629,6 +629,23 @@ describe('accounts sync between two devices', () => { expect(exported.seeds).toHaveLength(0) }) + test('leaves the seed behind when the user opted out of exporting it', async () => { + const exported = await exportingKeystore.exportForSync([keyPublicAddress], false) + + expect(exported.keys.map((k) => k.addr)).toEqual([keyPublicAddress]) + expect(exported.seeds).toHaveLength(0) + + // The key still signs on the other device, it is just no longer tied to a seed + await importingKeystore.addSecret('password', importingPass, '', true) + await importingKeystore.importFromSync( + { v: 1 as const, accounts: [], ...exported }, + exportingPass + ) + + expect(importingKeystore.seeds).toHaveLength(0) + expect(importingKeystore.keys[0]!.meta.fromSeedId).toBeUndefined() + }) + describe('Negative cases', () => { suppressConsoleBeforeEach() diff --git a/src/controllers/keystore/keystore.ts b/src/controllers/keystore/keystore.ts index 5e15cf81c4..793d73fa04 100644 --- a/src/controllers/keystore/keystore.ts +++ b/src/controllers/keystore/keystore.ts @@ -1212,9 +1212,14 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl * keys: the keys themselves and the seeds they were derived from (still encrypted * with this device's main key), plus this device's main key wrapped with its password. * Nothing gets decrypted here, so this works even when the keystore is locked. + * + * `includeSeeds` is what the user chose on the confirmation screen: with it off the + * keys still travel (so the accounts can sign on the other device), but the recovery + * phrases they were derived from stay on this device. */ async exportForSync( - keyAddrs: Key['addr'][] + keyAddrs: Key['addr'][], + includeSeeds: boolean = true ): Promise> { await this.initialLoadPromise @@ -1236,7 +1241,7 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl const keys = this.#keystoreKeys.filter((key) => keyAddrs.includes(key.addr)) const seedIds = new Set(keys.map((key) => key.meta?.fromSeedId).filter(Boolean)) - const seeds = this.#keystoreSeeds.filter((seed) => seedIds.has(seed.id)) + const seeds = includeSeeds ? this.#keystoreSeeds.filter((seed) => seedIds.has(seed.id)) : [] // Keys and seeds are migrated to GCM on unlock, so this should never happen. If it // does, fail here rather than on the other device, which would reject the payload diff --git a/src/controllers/main/main.ts b/src/controllers/main/main.ts index fe7c166828..1490030f9b 100644 --- a/src/controllers/main/main.ts +++ b/src/controllers/main/main.ts @@ -1695,8 +1695,15 @@ export class MainController extends EventEmitter implements IMainController { * Prepares the selected accounts and the keys controlling them for the other Ambire * product and returns the payload, which the UI displays as animated QR codes. * Everything sensitive leaves this device encrypted, see `keystore.exportForSync`. + * + * `includeSeeds` lets the user leave the recovery phrases of the selected accounts + * behind, in which case only the accounts and their keys are sent over. */ - async exportAccountsForSync(addrs: Account['addr'][], requestId?: string) { + async exportAccountsForSync( + addrs: Account['addr'][], + includeSeeds: boolean = true, + requestId?: string + ) { await this.#withSyncResponse('exportAccountsForSync', requestId, async () => { const accounts = this.accounts.getAccountsForSync(addrs) @@ -1708,7 +1715,7 @@ export class MainController extends EventEmitter implements IMainController { }) const keyAddrs = Array.from(new Set(accounts.flatMap((account) => account.associatedKeys))) - const { secret, keys, seeds } = await this.keystore.exportForSync(keyAddrs) + const { secret, keys, seeds } = await this.keystore.exportForSync(keyAddrs, includeSeeds) return serializeAccountsSyncPayload({ v: ACCOUNTS_SYNC_PAYLOAD_VERSION, diff --git a/src/controllers/main/mainAccountsSync.test.ts b/src/controllers/main/mainAccountsSync.test.ts index b7646dd637..d7ff7f533c 100644 --- a/src/controllers/main/mainAccountsSync.test.ts +++ b/src/controllers/main/mainAccountsSync.test.ts @@ -79,7 +79,7 @@ const makeImportingDevice = async ({ withPassword }: { withPassword: boolean }) const exportPayload = async (mainCtrl: MainController, addrs: string[]) => { const sendUiMessage = jest.spyOn(mainCtrl.ui.message, 'sendUiMessage') - await mainCtrl.exportAccountsForSync(addrs, 'request-1') + await mainCtrl.exportAccountsForSync(addrs, true, 'request-1') const response = (sendUiMessage.mock.calls[0]?.[0] || {}) as { ok?: boolean; res?: string } sendUiMessage.mockRestore() @@ -201,7 +201,7 @@ describe('MainController accounts sync', () => { const exportingDevice = await makeExportingDevice() const sendUiMessage = jest.spyOn(exportingDevice.ui.message, 'sendUiMessage') - await exportingDevice.exportAccountsForSync([], 'request-1') + await exportingDevice.exportAccountsForSync([], true, 'request-1') expect(exportingDevice.emittedErrors.at(-1)?.message).toBe( 'Select at least one account to sync.' From b570cae3b2f27d752aa37bd4ead627a1d16a1162 Mon Sep 17 00:00:00 2001 From: sonytooo Date: Fri, 14 Aug 2026 13:17:09 +0300 Subject: [PATCH 06/15] fixes --- src/controllers/keystore/keystore.test.ts | 53 +++++++++++++++++++++++ src/controllers/keystore/keystore.ts | 14 ++++-- 2 files changed, 63 insertions(+), 4 deletions(-) diff --git a/src/controllers/keystore/keystore.test.ts b/src/controllers/keystore/keystore.test.ts index 6407f845d6..2a52a4936a 100644 --- a/src/controllers/keystore/keystore.test.ts +++ b/src/controllers/keystore/keystore.test.ts @@ -15,6 +15,7 @@ import { import { ExternalKey, IKeystoreController, InternalKey } from '../../interfaces/keystore' import { getPrivateKeyFromSeed, KeyIterator } from '../../libs/keyIterator/keyIterator' import { stripHexPrefix } from '../../utils/stripHexPrefix' +import wait from '../../utils/wait' import { StorageController } from '../storage/storage' import { UiController } from '../ui/ui' import { KeystoreController } from './keystore' @@ -562,6 +563,17 @@ describe('accounts sync between two devices', () => { uiCtrl ) + // The in-memory store resolves writes immediately, which hides ordering a real + // (async) device store would expose + const withSlowWrites = (store: ReturnType) => ({ + ...store, + set: async (key: any, value: any) => { + await wait(10) + + return store.set(key, value) + } + }) + const buildPayload = (keyAddrs: string[]) => exportingKeystore .exportForSync(keyAddrs) @@ -712,6 +724,47 @@ describe('accounts sync between two devices', () => { expect(signer.key.addr).toBe(keyPublicAddress) }) + test('tells the UI about the synced keys right after the import', async () => { + const payload = await buildPayload([keyPublicAddress, EXTERNAL_ADDR]) + await importingKeystore.addSecret('password', importingPass, '', true) + + // Without an update the UI keeps the keystore state it had before the sync, which + // makes every imported account look view-only + const keyCountsSeenByTheUi: number[] = [] + importingKeystore.onUpdate(() => keyCountsSeenByTheUi.push(importingKeystore.keys.length)) + + await importingKeystore.importFromSync(payload, exportingPass) + + expect(importingKeystore.keys).toHaveLength(2) + expect(keyCountsSeenByTheUi.at(-1)).toBe(2) + }) + + test('tells the UI about the synced keys once the onboarding password stores them', async () => { + const payload = await buildPayload([keyPublicAddress, EXTERNAL_ADDR]) + // Storing the queued keys is detached from `addSecret`, so on a real device (where + // writes take a moment) it finishes after `addSecret` has already emitted. Without an + // update of its own the UI keeps rendering the synced accounts as if they had no keys. + const slowKeystore = new KeystoreController( + 'default', + new StorageController(withSlowWrites(produceMemoryStore())), + keystoreSigners, + uiCtrl + ) + await slowKeystore.importFromSync(payload, exportingPass) + + const keyCountsSeenByTheUi: number[] = [] + slowKeystore.onUpdate(() => keyCountsSeenByTheUi.push(slowKeystore.keys.length)) + + await slowKeystore.addSecret('password', importingPass, '', true) + // The detached storing is still in flight here, so give it room to finish and emit + for (let i = 0; i < 100 && keyCountsSeenByTheUi.at(-1) !== 2; i++) { + await wait(20) + } + + expect(slowKeystore.keys).toHaveLength(2) + expect(keyCountsSeenByTheUi.at(-1)).toBe(2) + }) + test('links the synced keys to a seed the device already has', async () => { await importingKeystore.addSecret('password', importingPass, '', true) // The very same recovery phrase was already imported on this device, under an id of diff --git a/src/controllers/keystore/keystore.ts b/src/controllers/keystore/keystore.ts index 793d73fa04..5c8a4ad5af 100644 --- a/src/controllers/keystore/keystore.ts +++ b/src/controllers/keystore/keystore.ts @@ -813,6 +813,8 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl 'Something went wrong when saving your keys. Please try again or contact support if the problem persists.', error: error instanceof Error ? error : new Error('keystore: failed to add queued keys') }) + } finally { + this.emitUpdate() } } @@ -1325,10 +1327,14 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl }) } - // Both queue themselves until the keystore is ready to store keys, which is what - // makes syncing before the device password is set (onboarding) work - await this.#addKeys(internalKeys) - await this.#addKeysExternallyStored(externalKeys) + try { + // Both queue themselves until the keystore is ready to store keys, which is what + // makes syncing before the device password is set (onboarding) work + await this.#addKeys(internalKeys) + await this.#addKeysExternallyStored(externalKeys) + } finally { + this.emitUpdate() + } } /** From fe1fff9b85ba54386ff42a890a4edcef0a55bce7 Mon Sep 17 00:00:00 2001 From: sonytooo Date: Mon, 17 Aug 2026 13:11:41 +0300 Subject: [PATCH 07/15] scan performance fixes --- package-lock.json | 25 +++++++++++ package.json | 2 + src/libs/accountsSync/accountsSync.test.ts | 34 ++++++++++++++- src/libs/accountsSync/accountsSync.ts | 50 +++++++++++++++++++++- 4 files changed, 107 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index 05e7ddfdce..b2d4e77af9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -23,6 +23,7 @@ "events": "^3.3.0", "hash-wasm": "^4.12.0", "js-yaml": "^4.1.0", + "pako": "^2.1.0", "scrypt-js": "^3.0.1", "tldts": "7.0.17", "uuid": "9.0.0", @@ -43,6 +44,7 @@ "@types/mocha": "10.0.1", "@types/node": "24.10.11", "@types/node-fetch": "2.6.11", + "@types/pako": "2.0.4", "@types/ungap__structured-clone": "1.2.0", "@types/validator": "13.7.17", "@typescript-eslint/eslint-plugin": "8.59.2", @@ -5167,6 +5169,13 @@ "form-data": "^4.0.0" } }, + "node_modules/@types/pako": { + "version": "2.0.4", + "resolved": "https://registry.npmjs.org/@types/pako/-/pako-2.0.4.tgz", + "integrity": "sha512-VWDCbrLeVXJM9fihYodcLiIv0ku+AlOa/TQ1SvYOaBuyrSKgEcro95LJyIsJ4vSo6BXIxOKxiJAat04CmST9Fw==", + "dev": true, + "license": "MIT" + }, "node_modules/@types/pbkdf2": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/@types/pbkdf2/-/pbkdf2-3.1.0.tgz", @@ -15448,6 +15457,22 @@ "dev": true, "license": "BlueOak-1.0.0" }, + "node_modules/pako": { + "version": "2.2.0", + "resolved": "https://registry.npmjs.org/pako/-/pako-2.2.0.tgz", + "integrity": "sha512-zJq6RP/5q+TO2OpFV3FHzlPnFjmkb7Nc99a5SNjJE+uu/PkpChs+NIZSSzbBoD+6kjiISXjfYdwj1ZRQ81dz/w==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/puzrin" + }, + { + "type": "github", + "url": "https://github.com/sponsors/nodeca" + } + ], + "license": "(MIT AND Zlib)" + }, "node_modules/parent-module": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/parent-module/-/parent-module-1.0.1.tgz", diff --git a/package.json b/package.json index d0e7e04584..9455b781b8 100644 --- a/package.json +++ b/package.json @@ -30,6 +30,7 @@ "events": "^3.3.0", "hash-wasm": "^4.12.0", "js-yaml": "^4.1.0", + "pako": "^2.1.0", "scrypt-js": "^3.0.1", "tldts": "7.0.17", "uuid": "9.0.0", @@ -62,6 +63,7 @@ "@types/mocha": "10.0.1", "@types/node": "24.10.11", "@types/node-fetch": "2.6.11", + "@types/pako": "2.0.4", "@types/ungap__structured-clone": "1.2.0", "@types/validator": "13.7.17", "@typescript-eslint/eslint-plugin": "8.59.2", diff --git a/src/libs/accountsSync/accountsSync.test.ts b/src/libs/accountsSync/accountsSync.test.ts index 40315db94a..72ff6bd92c 100644 --- a/src/libs/accountsSync/accountsSync.test.ts +++ b/src/libs/accountsSync/accountsSync.test.ts @@ -1,4 +1,5 @@ -import { getBytes } from 'ethers' +import { getBytes, toUtf8Bytes } from 'ethers' +import { gzip } from 'pako' import { CIPHER } from '@/libs/keystore/keystore' @@ -86,12 +87,41 @@ describe('accountsSync payload', () => { expect(serializeAndParse(payload)).toEqual(payload) }) - it('rejects data that is not a sync payload', () => { + it('rejects data that is not compressed at all', () => { expect(() => parseAccountsSyncPayload(new Uint8Array([1, 2, 3]))).toThrow( + 'failed to decompress the payload' + ) + }) + + it('rejects an uncompressed payload, which no Ambire product produces', () => { + const uncompressed = toUtf8Bytes(JSON.stringify(buildPayload())) + + expect(() => parseAccountsSyncPayload(uncompressed)).toThrow('failed to decompress the payload') + }) + + it('rejects compressed data that is not a sync payload', () => { + expect(() => parseAccountsSyncPayload(gzip(toUtf8Bytes('not json')))).toThrow( 'not a valid sync payload' ) }) + it('rejects a payload that decompresses to more than a sync payload could ever be', () => { + // Compresses to a few KB but inflates past the 2MB limit, which is how a hostile QR + // code would try to exhaust the memory of the device scanning it + const bomb = gzip(new Uint8Array(3 * 1024 * 1024)) + + expect(() => parseAccountsSyncPayload(bomb)).toThrow('too large to be a sync payload') + }) + + it('compresses the payload well below its JSON size', () => { + const payload = buildPayload() + const jsonSize = toUtf8Bytes(JSON.stringify(payload)).length + // `serializeAccountsSyncPayload` returns a hex string, so 2 chars per wire byte + const wireSize = (serializeAccountsSyncPayload(payload).length - 2) / 2 + + expect(wireSize).toBeLessThan(jsonSize / 2) + }) + it('rejects an unsupported payload version', () => { expect(() => serializeAndParse({ ...buildPayload(), v: 2 })).toThrow( 'unsupported payload version 2' diff --git a/src/libs/accountsSync/accountsSync.ts b/src/libs/accountsSync/accountsSync.ts index ec61bdb0f1..cb07e29931 100644 --- a/src/libs/accountsSync/accountsSync.ts +++ b/src/libs/accountsSync/accountsSync.ts @@ -1,4 +1,5 @@ import { hexlify, isAddress, toUtf8Bytes, toUtf8String } from 'ethers' +import { gzip, Inflate } from 'pako' import { Account } from '../../interfaces/account' import { @@ -88,11 +89,52 @@ const validateSeeds = (seeds: any) => { }) } +/** + * A payload takes a few hundred bytes per account, so this leaves room for far more + * accounts than anyone holds, while keeping a hostile QR code from inflating into + * gigabytes of memory in the background. + */ +const MAX_DECOMPRESSED_PAYLOAD_SIZE = 2 * 1024 * 1024 + +/** + * Inflates the scanned bytes, giving up as soon as they expand past what a sync payload + * could ever be, instead of allocating whatever the compressed data asks for. + */ +const decompress = (bytes: Uint8Array): Uint8Array => { + const inflate = new Inflate() + const chunks: Uint8Array[] = [] + let size = 0 + + // pako has no way to abort a stream, so oversized chunks are counted and dropped + // (which keeps the memory flat) and the whole stream is rejected afterwards + inflate.onData = (chunk: Uint8Array) => { + size += chunk.length + if (size <= MAX_DECOMPRESSED_PAYLOAD_SIZE) chunks.push(chunk) + } + + inflate.push(bytes, true) + + if (inflate.err) throw new Error(`accountsSync: failed to decompress the payload: ${inflate.msg}`) + if (size > MAX_DECOMPRESSED_PAYLOAD_SIZE) + throw new Error('accountsSync: the payload is too large to be a sync payload') + + const decompressed = new Uint8Array(size) + let offset = 0 + chunks.forEach((chunk) => { + decompressed.set(chunk, offset) + offset += chunk.length + }) + + return decompressed +} + /** * Serializes the payload to the hex encoded bytes carried by the animated QR codes. + * Gzipped first, because JSON full of hex strings compresses 3x or better, and every + * byte saved is one less QR frame the other device has to catch. */ export const serializeAccountsSyncPayload = (payload: AccountsSyncPayload): string => - hexlify(toUtf8Bytes(JSON.stringify(payload))) + hexlify(gzip(toUtf8Bytes(JSON.stringify(payload)))) /** * Parses and validates the bytes assembled from the scanned animated QR codes. @@ -100,9 +142,13 @@ export const serializeAccountsSyncPayload = (payload: AccountsSyncPayload): stri * incompatible (newer) app version. */ export const parseAccountsSyncPayload = (bytes: Uint8Array): AccountsSyncPayload => { + // Left to throw on its own, so that a corrupt stream and an oversized one stay + // distinguishable from data that decompressed fine but isn't a sync payload + const decompressed = decompress(bytes) + let payload: any try { - payload = JSON.parse(toUtf8String(bytes)) + payload = JSON.parse(toUtf8String(decompressed)) } catch { throw new Error('accountsSync: the scanned data is not a valid sync payload') } From 45a6f8fb5fa92cce3238abab757f145a5c737c1b Mon Sep 17 00:00:00 2001 From: sonytooo Date: Mon, 17 Aug 2026 14:48:50 +0300 Subject: [PATCH 08/15] fixes --- src/controllers/accounts/accounts.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/controllers/accounts/accounts.ts b/src/controllers/accounts/accounts.ts index a5fbdf826d..c0236cc53d 100644 --- a/src/controllers/accounts/accounts.ts +++ b/src/controllers/accounts/accounts.ts @@ -406,11 +406,6 @@ export class AccountsController extends EventEmitter implements IAccountsControl return this.accountStates[addr]?.[chainId.toString()] } - /** - * The account records another Ambire product needs in order to take over the given - * accounts (accounts sync). Preferences travel along, so that the accounts look the - * same on both devices. - */ getAccountsForSync(addrs: Account['addr'][]): Account[] { return this.accounts.filter((account) => addrs.includes(account.addr)) } From 24d4fc5245aa7c600e172807dee775bc87ba48be Mon Sep 17 00:00:00 2001 From: sonytooo Date: Mon, 17 Aug 2026 15:23:00 +0300 Subject: [PATCH 09/15] cleanup --- src/controllers/keystore/keystore.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/controllers/keystore/keystore.ts b/src/controllers/keystore/keystore.ts index 5c8a4ad5af..62ae5a0713 100644 --- a/src/controllers/keystore/keystore.ts +++ b/src/controllers/keystore/keystore.ts @@ -202,8 +202,7 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl lock() { this.#mainKey = null if (this.#tempSeed) this.deleteTempSeed(false) - // Anything still queued holds decrypted keys and seed phrases (a sync that happened - // before the device password was set), so it must not outlive a lock + this.#seedsToAddOnKeystoreReady = [] this.#internalKeysToAddOnKeystoreReady = [] this.emitUpdate() From 80a9c30d23b73ab4b794bb7e257094fac8e7e2f5 Mon Sep 17 00:00:00 2001 From: sonytooo Date: Wed, 19 Aug 2026 12:19:40 +0300 Subject: [PATCH 10/15] feedback fixes --- src/controllers/accounts/accounts.ts | 4 ---- src/controllers/keystore/keystore.ts | 3 +-- src/controllers/main/main.ts | 2 +- 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/controllers/accounts/accounts.ts b/src/controllers/accounts/accounts.ts index c0236cc53d..193c37c8c9 100644 --- a/src/controllers/accounts/accounts.ts +++ b/src/controllers/accounts/accounts.ts @@ -406,10 +406,6 @@ export class AccountsController extends EventEmitter implements IAccountsControl return this.accountStates[addr]?.[chainId.toString()] } - getAccountsForSync(addrs: Account['addr'][]): Account[] { - return this.accounts.filter((account) => addrs.includes(account.addr)) - } - resetAccountsNewlyAddedState() { this.accounts = this.accounts.map((a) => ({ ...a, newlyAdded: false })) this.emitUpdate() diff --git a/src/controllers/keystore/keystore.ts b/src/controllers/keystore/keystore.ts index 62ae5a0713..94dc37ab5b 100644 --- a/src/controllers/keystore/keystore.ts +++ b/src/controllers/keystore/keystore.ts @@ -757,9 +757,8 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl const label = `Recovery Phrase ${this.#keystoreSeeds.length + 1}` - const isIdTaken = !!id && this.#keystoreSeeds.some((s) => s.id === id) const newEntry: StoredKeystoreSeed = { - id: isIdTaken || !id ? generateUuid() : id, + id: id || generateUuid(), label, seed: await encryptWithKey(this.#mainKey, entropy), seedPassphrase: seedPassphrase diff --git a/src/controllers/main/main.ts b/src/controllers/main/main.ts index 1490030f9b..d794ba6db7 100644 --- a/src/controllers/main/main.ts +++ b/src/controllers/main/main.ts @@ -1705,7 +1705,7 @@ export class MainController extends EventEmitter implements IMainController { requestId?: string ) { await this.#withSyncResponse('exportAccountsForSync', requestId, async () => { - const accounts = this.accounts.getAccountsForSync(addrs) + const accounts = this.accounts.accounts.filter((account) => addrs.includes(account.addr)) if (!accounts.length) throw new EmittableError({ From 100be9245914547f65b98206ba999f718779e368 Mon Sep 17 00:00:00 2001 From: sonytooo Date: Wed, 19 Aug 2026 12:29:01 +0300 Subject: [PATCH 11/15] feedback fixes --- src/controllers/keystore/keystore.ts | 82 ++++++++++++++++------------ 1 file changed, 48 insertions(+), 34 deletions(-) diff --git a/src/controllers/keystore/keystore.ts b/src/controllers/keystore/keystore.ts index 94dc37ab5b..cf0c0dbd78 100644 --- a/src/controllers/keystore/keystore.ts +++ b/src/controllers/keystore/keystore.ts @@ -715,25 +715,25 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl async persistTempSeed() { if (!this.#tempSeed) return - await this.#addSeed(this.#tempSeed) + await this.#addSeeds([this.#tempSeed]) this.#tempSeed = null this.emitUpdate() } /** - * Adds a seed to the keystore and returns the id it is stored with. An `id` can be - * passed to preserve the id a seed already had on another device (accounts sync), - * so that the synced keys' `meta.fromSeedId` keeps pointing to it. + * Adds seeds to the keystore in a single storage write and returns the ids they are + * stored with, in the order they were passed. An `id` can be passed per seed to + * preserve the id it already had on another device (accounts sync), so that the + * synced keys' `meta.fromSeedId` keeps pointing to it. */ - async #addSeed({ - seed, - seedPassphrase, - hdPathTemplate, - notBackedUp, - id - }: KeystoreTempSeed & { id?: StoredKeystoreSeed['id'] }): Promise { + async #addSeeds( + seedsToAdd: (KeystoreTempSeed & { id?: StoredKeystoreSeed['id'] })[], + shouldEmitUpdate: boolean = true + ): Promise { await this.initialLoadPromise + if (!seedsToAdd.length) return [] + if (this.#mainKey === null) throw new EmittableError({ message: KEYSTORE_UNEXPECTED_ERROR_MESSAGE, @@ -741,7 +741,7 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl error: new Error('keystore: needs to be unlocked') }) - if (!Mnemonic.isValidMnemonic(seed)) { + if (seedsToAdd.some(({ seed }) => !Mnemonic.isValidMnemonic(seed))) { throw new EmittableError({ message: 'The provided seed phrase is invalid. Try again with a valid seed or contact support if you think this is a mistake.', @@ -750,35 +750,49 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl }) } - const existingEntry = await this.#findStoredSeed(seed, seedPassphrase) - if (existingEntry) return existingEntry.id + const seedsBeforeAdd = [...this.#keystoreSeeds] + const ids: StoredKeystoreSeed['id'][] = [] - const entropy = extractEntropyFromSeed(seed) + try { + // Entries are pushed as they are built, so that duplicates and labels are + // resolved against the seeds added earlier in the same batch as well + for (const { seed, seedPassphrase, hdPathTemplate, notBackedUp, id } of seedsToAdd) { + const existingEntry = await this.#findStoredSeed(seed, seedPassphrase) + if (existingEntry) { + ids.push(existingEntry.id) + continue + } - const label = `Recovery Phrase ${this.#keystoreSeeds.length + 1}` + const newEntry: StoredKeystoreSeed = { + id: id || generateUuid(), + label: `Recovery Phrase ${this.#keystoreSeeds.length + 1}`, + seed: await encryptWithKey(this.#mainKey, extractEntropyFromSeed(seed)), + seedPassphrase: seedPassphrase + ? await encryptWithKey(this.#mainKey, new TextEncoder().encode(seedPassphrase)) + : null, + hdPathTemplate, + notBackedUp + } - const newEntry: StoredKeystoreSeed = { - id: id || generateUuid(), - label, - seed: await encryptWithKey(this.#mainKey, entropy), - seedPassphrase: seedPassphrase - ? await encryptWithKey(this.#mainKey, new TextEncoder().encode(seedPassphrase)) - : null, - hdPathTemplate, - notBackedUp + this.#keystoreSeeds.push(newEntry) + ids.push(newEntry.id) + } + } catch (error) { + // Nothing has been persisted yet, so the in-memory seeds are rolled back to + // keep them in sync with storage + this.#keystoreSeeds = seedsBeforeAdd + throw error } - this.#keystoreSeeds.push(newEntry) - await this.#storage.set('keystoreSeeds', this.#keystoreSeeds) - this.emitUpdate() + if (shouldEmitUpdate) this.emitUpdate() - return newEntry.id + return ids } async addSeed(keystoreSeed: KeystoreTempSeed) { - await this.withStatus('addSeed', () => this.#addSeed(keystoreSeed), true) + await this.withStatus('addSeed', () => this.#addSeeds([keystoreSeed]), true) } /** @@ -799,9 +813,7 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl try { // Seeds first, so the keys can keep pointing to the seed they were derived from - for (const seed of seedsToAdd) { - await this.#addSeed(seed) - } + await this.#addSeeds(seedsToAdd, false) await this.#addKeys(internalKeysToAdd) await this.#addKeysExternallyStored(externalKeysToAdd) } catch (error: any) { @@ -1349,7 +1361,9 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl return seed.id } - return this.#addSeed(seed) + const [storedSeedId] = await this.#addSeeds([seed]) + + return storedSeedId } async getSigner(keyAddress: Key['addr'], keyType: Key['type']): Promise { From 0af48a9d58dcd0ec6596827ff65f74ba312fdfcb Mon Sep 17 00:00:00 2001 From: sonytooo Date: Wed, 19 Aug 2026 12:39:33 +0300 Subject: [PATCH 12/15] feedback fixes --- src/controllers/keystore/keystore.ts | 41 ++++------------------------ src/libs/keystore/keystore.ts | 29 ++++++++++++++++++++ 2 files changed, 35 insertions(+), 35 deletions(-) diff --git a/src/controllers/keystore/keystore.ts b/src/controllers/keystore/keystore.ts index cf0c0dbd78..8b4668ad55 100644 --- a/src/controllers/keystore/keystore.ts +++ b/src/controllers/keystore/keystore.ts @@ -11,6 +11,7 @@ import { CIPHER, CIPHER_OLD, decryptMainKeyWithSecret, + decryptStoredSeed, decryptWithKey, deriveSecret, encryptMainKeyWithSecret, @@ -18,7 +19,6 @@ import { extractEntropyFromSeed, getBytesForSecret, migrateStoredPayloadsToGCM, - reconstructSeedFromEntropy, SCRYPT_PARAMS } from '@/libs/keystore/keystore' @@ -1300,14 +1300,11 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl // Seeds first, so the keys below can be linked to the seed they were derived from const syncedSeedIds: { [exportedSeedId: string]: StoredKeystoreSeed['id'] } = {} for (const seed of seeds) { - const entropy = await decryptWithKey(exportedMainKey, seed.seed) - const seedPassphrase = seed.seedPassphrase - ? new TextDecoder().decode(await decryptWithKey(exportedMainKey, seed.seedPassphrase)) - : null + const { seed: decryptedSeed, seedPassphrase } = await decryptStoredSeed(exportedMainKey, seed) syncedSeedIds[seed.id] = await this.#storeSyncedSeed({ id: seed.id, - seed: reconstructSeedFromEntropy(entropy, seedPassphrase), + seed: decryptedSeed, seedPassphrase, hdPathTemplate: seed.hdPathTemplate, notBackedUp: seed.notBackedUp @@ -1363,7 +1360,7 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl const [storedSeedId] = await this.#addSeeds([seed]) - return storedSeedId + return storedSeedId! } async getSigner(keyAddress: Key['addr'], keyType: Key['type']): Promise { @@ -1412,35 +1409,9 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl if (!keystoreSeed) throw new Error(`keystore seed with id:${id} not found`) - const seedBytes = await decryptWithKey(this.#mainKey, keystoreSeed.seed) - let seedPassphrase: string | null = null + const { seed, seedPassphrase } = await decryptStoredSeed(this.#mainKey, keystoreSeed) - if (keystoreSeed.seedPassphrase) { - const decryptedSeedPassphraseBytes = await decryptWithKey( - this.#mainKey, - keystoreSeed.seedPassphrase - ) - - seedPassphrase = new TextDecoder().decode(decryptedSeedPassphraseBytes) - if (seedPassphrase === '') seedPassphrase = null - } - - // Decrypt as encoded text first, even if it's entropy - let decryptedSeed = new TextDecoder().decode(seedBytes) - - // Seeds after the GCM migration are stored as entropy bytes, so we have to - // reconstruct the seed from that - if (typeof keystoreSeed.seed !== 'string') { - decryptedSeed = reconstructSeedFromEntropy(seedBytes, seedPassphrase) - } else if (!Mnemonic.isValidMnemonic(decryptedSeed)) { - throw new Error('keystore: invalid seed stored') - } - - return { - ...keystoreSeed, - seed: decryptedSeed, - seedPassphrase: seedPassphrase - } + return { ...keystoreSeed, seed, seedPassphrase } } async #changeKeystorePassword(newSecret: string, oldSecret?: string, extraEntropy?: string) { diff --git a/src/libs/keystore/keystore.ts b/src/libs/keystore/keystore.ts index 02c16e8acb..108ea2afe1 100644 --- a/src/libs/keystore/keystore.ts +++ b/src/libs/keystore/keystore.ts @@ -187,6 +187,35 @@ export const decryptWithKey = async ( return new Uint8Array(decrypted) } +/** + * Decrypts a stored seed entry with the main key it was encrypted with. Handles both + * the entropy bytes seeds are stored as after the GCM migration and the plain mnemonic + * legacy ones stored before it. + */ +export const decryptStoredSeed = async ( + mainKey: MainKey, + storedSeed: Pick +): Promise<{ seed: string; seedPassphrase: string | null }> => { + const seedBytes = await decryptWithKey(mainKey, storedSeed.seed) + + let seedPassphrase: string | null = null + if (storedSeed.seedPassphrase) { + const seedPassphraseBytes = await decryptWithKey(mainKey, storedSeed.seedPassphrase) + + seedPassphrase = new TextDecoder().decode(seedPassphraseBytes) || null + } + + // Seeds after the GCM migration are stored as entropy bytes, so the mnemonic has to + // be reconstructed from them. Legacy ones decrypt to the mnemonic itself. + if (typeof storedSeed.seed !== 'string') + return { seed: reconstructSeedFromEntropy(seedBytes, seedPassphrase), seedPassphrase } + + const seed = new TextDecoder().decode(seedBytes) + if (!Mnemonic.isValidMnemonic(seed)) throw new Error('keystore: invalid seed stored') + + return { seed, seedPassphrase } +} + /** * Used during migration to read legacy payloads */ From a8b7cb388631a9aef20a6a261983cc956f03d140 Mon Sep 17 00:00:00 2001 From: sonytooo Date: Wed, 19 Aug 2026 14:23:46 +0300 Subject: [PATCH 13/15] feedback fixes --- src/controllers/keystore/keystore.ts | 98 +++++++++++++++++----------- src/libs/keystore/keystore.ts | 26 ++++++++ 2 files changed, 86 insertions(+), 38 deletions(-) diff --git a/src/controllers/keystore/keystore.ts b/src/controllers/keystore/keystore.ts index 8b4668ad55..1e8a39a186 100644 --- a/src/controllers/keystore/keystore.ts +++ b/src/controllers/keystore/keystore.ts @@ -12,6 +12,7 @@ import { CIPHER_OLD, decryptMainKeyWithSecret, decryptStoredSeed, + decryptSyncedMainKeyWithSecret, decryptWithKey, deriveSecret, encryptMainKeyWithSecret, @@ -397,6 +398,17 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl this.errorMessage = '' } + /** + * Unwraps the main key of another device from a scanned accounts sync payload, with the + * same wrong password handling as unlocking this device. + */ + async #unwrapSyncedMainKey( + secretKey: Uint8Array, + aesEncrypted: AESGCMEncrypted + ): Promise { + return this.#decryptMainKeyWithSecret(secretKey, aesEncrypted, decryptSyncedMainKeyWithSecret) + } + /** * Decrypts a main key wrapped with the provided secret, translating a wrong * secret into the user facing "Incorrect password" error. Used both when @@ -405,10 +417,11 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl */ async #decryptMainKeyWithSecret( secretKey: Uint8Array, - aesEncrypted: AESGCMEncrypted + aesEncrypted: AESGCMEncrypted, + decrypt: typeof decryptMainKeyWithSecret = decryptMainKeyWithSecret ): Promise { try { - return await decryptMainKeyWithSecret(secretKey, aesEncrypted) + return await decrypt(secretKey, aesEncrypted) } catch (error: any) { // Either wrong password or corrupted/tampered ciphertext if (error?.name === 'OperationError') { @@ -1294,52 +1307,61 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl throw new Error('keystore: synced main key is not encrypted with GCM') const secretKey = await deriveSecret(this.#scryptAdapter, password, secret.scryptParams.salt) - // The exporting device's main key. Kept in this scope only, never persisted. - const exportedMainKey = await this.#decryptMainKeyWithSecret(secretKey, secret.aesEncrypted) - - // Seeds first, so the keys below can be linked to the seed they were derived from - const syncedSeedIds: { [exportedSeedId: string]: StoredKeystoreSeed['id'] } = {} - for (const seed of seeds) { - const { seed: decryptedSeed, seedPassphrase } = await decryptStoredSeed(exportedMainKey, seed) - - syncedSeedIds[seed.id] = await this.#storeSyncedSeed({ - id: seed.id, - seed: decryptedSeed, - seedPassphrase, - hdPathTemplate: seed.hdPathTemplate, - notBackedUp: seed.notBackedUp - }) - } + // The exporting device's main key. Kept in this scope only, never persisted, and only + // able to decrypt, so its bytes never reach this app. + const exportedMainKey = await this.#unwrapSyncedMainKey(secretKey, secret.aesEncrypted) - const internalKeys: ReadyToAddKeys['internal'] = [] - const externalKeys: ReadyToAddKeys['external'] = [] + try { + // Seeds first, so the keys below can be linked to the seed they were derived from + const syncedSeedIds: { [exportedSeedId: string]: StoredKeystoreSeed['id'] } = {} + for (const seed of seeds) { + const { seed: decryptedSeed, seedPassphrase } = await decryptStoredSeed( + exportedMainKey, + seed + ) - for (const key of keys) { - if (key.type !== 'internal') { - externalKeys.push(key) - continue + syncedSeedIds[seed.id] = await this.#storeSyncedSeed({ + id: seed.id, + seed: decryptedSeed, + seedPassphrase, + hdPathTemplate: seed.hdPathTemplate, + notBackedUp: seed.notBackedUp + }) } - const { fromSeedId, ...restMeta } = key.meta - // Drop the reference if the seed didn't come along, so it doesn't dangle - const syncedFromSeedId = fromSeedId ? syncedSeedIds[fromSeedId] : undefined - - internalKeys.push({ - addr: key.addr, - type: 'internal', - label: key.label, - dedicatedToOneSA: key.dedicatedToOneSA, - privateKey: hexlify(await decryptWithKey(exportedMainKey, key.privKey)), - meta: syncedFromSeedId ? { ...restMeta, fromSeedId: syncedFromSeedId } : restMeta - }) - } + const internalKeys: ReadyToAddKeys['internal'] = [] + const externalKeys: ReadyToAddKeys['external'] = [] + + for (const key of keys) { + if (key.type !== 'internal') { + externalKeys.push(key) + continue + } + + const { fromSeedId, ...restMeta } = key.meta + // Drop the reference if the seed didn't come along, so it doesn't dangle + const syncedFromSeedId = fromSeedId ? syncedSeedIds[fromSeedId] : undefined + + internalKeys.push({ + addr: key.addr, + type: 'internal', + label: key.label, + dedicatedToOneSA: key.dedicatedToOneSA, + privateKey: hexlify(await decryptWithKey(exportedMainKey, key.privKey)), + meta: syncedFromSeedId ? { ...restMeta, fromSeedId: syncedFromSeedId } : restMeta + }) + } - try { // Both queue themselves until the keystore is ready to store keys, which is what // makes syncing before the device password is set (onboarding) work await this.#addKeys(internalKeys) await this.#addKeysExternallyStored(externalKeys) } finally { + // Everything is re-encrypted with this device's main key by now, so the key derived + // from the other device's password is of no further use and gets wiped. Its main key + // itself is a non-extractable handle that goes out of scope with this method, so + // there are no bytes of it left to wipe + secretKey.fill(0) this.emitUpdate() } } diff --git a/src/libs/keystore/keystore.ts b/src/libs/keystore/keystore.ts index 108ea2afe1..e6b0b8aed6 100644 --- a/src/libs/keystore/keystore.ts +++ b/src/libs/keystore/keystore.ts @@ -216,6 +216,32 @@ export const decryptStoredSeed = async ( return { seed, seedPassphrase } } +/** + * Unwraps the main key of another device from a scanned accounts sync payload. Imported + * as non-extractable and for decryption only, so that its raw bytes never reach the + * importing app and it cannot be used to encrypt anything - decrypting the synced keys + * and seeds is all it is ever needed for. + */ +export const decryptSyncedMainKeyWithSecret = async ( + secretKey: Uint8Array, + aesEncrypted: AESGCMEncrypted +): Promise => { + const importedSecretKey = await crypto.subtle.importKey( + 'raw', + // use 256 bits (first 32 bytes) + secretKey.slice(0, 32), + { name: CIPHER }, + false, + ['decrypt'] + ) + + const decrypted = await decryptWithKey(importedSecretKey, aesEncrypted) + + return crypto.subtle.importKey('raw', decrypted.slice(0, 32), { name: CIPHER }, false, [ + 'decrypt' + ]) +} + /** * Used during migration to read legacy payloads */ From 912ed26c76490f623c237ff809af20f18fa4cc2f Mon Sep 17 00:00:00 2001 From: sonytooo Date: Wed, 19 Aug 2026 14:33:34 +0300 Subject: [PATCH 14/15] feedback fixes --- src/controllers/keystore/keystore.ts | 9 ++++---- src/libs/keystore/keystore.test.ts | 34 +++++++++++++++++++++++----- src/libs/keystore/keystore.ts | 14 ++++++++---- 3 files changed, 42 insertions(+), 15 deletions(-) diff --git a/src/controllers/keystore/keystore.ts b/src/controllers/keystore/keystore.ts index 1e8a39a186..5ae3b83e70 100644 --- a/src/controllers/keystore/keystore.ts +++ b/src/controllers/keystore/keystore.ts @@ -279,7 +279,7 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl }) } - const secretKey = await deriveSecret(this.#scryptAdapter, secret, scryptParams.salt) + const secretKey = await deriveSecret(this.#scryptAdapter, secret, scryptParams) const isOldSecretCipher = aesEncrypted.cipherType === undefined || aesEncrypted.cipherType === CIPHER_OLD @@ -593,12 +593,13 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl const entropyGenerator = new EntropyGenerator() const salt = entropyGenerator.generateRandomBytes(32, extraEntropy) - const secretKey = await deriveSecret(this.#scryptAdapter, secret, hexlify(salt)) + const scryptParams = { salt: hexlify(salt), ...SCRYPT_PARAMS } + const secretKey = await deriveSecret(this.#scryptAdapter, secret, scryptParams) const mainKeyEncryptedWithSecret = await encryptMainKeyWithSecret(mainKey, secretKey) this.#keystoreSecrets.push({ id: secretId, - scryptParams: { salt: hexlify(salt), ...SCRYPT_PARAMS }, + scryptParams, aesEncrypted: mainKeyEncryptedWithSecret }) @@ -1306,7 +1307,7 @@ export class KeystoreController extends EventEmitter implements IKeystoreControl if (secret.aesEncrypted.cipherType !== CIPHER) throw new Error('keystore: synced main key is not encrypted with GCM') - const secretKey = await deriveSecret(this.#scryptAdapter, password, secret.scryptParams.salt) + const secretKey = await deriveSecret(this.#scryptAdapter, password, secret.scryptParams) // The exporting device's main key. Kept in this scope only, never persisted, and only // able to decrypt, so its bytes never reach this app. const exportedMainKey = await this.#unwrapSyncedMainKey(secretKey, secret.aesEncrypted) diff --git a/src/libs/keystore/keystore.test.ts b/src/libs/keystore/keystore.test.ts index cebf5fdca1..c396e04da5 100644 --- a/src/libs/keystore/keystore.test.ts +++ b/src/libs/keystore/keystore.test.ts @@ -249,11 +249,10 @@ describe('Keystore lib', () => { describe('encryptMainKeyWithSecret', () => { test('uses the first 32 bytes of the derived secret key', async () => { const mainKey = await createMainKey() - const secretKey = await deriveSecret( - new ScryptAdapter('browser-webkit'), - 'password', - TEST_SALT_HEX - ) + const secretKey = await deriveSecret(new ScryptAdapter('browser-webkit'), 'password', { + salt: TEST_SALT_HEX, + ...SCRYPT_PARAMS + }) const expectedImportedKey = await crypto.subtle.importKey( 'raw', secretKey.slice(0, 32), @@ -280,7 +279,10 @@ describe('Keystore lib', () => { scryptMock.mockResolvedValue(getBytes(MOCK)) const scryptAdapter = { scrypt: scryptMock } as unknown as ScryptAdapter - const result = await deriveSecret(scryptAdapter, 'cafe\u0301', TEST_SALT_HEX) + const result = await deriveSecret(scryptAdapter, 'cafe\u0301', { + salt: TEST_SALT_HEX, + ...SCRYPT_PARAMS + }) expect(hexlify(result)).toBe(MOCK) expect(scryptMock).toHaveBeenCalledWith( @@ -294,6 +296,26 @@ describe('Keystore lib', () => { } ) }) + + test('derives with the params it is given, not the current defaults', async () => { + const scryptMock = jest.fn< + ReturnType, + Parameters + >() + scryptMock.mockResolvedValue(getBytes(hexlify(crypto.getRandomValues(new Uint8Array(32))))) + + // A secret created by an older version, or by another Ambire product configured + // differently, only derives the same key when its own params are used + const storedParams = { salt: TEST_SALT_HEX, N: 16384, r: 4, p: 2, dkLen: 32 } + await deriveSecret({ scrypt: scryptMock } as unknown as ScryptAdapter, 'pass', storedParams) + + expect(scryptMock).toHaveBeenCalledWith(getBytesForSecret('pass'), getBytes(TEST_SALT_HEX), { + N: 16384, + r: 4, + p: 2, + dkLen: 32 + }) + }) }) describe('decryptWithKeyOld', () => { diff --git a/src/libs/keystore/keystore.ts b/src/libs/keystore/keystore.ts index e6b0b8aed6..e9e7929c05 100644 --- a/src/libs/keystore/keystore.ts +++ b/src/libs/keystore/keystore.ts @@ -6,6 +6,7 @@ import { KeystoreEncryptedPayload, MainKey, MainKeyOld, + ScryptParams, StoredKey, StoredKeystoreSeed } from '@/interfaces/keystore' @@ -291,15 +292,18 @@ export const decryptSeedWithKeyOld = async ( export const deriveSecret = async ( scryptAdapter: ScryptAdapter, secretValue: string, - salt: string + scryptParams: ScryptParams ): Promise> => { + const { salt, N, r, p, dkLen } = scryptParams + // Use wait(0) to yield to the event loop and avoid blocking the UI await wait(0) + const secretKey = await scryptAdapter.scrypt(getBytesForSecret(secretValue), getBytes(salt), { - N: SCRYPT_PARAMS.N, - r: SCRYPT_PARAMS.r, - p: SCRYPT_PARAMS.p, - dkLen: SCRYPT_PARAMS.dkLen + N, + r, + p, + dkLen }) await wait(0) From d2e3817cd34818419d3ec932305249491d651425 Mon Sep 17 00:00:00 2001 From: sonytooo Date: Thu, 20 Aug 2026 09:38:01 +0300 Subject: [PATCH 15/15] feedback fixes --- package-lock.json | 18 ++--- package.json | 2 +- src/libs/accountsSync/accountsSync.test.ts | 80 ++++++++++++++++++++-- src/libs/accountsSync/accountsSync.ts | 58 ++++++++++++++-- 4 files changed, 129 insertions(+), 29 deletions(-) diff --git a/package-lock.json b/package-lock.json index b2d4e77af9..e0bbc09499 100644 --- a/package-lock.json +++ b/package-lock.json @@ -23,7 +23,7 @@ "events": "^3.3.0", "hash-wasm": "^4.12.0", "js-yaml": "^4.1.0", - "pako": "^2.1.0", + "pako": "2.1.0", "scrypt-js": "^3.0.1", "tldts": "7.0.17", "uuid": "9.0.0", @@ -15458,19 +15458,9 @@ "license": "BlueOak-1.0.0" }, "node_modules/pako": { - "version": "2.2.0", - "resolved": "https://registry.npmjs.org/pako/-/pako-2.2.0.tgz", - "integrity": "sha512-zJq6RP/5q+TO2OpFV3FHzlPnFjmkb7Nc99a5SNjJE+uu/PkpChs+NIZSSzbBoD+6kjiISXjfYdwj1ZRQ81dz/w==", - "funding": [ - { - "type": "github", - "url": "https://github.com/sponsors/puzrin" - }, - { - "type": "github", - "url": "https://github.com/sponsors/nodeca" - } - ], + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/pako/-/pako-2.1.0.tgz", + "integrity": "sha512-w+eufiZ1WuJYgPXbV/PO3NCMEc3xqylkKHzp8bxp1uW4qaSNQUkwmLLEc3kKsfz8lpV1F8Ht3U1Cm+9Srog2ug==", "license": "(MIT AND Zlib)" }, "node_modules/parent-module": { diff --git a/package.json b/package.json index 9455b781b8..03851d0a40 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,7 @@ "events": "^3.3.0", "hash-wasm": "^4.12.0", "js-yaml": "^4.1.0", - "pako": "^2.1.0", + "pako": "2.1.0", "scrypt-js": "^3.0.1", "tldts": "7.0.17", "uuid": "9.0.0", diff --git a/src/libs/accountsSync/accountsSync.test.ts b/src/libs/accountsSync/accountsSync.test.ts index 72ff6bd92c..0dc7e0c487 100644 --- a/src/libs/accountsSync/accountsSync.test.ts +++ b/src/libs/accountsSync/accountsSync.test.ts @@ -1,6 +1,7 @@ -import { getBytes, toUtf8Bytes } from 'ethers' +import { getBytes, getCreate2Address, keccak256, toUtf8Bytes } from 'ethers' import { gzip } from 'pako' +import { AMBIRE_ACCOUNT_FACTORY } from '@/consts/deploy' import { CIPHER } from '@/libs/keystore/keystore' import { @@ -10,7 +11,18 @@ import { serializeAccountsSyncPayload } from './accountsSync' -const ACCOUNT_ADDR = '0x8DC9b3e1F5b0Dc9F6b2e0d3D0Ba0A5a32B0E7C4B' +const ACCOUNT_CREATION = { + factoryAddr: AMBIRE_ACCOUNT_FACTORY, + bytecode: `0x${'60'.repeat(120)}`, + salt: `0x${'00'.repeat(32)}` +} +// The parser verifies the address against the creation data, so it can't be an arbitrary one +const ACCOUNT_ADDR = getCreate2Address( + ACCOUNT_CREATION.factoryAddr, + ACCOUNT_CREATION.salt, + keccak256(ACCOUNT_CREATION.bytecode) +) +const EOA_ADDR = '0x8DC9b3e1F5b0Dc9F6b2e0d3D0Ba0A5a32B0E7C4B' const KEY_ADDR = '0x085f8A348f6fBc6F8d8FC3f1e427473436506D65' const EXTERNAL_KEY_ADDR = '0x1A2C3802A9eC12725678dAF23DbFD13134e5893A' @@ -34,11 +46,7 @@ const buildPayload = (): AccountsSyncPayload => ({ initialPrivileges: [ [KEY_ADDR, '0x0000000000000000000000000000000000000000000000000000000000000002'] ], - creation: { - factoryAddr: '0xa8202f888b9b2dFA5Ceb2204865018133F6F179A', - bytecode: `0x${'60'.repeat(120)}`, - salt: `0x${'00'.repeat(32)}` - }, + creation: ACCOUNT_CREATION, preferences: { label: 'Account 1', pfp: ACCOUNT_ADDR } } ], @@ -144,6 +152,13 @@ describe('accountsSync payload', () => { expect(() => serializeAndParse(payload)).toThrow('invalid scrypt params') }) + it('rejects scrypt params that would make deriving the key allocate gigabytes', () => { + const payload: any = buildPayload() + payload.secret.scryptParams = { ...payload.secret.scryptParams, N: 2 ** 21, p: 64 } + + expect(() => serializeAndParse(payload)).toThrow('invalid scrypt params') + }) + it('rejects a main key that is not AES-GCM encrypted', () => { const payload: any = buildPayload() payload.secret.aesEncrypted = { @@ -170,6 +185,57 @@ describe('accountsSync payload', () => { expect(() => serializeAndParse(payload)).toThrow('invalid account addr') }) + it('rejects an account that does not match its creation data', () => { + const payload: any = buildPayload() + payload.accounts[0].creation = { ...ACCOUNT_CREATION, salt: `0x${'11'.repeat(32)}` } + + expect(() => serializeAndParse(payload)).toThrow( + `account ${ACCOUNT_ADDR} does not match its creation data` + ) + }) + + it('rejects an account with a non-address in associatedKeys', () => { + const payload: any = buildPayload() + payload.accounts[0].associatedKeys = [KEY_ADDR, '0xnot-an-address'] + + expect(() => serializeAndParse(payload)).toThrow('invalid account associatedKeys') + }) + + it('rejects an account with malformed initialPrivileges', () => { + const payload: any = buildPayload() + payload.accounts[0].initialPrivileges = [[KEY_ADDR, 'not-a-hex-privilege']] + + expect(() => serializeAndParse(payload)).toThrow('invalid account initialPrivileges') + }) + + it('accepts an EOA controlled by its own address', () => { + const payload: any = buildPayload() + payload.accounts[0] = { + addr: EOA_ADDR, + associatedKeys: [EOA_ADDR], + initialPrivileges: [], + creation: null, + preferences: { label: 'Account 1', pfp: EOA_ADDR } + } + + expect(serializeAndParse(payload)).toEqual(payload) + }) + + it('rejects an EOA that lists keys other than its own address', () => { + const payload: any = buildPayload() + payload.accounts[0] = { + addr: EOA_ADDR, + associatedKeys: [KEY_ADDR], + initialPrivileges: [], + creation: null, + preferences: { label: 'Account 1', pfp: EOA_ADDR } + } + + expect(() => serializeAndParse(payload)).toThrow( + `account ${EOA_ADDR} has unexpected associatedKeys` + ) + }) + it('rejects an internal key that is not AES-GCM encrypted', () => { const payload: any = buildPayload() payload.keys[0].privKey = '0xdeadbeef' diff --git a/src/libs/accountsSync/accountsSync.ts b/src/libs/accountsSync/accountsSync.ts index cb07e29931..965d73c261 100644 --- a/src/libs/accountsSync/accountsSync.ts +++ b/src/libs/accountsSync/accountsSync.ts @@ -1,4 +1,12 @@ -import { hexlify, isAddress, toUtf8Bytes, toUtf8String } from 'ethers' +import { + getCreate2Address, + hexlify, + isAddress, + isHexString, + keccak256, + toUtf8Bytes, + toUtf8String +} from 'ethers' import { gzip, Inflate } from 'pako' import { Account } from '../../interfaces/account' @@ -7,7 +15,7 @@ import { StoredKey, StoredKeystoreSeed } from '../../interfaces/keystore' -import { CIPHER, tryParseGcmPayload } from '../keystore/keystore' +import { CIPHER, SCRYPT_PARAMS, tryParseGcmPayload } from '../keystore/keystore' /** * The UR type used to transport the accounts sync payload over animated QR codes. @@ -41,27 +49,63 @@ const validateSecret = (secret: any) => { if (!secret || secret.id !== 'password') throw new Error('accountsSync: missing the password protected main key') + // Deriving the key allocates 128 * N * r bytes, so the scanned params are pinned to the + // ones every Ambire product uses, instead of letting a hostile QR code ask for gigabytes const { salt, N, r, p, dkLen } = secret.scryptParams || {} const hasValidScryptParams = typeof salt === 'string' && - typeof N === 'number' && - typeof r === 'number' && - typeof p === 'number' && - typeof dkLen === 'number' + N === SCRYPT_PARAMS.N && + r === SCRYPT_PARAMS.r && + p === SCRYPT_PARAMS.p && + dkLen === SCRYPT_PARAMS.dkLen if (!hasValidScryptParams) throw new Error('accountsSync: invalid scrypt params') requireGcmPayload(secret.aesEncrypted, 'the main key') } +/** + * Recomputes the address the factory deploys for the scanned bytecode, the same way the + * AccountPicker does for linked accounts, so a scanned account cannot claim to be an + * address it isn't. Safe accounts are left out, because deriving their address needs the + * factory's proxy creation code, which is only available over an RPC call. + */ +const validateAccountCreation = (account: any) => { + const { factoryAddr, bytecode, salt } = account.creation + if (!isAddress(factoryAddr) || !isHexString(bytecode) || !isHexString(salt, 32)) + throw new Error(`accountsSync: invalid creation data for account ${account.addr}`) + + if ( + getCreate2Address(factoryAddr, salt, keccak256(bytecode)).toLowerCase() !== + account.addr.toLowerCase() + ) + throw new Error(`accountsSync: account ${account.addr} does not match its creation data`) +} + +const isPrivilege = (privilege: any) => + Array.isArray(privilege) && isAddress(privilege[0]) && isHexString(privilege[1]) + const validateAccounts = (accounts: any) => { if (!Array.isArray(accounts) || !accounts.length) throw new Error('accountsSync: no accounts in the payload') accounts.forEach((account) => { if (!account || !isAddress(account.addr)) throw new Error('accountsSync: invalid account addr') - if (!Array.isArray(account.associatedKeys)) + if (!Array.isArray(account.associatedKeys) || !account.associatedKeys.every(isAddress)) throw new Error('accountsSync: invalid account associatedKeys') + if (!Array.isArray(account.initialPrivileges) || !account.initialPrivileges.every(isPrivilege)) + throw new Error('accountsSync: invalid account initialPrivileges') if (!account.preferences?.label) throw new Error('accountsSync: invalid account preferences') + + if (account.creation) return validateAccountCreation(account) + + // An EOA is controlled by its own address only, so anything else means the scanned + // account was tampered with. Safe accounts have no `creation`, but do list their owners + if ( + !account.safeCreation && + (account.associatedKeys.length !== 1 || + account.associatedKeys[0].toLowerCase() !== account.addr.toLowerCase()) + ) + throw new Error(`accountsSync: account ${account.addr} has unexpected associatedKeys`) }) }