perf(payload): skip nonfitting general transactions before prewarming - #7972
mediocregopher wants to merge 6 commits into
Conversation
|
cyclops audit |
tempoxyz-bot
left a comment
tempoxyz-bot
left a comment
There was a problem hiding this comment.
👁️ Cyclops Review — 2 findings.
- **Needs review:** Unbounded filtering can overrun the proposal build budget
- **Needs review:** Buffered candidate drain can postpone proposal deadline checks
| let tx = loop { | ||
| let Some(tx) = ctx.best_txs.next() else { | ||
| let _ = ctx.transactions_tx.send(None); | ||
| return; | ||
| }; | ||
| match ctx.general_lane.lock().unwrap().skip(&tx) { | ||
| GeneralSkip::No => break tx, | ||
| GeneralSkip::Skip => {} | ||
| GeneralSkip::NonFitting => ctx.best_txs.mark_invalid( | ||
| &tx, | ||
| InvalidPoolTransactionError::Other(Box::new( | ||
| TempoPoolTransactionError::ExceedsNonPaymentLimit, | ||
| )), | ||
| ), | ||
| } |
There was a problem hiding this comment.
Needs review: Unbounded filtering can overrun the proposal build budget
On Presto T11 with default-enabled prewarming, first include a general transaction that reduces remaining general gas, then place many independent, higher-priority general transactions whose gas limits exceed that remainder ahead of an independent payment. A coordinator advance scans and invalidates every nonfitting transaction before sending a reply; the consumer can wait on that reply, so the…
| loop { | ||
| let tx = if let Some(tx) = self.transactions_rx.try_iter().flatten().next() { | ||
| tx | ||
| } else { | ||
| self.commands_tx | ||
| .send(BestTransactionsCommand::Advance) | ||
| .ok()?; | ||
| // An eager advance can also reply empty while this receive is waiting. | ||
| // Check for buffered transactions before reporting empty to the builder, | ||
| // but do not wait for more replies: it must still check its build budget. | ||
| self.transactions_rx | ||
| .recv() | ||
| .ok()? | ||
| .or_else(|| self.transactions_rx.try_iter().flatten().next())? | ||
| }; | ||
| match self.general_lane.lock().unwrap().skip(&tx.tx) { | ||
| GeneralSkip::No => return Some(tx), | ||
| GeneralSkip::Skip => {} | ||
| GeneralSkip::NonFitting => { | ||
| let _ = self | ||
| .commands_tx | ||
| .send(BestTransactionsCommand::InvalidWithoutDrain( | ||
| InvalidTransaction { | ||
| tx: tx.tx, | ||
| kind: InvalidPoolTransactionError::Other(Box::new( | ||
| TempoPoolTransactionError::ExceedsNonPaymentLimit, | ||
| )), | ||
| }, | ||
| )); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Needs review: Buffered candidate drain can postpone proposal deadline checks
Production try_build() feeds pool candidates to prewarming, which is enabled by default (E9–E11). If prewarming buffers many independent non-payment transactions while the builder executes earlier transactions, a subsequent reduction in remaining general gas can make those buffered candidates nonfitting (E1, E2, E4, E6). The new BestTransactionsPrewarming::next() loop then consumes and…
2 participants
The payload builder now feeds remaining general-lane gas to prewarming. The coordinator skips general transactions that cannot fit before scheduling work, while the builder lazily drops already-buffered candidates and nonce-dependent descendants without rebuilding the delivery queue.
Smaller general transactions and independent payments remain eligible.
Benchmark
Multi-region bench run:
public-mixpreset from #7569, 10 validators across 5 GCP regions, 100 GiB bloat, 50k target TPS, 300s. Baseline ismainatc2860f54f3a08a75b6f7cd03b176a584f85f4135; feature is this PR at394778814cfae196b50f2c2daea42288a9314938. One run pair.Throughput and block times improve a lot. Validator execution latency goes up, which is expected because blocks carry far more gas. Pool fetch P99 (6.4 → 10.2 ms) and reverted txs (24,677 → 43,936) also go up. The feature phase took longer to drain (1,185s vs 660s elapsed).