Skip to content

Implement basic validator custody framework (no backfill) #7578

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 31 commits into from
Jun 11, 2025

Conversation

pawanjay176
Copy link
Member

@pawanjay176 pawanjay176 commented Jun 7, 2025

Issue Addressed

Resolves #6767

Proposed Changes

This PR implements a basic version of validator custody.

  • It introduces a new CustodyContext object which contains info regarding number of validators attached to a node and the custody count they contribute to the cgc.
  • The CustodyContext is added in the da_checker and has methods for returning the current cgc and the number of columns to sample at head. Note that the logic for returning the cgc existed previously in the network globals.
  • To estimate the number of validators attached, we use the beacon_committee_subscriptions endpoint. This might overestimate the number of validators actually publishing attestations from the node in the case of multi BN setups. We could also potentially use the publish_attestations endpoint to get a more conservative estimate at a later point.
  • Anytime there's a change in the custody_group_count due to addition/removal of validators, the custody context should send an event on a broadcast channnel. The only subscriber for the channel exists in the network service which simply subscribes to more subnets. There can be additional subscribers in sync that will start a backfill once the cgc changes.

TODO

  • NOT REQUIRED: Currently, the logic only handles an increase in validator count and does not handle a decrease. We should ideally unsubscribe from subnets when the cgc has decreased.
  • NOT REQUIRED: Add a service in the CustodyContext that emits an event once MIN_EPOCHS_FOR_BLOB_SIDECARS_REQUESTS passes after updating the current cgc. This event should be picked up by a subscriber which updates the enr and metadata.
  • Add more tests

@pawanjay176 pawanjay176 requested a review from jxs as a code owner June 7, 2025 07:27
@pawanjay176 pawanjay176 added work-in-progress PR is a work-in-progress fulu Required for the upcoming Fulu hard fork labels Jun 7, 2025
Copy link
Member

@dknopik dknopik left a comment

Choose a reason for hiding this comment

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

Took a look at this as an opportunity to refamiliarise myself with the DAS code and spec. I hope I looked at the correct spec version x)

Also, just noting as I found no explicit TODO or code for this: I think we also need to update the ValidatorRegistrations if the effective balance changes due to beacon chain rewards, penalties, deposits and (partial) withdrawals.

@jimmygchen jimmygchen added ready-for-review The code is ready for review syncing and removed work-in-progress PR is a work-in-progress labels Jun 11, 2025
@jimmygchen jimmygchen force-pushed the validator-custody branch 2 times, most recently from 4bb3ec9 to 178b08e Compare June 11, 2025 14:43
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.

LGTM

Next changes (separate PRs)

  • add lowest block slot to status message, update it on CGC change, & use lowest block slot for peer selection when making rpc requests
  • use additional http api to register validators
  • backfill

Copy link
Member Author

@pawanjay176 pawanjay176 left a comment

Choose a reason for hiding this comment

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

LGTM as well. Thanks for fixing up most of the outstanding issues!

Copy link

mergify bot commented Jun 11, 2025

Some required checks have failed. Could you please take a look @pawanjay176? 🙏

@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 Jun 11, 2025
@jimmygchen jimmygchen added ready-for-review The code is ready for review ready-for-merge This PR is ready to merge. and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. ready-for-review The code is ready for review labels Jun 11, 2025
@mergify mergify bot merged commit 5f208bb into sigp:unstable Jun 11, 2025
35 of 37 checks passed
@michaelsproul michaelsproul added the backwards-incompat Backwards-incompatible API change label Jun 30, 2025
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 fulu Required for the upcoming Fulu hard fork ready-for-merge This PR is ready to merge. syncing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants