fix(core-database): straight delete blocks during rollback (#3773)

ArkEcosystem · Jun 4, 2020 · b87a7a8 · b87a7a8
1 parent 62f5ae9
commit b87a7a8
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 35 deletions.
diff --git a/__tests__/functional/core-database/repositories/block-repository.test.ts b/__tests__/functional/core-database/repositories/block-repository.test.ts
@@ -284,3 +284,17 @@ describe("BlockRepository.listByExpression", () => {
         expect(listResult.rows).toStrictEqual([toBlockModel(block3), toBlockModel(block2)]);
     });
 });
+
+describe("BlockRepository.deleteTopBlocks", () => {
+    it("should delete blocks", async () => {
+        const blockRepository = getCustomRepository(BlockRepository);
+        await blockRepository.saveBlocks([block1, block2, block3]);
+        await blockRepository.deleteTopBlocks(2);
+        const block1ById = await blockRepository.findById(block1.data.id);
+        const block2ById = await blockRepository.findById(block2.data.id);
+        const block3ById = await blockRepository.findById(block3.data.id);
+        expect(block1ById).toStrictEqual(toBlockModel(block1));
+        expect(block2ById).toBeUndefined();
+        expect(block3ById).toBeUndefined();
+    });
+});
diff --git a/__tests__/unit/core-blockchain/blockchain.test.ts b/__tests__/unit/core-blockchain/blockchain.test.ts
@@ -78,6 +78,7 @@ describe("Blockchain", () => {
         databaseService.getActiveDelegates = jest.fn().mockReturnValue([]);
 
         blockRepository.deleteBlocks = jest.fn();
+        blockRepository.deleteTopBlocks = jest.fn();
         blockRepository.saveBlocks = jest.fn();
 
         stateMachine.transition = jest.fn();
@@ -540,34 +541,16 @@ describe("Blockchain", () => {
 
     describe("removeTopBlocks", () => {
         it.each([[1], [5], [1329]])(
-            "should get the top %i blocks from database and delete them with blockRepository",
+            "should call deleteTopBlocks with blockRepository and call loadBlocksFromCurrentRound",
             async (numberOfBlocks) => {
                 const blockchain = sandbox.app.resolve<Blockchain>(Blockchain);
 
-                const mockTopBlocks = [];
-                for (let i = 0; i < numberOfBlocks; i++) {
-                    // @ts-ignore
-                    mockTopBlocks.push({ height: 1000 + i });
-                }
-                databaseService.getTopBlocks.mockReturnValueOnce(mockTopBlocks);
-
                 await blockchain.removeTopBlocks(numberOfBlocks);
 
-                expect(databaseService.getTopBlocks).toHaveBeenLastCalledWith(numberOfBlocks);
-                expect(blockRepository.deleteBlocks).toHaveBeenLastCalledWith(mockTopBlocks);
+                expect(blockRepository.deleteTopBlocks).toHaveBeenLastCalledWith(numberOfBlocks);
                 expect(databaseService.loadBlocksFromCurrentRound).toHaveBeenCalled();
             },
         );
-
-        it("should log an error if deleteBlocks throws", async () => {
-            const blockchain = sandbox.app.resolve<Blockchain>(Blockchain);
-            blockRepository.deleteBlocks.mockRejectedValueOnce(new Error("error deleteBlocks"));
-            databaseService.getTopBlocks.mockReturnValueOnce([{ height: 48990 }]);
-
-            await blockchain.removeTopBlocks(1);
-
-            expect(logService.error).toBeCalledTimes(1);
-        });
     });
 
     describe("processBlocks", () => {

diff --git a/packages/core-blockchain/src/blockchain.ts b/packages/core-blockchain/src/blockchain.ts
@@ -344,22 +344,10 @@ export class Blockchain implements Contracts.Blockchain.Blockchain {
      * @return {void}
      */
     public async removeTopBlocks(count: number): Promise<void> {
-        const blocks: Interfaces.IBlockData[] = await this.database.getTopBlocks(count);
+        this.app.log.info(`Removing top ${Utils.pluralize("block", count, true)}`);
 
-        this.app.log.info(
-            `Removing ${Utils.pluralize(
-                "block",
-                blocks.length,
-                true,
-            )} from height ${(blocks[0] as any).height.toLocaleString()}`,
-        );
-
-        try {
-            await this.blockRepository.deleteBlocks(blocks);
-            await this.database.loadBlocksFromCurrentRound();
-        } catch (error) {
-            this.app.log.error(`Encountered error while removing blocks: ${error.message}`);
-        }
+        await this.blockRepository.deleteTopBlocks(count);
+        await this.database.loadBlocksFromCurrentRound();
     }
 
     /**

diff --git a/packages/core-database/src/repositories/block-repository.ts b/packages/core-database/src/repositories/block-repository.ts
@@ -221,4 +221,52 @@ export class BlockRepository extends AbstractRepository<Block> {
                 .execute();
         });
     }
+
+    public async deleteTopBlocks(count: number): Promise<void> {
+        await this.manager.transaction(async (manager) => {
+            const maxHeightRow = await manager
+                .createQueryBuilder()
+                .select("MAX(height) AS max_height")
+                .from(Block, "blocks")
+                .getRawOne();
+
+            const targetHeight = maxHeightRow["max_height"] - count;
+            const roundInfo = Utils.roundCalculator.calculateRound(targetHeight);
+            const targetRound = roundInfo.round;
+
+            const blockIdRows = await manager
+                .createQueryBuilder()
+                .select(["id"])
+                .from(Block, "blocks")
+                .where("height > :targetHeight", { targetHeight })
+                .getRawMany();
+
+            const blockIds = blockIdRows.map((row) => row["id"]);
+
+            if (blockIds.length !== count) {
+                throw new Error("Corrupt database");
+            }
+
+            await manager
+                .createQueryBuilder()
+                .delete()
+                .from(Transaction)
+                .where("block_id IN (:...blockIds)", { blockIds })
+                .execute();
+
+            await manager
+                .createQueryBuilder()
+                .delete()
+                .from(Block)
+                .where("id IN (:...blockIds)", { blockIds })
+                .execute();
+
+            await manager
+                .createQueryBuilder()
+                .delete()
+                .from(Round)
+                .where("round > :targetRound", { targetRound })
+                .execute();
+        });
+    }
 }