From 21436df52ee4e92945f2877751398513f8ed808e Mon Sep 17 00:00:00 2001 From: air1one <36802613+air1one@users.noreply.github.com> Date: Mon, 9 Mar 2020 13:51:37 +0400 Subject: [PATCH] fix(core-blockchain): AcceptBlockHandler apply block to tx pool before db (#3590) --- .github/workflows/functional.yml | 48 +++++ .../multipayment.test.ts | 73 ------- .../__support__/index.ts | 0 __tests__/functional/pool/pool.test.ts | 180 ++++++++++++++++++ .../core-blockchain/mocks/transactionPool.ts | 6 + .../__stubs__/connection.ts | 4 + .../handlers/accept-block-handler.ts | 37 +++- .../src/core-transaction-pool/connection.ts | 1 + .../src/functional/index.ts | 1 + .../src/functional/unconfirmed.ts | 41 ++++ .../core-transaction-pool/src/connection.ts | 4 + 11 files changed, 314 insertions(+), 81 deletions(-) delete mode 100644 __tests__/functional/multipayment-applyToRecipient/multipayment.test.ts rename __tests__/functional/{multipayment-applyToRecipient => pool}/__support__/index.ts (100%) create mode 100644 __tests__/functional/pool/pool.test.ts create mode 100644 packages/core-jest-matchers/src/functional/unconfirmed.ts diff --git a/.github/workflows/functional.yml b/.github/workflows/functional.yml index 23f170c8e1..c568cebac1 100644 --- a/.github/workflows/functional.yml +++ b/.github/workflows/functional.yml @@ -824,3 +824,51 @@ jobs: POSTGRES_USER: ark POSTGRES_PASSWORD: password POSTGRES_DB: ark_unitnet + + pool: + runs-on: ubuntu-latest + + services: + postgres: + image: postgres:10.8 + env: + POSTGRES_USER: ark + POSTGRES_PASSWORD: password + POSTGRES_DB: ark_unitnet + ports: + - 5432:5432 + options: --health-cmd pg_isready --health-interval 10s --health-timeout 5s --health-retries 5 + + strategy: + matrix: + node-version: [12.x] + + steps: + - uses: actions/checkout@v1 + - name: Cache node modules + uses: actions/cache@v1 + with: + path: node_modules + key: ${{ runner.os }}-node-${{ hashFiles('**/yarn.lock') }} + restore-keys: ${{ runner.os }}-node- + - name: Use Node.js ${{ matrix.node-version }} + uses: actions/setup-node@v1 + with: + node-version: ${{ matrix.node-version }} + - name: Update system + run: sudo apt-get update -y + - name: Install xsel & postgresql-client + run: sudo apt-get install -q xsel postgresql-client + - name: Install and build packages + run: yarn setup + - name: Create .core/database directory + run: mkdir -p $HOME/.core/database + - name: Functional tests + run: yarn test __tests__/functional/pool/pool.test.ts + + env: + CORE_DB_DATABASE: ark_unitnet + CORE_DB_USERNAME: ark + POSTGRES_USER: ark + POSTGRES_PASSWORD: password + POSTGRES_DB: ark_unitnet diff --git a/__tests__/functional/multipayment-applyToRecipient/multipayment.test.ts b/__tests__/functional/multipayment-applyToRecipient/multipayment.test.ts deleted file mode 100644 index 86665c793a..0000000000 --- a/__tests__/functional/multipayment-applyToRecipient/multipayment.test.ts +++ /dev/null @@ -1,73 +0,0 @@ -import { Identities, Managers } from "@arkecosystem/crypto"; -import delay from "delay"; -import { TransactionFactory } from "../../helpers/transaction-factory"; -import { secrets } from "../../utils/config/testnet/delegates.json"; -import * as support from "./__support__"; - -beforeAll(async () => { - await support.setUp(); - Managers.configManager.setFromPreset("testnet"); -}); -afterAll(support.tearDown); - -describe("Transaction Forging - Multipayment", () => { - /* - * Scenario : - * - init bob and alice wallet - * - send an initial tx from bob to index his wallet in tx pool - * - send a multipayment from alice including bob in payment recipients - * - send bob funds received from multipayment to a random address - * - this last transaction from bob fails if pool wallet is not updated correctly bu multipayment tx - */ - const bobPassphrase = "bob pass phrase"; - const bobAddress = Identities.Address.fromPassphrase(bobPassphrase, 23); - const bobInitialFund = 50 * 1e8; // 50 ARK - - const alicePassphrase = "alice pass phrase"; - const aliceAddress = Identities.Address.fromPassphrase(alicePassphrase, 23); - const aliceInitialFund = 2500 * 1e8; // 2500 ARK - - const randomAddress = Identities.Address.fromPassphrase("ran dom addr", 23); - - it("should accept and forge all the transactions", async () => { - const initialTxToBob = TransactionFactory.transfer(bobAddress, bobInitialFund) - .withPassphrase(secrets[1]) - .createOne(); - const initialTxToAlice = TransactionFactory.transfer(aliceAddress, aliceInitialFund) - .withPassphrase(secrets[2]) - .createOne(); - await expect(initialTxToBob).toBeAccepted(); - await support.forge([initialTxToBob, initialTxToAlice]); - await delay(1000); - - const initialTxFromBob = TransactionFactory.transfer(bobAddress, 1) - .withPassphrase(bobPassphrase) - .createOne(); - await expect(initialTxFromBob).toBeAccepted(); - await support.forge([initialTxFromBob]); - await delay(1000); - - const multipaymentToBobAndAlice = TransactionFactory.multiPayment([ - { - recipientId: bobAddress, - amount: (2000 * 1e8).toFixed(), // 2000 ARK - }, - { - recipientId: aliceAddress, - amount: (10 * 1e8).toFixed(), // 10 ARK - }, - ]) - .withPassphrase(alicePassphrase) - .createOne(); - await support.forge([multipaymentToBobAndAlice]); - await delay(1000); - await expect(multipaymentToBobAndAlice.id).toBeForged(); - - const bobTransfer = TransactionFactory.transfer(randomAddress, 2000 * 1e8) - .withPassphrase(bobPassphrase) - .createOne(); - await expect(bobTransfer).toBeAccepted(); - await support.forge([bobTransfer]); - await delay(1000); - }); -}); diff --git a/__tests__/functional/multipayment-applyToRecipient/__support__/index.ts b/__tests__/functional/pool/__support__/index.ts similarity index 100% rename from __tests__/functional/multipayment-applyToRecipient/__support__/index.ts rename to __tests__/functional/pool/__support__/index.ts diff --git a/__tests__/functional/pool/pool.test.ts b/__tests__/functional/pool/pool.test.ts new file mode 100644 index 0000000000..2aceb90b1e --- /dev/null +++ b/__tests__/functional/pool/pool.test.ts @@ -0,0 +1,180 @@ +import { Identities, Managers, Utils } from "@arkecosystem/crypto"; +import delay from "delay"; +import { TransactionFactory } from "../../helpers/transaction-factory"; +import { secrets } from "../../utils/config/testnet/delegates.json"; +import * as support from "./__support__"; + +beforeAll(async () => { + await support.setUp(); + Managers.configManager.setFromPreset("testnet"); +}); +afterAll(support.tearDown); + +describe("applyToRecipient - Multipayment scenario", () => { + /* + * Scenario : + * - init bob and alice wallet + * - send an initial tx from bob to index his wallet in tx pool + * - send a multipayment from alice including bob in payment recipients + * - send bob funds received from multipayment to a random address + * - this last transaction from bob fails if pool wallet is not updated correctly by multipayment tx + */ + const bobPassphrase = "bob pass phrase1"; + const bobAddress = Identities.Address.fromPassphrase(bobPassphrase, 23); + const bobInitialFund = 50 * 1e8; // 50 ARK + + const alicePassphrase = "alice pass phrase1"; + const aliceAddress = Identities.Address.fromPassphrase(alicePassphrase, 23); + const aliceInitialFund = 2500 * 1e8; // 2500 ARK + + const randomAddress = Identities.Address.fromPassphrase("ran dom addr1", 23); + + it("should correctly update recipients pool wallet balance after a multipayment", async () => { + const initialTxToBob = TransactionFactory.transfer(bobAddress, bobInitialFund) + .withPassphrase(secrets[1]) + .createOne(); + const initialTxToAlice = TransactionFactory.transfer(aliceAddress, aliceInitialFund) + .withPassphrase(secrets[2]) + .createOne(); + await expect(initialTxToBob).toBeAccepted(); + await support.forge([initialTxToBob, initialTxToAlice]); + await delay(1000); + + const initialTxFromBob = TransactionFactory.transfer(bobAddress, 1) + .withPassphrase(bobPassphrase) + .createOne(); + await expect(initialTxFromBob).toBeAccepted(); + await support.forge([initialTxFromBob]); + await delay(1000); + + const multipaymentToBobAndAlice = TransactionFactory.multiPayment([ + { + recipientId: bobAddress, + amount: (2000 * 1e8).toFixed(), // 2000 ARK + }, + { + recipientId: aliceAddress, + amount: (10 * 1e8).toFixed(), // 10 ARK + }, + ]) + .withPassphrase(alicePassphrase) + .createOne(); + await support.forge([multipaymentToBobAndAlice]); + await delay(1000); + await expect(multipaymentToBobAndAlice.id).toBeForged(); + + const bobTransfer = TransactionFactory.transfer(randomAddress, 2000 * 1e8) + .withPassphrase(bobPassphrase) + .createOne(); + await expect(bobTransfer).toBeAccepted(); + await support.forge([bobTransfer]); + await delay(1000); + }); +}); + +describe("applyToRecipient - transfer and multipayment classic scenarios", () => { + it("should not accept a transfer in the pool with more amount than sender balance", async () => { + // just send funds to a new wallet, and try to send more than the funds from this new wallet + const bobPassphrase = "bob pass phrase2"; + const bobAddress = Identities.Address.fromPassphrase(bobPassphrase, 23); + const bobInitialFund = 100 * 1e8; // 100 ARK + + const randomAddress = Identities.Address.fromPassphrase(secrets[1], 23); + const initialTxToBob = TransactionFactory.transfer(bobAddress, bobInitialFund) + .withPassphrase(secrets[1]) + .createOne(); + + await support.forge([initialTxToBob]); + await delay(1000); + + // the fees for this are making the transfer worth more than bob balance + const bobTransferMoreThanBalance = TransactionFactory.transfer(randomAddress, bobInitialFund) + .withPassphrase(bobPassphrase) + .createOne(); + await expect(bobTransferMoreThanBalance).not.toBeAccepted(); + + // now a transaction with fees + amount === balance should pass + const fee = 1e7; + const bobTransferValid = TransactionFactory.transfer(randomAddress, bobInitialFund - fee) + .withPassphrase(bobPassphrase) + .withFee(fee) + .createOne(); + await expect(bobTransferValid).toBeAccepted(); + await delay(1000); + }); + + it("should not accept a transfer in the pool with more amount than sender balance", async () => { + // just send funds to a new wallet with multipayment, and try to send more than the funds from this new wallet + const bobPassphrase = "bob pass phrase3"; + const bobAddress = Identities.Address.fromPassphrase(bobPassphrase, 23); + const bobInitialFund = 100 * 1e8; // 100 ARK + + const randomAddress = Identities.Address.fromPassphrase("a b c", 23); + + const initialTxToBob = TransactionFactory.multiPayment([ + { + recipientId: bobAddress, + amount: bobInitialFund.toFixed(), + }, + { + recipientId: randomAddress, + amount: bobInitialFund.toFixed(), + }, + ]) + .withPassphrase(secrets[1]) + .createOne(); + + await support.forge([initialTxToBob]); + await delay(1000); + + // the fees for this are making the transfer worth more than bob balance + const bobTransferMoreThanBalance = TransactionFactory.transfer(randomAddress, bobInitialFund) + .withPassphrase(bobPassphrase) + .createOne(); + await expect(bobTransferMoreThanBalance).not.toBeAccepted(); + + // now a transaction with fees + amount === balance should pass + const fee = 1e7; + const bobTransferValid = TransactionFactory.transfer(randomAddress, bobInitialFund - fee) + .withPassphrase(bobPassphrase) + .withFee(fee) + .createOne(); + await expect(bobTransferValid).toBeAccepted(); + await delay(1000); + }); +}); + +describe("Pool transactions when AcceptBlockHandler fails", () => { + // just send funds to a new wallet, and try to send more than the funds from this new wallet + const bobPassphrase = "bob pass phrase4"; + const bobAddress = Identities.Address.fromPassphrase(bobPassphrase, 23); + const bobInitialFund = 100 * 1e8; // 100 ARK + + const randomAddress = Identities.Address.fromPassphrase(secrets[1], 23); + + it("should keep transactions in the pool after AcceptBlockHandler fails to accept a block", async () => { + const initialTxToBob = TransactionFactory.transfer(bobAddress, bobInitialFund) + .withPassphrase(secrets[1]) + .createOne(); + + await support.forge([initialTxToBob]); + await delay(1000); + + // a valid tx to accept in the pool + const bobTransfer = TransactionFactory.transfer(randomAddress, 100) + .withPassphrase(bobPassphrase) + .createOne(); + await expect(bobTransfer).toBeAccepted(); + await expect(bobTransfer).toBeUnconfirmed(); + + // this one will make AcceptBlockHandler fail to accept the block + const bobBusinessResignation = TransactionFactory.businessResignation() + .withPassphrase(bobPassphrase) + .withNonce(Utils.BigNumber.ZERO) + .createOne(); + await support.forge([bobBusinessResignation]); + await delay(1000); + + await expect(bobTransfer).toBeUnconfirmed(); + }); +}); diff --git a/__tests__/unit/core-blockchain/mocks/transactionPool.ts b/__tests__/unit/core-blockchain/mocks/transactionPool.ts index 68193fcbdd..36291c7073 100644 --- a/__tests__/unit/core-blockchain/mocks/transactionPool.ts +++ b/__tests__/unit/core-blockchain/mocks/transactionPool.ts @@ -2,4 +2,10 @@ export const transactionPool = { buildWallets: () => undefined, acceptChainedBlock: () => undefined, removeTransactionsById: () => undefined, + flush: () => undefined, + getAllTransactions: () => [], + addTransactions: () => undefined, + walletManager: { + reset: () => undefined, + }, }; diff --git a/__tests__/unit/core-transaction-pool/__stubs__/connection.ts b/__tests__/unit/core-transaction-pool/__stubs__/connection.ts index 627bf0a846..66a62e5f82 100644 --- a/__tests__/unit/core-transaction-pool/__stubs__/connection.ts +++ b/__tests__/unit/core-transaction-pool/__stubs__/connection.ts @@ -69,6 +69,10 @@ export class Connection implements TransactionPool.IConnection { return; } + public getAllTransactions(): Interfaces.ITransaction[] { + return []; + } + public async getTransactionsForForging(blockSize: number): Promise { return []; } diff --git a/packages/core-blockchain/src/processor/handlers/accept-block-handler.ts b/packages/core-blockchain/src/processor/handlers/accept-block-handler.ts index 7c961f3fab..f5a1475116 100644 --- a/packages/core-blockchain/src/processor/handlers/accept-block-handler.ts +++ b/packages/core-blockchain/src/processor/handlers/accept-block-handler.ts @@ -1,3 +1,4 @@ +import { TransactionPool } from "@arkecosystem/core-interfaces"; import { BlockProcessorResult } from "../block-processor"; import { BlockHandler } from "./block-handler"; @@ -5,24 +6,29 @@ export class AcceptBlockHandler extends BlockHandler { public async execute(): Promise { const { database, state, transactionPool } = this.blockchain; + let transactionPoolWasReset: boolean = false; try { - await database.applyBlock(this.block); - - // Check if we recovered from a fork - if (state.forkedBlock && state.forkedBlock.data.height === this.block.data.height) { - this.logger.info("Successfully recovered from fork"); - state.forkedBlock = undefined; - } - if (transactionPool) { try { await transactionPool.acceptChainedBlock(this.block); } catch (error) { + // reset transaction pool as it could be out of sync with db state + await this.resetTransactionPool(transactionPool); + transactionPoolWasReset = true; + this.logger.warn("Issue applying block to transaction pool"); this.logger.debug(error.stack); } } + await database.applyBlock(this.block); + + // Check if we recovered from a fork + if (state.forkedBlock && state.forkedBlock.data.height === this.block.data.height) { + this.logger.info("Successfully recovered from fork"); + state.forkedBlock = undefined; + } + // Reset wake-up timer after chaining a block, since there's no need to // wake up at all if blocks arrive periodically. Only wake up when there are // no new blocks. @@ -39,10 +45,25 @@ export class AcceptBlockHandler extends BlockHandler { return BlockProcessorResult.Accepted; } catch (error) { + if (transactionPool && !transactionPoolWasReset) { + // reset transaction pool as it could be out of sync with db state + await this.resetTransactionPool(transactionPool); + } + this.logger.warn(`Refused new block ${JSON.stringify(this.block.data)}`); this.logger.debug(error.stack); return super.execute(); } } + + private async resetTransactionPool(transactionPool: TransactionPool.IConnection): Promise { + // backup transactions from pool, flush it, reset wallet manager, re-add transactions + const transactions = transactionPool.getAllTransactions(); + + transactionPool.flush(); + transactionPool.walletManager.reset(); + + await transactionPool.addTransactions(transactions); + } } diff --git a/packages/core-interfaces/src/core-transaction-pool/connection.ts b/packages/core-interfaces/src/core-transaction-pool/connection.ts index 7f04a7f83a..ee4eb48b54 100644 --- a/packages/core-interfaces/src/core-transaction-pool/connection.ts +++ b/packages/core-interfaces/src/core-transaction-pool/connection.ts @@ -26,6 +26,7 @@ export interface IConnection { buildWallets(): Promise; replay(transactions: Interfaces.ITransaction[]): Promise; flush(): void; + getAllTransactions(): Interfaces.ITransaction[]; getTransaction(id: string): Promise; getTransactionIdsForForging(start: number, size: number): Promise; getTransactions(start: number, size: number, maxBytes?: number): Promise; diff --git a/packages/core-jest-matchers/src/functional/index.ts b/packages/core-jest-matchers/src/functional/index.ts index a7699103e5..5cb0520da2 100644 --- a/packages/core-jest-matchers/src/functional/index.ts +++ b/packages/core-jest-matchers/src/functional/index.ts @@ -1,3 +1,4 @@ import "./accepted"; import "./forged"; import "./rejected"; +import "./unconfirmed"; diff --git a/packages/core-jest-matchers/src/functional/unconfirmed.ts b/packages/core-jest-matchers/src/functional/unconfirmed.ts new file mode 100644 index 0000000000..1864af3798 --- /dev/null +++ b/packages/core-jest-matchers/src/functional/unconfirmed.ts @@ -0,0 +1,41 @@ +import { Interfaces } from "@arkecosystem/crypto"; +import got from "got"; + +export {}; + +declare global { + namespace jest { + // tslint:disable-next-line:interface-name + interface Matchers { + toBeUnconfirmed(): Promise; + } + } +} + +expect.extend({ + toBeUnconfirmed: async (transaction: Interfaces.ITransactionData) => { + let pass: boolean = false; + let error: string; + + try { + const { body } = await got.get(`http://localhost:4003/api/v2/transactions/unconfirmed`); + + const parsedBody = JSON.parse(body); + + pass = !!(parsedBody.data as any[]).find(tx => tx.id === transaction.id); + + error = JSON.stringify(parsedBody.errors); + } catch (e) { + error = e.message; + console.error(error); + } + + return { + pass, + message: () => + `expected ${transaction.id} ${this.isNot ? "not" : ""} to be unconfirmed (in the pool) ${ + error ? "(error: " + error + ")" : "" + }`, + }; + }, +}); diff --git a/packages/core-transaction-pool/src/connection.ts b/packages/core-transaction-pool/src/connection.ts index a5a091c45d..f0170b79ad 100644 --- a/packages/core-transaction-pool/src/connection.ts +++ b/packages/core-transaction-pool/src/connection.ts @@ -75,6 +75,10 @@ export class Connection implements TransactionPool.IConnection { return new Processor(this); } + public getAllTransactions(): Interfaces.ITransaction[] { + return this.memory.allSortedByFee(); + } + public async getTransactionsByType(type: number, typeGroup?: number): Promise> { if (typeGroup === undefined) { typeGroup = Enums.TransactionTypeGroup.Core;