-
Notifications
You must be signed in to change notification settings - Fork 59
Add automatic registry mux refreshing #384
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
base: main
Are you sure you want to change the base?
Conversation
let mut is_refreshing_required = false; | ||
if let Some(muxes) = &state.config.registry_muxes { | ||
is_refreshing_required = muxes.iter().any(|(loader, _)| { | ||
matches!(loader, MuxKeysLoader::Registry { enable_refreshing: true, .. }) | ||
}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a bit more idiomatic
let mut is_refreshing_required = false; | |
if let Some(muxes) = &state.config.registry_muxes { | |
is_refreshing_required = muxes.iter().any(|(loader, _)| { | |
matches!(loader, MuxKeysLoader::Registry { enable_refreshing: true, .. }) | |
}); | |
} | |
llet is_refreshing_required = state.config.registry_muxes.as_ref().is_some_and(|muxes| { | |
muxes.iter().any(|(loader, _)| { | |
matches!(loader, MuxKeysLoader::Registry { enable_refreshing: true, .. }) | |
}) | |
}); |
// Initialize an empty lookup if the config doesn't have one yet | ||
let mux_lookup = match &config.mux_lookup { | ||
Some(lookup) => lookup, | ||
None => &HashMap::new(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we not return here?
for (pubkey, runtime_config) in new_pubkeys.iter() { | ||
debug!("adding new pubkey {pubkey} to mux {}", runtime_config.id); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure this adds a lot, and it iterates over the map whether debug
is enabled or not
for pubkey in removed_pubkeys.iter() { | ||
debug!("removing old pubkey {pubkey} from mux lookup"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
let mut state = state.write().await; | ||
let config = state.config.as_ref(); | ||
let new_mux_lookup = if let Some(existing) = &config.mux_lookup { | ||
let mut map = HashMap::new(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of allocating a new map here, we can just take a mutable reference, remove all removed_pubkeys
and add all new_pubkeys
match mux_lookup.get(&pubkey) { | ||
Some(_) => { | ||
// Pubkey already existed | ||
} | ||
None => { | ||
// New pubkey | ||
new_pubkeys.insert(pubkey.clone(), runtime_config.clone()); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
match mux_lookup.get(&pubkey) { | |
Some(_) => { | |
// Pubkey already existed | |
} | |
None => { | |
// New pubkey | |
new_pubkeys.insert(pubkey.clone(), runtime_config.clone()); | |
} | |
if mux_lookup.get(&pubkey).is_none() { | |
// New pubkey | |
new_pubkeys.insert(pubkey.clone(), runtime_config.clone()); | |
} |
This is part of #382 and incorporates some additional user feedback. Basically, users want the PBS service to routinely query SSV or Lido when set up with muxes that use them as registries, and automatically support new pubkeys that get added to the corresponding operators. This PR adds that capability by:
Note this has a substantial rewrite of the SSV loader, including breaking the core structs out into a public module and a more robust mock of the SSV API server for testing.