Skip to content

Commit

Permalink
Move crypto checks in the approval-distribution
Browse files Browse the repository at this point in the history 
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
  • Loading branch information
@alexggh
alexggh committed Jul 2, 2024
1 parent 78708d7 commit 88e15da
Show file tree
Hide file tree
Showing 10 changed files with 209 additions and 254 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
View file

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

4 changes: 2 additions & 2 deletions polkadot/node/core/approval-voting/src/criteria.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ impl<'a> From<&'a SessionInfo> for Config {
}

/// A trait for producing and checking assignments. Used to mock.
pub(crate) trait AssignmentCriteria {
pub trait AssignmentCriteria {
fn compute_assignments(
&self,
keystore: &LocalKeystore,
Expand All @@ -276,7 +276,7 @@ pub(crate) trait AssignmentCriteria {
) -> Result<DelayTranche, InvalidAssignment>;
}

pub(crate) struct RealAssignmentCriteria;
pub struct RealAssignmentCriteria;

impl AssignmentCriteria for RealAssignmentCriteria {
fn compute_assignments(
Expand Down
7 changes: 6 additions & 1 deletion polkadot/node/core/approval-voting/src/import.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -574,9 +574,14 @@ pub(crate) async fn handle_new_head<Context, B: Backend>(
hash: block_hash,
number: block_header.number,
parent_hash: block_header.parent_hash,
candidates: included_candidates.iter().map(|(hash, _, _, _)| *hash).collect(),
candidates: included_candidates
.iter()
.map(|(hash, _, core_index, group_index)| (*hash, *core_index, *group_index))
.collect(),
slot,
session: session_index,
session_info: session_info.clone(),
vrf_story: relay_vrf_story,
});

imported_candidates.push(BlockImportedCandidates {
Expand Down
174 changes: 53 additions & 121 deletions polkadot/node/core/approval-voting/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,8 @@ use polkadot_node_subsystem_util::{
};
use polkadot_primitives::{
ApprovalVoteMultipleCandidates, ApprovalVotingParams, BlockNumber, CandidateHash,
CandidateIndex, CandidateReceipt, CoreIndex, DisputeStatement, ExecutorParams, GroupIndex,
Hash, PvfExecKind, SessionIndex, SessionInfo, ValidDisputeStatementKind, ValidatorId,
ValidatorIndex, ValidatorPair, ValidatorSignature,
CandidateIndex, CandidateReceipt, CoreIndex, ExecutorParams, GroupIndex, Hash, PvfExecKind,
SessionIndex, SessionInfo, ValidatorId, ValidatorIndex, ValidatorPair, ValidatorSignature,
};
use sc_keystore::LocalKeystore;
use sp_application_crypto::Pair;
Expand Down Expand Up @@ -91,7 +90,7 @@ use schnellru::{ByLength, LruMap};

use approval_checking::RequiredTranches;
use bitvec::{order::Lsb0, vec::BitVec};
use criteria::{AssignmentCriteria, RealAssignmentCriteria};
pub use criteria::{AssignmentCriteria, Config as AssignmentConfig, RealAssignmentCriteria};
use persisted_entries::{ApprovalEntry, BlockEntry, CandidateEntry};
use time::{slot_number_to_tick, Clock, ClockExt, DelayedApprovalTimer, SystemClock, Tick};

Expand Down Expand Up @@ -123,7 +122,7 @@ const APPROVAL_CHECKING_TIMEOUT: Duration = Duration::from_secs(120);
const WAIT_FOR_SIGS_TIMEOUT: Duration = Duration::from_millis(500);
const APPROVAL_CACHE_SIZE: u32 = 1024;

const TICK_TOO_FAR_IN_FUTURE: Tick = 20; // 10 seconds.
pub const TICK_TOO_FAR_IN_FUTURE: Tick = 20; // 10 seconds.
const APPROVAL_DELAY: Tick = 2;
pub(crate) const LOG_TARGET: &str = "parachain::approval-voting";

Expand Down Expand Up @@ -1603,14 +1602,39 @@ async fn distribution_messages_for_activation<Context>(
distribution_message_span.add_string_tag("block-hash", &block_hash.to_string());
distribution_message_span
.add_string_tag("parent-hash", &block_entry.parent_hash().to_string());
let ExtendedSessionInfo { ref session_info, .. } = match get_extended_session_info(
session_info_provider,
sender,
block_entry.block_hash(),
block_entry.session(),
)
.await
{
Some(i) => i,
None => continue,
};
approval_meta.push(BlockApprovalMeta {
hash: block_hash,
number: block_entry.block_number(),
parent_hash: block_entry.parent_hash(),
candidates: block_entry.candidates().iter().map(|(_, c_hash)| *c_hash).collect(),
candidates: block_entry
.candidates()
.iter()
.flat_map(|(core_index, c_hash)| {
let candidate = db.load_candidate_entry(c_hash).ok().flatten();
candidate
.and_then(|entry| {
entry.approval_entry(&block_hash).map(|entry| entry.backing_group())
})
.and_then(|group_index| Some((*c_hash, *core_index, group_index)))
})
.collect(),
slot: block_entry.slot(),
session: block_entry.session(),
vrf_story: block_entry.relay_vrf_story(),
session_info: session_info.clone(),
});

let mut signatures_queued = HashSet::new();
for (core_index, candidate_hash) in block_entry.candidates() {
let _candidate_span =
Expand Down Expand Up @@ -1872,35 +1896,38 @@ async fn handle_from_overseer<Context>(
vec![Action::Conclude]
},
FromOrchestra::Communication { msg } => match msg {
ApprovalVotingMessage::CheckAndImportAssignment(a, claimed_cores, res) => {
ApprovalVotingMessage::CheckAndImportAssignment(a, claimed_cores, tranche) => {
let (check_outcome, actions) = check_and_import_assignment(
ctx.sender(),
state,
db,
session_info_provider,
a,
claimed_cores,
tranche,
)
.await?;
let _ = res.send(check_outcome);

if let AssignmentCheckResult::Bad(err) = check_outcome {
gum::error!(target: LOG_TARGET, ?err, "Unexpected fail to check and import assignment");
}
actions
},
ApprovalVotingMessage::CheckAndImportApproval(a, res) =>
check_and_import_approval(
ApprovalVotingMessage::CheckAndImportApproval(a) => {
let result = check_and_import_approval(
ctx.sender(),
state,
db,
session_info_provider,
metrics,
a,
|r| {
let _ = res.send(r);
},
&wakeups,
)
.await?
.0,
.await?;
if let ApprovalCheckResult::Bad(err) = result.1 {
gum::error!(target: LOG_TARGET, ?err, "Unexpected fail to check and import approval");
}
result.0
},
ApprovalVotingMessage::ApprovedAncestor(target, lower_bound, res) => {
let mut approved_ancestor_span = state
.spans
Expand Down Expand Up @@ -2446,6 +2473,7 @@ async fn check_and_import_assignment<Sender>(
session_info_provider: &mut RuntimeInfo,
assignment: IndirectAssignmentCertV2,
candidate_indices: CandidateBitfield,
tranche: DelayTranche,
) -> SubsystemResult<(AssignmentCheckResult, Vec<Action>)>
where
Sender: SubsystemSender<RuntimeApiMessage>,
Expand Down Expand Up @@ -2579,42 +2607,6 @@ where
))
}

// Check the assignment certificate.
let res = state.assignment_criteria.check_assignment_cert(
claimed_core_indices
.clone()
.try_into()
.expect("Checked for null assignment above; qed"),
assignment.validator,
&criteria::Config::from(session_info),
block_entry.relay_vrf_story(),
&assignment.cert,
backing_groups,
);

let tranche = match res {
Err(crate::criteria::InvalidAssignment(reason)) =>
return Ok((
AssignmentCheckResult::Bad(AssignmentCheckError::InvalidCert(
assignment.validator,
format!("{:?}", reason),
)),
Vec::new(),
)),
Ok(tranche) => {
let current_tranche =
state.clock.tranche_now(state.slot_duration_millis, block_entry.slot());

let too_far_in_future = current_tranche + TICK_TOO_FAR_IN_FUTURE as DelayTranche;

if tranche >= too_far_in_future {
return Ok((AssignmentCheckResult::TooFarInFuture, Vec::new()))
}

tranche
},
};

let mut actions = Vec::new();
let res = {
let mut is_duplicate = true;
Expand Down Expand Up @@ -2704,23 +2696,21 @@ where
Ok((res, actions))
}

async fn check_and_import_approval<T, Sender>(
async fn check_and_import_approval<Sender>(
sender: &mut Sender,
state: &mut State,
db: &mut OverlayedBackend<'_, impl Backend>,
session_info_provider: &mut RuntimeInfo,
metrics: &Metrics,
approval: IndirectSignedApprovalVoteV2,
with_response: impl FnOnce(ApprovalCheckResult) -> T,
wakeups: &Wakeups,
) -> SubsystemResult<(Vec<Action>, T)>
) -> SubsystemResult<(Vec<Action>, ApprovalCheckResult)>
where
Sender: SubsystemSender<RuntimeApiMessage>,
{
macro_rules! respond_early {
($e: expr) => {{
let t = with_response($e);
return Ok((Vec::new(), t))
return Ok((Vec::new(), $e))
}};
}
let mut span = state
Expand Down Expand Up @@ -2774,67 +2764,11 @@ where
),
);

{
let session_info = match get_session_info(
session_info_provider,
sender,
approval.block_hash,
block_entry.session(),
)
.await
{
Some(s) => s,
None => {
respond_early!(ApprovalCheckResult::Bad(ApprovalCheckError::UnknownSessionIndex(
block_entry.session()
),))
},
};

let pubkey = match session_info.validators.get(approval.validator) {
Some(k) => k,
None => respond_early!(ApprovalCheckResult::Bad(
ApprovalCheckError::InvalidValidatorIndex(approval.validator),
)),
};

gum::trace!(
target: LOG_TARGET,
"Received approval for num_candidates {:}",
approval.candidate_indices.count_ones()
);

let candidate_hashes: Vec<CandidateHash> =
approved_candidates_info.iter().map(|candidate| candidate.1).collect();
// Signature check:
match DisputeStatement::Valid(
ValidDisputeStatementKind::ApprovalCheckingMultipleCandidates(candidate_hashes.clone()),
)
.check_signature(
&pubkey,
if let Some(candidate_hash) = candidate_hashes.first() {
*candidate_hash
} else {
respond_early!(ApprovalCheckResult::Bad(ApprovalCheckError::InvalidValidatorIndex(
approval.validator
),))
},
block_entry.session(),
&approval.signature,
) {
Err(_) => {
gum::error!(
target: LOG_TARGET,
"Error while checking signature {:}",
approval.candidate_indices.count_ones()
);
respond_early!(ApprovalCheckResult::Bad(ApprovalCheckError::InvalidSignature(
approval.validator
),))
},
Ok(()) => {},
};
}
gum::trace!(
target: LOG_TARGET,
"Received approval for num_candidates {:}",
approval.candidate_indices.count_ones()
);

let mut actions = Vec::new();
for (approval_candidate_index, approved_candidate_hash) in approved_candidates_info {
Expand Down Expand Up @@ -2898,9 +2832,7 @@ where
}

// importing the approval can be heavy as it may trigger acceptance for a series of blocks.
let t = with_response(ApprovalCheckResult::Accepted);

Ok((actions, t))
Ok((actions, ApprovalCheckResult::Accepted))
}

#[derive(Debug)]
Expand Down
1 change: 1 addition & 0 deletions polkadot/node/network/approval-distribution/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ license.workspace = true
workspace = true

[dependencies]
polkadot-node-core-approval-voting = { workspace = true, default-features = true }
polkadot-node-metrics = { workspace = true, default-features = true }
polkadot-node-network-protocol = { workspace = true, default-features = true }
polkadot-node-primitives = { workspace = true, default-features = true }
Expand Down
Loading

0 comments on commit 88e15da

Please sign in to comment.