From 8a920f19e665ace8759f4cb81becfc8ac9c26cc9 Mon Sep 17 00:00:00 2001 From: Rahul Patni Date: Thu, 15 Feb 2024 15:57:09 -0800 Subject: [PATCH] removing unsigned transaction from aggregate signature share (#4739) * removing unsigned transaction from aggregate signature share * rust lint fix --- .../src/commands/wallet/multisig/sign.ts | 9 --- ironfish-rust-nodejs/index.d.ts | 2 +- ironfish-rust-nodejs/index.js | 3 +- .../src/structs/transaction.rs | 61 ++++++++++--------- ironfish-rust/src/transaction/tests.rs | 2 +- ironfish-rust/src/transaction/unsigned.rs | 4 +- .../multisig/aggregateSignatureShares.ts | 10 +-- .../wallet/multisig/integration.test.slow.ts | 1 - ironfish/src/wallet/wallet.test.slow.ts | 3 +- 9 files changed, 43 insertions(+), 52 deletions(-) diff --git a/ironfish-cli/src/commands/wallet/multisig/sign.ts b/ironfish-cli/src/commands/wallet/multisig/sign.ts index 596343cae5..185ad15894 100644 --- a/ironfish-cli/src/commands/wallet/multisig/sign.ts +++ b/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 { 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, }) diff --git a/ironfish-rust-nodejs/index.d.ts b/ironfish-rust-nodejs/index.d.ts index 8b98c2d4d9..b355ec7b70 100644 --- a/ironfish-rust-nodejs/index.d.ts +++ b/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): boolean +export function aggregateSignatureShares(publicKeyPackageStr: string, signingPackageStr: string, signatureSharesArr: Array): Buffer export interface IdentityKeyPackage { identity: string keyPackage: string @@ -244,7 +245,6 @@ export class UnsignedTransaction { hash(): Buffer signingPackage(nativeIdentiferCommitments: Array): string sign(spenderHexKey: string): Buffer - aggregateSignatureShares(publicKeyPackageStr: string, signingPackageStr: string, signatureSharesArr: Array): Buffer } export class FoundBlockResult { randomness: string diff --git a/ironfish-rust-nodejs/index.js b/ironfish-rust-nodejs/index.js index 8c5d729d83..8845115cb6 100644 --- a/ironfish-rust-nodejs/index.js +++ b/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 diff --git a/ironfish-rust-nodejs/src/structs/transaction.rs b/ironfish-rust-nodejs/src/structs/transaction.rs index 40bc4e7549..fb1f49a7da 100644 --- a/ironfish-rust-nodejs/src/structs/transaction.rs +++ b/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, - ) -> Result { - 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, +) -> Result { + 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::::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::::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 = vec![]; + signed_transaction.write(&mut vec).map_err(to_napi_err)?; - let mut vec: Vec = vec![]; - signed_transaction.write(&mut vec).map_err(to_napi_err)?; - - Ok(Buffer::from(vec)) - } + Ok(Buffer::from(vec)) } diff --git a/ironfish-rust/src/transaction/tests.rs b/ironfish-rust/src/transaction/tests.rs index c24ca948e6..4a2a0dbfdf 100644 --- a/ironfish-rust/src/transaction/tests.rs +++ b/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"); diff --git a/ironfish-rust/src/transaction/unsigned.rs b/ironfish-rust/src/transaction/unsigned.rs index f89df4c931..f774095278 100644 --- a/ironfish-rust/src/transaction/unsigned.rs +++ b/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, ) -> Result { // 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, diff --git a/ironfish/src/rpc/routes/wallet/multisig/aggregateSignatureShares.ts b/ironfish/src/rpc/routes/wallet/multisig/aggregateSignatureShares.ts index 51e1eb11d9..7016969c40 100644 --- a/ironfish/src/rpc/routes/wallet/multisig/aggregateSignatureShares.ts +++ b/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 broadcast?: boolean @@ -28,7 +27,6 @@ export const AggregateSignatureSharesRequestSchema: yup.ObjectSchema { // aggregate signing shares const aggregateResponse = await routeTest.client.wallet.multisig.aggregateSignatureShares({ account: coordinatorAccount.name, - unsignedTransaction, signingPackage, signatureShares, }) diff --git a/ironfish/src/wallet/wallet.test.slow.ts b/ironfish/src/wallet/wallet.test.slow.ts index 8a6cc1aab8..7c975fc4cb 100644 --- a/ironfish/src/wallet/wallet.test.slow.ts +++ b/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,