fix(crypto): transaction builder should build and sign versioned transactions

__tests__/unit/crypto/transactions/builders/transactions/transaction-builder.test.ts

@@ -1,5 +1,7 @@
 import { Factories, Generators } from "@packages/core-test-framework/src";
+import { TransactionVersionError } from "@packages/crypto/src/errors";
 import { Keys } from "@packages/crypto/src/identities";
+import { Address } from "@packages/crypto/src/identities";
 import { configManager } from "@packages/crypto/src/managers";
 import { BuilderFactory, Signer } from "@packages/crypto/src/transactions";
 import { BigNumber } from "@packages/crypto/src/utils";
@@ -158,7 +160,9 @@ describe.each([
                 builder.sign(identity.bip39);
 
                 expect(spyKeys).toHaveBeenCalledWith(identity.bip39);
-                expect(spySign).toHaveBeenCalledWith((builder as any).getSigningObject(), identity.keys);
+                expect(spySign).toHaveBeenCalledWith((builder as any).getSigningObject(), identity.keys, {
+                    disableVersionCheck: false,
+                });
             });
 
             it("establishes the public key of the sender", () => {
@@ -170,7 +174,9 @@ describe.each([
 
                 expect(builder.data.senderPublicKey).toBe(identity.keys.publicKey);
                 expect(spyKeys).toHaveBeenCalledWith(identity.bip39);
-                expect(spySign).toHaveBeenCalledWith((builder as any).getSigningObject(), identity.keys);
+                expect(spySign).toHaveBeenCalledWith((builder as any).getSigningObject(), identity.keys, {
+                    disableVersionCheck: false,
+                });
             });
         });
 
@@ -185,7 +191,9 @@ describe.each([
                 expect(spyKeys).toHaveBeenCalledWith(identity.bip39, {
                     wif: 186,
                 });
-                expect(spySign).toHaveBeenCalledWith((builder as any).getSigningObject(), identity.keys);
+                expect(spySign).toHaveBeenCalledWith((builder as any).getSigningObject(), identity.keys, {
+                    disableVersionCheck: false,
+                });
             });
 
             it("establishes the public key of the sender", () => {
@@ -195,7 +203,9 @@ describe.each([
                 builder.signWithWif(identity.wif);
 
                 expect(builder.data.senderPublicKey).toBe(identity.publicKey);
-                expect(spySign).toHaveBeenCalledWith((builder as any).getSigningObject(), identity.keys);
+                expect(spySign).toHaveBeenCalledWith((builder as any).getSigningObject(), identity.keys, {
+                    disableVersionCheck: false,
+                });
             });
         });
 
@@ -244,3 +254,89 @@ describe.each([
         });
     });
 });
+
+describe("Setting the version number explicitly", () => {
+    it("should not throw transaction version error when specifically setting version 1 and aip11 is false", () => {
+        configManager.setFromPreset("devnet");
+        configManager.getMilestone().aip11 = false;
+
+        const recipientAddress = Address.fromPassphrase("recipient's secret");
+        const transaction = BuilderFactory.transfer().version(1).amount("100").recipientId(recipientAddress);
+
+        let signedTransaction;
+        expect(() => (signedTransaction = transaction.sign("sender's secret"))).not.toThrowError(
+            TransactionVersionError,
+        );
+        expect(signedTransaction.data.version).toEqual(1);
+        expect(() => signedTransaction.build()).not.toThrowError(TransactionVersionError);
+    });
+    it("should not throw transaction version error when specifically setting version 1 and aip11 is true", () => {
+        configManager.setFromPreset("devnet");
+        configManager.getMilestone().aip11 = true;
+
+        const recipientAddress = Address.fromPassphrase("recipient's secret");
+        const transaction = BuilderFactory.transfer().version(1).amount("100").recipientId(recipientAddress);
+
+        let signedTransaction;
+        expect(() => (signedTransaction = transaction.sign("sender's secret"))).not.toThrowError(
+            TransactionVersionError,
+        );
+        expect(signedTransaction.data.version).toEqual(1);
+        expect(() => signedTransaction.build()).not.toThrowError(TransactionVersionError);
+    });
+
+    it("should not throw transaction version error when specifically setting version 2 and aip11 is false", () => {
+        configManager.setFromPreset("devnet");
+        configManager.getMilestone().aip11 = false;
+
+        const recipientAddress = Address.fromPassphrase("recipient's secret");
+        const transaction = BuilderFactory.transfer().version(2).amount("100").recipientId(recipientAddress);
+
+        let signedTransaction;
+
+        expect(() => (signedTransaction = transaction.sign("sender's secret"))).not.toThrowError(
+            TransactionVersionError,
+        );
+        expect(signedTransaction.data.version).toEqual(2);
+        expect(() => signedTransaction.build()).not.toThrowError(TransactionVersionError);
+    });
+
+    it("should not throw transaction version error when specifically setting version 2 and aip11 is true", () => {
+        configManager.setFromPreset("devnet");
+        configManager.getMilestone().aip11 = true;
+
+        const recipientAddress = Address.fromPassphrase("recipient's secret");
+        const transaction = BuilderFactory.transfer().version(2).amount("100").recipientId(recipientAddress);
+
+        let signedTransaction;
+
+        expect(() => (signedTransaction = transaction.sign("sender's secret"))).not.toThrowError(
+            TransactionVersionError,
+        );
+        expect(signedTransaction.data.version).toEqual(2);
+
+        expect(() => signedTransaction.build()).not.toThrowError(TransactionVersionError);
+    });
+
+    it("should throw transaction version error when no version is specified, but it is version 1 and we have reached aip11", () => {
+        configManager.setFromPreset("devnet");
+        configManager.getMilestone().aip11 = false;
+
+        const recipientAddress = Address.fromPassphrase("recipient's secret");
+        const transaction = BuilderFactory.transfer().amount("100").recipientId(recipientAddress);
+        configManager.getMilestone().aip11 = true;
+
+        expect(() => transaction.sign("sender's secret")).toThrowError(TransactionVersionError);
+    });
+
+    it("should throw transaction version error when no version is specified, but it is version 2 and we have not reached aip11", () => {
+        configManager.setFromPreset("devnet");
+        configManager.getMilestone().aip11 = true;
+
+        const recipientAddress = Address.fromPassphrase("recipient's secret");
+        const transaction = BuilderFactory.transfer().amount("100").recipientId(recipientAddress);
+        configManager.getMilestone().aip11 = false;
+
+        expect(() => transaction.sign("sender's secret")).toThrowError(TransactionVersionError);
+    });
+});

packages/crypto/src/interfaces/transactions.ts

@@ -21,7 +21,7 @@ export interface ITransaction {
     serialize(options?: ISerializeOptions): ByteBuffer | undefined;
     deserialize(buf: ByteBuffer): void;
 
-    verify(): boolean;
+    verify(options?: IVerifyOptions): boolean;
     verifySchema(strict?: boolean): ISchemaValidationResult;
 
     toJson(): ITransactionJson;
@@ -164,10 +164,16 @@ export interface IHtlcExpiration {
 
 export interface IDeserializeOptions {
     acceptLegacyVersion?: boolean;
+    disableVersionCheck?: boolean;
+}
+
+export interface IVerifyOptions {
+    disableVersionCheck?: boolean;
 }
 
 export interface ISerializeOptions {
     acceptLegacyVersion?: boolean;
+    disableVersionCheck?: boolean;
     excludeSignature?: boolean;
     excludeSecondSignature?: boolean;
     excludeMultiSignature?: boolean;

packages/crypto/src/transactions/builders/transactions/transaction.ts

@@ -15,6 +15,8 @@ export abstract class TransactionBuilder<TBuilder extends TransactionBuilder<TBu
 
     protected signWithSenderAsRecipient = false;
 
+    private disableVersionCheck = false;
+
     public constructor() {
         this.data = {
             id: undefined,
@@ -26,12 +28,14 @@ export abstract class TransactionBuilder<TBuilder extends TransactionBuilder<TBu
     }
 
     public build(data: Partial<ITransactionData> = {}): ITransaction {
-        return TransactionFactory.fromData({ ...this.data, ...data }, false);
+        return TransactionFactory.fromData({ ...this.data, ...data }, false, {
+            disableVersionCheck: this.disableVersionCheck,
+        });
     }
 
     public version(version: number): TBuilder {
         this.data.version = version;
-
+        this.disableVersionCheck = true;
         return this.instance();
     }
 
@@ -134,7 +138,7 @@ export abstract class TransactionBuilder<TBuilder extends TransactionBuilder<TBu
     }
 
     public verify(): boolean {
-        return Verifier.verifyHash(this.data);
+        return Verifier.verifyHash(this.data, this.disableVersionCheck);
     }
 
     public getStruct(): ITransactionData {
@@ -174,7 +178,9 @@ export abstract class TransactionBuilder<TBuilder extends TransactionBuilder<TBu
             this.data.recipientId = Address.fromPublicKey(keys.publicKey, this.data.network);
         }
 
-        this.data.signature = Signer.sign(this.getSigningObject(), keys);
+        this.data.signature = Signer.sign(this.getSigningObject(), keys, {
+            disableVersionCheck: this.disableVersionCheck,
+        });
 
         return this.instance();
     }

packages/crypto/src/transactions/deserializer.ts

@@ -45,7 +45,11 @@ export class Deserializer {
         this.deserializeSignatures(data, buffer);
 
         if (data.version) {
-            if (options.acceptLegacyVersion || isSupportedTransactionVersion(data.version)) {
+            if (
+                options.acceptLegacyVersion ||
+                options.disableVersionCheck ||
+                isSupportedTransactionVersion(data.version)
+            ) {
                 if (data.version === 1) {
                     this.applyV1Compatibility(data);
                 }

packages/crypto/src/transactions/factory.ts

@@ -23,8 +23,8 @@ export class TransactionFactory {
         return this.fromSerialized(hex);
     }
 
-    public static fromBytes(buffer: Buffer, strict = true): ITransaction {
-        return this.fromSerialized(buffer.toString("hex"), strict);
+    public static fromBytes(buffer: Buffer, strict = true, options: IDeserializeOptions = {}): ITransaction {
+        return this.fromSerialized(buffer.toString("hex"), strict, options);
     }
 
     /**
@@ -55,7 +55,7 @@ export class TransactionFactory {
         return this.fromData(data);
     }
 
-    public static fromData(data: ITransactionData, strict = true): ITransaction {
+    public static fromData(data: ITransactionData, strict = true, options: IDeserializeOptions = {}): ITransaction {
         const { value, error } = Verifier.verifySchema(data, strict);
 
         if (error && !isException(value.id)) {
@@ -71,21 +71,21 @@ export class TransactionFactory {
 
         Serializer.serialize(transaction);
 
-        return this.fromBytes(transaction.serialized, strict);
+        return this.fromBytes(transaction.serialized, strict, options);
     }
 
-    private static fromSerialized(serialized: string, strict = true): ITransaction {
+    private static fromSerialized(serialized: string, strict = true, options: IDeserializeOptions = {}): ITransaction {
         try {
-            const transaction = Deserializer.deserialize(serialized);
-            transaction.data.id = Utils.getId(transaction.data);
+            const transaction = Deserializer.deserialize(serialized, options);
+            transaction.data.id = Utils.getId(transaction.data, options);
 
             const { value, error } = Verifier.verifySchema(transaction.data, strict);
 
             if (error && !isException(value.id)) {
                 throw new TransactionSchemaError(error);
             }
 
-            transaction.isVerified = transaction.verify();
+            transaction.isVerified = transaction.verify(options);
 
             return transaction;
         } catch (error) {

packages/crypto/src/transactions/serializer.ts

@@ -15,7 +15,7 @@ export class Serializer {
     public static getBytes(transaction: ITransactionData, options: ISerializeOptions = {}): Buffer {
         const version: number = transaction.version || 1;
 
-        if (options.acceptLegacyVersion || isSupportedTransactionVersion(version)) {
+        if (options.acceptLegacyVersion || options.disableVersionCheck || isSupportedTransactionVersion(version)) {
             if (version === 1) {
                 return this.getBytesV1(transaction, options);
             }

packages/crypto/src/transactions/signer.ts

@@ -5,7 +5,9 @@ import { Utils } from "./utils";
 
 export class Signer {
     public static sign(transaction: ITransactionData, keys: IKeyPair, options?: ISerializeOptions): string {
-        options = options || { excludeSignature: true, excludeSecondSignature: true };
+        if (!options || (options.excludeSignature === undefined && options.excludeSecondSignature === undefined)) {
+            options = { excludeSignature: true, excludeSecondSignature: true, ...options };
+        }
 
         const hash: Buffer = Utils.toHash(transaction, options);
         const signature: string =

packages/crypto/src/transactions/types/transaction.ts

@@ -3,7 +3,13 @@ import ByteBuffer from "bytebuffer";
 import { TransactionTypeGroup } from "../../enums";
 import { NotImplemented } from "../../errors";
 import { Address } from "../../identities";
-import { ISchemaValidationResult, ITransaction, ITransactionData, ITransactionJson } from "../../interfaces";
+import {
+    ISchemaValidationResult,
+    ISerializeOptions,
+    ITransaction,
+    ITransactionData,
+    ITransactionJson,
+} from "../../interfaces";
 import { configManager } from "../../managers/config";
 import { BigNumber } from "../../utils/bignum";
 import { Verifier } from "../verifier";
@@ -42,8 +48,8 @@ export abstract class Transaction implements ITransaction {
         return this.defaultStaticFee;
     }
 
-    public verify(): boolean {
-        return Verifier.verify(this.data);
+    public verify(options?: ISerializeOptions): boolean {
+        return Verifier.verify(this.data, options);
     }
 
     public verifySecondSignature(publicKey: string): boolean {

packages/crypto/src/transactions/verifier.ts

@@ -1,14 +1,14 @@
 import { Hash } from "../crypto/hash";
 import { DuplicateParticipantInMultiSignatureError, InvalidMultiSignatureAssetError } from "../errors";
-import { IMultiSignatureAsset, ISchemaValidationResult, ITransactionData } from "../interfaces";
+import { IMultiSignatureAsset, ISchemaValidationResult, ITransactionData, IVerifyOptions } from "../interfaces";
 import { configManager } from "../managers";
 import { isException } from "../utils";
 import { validator } from "../validation";
 import { TransactionTypeFactory } from "./types/factory";
 import { Utils } from "./utils";
 
 export class Verifier {
-    public static verify(data: ITransactionData): boolean {
+    public static verify(data: ITransactionData, options?: IVerifyOptions): boolean {
         if (isException(data.id)) {
             return true;
         }
@@ -17,17 +17,24 @@ export class Verifier {
             return false;
         }
 
-        return Verifier.verifyHash(data);
+        return Verifier.verifyHash(data, options?.disableVersionCheck);
     }
 
-    public static verifySecondSignature(transaction: ITransactionData, publicKey: string): boolean {
+    public static verifySecondSignature(
+        transaction: ITransactionData,
+        publicKey: string,
+        options?: IVerifyOptions,
+    ): boolean {
         const secondSignature: string | undefined = transaction.secondSignature || transaction.signSignature;
 
         if (!secondSignature) {
             return false;
         }
 
-        const hash: Buffer = Utils.toHash(transaction, { excludeSecondSignature: true });
+        const hash: Buffer = Utils.toHash(transaction, {
+            disableVersionCheck: options?.disableVersionCheck,
+            excludeSecondSignature: true,
+        });
         return this.internalVerifySignature(hash, secondSignature, publicKey);
     }
 
@@ -79,14 +86,15 @@ export class Verifier {
         return verified;
     }
 
-    public static verifyHash(data: ITransactionData): boolean {
+    public static verifyHash(data: ITransactionData, disableVersionCheck = false): boolean {
         const { signature, senderPublicKey } = data;
 
         if (!signature || !senderPublicKey) {
             return false;
         }
 
         const hash: Buffer = Utils.toHash(data, {
+            disableVersionCheck,
             excludeSignature: true,
             excludeSecondSignature: true,
         });