evmonly: revert the background receipt/state pipeline (#4264, #4266) - #4268
Conversation
Reverts 5c37395 and 187cd78 on top of #4265 and #4267. Receipts are encoded and written on the block loop before the state commit again, and Commit no longer waits for them. The #4265 nil receipt store is kept: the executor skips the receipt write. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
PR SummaryMedium Risk Overview Executor block tail is back on the block loop: optionally await the prior state commit when encoding reads the store, encode state (+ block) changesets while the execution snapshot is still open, encode and Failure and visibility semantics change: encode/receipt/state errors return without handing results to Docs and tests are updated to match (including removing Commit-must-have-receipts assertions and most pipeline ordering tests). Reviewed by Cursor Bugbot for commit e1e27fe. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## giga-1 #4268 +/- ##
===========================================
+ Coverage 65.55% 86.41% +20.86%
===========================================
Files 2081 35 -2046
Lines 157460 4793 -152667
===========================================
- Hits 103222 4142 -99080
+ Misses 54097 650 -53447
+ Partials 141 1 -140
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
A faithful, well-scoped revert of the #4264/#4266 receipt/state pipeline back to the measured-faster synchronous path, keeping #4265's optional receipt store and #4267's timers; the code, tests, docs and metric removals are all consistent with no dangling references. The only notes are about the durability/read-your-writes window that the removed waits used to close.
Findings: 0 blocking | 3 non-blocking | 1 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- [suggestion]
evmOnlyApplication.Commitno longer waits for the block's receipts, so RPC advanceslatestto height N while N's receipts may still be in the receipt store's async queue.giga/evmonly/rpc/tx.goserveseth_getTransactionReceiptfrom that store, so a client that reads the head and immediately asks for a receipt in it can get not-found for a short window. This is the pre-#4264 behaviour and polling clients tolerate it, but the guarantee thatTestEVMOnlyApplicationCommitReturnsWithTheBlocksReceiptsReadablepinned is now gone with nothing recording the weaker contract — worth a line ingiga/evmonly/README.mdor theCommitgodoc so the next reader does not re-derive it. - 1 suggestion(s)/nit(s) flagged inline on specific lines.
- 1 non-blocking pre-existing issue(s) listed below under pre-existing issues.
Pre-existing issues
- [suggestion]
recoveryTargetinsei-db/bootstrap/recovery.go:181deliberately ignores a receipt head of 0 so newly-enabled receipts do not drag the target down. That also means a node whose receipt store is genuinely empty while the block store and state WAL hold blocks recovers tomin(block, state)and never regenerates receipts for those blocks — the one case where a receipt/state head divergence is silently accepted rather than replayed.
…en the state head advances Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Describe your changes and provide context
Reverts #4264 (receipts and state encoding persisted behind the block,
Commitwaits for receipts) and #4266 (receipt write started right after execution) on top of #4265 and #4267, which stay.On testnet-2 the pair never beat the pre-#4264 image: #4264 warmed at ~86.8k tx/s vs ~96k before it (
Commit's receipt wait,app_commit0.16 s/s, cost more loop time than the encodes it removed), and #4266 moved that wait offapp_commitbutvault_commitrose from ~0.05 to ~0.18 s/s and stayed there for the whole run (~89k tx/s). The likely reason is the FlatKV WAL fsync now coinciding with the HashVault fsync for the same block, but rather than tune a pipeline that has not paid for itself, this returns the executor to the path that measured 96k.State after this PR (
giga/evmonly/giga_store.go,executePreparedBlockWithStore):Executor.AwaitReceipts,startReceiptWrite,persistReceipts,commitStateChanges, theevmonly_pipeline/evmonly_receiptsphase timers andencodingReadsTheStore's removal are all undone;encodingReadsTheStoreis back.evmOnlyApplication.Commitno longer waits on receipts.littReceiptStore.WaitForPendingWrites/receiptWrite.landed(added in evmonly: persist receipts and encode state changes behind the block; Commit advances the head once the receipts have landed #4264 only for that wait) are removed;receiptStore.WaitForPendingWritesinsei-db/ledger_db/receipt/receipt_store.golikewise.enable_receipt_store) #4265's optional receipt store is preserved in the synchronous path:e.receiptStore == nilskips encode+write (errMissingReceiptStorestays removed). Its pipeline-specific test is replaced byTestNoReceiptStoreCommitsStateOnly.evmonly_preparetimer) and autobahn: split the execute loop's storage phase into vault commit / app commit / push app hash / prune #4263's router-tail timers are untouched.Not a pure
git revertbecause #4265 landed on top of both commits: the 12 files #4264/#4266 touched were restored from9a731d5and the #4265/#4267 hunks re-applied by 3-way merge; the only hand-resolved hunk is the nil-store guard around the receipt write above.Testing performed to validate your change
scripts/ramtest.sh ./giga/evmonly/... ./sei-db/ledger_db/receipt/... -race— green.go test -race -count=2 ./internal/evmonlyapp/(sei-tendermint) — green;go vetonevmonlyapp,node.golangci-lint run ./giga/evmonly/...0 issues;make fmtcheckclean.Link to Devin session: https://app.devin.ai/sessions/ff612badcded4aa5914ea408dbb41888
Open in Devin Desktop: https://app.devin.ai/desktop/session/ff612badcded4aa5914ea408dbb41888?variant=devin
Requested by: @bdchatham