-
Notifications
You must be signed in to change notification settings - Fork 607
feat: price bump for RPC transaction replacement #20806
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
f0066fa
750fa8b
d69e055
6258b1f
3e37d6c
8c1752a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| import { type TxMetaData, stubTxMetaValidationData } 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', () => { | ||
|
|
@@ -270,6 +270,112 @@ 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<string, TxMetaData>([['0x2222', existingMeta]]); | ||
| const nullifierMap = new Map<string, string>([[sharedNullifier, '0x2222']]); | ||
| poolAccess = createPoolAccess(nullifierMap, metadataMap); | ||
|
|
||
| const context: PreAddContext = { feeComparisonOnly: true, priceBumpPercentage: 10 }; | ||
| const result = await rule.check(incomingMeta, poolAccess, context); | ||
|
|
||
| expect(result.shouldIgnore).toBe(false); | ||
| expect(result.txHashesToEvict).toContain('0x2222'); | ||
| }); | ||
|
|
||
| it('rejects 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% — not enough | ||
|
|
||
| const metadataMap = new Map<string, TxMetaData>([['0x2222', existingMeta]]); | ||
| const nullifierMap = new Map<string, string>([[sharedNullifier, '0x2222']]); | ||
| poolAccess = createPoolAccess(nullifierMap, metadataMap); | ||
|
|
||
| const context: PreAddContext = { feeComparisonOnly: true, priceBumpPercentage: 10 }; | ||
| 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); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed - I changed the logic to be >= price bump. |
||
| expect(result.reason.txPriorityFee).toBe(110n); | ||
| } | ||
| }); | ||
|
|
||
| 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<string, TxMetaData>([['0x2222', existingMeta]]); | ||
| const nullifierMap = new Map<string, string>([[sharedNullifier, '0x2222']]); | ||
| poolAccess = createPoolAccess(nullifierMap, metadataMap); | ||
|
|
||
| const context: PreAddContext = { feeComparisonOnly: true, priceBumpPercentage: 10 }; | ||
| 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<string, TxMetaData>([['0x2222', existingMeta]]); | ||
| const nullifierMap = new Map<string, string>([[sharedNullifier, '0x2222']]); | ||
| poolAccess = createPoolAccess(nullifierMap, metadataMap); | ||
|
|
||
| const context: PreAddContext = { feeComparisonOnly: true, priceBumpPercentage: 10 }; | ||
| 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<string, TxMetaData>([['0x2222', existingMeta]]); | ||
| const nullifierMap = new Map<string, string>([[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 (strict >)', async () => { | ||
| const sharedNullifier = '0xshared_null'; | ||
| const existingMeta = createMeta('0x2222', 100n, [sharedNullifier]); | ||
| const incomingMeta = createMeta('0x1111', 100n, [sharedNullifier]); | ||
|
|
||
| const metadataMap = new Map<string, TxMetaData>([['0x2222', existingMeta]]); | ||
| const nullifierMap = new Map<string, string>([[sharedNullifier, '0x2222']]); | ||
| poolAccess = createPoolAccess(nullifierMap, metadataMap); | ||
|
|
||
| const context: PreAddContext = { feeComparisonOnly: true, priceBumpPercentage: 0 }; | ||
| 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'; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed - updated the env variable description