Repository navigation
Add giga.storage.receipts to toggle the Autobahn receipt store - #4333
Conversation
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
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 @@
## main #4333 +/- ##
==========================================
- Coverage 67.58% 66.49% -1.09%
==========================================
Files 2190 2069 -121
Lines 168242 156250 -11992
==========================================
- Hits 113708 103900 -9808
+ Misses 54524 52340 -2184
Partials 10 10
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
PR SummaryMedium Risk Overview When receipts are disabled, the node does not start the EVM-only RPC (it depends on the store), and store-backed execution commits state without calling Config reading, app.toml template, registry keys, and tests cover the new setting, composed layouts, recovery guard, and an EVM-only app path without a receipt store. Reviewed by Cursor Bugbot for commit 6235848. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 26277da. Configure here.
There was a problem hiding this comment.
The new giga.storage.receipts key is read, templated and tested correctly, but setting it to false stops the node from finalizing any block. The evmonly executor still requires a receipt store and returns an error when it is missing.
Findings: 1 blocking | 1 non-blocking | 1 posted inline
Blockers
- None at the file/PR level.
- 1 blocking issue(s) flagged inline on specific lines.
Non-blocking
- [suggestion]
TestPrepareApplicationReceiptsOffOpensNoReceiptStoreonly checks thatReceiptDB()is nil after construction. Add a test that runs InitChain and finalizes at least one block (even an empty one) with receipts disabled. That test would have caught the executor failure, and it is the behaviour the new flag needs to guarantee.
|
@seidroid review |
There was a problem hiding this comment.
The previous blocker is fixed: the executor now treats the receipt store as optional, and a test now finalizes and commits a block without one. One new problem comes from exposing the toggle. If an operator turns receipts off and later back on, the old receipt head drags the startup recovery target down, which can roll state back a long way or stop the node from starting.
Findings: 1 blocking | 0 non-blocking | 1 posted inline
Blockers
- None at the file/PR level.
- 1 blocking issue(s) flagged inline on specific lines.
Non-blocking
- None at the file/PR level.
…n it A receipt head more than rollback_window below the block and state heads is left by a stretch with receipts disabled, not a crash: state cannot roll back that far, so taking it as the recovery target either rewinds state or refuses to start. Recovery now leaves such a head out of the target and deletes the store, since the receipt store refuses writes that skip blocks and could not resume from where it stopped. The new store starts recording at the next block.
|
@seidroid review |
There was a problem hiding this comment.
The earlier problem is fixed: turning receipts off and back on for longer than rollback_window no longer rolls state back or stops the node from starting. But the fix treats "more than rollback_window behind" as "receipts were disabled". With a small or zero window, an ordinary crash, where the async receipt queue simply lags, now deletes the whole receipt store.
Findings: 1 blocking | 1 non-blocking | 1 posted inline
Blockers
- None at the file/PR level.
- 1 blocking issue(s) flagged inline on specific lines.
Non-blocking
- [suggestion] The in-window half of the previous on → off → on finding is still there. If receipts are off for fewer than
rollback_windowblocks (default 1000) and then turned back on,recoveryTargetstill uses the old receipt head as the target, so recovery still silently rolls block and state stores back by up to that many blocks. If this is intended, document it on thereceiptskey. If not, fix it with the same explicit marker suggested inline, and add a restart test for a short off stretch.
…story Replaces the stale-head auto-discard: receipt lag alone cannot tell an intentional off period from a crash with pending async receipt writes, so recovery no longer deletes anything. Instead OpenDBWithRecovery fails with an actionable error when receipts are disabled but the store on disk has a nonzero head, leaving the operator to move the directory or re-enable receipts. Reverts the recoveryTarget rollback-window filtering and receipt.Discard.
|
@seidroid review |
There was a problem hiding this comment.
The PR adds a giga.storage.receipts toggle. Every earlier finding is fixed: the executor now runs without a receipt store, and startup refuses to run with receipts disabled while the receipt directory still holds data, so there is no stale-head recovery and no automatic deletion. I found no new issues.
Findings: 0 blocking | 0 non-blocking | 0 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- None at the file/PR level.
Superseded: latest AI review found no blocking issues.
* main: (21 commits) Backport evmonly parse, app-hash and changeset perf fixes from giga-1 (#4345) Remove the oracle module behind a v6.8 upgrade (#4319) fix(seidb): report only the current migration boundary on the snapshot gauge (#4327) fix(flatkv): keep 10 old checkpoints instead of mirroring memIAVL's count (#4322) Backport Autobahn execute-loop and produced-tx metrics from giga-1 (#4330) Add giga.storage.receipts to toggle the Autobahn receipt store (#4333) optimize gather phase (#4326) Regenerate the Unreleased changelog as a plain PR list (#4336) Bump sei-protocol/go-ethereum to v1.15.7-sei-21 (#4332) Fail dynamic-gas precompile out-of-gas as an EVM out-of-gas call (#4318) Add dashboard and topology option for Autobahn e2e (#4167) Add Giga fetch/serve and BlockDB prune metrics (#4329) Add eth_getLogs to the EVM-only Giga RPC (#4308) Generate v6.8 precompiles (#4320) Add [giga] app.toml section and honor it on the Autobahn node (#4323) feat(evmonly): add eth_estimateGas via existing libraries (#4325) Use Pebble batch directly in SS (#4300) Fix pruning issue in SS causing huge disk spike (#4321) Make Autobahn always run the EVM-only executor, disable/remove some integration tests (#4316) reduce seal lock contention (#4314) ...

On an Autobahn node the receipt store is always open:
AutobahnStorageConfigapplies validator mode and then re-enables receipts, because the node's EVM-only RPC serveseth_getTransactionReceipt,eth_getLogs,eth_feeHistoryand block encodings from it.[giga.storage] modetherefore only decides whether the state store opens, and there is no way to run a validator that neither serves EVM queries nor pays for the receipt store, nor to see from app.toml that receipts are on at all.This adds
receipts = true|falseto[giga.storage], defaulting to true so every existing layout is unchanged.buildGigaStorageConfigapplies it after mode selection, somodeandreceiptscompose freely (validator + receipts is today's default; full + no receipts is state store only). When the store is disabled the node logs that and does not start the EVM-only RPC, since every registered namespace reads the store; the storage manager already returns a nilReceiptDB()in that case. The executor's store-backed path also required a receipt store, so it now treats it as optional and skipsSetReceiptswhen none is configured; otherwise a receipt-less node could open but never finalize a block. Tests cover the new key in the reader and registry, the composed layouts inbuildGigaStorageConfig, thatprepareApplicationopens no receipt store when the flag is off, and that the EVM-only application finalizes and commits a block carrying a transaction without one.Turning receipts off and later on again would leave a receipt head well below the block and state heads, which recovery takes as the convergence height and either rewinds state or refuses to start, and receipt lag alone cannot tell that off period from a crash with async receipt writes still queued. So the transition is refused up front:
OpenDBWithRecoveryfails with an actionable error when receipts are disabled while the store on disk holds receipts, telling the operator to move the directory away or enable receipts again. Nothing is deleted automatically; a store that was opened but never written does not block the switch. The key's documentation says so.