deletes multisig identity on account import (#5692)

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

Author: hughy

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'
@@ -470,187 +469,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'),
-      },
-      ledger: false,
-    }
-
-    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,
-      },
-      ledger: false,
-    }
-
-    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,
-      },
-      ledger: false,
-    }
-
-    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'

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<typeof ImportAccountRequestSchema, ImportResponse>(
         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) {

ironfish/src/wallet/errors.ts

@@ -51,15 +51,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
 

ironfish/src/wallet/wallet.ts

@@ -48,7 +48,6 @@ import { EncryptedAccount } from './account/encryptedAccount'
 import { AssetBalances } from './assetBalances'
 import {
   DuplicateAccountNameError,
-  DuplicateIdentityNameError,
   DuplicateMultisigSecretNameError,
   DuplicateSpendingKeyError,
   MaxMemoLengthError,
@@ -1570,27 +1569,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 =