Skip to content

Use async RwLock for the VC's initialized_validators #7423

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 8 commits into
base: unstable
Choose a base branch
from

Conversation

michaelsproul
Copy link
Member

Issue Addressed

Remove a parking_lot::RwLockReadGuard which was being held across an await in the VC.

Possibly addressing:

Proposed Changes

Use a tokio::sync::RwLock which requires await to be unlocked.

Unfortunately this has a huge ripple effect, with many functions needing to be async 🙄

I've been grinding through this refactor for hours and it isn't quite finished yet. I'm offline until May 13. If anyone wants to pick this up before then just comment below.

@michaelsproul michaelsproul added work-in-progress PR is a work-in-progress val-client Relates to the validator client binary labels May 8, 2025
@michaelsproul michaelsproul changed the title Vc async lock Use async RwLock for the VC's initialized_validators May 8, 2025
@eserilev
Copy link
Member

eserilev commented May 10, 2025

Was poking around a bit here trying to get things to compile and found a subtle tracing issue

subte tracing thing

info!(
    voting_validators = validator_store.num_voting_validators().await,
    "Loaded validator keypair store"
);
        

Calling a future inside a tracing macro isn't Send

@michaelsproul
Copy link
Member Author

Oh nice find! Good to know!

@michaelsproul michaelsproul added ready-for-review The code is ready for review and removed work-in-progress PR is a work-in-progress labels May 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-review The code is ready for review val-client Relates to the validator client binary
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants