db/state: restore empty tombstones on unwind and honor them in getLatestFromDb (#20627)

## Summary

Re-land the narrow core of #20483 (reverted by #20509), addressing the
DB-layer siblings of the post-unwind stale-read bug. Complements #20625
which addressed the in-memory overlay side.

Two linked bugs in the DB-level domain unwind and read paths caused
stale data resurrection after an unwind that reverted a first-time write
or a deletion inside the unwound range.

### Symptom observed on mainnet

Post-Fusaka mainnet catch-up sync with a chaindata/ wipe (snapshots/
retained). On the first re-executed block after a forkchoice-driven
unwind, execution returned less gas than the header — diffs observed of
`-34200` (block 24,898,955), `-73829` (block 24,899,403), `-118872`
(block 24,899,594). The diffs break down into multiples of `SSTORE_SET -
SSTORE_RESET = 17100` plus cold-access flips of `2600`.

Previous PR #20625 cleared the first two by pruning the in-memory
overlay on Unwind. Block 24,899,594 still failed because the overlay was
already flushed to DB at Unwind time — the stale-read path now was
purely DB-layer, addressed here.

## Fixes

### 1. `unwind()` must restore empty tombstones —
`db/state/domain.go:1317` (both DupSort and LargeValues paths)

`DomainEntryDiff.Value` has three shapes, documented in
`db/kv/helpers.go:247`:
- `nil` — "different step": prev value lives at another step, skip
restore (legacy V0 changeset shape)
- `[]byte{}` — "no previous value": key was absent before this step;
write an empty tombstone so the key appears absent again after the
unwind completes
- non-empty — restore the actual previous value

The old guard `if len(value) > 0` skipped *both* `nil` and `[]byte{}`,
leaving no tombstone after unwinding a first-time write. Corrected to
`if value != nil`.

### 2. `getLatestFromDb` must treat empty values as authoritative —
`db/state/domain.go:1665`

Empty-value entries are deletion tombstones. The step-age guard
previously discarded them when their step fell within the frozen file
range, causing the caller to fall through to `getLatestFromFiles`.
Frozen files encode deletions as absence, so the file returns the
pre-deletion value — the exact resurrection the deletion was meant to
prevent. Empty entries are now returned as `found=true` regardless of
step age; the step-age guard still applies to non-empty entries.

## Relationship to #20483 / #20509

This is a deliberate re-land of the narrow core of #20483. Key
differences:

- **Excluded**: the LargeValues cross-check in `getLatest` (PR #20483
lines 1697–1737). That handled an interrupted-`PruneSmallBatches` edge
case specific to `CodeDomain` / `RCacheDomain` and was the most likely
source of the regressions that motivated #20509's revert. If it proves
necessary, it can be added later as a separate PR with its own dedicated
test.
- **Included**: both matched fixes (write-side tombstone + read-side
authoritativeness). They are a pair — neither is useful alone; staging
them separately risks merging half and shipping a version that's still
broken.

## Tests

- `TestDomain_UnwindRestoresDeletionMarker` (DupSort + LargeValues
subtests) — writes a key, deletes it, re-writes within the same step,
builds files, then unwinds the re-write. **Fails on pre-fix code**
(getLatest returns the stale post-unwind `value2` from the frozen file);
passes with the fix. Exercises the write-side bug directly.

- `TestDomain_DeletedKeyNotResurrectedByFiles` (DupSort + LargeValues
subtests) — documents the read-side contract by writing a key and
deleting it at a step that falls within file range. Passes on current
`main` even without the fix (the file-build + prune semantics evolved
since #20483 and no longer hit the specific stale-read in this exact
test scenario), but retained as a forward regression guard and as
documentation of the invariant.

## Test plan

- [x] `go test -short ./db/state/...` — all pass
- [x] `make lint` — 0 issues
- [x] `make erigon` — builds clean
- [x] Manual repro of the production symptom (mainnet sync from
snapshots-only) in combination with #20625 — sync progresses past the
catch-up / first-forkchoice-unwind window without a gas mismatch.
(Re-verification run in progress alongside this PR.)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: mh0lt

db/state/domain.go

@@ -1292,7 +1292,14 @@ func (dt *DomainRoTx) unwind(ctx context.Context, rwTx kv.RwTx, step, txNumUnwin
 	defer valsCursor.Close()
 	// Revert keys using diff entries.
 	// Always: delete current entry at the write step, restore prevValue at unwind target step.
-	// value == []byte{} means key was new (no previous value to restore).
+	//
+	// DomainEntryDiff.Value semantics:
+	//   - nil            → "different step" — prev value lives at another step, skip the restore
+	//                       (only produced by legacy V0 changesets where valueLen==0 deserializes as nil)
+	//   - []byte{}       → "no previous value" — key was absent before this step, so write an
+	//                       empty tombstone to prevent getLatestFromDb falling through to files
+	//                       (which have no concept of deletions) and returning stale data
+	//   - non-empty      → restore the actual previous value
 	//
 	// The step tag for restored entries must be BEYOND the filed range, otherwise
 	// getLatestFromDb will discard them (step covered by files → fall through to
@@ -1313,8 +1320,8 @@ func (dt *DomainRoTx) unwind(ctx context.Context, rwTx kv.RwTx, step, txNumUnwin
 			if err := rwTx.Delete(d.ValuesTable, key); err != nil {
 				return err
 			}
-			// Restore previous value at unwind step ([]byte{} = key was new, nothing to restore)
-			if len(value) > 0 {
+			// nil = different step, skip; []byte{} = absent previously, write empty tombstone
+			if value != nil {
 				fullKey := key[:len(key)-8]
 				if err := rwTx.Put(d.ValuesTable, append(fullKey, unwindStepBytes...), value); err != nil {
 					return err
@@ -1338,8 +1345,8 @@ func (dt *DomainRoTx) unwind(ctx context.Context, rwTx kv.RwTx, step, txNumUnwin
 			}
 		}
 
-		// Restore previous value at unwind step ([]byte{} = key was new, nothing to restore)
-		if len(value) > 0 {
+		// nil = different step, skip; []byte{} = absent previously, write empty tombstone
+		if value != nil {
 			if err := valsCursor.Put(fullKey, append(unwindStepBytes, value...)); err != nil {
 				return err
 			}

db/state/domain_test.go

@@ -3541,3 +3541,190 @@ func TestDomain_KeyPosResetOnCollisionRetry(t *testing.T) {
 		require.Equal(t, expected[:], val, "key %x value mismatch", k)
 	}
 }
+
+// TestDomain_DeletedKeyNotResurrectedByFiles is a regression test for the bug
+// where getLatestFromDb discards deletion entries whose step is already covered
+// by frozen files. Since frozen files lack tombstones, the fallthrough to
+// getLatestFromFiles returns a stale pre-deletion value.
+func TestDomain_DeletedKeyNotResurrectedByFiles(t *testing.T) {
+	if testing.Short() {
+		t.Skip()
+	}
+
+	t.Parallel()
+
+	// Cover both DupSort (LargeValues=false) and non-DupSort (LargeValues=true) paths.
+	for _, tc := range []struct {
+		name      string
+		domainCfg statecfg.DomainCfg
+	}{
+		{"DupSort", statecfg.Schema.AccountsDomain},
+		{"LargeValues", statecfg.Schema.CodeDomain},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			logger := log.New()
+			db, d := testDbAndDomainOfStep(t, tc.domainCfg, 16, logger)
+			ctx := context.Background()
+			require := require.New(t)
+
+			tx, err := db.BeginRw(ctx)
+			require.NoError(err)
+			defer tx.Rollback()
+
+			domainRoTx := d.beginForTests()
+			defer domainRoTx.Close()
+			writer := domainRoTx.NewWriter()
+			defer writer.Close()
+
+			key := []byte("key1")
+			value := []byte("value1")
+
+			// Step 0 (txNum 2): write key1=value1
+			err = writer.PutWithPrev(key, value, 2, nil)
+			require.NoError(err)
+
+			// Step 1 (txNum 18): delete key1
+			err = writer.DeleteWithPrev(key, 18, value)
+			require.NoError(err)
+
+			err = writer.Flush(ctx, tx)
+			require.NoError(err)
+			domainRoTx.Close()
+
+			// Build files covering steps 0 and 1 without pruning DB entries.
+			// After this, files.EndTxNum() = 32 so the step-age guard considers
+			// step 1 "already covered by files" and would discard the deletion entry.
+			require.NoError(d.collateBuildIntegrate(ctx, 0, tx, background.NewProgressSet()))
+			require.NoError(d.collateBuildIntegrate(ctx, 1, tx, background.NewProgressSet()))
+
+			require.NoError(tx.Commit())
+
+			roTx, err := db.BeginRo(ctx)
+			require.NoError(err)
+			defer roTx.Rollback()
+
+			domainRoTx = d.beginForTests()
+			defer domainRoTx.Close()
+
+			v, _, found, err := domainRoTx.GetLatest(key, roTx)
+			require.NoError(err)
+			// The key was deleted at step 1. The deletion entry in DB must be
+			// authoritative even though step 1 is covered by frozen files.
+			// Without the fix, getLatestFromDb discards the deletion entry and
+			// getLatestFromFiles returns stale "value1" from the step 0 file.
+			require.True(found, "deletion entry should be found in DB")
+			require.Empty(v, "deleted key should have empty value, got %q", v)
+		})
+	}
+}
+
+// TestDomain_UnwindRestoresDeletionMarker is a regression test for the bug
+// where unwind() fails to restore empty tombstones. When a slot is deleted
+// then re-written within the same step, unwinding the re-write must restore
+// the deletion marker so that getLatestFromFiles doesn't return the stale
+// pre-unwind value from frozen files.
+func TestDomain_UnwindRestoresDeletionMarker(t *testing.T) {
+	if testing.Short() {
+		t.Skip()
+	}
+
+	t.Parallel()
+
+	// Cover both DupSort (LargeValues=false) and non-DupSort (LargeValues=true) paths.
+	for _, tc := range []struct {
+		name      string
+		domainCfg statecfg.DomainCfg
+	}{
+		{"DupSort", statecfg.Schema.AccountsDomain},
+		{"LargeValues", statecfg.Schema.CodeDomain},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			logger := log.New()
+			db, d := testDbAndDomainOfStep(t, tc.domainCfg, 16, logger)
+			ctx := context.Background()
+			require := require.New(t)
+
+			// Phase 1: Write key1 through three states within step 0 and record diffs.
+			tx, err := db.BeginRw(ctx)
+			require.NoError(err)
+			defer tx.Rollback()
+
+			domainRoTx := d.beginForTests()
+			defer domainRoTx.Close()
+			writer := domainRoTx.NewWriter()
+			defer writer.Close()
+
+			key := []byte("key1")
+			value1 := []byte("value1")
+			value2 := []byte("value2")
+
+			// txNum 0: write key1=value1 (new key, prev=nil)
+			writer.diff = &kv.DomainDiff{}
+			err = writer.PutWithPrev(key, value1, 0, nil)
+			require.NoError(err)
+
+			// txNum 1: delete key1 (prev=value1)
+			writer.diff = &kv.DomainDiff{}
+			err = writer.DeleteWithPrev(key, 1, value1)
+			require.NoError(err)
+
+			// txNum 2: re-write key1=value2 (prev=nil, key was deleted)
+			// Only this diff is needed for the unwind — it captures the previous
+			// state (deleted → Value=[]byte{}) that unwind must restore.
+			writer.diff = &kv.DomainDiff{}
+			err = writer.PutWithPrev(key, value2, 2, nil)
+			require.NoError(err)
+			txNum2Diff := writer.diff.GetDiffSet()
+
+			// Verify the nil-vs-empty distinction survives serialization round-trip.
+			// Production diffs go through SerializeDiffSet/DeserializeDiffSet when
+			// stored in ChangeSets3; this ensures the []byte{} tombstone is preserved.
+			txNum2Diff = changeset.DeserializeDiffSet(changeset.SerializeDiffSet(txNum2Diff, nil))
+
+			err = writer.Flush(ctx, tx)
+			require.NoError(err)
+			domainRoTx.Close()
+
+			// Phase 2: Build files for step 0. The file will contain key1=value2
+			// (the latest value within step 0).
+			require.NoError(d.collateBuildIntegrate(ctx, 0, tx, background.NewProgressSet()))
+			require.NoError(tx.Commit())
+
+			// Phase 3: Unwind to revert txNum 2 (keep txNums 0 and 1).
+			tx, err = db.BeginRw(ctx)
+			require.NoError(err)
+			defer tx.Rollback()
+
+			domainRoTx = d.beginForTests()
+			defer domainRoTx.Close()
+
+			// diff for txNum 2: prev was empty (key was deleted) → Value=[]byte{}
+			err = domainRoTx.unwind(ctx, tx, 0, 2, 1, txNum2Diff)
+			require.NoError(err)
+			domainRoTx.Close()
+			require.NoError(tx.Commit())
+
+			// Phase 4: Verify GetLatest returns empty (deletion marker restored).
+			roTx, err := db.BeginRo(ctx)
+			require.NoError(err)
+			defer roTx.Rollback()
+
+			domainRoTx = d.beginForTests()
+			defer domainRoTx.Close()
+
+			v, _, found, err := domainRoTx.GetLatest(key, roTx)
+			require.NoError(err)
+			// After unwinding txNum 2, the state should reflect txNums 0-1.
+			// At txNum 1, key1 was deleted. The unwind must restore the deletion
+			// marker (empty tombstone) in DB. Without the fix, `if len(value) > 0`
+			// skips restoring the empty tombstone, and getLatestFromFiles returns
+			// stale "value2" from the step 0 file.
+			require.True(found, "deletion marker should be found after unwind")
+			require.Empty(v, "deleted key should have empty value after unwind, got %q", v)
+		})
+	}
+}