From 48d0b68858946379fae4cb17ab25b6fac0b32f31 Mon Sep 17 00:00:00 2001 From: Hugh Cunningham Date: Fri, 6 Dec 2024 15:52:27 -0800 Subject: [PATCH] deletes multisig identity on account import since the identity will always be stored on the multisig identity we don't need to maintain the record of the identity separately in the multisigIdenitities store keeping identities in this store can be confusing for future use when generating new accounts deletes any existing identity that matches the identity on the imported account instead of adding an identity to the store if it is missing removes DuplicateIdentityNameError, which is no longer thrown removes tests that checked duplicate identity name logic on import --- .../rpc/routes/wallet/importAccount.test.ts | 179 ------------------ .../src/rpc/routes/wallet/importAccount.ts | 5 +- ironfish/src/wallet/errors.ts | 9 - ironfish/src/wallet/wallet.ts | 23 +-- 4 files changed, 2 insertions(+), 214 deletions(-) diff --git a/ironfish/src/rpc/routes/wallet/importAccount.test.ts b/ironfish/src/rpc/routes/wallet/importAccount.test.ts index b7be8c5be5..05de168ddb 100644 --- a/ironfish/src/rpc/routes/wallet/importAccount.test.ts +++ b/ironfish/src/rpc/routes/wallet/importAccount.test.ts @@ -4,7 +4,6 @@ import { generateKey, LanguageCode, multisig, spendingKeyToWords } from '@ironfish/rust-nodejs' import fs from 'fs' import path from 'path' -import { Assert } from '../../../assert' import { createTrustedDealerKeyPackages, useMinerBlockFixture } from '../../../testUtilities' import { createRouteTest } from '../../../testUtilities/routeTest' import { JsonEncoder } from '../../../wallet' @@ -465,184 +464,6 @@ describe('Route wallet/importAccount', () => { expect.assertions(2) }) - it('should not import multisig account with secret with the same identity name', async () => { - const name = 'duplicateIdentityNameTest' - - const { - dealer: trustedDealerPackages, - secrets, - identities, - } = createTrustedDealerKeyPackages() - - await routeTest.node.wallet.walletDb.putMultisigIdentity( - Buffer.from(identities[0], 'hex'), - { - secret: secrets[0].serialize(), - name, - }, - ) - - const indentityCountBefore = (await routeTest.client.wallet.multisig.getIdentities()) - .content.identities.length - - const account: AccountImport = { - version: 1, - name, - viewKey: trustedDealerPackages.viewKey, - incomingViewKey: trustedDealerPackages.incomingViewKey, - outgoingViewKey: trustedDealerPackages.outgoingViewKey, - publicAddress: trustedDealerPackages.publicAddress, - spendingKey: null, - createdAt: null, - proofAuthorizingKey: trustedDealerPackages.proofAuthorizingKey, - multisigKeys: { - publicKeyPackage: trustedDealerPackages.publicKeyPackage, - keyPackage: trustedDealerPackages.keyPackages[1].keyPackage.toString(), - secret: secrets[1].serialize().toString('hex'), - }, - } - - try { - await routeTest.client.wallet.importAccount({ - account: new JsonEncoder().encode(account), - name, - rescan: false, - }) - } catch (e: unknown) { - if (!(e instanceof RpcRequestError)) { - throw e - } - - /** - * These assertions ensures that we cannot import multiple identities with the same name. - * This is done by creating an identity, storing it and attempting to import another identity but give it the same name. - */ - expect(e.status).toBe(400) - expect(e.code).toBe(RPC_ERROR_CODES.DUPLICATE_IDENTITY_NAME) - } - - if (account.multisigKeys && isMultisigSignerImport(account.multisigKeys)) { - account.multisigKeys.secret = secrets[0].serialize().toString('hex') - } else { - throw new Error('Invalid multisig keys') - } - - const response = await routeTest.client.wallet.importAccount({ - account: new JsonEncoder().encode(account), - name: 'account2', - rescan: false, - }) - - expect(response.status).toBe(200) - expect(response.content.name).toEqual('account2') - - const identitiesAfter = (await routeTest.client.wallet.multisig.getIdentities()).content - .identities - const newIdentity = identitiesAfter.find((identity) => identity.name === name) - - /** - * These assertions ensure that if we try to import an identity with the same secret but different name, it will pass. - * However, the identity name will remain the same as the original identity that was imported first. - */ - expect(identitiesAfter.length).toBe(indentityCountBefore) - expect(newIdentity).toBeDefined() - expect(newIdentity?.name).toBe(name) - - expect.assertions(7) - }) - - it('should not import hardware multisig account with same identity name', async () => { - const name = 'duplicateIdentityNameTest' - - const { - dealer: trustedDealerPackages, - secrets, - identities, - } = createTrustedDealerKeyPackages() - - const identity = identities[0] - const nextIdentity = identities[1] - - await routeTest.node.wallet.walletDb.putMultisigIdentity(Buffer.from(identity, 'hex'), { - secret: secrets[0].serialize(), - name, - }) - - const account: AccountImport = { - version: 1, - name, - viewKey: trustedDealerPackages.viewKey, - incomingViewKey: trustedDealerPackages.incomingViewKey, - outgoingViewKey: trustedDealerPackages.outgoingViewKey, - publicAddress: trustedDealerPackages.publicAddress, - proofAuthorizingKey: trustedDealerPackages.proofAuthorizingKey, - spendingKey: null, - createdAt: null, - multisigKeys: { - publicKeyPackage: trustedDealerPackages.publicKeyPackage, - identity: nextIdentity, - }, - } - - try { - await routeTest.client.wallet.importAccount({ - account: new JsonEncoder().encode(account), - name, - rescan: false, - }) - } catch (e: unknown) { - if (!(e instanceof RpcRequestError)) { - throw e - } - - expect(e.status).toBe(400) - expect(e.code).toBe(RPC_ERROR_CODES.DUPLICATE_IDENTITY_NAME) - } - - expect.assertions(2) - }) - - it('should not modify existing identity if a new one is being imported with a different name', async () => { - const { dealer: trustedDealerPackages, identities } = createTrustedDealerKeyPackages() - - const identity = identities[0] - - await routeTest.node.wallet.walletDb.putMultisigIdentity(Buffer.from(identity, 'hex'), { - name: 'existingIdentity', - }) - - const account: AccountImport = { - version: 1, - name: 'newIdentity', - viewKey: trustedDealerPackages.viewKey, - incomingViewKey: trustedDealerPackages.incomingViewKey, - outgoingViewKey: trustedDealerPackages.outgoingViewKey, - publicAddress: trustedDealerPackages.publicAddress, - proofAuthorizingKey: trustedDealerPackages.proofAuthorizingKey, - spendingKey: null, - createdAt: null, - multisigKeys: { - publicKeyPackage: trustedDealerPackages.publicKeyPackage, - identity: identity, - }, - } - - const response = await routeTest.client.wallet.importAccount({ - account: new JsonEncoder().encode(account), - name: 'newIdentity', - rescan: false, - }) - - expect(response.status).toBe(200) - expect(response.content.name).toEqual('newIdentity') - - const existingIdentity = await routeTest.wallet.walletDb.getMultisigIdentity( - Buffer.from(identity, 'hex'), - ) - Assert.isNotUndefined(existingIdentity) - expect(existingIdentity.name).toEqual('existingIdentity') - }) - describe('account format', () => { it('should decode an account import with the requested format', async () => { const name = 'mnemonic-format' diff --git a/ironfish/src/rpc/routes/wallet/importAccount.ts b/ironfish/src/rpc/routes/wallet/importAccount.ts index 0b3d310f6d..87629b4f56 100644 --- a/ironfish/src/rpc/routes/wallet/importAccount.ts +++ b/ironfish/src/rpc/routes/wallet/importAccount.ts @@ -3,7 +3,7 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ import * as yup from 'yup' import { DecodeInvalidName } from '../../../wallet' -import { DuplicateAccountNameError, DuplicateIdentityNameError } from '../../../wallet/errors' +import { DuplicateAccountNameError } from '../../../wallet/errors' import { AccountFormat, decodeAccountImport } from '../../../wallet/exporter/account' import { decryptEncodedAccount } from '../../../wallet/exporter/encryption' import { RPC_ERROR_CODES, RpcValidationError } from '../../adapters' @@ -75,9 +75,6 @@ routes.register( isDefaultAccount, }) } catch (e) { - if (e instanceof DuplicateIdentityNameError) { - throw new RpcValidationError(e.message, 400, RPC_ERROR_CODES.DUPLICATE_IDENTITY_NAME) - } if (e instanceof DuplicateAccountNameError) { throw new RpcValidationError(e.message, 400, RPC_ERROR_CODES.DUPLICATE_ACCOUNT_NAME) } else if (e instanceof DecodeInvalidName) { diff --git a/ironfish/src/wallet/errors.ts b/ironfish/src/wallet/errors.ts index c92a267da0..b83564cd4d 100644 --- a/ironfish/src/wallet/errors.ts +++ b/ironfish/src/wallet/errors.ts @@ -47,15 +47,6 @@ export class DuplicateAccountNameError extends Error { } } -export class DuplicateIdentityNameError extends Error { - name = this.constructor.name - - constructor(name: string) { - super() - this.message = `Multisig identity already exists with the name ${name}` - } -} - export class DuplicateIdentityError extends Error { name = this.constructor.name diff --git a/ironfish/src/wallet/wallet.ts b/ironfish/src/wallet/wallet.ts index 858ce44080..914ba1e7a0 100644 --- a/ironfish/src/wallet/wallet.ts +++ b/ironfish/src/wallet/wallet.ts @@ -47,7 +47,6 @@ import { EncryptedAccount } from './account/encryptedAccount' import { AssetBalances } from './assetBalances' import { DuplicateAccountNameError, - DuplicateIdentityNameError, DuplicateMultisigSecretNameError, DuplicateSpendingKeyError, MaxMemoLengthError, @@ -1587,27 +1586,7 @@ export class Wallet { } if (identity) { - const existingIdentity = await this.walletDb.getMultisigIdentity(identity, tx) - - if (!existingIdentity) { - const duplicateSecret = await this.walletDb.getMultisigSecretByName( - accountValue.name, - tx, - ) - - if (duplicateSecret) { - throw new DuplicateIdentityNameError(accountValue.name) - } - - await this.walletDb.putMultisigIdentity( - identity, - { - name: account.name, - secret, - }, - tx, - ) - } + await this.walletDb.deleteMultisigIdentity(identity, tx) } const accountHead =