From e7dcd39ad311c83f9519493bef60a4d26fef9cb0 Mon Sep 17 00:00:00 2001 From: oXtxNt9U <120286271+oXtxNt9U@users.noreply.github.com> Date: Fri, 20 Dec 2024 13:36:51 +0900 Subject: [PATCH 1/6] fix tx pool priority --- .../source/query.test.ts | 24 +++--- .../transaction-pool-service/source/query.ts | 77 +++++++++++++++---- 2 files changed, 73 insertions(+), 28 deletions(-) diff --git a/packages/transaction-pool-service/source/query.test.ts b/packages/transaction-pool-service/source/query.test.ts index a9739b294..dabd3bba3 100644 --- a/packages/transaction-pool-service/source/query.test.ts +++ b/packages/transaction-pool-service/source/query.test.ts @@ -30,7 +30,7 @@ describe<{ amount: BigNumber.make(100), gasPrice: 100, nonce: BigNumber.make(1), - senderPublicKey: "sender-public-key", + senderAddress: "sender1", type: 1, version: 2, }, @@ -46,7 +46,7 @@ describe<{ amount: BigNumber.make(100), gasPrice: 200, nonce: BigNumber.make(2), - senderPublicKey: "sender-public-key", + senderAddress: "sender1", type: 1, version: 2, }, @@ -62,7 +62,7 @@ describe<{ amount: BigNumber.make(100), gasPrice: 300, nonce: BigNumber.make(3), - senderPublicKey: "sender-public-key", + senderAddress: "sender2", type: 1, version: 2, }, @@ -78,7 +78,7 @@ describe<{ amount: BigNumber.make(100), gasPrice: 400, nonce: BigNumber.make(4), - senderPublicKey: "sender-public-key", + senderAddress: "sender2", type: 1, version: 2, }, @@ -121,26 +121,26 @@ describe<{ getSenderStub.calledWith("sender public key"); }); - it("getFromLowestPriority - should return transactions reverse ordered by fee", async (context) => { + it("getFromLowestPriority - should return transactions reverse ordered by nonce/fee", async (context) => { stub(context.mempool, "getSenderMempools").returnValueOnce([ { getFromLatest: () => [context.sender1Transaction200, context.sender1Transaction100] }, - { getFromLatest: () => [context.sender2Transaction100, context.sender2Transaction200] }, + { getFromLatest: () => [context.sender2Transaction200, context.sender2Transaction100] }, ]); const query = context.container.resolve(Query); const result = await query.getFromLowestPriority().all(); assert.equal(result, [ - context.sender1Transaction100, context.sender1Transaction200, - context.sender2Transaction100, + context.sender1Transaction100, context.sender2Transaction200, + context.sender2Transaction100, ]); }); - it("getFromHighestPriority - should return transactions order by fee", async (context) => { + it("getFromHighestPriority - should return transactions order by nonce/fee", async (context) => { stub(context.mempool, "getSenderMempools").returnValueOnce([ - { getFromEarliest: () => [context.sender1Transaction200, context.sender1Transaction100] }, + { getFromEarliest: () => [context.sender1Transaction100, context.sender1Transaction200] }, { getFromEarliest: () => [context.sender2Transaction100, context.sender2Transaction200] }, ]); @@ -148,10 +148,10 @@ describe<{ const result = await query.getFromHighestPriority().all(); assert.equal(result, [ - context.sender2Transaction200, context.sender2Transaction100, - context.sender1Transaction200, + context.sender2Transaction200, context.sender1Transaction100, + context.sender1Transaction200, ]); }); diff --git a/packages/transaction-pool-service/source/query.ts b/packages/transaction-pool-service/source/query.ts index 08d219e2e..803b0de0c 100644 --- a/packages/transaction-pool-service/source/query.ts +++ b/packages/transaction-pool-service/source/query.ts @@ -1,4 +1,5 @@ import { inject, injectable } from "@mainsail/container"; +import { Utils } from "@mainsail/kernel"; import { Contracts, Identifiers } from "@mainsail/contracts"; export class QueryIterable implements Contracts.TransactionPool.QueryIterable { @@ -85,24 +86,68 @@ export class Query implements Contracts.TransactionPool.Query { } public getFromLowestPriority(): QueryIterable { - return new QueryIterable( - [...this.mempool.getSenderMempools()] - .flatMap((senderMempool) => [...senderMempool.getFromLatest()]) - .sort( - (a: Contracts.Crypto.Transaction, b: Contracts.Crypto.Transaction) => - a.data.gasPrice - b.data.gasPrice, - ), - ); + return this.#getTransactions((senderMempool) => [...senderMempool.getFromLatest()], sortByLowestGasPrice); } public getFromHighestPriority(): QueryIterable { - return new QueryIterable( - [...this.mempool.getSenderMempools()] - .flatMap((senderMempool) => [...senderMempool.getFromEarliest()]) - .sort( - (a: Contracts.Crypto.Transaction, b: Contracts.Crypto.Transaction) => - b.data.gasPrice - a.data.gasPrice, - ), - ); + return this.#getTransactions((senderMempool) => [...senderMempool.getFromEarliest()], sortByHighestGasPrice); + } + + #getTransactions(selector: SenderMempoolSelectorFunction, prioritySort: SortFunction): QueryIterable { + // Get transactions by sender in ascending nonce order + const transactionsBySenderMempool: Record = {}; + for (const senderMempool of this.mempool.getSenderMempools()) { + const transactions = selector(senderMempool); + if (transactions.length === 0) { + continue; + } + + transactionsBySenderMempool[transactions[0].data.senderAddress] = transactions; + } + + // Add first transaction of each sender mempool + const priorityQueue: Contracts.Crypto.Transaction[] = []; + for (const transactions of Object.values(transactionsBySenderMempool)) { + priorityQueue.push(transactions[0]); + } + + // Sort by priority (nonces are per sender and already taken care by the caller) + priorityQueue.sort(prioritySort); + + // Lastly, select transactions from priority queue until all sender mempools are empty. + const selectedTransactions: Contracts.Crypto.Transaction[] = []; + while (priorityQueue.length > 0) { + // Pick next priority transaction + const transaction = priorityQueue.shift(); + Utils.assert.defined(transaction); + selectedTransactions.push(transaction); + + // Remove the selected transaction from sender mempool + const senderMempool = transactionsBySenderMempool[transaction.data.senderAddress]; + senderMempool.shift(); + + // If the sender has more transactions, add the next one to the queue + if (senderMempool.length > 0) { + priorityQueue.push(senderMempool[0]); + } + + // Re-sort the priority queue + priorityQueue.sort(prioritySort); + } + + return new QueryIterable(selectedTransactions); } } + +type SortFunction = (a: Contracts.Crypto.Transaction, b: Contracts.Crypto.Transaction) => number; +type SenderMempoolSelectorFunction = ( + mempool: Contracts.TransactionPool.SenderMempool, +) => Contracts.Crypto.Transaction[]; + +const sortByHighestGasPrice = (a: Contracts.Crypto.Transaction, b: Contracts.Crypto.Transaction) => { + return b.data.gasPrice - a.data.gasPrice; +}; + +const sortByLowestGasPrice = (a: Contracts.Crypto.Transaction, b: Contracts.Crypto.Transaction) => { + return a.data.gasPrice - b.data.gasPrice; +}; From 88bbfc94bc2d5339603ce6a88355cb20dc38e08d Mon Sep 17 00:00:00 2001 From: oXtxNt9U <120286271+oXtxNt9U@users.noreply.github.com> Date: Wed, 8 Jan 2025 13:41:07 +0900 Subject: [PATCH 2/6] remove obsolete tx pool api routes --- .../source/controllers/transactions.ts | 53 ------------------- .../source/resources/transaction.ts | 24 +-------- .../source/routes/transactions.ts | 12 ----- 3 files changed, 1 insertion(+), 88 deletions(-) diff --git a/packages/api-transaction-pool/source/controllers/transactions.ts b/packages/api-transaction-pool/source/controllers/transactions.ts index e6e5c29c9..c8090b231 100644 --- a/packages/api-transaction-pool/source/controllers/transactions.ts +++ b/packages/api-transaction-pool/source/controllers/transactions.ts @@ -3,8 +3,6 @@ import Hapi from "@hapi/hapi"; import { AbstractController } from "@mainsail/api-common"; import { inject, injectable } from "@mainsail/container"; import { Contracts, Identifiers } from "@mainsail/contracts"; -import { Utils } from "@mainsail/kernel"; -import { Handlers } from "@mainsail/transactions"; import { TransactionResource } from "../resources/index.js"; @@ -16,9 +14,6 @@ export class TransactionsController extends AbstractController { @inject(Identifiers.TransactionPool.Query) private readonly poolQuery!: Contracts.TransactionPool.Query; - @inject(Identifiers.Transaction.Handler.Registry) - private readonly nullHandlerRegistry!: Handlers.Registry; - public async store(request: Hapi.Request) { const result = await this.processor.process( // @ts-ignore @@ -64,52 +59,4 @@ export class TransactionsController extends AbstractController { return super.respondWithResource(transaction.data, TransactionResource, !!request.query.transform); } - - public async types(request: Hapi.Request) { - const activatedTransactionHandlers = await this.nullHandlerRegistry.getActivatedHandlers(); - const typeGroups: Record> = {}; - - for (const handler of activatedTransactionHandlers) { - const constructor = handler.getConstructor(); - - const type: number | undefined = constructor.type; - const typeGroup: number | undefined = constructor.typeGroup; - const key: string | undefined = constructor.key; - - Utils.assert.defined(type); - Utils.assert.defined(typeGroup); - Utils.assert.defined(key); - - if (typeGroups[typeGroup] === undefined) { - typeGroups[typeGroup] = {}; - } - - typeGroups[typeGroup][key[0].toUpperCase() + key.slice(1)] = type; - } - - return { data: typeGroups }; - } - - public async schemas(request: Hapi.Request) { - const activatedTransactionHandlers = await this.nullHandlerRegistry.getActivatedHandlers(); - const schemasByType: Record> = {}; - - for (const handler of activatedTransactionHandlers) { - const constructor = handler.getConstructor(); - - const type: number | undefined = constructor.type; - const typeGroup: number | undefined = constructor.typeGroup; - - Utils.assert.defined(type); - Utils.assert.defined(typeGroup); - - if (schemasByType[typeGroup] === undefined) { - schemasByType[typeGroup] = {}; - } - - schemasByType[typeGroup][type] = constructor.getSchema().properties; - } - - return { data: schemasByType }; - } } diff --git a/packages/api-transaction-pool/source/resources/transaction.ts b/packages/api-transaction-pool/source/resources/transaction.ts index 99cb6f309..866f314f2 100644 --- a/packages/api-transaction-pool/source/resources/transaction.ts +++ b/packages/api-transaction-pool/source/resources/transaction.ts @@ -8,28 +8,6 @@ export class TransactionResource implements Contracts.Api.Resource { } public async transform(resource: Contracts.Crypto.TransactionData): Promise { - //AppUtils.assert.defined(resource.senderPublicKey); - - // const wallet = await this.stateService.getStore().walletRepository.findByPublicKey(resource.senderPublicKey); - // const sender: string = wallet.getAddress(); - - return { - // amount: resource.amount.toFixed(), - // asset: resource.asset, - // blockId: resource.blockId, - // fee: resource.fee.toFixed(), - // id: resource.id, - // nonce: resource.nonce?.toFixed(), - // recipient: resource.recipientId, - // // sender, - // // senderPublicKey: resource.senderPublicKey, - // signature: resource.signature, - // signatures: resource.signatures, - // timestamp: resource.timestamp, - // type: resource.type, - // typeGroup: resource.typeGroup, - // vendorField: resource.vendorField, - // version: resource.version, - }; + return this.raw(resource); } } diff --git a/packages/api-transaction-pool/source/routes/transactions.ts b/packages/api-transaction-pool/source/routes/transactions.ts index 701ad8876..61c84275d 100644 --- a/packages/api-transaction-pool/source/routes/transactions.ts +++ b/packages/api-transaction-pool/source/routes/transactions.ts @@ -80,16 +80,4 @@ export const register = (server: Contracts.Api.ApiServer): void => { }, path: "/transactions/unconfirmed/{id}", }); - - server.route({ - handler: (request: Hapi.Request) => controller.types(request), - method: "GET", - path: "/transactions/types", - }); - - server.route({ - handler: (request: Hapi.Request) => controller.schemas(request), - method: "GET", - path: "/transactions/schemas", - }); }; From e7fbfb3e7e04ac6d8c89b5c4a10c0140cce27bb2 Mon Sep 17 00:00:00 2001 From: oXtxNt9U <120286271+oXtxNt9U@users.noreply.github.com> Date: Wed, 8 Jan 2025 13:44:02 +0900 Subject: [PATCH 3/6] update comments --- packages/transaction-pool-service/source/query.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/transaction-pool-service/source/query.ts b/packages/transaction-pool-service/source/query.ts index 803b0de0c..a91f161c1 100644 --- a/packages/transaction-pool-service/source/query.ts +++ b/packages/transaction-pool-service/source/query.ts @@ -94,7 +94,7 @@ export class Query implements Contracts.TransactionPool.Query { } #getTransactions(selector: SenderMempoolSelectorFunction, prioritySort: SortFunction): QueryIterable { - // Get transactions by sender in ascending nonce order + // Group transactions by sender mempool const transactionsBySenderMempool: Record = {}; for (const senderMempool of this.mempool.getSenderMempools()) { const transactions = selector(senderMempool); @@ -111,7 +111,7 @@ export class Query implements Contracts.TransactionPool.Query { priorityQueue.push(transactions[0]); } - // Sort by priority (nonces are per sender and already taken care by the caller) + // Sort by priority (nonces are per sender and already handled by the provided selector) priorityQueue.sort(prioritySort); // Lastly, select transactions from priority queue until all sender mempools are empty. From 33babb78c3d84b3a8182524df216cea10e820ab8 Mon Sep 17 00:00:00 2001 From: oXtxNt9U Date: Wed, 8 Jan 2025 04:45:56 +0000 Subject: [PATCH 4/6] style: resolve style guide violations --- packages/transaction-pool-service/source/query.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/transaction-pool-service/source/query.ts b/packages/transaction-pool-service/source/query.ts index a91f161c1..67f2593c0 100644 --- a/packages/transaction-pool-service/source/query.ts +++ b/packages/transaction-pool-service/source/query.ts @@ -1,6 +1,6 @@ import { inject, injectable } from "@mainsail/container"; -import { Utils } from "@mainsail/kernel"; import { Contracts, Identifiers } from "@mainsail/contracts"; +import { Utils } from "@mainsail/kernel"; export class QueryIterable implements Contracts.TransactionPool.QueryIterable { public transactions: Contracts.Crypto.Transaction[]; @@ -144,10 +144,8 @@ type SenderMempoolSelectorFunction = ( mempool: Contracts.TransactionPool.SenderMempool, ) => Contracts.Crypto.Transaction[]; -const sortByHighestGasPrice = (a: Contracts.Crypto.Transaction, b: Contracts.Crypto.Transaction) => { - return b.data.gasPrice - a.data.gasPrice; -}; +const sortByHighestGasPrice = (a: Contracts.Crypto.Transaction, b: Contracts.Crypto.Transaction) => + b.data.gasPrice - a.data.gasPrice; -const sortByLowestGasPrice = (a: Contracts.Crypto.Transaction, b: Contracts.Crypto.Transaction) => { - return a.data.gasPrice - b.data.gasPrice; -}; +const sortByLowestGasPrice = (a: Contracts.Crypto.Transaction, b: Contracts.Crypto.Transaction) => + a.data.gasPrice - b.data.gasPrice; From 3e6c14787b1646758a1c48f157c1ce0fdb4a06c3 Mon Sep 17 00:00:00 2001 From: oXtxNt9U <120286271+oXtxNt9U@users.noreply.github.com> Date: Wed, 8 Jan 2025 13:48:01 +0900 Subject: [PATCH 5/6] remove unused dep --- packages/api-transaction-pool/package.json | 1 - pnpm-lock.yaml | 3 --- 2 files changed, 4 deletions(-) diff --git a/packages/api-transaction-pool/package.json b/packages/api-transaction-pool/package.json index 472c67380..4cc181718 100644 --- a/packages/api-transaction-pool/package.json +++ b/packages/api-transaction-pool/package.json @@ -28,7 +28,6 @@ "@mainsail/container": "workspace:*", "@mainsail/contracts": "workspace:*", "@mainsail/kernel": "workspace:*", - "@mainsail/transactions": "workspace:*", "joi": "17.12.2" }, "devDependencies": { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1af52b3fa..9a674cf02 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -444,9 +444,6 @@ importers: '@mainsail/kernel': specifier: workspace:* version: link:../kernel - '@mainsail/transactions': - specifier: workspace:* - version: link:../transactions joi: specifier: 17.12.2 version: 17.12.2 From 646df84fa3847cde22f112015483617178c46cad Mon Sep 17 00:00:00 2001 From: sebastijankuzner Date: Wed, 8 Jan 2025 10:51:27 +0100 Subject: [PATCH 6/6] Reorder --- .../transaction-pool-service/source/query.ts | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/transaction-pool-service/source/query.ts b/packages/transaction-pool-service/source/query.ts index 67f2593c0..f4a6b5802 100644 --- a/packages/transaction-pool-service/source/query.ts +++ b/packages/transaction-pool-service/source/query.ts @@ -2,6 +2,17 @@ import { inject, injectable } from "@mainsail/container"; import { Contracts, Identifiers } from "@mainsail/contracts"; import { Utils } from "@mainsail/kernel"; +type SortFunction = (a: Contracts.Crypto.Transaction, b: Contracts.Crypto.Transaction) => number; +type SenderMempoolSelectorFunction = ( + mempool: Contracts.TransactionPool.SenderMempool, +) => Contracts.Crypto.Transaction[]; + +const sortByHighestGasPrice = (a: Contracts.Crypto.Transaction, b: Contracts.Crypto.Transaction) => + b.data.gasPrice - a.data.gasPrice; + +const sortByLowestGasPrice = (a: Contracts.Crypto.Transaction, b: Contracts.Crypto.Transaction) => + a.data.gasPrice - b.data.gasPrice; + export class QueryIterable implements Contracts.TransactionPool.QueryIterable { public transactions: Contracts.Crypto.Transaction[]; public predicates: Contracts.TransactionPool.QueryPredicate[] = []; @@ -138,14 +149,3 @@ export class Query implements Contracts.TransactionPool.Query { return new QueryIterable(selectedTransactions); } } - -type SortFunction = (a: Contracts.Crypto.Transaction, b: Contracts.Crypto.Transaction) => number; -type SenderMempoolSelectorFunction = ( - mempool: Contracts.TransactionPool.SenderMempool, -) => Contracts.Crypto.Transaction[]; - -const sortByHighestGasPrice = (a: Contracts.Crypto.Transaction, b: Contracts.Crypto.Transaction) => - b.data.gasPrice - a.data.gasPrice; - -const sortByLowestGasPrice = (a: Contracts.Crypto.Transaction, b: Contracts.Crypto.Transaction) => - a.data.gasPrice - b.data.gasPrice;