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

Commit

Permalink
Issue RevertBlocks as soon as a dispute has `byzantine threshold + …
Browse files Browse the repository at this point in the history
…1` invalid votes.
  • Loading branch information
tdimitrov committed Apr 29, 2023
1 parent 9d0a8da commit 344bbcb
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 10 deletions.
29 changes: 29 additions & 0 deletions node/core/dispute-coordinator/src/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,35 @@ impl ImportResult {
self.is_freshly_concluded_against() || self.is_freshly_concluded_for()
}

// Returns the number of invalid own votes from the state provided. This function is a helper for
// `has_fresh_byzantine_threshold_against`
fn get_own_invalid_votes<T>(state: &CandidateVoteState<T>) -> usize {
state
.own_votes()
.map(|votes| {
votes.iter().fold(0, |acc, (_, (vote, _))| match vote {
DisputeStatement::Invalid(_) => acc + 1,
_ => acc,
})
})
.unwrap_or(0)
}

/// Whether or not the invalid vote count for the dispute went beyond the byzantine threshold
/// after the last import
pub fn has_fresh_byzantine_threshold_against(&self, byzantine_threshold: usize) -> bool {
// Total number of imported invalid votes + our own invalid votes
let imported_invalid_votes =
self.imported_invalid_votes() as usize + Self::get_own_invalid_votes(self.new_state());
// The number of invalid votes + our own invalid votes before the import
let old_invalid_votes = (self.imported_invalid_votes() as usize)
.saturating_sub(self.new_invalid_voters().len()) +
Self::get_own_invalid_votes(self.old_state());

old_invalid_votes as usize <= byzantine_threshold &&
imported_invalid_votes as usize > byzantine_threshold
}

/// Modify this `ImportResult`s, by importing additional approval votes.
///
/// Both results and `new_state` will be changed as if those approval votes had been in the
Expand Down
10 changes: 6 additions & 4 deletions node/core/dispute-coordinator/src/initialized.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ use polkadot_node_subsystem::{
};
use polkadot_node_subsystem_util::runtime::RuntimeInfo;
use polkadot_primitives::{
BlockNumber, CandidateHash, CandidateReceipt, CompactStatement, DisputeStatement,
DisputeStatementSet, Hash, ScrapedOnChainVotes, SessionIndex, ValidDisputeStatementKind,
ValidatorId, ValidatorIndex,
byzantine_threshold, BlockNumber, CandidateHash, CandidateReceipt, CompactStatement,
DisputeStatement, DisputeStatementSet, Hash, ScrapedOnChainVotes, SessionIndex,
ValidDisputeStatementKind, ValidatorId, ValidatorIndex,
};

use crate::{
Expand Down Expand Up @@ -1094,7 +1094,9 @@ impl Initialized {

// Notify ChainSelection if a dispute has concluded against a candidate. ChainSelection
// will need to mark the candidate's relay parent as reverted.
if import_result.is_freshly_concluded_against() {
if import_result
.has_fresh_byzantine_threshold_against(byzantine_threshold(env.validators().len()))
{
let blocks_including = self.scraper.get_blocks_including_candidate(&candidate_hash);
for (parent_block_number, parent_block_hash) in &blocks_including {
gum::trace!(
Expand Down
13 changes: 7 additions & 6 deletions node/core/dispute-coordinator/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3284,8 +3284,8 @@ fn informs_chain_selection_when_dispute_concluded_against() {
)
.await;

let supermajority_threshold =
polkadot_primitives::supermajority_threshold(test_state.validators.len());
let byzantine_threshold =
polkadot_primitives::byzantine_threshold(test_state.validators.len());

let (valid_vote, invalid_vote) = generate_opposing_votes_pair(
&test_state,
Expand Down Expand Up @@ -3326,8 +3326,8 @@ fn informs_chain_selection_when_dispute_concluded_against() {
.await;

let mut statements = Vec::new();
// minus 2, because of local vote and one previously imported invalid vote.
for i in (0_u32..supermajority_threshold as u32 - 2).map(|i| i + 3) {
// own vote + `byzantine_threshold` more votes should be enough to issue `RevertBlocks`
for i in (0_u32..byzantine_threshold as u32).map(|i| i + 3) {
let vote = test_state.issue_explicit_statement_with_index(
ValidatorIndex(i),
candidate_hash,
Expand All @@ -3348,8 +3348,6 @@ fn informs_chain_selection_when_dispute_concluded_against() {
},
})
.await;
handle_approval_vote_request(&mut virtual_overseer, &candidate_hash, HashMap::new())
.await;

// Checking that concluded dispute has signaled the reversion of all parent blocks.
assert_matches!(
Expand All @@ -3363,6 +3361,9 @@ fn informs_chain_selection_when_dispute_concluded_against() {
"Overseer did not receive `ChainSelectionMessage::RevertBlocks` message"
);

// handle_approval_vote_request(&mut virtual_overseer, &candidate_hash, HashMap::new())
// .await;

// Wrap up
virtual_overseer.send(FromOrchestra::Signal(OverseerSignal::Conclude)).await;
assert_matches!(
Expand Down

0 comments on commit 344bbcb

Please sign in to comment.