From cd14715b892222d2567ba6805a17989ddb44c024 Mon Sep 17 00:00:00 2001 From: ivaylogarnev Date: Wed, 18 Sep 2024 13:54:47 +0300 Subject: [PATCH] =?UTF-8?q?feat:=20Add=20multi-signature=20and=20multi-nod?= =?UTF-8?q?e=20support=20for=20signing=20and=20addi=E2=80=A6=20(#2514)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: Add multi-signature and multi-node support for signing and adding signatures Signed-off-by: ivaylogarnev-limechain * test: Added signTransaction method tests Signed-off-by: ivaylogarnev-limechain * test: Added some integration tests for signTransaction method Signed-off-by: ivaylogarnev-limechain --------- Signed-off-by: ivaylogarnev-limechain --- src/PrivateKey.js | 20 +++-- src/transaction/Transaction.js | 56 +++++++----- test/integration/PrivateKey.js | 118 +++++++++++++++++++++++++ test/unit/PrivateKey.js | 121 +++++++++++++++++++++++++ test/unit/Transaction.js | 155 +++++++++++++++++++++++++++++++++ 5 files changed, 443 insertions(+), 27 deletions(-) create mode 100644 test/integration/PrivateKey.js create mode 100644 test/unit/PrivateKey.js diff --git a/src/PrivateKey.js b/src/PrivateKey.js index 418d3bbb2..41b35816e 100644 --- a/src/PrivateKey.js +++ b/src/PrivateKey.js @@ -325,16 +325,24 @@ export default class PrivateKey extends Key { /** * @param {Transaction} transaction - * @returns {Uint8Array} + * @returns {Uint8Array | Uint8Array[]} */ signTransaction(transaction) { - const tx = transaction._signedTransactions.get(0); - const signature = - tx.bodyBytes != null ? this.sign(tx.bodyBytes) : new Uint8Array(); + const signatures = transaction._signedTransactions.list.map( + (signedTransaction) => { + const bodyBytes = signedTransaction.bodyBytes; + + if (!bodyBytes) { + return new Uint8Array(); + } + + return this._key.sign(bodyBytes); + }, + ); - transaction.addSignature(this.publicKey, signature); + transaction.addSignature(this.publicKey, signatures); - return signature; + return signatures; } /** diff --git a/src/transaction/Transaction.js b/src/transaction/Transaction.js index e08a0d36e..15c97c8eb 100644 --- a/src/transaction/Transaction.js +++ b/src/transaction/Transaction.js @@ -789,20 +789,33 @@ export default class Transaction extends Executable { /** * Add a signature explicitly * - * This method requires the transaction to have exactly 1 node account ID set - * since different node account IDs have different byte representations and - * hence the same signature would not work for all transactions that are the same - * except for node account ID being different. + * This method supports both single and multiple signatures. A single signature will be applied to all transactions, + * While an array of signatures must correspond to each transaction individually. * * @param {PublicKey} publicKey - * @param {Uint8Array} signature + * @param {Uint8Array | Uint8Array[]} signature * @returns {this} */ addSignature(publicKey, signature) { - // Require that only one node is set on this transaction - // FIXME: This doesn't consider if we have one node account ID set, but we're - // also a chunked transaction. We should also check transaction IDs is of length 1 - this._requireOneNodeAccountId(); + const isSingleSignature = signature instanceof Uint8Array; + const isArraySignature = Array.isArray(signature); + + // Check if it is a single signature with NOT exactly one transaction + if (isSingleSignature && this._signedTransactions.length !== 1) { + throw new Error( + "Signature array must match the number of transactions", + ); + } + + // Check if it's an array but the array length doesn't match the number of transactions + if ( + isArraySignature && + signature.length !== this._signedTransactions.length + ) { + throw new Error( + "Signature array must match the number of transactions", + ); + } // If the transaction isn't frozen, freeze it. if (!this.isFrozen()) { @@ -817,7 +830,7 @@ export default class Transaction extends Executable { return this; } - // Transactions will have to be regenerated + // If we add a new signer, then we need to re-create all transactions this._transactions.clear(); // Locking the transaction IDs and node account IDs is necessary for consistency @@ -826,21 +839,22 @@ export default class Transaction extends Executable { this._nodeAccountIds.setLocked(); this._signedTransactions.setLocked(); - // Add the signature to the signed transaction list. This is a copy paste - // of `.signWith()`, but it really shouldn't be if `_signedTransactions.list` - // must be a length of one. - // FIXME: Remove unnecessary for loop. - for (const transaction of this._signedTransactions.list) { - if (transaction.sigMap == null) { - transaction.sigMap = {}; + const signatureArray = isSingleSignature ? [signature] : signature; + + // Add the signature to the signed transaction list + for (let index = 0; index < this._signedTransactions.length; index++) { + const signedTransaction = this._signedTransactions.get(index); + + if (signedTransaction.sigMap == null) { + signedTransaction.sigMap = {}; } - if (transaction.sigMap.sigPair == null) { - transaction.sigMap.sigPair = []; + if (signedTransaction.sigMap.sigPair == null) { + signedTransaction.sigMap.sigPair = []; } - transaction.sigMap.sigPair.push( - publicKey._toProtobufSignature(signature), + signedTransaction.sigMap.sigPair.push( + publicKey._toProtobufSignature(signatureArray[index]), ); } diff --git a/test/integration/PrivateKey.js b/test/integration/PrivateKey.js new file mode 100644 index 000000000..dc6a2ee8b --- /dev/null +++ b/test/integration/PrivateKey.js @@ -0,0 +1,118 @@ +import { + PrivateKey, + AccountCreateTransaction, + Hbar, + AccountId, + KeyList, + TransferTransaction, + Transaction, + Status, + FileAppendTransaction, + FileCreateTransaction, +} from "../../src/exports.js"; +import dotenv from "dotenv"; +import IntegrationTestEnv from "./client/NodeIntegrationTestEnv.js"; + +import { expect } from "chai"; + +dotenv.config(); + +describe("PrivateKey signTransaction", function () { + let env, user1Key, user2Key, createdAccountId, keyList; + + // Setting up the environment and creating a new account with a key list + before(async () => { + env = await IntegrationTestEnv.new(); + + user1Key = PrivateKey.generate(); + user2Key = PrivateKey.generate(); + keyList = new KeyList([user1Key.publicKey, user2Key.publicKey]); + + // Create account + const createAccountTransaction = new AccountCreateTransaction() + .setInitialBalance(new Hbar(2)) + .setKey(keyList); + + const createResponse = await createAccountTransaction.execute( + env.client, + ); + const createReceipt = await createResponse.getReceipt(env.client); + + createdAccountId = createReceipt.accountId; + + expect(createdAccountId).to.exist; + }); + + it("Transfer Transaction Execution with Multiple Nodes", async () => { + // Create and sign transfer transaction + const transferTransaction = new TransferTransaction() + .addHbarTransfer(createdAccountId, new Hbar(-1)) + .addHbarTransfer("0.0.3", new Hbar(1)) + .setNodeAccountIds([ + new AccountId(3), + new AccountId(4), + new AccountId(5), + ]) + .freezeWith(env.client); + + // Serialize and sign the transaction + const transferTransactionBytes = transferTransaction.toBytes(); + const user1Signatures = user1Key.signTransaction(transferTransaction); + const user2Signatures = user2Key.signTransaction(transferTransaction); + + // Deserialize the transaction and add signatures + const signedTransaction = Transaction.fromBytes( + transferTransactionBytes, + ); + signedTransaction.addSignature(user1Key.publicKey, user1Signatures); + signedTransaction.addSignature(user2Key.publicKey, user2Signatures); + + // Execute the signed transaction + const result = await signedTransaction.execute(env.client); + const receipt = await result.getReceipt(env.client); + + expect(receipt.status).to.be.equal(Status.Success); + }); + + it("File Append Transaction Execution with Multiple Nodes", async () => { + const operatorKey = env.operatorKey.publicKey; + + // Create file + let response = await new FileCreateTransaction() + .setKeys([operatorKey]) + .setContents("[e2e::FileCreateTransaction]") + .execute(env.client); + + let createTxReceipt = await response.getReceipt(env.client); + const file = createTxReceipt.fileId; + + // Append content to the file + const fileAppendTx = new FileAppendTransaction() + .setFileId(file) + .setContents("[e2e::FileAppendTransaction]") + .setNodeAccountIds([ + new AccountId(3), + new AccountId(4), + new AccountId(5), + ]) + .freezeWith(env.client); + + // Serialize and sign the transaction + const fileAppendTransactionBytes = fileAppendTx.toBytes(); + const user1Signatures = user1Key.signTransaction(fileAppendTx); + const user2Signatures = user2Key.signTransaction(fileAppendTx); + + // Deserialize the transaction and add signatures + const signedTransaction = Transaction.fromBytes( + fileAppendTransactionBytes, + ); + signedTransaction.addSignature(user1Key.publicKey, user1Signatures); + signedTransaction.addSignature(user2Key.publicKey, user2Signatures); + + // Execute the signed transaction + const result = await signedTransaction.execute(env.client); + const receipt = await result.getReceipt(env.client); + + expect(receipt.status).to.be.equal(Status.Success); + }); +}); diff --git a/test/unit/PrivateKey.js b/test/unit/PrivateKey.js new file mode 100644 index 000000000..31f2a7859 --- /dev/null +++ b/test/unit/PrivateKey.js @@ -0,0 +1,121 @@ +import { expect } from "chai"; +import sinon from "sinon"; + +import { PrivateKey } from "../../src/index.js"; +import Transaction from "../../src/transaction/Transaction.js"; + +describe("PrivateKey signTransaction", function () { + let privateKey, mockedTransaction, mockedSignature; + + beforeEach(() => { + privateKey = PrivateKey.generate(); + + mockedTransaction = sinon.createStubInstance(Transaction); + mockedSignature = new Uint8Array([4, 5, 6]); + + // Mock addSignature method on the transaction + mockedTransaction.addSignature = sinon.spy(); + }); + + it("should sign transaction and add signature", function () { + // Mock _signedTransactions.list to return an array with one signed transaction + mockedTransaction._signedTransactions = { + list: [{ bodyBytes: new Uint8Array([1, 2, 3]) }], + }; + + // Stub the _key.sign method to return a mock signature + privateKey._key = { + sign: sinon.stub().returns(mockedSignature), + }; + + // Call the real signTransaction method + const signatures = privateKey.signTransaction(mockedTransaction); + + // Validate that the signatures are correct + expect(signatures).to.deep.equal([mockedSignature]); + + sinon.assert.calledWith( + mockedTransaction.addSignature, + privateKey.publicKey, + [mockedSignature], + ); + + // Ensure that sign method of the privateKey._key was called + sinon.assert.calledOnce(privateKey._key.sign); + }); + + it("should return empty signature if bodyBytes are missing", function () { + // Set bodyBytes to null to simulate missing bodyBytes + mockedTransaction._signedTransactions = { + list: [{ bodyBytes: null }], + }; + + // Stub the _key.sign method to return a mock signature + privateKey._key = { + sign: sinon.stub().returns(mockedSignature), + }; + + // Call signTransaction method + const signatures = privateKey.signTransaction(mockedTransaction); + + // Validate that an empty Uint8Array was returned + expect(signatures).to.deep.equal([new Uint8Array()]); + + // Ensure that the transaction's addSignature method was called with the empty signature + sinon.assert.calledWith( + mockedTransaction.addSignature, + privateKey.publicKey, + [new Uint8Array()], + ); + + // Ensure that sign method of the privateKey._key was not called + sinon.assert.notCalled(privateKey._key.sign); + }); + + it("should sign transaction and add multiple signature", function () { + const mockedSignatures = [ + new Uint8Array([10, 11, 12]), + new Uint8Array([13, 14, 15]), + new Uint8Array([16, 17, 18]), + ]; + + const signedTransactions = [ + { bodyBytes: new Uint8Array([1, 2, 3]) }, + { bodyBytes: new Uint8Array([4, 5, 6]) }, + { bodyBytes: new Uint8Array([7, 8, 9]) }, + ]; + + // Mock _signedTransactions.list to return an array of transaction + mockedTransaction._signedTransactions = { + list: signedTransactions, + }; + + // Stub the _key.sign method to return a list of mock signature + privateKey._key = { + sign: sinon + .stub() + .onCall(0) + .returns(mockedSignatures[0]) + .onCall(1) + .returns(mockedSignatures[1]) + .onCall(2) + .returns(mockedSignatures[2]), + }; + + // Call the real signTransaction method + const signatures = privateKey.signTransaction(mockedTransaction); + + // Validate that the signatures are correct + expect(signatures).to.deep.equal(mockedSignatures); + + // Ensure that the transaction's addSignature method was called with the correct arguments + sinon.assert.calledWith( + mockedTransaction.addSignature, + privateKey.publicKey, + mockedSignatures, + ); + + // Ensure that sign method of the privateKey._key was called the correct number of times + sinon.assert.callCount(privateKey._key.sign, 3); + }); +}); diff --git a/test/unit/Transaction.js b/test/unit/Transaction.js index cddded978..648005461 100644 --- a/test/unit/Transaction.js +++ b/test/unit/Transaction.js @@ -10,12 +10,14 @@ import { Timestamp, Transaction, TransactionId, + PublicKey, } from "../../src/index.js"; import * as hex from "../../src/encoding/hex.js"; import Client from "../../src/client/NodeClient.js"; import * as HashgraphProto from "@hashgraph/proto"; import Long from "long"; import BigNumber from "bignumber.js"; +import sinon from "sinon"; describe("Transaction", function () { it("toBytes", async function () { @@ -313,4 +315,157 @@ describe("Transaction", function () { }); }); }); + + describe("addSignature tests", () => { + let transaction, mockedPublicKey, mockedSignature; + + beforeEach(() => { + transaction = new Transaction(); + mockedPublicKey = sinon.createStubInstance(PublicKey); + mockedSignature = new Uint8Array([4, 5, 6]); + + // Mock methods of PublicKey + mockedPublicKey.toBytesRaw.returns(new Uint8Array([1, 2, 3])); + mockedPublicKey._toProtobufSignature = sinon + .stub() + .returns({ ed25519: mockedSignature }); + + // Mock the necessary internals of the Transaction object + transaction._signedTransactions = { + length: 1, + list: [ + { + bodyBytes: new Uint8Array([1, 2, 3]), + sigMap: { sigPair: [] }, + }, + ], + get(index) { + return this.list[index]; + }, + }; + + transaction._transactionIds = { setLocked: sinon.spy() }; + transaction._nodeAccountIds = { setLocked: sinon.spy() }; + transaction._signedTransactions.setLocked = sinon.spy(); + + // Assume that the transaction is not frozen initially + transaction.isFrozen = sinon.stub().returns(false); + transaction.freeze = sinon.spy(); + }); + + it("should add a single signature when one transaction is present", () => { + transaction.addSignature(mockedPublicKey, mockedSignature); + + // Verify the signature was added correctly to sigMap.sigPair + expect( + transaction._signedTransactions.get(0).sigMap.sigPair, + ).to.deep.equal([{ ed25519: mockedSignature }]); + + // Verify freeze was called since the transaction was not frozen + sinon.assert.calledOnce(transaction.freeze); + }); + + it("should throw an error when adding a single signature to multiple transactions", () => { + transaction._signedTransactions.length = 2; + transaction._signedTransactions.list = [ + { + bodyBytes: new Uint8Array([1, 2, 3]), + sigMap: { sigPair: [] }, + }, + { + bodyBytes: new Uint8Array([4, 5, 6]), + sigMap: { sigPair: [] }, + }, + ]; + + expect(() => { + transaction.addSignature(mockedPublicKey, mockedSignature); + }).to.throw( + "Signature array must match the number of transactions", + ); + }); + + it("should add multiple signatures corresponding to each transaction", () => { + const mockedSignatures = [ + new Uint8Array([10, 11, 12]), + new Uint8Array([13, 14, 15]), + ]; + + transaction._signedTransactions.length = 2; + transaction._signedTransactions.list = [ + { + bodyBytes: new Uint8Array([1, 2, 3]), + sigMap: { sigPair: [] }, + }, + { + bodyBytes: new Uint8Array([4, 5, 6]), + sigMap: { sigPair: [] }, + }, + ]; + + // Update stub to return correct signatures for each call + mockedPublicKey._toProtobufSignature + .onCall(0) + .returns({ ed25519: mockedSignatures[0] }) + .onCall(1) + .returns({ ed25519: mockedSignatures[1] }); + + transaction.addSignature(mockedPublicKey, mockedSignatures); + + // Verify the signatures were added correctly + expect( + transaction._signedTransactions.get(0).sigMap.sigPair, + ).to.deep.equal([{ ed25519: mockedSignatures[0] }]); + expect( + transaction._signedTransactions.get(1).sigMap.sigPair, + ).to.deep.equal([{ ed25519: mockedSignatures[1] }]); + + // Validate the calls to _toProtobufSignature + sinon.assert.calledWith( + mockedPublicKey._toProtobufSignature, + mockedSignatures[0], + ); + sinon.assert.calledWith( + mockedPublicKey._toProtobufSignature, + mockedSignatures[1], + ); + }); + + it("should throw an error when signature array length doesn't match the number of transactions", () => { + const mismatchedSignatures = [new Uint8Array([10, 11, 12])]; + transaction._signedTransactions.length = 2; + transaction._signedTransactions.list = [ + { + bodyBytes: new Uint8Array([1, 2, 3]), + sigMap: { sigPair: [] }, + }, + { + bodyBytes: new Uint8Array([4, 5, 6]), + sigMap: { sigPair: [] }, + }, + ]; + + expect(() => { + transaction.addSignature(mockedPublicKey, mismatchedSignatures); + }).to.throw( + "Signature array must match the number of transactions", + ); + }); + + it("should freeze the transaction if it is not frozen", () => { + transaction.isFrozen.returns(false); + + transaction._signedTransactions.length = 1; + transaction._signedTransactions.list = [ + { + bodyBytes: new Uint8Array([1, 2, 3]), + sigMap: { sigPair: [] }, + }, + ]; + + transaction.addSignature(mockedPublicKey, mockedSignature); + + sinon.assert.calledOnce(transaction.freeze); + }); + }); });