Repository navigation
Conversation
PublishOrigins kept the published origin cells, and only Reset/Release cleared them. A state reused for all txs of a block (debug_traceBlock*, trace_*, eth_callMany, eth_simulateV1, the block assembler with BAL) published again the origins of all previous txs at every tx, so the cost grew with the square of the block's tx count. Release the cells once they are in the versionMap.
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are reported for the origin-cell cleanup intended to prevent repeated publication.
No open issues 🎉
What changed in this PR
This PR prevents repeated publication of transaction origins when Erigon reuses a versioned state across a block, removing the resulting quadratic work.
Changes:
- Release and reset origin cells after publishing them to the version map.
- Add a regression test confirming that
FinalizedWritesclears pending origins while preserving the published storage value.
| File | Description |
|---|---|
execution/state/storage_origin_seed_test.go |
Tests origin cleanup and preservation in the version map. |
execution/state/intra_block_state.go |
Clears origin cells after PublishOrigins completes. |
Review effort: Balanced
yperbasis
left a comment
There was a problem hiding this comment.
Two non-blocking notes:
-
PublishOriginsiterates withforEachAddr, which visits an address once per populated path map. An address with both an account origin and storage origins therefore runs the closure twice and re-issues everyStoreOriginStorage. Rangingws.addressand thenws.storagedirectly would halve theLoadOrStoreboxing and make theIsEmptyhalf of the guard redundant. Fine as a follow-up. -
TestFinalizedWritesHandsOverOriginsrecords only a storage origin (a coldGetStatenever reachesseedOrigin), so nothing in the package asserts thatStoreOriginAccountruns beforereleaseVWAddressnils the cell. A coldGetBalanceorExistplus an assertion thatversionMap.LoadOriginAccount(addr)returns the committed account would cover that ordering.
Problem
IntraBlockState.PublishOriginscopies the tx's cold-read origin cells into the versionMap, but it keeps them inversionedOrigins. OnlyReset/Releaseclear that set, andResetVersionedIOdoes not. So a state that is reused for all txs of a block publishes again the origins of all previous txs at every tx, and the cost grows with the square of the block's tx count. EveryStoreOriginStoragecall also boxes key and value forsync.Map.LoadOrStore, even when the key is already there.Affected paths: everything that finalizes several txs on one versioned state through
FinalizedWrites. This includesdebug_traceBlockByHash/Number,trace_*,eth_callMany,eth_simulateV1,debug_executionWitness, the overlay API, andexecution/exec/block_assembler.gowhen BAL is active. Parallel execution is not affected, because it callsResetfor every tx.Fix
Release the origin cells at the end of
PublishOrigins, once they are in the versionMap. Nothing readsversionedOriginsafter the publish.Test
TestFinalizedWritesHandsOverOrigins(TDD): afterFinalizedWrites, the origin cells are no longer kept for the next tx, and the origin can still be read from the versionMap. It fails without the fix.Measurements
Mainnet node at the tip,
--prune.mode=minimal. Load is themixedprofile of rpc-tests (rpc_pattern_gen --profile perf/pattern/mainnet/config/mixed.profile --counts 200000 --verify) replayed withrpc_perf, 60s per step.Main (dedabcf) CPU profile at 2000 qps:
debug_traceBlockByHashis 4.3% of the requests but 83% of the CPU.PublishOriginsalone is 47% of the CPU, andStoreOriginStorageis 62% of the allocated bytes (at 1000 qps).debug_traceBlockByHash(callTracer), serial, best of 3PublishOriginsshare of CPU at 2000 qps