Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deletes multisig identity on account import #5692

Draft
wants to merge 1 commit into
base: feat/hughy/multisig-keys-identity
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
179 changes: 0 additions & 179 deletions ironfish/src/rpc/routes/wallet/importAccount.test.ts
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
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
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
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