Skip to content
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

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

Open
wants to merge 5 commits into
base: unstable
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 20 additions & 10 deletions testing/web3signer_tests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ mod tests {

/// A testing rig which holds a `ValidatorStore`.
struct ValidatorStoreRig {
validator_store: Arc<ValidatorStore<TestingSlotClock, E>>,
validator_store: Arc<ValidatorStore<TestingSlotClock>>,
_validator_dir: TempDir,
runtime: Arc<tokio::runtime::Runtime>,
_runtime_shutdown: async_channel::Sender<()>,
Expand Down Expand Up @@ -363,7 +363,7 @@ mod tests {
..Default::default()
};

let validator_store = ValidatorStore::<_, E>::new(
let validator_store = ValidatorStore::<_>::new(
initialized_validators,
slashing_protection,
Hash256::repeat_byte(42),
Expand All @@ -372,6 +372,7 @@ mod tests {
slot_clock,
&config,
executor,
E::slots_per_epoch(),
log.clone(),
);

Expand Down Expand Up @@ -488,7 +489,7 @@ mod tests {
generate_sig: F,
) -> Self
where
F: Fn(PublicKeyBytes, Arc<ValidatorStore<TestingSlotClock, E>>) -> R,
F: Fn(PublicKeyBytes, Arc<ValidatorStore<TestingSlotClock>>) -> R,
R: Future<Output = S>,
// We use the `SignedObject` trait to white-list objects for comparison. This avoids
// accidentally comparing something meaningless like a `()`.
Expand Down Expand Up @@ -523,7 +524,7 @@ mod tests {
web3signer_should_sign: bool,
) -> Self
where
F: Fn(PublicKeyBytes, Arc<ValidatorStore<TestingSlotClock, E>>) -> R,
F: Fn(PublicKeyBytes, Arc<ValidatorStore<TestingSlotClock>>) -> R,
R: Future<Output = Result<(), ValidatorStoreError>>,
{
for validator_rig in &self.validator_rigs {
Expand Down Expand Up @@ -590,7 +591,7 @@ mod tests {
.await
.assert_signatures_match("randao_reveal", |pubkey, validator_store| async move {
validator_store
.randao_reveal(pubkey, Epoch::new(0))
.randao_reveal::<E>(pubkey, Epoch::new(0))
.await
.unwrap()
})
Expand Down Expand Up @@ -631,7 +632,7 @@ mod tests {
.await
.assert_signatures_match("selection_proof", |pubkey, validator_store| async move {
validator_store
.produce_selection_proof(pubkey, Slot::new(0))
.produce_selection_proof::<E>(pubkey, Slot::new(0))
.await
.unwrap()
})
Expand All @@ -641,7 +642,7 @@ mod tests {
|pubkey, validator_store| async move {
let val_reg_data = get_validator_registration(pubkey);
validator_store
.sign_validator_registration_data(val_reg_data)
.sign_validator_registration_data::<E>(val_reg_data)
.await
.unwrap()
},
Expand Down Expand Up @@ -681,7 +682,11 @@ mod tests {
"sync_selection_proof",
|pubkey, validator_store| async move {
validator_store
.produce_sync_selection_proof(&pubkey, altair_fork_slot, SyncSubnetId::from(0))
.produce_sync_selection_proof::<E>(
&pubkey,
altair_fork_slot,
SyncSubnetId::from(0),
)
.await
.unwrap()
},
Expand All @@ -691,7 +696,12 @@ mod tests {
"sync_committee_signature",
|pubkey, validator_store| async move {
validator_store
.produce_sync_committee_signature(altair_fork_slot, Hash256::zero(), 0, &pubkey)
.produce_sync_committee_signature::<E>(
altair_fork_slot,
Hash256::zero(),
0,
&pubkey,
)
.await
.unwrap()
},
Expand Down Expand Up @@ -724,7 +734,7 @@ mod tests {
|pubkey, validator_store| async move {
let val_reg_data = get_validator_registration(pubkey);
validator_store
.sign_validator_registration_data(val_reg_data)
.sign_validator_registration_data::<E>(val_reg_data)
.await
.unwrap()
},
Expand Down
47 changes: 23 additions & 24 deletions validator_client/beacon_node_fallback/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use std::cmp::Ordering;
use std::fmt;
use std::fmt::Debug;
use std::future::Future;
use std::marker::PhantomData;
use std::sync::Arc;
use std::time::{Duration, Instant};
use strum::{EnumString, EnumVariantNames};
Expand Down Expand Up @@ -60,7 +59,7 @@ pub struct LatencyMeasurement {
/// See `SLOT_LOOKAHEAD` for information about when this should run.
pub fn start_fallback_updater_service<T: SlotClock + 'static, E: EthSpec>(
context: RuntimeContext<E>,
beacon_nodes: Arc<BeaconNodeFallback<T, E>>,
beacon_nodes: Arc<BeaconNodeFallback<T>>,
) -> Result<(), &'static str> {
let executor = context.executor;
if beacon_nodes.slot_clock.is_none() {
Expand All @@ -69,7 +68,7 @@ pub fn start_fallback_updater_service<T: SlotClock + 'static, E: EthSpec>(

let future = async move {
loop {
beacon_nodes.update_all_candidates().await;
beacon_nodes.update_all_candidates::<E>().await;

let sleep_time = beacon_nodes
.slot_clock
Expand Down Expand Up @@ -184,29 +183,27 @@ impl Serialize for CandidateInfo {
/// Represents a `BeaconNodeHttpClient` inside a `BeaconNodeFallback` that may or may not be used
/// for a query.
#[derive(Clone, Debug)]
pub struct CandidateBeaconNode<E> {
pub struct CandidateBeaconNode {
pub index: usize,
pub beacon_node: BeaconNodeHttpClient,
pub health: Arc<RwLock<Result<BeaconNodeHealth, CandidateError>>>,
_phantom: PhantomData<E>,
}

impl<E: EthSpec> PartialEq for CandidateBeaconNode<E> {
impl PartialEq for CandidateBeaconNode {
fn eq(&self, other: &Self) -> bool {
self.index == other.index && self.beacon_node == other.beacon_node
}
}

impl<E: EthSpec> Eq for CandidateBeaconNode<E> {}
impl Eq for CandidateBeaconNode {}

impl<E: EthSpec> CandidateBeaconNode<E> {
impl CandidateBeaconNode {
/// Instantiate a new node.
pub fn new(beacon_node: BeaconNodeHttpClient, index: usize) -> Self {
Self {
index,
beacon_node,
health: Arc::new(RwLock::new(Err(CandidateError::Uninitialized))),
_phantom: PhantomData,
}
}

Expand All @@ -215,14 +212,14 @@ impl<E: EthSpec> CandidateBeaconNode<E> {
*self.health.read().await
}

pub async fn refresh_health<T: SlotClock>(
pub async fn refresh_health<E: EthSpec, T: SlotClock>(
&self,
distance_tiers: &BeaconNodeSyncDistanceTiers,
slot_clock: Option<&T>,
spec: &ChainSpec,
log: &Logger,
) -> Result<(), CandidateError> {
if let Err(e) = self.is_compatible(spec, log).await {
if let Err(e) = self.is_compatible::<E>(spec, log).await {
*self.health.write().await = Err(e);
return Err(e);
}
Expand Down Expand Up @@ -286,7 +283,11 @@ impl<E: EthSpec> CandidateBeaconNode<E> {
}

/// Checks if the node has the correct specification.
async fn is_compatible(&self, spec: &ChainSpec, log: &Logger) -> Result<(), CandidateError> {
async fn is_compatible<E: EthSpec>(
&self,
spec: &ChainSpec,
log: &Logger,
) -> Result<(), CandidateError> {
let config = self
.beacon_node
.get_config_spec::<ConfigSpec>()
Expand Down Expand Up @@ -371,18 +372,18 @@ impl<E: EthSpec> CandidateBeaconNode<E> {
/// behaviour, where the failure of one candidate results in the next candidate receiving an
/// identical query.
#[derive(Clone, Debug)]
pub struct BeaconNodeFallback<T, E> {
pub candidates: Arc<RwLock<Vec<CandidateBeaconNode<E>>>>,
pub struct BeaconNodeFallback<T> {
pub candidates: Arc<RwLock<Vec<CandidateBeaconNode>>>,
distance_tiers: BeaconNodeSyncDistanceTiers,
slot_clock: Option<T>,
broadcast_topics: Vec<ApiTopic>,
spec: Arc<ChainSpec>,
log: Logger,
}

impl<T: SlotClock, E: EthSpec> BeaconNodeFallback<T, E> {
impl<T: SlotClock> BeaconNodeFallback<T> {
pub fn new(
candidates: Vec<CandidateBeaconNode<E>>,
candidates: Vec<CandidateBeaconNode>,
config: Config,
broadcast_topics: Vec<ApiTopic>,
spec: Arc<ChainSpec>,
Expand Down Expand Up @@ -466,15 +467,15 @@ impl<T: SlotClock, E: EthSpec> BeaconNodeFallback<T, E> {
/// It is possible for a node to return an unsynced status while continuing to serve
/// low quality responses. To route around this it's best to poll all connected beacon nodes.
/// A previous implementation of this function polled only the unavailable BNs.
pub async fn update_all_candidates(&self) {
pub async fn update_all_candidates<E: EthSpec>(&self) {
// Clone the vec, so we release the read lock immediately.
// `candidate.health` is behind an Arc<RwLock>, so this would still allow us to mutate the values.
let candidates = self.candidates.read().await.clone();
let mut futures = Vec::with_capacity(candidates.len());
let mut nodes = Vec::with_capacity(candidates.len());

for candidate in candidates.iter() {
futures.push(candidate.refresh_health(
futures.push(candidate.refresh_health::<E, T>(
&self.distance_tiers,
self.slot_clock.as_ref(),
&self.spec,
Expand Down Expand Up @@ -693,7 +694,7 @@ impl<T: SlotClock, E: EthSpec> BeaconNodeFallback<T, E> {
}

/// Helper functions to allow sorting candidate nodes by health.
async fn sort_nodes_by_health<E: EthSpec>(nodes: &mut Vec<CandidateBeaconNode<E>>) {
async fn sort_nodes_by_health(nodes: &mut Vec<CandidateBeaconNode>) {
// Fetch all health values.
let health_results: Vec<Result<BeaconNodeHealth, CandidateError>> =
future::join_all(nodes.iter().map(|node| node.health())).await;
Expand All @@ -711,7 +712,7 @@ async fn sort_nodes_by_health<E: EthSpec>(nodes: &mut Vec<CandidateBeaconNode<E>
});

// Reorder candidates based on the sorted indices.
let sorted_nodes: Vec<CandidateBeaconNode<E>> = indices_with_health
let sorted_nodes: Vec<CandidateBeaconNode> = indices_with_health
.into_iter()
.map(|(index, _)| nodes[index].clone())
.collect();
Expand Down Expand Up @@ -743,9 +744,7 @@ mod tests {
use eth2::Timeouts;
use std::str::FromStr;
use strum::VariantNames;
use types::{MainnetEthSpec, Slot};

type E = MainnetEthSpec;
use types::Slot;

#[test]
fn api_topic_all() {
Expand All @@ -764,7 +763,7 @@ mod tests {
let optimistic_status = IsOptimistic::No;
let execution_status = ExecutionEngineHealth::Healthy;

fn new_candidate(index: usize) -> CandidateBeaconNode<E> {
fn new_candidate(index: usize) -> CandidateBeaconNode {
let beacon_node = BeaconNodeHttpClient::new(
SensitiveUrl::parse(&format!("http://example_{index}.com")).unwrap(),
Timeouts::set_all(Duration::from_secs(index as u64)),
Expand Down
10 changes: 5 additions & 5 deletions validator_client/doppelganger_service/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,8 @@ impl DoppelgangerState {
/// If the BN fails to respond to either of these requests, simply return an empty response.
/// This behaviour is to help prevent spurious failures on the BN from needlessly preventing
/// doppelganger progression.
async fn beacon_node_liveness<'a, T: 'static + SlotClock, E: EthSpec>(
beacon_nodes: Arc<BeaconNodeFallback<T, E>>,
async fn beacon_node_liveness<'a, T: 'static + SlotClock>(
beacon_nodes: Arc<BeaconNodeFallback<T>>,
log: Logger,
current_epoch: Epoch,
validator_indices: Vec<u64>,
Expand Down Expand Up @@ -290,7 +290,7 @@ impl DoppelgangerService {
service: Arc<Self>,
context: RuntimeContext<E>,
validator_store: Arc<V>,
beacon_nodes: Arc<BeaconNodeFallback<T, E>>,
beacon_nodes: Arc<BeaconNodeFallback<T>>,
slot_clock: T,
) -> Result<(), String>
where
Expand Down Expand Up @@ -400,7 +400,7 @@ impl DoppelgangerService {
///
/// Validators added during the genesis epoch will not have doppelganger protection applied to
/// them.
pub fn register_new_validator<E: EthSpec, T: SlotClock>(
pub fn register_new_validator<T: SlotClock, E: EthSpec>(
&self,
validator: PublicKeyBytes,
slot_clock: &T,
Expand Down Expand Up @@ -805,7 +805,7 @@ mod test {
.expect("index should exist");

self.doppelganger
.register_new_validator::<E, _>(pubkey, &self.slot_clock)
.register_new_validator::<_, E>(pubkey, &self.slot_clock)
.unwrap();
self.doppelganger
.doppelganger_states
Expand Down
4 changes: 2 additions & 2 deletions validator_client/http_api/src/create_signed_voluntary_exit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use validator_store::ValidatorStore;
pub async fn create_signed_voluntary_exit<T: 'static + SlotClock + Clone, E: EthSpec>(
pubkey: PublicKey,
maybe_epoch: Option<Epoch>,
validator_store: Arc<ValidatorStore<T, E>>,
validator_store: Arc<ValidatorStore<T>>,
slot_clock: T,
log: Logger,
) -> Result<GenericResponse<SignedVoluntaryExit>, warp::Rejection> {
Expand Down Expand Up @@ -52,7 +52,7 @@ pub async fn create_signed_voluntary_exit<T: 'static + SlotClock + Clone, E: Eth
);

let signed_voluntary_exit = validator_store
.sign_voluntary_exit(pubkey_bytes, voluntary_exit)
.sign_voluntary_exit::<E>(pubkey_bytes, voluntary_exit)
.await
.map_err(|e| {
warp_utils::reject::custom_server_error(format!(
Expand Down
11 changes: 5 additions & 6 deletions validator_client/http_api/src/create_validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ use account_utils::{
use eth2::lighthouse_vc::types::{self as api_types};
use slot_clock::SlotClock;
use std::path::{Path, PathBuf};
use types::ChainSpec;
use types::EthSpec;
use types::{ChainSpec, EthSpec};
use validator_dir::{keystore_password_path, Builder as ValidatorDirBuilder};
use validator_store::ValidatorStore;
use zeroize::Zeroizing;
Expand All @@ -30,7 +29,7 @@ pub async fn create_validators_mnemonic<P: AsRef<Path>, T: 'static + SlotClock,
validator_requests: &[api_types::ValidatorRequest],
validator_dir: P,
secrets_dir: Option<PathBuf>,
validator_store: &ValidatorStore<T, E>,
validator_store: &ValidatorStore<T>,
spec: &ChainSpec,
) -> Result<(Vec<api_types::CreatedValidator>, Mnemonic), warp::Rejection> {
let mnemonic = mnemonic_opt.unwrap_or_else(random_mnemonic);
Expand Down Expand Up @@ -141,7 +140,7 @@ pub async fn create_validators_mnemonic<P: AsRef<Path>, T: 'static + SlotClock,
drop(validator_dir);

validator_store
.add_validator_keystore(
.add_validator_keystore::<_, E>(
voting_keystore_path,
voting_password_storage,
request.enable,
Expand Down Expand Up @@ -178,11 +177,11 @@ pub async fn create_validators_mnemonic<P: AsRef<Path>, T: 'static + SlotClock,

pub async fn create_validators_web3signer<T: 'static + SlotClock, E: EthSpec>(
validators: Vec<ValidatorDefinition>,
validator_store: &ValidatorStore<T, E>,
validator_store: &ValidatorStore<T>,
) -> Result<(), warp::Rejection> {
for validator in validators {
validator_store
.add_validator(validator)
.add_validator::<E>(validator)
.await
.map_err(|e| {
warp_utils::reject::custom_server_error(format!(
Expand Down
Loading
Loading