feat: price bump for RPC transaction replacement#20806
feat: price bump for RPC transaction replacement#20806PhilWindle merged 6 commits intomerge-train/spartanfrom
Conversation
| 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); |
There was a problem hiding this comment.
I think we probably want it to be >= the price bump. This rejection tells me the minimum price bump is the exact fee I sent but I still got rejected.
There was a problem hiding this comment.
Fixed - I changed the logic to be >= price bump.
Edge case when price bump is configured to 0 - replacement always requires paying strictly more (1 unit more)
| * Uses integer arithmetic: `existingFee + existingFee * priceBumpPercentage / 100`. | ||
| */ | ||
| export function getMinimumPriceBumpFee(existingFee: bigint, priceBumpPercentage: number): bigint { | ||
| return existingFee + (existingFee * BigInt(priceBumpPercentage)) / 100n; |
There was a problem hiding this comment.
We should ensure that priceBumpPercentage is an integer before passing it to BigInt().
There was a problem hiding this comment.
Fixed - I changed the type
yarn-project/p2p/src/config.ts
Outdated
| @@ -471,7 +472,7 @@ export const p2pConfigMappings: ConfigMappingsType<P2PConfig> = { | |||
| env: 'P2P_RPC_PRICE_BUMP_PERCENTAGE', | |||
| description: | |||
| 'Minimum percentage fee increase required to replace an existing tx via RPC. Set to 0 to disable price bumps.', | |||
There was a problem hiding this comment.
Set to 0 to disable price bumps.
This isn't really true is it? In reality the minimum bump is 1.
There was a problem hiding this comment.
fixed - updated the env variable description
# Conflicts: # docs/docs-operate/operators/reference/changelog/v4.md
|
❌ Failed to cherry-pick to |
Adds a configurable percentage-based "price bump" requirement for RPC-submitted transactions that clash on nullifiers with existing pool transactions, or that need to evict the lowest-priority tx when the pool is full. This prevents spam via infinitesimally small fee increments. - When a tx arrives via RPC with nullifier conflicts, it must now pay at least X% above each conflicting tx's priority fee (default: 10%) — i.e. `>= existingFee + existingFee * bump / 100` - The same bump applies to pool-full eviction via `LowPriorityPreAddRule` - P2P gossip path is unchanged — continues using `comparePriority` (fee + hash tiebreaker) with no bump - Rejection errors now include the minimum required fee so callers know how much to bid - New env var `P2P_RPC_PRICE_BUMP_PERCENTAGE` (default: 10) controls the bump percentage - `getMinimumPriceBumpFee(existingFee, priceBumpPercentage)` helper computes the threshold using integer arithmetic: `existingFee + max(existingFee * bump / 100, 1)` — the minimum bump is always at least 1 unit, so replacement always requires paying strictly more (even with 0% bump or zero existing fee) - `priceBumpPercentage` is typed as `bigint` throughout the config chain to avoid `BigInt()` conversion issues with non-integer values - `checkNullifierConflict` accepts an optional `priceBumpPercentage` param; when set, uses fee-only `>=` comparison against the bumped threshold instead of `comparePriority` - `NullifierConflictRule` now passes `context.priceBumpPercentage` through to the conflict check (previously ignored context entirely) - `LowPriorityPreAddRule` uses the bumped fee threshold when both `feeComparisonOnly` and `priceBumpPercentage` are set - Config flows: `P2P_RPC_PRICE_BUMP_PERCENTAGE` env var -> `P2PConfig` -> `TxPoolV2Config` -> `PreAddContext` (only for RPC path) - `NULLIFIER_CONFLICT` rejection error enriched with `minimumPriceBumpFee` and `txPriorityFee` fields - 15 new unit tests across `tx_metadata.test.ts`, `nullifier_conflict_rule.test.ts`, and `low_priority_pre_add_rule.test.ts` - Tests cover: exact threshold acceptance, below-threshold rejection, well-above threshold, 0% bump edge case, P2P path unchanged, error field population - All existing tests continue to pass Fixes A-452
…o v4 (#21036) Backport of #20806 to v4. Adds a configurable percentage-based price bump requirement for RPC-submitted transactions that clash on nullifiers with existing pool transactions, or that need to evict the lowest-priority tx when the pool is full. This prevents spam via infinitesimally small fee increments. ## Conflict resolution Three files had conflicts due to `dropTransactionsProbability` (present on next, absent on v4): - `docs/docs-operate/operators/reference/changelog/v4.md` — added the price bump changelog section without the unrelated "Setup allow list" section - `yarn-project/p2p/src/client/factory.ts` — added `priceBumpPercentage` config pass-through (without `dropTransactionsProbability`) - `yarn-project/p2p/src/mem_pools/tx_pool_v2/interfaces.ts` — added `priceBumpPercentage` to type and defaults (without `dropTransactionsProbability`) ClaudeBox log: http://ci.aztec-labs.com/ff393a9b4d8e95fb-1 Co-authored-by: Michal Rzeszutko <michal.rzeszutko@gmail.com>
BEGIN_COMMIT_OVERRIDE test: update proving-real test to mbps (#20991) chore: epoch proving log analyzer (#21033) chore: update pause script to allow resume (#21032) feat: price bump for RPC transaction replacement (#20806) refactor: remove update checker, retain version checks (#20898) fix: (A-592) p2p client proposal tx collector test (#20998) refactor: use publishers-per-pod in deployments (#21039) chore: web3signer refreshes keystore (#21045) feat(sequencer): set block building limits from checkpoint limits (#20974) chore(e2e): fix e2e bot L1 tx nonce reuse (#21052) feat: Update L1 to L2 message APIs (#20913) fix: (A-589) epochs l1 reorgs test (#20999) feat(sequencer): add SEQ_MAX_TX_PER_CHECKPOINT config (#21016) fix: drop --pid=host from docker_isolate (#21081) feat: standby mode for prover broker (#21098) fix(p2p): remove default block handler in favor of block handler (#21105) feat(validator): add VALIDATOR_ env vars for independent block limits (#21060) refactor(p2p): decouple proposal validators from base class via composition (#21075) feat: additional validation in public setup allowlist (onlySelf + null msg sender) (#21122) fix: (A-591) aztecProofSubmissionEpochs incorrectly named as aztecProofSubmissionWindow (#21108) refactor(sequencer): rename SEQ_GAS_PER_BLOCK_ALLOCATION_MULTIPLIER to SEQ_PER_BLOCK_ALLOCATION_MULTIPLIER (#21125) fix: unbound variable in check_doc_references.sh with set -u (#21126) feat: calldata length validation of public setup function allowlist (#21139) fix: include mismatched values in tx metadata validation errors (#21147) feat: single-node implementation of slash-protection signer (#20894) feat: Remove non-protocol contracts from public setup allowlist (#21154) chore: More updated Alpha configuration (#21155) chore: tally slashing pruning improvements (#21161) fix: update dependencies (#20997) fix: omit bigint priceBumpPercentage from IPC config in testbench worker (#21169) refactor(p2p): (A-588) maintain sorted array in tx pool instead of sorting on read (#21079) fix(p2p): report most severe failure in runValidations (#21185) fix: use dedicated L1 account for bot bridge resume tests to avoid nonce race (#21148) fix: parse error.message in formatViemError (#21163) fix: bump lighthouse consensus client v7.1.0 -> v8.0.1 (#21170) chore: code decuplication + refactor (public setup allowlist) (#21200) END_COMMIT_OVERRIDE
Summary
Adds a configurable percentage-based "price bump" requirement for RPC-submitted transactions that clash on nullifiers with existing pool transactions, or that need to evict the lowest-priority tx when the pool is full. This prevents spam via infinitesimally small fee increments.
>= existingFee + existingFee * bump / 100LowPriorityPreAddRulecomparePriority(fee + hash tiebreaker) with no bumpP2P_RPC_PRICE_BUMP_PERCENTAGE(default: 10) controls the bump percentageImplementation details
getMinimumPriceBumpFee(existingFee, priceBumpPercentage)helper computes the threshold using integer arithmetic:existingFee + max(existingFee * bump / 100, 1)— the minimum bump is always at least 1 unit, so replacement always requires paying strictly more (even with 0% bump or zero existing fee)priceBumpPercentageis typed asbigintthroughout the config chain to avoidBigInt()conversion issues with non-integer valuescheckNullifierConflictaccepts an optionalpriceBumpPercentageparam; when set, uses fee-only>=comparison against the bumped threshold instead ofcomparePriorityNullifierConflictRulenow passescontext.priceBumpPercentagethrough to the conflict check (previously ignored context entirely)LowPriorityPreAddRuleuses the bumped fee threshold when bothfeeComparisonOnlyandpriceBumpPercentageare setP2P_RPC_PRICE_BUMP_PERCENTAGEenv var ->P2PConfig->TxPoolV2Config->PreAddContext(only for RPC path)NULLIFIER_CONFLICTrejection error enriched withminimumPriceBumpFeeandtxPriorityFeefieldsTest plan
tx_metadata.test.ts,nullifier_conflict_rule.test.ts, andlow_priority_pre_add_rule.test.tsFixes A-452