Correct return signed vs unsigned serialized data

dominant-strategies · Jun 7, 2024 · bfa7f50 · bfa7f50
1 parent 7da18f1
commit bfa7f50
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 50 deletions.
diff --git a/src/transaction/abstract-transaction.ts b/src/transaction/abstract-transaction.ts
@@ -1,4 +1,4 @@
-import { Signature } from '../crypto/index.js';
+import { keccak256, Signature } from '../crypto/index.js';
 import { getBigInt, assert, assertArgument } from '../utils/index.js';
 
 import type { BigNumberish } from '../utils/index.js';
@@ -281,8 +281,8 @@ export abstract class AbstractTransaction<S extends allowedSignatureTypes> imple
      *
      * This is the digest that a [Signer](../interfaces/Signer) must sign to authorize this transaction.
      */
-    get unsignedHash(): string {
-        throw new Error('Method not implemented.');
+    get digest(): string {
+        return keccak256(this.unsignedSerialized);
     }
 
     /**
@@ -315,7 +315,7 @@ export abstract class AbstractTransaction<S extends allowedSignatureTypes> imple
             'UNSUPPORTED_OPERATION',
             { operation: '.serialized' },
         );
-        return this.#serialize();
+        return encodeProtoTransaction(this.toProtobuf(true));
     }
 
     /**
@@ -324,7 +324,7 @@ export abstract class AbstractTransaction<S extends allowedSignatureTypes> imple
      * The hash of this is the digest which needs to be signed to authorize this transaction.
      */
     get unsignedSerialized(): string {
-        return this.#serialize();
+        return encodeProtoTransaction(this.toProtobuf(false));
     }
 
     /**
@@ -362,7 +362,7 @@ export abstract class AbstractTransaction<S extends allowedSignatureTypes> imple
      *
      * @returns {ProtoTransaction} The protobuf-friendly JSON object.
      */
-    abstract toProtobuf(): ProtoTransaction;
+    abstract toProtobuf(includeSignature: boolean): ProtoTransaction;
 
     abstract get originZone(): Zone | undefined;
 
@@ -371,13 +371,4 @@ export abstract class AbstractTransaction<S extends allowedSignatureTypes> imple
     get isExternal(): boolean {
         return this.destZone !== undefined && this.originZone !== this.destZone;
     }
-
-    /**
-     * Serializes the WorkObject to a string.
-     *
-     * @returns {string} The serialized WorkObject.
-     */
-    #serialize(): string {
-        return encodeProtoTransaction(this.toProtobuf());
-    }
 }
diff --git a/src/transaction/qi-transaction.ts b/src/transaction/qi-transaction.ts
@@ -1,15 +1,10 @@
-import {keccak256} from "../crypto/index.js";
-import {AbstractTransaction, TransactionLike, TxInput, TxOutput} from "./index.js";
-import {
-    assertArgument,
-    getBytes, getZoneForAddress,
-    hexlify, isQiAddress,
-    toBigInt
-} from "../utils/index.js";
+import { keccak256 } from '../crypto/index.js';
+import { AbstractTransaction, TransactionLike, TxInput, TxOutput } from './index.js';
+import { assertArgument, getBytes, getZoneForAddress, hexlify, isQiAddress, toBigInt } from '../utils/index.js';
 import { decodeProtoTransaction } from '../encoding/index.js';
-import {formatNumber} from "../providers/format.js";
-import { computeAddress } from "../address/index.js";
-import { ProtoTransaction} from "./abstract-transaction.js";
+import { formatNumber } from '../providers/format.js';
+import { computeAddress } from '../address/index.js';
+import { ProtoTransaction } from './abstract-transaction.js';
 import { Zone } from '../constants/index.js';
 
 /**
@@ -57,28 +52,30 @@ export class QiTransaction extends AbstractTransaction<string> implements QiTran
         this.#txOutputs = value.map((output) => ({ ...output }));
     }
 
+    /**
+     * The permuted hash of the transaction as specified by
+     * [QIP-0010](https://github.com/quai-network/qips/blob/master/qip-0010.md).
+     */
     get hash(): null | string {
         if (this.signature == null) {
             return null;
         }
-        return this.unsignedHash;
-    }
-    get unsignedHash(): string {
+
         if (this.txInputs.length < 1 || this.txOutputs.length < 1) {
             throw new Error('Transaction must have at least one input and one output');
         }
 
-        const destUtxo = isQiAddress(hexlify(this.txOutputs[0].address) || '');
         const pubKey = hexlify(this.txInputs[0].pub_key);
         const senderAddr = computeAddress(pubKey || '');
-        const originUtxo = isQiAddress(senderAddr);
 
         if (!this.destZone || !this.originZone) {
             throw new Error(
                 `Invalid zones: origin ${this.originZone} ->  destination ${this.destZone} (address: ${senderAddr})`,
             );
         }
-        if (this.isExternal && destUtxo !== originUtxo) {
+
+        const isSameLedger = isQiAddress(senderAddr) === isQiAddress(hexlify(this.txOutputs[0].address) || '');
+        if (this.isExternal && !isSameLedger) {
             throw new Error('Cross-zone & cross-ledger transactions are not supported');
         }
 
@@ -97,6 +94,9 @@ export class QiTransaction extends AbstractTransaction<string> implements QiTran
         return '0x' + hashBuffer.toString('hex');
     }
 
+    /**
+     * The zone of the sender address
+     */
     get originZone(): Zone | undefined {
         const pubKey = hexlify(this.txInputs[0].pub_key);
         const senderAddr = computeAddress(pubKey || '');
@@ -105,6 +105,9 @@ export class QiTransaction extends AbstractTransaction<string> implements QiTran
         return zone ?? undefined;
     }
 
+    /**
+     * The zone of the recipient address
+     */
     get destZone(): Zone | undefined {
         const zone = getZoneForAddress(hexlify(this.txOutputs[0].address) || '');
         return zone ?? undefined;
@@ -176,15 +179,15 @@ export class QiTransaction extends AbstractTransaction<string> implements QiTran
      *
      * @returns {ProtoTransaction} The protobuf-friendly JSON object.
      */
-    toProtobuf(): ProtoTransaction {
+    toProtobuf(includeSignature: boolean = true): ProtoTransaction {
         const protoTx: ProtoTransaction = {
             type: this.type || 2,
             chain_id: formatNumber(this.chainId || 0, 'chainId'),
             tx_ins: { tx_ins: this.txInputs },
             tx_outs: { tx_outs: this.txOutputs },
         };
 
-        if (this.signature) {
+        if (this.signature && includeSignature) {
             protoTx.signature = getBytes(this.signature);
         }
 
@@ -237,7 +240,6 @@ export class QiTransaction extends AbstractTransaction<string> implements QiTran
      * @returns {QiTransaction} The decoded transaction.
      */
     static fromProto(protoTx: ProtoTransaction): QiTransaction {
-        //  TODO: Fix this because new tx instance requires a 'from' address
         const tx = new QiTransaction();
 
         tx.type = protoTx.type;

diff --git a/src/transaction/quai-transaction.ts b/src/transaction/quai-transaction.ts
@@ -120,21 +120,19 @@ export class QuaiTransaction extends AbstractTransaction<Signature> implements Q
         this.#to = value;
     }
 
+    /**
+     * The permuted hash of the transaction as specified by
+     * [QIP-0010](https://github.com/quai-network/qips/blob/master/qip-0010.md).
+     */
     get hash(): null | string {
-        if (this.signature == null) {
-            return null;
-        }
-        return this.unsignedHash;
-    }
-
-    get unsignedHash(): string {
-        const destUtxo = isQiAddress(this.to || '');
-        const originUtxo = isQiAddress(this.from);
+        if (this.signature == null) return null;
 
         if (!this.originZone) {
             throw new Error('Invalid Zone for from address');
         }
-        if (this.isExternal && destUtxo !== originUtxo) {
+
+        const isSameLedger = isQiAddress(this.from) === isQiAddress(this.to || '');
+        if (this.isExternal && !isSameLedger) {
             throw new Error('Cross-zone & cross-ledger transactions are not supported');
         }
 
@@ -153,13 +151,17 @@ export class QuaiTransaction extends AbstractTransaction<Signature> implements Q
         return '0x' + hashBuffer.toString('hex');
     }
 
+    /**
+     * The zone of the sender address
+     */
     get originZone(): Zone | undefined {
-        const senderAddr = this.from;
-
-        const zone = getZoneForAddress(senderAddr);
+        const zone = getZoneForAddress(this.from);
         return zone ?? undefined;
     }
 
+    /**
+     * The zone of the recipient address
+     */
     get destZone(): Zone | undefined {
         const zone = this.to !== null ? getZoneForAddress(this.to || '') : undefined;
         return zone ?? undefined;
@@ -361,7 +363,7 @@ export class QuaiTransaction extends AbstractTransaction<Signature> implements Q
      *
      * @returns {ProtoTransaction} The protobuf-friendly JSON object.
      */
-    toProtobuf(): ProtoTransaction {
+    toProtobuf(includeSignature: boolean = true): ProtoTransaction {
         const protoTx: ProtoTransaction = {
             type: this.type || 0,
             chain_id: formatNumber(this.chainId || 0, 'chainId'),
@@ -375,7 +377,7 @@ export class QuaiTransaction extends AbstractTransaction<Signature> implements Q
             access_list: { access_tuples: [] },
         };
 
-        if (this.signature) {
+        if (this.signature && includeSignature) {
             protoTx.v = formatNumber(this.signature.yParity, 'yParity');
             protoTx.r = toBeArray(this.signature.r);
             protoTx.s = toBeArray(this.signature.s);