From f02f743666e7db37b923f1fff1a6525f9426d655 Mon Sep 17 00:00:00 2001
From: cgeek <cem.moreau@gmail.com>
Date: Tue, 15 Jan 2019 18:49:39 +0100
Subject: [PATCH] [fix] #1330 Revert of UD consumption was incorrectly done
---
.../dal/indexDAL/common/DividendDaoHandler.ts | 39 +++++----
app/service/BlockchainService.ts | 9 +-
test/integration/branches/branches_revert2.ts | 2 +-
.../consumption-of-several-uds-with-revert.ts | 56 ++++++++++++
.../consumption-on-two-branches.ts | 86 +++++++++++++++++++
test/integration/sources/source-replay.ts | 62 +++++++++++++
test/integration/tools/toolbox.ts | 20 ++++-
test/integration/transactions-csv-cltv-sig.ts | 1 +
8 files changed, 250 insertions(+), 25 deletions(-)
create mode 100644 test/integration/fork-resolution/consumption-of-several-uds-with-revert.ts
create mode 100644 test/integration/fork-resolution/consumption-on-two-branches.ts
create mode 100644 test/integration/sources/source-replay.ts
diff --git a/app/lib/dal/indexDAL/common/DividendDaoHandler.ts b/app/lib/dal/indexDAL/common/DividendDaoHandler.ts
index dc7506c5d..71649a379 100644
--- a/app/lib/dal/indexDAL/common/DividendDaoHandler.ts
+++ b/app/lib/dal/indexDAL/common/DividendDaoHandler.ts
@@ -121,26 +121,31 @@ export class DividendDaoHandler {
}
static unconsumeDividends(m: DividendEntry, number: number, consumedUDsRecoveredByRevert: SimpleUdEntryForWallet[]) {
- const index = m.consumed.indexOf(number)
+ let index;
+ do {
+ index = m.consumed.indexOf(number)
- const src = m.consumedUDs[index].dividend
- consumedUDsRecoveredByRevert.push({
- conditions: 'SIG(' + m.pub + ')',
- pos: m.consumedUDs[index].dividendNumber,
- identifier: m.pub,
- amount: src.amount,
- base: src.base,
- srcType: 'D',
- op: 'CREATE'
- })
+ if (index !== -1) {
+ const src = m.consumedUDs[index].dividend
+ consumedUDsRecoveredByRevert.push({
+ conditions: 'SIG(' + m.pub + ')',
+ pos: m.consumedUDs[index].dividendNumber,
+ identifier: m.pub,
+ amount: src.amount,
+ base: src.base,
+ srcType: 'D',
+ op: 'CREATE'
+ })
- // We put it back as available
- m.availables.push(m.consumedUDs[index].dividendNumber)
- m.dividends.push(m.consumedUDs[index].dividend)
+ // We put it back as available
+ m.availables.push(m.consumedUDs[index].dividendNumber)
+ m.dividends.push(m.consumedUDs[index].dividend)
- // We remove it from consumed
- m.consumed.splice(index, 1)
- m.consumedUDs.splice(index, 1)
+ // We remove it from consumed
+ m.consumed.splice(index, 1)
+ m.consumedUDs.splice(index, 1)
+ }
+ } while (index !== -1);
}
static trimConsumed(m: DividendEntry, belowNumber: number) {
diff --git a/app/service/BlockchainService.ts b/app/service/BlockchainService.ts
index fbda8d34e..a6f96216c 100644
--- a/app/service/BlockchainService.ts
+++ b/app/service/BlockchainService.ts
@@ -219,7 +219,7 @@ export class BlockchainService extends FIFOService {
})
}
- async blockResolution(max = 0): Promise<BlockDTO|null> {
+ async blockResolution(filterFunc: (b: DBBlock) => boolean = () => true): Promise<BlockDTO|null> {
let lastAdded:BlockDTO|null = null
let added:BlockDTO|null
let nbAdded = 0
@@ -227,15 +227,15 @@ export class BlockchainService extends FIFOService {
const current = await this.current()
let potentials = []
if (current) {
- potentials = await this.dal.getForkBlocksFollowing(current)
+ potentials = (await this.dal.getForkBlocksFollowing(current)).filter(filterFunc)
this.logger.info('Block resolution: %s potential blocks after current#%s...', potentials.length, current.number)
} else {
- potentials = await this.dal.getPotentialRootBlocks()
+ potentials = (await this.dal.getPotentialRootBlocks()).filter(filterFunc)
this.logger.info('Block resolution: %s potential blocks for root block...', potentials.length)
}
added = null
let i = 0
- while (!added && i < potentials.length && (!max || nbAdded < max)) {
+ while (!added && i < potentials.length) {
const dto = BlockDTO.fromJSONObject(potentials[i])
try {
if (dto.issuer === this.conf.pair.pub) {
@@ -274,6 +274,7 @@ export class BlockchainService extends FIFOService {
block: newCurrent
})
}
+ return newCurrent
}
revertCurrentBlock() {
diff --git a/test/integration/branches/branches_revert2.ts b/test/integration/branches/branches_revert2.ts
index 0928ee375..78748f5e7 100644
--- a/test/integration/branches/branches_revert2.ts
+++ b/test/integration/branches/branches_revert2.ts
@@ -230,7 +230,7 @@ describe("Revert two blocks", function() {
before(async () => {
await s1.dal.txsDAL.removeAll()
- await s1.resolveExistingBlock(1) // UD block
+ await s1.resolveExistingBlock(b => b.number === 2) // UD block
await cat.sendMoney(19, toc);
await s1.dal.blockDAL.removeForkBlock(3)
await s1.commit({ time: now + 1 });
diff --git a/test/integration/fork-resolution/consumption-of-several-uds-with-revert.ts b/test/integration/fork-resolution/consumption-of-several-uds-with-revert.ts
new file mode 100644
index 000000000..828236e4c
--- /dev/null
+++ b/test/integration/fork-resolution/consumption-of-several-uds-with-revert.ts
@@ -0,0 +1,56 @@
+// Source file from duniter: Crypto-currency software to manage libre currency such as Äž1
+// Copyright (C) 2018 Cedric Moreau <cem.moreau@gmail.com>
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Affero General Public License for more details.
+
+import {assertEqual, writeBasicTestWithConfAnd2Users} from "../tools/test-framework"
+
+describe('Consumption of several UDS with revert', () => writeBasicTestWithConfAnd2Users({
+ dt: 10,
+ udTime0: 1500000000 + 10,
+ ud0: 100,
+}, (test) => {
+
+ const now = 1500000000
+
+ test('should init with a Dividend at block#3', async (s1, cat, tac) => {
+ await cat.createIdentity()
+ await tac.createIdentity()
+ await cat.cert(tac)
+ await tac.cert(cat)
+ await cat.join()
+ await tac.join()
+ await s1.commit({ time: now })
+ await s1.commit({ time: now })
+ await s1.commit({ time: now + 10 })
+ await s1.commit({ time: now + 20 })
+ await s1.commit({ time: now + 20 })
+ assertEqual((await s1._server.dal.dividendDAL.getUDSources(cat.pub)).length, 2)
+ })
+
+ test('should allow cat to send its money', async (s1, cat, tac) => {
+ await s1.commit({ time: now + 20 })
+ await cat.sendMoney(200, tac)
+ await s1.commit({ time: now + 20 })
+ assertEqual((await s1._server.dal.dividendDAL.getUDSources(cat.pub)).length, 0)
+ assertEqual((await s1._server.dal.dividendDAL.getUDSources(tac.pub)).length, 2)
+ assertEqual((await s1._server.dal.sindexDAL.getAvailableForPubkey(tac.pub)).length, 1)
+ })
+
+ test('revert and re-commit the transaction a block earlier than main branch', async (s1, cat, tac) => {
+ await s1.revert()
+ await s1.revert()
+ await s1.resolve()
+ assertEqual((await s1._server.dal.dividendDAL.getUDSources(cat.pub)).length, 0)
+ assertEqual((await s1._server.dal.dividendDAL.getUDSources(tac.pub)).length, 2)
+ assertEqual((await s1._server.dal.sindexDAL.getAvailableForPubkey(tac.pub)).length, 1)
+ })
+}))
diff --git a/test/integration/fork-resolution/consumption-on-two-branches.ts b/test/integration/fork-resolution/consumption-on-two-branches.ts
new file mode 100644
index 000000000..ee3c34f74
--- /dev/null
+++ b/test/integration/fork-resolution/consumption-on-two-branches.ts
@@ -0,0 +1,86 @@
+// Source file from duniter: Crypto-currency software to manage libre currency such as Äž1
+// Copyright (C) 2018 Cedric Moreau <cem.moreau@gmail.com>
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Affero General Public License for more details.
+
+import {assertEqual, assertNotNull, writeBasicTestWithConfAnd2Users} from "../tools/test-framework"
+import {BlockDTO} from "../../../app/lib/dto/BlockDTO"
+
+describe('Parallel consumption', () => writeBasicTestWithConfAnd2Users({
+ dt: 10,
+ udTime0: 1500000000 + 10,
+ ud0: 100,
+ switchOnHeadAdvance: 0,
+}, (test) => {
+
+ const now = 1500000000
+
+ test('should init with a Dividend at block#3', async (s1, cat, tac) => {
+ await cat.createIdentity()
+ await tac.createIdentity()
+ await cat.cert(tac)
+ await tac.cert(cat)
+ await cat.join()
+ await tac.join()
+ await s1.commit({ time: now })
+ await s1.commit({ time: now })
+ await s1.commit({ time: now + 10 })
+ await s1.commit({ time: now + 20 })
+ await s1.commit({ time: now + 20 })
+ assertEqual((await s1._server.dal.dividendDAL.getUDSources(cat.pub)).length, 2)
+ })
+
+ test('should allow cat to send its money', async (s1, cat, tac) => {
+ await s1.commit({ time: now + 20 })
+ await cat.sendMoney(200, tac)
+ await s1.commit({ time: now + 20 })
+ assertEqual((await s1._server.dal.dividendDAL.getUDSources(cat.pub)).length, 0)
+ assertEqual((await s1._server.dal.dividendDAL.getUDSources(tac.pub)).length, 2)
+ assertEqual((await s1._server.dal.sindexDAL.getAvailableForPubkey(tac.pub)).length, 1)
+ })
+
+ test('revert and re-commit the transaction a block earlier than main branch', async (s1, cat, tac) => {
+ await s1.revert()
+ await s1.revert()
+ await cat.sendMoney(200, tac)
+ await s1.justCommit({ time: now + 21 }) // Time with 11 is the fork
+ await s1.resolve(b => b.time === now + 21)
+ await s1.commit({ time: now + 21 })
+ await s1.commit({ time: now + 21 })
+ await s1.commit({ time: now + 21 })
+ assertEqual((await s1._server.dal.dividendDAL.getUDSources(cat.pub)).length, 0)
+ assertEqual((await s1._server.dal.dividendDAL.getUDSources(tac.pub)).length, 2)
+ assertEqual((await s1._server.dal.sindexDAL.getAvailableForPubkey(tac.pub)).length, 1)
+ })
+
+ test('switching on main branch should succeed', async (s1, cat, tac) => {
+ await s1.revert()
+ await s1.revert()
+ await s1.revert()
+ await s1.revert()
+ const resolved = await s1.resolve(b => b.time % 2 === 0)
+ assertEqual(resolved.number, 6)
+ assertEqual((await s1._server.dal.dividendDAL.getUDSources(cat.pub)).length, 0)
+ assertEqual((await s1._server.dal.dividendDAL.getUDSources(cat.pub)).length, 0)
+ assertEqual((await s1._server.dal.dividendDAL.getUDSources(tac.pub)).length, 2)
+ assertEqual((await s1._server.dal.sindexDAL.getAvailableForPubkey(tac.pub)).length, 1)
+ })
+
+ test('switching on fork branch should succeed', async (s1, cat, tac) => {
+ const newHead = await s1.resolveFork()
+ assertNotNull(newHead)
+ assertEqual((newHead as BlockDTO).number, 7)
+ assertEqual((await s1._server.dal.dividendDAL.getUDSources(cat.pub)).length, 0)
+ assertEqual((await s1._server.dal.dividendDAL.getUDSources(cat.pub)).length, 0)
+ assertEqual((await s1._server.dal.dividendDAL.getUDSources(tac.pub)).length, 2)
+ assertEqual((await s1._server.dal.sindexDAL.getAvailableForPubkey(tac.pub)).length, 1)
+ })
+}))
diff --git a/test/integration/sources/source-replay.ts b/test/integration/sources/source-replay.ts
new file mode 100644
index 000000000..e03c56b64
--- /dev/null
+++ b/test/integration/sources/source-replay.ts
@@ -0,0 +1,62 @@
+// Source file from duniter: Crypto-currency software to manage libre currency such as Äž1
+// Copyright (C) 2018 Cedric Moreau <cem.moreau@gmail.com>
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Affero General Public License for more details.
+
+import {assertEqual, writeBasicTestWithConfAnd2Users} from "../tools/test-framework"
+
+describe('Source replay', () => writeBasicTestWithConfAnd2Users({
+ dt: 10,
+ udTime0: 1500000000 + 10,
+ ud0: 100,
+}, (test) => {
+
+ const now = 1500000000
+
+ test('should init with a Dividend at block#3', async (s1, cat, tac) => {
+ await cat.createIdentity()
+ await tac.createIdentity()
+ await cat.cert(tac)
+ await tac.cert(cat)
+ await cat.join()
+ await tac.join()
+ await s1.commit({ time: now })
+ await s1.commit({ time: now })
+ await s1.commit({ time: now + 10 })
+ await s1.commit({ time: now + 10 })
+ assertEqual((await s1._server.dal.dividendDAL.getUDSources(cat.pub)).length, 1)
+ })
+
+ test('should allow cat to send its money', async (s1, cat, tac) => {
+ await cat.sendMoney(100, tac)
+ await s1.commit({ time: now + 10 })
+ assertEqual((await s1._server.dal.dividendDAL.getUDSources(cat.pub)).length, 0)
+ assertEqual((await s1._server.dal.dividendDAL.getUDSources(tac.pub)).length, 1)
+ assertEqual((await s1._server.dal.sindexDAL.getAvailableForPubkey(tac.pub)).length, 1)
+ })
+
+ test('revert and re-commit should not change anything', async (s1, cat, tac) => {
+ await s1.revert()
+ await s1.resolve()
+ assertEqual((await s1._server.dal.dividendDAL.getUDSources(cat.pub)).length, 0)
+ assertEqual((await s1._server.dal.dividendDAL.getUDSources(tac.pub)).length, 1)
+ assertEqual((await s1._server.dal.sindexDAL.getAvailableForPubkey(tac.pub)).length, 1)
+ })
+
+ test('revert and re-commit dividend should not change anything', async (s1, cat, tac) => {
+ await s1.revert()
+ await s1.revert()
+ await s1.resolve()
+ assertEqual((await s1._server.dal.dividendDAL.getUDSources(cat.pub)).length, 0)
+ assertEqual((await s1._server.dal.dividendDAL.getUDSources(tac.pub)).length, 1)
+ assertEqual((await s1._server.dal.sindexDAL.getAvailableForPubkey(tac.pub)).length, 1)
+ })
+}))
diff --git a/test/integration/tools/toolbox.ts b/test/integration/tools/toolbox.ts
index 7edb1fd25..39e3669e5 100644
--- a/test/integration/tools/toolbox.ts
+++ b/test/integration/tools/toolbox.ts
@@ -375,6 +375,20 @@ export class TestingServer {
return this.server.revert()
}
+ async resolve(filterFunc?: (b: DBBlock) => boolean): Promise<BlockDTO> {
+ const blocksResolved = await this.server.BlockchainService.blockResolution(filterFunc)
+ if (!blocksResolved) {
+ await new Promise(res => setTimeout(res, 200))
+ throw Error(DataErrors[DataErrors.BLOCK_WASNT_COMMITTED])
+ }
+ console.log(BlockDTO.fromJSONObject(blocksResolved).getRawSigned())
+ return blocksResolved
+ }
+
+ async resolveFork(): Promise<BlockDTO|null> {
+ return this.server.BlockchainService.forkResolution()
+ }
+
resetHome() {
return this.server.resetHome()
}
@@ -490,7 +504,7 @@ export class TestingServer {
async justCommit(options:any = null) {
const proven = await this.generateNext(options)
- await this.server.writeBlock(proven, true, false)
+ await this.server.writeBlock(proven, false, true)
return proven
}
@@ -505,8 +519,8 @@ export class TestingServer {
return blocksResolved
}
- async resolveExistingBlock(max = 0) {
- const blocksResolved = await this.server.BlockchainService.blockResolution(max)
+ async resolveExistingBlock(filterFunc: (b: DBBlock) => boolean) {
+ const blocksResolved = await this.server.BlockchainService.blockResolution(filterFunc)
if (!blocksResolved) {
throw Error(DataErrors[DataErrors.BLOCK_WASNT_COMMITTED])
}
diff --git a/test/integration/transactions-csv-cltv-sig.ts b/test/integration/transactions-csv-cltv-sig.ts
index a9ada9607..accc64c27 100644
--- a/test/integration/transactions-csv-cltv-sig.ts
+++ b/test/integration/transactions-csv-cltv-sig.ts
@@ -60,6 +60,7 @@ describe("Transaction: CSV+CLTV+1of2SIG", function() {
await cat.sendTX(tx)
const block = await s1.justCommit({ time: now })
block.should.have.property('transactions').length(1)
+ s1.resolve()
await s1.waitToHaveBlock(2)
})
--
GitLab