fix(core-transaction-pool): handle transaction deserialize error

__tests__/unit/core-transaction-pool/dynamic-fee-matcher.test.ts

@@ -1,9 +1,13 @@
 import "jest-extended";
 
-import { Container, Contracts } from "@packages/core-kernel";
-import { DynamicFeeMatcher } from "@packages/core-transaction-pool/src/dynamic-fee-matcher";
-import { TransactionFeeToHighError, TransactionFeeToLowError } from "@packages/core-transaction-pool/src/errors";
-import { Interfaces, Utils } from "@packages/crypto";
+import { Container, Contracts } from "@arkecosystem/core-kernel";
+import { Interfaces, Utils } from "@arkecosystem/crypto";
+
+import { DynamicFeeMatcher } from "../../../packages/core-transaction-pool/src/dynamic-fee-matcher";
+import {
+    TransactionFeeToHighError,
+    TransactionFeeToLowError,
+} from "../../../packages/core-transaction-pool/src/errors";
 
 const handler = { dynamicFee: jest.fn() };
 const configuration = { getRequired: jest.fn() };

__tests__/unit/core-transaction-pool/errors.test.ts

@@ -2,6 +2,7 @@ import { Contracts } from "@arkecosystem/core-kernel";
 import { Identities, Managers, Transactions, Utils } from "@arkecosystem/crypto";
 
 import {
+    InvalidTransactionDataError,
     RetryTransactionError,
     SenderExceededMaximumTransactionCountError,
     TransactionAlreadyInPoolError,
@@ -33,7 +34,6 @@ test("RetryTransactionError", () => {
 
     expect(error).toBeInstanceOf(Contracts.TransactionPool.PoolError);
     expect(error.type).toBe("ERR_RETRY");
-    expect(error.transaction).toBe(transaction);
     expect(error.message).toBe(`${transaction} cannot be added to pool, please retry`);
 });
 
@@ -42,7 +42,6 @@ test("TransactionAlreadyInPoolError", () => {
 
     expect(error).toBeInstanceOf(Contracts.TransactionPool.PoolError);
     expect(error.type).toBe("ERR_DUPLICATE");
-    expect(error.transaction).toBe(transaction);
     expect(error.message).toBe(`${transaction} is already in pool`);
 });
 
@@ -51,7 +50,6 @@ test("TransactionExceedsMaximumByteSizeError", () => {
 
     expect(error).toBeInstanceOf(Contracts.TransactionPool.PoolError);
     expect(error.type).toBe("ERR_TOO_LARGE");
-    expect(error.transaction).toBe(transaction);
     expect(error.message).toBe(`${transaction} exceeds size limit of 1024 bytes`);
 });
 
@@ -60,7 +58,6 @@ test("TransactionHasExpiredError", () => {
 
     expect(error).toBeInstanceOf(Contracts.TransactionPool.PoolError);
     expect(error.type).toBe("ERR_EXPIRED");
-    expect(error.transaction).toBe(transaction);
     expect(error.message).toBe(`${transaction} expired at height 100`);
 });
 
@@ -69,7 +66,6 @@ test("TransactionFeeToLowError", () => {
 
     expect(error).toBeInstanceOf(Contracts.TransactionPool.PoolError);
     expect(error.type).toBe("ERR_LOW_FEE");
-    expect(error.transaction).toBe(transaction);
     expect(error.message).toBe(`${transaction} fee is to low to enter the pool`);
 });
 
@@ -78,7 +74,6 @@ test("SenderExceededMaximumTransactionCountError", () => {
 
     expect(error).toBeInstanceOf(Contracts.TransactionPool.PoolError);
     expect(error.type).toBe("ERR_EXCEEDS_MAX_COUNT");
-    expect(error.transaction).toBe(transaction);
     expect(error.message).toBe(`${transaction} exceeds sender's 1 transaction count limit`);
 });
 
@@ -87,7 +82,6 @@ test("TransactionPoolFullError", () => {
 
     expect(error).toBeInstanceOf(Contracts.TransactionPool.PoolError);
     expect(error.type).toBe("ERR_POOL_FULL");
-    expect(error.transaction).toBe(transaction);
     expect(error.message).toBe(`${transaction} fee 0.000009 DѦ is lower than 0.00001 DѦ already in pool`);
 });
 
@@ -96,7 +90,6 @@ test("TransactionFailedToApplyError", () => {
 
     expect(error).toBeInstanceOf(Contracts.TransactionPool.PoolError);
     expect(error.type).toBe("ERR_APPLY");
-    expect(error.transaction).toBe(transaction);
     expect(error.message).toBe(`${transaction} cannot be applied: Something went horribly wrong`);
 });
 
@@ -105,7 +98,6 @@ test("TransactionFailedToVerifyError", () => {
 
     expect(error).toBeInstanceOf(Contracts.TransactionPool.PoolError);
     expect(error.type).toBe("ERR_BAD_DATA");
-    expect(error.transaction).toBe(transaction);
     expect(error.message).toBe(`${transaction} didn't passed verification`);
 });
 
@@ -114,7 +106,6 @@ test("TransactionFromFutureError", () => {
 
     expect(error).toBeInstanceOf(Contracts.TransactionPool.PoolError);
     expect(error.type).toBe("ERR_FROM_FUTURE");
-    expect(error.transaction).toBe(transaction);
     expect(error.message).toBe(`${transaction} is 1 second in future`);
 });
 
@@ -123,6 +114,13 @@ test("TransactionFromWrongNetworkError", () => {
 
     expect(error).toBeInstanceOf(Contracts.TransactionPool.PoolError);
     expect(error.type).toBe("ERR_WRONG_NETWORK");
-    expect(error.transaction).toBe(transaction);
     expect(error.message).toBe(`${transaction} network 30 doesn't match node's network 23`);
 });
+
+test("InvalidTransactionDataError", () => {
+    const error = new InvalidTransactionDataError("Version 1 not supported");
+
+    expect(error).toBeInstanceOf(Contracts.TransactionPool.PoolError);
+    expect(error.type).toBe("ERR_BAD_DATA");
+    expect(error.message).toBe("Invalid transaction data: Version 1 not supported");
+});

__tests__/unit/core-transaction-pool/processor.test.ts

@@ -1,7 +1,8 @@
-import { Container, Contracts } from "@packages/core-kernel";
-import { TransactionFeeToLowError } from "@packages/core-transaction-pool/src/errors";
-import { Processor } from "@packages/core-transaction-pool/src/processor";
-import { Identities, Managers, Transactions } from "@packages/crypto";
+import { Container, Contracts } from "@arkecosystem/core-kernel";
+import { Identities, Managers, Transactions } from "@arkecosystem/crypto";
+
+import { TransactionFeeToLowError } from "../../../packages/core-transaction-pool/src/errors";
+import { Processor } from "../../../packages/core-transaction-pool/src/processor";
 
 Managers.configManager.getMilestone().aip11 = true;
 const transaction1 = Transactions.BuilderFactory.transfer()
@@ -19,21 +20,18 @@ const transaction2 = Transactions.BuilderFactory.transfer()
     .sign("sender's secret")
     .build();
 
-const logger = { warning: jest.fn(), error: jest.fn() };
 const pool = { addTransaction: jest.fn() };
 const dynamicFeeMatcher = { throwIfCannotEnterPool: jest.fn(), throwIfCannotBroadcast: jest.fn() };
 const transactionBroadcaster = { broadcastTransactions: jest.fn() };
 const workerPool = { isTypeGroupSupported: jest.fn(), getTransactionFromData: jest.fn() };
 
 const container = new Container.Container();
-container.bind(Container.Identifiers.LogService).toConstantValue(logger);
 container.bind(Container.Identifiers.TransactionPoolService).toConstantValue(pool);
 container.bind(Container.Identifiers.TransactionPoolDynamicFeeMatcher).toConstantValue(dynamicFeeMatcher);
 container.bind(Container.Identifiers.PeerTransactionBroadcaster).toConstantValue(transactionBroadcaster);
 container.bind(Container.Identifiers.TransactionPoolWorkerPool).toConstantValue(workerPool);
 
 beforeEach(() => {
-    logger.warning.mockReset();
     pool.addTransaction.mockReset();
     dynamicFeeMatcher.throwIfCannotEnterPool.mockReset();
     dynamicFeeMatcher.throwIfCannotBroadcast.mockReset();
@@ -72,6 +70,26 @@ describe("Processor.process", () => {
         expect(processor.errors).toBeUndefined();
     });
 
+    it("should wrap deserialize errors into BAD_DATA pool error", async () => {
+        const processor = container.resolve(Processor);
+
+        workerPool.isTypeGroupSupported.mockReturnValueOnce(true);
+        workerPool.getTransactionFromData.mockRejectedValueOnce(new Error("Version 1 not supported"));
+
+        await processor.process([transaction1.data]);
+
+        expect(workerPool.isTypeGroupSupported).toBeCalledWith(transaction1.data.typeGroup);
+        expect(workerPool.getTransactionFromData).toBeCalledWith(transaction1.data);
+
+        expect(processor.invalid).toEqual([transaction1.id]);
+        expect(processor.errors).toEqual({
+            [transaction1.data.id]: {
+                type: "ERR_BAD_DATA",
+                message: "Invalid transaction data: Version 1 not supported",
+            },
+        });
+    });
+
     it("should add eligible transactions to pool", async () => {
         workerPool.isTypeGroupSupported.mockReturnValue(false);
 
@@ -143,7 +161,7 @@ describe("Processor.process", () => {
     });
 
     it("should track excess transactions", async () => {
-        const exceedsError = new Contracts.TransactionPool.PoolError("Exceeds", "ERR_EXCEEDS_MAX_COUNT", transaction1);
+        const exceedsError = new Contracts.TransactionPool.PoolError("Exceeds", "ERR_EXCEEDS_MAX_COUNT");
 
         workerPool.isTypeGroupSupported.mockReturnValue(false);
         pool.addTransaction.mockRejectedValueOnce(exceedsError);

__tests__/unit/core-transaction-pool/service-provider.test.ts

@@ -1,10 +1,10 @@
 import "jest-extended";
 
-import { Contracts } from "@arkecosystem/core-kernel";
-import { Application, Container, Services } from "@packages/core-kernel";
-import { ServiceProvider } from "@packages/core-transaction-pool/src";
+import { Application, Container, Contracts, Services } from "@arkecosystem/core-kernel";
 import { fork } from "child_process";
 
+import { ServiceProvider } from "../../../packages/core-transaction-pool/src";
+
 jest.mock("child_process");
 
 let app: Application;

packages/core-kernel/src/contracts/transaction-pool/errors.ts

@@ -1,12 +1,8 @@
-import { Interfaces } from "@arkecosystem/crypto";
-
 export class PoolError extends Error {
     public readonly type: string;
-    public readonly transaction: Interfaces.ITransaction;
 
-    public constructor(message: string, type: string, transaction: Interfaces.ITransaction) {
+    public constructor(message: string, type: string) {
         super(message);
         this.type = type;
-        this.transaction = transaction;
     }
 }

packages/core-magistrate-transactions/src/handlers/bridgechain-resignation.ts

@@ -113,7 +113,6 @@ export class BridgechainResignationTransactionHandler extends MagistrateTransact
             throw new Contracts.TransactionPool.PoolError(
                 `Bridgechain resignation for bridgechainId "${bridgechainId}" already in the pool`,
                 "ERR_PENDING",
-                transaction,
             );
         }
     }

packages/core-magistrate-transactions/src/handlers/bridgechain-update.ts

@@ -130,7 +130,6 @@ export class BridgechainUpdateTransactionHandler extends MagistrateTransactionHa
             throw new Contracts.TransactionPool.PoolError(
                 `Bridgechain update for bridgechainId "${bridgechainId}" already in the pool`,
                 "ERR_PENDING",
-                transaction,
             );
         }
     }

packages/core-magistrate-transactions/src/handlers/business-registration.ts

@@ -76,11 +76,7 @@ export class BusinessRegistrationTransactionHandler extends MagistrateTransactio
             .has();
 
         if (hasSender) {
-            throw new Contracts.TransactionPool.PoolError(
-                `Business registration already in the pool`,
-                "ERR_PENDING",
-                transaction,
-            );
+            throw new Contracts.TransactionPool.PoolError(`Business registration already in the pool`, "ERR_PENDING");
         }
     }
 

packages/core-magistrate-transactions/src/handlers/business-resignation.ts

@@ -67,11 +67,7 @@ export class BusinessResignationTransactionHandler extends MagistrateTransaction
             .has();
 
         if (hasSender) {
-            throw new Contracts.TransactionPool.PoolError(
-                `Business resignation already in the pool`,
-                "ERR_PENDING",
-                transaction,
-            );
+            throw new Contracts.TransactionPool.PoolError(`Business resignation already in the pool`, "ERR_PENDING");
         }
     }
 

packages/core-magistrate-transactions/src/handlers/business-update.ts

@@ -82,11 +82,7 @@ export class BusinessUpdateTransactionHandler extends MagistrateTransactionHandl
             .has();
 
         if (hasSender) {
-            throw new Contracts.TransactionPool.PoolError(
-                `Business update already in the pool`,
-                "ERR_PENDING",
-                transaction,
-            );
+            throw new Contracts.TransactionPool.PoolError(`Business update already in the pool`, "ERR_PENDING");
         }
     }
 

packages/core-transaction-pool/src/errors.ts

@@ -3,13 +3,13 @@ import { Interfaces, Utils } from "@arkecosystem/crypto";
 
 export class RetryTransactionError extends Contracts.TransactionPool.PoolError {
     public constructor(transaction: Interfaces.ITransaction) {
-        super(`${transaction} cannot be added to pool, please retry`, "ERR_RETRY", transaction);
+        super(`${transaction} cannot be added to pool, please retry`, "ERR_RETRY");
     }
 }
 
 export class TransactionAlreadyInPoolError extends Contracts.TransactionPool.PoolError {
     public constructor(transaction: Interfaces.ITransaction) {
-        super(`${transaction} is already in pool`, "ERR_DUPLICATE", transaction);
+        super(`${transaction} is already in pool`, "ERR_DUPLICATE");
     }
 }
 
@@ -20,7 +20,6 @@ export class TransactionExceedsMaximumByteSizeError extends Contracts.Transactio
         super(
             `${transaction} exceeds size limit of ${AppUtils.pluralize("byte", maxSize, true)}`,
             "ERR_TOO_LARGE", // ! should be "ERR_TO_LARGE" instead of "ERR_TOO_LARGE"
-            transaction,
         );
         this.maxSize = maxSize;
     }
@@ -30,20 +29,20 @@ export class TransactionHasExpiredError extends Contracts.TransactionPool.PoolEr
     public readonly expirationHeight: number;
 
     public constructor(transaction: Interfaces.ITransaction, expirationHeight: number) {
-        super(`${transaction} expired at height ${expirationHeight}`, "ERR_EXPIRED", transaction);
+        super(`${transaction} expired at height ${expirationHeight}`, "ERR_EXPIRED");
         this.expirationHeight = expirationHeight;
     }
 }
 
 export class TransactionFeeToLowError extends Contracts.TransactionPool.PoolError {
     public constructor(transaction: Interfaces.ITransaction) {
-        super(`${transaction} fee is to low to enter the pool`, "ERR_LOW_FEE", transaction);
+        super(`${transaction} fee is to low to enter the pool`, "ERR_LOW_FEE");
     }
 }
 
 export class TransactionFeeToHighError extends Contracts.TransactionPool.PoolError {
     public constructor(transaction: Interfaces.ITransaction) {
-        super(`${transaction} fee is to high to enter the pool`, "ERR_HIGH_FEE", transaction);
+        super(`${transaction} fee is to high to enter the pool`, "ERR_HIGH_FEE");
     }
 }
 
@@ -54,7 +53,6 @@ export class SenderExceededMaximumTransactionCountError extends Contracts.Transa
         super(
             `${transaction} exceeds sender's ${AppUtils.pluralize("transaction", maxCount, true)} count limit`,
             "ERR_EXCEEDS_MAX_COUNT",
-            transaction,
         );
         this.maxCount = maxCount;
     }
@@ -67,7 +65,7 @@ export class TransactionPoolFullError extends Contracts.TransactionPool.PoolErro
         const msg =
             `${transaction} fee ${Utils.formatSatoshi(transaction.data.fee)} ` +
             `is lower than ${Utils.formatSatoshi(required)} already in pool`;
-        super(msg, "ERR_POOL_FULL", transaction);
+        super(msg, "ERR_POOL_FULL");
         this.required = required;
     }
 }
@@ -76,26 +74,22 @@ export class TransactionFailedToApplyError extends Contracts.TransactionPool.Poo
     public readonly error: Error;
 
     public constructor(transaction: Interfaces.ITransaction, error: Error) {
-        super(`${transaction} cannot be applied: ${error.message}`, "ERR_APPLY", transaction);
+        super(`${transaction} cannot be applied: ${error.message}`, "ERR_APPLY");
         this.error = error;
     }
 }
 
 export class TransactionFailedToVerifyError extends Contracts.TransactionPool.PoolError {
     public constructor(transaction: Interfaces.ITransaction) {
-        super(`${transaction} didn't passed verification`, "ERR_BAD_DATA", transaction);
+        super(`${transaction} didn't passed verification`, "ERR_BAD_DATA");
     }
 }
 
 export class TransactionFromFutureError extends Contracts.TransactionPool.PoolError {
     public secondsInFuture: number;
 
     public constructor(transaction: Interfaces.ITransaction, secondsInFuture: number) {
-        super(
-            `${transaction} is ${AppUtils.pluralize("second", secondsInFuture, true)} in future`,
-            "ERR_FROM_FUTURE",
-            transaction,
-        );
+        super(`${transaction} is ${AppUtils.pluralize("second", secondsInFuture, true)} in future`, "ERR_FROM_FUTURE");
         this.secondsInFuture = secondsInFuture;
     }
 }
@@ -107,8 +101,13 @@ export class TransactionFromWrongNetworkError extends Contracts.TransactionPool.
         super(
             `${transaction} network ${transaction.data.network} doesn't match node's network ${currentNetwork}`,
             "ERR_WRONG_NETWORK",
-            transaction,
         );
         this.currentNetwork = currentNetwork;
     }
 }
+
+export class InvalidTransactionDataError extends Contracts.TransactionPool.PoolError {
+    public constructor(reason: string) {
+        super(`Invalid transaction data: ${reason}`, "ERR_BAD_DATA");
+    }
+}