Skip to content

Commit

Permalink
address review
Browse files Browse the repository at this point in the history
  • Loading branch information
jxs committed Dec 20, 2024
1 parent a9cc2fb commit 6533a67
Show file tree
Hide file tree
Showing 9 changed files with 13 additions and 33 deletions.
6 changes: 0 additions & 6 deletions Cargo.lock

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

1 change: 0 additions & 1 deletion testing/web3signer_tests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,6 @@ mod tests {
slot_clock,
&config,
executor,
E::slots_per_epoch(),
log.clone(),
);

Expand Down
4 changes: 2 additions & 2 deletions validator_client/doppelganger_service/src/lib.rs
Original file line number Diff line number Diff line change
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<T: SlotClock, E: EthSpec>(
pub fn register_new_validator<E: EthSpec, T: SlotClock>(
&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
1 change: 0 additions & 1 deletion validator_client/http_api/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ impl ApiTester {
slot_clock.clone(),
&config,
test_runtime.task_executor.clone(),
E::slots_per_epoch(),
log.clone(),
));

Expand Down
1 change: 0 additions & 1 deletion validator_client/http_api/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ impl ApiTester {
slot_clock.clone(),
&config,
test_runtime.task_executor.clone(),
E::slots_per_epoch(),
log.clone(),
));

Expand Down
4 changes: 2 additions & 2 deletions validator_client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,6 @@ impl<E: EthSpec> ProductionValidatorClient<E> {
slot_clock.clone(),
&config.validator_store,
context.executor.clone(),
E::slots_per_epoch(),
log.clone(),
));

Expand All @@ -462,7 +461,8 @@ impl<E: EthSpec> ProductionValidatorClient<E> {
// oversized from having not been pruned (by a prior version) we don't want to prune
// concurrently, as it will hog the lock and cause the attestation service to spew CRITs.
if let Some(slot) = slot_clock.now() {
validator_store.prune_slashing_protection_db(slot.epoch(E::slots_per_epoch()), true);
validator_store
.prune_slashing_protection_db::<E>(slot.epoch(E::slots_per_epoch()), true);
}

let duties_context = context.service_context("duties".into());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,7 @@ impl<T: SlotClock + 'static, E: EthSpec> AttestationService<T, E> {
move || {
attestation_service
.validator_store
.prune_slashing_protection_db(current_epoch, false)
.prune_slashing_protection_db::<E>(current_epoch, false)
},
"slashing_protection_pruning",
)
Expand Down
8 changes: 0 additions & 8 deletions validator_client/validator_store/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,7 @@ path = "src/lib.rs"

[dependencies]
account_utils = { workspace = true }
beacon_node_fallback = { workspace = true }
doppelganger_service = { workspace = true }
environment = { workspace = true }
eth2 = { workspace = true }
initialized_validators = { workspace = true }
parking_lot = { workspace = true }
serde = { workspace = true }
Expand All @@ -22,10 +19,5 @@ slashing_protection = { workspace = true }
slog = { workspace = true }
slot_clock = { workspace = true }
task_executor = { workspace = true }
tokio = { workspace = true }
types = { workspace = true }
validator_metrics = { workspace = true }

[dev-dependencies]
futures = { workspace = true }
logging = { workspace = true }
19 changes: 8 additions & 11 deletions validator_client/validator_store/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ pub struct ValidatorStore<T> {
prefer_builder_proposals: bool,
builder_boost_factor: Option<u64>,
task_executor: TaskExecutor,
slots_per_epoch: u64,
}

impl<T: SlotClock + 'static> DoppelgangerValidatorStore for ValidatorStore<T> {
Expand All @@ -113,7 +112,6 @@ impl<T: SlotClock + 'static> ValidatorStore<T> {
slot_clock: T,
config: &Config,
task_executor: TaskExecutor,
slots_per_epoch: u64,
log: Logger,
) -> Self {
Self {
Expand All @@ -132,7 +130,6 @@ impl<T: SlotClock + 'static> ValidatorStore<T> {
prefer_builder_proposals: config.prefer_builder_proposals,
builder_boost_factor: config.builder_boost_factor,
task_executor,
slots_per_epoch,
}
}

Expand All @@ -145,7 +142,7 @@ impl<T: SlotClock + 'static> ValidatorStore<T> {
) -> Result<(), String> {
if let Some(doppelganger_service) = &self.doppelganger_service {
for pubkey in self.validators.read().iter_voting_pubkeys() {
doppelganger_service.register_new_validator::<_, E>(*pubkey, &self.slot_clock)?
doppelganger_service.register_new_validator::<E, _>(*pubkey, &self.slot_clock)?
}
}

Expand Down Expand Up @@ -222,7 +219,7 @@ impl<T: SlotClock + 'static> ValidatorStore<T> {

if let Some(doppelganger_service) = &self.doppelganger_service {
doppelganger_service
.register_new_validator::<_, E>(validator_pubkey, &self.slot_clock)?;
.register_new_validator::<E, _>(validator_pubkey, &self.slot_clock)?;
}

self.validators
Expand Down Expand Up @@ -868,7 +865,7 @@ impl<T: SlotClock + 'static> ValidatorStore<T> {
validator_pubkey: PublicKeyBytes,
slot: Slot,
) -> Result<SelectionProof, Error> {
let signing_epoch = slot.epoch(self.slots_per_epoch);
let signing_epoch = slot.epoch(E::slots_per_epoch());
let signing_context = self.signing_context(Domain::SelectionProof, signing_epoch);

// Bypass the `with_validator_signing_method` function.
Expand Down Expand Up @@ -905,7 +902,7 @@ impl<T: SlotClock + 'static> ValidatorStore<T> {
slot: Slot,
subnet_id: SyncSubnetId,
) -> Result<SyncSelectionProof, Error> {
let signing_epoch = slot.epoch(self.slots_per_epoch);
let signing_epoch = slot.epoch(E::slots_per_epoch());
let signing_context =
self.signing_context(Domain::SyncCommitteeSelectionProof, signing_epoch);

Expand Down Expand Up @@ -942,7 +939,7 @@ impl<T: SlotClock + 'static> ValidatorStore<T> {
validator_index: u64,
validator_pubkey: &PublicKeyBytes,
) -> Result<SyncCommitteeMessage, Error> {
let signing_epoch = slot.epoch(self.slots_per_epoch);
let signing_epoch = slot.epoch(E::slots_per_epoch());
let signing_context = self.signing_context(Domain::SyncCommittee, signing_epoch);

// Bypass `with_validator_signing_method`: sync committee messages are not slashable.
Expand Down Expand Up @@ -981,7 +978,7 @@ impl<T: SlotClock + 'static> ValidatorStore<T> {
contribution: SyncCommitteeContribution<E>,
selection_proof: SyncSelectionProof,
) -> Result<SignedContributionAndProof<E>, Error> {
let signing_epoch = contribution.slot.epoch(self.slots_per_epoch);
let signing_epoch = contribution.slot.epoch(E::slots_per_epoch());
let signing_context = self.signing_context(Domain::ContributionAndProof, signing_epoch);

// Bypass `with_validator_signing_method`: sync committee messages are not slashable.
Expand Down Expand Up @@ -1058,7 +1055,7 @@ impl<T: SlotClock + 'static> ValidatorStore<T> {
/// This function will only do actual pruning periodically, so it should usually be
/// cheap to call. The `first_run` flag can be used to print a more verbose message when pruning
/// runs.
pub fn prune_slashing_protection_db(&self, current_epoch: Epoch, first_run: bool) {
pub fn prune_slashing_protection_db<E: EthSpec>(&self, current_epoch: Epoch, first_run: bool) {
// Attempt to prune every SLASHING_PROTECTION_HISTORY_EPOCHs, with a tolerance for
// missing the epoch that aligns exactly.
let mut last_prune = self.slashing_protection_last_prune.lock();
Expand All @@ -1083,7 +1080,7 @@ impl<T: SlotClock + 'static> ValidatorStore<T> {
validator_metrics::start_timer(&validator_metrics::SLASHING_PROTECTION_PRUNE_TIMES);

let new_min_target_epoch = current_epoch.saturating_sub(SLASHING_PROTECTION_HISTORY_EPOCHS);
let new_min_slot = new_min_target_epoch.start_slot(self.slots_per_epoch);
let new_min_slot = new_min_target_epoch.start_slot(E::slots_per_epoch());

let all_pubkeys: Vec<_> = self.voting_pubkeys(DoppelgangerStatus::ignored);

Expand Down

0 comments on commit 6533a67

Please sign in to comment.