refactor: getSignatureHash renamed to getTransactionHash since th…

…at is what we are hashing (#1404) * refactor: first commit * refactor: renamed also for unit tests
vechain · Oct 9, 2024 · 7ea84ca · 7ea84ca · github-actions · Oct 9, 2024
1 parent 9a56b0e
commit 7ea84ca
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 38 deletions.
diff --git a/docs/diagrams/architecture/transaction.md b/docs/diagrams/architecture/transaction.md
@@ -34,7 +34,7 @@ classDiagram
         +boolean isSigned
         +Address origin
         +Transaction decode(Uint8Array rawTransaction, boolean isSigned)$
-        +Blake2b256 getSignatureHash(Address delegator?)
+        +Blake2b256 getTransactionHash(Address delegator?)
         +VTHO intrinsicGas(TransactionClause[] clauses)$
         +boolean isValidBody(TransactionBody body)$
         +Transaction of(TransactionBody: body, Uint8Array signature?)$

diff --git a/packages/core/src/transaction/Transaction.ts b/packages/core/src/transaction/Transaction.ts
@@ -183,7 +183,7 @@ class Transaction {
      * @remarks Security auditable method, depends on
      * - {@link Address.ofPublicKey};
      * - {@link Secp256k1.recover};
-     * - {@link Transaction.getSignatureHash}.
+     * - {@link Transaction.getTransactionHash}.
      */
     public get delegator(): Address {
         if (this.isDelegated) {
@@ -195,7 +195,7 @@ class Transaction {
                 );
                 // Recover the delegator's public key.
                 const delegatorPublicKey = Secp256k1.recover(
-                    this.getSignatureHash(this.origin).bytes,
+                    this.getTransactionHash(this.origin).bytes,
                     delegator
                 );
                 return Address.ofPublicKey(delegatorPublicKey);
@@ -244,7 +244,7 @@ class Transaction {
         if (this.isSigned) {
             return Blake2b256.of(
                 nc_utils.concatBytes(
-                    this.getSignatureHash().bytes,
+                    this.getTransactionHash().bytes,
                     this.origin.bytes
                 )
             );
@@ -301,7 +301,7 @@ class Transaction {
             return Address.ofPublicKey(
                 // Get the origin public key.
                 Secp256k1.recover(
-                    this.getSignatureHash().bytes,
+                    this.getTransactionHash().bytes,
                     // Get the (r, s) of ECDSA digital signature without delegator params.
                     this.signature.slice(0, Secp256k1.SIGNATURE_LENGTH)
                 )
@@ -370,18 +370,18 @@ class Transaction {
     }
 
     /**
-     * Computes the signature hash, optionally incorporating a delegator's address.
+     * Computes the transaction hash, optionally incorporating a delegator's address.
      *
      * @param {Address} [delegator] - Optional delegator's address to include in the hash computation.
-     * @return {Blake2b256} - The computed signature hash.
+     * @return {Blake2b256} - The computed transaction hash.
      *
      * @remarks
      * `delegator` is used to sign a transaction on behalf of another account.
      *
      * @remarks Security auditable method, depends on
      * - {@link Blake2b256.of}.
      */
-    public getSignatureHash(delegator?: Address): Blake2b256 {
+    public getTransactionHash(delegator?: Address): Blake2b256 {
         const txHash = Blake2b256.of(this.encode(false));
         if (delegator !== undefined) {
             return Blake2b256.of(
@@ -518,7 +518,7 @@ class Transaction {
             if (!this.isDelegated) {
                 // Sign transaction
                 const signature = Secp256k1.sign(
-                    this.getSignatureHash().bytes,
+                    this.getTransactionHash().bytes,
                     signerPrivateKey
                 );
                 // Return new signed transaction.
@@ -561,8 +561,8 @@ class Transaction {
             // Check if the private key of the delegator is valid.
             if (Secp256k1.isValidPrivateKey(delegatorPrivateKey)) {
                 if (this.isDelegated) {
-                    const transactionHash = this.getSignatureHash().bytes;
-                    const delegatedHash = this.getSignatureHash(
+                    const transactionHash = this.getTransactionHash().bytes;
+                    const delegatedHash = this.getTransactionHash(
                         Address.ofPublicKey(
                             Secp256k1.derivePublicKey(signerPrivateKey)
                         )

diff --git a/packages/core/tests/transaction/Transaction.unit.test.ts b/packages/core/tests/transaction/Transaction.unit.test.ts
@@ -1,4 +1,12 @@
 import { describe, expect } from '@jest/globals';
+import {
+    InvalidDataType,
+    InvalidSecp256k1PrivateKey,
+    InvalidSecp256k1Signature,
+    InvalidTransactionField,
+    NotDelegatedTransaction,
+    UnavailableTransactionField
+} from '@vechain/sdk-errors';
 import {
     Address,
     HexUInt,
@@ -9,14 +17,6 @@ import {
     Units,
     VTHO
 } from '../../src';
-import {
-    InvalidDataType,
-    InvalidSecp256k1PrivateKey,
-    InvalidSecp256k1Signature,
-    InvalidTransactionField,
-    NotDelegatedTransaction,
-    UnavailableTransactionField
-} from '@vechain/sdk-errors';
 
 const DelegatorPrivateKeyFix = HexUInt.of(
     '40de805e918403683fb9a6081c3fba072cdc5c88232c62a9509165122488dab7'
@@ -68,7 +68,7 @@ const TransactionFixture = {
     },
     undelegated: {
         body: TxBodyFix,
-        signatureHash: HexUInt.of(
+        transactionHash: HexUInt.of(
             '0x2a1c25ce0d66f45276a5f308b99bf410e2fc7d5b6ea37a49f2ab9f1da9446478'
         ),
         signedTransactionId: HexUInt.of(
@@ -89,7 +89,7 @@ const TransactionFixture = {
                 features: 1
             }
         },
-        signatureHash: HexUInt.of(
+        transactionHash: HexUInt.of(
             '0x005fb0b47dfd16b7f2f61bb17df791242bc37ed1fffe9b05fa55fb0fe069f9a3'
         ),
         encodedUnsigned: HexUInt.of(
@@ -116,7 +116,7 @@ const TransactionFixture = {
                 ]
             }
         },
-        signatureHash: HexUInt.of(
+        transactionHash: HexUInt.of(
             '0xd6e8f162e3e08585ee8fcf81868e5bd57a59966fef218528339766ee2587726c'
         ),
         encodedUnsigned: HexUInt.of(
@@ -148,8 +148,8 @@ describe('Transaction class tests', () => {
                 expect(actual.isDelegated).toBe(false);
                 expect(
                     actual
-                        .getSignatureHash()
-                        .isEqual(TransactionFixture.undelegated.signatureHash)
+                        .getTransactionHash()
+                        .isEqual(TransactionFixture.undelegated.transactionHash)
                 ).toBe(true);
                 expect(() => actual.id).toThrowError(
                     UnavailableTransactionField
@@ -173,7 +173,9 @@ describe('Transaction class tests', () => {
                 expect(actual.isSigned).toBe(false);
                 expect(actual.isDelegated).toEqual(true);
                 expect(
-                    actual.getSignatureHash().isEqual(expected.signatureHash)
+                    actual
+                        .getTransactionHash()
+                        .isEqual(expected.transactionHash)
                 ).toBe(true);
                 expect(() => actual.id).toThrowError(
                     UnavailableTransactionField
@@ -197,7 +199,9 @@ describe('Transaction class tests', () => {
                 expect(actual.isSigned).toBe(false);
                 expect(actual.isDelegated).toEqual(true);
                 expect(
-                    actual.getSignatureHash().isEqual(expected.signatureHash)
+                    actual
+                        .getTransactionHash()
+                        .isEqual(expected.transactionHash)
                 ).toBe(true);
                 expect(() => actual.id).toThrowError(
                     UnavailableTransactionField
@@ -229,7 +233,9 @@ describe('Transaction class tests', () => {
                 expect(actual.isSigned).toEqual(true);
                 expect(actual.isDelegated).toEqual(false);
                 expect(
-                    actual.getSignatureHash().isEqual(expected.signatureHash)
+                    actual
+                        .getTransactionHash()
+                        .isEqual(expected.transactionHash)
                 ).toBe(true);
                 expect(actual.origin.isEqual(SignerFix.address)).toBe(true);
                 expect(() => actual.delegator).toThrowError(
@@ -255,7 +261,9 @@ describe('Transaction class tests', () => {
                 expect(actual.isSigned).toEqual(true);
                 expect(actual.isDelegated).toEqual(true);
                 expect(
-                    actual.getSignatureHash().isEqual(expected.signatureHash)
+                    actual
+                        .getTransactionHash()
+                        .isEqual(expected.transactionHash)
                 ).toBe(true);
                 expect(actual.origin.isEqual(SignerFix.address)).toBe(true);
                 expect(actual.delegator.isEqual(DelegatorFix.address)).toBe(
@@ -281,7 +289,9 @@ describe('Transaction class tests', () => {
                 expect(actual.isSigned).toEqual(true);
                 expect(actual.isDelegated).toEqual(true);
                 expect(
-                    actual.getSignatureHash().isEqual(expected.signatureHash)
+                    actual
+                        .getTransactionHash()
+                        .isEqual(expected.transactionHash)
                 ).toBe(true);
                 expect(actual.origin.isEqual(SignerFix.address)).toBe(true);
                 expect(actual.delegator.isEqual(DelegatorFix.address)).toBe(
@@ -337,7 +347,7 @@ describe('Transaction class tests', () => {
                     UnavailableTransactionField
                 );
                 expect(actual.isSigned).toBe(false);
-                expect(actual.getSignatureHash()).toBeDefined();
+                expect(actual.getTransactionHash()).toBeDefined();
                 expect(actual.encoded).toBeDefined();
                 expect(actual.encoded).toEqual(expected.encodedUnsigned);
             });
@@ -355,7 +365,7 @@ describe('Transaction class tests', () => {
                 expect(actual.isDelegated).toBe(false);
                 expect(actual.id).toBeDefined();
                 expect(actual.isSigned).toBe(true);
-                expect(actual.getSignatureHash()).toBeDefined();
+                expect(actual.getTransactionHash()).toBeDefined();
                 expect(actual.encoded).toBeDefined();
                 expect(actual.encoded).toEqual(expected.encodedSigned);
             });
@@ -382,8 +392,8 @@ describe('Transaction class tests', () => {
                     UnavailableTransactionField
                 );
                 expect(actual.isSigned).toBe(false);
-                expect(actual.getSignatureHash()).toBeDefined();
-                expect(actual.getSignatureHash().bytes.length).toBe(32);
+                expect(actual.getTransactionHash()).toBeDefined();
+                expect(actual.getTransactionHash().bytes.length).toBe(32);
                 expect(actual.encoded).toBeDefined();
                 expect(actual.encoded).toEqual(expected.encodedUnsigned);
             });
@@ -403,7 +413,7 @@ describe('Transaction class tests', () => {
                 expect(actual.isDelegated).toBe(true);
                 expect(actual.id).toBeDefined();
                 expect(actual.isSigned).toBe(true);
-                expect(actual.getSignatureHash()).toBeDefined();
+                expect(actual.getTransactionHash()).toBeDefined();
                 expect(actual.encoded).toBeDefined();
                 expect(actual.encoded).toEqual(expected.encodedSigned);
                 expect(actual.signature).toBeDefined();
@@ -434,8 +444,8 @@ describe('Transaction class tests', () => {
                     UnavailableTransactionField
                 );
                 expect(actual.isSigned).toBe(false);
-                expect(actual.getSignatureHash()).toBeDefined();
-                expect(actual.getSignatureHash().bytes.length).toBe(32);
+                expect(actual.getTransactionHash()).toBeDefined();
+                expect(actual.getTransactionHash().bytes.length).toBe(32);
                 expect(actual.encoded).toBeDefined();
                 expect(actual.encoded).toEqual(expected.encodedUnsigned);
             });
@@ -455,7 +465,7 @@ describe('Transaction class tests', () => {
                 expect(actual.isDelegated).toBe(true);
                 expect(actual.id).toBeDefined();
                 expect(actual.isSigned).toBe(true);
-                expect(actual.getSignatureHash()).toBeDefined();
+                expect(actual.getTransactionHash()).toBeDefined();
                 expect(actual.encoded).toBeDefined();
                 expect(actual.encoded).toEqual(expected.encodedSigned);
                 expect(actual.signature).toBeDefined();

diff --git a/packages/network/src/signer/signers/vechain-private-key-signer/vechain-private-key-signer.ts b/packages/network/src/signer/signers/vechain-private-key-signer/vechain-private-key-signer.ts
@@ -314,7 +314,7 @@ class VeChainPrivateKeySigner extends VeChainAbstractSigner {
 
         // Sign transaction with origin private key
         const originSignature = Secp256k1.sign(
-            unsignedTx.getSignatureHash().bytes,
+            unsignedTx.getTransactionHash().bytes,
             originPrivateKey
         );
Title	Tests	Time
core	799	1m 45s ⏱️
network	735	4m 42s ⏱️
errors	42	15.656s ⏱️