Skip to content

Conversation

michaelsproul
Copy link
Member

@michaelsproul michaelsproul commented Jul 28, 2025

Issue Addressed

Closes:

Proposed Changes

  • Remove balances_cache from PersistedForkChoiceStore (~65 MB saving on mainnet)
  • Remove justified_balances from PersistedForkChoiceStore (~16 MB saving on mainnet)
  • Remove balances from ProtoArray/SszContainer.
  • Implement zstd compression for votes
  • Fix bug in justified state usage
  • Bump schema version to V28 and implement migration.

Testing Results

Total bytes written is way down:

write_bytes_total

Time spent writing to disk is also way down:

write_bytes_seconds

@michaelsproul michaelsproul added work-in-progress PR is a work-in-progress optimization Something to make Lighthouse run more efficiently. consensus An issue/PR that touches consensus code, such as state_processing or block verification. database backwards-incompat Backwards-incompatible API change labels Jul 28, 2025
@dapplion
Copy link
Collaborator

Closes #7760 or we want to go further?

@michaelsproul
Copy link
Member Author

This isn't finished yet, I'm working on the compression now and it gets the size down to 1.9MB total

@jimmygchen jimmygchen added the v8.0.0-rc.0 Q3 2025 release for Fusaka on Holesky label Aug 6, 2025
@michaelsproul michaelsproul added ready-for-review The code is ready for review and removed work-in-progress PR is a work-in-progress labels Aug 7, 2025
Copy link

mergify bot commented Aug 12, 2025

This pull request has merge conflicts. Could you please resolve them @michaelsproul? 🙏

@mergify mergify bot added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Aug 12, 2025
@mergify mergify bot added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Aug 12, 2025
@@ -162,9 +164,9 @@ where
/// It is assumed that `anchor` is already persisted in `store`.
pub fn get_forkchoice_store(
store: Arc<HotColdDB<E, Hot, Cold>>,
anchor: &BeaconSnapshot<E>,
anchor: BeaconSnapshot<E>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need ownership here? we're cloning the beacon state below, and i dont see this moved anywhere

)
.map_err(|e| {
Error::MigrationError(format!(
"Error loading fork choise store from persisted: {e:?}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small typo fork choice

Copy link
Member

@jimmygchen jimmygchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great optimisation 🎉

I wasn't very familiar with this part of the codebase before so it took a while to review. I've reviewed this and it looks solid and clean to me. Really nice work!

@jimmygchen
Copy link
Member

Will leave it to you to merge if you'd like to do more testing or another pair of eyes (maybe @dapplion ?)

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Aug 18, 2025
mergify bot added a commit that referenced this pull request Aug 18, 2025
@mergify mergify bot merged commit 836c39e into unstable Aug 18, 2025
34 checks passed
@mergify mergify bot deleted the fc-mem branch August 18, 2025 06:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backwards-incompat Backwards-incompatible API change consensus An issue/PR that touches consensus code, such as state_processing or block verification. database optimization Something to make Lighthouse run more efficiently. ready-for-merge This PR is ready to merge. v8.0.0-rc.0 Q3 2025 release for Fusaka on Holesky
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants