Pull from go-ethereum up to 2f24e25 (6 Mar 2019) - #2
Closed
AlexeyAkhunov wants to merge 145 commits into
Closed
AlexeyAkhunov wants to merge 145 commits into
AlexeyAkhunov wants to merge 145 commits into
Conversation
* node: close AccountsManager in new Close method * p2p/simulations, p2p/simulations/adapters: handle node close on shutdown * node: move node ephemeralKeystore cleanup to stop method * node: call Stop in Node.Close method * cmd/geth: close node.Node created with makeFullNode in cli commands * node: close Node instances in tests * cmd/geth, node: minor code style fixes * cmd, console, miner, mobile: proper node Close() termination
contracts/*: golint updates for this or self warning
cmd/utils, eth: relinquish GC cache to read cache in archive mode
ethapi: default to use eip-155 protected transactions
core: repro #18977
* build: use sftp for launchpad uploads * .travis.yml: configure sftp export * build: update CI docs
* common/fdlimit: cap on MacOS file limits, fixes #18994 * common/fdlimit: fix Maximum-check to respect OPEN_MAX * common/fdlimit: return error if OPEN_MAX is exceeded in Raise() * common/fdlimit: goimports * common/fdlimit: check value after setting fdlimit * common/fdlimit: make comment a bit more descriptive * cmd/utils: make fdlimit happy path a bit cleaner
* signer/clef: make use of json-rpc notification * signer: tidy up output of OnApprovedTx * accounts/external, signer: implement remote signing of text, make accounts_sign take hexdata * clef: added basic testscript * signer, external, api: add clique signing test to debug rpc, fix clique signing in clef * signer: fix clique interoperability between geth and clef * clef: rename networkid switch to chainid * clef: enable chainid flag * clef, signer: minor changes from review * clef: more tests for signer
* clef: initial implementation of bidirectional RPC communication for the UI * signer: fix tests to pass + formatting * clef: fix unused import + formatting * signer: gosimple nitpicks
This was referenced May 13, 2026
This was referenced May 15, 2026
3 tasks
sudeepdino008
added a commit
that referenced
this pull request
May 21, 2026
…21310) ## Summary Two spec-deviations in `getFilterBlockTree` compound to reject valid current-epoch leaves at every epoch boundary, causing Caplin's `GetHead` to fall back to the justified checkpoint root (a ~30-50 slot regression) and triggering execution-side unwinds. ## Observed symptom (from #21301) On bloatnet, once execution catches up to chain tip, the node enters a steady-state cycle of **22-36 block execution unwinds every ~6 minutes** — one per epoch boundary. Histogram from one ~3h run (110 unwind events): ``` 1 size=11 1 size=14 4 size=15 1 size=16 4 size=18 2 size=19 3 size=20 5 size=21 9 size=22 14 size=23 ← most common 3 size=24 5 size=25 4 size=26 4 size=27 8 size=28 1 size=30 2 size=31 4 size=32 3 size=33 4 size=34 ...many size=35-36 ``` The unwinds are on the **same chain** (no real reorg) — execution is forced to roll back to an older head because Caplin regresses its `GetHead()` return value: ``` 08:38:45 Caplin FCU: headSlot=652928 currentSlot=652988 lagSlots=60 eth1Head=0xe0f3... 08:39:36 Caplin FCU: headSlot=652993 currentSlot=652993 lagSlots=0 eth1Head=0x263b... ← at tip 08:44:00 Caplin FCU: headSlot=652959 currentSlot=653015 lagSlots=56 eth1Head=0x7674... ← regressed 34 slots ``` The 08:44:00 FCU triggers a 23-block exec unwind: ``` [forkchoice] entering unwind path fcuNum=24701328 canonHash=0x7674... fcuHash=0x7674... hashesDiffer=false finishProgress=24701350 executionAhead=true Unwind Execution from=24701350 to=24701327 ``` `hashesDiffer=false` confirms same-chain. `executionAhead=true` because exec raced ahead of Caplin's regressing head. ## Root cause: filter rejects mid-epoch leaves Tracing inside `GetHead`: ``` [GetHead] cache miss, rebuilding from justifiedCheckpoint justifiedEpoch=20410 [filterTree] reject leaf: checkpoint mismatch blockRoot=0xe644... slot=653150 justifiedOk=false finalizedOk=false storeJustEpoch=20410 blockJustEpoch=20411 storeFinalEpoch=20409 blockFinalEpoch=20410 [GetHead] filtered tree size viableBlocks=0 totalHeads=1 [GetHead] rebuilt head headHash=0x... headSlot=653119 ← fallback to justified root Caplin is sending forkchoice headSlot=653119 currentSlot=653174 lagSlots=55 ``` Every leaf in the tree gets filtered because its `unrealizedJustifications[blockRoot].epoch = N+1` but the store's `justifiedCheckpoint.epoch = N` (store's realized lags by 1 epoch mid-epoch). The walk falls back to the justified root → 50+ slot regression. ## Spec deviations (the fix) ### 1. Voting-source selection used unrealized unconditionally Spec [`get_voting_source`](https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/fork-choice.md#get_voting_source) picks unrealized only for **prior-epoch** blocks (pull-up justification view); current-epoch blocks should use the block's realized state checkpoint: ```python def get_voting_source(store: Store, block_root: Root) -> Checkpoint: block = store.blocks[block_root] current_epoch = get_current_store_epoch(store) block_epoch = compute_epoch_at_slot(block.slot) if current_epoch > block_epoch: return store.unrealized_justifications[block_root] else: return store.block_states[block_root].current_justified_checkpoint ``` Erigon's code: ```go // Use per-block unrealized justifications (spec: store.unrealized_justifications[block_root]) // Fall back to realized checkpoints if unrealized not available currentJustifiedCheckpoint, has := f.getUnrealizedJustification(blockRoot) if !has { currentJustifiedCheckpoint, has = f.forkGraph.GetCurrentJustifiedCheckpoint(blockRoot) ... } ``` → Always picks unrealized when available. The "fall back" comment misreads the spec — spec is a conditional branch, not a fallback. ### 2. Justified/finalized check was strict equality Spec [`filter_block_tree`](https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/fork-choice.md#filter_block_tree) has a lenient `+2 >= current_epoch` lookback: ```python correct_justified = ( store.justified_checkpoint.epoch == GENESIS_EPOCH or voting_source.epoch == store.justified_checkpoint.epoch or voting_source.epoch + 2 >= current_epoch # ← lenient lookback ) ``` Erigon's code only allowed the first two clauses (strict `Equal()`). ## Regression provenance The deviations were introduced in #20035 (`caplin: unified Engine API client for standalone mode`, Mar 25 2026) by Mark Holt: ``` 082697d cl/phase1/forkchoice/get_head.go +Use per-block unrealized justifications ``` Before that PR, `getFilterBlockTree` used `f.forkGraph.GetCurrentJustifiedCheckpoint(blockRoot)` directly — which returns the block's **realized** justified checkpoint. Block.realized matched store.realized (both on the same epoch-boundary timeline), so the equality check passed. PR #20035 introduced the per-block unrealized lookup (`store.unrealized_justifications` per the spec), but missed the conditional branching (spec deviation #1) and didn't add the +2 lookback (spec deviation #2). The result: block.unrealized goes ahead by 1 epoch mid-epoch, and store.realized doesn't catch up until `OnTick`'s epoch-boundary path runs. ## Fix Restore spec-faithful behavior in `getFilterBlockTree`: 1. Branch the voting source selection on `currentEpoch > blockEpoch`: - Prior-epoch → `store.unrealized_justifications[block_root]` - Current/future-epoch → block's realized `current_justified_checkpoint` In both branches, return `false` (reject the leaf) if the lookup is absent. `OnBlock` populates `unrealizedJustifications` for every imported block above the finalized slot, so a missing entry indicates either an invariant violation or a leaf below finalized — neither should produce a viable head. 2. Replace the strict `Equal()` check on the justified side with the spec's three flat disjuncts: `store.justified_checkpoint.epoch == GENESIS_EPOCH || voting_source.epoch == store.justified_checkpoint.epoch || voting_source.epoch + 2 >= current_epoch`. 3. Replace the finalized-side checkpoint-equality check with the spec's ancestor-descent rule: `store.finalized_checkpoint.root == get_checkpoint_block(block_root, finalized.epoch)`, implemented via `f.Ancestor(blockRoot, finalizedSlot)`. 4. Snapshot `currentEpoch` once at the start of the walk (in `getFilteredBlockTree`) and thread it through the recursion so the whole rebuild uses a single store-epoch value — `OnTick` updates `f.time` without holding `f.mu`, so per-leaf `f.Slot()` reads can mix epochs across leaves around a slot boundary. ## Validation Tested on bloatnet at chain tip, on `performance` HEAD + this fix: - **Before fix**: 22-36 block unwinds every ~6 min, one per epoch boundary, persisting indefinitely. 110 events in 3h. - **After fix**: zero unwinds at epoch boundaries observed across 30+ minutes (~5 epoch boundaries crossed). `[filterTree] reject` no longer fires. Effects: - **CPU waste eliminated**: ~30 blocks of execution work were being discarded and re-done every cycle. - **db_size**: unchanged (was already not affected since rolled-back state was in MemoryMutation overlay, not MDBX). - **Logs**: clean — no more spurious `Unwind Execution` lines at tip. ## Test plan - [x] Run on bloatnet to chain tip, observe no large unwinds at epoch boundaries - [x] Confirm no head regression in `Caplin is sending forkchoice` logs (lagSlots stays ~0 at tip) - [ ] Spectest still passes (`make spectest`) ## Refs Full diagnosis trail: #21301
lystopad
added a commit
that referenced
this pull request
May 21, 2026
Findings from Copilot (3) and yperbasis (8). The Copilot findings on Peers routing, empty NodeInfo, and close(statusReady) are real; yperbasis identified the matching doc/code mismatches and a few polish items. 1. Peers() / message routing (Copilot #1). The multi-sentry client uses Peers() to decide which sentry owns each peer and routes SendMessageById via that sentry's gRPC. With the previous reporter-only gating, every peer mapped to Servers[0] (which is the *lowest* configured protocol, ETH69 in the default — yperbasis #1 noted the comment said "highest"). Servers[0]'s goodPeers doesn't have eth/70 or eth/71 peers, so SendMessageById silently no-op'd. Fix: each GrpcServer.Peers() now reports its own goodPeers, filtered to skip entries where both protocol and witProtocol are zero. Each peer ends up in exactly one eth-sentry's goodPeers (the negotiated version) plus, at most, the sentry hosting the wit sideprotocol, so admin_peers aggregation is naturally non-duplicating and routing is correct. 2. NodeInfo() (Copilot #3). Non-reporters returning empty replies polluted admin_nodeInfo with blank entries that sorted first. With the shared p2p.Server every sentry has the same Node ID and the same enode, so they now return identical NodeInfo. node/eth.NodesInfo deduplicates by Enode before sorting. 3. SetStatus's close(ss.statusReady) (Copilot #2) panicked for callers that construct GrpcServer outside NewGrpcServer (existing TestSentryServerImpl_* does this). Guarded the close with a nil check; awaitStatus tolerates a nil channel via the select's other cases. 4. SetP2PServer returns an error instead of panicking on double-call (yperbasis #3). The "ownership decided up front" invariant is still enforced, just propagated up the Provider.Initialize path cleanly. 5. SimplePeerCount filters protocol=0 ghosts (yperbasis #4). With wit/0 deduped to one sentry, peers that negotiate eth/N on a different sentry end up as protocol=0/witProtocol=0 ghosts on the wit-hosting sentry. Counting them would emit a bogus eth.ProtocolToString[0] bucket in the GoodPeers log. The Peers() filter already drops them from admin_peers; this aligns SimplePeerCount. 6. awaitStatus logs a Debug line when its timeout fires (yperbasis #5) so operators can tell "core didn't send status in time" from "core never tried" when the caller disconnects the peer with PeerErrorLocalStatusNeeded. Doc comment clarifies the ctx.Done case too (yperbasis #7). Drops the reportsPeers flag and IsPeerReporter accessor — no longer needed once per-sentry goodPeers replaces the reporter gating. SetP2PServer signature loses its second argument. Tests updated for the new shape; new TestGrpcServer_PeersReturnsPerSentryGoodPeers (per-sentry view + ghost-entry filter) and TestGrpcServer_SetStatus_NilStatusReadyIsSafe (close-nil guard). Deferred to follow-up: - BootstrapNodes/DNS resolution helper to dedupe logic between makeP2PServer and startSharedP2PServer (yperbasis #6). - End-to-end Provider.Initialize test in local mode (yperbasis #8). - [r3.4] backport PR (yperbasis #9). Co-Authored-By: Claude
AskAlexSharov
pushed a commit
to HoustonOla35/erigon
that referenced
this pull request
May 22, 2026
…rgs) (erigontech#21211) ## Summary First incremental cut toward [erigontech#21138](erigontech#21138 structural goal: **one finalize function per parallel-exec result, with `IntraBlockState` used nowhere outside workers**. This PR removes two finalize variants that are already unreachable from production: | Function | LOC | Production callers on main | |---|---|---| | `finalizeWithIBS` (full IBS reconstruction, BAL-compat path) | ~120 | 0 | | `finalizeTx` (delta-args variant, direct fee-balance path) | ~250 | 0 (only `TestFinalizeTx_AllScenarios`) | Plus the test suite that exclusively exercised the delta-args path (`TestFinalizeTx_*`, fixture builders `coinbaseIsRecipientScenario` / `selfTransferScenario`, helpers `hasCoinbaseDelta` / `adjustForTransferDelta` / `buildWriteMap` / `fmtWriteVal` / `extractBalanceReads`) and one stale comment in `engine_api_bal_test.go`. Net: **-690 lines**, **+1 line**, no semantic change. ## Why now The parallel-exec correctness stack landed in erigontech#21153 (merged 2026-05-15). The combined effect of that PR plus erigontech#21177 routed all production finalize flows through `finalizeTxSimple` — these two functions became unreachable. Removing them shrinks `exec3_parallel.go` from 3640 → 3268 lines, making subsequent IBS-dependency drains easier to review. The next steps in the erigontech#21138 sequence: - **PR 2** — drain IBS dependency erigontech#1 (SD address lookup): `LogSelfDestructedAccounts` consumes `result.SelfDestructedWithBalance` only, no `ibs.GetRemovedAccountsWithBalance()` call. - **PR 3** — drain IBS deps erigontech#2 (`AddLog` → return logs) and erigontech#3 (`AddBalance` bookkeeping → already on `CollectorWrites`); `finalizeTxSimple` becomes IBS-free. - Later — `normalizeWriteSet` → `filterWritesByVersionMap`; `calcState.ApplyWrites` → `VersionedWrites.TouchUpdates`; move EIP-7002/7251 syscall execution into the worker pool. End state: one `finalizeTx`, no IBS outside workers. ## Test plan - [x] `make lint` clean - [x] `make test-short` (full `execution/stagedsync`, `execution/state`, `execution/tests`, `rpc/jsonrpc` packages) green under `EXEC3_PARALLEL=true` - [x] BAL family (`TestEngineApiBAL*`) 8/8 parallel - [x] `TestEIP7708BurnLogWhenCoinbaseSelfDestructs` green - [x] Surviving `TestFinalizeTxSimple_*` family green - [ ] CI: race-tests, kurtosis, hive matrix legs green on both serial and parallel ## Related - erigontech#21017 — serial/parallel CI matrix that surfaces parallel-leg failures (now rebuilt on post-erigontech#21153 main; CI fresh-running) - erigontech#21153 — parallel-exec correctness stack (merged) - erigontech#21138 — heuristic-removal / IBS-dependency-removal tracker (the parent)
mh0lt
pushed a commit
that referenced
this pull request
May 24, 2026
…ash LRU Two surgical commits bundled (both touch the code-read hot path): 1. IntraBlockState.GetCodeSize now loads the full bytes via stateReader.ReadAccountCode on first touch and populates stateObject.code, so subsequent same-addr EXTCODESIZE / EXTCODEHASH / CALL within the tx are in-struct slice-len calls (~50 ns), not full reader round-trips. Mirrors geth's pattern at core/state/state_object.go ~Code() — pay one read per addr per tx, free for the rest. 2. CodeCache.addrToHash switched from a no-op-when-full maphash.Map[versionedAddressID] to an LRU lru.Cache[[20]byte, versionedAddressID] (hashicorp/golang-lru/v2, already imported elsewhere). Cap derived from the existing byte budget at ~28 bytes/entry (~580 k entries for the 16 MB default). Fresh-address workloads (mainnet thousands of new addrs per block) now warm up the addr layer over time instead of silently dropping new entries forever; matches geth's lru.Cache at core/state/database_code.go. The hashToCode layer is unchanged (content-addressed bytes, immutable, byte-capped with new-entry no-op when full — the same semantic as before since code bytes by codeHash never change). Bench on the EXTCODESIZE-EXISTING_CONTRACT-30M family: 62.34 mgas/s (was 61.50). The marginal gain is small on this bench because BAL prefetch already populates the cache layers; neither lever fires heavily. The expected wins are on non-BAL workloads where EXTCODESIZE-loop patterns repeat within a tx (#1) and fresh-address-churn mainnet blocks fill the addr layer (#2). Updated TestCodeCache_AddrCapacityLimit to assert LRU eviction (was asserting no-op-when-full); the prior behaviour was the bug. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mh0lt
pushed a commit
that referenced
this pull request
May 25, 2026
…ash LRU Two surgical commits bundled (both touch the code-read hot path): 1. IntraBlockState.GetCodeSize now loads the full bytes via stateReader.ReadAccountCode on first touch and populates stateObject.code, so subsequent same-addr EXTCODESIZE / EXTCODEHASH / CALL within the tx are in-struct slice-len calls (~50 ns), not full reader round-trips. Mirrors geth's pattern at core/state/state_object.go ~Code() — pay one read per addr per tx, free for the rest. 2. CodeCache.addrToHash switched from a no-op-when-full maphash.Map[versionedAddressID] to an LRU lru.Cache[[20]byte, versionedAddressID] (hashicorp/golang-lru/v2, already imported elsewhere). Cap derived from the existing byte budget at ~28 bytes/entry (~580 k entries for the 16 MB default). Fresh-address workloads (mainnet thousands of new addrs per block) now warm up the addr layer over time instead of silently dropping new entries forever; matches geth's lru.Cache at core/state/database_code.go. The hashToCode layer is unchanged (content-addressed bytes, immutable, byte-capped with new-entry no-op when full — the same semantic as before since code bytes by codeHash never change). Bench on the EXTCODESIZE-EXISTING_CONTRACT-30M family: 62.34 mgas/s (was 61.50). The marginal gain is small on this bench because BAL prefetch already populates the cache layers; neither lever fires heavily. The expected wins are on non-BAL workloads where EXTCODESIZE-loop patterns repeat within a tx (#1) and fresh-address-churn mainnet blocks fill the addr layer (#2). Updated TestCodeCache_AddrCapacityLimit to assert LRU eviction (was asserting no-op-when-full); the prior behaviour was the bug. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Jun 16, 2026
Merged
Sahil-4555
pushed a commit
to Sahil-4555/erigon
that referenced
this pull request
Jun 17, 2026
This is **PR #1 of a 3-PR perf stack**. It consolidates state caching across all modes — sync, tip-tracking, integration and testing — so the state cache is either **on and completely trustworthy**, or **off to measure** — never off because it unexpectedly breaks things. The caches (the account/storage `StateCache` and the commitment `BranchCache`) are an **internal implementation detail of `SharedDomains`**. No external entity accesses or mutates them directly: callers drive state through `Flush` / `Commit` / `GetLatest` / `DomainPut`, and the full cache lifecycle (population, invalidation, commit-gating) is owned inside `SharedDomains`. ## What this PR contains ### `BranchCache` — single aggregator-scope commitment cache - Aggregator-lifetime cache: pinned root slot + bounded LRU tail, behind the `sd.mem` chain so unwinds and fork-validations see consistent state. - Wired into the trie read + encoder write paths. - Tx-precise unwind invalidation: entries are stamped with their per-key write `txNum`; `sd.Unwind` evicts everything above the unwind watermark (`BranchCache.UnwindTo`). ### One switch for all caches The `BranchCache` is a *type of* state cache, so it rides the existing `USE_STATE_CACHE` toggle rather than getting its own env. One operator switch turns **all** caching off — the relevant operation when bisecting a state-root mismatch, where an operator shouldn't have to reason about the interaction of several independent caches. (This is a deliberate deviation from the review's "add a separate `BranchCache` kill-switch" suggestion — flagged for confirmation.) ### The BranchCache reflects only committed state — by construction The BranchCache can never hold a value a failed commit rolled back: `SharedDomains.Commit` flushes the in-memory batch into the tx, commits, and **only then** applies the flushed commitment branches to the cache (the flush is implicit in committing; a failed commit applies nothing). Plain `Flush` — callers that own their own commit, e.g. offline tools — never touches the cache; read-through populates from committed files. The caches are an internal detail of `SharedDomains`; nothing else writes to them. > **StateCache no-poisoning is erigontech#21386, not this PR.** Unlike the BranchCache, the account/storage StateCache is an *in-flight, cross-transaction* cache — it holds prior txs' not-yet-committed writes within a batch, and later txs read them from it. So it can't be made commit-safe by simply invalidating on write (that breaks cross-tx reads in serial exec). Its no-poisoning is the txNum/epoch rework in erigontech#21386; this PR keeps its existing ValidateAndPrepare/unwind invalidation. ### BUG erigontech#21138 — parallel-exec from-0 wrong trie root `ResetExec` wipes the commitment DB table; the aggregator's in-memory `BranchCache` could still reference the just-deleted trie nodes, so a from-0 re-exec served stale entries when computing block 0's commitment → wrong root, dropping genesis-allocated balances no later block touched (mainnet block 46147, `0xA1E4380A3B1f749673E270229993eE55F35663b4`). Fix: `ResetExec` clears the aggregator's `BranchCache`. `TestFromZero_GenesisAllocPreservedAfterResetReExec` passes on current `main`; the test's value here is keeping *this PR's* cache safe across reset, not fixing a live `main` bug. ## Follow-ups (the rest of the stack) - **erigontech#21386 (PR erigontech#2 of the stack) — StateCache LRU + Mode rework:** consistency + no performance drop-off at a 1 GB cache for long-running nodes; re-adds the warm StateCache repopulation deferred above, under its `txNum`/`epoch` model. - **Pinning** (the stack's third step) — **no PR yet**; in progress and under test on branch [`mh/branch-cache-trunk-pin`](https://github.com/erigontech/erigon/tree/mh/branch-cache-trunk-pin), to be **re-benchmarked before merge**. - **erigontech#21739** — interface-unification follow-up: collapse the duck-typed `GetLatest` variants into a single metered, `txNum`-returning `GetLatest`. ## Testing Behaves identically across parallel and serial exec — confirmed in CI across both exec modes. Unit coverage for the BranchCache (tiers, tx-precise `UnwindTo`, commit-gated population) plus the engine/exec-module FCU commit paths. Given today's changes (the commit-gating of both caches), we will do another A/B performance run before merging. --------- Co-authored-by: Mark Holt <erigon@dev-bm-e3-ethmainnet-n4.erigon.io> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
yperbasis
pushed a commit
that referenced
this pull request
Jun 17, 2026
…o-op execution unwind (#21847) Follow-up to #21824, addressing review feedback from @yperbasis and @AskAlexSharov. ## The gap in #21824 #21824 made `UnwindExecutionStage`'s no-op-disk-unwind branch prune the in-RAM overlay, but to `Min(u.UnwindPoint+1)`. That branch deliberately does **not** call `u.Done`, so re-execution resumes from the *committed* progress (`doms.SeekCommitment == s.BlockNumber+1`), **not** `u.UnwindPoint+1`. When committed progress sits below `u.UnwindPoint` — common with `BatchCommitments` (commitment is persisted on a ~20s timer or when the batch fills), so `s.BlockNumber` can be several blocks behind — the overlay writes for blocks `(s.BlockNumber, u.UnwindPoint]` are **kept** but then re-executed from `s.BlockNumber+1`, where they re-read their own stale overlay: the same bug class, just deferred. It converges (the unwind point walks back to the committed block, then the prune clears everything) instead of looping forever, but it wastes a partial re-execution and emits spurious `gas used mismatch` warnings for blocks that aren't actually bad. For the original Hoodi 3004265 incident `s.BlockNumber == u.UnwindPoint` (one block since the last commit), so the two boundaries coincided and #21824 worked. ## This PR - **Prune to the committed boundary** (`Min(s.BlockNumber+1)`) instead of `Min(u.UnwindPoint+1)`. Clears all uncommitted overlay in one step, matches the resume point, can't over-prune (committed state is in the DB). - **Restructure** `UnwindExecutionStage` into an explicit `disk-unwind` / `overlay-only` `if`/`else` with a shared `SeekCommitment` tail, so the no-op case is no longer a bare early-return that buries logic / risks skipping future shared steps (@AskAlexSharov). This also drops the redundant `SetTxNum` (@yperbasis nit — `SeekCommitment` sets it) and documents **why** the no-op path must not call `u.Done`, plus the state-cache caveat (@yperbasis #2: not a hole today — executors clear the cache per-block via `ValidateAndPrepare` and the snapshotter attaches no cache — but the path relies on that implicit clear). - **Test**: now asserts the `(s.BlockNumber, u.UnwindPoint]` write is pruned too, not just the failed block's; a write at/below committed still survives. ## Verification - red→green: with the merged `Min(u.UnwindPoint+1)` boundary the new test fails (the `(committed, unwindPoint]` write `0x1122` survives); with this PR it passes. - `go build ./execution/stagedsync/...` — clean - `go test ./execution/stagedsync/ -run 'TestUnwindExecutionStage_PrunesUncommittedOverlayWrite|TestFindExecutedDiffsetAtHeight'` — pass - `gofmt` / `golangci-lint run ./execution/stagedsync/` — 0 issues Relates to #21681. Will be forward-ported to `main` and `release/3.5` (which carry #21825 / #21826). Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: JkLondon <me@ilyamikheev.com> Co-authored-by: Claude <noreply@anthropic.com>
yperbasis
pushed a commit
that referenced
this pull request
Jun 17, 2026
…o-op execution unwind (#21849) Cherry-pick of #21847 (release/3.4) / #21848 (main) to `release/3.5` (which carries #21826). Same change: in `UnwindExecutionStage`, prune the in-RAM overlay to the **committed** boundary (`Min(s.BlockNumber+1)`) on the no-op-disk-unwind path instead of `Min(u.UnwindPoint+1)`, and restructure into an explicit `disk-unwind` / `overlay-only` `if`/`else` with a shared tail. Full rationale in #21847 (yperbasis #1 boundary, AskAlexSharov structure, yperbasis #2 state-cache note). Clean cherry-pick of the `main` commit — `release/3.5` matches `main` here (it has `doms.ResetPendingUpdates()`, which the restructure hoists ahead of the branch so it runs on both paths; the test uses `[:]` slicing as `common.Hash`/ `common.Address` are raw arrays). ## Verification on release/3.5 + this patch - red→green: with the merged `Min(u.UnwindPoint+1)` boundary the test fails (the `(committed, unwindPoint]` write `0x1122` survives); with this PR it passes. - `go build ./execution/stagedsync/...` — clean - `go test ./execution/stagedsync/ -run TestUnwindExecutionStage_PrunesUncommittedOverlayWrite` — pass - `gofmt` / `golangci-lint run ./execution/stagedsync/` — 0 issues Relates to #21681. Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: JkLondon <me@ilyamikheev.com> Co-authored-by: Claude <noreply@anthropic.com>
Sahil-4555
pushed a commit
to Sahil-4555/erigon
that referenced
this pull request
Jun 20, 2026
…ecution unwind (erigontech#21848) Forward-port of erigontech#21847 to `main` (which carries erigontech#21825). Same change as erigontech#21847: in `UnwindExecutionStage`, prune the in-RAM overlay to the **committed** boundary (`Min(s.BlockNumber+1)`) on the no-op-disk-unwind path instead of `Min(u.UnwindPoint+1)`, and restructure into an explicit `disk-unwind` / `overlay-only` `if`/`else` with a shared tail. See erigontech#21847 for the full rationale (yperbasis #1 boundary, AskAlexSharov structure, yperbasis erigontech#2 state-cache note). ## main-specific adaptation (and a latent bug this surfaced) `main`'s disk-unwind path calls `doms.ResetPendingUpdates()` (discards deferred commitment updates from the failed execution — `release/3.4` has no such call). The pre-refactor `u.UnwindPoint >= s.BlockNumber` **early return skipped it** — exactly the "the early return skips important biz-logic" failure mode @AskAlexSharov flagged. The restructure **hoists `ResetPendingUpdates()` ahead of the branch so it runs on both paths**, so a block that failed its gas check mid-batch no longer leaks deferred commitment updates into the re-execution. The test file is `main`-only here (the release/3.4 `TestFindExecutedDiffsetAtHeight…` test was never forward-ported), and `common.Address`/`common.Hash` are raw arrays on `main`, so the test slices them with `[:]`. ## Verification on main + this patch - red→green: with the merged `Min(u.UnwindPoint+1)` boundary the test fails (the `(committed, unwindPoint]` write `0x1122` survives); with this PR it passes. - `go build ./execution/stagedsync/...` — clean - `go test ./execution/stagedsync/ -run TestUnwindExecutionStage_PrunesUncommittedOverlayWrite` — pass - `gofmt` / `golangci-lint run ./execution/stagedsync/` — 0 issues Relates to erigontech#21681. Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: JkLondon <me@ilyamikheev.com> Co-authored-by: Claude <noreply@anthropic.com>
mh0lt
added a commit
that referenced
this pull request
Jul 2, 2026
…view) Addresses yperbasis's blocking review on the typed-vio PR. SetAccountBalanceOrDelete had collapsed main's three-case branch to two: when an address had a non-balance write but no balance write, it re-emitted the full account (Balance+Nonce+Incarnation+CodeHash) from the pre-block snapshot, clobbering the worker's already-written Nonce/CodeHash under the last-wins merge (the #21017 "bug #2" — a miner self-send loses its nonce bump). Restore the middle case: when the address already has any write, append only Balance. Re-add the tests that pin this, ported to the typed WriteSet/VersionMap API: - versionedio_test.go: TestSetAccountBalanceOrDelete_{Nonce,Incarnation, CodeHash}PathOnly_AppendBalanceNotFullAccount. - versionmap_test.go: TestBALPrePop_SameSenderTxs_NoConflicts and TestNoBAL_SameSenderTxs_DetectsConflicts (same-sender BAL conflict detection). Also restore two dropped assertions the review flagged: readFloor panics on an unknown cell flag, and AnyDoneBoolWriteEquals panics if called with a non- SelfDestruct path (it ignores the path argument and only reads SelfDestruct).
Sahil-4555
pushed a commit
to Sahil-4555/erigon
that referenced
this pull request
Jul 9, 2026
…p window (erigontech#21611) ## Problem `go test -race` on darwin (Apple Silicon) flakily dies in mdbx-heavy packages — reproduced at **6/6** on `main` with `EXEC3_PARALLEL=true go test -race ./execution/execmodule ./execution/state` on an M-series Mac — with either of: ``` fatal error: runtime: split stack overflow (sigpanic → racecall, no DATA RACE report) fatal error: too many address space collisions for -race mode ``` The same packages pass the Linux race CI legs, which long disguised this as an environment flake. It isn't — both fatals share one root cause. ## Root cause (caught live in lldb) Catching the original fault under lldb (before the Go runtime mangles it into "split stack overflow") shows: ``` thread erigontech#26, stop reason = EXC_BAD_ACCESS (code=1, address=0x21882000bbb0) frame #0: __tsan_read + 44 frame erigontech#2: txnprovider/txpool.(*TxPool).fromDB (lldb) memory region `($x1 - 0x200000000000)/2` ← app address for that shadow [0x000000c410004000-0x000000c810000000) r-- ← a 16GiB mdbx data map (lldb) memory region $x1 ← its TSAN shadow [0x21884800bbb0-0x219a00000000) --- ← unmapped ``` 1. Go's race-mode heap lives in TSAN's Go/darwin window `[0x00c0…, 0x00e0…)`; shadow (`shadow = app*2 + 0x2000_0000_0000`) is mapped **per heap arena**. 2. Each in-mem test env reserves a **16GiB** VA map (`InMem` geometry upper bound); with dozens of parallel testers the kernel's bottom-up placement exhausts low VA and drops mdbx maps **between Go heap arenas**. 3. The runtime's `racecalladdr` validity filter checks one coarse interval `[racearenastart, racearenaend)` (min/max over arenas) — a sandwiched map passes the check with no shadow → the first instrumented read (txpool `fromDB`) faults inside `__tsan_read`. The SEGV lands while `racecall` is on the g0 stack, so the runtime dies with the misleading split-stack throw. The same squatting also makes heap-arena reservation collide repeatedly → the "too many address space collisions" fatal. Linux is unaffected because `mmap(NULL)` there places file maps near `0x7f…`, far from the heap window, so they always fail the `racecalladdr` filter and are simply (silently) invisible to TSAN. Two repair strategies were tried and rejected with evidence before the final one: - **Calling the runtime's own `__tsan_map_shadow` per mapping** verifiably does nothing here: compiler-rt's Go-mode `MapShadow` tracks a monotonic `ctx->mapped_shadow_*` interval and silently `return`s for requests inside it — interior holes (precisely this case) are skipped. Confirmed in compiler-rt source and empirically (crash inside a region the call had "covered"). - **Direct shadow mmaps per mdbx env / per `unix.Mmap`**, locating regions via `mach_vm_region` + `proc_regionfilename`, fixed the crash but serialized an O(all-VM-regions) walk into every env open — `execution/tests` (thousands of env opens) went from 9 minutes to a 1h timeout. ## Fix `common/race` (linked via blank imports from `db/kv/mdbx` and `common/mmap`; everything is compiled out unless `race && darwin`): - At init, fill every unmapped gap in the heap window's shadow `[app*2+0x2000…, …)` with zeroed `MAP_FIXED|MAP_ANON|MAP_NORESERVE` mappings, leaving existing arena shadow untouched. One-time cost of a few mmaps; zero per-open cost; covers every file mapping the kernel ever places in the window — current or future, mdbx or otherwise. Zero shadow is valid "no prior access" TSAN state, so races on such mappings also become *detectable* where they were previously fatal — for plain reads/writes; Go atomics on such mappings would still fault, since meta shadow is not pre-mapped. - The hard-coded layout (window bounds + shadow formula) is self-checked at init against a live heap allocation (it must be in-window with mapped shadow); on mismatch the package disables itself with a warning, restoring old behavior. Plus: `InMem` test geometry **16GiB → 1GiB** upper bound (only when a `testing.TB` is supplied, and not for benchmarks, which run sequentially and can need the full map). Unit tests never approach 1GiB per env; this removes the TB-scale VA squatting that pushes file maps into the heap window in the first place and is what triggers the "address space collisions" fatal. ## Verification - Repro rate of `EXEC3_PARALLEL=true go test -race ./execution/execmodule ./execution/state` (sequential, quiet M-series machine): `main` **6/6 fatal** → this branch **0/8**. - `EXEC3_PARALLEL=true go test -race ./execution/tests` completes in normal time (the rejected per-open design hung it — kept as a regression gate). - `go test -race ./common/race`: asserts the layout self-check and that shadow is actually mapped across the whole window. - Full `EXEC3_PARALLEL=true go test -race ./execution/...` green — re-verified after merging main (2026-07-08, `-count=1`: 52 ok / 0 fail); `make lint` clean; `make erigon integration` builds; stubs cross-compile (linux/windows). - The post-merge sweep initially caught `./execution/engineapi` dying **6/6** with the "address space collisions" fatal — a sibling mechanism, not a regression of this fix: the txpool DB's hard-coded **1TB** geometry (`txnprovider/txpool/assemble.go`) cannot fit below the 768GiB heap-window base on darwin, so each tester node's txpool map spanned the window (caught live in vmmap: two 1TB `txpool/mdbx.dat` maps starting 320MB above the window base) and burned the runtime's 32 race-mode arena hints, which are discarded permanently on collision; the fatal then hits a later innocent allocation. Fixed by flowing the tester's existing 1GB `MdbxDBSizeLimit` into the txpool config: **6/6 fatal → 6/6 green**. Linux is unaffected (top-down mmap places the reservation near `0x7f…`). TDD note: natural occurrence depends on kernel VM placement (hence the flakiness), but the crash is deterministically reproducible — golang/go#80292 carries a standalone ~50-line reproducer (mmap with an address hint into the heap window + heap ballast + one instrumented read, 100% fatal). In-repo, a deterministic crash test would need a separate crashing subprocess (the fault kills the whole test binary), so the unit test pins the fix's load-bearing properties (layout constants, window coverage) and the repetition harness above is the end-to-end gate; each design iteration was validated live in lldb. This is arguably a Go runtime/TSAN deficiency (coarse interval in `racecalladdr`, interior-skip in Go-mode `MapShadow`) — reported upstream as golang/go#80292 with a deterministic standalone reproducer; this PR makes erigon's macOS race runs work today. Forensics trail: first reported as a suspected environment flake on erigontech#21605 (erigontech#21605 (comment)).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.