diff --git a/docs/docs-operate/operators/reference/changelog/v4.md b/docs/docs-operate/operators/reference/changelog/v4.md index dc8e0cce3d35..8bd0522245d0 100644 --- a/docs/docs-operate/operators/reference/changelog/v4.md +++ b/docs/docs-operate/operators/reference/changelog/v4.md @@ -161,6 +161,18 @@ Transaction submission via RPC now returns structured rejection codes when a tra **Impact**: Improved developer experience — callers can now programmatically handle specific rejection reasons. +### RPC transaction replacement price bump + +Transactions submitted via RPC that clash on nullifiers with existing pool transactions must now pay at least X% more in priority fee to replace them. The same bump applies when the pool is full and the incoming tx needs to evict the lowest-priority tx. P2P gossip behavior is unchanged. + +**Configuration:** + +```bash +P2P_RPC_PRICE_BUMP_PERCENTAGE=10 # default: 10 (percent) +``` + +Set to `0` to disable the percentage-based bump (still requires strictly higher fee). + ### Setup allow list extendable via network config The setup phase allow list can now be extended via the network configuration JSON (`txPublicSetupAllowListExtend` field). This allows network operators to distribute additional allowed setup functions to all nodes without requiring code changes. The local environment variable takes precedence over the network-json value. diff --git a/yarn-project/foundation/src/config/env_var.ts b/yarn-project/foundation/src/config/env_var.ts index c7bdfed39b44..666eb044454d 100644 --- a/yarn-project/foundation/src/config/env_var.ts +++ b/yarn-project/foundation/src/config/env_var.ts @@ -152,6 +152,7 @@ export type EnvVar = | 'P2P_DROP_TX_CHANCE' | 'P2P_TX_POOL_DELETE_TXS_AFTER_REORG' | 'P2P_MIN_TX_POOL_AGE_MS' + | 'P2P_RPC_PRICE_BUMP_PERCENTAGE' | 'DEBUG_P2P_INSTRUMENT_MESSAGES' | 'PEER_ID_PRIVATE_KEY' | 'PEER_ID_PRIVATE_KEY_PATH' diff --git a/yarn-project/p2p/src/client/factory.ts b/yarn-project/p2p/src/client/factory.ts index e8de6a8055a7..0895c1b624cc 100644 --- a/yarn-project/p2p/src/client/factory.ts +++ b/yarn-project/p2p/src/client/factory.ts @@ -100,6 +100,7 @@ export async function createP2PClient( archivedTxLimit: config.archivedTxLimit, minTxPoolAgeMs: config.minTxPoolAgeMs, dropTransactionsProbability: config.dropTransactionsProbability, + priceBumpPercentage: config.priceBumpPercentage, }, dateProvider, ); diff --git a/yarn-project/p2p/src/config.ts b/yarn-project/p2p/src/config.ts index 050f2b8bb233..5189cab7adb9 100644 --- a/yarn-project/p2p/src/config.ts +++ b/yarn-project/p2p/src/config.ts @@ -1,6 +1,7 @@ import { type ConfigMappingsType, SecretValue, + bigintConfigHelper, booleanConfigHelper, getConfigFromMappings, getDefaultConfig, @@ -190,6 +191,9 @@ export interface P2PConfig /** Minimum age (ms) a transaction must have been in the pool before it's eligible for block building. */ minTxPoolAgeMs: number; + + /** Minimum percentage fee increase required to replace an existing tx via RPC (0 = no bump). */ + priceBumpPercentage: bigint; } export const DEFAULT_P2P_PORT = 40400; @@ -465,6 +469,12 @@ export const p2pConfigMappings: ConfigMappingsType = { description: 'Minimum age (ms) a transaction must have been in the pool before it is eligible for block building.', ...numberConfigHelper(2_000), }, + priceBumpPercentage: { + env: 'P2P_RPC_PRICE_BUMP_PERCENTAGE', + description: + 'Minimum percentage fee increase required to replace an existing tx via RPC. Even at 0%, replacement still requires paying at least 1 unit more.', + ...bigintConfigHelper(10n), + }, ...sharedSequencerConfigMappings, ...p2pReqRespConfigMappings, ...batchTxRequesterConfigMappings, diff --git a/yarn-project/p2p/src/mem_pools/tx_pool_v2/README.md b/yarn-project/p2p/src/mem_pools/tx_pool_v2/README.md index 25dfc1d12435..64cf64650ba3 100644 --- a/yarn-project/p2p/src/mem_pools/tx_pool_v2/README.md +++ b/yarn-project/p2p/src/mem_pools/tx_pool_v2/README.md @@ -158,7 +158,7 @@ Checked before adding a transaction to the pending pool: | Rule | Purpose | |------|---------| -| `NullifierConflictRule` | Handles transactions with conflicting nullifiers. Higher priority tx wins. | +| `NullifierConflictRule` | Handles transactions with conflicting nullifiers. Higher priority tx wins. For RPC submissions, a configurable price bump percentage is required. | | `FeePayerBalancePreAddRule` | Ensures fee payer has sufficient balance for all their pending txs. | | `LowPriorityPreAddRule` | Rejects txs when pool is full and new tx has lowest priority. | @@ -233,6 +233,14 @@ await pool.updateConfig({ }); ``` +### Price Bump (RPC Transaction Replacement) + +When a transaction is submitted via RPC and clashes on nullifiers with an existing pool transaction, the incoming tx must pay at least `priceBumpPercentage`% more in priority fee (i.e. `>= existingFee + existingFee * bump / 100`) to replace it. This prevents spam via small fee increments. The same bump applies when the pool is full and the incoming tx needs to evict the lowest-priority tx. + +- **Env var**: `P2P_RPC_PRICE_BUMP_PERCENTAGE` (default: 10) +- **Scope**: RPC submissions only. P2P gossip uses `comparePriority` (fee + hash tiebreaker) with no bump. +- Even with a 0% bump, a replacement tx must pay at least 1 unit more than the existing fee. + ## Return Values ### AddTxsResult diff --git a/yarn-project/p2p/src/mem_pools/tx_pool_v2/eviction/interfaces.ts b/yarn-project/p2p/src/mem_pools/tx_pool_v2/eviction/interfaces.ts index 32135758973d..dd488bb1597e 100644 --- a/yarn-project/p2p/src/mem_pools/tx_pool_v2/eviction/interfaces.ts +++ b/yarn-project/p2p/src/mem_pools/tx_pool_v2/eviction/interfaces.ts @@ -100,7 +100,15 @@ export type TxPoolRejectionError = availableBalance: bigint; feeLimit: bigint; } - | { code: typeof TxPoolRejectionCode.NULLIFIER_CONFLICT; message: string; conflictingTxHash: string } + | { + code: typeof TxPoolRejectionCode.NULLIFIER_CONFLICT; + message: string; + conflictingTxHash: string; + /** Minimum fee needed to replace the conflicting tx (only set when price bump applies). */ + minimumPriceBumpFee?: bigint; + /** Incoming tx's priority fee. */ + txPriorityFee?: bigint; + } | { code: typeof TxPoolRejectionCode.INTERNAL_ERROR; message: string }; /** @@ -121,6 +129,8 @@ export interface PreAddResult { export interface PreAddContext { /** If true, compare priority fee only (no tx hash tiebreaker). Used for RPC submissions. */ feeComparisonOnly?: boolean; + /** Percentage-based price bump required for tx replacement. Only set for RPC submissions. */ + priceBumpPercentage?: bigint; } /** diff --git a/yarn-project/p2p/src/mem_pools/tx_pool_v2/eviction/low_priority_pre_add_rule.test.ts b/yarn-project/p2p/src/mem_pools/tx_pool_v2/eviction/low_priority_pre_add_rule.test.ts index fd66a0df4aee..57df8e341e49 100644 --- a/yarn-project/p2p/src/mem_pools/tx_pool_v2/eviction/low_priority_pre_add_rule.test.ts +++ b/yarn-project/p2p/src/mem_pools/tx_pool_v2/eviction/low_priority_pre_add_rule.test.ts @@ -209,5 +209,71 @@ describe('LowPriorityPreAddRule', () => { expect(result2.shouldIgnore).toBe(true); }); }); + + describe('with priceBumpPercentage', () => { + it('evicts when incoming fee exceeds the bump threshold', async () => { + const lowestPriorityMeta = createMeta('0x2222', 100n); + const poolAccess = createPoolAccess(100, lowestPriorityMeta); + const incomingMeta = createMeta('0x1111', 111n); // Above 10% bump + + const context: PreAddContext = { feeComparisonOnly: true, priceBumpPercentage: 10n }; + const result = await rule.check(incomingMeta, poolAccess, context); + + expect(result.shouldIgnore).toBe(false); + expect(result.txHashesToEvict).toContain(lowestPriorityMeta.txHash); + }); + + it('evicts when incoming fee is exactly at the bump threshold', async () => { + const lowestPriorityMeta = createMeta('0x2222', 100n); + const poolAccess = createPoolAccess(100, lowestPriorityMeta); + const incomingMeta = createMeta('0x1111', 110n); // Exactly 10% bump — accepted + + const context: PreAddContext = { feeComparisonOnly: true, priceBumpPercentage: 10n }; + const result = await rule.check(incomingMeta, poolAccess, context); + + expect(result.shouldIgnore).toBe(false); + expect(result.txHashesToEvict).toContain(lowestPriorityMeta.txHash); + }); + + it('ignores when incoming fee is below the bump threshold', async () => { + const lowestPriorityMeta = createMeta('0x2222', 100n); + const poolAccess = createPoolAccess(100, lowestPriorityMeta); + const incomingMeta = createMeta('0x1111', 109n); // Below 10% bump + + const context: PreAddContext = { feeComparisonOnly: true, priceBumpPercentage: 10n }; + const result = await rule.check(incomingMeta, poolAccess, context); + + expect(result.shouldIgnore).toBe(true); + expect(result.reason?.code).toBe(TxPoolRejectionCode.LOW_PRIORITY_FEE); + if (result.reason?.code === TxPoolRejectionCode.LOW_PRIORITY_FEE) { + expect(result.reason.minimumPriorityFee).toBe(110n); + expect(result.reason.txPriorityFee).toBe(109n); + } + }); + + it('without price bump (P2P path), behavior unchanged', async () => { + const lowestPriorityMeta = createMeta('0x2222', 100n); + const poolAccess = createPoolAccess(100, lowestPriorityMeta); + const incomingMeta = createMeta('0x1111', 101n); + + // No context — uses comparePriority, 101 > 100 so incoming wins + const result = await rule.check(incomingMeta, poolAccess); + + expect(result.shouldIgnore).toBe(false); + expect(result.txHashesToEvict).toContain(lowestPriorityMeta.txHash); + }); + + it('with 0% bump, rejects equal fee (minimum bump of 1)', async () => { + const lowestPriorityMeta = createMeta('0x2222', 100n); + const poolAccess = createPoolAccess(100, lowestPriorityMeta); + const incomingMeta = createMeta('0x1111', 100n); + + const context: PreAddContext = { feeComparisonOnly: true, priceBumpPercentage: 0n }; + const result = await rule.check(incomingMeta, poolAccess, context); + + expect(result.shouldIgnore).toBe(true); + expect(result.txHashesToEvict).toHaveLength(0); + }); + }); }); }); diff --git a/yarn-project/p2p/src/mem_pools/tx_pool_v2/eviction/low_priority_pre_add_rule.ts b/yarn-project/p2p/src/mem_pools/tx_pool_v2/eviction/low_priority_pre_add_rule.ts index b4d5ef8382db..013ffe6f8c6e 100644 --- a/yarn-project/p2p/src/mem_pools/tx_pool_v2/eviction/low_priority_pre_add_rule.ts +++ b/yarn-project/p2p/src/mem_pools/tx_pool_v2/eviction/low_priority_pre_add_rule.ts @@ -1,6 +1,6 @@ import { createLogger } from '@aztec/foundation/log'; -import { type TxMetaData, comparePriority } from '../tx_metadata.js'; +import { type TxMetaData, comparePriority, getMinimumPriceBumpFee } from '../tx_metadata.js'; import { type EvictionConfig, type PreAddContext, @@ -48,10 +48,14 @@ export class LowPriorityPreAddRule implements PreAddRule { } // Compare incoming tx against lowest priority tx. - // feeOnly mode (RPC): use strict fee comparison only — avoids churn from hash ordering - // Default (gossip): use full comparePriority (fee + tx hash tiebreaker) for determinism + // feeOnly mode (RPC): use strict fee comparison only — avoids churn from hash ordering. + // When price bump is also set, require the bumped fee threshold. + // Default (gossip): use full comparePriority (fee + tx hash tiebreaker) for determinism. const isHigherPriority = context?.feeComparisonOnly - ? incomingMeta.priorityFee > lowestPriorityMeta.priorityFee + ? context.priceBumpPercentage !== undefined + ? incomingMeta.priorityFee >= + getMinimumPriceBumpFee(lowestPriorityMeta.priorityFee, context.priceBumpPercentage) + : incomingMeta.priorityFee > lowestPriorityMeta.priorityFee : comparePriority(incomingMeta, lowestPriorityMeta) > 0; if (isHigherPriority) { @@ -66,6 +70,11 @@ export class LowPriorityPreAddRule implements PreAddRule { } // Incoming tx has equal or lower priority - ignore it (it would be evicted anyway) + const minimumFee = + context?.feeComparisonOnly && context.priceBumpPercentage !== undefined + ? getMinimumPriceBumpFee(lowestPriorityMeta.priorityFee, context.priceBumpPercentage) + : lowestPriorityMeta.priorityFee + 1n; + this.log.debug( `Pool at capacity (${currentCount}/${this.maxPoolSize}), ignoring ${incomingMeta.txHash} ` + `(priority ${incomingMeta.priorityFee}) - lower than existing minimum (priority ${lowestPriorityMeta.priorityFee})`, @@ -75,8 +84,8 @@ export class LowPriorityPreAddRule implements PreAddRule { txHashesToEvict: [], reason: { code: TxPoolRejectionCode.LOW_PRIORITY_FEE, - message: `Tx does not meet minimum priority fee. Required: ${lowestPriorityMeta.priorityFee + 1n}, got: ${incomingMeta.priorityFee}`, - minimumPriorityFee: lowestPriorityMeta.priorityFee + 1n, + message: `Tx does not meet minimum priority fee. Required: ${minimumFee}, got: ${incomingMeta.priorityFee}`, + minimumPriorityFee: minimumFee, txPriorityFee: incomingMeta.priorityFee, }, }); diff --git a/yarn-project/p2p/src/mem_pools/tx_pool_v2/eviction/nullifier_conflict_rule.test.ts b/yarn-project/p2p/src/mem_pools/tx_pool_v2/eviction/nullifier_conflict_rule.test.ts index 5108966f9047..f30ba1387587 100644 --- a/yarn-project/p2p/src/mem_pools/tx_pool_v2/eviction/nullifier_conflict_rule.test.ts +++ b/yarn-project/p2p/src/mem_pools/tx_pool_v2/eviction/nullifier_conflict_rule.test.ts @@ -1,5 +1,5 @@ import { type TxMetaData, stubTxMetaData } from '../tx_metadata.js'; -import type { PreAddPoolAccess } from './interfaces.js'; +import { type PreAddContext, type PreAddPoolAccess, TxPoolRejectionCode } from './interfaces.js'; import { NullifierConflictRule } from './nullifier_conflict_rule.js'; describe('NullifierConflictRule', () => { @@ -255,6 +255,108 @@ describe('NullifierConflictRule', () => { }); }); + describe('with priceBumpPercentage context', () => { + it('accepts tx when fee exceeds 10% bump threshold', async () => { + const sharedNullifier = '0xshared_null'; + const existingMeta = createMeta('0x2222', 100n, [sharedNullifier]); + const incomingMeta = createMeta('0x1111', 111n, [sharedNullifier]); // Above 10% + + const metadataMap = new Map([['0x2222', existingMeta]]); + const nullifierMap = new Map([[sharedNullifier, '0x2222']]); + poolAccess = createPoolAccess(nullifierMap, metadataMap); + + const context: PreAddContext = { feeComparisonOnly: true, priceBumpPercentage: 10n }; + const result = await rule.check(incomingMeta, poolAccess, context); + + expect(result.shouldIgnore).toBe(false); + expect(result.txHashesToEvict).toContain('0x2222'); + }); + + it('accepts tx when fee is exactly at 10% bump threshold', async () => { + const sharedNullifier = '0xshared_null'; + const existingMeta = createMeta('0x2222', 100n, [sharedNullifier]); + const incomingMeta = createMeta('0x1111', 110n, [sharedNullifier]); // Exactly 10% — accepted + + const metadataMap = new Map([['0x2222', existingMeta]]); + const nullifierMap = new Map([[sharedNullifier, '0x2222']]); + poolAccess = createPoolAccess(nullifierMap, metadataMap); + + const context: PreAddContext = { feeComparisonOnly: true, priceBumpPercentage: 10n }; + const result = await rule.check(incomingMeta, poolAccess, context); + + expect(result.shouldIgnore).toBe(false); + expect(result.txHashesToEvict).toContain('0x2222'); + }); + + it('rejects tx when fee is below 10% bump threshold', async () => { + const sharedNullifier = '0xshared_null'; + const existingMeta = createMeta('0x2222', 100n, [sharedNullifier]); + const incomingMeta = createMeta('0x1111', 109n, [sharedNullifier]); // Below 10% + + const metadataMap = new Map([['0x2222', existingMeta]]); + const nullifierMap = new Map([[sharedNullifier, '0x2222']]); + poolAccess = createPoolAccess(nullifierMap, metadataMap); + + const context: PreAddContext = { feeComparisonOnly: true, priceBumpPercentage: 10n }; + const result = await rule.check(incomingMeta, poolAccess, context); + + expect(result.shouldIgnore).toBe(true); + expect(result.reason?.code).toBe(TxPoolRejectionCode.NULLIFIER_CONFLICT); + if (result.reason?.code === TxPoolRejectionCode.NULLIFIER_CONFLICT) { + expect(result.reason.minimumPriceBumpFee).toBe(110n); + expect(result.reason.txPriorityFee).toBe(109n); + } + }); + + it('accepts tx well above bump threshold', async () => { + const sharedNullifier = '0xshared_null'; + const existingMeta = createMeta('0x2222', 100n, [sharedNullifier]); + const incomingMeta = createMeta('0x1111', 200n, [sharedNullifier]); + + const metadataMap = new Map([['0x2222', existingMeta]]); + const nullifierMap = new Map([[sharedNullifier, '0x2222']]); + poolAccess = createPoolAccess(nullifierMap, metadataMap); + + const context: PreAddContext = { feeComparisonOnly: true, priceBumpPercentage: 10n }; + const result = await rule.check(incomingMeta, poolAccess, context); + + expect(result.shouldIgnore).toBe(false); + expect(result.txHashesToEvict).toContain('0x2222'); + }); + + it('without price bump (P2P path), behavior is unchanged', async () => { + const sharedNullifier = '0xshared_null'; + const existingMeta = createMeta('0x2222', 100n, [sharedNullifier]); + const incomingMeta = createMeta('0x1111', 101n, [sharedNullifier]); // 1% above, not enough for 10% bump + + const metadataMap = new Map([['0x2222', existingMeta]]); + const nullifierMap = new Map([[sharedNullifier, '0x2222']]); + poolAccess = createPoolAccess(nullifierMap, metadataMap); + + // No context (P2P) — uses comparePriority, 101 > 100 means incoming wins + const result = await rule.check(incomingMeta, poolAccess); + + expect(result.shouldIgnore).toBe(false); + expect(result.txHashesToEvict).toContain('0x2222'); + }); + + it('with 0% price bump, rejects equal fee (minimum bump of 1)', async () => { + const sharedNullifier = '0xshared_null'; + const existingMeta = createMeta('0x2222', 100n, [sharedNullifier]); + const incomingMeta = createMeta('0x1111', 100n, [sharedNullifier]); + + const metadataMap = new Map([['0x2222', existingMeta]]); + const nullifierMap = new Map([[sharedNullifier, '0x2222']]); + poolAccess = createPoolAccess(nullifierMap, metadataMap); + + const context: PreAddContext = { feeComparisonOnly: true, priceBumpPercentage: 0n }; + const result = await rule.check(incomingMeta, poolAccess, context); + + expect(result.shouldIgnore).toBe(true); + expect(result.txHashesToEvict).toHaveLength(0); + }); + }); + describe('edge cases', () => { it('skips self-reference (incoming tx hash in conflict list)', async () => { const nullifier = '0xnull1'; diff --git a/yarn-project/p2p/src/mem_pools/tx_pool_v2/eviction/nullifier_conflict_rule.ts b/yarn-project/p2p/src/mem_pools/tx_pool_v2/eviction/nullifier_conflict_rule.ts index 9b638e13e83d..534a6fa4526e 100644 --- a/yarn-project/p2p/src/mem_pools/tx_pool_v2/eviction/nullifier_conflict_rule.ts +++ b/yarn-project/p2p/src/mem_pools/tx_pool_v2/eviction/nullifier_conflict_rule.ts @@ -15,11 +15,12 @@ export class NullifierConflictRule implements PreAddRule { private log = createLogger('p2p:tx_pool_v2:nullifier_conflict_rule'); - check(incomingMeta: TxMetaData, poolAccess: PreAddPoolAccess, _context?: PreAddContext): Promise { + check(incomingMeta: TxMetaData, poolAccess: PreAddPoolAccess, context?: PreAddContext): Promise { const result = checkNullifierConflict( incomingMeta, nullifier => poolAccess.getTxHashByNullifier(nullifier), txHash => poolAccess.getMetadata(txHash), + context?.priceBumpPercentage, ); if (result.shouldIgnore) { diff --git a/yarn-project/p2p/src/mem_pools/tx_pool_v2/interfaces.ts b/yarn-project/p2p/src/mem_pools/tx_pool_v2/interfaces.ts index a1a1ed0d9d69..a78a70482024 100644 --- a/yarn-project/p2p/src/mem_pools/tx_pool_v2/interfaces.ts +++ b/yarn-project/p2p/src/mem_pools/tx_pool_v2/interfaces.ts @@ -46,6 +46,8 @@ export type TxPoolV2Config = { evictedTxCacheSize: number; /** The probability (0-1) that a transaction is discarded. 0 disables dropping. For testing purposes only. */ dropTransactionsProbability: number; + /** Minimum percentage fee increase required to replace an existing tx via RPC (0 = no bump). */ + priceBumpPercentage: bigint; }; /** @@ -57,6 +59,7 @@ export const DEFAULT_TX_POOL_V2_CONFIG: TxPoolV2Config = { minTxPoolAgeMs: 2_000, evictedTxCacheSize: 10_000, dropTransactionsProbability: 0, + priceBumpPercentage: 10n, }; /** diff --git a/yarn-project/p2p/src/mem_pools/tx_pool_v2/tx_metadata.test.ts b/yarn-project/p2p/src/mem_pools/tx_pool_v2/tx_metadata.test.ts index d139138d5489..a4ba74f53105 100644 --- a/yarn-project/p2p/src/mem_pools/tx_pool_v2/tx_metadata.test.ts +++ b/yarn-project/p2p/src/mem_pools/tx_pool_v2/tx_metadata.test.ts @@ -1,7 +1,13 @@ import { mockTx } from '@aztec/stdlib/testing'; import { TxPoolRejectionCode } from './eviction/interfaces.js'; -import { buildTxMetaData, checkNullifierConflict, comparePriority, stubTxMetaData } from './tx_metadata.js'; +import { + buildTxMetaData, + checkNullifierConflict, + comparePriority, + getMinimumPriceBumpFee, + stubTxMetaData, +} from './tx_metadata.js'; describe('TxMetaData', () => { describe('buildTxMetaData', () => { @@ -260,5 +266,127 @@ describe('TxMetaData', () => { expect(result.shouldIgnore).toBe(false); expect(result.txHashesToEvict).toEqual([]); }); + + describe('with priceBumpPercentage', () => { + it('accepts incoming tx when fee exceeds the bump threshold', () => { + const existing = makeMeta('0x2222', 100n, ['0xnull1']); + const incoming = makeMeta('0x1111', 111n, ['0xnull1']); // Above 10% bump + + const result = checkNullifierConflict( + incoming, + () => existing.txHash, + () => existing, + 10n, // 10% bump + ); + + expect(result.shouldIgnore).toBe(false); + expect(result.txHashesToEvict).toEqual([existing.txHash]); + }); + + it('accepts incoming tx when fee is exactly at the bump threshold', () => { + const existing = makeMeta('0x2222', 100n, ['0xnull1']); + const incoming = makeMeta('0x1111', 110n, ['0xnull1']); // Exactly 10% bump — accepted + + const result = checkNullifierConflict( + incoming, + () => existing.txHash, + () => existing, + 10n, + ); + + expect(result.shouldIgnore).toBe(false); + expect(result.txHashesToEvict).toEqual([existing.txHash]); + }); + + it('rejects incoming tx when fee is below the bump threshold', () => { + const existing = makeMeta('0x2222', 100n, ['0xnull1']); + const incoming = makeMeta('0x1111', 109n, ['0xnull1']); // Below 10% bump + + const result = checkNullifierConflict( + incoming, + () => existing.txHash, + () => existing, + 10n, + ); + + expect(result.shouldIgnore).toBe(true); + expect(result.txHashesToEvict).toEqual([]); + expect(result.reason?.code).toBe(TxPoolRejectionCode.NULLIFIER_CONFLICT); + if (result.reason?.code === TxPoolRejectionCode.NULLIFIER_CONFLICT) { + expect(result.reason.minimumPriceBumpFee).toBe(110n); + expect(result.reason.txPriorityFee).toBe(109n); + } + }); + + it('accepts incoming tx well above the bump threshold', () => { + const existing = makeMeta('0x2222', 100n, ['0xnull1']); + const incoming = makeMeta('0x1111', 200n, ['0xnull1']); + + const result = checkNullifierConflict( + incoming, + () => existing.txHash, + () => existing, + 10n, + ); + + expect(result.shouldIgnore).toBe(false); + expect(result.txHashesToEvict).toEqual([existing.txHash]); + }); + + it('with 0% bump, rejects equal fee (minimum bump of 1)', () => { + const existing = makeMeta('0x2222', 100n, ['0xnull1']); + const incoming = makeMeta('0x1111', 100n, ['0xnull1']); + + const result = checkNullifierConflict( + incoming, + () => existing.txHash, + () => existing, + 0n, // 0% bump + ); + + expect(result.shouldIgnore).toBe(true); + expect(result.txHashesToEvict).toEqual([]); + }); + + it('without price bump, uses comparePriority (P2P path unchanged)', () => { + const existing = makeMeta('0x2222', 100n, ['0xnull1']); + const incoming = makeMeta('0x1111', 100n, ['0xnull1']); + + // No priceBumpPercentage — uses comparePriority, which for equal fees uses hash tiebreaker + const result = checkNullifierConflict( + incoming, + () => existing.txHash, + () => existing, + ); + + // With equal fees, the result depends on hash tiebreaker + // 0x1111 < 0x2222 so incoming has lower priority → should be ignored + expect(result.shouldIgnore).toBe(true); + }); + }); + }); + + describe('getMinimumPriceBumpFee', () => { + it('calculates 10% bump correctly', () => { + expect(getMinimumPriceBumpFee(100n, 10n)).toBe(110n); + }); + + it('calculates 0% bump (returns fee + 1 minimum bump)', () => { + expect(getMinimumPriceBumpFee(100n, 0n)).toBe(101n); + }); + + it('handles 0 existing fee (minimum bump of 1)', () => { + expect(getMinimumPriceBumpFee(0n, 10n)).toBe(1n); + }); + + it('handles large percentages', () => { + expect(getMinimumPriceBumpFee(100n, 100n)).toBe(200n); + expect(getMinimumPriceBumpFee(100n, 200n)).toBe(300n); + }); + + it('truncates fractional result (integer division)', () => { + // 33 * 10 / 100 = 3.3 → truncated to 3, so 33 + 3 = 36 + expect(getMinimumPriceBumpFee(33n, 10n)).toBe(36n); + }); }); }); diff --git a/yarn-project/p2p/src/mem_pools/tx_pool_v2/tx_metadata.ts b/yarn-project/p2p/src/mem_pools/tx_pool_v2/tx_metadata.ts index 316f551bcc6c..3874a7aab292 100644 --- a/yarn-project/p2p/src/mem_pools/tx_pool_v2/tx_metadata.ts +++ b/yarn-project/p2p/src/mem_pools/tx_pool_v2/tx_metadata.ts @@ -190,21 +190,38 @@ export function comparePriority(a: PriorityComparable, b: PriorityComparable): n return compareTxHash(a.txHashBigInt, b.txHashBigInt); } +/** + * Returns the minimum fee required to replace an existing tx with the given price bump percentage. + * Uses integer arithmetic: `existingFee + existingFee * priceBumpPercentage / 100`. + */ +export function getMinimumPriceBumpFee(existingFee: bigint, priceBumpPercentage: bigint): bigint { + const bump = (existingFee * priceBumpPercentage) / 100n; + // Ensure the minimum bump is at least 1, so that replacement always requires + // paying strictly more — even with 0% bump or zero existing fee. + const effectiveBump = bump > 0n ? bump : 1n; + return existingFee + effectiveBump; +} + /** * Checks for nullifier conflicts between an incoming transaction and existing pool state. * * When the incoming tx shares nullifiers with existing pending txs: - * - If the incoming tx has strictly higher priority, mark conflicting txs for eviction - * - If any conflicting tx has equal or higher priority, ignore the incoming tx + * - If the incoming tx meets or exceeds the required priority, mark conflicting txs for eviction + * - Otherwise, ignore the incoming tx + * + * When `priceBumpPercentage` is provided (RPC path), uses fee-only comparison with the + * percentage bump instead of `comparePriority`. * * @param incomingMeta - Metadata for the incoming transaction * @param getTxHashByNullifier - Accessor to find which tx uses a nullifier * @param getMetadata - Accessor to get metadata for a tx hash + * @param priceBumpPercentage - Optional percentage bump required for fee-based replacement */ export function checkNullifierConflict( incomingMeta: TxMetaData, getTxHashByNullifier: (nullifier: string) => string | undefined, getMetadata: (txHash: string) => TxMetaData | undefined, + priceBumpPercentage?: bigint, ): PreAddResult { const txHashesToEvict: string[] = []; @@ -225,19 +242,32 @@ export function checkNullifierConflict( continue; } - // If incoming tx has strictly higher priority, mark for eviction - // Otherwise, ignore incoming tx (ties go to existing tx) - // Use comparePriority for deterministic ordering (includes txHash as tiebreaker) - if (comparePriority(incomingMeta, conflictingMeta) > 0) { + // When price bump is set (RPC path), require the incoming fee to meet the bumped threshold. + // Otherwise (P2P path), use full comparePriority with tx hash tiebreaker. + const isHigherPriority = + priceBumpPercentage !== undefined + ? incomingMeta.priorityFee >= getMinimumPriceBumpFee(conflictingMeta.priorityFee, priceBumpPercentage) + : comparePriority(incomingMeta, conflictingMeta) > 0; + + if (isHigherPriority) { txHashesToEvict.push(conflictingHashStr); } else { + const minimumFee = + priceBumpPercentage !== undefined + ? getMinimumPriceBumpFee(conflictingMeta.priorityFee, priceBumpPercentage) + : undefined; return { shouldIgnore: true, txHashesToEvict: [], reason: { code: TxPoolRejectionCode.NULLIFIER_CONFLICT, - message: `Nullifier conflict with existing tx ${conflictingHashStr}`, + message: + minimumFee !== undefined + ? `Nullifier conflict with existing tx ${conflictingHashStr}. Minimum required fee: ${minimumFee}, got: ${incomingMeta.priorityFee}` + : `Nullifier conflict with existing tx ${conflictingHashStr}`, conflictingTxHash: conflictingHashStr, + minimumPriceBumpFee: minimumFee, + txPriorityFee: minimumFee !== undefined ? incomingMeta.priorityFee : undefined, }, }; } diff --git a/yarn-project/p2p/src/mem_pools/tx_pool_v2/tx_pool_v2_impl.ts b/yarn-project/p2p/src/mem_pools/tx_pool_v2/tx_pool_v2_impl.ts index 15f6eb4b051e..53e88e0e806e 100644 --- a/yarn-project/p2p/src/mem_pools/tx_pool_v2/tx_pool_v2_impl.ts +++ b/yarn-project/p2p/src/mem_pools/tx_pool_v2/tx_pool_v2_impl.ts @@ -214,7 +214,9 @@ export class TxPoolV2Impl { // in-memory reads, and buffered DB writes. Nothing here can throw an unhandled exception. const poolAccess = this.#createPreAddPoolAccess(); const preAddContext: PreAddContext | undefined = - opts.feeComparisonOnly !== undefined ? { feeComparisonOnly: opts.feeComparisonOnly } : undefined; + opts.feeComparisonOnly !== undefined + ? { feeComparisonOnly: opts.feeComparisonOnly, priceBumpPercentage: this.#config.priceBumpPercentage } + : undefined; await this.#store.transactionAsync(async () => { for (const tx of txs) {