Modularize validator store #6705

dknopik · 2024-12-16T14:25:36Z

This is a PR that modularises the validator store so that we can reuse the validator services in Anchor.

Proposed Changes

Create trait ValidatorStore with all functions used by the validator_services
Make validator_services generic on S: ValidatorStore
Introduce LighthouseValidatorStore, which has identical functionality to the old ValidatorStore
Remove dependencies (especially environment) from validator_services and beacon_node_fallback in order to be able to cleanly use them in Anchor

Additional Info

This also includes some changes from @jxs's closed PR #6727 to remove E from some structs and make functions generic instead (where possible).

mergify · 2025-01-09T12:31:13Z

⚠️ The sha of the head commit of this PR conflicts with #6771. Mergify cannot evaluate rules on this PR. ⚠️

mergify · 2025-01-09T12:56:21Z

⚠️ The sha of the head commit of this PR conflicts with #6771. Mergify cannot evaluate rules on this PR. ⚠️

dknopik · 2025-01-16T07:14:18Z

Will rebase this to clean up the messy history after

are merged. Still, feel free to start reviewing already.

mergify · 2025-01-27T13:24:02Z

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

…tor-store

michaelsproul

This is fantastic, very clean!

I'm highly motivated to get this merged soon, not just because of Anchor, but also because I want to solve this FIXME building on your branch:

lighthouse/validator_client/validator_store/src/lib.rs

Line 209 in e42406d

    
               // FIXME: ignore this clippy lint until the validator store is refactored to use async locks

It will require changing a few of the ValidatorStore methods to return impl Future, but I think we should merge this first, and then adapt Anchor to that change once it's also ready and merged.

validator_client/http_api/Cargo.toml

validator_client/http_api/src/tests.rs

michaelsproul · 2025-05-05T06:33:55Z

validator_client/signing_method/src/lib.rs

-                    object,
-                    Web3SignerObject::Deposit { .. } | Web3SignerObject::ValidatorRegistration(_)
-                ) && fork_info.is_some()
+                if matches!(message_type, MessageType::ValidatorRegistration) && fork_info.is_some()


Is this a bugfix?

Good catch. Checking the git blame, this seems to originate from @jxs's #6727. @jxs, do you remember this? There is a non-zero chance that I messed this up when rebasing, though I do not remember ever touching this.

validator_client/validator_services/src/block_service.rs

Co-authored-by: Michael Sproul <[email protected]>

dknopik · 2025-05-05T14:25:46Z

There is one more change that would be nice: currently, sign_validator_exit is part of the new ValidatorStore trait, as I wanted to keep the interface (mostly) the same. However, in the meantime I realized that sign_validator_exit is not actually used in the validator_services crate, but in the VC HTTP API, where the actual type "LighthouseValidatorStore" is available. So we could move it back to be a method of that type. This would make the Anchor code a bit nicer as the function signature is not sufficient for our needs, so we only return an error in there anyway (as it will never be called by the validator_services) and have a custom function for Anchor internal use.

Does that sound fine by you?

michaelsproul · 2025-05-06T00:05:18Z

Does that sound fine by you?

Yeah that sounds totally fine, happy for it to move to LighthouseValidatorStore!

…torStore`

michaelsproul

Let's goooo

While the Lighthouse implementation of the `ValidatorStore` does not really care about blobs, Anchor needs to be able to return different blobs from `sign_blocks` than what was passed into it, in case it decides to sign another Anchor node's block. Only passing the unsigned block into `sign_block` and only returning a signed block from it (without any blobs and proofs) was an oversight in #6705. - Replace `validator_store::{Uns,S}ignedBlock` with `validator_store::block_service::{Uns,S}ignedBlock`, as we need all data in there. - In `lighthouse_validator_store`, just add the received blobs back to the signed block after signing it.

chong-he added the work-in-progress PR is a work-in-progress label Dec 18, 2024

dknopik force-pushed the modularize-validator-store branch 3 times, most recently from 3c253b4 to 838eed6 Compare December 19, 2024 16:50

dknopik mentioned this pull request Jan 3, 2025

Duty services sigp/anchor#73

Merged

mergify bot mentioned this pull request Jan 8, 2025

Anchor pre-PR: Modularize validator store #6771

Merged

dknopik marked this pull request as ready for review January 16, 2025 07:14

dknopik requested a review from jxs as a code owner January 16, 2025 07:14

dknopik mentioned this pull request Jan 16, 2025

Remove E from validator_client types where it's not required #6727

Closed

dknopik force-pushed the modularize-validator-store branch 2 times, most recently from dee1f90 to 394ef61 Compare February 7, 2025 15:59

dknopik mentioned this pull request Feb 10, 2025

Anchor Pre-PR: unstable and some changes from modularized-validator-store #6966

Merged

jxs and others added 3 commits March 13, 2025 08:44

pass slots_per_epoch at runtime

a57ed6b

remove generic E from unrequired types

ceb1706

move validator_store to lighthouse_validator_store

bbddbba

dknopik force-pushed the modularize-validator-store branch from 394ef61 to 36f2916 Compare March 13, 2025 08:35

dknopik and others added 7 commits March 13, 2025 11:25

make validator_store into a trait

3b7b9c6

further reduce dependencies

4505298

remove environment dependency on beacon_node_fallback

d90ad1f

remove environment from validator_services

36925e3

unify boost factor accessors

87068cb

add builder for DutiesService

74cc922

remove PhantomData from SyncDutiesMap

fb62fec

dknopik force-pushed the modularize-validator-store branch from 36f2916 to 29b4ce5 Compare March 13, 2025 11:27

dknopik added 2 commits March 13, 2025 13:06

Add E to ValidatorStore as associated type

61a932b

derive Clone for ValidatorStore's Error and required sub-errors

b050917

switch to enum for block signing to allow differing types

0ab9c9a

dknopik force-pushed the modularize-validator-store branch from 29b4ce5 to 0ab9c9a Compare March 13, 2025 12:07

Merge branch 'unstable' into modularize-validator-store

b30f739

jxs requested review from michaelsproul and macladson March 14, 2025 17:06

dknopik and others added 6 commits April 2, 2025 09:49

Merge branch 'unstable' into modularize-validator-store

05f9669

Merge branch 'unstable' into modularize-validator-store

47e3e38

remove unused dep

05367e0

Merge branch 'unstable' into modularize-validator-store

9f1b47d

Merge remote-tracking branch 'origin/unstable' into modularize-valida…

07cf6b9

…tor-store

Fix URL

7a9a9c4

michaelsproul reviewed May 5, 2025

View reviewed changes

michaelsproul added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed work-in-progress PR is a work-in-progress labels May 5, 2025

Fix blank line in Cargo.toml

eab1c64

Co-authored-by: Michael Sproul <[email protected]>

michaelsproul and others added 3 commits May 6, 2025 15:39

Remove BlockService::get_builder_boost_factor

508bbed

Update comment on determine_builder_boost_factor

ffe96a4

Move sign_voluntary_exit from ValidatorStore to `LighthouseValida…

d0f169d

…torStore`

dknopik mentioned this pull request May 6, 2025

Update Lighthouse sigp/anchor#304

Merged

michaelsproul approved these changes May 7, 2025

View reviewed changes

michaelsproul added val-client Relates to the validator client binary ready-for-merge This PR is ready to merge. and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels May 7, 2025

mergify bot added a commit that referenced this pull request May 7, 2025

Merge of #6705

35bb5a2

mergify bot mentioned this pull request May 7, 2025

merge queue: embarking unstable (beb0ce6) and #6705 together #7407

Closed

6 tasks

mergify bot merged commit 3d92e36 into sigp:unstable May 7, 2025
31 checks passed

dknopik mentioned this pull request May 20, 2025

Pass blobs into ValidatorStore::sign_block #7497

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Modularize validator store #6705

Modularize validator store #6705

Uh oh!

dknopik commented Dec 16, 2024 •

edited

Loading

Uh oh!

mergify bot commented Jan 9, 2025

Uh oh!

mergify bot commented Jan 9, 2025

Uh oh!

dknopik commented Jan 16, 2025

Uh oh!

mergify bot commented Jan 27, 2025

Uh oh!

michaelsproul left a comment

Uh oh!

Uh oh!

Uh oh!

michaelsproul May 5, 2025

Uh oh!

dknopik May 5, 2025

Uh oh!

Uh oh!

dknopik commented May 5, 2025

Uh oh!

michaelsproul commented May 6, 2025

Uh oh!

michaelsproul left a comment

Uh oh!

Uh oh!

Uh oh!

Modularize validator store #6705

Modularize validator store #6705

Uh oh!

Conversation

dknopik commented Dec 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Additional Info

Uh oh!

mergify bot commented Jan 9, 2025

Uh oh!

mergify bot commented Jan 9, 2025

Uh oh!

dknopik commented Jan 16, 2025

Uh oh!

mergify bot commented Jan 27, 2025

Uh oh!

michaelsproul left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

michaelsproul May 5, 2025

Choose a reason for hiding this comment

Uh oh!

dknopik May 5, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dknopik commented May 5, 2025

Uh oh!

michaelsproul commented May 6, 2025

Uh oh!

michaelsproul left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dknopik commented Dec 16, 2024 •

edited

Loading