Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions Cargo.lock

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

29 changes: 5 additions & 24 deletions polkadot/node/core/dispute-coordinator/src/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,12 @@ use polkadot_node_primitives::{
disputes::ValidCandidateVotes, CandidateVotes, DisputeStatus, SignedDisputeStatement, Timestamp,
};
use polkadot_node_subsystem::overseer;
use polkadot_node_subsystem_util::runtime::RuntimeInfo;
use polkadot_node_subsystem_util::{runtime::RuntimeInfo, ControlledValidatorIndices};
use polkadot_primitives::{
vstaging::CandidateReceiptV2 as CandidateReceipt, CandidateHash, DisputeStatement,
ExecutorParams, Hash, IndexedVec, SessionIndex, SessionInfo, ValidDisputeStatementKind,
ValidatorId, ValidatorIndex, ValidatorPair, ValidatorSignature,
ValidatorId, ValidatorIndex, ValidatorSignature,
};
use sc_keystore::LocalKeystore;

use crate::LOG_TARGET;

Expand All @@ -63,12 +62,12 @@ impl<'a> CandidateEnvironment<'a> {
///
/// Return: `None` in case session is outside of session window.
pub async fn new<Context>(
keystore: &LocalKeystore,
ctx: &mut Context,
runtime_info: &'a mut RuntimeInfo,
session_index: SessionIndex,
relay_parent: Hash,
disabled_offchain: impl IntoIterator<Item = ValidatorIndex>,
controlled_indices: &mut ControlledValidatorIndices,
) -> Option<CandidateEnvironment<'a>> {
let disabled_onchain = runtime_info
.get_disabled_validators(ctx.sender(), relay_parent)
Expand Down Expand Up @@ -105,7 +104,8 @@ impl<'a> CandidateEnvironment<'a> {
d
};

let controlled_indices = find_controlled_validator_indices(keystore, &session.validators);
let controlled_indices = controlled_indices.get(session_index, &session.validators).clone();

Some(Self { session_index, session, executor_params, controlled_indices, disabled_indices })
}

Expand Down Expand Up @@ -632,22 +632,3 @@ impl ImportResult {
}
}
}

/// Find indices controlled by this validator.
///
/// That is all `ValidatorIndex`es we have private keys for. Usually this will only be one.
fn find_controlled_validator_indices(
keystore: &LocalKeystore,
validators: &IndexedVec<ValidatorIndex, ValidatorId>,
) -> HashSet<ValidatorIndex> {
let mut controlled = HashSet::new();
for (index, validator) in validators.iter().enumerate() {
if keystore.key_pair::<ValidatorPair>(validator).ok().flatten().is_none() {
continue
}

controlled.insert(ValidatorIndex(index as _));
}

controlled
}
13 changes: 9 additions & 4 deletions polkadot/node/core/dispute-coordinator/src/initialized.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,9 @@ use polkadot_node_subsystem::{
},
overseer, ActivatedLeaf, ActiveLeavesUpdate, FromOrchestra, OverseerSignal, RuntimeApiError,
};
use polkadot_node_subsystem_util::runtime::{
self, key_ownership_proof, submit_report_dispute_lost, RuntimeInfo,
use polkadot_node_subsystem_util::{
runtime::{self, key_ownership_proof, submit_report_dispute_lost, RuntimeInfo},
ControlledValidatorIndices,
};
use polkadot_primitives::{
slashing,
Expand Down Expand Up @@ -98,6 +99,8 @@ pub(crate) struct Initialized {
/// We have the onchain state of disabled validators as well as the offchain
/// state that is based on the lost disputes.
offchain_disabled_validators: OffchainDisabledValidators,
/// The indices of the controlled validators, cached by session.
controlled_validator_indices: ControlledValidatorIndices,
/// This is the highest `SessionIndex` seen via `ActiveLeavesUpdate`. It doesn't matter if it
/// was cached successfully or not. It is used to detect ancient disputes.
highest_session_seen: SessionIndex,
Expand Down Expand Up @@ -133,6 +136,7 @@ impl Initialized {
highest_session_seen: SessionIndex,
gaps_in_cache: bool,
offchain_disabled_validators: OffchainDisabledValidators,
controlled_validator_indices: ControlledValidatorIndices,
) -> Self {
let DisputeCoordinatorSubsystem {
config: _,
Expand All @@ -149,6 +153,7 @@ impl Initialized {
keystore,
runtime_info,
offchain_disabled_validators,
controlled_validator_indices,
highest_session_seen,
gaps_in_cache,
spam_slots,
Expand Down Expand Up @@ -975,12 +980,12 @@ impl Initialized {
};

let env = match CandidateEnvironment::new(
&self.keystore,
ctx,
&mut self.runtime_info,
session,
relay_parent,
self.offchain_disabled_validators.iter(session),
&mut self.controlled_validator_indices,
)
.await
{
Expand Down Expand Up @@ -1450,12 +1455,12 @@ impl Initialized {

// Load environment:
let env = match CandidateEnvironment::new(
&self.keystore,
ctx,
&mut self.runtime_info,
session,
candidate_receipt.descriptor.relay_parent(),
self.offchain_disabled_validators.iter(session),
&mut self.controlled_validator_indices,
)
.await
{
Expand Down
9 changes: 8 additions & 1 deletion polkadot/node/core/dispute-coordinator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ use polkadot_node_subsystem::{
use polkadot_node_subsystem_util::{
database::Database,
runtime::{Config as RuntimeInfoConfig, RuntimeInfo},
ControlledValidatorIndices,
};
use polkadot_primitives::{
vstaging::ScrapedOnChainVotes, DisputeStatement, SessionIndex, SessionInfo, ValidatorIndex,
Expand Down Expand Up @@ -236,6 +237,7 @@ impl DisputeCoordinatorSubsystem {
highest_session_seen,
gaps_in_cache,
offchain_disabled_validators,
controlled_validator_indices,
) = match self
.handle_startup(ctx, first_leaf.clone(), &mut runtime_info, &mut overlay_db, clock)
.await
Expand Down Expand Up @@ -263,6 +265,7 @@ impl DisputeCoordinatorSubsystem {
highest_session_seen,
gaps_in_cache,
offchain_disabled_validators,
controlled_validator_indices,
),
backend,
)))
Expand All @@ -289,6 +292,7 @@ impl DisputeCoordinatorSubsystem {
SessionIndex,
bool,
initialized::OffchainDisabledValidators,
ControlledValidatorIndices,
)> {
let now = clock.now();

Expand Down Expand Up @@ -357,16 +361,18 @@ impl DisputeCoordinatorSubsystem {

let mut participation_requests = Vec::new();
let mut spam_disputes: UnconfirmedDisputes = UnconfirmedDisputes::new();
let mut controlled_indices =
ControlledValidatorIndices::new(self.keystore.clone(), DISPUTE_WINDOW.get());
let leaf_hash = initial_head.hash;
let (scraper, votes) = ChainScraper::new(ctx.sender(), initial_head).await?;
for ((session, ref candidate_hash), _) in active_disputes {
let env = match CandidateEnvironment::new(
&self.keystore,
ctx,
runtime_info,
highest_session,
leaf_hash,
offchain_disabled_validators.iter(session),
&mut controlled_indices,
)
.await
{
Expand Down Expand Up @@ -452,6 +458,7 @@ impl DisputeCoordinatorSubsystem {
highest_session,
gap_in_cache,
offchain_disabled_validators,
controlled_indices,
))
}
}
Expand Down
1 change: 1 addition & 0 deletions polkadot/node/subsystem-util/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ polkadot-primitives.default-features = true
polkadot-primitives.workspace = true
sc-client-api.default-features = true
sc-client-api.workspace = true
sc-keystore.workspace = true
sp-application-crypto.default-features = true
sp-application-crypto.workspace = true
sp-core.default-features = true
Expand Down
77 changes: 77 additions & 0 deletions polkadot/node/subsystem-util/src/controlled_validator_indices.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// Copyright (C) Parity Technologies (UK) Ltd.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we using some tool to insert copy right headers? Or are we using some tool, e.g. https://github.com/Lucas-C/pre-commit-hooks ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do it by hand :)

// This file is part of Polkadot.

// Polkadot is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.

// Polkadot is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.

// You should have received a copy of the GNU General Public License
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.

//! `ControlledValidatorIndices` implementation.

use polkadot_primitives::{IndexedVec, SessionIndex, ValidatorId, ValidatorIndex, ValidatorPair};
use sc_keystore::LocalKeystore;
use schnellru::{ByLength, LruMap};
use sp_application_crypto::{AppCrypto, ByteArray};
use sp_keystore::Keystore;
use std::{collections::HashSet, sync::Arc};

/// Keeps track of the validator indices controlled by the local validator in a given session. For
/// better performance, the values for each session are cached.
pub struct ControlledValidatorIndices {
/// The indices of the controlled validators, cached by session.
controlled_validator_indices: LruMap<SessionIndex, HashSet<ValidatorIndex>>,
keystore: Arc<LocalKeystore>,
}

impl ControlledValidatorIndices {
/// Create a new instance of `ControlledValidatorIndices`.
pub fn new(keystore: Arc<LocalKeystore>, cache_size: u32) -> Self {
let controlled_validator_indices = LruMap::new(ByLength::new(cache_size));
Self { controlled_validator_indices, keystore }
}

/// Get the controlled validator indices for a given session. If the indices are not known they
/// will be fetched from `session_validators` and cached.
pub fn get(
Copy link
Contributor

@Sajjon Sajjon Jun 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: This get can be simplified a bit with LruMap::get_or_insert

like this:

pub fn get(
    &mut self,
    session: SessionIndex,
    session_validators: &IndexedVec<ValidatorIndex, ValidatorId>,
) -> &HashSet<ValidatorIndex> {
    self.controlled_validator_indices.get_or_insert(session, || {
        Self::find_controlled_validator_indices(&self.keystore, session_validators)
    }).expect("We just inserted the controlled indices; qed")
}

If the key is PartialEq and Hash, and looks like SessionIndex is a typealias for u32 so should be, right?

If I'm not mistaken :)

&mut self,
session: SessionIndex,
session_validators: &IndexedVec<ValidatorIndex, ValidatorId>,
) -> &HashSet<ValidatorIndex> {
if self.controlled_validator_indices.get(&session).is_none() {
let indices =
Self::find_controlled_validator_indices(&self.keystore, session_validators);
self.controlled_validator_indices.insert(session, indices.clone());
}

self.controlled_validator_indices
.get(&session)
.expect("We just inserted the controlled indices; qed")
}

/// Find indices controlled by this validator.
///
/// That is all `ValidatorIndex`es we have private keys for. Usually this will only be one.
fn find_controlled_validator_indices(
keystore: &LocalKeystore,
validators: &IndexedVec<ValidatorIndex, ValidatorId>,
) -> HashSet<ValidatorIndex> {
let mut controlled = HashSet::new();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this not just a filter and a map? So something like:

fn find_controlled_validator_indices(
    keystore: &LocalKeystore,
    validators: &IndexedVec<ValidatorIndex, ValidatorId>,
) -> HashSet<ValidatorIndex> {
    validators
        .iter()
        .enumerate()
        .filter(|(_, validator)| {
            Keystore::has_keys(keystore, &[(validator.to_raw_vec(), ValidatorPair::ID)])
        })
        .map(|(index, _)| ValidatorIndex(index as _))
        .collect()
}

I think?

for (index, validator) in validators.iter().enumerate() {
if !Keystore::has_keys(keystore, &[(validator.to_raw_vec(), ValidatorPair::ID)]) {
continue
}

controlled.insert(ValidatorIndex(index as _));
}

controlled
}
}
3 changes: 3 additions & 0 deletions polkadot/node/subsystem-util/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,9 @@ pub mod reputation;

mod determine_new_blocks;

mod controlled_validator_indices;
pub use controlled_validator_indices::ControlledValidatorIndices;

#[cfg(test)]
mod tests;

Expand Down
14 changes: 14 additions & 0 deletions prdoc/pr_8837.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
title: Cache locally controlled validator indices in dispute-coordinator
doc:
- audience: Node Dev
description: |
`dispute-coordinator` uses `keystore.key_pair()` to obtain the set of locally controlled
validator IDs. This operation happens on each import and is expensive because it involves key
generation from a seed phrase. This patch lazily determines the set of locally controlled
validator IDs and caches the result for each session.

crates:
- name: polkadot-node-core-dispute-coordinator
bump: minor
- name: polkadot-node-subsystem-util
bump: minor
Loading