feat: Add multi-signature and multi-node support for signing and addi… (

#2514) * feat: Add multi-signature and multi-node support for signing and adding signatures Signed-off-by: ivaylogarnev-limechain <[email protected]> * test: Added signTransaction method tests Signed-off-by: ivaylogarnev-limechain <[email protected]> * test: Added some integration tests for signTransaction method Signed-off-by: ivaylogarnev-limechain <[email protected]> --------- Signed-off-by: ivaylogarnev-limechain <[email protected]>
hiero-ledger · Sep 18, 2024 · cd14715 · cd14715
1 parent ba47014
commit cd14715
Show file tree

Hide file tree

Showing 5 changed files with 443 additions and 27 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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);
+    });
+});