fix(core-database): guaranteed transaction order (#3879)

__tests__/integration/core-api/handlers/transactions.test.ts

@@ -133,6 +133,7 @@ describe("API 2.0 - Transactions", () => {
                 fee: "0",
                 amount: "300000000000000",
                 blockId: expect.anything(), // ? how is that blockId isn't constant
+                blockHeight: expect.anything(),
                 recipientId: genesisTransaction.recipientId,
                 senderPublicKey: genesisTransaction.senderPublicKey,
                 expiration: 0,

__tests__/unit/core-database/__fixtures__/block1760000.ts

@@ -29,8 +29,6 @@ export default {
             signature:
                 "30440220714c2627f0e9c3bd6bf13b8b4faa5ec2d677694c27f580e2f9e3875bde9bc36f02201c33faacab9eafd799d9ceecaa153e3b87b4cd04535195261fd366e552652549",
             id: "188b4d9d95a58e4e18d9ce9db28f2010323b90b5afd36a474d7ae7bf70772bb0",
-            blockId: "17605317082329008056",
-            sequence: 0,
         },
         {
             version: 1,
@@ -45,8 +43,6 @@ export default {
             signature:
                 "3045022100e6039f810684515c0d6b31039040a76c98f3624b6454cb156a0a2137e5f8dba7022001ada19bcca5798e1c7cc8cc39bab5d4019525e3d72a42bd2c4129352b8ead87",
             id: "23084f2cc566f6144a8f447bc784de64a0b0646776060482d8550856145e11e2",
-            blockId: "17605317082329008056",
-            sequence: 1,
         },
         {
             version: 1,
@@ -61,8 +57,6 @@ export default {
             signature:
                 "3045022100c2b5ef772b36e468e95ec2e457bfaba7bad0e13b3faf57e229ff5d67a0e017c902202339664595ea5c70ce20e4dd182532f7fa385d86575b0476ff3eda9f9785e1e9",
             id: "743ce0a590c2af90e4734db3630b52d7a7cbc2bc228d75ae6409c0b6d184bfad",
-            blockId: "17605317082329008056",
-            sequence: 2,
         },
         {
             version: 1,
@@ -77,8 +71,6 @@ export default {
             signature:
                 "30450221009ceb56688705e6b12000bde726ca123d84982231d7434f059612ff5f987409c602200d908667877c902e7ba35024951046b883e0bce9103d4717928d94ecc958884a",
             id: "877780706b62b437913ef4ea30c6e370f8877ef7a5bac58d8cebca83b7e20060",
-            blockId: "17605317082329008056",
-            sequence: 3,
         },
         {
             version: 1,
@@ -93,8 +85,6 @@ export default {
             signature:
                 "30440220464beac6d49943ad8afaac4fdc863c9cd7cf3a84f9938c1d7269ed522298f11a02203581bf180de1966f86d914afeb005e1e818c9213514f96a34e1391c2a08514fa",
             id: "947fe8745eeed8fa6e5ad62a8dad29bcf3d50ce001907926c486460d1cc1f1c0",
-            blockId: "17605317082329008056",
-            sequence: 4,
         },
         {
             version: 1,
@@ -109,8 +99,6 @@ export default {
             signature:
                 "3045022100c7b40d7134d909762d18d6bfb7ac1c32be0ee8c047020131f499faea70ca0b2b0220117c0cf026f571f5a85e3ae800a6fd595185076ff38e64c7a4bd14f34e1d4dd1",
             id: "98387933d65fabffe2642464d4c7b1ff5fe1fa5a35992f834b0ac145dff462ea",
-            blockId: "17605317082329008056",
-            sequence: 5,
         },
         {
             version: 1,
@@ -125,8 +113,6 @@ export default {
             signature:
                 "304402206a4a8e4e6918fbc15728653b117f51db716aeb04e5ee1de047f80b0476ee4efb02200f486dfaf0def3f3e8636d46ee75a2c07de9714ce4283a25fde9b6218b5e7923",
             id: "e93345dd9a87ac4e84d9bfd892dfbfeb02e546e5bd7822168d0f72c7662e6176",
-            blockId: "17605317082329008056",
-            sequence: 6,
         },
     ],
 };

__tests__/unit/core-database/block-history-service.test.ts

@@ -1,7 +1,9 @@
-import { Container } from "@arkecosystem/core-kernel";
+import { Container, Contracts } from "@arkecosystem/core-kernel";
 
 import { BlockHistoryService } from "../../../packages/core-database/src/block-history-service";
 
+const defaultBlockOrder: Contracts.Search.ListOrder = [{ property: "height", direction: "asc" }];
+
 const blockRepository = {
     findManyByExpression: jest.fn(),
     listByExpression: jest.fn(),
@@ -58,7 +60,7 @@ describe("BlockHistoryService.findOneByCriteria", () => {
         const result = await blockHistoryService.findOneByCriteria(criteria);
 
         expect(blockFilter.getExpression).toBeCalledWith(criteria);
-        expect(blockRepository.findManyByExpression).toBeCalledWith(expression);
+        expect(blockRepository.findManyByExpression).toBeCalledWith(expression, defaultBlockOrder);
         expect(modelConverter.getBlockData).toBeCalledWith([]);
         expect(result).toBeUndefined();
     });
@@ -77,7 +79,7 @@ describe("BlockHistoryService.findOneByCriteria", () => {
         const result = await blockHistoryService.findOneByCriteria(criteria);
 
         expect(blockFilter.getExpression).toBeCalledWith(criteria);
-        expect(blockRepository.findManyByExpression).toBeCalledWith(expression);
+        expect(blockRepository.findManyByExpression).toBeCalledWith(expression, defaultBlockOrder);
         expect(modelConverter.getBlockData).toBeCalledWith([model]);
 
         expect(result).toBe(data);
@@ -101,7 +103,7 @@ describe("BlockHistoryService.findManyByCriteria", () => {
         const result = await blockHistoryService.findManyByCriteria(criteria);
 
         expect(blockFilter.getExpression).toBeCalledWith(criteria);
-        expect(blockRepository.findManyByExpression).toBeCalledWith(expression);
+        expect(blockRepository.findManyByExpression).toBeCalledWith(expression, defaultBlockOrder);
         expect(modelConverter.getBlockData).toBeCalledWith([model1, model2]);
 
         expect(result.length).toBe(2);

__tests__/unit/core-database/transaction-history-service.test.ts

@@ -10,6 +10,11 @@ const jestfn = <T extends (...args: unknown[]) => unknown>(
     return jest.fn(implementation);
 };
 
+const defaultTransactionOrder: Contracts.Search.ListOrder = [
+    { property: "blockHeight", direction: "asc" },
+    { property: "sequence", direction: "asc" },
+];
+
 const blockRepository = {};
 
 const transactionRepository = {
@@ -58,7 +63,7 @@ describe("TransactionHistoryService.findOneByCriteria", () => {
         const result = await blockHistoryService.findOneByCriteria(criteria);
 
         expect(transactionFilter.getExpression).toBeCalledWith(criteria);
-        expect(transactionRepository.findManyByExpression).toBeCalledWith(expression);
+        expect(transactionRepository.findManyByExpression).toBeCalledWith(expression, defaultTransactionOrder);
         expect(modelConverter.getTransactionData).toBeCalledWith([]);
         expect(result).toBeUndefined();
     });
@@ -77,7 +82,7 @@ describe("TransactionHistoryService.findOneByCriteria", () => {
         const result = await blockHistoryService.findOneByCriteria(criteria);
 
         expect(transactionFilter.getExpression).toBeCalledWith(criteria);
-        expect(transactionRepository.findManyByExpression).toBeCalledWith(expression);
+        expect(transactionRepository.findManyByExpression).toBeCalledWith(expression, defaultTransactionOrder);
         expect(modelConverter.getTransactionData).toBeCalledWith([model]);
 
         expect(result).toBe(data);
@@ -101,7 +106,7 @@ describe("TransactionHistoryService.findManyByCriteria", () => {
         const result = await transactionHistoryService.findManyByCriteria(criteria);
 
         expect(transactionFilter.getExpression).toBeCalledWith(criteria);
-        expect(transactionRepository.findManyByExpression).toBeCalledWith(expression);
+        expect(transactionRepository.findManyByExpression).toBeCalledWith(expression, defaultTransactionOrder);
         expect(modelConverter.getTransactionData).toBeCalledWith([model1, model2]);
 
         expect(result.length).toBe(2);
@@ -134,7 +139,7 @@ describe("TransactionHistoryService.streamByCriteria", () => {
         }
 
         expect(transactionFilter.getExpression).toBeCalledWith(criteria);
-        expect(transactionRepository.streamByExpression).toBeCalledWith(expression);
+        expect(transactionRepository.streamByExpression).toBeCalledWith(expression, defaultTransactionOrder);
         expect(modelConverter.getTransactionData).toBeCalledWith([model1]);
         expect(modelConverter.getTransactionData).toBeCalledWith([model2]);
 

__tests__/unit/core-snapshots/__fixtures__/assets.ts

@@ -1,6 +1,6 @@
-import { Meta } from "@packages/core-snapshots/src/contracts";
 import { Models } from "@arkecosystem/core-database";
 import { BigNumber } from "@arkecosystem/utils";
+import { Meta } from "@packages/core-snapshots/src/contracts";
 
 export const metaData: Meta.MetaData = {
     blocks: {
@@ -112,6 +112,7 @@ export const transactions: Models.Transaction[] = [
         id: "db1aa687737858cc9199bfa336f9b1c035915c30aaee60b1e0f8afadfdb946bd",
         version: 1,
         blockId: "17184958558311101492",
+        blockHeight: 1,
         sequence: 0,
         timestamp: 0,
         nonce: BigNumber.make(1),

__tests__/unit/core-test-framework/utils/mapper.test.ts

@@ -32,6 +32,7 @@ beforeEach(() => {
             signature:
                 "efd9acefcc843123f7f518ebc34cd8a81ba536b9604d42eaf3bc84fa9df4e2f51e4596ca2671e5726e99bff83a85f98122512ec9ed5f9a3de97045d23f6c94f3",
             blockId: "717093ac984e1a82a2de1fb334e92bda648547955417bc830d7825c515b5f2f9",
+            blockHeight: 5,
             sequence: 1,
             asset: {},
         },
@@ -48,7 +49,8 @@ beforeEach(() => {
         id: "f0880e972206698bf48e43325ec03045a3b2ab215b8f716a51742a909b718177",
         version: 2,
         blockId: "717093ac984e1a82a2de1fb334e92bda648547955417bc830d7825c515b5f2f9",
-        sequence: 0,
+        blockHeight: 5,
+        sequence: 1,
         timestamp: 2000123,
         nonce: Utils.BigNumber.make(5),
         senderPublicKey: "025805c82bb3ff7068e1b20da4ad2f89638e404950b0af7a0d2e23512b3701a21a",
@@ -69,6 +71,21 @@ describe("Mapper", () => {
             expect(mapTransactionToModel(transaction)).toEqual(transactionModel);
         });
 
+        it("should convert crypto transaction to database model replacing blockHeight", async () => {
+            expect(mapTransactionToModel(transaction, 100)).toEqual({ ...transactionModel, blockHeight: 100 });
+        });
+
+        it("should convert crypto transaction to database model replacing sequence", async () => {
+            expect(mapTransactionToModel(transaction, undefined, 10)).toEqual({ ...transactionModel, sequence: 10 });
+        });
+
+        it("should convert crypto transaction to database model without blockHeight or sequence", async () => {
+            const clone = { ...transaction, data: { ...transaction.data } };
+            delete clone.data.blockHeight;
+            delete clone.data.sequence;
+            expect(mapTransactionToModel(clone)).toEqual({ ...transactionModel, blockHeight: 0, sequence: 0 });
+        });
+
         it("should convert crypto transaction to database model without optional parameters", async () => {
             delete transaction.data.version;
             delete transaction.data.blockId;

packages/core-database/src/block-history-service.ts

@@ -1,5 +1,6 @@
 import { Container, Contracts } from "@arkecosystem/core-kernel";
 import { Interfaces } from "@arkecosystem/crypto";
+import assert from "assert";
 
 import { BlockRepository } from "./repositories/block-repository";
 import { TransactionRepository } from "./repositories/transaction-repository";
@@ -25,14 +26,15 @@ export class BlockHistoryService implements Contracts.Shared.BlockHistoryService
         criteria: Contracts.Shared.OrBlockCriteria,
     ): Promise<Interfaces.IBlockData | undefined> {
         const data = await this.findManyByCriteria(criteria);
+        assert(data.length <= 1);
         return data[0];
     }
 
     public async findManyByCriteria(criteria: Contracts.Shared.OrBlockCriteria): Promise<Interfaces.IBlockData[]> {
         const expression = await this.blockFilter.getExpression(criteria);
-        const models = await this.blockRepository.findManyByExpression(expression);
-        const data = this.modelConverter.getBlockData(models);
-        return data;
+        const order: Contracts.Search.ListOrder = [{ property: "height", direction: "asc" }];
+        const models = await this.blockRepository.findManyByExpression(expression, order);
+        return this.modelConverter.getBlockData(models);
     }
 
     public async listByCriteria(

packages/core-database/src/migrations/20200705000000-add-block-height-column-to-transactions-table.ts

@@ -0,0 +1,23 @@
+import { MigrationInterface, QueryRunner } from "typeorm";
+
+export class AddBlockHeightColumnToTransactionsTable20200705000000 implements MigrationInterface {
+    public async up(queryRunner: QueryRunner): Promise<any> {
+        await queryRunner.query(`
+            ALTER TABLE blocks ADD CONSTRAINT blocks_id_height_unique UNIQUE (id, height);
+            ALTER TABLE transactions ADD COLUMN block_height integer;
+            UPDATE transactions SET block_height = (SELECT height FROM blocks WHERE id = block_id);
+            ALTER TABLE transactions ALTER COLUMN block_height SET NOT NULL;
+            ALTER TABLE transactions ADD CONSTRAINT transactions_block_id_block_height FOREIGN KEY (block_id, block_height) REFERENCES blocks(id, height);
+            CREATE INDEX transactions_block_height_sequence ON transactions(block_height, sequence);
+        `);
+    }
+
+    public async down(queryRunner: QueryRunner): Promise<any> {
+        await queryRunner.query(`
+            DROP INDEX transactions_block_height_sequence;
+            ALTER TABLE transactions DROP CONSTRAINT transactions_block_id_block_height;
+            ALTER TABLE transactions DROP COLUMN block_height;
+            ALTER TABLE blocks DROP CONSTRAINT blocks_id_height_unique;
+        `);
+    }
+}

packages/core-database/src/model-converter.ts

@@ -47,6 +47,7 @@ export class ModelConverter implements Contracts.Database.ModelConverter {
 
             // block constructor
             data.blockId = model.blockId;
+            data.blockHeight = model.blockHeight;
             data.sequence = model.sequence;
 
             return data;

packages/core-database/src/models/transaction.ts

@@ -34,6 +34,12 @@ export class Transaction implements Contracts.Database.TransactionModel {
     })
     public blockId!: string;
 
+    @Column({
+        type: "integer",
+        nullable: false,
+    })
+    public blockHeight!: number;
+
     @Column({
         type: "smallint",
         nullable: false,

packages/core-database/src/repositories/abstract-repository.ts

@@ -13,19 +13,24 @@ export abstract class AbstractRepository<TEntity extends ObjectLiteral> extends
         return (await this.findByIds([id]))[0];
     }
 
-    public async findManyByExpression(expression: Contracts.Search.Expression<TEntity>): Promise<TEntity[]> {
+    public async findManyByExpression(
+        expression: Contracts.Search.Expression<TEntity>,
+        order: Contracts.Search.ListOrder = [],
+    ): Promise<TEntity[]> {
         const queryBuilder: SelectQueryBuilder<TEntity> = this.createQueryBuilder().select();
-        const sqlExpression = this.queryHelper.getWhereExpressionSql(this.metadata, expression);
-        queryBuilder.where(sqlExpression.query, sqlExpression.parameters);
+        this.addWhere(queryBuilder, expression);
+        this.addOrderBy(queryBuilder, order);
         return queryBuilder.getMany();
     }
 
-    public async *streamByExpression(expression: Contracts.Search.Expression<TEntity>): AsyncIterable<TEntity> {
-        const sqlExpression = this.queryHelper.getWhereExpressionSql(this.metadata, expression);
-        const stream = await this.createQueryBuilder()
-            .select("*")
-            .where(sqlExpression.query, sqlExpression.parameters)
-            .stream();
+    public async *streamByExpression(
+        expression: Contracts.Search.Expression<TEntity>,
+        order: Contracts.Search.ListOrder = [],
+    ): AsyncIterable<TEntity> {
+        const queryBuilder = this.createQueryBuilder().select("*");
+        this.addWhere(queryBuilder, expression);
+        this.addOrderBy(queryBuilder, order);
+        const stream = await queryBuilder.stream();
 
         for await (const raw of stream) {
             yield this.rawToEntity(raw);
@@ -38,20 +43,10 @@ export abstract class AbstractRepository<TEntity extends ObjectLiteral> extends
         page: Contracts.Search.ListPage,
         options?: Contracts.Search.ListOptions,
     ): Promise<Contracts.Search.ListResult<TEntity>> {
-        const queryBuilder = this.createQueryBuilder().select().skip(page.offset).take(page.limit);
-
-        const sqlExpression = this.queryHelper.getWhereExpressionSql(this.metadata, expression);
-        queryBuilder.where(sqlExpression.query, sqlExpression.parameters);
-
-        if (order.length) {
-            const column = this.queryHelper.getColumnName(this.metadata, order[0].property);
-            queryBuilder.orderBy(column, order[0].direction === "desc" ? "DESC" : "ASC");
-
-            for (const item of order.slice(1)) {
-                const column = this.queryHelper.getColumnName(this.metadata, item.property);
-                queryBuilder.addOrderBy(column, item.direction === "desc" ? "DESC" : "ASC");
-            }
-        }
+        const queryBuilder = this.createQueryBuilder().select();
+        this.addWhere(queryBuilder, expression);
+        this.addOrderBy(queryBuilder, order);
+        this.addSkipOffset(queryBuilder, page);
 
         if (options?.estimateTotalCount === false) {
             const [rows, count]: [TEntity[], number] = await queryBuilder.getManyAndCount();
@@ -107,4 +102,28 @@ export abstract class AbstractRepository<TEntity extends ObjectLiteral> extends
 
         return entity;
     }
+
+    private addWhere(
+        queryBuilder: SelectQueryBuilder<TEntity>,
+        expression: Contracts.Search.Expression<TEntity>,
+    ): void {
+        const sqlExpression = this.queryHelper.getWhereExpressionSql(this.metadata, expression);
+        queryBuilder.where(sqlExpression.query, sqlExpression.parameters);
+    }
+
+    private addOrderBy(queryBuilder: SelectQueryBuilder<TEntity>, order: Contracts.Search.ListOrder): void {
+        if (order.length) {
+            const column = this.queryHelper.getColumnName(this.metadata, order[0].property);
+            queryBuilder.orderBy(column, order[0].direction === "desc" ? "DESC" : "ASC");
+
+            for (const item of order.slice(1)) {
+                const column = this.queryHelper.getColumnName(this.metadata, item.property);
+                queryBuilder.addOrderBy(column, item.direction === "desc" ? "DESC" : "ASC");
+            }
+        }
+    }
+
+    private addSkipOffset(queryBuilder: SelectQueryBuilder<TEntity>, page: Contracts.Search.ListPage): void {
+        queryBuilder.skip(page.offset).take(page.limit);
+    }
 }