feat: Changed the return type of removeAllSignatures method (#2559)

* feat: Changed the return type of removeAllSignatures method of Transaction class and fixed unit and integration related tests Signed-off-by: ivaylogarnev-limechain <[email protected]> * fix: Fixed some CI comments for typezation in collectSignaturesByPublicKey method Signed-off-by: ivaylogarnev-limechain <[email protected]> * tests: Added tests for early return checks in _coolectSignaturesByPublicKey method Signed-off-by: ivaylogarnev-limechain <[email protected]> --------- Signed-off-by: ivaylogarnev-limechain <[email protected]> Signed-off-by: Ivaylo Nikolov <[email protected]> Co-authored-by: Ivaylo Nikolov <[email protected]>
hashgraph · Oct 10, 2024 · 10dfd3f · 10dfd3f
1 parent 6b3b643
commit 10dfd3f
Show file tree

Hide file tree

Showing 3 changed files with 87 additions and 59 deletions.
diff --git a/examples/multi-node-multi-signature-removeAll.js b/examples/multi-node-multi-signature-removeAll.js
@@ -7,7 +7,6 @@ import {
  KeyList,
  TransferTransaction,
  Transaction,
- PublicKey,
 } from "@hashgraph/sdk";
 
 import dotenv from "dotenv";
@@ -146,34 +145,20 @@ async function main() {
  signaturesInTheTransactionAfter,
  );
 
- /**
- * Print the signatures in DER format
- * @param {Uint8Array[]} signatures - The signatures to print.
- * @returns {void} An array of signatures in DER format.
- */
- const printSignatures = (signatures) => {
- return signatures.forEach((sig) => {
- console.log(PrivateKey.fromBytes(sig).toStringDer());
- });
- };
-
- for (const [publicKey, signatures] of Object.entries(
- allSignaturesRemoved,
- )) {
- // Show the signatures for Alice & Bob
- console.log(`\nRemoved signatures for ${publicKey}:`);
-
+ for (const [publicKey, signatures] of allSignaturesRemoved) {
+ // Show the signatures for each publicKey
+ console.log(`\nRemoved signatures for ${publicKey.toStringDer()}:`);
  if (Array.isArray(signatures)) {
- printSignatures(signatures);
+ console.log(
+ signatures.map((sig) => {
+ return PrivateKey.fromBytes(sig).toStringDer();
+ }),
+ );
  }
 
  // Add the removed signatures back
- signedTransaction.addSignature(
- PublicKey.fromString(publicKey),
- signatures,
- );
+ signedTransaction.addSignature(publicKey, signatures);
  }
-
  /**
  *
  * Step 8: Execute and take the receipt

diff --git a/src/transaction/Transaction.js b/src/transaction/Transaction.js
@@ -930,7 +930,7 @@ export default class Transaction extends Executable {
  * It will call collectSignatures to get the removed signatures, then clear all signatures
  * from the internal tracking.
  *
- * @returns {{ [userPublicKey: string]: Uint8Array[] | Uint8Array }} The removed signatures in the specified format.
+ * @returns { Map<PublicKey, Uint8Array[] | Uint8Array> } The removed signatures in the specified format.
  */
  removeAllSignatures() {
  if (!this.isFrozen()) {
@@ -1904,37 +1904,51 @@ export default class Transaction extends Executable {
  };
 
  /**
- * Collects all signatures from signed transactions and returns them in a format keyed by public key.
+ * Collects all signatures from signed transactions and returns them in a format keyed by PublicKey.
  *
- * @returns {{ [publicKey: PublicKey]: Uint8Array[] }} The collected signatures keyed by public key.
+ * @returns { Map<PublicKey, Uint8Array[]> } The collected signatures keyed by PublicKey.
  */
  _collectSignaturesByPublicKey() {
- /** @type {{ [publicKey: string]: Uint8Array[] }} */
- const collectedSignatures = {};
+ /** @type { Map<PublicKey, Uint8Array[]>} */
+ const collectedSignatures = new Map();
+ /** @type { Record<string, PublicKey> } */
+ const publicKeyMap = {}; // Map to hold string representation of the PublicKey object
 
  // Iterate over the signed transactions and collect signatures
  for (const transaction of this._signedTransactions.list) {
  if (!(transaction.sigMap && transaction.sigMap.sigPair)) {
- return [];
+ return new Map();
  }
 
  // Collect the signatures
  for (const sigPair of transaction.sigMap.sigPair) {
  const signature = sigPair.ed25519 ?? sigPair.ECDSASecp256k1;
 
  if (!signature || !sigPair.pubKeyPrefix) {
- return [];
+ return new Map();
  }
 
- const publicKeyHex = hex.encode(sigPair.pubKeyPrefix);
+ const publicKeyStr = hex.encode(sigPair.pubKeyPrefix);
+ let publicKeyObj = publicKeyMap[publicKeyStr];
+
+ // If the PublicKey instance for this string representation doesn't exist, create and store it
+ if (!publicKeyObj) {
+ publicKeyObj = PublicKey.fromString(publicKeyStr);
+ publicKeyMap[publicKeyStr] = publicKeyObj;
+ }
 
  // Initialize the structure for this publicKey if it doesn't exist
- if (!collectedSignatures[publicKeyHex]) {
- collectedSignatures[publicKeyHex] = [];
+ if (!collectedSignatures.has(publicKeyObj)) {
+ collectedSignatures.set(publicKeyObj, []);
  }
 
+ const existingSignatures =
+ collectedSignatures.get(publicKeyObj);
+
  // Add the signature to the corresponding public key
- collectedSignatures[publicKeyHex].push(signature);
+ if (existingSignatures) {
+ existingSignatures.push(signature);
+ }
  }
  }
 

diff --git a/test/unit/Transaction.js b/test/unit/Transaction.js
@@ -641,44 +641,57 @@ describe("Transaction", function () {
  expect(removedSignatures).to.include.members([signature1]);
  });
 
- it("should return all removed signatures in the expected object format when clearing all signatures", function () {
+ it("should return all removed signatures in the expected Map format when clearing all signatures", function () {
  // Sign the transaction with multiple keys
  signAndAddSignatures(transaction, key1, key2, key3);
 
- // Clear all signatures and capture the returned value
+ // Clear all signatures and capture the returned Map
  const removedSignatures = transaction.removeAllSignatures();
 
  // Check the format of the returned value
- expect(removedSignatures).to.be.an("object");
+ expect(removedSignatures).to.be.instanceOf(Map);
 
- // Ensure the object has the correct structure
- expect(removedSignatures).to.have.property(
- key1.publicKey.toStringRaw(),
+ // Check if the Map contains keys using the toString() method
+ const key1Exists = Array.from(removedSignatures.keys()).some(
+ (key) => key.toString() === key1.publicKey.toString(),
  );
- expect(removedSignatures[key1.publicKey.toStringRaw()]).to.be.an(
- "array",
+ const key2Exists = Array.from(removedSignatures.keys()).some(
+ (key) => key.toString() === key2.publicKey.toString(),
  );
- expect(removedSignatures).to.have.property(
- key2.publicKey.toStringRaw(),
+ const key3Exists = Array.from(removedSignatures.keys()).some(
+ (key) => key.toString() === key3.publicKey.toString(),
  );
- expect(removedSignatures[key2.publicKey.toStringRaw()]).to.be.an(
- "array",
+
+ // Assert that all keys exist
+ expect(key1Exists).to.be.true;
+ expect(key2Exists).to.be.true;
+ expect(key3Exists).to.be.true;
+
+ // Retrieve values using the keys and convert them to strings for comparison
+ const signaturesArray1 = removedSignatures.get(
+ Array.from(removedSignatures.keys()).find(
+ (key) => key.toString() === key1.publicKey.toString(),
+ ),
  );
- expect(removedSignatures).to.have.property(
- key3.publicKey.toStringRaw(),
+
+ const signaturesArray2 = removedSignatures.get(
+ Array.from(removedSignatures.keys()).find(
+ (key) => key.toString() === key2.publicKey.toString(),
+ ),
  );
- expect(removedSignatures[key3.publicKey.toStringRaw()]).to.be.an(
- "array",
+
+ const signaturesArray3 = removedSignatures.get(
+ Array.from(removedSignatures.keys()).find(
+ (key) => key.toString() === key3.publicKey.toString(),
+ ),
  );
 
- // Ensure the removed signatures are in the expected format
- const signaturesArray1 =
- removedSignatures[key1.publicKey.toStringRaw()];
- const signaturesArray2 =
- removedSignatures[key2.publicKey.toStringRaw()];
- const signaturesArray3 =
- removedSignatures[key3.publicKey.toStringRaw()];
+ // Validate that the retrieved arrays are present
+ expect(signaturesArray1).to.be.an("array").that.is.not.empty;
+ expect(signaturesArray2).to.be.an("array").that.is.not.empty;
+ expect(signaturesArray3).to.be.an("array").that.is.not.empty;
 
+ // Ensure the removed signatures are in the expected format
  [signaturesArray1, signaturesArray2, signaturesArray3].forEach(
  (signaturesArray) => {
  signaturesArray.forEach((sig) => {
@@ -695,8 +708,24 @@ describe("Transaction", function () {
  const removedSignatures = transaction.removeAllSignatures();
 
  // Check the format of the returned value
- expect(removedSignatures).to.be.an("object");
+ expect(removedSignatures).to.be.instanceOf(Map);
  expect(Object.keys(removedSignatures)).to.have.lengthOf(0);
  });
+
+ it("should return an empty Map if transaction.sigMap is undefined", function () {
+ transaction._signedTransactions.list = [1];
+
+ const result = transaction.removeAllSignatures();
+ expect(result).to.be.instanceOf(Map);
+ expect(result.size).to.equal(0);
+ });
+
+ it("should return an empty Map if sigPair.pubKeyPrefix is undefined", function () {
+ transaction._signedTransactions.list[0].sigMap = { sigPair: [1] };
+
+ const result = transaction.removeAllSignatures();
+ expect(result).to.be.instanceOf(Map);
+ expect(result.size).to.equal(0);
+ });
  });
 });