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

Issue 6274: keeping all backing votes in provisioner vote set #6494

Merged
merged 7 commits into from
Jan 4, 2023
Merged

Issue 6274: keeping all backing votes in provisioner vote set #6494

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
841325f
Fixing filter to keep all backing votes
BradleyOlson64 Jan 2, 2023
5ae8bcc
Merge branch 'master' of https://github.com/paritytech/polkadot into …
BradleyOlson64 Jan 2, 2023
53f8faf
Comment and implementers guide edit
BradleyOlson64 Jan 2, 2023
8fc43b5
Formatting
BradleyOlson64 Jan 2, 2023
ed88f6f
Using fallthrough
BradleyOlson64 Jan 3, 2023
b8e4d64
Fmt
BradleyOlson64 Jan 3, 2023
b08e15b
Merge branch 'master' into brad-issue-6274
BradleyOlson64 Jan 4, 2023
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
18 changes: 14 additions & 4 deletions node/core/provisioner/src/disputes/prioritized_selection/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use polkadot_node_subsystem::{
};
use polkadot_primitives::v2::{
supermajority_threshold, CandidateHash, DisputeState, DisputeStatement, DisputeStatementSet,
Hash, MultiDisputeStatementSet, SessionIndex, ValidatorIndex,
Hash, MultiDisputeStatementSet, SessionIndex, ValidDisputeStatementKind, ValidatorIndex,
};
use std::{
collections::{BTreeMap, HashMap},
Expand Down Expand Up @@ -364,10 +364,20 @@ fn is_vote_worth_to_keep(
dispute_statement: DisputeStatement,
onchain_state: &DisputeState,
) -> bool {
let offchain_vote = match dispute_statement {
DisputeStatement::Valid(_) => true,
DisputeStatement::Invalid(_) => false,
let (offchain_vote, valid_kind) = match dispute_statement {
DisputeStatement::Valid(kind) => (true, Some(kind)),
DisputeStatement::Invalid(_) => (false, None),
};
// We want to keep all backing votes. This maximizes the number of backers
// punished when misbehaving.
if let Some(kind) = valid_kind {
match kind {
ValidDisputeStatementKind::BackingValid(_) => return true,
ValidDisputeStatementKind::BackingSeconded(_) => return true,
_ => (),
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Your version is totally fine, but I think you can also do something like this 🙂 (haven't checked it):

Suggested change
if let Some(kind) = valid_kind {
match kind {
ValidDisputeStatementKind::BackingValid(_) => return true,
ValidDisputeStatementKind::BackingSeconded(_) => return true,
_ => (),
}
}
if matches!(valid_kind, Some(ValidDisputeStatementKind::BackingValid(_) | ValidDisputeStatementKind::BackingSeconded(_)) {
return true
}


let in_validators_for = onchain_state
.validators_for
.get(validator_index.0 as usize)
Expand Down
2 changes: 1 addition & 1 deletion roadmap/implementers-guide/src/node/utility/provisioner.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ The end result of this process is a vector of `BackedCandidate`s, sorted in orde

This is the point at which the block author provides further votes to active disputes or initiates new disputes in the runtime state.

The block-authoring logic of the runtime has an extra step between handling the inherent-data and producing the actual inherent call, which we assume performs the work of filtering out disputes which are not relevant to the on-chain state.
The block-authoring logic of the runtime has an extra step between handling the inherent-data and producing the actual inherent call, which we assume performs the work of filtering out disputes which are not relevant to the on-chain state. Backing votes are always kept in the dispute statement set. This ensures we punish the maximum number of misbehaving backers.

To select disputes:

Expand Down