Skip to content

Commit ca9dc8e

Browse files
committed
Optimise HTTP validator lookups (#3559)
## Issue Addressed While digging around in some logs I noticed that queries for validators by pubkey were taking 10ms+, which seemed too long. This was due to a loop through the entire validator registry for each lookup. ## Proposed Changes Rather than using a loop through the register, this PR utilises the pubkey cache which is usually initialised at the head*. In case the cache isn't built, we fall back to the previous loop logic. In the vast majority of cases I expect the cache will be built, as the validator client queries at the `head` where all caches should be built. ## Additional Info *I had to modify the cache build that runs after fork choice to build the pubkey cache. I think it had been optimised out, perhaps accidentally. I think it's preferable to have the exit cache and the pubkey cache built on the head state, as they are required for verifying deposits and exits respectively, and we may as well build them off the hot path of block processing. Previously they'd get built the first time a deposit or exit needed to be verified. I've deleted the unused `map_state` function which was obsoleted by `map_state_and_execution_optimistic`.
1 parent 9f24213 commit ca9dc8e

File tree

5 files changed

+40
-31
lines changed

5 files changed

+40
-31
lines changed

beacon_node/beacon_chain/src/canonical_head.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -654,11 +654,11 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
654654
})
655655
})
656656
.and_then(|mut snapshot| {
657-
// Regardless of where we got the state from, attempt to build the committee
658-
// caches.
657+
// Regardless of where we got the state from, attempt to build all the
658+
// caches except the tree hash cache.
659659
snapshot
660660
.beacon_state
661-
.build_all_committee_caches(&self.spec)
661+
.build_all_caches(&self.spec)
662662
.map_err(Into::into)
663663
.map(|()| snapshot)
664664
})?;

beacon_node/http_api/src/lib.rs

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -668,17 +668,34 @@ pub fn serve<T: BeaconChainTypes>(
668668
"Invalid validator ID".to_string(),
669669
))
670670
}))
671+
.and(log_filter.clone())
671672
.and(warp::path::end())
672673
.and_then(
673-
|state_id: StateId, chain: Arc<BeaconChain<T>>, validator_id: ValidatorId| {
674+
|state_id: StateId, chain: Arc<BeaconChain<T>>, validator_id: ValidatorId, log| {
674675
blocking_json_task(move || {
675676
let (data, execution_optimistic) = state_id
676677
.map_state_and_execution_optimistic(
677678
&chain,
678679
|state, execution_optimistic| {
679680
let index_opt = match &validator_id {
680681
ValidatorId::PublicKey(pubkey) => {
681-
state.validators().iter().position(|v| v.pubkey == *pubkey)
682+
// Fast path: use the pubkey cache which is probably
683+
// initialised at the head.
684+
match state.get_validator_index_read_only(pubkey) {
685+
Ok(result) => result,
686+
Err(e) => {
687+
// Slow path, fall back to iteration.
688+
debug!(
689+
log,
690+
"Validator look-up cache miss";
691+
"reason" => ?e,
692+
);
693+
state
694+
.validators()
695+
.iter()
696+
.position(|v| v.pubkey == *pubkey)
697+
}
698+
}
682699
}
683700
ValidatorId::Index(index) => Some(*index as usize),
684701
};

beacon_node/http_api/src/state_id.rs

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -155,33 +155,12 @@ impl StateId {
155155
Ok((state, execution_optimistic))
156156
}
157157

158-
/*
159158
/// Map a function across the `BeaconState` identified by `self`.
160159
///
160+
/// The optimistic status of the requested state is also provided to the `func` closure.
161+
///
161162
/// This function will avoid instantiating/copying a new state when `self` points to the head
162163
/// of the chain.
163-
#[allow(dead_code)]
164-
pub fn map_state<T: BeaconChainTypes, F, U>(
165-
&self,
166-
chain: &BeaconChain<T>,
167-
func: F,
168-
) -> Result<U, warp::Rejection>
169-
where
170-
F: Fn(&BeaconState<T::EthSpec>) -> Result<U, warp::Rejection>,
171-
{
172-
match &self.0 {
173-
CoreStateId::Head => chain
174-
.with_head(|snapshot| Ok(func(&snapshot.beacon_state)))
175-
.map_err(warp_utils::reject::beacon_chain_error)?,
176-
_ => func(&self.state(chain)?),
177-
}
178-
}
179-
*/
180-
181-
/// Functions the same as `map_state` but additionally computes the value of
182-
/// `execution_optimistic` of the state identified by `self`.
183-
///
184-
/// This is to avoid re-instantiating `state` unnecessarily.
185164
pub fn map_state_and_execution_optimistic<T: BeaconChainTypes, F, U>(
186165
&self,
187166
chain: &BeaconChain<T>,

consensus/state_processing/src/per_block_processing/verify_deposit.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,7 @@ pub fn verify_deposit_signature(deposit_data: &DepositData, spec: &ChainSpec) ->
2929
/// Returns a `Some(validator index)` if a pubkey already exists in the `validators`,
3030
/// otherwise returns `None`.
3131
///
32-
/// ## Errors
33-
///
34-
/// Errors if the state's `pubkey_cache` is not current.
32+
/// Builds the pubkey cache if it is not already built.
3533
pub fn get_existing_validator_index<T: EthSpec>(
3634
state: &mut BeaconState<T>,
3735
pub_key: &PublicKeyBytes,

consensus/types/src/beacon_state.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,21 @@ impl<T: EthSpec> BeaconState<T> {
447447
Ok(self.pubkey_cache().get(pubkey))
448448
}
449449

450+
/// Immutable variant of `get_validator_index` which errors if the cache is not up to date.
451+
pub fn get_validator_index_read_only(
452+
&self,
453+
pubkey: &PublicKeyBytes,
454+
) -> Result<Option<usize>, Error> {
455+
let pubkey_cache = self.pubkey_cache();
456+
if pubkey_cache.len() != self.validators().len() {
457+
return Err(Error::PubkeyCacheIncomplete {
458+
cache_len: pubkey_cache.len(),
459+
registry_len: self.validators().len(),
460+
});
461+
}
462+
Ok(pubkey_cache.get(pubkey))
463+
}
464+
450465
/// The epoch corresponding to `self.slot()`.
451466
pub fn current_epoch(&self) -> Epoch {
452467
self.slot().epoch(T::slots_per_epoch())

0 commit comments

Comments
 (0)