refactor(test): add gossip screening for fees and fix breakpoint test - #830
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The new IngestScreen type currently won’t compile due to anyhow::Result generic misuse, and the gossip driver’s inline async screening also introduces a potential responsiveness/DoS concern that should be addressed.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR reduces unit-test CI wall time and tightens fee-admission hygiene by introducing a cheap fee screen earlier in both the block builder and the gossip-ingest path, plus refactoring an expensive storage breakpoint test to avoid unnecessary zkVM guest execution.
Changes:
- Refactors
put_block_stores_breakpoint_in_same_batchto use dummy (unsettleable) blocks, eliminating zkVM execution in that test. - Adds a fee-admission screen (
fees::screen) before scratch-clone settlement in the sequencer’s mempool candidate path. - Adds an advisory gossip-ingest fee screen via a new
ScreenTransactionexecutor message and anIngestScreenhook inGossipNetwork.
File summaries
| File | Description |
|---|---|
| lez/storage/src/indexer/tests.rs | Refactors breakpoint test to chain dummy blocks and avoid zkVM guest runs. |
| lez/sequencer/service/src/lib.rs | Wires a gossip-ingest fee screen via the executor actor before mempool admission. |
| lez/sequencer/core/src/lib.rs | Runs fees::screen before scratch-clone settlement for User/Gossip candidates. |
| lez/sequencer/core/src/gossip/tests.rs | Updates gossip tests to pass an admit-all ingest screen. |
| lez/sequencer/core/src/gossip/network.rs | Adds IngestScreen, applies it during gossip ingest, and makes swarm event handling async. |
| lez/sequencer/core/src/gossip/mod.rs | Re-exports IngestScreen and admit_all_screen. |
| lez/sequencer/actors/executor/src/protocol.rs | Introduces ScreenTransaction message for screen-only admission checks. |
| lez/sequencer/actors/executor/src/actor.rs | Implements handler for ScreenTransaction to run fees::screen on head state. |
| lez/programs/fee/core/src/market.rs | Documents that MAX_GAS_EXEC caps action-phase gas only (reserve/refund + tails unmetered). |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
2a476c8 to
7d956aa
Compare
|
#831 must be merged BEFORE this one |
Arjentix
left a comment
There was a problem hiding this comment.
LGTM, looking forward to make gossiper a separate actor
put_block never settles, so the breakpoint-batch test does not need settleable blocks; dummy blocks keep it off the zkVM entirely (~795 guest executions to zero). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fee-screen transactions at both cheap chokepoints: - builder: drop unfundable/fee-invalid candidates before paying for the scratch clone and the settlement's guest executions, for every origin - gossip ingest: run the same screen (via the new screen-only ScreenTransaction executor message) before a peer's transaction takes a mempool slot; advisory only — the verdict never affects mesh acceptance, and the tx stays unseen so a rebroadcast can retry Also document that MAX_GAS_EXEC caps action-phase gas only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
9dcbe24 to
c418e46
Compare
🎯 Purpose
The #801 (fees) merge tripled
nextestwall time (525s → 1530s; the unit-tests job went 24.5 min → ~53 min and started timing out): every charged transaction now runs up to three zkVM guest emulations (reserve → action → refund) and dev mode does not help (as it skips proving, not execution).This PR takes the free wins from that investigation (kudos to @moudyellaz ):
A test
put_block_stores_breakpoint_in_same_batchthat never needed settleable blocks was paying for ~795 guest executions.The block builder ran a full scratch-clone settlement (all three guest runs) on every mempool candidate before discovering it could not pay, and gossiped transactions entered the mempool with no fee screening at all.
⚙️ Approach
put_block_stores_breakpoint_in_same_batch(storage): chainproduce_dummy_blocks instead of signed, settled transfer blocks —put_blocknever settles, so the breakpoint-batch assertions are unchanged and the test goes from ~795 guest executions to zero (327s → seconds in CI)fees::screen(static checks + one balance read, no clone, no execution) on every User/Gossip candidate before the scratch clone and settlement, so junk is dropped for a balance read instead of three guest runsfees::screenbefore it takes a mempool slot, via a new screen-onlyScreenTransactionexecutor message — one admission door for RPC and gossip. Deliberately advisory: the verdict never affects gossipsub mesh acceptance (admission is priced off the local head state, which drifts, so peers legitimately disagree), and a screened-out tx stays un-seenso a rebroadcast can retry once e.g. its payer is funded. Closes fees: screen gossip-ingested transactions through fee admission #797MAX_GAS_EXECcaps action-phase gas only; the reserve/refund invocations and the fee/clock tail run unmetered as a bounded per-transaction constant🧪 How to Test
🔗 Dependencies
None
🔜 Future Work
None
📋 PR Completion Checklist