fix(load-tests): Base fee reservation and funder affordability - #4294
fix(load-tests): Base fee reservation and funder affordability#4294meyer9 wants to merge 5 commits into
Conversation
ab1fb97 to
f2aff2e
Compare
910614b to
ed15a94
Compare
f2aff2e to
caacca2
Compare
ed15a94 to
0a90d26
Compare
caacca2 to
3eb0330
Compare
0a90d26 to
5628939
Compare
| queued_funder_transactions = | ||
| queued_funder_transactions.saturating_add(queued_nonces.len()); |
There was a problem hiding this comment.
queued_funder_transactions is accumulated with saturating_add(queued_nonces.len()) across every txpool endpoint. If multiple nodes gossip the same queued transactions, the same nonces are counted multiple times, which could inflate the total above zero even when a single node's queued set was successfully drained by the earlier drop_sender_transactions call.
Consider deduplicating nonces (e.g. collect into a HashSet) or taking the max across endpoints rather than the sum, similar to what highest_txpool_nonce already does.
Amp-Thread-ID: https://ampcode.com/threads/T-019fa48a-da77-7350-ba7c-64e3d06edc76 Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019fa48a-da77-7350-ba7c-64e3d06edc76 Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019fa48a-da77-7350-ba7c-64e3d06edc76 Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019fa48a-da77-7350-ba7c-64e3d06edc76 Co-authored-by: Amp <amp@ampcode.com>
3eb0330 to
99616c5
Compare
5628939 to
0ab49e4
Compare
…fordability Co-authored-by: Cursor <cursoragent@cursor.com>
| .saturating_mul(MAX_FEE_BASE_FEE_MULTIPLIER) | ||
| .max(base_fee.saturating_add(priority_fee)); | ||
| target.min(max_gas_price).max(priority_fee) | ||
| target.min(max_gas_price) |
There was a problem hiding this comment.
Removing the .max(priority_fee) floor means submission_max_fee can now return a value less than priority_fee. This produces an invalid EIP-1559 transaction (maxFeePerGas < maxPriorityFeePerGas).
The call sites touched by this PR correctly pre-clamp priority_fee with .min(max_gas_price), but at least two existing callers do not:
load_runner.rs:262(calibration path)pacing.rs:1172(batch signing path)
With the new default max_gas_price of 0.01 gwei, if base_fee exceeds 0.1 gwei then priority_fee = base_fee/10 will exceed max_gas_price, and these call sites will build transactions with maxPriorityFeePerGas > maxFeePerGas.
Either add the .min(max_gas_price) clamp to all remaining callers, or keep the .max(priority_fee) floor here as a safety net.
| queued_funder_transactions = | ||
| queued_funder_transactions.saturating_add(queued_nonces.len()); |
There was a problem hiding this comment.
queued_funder_transactions is summed across all txpool endpoints with saturating_add. If multiple nodes gossip the same queued transactions, identical nonces are double-counted.
This matters because the check at line 153 hard-errors when the total exceeds zero. A single queued transaction visible on two nodes would report a count of 2 even if drop_sender_transactions successfully cleared one of them — the second node may just not have propagated the removal yet.
Consider deduplicating nonces (e.g. collect into a HashSet<u64>) or taking the max count across endpoints rather than the sum, similar to the highest_txpool_nonce logic.
Review SummaryChanges reviewed: Enforce funder max-cost affordability, clamp Findings1. 2. Queued funder transaction count inflated by multi-node double-counting (correctness) Other notes
|
Summary
Stack
PR 4/5. Base:
feat/load-tests-cumulative-gps-pacing(#4293).Test plan
cargo test -p base-load-tests --libbase-builder-core