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())); + }); }