removing unsigned transaction from aggregate signature share (#4739)

* removing unsigned transaction from aggregate signature share

* rust lint fix

ironfish-cli/src/commands/wallet/multisig/sign.ts

@@ -18,10 +18,6 @@ export class MultisigSign extends IronfishCommand {
       description: 'Account to use when aggregating signature shares',
       required: false,
     }),
-    unsignedTransaction: Flags.string({
-      char: 'u',
-      description: 'Unsigned transaction',
-    }),
     signingPackage: Flags.string({
       char: 'p',
       description: 'Signing package',
@@ -41,10 +37,6 @@ export class MultisigSign extends IronfishCommand {
   async start(): Promise<void> {
     const { flags } = await this.parse(MultisigSign)
 
-    const unsignedTransaction =
-      flags.unsignedTransaction?.trim() ??
-      (await longPrompt('Enter the unsigned transaction: ', { required: true }))
-
     const signingPackage =
       flags.signingPackage?.trim() ??
       (await longPrompt('Enter the signing package: ', { required: true }))
@@ -65,7 +57,6 @@ export class MultisigSign extends IronfishCommand {
     const response = await client.wallet.multisig.aggregateSignatureShares({
       account: flags.account,
       broadcast: flags.broadcast,
-      unsignedTransaction,
       signingPackage,
       signatureShares,
     })

ironfish-rust-nodejs/index.d.ts

@@ -52,6 +52,7 @@ export const TRANSACTION_EXPIRATION_LENGTH: number
 export const TRANSACTION_FEE_LENGTH: number
 export const LATEST_TRANSACTION_VERSION: number
 export function verifyTransactions(serializedTransactions: Array<Buffer>): boolean
+export function aggregateSignatureShares(publicKeyPackageStr: string, signingPackageStr: string, signatureSharesArr: Array<string>): Buffer
 export interface IdentityKeyPackage {
   identity: string
   keyPackage: string
@@ -244,7 +245,6 @@ export class UnsignedTransaction {
   hash(): Buffer
   signingPackage(nativeIdentiferCommitments: Array<string>): string
   sign(spenderHexKey: string): Buffer
-  aggregateSignatureShares(publicKeyPackageStr: string, signingPackageStr: string, signatureSharesArr: Array<string>): Buffer
 }
 export class FoundBlockResult {
   randomness: string

ironfish-rust-nodejs/index.js

@@ -252,7 +252,7 @@ if (!nativeBinding) {
   throw new Error(`Failed to load native binding`)
 }
 
-const { FishHashContext, createSigningCommitment, createSignatureShare, ParticipantSecret, ParticipantIdentity, splitSecret, contribute, verifyTransform, KEY_LENGTH, NONCE_LENGTH, BoxKeyPair, randomBytes, boxMessage, unboxMessage, RollingFilter, initSignalHandler, triggerSegfault, ASSET_ID_LENGTH, ASSET_METADATA_LENGTH, ASSET_NAME_LENGTH, ASSET_LENGTH, Asset, NOTE_ENCRYPTION_KEY_LENGTH, MAC_LENGTH, ENCRYPTED_NOTE_PLAINTEXT_LENGTH, ENCRYPTED_NOTE_LENGTH, NoteEncrypted, PUBLIC_ADDRESS_LENGTH, RANDOMNESS_LENGTH, MEMO_LENGTH, AMOUNT_VALUE_LENGTH, DECRYPTED_NOTE_LENGTH, Note, PROOF_LENGTH, TRANSACTION_SIGNATURE_LENGTH, TRANSACTION_PUBLIC_KEY_RANDOMNESS_LENGTH, TRANSACTION_EXPIRATION_LENGTH, TRANSACTION_FEE_LENGTH, LATEST_TRANSACTION_VERSION, TransactionPosted, Transaction, verifyTransactions, UnsignedTransaction, LanguageCode, generateKey, spendingKeyToWords, wordsToSpendingKey, generateKeyFromPrivateKey, initializeSapling, FoundBlockResult, ThreadPoolHandler, isValidPublicAddress } = nativeBinding
+const { FishHashContext, createSigningCommitment, createSignatureShare, ParticipantSecret, ParticipantIdentity, splitSecret, contribute, verifyTransform, KEY_LENGTH, NONCE_LENGTH, BoxKeyPair, randomBytes, boxMessage, unboxMessage, RollingFilter, initSignalHandler, triggerSegfault, ASSET_ID_LENGTH, ASSET_METADATA_LENGTH, ASSET_NAME_LENGTH, ASSET_LENGTH, Asset, NOTE_ENCRYPTION_KEY_LENGTH, MAC_LENGTH, ENCRYPTED_NOTE_PLAINTEXT_LENGTH, ENCRYPTED_NOTE_LENGTH, NoteEncrypted, PUBLIC_ADDRESS_LENGTH, RANDOMNESS_LENGTH, MEMO_LENGTH, AMOUNT_VALUE_LENGTH, DECRYPTED_NOTE_LENGTH, Note, PROOF_LENGTH, TRANSACTION_SIGNATURE_LENGTH, TRANSACTION_PUBLIC_KEY_RANDOMNESS_LENGTH, TRANSACTION_EXPIRATION_LENGTH, TRANSACTION_FEE_LENGTH, LATEST_TRANSACTION_VERSION, TransactionPosted, Transaction, verifyTransactions, UnsignedTransaction, aggregateSignatureShares, LanguageCode, generateKey, spendingKeyToWords, wordsToSpendingKey, generateKeyFromPrivateKey, initializeSapling, FoundBlockResult, ThreadPoolHandler, isValidPublicAddress } = nativeBinding
 
 module.exports.FishHashContext = FishHashContext
 module.exports.createSigningCommitment = createSigningCommitment
@@ -297,6 +297,7 @@ module.exports.TransactionPosted = TransactionPosted
 module.exports.Transaction = Transaction
 module.exports.verifyTransactions = verifyTransactions
 module.exports.UnsignedTransaction = UnsignedTransaction
+module.exports.aggregateSignatureShares = aggregateSignatureShares
 module.exports.LanguageCode = LanguageCode
 module.exports.generateKey = generateKey
 module.exports.spendingKeyToWords = spendingKeyToWords

ironfish-rust-nodejs/src/structs/transaction.rs

@@ -457,38 +457,41 @@ impl NativeUnsignedTransaction {
 
         Ok(Buffer::from(vec))
     }
+}
 
-    #[napi]
-    pub fn aggregate_signature_shares(
-        &mut self,
-        public_key_package_str: String,
-        signing_package_str: String,
-        signature_shares_arr: Vec<String>,
-    ) -> Result<Buffer> {
-        let public_key_package = PublicKeyPackage::deserialize(
-            &hex_to_vec_bytes(&public_key_package_str).map_err(to_napi_err)?,
+#[napi]
+pub fn aggregate_signature_shares(
+    public_key_package_str: String,
+    signing_package_str: String,
+    signature_shares_arr: Vec<String>,
+) -> Result<Buffer> {
+    let public_key_package = PublicKeyPackage::deserialize(
+        &hex_to_vec_bytes(&public_key_package_str).map_err(to_napi_err)?,
+    )
+    .map_err(to_napi_err)?;
+
+    let bytes = hex_to_vec_bytes(&signing_package_str).map_err(to_napi_err)?;
+    let signing_package = SigningPackage::read(&bytes[..]).map_err(to_napi_err)?;
+
+    let mut unsigned_transaction = signing_package.unsigned_transaction;
+
+    let mut signature_shares = BTreeMap::<Identifier, FrostSignatureShare>::new();
+    for signature_share in signature_shares_arr.iter() {
+        let iss = SignatureShare::deserialize(&hex_to_bytes(signature_share).map_err(to_napi_err)?)
+            .map_err(to_napi_err)?;
+        signature_shares.insert(iss.identity.to_frost_identifier(), iss.signature_share);
+    }
+
+    let signed_transaction = unsigned_transaction
+        .aggregate_signature_shares(
+            &public_key_package,
+            &signing_package.frost_signing_package,
+            signature_shares,
         )
         .map_err(to_napi_err)?;
 
-        let bytes = hex_to_vec_bytes(&signing_package_str).map_err(to_napi_err)?;
-        let signing_package = SigningPackage::read(&bytes[..]).map_err(to_napi_err)?;
-
-        let mut signature_shares = BTreeMap::<Identifier, FrostSignatureShare>::new();
-        for signature_share in signature_shares_arr.iter() {
-            let iss =
-                SignatureShare::deserialize(&hex_to_bytes(signature_share).map_err(to_napi_err)?)
-                    .map_err(to_napi_err)?;
-            signature_shares.insert(iss.identity.to_frost_identifier(), iss.signature_share);
-        }
-
-        let signed_transaction = self
-            .transaction
-            .aggregate_signature_shares(&public_key_package, &signing_package, signature_shares)
-            .map_err(to_napi_err)?;
+    let mut vec: Vec<u8> = vec![];
+    signed_transaction.write(&mut vec).map_err(to_napi_err)?;
 
-        let mut vec: Vec<u8> = vec![];
-        signed_transaction.write(&mut vec).map_err(to_napi_err)?;
-
-        Ok(Buffer::from(vec))
-    }
+    Ok(Buffer::from(vec))
 }

ironfish-rust/src/transaction/tests.rs

@@ -829,7 +829,7 @@ fn test_aggregate_signature_shares() {
     let signed_transaction = unsigned_transaction
         .aggregate_signature_shares(
             &key_packages.public_key_package,
-            &signing_package,
+            &signing_package.frost_signing_package,
             signature_shares,
         )
         .expect("should be able to sign transaction");

ironfish-rust/src/transaction/unsigned.rs

@@ -197,7 +197,7 @@ impl UnsignedTransaction {
     pub fn aggregate_signature_shares(
         &mut self,
         public_key_package: &PublicKeyPackage,
-        authorizing_signing_package: &SigningPackage,
+        authorizing_signing_package: &FrostSigningPackage,
         authorizing_signature_shares: BTreeMap<Identifier, SignatureShare>,
     ) -> Result<Transaction, IronfishError> {
         // Create the transaction signature hash
@@ -209,7 +209,7 @@ impl UnsignedTransaction {
             RandomizedParams::from_randomizer(public_key_package.verifying_key(), randomizer);
 
         let authorizing_group_signature = aggregate(
-            &authorizing_signing_package.frost_signing_package,
+            authorizing_signing_package,
             &authorizing_signature_shares,
             public_key_package,
             &randomized_params,

ironfish/src/rpc/routes/wallet/multisig/aggregateSignatureShares.ts

@@ -1,7 +1,7 @@
 /* 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 { UnsignedTransaction } from '@ironfish/rust-nodejs'
+import { aggregateSignatureShares } from '@ironfish/rust-nodejs'
 import * as yup from 'yup'
 import { Transaction } from '../../../../primitives/transaction'
 import { AssertMultisig } from '../../../../wallet'
@@ -12,7 +12,6 @@ import { getAccount } from '../utils'
 
 export type AggregateSignatureSharesRequest = {
   account?: string
-  unsignedTransaction: string
   signingPackage: string
   signatureShares: Array<string>
   broadcast?: boolean
@@ -28,7 +27,6 @@ export const AggregateSignatureSharesRequestSchema: yup.ObjectSchema<AggregateSi
   yup
     .object({
       account: yup.string().optional(),
-      unsignedTransaction: yup.string().defined(),
       signingPackage: yup.string().defined(),
       signatureShares: yup.array(yup.string().defined()).defined(),
       broadcast: yup.boolean().optional().default(true),
@@ -52,14 +50,12 @@ routes.register<typeof AggregateSignatureSharesRequestSchema, AggregateSignature
     const account = getAccount(node.wallet, request.data.account)
     AssertMultisig(account)
 
-    const unsigned = new UnsignedTransaction(
-      Buffer.from(request.data.unsignedTransaction, 'hex'),
-    )
-    const serialized = unsigned.aggregateSignatureShares(
+    const serialized = aggregateSignatureShares(
       account.multisigKeys.publicKeyPackage,
       request.data.signingPackage,
       request.data.signatureShares,
     )
+
     const transaction = new Transaction(serialized)
 
     let accepted = false

ironfish/src/rpc/routes/wallet/multisig/integration.test.slow.ts

@@ -141,7 +141,6 @@ describe('multisig RPC integration', () => {
     // aggregate signing shares
     const aggregateResponse = await routeTest.client.wallet.multisig.aggregateSignatureShares({
       account: coordinatorAccount.name,
-      unsignedTransaction,
       signingPackage,
       signatureShares,
     })

ironfish/src/wallet/wallet.test.slow.ts

@@ -2,6 +2,7 @@
  * 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 {
+  aggregateSignatureShares,
   Asset,
   ASSET_ID_LENGTH,
   createSignatureShare,
@@ -1307,7 +1308,7 @@ describe('Wallet', () => {
       }
 
       Assert.isNotUndefined(coordinator.multisigKeys)
-      const serializedFrostTransaction = unsignedTransaction.aggregateSignatureShares(
+      const serializedFrostTransaction = aggregateSignatureShares(
         coordinator.multisigKeys.publicKeyPackage,
         signingPackage,
         signatureShares,