Skip to content

Commit 21436df

Browse files
air1oneair1one
authored
fix(core-blockchain): AcceptBlockHandler apply block to tx pool before db (#3590)
1 parent 03976a6 commit 21436df

File tree

11 files changed

+314
-81
lines changed

11 files changed

+314
-81
lines changed

‎.github/workflows/functional.yml

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -824,3 +824,51 @@ jobs:
824824
POSTGRES_USER: ark
825825
POSTGRES_PASSWORD: password
826826
POSTGRES_DB: ark_unitnet
827+
828+
pool:
829+
runs-on: ubuntu-latest
830+
831+
services:
832+
postgres:
833+
image: postgres:10.8
834+
env:
835+
POSTGRES_USER: ark
836+
POSTGRES_PASSWORD: password
837+
POSTGRES_DB: ark_unitnet
838+
ports:
839+
- 5432:5432
840+
options: --health-cmd pg_isready --health-interval 10s --health-timeout 5s --health-retries 5
841+
842+
strategy:
843+
matrix:
844+
node-version: [12.x]
845+
846+
steps:
847+
- uses: actions/checkout@v1
848+
- name: Cache node modules
849+
uses: actions/cache@v1
850+
with:
851+
path: node_modules
852+
key: ${{ runner.os }}-node-${{ hashFiles('**/yarn.lock') }}
853+
restore-keys: ${{ runner.os }}-node-
854+
- name: Use Node.js ${{ matrix.node-version }}
855+
uses: actions/setup-node@v1
856+
with:
857+
node-version: ${{ matrix.node-version }}
858+
- name: Update system
859+
run: sudo apt-get update -y
860+
- name: Install xsel & postgresql-client
861+
run: sudo apt-get install -q xsel postgresql-client
862+
- name: Install and build packages
863+
run: yarn setup
864+
- name: Create .core/database directory
865+
run: mkdir -p $HOME/.core/database
866+
- name: Functional tests
867+
run: yarn test __tests__/functional/pool/pool.test.ts
868+
869+
env:
870+
CORE_DB_DATABASE: ark_unitnet
871+
CORE_DB_USERNAME: ark
872+
POSTGRES_USER: ark
873+
POSTGRES_PASSWORD: password
874+
POSTGRES_DB: ark_unitnet

‎__tests__/functional/multipayment-applyToRecipient/multipayment.test.ts

Lines changed: 0 additions & 73 deletions
This file was deleted.

‎__tests__/functional/multipayment-applyToRecipient/__support__/index.ts renamed to ‎__tests__/functional/pool/__support__/index.ts

File renamed without changes.

‎__tests__/functional/pool/pool.test.ts

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
import { Identities, Managers, Utils } from "@arkecosystem/crypto";
2+
import delay from "delay";
3+
import { TransactionFactory } from "../../helpers/transaction-factory";
4+
import { secrets } from "../../utils/config/testnet/delegates.json";
5+
import * as support from "./__support__";
6+
7+
beforeAll(async () => {
8+
await support.setUp();
9+
Managers.configManager.setFromPreset("testnet");
10+
});
11+
afterAll(support.tearDown);
12+
13+
describe("applyToRecipient - Multipayment scenario", () => {
14+
/*
15+
* Scenario :
16+
* - init bob and alice wallet
17+
* - send an initial tx from bob to index his wallet in tx pool
18+
* - send a multipayment from alice including bob in payment recipients
19+
* - send bob funds received from multipayment to a random address
20+
* - this last transaction from bob fails if pool wallet is not updated correctly by multipayment tx
21+
*/
22+
const bobPassphrase = "bob pass phrase1";
23+
const bobAddress = Identities.Address.fromPassphrase(bobPassphrase, 23);
24+
const bobInitialFund = 50 * 1e8; // 50 ARK
25+
26+
const alicePassphrase = "alice pass phrase1";
27+
const aliceAddress = Identities.Address.fromPassphrase(alicePassphrase, 23);
28+
const aliceInitialFund = 2500 * 1e8; // 2500 ARK
29+
30+
const randomAddress = Identities.Address.fromPassphrase("ran dom addr1", 23);
31+
32+
it("should correctly update recipients pool wallet balance after a multipayment", async () => {
33+
const initialTxToBob = TransactionFactory.transfer(bobAddress, bobInitialFund)
34+
.withPassphrase(secrets[1])
35+
.createOne();
36+
const initialTxToAlice = TransactionFactory.transfer(aliceAddress, aliceInitialFund)
37+
.withPassphrase(secrets[2])
38+
.createOne();
39+
await expect(initialTxToBob).toBeAccepted();
40+
await support.forge([initialTxToBob, initialTxToAlice]);
41+
await delay(1000);
42+
43+
const initialTxFromBob = TransactionFactory.transfer(bobAddress, 1)
44+
.withPassphrase(bobPassphrase)
45+
.createOne();
46+
await expect(initialTxFromBob).toBeAccepted();
47+
await support.forge([initialTxFromBob]);
48+
await delay(1000);
49+
50+
const multipaymentToBobAndAlice = TransactionFactory.multiPayment([
51+
{
52+
recipientId: bobAddress,
53+
amount: (2000 * 1e8).toFixed(), // 2000 ARK
54+
},
55+
{
56+
recipientId: aliceAddress,
57+
amount: (10 * 1e8).toFixed(), // 10 ARK
58+
},
59+
])
60+
.withPassphrase(alicePassphrase)
61+
.createOne();
62+
await support.forge([multipaymentToBobAndAlice]);
63+
await delay(1000);
64+
await expect(multipaymentToBobAndAlice.id).toBeForged();
65+
66+
const bobTransfer = TransactionFactory.transfer(randomAddress, 2000 * 1e8)
67+
.withPassphrase(bobPassphrase)
68+
.createOne();
69+
await expect(bobTransfer).toBeAccepted();
70+
await support.forge([bobTransfer]);
71+
await delay(1000);
72+
});
73+
});
74+
75+
describe("applyToRecipient - transfer and multipayment classic scenarios", () => {
76+
it("should not accept a transfer in the pool with more amount than sender balance", async () => {
77+
// just send funds to a new wallet, and try to send more than the funds from this new wallet
78+
const bobPassphrase = "bob pass phrase2";
79+
const bobAddress = Identities.Address.fromPassphrase(bobPassphrase, 23);
80+
const bobInitialFund = 100 * 1e8; // 100 ARK
81+
82+
const randomAddress = Identities.Address.fromPassphrase(secrets[1], 23);
83+
const initialTxToBob = TransactionFactory.transfer(bobAddress, bobInitialFund)
84+
.withPassphrase(secrets[1])
85+
.createOne();
86+
87+
await support.forge([initialTxToBob]);
88+
await delay(1000);
89+
90+
// the fees for this are making the transfer worth more than bob balance
91+
const bobTransferMoreThanBalance = TransactionFactory.transfer(randomAddress, bobInitialFund)
92+
.withPassphrase(bobPassphrase)
93+
.createOne();
94+
await expect(bobTransferMoreThanBalance).not.toBeAccepted();
95+
96+
// now a transaction with fees + amount === balance should pass
97+
const fee = 1e7;
98+
const bobTransferValid = TransactionFactory.transfer(randomAddress, bobInitialFund - fee)
99+
.withPassphrase(bobPassphrase)
100+
.withFee(fee)
101+
.createOne();
102+
await expect(bobTransferValid).toBeAccepted();
103+
await delay(1000);
104+
});
105+
106+
it("should not accept a transfer in the pool with more amount than sender balance", async () => {
107+
// just send funds to a new wallet with multipayment, and try to send more than the funds from this new wallet
108+
const bobPassphrase = "bob pass phrase3";
109+
const bobAddress = Identities.Address.fromPassphrase(bobPassphrase, 23);
110+
const bobInitialFund = 100 * 1e8; // 100 ARK
111+
112+
const randomAddress = Identities.Address.fromPassphrase("a b c", 23);
113+
114+
const initialTxToBob = TransactionFactory.multiPayment([
115+
{
116+
recipientId: bobAddress,
117+
amount: bobInitialFund.toFixed(),
118+
},
119+
{
120+
recipientId: randomAddress,
121+
amount: bobInitialFund.toFixed(),
122+
},
123+
])
124+
.withPassphrase(secrets[1])
125+
.createOne();
126+
127+
await support.forge([initialTxToBob]);
128+
await delay(1000);
129+
130+
// the fees for this are making the transfer worth more than bob balance
131+
const bobTransferMoreThanBalance = TransactionFactory.transfer(randomAddress, bobInitialFund)
132+
.withPassphrase(bobPassphrase)
133+
.createOne();
134+
await expect(bobTransferMoreThanBalance).not.toBeAccepted();
135+
136+
// now a transaction with fees + amount === balance should pass
137+
const fee = 1e7;
138+
const bobTransferValid = TransactionFactory.transfer(randomAddress, bobInitialFund - fee)
139+
.withPassphrase(bobPassphrase)
140+
.withFee(fee)
141+
.createOne();
142+
await expect(bobTransferValid).toBeAccepted();
143+
await delay(1000);
144+
});
145+
});
146+
147+
describe("Pool transactions when AcceptBlockHandler fails", () => {
148+
// just send funds to a new wallet, and try to send more than the funds from this new wallet
149+
const bobPassphrase = "bob pass phrase4";
150+
const bobAddress = Identities.Address.fromPassphrase(bobPassphrase, 23);
151+
const bobInitialFund = 100 * 1e8; // 100 ARK
152+
153+
const randomAddress = Identities.Address.fromPassphrase(secrets[1], 23);
154+
155+
it("should keep transactions in the pool after AcceptBlockHandler fails to accept a block", async () => {
156+
const initialTxToBob = TransactionFactory.transfer(bobAddress, bobInitialFund)
157+
.withPassphrase(secrets[1])
158+
.createOne();
159+
160+
await support.forge([initialTxToBob]);
161+
await delay(1000);
162+
163+
// a valid tx to accept in the pool
164+
const bobTransfer = TransactionFactory.transfer(randomAddress, 100)
165+
.withPassphrase(bobPassphrase)
166+
.createOne();
167+
await expect(bobTransfer).toBeAccepted();
168+
await expect(bobTransfer).toBeUnconfirmed();
169+
170+
// this one will make AcceptBlockHandler fail to accept the block
171+
const bobBusinessResignation = TransactionFactory.businessResignation()
172+
.withPassphrase(bobPassphrase)
173+
.withNonce(Utils.BigNumber.ZERO)
174+
.createOne();
175+
await support.forge([bobBusinessResignation]);
176+
await delay(1000);
177+
178+
await expect(bobTransfer).toBeUnconfirmed();
179+
});
180+
});

‎__tests__/unit/core-blockchain/mocks/transactionPool.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,10 @@ export const transactionPool = {
22
buildWallets: () => undefined,
33
acceptChainedBlock: () => undefined,
44
removeTransactionsById: () => undefined,
5+
flush: () => undefined,
6+
getAllTransactions: () => [],
7+
addTransactions: () => undefined,
8+
walletManager: {
9+
reset: () => undefined,
10+
},
511
};

‎__tests__/unit/core-transaction-pool/__stubs__/connection.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,10 @@ export class Connection implements TransactionPool.IConnection {
6969
return;
7070
}
7171

72+
public getAllTransactions(): Interfaces.ITransaction[] {
73+
return [];
74+
}
75+
7276
public async getTransactionsForForging(blockSize: number): Promise<string[]> {
7377
return [];
7478
}

‎packages/core-blockchain/src/processor/handlers/accept-block-handler.ts

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,34 @@
1+
import { TransactionPool } from "@arkecosystem/core-interfaces";
12
import { BlockProcessorResult } from "../block-processor";
23
import { BlockHandler } from "./block-handler";
34

45
export class AcceptBlockHandler extends BlockHandler {
56
public async execute(): Promise<BlockProcessorResult> {
67
const { database, state, transactionPool } = this.blockchain;
78

9+
let transactionPoolWasReset: boolean = false;
810
try {
9-
await database.applyBlock(this.block);
10-
11-
// Check if we recovered from a fork
12-
if (state.forkedBlock && state.forkedBlock.data.height === this.block.data.height) {
13-
this.logger.info("Successfully recovered from fork");
14-
state.forkedBlock = undefined;
15-
}
16-
1711
if (transactionPool) {
1812
try {
1913
await transactionPool.acceptChainedBlock(this.block);
2014
} catch (error) {
15+
// reset transaction pool as it could be out of sync with db state
16+
await this.resetTransactionPool(transactionPool);
17+
transactionPoolWasReset = true;
18+
2119
this.logger.warn("Issue applying block to transaction pool");
2220
this.logger.debug(error.stack);
2321
}
2422
}
2523

24+
await database.applyBlock(this.block);
25+
26+
// Check if we recovered from a fork
27+
if (state.forkedBlock && state.forkedBlock.data.height === this.block.data.height) {
28+
this.logger.info("Successfully recovered from fork");
29+
state.forkedBlock = undefined;
30+
}
31+
2632
// Reset wake-up timer after chaining a block, since there's no need to
2733
// wake up at all if blocks arrive periodically. Only wake up when there are
2834
// no new blocks.
@@ -39,10 +45,25 @@ export class AcceptBlockHandler extends BlockHandler {
3945

4046
return BlockProcessorResult.Accepted;
4147
} catch (error) {
48+
if (transactionPool && !transactionPoolWasReset) {
49+
// reset transaction pool as it could be out of sync with db state
50+
await this.resetTransactionPool(transactionPool);
51+
}
52+
4253
this.logger.warn(`Refused new block ${JSON.stringify(this.block.data)}`);
4354
this.logger.debug(error.stack);
4455

4556
return super.execute();
4657
}
4758
}
59+
60+
private async resetTransactionPool(transactionPool: TransactionPool.IConnection): Promise<void> {
61+
// backup transactions from pool, flush it, reset wallet manager, re-add transactions
62+
const transactions = transactionPool.getAllTransactions();
63+
64+
transactionPool.flush();
65+
transactionPool.walletManager.reset();
66+
67+
await transactionPool.addTransactions(transactions);
68+
}
4869
}

0 commit comments

Comments
 (0)