Skip to content

Commit

Permalink
do not block finality for spam disputes
Browse files Browse the repository at this point in the history
  • Loading branch information
ordian committed Feb 16, 2024
1 parent fde4447 commit f02add4
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 12 deletions.
8 changes: 6 additions & 2 deletions polkadot/node/core/dispute-coordinator/src/initialized.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1292,7 +1292,11 @@ impl Initialized {
session,
"New dispute initiated for candidate.",
);
DisputeStatus::active()
if potential_spam {
DisputeStatus::potential_spam()
} else {
DisputeStatus::active()
}
});

*status = *new_status;
Expand Down Expand Up @@ -1622,7 +1626,7 @@ fn determine_undisputed_chain(
let is_possibly_invalid = |session, candidate_hash| {
recent_disputes
.get(&(session, candidate_hash))
.map_or(false, |status| status.is_possibly_invalid())
.map_or(false, |status| status.is_possibly_invalid() && !status.is_potential_spam())
};

for (i, BlockDescription { session, candidates, .. }) in block_descriptions.iter().enumerate() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ impl TestDisputes {
onchain_votes_count: usize,
) {
let concluded_at = match dispute.2 {
DisputeStatus::Active | DisputeStatus::Confirmed => None,
DisputeStatus::Active | DisputeStatus::Confirmed | DisputeStatus::PotentialSpam => None,
DisputeStatus::ConcludedAgainst(_) | DisputeStatus::ConcludedFor(_) => Some(1),
};
self.onchain_disputes.insert(
Expand Down
38 changes: 29 additions & 9 deletions polkadot/node/primitives/src/disputes/status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ pub enum DisputeStatus {
/// we have seen the candidate included already/participated successfully ourselves).
#[codec(index = 3)]
Confirmed,
/// The dispute is not confirmed and potentially a spam.
#[codec(index = 4)]
PotentialSpam,
}

impl DisputeStatus {
Expand All @@ -55,36 +58,43 @@ impl DisputeStatus {
DisputeStatus::Active
}

/// Initialize the status to the potentially spam state.
pub fn potential_spam() -> DisputeStatus {
DisputeStatus::PotentialSpam
}

/// Move status to confirmed status, if not yet concluded/confirmed already.
pub fn confirm(self) -> DisputeStatus {
match self {
DisputeStatus::Active => DisputeStatus::Confirmed,
DisputeStatus::Confirmed => DisputeStatus::Confirmed,
DisputeStatus::PotentialSpam => DisputeStatus::Confirmed,
DisputeStatus::ConcludedFor(_) | DisputeStatus::ConcludedAgainst(_) => self,
}
}

/// Check whether the dispute is not a spam dispute.
/// Check whether the dispute has been confirmed or concluded.
pub fn is_confirmed_concluded(&self) -> bool {
match self {
&DisputeStatus::Confirmed |
&DisputeStatus::ConcludedFor(_) |
DisputeStatus::Confirmed |
DisputeStatus::ConcludedFor(_) |
DisputeStatus::ConcludedAgainst(_) => true,
&DisputeStatus::Active => false,
DisputeStatus::Active => false,
DisputeStatus::PotentialSpam => false,
}
}

/// Concluded valid?
pub fn has_concluded_for(&self) -> bool {
match self {
&DisputeStatus::ConcludedFor(_) => true,
DisputeStatus::ConcludedFor(_) => true,
_ => false,
}
}
/// Concluded invalid?
pub fn has_concluded_against(&self) -> bool {
match self {
&DisputeStatus::ConcludedAgainst(_) => true,
DisputeStatus::ConcludedAgainst(_) => true,
_ => false,
}
}
Expand All @@ -93,7 +103,8 @@ impl DisputeStatus {
/// candidate. This may be a no-op if the status was already concluded.
pub fn conclude_for(self, now: Timestamp) -> DisputeStatus {
match self {
DisputeStatus::Active | DisputeStatus::Confirmed => DisputeStatus::ConcludedFor(now),
DisputeStatus::Active | DisputeStatus::Confirmed | DisputeStatus::PotentialSpam =>
DisputeStatus::ConcludedFor(now),
DisputeStatus::ConcludedFor(at) => DisputeStatus::ConcludedFor(std::cmp::min(at, now)),
against => against,
}
Expand All @@ -103,7 +114,7 @@ impl DisputeStatus {
/// candidate. This may be a no-op if the status was already concluded.
pub fn conclude_against(self, now: Timestamp) -> DisputeStatus {
match self {
DisputeStatus::Active | DisputeStatus::Confirmed =>
DisputeStatus::Active | DisputeStatus::Confirmed | DisputeStatus::PotentialSpam =>
DisputeStatus::ConcludedAgainst(now),
DisputeStatus::ConcludedFor(at) =>
DisputeStatus::ConcludedAgainst(std::cmp::min(at, now)),
Expand All @@ -116,16 +127,25 @@ impl DisputeStatus {
pub fn is_possibly_invalid(&self) -> bool {
match self {
DisputeStatus::Active |
DisputeStatus::PotentialSpam |
DisputeStatus::Confirmed |
DisputeStatus::ConcludedAgainst(_) => true,
DisputeStatus::ConcludedFor(_) => false,
}
}

/// Whether the disputed candidate is potential spam.
pub fn is_potential_spam(&self) -> bool {
match self {
DisputeStatus::PotentialSpam => true,
_ => false,
}
}

/// Yields the timestamp this dispute concluded at, if any.
pub fn concluded_at(&self) -> Option<Timestamp> {
match self {
DisputeStatus::Active | DisputeStatus::Confirmed => None,
DisputeStatus::Active | DisputeStatus::Confirmed | DisputeStatus::PotentialSpam => None,
DisputeStatus::ConcludedFor(at) | DisputeStatus::ConcludedAgainst(at) => Some(*at),
}
}
Expand Down

0 comments on commit f02add4

Please sign in to comment.