Skip to content

Initial feedback from Lighthouse integration #1

Description

@michaelsproul

Hey @galadd,

I had a quick run at integrating eth-state-diff into Lighthouse, in place of our current HDiff scheme that uses xdelta3 and some manually defined diffs for the balances and validators.

The results are promising, your library is generally faster, and the sizes of the diffs are competitive. There are a couple of assumptions that your library makes that aren't compatible with our scheme, I wonder if you'd be open to tweaking them.

Benchmark numbers per Fable:

xdelta3 eth-state-diff
92,379-slot span (*084*463) compute 340 ms 64 ms
apply 175 ms 24 ms
diff size 6.37 MB 10.5 MB
64-slot span (skip-slots) compute 72 ms 32 ms
apply 18.5 ms 9.5 ms
diff size 684 KB 522 KB

(Fable) Both algorithms' compute → apply recovered state_14828463.ssz byte-for-byte (Verification OK). eth-state-diff is ~5× faster to compute and ~7× faster to apply. Its large-span diffs are 65% bigger because everything the crate doesn't diff (notably current_epoch_participation, historical summaries, the execution payload header) rides along in full in its scalar_header on every diff. The 64-slot size win is flattering — skip-slots leaves current-epoch participation all-zero, which a real chain wouldn't.

Fable also had this to say about the integration (the rest of this issue text is LLM-generated, if you prefer not to read it I can revisit soon and pull out the most pertinent things):

Issues found integrating eth-state-diff 0.1.1 into Lighthouse

Context: we integrated eth-state-diff as an experimental alternative diff engine for Lighthouse's hierarchical state diffs, and validated it on two mainnet Fulu states 92,379 slots (~2,887 epochs) apart (~2M validators). To make reconstruction exact we had to bypass the top-level create()/apply() and call the per-field encoders directly. With those workarounds, compute/apply roundtrips byte-for-byte, and the per-field encoders are fast (~5× faster than our xdelta3-based scheme). The issues below are roughly ordered by severity.

Correctness

1. create() hardcodes single-epoch windows for state_roots / randao_mixes / slashings. In lib.rs::create(), state_roots, randao_mixes and slashings are diffed over [base_slot, base_slot + slots_per_epoch] regardless of the actual target slot (only block_roots uses the real target_slot). For any delta spanning more than one epoch, entries written after base_slot + 32 are silently missing and the reconstructed state is wrong. DiffSource::slot() already returns both slots, so this looks like an oversight rather than a design constraint. Fix: use the real target_slot for all four ring buffers.

2. diff_slashings never inspects the base epoch. slashings.rs::diff_slashings starts iterating at base_epoch + 1, but slash_validator adds to slashings[current_epoch % N] at any slot within an epoch. If the base state is captured at (or before) a slashing later in that same epoch, the change to slashings[base_epoch % N] is missed. Fix: start the scan at base_epoch — comparing an extra unchanged entry is harmless since the encoding is sparse.

3. diff_eth1_votes mis-encodes growth across a voting-period reset. It treats target_len >= base_len as pure append and stores target[base_len..]. The vote list resets every voting period (2,048 slots on mainnet), so for any window crossing a reset where the list has regrown to ≥ the base length, apply keeps the base's (now wrong) prefix. Fix: use the append encoding only when target.starts_with(base) (byte prefix check), otherwise ResetAndAppend.

4. diff_fifo_queue append fast-path has the same prefix assumption. target.len() >= base.len() is treated as pure append; if items were consumed from the front and enough appended that the queue grew, reconstruction is corrupt. Additionally, find_chunk searches with windows(needle.len()), so a match can land at an offset that isn't a multiple of item_ssz_size, producing a misaligned consumed_count. Fixes: same prefix check before the append path; step the overlap search by item_ssz_size; and/or verify the candidate delta reconstructs the target (a cheap byte compare) with a full-replacement fallback.

5. apply() uses wrong SSZ item sizes for the Electra pending queues. It passes 88 / 121 / 169 for pending_deposits / pending_partial_withdrawals / pending_consolidations. Actual SSZ sizes: PendingDeposit = 48+32+8+96+8 = 192, PendingPartialWithdrawal = 8+8+8 = 24, PendingConsolidation = 8+8 = 16. With a non-zero consumed_count the wrong number of bytes is drained and the queue is corrupted. (121 is VALIDATOR_SSZ_SIZE, which suggests a copy-paste slip.)

6. diff_roots stores O(span) entries instead of O(min(span, capacity)). It records one root per slot in [base_slot, target_slot). When the span exceeds the ring capacity (8,192 for block/state roots), only the last capacity writes matter, but the encoder still emits span entries — ~2.9 MB per ring for our 92k-slot window, ~64× larger than needed. (Size-only, not correctness: wrapped writes repeat the same final values.) Fix: clamp the range to [max(base_slot, target_slot - capacity), target_slot), with the same clamp in apply_roots so the replay base matches.

Robustness / portability

7. Panics on malformed delta input. Several apply paths index unchecked into delta-supplied data: read_varint does buf[*cursor] (the "SAFETY" comment only holds for self-produced deltas), apply_inactivity does base[i] with a delta-supplied index, apply_validators does copy_from_slice with delta-supplied patch lengths, and apply() asserts on fork mismatch. rkyv validation checks the outer structure but not these invariants, so a corrupt or mismatched delta read from disk panics instead of returning an error. For consensus-client storage a Result-based API would be much easier to adopt.

8. Hardcoded chain constants. slots_per_epoch = 32 (in create()/apply()) and MIN_VALIDATOR_WITHDRAWABILITY_DELAY = 256 (used to reconstruct withdrawable_epoch) are preset/config values that differ on other networks (e.g. minimal-spec testing uses 8 slots per epoch). Making them parameters would let integrators pass their chain config.

9. apply() fork assertion precludes cross-fork deltas. Archival storage needs deltas whose base and target straddle a fork boundary. The per-field encoders handle this fine (new fields appear as appends / via the scalar header), but the top-level apply() asserts state_fork == delta_fork. Also, ForkName has no variant for Gloas, which clients are already implementing.

Design limitation worth documenting: scalar_header dominates delta size

Everything not covered by a specialized encoder is carried in full in scalar_header on every delta. On mainnet today this includes current_epoch_participation (~1 byte/validator, ~2 MB), historical_summaries (append-only, ~67 KB and growing) and historical_roots (pure entropy). In our benchmark this made eth-state-diff deltas ~65% larger than our existing scheme (10.5 MB vs 6.4 MB over the 92k-slot window) despite the specialized encodings being excellent. Two cheap wins: diff current_epoch_participation with the existing participation encoder (a second ParticipationDiff field), and encode historical_summaries/historical_roots as append-only lists (they only ever grow).

Also worth documenting: the validators encoder assumes pubkeys are immutable at a given index (no index re-use) — true on mainnet post-genesis, but an assumption integrators should be aware of.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions