From 0b3f128dc47a6fd31eb744b835820a191d15ba14 Mon Sep 17 00:00:00 2001 From: Andrea Date: Mon, 22 Apr 2024 14:06:13 -0700 Subject: [PATCH] Multisig: fetch participant identities from the public key package When importing a multisig account, participant identities are stored in two places: 1. the public key package (part of the account record in the walletdb) 2. on dedicated records in the walletdb The #2 records can sometimes get lost. This is the case currently of account rescans. Because there is no future development scheduled that will benefit from #2, it's easier to just ignore those records and instead rely only on the public key package. --- .../multisig/createSignatureShare.test.ts | 17 +++++++---- .../wallet/multisig/createSignatureShare.ts | 8 ++---- .../wallet/multisig/getAccountIdentities.ts | 8 ++---- ironfish/src/wallet/account/account.ts | 7 +++++ ironfish/src/wallet/wallet.test.slow.ts | 6 +--- ironfish/src/wallet/wallet.ts | 10 ------- ironfish/src/wallet/walletdb/walletdb.test.ts | 20 ------------- ironfish/src/wallet/walletdb/walletdb.ts | 28 ------------------- 8 files changed, 25 insertions(+), 79 deletions(-) diff --git a/ironfish/src/rpc/routes/wallet/multisig/createSignatureShare.test.ts b/ironfish/src/rpc/routes/wallet/multisig/createSignatureShare.test.ts index d9917fa587..1091ab900c 100644 --- a/ironfish/src/rpc/routes/wallet/multisig/createSignatureShare.test.ts +++ b/ironfish/src/rpc/routes/wallet/multisig/createSignatureShare.test.ts @@ -5,7 +5,7 @@ import { generateKey } from '@ironfish/rust-nodejs' import { Assert } from '../../../../assert' import { useAccountAndAddFundsFixture, useUnsignedTxFixture } from '../../../../testUtilities' import { createRouteTest } from '../../../../testUtilities/routeTest' -import { ACCOUNT_SCHEMA_VERSION } from '../../../../wallet' +import { ACCOUNT_SCHEMA_VERSION, AssertMultisig } from '../../../../wallet' describe('Route wallt/multisig/createSignatureShare', () => { const routeTest = createRouteTest() @@ -108,15 +108,22 @@ describe('Route wallt/multisig/createSignatureShare', () => { }) ).content.signingPackage - // Remove one participant from the participants store to simulate unknown signer + // Alter the public key package to replace one identity with another, so + // that we can later pretend that we created a signature share from an + // unknown identity const account = routeTest.wallet.getAccountByName(accountNames[0]) Assert.isNotNull(account) + AssertMultisig(account) - await routeTest.wallet.walletDb.deleteParticipantIdentity( - account, - Buffer.from(participants[1].identity, 'hex'), + const fromIdentity = participants[1].identity + const toIdentity = participants[2].identity + account.multisigKeys.publicKeyPackage = account.multisigKeys.publicKeyPackage.replace( + fromIdentity, + toIdentity, ) + await routeTest.wallet.walletDb.setAccount(account) + // Attempt to create signature share await expect( routeTest.client.wallet.multisig.createSignatureShare({ diff --git a/ironfish/src/rpc/routes/wallet/multisig/createSignatureShare.ts b/ironfish/src/rpc/routes/wallet/multisig/createSignatureShare.ts index 5e7db581d8..29141a3166 100644 --- a/ironfish/src/rpc/routes/wallet/multisig/createSignatureShare.ts +++ b/ironfish/src/rpc/routes/wallet/multisig/createSignatureShare.ts @@ -4,7 +4,6 @@ import { multisig } from '@ironfish/rust-nodejs' import { BufferSet } from 'buffer-map' import * as yup from 'yup' -import { AsyncUtils } from '../../../../utils' import { AssertMultisigSigner } from '../../../../wallet' import { RpcValidationError } from '../../../adapters' import { ApiNamespace } from '../../namespaces' @@ -39,7 +38,7 @@ export const CreateSignatureShareResponseSchema: yup.ObjectSchema( `${ApiNamespace.wallet}/multisig/createSignatureShare`, CreateSignatureShareRequestSchema, - async (request, node): Promise => { + (request, node) => { AssertHasRpcContext(request, node, 'wallet') const account = getAccount(node.wallet, request.data.account) @@ -49,10 +48,7 @@ routes.register identity.toString('hex')) + const identities = account + .getMultisigParticipantIdentities() + .map((identity) => identity.toString('hex')) request.end({ identities }) }, diff --git a/ironfish/src/wallet/account/account.ts b/ironfish/src/wallet/account/account.ts index 565e5352a3..90a9a6c509 100644 --- a/ironfish/src/wallet/account/account.ts +++ b/ironfish/src/wallet/account/account.ts @@ -1,6 +1,7 @@ /* This Source Code Form is subject to the terms of the Mozilla Public * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ +import { multisig } from '@ironfish/rust-nodejs' import { Asset } from '@ironfish/rust-nodejs' import { BufferMap, BufferSet } from 'buffer-map' import MurmurHash3 from 'imurmurhash' @@ -1282,6 +1283,12 @@ export class Account { return notes } + + getMultisigParticipantIdentities(): Array { + AssertMultisig(this) + const publicKeyPackage = new multisig.PublicKeyPackage(this.multisigKeys.publicKeyPackage) + return publicKeyPackage.identities() + } } export function calculateAccountPrefix(id: string): Buffer { diff --git a/ironfish/src/wallet/wallet.test.slow.ts b/ironfish/src/wallet/wallet.test.slow.ts index b5f73e2c01..7ab1e396a4 100644 --- a/ironfish/src/wallet/wallet.test.slow.ts +++ b/ironfish/src/wallet/wallet.test.slow.ts @@ -1362,11 +1362,7 @@ describe('Wallet', () => { ...trustedDealerPackage, }) - const storedIdentities: string[] = [] - for await (const identity of node.wallet.walletDb.getParticipantIdentities(account)) { - storedIdentities.push(identity.toString('hex')) - } - + const storedIdentities = account.getMultisigParticipantIdentities() expect(identities.sort()).toEqual(storedIdentities.sort()) }) }) diff --git a/ironfish/src/wallet/wallet.ts b/ironfish/src/wallet/wallet.ts index 1b13d0ab48..161d88d2f3 100644 --- a/ironfish/src/wallet/wallet.ts +++ b/ironfish/src/wallet/wallet.ts @@ -1640,16 +1640,6 @@ export class Wallet { } else { await account.updateHead(null, tx) } - - if (account.multisigKeys) { - const publicKeyPackage = new multisig.PublicKeyPackage( - account.multisigKeys.publicKeyPackage, - ) - - for (const identity of publicKeyPackage.identities()) { - await this.walletDb.addParticipantIdentity(account, identity, tx) - } - } }) this.accounts.set(account.id, account) diff --git a/ironfish/src/wallet/walletdb/walletdb.test.ts b/ironfish/src/wallet/walletdb/walletdb.test.ts index ac0f5bd6b9..751117ec33 100644 --- a/ironfish/src/wallet/walletdb/walletdb.test.ts +++ b/ironfish/src/wallet/walletdb/walletdb.test.ts @@ -410,24 +410,4 @@ describe('WalletDB', () => { expect(storedSecret.secret).toEqualBuffer(serializedSecret) }) }) - - describe('participantIdentities', () => { - it('should store participant identities for a multisig account', async () => { - const node = (await nodeTest.createSetup()).node - const walletDb = node.wallet.walletDb - - const account = await useAccountFixture(node.wallet, 'multisig') - - const identity = multisig.ParticipantSecret.random().toIdentity() - - await walletDb.addParticipantIdentity(account, identity.serialize()) - - const storedIdentities = await AsyncUtils.materialize( - walletDb.getParticipantIdentities(account), - ) - - expect(storedIdentities.length).toEqual(1) - expect(storedIdentities[0]).toEqualBuffer(identity.serialize()) - }) - }) }) diff --git a/ironfish/src/wallet/walletdb/walletdb.ts b/ironfish/src/wallet/walletdb/walletdb.ts index ec90c3d1c6..b494087d38 100644 --- a/ironfish/src/wallet/walletdb/walletdb.ts +++ b/ironfish/src/wallet/walletdb/walletdb.ts @@ -1297,32 +1297,4 @@ export class WalletDB { yield value } } - - async addParticipantIdentity( - account: Account, - identity: Buffer, - tx?: IDatabaseTransaction, - ): Promise { - await this.participantIdentities.put([account.prefix, identity], { identity }, tx) - } - - async deleteParticipantIdentity( - account: Account, - identity: Buffer, - tx?: IDatabaseTransaction, - ): Promise { - await this.participantIdentities.del([account.prefix, identity], tx) - } - - async *getParticipantIdentities( - account: Account, - tx?: IDatabaseTransaction, - ): AsyncGenerator { - for await (const [_, identity] of this.participantIdentities.getAllKeysIter( - tx, - account.prefixRange, - )) { - yield identity - } - } }