Merge pull request #5364 from iron-fish/rahul/make-secret-optional-multisigSecrets

patnir · web-flow · commit 40b367bcf917 · 2024-09-12T17:45:09.000-07:00
Rahul/make secret optional multisig secrets
diff --git a/ironfish/src/rpc/routes/wallet/exportAccount.test.ts b/ironfish/src/rpc/routes/wallet/exportAccount.test.ts
@@ -127,10 +127,10 @@ describe('Route wallet/exportAccount', () => {
       ),
     )
 
-    const multisigSecret = await routeTest.node.wallet.walletDb.getMultisigSecret(
+    const multisigIdentity = await routeTest.node.wallet.walletDb.getMultisigIdentity(
       Buffer.from(participants[0].identity, 'hex'),
     )
-    Assert.isNotUndefined(multisigSecret)
+    Assert.isNotUndefined(multisigIdentity)
 
     // Initialize the group though TDK and import one of the accounts generated
     const trustedDealerPackage = (
@@ -157,6 +157,9 @@ describe('Route wallet/exportAccount', () => {
     })
 
     expect(response.status).toBe(200)
+
+    Assert.isNotUndefined(multisigIdentity.secret)
+
     expect(JSON.parse(response.content.account)).toMatchObject({
       name: accountNames[0],
       spendingKey: null,
@@ -165,7 +168,7 @@ describe('Route wallet/exportAccount', () => {
       outgoingViewKey: trustedDealerPackage.outgoingViewKey,
       publicAddress: trustedDealerPackage.publicAddress,
       multisigKeys: {
-        secret: multisigSecret.secret.toString('hex'),
+        secret: multisigIdentity.secret.toString('hex'),
         publicKeyPackage: trustedDealerPackage.publicKeyPackage,
       },
     })
@@ -185,10 +188,10 @@ describe('Route wallet/exportAccount', () => {
       ),
     )
 
-    const multisigSecret = await routeTest.node.wallet.walletDb.getMultisigSecret(
+    const multisigIdentity = await routeTest.node.wallet.walletDb.getMultisigIdentity(
       Buffer.from(participants[0].identity, 'hex'),
     )
-    Assert.isNotUndefined(multisigSecret)
+    Assert.isNotUndefined(multisigIdentity)
 
     // Initialize the group though TDK and import one of the accounts generated
     const trustedDealerPackage = (
diff --git a/ironfish/src/rpc/routes/wallet/importAccount.test.ts b/ironfish/src/rpc/routes/wallet/importAccount.test.ts
@@ -370,7 +370,7 @@ describe('Route wallet/importAccount', () => {
         const secret = new multisig.ParticipantSecret(Buffer.from(key, 'hex'))
         const identity = secret.toIdentity()
 
-        await routeTest.node.wallet.walletDb.putMultisigSecret(identity.serialize(), {
+        await routeTest.node.wallet.walletDb.putMultisigIdentity(identity.serialize(), {
           secret: secret.serialize(),
           name: testCaseFile,
         })
diff --git a/ironfish/src/rpc/routes/wallet/multisig/createParticipant.test.ts b/ironfish/src/rpc/routes/wallet/multisig/createParticipant.test.ts
@@ -45,7 +45,7 @@ describe('Route wallet/multisig/createParticipant', () => {
     const name = 'identity'
     const response = await routeTest.client.wallet.multisig.createParticipant({ name })
 
-    const secretValue = await routeTest.node.wallet.walletDb.getMultisigSecret(
+    const secretValue = await routeTest.node.wallet.walletDb.getMultisigIdentity(
       Buffer.from(response.content.identity, 'hex'),
     )
     Assert.isNotUndefined(secretValue)
diff --git a/ironfish/src/rpc/routes/wallet/multisig/dkg/round1.test.ts b/ironfish/src/rpc/routes/wallet/multisig/dkg/round1.test.ts
@@ -33,7 +33,7 @@ describe('Route multisig/dkg/round1', () => {
       participantName,
     )
     Assert.isNotUndefined(secretValue)
-    const secret = new multisig.ParticipantSecret(secretValue.secret)
+    const secret = new multisig.ParticipantSecret(secretValue)
     secret.decryptData(Buffer.from(response.content.round1SecretPackage, 'hex'))
   })
 
diff --git a/ironfish/src/rpc/routes/wallet/multisig/dkg/round1.ts b/ironfish/src/rpc/routes/wallet/multisig/dkg/round1.ts
@@ -55,7 +55,7 @@ routes.register<typeof DkgRound1RequestSchema, DkgRound1Response>(
     }
 
     const participantIdentities = participants.map((p) => p.identity)
-    const selfIdentity = new multisig.ParticipantSecret(multisigSecret.secret)
+    const selfIdentity = new multisig.ParticipantSecret(multisigSecret)
       .toIdentity()
       .serialize()
       .toString('hex')
diff --git a/ironfish/src/rpc/routes/wallet/multisig/dkg/round2.ts b/ironfish/src/rpc/routes/wallet/multisig/dkg/round2.ts
@@ -51,7 +51,7 @@ routes.register<typeof DkgRound2RequestSchema, DkgRound2Response>(
       )
     }
 
-    const secret = multisigSecret.secret.toString('hex')
+    const secret = multisigSecret.toString('hex')
 
     const packages = multisig.dkgRound2(secret, round1SecretPackage, round1PublicPackages)
 
diff --git a/ironfish/src/rpc/routes/wallet/multisig/dkg/round3.ts b/ironfish/src/rpc/routes/wallet/multisig/dkg/round3.ts
@@ -57,7 +57,7 @@ routes.register<typeof DkgRound3RequestSchema, DkgRound3Response>(
       )
     }
 
-    const secret = new multisig.ParticipantSecret(multisigSecret.secret)
+    const secret = new multisig.ParticipantSecret(multisigSecret)
     const identity = secret.toIdentity().serialize().toString('hex')
 
     const {
diff --git a/ironfish/src/rpc/routes/wallet/multisig/getIdentities.ts b/ironfish/src/rpc/routes/wallet/multisig/getIdentities.ts
@@ -45,7 +45,7 @@ routes.register<typeof GetIdentitiesRequestSchema, GetIdentitiesResponse>(
     for await (const [
       identity,
       { name },
-    ] of context.wallet.walletDb.multisigSecrets.getAllIter()) {
+    ] of context.wallet.walletDb.multisigIdentities.getAllIter()) {
       identities.push({
         name,
         identity: identity.toString('hex'),
diff --git a/ironfish/src/rpc/routes/wallet/multisig/getIdentity.ts b/ironfish/src/rpc/routes/wallet/multisig/getIdentity.ts
@@ -36,13 +36,12 @@ routes.register<typeof GetIdentityRequestSchema, GetIdentityResponse>(
 
     const { name } = request.data
 
-    const record = await context.wallet.walletDb.getMultisigSecretByName(name)
-    if (record === undefined) {
+    const secret = await context.wallet.walletDb.getMultisigSecretByName(name)
+    if (secret === undefined) {
       throw new RpcValidationError(`No identity found with name ${name}`, 404)
     }
 
-    const secret = new multisig.ParticipantSecret(record.secret)
-    const identity = secret.toIdentity()
+    const identity = new multisig.ParticipantSecret(secret).toIdentity()
 
     request.end({ identity: identity.serialize().toString('hex') })
   },
diff --git a/ironfish/src/wallet/exporter/encryption.ts b/ironfish/src/wallet/exporter/encryption.ts
@@ -33,7 +33,11 @@ export async function decryptEncodedAccount(
   if (encrypted.startsWith(BASE64_JSON_MULTISIG_ENCRYPTED_ACCOUNT_PREFIX)) {
     const encoded = encrypted.slice(BASE64_JSON_MULTISIG_ENCRYPTED_ACCOUNT_PREFIX.length)
 
-    for await (const { secret: secretBuffer } of wallet.walletDb.getMultisigSecrets()) {
+    for await (const { secret: secretBuffer } of wallet.walletDb.getMultisigIdentities()) {
+      if (secretBuffer === undefined) {
+        continue
+      }
+
       const secret = new multisig.ParticipantSecret(secretBuffer)
       const decrypted = decryptEncodedAccountWithMultisigSecret(encoded, secret)
 
diff --git a/ironfish/src/wallet/wallet.test.slow.ts b/ironfish/src/wallet/wallet.test.slow.ts
@@ -1141,7 +1141,7 @@ describe('Wallet', () => {
           const secret = multisig.ParticipantSecret.random()
           const identity = secret.toIdentity()
 
-          await node.wallet.walletDb.putMultisigSecret(identity.serialize(), {
+          await node.wallet.walletDb.putMultisigIdentity(identity.serialize(), {
             name,
             secret: secret.serialize(),
           })
@@ -1340,7 +1340,7 @@ describe('Wallet', () => {
         const secret = multisig.ParticipantSecret.random()
         const identity = secret.toIdentity()
 
-        await node.wallet.walletDb.putMultisigSecret(identity.serialize(), {
+        await node.wallet.walletDb.putMultisigIdentity(identity.serialize(), {
           name,
           secret: secret.serialize(),
         })
diff --git a/ironfish/src/wallet/wallet.ts b/ironfish/src/wallet/wallet.ts
@@ -1421,17 +1421,17 @@ export class Wallet {
       accountValue.multisigKeys &&
       isMultisigSignerTrustedDealerImport(accountValue.multisigKeys)
     ) {
-      const multisigSecret = await this.walletDb.getMultisigSecret(
+      const multisigIdentity = await this.walletDb.getMultisigIdentity(
         Buffer.from(accountValue.multisigKeys.identity, 'hex'),
       )
-      if (!multisigSecret) {
+      if (!multisigIdentity || !multisigIdentity.secret) {
         throw new Error('Cannot import identity without a corresponding multisig secret')
       }
 
       multisigKeys = {
         keyPackage: accountValue.multisigKeys.keyPackage,
         publicKeyPackage: accountValue.multisigKeys.publicKeyPackage,
-        secret: multisigSecret.secret.toString('hex'),
+        secret: multisigIdentity.secret.toString('hex'),
       }
     }
 
@@ -1856,7 +1856,7 @@ export class Wallet {
       const secret = multisig.ParticipantSecret.random()
       const identity = secret.toIdentity()
 
-      await this.walletDb.putMultisigSecret(
+      await this.walletDb.putMultisigIdentity(
         identity.serialize(),
         {
           name,
diff --git a/ironfish/src/wallet/walletdb/multisigIdentityValue.ts b/ironfish/src/wallet/walletdb/multisigIdentityValue.ts
@@ -0,0 +1,54 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
+import { multisig } from '@ironfish/rust-nodejs'
+import bufio from 'bufio'
+import { IDatabaseEncoding } from '../../storage'
+
+export interface MultisigIdentityValue {
+  name: string
+  /**
+   * The secret is optional when a multisig account is generated on a Ledger device.
+   * The secret never leaves the Ledger device.
+   *
+   * We use a zero buffer encoding approach for the optional 'secret' field:
+   * - Present secret: written directly to buffer
+   * - Undefined secret: zero buffer of same length written
+   *
+   * This approach maintains consistent serialized size and avoids database migrations,
+   * while allowing distinction between undefined and actual secrets during deserialization.
+   */
+  secret?: Buffer
+}
+
+export class MultisigIdentityValueEncoder implements IDatabaseEncoding<MultisigIdentityValue> {
+  serialize(value: MultisigIdentityValue): Buffer {
+    const bw = bufio.write(this.getSize(value))
+    bw.writeVarString(value.name, 'utf-8')
+    if (value.secret) {
+      bw.writeBytes(value.secret)
+    } else {
+      // Write a zero buffer of the same length as the secret
+      bw.writeBytes(Buffer.alloc(multisig.SECRET_LEN))
+    }
+    return bw.render()
+  }
+
+  deserialize(buffer: Buffer): MultisigIdentityValue {
+    const reader = bufio.read(buffer, true)
+    const name = reader.readVarString('utf-8')
+    const secret = reader.readBytes(multisig.SECRET_LEN)
+    // Check if the secret is all zeros
+    if (Buffer.compare(secret, Buffer.alloc(multisig.SECRET_LEN)) === 0) {
+      return { name, secret: undefined }
+    }
+    return { name, secret }
+  }
+
+  getSize(value: MultisigIdentityValue): number {
+    let size = 0
+    size += bufio.sizeVarString(value.name, 'utf8')
+    size += multisig.SECRET_LEN
+    return size
+  }
+}
diff --git a/ironfish/src/wallet/walletdb/multisigSecretValue.ts b/ironfish/src/wallet/walletdb/multisigSecretValue.ts
diff --git a/ironfish/src/wallet/walletdb/walletdb.test.ts b/ironfish/src/wallet/walletdb/walletdb.test.ts
@@ -450,14 +450,14 @@ describe('WalletDB', () => {
       const secret = multisig.ParticipantSecret.random()
       const serializedSecret = secret.serialize()
 
-      await walletDb.putMultisigSecret(secret.toIdentity().serialize(), {
+      await walletDb.putMultisigIdentity(secret.toIdentity().serialize(), {
         secret: serializedSecret,
         name,
       })
 
       const storedSecret = await walletDb.getMultisigSecretByName(name)
       Assert.isNotUndefined(storedSecret)
-      expect(storedSecret.secret).toEqualBuffer(serializedSecret)
+      expect(storedSecret).toEqualBuffer(serializedSecret)
     })
   })
 
diff --git a/ironfish/src/wallet/walletdb/walletdb.ts b/ironfish/src/wallet/walletdb/walletdb.ts
@@ -40,7 +40,7 @@ import { BalanceValue, BalanceValueEncoding } from './balanceValue'
 import { DecryptedNoteValue, DecryptedNoteValueEncoding } from './decryptedNoteValue'
 import { HeadValue, NullableHeadValueEncoding } from './headValue'
 import { AccountsDBMeta, MetaValue, MetaValueEncoding } from './metaValue'
-import { MultisigSecretValue, MultisigSecretValueEncoding } from './multisigSecretValue'
+import { MultisigIdentityValue, MultisigIdentityValueEncoder } from './multisigIdentityValue'
 import { ParticipantIdentity, ParticipantIdentityEncoding } from './participantIdentity'
 import { TransactionValue, TransactionValueEncoding } from './transactionValue'
 
@@ -138,9 +138,9 @@ export class WalletDB {
     value: null
   }>
 
-  multisigSecrets: IDatabaseStore<{
+  multisigIdentities: IDatabaseStore<{
     key: Buffer
-    value: MultisigSecretValue
+    value: MultisigIdentityValue
   }>
 
   participantIdentities: IDatabaseStore<{
@@ -298,10 +298,10 @@ export class WalletDB {
       valueEncoding: NULL_ENCODING,
     })
 
-    this.multisigSecrets = this.db.addStore({
+    this.multisigIdentities = this.db.addStore({
       name: 'ms',
       keyEncoding: new BufferEncoding(),
-      valueEncoding: new MultisigSecretValueEncoding(),
+      valueEncoding: new MultisigIdentityValueEncoder(),
     })
 
     this.participantIdentities = this.db.addStore({
@@ -1420,36 +1420,36 @@ export class WalletDB {
     await this.nullifierToTransactionHash.del([account.prefix, nullifier], tx)
   }
 
-  async putMultisigSecret(
+  async putMultisigIdentity(
     identity: Buffer,
-    value: MultisigSecretValue,
+    value: MultisigIdentityValue,
     tx?: IDatabaseTransaction,
   ): Promise<void> {
-    await this.multisigSecrets.put(identity, value, tx)
+    await this.multisigIdentities.put(identity, value, tx)
   }
 
-  async getMultisigSecret(
+  async getMultisigIdentity(
     identity: Buffer,
     tx?: IDatabaseTransaction,
-  ): Promise<MultisigSecretValue | undefined> {
-    return this.multisigSecrets.get(identity, tx)
+  ): Promise<MultisigIdentityValue | undefined> {
+    return this.multisigIdentities.get(identity, tx)
   }
 
-  async hasMultisigSecret(identity: Buffer, tx?: IDatabaseTransaction): Promise<boolean> {
-    return (await this.getMultisigSecret(identity, tx)) !== undefined
+  async hasMultisigIdentity(identity: Buffer, tx?: IDatabaseTransaction): Promise<boolean> {
+    return (await this.getMultisigIdentity(identity, tx)) !== undefined
   }
 
-  async deleteMultisigSecret(identity: Buffer, tx?: IDatabaseTransaction): Promise<void> {
-    await this.multisigSecrets.del(identity, tx)
+  async deleteMultisigIdentity(identity: Buffer, tx?: IDatabaseTransaction): Promise<void> {
+    await this.multisigIdentities.del(identity, tx)
   }
 
   async getMultisigSecretByName(
     name: string,
     tx?: IDatabaseTransaction,
-  ): Promise<MultisigSecretValue | undefined> {
-    for await (const value of this.multisigSecrets.getAllValuesIter(tx)) {
+  ): Promise<Buffer | undefined> {
+    for await (const value of this.multisigIdentities.getAllValuesIter(tx)) {
       if (value.name === name) {
-        return value
+        return value.secret
       }
     }
 
@@ -1460,8 +1460,10 @@ export class WalletDB {
     return (await this.getMultisigSecretByName(name, tx)) !== undefined
   }
 
-  async *getMultisigSecrets(tx?: IDatabaseTransaction): AsyncGenerator<MultisigSecretValue> {
-    for await (const value of this.multisigSecrets.getAllValuesIter(tx)) {
+  async *getMultisigIdentities(
+    tx?: IDatabaseTransaction,
+  ): AsyncGenerator<MultisigIdentityValue> {
+    for await (const value of this.multisigIdentities.getAllValuesIter(tx)) {
       yield value
     }
   }

Original file line number	Diff line number	Diff line change
`@@ -45,7 +45,7 @@ describe('Route wallet/multisig/createParticipant', () => {`
`45`	`45`	`const name = 'identity'`
`46`	`46`	`const response = await routeTest.client.wallet.multisig.createParticipant({ name })`
`47`	`47`
`48`		`- const secretValue = await routeTest.node.wallet.walletDb.getMultisigSecret(`
	`48`	`+ const secretValue = await routeTest.node.wallet.walletDb.getMultisigIdentity(`
`49`	`49`	`Buffer.from(response.content.identity, 'hex'),`
`50`	`50`	`)`
`51`	`51`	`Assert.isNotUndefined(secretValue)`
Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,7 @@ describe('Route multisig/dkg/round1', () => {`
`33`	`33`	`participantName,`
`34`	`34`	`)`
`35`	`35`	`Assert.isNotUndefined(secretValue)`
`36`		`- const secret = new multisig.ParticipantSecret(secretValue.secret)`
	`36`	`+ const secret = new multisig.ParticipantSecret(secretValue)`
`37`	`37`	`secret.decryptData(Buffer.from(response.content.round1SecretPackage, 'hex'))`
`38`	`38`	`})`
`39`	`39`
Original file line number	Diff line number	Diff line change
`@@ -55,7 +55,7 @@ routes.register<typeof DkgRound1RequestSchema, DkgRound1Response>(`
`55`	`55`	`}`
`56`	`56`
`57`	`57`	`const participantIdentities = participants.map((p) => p.identity)`
`58`		`- const selfIdentity = new multisig.ParticipantSecret(multisigSecret.secret)`
	`58`	`+ const selfIdentity = new multisig.ParticipantSecret(multisigSecret)`
`59`	`59`	`.toIdentity()`
`60`	`60`	`.serialize()`
`61`	`61`	`.toString('hex')`
Original file line number	Diff line number	Diff line change
`@@ -51,7 +51,7 @@ routes.register<typeof DkgRound2RequestSchema, DkgRound2Response>(`
`51`	`51`	`)`
`52`	`52`	`}`
`53`	`53`
`54`		`- const secret = multisigSecret.secret.toString('hex')`
	`54`	`+ const secret = multisigSecret.toString('hex')`
`55`	`55`
`56`	`56`	`const packages = multisig.dkgRound2(secret, round1SecretPackage, round1PublicPackages)`
`57`	`57`
Original file line number	Diff line number	Diff line change
`@@ -57,7 +57,7 @@ routes.register<typeof DkgRound3RequestSchema, DkgRound3Response>(`
`57`	`57`	`)`
`58`	`58`	`}`
`59`	`59`
`60`		`- const secret = new multisig.ParticipantSecret(multisigSecret.secret)`
	`60`	`+ const secret = new multisig.ParticipantSecret(multisigSecret)`
`61`	`61`	`const identity = secret.toIdentity().serialize().toString('hex')`
`62`	`62`
`63`	`63`	`const {`