fix: prevent seed state cache poisoning in loadState clone (#9246)

## Summary

Fixes the v1.42.0 "Withdrawal mismatch at index=0" regression by
changing the two `clone()` calls in `loadState.ts` to `clone(true)`
(i.e. `dontTransferCache=true`).

The default `clone()` in `@chainsafe/ssz` transfers the source
sub-view's cache to the new instance, which means the
`migratedState.validators` sub-view and the seed container's cached
child-view snapshot share the **same** internal `nodes[]` and `caches[]`
arrays. A subsequent `migratedState.commit()` writes modified validator
/ inactivity-score nodes into those shared arrays, silently corrupting
the seed state's cache snapshot at the modified indices.

The corruption stays latent until the seed state is later cloned with
transfer-cache enabled — the path `verifyBlock` takes via
`preState.clone({dontTransferCache: false})`. On that next-block clone,
reads at the modified index return the migrated state's validator
instead of the seed's, which surfaces as `Withdrawal mismatch at
index=0` divergences between Lodestar and EL.

### Timeline of the corruption

```
// Inside loadState() with seedState = head state:
migratedState.validators = seedState.validators.clone();
// ^ migrated.validators.nodes === seedState.caches[validatorsIndex].nodes (shared)

for (const i of modifiedValidators) {
  migratedState.validators.set(i, loadValidator(...));  // staged in viewsChanged
}

migratedState.commit();
// ^ arrayComposite.js commit(): `this.nodes[index] = node`
//   writes newValidator into the SHARED nodes[] array.
//   seedState.caches[validatorsIndex].nodes[modifiedIndex] is now poisoned.
```

On the next block:

```
const preState = headState.clone();  // default dontTransferCache=false
// ^ transfers the poisoned caches[] snapshot to preState
preState.validators.getReadonly(i);  // returns migrated validator, not seed's
// -> getExpectedWithdrawals reads wrong validator at index 0
// -> "Withdrawal mismatch at index=0"
```

### Root cause

Introduced by #8857 (`chore: consume BeaconStateView`) which added the
`loadOtherState` / shared-head seed path that exercises this clone in
production.

## Test plan

- [x] New regression test `loadState does not poison seed state's cache`
in `packages/state-transition/test/unit/util/loadState.test.ts`
- [x] Verified the test FAILS without the fix (reads `0xaa`-filled
validator on `postState.clone()`) and PASSES with the fix
- [x] All 176 existing `state-transition` util tests pass
- [x] `check-types` and `lint` clean

## Relation to #9245

PR #9245 (`fix: gate loadOtherState validators/balances preload behind
opt-in`) addresses a different regression from the same #8857-era
changes — the eager `getAllReadonlyValues()` preload causing memory
spikes on the API path. The two fixes are **independent and both
needed**:

- #9245 fixes the API-path memory spike.
- This PR fixes the correctness bug (withdrawal mismatch). Notably,
`persistentCheckpointsCache` also exercises `loadState()` with the
problematic `clone()`, so #9245's preload gating alone doesn't cover the
full surface.

🤖 Generated with AI assistance

---------

Co-authored-by: lodekeeper <lodekeeper@users.noreply.github.com>
Co-authored-by: Nico Flaig <nflaig@protonmail.com>

Author: 3 people

packages/state-transition/src/util/loadState/loadState.ts

@@ -110,8 +110,8 @@ function loadInactivityScores(
   seedState: BeaconStateAltair,
   inactivityScoresBytes: Uint8Array
 ): void {
-  // migratedState starts with the same inactivityScores to seed state
-  migratedState.inactivityScores = seedState.inactivityScores.clone();
+  // true = do not transfer cache
+  migratedState.inactivityScores = seedState.inactivityScores.clone(true);
   const oldValidator = migratedState.inactivityScores.length;
   // UintNum64 = 8 bytes
   const newValidator = inactivityScoresBytes.length / 8;
@@ -187,8 +187,8 @@ function loadValidators(
   const newValidatorCount = Math.floor(newValidatorsBytes.length / VALIDATOR_BYTES_SIZE);
   const isMoreValidator = newValidatorCount >= seedValidatorCount;
   const minValidatorCount = Math.min(seedValidatorCount, newValidatorCount);
-  // migrated state starts with the same validators to seed state
-  migratedState.validators = seedState.validators.clone();
+  // true = do not transfer cache
+  migratedState.validators = seedState.validators.clone(true);
   // 80% of validators serialization time comes from memory allocation
   // seedStateValidatorsBytes is an optimization at beacon-node side to avoid memory allocation here
   const seedValidatorsBytes = seedStateValidatorsBytes ?? seedState.validators.serialize();

packages/state-transition/test/unit/util/loadState.test.ts

@@ -3,7 +3,8 @@ import {createChainForkConfig} from "@lodestar/config";
 import {mainnetChainConfig} from "@lodestar/config/networks";
 import {ForkName, SLOTS_PER_EPOCH} from "@lodestar/params";
 import {ssz} from "@lodestar/types";
-import {loadStateAndValidators} from "../../../src/util/loadState/loadState.js";
+import {BeaconStateAltair} from "../../../src/types.js";
+import {loadState, loadStateAndValidators} from "../../../src/util/loadState/loadState.js";
 
 describe("loadStateAndValidators", () => {
   const numValidator = 10;
@@ -22,7 +23,7 @@ describe("loadStateAndValidators", () => {
       state.slot = slot;
       for (let i = 0; i < numValidator; i++) {
         const validator = ssz.phase0.Validator.defaultViewDU();
-        validator.pubkey = Buffer.alloc(48, i);
+        validator.pubkey = new Uint8Array(48).fill(i);
         state.validators.push(validator);
         state.balances.push(32 * 1e9);
       }
@@ -38,3 +39,70 @@ describe("loadStateAndValidators", () => {
     });
   }
 });
+
+describe("loadState does not poison seed state's cache", () => {
+  // Regression test for a subtle SSZ TreeViewDU cache-transfer bug. loadState() used to clone the
+  // seed's validators / inactivityScores subviews with the default transfer-cache semantics.
+  // That does transfer cache away from the source subview, but the transferred `nodes[]` /
+  // `caches[]` arrays are still the same ones referenced by the seed container's cached child
+  // snapshot. A subsequent `migratedState.commit()` writes modified validator nodes into those
+  // shared arrays, silently corrupting the seed container cache. The corruption is only
+  // observed when the seed is later cloned with transfer-cache (the path verifyBlock takes via
+  // the default `preState.clone()`) and the new clone reads a modified index, at which point
+  // it returns the migrated state's validator instead of the seed's.
+  //
+  // Fix: clone the seed's validators/inactivityScores subviews with `clone(true)` so the
+  // migrated subview gets a fresh (empty) cache and its commit cannot reach into the seed's
+  // cache arrays.
+  const numValidator = 10;
+  const config = createChainForkConfig(mainnetChainConfig);
+  // altair+ so both validators and inactivityScores paths are exercised
+  const slot = mainnetChainConfig.ALTAIR_FORK_EPOCH * SLOTS_PER_EPOCH + 100;
+  const modifiedIndex = 0;
+
+  function buildState(mutateFn?: (state: BeaconStateAltair) => void): BeaconStateAltair {
+    const state = config.getForkTypes(slot).BeaconState.defaultViewDU() as BeaconStateAltair;
+    state.slot = slot;
+    for (let i = 0; i < numValidator; i++) {
+      const validator = ssz.phase0.Validator.defaultViewDU();
+      validator.pubkey = new Uint8Array(48).fill(i);
+      validator.withdrawalCredentials = new Uint8Array(32).fill(i);
+      validator.effectiveBalance = 32 * 1e9;
+      state.validators.push(validator);
+      state.balances.push(32 * 1e9);
+      state.inactivityScores.push(i);
+    }
+    mutateFn?.(state);
+    state.commit();
+    return state;
+  }
+
+  it("seed state's transfer-cache clone does not surface the migrated validator at a modified index", () => {
+    const seedState = buildState();
+    const originalRoot = seedState.hashTreeRoot();
+    const originalWC = new Uint8Array(32).fill(modifiedIndex);
+
+    const modifiedState = buildState((state) => {
+      const v = state.validators.get(modifiedIndex);
+      v.withdrawalCredentials = new Uint8Array(32).fill(0xaa);
+      state.validators.set(modifiedIndex, v);
+      state.inactivityScores.set(modifiedIndex, 999);
+    });
+    const modifiedBytes = modifiedState.serialize();
+
+    const {state: migrated} = loadState(config, seedState, modifiedBytes);
+    expect(migrated.validators.getReadonly(modifiedIndex).withdrawalCredentials).toEqual(new Uint8Array(32).fill(0xaa));
+    expect((migrated as BeaconStateAltair).inactivityScores.get(modifiedIndex)).toEqual(999);
+
+    // IMPORTANT: do not read seedState.validators / call seedState.commit() between
+    // loadState() and the clone. In production the head state keeps its cache snapshot
+    // untouched across loadState, and the poisoning only surfaces on the NEXT block where
+    // getPreState() clones with transfer-cache. Calling hashTreeRoot() / commit() here
+    // would rewrite `seedState.caches[validatorsFieldIndex]` with the cleared subview's
+    // cache and hide the bug.
+    const postState = seedState.clone() as BeaconStateAltair;
+    expect(postState.validators.getReadonly(modifiedIndex).withdrawalCredentials).toEqual(originalWC);
+    expect(postState.inactivityScores.get(modifiedIndex)).toEqual(modifiedIndex);
+    expect(postState.hashTreeRoot()).toEqual(originalRoot);
+  });
+});