From abb83df9995ac310d8212f2fffff330b7e6b54fc Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Tue, 7 Jan 2025 16:37:48 -0300 Subject: [PATCH] feat: Build blocks using txs with higher fee first Updates the tx pool to store pending tx hashes sorted by fee. We use the sum of the l2 and da priority fees for this. Fixes #11084 --- yarn-project/circuit-types/src/tx/tx.ts | 5 + .../src/mem_pools/tx_pool/aztec_kv_tx_pool.ts | 101 ++++++++++-------- .../src/mem_pools/tx_pool/memory_tx_pool.ts | 6 +- .../p2p/src/mem_pools/tx_pool/priority.ts | 13 +++ .../p2p/src/mem_pools/tx_pool/tx_pool.ts | 2 +- .../mem_pools/tx_pool/tx_pool_test_suite.ts | 22 +++- 6 files changed, 102 insertions(+), 47 deletions(-) create mode 100644 yarn-project/p2p/src/mem_pools/tx_pool/priority.ts diff --git a/yarn-project/circuit-types/src/tx/tx.ts b/yarn-project/circuit-types/src/tx/tx.ts index 00b8a8593e5..969a069e4f7 100644 --- a/yarn-project/circuit-types/src/tx/tx.ts +++ b/yarn-project/circuit-types/src/tx/tx.ts @@ -1,6 +1,7 @@ import { ClientIvcProof, Fr, + type GasSettings, PrivateKernelTailCircuitPublicInputs, PrivateLog, type PrivateToPublicAccumulatedData, @@ -88,6 +89,10 @@ export class Tx extends Gossipable { return this.publicTeardownFunctionCall.isEmpty() ? undefined : this.publicTeardownFunctionCall; } + getGasSettings(): GasSettings { + return this.data.constants.txContext.gasSettings; + } + /** * Deserializes the Tx object from a Buffer. * @param buffer - Buffer or BufferReader object to deserialize. diff --git a/yarn-project/p2p/src/mem_pools/tx_pool/aztec_kv_tx_pool.ts b/yarn-project/p2p/src/mem_pools/tx_pool/aztec_kv_tx_pool.ts index 70c858f08fb..3247c88355c 100644 --- a/yarn-project/p2p/src/mem_pools/tx_pool/aztec_kv_tx_pool.ts +++ b/yarn-project/p2p/src/mem_pools/tx_pool/aztec_kv_tx_pool.ts @@ -1,10 +1,11 @@ import { Tx, TxHash } from '@aztec/circuit-types'; import { type TxAddedToPoolStats } from '@aztec/circuit-types/stats'; import { type Logger, createLogger } from '@aztec/foundation/log'; -import { type AztecKVStore, type AztecMap, type AztecSet } from '@aztec/kv-store'; +import { type AztecKVStore, type AztecMap, type AztecMultiMap } from '@aztec/kv-store'; import { type TelemetryClient } from '@aztec/telemetry-client'; import { PoolInstrumentation, PoolName } from '../instrumentation.js'; +import { getPendingTxPriority } from './priority.js'; import { type TxPool } from './tx_pool.js'; /** @@ -16,10 +17,11 @@ export class AztecKVTxPool implements TxPool { /** Our tx pool, stored as a Map, with K: tx hash and V: the transaction. */ #txs: AztecMap; - /** Index for pending txs. */ - #pendingTxs: AztecSet; - /** Index for mined txs. */ - #minedTxs: AztecMap; + /** Index from tx hash to the block number in which they were mined, filtered by mined txs. */ + #minedTxHashToBlock: AztecMap; + + /** Index from tx priority (stored as hex) to its tx hash, filtered by pending txs. */ + #pendingTxPriorityToHash: AztecMultiMap; #log: Logger; @@ -32,8 +34,8 @@ export class AztecKVTxPool implements TxPool { */ constructor(store: AztecKVStore, telemetry: TelemetryClient, log = createLogger('p2p:tx_pool')) { this.#txs = store.openMap('txs'); - this.#minedTxs = store.openMap('minedTxs'); - this.#pendingTxs = store.openSet('pendingTxs'); + this.#minedTxHashToBlock = store.openMap('txHashToBlockMined'); + this.#pendingTxPriorityToHash = store.openMultiMap('pendingTxFeeToHash'); this.#store = store; this.#log = log; @@ -41,18 +43,25 @@ export class AztecKVTxPool implements TxPool { } public markAsMined(txHashes: TxHash[], blockNumber: number): Promise { + if (txHashes.length === 0) { + return Promise.resolve(); + } + + let deletedPending = 0; return this.#store.transaction(() => { - let deleted = 0; for (const hash of txHashes) { const key = hash.toString(); - void this.#minedTxs.set(key, blockNumber); - if (this.#pendingTxs.has(key)) { - deleted++; - void this.#pendingTxs.delete(key); + void this.#minedTxHashToBlock.set(key, blockNumber); + + const tx = this.getTxByHash(hash); + if (tx) { + deletedPending++; + const fee = getPendingTxPriority(tx); + void this.#pendingTxPriorityToHash.deleteValue(fee, key); } } - this.#metrics.recordRemovedObjects(deleted, 'pending'); this.#metrics.recordAddedObjects(txHashes.length, 'mined'); + this.#metrics.recordRemovedObjects(deletedPending, 'pending'); }); } @@ -61,33 +70,30 @@ export class AztecKVTxPool implements TxPool { return Promise.resolve(); } + let markedAsPending = 0; return this.#store.transaction(() => { - let deleted = 0; - let added = 0; for (const hash of txHashes) { const key = hash.toString(); - if (this.#minedTxs.has(key)) { - deleted++; - void this.#minedTxs.delete(key); - } + void this.#minedTxHashToBlock.delete(key); - if (this.#txs.has(key)) { - added++; - void this.#pendingTxs.add(key); + const tx = this.getTxByHash(hash); + if (tx) { + void this.#pendingTxPriorityToHash.set(getPendingTxPriority(tx), key); + markedAsPending++; } } - this.#metrics.recordRemovedObjects(deleted, 'mined'); - this.#metrics.recordAddedObjects(added, 'pending'); + this.#metrics.recordAddedObjects(markedAsPending, 'pending'); + this.#metrics.recordRemovedObjects(markedAsPending, 'mined'); }); } public getPendingTxHashes(): TxHash[] { - return Array.from(this.#pendingTxs.entries()).map(x => TxHash.fromString(x)); + return Array.from(this.#pendingTxPriorityToHash.values({ reverse: true })).map(x => TxHash.fromString(x)); } public getMinedTxHashes(): [TxHash, number][] { - return Array.from(this.#minedTxs.entries()).map(([txHash, blockNumber]) => [ + return Array.from(this.#minedTxHashToBlock.entries()).map(([txHash, blockNumber]) => [ TxHash.fromString(txHash), blockNumber, ]); @@ -95,10 +101,10 @@ export class AztecKVTxPool implements TxPool { public getTxStatus(txHash: TxHash): 'pending' | 'mined' | undefined { const key = txHash.toString(); - if (this.#pendingTxs.has(key)) { - return 'pending'; - } else if (this.#minedTxs.has(key)) { + if (this.#minedTxHashToBlock.has(key)) { return 'mined'; + } else if (this.#txs.has(key)) { + return 'pending'; } else { return undefined; } @@ -120,11 +126,10 @@ export class AztecKVTxPool implements TxPool { * @returns Empty promise. */ public addTxs(txs: Tx[]): Promise { - const txHashes = txs.map(tx => tx.getTxHash()); return this.#store.transaction(() => { let pendingCount = 0; - for (const [i, tx] of txs.entries()) { - const txHash = txHashes[i]; + for (const tx of txs) { + const txHash = tx.getTxHash(); this.#log.verbose(`Adding tx ${txHash.toString()} to pool`, { eventName: 'tx-added-to-pool', ...tx.getStats(), @@ -132,10 +137,11 @@ export class AztecKVTxPool implements TxPool { const key = txHash.toString(); void this.#txs.set(key, tx.toBuffer()); - if (!this.#minedTxs.has(key)) { + + if (!this.#minedTxHashToBlock.has(key)) { pendingCount++; // REFACTOR: Use an lmdb conditional write to avoid race conditions with this write tx - void this.#pendingTxs.add(key); + void this.#pendingTxPriorityToHash.set(getPendingTxPriority(tx), key); this.#metrics.recordSize(tx); } } @@ -150,20 +156,27 @@ export class AztecKVTxPool implements TxPool { * @returns The number of transactions that was deleted from the pool. */ public deleteTxs(txHashes: TxHash[]): Promise { + let pendingDeleted = 0; + let minedDeleted = 0; + return this.#store.transaction(() => { - let pendingDeleted = 0; - let minedDeleted = 0; for (const hash of txHashes) { const key = hash.toString(); - void this.#txs.delete(key); - if (this.#pendingTxs.has(key)) { - pendingDeleted++; - void this.#pendingTxs.delete(key); - } + const tx = this.getTxByHash(hash); + + if (tx) { + const fee = getPendingTxPriority(tx); + void this.#pendingTxPriorityToHash.deleteValue(fee, key); + + const isMined = this.#minedTxHashToBlock.has(key); + if (isMined) { + minedDeleted++; + } else { + pendingDeleted++; + } - if (this.#minedTxs.has(key)) { - minedDeleted++; - void this.#minedTxs.delete(key); + void this.#txs.delete(key); + void this.#minedTxHashToBlock.delete(key); } } diff --git a/yarn-project/p2p/src/mem_pools/tx_pool/memory_tx_pool.ts b/yarn-project/p2p/src/mem_pools/tx_pool/memory_tx_pool.ts index c727ad07098..485dd86b1b0 100644 --- a/yarn-project/p2p/src/mem_pools/tx_pool/memory_tx_pool.ts +++ b/yarn-project/p2p/src/mem_pools/tx_pool/memory_tx_pool.ts @@ -4,6 +4,7 @@ import { createLogger } from '@aztec/foundation/log'; import { type TelemetryClient } from '@aztec/telemetry-client'; import { PoolInstrumentation, PoolName } from '../instrumentation.js'; +import { getPendingTxPriority } from './priority.js'; import { type TxPool } from './tx_pool.js'; /** @@ -68,7 +69,10 @@ export class InMemoryTxPool implements TxPool { } public getPendingTxHashes(): TxHash[] { - return Array.from(this.pendingTxs).map(x => TxHash.fromBigInt(x)); + return this.getAllTxs() + .sort((tx1, tx2) => -getPendingTxPriority(tx1).localeCompare(getPendingTxPriority(tx2))) + .map(tx => tx.getTxHash()) + .filter(txHash => this.pendingTxs.has(txHash.toBigInt())); } public getMinedTxHashes(): [TxHash, number][] { diff --git a/yarn-project/p2p/src/mem_pools/tx_pool/priority.ts b/yarn-project/p2p/src/mem_pools/tx_pool/priority.ts new file mode 100644 index 00000000000..dfbdfc45f17 --- /dev/null +++ b/yarn-project/p2p/src/mem_pools/tx_pool/priority.ts @@ -0,0 +1,13 @@ +import { type Tx } from '@aztec/circuit-types'; +import { Buffer32 } from '@aztec/foundation/buffer'; + +/** + * Returns a string representing the priority of a tx. + * Txs with a higher priority value are returned first when retrieving pending tx hashes. + * We currently use the sum of the priority fees for the tx for this value, represented as hex. + */ +export function getPendingTxPriority(tx: Tx): string { + const priorityFees = tx.getGasSettings().maxPriorityFeesPerGas; + const totalFees = priorityFees.feePerDaGas.toBigInt() + priorityFees.feePerL2Gas.toBigInt(); + return Buffer32.fromBigInt(totalFees).toString(); +} diff --git a/yarn-project/p2p/src/mem_pools/tx_pool/tx_pool.ts b/yarn-project/p2p/src/mem_pools/tx_pool/tx_pool.ts index 01511951f8a..173565c8293 100644 --- a/yarn-project/p2p/src/mem_pools/tx_pool/tx_pool.ts +++ b/yarn-project/p2p/src/mem_pools/tx_pool/tx_pool.ts @@ -49,7 +49,7 @@ export interface TxPool { getAllTxHashes(): TxHash[]; /** - * Gets the hashes of pending transactions currently in the tx pool. + * Gets the hashes of pending transactions currently in the tx pool sorted by priority (see getPendingTxPriority). * @returns An array of pending transaction hashes found in the tx pool. */ getPendingTxHashes(): TxHash[]; diff --git a/yarn-project/p2p/src/mem_pools/tx_pool/tx_pool_test_suite.ts b/yarn-project/p2p/src/mem_pools/tx_pool/tx_pool_test_suite.ts index 35af12fbd68..f3c92e688b8 100644 --- a/yarn-project/p2p/src/mem_pools/tx_pool/tx_pool_test_suite.ts +++ b/yarn-project/p2p/src/mem_pools/tx_pool/tx_pool_test_suite.ts @@ -1,4 +1,6 @@ -import { mockTx } from '@aztec/circuit-types'; +import { type Tx, mockTx } from '@aztec/circuit-types'; +import { GasFees } from '@aztec/circuits.js'; +import { unfreeze } from '@aztec/foundation/types'; import { type TxPool } from './tx_pool.js'; @@ -101,4 +103,22 @@ export function describeTxPool(getTxPool: () => TxPool) { expect(poolTxHashes).toHaveLength(3); expect(poolTxHashes).toEqual(expect.arrayContaining([tx1.getTxHash(), tx2.getTxHash(), tx3.getTxHash()])); }); + + it('Returns pending tx hashes sorted by priority', async () => { + const withPriorityFee = (tx: Tx, fee: number) => { + unfreeze(tx.data.constants.txContext.gasSettings).maxPriorityFeesPerGas = new GasFees(fee, fee); + return tx; + }; + + const tx1 = withPriorityFee(mockTx(0), 1000); + const tx2 = withPriorityFee(mockTx(1), 100); + const tx3 = withPriorityFee(mockTx(2), 200); + const tx4 = withPriorityFee(mockTx(3), 3000); + + await pool.addTxs([tx1, tx2, tx3, tx4]); + + const poolTxHashes = pool.getPendingTxHashes(); + expect(poolTxHashes).toHaveLength(4); + expect(poolTxHashes).toEqual([tx4, tx1, tx3, tx2].map(tx => tx.getTxHash())); + }); }