Merge pull request #6447 from BitGo/coin-4819

fix(sdk-coin-vet): fix deserialization in vet sdk

Author: at31416

modules/sdk-coin-vet/src/lib/transaction/transaction.ts

@@ -1,11 +1,5 @@
 import BigNumber from 'bignumber.js';
-import {
-  BaseTransaction,
-  TransactionType,
-  InvalidTransactionError,
-  TransactionRecipient,
-  PublicKey,
-} from '@bitgo/sdk-core';
+import { BaseTransaction, TransactionType, InvalidTransactionError, TransactionRecipient } from '@bitgo/sdk-core';
 import { BaseCoin as CoinConfig } from '@bitgo/statics';
 import {
   TransactionBody,
@@ -35,13 +29,9 @@ export class Transaction extends BaseTransaction {
   private _dependsOn: string | null;
   private _nonce: number;
   private _sender: string;
-  private _senderSignature: Buffer;
+  private _senderSignature: Buffer | null;
   private _feePayerAddress: string;
-  private _feePayerSignature: Buffer;
-  private _feePayerPubKey: PublicKey;
-
-  static EMPTY_PUBLIC_KEY = Buffer.alloc(32);
-  static EMPTY_SIGNATURE = Buffer.alloc(64);
+  private _feePayerSignature: Buffer | null;
 
   constructor(_coinConfig: Readonly<CoinConfig>) {
     super(_coinConfig);
@@ -55,15 +45,12 @@ export class Transaction extends BaseTransaction {
     this._dependsOn = null;
     this._nonce = 0x0;
     this._recipients = [];
-    this._feePayerPubKey = {
-      pub: HexUInt.of(Transaction.EMPTY_PUBLIC_KEY).toString(),
-    };
-    this._senderSignature = Transaction.EMPTY_SIGNATURE;
-    this._feePayerSignature = Transaction.EMPTY_SIGNATURE;
+    this._senderSignature = null;
+    this._feePayerSignature = null;
   }
 
   public get id(): string {
-    this.generateTxnId();
+    this.generateTxnIdAndSetSender();
     return this._id ?? 'UNAVAILABLE';
   }
 
@@ -92,7 +79,11 @@ export class Transaction extends BaseTransaction {
   }
 
   get senderSignature(): Uint8Array | undefined {
-    return this._senderSignature;
+    if (this._senderSignature) {
+      return new Uint8Array(this._senderSignature);
+    } else {
+      return undefined;
+    }
   }
 
   set senderSignature(sig: Buffer) {
@@ -107,8 +98,12 @@ export class Transaction extends BaseTransaction {
     this._feePayerAddress = address;
   }
 
-  get feePayerSignature(): Uint8Array {
-    return this._feePayerSignature;
+  get feePayerSignature(): Uint8Array | undefined {
+    if (this._feePayerSignature) {
+      return new Uint8Array(this._feePayerSignature);
+    } else {
+      return undefined;
+    }
   }
 
   set feePayerSignature(sig: Buffer) {
@@ -187,14 +182,6 @@ export class Transaction extends BaseTransaction {
     this._nonce = n;
   }
 
-  get feePayerPubKey(): PublicKey {
-    return this._feePayerPubKey;
-  }
-
-  set feePayerPubKey(pubKey: PublicKey) {
-    this._feePayerPubKey = pubKey;
-  }
-
   get contract(): string {
     return this._contract;
   }
@@ -300,7 +287,7 @@ export class Transaction extends BaseTransaction {
       this.loadInputsAndOutputs();
 
       // Set sender address
-      if (signedTx.origin) {
+      if (signedTx.signature && signedTx.origin) {
         this.sender = signedTx.origin.toString().toLowerCase();
       }
 
@@ -323,56 +310,52 @@ export class Transaction extends BaseTransaction {
     this.senderSignature = signature;
   }
 
-  getFeePayerPubKey(): string {
-    return this.feePayerPubKey.pub;
-  }
-
-  addFeePayerSignature(publicKey: PublicKey, signature: Buffer): void {
-    this.feePayerPubKey = publicKey;
+  addFeePayerSignature(signature: Buffer): void {
     this.feePayerSignature = signature;
   }
 
   async build(): Promise<void> {
     this.buildClauses();
     await this.buildRawTransaction();
-    this.generateTxnId();
+    this.generateTxnIdAndSetSender();
     this.loadInputsAndOutputs();
   }
 
   /**
    * Sets the transaction ID from the raw transaction if it is signed
    * @protected
    */
-  protected generateTxnId(): void {
+  protected generateTxnIdAndSetSender(): void {
     // Check if we have a raw transaction
     if (!this.rawTransaction) {
       return;
     }
-
-    // Check if the transaction is signed by verifying signature exists
-    if (!this.senderSignature || !this.feePayerSignature) {
-      return;
-    }
-
-    const halfSignedTransaction: VetTransaction = VetTransaction.of(this.rawTransaction.body, this.senderSignature);
-    if (!halfSignedTransaction.signature) {
-      return;
-    }
-    const fullSignedTransaction: VetTransaction = VetTransaction.of(
-      halfSignedTransaction.body,
-      nc_utils.concatBytes(
-        // Drop any previous gas payer signature.
-        halfSignedTransaction.signature.slice(0, Secp256k1.SIGNATURE_LENGTH),
-        this.feePayerSignature
-      )
-    );
-    if (!fullSignedTransaction.signature) {
-      return;
-    }
-    try {
-      this._id = fullSignedTransaction.id.toString();
-    } catch (e) {
+    if (!this.senderSignature) {
       return;
+    } else {
+      const halfSignedTransaction: VetTransaction = VetTransaction.of(this.rawTransaction.body, this.senderSignature);
+      if (halfSignedTransaction.signature) {
+        this._rawTransaction = halfSignedTransaction;
+        this._sender = halfSignedTransaction.origin.toString().toLowerCase();
+      } else {
+        return;
+      }
+      if (this.feePayerSignature) {
+        const fullSignedTransaction: VetTransaction = VetTransaction.of(
+          halfSignedTransaction.body,
+          nc_utils.concatBytes(
+            // Drop any previous gas payer signature.
+            halfSignedTransaction.signature.slice(0, Secp256k1.SIGNATURE_LENGTH),
+            this.feePayerSignature
+          )
+        );
+        if (fullSignedTransaction.signature) {
+          this._rawTransaction = fullSignedTransaction;
+          this._id = fullSignedTransaction.id.toString();
+        } else {
+          return;
+        }
+      }
     }
   }
 
@@ -468,22 +451,34 @@ export class Transaction extends BaseTransaction {
   }
 
   serialize(): string {
-    const halfSignedTransaction: VetTransaction = VetTransaction.of(this.rawTransaction.body, this.senderSignature);
-    if (!halfSignedTransaction.signature) {
-      throw new InvalidTransactionError('Invalid sender signature in half-signed transaction');
-    }
-    if (!this.feePayerSignature) {
-      throw new InvalidTransactionError('Missing fee payer signature');
+    if (!this.senderSignature) {
+      return HexUInt.of(this.rawTransaction.encoded).toString();
+    } else {
+      if (!this.feePayerSignature) {
+        const senderSignedTransaction: VetTransaction = VetTransaction.of(
+          this.rawTransaction.body,
+          this.senderSignature
+        );
+        return HexUInt.of(senderSignedTransaction.encoded).toString();
+      } else {
+        const senderSignedTransaction: VetTransaction = VetTransaction.of(
+          this.rawTransaction.body,
+          this.senderSignature
+        );
+        if (senderSignedTransaction.signature) {
+          const fullSignedTransaction: VetTransaction = VetTransaction.of(
+            senderSignedTransaction.body,
+            nc_utils.concatBytes(
+              senderSignedTransaction.signature.slice(0, Secp256k1.SIGNATURE_LENGTH),
+              this.feePayerSignature
+            )
+          );
+          return HexUInt.of(fullSignedTransaction.encoded).toString();
+        } else {
+          throw new InvalidTransactionError('Transaction is not signed properly');
+        }
+      }
     }
-    const fullSignedTransaction: VetTransaction = VetTransaction.of(
-      halfSignedTransaction.body,
-      nc_utils.concatBytes(
-        // Drop any previous gas payer signature.
-        halfSignedTransaction.signature.slice(0, Secp256k1.SIGNATURE_LENGTH),
-        this.feePayerSignature
-      )
-    );
-    return HexUInt.of(fullSignedTransaction.encoded).toString();
   }
 
   static deserializeTransaction(rawTx: string): VetTransaction {

modules/sdk-coin-vet/src/lib/transactionBuilder/transactionBuilder.ts

@@ -6,7 +6,6 @@ import {
   BuildTransactionError,
   Recipient,
   TransactionType,
-  PublicKey,
   ParseTransactionError,
 } from '@bitgo/sdk-core';
 import { BaseCoin as CoinConfig } from '@bitgo/statics';
@@ -41,6 +40,11 @@ export abstract class TransactionBuilder extends BaseTransactionBuilder {
     this._transaction = tx;
   }
 
+  chainTag(tag: number): this {
+    this._transaction.chainTag = tag;
+    return this;
+  }
+
   /**
    * Sets the sender of this transaction.
    *
@@ -104,8 +108,8 @@ export abstract class TransactionBuilder extends BaseTransactionBuilder {
     this.transaction.addSenderSignature(signature);
   }
 
-  addFeePayerSignature(publicKey: PublicKey, signature: Buffer): void {
-    this.transaction.addFeePayerSignature(publicKey, signature);
+  addFeePayerSignature(signature: Buffer): void {
+    this.transaction.addFeePayerSignature(signature);
   }
 
   /** @inheritdoc */
@@ -129,7 +133,6 @@ export abstract class TransactionBuilder extends BaseTransactionBuilder {
     if (!transaction) {
       throw new Error('transaction not defined');
     }
-    this.validateAddress({ address: transaction.sender });
     for (const recipient of transaction.recipients) {
       this.validateAddress({ address: recipient.address });
       this.validateValue(new BigNumber(recipient.amount));
@@ -167,10 +170,6 @@ export abstract class TransactionBuilder extends BaseTransactionBuilder {
     this.transaction.feePayerAddress = address;
   }
 
-  getFeePayerPubKey(): string {
-    return this.transaction.getFeePayerPubKey();
-  }
-
   /** @inheritdoc */
   protected signImplementation(key: BaseKey): Transaction {
     throw new Error('Method not implemented');

modules/sdk-coin-vet/test/resources/vet.ts

@@ -7,7 +7,7 @@ export const SPONSORED_TRANSACTION =
   '0xf8bc2788014e9cad44bade0940e0df94e59f1cea4e0fef511e3d0f4eec44adf19c4cbeec88016345785d8a0000808180825208808302d6b5c101b882ee76129c1259eb0c8a2b3b3e5f2b089cd11068da1c0db32a9e22228db83dd4be5c721858bc813514141fbd5cf641d0972ce47ceb9be61133fa2ebf0ea37c1f290011fdce201f56d639d827035a5ed8bcef42a42f6eb562bc76a6d95c7736cf8cf340122d1e2fb034668dc491d47b7d3bb10724ba2338a6e79df87bce9617fdce9c00';
 
 export const UNSIGNED_TRANSACTION =
-  '0xf8772788014eabfe2b8fcc1440dddc94e59f1cea4e0fef511e3d0f4eec44adf19c4cbeec85e8d4a51000808180825208808301bff7c0b8414f4b195e2dd666a01c1186df341ff3c0cbe2d6b4a58dca3c8c484f3eac05c4100a165a2730fe67b4c11b4a6c78ea8ab0757bf59735adef327a36737e1694536b00';
+  '0xf4278801536ce9e9fb063840dddc94c52584d1c56e7bddcb6f65d50ff00f71e0ef897a85e8d4a510008081808252088083061c70c0';
 
 export const UNSIGNED_TRANSACTION_2 =
   '0xf72788014ead140e77bbc140e0df94e59f1cea4e0fef511e3d0f4eec44adf19c4cbeec88016345785d8a00008081808252088082faf8c101';
@@ -45,14 +45,30 @@ export const TRANSFER_CLAUSE = [
 ];
 
 export const addresses = {
-  validAddresses: ['0x7ca00e3bc8a836026c2917c6c7c6d049e52099dd', '0xe59f1cea4e0fef511e3d0f4eec44adf19c4cbeec'],
+  validAddresses: [
+    '0x7ca00e3bc8a836026c2917c6c7c6d049e52099dd',
+    '0xe59f1cea4e0fef511e3d0f4eec44adf19c4cbeec',
+    '0x7c87b9ffc6fd6c167c0e4fa9418720f3d659358e',
+  ],
   invalidAddresses: [
     'randomString',
     '0xc4173a804406a365e69dfb297ddfgsdcvf',
     '5ne7phA48Jrvpn39AtupB8ZkCCAy8gLTfpGihZPuDqen',
   ],
 };
 
+export const senderSig =
+  '062080cc77db9d5ce5db84f1cfb57c9906c9bfdce78e4d53757bbd3d536c731e1e5e406a4e783a4f63446aa28d21dc27796aae90fac79151c3d0b3ba68cde36801';
+
+export const feePayerSig =
+  '47ae79e23effd233206180ece3ab3d6d496310c03c55bd0566a54bb7d42095c97a90ca9ff4cfb94ec579a7ec7f79de4814dfc816d0a130189c07b746945482fb00';
+
+export const senderSignedSerializedTxHex =
+  '0xf72788014ead140e77bbc140e0df94e59f1cea4e0fef511e3d0f4eec44adf19c4cbeec88016345785d8a00008081808252088082faf8c101';
+
+export const completeSignedSerializedHex =
+  '0xf8bb2788014ead140e77bbc140e0df94e59f1cea4e0fef511e3d0f4eec44adf19c4cbeec88016345785d8a00008081808252088082faf8c101b882062080cc77db9d5ce5db84f1cfb57c9906c9bfdce78e4d53757bbd3d536c731e1e5e406a4e783a4f63446aa28d21dc27796aae90fac79151c3d0b3ba68cde3680147ae79e23effd233206180ece3ab3d6d496310c03c55bd0566a54bb7d42095c97a90ca9ff4cfb94ec579a7ec7f79de4814dfc816d0a130189c07b746945482fb00';
+
 export const blockIds: { validBlockIds: string[]; invalidBlockIds: string[] } = {
   validBlockIds: [
     '0x014f12ed94c4b4770f7f9a73e2aa41a9dfbac02a49f36ec05acfdba8c7244ff0',

modules/sdk-coin-vet/test/transactionBuilder/transferBuilder.ts

@@ -138,6 +138,34 @@ describe('Vet Transfer Transaction', () => {
         should.equal(toJson.gasPriceCoef, 128);
         should.equal(toJson.expiration, 64);
       });
+
+      it('should build a unsigned tx then add sender sig and build again', async function () {
+        const transaction = new Transaction(coins.get('tvet'));
+        const txBuilder = factory.getTransferBuilder(transaction);
+        txBuilder.sender(testData.addresses.validAddresses[2]);
+        txBuilder.recipients(testData.recipients);
+        txBuilder.gas(21000);
+        txBuilder.nonce(64248);
+        txBuilder.blockRef('0x014ead140e77bbc1');
+        txBuilder.addFeePayerAddress(testData.feePayer.address);
+        txBuilder.expiration(64);
+        txBuilder.gasPriceCoef(128);
+        const tx = (await txBuilder.build()) as Transaction;
+        const unsignedSerializedTx = tx.toBroadcastFormat();
+
+        const builder1 = factory.from(unsignedSerializedTx);
+        builder1.addSenderSignature(Buffer.from(testData.senderSig, 'hex'));
+        const senderSignedTx = await builder1.build();
+        const senderSignedSerializedTx = senderSignedTx.toBroadcastFormat();
+        should.equal(senderSignedSerializedTx, testData.senderSignedSerializedTxHex);
+
+        const builder2 = factory.from(testData.senderSignedSerializedTxHex);
+        builder2.addSenderSignature(Buffer.from(testData.senderSig, 'hex'));
+        builder2.addFeePayerSignature(Buffer.from(testData.feePayerSig, 'hex'));
+        const completelySignedTx = await builder2.build();
+        should.equal(completelySignedTx.toBroadcastFormat(), testData.completeSignedSerializedHex);
+        should.equal(completelySignedTx.id, '0xb988d614d3c24420cb2183239ab601b53e63ff4c81cea1e4888bc9d0aa6aad13');
+      });
     });
 
     describe('Fail', () => {

modules/sdk-coin-vet/test/unit/utils.ts

@@ -46,11 +46,16 @@ describe('Vechain util library', function () {
   describe('isValidDeserialize', function () {
     it('should succeed to correctly deserialize sponsored signed serialized transaction', function () {
       const signedTxn: VetTransaction = utils.deserializeTransaction(testData.SPONSORED_TRANSACTION);
+      should.equal(signedTxn.origin.toString().toLowerCase(), '0x7ca00e3bc8a836026c2917c6c7c6d049e52099dd');
       should.equal(signedTxn.gasPayer.toString().toLowerCase(), '0xdc9fef0b84a0ccf3f1bd4b84e41743e3e051a083');
     });
     it('should succeed to correctly deserialize unsigned serialized transaction', function () {
       const signedTxn: VetTransaction = utils.deserializeTransaction(testData.UNSIGNED_TRANSACTION);
-      should.equal(signedTxn.origin.toString().toLowerCase(), '0x7ca00e3bc8a836026c2917c6c7c6d049e52099dd');
+      should.equal(signedTxn.isSigned, false);
+      should.equal(signedTxn.body.gas, 21000);
+      should.equal(signedTxn.body.nonce, 400496);
+      should.equal(signedTxn.body.expiration, 64);
+      should.equal(signedTxn.body.blockRef, '0x01536ce9e9fb0638');
     });
     it('should fail to deserialize an invalid serialized transaction', function () {
       should.throws(() => utils.deserializeTransaction(testData.INVALID_TRANSACTION));