From acb6e90187e9f8fa5f99d8cf79815f7b271ab64b Mon Sep 17 00:00:00 2001 From: Air1 Date: Wed, 29 May 2019 19:18:16 +0400 Subject: [PATCH 01/34] test: 1st commit getTransactionsForForging tests --- .../core-transaction-pool/connection.test.ts | 183 ++++++++++++++++++ 1 file changed, 183 insertions(+) create mode 100644 __tests__/integration/core-transaction-pool/connection.test.ts diff --git a/__tests__/integration/core-transaction-pool/connection.test.ts b/__tests__/integration/core-transaction-pool/connection.test.ts new file mode 100644 index 0000000000..dcfb0a2691 --- /dev/null +++ b/__tests__/integration/core-transaction-pool/connection.test.ts @@ -0,0 +1,183 @@ +import "jest-extended"; + +import { Container, TransactionPool } from "@arkecosystem/core-interfaces"; +import { Crypto, Identities, Networks, Transactions, Utils } from "@arkecosystem/crypto"; + +import { Connection } from "../../../packages/core-transaction-pool/src/connection"; +import { defaults } from "../../../packages/core-transaction-pool/src/defaults"; +import { ConnectionManager } from "../../../packages/core-transaction-pool/src/manager"; +import { Memory } from "../../../packages/core-transaction-pool/src/memory"; +import { Storage } from "../../../packages/core-transaction-pool/src/storage"; +import { WalletManager } from "../../../packages/core-transaction-pool/src/wallet-manager"; +import { setUpFull, tearDownFull } from "./__support__/setup"; + +const bignum = Utils.BigNumber.make; + +let container: Container.IContainer; +let connection: TransactionPool.IConnection; +let memory: Memory; + +beforeAll(async () => { + container = await setUpFull(); + + memory = new Memory(); + connection = container.resolvePlugin("transaction-pool"); + connection = await new ConnectionManager().createConnection( + new Connection({ + options: defaults, + walletManager: new WalletManager(), + memory, + storage: new Storage(), + }), + ); +}); + +afterAll(async () => { + await tearDownFull(); +}); + +beforeEach(() => { + connection.flush(); +}); + +describe("Connection", () => { + describe("getTransactionsForForging", () => { + const createTransaction = (transactionData, passphrase) => { + const sign = (transactionData, passphrase) => { + const keys = Identities.Keys.fromPassphrase(passphrase); + + transactionData.senderPublicKey = transactionData.senderPublicKey || keys.publicKey; + transactionData.id = Transactions.Utils.getId(transactionData); + + transactionData.signature = Transactions.Signer.sign(transactionData, keys); + + return transactionData; + }; + + const transaction = Transactions.TransactionTypeFactory.create(sign(transactionData, passphrase)); + + Transactions.Serializer.serialize(transaction); + + return Transactions.TransactionFactory.fromBytesUnsafe(transaction.serialized); + }; + + const createTransfer = options => { + const builder = Transactions.BuilderFactory.transfer() + .recipientId(Identities.Address.fromPassphrase("this is fine", Networks.unitnet.network.pubKeyHash)) + .amount("111") + .fee("11") + .expiration(11); + for (const [option, value] of Object.entries(options)) { + builder.data[option] = value; + } + + return createTransaction(builder.data, options.passphrase || "the sender passphrase"); + }; + + const addTransactionsToMemory = transactions => { + for (const tx of transactions) { + memory.remember(tx); + expect(memory.has(tx.id)).toBeTrue(); + } + expect(memory.count()).toBe(transactions.length); + }; + + it("should remove transactions that have expired", () => { + const transactions = [createTransfer({}), createTransfer({ amount: bignum(123), expiration: 1 })]; + + addTransactionsToMemory(transactions); + + expect(connection.getTransactionsForForging(100)).toEqual([transactions[0].serialized.toString("hex")]); + }); + + it("should remove transactions that have a fee of 0 or less", () => { + const transactions = [ + createTransfer({}), + createTransfer({ amount: bignum(1234), fee: bignum(-2) }), + createTransfer({ amount: bignum(2345), fee: bignum(0) }), + ]; + + addTransactionsToMemory(transactions); + + expect(connection.getTransactionsForForging(100)).toEqual([transactions[0].serialized.toString("hex")]); + }); + + it("should remove transactions that have an amount of 0 or less", () => { + const transactions = [ + createTransfer({}), + createTransfer({ amount: bignum(-2) }), + createTransfer({ amount: bignum(0) }), + ]; + + addTransactionsToMemory(transactions); + + expect(connection.getTransactionsForForging(100)).toEqual([transactions[0].serialized.toString("hex")]); + }); + + it("should remove transactions that have data from another network", () => { + const transactions = [ + createTransfer({}), + createTransfer({ + recipientId: Identities.Address.fromPassphrase("this is fine", Networks.devnet.network.pubKeyHash), + }), + ]; + + addTransactionsToMemory(transactions); + + expect(connection.getTransactionsForForging(100)).toEqual([transactions[0].serialized.toString("hex")]); + }); + + it.skip("should remove transactions that have wrong sender public keys", () => { + const transactions = [ + createTransfer({}), + createTransfer({ senderPublicKey: Identities.PublicKey.fromPassphrase("this is wrong") }), + ]; + + expect(transactions[1]).toEqual({}); + addTransactionsToMemory(transactions); + + expect(connection.getTransactionsForForging(100)).toEqual([transactions[0].serialized.toString("hex")]); + }); + + it.skip("should remove transactions that have timestamps in the future", () => { + const transactions = [ + createTransfer({}), + createTransfer({ amount: bignum(1234), timestamp: Crypto.Slots.getTime() + 100 * 1000 }), + ]; + + addTransactionsToMemory(transactions); + + expect(connection.getTransactionsForForging(100)).toEqual([transactions[0].serialized.toString("hex")]); + }); + + it.skip("should remove transactions that have different IDs when entering and leaving", () => { + const transactions = [createTransfer({}), createTransfer({ amount: bignum(1234), id: "64738638929" })]; + + expect(transactions[1]).toEqual({}); + addTransactionsToMemory(transactions); + + expect(connection.getTransactionsForForging(100)).toEqual([transactions[0].serialized.toString("hex")]); + }); + + it.todo("should remove transactions that have an unknown type"); + it.todo("should remove transactions that have unknown properties"); + it.todo("should remove transactions that have missing properties"); + it.todo("should remove transactions that have malformed properties"); + it.todo("should remove transactions that have malformed bytes"); + it.todo("should remove transactions that have additional bytes attached"); + it.todo("should remove transactions that have already been forged"); + it.todo("should remove transactions that have been persisted to the disk"); + it.todo("should remove transactions that have a disabled type"); + it.todo("should remove transactions that have have data of a another transaction type"); + it.todo("should remove transactions that have been altered after entering the pool"); + it.todo("should remove transactions that have negative numerical values"); + it.todo("should remove transactions that have malformed signatures"); + it.todo("should remove transactions that have malformed second signatures"); + it.todo("should remove transactions that have malformed multi signatures"); + it.todo("should remove transactions that fail to deserialize for unknown reasons"); + it.todo("should remove transactions that have a mismatch of expected and actual length of the vendor field"); + it.todo("should remove transactions that have an invalid vendor field length"); + it.todo("should remove transactions that have an invalid vendor field"); + it.todo("should remove transactions that have an invalid version"); + }); +}); From 25ade7e48982a3418c7d8c6a7abe649be51ca32d Mon Sep 17 00:00:00 2001 From: supaiku Date: Sat, 1 Jun 2019 03:13:44 +0200 Subject: [PATCH 02/34] test: extend TransactionFactory to build customized payloads --- .circleci/config.yml | 54 ++++++++++++++++-------- __tests__/helpers/transaction-factory.ts | 36 +++++++++++++++- 2 files changed, 71 insertions(+), 19 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 5becebe137..6cb518a3a9 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -30,7 +30,7 @@ jobs: - save_cache: key: 'core-node10-{{ checksum "checksum.txt" }}-unit-2' paths: - - ./packages/core-jest-matchers/node_modules + - ./packages/core-json-rpc/node_modules - ./packages/core-logger/node_modules - ./packages/core-logger-pino/node_modules - ./packages/core-logger-signale/node_modules @@ -70,6 +70,7 @@ jobs: - ./packages/core-forger/node_modules - ./packages/core-http-utils/node_modules - ./packages/core-interfaces/node_modules + - ./packages/core-jest-matchers/node_modules - run: name: Create .core/database directory command: mkdir -p $HOME/.core/database @@ -118,7 +119,7 @@ jobs: - save_cache: key: 'core-node11-{{ checksum "checksum.txt" }}-unit-2' paths: - - ./packages/core-jest-matchers/node_modules + - ./packages/core-json-rpc/node_modules - ./packages/core-logger/node_modules - ./packages/core-logger-pino/node_modules - ./packages/core-logger-signale/node_modules @@ -158,6 +159,7 @@ jobs: - ./packages/core-forger/node_modules - ./packages/core-http-utils/node_modules - ./packages/core-interfaces/node_modules + - ./packages/core-jest-matchers/node_modules - run: name: Create .core/database directory command: mkdir -p $HOME/.core/database @@ -206,7 +208,7 @@ jobs: - save_cache: key: 'core-node12-{{ checksum "checksum.txt" }}-unit-2' paths: - - ./packages/core-jest-matchers/node_modules + - ./packages/core-json-rpc/node_modules - ./packages/core-logger/node_modules - ./packages/core-logger-pino/node_modules - ./packages/core-logger-signale/node_modules @@ -246,6 +248,7 @@ jobs: - ./packages/core-forger/node_modules - ./packages/core-http-utils/node_modules - ./packages/core-interfaces/node_modules + - ./packages/core-jest-matchers/node_modules - run: name: Create .core/database directory command: mkdir -p $HOME/.core/database @@ -302,7 +305,7 @@ jobs: - save_cache: key: 'core-node10-{{ checksum "checksum.txt" }}-functional-2' paths: - - ./packages/core-jest-matchers/node_modules + - ./packages/core-json-rpc/node_modules - ./packages/core-logger/node_modules - ./packages/core-logger-pino/node_modules - ./packages/core-logger-signale/node_modules @@ -342,6 +345,7 @@ jobs: - ./packages/core-forger/node_modules - ./packages/core-http-utils/node_modules - ./packages/core-interfaces/node_modules + - ./packages/core-jest-matchers/node_modules - run: name: Create .core/database directory command: mkdir -p $HOME/.core/database @@ -398,7 +402,7 @@ jobs: - save_cache: key: 'core-node11-{{ checksum "checksum.txt" }}-functional-2' paths: - - ./packages/core-jest-matchers/node_modules + - ./packages/core-json-rpc/node_modules - ./packages/core-logger/node_modules - ./packages/core-logger-pino/node_modules - ./packages/core-logger-signale/node_modules @@ -438,6 +442,7 @@ jobs: - ./packages/core-forger/node_modules - ./packages/core-http-utils/node_modules - ./packages/core-interfaces/node_modules + - ./packages/core-jest-matchers/node_modules - run: name: Create .core/database directory command: mkdir -p $HOME/.core/database @@ -494,7 +499,7 @@ jobs: - save_cache: key: 'core-node12-{{ checksum "checksum.txt" }}-functional-2' paths: - - ./packages/core-jest-matchers/node_modules + - ./packages/core-json-rpc/node_modules - ./packages/core-logger/node_modules - ./packages/core-logger-pino/node_modules - ./packages/core-logger-signale/node_modules @@ -534,6 +539,7 @@ jobs: - ./packages/core-forger/node_modules - ./packages/core-http-utils/node_modules - ./packages/core-interfaces/node_modules + - ./packages/core-jest-matchers/node_modules - run: name: Create .core/database directory command: mkdir -p $HOME/.core/database @@ -590,7 +596,7 @@ jobs: - save_cache: key: 'core-node10-{{ checksum "checksum.txt" }}-1-2' paths: - - ./packages/core-jest-matchers/node_modules + - ./packages/core-json-rpc/node_modules - ./packages/core-logger/node_modules - ./packages/core-logger-pino/node_modules - ./packages/core-logger-signale/node_modules @@ -630,6 +636,7 @@ jobs: - ./packages/core-forger/node_modules - ./packages/core-http-utils/node_modules - ./packages/core-interfaces/node_modules + - ./packages/core-jest-matchers/node_modules - run: name: Create .core/database directory command: mkdir -p $HOME/.core/database @@ -705,7 +712,7 @@ jobs: - save_cache: key: 'core-node11-{{ checksum "checksum.txt" }}-1-2' paths: - - ./packages/core-jest-matchers/node_modules + - ./packages/core-json-rpc/node_modules - ./packages/core-logger/node_modules - ./packages/core-logger-pino/node_modules - ./packages/core-logger-signale/node_modules @@ -745,6 +752,7 @@ jobs: - ./packages/core-forger/node_modules - ./packages/core-http-utils/node_modules - ./packages/core-interfaces/node_modules + - ./packages/core-jest-matchers/node_modules - run: name: Create .core/database directory command: mkdir -p $HOME/.core/database @@ -820,7 +828,7 @@ jobs: - save_cache: key: 'core-node12-{{ checksum "checksum.txt" }}-1-2' paths: - - ./packages/core-jest-matchers/node_modules + - ./packages/core-json-rpc/node_modules - ./packages/core-logger/node_modules - ./packages/core-logger-pino/node_modules - ./packages/core-logger-signale/node_modules @@ -860,6 +868,7 @@ jobs: - ./packages/core-forger/node_modules - ./packages/core-http-utils/node_modules - ./packages/core-interfaces/node_modules + - ./packages/core-jest-matchers/node_modules - run: name: Create .core/database directory command: mkdir -p $HOME/.core/database @@ -924,7 +933,7 @@ jobs: - save_cache: key: 'core-node10-{{ checksum "checksum.txt" }}-benchmark-2' paths: - - ./packages/core-jest-matchers/node_modules + - ./packages/core-json-rpc/node_modules - ./packages/core-logger/node_modules - ./packages/core-logger-pino/node_modules - ./packages/core-logger-signale/node_modules @@ -964,6 +973,7 @@ jobs: - ./packages/core-forger/node_modules - ./packages/core-http-utils/node_modules - ./packages/core-interfaces/node_modules + - ./packages/core-jest-matchers/node_modules - run: name: Benchmark command: cd ~/core && yarn bench @@ -994,7 +1004,7 @@ jobs: - save_cache: key: 'core-node11-{{ checksum "checksum.txt" }}-benchmark-2' paths: - - ./packages/core-jest-matchers/node_modules + - ./packages/core-json-rpc/node_modules - ./packages/core-logger/node_modules - ./packages/core-logger-pino/node_modules - ./packages/core-logger-signale/node_modules @@ -1034,6 +1044,7 @@ jobs: - ./packages/core-forger/node_modules - ./packages/core-http-utils/node_modules - ./packages/core-interfaces/node_modules + - ./packages/core-jest-matchers/node_modules - run: name: Benchmark command: cd ~/core && yarn bench @@ -1064,7 +1075,7 @@ jobs: - save_cache: key: 'core-node12-{{ checksum "checksum.txt" }}-benchmark-2' paths: - - ./packages/core-jest-matchers/node_modules + - ./packages/core-json-rpc/node_modules - ./packages/core-logger/node_modules - ./packages/core-logger-pino/node_modules - ./packages/core-logger-signale/node_modules @@ -1104,6 +1115,7 @@ jobs: - ./packages/core-forger/node_modules - ./packages/core-http-utils/node_modules - ./packages/core-interfaces/node_modules + - ./packages/core-jest-matchers/node_modules - run: name: Benchmark command: cd ~/core && yarn bench @@ -1142,7 +1154,7 @@ jobs: - save_cache: key: 'ark-node10-e2e-{{ checksum "checksum.txt" }}-1-2' paths: - - ./packages/core-jest-matchers/node_modules + - ./packages/core-json-rpc/node_modules - ./packages/core-logger/node_modules - ./packages/core-logger-pino/node_modules - ./packages/core-logger-signale/node_modules @@ -1182,6 +1194,7 @@ jobs: - ./packages/core-forger/node_modules - ./packages/core-http-utils/node_modules - ./packages/core-interfaces/node_modules + - ./packages/core-jest-matchers/node_modules - run: name: Generate files command: >- @@ -1262,7 +1275,7 @@ jobs: - save_cache: key: 'ark-node11-e2e-{{ checksum "checksum.txt" }}-2-2' paths: - - ./packages/core-jest-matchers/node_modules + - ./packages/core-json-rpc/node_modules - ./packages/core-logger/node_modules - ./packages/core-logger-pino/node_modules - ./packages/core-logger-signale/node_modules @@ -1302,6 +1315,7 @@ jobs: - ./packages/core-forger/node_modules - ./packages/core-http-utils/node_modules - ./packages/core-interfaces/node_modules + - ./packages/core-jest-matchers/node_modules - run: name: Generate files command: >- @@ -1382,7 +1396,7 @@ jobs: - save_cache: key: 'ark-node12-e2e-{{ checksum "checksum.txt" }}-2-2' paths: - - ./packages/core-jest-matchers/node_modules + - ./packages/core-json-rpc/node_modules - ./packages/core-logger/node_modules - ./packages/core-logger-pino/node_modules - ./packages/core-logger-signale/node_modules @@ -1422,6 +1436,7 @@ jobs: - ./packages/core-forger/node_modules - ./packages/core-http-utils/node_modules - ./packages/core-interfaces/node_modules + - ./packages/core-jest-matchers/node_modules - run: name: Generate files command: >- @@ -1505,7 +1520,7 @@ jobs: - save_cache: key: 'core-node10-{{ checksum "checksum.txt" }}-1-2' paths: - - ./packages/core-jest-matchers/node_modules + - ./packages/core-json-rpc/node_modules - ./packages/core-logger/node_modules - ./packages/core-logger-pino/node_modules - ./packages/core-logger-signale/node_modules @@ -1545,6 +1560,7 @@ jobs: - ./packages/core-forger/node_modules - ./packages/core-http-utils/node_modules - ./packages/core-interfaces/node_modules + - ./packages/core-jest-matchers/node_modules - run: name: Create .core/database directory command: mkdir -p $HOME/.core/database @@ -1620,7 +1636,7 @@ jobs: - save_cache: key: 'core-node11-{{ checksum "checksum.txt" }}-1-2' paths: - - ./packages/core-jest-matchers/node_modules + - ./packages/core-json-rpc/node_modules - ./packages/core-logger/node_modules - ./packages/core-logger-pino/node_modules - ./packages/core-logger-signale/node_modules @@ -1660,6 +1676,7 @@ jobs: - ./packages/core-forger/node_modules - ./packages/core-http-utils/node_modules - ./packages/core-interfaces/node_modules + - ./packages/core-jest-matchers/node_modules - run: name: Create .core/database directory command: mkdir -p $HOME/.core/database @@ -1735,7 +1752,7 @@ jobs: - save_cache: key: 'core-node12-{{ checksum "checksum.txt" }}-1-2' paths: - - ./packages/core-jest-matchers/node_modules + - ./packages/core-json-rpc/node_modules - ./packages/core-logger/node_modules - ./packages/core-logger-pino/node_modules - ./packages/core-logger-signale/node_modules @@ -1775,6 +1792,7 @@ jobs: - ./packages/core-forger/node_modules - ./packages/core-http-utils/node_modules - ./packages/core-interfaces/node_modules + - ./packages/core-jest-matchers/node_modules - run: name: Create .core/database directory command: mkdir -p $HOME/.core/database diff --git a/__tests__/helpers/transaction-factory.ts b/__tests__/helpers/transaction-factory.ts index 1ca54e5de5..ad37f685b7 100644 --- a/__tests__/helpers/transaction-factory.ts +++ b/__tests__/helpers/transaction-factory.ts @@ -1,4 +1,5 @@ import { Identities, Interfaces, Managers, Transactions, Types, Utils } from "@arkecosystem/crypto"; +import cloneDeep from "lodash.clonedeep"; import { secrets } from "../utils/config/testnet/delegates.json"; const defaultPassphrase: string = secrets[0]; @@ -92,6 +93,10 @@ export class TransactionFactory { private version: number; private senderPublicKey: string; private expiration: number; + private customizedPayload: Array>; + private customizedPayloadOptions: { + quantity?: number; + }; public constructor(builder) { this.builder = builder; @@ -172,6 +177,17 @@ export class TransactionFactory { return this.create(1)[0]; } + public withCustomizedPayload( + customizedPayload: Array>, + options: { + quantity?: number; + } = {}, + ): TransactionFactory { + this.customizedPayload = customizedPayload; + this.customizedPayloadOptions = options; + return this; + } + public build(quantity: number = 1): Interfaces.ITransaction[] { return this.make(quantity, "build"); } @@ -194,6 +210,8 @@ export class TransactionFactory { const transactions: T[] = []; + let countMalformed: number = 0; + for (let i = 0; i < quantity; i++) { if (this.builder.constructor.name === "TransferBuilder") { // @FIXME: when we use any of the "withPassphrase*" methods the builder will @@ -248,7 +266,23 @@ export class TransactionFactory { } } - transactions.push(this.builder[method]()); + if ( + this.customizedPayload && + (!this.customizedPayloadOptions.quantity || this.customizedPayloadOptions.quantity > countMalformed) + ) { + countMalformed++; + + const builderData = this.builder.data; + this.builder.data = Object.assign( + cloneDeep(this.builder.data), + this.customizedPayload[countMalformed % this.customizedPayload.length], + ); + + transactions.push(this.builder[method]()); + this.builder.data = builderData; + } else { + transactions.push(this.builder[method]()); + } } return transactions; From b82deeef3e4264d54a49eaf46f3b987dc2713f04 Mon Sep 17 00:00:00 2001 From: supaiku Date: Sat, 1 Jun 2019 03:14:11 +0200 Subject: [PATCH 03/34] test: add types to delegates --- __tests__/utils/fixtures/testnet/delegates.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/__tests__/utils/fixtures/testnet/delegates.ts b/__tests__/utils/fixtures/testnet/delegates.ts index d591d84077..34a618ced3 100644 --- a/__tests__/utils/fixtures/testnet/delegates.ts +++ b/__tests__/utils/fixtures/testnet/delegates.ts @@ -1,19 +1,20 @@ -import { Identities, Managers } from "@arkecosystem/crypto"; - -/** - * Get the testnet genesis delegates information - * @return {Array} array of objects like { secret, publicKey, address, balance } - */ +import { Identities, Managers, Utils } from "@arkecosystem/crypto"; Managers.configManager.setFromPreset("testnet"); import { secrets } from "../../config/testnet/delegates.json"; import { genesisBlock } from "../../config/testnet/genesisBlock"; -export const delegates: any = secrets.map(secret => { +export const delegates: Array<{ + secret: string; + passphrase: string; + publicKey: string; + address: string; + balance: Utils.BigNumber; +}> = secrets.map(secret => { const publicKey: string = Identities.PublicKey.fromPassphrase(secret); const address: string = Identities.Address.fromPassphrase(secret); - const balance = genesisBlock.transactions.find( + const balance: Utils.BigNumber = genesisBlock.transactions.find( transaction => transaction.recipientId === address && transaction.type === 0, ).amount; return { From a85bb3e44ad47d85c5a0eccabd4f4bbc580670ff Mon Sep 17 00:00:00 2001 From: supaiku Date: Sat, 1 Jun 2019 03:15:03 +0200 Subject: [PATCH 04/34] test: turn getTransactionsForForging tests into unit tests --- .../connection.forging.test.ts | 187 ++++++++++++++++++ 1 file changed, 187 insertions(+) create mode 100644 __tests__/unit/core-transaction-pool/connection.forging.test.ts diff --git a/__tests__/unit/core-transaction-pool/connection.forging.test.ts b/__tests__/unit/core-transaction-pool/connection.forging.test.ts new file mode 100644 index 0000000000..a74d1481c6 --- /dev/null +++ b/__tests__/unit/core-transaction-pool/connection.forging.test.ts @@ -0,0 +1,187 @@ +import "jest-extended"; + +import "./mocks/core-container"; + +import { Wallets } from "@arkecosystem/core-state"; + +// @ts-ignore +import { Constants, Identities, Interfaces, Managers, Transactions, Utils } from "@arkecosystem/crypto"; +import { Connection } from "../../../packages/core-transaction-pool/src/connection"; +import { defaults } from "../../../packages/core-transaction-pool/src/defaults"; +import { Memory } from "../../../packages/core-transaction-pool/src/memory"; +import { Storage } from "../../../packages/core-transaction-pool/src/storage"; +import { WalletManager } from "../../../packages/core-transaction-pool/src/wallet-manager"; +import { TransactionFactory } from "../../helpers/transaction-factory"; +import { delegates } from "../../utils/fixtures/testnet/delegates"; + +let connection: Connection; +let memory: Memory; +let poolWalletManager: WalletManager; +let databaseWalletManager: Wallets.WalletManager; + +beforeAll(async () => { + Managers.configManager.setFromPreset("testnet"); + + memory = new Memory(); + poolWalletManager = new WalletManager(); + connection = new Connection({ + options: defaults, + walletManager: poolWalletManager, + memory, + storage: new Storage(), + }); + + await connection.make(); + + jest.spyOn(Transactions.Verifier, "verify").mockReturnValue(true); +}); + +const mockCurrentHeight = (height: number) => { + // @ts-ignore + jest.spyOn(memory, "currentHeight").mockReturnValue(height); + Managers.configManager.setHeight(height); +}; + +beforeEach(() => { + mockCurrentHeight(1); + + connection.flush(); + poolWalletManager.reset(); + + databaseWalletManager = new Wallets.WalletManager(); + + for (let i = 0; i < delegates.length; i++) { + const { publicKey } = delegates[i]; + const wallet = databaseWalletManager.findByPublicKey(publicKey); + wallet.balance = Utils.BigNumber.make(100_000 * Constants.ARKTOSHI); + wallet.username = `delegate-${i + 1}`; + wallet.vote = publicKey; + + databaseWalletManager.reindex(wallet); + } + + databaseWalletManager.buildDelegateRanking(); + databaseWalletManager.buildVoteBalances(); + + // @ts-ignore + connection.databaseService.walletManager = databaseWalletManager; +}); + +describe("Connection", () => { + const addTransactionsToMemory = transactions => { + for (const tx of transactions) { + memory.remember(tx); + expect(memory.has(tx.id)).toBeTrue(); + } + expect(memory.count()).toBe(transactions.length); + }; + + describe("getTransactionsForForging", () => { + it("should remove transactions that have expired [5 Good, 5 Bad]", () => { + mockCurrentHeight(100); + + const transactions = TransactionFactory.transfer() + .withCustomizedPayload([{ expiration: 1 }], { quantity: 5 }) + .build(10); + + addTransactionsToMemory(transactions); + + const forgingTransactions = connection.getTransactionsForForging(100); + + expect(forgingTransactions).toHaveLength(5); + expect(forgingTransactions).toEqual( + transactions.slice(5).map(({ serialized }) => serialized.toString("hex")), + ); + }); + + // it("should remove transactions that have a fee of 0 or less", () => { + // const transactions = [ + // createTransfer({}), + // createTransfer({ amount: bignum(1234), fee: bignum(-2) }), + // createTransfer({ amount: bignum(2345), fee: bignum(0) }), + // ]; + + // addTransactionsToMemory(transactions); + + // expect(connection.getTransactionsForForging(100)).toEqual([transactions[0].serialized.toString("hex")]); + // }); + + // it("should remove transactions that have an amount of 0 or less", () => { + // const transactions = [ + // createTransfer({}), + // createTransfer({ amount: bignum(-2) }), + // createTransfer({ amount: bignum(0) }), + // ]; + + // addTransactionsToMemory(transactions); + + // expect(connection.getTransactionsForForging(100)).toEqual([transactions[0].serialized.toString("hex")]); + // }); + + // it("should remove transactions that have data from another network", () => { + // const transactions = [ + // createTransfer({}), + // createTransfer({ + // recipientId: Identities.Address.fromPassphrase("this is fine", Networks.devnet.network.pubKeyHash), + // }), + // ]; + + // addTransactionsToMemory(transactions); + + // expect(connection.getTransactionsForForging(100)).toEqual([transactions[0].serialized.toString("hex")]); + // }); + + // it.skip("should remove transactions that have wrong sender public keys", () => { + // const transactions = [ + // createTransfer({}), + // createTransfer({ senderPublicKey: Identities.PublicKey.fromPassphrase("this is wrong") }), + // ]; + + // expect(transactions[1]).toEqual({}); + // addTransactionsToMemory(transactions); + + // expect(connection.getTransactionsForForging(100)).toEqual([transactions[0].serialized.toString("hex")]); + // }); + + // it.skip("should remove transactions that have timestamps in the future", () => { + // const transactions = [ + // createTransfer({}), + // createTransfer({ amount: bignum(1234), timestamp: Crypto.Slots.getTime() + 100 * 1000 }), + // ]; + + // addTransactionsToMemory(transactions); + + // expect(connection.getTransactionsForForging(100)).toEqual([transactions[0].serialized.toString("hex")]); + // }); + + // it.skip("should remove transactions that have different IDs when entering and leaving", () => { + // const transactions = [createTransfer({}), createTransfer({ amount: bignum(1234), id: "64738638929" })]; + + // expect(transactions[1]).toEqual({}); + // addTransactionsToMemory(transactions); + + // expect(connection.getTransactionsForForging(100)).toEqual([transactions[0].serialized.toString("hex")]); + // }); + + it.todo("should remove transactions that have an unknown type"); + it.todo("should remove transactions that have unknown properties"); + it.todo("should remove transactions that have missing properties"); + it.todo("should remove transactions that have malformed properties"); + it.todo("should remove transactions that have malformed bytes"); + it.todo("should remove transactions that have additional bytes attached"); + it.todo("should remove transactions that have already been forged"); + it.todo("should remove transactions that have been persisted to the disk"); + it.todo("should remove transactions that have a disabled type"); + it.todo("should remove transactions that have have data of a another transaction type"); + it.todo("should remove transactions that have been altered after entering the pool"); + it.todo("should remove transactions that have negative numerical values"); + it.todo("should remove transactions that have malformed signatures"); + it.todo("should remove transactions that have malformed second signatures"); + it.todo("should remove transactions that have malformed multi signatures"); + it.todo("should remove transactions that fail to deserialize for unknown reasons"); + it.todo("should remove transactions that have a mismatch of expected and actual length of the vendor field"); + it.todo("should remove transactions that have an invalid vendor field length"); + it.todo("should remove transactions that have an invalid vendor field"); + it.todo("should remove transactions that have an invalid version"); + }); +}); From 1e7a220fc5e2ddc2979250582ac69cba5798bfc8 Mon Sep 17 00:00:00 2001 From: supaiku Date: Sat, 1 Jun 2019 04:01:51 +0200 Subject: [PATCH 05/34] test: tweak TransactionFactory --- __tests__/helpers/transaction-factory.ts | 35 +++++++++++++++++------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/__tests__/helpers/transaction-factory.ts b/__tests__/helpers/transaction-factory.ts index ad37f685b7..59892d7116 100644 --- a/__tests__/helpers/transaction-factory.ts +++ b/__tests__/helpers/transaction-factory.ts @@ -210,7 +210,7 @@ export class TransactionFactory { const transactions: T[] = []; - let countMalformed: number = 0; + let countCustomized: number = 0; for (let i = 0; i < quantity; i++) { if (this.builder.constructor.name === "TransferBuilder") { @@ -268,18 +268,33 @@ export class TransactionFactory { if ( this.customizedPayload && - (!this.customizedPayloadOptions.quantity || this.customizedPayloadOptions.quantity > countMalformed) + (!this.customizedPayloadOptions.quantity || this.customizedPayloadOptions.quantity > countCustomized) ) { - countMalformed++; + countCustomized++; - const builderData = this.builder.data; - this.builder.data = Object.assign( - cloneDeep(this.builder.data), - this.customizedPayload[countMalformed % this.customizedPayload.length], - ); + const payload = this.customizedPayload[countCustomized % this.customizedPayload.length]; + const transaction: Interfaces.ITransaction = this.builder[method](); + transaction.data = Object.assign(cloneDeep(transaction.data), payload); - transactions.push(this.builder[method]()); - this.builder.data = builderData; + if (!payload.signature) { + transaction.data.signature = Transactions.Signer.sign( + transaction.data, + Identities.Keys.fromPassphrase(this.passphrase), + ); + } + + // TODO: second sig + // TODO: multi sig + + if (!payload.id) { + transaction.data.id = Transactions.Utils.getId(transaction.data); + } + + // TODO: exclude certain malformed properties that cannot be serialized properly + // like signatures that are too long. Add support for customizing signatures. + Transactions.Serializer.serialize(transaction); + + transactions.push((transaction as unknown) as T); } else { transactions.push(this.builder[method]()); } From ddfb52793a8d68b916d88936b4d8067d47bd529b Mon Sep 17 00:00:00 2001 From: supaiku Date: Sat, 1 Jun 2019 04:02:22 +0200 Subject: [PATCH 06/34] test: restore all commented tests --- .../connection.forging.test.ts | 178 +++++++++--------- 1 file changed, 93 insertions(+), 85 deletions(-) diff --git a/__tests__/unit/core-transaction-pool/connection.forging.test.ts b/__tests__/unit/core-transaction-pool/connection.forging.test.ts index a74d1481c6..ffd2c1d216 100644 --- a/__tests__/unit/core-transaction-pool/connection.forging.test.ts +++ b/__tests__/unit/core-transaction-pool/connection.forging.test.ts @@ -5,7 +5,16 @@ import "./mocks/core-container"; import { Wallets } from "@arkecosystem/core-state"; // @ts-ignore -import { Constants, Identities, Interfaces, Managers, Transactions, Utils } from "@arkecosystem/crypto"; +import { + Constants, + Crypto, + Identities, + Interfaces, + Managers, + Networks, + Transactions, + Utils, +} from "@arkecosystem/crypto"; import { Connection } from "../../../packages/core-transaction-pool/src/connection"; import { defaults } from "../../../packages/core-transaction-pool/src/defaults"; import { Memory } from "../../../packages/core-transaction-pool/src/memory"; @@ -42,32 +51,32 @@ const mockCurrentHeight = (height: number) => { Managers.configManager.setHeight(height); }; -beforeEach(() => { - mockCurrentHeight(1); +describe("Connection", () => { + beforeEach(() => { + mockCurrentHeight(1); - connection.flush(); - poolWalletManager.reset(); + connection.flush(); + poolWalletManager.reset(); - databaseWalletManager = new Wallets.WalletManager(); + databaseWalletManager = new Wallets.WalletManager(); - for (let i = 0; i < delegates.length; i++) { - const { publicKey } = delegates[i]; - const wallet = databaseWalletManager.findByPublicKey(publicKey); - wallet.balance = Utils.BigNumber.make(100_000 * Constants.ARKTOSHI); - wallet.username = `delegate-${i + 1}`; - wallet.vote = publicKey; + for (let i = 0; i < delegates.length; i++) { + const { publicKey } = delegates[i]; + const wallet = databaseWalletManager.findByPublicKey(publicKey); + wallet.balance = Utils.BigNumber.make(100_000 * Constants.ARKTOSHI); + wallet.username = `delegate-${i + 1}`; + wallet.vote = publicKey; - databaseWalletManager.reindex(wallet); - } + databaseWalletManager.reindex(wallet); + } - databaseWalletManager.buildDelegateRanking(); - databaseWalletManager.buildVoteBalances(); + databaseWalletManager.buildDelegateRanking(); + databaseWalletManager.buildVoteBalances(); - // @ts-ignore - connection.databaseService.walletManager = databaseWalletManager; -}); + // @ts-ignore + connection.databaseService.walletManager = databaseWalletManager; + }); -describe("Connection", () => { const addTransactionsToMemory = transactions => { for (const tx of transactions) { memory.remember(tx); @@ -76,6 +85,16 @@ describe("Connection", () => { expect(memory.count()).toBe(transactions.length); }; + const expectForgingTransactions = (transactions: Interfaces.ITransaction[], countGood: number) => { + addTransactionsToMemory(transactions); + + const forgingTransactions = connection.getTransactionsForForging(100); + expect(forgingTransactions).toHaveLength(countGood); + expect(forgingTransactions).toEqual( + transactions.slice(transactions.length - countGood).map(({ serialized }) => serialized.toString("hex")), + ); + }; + describe("getTransactionsForForging", () => { it("should remove transactions that have expired [5 Good, 5 Bad]", () => { mockCurrentHeight(100); @@ -84,84 +103,73 @@ describe("Connection", () => { .withCustomizedPayload([{ expiration: 1 }], { quantity: 5 }) .build(10); - addTransactionsToMemory(transactions); - - const forgingTransactions = connection.getTransactionsForForging(100); - - expect(forgingTransactions).toHaveLength(5); - expect(forgingTransactions).toEqual( - transactions.slice(5).map(({ serialized }) => serialized.toString("hex")), - ); + expectForgingTransactions(transactions, 5); }); - // it("should remove transactions that have a fee of 0 or less", () => { - // const transactions = [ - // createTransfer({}), - // createTransfer({ amount: bignum(1234), fee: bignum(-2) }), - // createTransfer({ amount: bignum(2345), fee: bignum(0) }), - // ]; - - // addTransactionsToMemory(transactions); - - // expect(connection.getTransactionsForForging(100)).toEqual([transactions[0].serialized.toString("hex")]); - // }); - - // it("should remove transactions that have an amount of 0 or less", () => { - // const transactions = [ - // createTransfer({}), - // createTransfer({ amount: bignum(-2) }), - // createTransfer({ amount: bignum(0) }), - // ]; - - // addTransactionsToMemory(transactions); - - // expect(connection.getTransactionsForForging(100)).toEqual([transactions[0].serialized.toString("hex")]); - // }); - - // it("should remove transactions that have data from another network", () => { - // const transactions = [ - // createTransfer({}), - // createTransfer({ - // recipientId: Identities.Address.fromPassphrase("this is fine", Networks.devnet.network.pubKeyHash), - // }), - // ]; + it("should remove transactions that have a fee of 0 or less [8 Good, 2 Bad]", () => { + const transactions = TransactionFactory.transfer() + .withCustomizedPayload( + [ + { amount: Utils.BigNumber.make(1000), fee: Utils.BigNumber.ZERO }, + { amount: Utils.BigNumber.make(1000), fee: Utils.BigNumber.make(-100) }, + ], + { quantity: 2 }, + ) + .build(10); - // addTransactionsToMemory(transactions); + expectForgingTransactions(transactions, 8); + }); - // expect(connection.getTransactionsForForging(100)).toEqual([transactions[0].serialized.toString("hex")]); - // }); + it("should remove transactions that have an amount of 0 or less [8 Good, 2 Bad]", () => { + const transactions = TransactionFactory.transfer() + .withCustomizedPayload( + [ + { amount: Utils.BigNumber.ZERO, fee: Utils.BigNumber.ONE }, + { amount: Utils.BigNumber.make(-1), fee: Utils.BigNumber.ONE }, + ], + { quantity: 2 }, + ) + .build(10); - // it.skip("should remove transactions that have wrong sender public keys", () => { - // const transactions = [ - // createTransfer({}), - // createTransfer({ senderPublicKey: Identities.PublicKey.fromPassphrase("this is wrong") }), - // ]; + expectForgingTransactions(transactions, 8); + }); - // expect(transactions[1]).toEqual({}); - // addTransactionsToMemory(transactions); + it("should remove transactions that have data from another network [5 Good, 5 Bad]", () => { + const transactions = TransactionFactory.transfer() + .withCustomizedPayload( + [{ recipientId: Identities.Address.fromPassphrase("secret", Networks.devnet.network.pubKeyHash) }], + { quantity: 5 }, + ) + .build(10); - // expect(connection.getTransactionsForForging(100)).toEqual([transactions[0].serialized.toString("hex")]); - // }); + expectForgingTransactions(transactions, 5); + }); - // it.skip("should remove transactions that have timestamps in the future", () => { - // const transactions = [ - // createTransfer({}), - // createTransfer({ amount: bignum(1234), timestamp: Crypto.Slots.getTime() + 100 * 1000 }), - // ]; + it("should remove transactions that have wrong sender public keys [5 Good, 5 Bad]", () => { + const transactions = TransactionFactory.transfer() + .withCustomizedPayload([{ senderPublicKey: Identities.PublicKey.fromPassphrase("this is wrong") }], { + quantity: 5, + }) + .build(10); - // addTransactionsToMemory(transactions); + expectForgingTransactions(transactions, 5); + }); - // expect(connection.getTransactionsForForging(100)).toEqual([transactions[0].serialized.toString("hex")]); - // }); + it("should remove transactions that have timestamps in the future [5 Good, 5 Bad]", () => { + const transactions = TransactionFactory.transfer() + .withCustomizedPayload([{ timestamp: Crypto.Slots.getTime() + 100 * 1000 }], { quantity: 5 }) + .build(10); - // it.skip("should remove transactions that have different IDs when entering and leaving", () => { - // const transactions = [createTransfer({}), createTransfer({ amount: bignum(1234), id: "64738638929" })]; + expectForgingTransactions(transactions, 5); + }); - // expect(transactions[1]).toEqual({}); - // addTransactionsToMemory(transactions); + it("should remove transactions that have different IDs when entering and leaving [8 Good, 2 Bad]", () => { + const transactions = TransactionFactory.transfer() + .withCustomizedPayload([{ id: "garbage" }, { id: "garbage 2" }], { quantity: 2 }) + .build(10); - // expect(connection.getTransactionsForForging(100)).toEqual([transactions[0].serialized.toString("hex")]); - // }); + expectForgingTransactions(transactions, 8); + }); it.todo("should remove transactions that have an unknown type"); it.todo("should remove transactions that have unknown properties"); From e1bbc83c30d0d0605fab63d0358a7d6fe817c898 Mon Sep 17 00:00:00 2001 From: supaiku Date: Sat, 1 Jun 2019 04:02:49 +0200 Subject: [PATCH 07/34] test: add log for debugging --- packages/core-transaction-pool/src/connection.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core-transaction-pool/src/connection.ts b/packages/core-transaction-pool/src/connection.ts index ff3b5fd0e9..ff2fa94fe8 100644 --- a/packages/core-transaction-pool/src/connection.ts +++ b/packages/core-transaction-pool/src/connection.ts @@ -168,7 +168,7 @@ export class Connection implements TransactionPool.IConnection { transactions.push(deserialized.serialized.toString("hex")); } catch (error) { this.removeTransactionById(transaction.id); - + console.error(error); this.logger.error(`Removed ${transaction.id} before forging because it is no longer valid.`); } } From e70efe6d383dcfcedb38979e98c62c780564f148 Mon Sep 17 00:00:00 2001 From: supaiku Date: Sat, 1 Jun 2019 04:06:03 +0200 Subject: [PATCH 08/34] test: disable verifier mock --- .../connection.forging.test.ts | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/__tests__/unit/core-transaction-pool/connection.forging.test.ts b/__tests__/unit/core-transaction-pool/connection.forging.test.ts index ffd2c1d216..f456665eb0 100644 --- a/__tests__/unit/core-transaction-pool/connection.forging.test.ts +++ b/__tests__/unit/core-transaction-pool/connection.forging.test.ts @@ -4,17 +4,7 @@ import "./mocks/core-container"; import { Wallets } from "@arkecosystem/core-state"; -// @ts-ignore -import { - Constants, - Crypto, - Identities, - Interfaces, - Managers, - Networks, - Transactions, - Utils, -} from "@arkecosystem/crypto"; +import { Constants, Crypto, Identities, Interfaces, Managers, Networks, Utils } from "@arkecosystem/crypto"; import { Connection } from "../../../packages/core-transaction-pool/src/connection"; import { defaults } from "../../../packages/core-transaction-pool/src/defaults"; import { Memory } from "../../../packages/core-transaction-pool/src/memory"; @@ -42,7 +32,7 @@ beforeAll(async () => { await connection.make(); - jest.spyOn(Transactions.Verifier, "verify").mockReturnValue(true); + // jest.spyOn(Transactions.Verifier, "verify").mockReturnValue(true); }); const mockCurrentHeight = (height: number) => { From 684f34617c532bd653570202db2073c77d3d30c3 Mon Sep 17 00:00:00 2001 From: supaiku Date: Sat, 1 Jun 2019 04:09:06 +0200 Subject: [PATCH 09/34] test: delete obsolete file --- .../core-transaction-pool/connection.test.ts | 183 ------------------ 1 file changed, 183 deletions(-) delete mode 100644 __tests__/integration/core-transaction-pool/connection.test.ts diff --git a/__tests__/integration/core-transaction-pool/connection.test.ts b/__tests__/integration/core-transaction-pool/connection.test.ts deleted file mode 100644 index dcfb0a2691..0000000000 --- a/__tests__/integration/core-transaction-pool/connection.test.ts +++ /dev/null @@ -1,183 +0,0 @@ -import "jest-extended"; - -import { Container, TransactionPool } from "@arkecosystem/core-interfaces"; -import { Crypto, Identities, Networks, Transactions, Utils } from "@arkecosystem/crypto"; - -import { Connection } from "../../../packages/core-transaction-pool/src/connection"; -import { defaults } from "../../../packages/core-transaction-pool/src/defaults"; -import { ConnectionManager } from "../../../packages/core-transaction-pool/src/manager"; -import { Memory } from "../../../packages/core-transaction-pool/src/memory"; -import { Storage } from "../../../packages/core-transaction-pool/src/storage"; -import { WalletManager } from "../../../packages/core-transaction-pool/src/wallet-manager"; -import { setUpFull, tearDownFull } from "./__support__/setup"; - -const bignum = Utils.BigNumber.make; - -let container: Container.IContainer; -let connection: TransactionPool.IConnection; -let memory: Memory; - -beforeAll(async () => { - container = await setUpFull(); - - memory = new Memory(); - connection = container.resolvePlugin("transaction-pool"); - connection = await new ConnectionManager().createConnection( - new Connection({ - options: defaults, - walletManager: new WalletManager(), - memory, - storage: new Storage(), - }), - ); -}); - -afterAll(async () => { - await tearDownFull(); -}); - -beforeEach(() => { - connection.flush(); -}); - -describe("Connection", () => { - describe("getTransactionsForForging", () => { - const createTransaction = (transactionData, passphrase) => { - const sign = (transactionData, passphrase) => { - const keys = Identities.Keys.fromPassphrase(passphrase); - - transactionData.senderPublicKey = transactionData.senderPublicKey || keys.publicKey; - transactionData.id = Transactions.Utils.getId(transactionData); - - transactionData.signature = Transactions.Signer.sign(transactionData, keys); - - return transactionData; - }; - - const transaction = Transactions.TransactionTypeFactory.create(sign(transactionData, passphrase)); - - Transactions.Serializer.serialize(transaction); - - return Transactions.TransactionFactory.fromBytesUnsafe(transaction.serialized); - }; - - const createTransfer = options => { - const builder = Transactions.BuilderFactory.transfer() - .recipientId(Identities.Address.fromPassphrase("this is fine", Networks.unitnet.network.pubKeyHash)) - .amount("111") - .fee("11") - .expiration(11); - for (const [option, value] of Object.entries(options)) { - builder.data[option] = value; - } - - return createTransaction(builder.data, options.passphrase || "the sender passphrase"); - }; - - const addTransactionsToMemory = transactions => { - for (const tx of transactions) { - memory.remember(tx); - expect(memory.has(tx.id)).toBeTrue(); - } - expect(memory.count()).toBe(transactions.length); - }; - - it("should remove transactions that have expired", () => { - const transactions = [createTransfer({}), createTransfer({ amount: bignum(123), expiration: 1 })]; - - addTransactionsToMemory(transactions); - - expect(connection.getTransactionsForForging(100)).toEqual([transactions[0].serialized.toString("hex")]); - }); - - it("should remove transactions that have a fee of 0 or less", () => { - const transactions = [ - createTransfer({}), - createTransfer({ amount: bignum(1234), fee: bignum(-2) }), - createTransfer({ amount: bignum(2345), fee: bignum(0) }), - ]; - - addTransactionsToMemory(transactions); - - expect(connection.getTransactionsForForging(100)).toEqual([transactions[0].serialized.toString("hex")]); - }); - - it("should remove transactions that have an amount of 0 or less", () => { - const transactions = [ - createTransfer({}), - createTransfer({ amount: bignum(-2) }), - createTransfer({ amount: bignum(0) }), - ]; - - addTransactionsToMemory(transactions); - - expect(connection.getTransactionsForForging(100)).toEqual([transactions[0].serialized.toString("hex")]); - }); - - it("should remove transactions that have data from another network", () => { - const transactions = [ - createTransfer({}), - createTransfer({ - recipientId: Identities.Address.fromPassphrase("this is fine", Networks.devnet.network.pubKeyHash), - }), - ]; - - addTransactionsToMemory(transactions); - - expect(connection.getTransactionsForForging(100)).toEqual([transactions[0].serialized.toString("hex")]); - }); - - it.skip("should remove transactions that have wrong sender public keys", () => { - const transactions = [ - createTransfer({}), - createTransfer({ senderPublicKey: Identities.PublicKey.fromPassphrase("this is wrong") }), - ]; - - expect(transactions[1]).toEqual({}); - addTransactionsToMemory(transactions); - - expect(connection.getTransactionsForForging(100)).toEqual([transactions[0].serialized.toString("hex")]); - }); - - it.skip("should remove transactions that have timestamps in the future", () => { - const transactions = [ - createTransfer({}), - createTransfer({ amount: bignum(1234), timestamp: Crypto.Slots.getTime() + 100 * 1000 }), - ]; - - addTransactionsToMemory(transactions); - - expect(connection.getTransactionsForForging(100)).toEqual([transactions[0].serialized.toString("hex")]); - }); - - it.skip("should remove transactions that have different IDs when entering and leaving", () => { - const transactions = [createTransfer({}), createTransfer({ amount: bignum(1234), id: "64738638929" })]; - - expect(transactions[1]).toEqual({}); - addTransactionsToMemory(transactions); - - expect(connection.getTransactionsForForging(100)).toEqual([transactions[0].serialized.toString("hex")]); - }); - - it.todo("should remove transactions that have an unknown type"); - it.todo("should remove transactions that have unknown properties"); - it.todo("should remove transactions that have missing properties"); - it.todo("should remove transactions that have malformed properties"); - it.todo("should remove transactions that have malformed bytes"); - it.todo("should remove transactions that have additional bytes attached"); - it.todo("should remove transactions that have already been forged"); - it.todo("should remove transactions that have been persisted to the disk"); - it.todo("should remove transactions that have a disabled type"); - it.todo("should remove transactions that have have data of a another transaction type"); - it.todo("should remove transactions that have been altered after entering the pool"); - it.todo("should remove transactions that have negative numerical values"); - it.todo("should remove transactions that have malformed signatures"); - it.todo("should remove transactions that have malformed second signatures"); - it.todo("should remove transactions that have malformed multi signatures"); - it.todo("should remove transactions that fail to deserialize for unknown reasons"); - it.todo("should remove transactions that have a mismatch of expected and actual length of the vendor field"); - it.todo("should remove transactions that have an invalid vendor field length"); - it.todo("should remove transactions that have an invalid vendor field"); - it.todo("should remove transactions that have an invalid version"); - }); -}); From e84037bd87238e2299a64df557696cbc365d661e Mon Sep 17 00:00:00 2001 From: alessiodf <35549818+alessiodf@users.noreply.github.com> Date: Sun, 2 Jun 2019 14:06:09 +0000 Subject: [PATCH 10/34] fix(core-transaction-pool): remove already forged transactions before forging Signed-off-by: supaiku --- .../internal/handlers/transactions.test.ts | 4 +-- .../__stubs__/connection.ts | 6 ++-- .../core-transaction-pool/connection.test.ts | 34 ++++++++++++++----- .../core-interfaces/src/core-p2p/server.ts | 5 +++ .../src/core-transaction-pool/connection.ts | 2 +- .../src/socket-server/versions/internal.ts | 7 ++-- .../core-transaction-pool/src/connection.ts | 14 ++++++-- 7 files changed, 51 insertions(+), 21 deletions(-) diff --git a/__tests__/unit/core-p2p/socket-server/versions/internal/handlers/transactions.test.ts b/__tests__/unit/core-p2p/socket-server/versions/internal/handlers/transactions.test.ts index 13753ba87a..5f623213fa 100644 --- a/__tests__/unit/core-p2p/socket-server/versions/internal/handlers/transactions.test.ts +++ b/__tests__/unit/core-p2p/socket-server/versions/internal/handlers/transactions.test.ts @@ -8,11 +8,11 @@ jest.mock("../../../../../../../packages/core-p2p/src/socket-server/utils/valida describe("Internal handlers - transactions", () => { describe("getUnconfirmedTransactions", () => { - it("should return unconfirmed transactions", () => { + it("should return unconfirmed transactions", async () => { transactionPool.getTransactionsForForging = jest.fn().mockReturnValue(["111"]); transactionPool.getPoolSize = jest.fn().mockReturnValue(1); - expect(getUnconfirmedTransactions()).toEqual({ poolSize: 1, transactions: ["111"] }); + expect(await getUnconfirmedTransactions()).toEqual({ poolSize: 1, transactions: ["111"] }); }); }); }); diff --git a/__tests__/unit/core-transaction-pool/__stubs__/connection.ts b/__tests__/unit/core-transaction-pool/__stubs__/connection.ts index 276497c99f..47df5ad495 100644 --- a/__tests__/unit/core-transaction-pool/__stubs__/connection.ts +++ b/__tests__/unit/core-transaction-pool/__stubs__/connection.ts @@ -70,8 +70,10 @@ export class Connection implements TransactionPool.IConnection { return; } - public getTransactionsForForging(blockSize: number): string[] { - return []; + public getTransactionsForForging(blockSize: number): Promise { + return new Promise(resolve => { + resolve([]); + }); } public getTransaction(id: string): Interfaces.ITransaction { diff --git a/__tests__/unit/core-transaction-pool/connection.test.ts b/__tests__/unit/core-transaction-pool/connection.test.ts index 661157a830..c762c5b47c 100644 --- a/__tests__/unit/core-transaction-pool/connection.test.ts +++ b/__tests__/unit/core-transaction-pool/connection.test.ts @@ -492,18 +492,35 @@ describe("Connection", () => { }); describe("getTransactionsForForging", () => { - it("should return an array of transactions serialized", () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + it("should return an array of transactions serialized", async () => { const transactions = [mockData.dummy1, mockData.dummy2, mockData.dummy3, mockData.dummy4]; addTransactions(transactions); - const spy = jest.spyOn(Handlers.Registry.get(0), "canBeApplied").mockReturnValue(true); - const transactionsForForging = connection.getTransactionsForForging(4); - spy.mockRestore(); + jest.spyOn(Handlers.Registry.get(0), "canBeApplied").mockReturnValue(true); + const transactionsForForging = await connection.getTransactionsForForging(4); expect(transactionsForForging).toEqual(transactions.map(tx => tx.serialized.toString("hex"))); }); - it("should only return transactions not exceeding the maximum payload size", () => { + it("should only return unforged transactions", async () => { + const transactions = [mockData.dummy1, mockData.dummy2, mockData.dummy3]; + addTransactions(transactions); + + jest.spyOn(databaseService, "getForgedTransactionsIds").mockReturnValue([ + mockData.dummy1.id, + mockData.dummy3.id, + ]); + jest.spyOn(Handlers.Registry.get(0), "canBeApplied").mockReturnValue(true); + + const transactionsForForging = await connection.getTransactionsForForging(3); + expect(transactionsForForging.length).toBe(1); + expect(transactionsForForging[0]).toEqual(mockData.dummy2.serialized.toString("hex")); + }); + + it("should only return transactions not exceeding the maximum payload size", async () => { // @FIXME: Uhm excuse me, what the? mockData.dummyLarge1.data.signatures = mockData.dummyLarge2.data.signatures = [""]; for (let i = 0; i < connection.options.maxTransactionBytes * 0.6; i++) { @@ -525,8 +542,8 @@ describe("Connection", () => { addTransactions(transactions); - const spy = jest.spyOn(Handlers.Registry.get(0), "canBeApplied").mockReturnValue(true); - let transactionsForForging = connection.getTransactionsForForging(7); + jest.spyOn(Handlers.Registry.get(0), "canBeApplied").mockReturnValue(true); + let transactionsForForging = await connection.getTransactionsForForging(7); expect(transactionsForForging.length).toBe(6); expect(transactionsForForging[0]).toEqual(mockData.dummyLarge1.serialized.toString("hex")); @@ -543,8 +560,7 @@ describe("Connection", () => { connection.removeTransactionById(mockData.dummy6.id); connection.removeTransactionById(mockData.dummy7.id); - transactionsForForging = connection.getTransactionsForForging(7); - spy.mockRestore(); + transactionsForForging = await connection.getTransactionsForForging(7); expect(transactionsForForging.length).toBe(1); expect(transactionsForForging[0]).toEqual(mockData.dummyLarge2.serialized.toString("hex")); diff --git a/packages/core-interfaces/src/core-p2p/server.ts b/packages/core-interfaces/src/core-p2p/server.ts index 0b1055b2ec..1b62956eb3 100644 --- a/packages/core-interfaces/src/core-p2p/server.ts +++ b/packages/core-interfaces/src/core-p2p/server.ts @@ -21,3 +21,8 @@ export interface IForgingTransactions { poolSize: number; count: number; } + +export interface IUnconfirmedTransactions { + transactions: string[]; + poolSize: number; +} diff --git a/packages/core-interfaces/src/core-transaction-pool/connection.ts b/packages/core-interfaces/src/core-transaction-pool/connection.ts index f44c9e0a70..708ef2f60a 100644 --- a/packages/core-interfaces/src/core-transaction-pool/connection.ts +++ b/packages/core-interfaces/src/core-transaction-pool/connection.ts @@ -32,7 +32,7 @@ export interface IConnection { getTransactions(start: number, size: number, maxBytes?: number): Buffer[]; getTransactionsByType(type: any): any; getTransactionsData(start: number, size: number, maxBytes?: number): Interfaces.ITransaction[]; - getTransactionsForForging(blockSize: number): string[]; + getTransactionsForForging(blockSize: number): Promise; has(transactionId: string): any; hasExceededMaxTransactions(senderPublicKey: string): boolean; isSenderBlocked(senderPublicKey: string): boolean; diff --git a/packages/core-p2p/src/socket-server/versions/internal.ts b/packages/core-p2p/src/socket-server/versions/internal.ts index da9059ddbc..3771a91243 100644 --- a/packages/core-p2p/src/socket-server/versions/internal.ts +++ b/packages/core-p2p/src/socket-server/versions/internal.ts @@ -7,10 +7,7 @@ export const emitEvent = ({ req }): void => { app.resolvePlugin("event-emitter").emit(req.data.event, req.data.body); }; -export const getUnconfirmedTransactions = (): { - transactions: string[]; - poolSize: number; -} => { +export const getUnconfirmedTransactions = async (): Promise => { const blockchain = app.resolvePlugin("blockchain"); const { maxTransactions } = app.getConfig().getMilestone(blockchain.getLastBlock().data.height).block; @@ -19,7 +16,7 @@ export const getUnconfirmedTransactions = (): { ); return { - transactions: transactionPool.getTransactionsForForging(maxTransactions), + transactions: await transactionPool.getTransactionsForForging(maxTransactions), poolSize: transactionPool.getPoolSize(), }; }; diff --git a/packages/core-transaction-pool/src/connection.ts b/packages/core-transaction-pool/src/connection.ts index ff2fa94fe8..40c8bd7653 100644 --- a/packages/core-transaction-pool/src/connection.ts +++ b/packages/core-transaction-pool/src/connection.ts @@ -144,7 +144,7 @@ export class Connection implements TransactionPool.IConnection { ); } - public getTransactionsForForging(blockSize: number): string[] { + public async getTransactionsForForging(blockSize: number): Promise { const transactionMemory: Interfaces.ITransaction[] = this.getTransactionsData( 0, blockSize, @@ -153,7 +153,17 @@ export class Connection implements TransactionPool.IConnection { const transactions: string[] = []; - for (const transaction of transactionMemory) { + const forgedIds: string[] = await this.databaseService.getForgedTransactionsIds( + transactionMemory.map(transaction => transaction.id), + ); + + this.removeTransactionsById(forgedIds); + + const unforgedTransactions = transactionMemory.filter( + (transaction: Interfaces.ITransaction) => !forgedIds.includes(transaction.id), + ); + + for (const transaction of unforgedTransactions) { try { const deserialized: Interfaces.ITransaction = Transactions.TransactionFactory.fromBytes( transaction.serialized, From 709dc0289cfe821a040d1c13ed2b417fa39217a9 Mon Sep 17 00:00:00 2001 From: supaiku Date: Tue, 4 Jun 2019 03:02:04 +0200 Subject: [PATCH 11/34] refactor(core-transaction-pool): extract into function --- .../core-transaction-pool/src/connection.ts | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/packages/core-transaction-pool/src/connection.ts b/packages/core-transaction-pool/src/connection.ts index 40c8bd7653..3c7f38b5ab 100644 --- a/packages/core-transaction-pool/src/connection.ts +++ b/packages/core-transaction-pool/src/connection.ts @@ -55,9 +55,7 @@ export class Connection implements TransactionPool.IConnection { this.purgeExpired(); - const forgedIds: string[] = await this.databaseService.getForgedTransactionsIds(all.map(t => t.id)); - - this.removeTransactionsById(forgedIds); + await this.removeForgedTransactions(all); this.purgeInvalidTransactions(); @@ -152,12 +150,7 @@ export class Connection implements TransactionPool.IConnection { ); const transactions: string[] = []; - - const forgedIds: string[] = await this.databaseService.getForgedTransactionsIds( - transactionMemory.map(transaction => transaction.id), - ); - - this.removeTransactionsById(forgedIds); + const forgedIds: string[] = await this.removeForgedTransactions(transactionMemory); const unforgedTransactions = transactionMemory.filter( (transaction: Interfaces.ITransaction) => !forgedIds.includes(transaction.id), @@ -505,6 +498,15 @@ export class Connection implements TransactionPool.IConnection { this.storage.bulkRemoveById(this.memory.pullDirtyRemoved()); } + private async removeForgedTransactions(transactions: Interfaces.ITransaction[]): Promise { + const forgedIds: string[] = await this.databaseService.getForgedTransactionsIds( + transactions.map(({ id }) => id), + ); + + this.removeTransactionsById(forgedIds); + return forgedIds; + } + private purgeExpired(): void { this.purgeTransactions(ApplicationEvents.TransactionExpired, this.memory.getExpired()); } From b07f82425f61008b3b06851d92092039b9bdb72e Mon Sep 17 00:00:00 2001 From: supaiku Date: Tue, 4 Jun 2019 03:07:27 +0200 Subject: [PATCH 12/34] test: make function async --- .../unit/core-transaction-pool/__stubs__/connection.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/__tests__/unit/core-transaction-pool/__stubs__/connection.ts b/__tests__/unit/core-transaction-pool/__stubs__/connection.ts index 47df5ad495..cb8d652df6 100644 --- a/__tests__/unit/core-transaction-pool/__stubs__/connection.ts +++ b/__tests__/unit/core-transaction-pool/__stubs__/connection.ts @@ -70,10 +70,8 @@ export class Connection implements TransactionPool.IConnection { return; } - public getTransactionsForForging(blockSize: number): Promise { - return new Promise(resolve => { - resolve([]); - }); + public async getTransactionsForForging(blockSize: number): Promise { + return []; } public getTransaction(id: string): Interfaces.ITransaction { From fd5fd008d5ae3a86604a1978b69e77d1a9ac21b0 Mon Sep 17 00:00:00 2001 From: Air1 Date: Tue, 4 Jun 2019 16:43:25 +0400 Subject: [PATCH 13/34] test: malformed serialized bytes tests --- __tests__/helpers/transaction-factory.ts | 8 +- .../connection.forging.test.ts | 122 +++++++++++++++--- 2 files changed, 108 insertions(+), 22 deletions(-) diff --git a/__tests__/helpers/transaction-factory.ts b/__tests__/helpers/transaction-factory.ts index 59892d7116..5143f94c48 100644 --- a/__tests__/helpers/transaction-factory.ts +++ b/__tests__/helpers/transaction-factory.ts @@ -290,9 +290,11 @@ export class TransactionFactory { transaction.data.id = Transactions.Utils.getId(transaction.data); } - // TODO: exclude certain malformed properties that cannot be serialized properly - // like signatures that are too long. Add support for customizing signatures. - Transactions.Serializer.serialize(transaction); + if (!transaction.serialized) { + // TODO: exclude certain malformed properties that cannot be serialized properly + // like signatures that are too long. Add support for customizing signatures. + Transactions.Serializer.serialize(transaction); + } transactions.push((transaction as unknown) as T); } else { diff --git a/__tests__/unit/core-transaction-pool/connection.forging.test.ts b/__tests__/unit/core-transaction-pool/connection.forging.test.ts index f456665eb0..e2522bbc0e 100644 --- a/__tests__/unit/core-transaction-pool/connection.forging.test.ts +++ b/__tests__/unit/core-transaction-pool/connection.forging.test.ts @@ -3,6 +3,8 @@ import "jest-extended"; import "./mocks/core-container"; import { Wallets } from "@arkecosystem/core-state"; +import bs58check from "bs58check"; +import ByteBuffer from "bytebuffer"; import { Constants, Crypto, Identities, Interfaces, Managers, Networks, Utils } from "@arkecosystem/crypto"; import { Connection } from "../../../packages/core-transaction-pool/src/connection"; @@ -75,28 +77,89 @@ describe("Connection", () => { expect(memory.count()).toBe(transactions.length); }; - const expectForgingTransactions = (transactions: Interfaces.ITransaction[], countGood: number) => { + const expectForgingTransactions = async (transactions: Interfaces.ITransaction[], countGood: number) => { addTransactionsToMemory(transactions); - const forgingTransactions = connection.getTransactionsForForging(100); + const forgingTransactions = await connection.getTransactionsForForging(100); expect(forgingTransactions).toHaveLength(countGood); expect(forgingTransactions).toEqual( transactions.slice(transactions.length - countGood).map(({ serialized }) => serialized.toString("hex")), ); }; + const customSerialize = (transaction: Interfaces.ITransactionData, options: any = {}) => { + const buffer = new ByteBuffer(512, true); + const writeByte = (txField, value) => (options[txField] ? options[txField](buffer) : buffer.writeByte(value)); + const writeUint32 = (txField, value) => + options[txField] ? options[txField](buffer) : buffer.writeUint32(value); + const writeUint64 = (txField, value) => + options[txField] ? options[txField](buffer) : buffer.writeUint64(value); + const append = (txField, value, encoding = "utf8") => + options[txField] ? options[txField](buffer) : buffer.append(value, encoding); + + buffer.writeByte(0xff); // fill, to disambiguate from v1 + writeByte("version", 0x01); + writeByte("network", transaction.network); // ark = 0x17, devnet = 0x30 + writeByte("type", transaction.type); + writeUint32("timestamp", transaction.timestamp); + append("senderPublicKey", transaction.senderPublicKey, "hex"); + writeUint64("fee", +transaction.fee); + + if (options.vendorField) { + options.vendorField(buffer); + } else if (transaction.vendorField) { + const vf: Buffer = Buffer.from(transaction.vendorField, "utf8"); + buffer.writeByte(vf.length); + buffer.append(vf); + } else if (transaction.vendorFieldHex) { + buffer.writeByte(transaction.vendorFieldHex.length / 2); + buffer.append(transaction.vendorFieldHex, "hex"); + } else { + buffer.writeByte(0x00); + } + + // only for transfer right now + writeUint64("amount", +transaction.amount); + writeUint32("expiration", transaction.expiration || 0); + append("recipientId", bs58check.decode(transaction.recipientId)); + + // signatures + if (transaction.signature || options.signature) { + append("signature", transaction.signature, "hex"); + } + + const secondSignature: string = transaction.secondSignature || transaction.signSignature; + + if (secondSignature || options.secondSignature) { + append("secondSignature", secondSignature, "hex"); + } + + if (options.signatures) { + options.signatures(buffer); + } else if (transaction.signatures) { + if (transaction.version === 1 && Utils.isException(transaction)) { + buffer.append("ff", "hex"); // 0xff separator to signal start of multi-signature transactions + buffer.append(transaction.signatures.join(""), "hex"); + } else { + buffer.append(transaction.signatures.join(""), "hex"); + } + } + + return buffer.flip().toBuffer(); + }; + describe("getTransactionsForForging", () => { - it("should remove transactions that have expired [5 Good, 5 Bad]", () => { + it("should remove transactions that have expired [5 Good, 5 Bad]", async () => { mockCurrentHeight(100); const transactions = TransactionFactory.transfer() .withCustomizedPayload([{ expiration: 1 }], { quantity: 5 }) .build(10); - expectForgingTransactions(transactions, 5); + await expectForgingTransactions(transactions, 5); }); - it("should remove transactions that have a fee of 0 or less [8 Good, 2 Bad]", () => { + it("should remove transactions that have a fee of 0 or less [8 Good, 2 Bad]", async () => { const transactions = TransactionFactory.transfer() .withCustomizedPayload( [ @@ -107,10 +170,10 @@ describe("Connection", () => { ) .build(10); - expectForgingTransactions(transactions, 8); + await expectForgingTransactions(transactions, 8); }); - it("should remove transactions that have an amount of 0 or less [8 Good, 2 Bad]", () => { + it("should remove transactions that have an amount of 0 or less [8 Good, 2 Bad]", async () => { const transactions = TransactionFactory.transfer() .withCustomizedPayload( [ @@ -121,10 +184,10 @@ describe("Connection", () => { ) .build(10); - expectForgingTransactions(transactions, 8); + await expectForgingTransactions(transactions, 8); }); - it("should remove transactions that have data from another network [5 Good, 5 Bad]", () => { + it("should remove transactions that have data from another network [5 Good, 5 Bad]", async () => { const transactions = TransactionFactory.transfer() .withCustomizedPayload( [{ recipientId: Identities.Address.fromPassphrase("secret", Networks.devnet.network.pubKeyHash) }], @@ -132,43 +195,61 @@ describe("Connection", () => { ) .build(10); - expectForgingTransactions(transactions, 5); + await expectForgingTransactions(transactions, 5); }); - it("should remove transactions that have wrong sender public keys [5 Good, 5 Bad]", () => { + it("should remove transactions that have wrong sender public keys [5 Good, 5 Bad]", async () => { const transactions = TransactionFactory.transfer() .withCustomizedPayload([{ senderPublicKey: Identities.PublicKey.fromPassphrase("this is wrong") }], { quantity: 5, }) .build(10); - expectForgingTransactions(transactions, 5); + await expectForgingTransactions(transactions, 5); }); - it("should remove transactions that have timestamps in the future [5 Good, 5 Bad]", () => { + it("should remove transactions that have timestamps in the future [5 Good, 5 Bad]", async () => { const transactions = TransactionFactory.transfer() .withCustomizedPayload([{ timestamp: Crypto.Slots.getTime() + 100 * 1000 }], { quantity: 5 }) .build(10); - expectForgingTransactions(transactions, 5); + await expectForgingTransactions(transactions, 5); }); - it("should remove transactions that have different IDs when entering and leaving [8 Good, 2 Bad]", () => { + it("should remove transactions that have different IDs when entering and leaving [8 Good, 2 Bad]", async () => { const transactions = TransactionFactory.transfer() .withCustomizedPayload([{ id: "garbage" }, { id: "garbage 2" }], { quantity: 2 }) .build(10); - expectForgingTransactions(transactions, 8); + await expectForgingTransactions(transactions, 8); + }); + + it("should remove transactions that have malformed bytes", async () => { + const malformedBytesFn = [ + { version: (b: ByteBuffer) => b.writeUint64(1111111) }, + { network: (b: ByteBuffer) => b.writeUint64(1111111) }, + { type: (b: ByteBuffer) => b.writeUint64(1111111) }, + { timestamp: (b: ByteBuffer) => b.writeByte(0x01) }, + { senderPublicKey: (b: ByteBuffer) => b.writeByte(0x01) }, + { vendorField: (b: ByteBuffer) => b.writeByte(0x01) }, + { amount: (b: ByteBuffer) => b.writeByte(0x01) }, + { expiration: (b: ByteBuffer) => b.writeByte(0x01) }, + { recipientId: (b: ByteBuffer) => b.writeByte(0x01) }, + { signature: (b: ByteBuffer) => b.writeByte(0x01) }, + { secondSignature: (b: ByteBuffer) => b.writeByte(0x01) }, + { signatures: (b: ByteBuffer) => b.writeByte(0x01) }, + ]; + const transactions = TransactionFactory.transfer().build(malformedBytesFn.length + 5); + transactions.map((tx, i) => (tx.serialized = customSerialize(tx.data, malformedBytesFn[i] || {}))); + + await expectForgingTransactions(transactions, 5); }); it.todo("should remove transactions that have an unknown type"); it.todo("should remove transactions that have unknown properties"); it.todo("should remove transactions that have missing properties"); it.todo("should remove transactions that have malformed properties"); - it.todo("should remove transactions that have malformed bytes"); it.todo("should remove transactions that have additional bytes attached"); - it.todo("should remove transactions that have already been forged"); - it.todo("should remove transactions that have been persisted to the disk"); it.todo("should remove transactions that have a disabled type"); it.todo("should remove transactions that have have data of a another transaction type"); it.todo("should remove transactions that have been altered after entering the pool"); @@ -181,5 +262,8 @@ describe("Connection", () => { it.todo("should remove transactions that have an invalid vendor field length"); it.todo("should remove transactions that have an invalid vendor field"); it.todo("should remove transactions that have an invalid version"); + + it.todo("should remove transactions that have already been forged"); + it.todo("should remove transactions that have been persisted to the disk"); }); }); From edff99d169fc83bbc2c0efa4373fffcb51d73109 Mon Sep 17 00:00:00 2001 From: supaiku Date: Wed, 5 Jun 2019 00:00:35 +0200 Subject: [PATCH 14/34] test: spy on fromBytes and canBeApplied --- .../connection.forging.test.ts | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/__tests__/unit/core-transaction-pool/connection.forging.test.ts b/__tests__/unit/core-transaction-pool/connection.forging.test.ts index e2522bbc0e..1050a13ae1 100644 --- a/__tests__/unit/core-transaction-pool/connection.forging.test.ts +++ b/__tests__/unit/core-transaction-pool/connection.forging.test.ts @@ -2,11 +2,21 @@ import "jest-extended"; import "./mocks/core-container"; -import { Wallets } from "@arkecosystem/core-state"; import bs58check from "bs58check"; import ByteBuffer from "bytebuffer"; -import { Constants, Crypto, Identities, Interfaces, Managers, Networks, Utils } from "@arkecosystem/crypto"; +import { Wallets } from "@arkecosystem/core-state"; +import { Handlers } from "@arkecosystem/core-transactions"; +import { + Constants, + Crypto, + Identities, + Interfaces, + Managers, + Networks, + Transactions, + Utils, +} from "@arkecosystem/crypto"; import { Connection } from "../../../packages/core-transaction-pool/src/connection"; import { defaults } from "../../../packages/core-transaction-pool/src/defaults"; import { Memory } from "../../../packages/core-transaction-pool/src/memory"; @@ -67,6 +77,8 @@ describe("Connection", () => { // @ts-ignore connection.databaseService.walletManager = databaseWalletManager; + + jest.restoreAllMocks(); }); const addTransactionsToMemory = transactions => { @@ -245,6 +257,20 @@ describe("Connection", () => { await expectForgingTransactions(transactions, 5); }); + it("should call `TransactionFactory.fromBytes`", async () => { + const transactions = TransactionFactory.transfer().build(5); + const spy = jest.spyOn(Transactions.TransactionFactory, "fromBytes"); + await expectForgingTransactions(transactions, 5); + expect(spy).toHaveBeenCalled(); + }); + + it("should call `TransactionHandler.canBeApplied`", async () => { + const transactions = TransactionFactory.transfer().build(5); + const spy = jest.spyOn(Handlers.Registry.get(0), "canBeApplied"); + await expectForgingTransactions(transactions, 5); + expect(spy).toHaveBeenCalled(); + }); + it.todo("should remove transactions that have an unknown type"); it.todo("should remove transactions that have unknown properties"); it.todo("should remove transactions that have missing properties"); From 9e9bae29bfc9b6c0f5938f2f5321936d548747cc Mon Sep 17 00:00:00 2001 From: supaiku Date: Wed, 5 Jun 2019 01:18:49 +0200 Subject: [PATCH 15/34] fix(crypto): throw if bytes are remaining after reading signatures --- .../connection.forging.test.ts | 14 ++++++++++++++ packages/crypto/src/transactions/deserializer.ts | 4 ++++ 2 files changed, 18 insertions(+) diff --git a/__tests__/unit/core-transaction-pool/connection.forging.test.ts b/__tests__/unit/core-transaction-pool/connection.forging.test.ts index 1050a13ae1..a7222815b4 100644 --- a/__tests__/unit/core-transaction-pool/connection.forging.test.ts +++ b/__tests__/unit/core-transaction-pool/connection.forging.test.ts @@ -236,6 +236,20 @@ describe("Connection", () => { await expectForgingTransactions(transactions, 8); }); + it("should call `TransactionFactory.fromBytes`", async () => { + const transactions = TransactionFactory.transfer().build(5); + const spy = jest.spyOn(Transactions.TransactionFactory, "fromBytes"); + await expectForgingTransactions(transactions, 5); + expect(spy).toHaveBeenCalled(); + }); + + it("should call `TransactionHandler.canBeApplied`", async () => { + const transactions = TransactionFactory.transfer().build(5); + const spy = jest.spyOn(Handlers.Registry.get(0), "canBeApplied"); + await expectForgingTransactions(transactions, 5); + expect(spy).toHaveBeenCalled(); + }); + it("should remove transactions that have malformed bytes", async () => { const malformedBytesFn = [ { version: (b: ByteBuffer) => b.writeUint64(1111111) }, diff --git a/packages/crypto/src/transactions/deserializer.ts b/packages/crypto/src/transactions/deserializer.ts index 9fd67027dd..8c09ae4cf2 100644 --- a/packages/crypto/src/transactions/deserializer.ts +++ b/packages/crypto/src/transactions/deserializer.ts @@ -107,6 +107,10 @@ class Deserializer { const multiSignature: string = buf.readBytes(buf.limit - buf.offset).toString("hex"); transaction.signatures = [multiSignature]; } + + if (buf.remaining()) { + throw new MalformedTransactionBytesError(); + } } private deserializeSchnorr(transaction: ITransactionData, buf: ByteBuffer) { From 673e441e2296c373f931d61f10a47c29681eee29 Mon Sep 17 00:00:00 2001 From: supaiku Date: Wed, 5 Jun 2019 01:40:32 +0200 Subject: [PATCH 16/34] test: implement most todos --- .../connection.forging.test.ts | 272 ++++++++++++++++-- 1 file changed, 247 insertions(+), 25 deletions(-) diff --git a/__tests__/unit/core-transaction-pool/connection.forging.test.ts b/__tests__/unit/core-transaction-pool/connection.forging.test.ts index a7222815b4..4d4596715e 100644 --- a/__tests__/unit/core-transaction-pool/connection.forging.test.ts +++ b/__tests__/unit/core-transaction-pool/connection.forging.test.ts @@ -69,6 +69,10 @@ describe("Connection", () => { wallet.username = `delegate-${i + 1}`; wallet.vote = publicKey; + if (i === 50) { + wallet.secondPublicKey = Identities.PublicKey.fromPassphrase("second secret"); + } + databaseWalletManager.reindex(wallet); } @@ -89,7 +93,10 @@ describe("Connection", () => { expect(memory.count()).toBe(transactions.length); }; - const expectForgingTransactions = async (transactions: Interfaces.ITransaction[], countGood: number) => { + const expectForgingTransactions = async ( + transactions: Interfaces.ITransaction[], + countGood: number, + ): Promise => { addTransactionsToMemory(transactions); const forgingTransactions = await connection.getTransactionsForForging(100); @@ -97,6 +104,8 @@ describe("Connection", () => { expect(forgingTransactions).toEqual( transactions.slice(transactions.length - countGood).map(({ serialized }) => serialized.toString("hex")), ); + + return forgingTransactions; }; const customSerialize = (transaction: Interfaces.ITransactionData, options: any = {}) => { @@ -271,39 +280,252 @@ describe("Connection", () => { await expectForgingTransactions(transactions, 5); }); - it("should call `TransactionFactory.fromBytes`", async () => { + it("should remove transactions that have an unknown type", async () => { + const transactions = TransactionFactory.transfer().build(2); + transactions[0].serialized = customSerialize(transactions[0].data, { + version: (b: ByteBuffer) => b.writeUint8(255), + }); + + await expectForgingTransactions(transactions, 1); + }); + + it("should remove transactions that have a disabled type", async () => { + const transactions = TransactionFactory.transfer() + .withVersion(1) + .build(2); + transactions[0].serialized = customSerialize(transactions[0].data, { + version: (b: ByteBuffer) => b.writeUint8(4), + }); + + await expectForgingTransactions(transactions, 1); + }); + + it("should remove transactions that have have data of a another transaction type", async () => { + const handlers: Handlers.TransactionHandler[] = Handlers.Registry.all(); + const transactions: Interfaces.ITransaction[] = TransactionFactory.transfer().build(handlers.length); + + for (let i = 0; i < handlers.length; i++) { + expect(handlers[0].getConstructor().type).toEqual(0); + transactions[i].serialized = customSerialize(transactions[i].data, { + type: (b: ByteBuffer) => b.writeUint8(handlers[i].getConstructor().type), + }); + } + + await expectForgingTransactions(transactions.reverse(), 1); + }); + + it("should remove transactions that have negative numerical values", async () => { + const transactions = TransactionFactory.transfer().build(2); + transactions[0].serialized = customSerialize(transactions[0].data, { + fee: (b: ByteBuffer) => b.writeUint64(-999999), + amount: (b: ByteBuffer) => b.writeUint64(-999999), + }); + + await expectForgingTransactions(transactions, 1); + }); + + it("should remove transactions that have been altered after entering the pool", async () => { + const transactions = TransactionFactory.transfer().build(2); + transactions[0].data.id = transactions[0].data.id + .split("") + .reverse() + .join(""); + + await expectForgingTransactions(transactions, 1); + }); + + it("should remove transactions that have an invalid version", async () => { + const transactions = TransactionFactory.transfer().build(2); + transactions[0].serialized = customSerialize(transactions[0].data, { + version: (b: ByteBuffer) => b.writeByte(0), + }); + + await expectForgingTransactions(transactions, 1); + }); + + it("should remove transactions that have a mismatch of expected and actual length of the vendor field", async () => { + const transactions = TransactionFactory.transfer().build(3); + transactions[0].serialized = customSerialize(transactions[0].data, { + vendorField: (b: ByteBuffer) => { + const vendorField = Buffer.from(transactions[0].data.vendorField, "utf8"); + b.writeByte(vendorField.length - 5); + b.append(vendorField); + }, + }); + + transactions[1].serialized = customSerialize(transactions[1].data, { + vendorField: (b: ByteBuffer) => { + const vendorField = Buffer.from(transactions[1].data.vendorField, "utf8"); + b.writeByte(vendorField.length + 5); + b.append(vendorField); + }, + }); + + await expectForgingTransactions(transactions, 1); + }); + + it("should remove transactions that have an invalid vendor field length", async () => { + const transactions = TransactionFactory.transfer().build(3); + transactions[0].serialized = customSerialize(transactions[0].data, { + vendorField: (b: ByteBuffer) => { + const vendorField = Buffer.from(transactions[0].data.vendorField, "utf8"); + b.writeByte(0); + b.append(vendorField); + }, + }); + + transactions[1].serialized = customSerialize(transactions[1].data, { + vendorField: (b: ByteBuffer) => { + b.writeByte(255); + }, + }); + + await expectForgingTransactions(transactions, 1); + }); + + it("should remove transactions that have an invalid vendor field", async () => { + const transactions = TransactionFactory.transfer().build(3); + transactions[0].serialized = customSerialize(transactions[0].data, { + vendorField: (b: ByteBuffer) => { + const vendorField = Buffer.from(transactions[0].data.vendorField.toUpperCase(), "utf8"); + b.writeByte(vendorField.length); + b.append(vendorField); + }, + }); + + transactions[1].serialized = customSerialize(transactions[1].data, { + vendorField: (b: ByteBuffer) => { + b.writeByte(255); + b.fill(0, b.offset); + }, + }); + + await expectForgingTransactions(transactions, 1); + }); + + it("should remove transactions that have additional bytes attached", async () => { const transactions = TransactionFactory.transfer().build(5); - const spy = jest.spyOn(Transactions.TransactionFactory, "fromBytes"); - await expectForgingTransactions(transactions, 5); - expect(spy).toHaveBeenCalled(); + + const appendBytes = (transaction: Interfaces.ITransaction, garbage: Buffer) => { + const buffer = new ByteBuffer(512, true); + buffer.append(transaction.serialized); + buffer.append(garbage); + + transaction.serialized = buffer.flip().toBuffer(); + }; + + appendBytes(transactions[0], Buffer.from("garbage", "utf8")); + appendBytes(transactions[1], Buffer.from("ff", "hex")); + appendBytes(transactions[2], Buffer.from("00011111", "hex")); + appendBytes(transactions[3], Buffer.from("0001", "hex")); + + await expectForgingTransactions(transactions, 1); }); - it("should call `TransactionHandler.canBeApplied`", async () => { + it("should remove transactions that have malformed signatures", async () => { const transactions = TransactionFactory.transfer().build(5); - const spy = jest.spyOn(Handlers.Registry.get(0), "canBeApplied"); - await expectForgingTransactions(transactions, 5); - expect(spy).toHaveBeenCalled(); + + const makeSignature = (from: string): string => { + return Crypto.Hash.signECDSA( + Buffer.from(Crypto.HashAlgorithms.sha256(from)), + Identities.Keys.fromPassphrase("garbage"), + ); + }; + + transactions[0].serialized = customSerialize(transactions[0].data, { + signatures: (b: ByteBuffer) => { + b.append(Buffer.from(makeSignature("garbage").slice(25), "hex")); + }, + }); + + transactions[1].serialized = customSerialize(transactions[0].data, { + signatures: (b: ByteBuffer) => { + b.append(Buffer.from(makeSignature("garbage").repeat(2), "hex")); + }, + }); + + transactions[2].serialized = customSerialize(transactions[0].data, { + signatures: (b: ByteBuffer) => { + b.append(Buffer.from(makeSignature("garbage") + "affe", "hex")); + }, + }); + + await expectForgingTransactions(transactions, 2); + }); + + it("should remove transactions that have malformed second signatures", async () => { + const transactions = TransactionFactory.transfer() + .withPassphrasePair({ + passphrase: delegates[50].passphrase, + secondPassphrase: "second secret", + }) + .build(5); + + const appendBytes = (transaction: Interfaces.ITransaction, garbage: Buffer) => { + const buffer = new ByteBuffer(512, true); + buffer.append(transaction.serialized); + buffer.append(garbage); + + transaction.serialized = buffer.flip().toBuffer(); + }; + + appendBytes(transactions[0], Buffer.from("ff", "hex")); + appendBytes(transactions[1], Buffer.from("00", "hex")); + appendBytes(transactions[2], Buffer.from("0011001100", "hex")); + + await expectForgingTransactions(transactions, 2); + }); + + it("should remove transactions that have malformed multi signatures", async () => { + const transactions = TransactionFactory.transfer().build(5); + + const appendBytes = (transaction: Interfaces.ITransaction, garbage: Buffer) => { + const buffer = new ByteBuffer(512, true); + buffer.append(transaction.serialized); + buffer.append(garbage); + + transaction.serialized = buffer.flip().toBuffer(); + }; + + const makeSignature = (from: string): string => { + return Crypto.Hash.signECDSA( + Buffer.from(Crypto.HashAlgorithms.sha256(from)), + Identities.Keys.fromPassphrase("garbage"), + ); + }; + + appendBytes(transactions[0], Buffer.from("ff" + makeSignature("garbage").repeat(5), "hex")); + + await expectForgingTransactions(transactions, 4); + }); + + it("should remove transactions that have malformed multi signatures", async () => { + const transactions = TransactionFactory.transfer().build(5); + + const appendBytes = (transaction: Interfaces.ITransaction, garbage: Buffer) => { + const buffer = new ByteBuffer(512, true); + buffer.append(transaction.serialized); + buffer.append(garbage); + + transaction.serialized = buffer.flip().toBuffer(); + }; + + const makeSignature = (from: string): string => { + return Crypto.Hash.signECDSA( + Buffer.from(Crypto.HashAlgorithms.sha256(from)), + Identities.Keys.fromPassphrase("garbage"), + ); + }; + + appendBytes(transactions[0], Buffer.from("ff" + makeSignature("garbage").repeat(5), "hex")); + + await expectForgingTransactions(transactions, 4); }); - it.todo("should remove transactions that have an unknown type"); it.todo("should remove transactions that have unknown properties"); it.todo("should remove transactions that have missing properties"); it.todo("should remove transactions that have malformed properties"); - it.todo("should remove transactions that have additional bytes attached"); - it.todo("should remove transactions that have a disabled type"); - it.todo("should remove transactions that have have data of a another transaction type"); - it.todo("should remove transactions that have been altered after entering the pool"); - it.todo("should remove transactions that have negative numerical values"); - it.todo("should remove transactions that have malformed signatures"); - it.todo("should remove transactions that have malformed second signatures"); - it.todo("should remove transactions that have malformed multi signatures"); - it.todo("should remove transactions that fail to deserialize for unknown reasons"); - it.todo("should remove transactions that have a mismatch of expected and actual length of the vendor field"); - it.todo("should remove transactions that have an invalid vendor field length"); - it.todo("should remove transactions that have an invalid vendor field"); - it.todo("should remove transactions that have an invalid version"); - - it.todo("should remove transactions that have already been forged"); + it.todo("should remove transactions that have been persisted to the disk"); }); }); From 0c80ade9ff702c9b2c16b4852b04ca1b48486c2d Mon Sep 17 00:00:00 2001 From: supaiku Date: Wed, 5 Jun 2019 01:45:55 +0200 Subject: [PATCH 17/34] test: spy on removeForgedTransactions --- .../unit/core-transaction-pool/connection.forging.test.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/__tests__/unit/core-transaction-pool/connection.forging.test.ts b/__tests__/unit/core-transaction-pool/connection.forging.test.ts index 4d4596715e..b67085565f 100644 --- a/__tests__/unit/core-transaction-pool/connection.forging.test.ts +++ b/__tests__/unit/core-transaction-pool/connection.forging.test.ts @@ -259,6 +259,13 @@ describe("Connection", () => { expect(spy).toHaveBeenCalled(); }); + it("should call `removeForgedTransactions`", async () => { + const transactions = TransactionFactory.transfer().build(5); + const spy = jest.spyOn(connection as any, "removeForgedTransactions"); + await expectForgingTransactions(transactions, 5); + expect(spy).toHaveBeenCalled(); + }); + it("should remove transactions that have malformed bytes", async () => { const malformedBytesFn = [ { version: (b: ByteBuffer) => b.writeUint64(1111111) }, @@ -516,7 +523,6 @@ describe("Connection", () => { Identities.Keys.fromPassphrase("garbage"), ); }; - appendBytes(transactions[0], Buffer.from("ff" + makeSignature("garbage").repeat(5), "hex")); await expectForgingTransactions(transactions, 4); From 16e26d4f54bd67778c7a86b0e1cbae38bddd85b3 Mon Sep 17 00:00:00 2001 From: supaiku Date: Wed, 5 Jun 2019 01:47:31 +0200 Subject: [PATCH 18/34] misc: remove console.log --- packages/core-transaction-pool/src/connection.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/core-transaction-pool/src/connection.ts b/packages/core-transaction-pool/src/connection.ts index 3c7f38b5ab..ef502a2709 100644 --- a/packages/core-transaction-pool/src/connection.ts +++ b/packages/core-transaction-pool/src/connection.ts @@ -171,7 +171,6 @@ export class Connection implements TransactionPool.IConnection { transactions.push(deserialized.serialized.toString("hex")); } catch (error) { this.removeTransactionById(transaction.id); - console.error(error); this.logger.error(`Removed ${transaction.id} before forging because it is no longer valid.`); } } From 6af7a434b5a67ce42d14a6e8a7a1cefc35ce75b6 Mon Sep 17 00:00:00 2001 From: supaiku Date: Wed, 5 Jun 2019 02:59:34 +0200 Subject: [PATCH 19/34] test: cast to number --- .../integration/core-api/v2/handlers/transactions.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/__tests__/integration/core-api/v2/handlers/transactions.test.ts b/__tests__/integration/core-api/v2/handlers/transactions.test.ts index 37b32ef332..b113683f06 100644 --- a/__tests__/integration/core-api/v2/handlers/transactions.test.ts +++ b/__tests__/integration/core-api/v2/handlers/transactions.test.ts @@ -557,8 +557,8 @@ describe("API 2.0 - Transactions", () => { it.each([3, 5, 8])("should accept and broadcast %i transactions emptying a wallet", async txNumber => { const sender = delegates[txNumber]; // use txNumber so that we use a different delegate for each test case const receivers = generateWallets("testnet", 2); - const amountPlusFee = Math.floor(sender.balance / txNumber); - const lastAmountPlusFee = sender.balance - (txNumber - 1) * amountPlusFee; + const amountPlusFee = Math.floor(+sender.balance / txNumber); + const lastAmountPlusFee = +sender.balance - (txNumber - 1) * amountPlusFee; const transactions = TransactionFactory.transfer(receivers[0].address, amountPlusFee - transferFee) .withNetwork("testnet") @@ -591,8 +591,8 @@ describe("API 2.0 - Transactions", () => { async txNumber => { const sender = delegates[txNumber + 1]; // use txNumber + 1 so that we don't use the same delegates as the above test const receivers = generateWallets("testnet", 2); - const amountPlusFee = Math.floor(sender.balance / txNumber); - const lastAmountPlusFee = sender.balance - (txNumber - 1) * amountPlusFee + 1; + const amountPlusFee = Math.floor(+sender.balance / txNumber); + const lastAmountPlusFee = +sender.balance - (txNumber - 1) * amountPlusFee + 1; const transactions = TransactionFactory.transfer(receivers[0].address, amountPlusFee - transferFee) .withNetwork("testnet") From d91484ee6b5147a394ffe268b9a9ffd283a4be30 Mon Sep 17 00:00:00 2001 From: supaiku Date: Wed, 5 Jun 2019 03:36:13 +0200 Subject: [PATCH 20/34] test: cleanup --- __tests__/helpers/transaction-factory.ts | 54 +----------------------- 1 file changed, 1 insertion(+), 53 deletions(-) diff --git a/__tests__/helpers/transaction-factory.ts b/__tests__/helpers/transaction-factory.ts index 5143f94c48..b0285575f6 100644 --- a/__tests__/helpers/transaction-factory.ts +++ b/__tests__/helpers/transaction-factory.ts @@ -1,5 +1,4 @@ import { Identities, Interfaces, Managers, Transactions, Types, Utils } from "@arkecosystem/crypto"; -import cloneDeep from "lodash.clonedeep"; import { secrets } from "../utils/config/testnet/delegates.json"; const defaultPassphrase: string = secrets[0]; @@ -93,10 +92,6 @@ export class TransactionFactory { private version: number; private senderPublicKey: string; private expiration: number; - private customizedPayload: Array>; - private customizedPayloadOptions: { - quantity?: number; - }; public constructor(builder) { this.builder = builder; @@ -177,17 +172,6 @@ export class TransactionFactory { return this.create(1)[0]; } - public withCustomizedPayload( - customizedPayload: Array>, - options: { - quantity?: number; - } = {}, - ): TransactionFactory { - this.customizedPayload = customizedPayload; - this.customizedPayloadOptions = options; - return this; - } - public build(quantity: number = 1): Interfaces.ITransaction[] { return this.make(quantity, "build"); } @@ -209,9 +193,6 @@ export class TransactionFactory { Managers.configManager.setFromPreset(this.network); const transactions: T[] = []; - - let countCustomized: number = 0; - for (let i = 0; i < quantity; i++) { if (this.builder.constructor.name === "TransferBuilder") { // @FIXME: when we use any of the "withPassphrase*" methods the builder will @@ -266,40 +247,7 @@ export class TransactionFactory { } } - if ( - this.customizedPayload && - (!this.customizedPayloadOptions.quantity || this.customizedPayloadOptions.quantity > countCustomized) - ) { - countCustomized++; - - const payload = this.customizedPayload[countCustomized % this.customizedPayload.length]; - const transaction: Interfaces.ITransaction = this.builder[method](); - transaction.data = Object.assign(cloneDeep(transaction.data), payload); - - if (!payload.signature) { - transaction.data.signature = Transactions.Signer.sign( - transaction.data, - Identities.Keys.fromPassphrase(this.passphrase), - ); - } - - // TODO: second sig - // TODO: multi sig - - if (!payload.id) { - transaction.data.id = Transactions.Utils.getId(transaction.data); - } - - if (!transaction.serialized) { - // TODO: exclude certain malformed properties that cannot be serialized properly - // like signatures that are too long. Add support for customizing signatures. - Transactions.Serializer.serialize(transaction); - } - - transactions.push((transaction as unknown) as T); - } else { - transactions.push(this.builder[method]()); - } + transactions.push(this.builder[method]()); } return transactions; From a6021dd2f38e1e5dafa1048aad4833271dd45f19 Mon Sep 17 00:00:00 2001 From: supaiku Date: Wed, 5 Jun 2019 03:36:27 +0200 Subject: [PATCH 21/34] test: use customSerialize --- .../connection.forging.test.ts | 153 ++++++++---------- 1 file changed, 66 insertions(+), 87 deletions(-) diff --git a/__tests__/unit/core-transaction-pool/connection.forging.test.ts b/__tests__/unit/core-transaction-pool/connection.forging.test.ts index b67085565f..3d42b9e2c6 100644 --- a/__tests__/unit/core-transaction-pool/connection.forging.test.ts +++ b/__tests__/unit/core-transaction-pool/connection.forging.test.ts @@ -7,16 +7,7 @@ import ByteBuffer from "bytebuffer"; import { Wallets } from "@arkecosystem/core-state"; import { Handlers } from "@arkecosystem/core-transactions"; -import { - Constants, - Crypto, - Identities, - Interfaces, - Managers, - Networks, - Transactions, - Utils, -} from "@arkecosystem/crypto"; +import { Constants, Crypto, Identities, Interfaces, Managers, Transactions, Utils } from "@arkecosystem/crypto"; import { Connection } from "../../../packages/core-transaction-pool/src/connection"; import { defaults } from "../../../packages/core-transaction-pool/src/defaults"; import { Memory } from "../../../packages/core-transaction-pool/src/memory"; @@ -43,8 +34,6 @@ beforeAll(async () => { }); await connection.make(); - - // jest.spyOn(Transactions.Verifier, "verify").mockReturnValue(true); }); const mockCurrentHeight = (height: number) => { @@ -170,81 +159,6 @@ describe("Connection", () => { }; describe("getTransactionsForForging", () => { - it("should remove transactions that have expired [5 Good, 5 Bad]", async () => { - mockCurrentHeight(100); - - const transactions = TransactionFactory.transfer() - .withCustomizedPayload([{ expiration: 1 }], { quantity: 5 }) - .build(10); - - await expectForgingTransactions(transactions, 5); - }); - - it("should remove transactions that have a fee of 0 or less [8 Good, 2 Bad]", async () => { - const transactions = TransactionFactory.transfer() - .withCustomizedPayload( - [ - { amount: Utils.BigNumber.make(1000), fee: Utils.BigNumber.ZERO }, - { amount: Utils.BigNumber.make(1000), fee: Utils.BigNumber.make(-100) }, - ], - { quantity: 2 }, - ) - .build(10); - - await expectForgingTransactions(transactions, 8); - }); - - it("should remove transactions that have an amount of 0 or less [8 Good, 2 Bad]", async () => { - const transactions = TransactionFactory.transfer() - .withCustomizedPayload( - [ - { amount: Utils.BigNumber.ZERO, fee: Utils.BigNumber.ONE }, - { amount: Utils.BigNumber.make(-1), fee: Utils.BigNumber.ONE }, - ], - { quantity: 2 }, - ) - .build(10); - - await expectForgingTransactions(transactions, 8); - }); - - it("should remove transactions that have data from another network [5 Good, 5 Bad]", async () => { - const transactions = TransactionFactory.transfer() - .withCustomizedPayload( - [{ recipientId: Identities.Address.fromPassphrase("secret", Networks.devnet.network.pubKeyHash) }], - { quantity: 5 }, - ) - .build(10); - - await expectForgingTransactions(transactions, 5); - }); - - it("should remove transactions that have wrong sender public keys [5 Good, 5 Bad]", async () => { - const transactions = TransactionFactory.transfer() - .withCustomizedPayload([{ senderPublicKey: Identities.PublicKey.fromPassphrase("this is wrong") }], { - quantity: 5, - }) - .build(10); - - await expectForgingTransactions(transactions, 5); - }); - - it("should remove transactions that have timestamps in the future [5 Good, 5 Bad]", async () => { - const transactions = TransactionFactory.transfer() - .withCustomizedPayload([{ timestamp: Crypto.Slots.getTime() + 100 * 1000 }], { quantity: 5 }) - .build(10); - - await expectForgingTransactions(transactions, 5); - }); - - it("should remove transactions that have different IDs when entering and leaving [8 Good, 2 Bad]", async () => { - const transactions = TransactionFactory.transfer() - .withCustomizedPayload([{ id: "garbage" }, { id: "garbage 2" }], { quantity: 2 }) - .build(10); - - await expectForgingTransactions(transactions, 8); - }); - it("should call `TransactionFactory.fromBytes`", async () => { const transactions = TransactionFactory.transfer().build(5); const spy = jest.spyOn(Transactions.TransactionFactory, "fromBytes"); @@ -287,6 +201,45 @@ describe("Connection", () => { await expectForgingTransactions(transactions, 5); }); + it("should remove transactions that have data from another network", async () => { + const transactions = TransactionFactory.transfer().build(5); + + transactions[0].serialized = customSerialize(transactions[0].data, { + network: (b: ByteBuffer) => b.writeUint8(3), + }); + + await expectForgingTransactions(transactions, 4); + }); + + it("should remove transactions that have wrong sender public keys", async () => { + const transactions = TransactionFactory.transfer().build(5); + + transactions[0].serialized = customSerialize(transactions[0].data, { + senderPublicKey: (b: ByteBuffer) => + b.append(Buffer.from(Identities.PublicKey.fromPassphrase("garbage"), "hex")), + }); + + await expectForgingTransactions(transactions, 4); + }); + + it("should remove transactions that have timestamps in the future", async () => { + const transactions = TransactionFactory.transfer().build(5); + + transactions[0].serialized = customSerialize(transactions[0].data, { + timestamp: (b: ByteBuffer) => b.writeUint32(Crypto.Slots.getTime() + 100 * 1000), + }); + + await expectForgingTransactions(transactions, 4); + }); + + it("should remove transactions that have different IDs when entering and leaving", async () => { + const transactions = TransactionFactory.transfer().build(5); + + transactions[0].data.id = "garbage"; + + await expectForgingTransactions(transactions, 4); + }); + it("should remove transactions that have an unknown type", async () => { const transactions = TransactionFactory.transfer().build(2); transactions[0].serialized = customSerialize(transactions[0].data, { @@ -331,6 +284,32 @@ describe("Connection", () => { await expectForgingTransactions(transactions, 1); }); + it("should remove transactions that have expired", async () => { + mockCurrentHeight(100); + + const transactions = TransactionFactory.transfer().build(5); + + transactions[0].serialized = customSerialize(transactions[0].data, { + expiration: (b: ByteBuffer) => b.writeByte(0x01), + }); + + await expectForgingTransactions(transactions, 4); + }); + + it("should remove transactions that have an amount or fee of 0", async () => { + const transactions = TransactionFactory.transfer().build(5); + + transactions[0].serialized = customSerialize(transactions[0].data, { + fee: (b: ByteBuffer) => b.writeByte(0x00), + }); + + transactions[1].serialized = customSerialize(transactions[0].data, { + amount: (b: ByteBuffer) => b.writeByte(0), + }); + + await expectForgingTransactions(transactions, 3); + }); + it("should remove transactions that have been altered after entering the pool", async () => { const transactions = TransactionFactory.transfer().build(2); transactions[0].data.id = transactions[0].data.id From 85b1c94e34a59d896d6786f94a71a9a97ad3bc34 Mon Sep 17 00:00:00 2001 From: supaiku Date: Wed, 5 Jun 2019 04:35:30 +0200 Subject: [PATCH 22/34] test: update remaining todo --- .../unit/core-transaction-pool/connection.forging.test.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/__tests__/unit/core-transaction-pool/connection.forging.test.ts b/__tests__/unit/core-transaction-pool/connection.forging.test.ts index 3d42b9e2c6..046b2ee3a0 100644 --- a/__tests__/unit/core-transaction-pool/connection.forging.test.ts +++ b/__tests__/unit/core-transaction-pool/connection.forging.test.ts @@ -507,10 +507,7 @@ describe("Connection", () => { await expectForgingTransactions(transactions, 4); }); - it.todo("should remove transactions that have unknown properties"); - it.todo("should remove transactions that have missing properties"); - it.todo("should remove transactions that have malformed properties"); - + // TOOD: make `getTransactionsForForging` logic reusable and call it during `loadAll`. it.todo("should remove transactions that have been persisted to the disk"); }); }); From f956d7ee33563fad5403276c3a8ed952327377da Mon Sep 17 00:00:00 2001 From: supaiku Date: Wed, 5 Jun 2019 13:32:39 +0200 Subject: [PATCH 23/34] refactor: apply same checks in `loadAll` --- .../core-transaction-pool/src/connection.ts | 70 ++++++++++--------- 1 file changed, 38 insertions(+), 32 deletions(-) diff --git a/packages/core-transaction-pool/src/connection.ts b/packages/core-transaction-pool/src/connection.ts index ef502a2709..7db91f78fa 100644 --- a/packages/core-transaction-pool/src/connection.ts +++ b/packages/core-transaction-pool/src/connection.ts @@ -47,16 +47,18 @@ export class Connection implements TransactionPool.IConnection { this.memory.flush(); this.storage.connect(this.options.storage); - const all: Interfaces.ITransaction[] = this.storage.loadAll(); + let transactionsFromDisk: Interfaces.ITransaction[] = this.storage.loadAll(); + const validTransactions = await this.getValidTransactions(transactionsFromDisk); - for (const transaction of all) { + transactionsFromDisk = transactionsFromDisk.filter(transaction => + validTransactions.includes(transaction.serialized.toString("hex")), + ); + + for (const transaction of transactionsFromDisk) { this.memory.remember(transaction, true); } this.purgeExpired(); - - await this.removeForgedTransactions(all); - this.purgeInvalidTransactions(); this.emitter.on("internal.milestone.changed", () => this.purgeInvalidTransactions()); @@ -149,33 +151,7 @@ export class Connection implements TransactionPool.IConnection { this.options.maxTransactionBytes, ); - const transactions: string[] = []; - const forgedIds: string[] = await this.removeForgedTransactions(transactionMemory); - - const unforgedTransactions = transactionMemory.filter( - (transaction: Interfaces.ITransaction) => !forgedIds.includes(transaction.id), - ); - - for (const transaction of unforgedTransactions) { - try { - const deserialized: Interfaces.ITransaction = Transactions.TransactionFactory.fromBytes( - transaction.serialized, - ); - - strictEqual(transaction.id, deserialized.id); - - const walletManager: State.IWalletManager = this.databaseService.walletManager; - const sender: State.IWallet = walletManager.findByPublicKey(transaction.data.senderPublicKey); - Handlers.Registry.get(transaction.type).canBeApplied(transaction, sender, walletManager); - - transactions.push(deserialized.serialized.toString("hex")); - } catch (error) { - this.removeTransactionById(transaction.id); - this.logger.error(`Removed ${transaction.id} before forging because it is no longer valid.`); - } - } - - return transactions; + return this.getValidTransactions(transactionMemory); } public getTransactionIdsForForging(start: number, size: number): string[] { @@ -497,6 +473,36 @@ export class Connection implements TransactionPool.IConnection { this.storage.bulkRemoveById(this.memory.pullDirtyRemoved()); } + private async getValidTransactions(transactions: Interfaces.ITransaction[]): Promise { + const validTransactions: string[] = []; + const forgedIds: string[] = await this.removeForgedTransactions(transactions); + + const unforgedTransactions = transactions.filter( + (transaction: Interfaces.ITransaction) => !forgedIds.includes(transaction.id), + ); + + for (const transaction of unforgedTransactions) { + try { + const deserialized: Interfaces.ITransaction = Transactions.TransactionFactory.fromBytes( + transaction.serialized, + ); + + strictEqual(transaction.id, deserialized.id); + + const walletManager: State.IWalletManager = this.databaseService.walletManager; + const sender: State.IWallet = walletManager.findByPublicKey(transaction.data.senderPublicKey); + Handlers.Registry.get(transaction.type).canBeApplied(transaction, sender, walletManager); + + validTransactions.push(deserialized.serialized.toString("hex")); + } catch (error) { + this.removeTransactionById(transaction.id); + this.logger.error(`Removed ${transaction.id} before forging because it is no longer valid.`); + } + } + + return validTransactions; + } + private async removeForgedTransactions(transactions: Interfaces.ITransaction[]): Promise { const forgedIds: string[] = await this.databaseService.getForgedTransactionsIds( transactions.map(({ id }) => id), From 4a1c95238fa667b0807be4eb1b899a090a4bcb24 Mon Sep 17 00:00:00 2001 From: supaiku Date: Wed, 5 Jun 2019 13:33:20 +0200 Subject: [PATCH 24/34] test: remove todo --- .../unit/core-transaction-pool/connection.forging.test.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/__tests__/unit/core-transaction-pool/connection.forging.test.ts b/__tests__/unit/core-transaction-pool/connection.forging.test.ts index 046b2ee3a0..f11d037282 100644 --- a/__tests__/unit/core-transaction-pool/connection.forging.test.ts +++ b/__tests__/unit/core-transaction-pool/connection.forging.test.ts @@ -506,8 +506,5 @@ describe("Connection", () => { await expectForgingTransactions(transactions, 4); }); - - // TOOD: make `getTransactionsForForging` logic reusable and call it during `loadAll`. - it.todo("should remove transactions that have been persisted to the disk"); }); }); From 03c82f007c5656dac30b285268e48c1a7c3e983c Mon Sep 17 00:00:00 2001 From: Brian Faust Date: Wed, 5 Jun 2019 14:55:31 +0300 Subject: [PATCH 25/34] test(core-transaction-pool): index wallet with sufficient balance --- .../core-transaction-pool/connection.test.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/__tests__/unit/core-transaction-pool/connection.test.ts b/__tests__/unit/core-transaction-pool/connection.test.ts index c762c5b47c..3de12a9387 100644 --- a/__tests__/unit/core-transaction-pool/connection.test.ts +++ b/__tests__/unit/core-transaction-pool/connection.test.ts @@ -28,6 +28,15 @@ const delegatesSecrets = delegates.map(d => d.secret); let connection: Connection; let memory: Memory; +const indexWalletWithSufficientBalance = (transaction: Interfaces.ITransaction): void => { + // @ts-ignore + const walletManager = connection.databaseService.walletManager; + + const wallet = walletManager.findByPublicKey(transaction.data.senderPublicKey); + wallet.balance = transaction.data.amount.plus(transaction.data.fee); + walletManager.reindex(wallet); +}; + beforeAll(async () => { memory = new Memory(); @@ -784,6 +793,9 @@ describe("Connection", () => { it("save and restore transactions", async () => { expect(connection.getPoolSize()).toBe(0); + indexWalletWithSufficientBalance(mockData.dummy1); + indexWalletWithSufficientBalance(mockData.dummy4); + const transactions = [mockData.dummy1, mockData.dummy4]; addTransactions(transactions); @@ -808,6 +820,10 @@ describe("Connection", () => { jest.spyOn(databaseService, "getForgedTransactionsIds").mockReturnValue([mockData.dummy2.id]); + indexWalletWithSufficientBalance(mockData.dummy1); + indexWalletWithSufficientBalance(mockData.dummy2); + indexWalletWithSufficientBalance(mockData.dummy4); + const transactions = [mockData.dummy1, mockData.dummy2, mockData.dummy4]; addTransactions(transactions); From eaed5ba8e20c5f1b322b8a2819f20826691fa9f9 Mon Sep 17 00:00:00 2001 From: supaiku Date: Wed, 5 Jun 2019 14:20:24 +0200 Subject: [PATCH 26/34] refactor: move to `getTransactionsData` --- .../__stubs__/connection.ts | 8 +- .../src/versions/1/transactions/controller.ts | 12 ++- .../src/versions/2/transactions/controller.ts | 9 +- .../src/core-transaction-pool/connection.ts | 5 +- .../core-transaction-pool/src/connection.ts | 99 ++++++++++--------- 5 files changed, 66 insertions(+), 67 deletions(-) diff --git a/__tests__/unit/core-transaction-pool/__stubs__/connection.ts b/__tests__/unit/core-transaction-pool/__stubs__/connection.ts index cb8d652df6..7758903031 100644 --- a/__tests__/unit/core-transaction-pool/__stubs__/connection.ts +++ b/__tests__/unit/core-transaction-pool/__stubs__/connection.ts @@ -78,15 +78,11 @@ export class Connection implements TransactionPool.IConnection { return undefined; } - public getTransactions(start: number, size: number, maxBytes?: number): Buffer[] { + public async getTransactions(start: number, size: number, maxBytes?: number): Promise { return []; } - public getTransactionIdsForForging(start: number, size: number): string[] { - return undefined; - } - - public getTransactionsData(start: number, size: number, maxBytes?: number): Interfaces.ITransaction[] { + public async getTransactionIdsForForging(start: number, size: number): Promise { return undefined; } diff --git a/packages/core-api/src/versions/1/transactions/controller.ts b/packages/core-api/src/versions/1/transactions/controller.ts index 3ae9523913..18507fccb7 100644 --- a/packages/core-api/src/versions/1/transactions/controller.ts +++ b/packages/core-api/src/versions/1/transactions/controller.ts @@ -33,11 +33,13 @@ export class TransactionsController extends Controller { try { const pagination = super.paginate(request); - const transactions = this.transactionPool - .getTransactions(pagination.offset, pagination.limit, 0) - .map(transaction => ({ - serialized: transaction, - })); + const transactions = (await this.transactionPool.getTransactions( + pagination.offset, + pagination.limit, + 0, + )).map(transaction => ({ + serialized: transaction, + })); return super.respondWith({ transactions: super.toCollection(request, transactions, "transaction"), diff --git a/packages/core-api/src/versions/2/transactions/controller.ts b/packages/core-api/src/versions/2/transactions/controller.ts index 68b1d994a4..facbbd51cc 100644 --- a/packages/core-api/src/versions/2/transactions/controller.ts +++ b/packages/core-api/src/versions/2/transactions/controller.ts @@ -59,10 +59,11 @@ export class TransactionsController extends Controller { try { const pagination = super.paginate(request); - const transactions = this.transactionPool.getTransactions(pagination.offset, pagination.limit); - const data = transactions.map(transaction => ({ - serialized: transaction.toString("hex"), - })); + const data = (await this.transactionPool.getTransactions(pagination.offset, pagination.limit)).map( + transaction => ({ + serialized: transaction.toString("hex"), + }), + ); return super.toPagination( request, diff --git a/packages/core-interfaces/src/core-transaction-pool/connection.ts b/packages/core-interfaces/src/core-transaction-pool/connection.ts index 708ef2f60a..307d2aca03 100644 --- a/packages/core-interfaces/src/core-transaction-pool/connection.ts +++ b/packages/core-interfaces/src/core-transaction-pool/connection.ts @@ -28,10 +28,9 @@ export interface IConnection { buildWallets(): Promise; flush(): void; getTransaction(id: string): Interfaces.ITransaction; - getTransactionIdsForForging(start: number, size: number): string[]; - getTransactions(start: number, size: number, maxBytes?: number): Buffer[]; + getTransactionIdsForForging(start: number, size: number): Promise; + getTransactions(start: number, size: number, maxBytes?: number): Promise; getTransactionsByType(type: any): any; - getTransactionsData(start: number, size: number, maxBytes?: number): Interfaces.ITransaction[]; getTransactionsForForging(blockSize: number): Promise; has(transactionId: string): any; hasExceededMaxTransactions(senderPublicKey: string): boolean; diff --git a/packages/core-transaction-pool/src/connection.ts b/packages/core-transaction-pool/src/connection.ts index 7db91f78fa..4f44024c85 100644 --- a/packages/core-transaction-pool/src/connection.ts +++ b/packages/core-transaction-pool/src/connection.ts @@ -138,67 +138,24 @@ export class Connection implements TransactionPool.IConnection { return this.memory.getById(id); } - public getTransactions(start: number, size: number, maxBytes?: number): Buffer[] { - return this.getTransactionsData(start, size, maxBytes).map( + public async getTransactions(start: number, size: number, maxBytes?: number): Promise { + return (await this.getTransactionsData(start, size, maxBytes)).map( (transaction: Interfaces.ITransaction) => transaction.serialized, ); } public async getTransactionsForForging(blockSize: number): Promise { - const transactionMemory: Interfaces.ITransaction[] = this.getTransactionsData( - 0, - blockSize, - this.options.maxTransactionBytes, + return (await this.getTransactionsData(0, blockSize, this.options.maxTransactionBytes)).map(transaction => + transaction.serialized.toString("hex"), ); - - return this.getValidTransactions(transactionMemory); } - public getTransactionIdsForForging(start: number, size: number): string[] { - return this.getTransactionsData(start, size, this.options.maxTransactionBytes).map( + public async getTransactionIdsForForging(start: number, size: number): Promise { + return (await this.getTransactionsData(start, size, this.options.maxTransactionBytes)).map( (transaction: Interfaces.ITransaction) => transaction.id, ); } - public getTransactionsData(start: number, size: number, maxBytes: number = 0): Interfaces.ITransaction[] { - this.purgeExpired(); - - const data: Interfaces.ITransaction[] = []; - - let transactionBytes: number = 0; - - let i = 0; - for (const transaction of this.memory.allSortedByFee()) { - if (i >= start + size) { - break; - } - - if (i >= start) { - let pushTransaction: boolean = false; - - if (maxBytes > 0) { - const transactionSize: number = JSON.stringify(transaction.data).length; - - if (transactionBytes + transactionSize <= maxBytes) { - transactionBytes += transactionSize; - pushTransaction = true; - } - } else { - pushTransaction = true; - } - - if (pushTransaction) { - data.push(transaction); - i++; - } - } else { - i++; - } - } - - return data; - } - public removeTransactionsForSender(senderPublicKey: string): void { for (const transaction of this.memory.getBySender(senderPublicKey)) { this.removeTransactionById(transaction.id); @@ -408,6 +365,50 @@ export class Connection implements TransactionPool.IConnection { return false; } + private async getTransactionsData( + start: number, + size: number, + maxBytes: number = 0, + ): Promise { + this.purgeExpired(); + + const data: Interfaces.ITransaction[] = []; + + let transactionBytes: number = 0; + + let i = 0; + for (const transaction of this.memory.allSortedByFee()) { + if (i >= start + size) { + break; + } + + if (i >= start) { + let pushTransaction: boolean = false; + + if (maxBytes > 0) { + const transactionSize: number = JSON.stringify(transaction.data).length; + + if (transactionBytes + transactionSize <= maxBytes) { + transactionBytes += transactionSize; + pushTransaction = true; + } + } else { + pushTransaction = true; + } + + if (pushTransaction) { + data.push(transaction); + i++; + } + } else { + i++; + } + } + + const validTransactions = await this.getValidTransactions(data); + return data.filter(transaction => validTransactions.includes(transaction.serialized.toString("hex"))); + } + private addTransaction(transaction: Interfaces.ITransaction): TransactionPool.IAddTransactionResponse { if (this.has(transaction.id)) { this.logger.debug( From 5afbf3bc320291f82f48a4873e5742bf817eb5a5 Mon Sep 17 00:00:00 2001 From: supaiku Date: Wed, 5 Jun 2019 14:27:55 +0200 Subject: [PATCH 27/34] refactor: rename --- packages/core-transaction-pool/src/connection.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/core-transaction-pool/src/connection.ts b/packages/core-transaction-pool/src/connection.ts index 4f44024c85..46f4b8c9ea 100644 --- a/packages/core-transaction-pool/src/connection.ts +++ b/packages/core-transaction-pool/src/connection.ts @@ -48,7 +48,7 @@ export class Connection implements TransactionPool.IConnection { this.storage.connect(this.options.storage); let transactionsFromDisk: Interfaces.ITransaction[] = this.storage.loadAll(); - const validTransactions = await this.getValidTransactions(transactionsFromDisk); + const validTransactions = await this.validateTransactions(transactionsFromDisk); transactionsFromDisk = transactionsFromDisk.filter(transaction => validTransactions.includes(transaction.serialized.toString("hex")), @@ -139,19 +139,19 @@ export class Connection implements TransactionPool.IConnection { } public async getTransactions(start: number, size: number, maxBytes?: number): Promise { - return (await this.getTransactionsData(start, size, maxBytes)).map( + return (await this.getValidTransactions(start, size, maxBytes)).map( (transaction: Interfaces.ITransaction) => transaction.serialized, ); } public async getTransactionsForForging(blockSize: number): Promise { - return (await this.getTransactionsData(0, blockSize, this.options.maxTransactionBytes)).map(transaction => + return (await this.getValidTransactions(0, blockSize, this.options.maxTransactionBytes)).map(transaction => transaction.serialized.toString("hex"), ); } public async getTransactionIdsForForging(start: number, size: number): Promise { - return (await this.getTransactionsData(start, size, this.options.maxTransactionBytes)).map( + return (await this.getValidTransactions(start, size, this.options.maxTransactionBytes)).map( (transaction: Interfaces.ITransaction) => transaction.id, ); } @@ -365,7 +365,7 @@ export class Connection implements TransactionPool.IConnection { return false; } - private async getTransactionsData( + private async getValidTransactions( start: number, size: number, maxBytes: number = 0, @@ -405,7 +405,7 @@ export class Connection implements TransactionPool.IConnection { } } - const validTransactions = await this.getValidTransactions(data); + const validTransactions = await this.validateTransactions(data); return data.filter(transaction => validTransactions.includes(transaction.serialized.toString("hex"))); } @@ -474,7 +474,7 @@ export class Connection implements TransactionPool.IConnection { this.storage.bulkRemoveById(this.memory.pullDirtyRemoved()); } - private async getValidTransactions(transactions: Interfaces.ITransaction[]): Promise { + private async validateTransactions(transactions: Interfaces.ITransaction[]): Promise { const validTransactions: string[] = []; const forgedIds: string[] = await this.removeForgedTransactions(transactions); From bb987e28a491f8f2fbe00a9ae38d0f7984e7575b Mon Sep 17 00:00:00 2001 From: supaiku Date: Wed, 5 Jun 2019 14:32:51 +0200 Subject: [PATCH 28/34] test: make async --- .../core-transaction-pool/connection.test.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/__tests__/unit/core-transaction-pool/connection.test.ts b/__tests__/unit/core-transaction-pool/connection.test.ts index 3de12a9387..34ed940fb9 100644 --- a/__tests__/unit/core-transaction-pool/connection.test.ts +++ b/__tests__/unit/core-transaction-pool/connection.test.ts @@ -411,7 +411,7 @@ describe("Connection", () => { }); describe("getTransactions", () => { - it("should return transactions within the specified range", () => { + it("should return transactions within the specified range", async () => { const transactions = [mockData.dummy1, mockData.dummy2]; addTransactions(transactions); @@ -421,9 +421,9 @@ describe("Connection", () => { } for (const i of [0, 1]) { - const retrieved = connection - .getTransactions(i, 1) - .map(serializedTx => Transactions.TransactionFactory.fromBytes(serializedTx)); + const retrieved = (await connection.getTransactions(i, 1)).map(serializedTx => + Transactions.TransactionFactory.fromBytes(serializedTx), + ); expect(retrieved.length).toBe(1); expect(retrieved[0]).toBeObject(); @@ -454,7 +454,7 @@ describe("Connection", () => { expect(transactionIds[5]).toBe(mockData.dummy6.id); }); - it("should only return transaction ids for transactions not exceeding the maximum payload size", () => { + it("should only return transaction ids for transactions not exceeding the maximum payload size", async () => { // @FIXME: Uhm excuse me, what the? mockData.dummyLarge1.data.signatures = mockData.dummyLarge2.data.signatures = [""]; for (let i = 0; i < connection.options.maxTransactionBytes * 0.6; i++) { @@ -476,7 +476,7 @@ describe("Connection", () => { addTransactions(transactions); - let transactionIds = connection.getTransactionIdsForForging(0, 7); + let transactionIds = await connection.getTransactionIdsForForging(0, 7); expect(transactionIds).toBeArray(); expect(transactionIds.length).toBe(6); expect(transactionIds[0]).toBe(mockData.dummyLarge1.id); @@ -493,7 +493,7 @@ describe("Connection", () => { connection.removeTransactionById(mockData.dummy6.id); connection.removeTransactionById(mockData.dummy7.id); - transactionIds = connection.getTransactionIdsForForging(0, 7); + transactionIds = await connection.getTransactionIdsForForging(0, 7); expect(transactionIds).toBeArray(); expect(transactionIds.length).toBe(1); expect(transactionIds[0]).toBe(mockData.dummyLarge2.id); @@ -920,7 +920,7 @@ describe("Connection", () => { connection.addTransactions([testTransactions[0]]); }); - it("add many then get first few", () => { + it("add many then get first few", async () => { const nAdd = 2000; // We use a predictable random number calculator in order to get @@ -947,7 +947,7 @@ describe("Connection", () => { .map(f => f.toString()); // console.time(`time to get first ${nGet}`) - const topTransactionsSerialized = connection.getTransactions(0, nGet); + const topTransactionsSerialized = await connection.getTransactions(0, nGet); // console.timeEnd(`time to get first ${nGet}`) const topFeesReceived = topTransactionsSerialized.map(e => From e14c28868ad5eea6a49d276fa586eca7e3090dc5 Mon Sep 17 00:00:00 2001 From: Brian Faust Date: Wed, 5 Jun 2019 15:51:25 +0300 Subject: [PATCH 29/34] test(core-blockchain): add missing await --- __tests__/integration/core-blockchain/blockchain.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/__tests__/integration/core-blockchain/blockchain.test.ts b/__tests__/integration/core-blockchain/blockchain.test.ts index c90206d6d1..cffb9a5d84 100644 --- a/__tests__/integration/core-blockchain/blockchain.test.ts +++ b/__tests__/integration/core-blockchain/blockchain.test.ts @@ -95,7 +95,7 @@ describe("Blockchain", () => { blockchain.transactionPool.flush(); await blockchain.postTransactions(transactionsWithoutType2); - const transactions = blockchain.transactionPool.getTransactions(0, 200); + const transactions = await blockchain.transactionPool.getTransactions(0, 200); expect(transactions.length).toBe(transactionsWithoutType2.length); From 9957dbf225e62b7928ce0b345b47d40b06a6d742 Mon Sep 17 00:00:00 2001 From: supaiku Date: Wed, 5 Jun 2019 15:12:25 +0200 Subject: [PATCH 30/34] test(core-transaction-pool): add missing await --- .../unit/core-transaction-pool/connection.test.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/__tests__/unit/core-transaction-pool/connection.test.ts b/__tests__/unit/core-transaction-pool/connection.test.ts index 34ed940fb9..6725cb48aa 100644 --- a/__tests__/unit/core-transaction-pool/connection.test.ts +++ b/__tests__/unit/core-transaction-pool/connection.test.ts @@ -651,17 +651,19 @@ describe("Connection", () => { expect(+mockWallet.balance).toBe(+balanceBefore.minus(block2.totalFee)); }); - it("should remove transaction from pool if it's in the chained block", () => { + it("should remove transaction from pool if it's in the chained block", async () => { addTransactions([mockData.dummy2]); - expect(connection.getTransactions(0, 10)).toEqual([mockData.dummy2.serialized]); + let transactions = await connection.getTransactions(0, 10); + expect(transactions).toEqual([mockData.dummy2.serialized]); const chainedBlock = BlockFactory.fromData(block2); chainedBlock.transactions.push(mockData.dummy2); connection.acceptChainedBlock(chainedBlock); - expect(connection.getTransactions(0, 10)).toEqual([]); + transactions = await connection.getTransactions(0, 10); + expect(transactions).toEqual([]); }); it("should purge and block sender if throwIfApplyingFails() failed for a transaction in the chained block", () => { @@ -712,7 +714,8 @@ describe("Connection", () => { it("should build wallets from transactions in the pool", async () => { addTransactions([mockData.dummy1]); - expect(connection.getTransactions(0, 10)).toEqual([mockData.dummy1.serialized]); + const transactions = await connection.getTransactions(0, 10); + expect(transactions).toEqual([mockData.dummy1.serialized]); await connection.buildWallets(); @@ -872,7 +875,7 @@ describe("Connection", () => { return testTransactions; }; - it("multiple additions and retrievals", () => { + it("multiple additions and retrievals", async () => { // Abstract number which decides how many iterations are run by the test. // Increase it to run more iterations. const testSize = connection.options.syncInterval * 2; @@ -899,7 +902,7 @@ describe("Connection", () => { connection.hasExceededMaxTransactions(senderPublicKey); } connection.getTransaction(transaction.id); - connection.getTransactions(0, i); + await connection.getTransactions(0, i); } for (let i = 0; i < testSize; i++) { From 8cfb5c4a221a99cff1cbf45cac9c978ce262a0e7 Mon Sep 17 00:00:00 2001 From: supaiku Date: Wed, 5 Jun 2019 15:20:29 +0200 Subject: [PATCH 31/34] test(core-transaction-pool): missing await --- .../unit/core-transaction-pool/connection.test.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/__tests__/unit/core-transaction-pool/connection.test.ts b/__tests__/unit/core-transaction-pool/connection.test.ts index 6725cb48aa..5ba6bc3c75 100644 --- a/__tests__/unit/core-transaction-pool/connection.test.ts +++ b/__tests__/unit/core-transaction-pool/connection.test.ts @@ -143,7 +143,7 @@ describe("Connection", () => { connection.options.maxTransactionsInPool = maxTransactionsInPoolOrig; }); - it("should replace lowest fee transaction when adding 1 more transaction than maxTransactionsInPool", () => { + it("should replace lowest fee transaction when adding 1 more transaction than maxTransactionsInPool", async () => { expect(connection.getPoolSize()).toBe(0); connection.addTransactions([ @@ -159,7 +159,9 @@ describe("Connection", () => { connection.options.maxTransactionsInPool = 4; expect(connection.addTransactions([mockData.dummy5])).toEqual({}); - expect(connection.getTransactionIdsForForging(0, 10)).toEqual([ + + const transactionIds = await connection.getTransactionIdsForForging(0, 10); + expect(transactionIds).toEqual([ mockData.dummy1.id, mockData.dummy2.id, mockData.dummy3.id, @@ -433,7 +435,7 @@ describe("Connection", () => { }); describe("getTransactionIdsForForging", () => { - it("should return an array of transactions ids", () => { + it("should return an array of transactions ids", async () => { addTransactions([ mockData.dummy1, mockData.dummy2, @@ -443,7 +445,7 @@ describe("Connection", () => { mockData.dummy6, ]); - const transactionIds = connection.getTransactionIdsForForging(0, 6); + const transactionIds = await connection.getTransactionIdsForForging(0, 6); expect(transactionIds).toBeArray(); expect(transactionIds[0]).toBe(mockData.dummy1.id); @@ -736,7 +738,7 @@ describe("Connection", () => { expect(getTransaction).toHaveBeenCalled(); expect(findByPublicKey).not.toHaveBeenCalled(); - expect(canBeApplied).not.toHaveBeenCalled(); + expect(canBeApplied).toHaveBeenCalled(); expect(applyToSenderInPool).not.toHaveBeenCalled(); }); From d6091b4b7c46ef84dce36bd27a3158b226cd9739 Mon Sep 17 00:00:00 2001 From: supaiku Date: Wed, 5 Jun 2019 15:57:57 +0200 Subject: [PATCH 32/34] test(core-transaction-pool): make it green --- .../core-transaction-pool/connection.test.ts | 47 ++++++++++++------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/__tests__/unit/core-transaction-pool/connection.test.ts b/__tests__/unit/core-transaction-pool/connection.test.ts index 5ba6bc3c75..8db57b5cf4 100644 --- a/__tests__/unit/core-transaction-pool/connection.test.ts +++ b/__tests__/unit/core-transaction-pool/connection.test.ts @@ -33,7 +33,7 @@ const indexWalletWithSufficientBalance = (transaction: Interfaces.ITransaction): const walletManager = connection.databaseService.walletManager; const wallet = walletManager.findByPublicKey(transaction.data.senderPublicKey); - wallet.balance = transaction.data.amount.plus(transaction.data.fee); + wallet.balance = wallet.balance = wallet.balance.plus(transaction.data.amount.plus(transaction.data.fee)); walletManager.reindex(wallet); }; @@ -50,6 +50,10 @@ beforeAll(async () => { // @ts-ignore connection.databaseService.walletManager = new Wallets.WalletManager(); + for (const transaction of Object.values(mockData)) { + indexWalletWithSufficientBalance(transaction); + } + await connection.make(); }); @@ -480,15 +484,13 @@ describe("Connection", () => { let transactionIds = await connection.getTransactionIdsForForging(0, 7); expect(transactionIds).toBeArray(); - expect(transactionIds.length).toBe(6); - expect(transactionIds[0]).toBe(mockData.dummyLarge1.id); - expect(transactionIds[1]).toBe(mockData.dummy3.id); - expect(transactionIds[2]).toBe(mockData.dummy4.id); - expect(transactionIds[3]).toBe(mockData.dummy5.id); - expect(transactionIds[4]).toBe(mockData.dummy6.id); - expect(transactionIds[5]).toBe(mockData.dummy7.id); + expect(transactionIds).toHaveLength(5); + expect(transactionIds[0]).toBe(mockData.dummy3.id); + expect(transactionIds[1]).toBe(mockData.dummy4.id); + expect(transactionIds[2]).toBe(mockData.dummy5.id); + expect(transactionIds[3]).toBe(mockData.dummy6.id); + expect(transactionIds[4]).toBe(mockData.dummy7.id); - connection.removeTransactionById(mockData.dummyLarge1.id); connection.removeTransactionById(mockData.dummy3.id); connection.removeTransactionById(mockData.dummy4.id); connection.removeTransactionById(mockData.dummy5.id); @@ -497,8 +499,7 @@ describe("Connection", () => { transactionIds = await connection.getTransactionIdsForForging(0, 7); expect(transactionIds).toBeArray(); - expect(transactionIds.length).toBe(1); - expect(transactionIds[0]).toBe(mockData.dummyLarge2.id); + expect(transactionIds).toHaveLength(0); }); }); @@ -695,12 +696,18 @@ describe("Connection", () => { let findByPublicKey; let canBeApplied; let applyToSenderInPool; - const findByPublicKeyWallet = new Wallets.Wallet("thisIsAnAddressIMadeUpJustLikeThis"); + const findByPublicKeyWallet = new Wallets.Wallet("ANwc3YQe3EBjuE5sNRacP7fhkngAPaBW4Y"); + findByPublicKeyWallet.publicKey = "02778aa3d5b332965ea2a5ef6ac479ce2478535bc681a098dff1d683ff6eccc417"; + beforeEach(() => { const transactionHandler = Handlers.Registry.get(TransactionTypes.Transfer); canBeApplied = jest.spyOn(transactionHandler, "canBeApplied").mockReturnValue(true); applyToSenderInPool = jest.spyOn(transactionHandler, "applyToSenderInPool").mockReturnValue(); + (connection as any).databaseService.walletManager.findByPublicKey( + mockData.dummy1.data.senderPublicKey, + ).balance = Utils.BigNumber.ZERO; + jest.spyOn(connection.walletManager, "has").mockReturnValue(true); findByPublicKey = jest .spyOn(connection.walletManager, "findByPublicKey") @@ -758,7 +765,7 @@ describe("Connection", () => { findByPublicKeyWallet, (connection as any).databaseService.walletManager, ); - expect(purgeByPublicKey).toHaveBeenCalledWith(mockData.dummy1.data.senderPublicKey); + expect(purgeByPublicKey).not.toHaveBeenCalledWith(mockData.dummy1.data.senderPublicKey); }); }); @@ -932,11 +939,17 @@ describe("Connection", () => { // a deterministic test. const rand = randomSeed.create("0"); - const testTransactions: Interfaces.ITransaction[] = generateTestTransactions(nAdd); + const testTransactions: Interfaces.ITransaction[] = []; for (let i = 0; i < nAdd; i++) { - // This will invalidate the transactions' signatures, not good, but irrelevant for this test. - testTransactions[i].data.fee = Utils.BigNumber.make(rand.intBetween(0.002 * SATOSHI, 2 * SATOSHI)); - testTransactions[i].serialized = Transactions.Utils.toBytes(testTransactions[i].data); + const transaction = TransactionFactory.transfer("AFzQCx5YpGg5vKMBg4xbuYbqkhvMkKfKe5") + .withNetwork("unitnet") + .withFee(rand.intBetween(0.002 * SATOSHI, 2 * SATOSHI)) + .withPassphrase(String(i)) + .build()[0]; + + testTransactions.push(transaction); + + indexWalletWithSufficientBalance(transaction); } // console.time(`time to add ${nAdd}`) From 451e06bcbad32d4742a305f613c0c3d0bd14deb3 Mon Sep 17 00:00:00 2001 From: supaiku Date: Wed, 5 Jun 2019 16:18:59 +0200 Subject: [PATCH 33/34] test(core-blockchain): fix insufficient balance --- .../core-blockchain/blockchain.test.ts | 30 ++++++++++++++----- .../core-transaction-pool/connection.test.ts | 2 +- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/__tests__/integration/core-blockchain/blockchain.test.ts b/__tests__/integration/core-blockchain/blockchain.test.ts index cffb9a5d84..7cce136efe 100644 --- a/__tests__/integration/core-blockchain/blockchain.test.ts +++ b/__tests__/integration/core-blockchain/blockchain.test.ts @@ -55,6 +55,16 @@ const addBlocks = async untilHeight => { } }; +const indexWalletWithSufficientBalance = (transaction: Interfaces.ITransaction): void => { + const walletManager = blockchain.database.walletManager; + + const wallet = walletManager.findByPublicKey(transaction.data.senderPublicKey); + wallet.balance = wallet.balance.abs().plus(transaction.data.amount.plus(transaction.data.fee)); + walletManager.reindex(wallet); + + blockchain.transactionPool.walletManager.reindex(wallet); +}; + describe("Blockchain", () => { beforeAll(async () => { container = await setUp(); @@ -91,19 +101,25 @@ describe("Blockchain", () => { describe("postTransactions", () => { it("should be ok", async () => { - const transactionsWithoutType2 = genesisBlock.transactions.filter(tx => tx.type !== 2); - blockchain.transactionPool.flush(); - await blockchain.postTransactions(transactionsWithoutType2); + + jest.spyOn(blockchain.transactionPool as any, "removeForgedTransactions").mockReturnValue([]); + + for (const transaction of genesisBlock.transactions) { + indexWalletWithSufficientBalance(transaction); + } + + const transferTransactions = genesisBlock.transactions.filter(tx => tx.type === 0); + + await blockchain.postTransactions(transferTransactions); const transactions = await blockchain.transactionPool.getTransactions(0, 200); - expect(transactions.length).toBe(transactionsWithoutType2.length); + expect(transactions.length).toBe(transferTransactions.length); - expect(transactions).toIncludeAllMembers( - transactionsWithoutType2.map(transaction => transaction.serialized), - ); + expect(transactions).toIncludeAllMembers(transferTransactions.map(transaction => transaction.serialized)); blockchain.transactionPool.flush(); + jest.restoreAllMocks(); }); }); diff --git a/__tests__/unit/core-transaction-pool/connection.test.ts b/__tests__/unit/core-transaction-pool/connection.test.ts index 8db57b5cf4..f11b85bff4 100644 --- a/__tests__/unit/core-transaction-pool/connection.test.ts +++ b/__tests__/unit/core-transaction-pool/connection.test.ts @@ -33,7 +33,7 @@ const indexWalletWithSufficientBalance = (transaction: Interfaces.ITransaction): const walletManager = connection.databaseService.walletManager; const wallet = walletManager.findByPublicKey(transaction.data.senderPublicKey); - wallet.balance = wallet.balance = wallet.balance.plus(transaction.data.amount.plus(transaction.data.fee)); + wallet.balance = wallet.balance.plus(transaction.data.amount.plus(transaction.data.fee)); walletManager.reindex(wallet); }; From 073c6ef97e75f40c447aae8943e9da5069e07ca4 Mon Sep 17 00:00:00 2001 From: supaiku Date: Wed, 5 Jun 2019 16:20:01 +0200 Subject: [PATCH 34/34] test(core-blockchain): dont reindex pool wallet --- __tests__/integration/core-blockchain/blockchain.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/__tests__/integration/core-blockchain/blockchain.test.ts b/__tests__/integration/core-blockchain/blockchain.test.ts index 7cce136efe..d616fbe21c 100644 --- a/__tests__/integration/core-blockchain/blockchain.test.ts +++ b/__tests__/integration/core-blockchain/blockchain.test.ts @@ -61,8 +61,6 @@ const indexWalletWithSufficientBalance = (transaction: Interfaces.ITransaction): const wallet = walletManager.findByPublicKey(transaction.data.senderPublicKey); wallet.balance = wallet.balance.abs().plus(transaction.data.amount.plus(transaction.data.fee)); walletManager.reindex(wallet); - - blockchain.transactionPool.walletManager.reindex(wallet); }; describe("Blockchain", () => {