Skip to content

db/state: invalidate BranchCache entries on Flush to fix stale trie reads - #21885

Open
erigon-copilot[bot] wants to merge 1 commit into
mainfrom
erigon-copilot/fix-branch-cache-flush-stale
Open

erigon-copilot[bot] wants to merge 1 commit into
mainfrom
erigon-copilot/fix-branch-cache-flush-stale

Conversation

@erigon-copilot

Copy link
Copy Markdown
Contributor

Summary

  • Fixes wrong trie root regression in stage-exec-test (from-0, parallel) at block 263641 and (from-0, serial) at block 513814
  • Root cause: SharedDomains.Flush() did not invalidate the BranchCache, so the integration-path from-0 loop (Flush + ClearRam + tx.Commit()) left stale commitment branches in the cache across batch boundaries
  • Fix: Flush() now uses flushMemWithCallback to invalidate every commitment-domain key it writes, forcing subsequent reads to fall through to MDBX

Root cause

The BranchCache introduced in #21380 is checked before MDBX in GetLatest for CommitmentDomain. Commit() updates the cache after a successful commit, but Flush() — used by the integration stage_exec from-0 loop — left the cache untouched. When the same commitment branch was read (cached), updated, flushed, and then read again in the next batch, GetLatest returned the stale cached value instead of the freshly committed DB value.

Test plan

  • TestBranchCacheFlushInvalidatesStaleEntries — new test that reproduces the exact stale-cache scenario; confirmed FAILS without the fix, PASSES with it
  • TestBranchCacheCommitRefreshesAfterReadThrough — existing test, still passes
  • TestFromZero_GenesisAllocPreservedAfterResetReExec — passes (serial + parallel)
  • All db/state/execctx tests pass with -count=3 -race
  • All execution/commitment tests pass with -count=3 -race
  • make lint clean
  • make erigon integration builds clean

…eads

Flush() did not touch the BranchCache, so the from-0 integration-path
loop (Flush + ClearRam + tx.Commit) left stale commitment branches in
the cache across batch boundaries. Subsequent reads hit the cache
instead of MDBX, returning outdated branch nodes and producing wrong
trie roots at blocks 263641 (parallel) / 513814 (serial).

Flush now uses flushMemWithCallback to invalidate every commitment key
it writes, forcing the next read to fall through to the DB.

Co-authored-by: Giulio Rebuffo <giulio.rebuffo@gmail.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants