Import duplicate spending key says account name (#5115)

When importing an account twice, it would not print out the name of the
account with the duplicate spending key. This will now include that in
the message.

It also improves tests to check the error type instead of error message.
We should avoid testing error messages in tests if there is a proper
typed error to check.

ironfish/src/wallet/errors.ts

@@ -43,6 +43,15 @@ export class DuplicateAccountNameError extends Error {
   }
 }
 
+export class DuplicateSpendingKeyError extends Error {
+  name = this.constructor.name
+
+  constructor(name: string) {
+    super()
+    this.message = `Account already exists with provided spending key: ${name}`
+  }
+}
+
 export class DuplicateMultisigSecretNameError extends Error {
   name = this.constructor.name
 

ironfish/src/wallet/wallet.test.ts

@@ -22,7 +22,11 @@ import {
 } from '../testUtilities'
 import { AsyncUtils, BufferUtils, ORE_TO_IRON } from '../utils'
 import { Account, TransactionStatus, TransactionType } from '../wallet'
-import { MaxMemoLengthError } from './errors'
+import {
+  DuplicateAccountNameError,
+  DuplicateSpendingKeyError,
+  MaxMemoLengthError,
+} from './errors'
 import { toAccountImport } from './exporter'
 import { AssetStatus, Wallet } from './wallet'
 
@@ -499,7 +503,7 @@ describe('Wallet', () => {
       expect(node.wallet.accountExists(account.name)).toEqual(true)
 
       await expect(node.wallet.importAccount(account)).rejects.toThrow(
-        'Account already exists with the name',
+        DuplicateAccountNameError,
       )
     })
 
@@ -513,9 +517,7 @@ describe('Wallet', () => {
       const clone = { ...account }
       clone.name = 'Different name'
 
-      await expect(node.wallet.importAccount(clone)).rejects.toThrow(
-        'Account already exists with provided spending key',
-      )
+      await expect(node.wallet.importAccount(clone)).rejects.toThrow(DuplicateSpendingKeyError)
     })
 
     it('should be able to import an account from solely its view keys', async () => {

ironfish/src/wallet/wallet.ts

@@ -46,6 +46,7 @@ import { AssetBalances } from './assetBalances'
 import {
   DuplicateAccountNameError,
   DuplicateMultisigSecretNameError,
+  DuplicateSpendingKeyError,
   MaxMemoLengthError,
   NotEnoughFundsError,
 } from './errors'
@@ -1348,12 +1349,14 @@ export class Wallet {
       throw new DuplicateAccountNameError(name)
     }
 
-    const accounts = this.listAccounts()
-    if (
-      accountValue.spendingKey &&
-      accounts.find((a) => accountValue.spendingKey === a.spendingKey)
-    ) {
-      throw new Error(`Account already exists with provided spending key`)
+    if (accountValue.spendingKey) {
+      const duplicateSpendingAccount = this.listAccounts().find(
+        (a) => accountValue.spendingKey === a.spendingKey,
+      )
+
+      if (duplicateSpendingAccount) {
+        throw new DuplicateSpendingKeyError(duplicateSpendingAccount.name)
+      }
     }
 
     validateAccountImport(accountValue)