Skip to content

Commit

Permalink
deletes multisig identity on account import
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@hughy
hughy committed Dec 6, 2024
1 parent b7253e0 commit 48d0b68
Show file tree
Hide file tree
Showing 4 changed files with 2 additions and 214 deletions.
179 changes: 0 additions & 179 deletions ironfish/src/rpc/routes/wallet/importAccount.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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'
Expand Down
5 changes: 1 addition & 4 deletions ironfish/src/rpc/routes/wallet/importAccount.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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) {
Expand Down
9 changes: 0 additions & 9 deletions ironfish/src/wallet/errors.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
23 changes: 1 addition & 22 deletions ironfish/src/wallet/wallet.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ import { EncryptedAccount } from './account/encryptedAccount'
import { AssetBalances } from './assetBalances'
import {
DuplicateAccountNameError,
DuplicateIdentityNameError,
DuplicateMultisigSecretNameError,
DuplicateSpendingKeyError,
MaxMemoLengthError,
Expand Down Expand Up @@ -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 =
Expand Down

0 comments on commit 48d0b68

Please sign in to comment.