Skip to content

Batch attestation slashibility checking #6219

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

Open
wants to merge 29 commits into
base: unstable
Choose a base branch
from

Conversation

eserilev
Copy link
Member

@eserilev eserilev commented Aug 4, 2024

Issue Addressed

Closes #1914

Proposed Changes

The attestation service spawns a thread for each validator duty for a given slot. Within that thread the service checks if the attestation is safe, and if safe signs and publishes it. The attestation safety check requires reading and writing to a Sqlite db instance. Sqlite only allows for one open write transaction at any given time, and that transaction cannot be shared across threads. Opening a write transaction on one thread will block all other threads. This PR aims to refactor the attestation service so that database read/writes are not causing performance bottlenecks. These performance issues only become evident when large numbers of validators are involved.

Threads are still spawned per validator duty. However, within each thread we immediately sign the attestation. We collect the signed attestations and then perform the attestation safety checks. The db related tasks all happen in a single thread. The attestations that are deemed safe are all broadcasted at once and then the service continues with its normal flow.

I've also introduced an attestation data serivce. Its a simple service that downloads attestations by slot/committee index from the BN and caches them for later. It also prevents us from trying to download duplicate attestation data and helps abstract away the differences between how we handle attestation data pre and post electra.

Additionally, i've also parallelized producing and publishing attestation aggregates.

@eserilev eserilev added work-in-progress PR is a work-in-progress val-client Relates to the validator client binary optimization Something to make Lighthouse run more efficiently. database labels Aug 4, 2024
/// The AttestationDataService is responsible for downloading and caching attestation data at a given slot
/// for a range of committee indexes. It also helps prevent us from re-downloading identical attestation data.
pub struct AttestationDataService<T: SlotClock, E: EthSpec> {
attestation_data_by_committee: HashMap<CommitteeIndex, AttestationData>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Note that all attestations votes are the same on each committee, the only change is in the index field which equals the key of this hashmap CommitteeIndex. If you are refactoring the flow you can consider an optimization to only get a single AttestationData for all committees.

However, some DVT solution relies on a call for each CommitteeIndex being made. I'm not sure if this is the case but it was ~1.5 years ago.

@eserilev eserilev marked this pull request as ready for review August 7, 2024 00:13
@eserilev
Copy link
Member Author

eserilev commented Aug 7, 2024

This one should be ready for a review. I still need to add some more granular metrics though.

@eserilev eserilev changed the title [WIP] Batch attestation slashibility checking Batch attestation slashibility checking Aug 7, 2024
@eserilev eserilev added ready-for-review The code is ready for review and removed work-in-progress PR is a work-in-progress labels Aug 19, 2024
@michaelsproul michaelsproul added the v7.0.0-beta.0 New release c. Q1 2025 label Dec 2, 2024
@michaelsproul michaelsproul added v7.1.0 Post-Electra release and removed v7.0.0-beta.0 New release c. Q1 2025 labels Feb 6, 2025
Copy link

mergify bot commented Feb 6, 2025

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

@eserilev eserilev added work-in-progress PR is a work-in-progress and removed ready-for-review The code is ready for review labels Mar 13, 2025
Copy link

mergify bot commented May 19, 2025

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

@mergify mergify bot added the waiting-on-author The reviewer has suggested changes and awaits thier implementation. label May 19, 2025
Copy link

mergify bot commented May 19, 2025

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

@mergify mergify bot closed this Jun 18, 2025
Copy link

mergify bot commented Jun 18, 2025

Hi @eserilev, this pull request has been closed automatically due to 30 days of inactivity. If you’d like to continue working on it, feel free to reopen at any time.

@mergify mergify bot added the stale Stale PRs that have been inactive and is now outdated label Jun 18, 2025
@eserilev eserilev reopened this Jun 18, 2025
@eserilev eserilev removed stale Stale PRs that have been inactive and is now outdated waiting-on-author The reviewer has suggested changes and awaits thier implementation. v7.1.0 Post-Electra release labels Jun 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
database optimization Something to make Lighthouse run more efficiently. val-client Relates to the validator client binary work-in-progress PR is a work-in-progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants