Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Fix cycle dispute-coordinator <-> dispute-distribution #6489

Merged
merged 24 commits into from
Jan 10, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
dispute-coordinator: Send disputes on startup.
+ Some fixes, cleanup.
  • Loading branch information
eskimor committed Dec 30, 2022
commit 033a4f6ee9d213d704b1592ef104ed6c5403cfae
1 change: 1 addition & 0 deletions node/core/dispute-coordinator/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ assert_matches = "1.4.0"
test-helpers = { package = "polkadot-primitives-test-helpers", path = "../../../primitives/test-helpers" }
futures-timer = "3.0.2"
sp-application-crypto = { git = "https://github.com/paritytech/substrate", branch = "master" }
sp-tracing = { git = "https://github.com/paritytech/substrate", branch = "master" }

[features]
# If not enabled, the dispute coordinator will do nothing.
Expand Down
108 changes: 65 additions & 43 deletions node/core/dispute-coordinator/src/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,60 +87,68 @@ impl<'a> CandidateEnvironment<'a> {

/// Whether or not we already issued some statement about a candidate.
pub enum OwnVoteState {
/// We already voted/issued a statement for the candidate.
Voted,
/// We already voted/issued a statement for the candidate and it was an approval vote.
/// Our votes, if any.
Voted(Vec<(ValidatorIndex, (DisputeStatement, ValidatorSignature))>),
/// We are not a parachain validator in the session.
///
/// Needs special treatment as we have to make sure to propagate it to peers, to guarantee the
/// dispute can conclude.
VotedApproval(Vec<(ValidatorIndex, ValidatorSignature)>),
/// We not yet voted for the dispute.
NoVote,
/// Hence we cannot vote.
CannotVote,
}

impl OwnVoteState {
fn new<'a>(votes: &CandidateVotes, env: &CandidateEnvironment<'a>) -> Self {
let mut our_valid_votes = env
.controlled_indices()
let controlled_indices = env.controlled_indices();
if controlled_indices.is_empty() {
return Self::CannotVote
}

let our_valid_votes = controlled_indices
.iter()
.filter_map(|i| votes.valid.raw().get_key_value(i))
.peekable();
let mut our_invalid_votes =
env.controlled_indices.iter().filter_map(|i| votes.invalid.get_key_value(i));
let has_valid_votes = our_valid_votes.peek().is_some();
let has_invalid_votes = our_invalid_votes.next().is_some();
let our_approval_votes: Vec<_> = our_valid_votes
.filter_map(|(index, (k, sig))| {
if let ValidDisputeStatementKind::ApprovalChecking = k {
Some((*index, sig.clone()))
} else {
None
}
})
.collect();
.map(|(index, (kind, sig))| (*index, (DisputeStatement::Valid(*kind), sig.clone())));
let our_invalid_votes = controlled_indices
.iter()
.filter_map(|i| votes.invalid.get_key_value(i))
.map(|(index, (kind, sig))| (*index, (DisputeStatement::Invalid(*kind), sig.clone())));

if !our_approval_votes.is_empty() {
return Self::VotedApproval(our_approval_votes)
}
if has_valid_votes || has_invalid_votes {
return Self::Voted
}
Self::NoVote
Self::Voted(our_valid_votes.chain(our_invalid_votes).collect())
}

/// Whether or not we issued a statement for the candidate already.
fn voted(&self) -> bool {
/// Is a vote from us missing but we are a validator able to vote?
fn vote_missing(&self) -> bool {
match self {
Self::Voted | Self::VotedApproval(_) => true,
Self::NoVote => false,
Self::Voted(votes) if votes.is_empty() => true,
Self::Voted(_) | Self::CannotVote => false,
}
}

/// Get own approval votes, if any.
fn approval_votes(&self) -> Option<&Vec<(ValidatorIndex, ValidatorSignature)>> {
///
/// Empty iterator means, no approval votes. `None` means, there will never be any (we cannot
/// vote).
fn approval_votes(
&self,
) -> Option<impl Iterator<Item = (ValidatorIndex, &ValidatorSignature)>> {
match self {
Self::VotedApproval(votes) => Some(&votes),
_ => None,
Self::Voted(votes) => Some(votes.iter().filter_map(|(index, (kind, sig))| {
if let DisputeStatement::Valid(ValidDisputeStatementKind::ApprovalChecking) = kind {
Some((*index, sig))
} else {
None
}
})),
Self::CannotVote => None,
}
}

/// Get our votes if there are any.
///
/// Empty iterator means, no votes. `None` means, there will never be any (we cannot
/// vote).
fn votes(&self) -> Option<&Vec<(ValidatorIndex, (DisputeStatement, ValidatorSignature))>> {
match self {
Self::Voted(votes) => Some(&votes),
Self::CannotVote => None,
}
}
}
Expand Down Expand Up @@ -170,7 +178,7 @@ impl CandidateVoteState<CandidateVotes> {
valid: ValidCandidateVotes::new(),
invalid: BTreeMap::new(),
};
Self { votes, own_vote: OwnVoteState::NoVote, dispute_status: None }
Self { votes, own_vote: OwnVoteState::CannotVote, dispute_status: None }
}

/// Create a new `CandidateVoteState` from already existing votes.
Expand Down Expand Up @@ -327,16 +335,25 @@ impl<V> CandidateVoteState<V> {
self.dispute_status.map_or(false, |s| s.is_confirmed_concluded())
}

/// This machine already cast some vote in that dispute/for that candidate.
pub fn has_own_vote(&self) -> bool {
self.own_vote.voted()
/// Are we a validator in the session, but have not yet voted?
pub fn own_vote_missing(&self) -> bool {
self.own_vote.vote_missing()
}

/// Own approval votes if any:
pub fn own_approval_votes(&self) -> Option<&Vec<(ValidatorIndex, ValidatorSignature)>> {
pub fn own_approval_votes(
&self,
) -> Option<impl Iterator<Item = (ValidatorIndex, &ValidatorSignature)>> {
self.own_vote.approval_votes()
}

/// Get own votes if there are any.
pub fn own_votes(
&self,
) -> Option<&Vec<(ValidatorIndex, (DisputeStatement, ValidatorSignature))>> {
self.own_vote.votes()
}

/// Whether or not there is a dispute and it has already enough valid votes to conclude.
pub fn has_concluded_for(&self) -> bool {
self.dispute_status.map_or(false, |s| s.has_concluded_for())
Expand All @@ -347,6 +364,11 @@ impl<V> CandidateVoteState<V> {
self.dispute_status.map_or(false, |s| s.has_concluded_against())
}

/// Has concluded. Either for or agains the candidate.
pub fn has_concluded(&self) -> bool {
self.has_concluded_against() || self.has_concluded_for()
}

/// Get access to the dispute status, in case there is one.
pub fn dispute_status(&self) -> &Option<DisputeStatus> {
&self.dispute_status
Expand Down
104 changes: 16 additions & 88 deletions node/core/dispute-coordinator/src/initialized.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ use futures::{
use sc_keystore::LocalKeystore;

use polkadot_node_primitives::{
disputes::ValidCandidateVotes, CandidateVotes, DisputeMessage, DisputeMessageCheckError,
DisputeStatus, SignedDisputeStatement, Timestamp,
disputes::ValidCandidateVotes, CandidateVotes, DisputeStatus, SignedDisputeStatement, Timestamp,
};
use polkadot_node_subsystem::{
messages::{
Expand All @@ -48,14 +47,15 @@ use polkadot_primitives::v2::{
use crate::{
error::{log_error, Error, FatalError, FatalResult, JfyiError, JfyiResult, Result},
import::{CandidateEnvironment, CandidateVoteState},
is_potential_spam,
metrics::Metrics,
status::{get_active_with_status, Clock},
DisputeCoordinatorSubsystem, LOG_TARGET,
};

use super::{
backend::Backend,
db,
db, make_dispute_message,
participation::{
self, Participation, ParticipationPriority, ParticipationRequest, ParticipationStatement,
WorkerMessageReceiver,
Expand Down Expand Up @@ -845,18 +845,16 @@ impl Initialized {

let is_included = self.scraper.is_candidate_included(&candidate_hash);
let is_backed = self.scraper.is_candidate_backed(&candidate_hash);
let has_own_vote = new_state.has_own_vote();
let own_vote_missing = new_state.own_vote_missing();
let is_disputed = new_state.is_disputed();
let has_controlled_indices = !env.controlled_indices().is_empty();
let is_confirmed = new_state.is_confirmed();
let potential_spam =
!is_included && !is_backed && !new_state.is_confirmed() && !new_state.has_own_vote();
// We participate only in disputes which are included, backed or confirmed
let allow_participation = is_included || is_backed || is_confirmed;
let potential_spam = is_potential_spam(&self.scraper, &new_state, &candidate_hash);
// We participate only in disputes which are not potential spam.
let allow_participation = !potential_spam;

gum::trace!(
target: LOG_TARGET,
has_own_vote = ?new_state.has_own_vote(),
?own_vote_missing,
?potential_spam,
?is_included,
?candidate_hash,
Expand Down Expand Up @@ -903,7 +901,7 @@ impl Initialized {
// - `is_included` lands in prioritised queue
// - `is_confirmed` | `is_backed` lands in best effort queue
// We don't participate in disputes on finalized candidates.
if !has_own_vote && is_disputed && has_controlled_indices && allow_participation {
if own_vote_missing && is_disputed && allow_participation {
let priority = ParticipationPriority::with_priority_if(is_included);
gum::trace!(
target: LOG_TARGET,
Expand All @@ -930,9 +928,8 @@ impl Initialized {
target: LOG_TARGET,
?candidate_hash,
?is_confirmed,
?has_own_vote,
?own_vote_missing,
?is_disputed,
?has_controlled_indices,
?allow_participation,
?is_included,
?is_backed,
Expand All @@ -946,10 +943,9 @@ impl Initialized {

// Also send any already existing approval vote on new disputes:
if import_result.is_freshly_disputed() {
let no_votes = Vec::new();
let our_approval_votes = new_state.own_approval_votes().unwrap_or(&no_votes);
let our_approval_votes = new_state.own_approval_votes().into_iter().flatten();
for (validator_index, sig) in our_approval_votes {
let pub_key = match env.validators().get(*validator_index) {
let pub_key = match env.validators().get(validator_index) {
None => {
gum::error!(
target: LOG_TARGET,
Expand Down Expand Up @@ -979,7 +975,7 @@ impl Initialized {
env.session_info(),
&new_state.votes(),
statement,
*validator_index,
validator_index,
) {
Err(err) => {
gum::error!(
Expand Down Expand Up @@ -1150,9 +1146,9 @@ impl Initialized {
Ok(None) => {},
Err(e) => {
gum::error!(
target: LOG_TARGET,
err = ?e,
"Encountered keystore error while signing dispute statement",
target: LOG_TARGET,
err = ?e,
"Encountered keystore error while signing dispute statement",
);
},
}
Expand Down Expand Up @@ -1251,74 +1247,6 @@ impl MaybeCandidateReceipt {
}
}

#[derive(Debug, thiserror::Error)]
enum DisputeMessageCreationError {
#[error("There was no opposite vote available")]
NoOppositeVote,
#[error("Found vote had an invalid validator index that could not be found")]
InvalidValidatorIndex,
#[error("Statement found in votes had invalid signature.")]
InvalidStoredStatement,
#[error(transparent)]
InvalidStatementCombination(DisputeMessageCheckError),
}

fn make_dispute_message(
info: &SessionInfo,
votes: &CandidateVotes,
our_vote: SignedDisputeStatement,
our_index: ValidatorIndex,
) -> std::result::Result<DisputeMessage, DisputeMessageCreationError> {
let validators = &info.validators;

let (valid_statement, valid_index, invalid_statement, invalid_index) =
if let DisputeStatement::Valid(_) = our_vote.statement() {
let (validator_index, (statement_kind, validator_signature)) =
votes.invalid.iter().next().ok_or(DisputeMessageCreationError::NoOppositeVote)?;
let other_vote = SignedDisputeStatement::new_checked(
DisputeStatement::Invalid(*statement_kind),
*our_vote.candidate_hash(),
our_vote.session_index(),
validators
.get(*validator_index)
.ok_or(DisputeMessageCreationError::InvalidValidatorIndex)?
.clone(),
validator_signature.clone(),
)
.map_err(|()| DisputeMessageCreationError::InvalidStoredStatement)?;
(our_vote, our_index, other_vote, *validator_index)
} else {
let (validator_index, (statement_kind, validator_signature)) = votes
.valid
.raw()
.iter()
.next()
.ok_or(DisputeMessageCreationError::NoOppositeVote)?;
let other_vote = SignedDisputeStatement::new_checked(
DisputeStatement::Valid(*statement_kind),
*our_vote.candidate_hash(),
our_vote.session_index(),
validators
.get(*validator_index)
.ok_or(DisputeMessageCreationError::InvalidValidatorIndex)?
.clone(),
validator_signature.clone(),
)
.map_err(|()| DisputeMessageCreationError::InvalidStoredStatement)?;
(other_vote, *validator_index, our_vote, our_index)
};

DisputeMessage::from_signed_statements(
valid_statement,
valid_index,
invalid_statement,
invalid_index,
votes.candidate_receipt.clone(),
info,
)
.map_err(DisputeMessageCreationError::InvalidStatementCombination)
}

/// Determine the best block and its block number.
/// Assumes `block_descriptions` are sorted from the one
/// with the lowest `BlockNumber` to the highest.
Expand Down
Loading