evmonly: let a node run without a receipt store (autobahn enable_receipt_store) - #4265
Conversation
…ipt_store) Validators serve no receipt reads, so the receipt write and the wait for it to land in Commit buy them nothing. NewValidatorStorageConfig now takes the choice; node/setup passes AutobahnFileConfig.EnableReceiptStore (default true). The executor commits state only when it has no receipt store and AwaitReceipts returns at once; EVM-only RPC still serves blocks and receipt lookups report ErrNoReceiptStore. 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 Storage & executor: RPC: Safety at startup: If receipts are disabled but the receipt directory still has data, startup fails with Reviewed by Cursor Bugbot for commit 5886a4e. 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 #4265 +/- ##
===========================================
+ Coverage 65.55% 82.36% +16.81%
===========================================
Files 2081 52 -2029
Lines 157460 7356 -150104
===========================================
- Hits 103222 6059 -97163
+ Misses 54097 1296 -52801
+ 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.
The receipt store is cleanly made optional across the executor, the EVM-only RPC handler and the storage config, with matching test coverage. The gap is the toggle's reverse direction: re-enabling enable_receipt_store on a node that ran without it leaves a stale receipt head that recovery folds into its convergence target, rolling state back or refusing startup — contrary to what the PR description states.
Findings: 1 blocking | 2 non-blocking | 1 posted inline
Blockers
- None at the file/PR level.
- 1 blocking issue(s) flagged inline on specific lines.
Non-blocking
- [suggestion]
evmOnlyApplication.Commit's godoc (sei-tendermint/internal/evmonlyapp/app.go:868-875) still states the invariant "once its receipts are readable: the height this advances is what RPC serves as latest, and a block it serves has its receipts." Withenable_receipt_store = falsethat is no longer true —AwaitReceiptsreturns with nothing written. The README,AwaitReceiptsandstartPipelineCommitdocs were all updated for the new mode; this one was missed, and it is the load-bearing statement of the property the flag removes. - [suggestion] Receipt-derived fields degrade to plausible-but-wrong zeros rather than an error on a receipt-less node:
blockGasUsedreturns 0 (documented in the PR) but so doeseth_feeHistory'sgasUsedRatio(giga/evmonly/rpc/info.go:222), which clients use for fee estimation. Worth either stating this in theAutobahnFileConfig.EnableReceiptStoredoc alongside "serves no receipts or logs", or handling fee history the way tx lookups are handled, so the two halves of the RPC surface answer consistently.
… document receipt-less RPC semantics Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
Both non-blocking notes taken in 84c817e:
|
|
@seidroid review |
There was a problem hiding this comment.
The optional receipt store is wired cleanly end to end — executor, RPC handler, storage config and recovery — and the previously blocking re-enable hazard is now closed by a startup refusal at the one function every open passes through, with a restart-loop test that exercises the whole sequence. Only a small doc-accuracy point remains.
Findings: 0 blocking | 1 non-blocking | 1 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- None at the file/PR level.
- 1 suggestion(s)/nit(s) flagged inline on specific lines.
Superseded: latest AI review found no blocking issues.
…tore Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ner) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
… skips the receipt write Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Since #4264,
Commiton an EVM-only node waits for the block's receipts to land in the litt store before advancing the RPC-visible head. On testnet-2 that wait (app_commit≈ 0.16 s/s,await_receipts≈ 0.17 s/s) outweighed the loop time #4264 saved, and the 40 validators went from ~96k to ~87k tx/s. Validators serve no receipt reads, so both the write and the wait buy them nothing;GigaStorageConfig.WithValidatorMode()already turns the receipt DB off, butNewValidatorStorageConfigimmediately forced it back on and the executor and RPC server refused to start without one.NewValidatorStorageConfig(homePath, receipts bool)now takes the choice, andnode/setup.gopasses a newAutobahnFileConfig.EnableReceiptStore(enable_receipt_store,utils.Option[bool], default true, so nothing changes for a node that does not set it). With it off,bootstrap.GigaStorageManager.ReceiptDB()is nil as its docs already allow; the executor dropserrMissingReceiptStore,startPipelineCommitskipspersistReceiptsand closes the receipt gate at once, soAwaitReceiptsreturns immediately and the pipeline goes straight tocommitStateChanges. Block results still carry receipts, they are just not persisted.evmonlyrpc.newHandleraccepts a nil store:eth_getBlockBy*still serve, withgasUsed0 asblockGasUsedalready did outside the receipt range, andeth_getTransactionReceipt/eth_getTransactionByHashreturn a newErrNoReceiptStorerather thannull, which a client would read as "not mined". Registering a reduced eth namespace was the rejected alternative: sei-load and the proxy still need block, send and nonce methods on validators, and a missing method is harder to diagnose than an explicit error.No consensus or on-disk format impact: the app hash and state commit are untouched, and a node that keeps receipts behaves exactly as before. Rollout is a config change per validator (
enable_receipt_store = false), leaving RPC nodes on. Turning it off on a node whose receipt directory already holds blocks is refused at startup (bootstrap.ErrDisabledReceiptStoreHoldsBlocks, naming the directory): a disabled store would otherwise sit out recovery at its old head, and turning receipts back on later would makefindTargetRecoveryHeightconverge state back onto that stale head. With the directory removed, a later re-enable is the empty-store case recovery already handles and receipts fill from the current height — so an RPC node that ever ran without receipts has a gap, which is why the default stays true. Validated with new tests for the storage config, the executor without a receipt store (state still commits, pool result released,AwaitReceiptsreturns), the RPC handler over a nil store, the disable → advance → re-enable restart loop insei-db/bootstrap, andprepareApplicationproducing a nilReceiptDB()from the file config; race tests forgiga/evmonly,giga/evmonly/rpc,evmonlyappandnodepass.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