From 8bd80d345d5e80f5155b62d1cc3f36f40c008190 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 28 Apr 2021 02:32:24 -0400 Subject: [PATCH 01/67] disputes module skeleton and storage --- runtime/parachains/src/disputes.rs | 98 ++++++++++++++++++++++++++++++ runtime/parachains/src/lib.rs | 1 + 2 files changed, 99 insertions(+) create mode 100644 runtime/parachains/src/disputes.rs diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs new file mode 100644 index 000000000000..781fc64bc1de --- /dev/null +++ b/runtime/parachains/src/disputes.rs @@ -0,0 +1,98 @@ +// Copyright 2021 Parity Technologies (UK) Ltd. +// This file is part of Polkadot. + +// Polkadot is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Polkadot is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Polkadot. If not, see . + +//! Runtime component for handling disputes of parachain candidates. + +use sp_std::prelude::*; +use sp_std::result; +#[cfg(feature = "std")] +use sp_std::marker::PhantomData; +use primitives::v1::{ + Id as ParaId, ValidationCode, HeadData, SessionIndex, Hash, BlockNumber, CandidateHash, + DisputeState, +}; +use sp_runtime::{traits::One, DispatchResult, SaturatedConversion}; +use frame_system::ensure_root; +use frame_support::{ + decl_storage, decl_module, decl_error, decl_event, ensure, + traits::Get, + weights::Weight, +}; +use parity_scale_codec::{Encode, Decode}; +use crate::{configuration, shared, initializer::SessionChangeNotification}; +use sp_core::RuntimeDebug; + +#[cfg(feature = "std")] +use serde::{Serialize, Deserialize}; + +pub trait Config: + frame_system::Config + + configuration::Config +{ + type Event: From + Into<::Event>; +} + +decl_storage! { + trait Store for Module as ParaDisputes { + /// The last pruneed session, if any. All data stored by this module + /// references sessions. + LastPrunedSession: Option; + // All ongoing or concluded disputes for the last several sessions. + Disputes: double_map + hasher(twox_64_concat) SessionIndex, + hasher(blake2_128_concat) CandidateHash + => Option>; + // All included blocks on the chain, as well as the block number in this chain that + // should be reverted back to if the candidate is disputed and determined to be invalid. + Included: double_map + hasher(twox_64_concat) SessionIndex, + hasher(blake2_128_concat) CandidateHash + => Option; + // Maps session indices to a vector indicating the number of potentially-spam disputes + // each validator is participating in. Potentially-spam disputes are remote disputes which have + // fewer than `byzantine_threshold + 1` validators. + // + // The i'th entry of the vector corresponds to the i'th validator in the session. + SpamSlots: map hasher(twox_64_concat) SessionIndex => Vec; + // Whether the chain is frozen or not. Starts as `false`. When this is `true`, + // the chain will not accept any new parachain blocks for backing or inclusion. + // It can only be set back to `false` by governance intervention. + Frozen: bool; + } +} + +decl_event! { + pub enum Event { + /// A dispute has been initiated. The boolean is true if the dispute is local. \[para_id\] + DisputeInitiated(ParaId, CandidateHash, bool), + /// A dispute has concluded for or against a candidate. The boolean is true if the candidate + /// is deemed valid (for) and false if invalid (against). + DisputeConcluded(ParaId, CandidateHash, bool), + /// A dispute has timed out due to insufficient participation. + DisputeTimedOut(ParaId, CandidateHash), + } +} + +decl_module! { + /// The parachains configuration module. + pub struct Module for enum Call where origin: ::Origin { + fn deposit_event() = default; + } +} + +impl Module { + +} diff --git a/runtime/parachains/src/lib.rs b/runtime/parachains/src/lib.rs index b486d82958ac..fc4e2d65b05e 100644 --- a/runtime/parachains/src/lib.rs +++ b/runtime/parachains/src/lib.rs @@ -23,6 +23,7 @@ #![cfg_attr(not(feature = "std"), no_std)] pub mod configuration; +pub mod disputes; pub mod shared; pub mod inclusion; pub mod initializer; From 101fde9f6c0d9e10edfcf3e430b5105ad99b7f1b Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 28 Apr 2021 02:50:16 -0400 Subject: [PATCH 02/67] implement dispute module initialization logic --- runtime/parachains/src/disputes.rs | 43 ++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 781fc64bc1de..562de99a9cd6 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -94,5 +94,48 @@ decl_module! { } impl Module { + /// Called by the iniitalizer to initialize the disputes module. + pub(crate) fn initializer_initialize(now: T::BlockNumber) -> Weight { + let config = >::config(); + let mut weight = 0; + for (session_index, candidate_hash, mut dispute) in >::iter() { + weight += T::DbWeight::get().reads_writes(1, 0); + + if dispute.concluded_at.is_none() + && dispute.start + config.dispute_conclusion_by_time_out_period < now + { + dispute.concluded_at = Some(now); + >::insert(session_index, candidate_hash, &dispute); + + // mildly punish all validators involved. they've failed to make + // data available to others, so this is most likely spam. + SpamSlots::mutate(session_index, |spam_slots| { + let participating = (dispute.validators_for | dispute.validators_against); + for validator_index in participating { + // TODO [now]: slight punishment. + + // also reduce spam slots for all validators involved. this does + // open us up to more spam, but only for validators who are willing + // to be punished more. + if let Some(occupied) = spam_slots.get_mut(validator_index as usize) { + *occupied = occupied.saturating_sub(1); + } + } + }); + + weight += T::DbWeight::get().reads_writes(1, 2); + } + } + + weight + } + + /// Called by the iniitalizer to finalize the disputes module. + pub(crate) fn initializer_finalize(now: T::BlockNumber) { } + + /// Called by the iniitalizer to note a new session in the disputes module. + pub(crate) fn initializer_on_new_session(now: T::BlockNumber) { + + } } From 4198797ce4d091f91c23a8061eb1efa02aba964f Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 28 Apr 2021 02:57:45 -0400 Subject: [PATCH 03/67] implement disputes session change logic --- runtime/parachains/src/disputes.rs | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 562de99a9cd6..7e893a288af0 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -132,10 +132,28 @@ impl Module { } /// Called by the iniitalizer to finalize the disputes module. - pub(crate) fn initializer_finalize(now: T::BlockNumber) { } + pub(crate) fn initializer_finalize() { } /// Called by the iniitalizer to note a new session in the disputes module. - pub(crate) fn initializer_on_new_session(now: T::BlockNumber) { + pub(crate) fn initializer_on_new_session(notification: &SessionChangeNotification) { + let config = >::config(); + + if notification.session_index <= config.dispute_period + 1 { + return + } + + let pruning_target = notification.session_index - config.dispute_period - 1; + + LastPrunedSession::mutate(|last_pruned| { + if let Some(last_pruned) = last_pruned { + for to_prune in *last_pruned + 1 ..= pruning_target { + >::remove_prefix(to_prune); + >::remove_prefix(to_prune); + SpamSlots::remove(to_prune); + } + } + *last_pruned = Some(pruning_target); + }); } } From 7dac19395ae610e6c15ebe598e3ebdfa57917403 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Tue, 4 May 2021 11:14:29 -0500 Subject: [PATCH 04/67] provide dispute skeletons --- runtime/parachains/src/disputes.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 7e893a288af0..a6263df12092 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -22,7 +22,7 @@ use sp_std::result; use sp_std::marker::PhantomData; use primitives::v1::{ Id as ParaId, ValidationCode, HeadData, SessionIndex, Hash, BlockNumber, CandidateHash, - DisputeState, + DisputeState, DisputeStatementSet, MultiDisputeStatementSet, }; use sp_runtime::{traits::One, DispatchResult, SaturatedConversion}; use frame_system::ensure_root; @@ -156,4 +156,17 @@ impl Module { *last_pruned = Some(pruning_target); }); } + + /// Handle sets of dispute statements corresponding to 0 or more candidates. + pub(crate) fn provide_multi_dispute_data(statements: MultiDisputeStatementSet) + -> Vec<(SessionIndex, CandidateHash)> + { + unimplemented!() + } + + /// Handle a set of dispute statements corresponding to a single candidate. + fn provide_dispute_data() -> bool { + unimplemented!() + // TODO [now] + } } From dd01c0608f8052144ceeec9847e259d79a5ec690 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Tue, 4 May 2021 18:14:43 -0500 Subject: [PATCH 05/67] deduplication & ancient check --- runtime/parachains/src/disputes.rs | 75 +++++++++++++++++++++++++++--- 1 file changed, 68 insertions(+), 7 deletions(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index a6263df12092..002790c3fec5 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -24,7 +24,7 @@ use primitives::v1::{ Id as ParaId, ValidationCode, HeadData, SessionIndex, Hash, BlockNumber, CandidateHash, DisputeState, DisputeStatementSet, MultiDisputeStatementSet, }; -use sp_runtime::{traits::One, DispatchResult, SaturatedConversion}; +use sp_runtime::{traits::{One, Saturating}, DispatchResult, DispatchError, SaturatedConversion}; use frame_system::ensure_root; use frame_support::{ decl_storage, decl_module, decl_error, decl_event, ensure, @@ -32,7 +32,11 @@ use frame_support::{ weights::Weight, }; use parity_scale_codec::{Encode, Decode}; -use crate::{configuration, shared, initializer::SessionChangeNotification}; +use crate::{ + configuration::{self, HostConfiguration}, + shared, + initializer::SessionChangeNotification, +}; use sp_core::RuntimeDebug; #[cfg(feature = "std")] @@ -93,6 +97,15 @@ decl_module! { } } +decl_error! { + pub enum Error for Module { + /// Duplicate dispute statement sets provided. + DuplicateDisputeStatementSets, + /// Ancient dispute statement provided. + AncientDisputeStatement, + } +} + impl Module { /// Called by the iniitalizer to initialize the disputes module. pub(crate) fn initializer_initialize(now: T::BlockNumber) -> Weight { @@ -136,7 +149,7 @@ impl Module { /// Called by the iniitalizer to note a new session in the disputes module. pub(crate) fn initializer_on_new_session(notification: &SessionChangeNotification) { - let config = >::config(); + let config = >::config(); if notification.session_index <= config.dispute_period + 1 { return @@ -158,15 +171,63 @@ impl Module { } /// Handle sets of dispute statements corresponding to 0 or more candidates. - pub(crate) fn provide_multi_dispute_data(statements: MultiDisputeStatementSet) - -> Vec<(SessionIndex, CandidateHash)> + pub(crate) fn provide_multi_dispute_data(statement_sets: MultiDisputeStatementSet) + -> Result, DispatchError> { - unimplemented!() + let config = >::config(); + + // Deduplicate. + { + let mut targets: Vec<_> = statement_sets.iter() + .map(|set| (set.candidate_hash.0, set.session)) + .collect(); + + targets.sort(); + + let submitted = targets.len(); + targets.dedup(); + + ensure!(submitted == targets.len(), Error::::DuplicateDisputeStatementSets); + } + + let mut fresh = Vec::with_capacity(statement_sets.len()); + for statement_set in statement_sets { + let dispute_target = (statement_set.session, statement_set.candidate_hash); + if Self::provide_dispute_data(&config, statement_set)? { + fresh.push(dispute_target); + } + } + + Ok(fresh) } /// Handle a set of dispute statements corresponding to a single candidate. - fn provide_dispute_data() -> bool { + /// + /// Fails if the dispute data is invalid. Returns a bool indicating whether the + /// dispute is fresh. + fn provide_dispute_data(config: &HostConfiguration, set: DisputeStatementSet) + -> Result + { + // Dispute statement sets on any dispute which concluded + // before this point are to be rejected. + let oldest_accepted = >::block_number() + .saturating_sub(config.dispute_post_conclusion_acceptance_period); + + // Check for ancient. + let fresh = if let Some(dispute_state) = >::get(&set.session, &set.candidate_hash) { + ensure!( + dispute_state.concluded_at.as_ref().map_or(true, |c| c >= &oldest_accepted), + Error::::AncientDisputeStatement, + ); + + false + } else { + true + }; + unimplemented!() // TODO [now] + + fresh } } From ef153dca7e3edb52ba5028fea9065767ba357256 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Tue, 4 May 2021 18:17:22 -0500 Subject: [PATCH 06/67] fix a couple of warnings --- runtime/parachains/src/disputes.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 002790c3fec5..b499846b7f96 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -37,11 +37,6 @@ use crate::{ shared, initializer::SessionChangeNotification, }; -use sp_core::RuntimeDebug; - -#[cfg(feature = "std")] -use serde::{Serialize, Deserialize}; - pub trait Config: frame_system::Config + configuration::Config @@ -124,7 +119,7 @@ impl Module { // mildly punish all validators involved. they've failed to make // data available to others, so this is most likely spam. SpamSlots::mutate(session_index, |spam_slots| { - let participating = (dispute.validators_for | dispute.validators_against); + let participating = dispute.validators_for | dispute.validators_against; for validator_index in participating { // TODO [now]: slight punishment. From 8dea7e6a95bb00622b33b6f4cfe7766752116ed9 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Tue, 4 May 2021 19:10:52 -0500 Subject: [PATCH 07/67] begin provide_dispute_data impl --- runtime/parachains/src/disputes.rs | 52 +++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 15 deletions(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index b499846b7f96..46271db986b4 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -32,14 +32,18 @@ use frame_support::{ weights::Weight, }; use parity_scale_codec::{Encode, Decode}; +use bitvec::{bitvec, order::Lsb0 as BitOrderLsb0}; use crate::{ configuration::{self, HostConfiguration}, - shared, initializer::SessionChangeNotification, + shared, + session_info, }; + pub trait Config: frame_system::Config + - configuration::Config + configuration::Config + + session_info::Config { type Event: From + Into<::Event>; } @@ -205,24 +209,42 @@ impl Module { { // Dispute statement sets on any dispute which concluded // before this point are to be rejected. - let oldest_accepted = >::block_number() - .saturating_sub(config.dispute_post_conclusion_acceptance_period); + let now = >::block_number(); + let oldest_accepted = now.saturating_sub(config.dispute_post_conclusion_acceptance_period); + + // Load session info to access validators + let session_info = match >::session_info(set.session) { + Some(s) => s, + None => return Err(Error::::AncientDisputeStatement.into()), + }; + + let n_validators = session_info.validators.len(); // Check for ancient. - let fresh = if let Some(dispute_state) = >::get(&set.session, &set.candidate_hash) { - ensure!( - dispute_state.concluded_at.as_ref().map_or(true, |c| c >= &oldest_accepted), - Error::::AncientDisputeStatement, - ); - - false - } else { - true + let (fresh, dispute_state) = { + if let Some(dispute_state) = >::get(&set.session, &set.candidate_hash) { + ensure!( + dispute_state.concluded_at.as_ref().map_or(true, |c| c >= &oldest_accepted), + Error::::AncientDisputeStatement, + ); + + (false, dispute_state) + } else { + ( + true, + DisputeState { + validators_for: bitvec![BitOrderLsb0, u8; 0; n_validators], + validators_against: bitvec![BitOrderLsb0, u8; 0; n_validators], + start: now, + concluded_at: None, + } + ) + } }; - unimplemented!() + unimplemented!(); // TODO [now] - fresh + Ok(fresh) } } From f70f6e2d1dde84684c334d6d5d6b4ea191629f57 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 5 May 2021 00:14:29 -0500 Subject: [PATCH 08/67] flesh out statement set import somewhat --- primitives/src/v1.rs | 4 +- .../src/runtime/disputes.md | 11 ++-- runtime/parachains/src/disputes.rs | 59 ++++++++++++++++++- 3 files changed, 63 insertions(+), 11 deletions(-) diff --git a/primitives/src/v1.rs b/primitives/src/v1.rs index 4fdab1f3e1ad..0c8ec2793497 100644 --- a/primitives/src/v1.rs +++ b/primitives/src/v1.rs @@ -1061,10 +1061,10 @@ pub enum ValidDisputeStatementKind { Explicit, /// A seconded statement on a candidate from the backing phase. #[codec(index = 1)] - BackingSeconded, + BackingSeconded(Hash), /// A valid statement on a candidate from the backing phase. #[codec(index = 2)] - BackingValid, + BackingValid(Hash), /// An approval vote from the approval checking phase. #[codec(index = 3)] ApprovalChecking, diff --git a/roadmap/implementers-guide/src/runtime/disputes.md b/roadmap/implementers-guide/src/runtime/disputes.md index 4faece7cb092..a8c608722aaa 100644 --- a/roadmap/implementers-guide/src/runtime/disputes.md +++ b/roadmap/implementers-guide/src/runtime/disputes.md @@ -65,7 +65,6 @@ Frozen: bool, ## Routines * `provide_multi_dispute_data(MultiDisputeStatementSet) -> Vec<(SessionIndex, Hash)>`: - 1. Fail if any disputes in the set are duplicate or concluded before the `config.dispute_post_conclusion_acceptance_period` window relative to now. 1. Pass on each dispute statement set to `provide_dispute_data`, propagating failure. 1. Return a list of all candidates who just had disputes initiated. @@ -75,13 +74,13 @@ Frozen: bool, 1. If there is no dispute under `Disputes`, create a new `DisputeState` with blank bitfields. 1. If `concluded_at` is `Some`, and is `concluded_at + config.post_conclusion_acceptance_period < now`, return false. 1. If the overlap of the validators in the `DisputeStatementSet` and those already present in the `DisputeState` is fewer in number than `byzantine_threshold + 1` and the candidate is not present in the `Included` map - 1. increment `SpamSlots` for each validator in the `DisputeStatementSet` which is not already in the `DisputeState`. Initialize the `SpamSlots` to a zeroed vector first, if necessary. - 1. If the value for any spam slot exceeds `config.dispute_max_spam_slots`, return false. + 1. increment `SpamSlots` for each validator in the `DisputeStatementSet` which is not already in the `DisputeState`. Initialize the `SpamSlots` to a zeroed vector first, if necessary. + 1. If the value for any spam slot exceeds `config.dispute_max_spam_slots`, return false. 1. If the overlap of the validators in the `DisputeStatementSet` and those already present in the `DisputeState` is at least `byzantine_threshold + 1`, the `DisputeState` has fewer than `byzantine_threshold + 1` validators, and the candidate is not present in the `Included` map, decrement `SpamSlots` for each validator in the `DisputeState`. - 1. Import all statements into the dispute. This should fail if any statements are duplicate; if the corresponding bit for the corresponding validator is set in the dispute already. - 1. If `concluded_at` is `None`, reward all statements slightly less. + 1. Import all statements into the dispute. This should fail if any statements are duplicate or if the corresponding bit for the corresponding validator is set in the dispute already. + 1. If `concluded_at` is `None`, reward all statements. 1. If `concluded_at` is `Some`, reward all statements slightly less. - 1. If either side now has supermajority, slash the other side. This may be both sides, and we support this possibility in code, but note that this requires validators to participate on both sides which has negative expected value. Set `concluded_at` to `Some(now)`. + 1. If either side now has supermajority and did not previously, slash the other side. This may be both sides, and we support this possibility in code, but note that this requires validators to participate on both sides which has negative expected value. Set `concluded_at` to `Some(now)` if it was `None`. 1. If just concluded against the candidate and the `Included` map contains `(session, candidate)`: invoke `revert_and_freeze` with the stored block number. 1. Return true if just initiated, false otherwise. diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 46271db986b4..b34c9057d00a 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -22,7 +22,9 @@ use sp_std::result; use sp_std::marker::PhantomData; use primitives::v1::{ Id as ParaId, ValidationCode, HeadData, SessionIndex, Hash, BlockNumber, CandidateHash, - DisputeState, DisputeStatementSet, MultiDisputeStatementSet, + DisputeState, DisputeStatementSet, MultiDisputeStatementSet, ValidatorId, ValidatorSignature, + DisputeStatement, ValidDisputeStatementKind, InvalidDisputeStatementKind, + ExplicitDisputeStatement, }; use sp_runtime::{traits::{One, Saturating}, DispatchResult, DispatchError, SaturatedConversion}; use frame_system::ensure_root; @@ -102,9 +104,26 @@ decl_error! { DuplicateDisputeStatementSets, /// Ancient dispute statement provided. AncientDisputeStatement, + /// Validator index on statement is out of bounds for session. + ValidatorIndexOutOfBounds, + /// Invalid signature on statement. + InvalidSignature, } } +// The maximum number of validators `f` which may safely be faulty. +// +// The total number of validators is `n = 3f + e` where `e in { 1, 2, 3 }`. +fn byzantine_threshold(n: usize) -> usize { + n.saturating_sub(1) / 3 +} + +// The supermajority threshold of validators which is required to +// conclude a dispute. +fn supermajority_threshold(n: usize) -> usize { + n - byzantine_threshold(n) +} + impl Module { /// Called by the iniitalizer to initialize the disputes module. pub(crate) fn initializer_initialize(now: T::BlockNumber) -> Weight { @@ -242,9 +261,43 @@ impl Module { } }; - unimplemented!(); - // TODO [now] + // Check all statement signatures + for (statement, validator_index, signature) in &set.statements { + let validator_public = session_info.validators.get(validator_index.0 as usize) + .ok_or(Error::::ValidatorIndexOutOfBounds)?; + + check_signature( + &validator_public, + set.candidate_hash, + set.session, + statement, + signature, + ).map_err(|()| Error::::InvalidSignature)?; + } + + let byzantine_threshold = byzantine_threshold(n_validators); + let supermajority_threshold = supermajority_threshold(n_validators); + + // TODO [now]: update `DisputeInfo` and determine: + // 1. Spam slot changes. Bail early if too many spam slots occupied. + // 2. Dispute state change. Bail early if duplicate + + // TODO [now]: Reward statements based on conclusion. + + // TODO [now]: Slash one side if fresh supermajority on the other. + + // TODO [now]: Freeze if just concluded local. Ok(fresh) } } + +fn check_signature( + validator_public: &ValidatorId, + candidate_hash: CandidateHash, + session: SessionIndex, + statement: &DisputeStatement, + validator_signature: &ValidatorSignature, +) -> Result<(), ()> { + unimplemented!() +} From 04511394f95b3fb6b8e5811119a538780b8cca38 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 5 May 2021 00:28:49 -0500 Subject: [PATCH 09/67] move ApprovalVote to shared primitives --- node/core/approval-voting/src/lib.rs | 26 +++++--------------------- node/core/approval-voting/src/tests.rs | 2 +- node/primitives/src/approval.rs | 4 ---- primitives/src/v1.rs | 16 ++++++++++++++++ 4 files changed, 22 insertions(+), 26 deletions(-) diff --git a/node/core/approval-voting/src/lib.rs b/node/core/approval-voting/src/lib.rs index eb287a75b4cb..07d778af3094 100644 --- a/node/core/approval-voting/src/lib.rs +++ b/node/core/approval-voting/src/lib.rs @@ -38,15 +38,13 @@ use polkadot_primitives::v1::{ ValidatorIndex, Hash, SessionIndex, SessionInfo, CandidateHash, CandidateReceipt, BlockNumber, PersistedValidationData, ValidationCode, CandidateDescriptor, ValidatorPair, ValidatorSignature, ValidatorId, - CandidateIndex, GroupIndex, + CandidateIndex, GroupIndex, ApprovalVote, }; use polkadot_node_primitives::{ValidationResult, PoV}; use polkadot_node_primitives::approval::{ - IndirectAssignmentCert, IndirectSignedApprovalVote, ApprovalVote, DelayTranche, - BlockApprovalMeta, + IndirectAssignmentCert, IndirectSignedApprovalVote, DelayTranche, BlockApprovalMeta, }; use polkadot_node_jaeger as jaeger; -use parity_scale_codec::Encode; use sc_keystore::LocalKeystore; use sp_consensus::SyncOracle; use sp_consensus_slots::Slot; @@ -1226,15 +1224,6 @@ async fn handle_approved_ancestor( Ok(all_approved_max) } -fn approval_signing_payload( - approval_vote: ApprovalVote, - session_index: SessionIndex, -) -> Vec { - const MAGIC: [u8; 4] = *b"APPR"; - - (MAGIC, approval_vote, session_index).encode() -} - // `Option::cmp` treats `None` as less than `Some`. fn min_prefer_some( a: Option, @@ -1466,10 +1455,8 @@ fn check_and_import_approval( None => respond_early!(ApprovalCheckResult::Bad) }; - let approval_payload = approval_signing_payload( - ApprovalVote(approved_candidate_hash), - block_entry.session(), - ); + let approval_payload = ApprovalVote(approved_candidate_hash) + .signing_payload(block_entry.session()); let pubkey = match session_info.validators.get(approval.validator.0 as usize) { Some(k) => k, @@ -2157,10 +2144,7 @@ fn sign_approval( ) -> Option { let key = keystore.key_pair::(public).ok().flatten()?; - let payload = approval_signing_payload( - ApprovalVote(candidate_hash), - session_index, - ); + let payload = ApprovalVote(candidate_hash).signing_payload(session_index); Some(key.sign(&payload[..])) } diff --git a/node/core/approval-voting/src/tests.rs b/node/core/approval-voting/src/tests.rs index 95b76d8a5526..5574c3b028f5 100644 --- a/node/core/approval-voting/src/tests.rs +++ b/node/core/approval-voting/src/tests.rs @@ -231,7 +231,7 @@ fn sign_approval( candidate_hash: CandidateHash, session_index: SessionIndex, ) -> ValidatorSignature { - key.sign(&super::approval_signing_payload(ApprovalVote(candidate_hash), session_index)).into() + key.sign(&ApprovalVote(candidate_hash).signing_payload(session_index)).into() } #[derive(Clone)] diff --git a/node/primitives/src/approval.rs b/node/primitives/src/approval.rs index 8303478aa53c..743c37f32759 100644 --- a/node/primitives/src/approval.rs +++ b/node/primitives/src/approval.rs @@ -98,10 +98,6 @@ pub struct IndirectAssignmentCert { pub cert: AssignmentCert, } -/// A vote of approval on a candidate. -#[derive(Debug, Clone, Encode, Decode)] -pub struct ApprovalVote(pub CandidateHash); - /// A signed approval vote which references the candidate indirectly via the block. /// /// In practice, we have a look-up from block hash and candidate index to candidate hash, diff --git a/primitives/src/v1.rs b/primitives/src/v1.rs index 0c8ec2793497..aaa83e2385db 100644 --- a/primitives/src/v1.rs +++ b/primitives/src/v1.rs @@ -822,6 +822,22 @@ pub struct SessionInfo { pub needed_approvals: u32, } +/// A vote of approval on a candidate. +#[derive(Clone, RuntimeDebug)] +pub struct ApprovalVote(pub CandidateHash); + +impl ApprovalVote { + /// Yields the signing payload for this approval vote. + pub fn signing_payload( + &self, + session_index: SessionIndex, + ) -> Vec { + const MAGIC: [u8; 4] = *b"APPR"; + + (MAGIC, &self.0, session_index).encode() + } +} + sp_api::decl_runtime_apis! { /// The API for querying the state of parachains on-chain. pub trait ParachainHost { From 67485788479ccde8be9f78b7435967969c3f7a4b Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 5 May 2021 00:31:12 -0500 Subject: [PATCH 10/67] add a signing-payload API to explicit dispute statements --- primitives/src/v1.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/primitives/src/v1.rs b/primitives/src/v1.rs index aaa83e2385db..8f1650e97a10 100644 --- a/primitives/src/v1.rs +++ b/primitives/src/v1.rs @@ -1095,7 +1095,7 @@ pub enum InvalidDisputeStatementKind { } /// An explicit statement on a candidate issued as part of a dispute. -#[derive(Encode, Decode, Clone, PartialEq, RuntimeDebug)] +#[derive(Clone, PartialEq, RuntimeDebug)] pub struct ExplicitDisputeStatement { /// Whether the candidate is valid pub valid: bool, @@ -1105,6 +1105,15 @@ pub struct ExplicitDisputeStatement { pub session: SessionIndex, } +impl ExplicitDisputeStatement { + /// Produce the payload used for signing this type of statement. + pub fn signing_payload(&self) -> Vec { + const MAGIC: [u8; 4] = *b"DISP"; + + (MAGIC, self.valid, self.candidate_hash, self.session).encode() + } +} + /// A set of statements about a specific candidate. #[derive(Encode, Decode, Clone, PartialEq, RuntimeDebug)] pub struct DisputeStatementSet { From 95d6c19d2215722ef1f940e53cafe0c8858638f1 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 5 May 2021 00:42:45 -0500 Subject: [PATCH 11/67] implement statement signature checking --- primitives/src/v0.rs | 8 ++++++ runtime/parachains/src/disputes.rs | 45 ++++++++++++++++++++++++++++-- 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/primitives/src/v0.rs b/primitives/src/v0.rs index 9a7112bccd0d..d12f01df5731 100644 --- a/primitives/src/v0.rs +++ b/primitives/src/v0.rs @@ -678,6 +678,14 @@ pub enum CompactStatement { Valid(CandidateHash), } +impl CompactStatement { + /// Yields the payload used for validator signatures on this kind + /// of statement. + pub fn signing_payload(&self, context: &SigningContext) -> Vec { + (self, context).encode() + } +} + // Inner helper for codec on `CompactStatement`. #[derive(Encode, Decode)] enum CompactStatementInner { diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index b34c9057d00a..a1898caf1752 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -24,9 +24,12 @@ use primitives::v1::{ Id as ParaId, ValidationCode, HeadData, SessionIndex, Hash, BlockNumber, CandidateHash, DisputeState, DisputeStatementSet, MultiDisputeStatementSet, ValidatorId, ValidatorSignature, DisputeStatement, ValidDisputeStatementKind, InvalidDisputeStatementKind, - ExplicitDisputeStatement, + ExplicitDisputeStatement, CompactStatement, SigningContext, ApprovalVote, +}; +use sp_runtime::{ + traits::{One, Saturating, AppVerify}, + DispatchResult, DispatchError, SaturatedConversion, }; -use sp_runtime::{traits::{One, Saturating}, DispatchResult, DispatchError, SaturatedConversion}; use frame_system::ensure_root; use frame_support::{ decl_storage, decl_module, decl_error, decl_event, ensure, @@ -299,5 +302,41 @@ fn check_signature( statement: &DisputeStatement, validator_signature: &ValidatorSignature, ) -> Result<(), ()> { - unimplemented!() + let payload = match *statement { + DisputeStatement::Valid(ValidDisputeStatementKind::Explicit) => { + ExplicitDisputeStatement { + valid: true, + candidate_hash, + session, + }.signing_payload() + }, + DisputeStatement::Valid(ValidDisputeStatementKind::BackingSeconded(inclusion_parent)) => { + CompactStatement::Seconded(candidate_hash).signing_payload(&SigningContext { + session_index: session, + parent_hash: inclusion_parent, + }) + }, + DisputeStatement::Valid(ValidDisputeStatementKind::BackingValid(inclusion_parent)) => { + CompactStatement::Valid(candidate_hash).signing_payload(&SigningContext { + session_index: session, + parent_hash: inclusion_parent, + }) + }, + DisputeStatement::Valid(ValidDisputeStatementKind::ApprovalChecking) => { + ApprovalVote(candidate_hash).signing_payload(session) + }, + DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit) => { + ExplicitDisputeStatement { + valid: false, + candidate_hash, + session, + }.signing_payload() + }, + }; + + if validator_signature.verify(&payload[..] , &validator_public) { + Ok(()) + } else { + Err(()) + } } From 01e948e5944bc6e42b986df448114ee16f6e78dc Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Thu, 6 May 2021 12:08:16 -0500 Subject: [PATCH 12/67] some bitflags glue for observing changes in disputes --- Cargo.lock | 1 + runtime/parachains/Cargo.toml | 1 + runtime/parachains/src/disputes.rs | 86 +++++++++++++++++++++++++++++- 3 files changed, 87 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 720cf343ad4d..15a2c05dc7b3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6416,6 +6416,7 @@ dependencies = [ name = "polkadot-runtime-parachains" version = "0.9.0" dependencies = [ + "bitflags", "bitvec", "derive_more", "frame-benchmarking", diff --git a/runtime/parachains/Cargo.toml b/runtime/parachains/Cargo.toml index 8849f9325ab5..bbaa1406e490 100644 --- a/runtime/parachains/Cargo.toml +++ b/runtime/parachains/Cargo.toml @@ -11,6 +11,7 @@ log = { version = "0.4.14", default-features = false } rustc-hex = { version = "2.1.0", default-features = false } serde = { version = "1.0.123", features = [ "derive" ], optional = true } derive_more = "0.99.11" +bitflags = "1" sp-api = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } inherents = { package = "sp-inherents", git = "https://github.com/paritytech/substrate", branch = "master", default-features = false } diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index a1898caf1752..ca673fed6ede 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -24,7 +24,7 @@ use primitives::v1::{ Id as ParaId, ValidationCode, HeadData, SessionIndex, Hash, BlockNumber, CandidateHash, DisputeState, DisputeStatementSet, MultiDisputeStatementSet, ValidatorId, ValidatorSignature, DisputeStatement, ValidDisputeStatementKind, InvalidDisputeStatementKind, - ExplicitDisputeStatement, CompactStatement, SigningContext, ApprovalVote, + ExplicitDisputeStatement, CompactStatement, SigningContext, ApprovalVote, ValidatorIndex, }; use sp_runtime::{ traits::{One, Saturating, AppVerify}, @@ -127,6 +127,90 @@ fn supermajority_threshold(n: usize) -> usize { n - byzantine_threshold(n) } +enum SpamSlotChange { + Dec, + Inc, +} + +bitflags::bitflags! { + #[derive(Default)] + struct DisputeStateFlags: u8 { + const CONFIRMED = 0b0001; + const FOR_SUPERMAJORITY = 0b0010; + const AGAINST_SUPERMAJORITY = 0b0100; + const CONCLUDED = 0b1000; + } +} + +impl DisputeStateFlags { + fn from_state( + state: &DisputeState, + ) -> Self { + let n = state.validators_for.len(); + + let byzantine_threshold = byzantine_threshold(n); + let supermajority_threshold = supermajority_threshold(n); + + let mut flags = DisputeStateFlags::default(); + let all_participants = { + let mut a = state.validators_for.clone(); + *a |= state.validators_against.iter().by_val(); + a + }; + if all_participants.count_ones() > byzantine_threshold { + flags |= DisputeStateFlags::CONFIRMED; + } + + if state.concluded_at.is_some() { + flags |= DisputeStateFlags::CONCLUDED; + } + + if state.validators_for.count_ones() >= supermajority_threshold { + flags |= DisputeStateFlags::FOR_SUPERMAJORITY; + } + + if state.validators_against.count_ones() >= supermajority_threshold { + flags |= DisputeStateFlags::AGAINST_SUPERMAJORITY; + } + + flags + } +} + +struct DisputeStateMutator { + state: DisputeState, + pre_flags: DisputeStateFlags, +} + +impl DisputeStateMutator { + fn initialize( + state: DisputeState, + ) -> Self { + let pre_flags = DisputeStateFlags::from_state(&state); + + DisputeStateMutator { + state, + pre_flags, + } + } + + fn push(&mut self, validator: ValidatorIndex, valid: bool) + -> Result<(), DispatchError> + { + // TODO [now]: fail if duplicate. + + Ok(()) + } + + fn conclude(self) -> DisputeState { + let post_flags = DisputeStateFlags::from_state(&self.state); + + // TODO [now]: prepare update to spam slots, rewarding, and slashing. + + self.state + } +} + impl Module { /// Called by the iniitalizer to initialize the disputes module. pub(crate) fn initializer_initialize(now: T::BlockNumber) -> Weight { From 8e25b48c115b3511a63c14d06d6ba1200ee4d37a Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 7 May 2021 17:37:18 -0500 Subject: [PATCH 13/67] implement dispute vote import logic --- runtime/parachains/src/disputes.rs | 144 +++++++++++++++++++++++++---- 1 file changed, 126 insertions(+), 18 deletions(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index ca673fed6ede..8c82f77f9e3f 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -111,6 +111,8 @@ decl_error! { ValidatorIndexOutOfBounds, /// Invalid signature on statement. InvalidSignature, + /// Validator vote submitted more than once to dispute. + DuplicateStatement, } } @@ -127,18 +129,12 @@ fn supermajority_threshold(n: usize) -> usize { n - byzantine_threshold(n) } -enum SpamSlotChange { - Dec, - Inc, -} - bitflags::bitflags! { #[derive(Default)] struct DisputeStateFlags: u8 { const CONFIRMED = 0b0001; const FOR_SUPERMAJORITY = 0b0010; const AGAINST_SUPERMAJORITY = 0b0100; - const CONCLUDED = 0b1000; } } @@ -161,10 +157,6 @@ impl DisputeStateFlags { flags |= DisputeStateFlags::CONFIRMED; } - if state.concluded_at.is_some() { - flags |= DisputeStateFlags::CONCLUDED; - } - if state.validators_for.count_ones() >= supermajority_threshold { flags |= DisputeStateFlags::FOR_SUPERMAJORITY; } @@ -177,37 +169,153 @@ impl DisputeStateFlags { } } -struct DisputeStateMutator { +enum SpamSlotChange { + Inc, + Dec, +} + +struct ImportSummary { + // The new state, with all votes imported. + state: DisputeState, + // Changes to spam slots. Validator index paired with directional change. + spam_slot_changes: Vec<(ValidatorIndex, SpamSlotChange)>, + // Validators to slash for being (wrongly) on the AGAINST side. + slash_against: Vec, + // Validators to slash for being (wrongly) on the FOR side. + slash_for: Vec, +} + +enum VoteImportError { + ValidatorIndexOutOfBounds, + DuplicateStatement, +} + +struct DisputeStateImporter { state: DisputeState, + now: BlockNumber, + new_participants: bitvec::vec::BitVec, pre_flags: DisputeStateFlags, } -impl DisputeStateMutator { +impl DisputeStateImporter { fn initialize( state: DisputeState, + now: BlockNumber, ) -> Self { let pre_flags = DisputeStateFlags::from_state(&state); + let new_participants = bitvec::bitvec![BitOrderLsb0, u8; 0; state.validators_for.len()]; - DisputeStateMutator { + DisputeStateImporter { state, + now, + new_participants, pre_flags, } } fn push(&mut self, validator: ValidatorIndex, valid: bool) - -> Result<(), DispatchError> + -> Result<(), VoteImportError> { - // TODO [now]: fail if duplicate. + let (bits, other_bits) = if valid { + (&mut self.state.validators_for, &mut self.state.validators_against) + } else { + (&mut self.state.validators_against, &mut self.state.validators_for) + }; + + // out of bounds or already participated + match bits.get(validator.0 as usize).map(|b| *b) { + None => return Err(VoteImportError::ValidatorIndexOutOfBounds), + Some(true) => return Err(VoteImportError::DuplicateStatement), + Some(false) => {} + } + + // inefficient, and just for extra sanity. + if validator.0 as usize >= self.new_participants.len() { + return Err(VoteImportError::ValidatorIndexOutOfBounds); + } + + bits.set(validator.0 as usize, true); + + // New participants tracks those which didn't appear on either + // side of the dispute until now. So we check the other side + // and checked the first side before. + if other_bits.get(validator.0 as usize).map_or(false, |b| !*b) { + self.new_participants.set(validator.0 as usize, true); + } Ok(()) } - fn conclude(self) -> DisputeState { + fn conclude(mut self) -> ImportSummary { + let pre_flags = self.pre_flags; let post_flags = DisputeStateFlags::from_state(&self.state); - // TODO [now]: prepare update to spam slots, rewarding, and slashing. + let pre_post_contains = |flags| (pre_flags.contains(flags), post_flags.contains(flags)); + + // 1. Act on confirmed flag state to inform spam slots changes. + let spam_slot_changes: Vec<_> = match pre_post_contains(DisputeStateFlags::CONFIRMED) { + (false, false) => { + // increment spam slots for all new participants. + self.new_participants.iter_ones() + .map(|i| (ValidatorIndex(i as _), SpamSlotChange::Inc)) + .collect() + } + (false, true) => { + let prev_participants = { + // all participants + let mut a = self.state.validators_for.clone(); + *a |= self.state.validators_against.iter().by_val(); - self.state + // which are not new participants + *a &= self.new_participants.iter().by_val().map(|b| !b); + + a + }; + + prev_participants.iter_ones() + .map(|i| (ValidatorIndex(i as _), SpamSlotChange::Dec)) + .collect() + } + (true, true) | (true, false) => { + // nothing to do. (true, false) is also impossible. + Vec::new() + } + }; + + // 2. Check for fresh FOR supermajority. + let slash_against = if let (false, true) = pre_post_contains(DisputeStateFlags::FOR_SUPERMAJORITY) { + if self.state.concluded_at.is_none() { + self.state.concluded_at = Some(self.now.clone()); + } + + // provide AGAINST voters to slash. + self.state.validators_against.iter_ones() + .map(|i| ValidatorIndex(i as _)) + .collect() + } else { + Vec::new() + }; + + // 3. Check for fresh AGAINST supermajority. + let slash_for = if let (false, true) = pre_post_contains(DisputeStateFlags::AGAINST_SUPERMAJORITY) { + if self.state.concluded_at.is_none() { + self.state.concluded_at = Some(self.now); + } + + // provide FOR voters to slash. + self.state.validators_for.iter_ones() + .map(|i| ValidatorIndex(i as _)) + .collect() + } else { + Vec::new() + }; + + ImportSummary { + state: self.state, + spam_slot_changes, + slash_against, + slash_for, + } } } From 271434b5df9eae4b2395de20db859a954d3f175e Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 7 May 2021 21:58:04 -0500 Subject: [PATCH 14/67] flesh out everything except slashing --- runtime/parachains/src/disputes.rs | 147 +++++++++++++++++++----- runtime/parachains/src/reward_points.rs | 2 + 2 files changed, 118 insertions(+), 31 deletions(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 8c82f77f9e3f..7485e66d1b06 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -45,12 +45,18 @@ use crate::{ session_info, }; +pub trait RewardValidators { + // Give each validator a reward, likely small, for participating in the dispute. + fn reward_dispute_statement(validators: impl IntoIterator); +} + pub trait Config: frame_system::Config + configuration::Config + session_info::Config { type Event: From + Into<::Event>; + type RewardValidators: RewardValidators; } decl_storage! { @@ -74,7 +80,7 @@ decl_storage! { // fewer than `byzantine_threshold + 1` validators. // // The i'th entry of the vector corresponds to the i'th validator in the session. - SpamSlots: map hasher(twox_64_concat) SessionIndex => Vec; + SpamSlots: map hasher(twox_64_concat) SessionIndex => Option>; // Whether the chain is frozen or not. Starts as `false`. When this is `true`, // the chain will not accept any new parachain blocks for backing or inclusion. // It can only be set back to `false` by governance intervention. @@ -113,6 +119,8 @@ decl_error! { InvalidSignature, /// Validator vote submitted more than once to dispute. DuplicateStatement, + /// Too many spam slots used by some specific validator. + PotentialSpam, } } @@ -183,6 +191,10 @@ struct ImportSummary { slash_against: Vec, // Validators to slash for being (wrongly) on the FOR side. slash_for: Vec, + // New participants in the dispute. + new_participants: bitvec::vec::BitVec, + // Difference in state flags from previous. + new_flags: DisputeStateFlags, } enum VoteImportError { @@ -190,6 +202,15 @@ enum VoteImportError { DuplicateStatement, } +impl From for Error { + fn from(e: VoteImportError) -> Self { + match e { + VoteImportError::ValidatorIndexOutOfBounds => Error::::ValidatorIndexOutOfBounds, + VoteImportError::DuplicateStatement => Error::::DuplicateStatement, + } + } +} + struct DisputeStateImporter { state: DisputeState, now: BlockNumber, @@ -198,7 +219,7 @@ struct DisputeStateImporter { } impl DisputeStateImporter { - fn initialize( + fn new( state: DisputeState, now: BlockNumber, ) -> Self { @@ -213,7 +234,7 @@ impl DisputeStateImporter { } } - fn push(&mut self, validator: ValidatorIndex, valid: bool) + fn import(&mut self, validator: ValidatorIndex, valid: bool) -> Result<(), VoteImportError> { let (bits, other_bits) = if valid { @@ -246,7 +267,7 @@ impl DisputeStateImporter { Ok(()) } - fn conclude(mut self) -> ImportSummary { + fn finish(mut self) -> ImportSummary { let pre_flags = self.pre_flags; let post_flags = DisputeStateFlags::from_state(&self.state); @@ -282,7 +303,7 @@ impl DisputeStateImporter { } }; - // 2. Check for fresh FOR supermajority. + // 2. Check for fresh FOR supermajority. Only if not already concluded. let slash_against = if let (false, true) = pre_post_contains(DisputeStateFlags::FOR_SUPERMAJORITY) { if self.state.concluded_at.is_none() { self.state.concluded_at = Some(self.now.clone()); @@ -299,7 +320,7 @@ impl DisputeStateImporter { // 3. Check for fresh AGAINST supermajority. let slash_for = if let (false, true) = pre_post_contains(DisputeStateFlags::AGAINST_SUPERMAJORITY) { if self.state.concluded_at.is_none() { - self.state.concluded_at = Some(self.now); + self.state.concluded_at = Some(self.now.clone()); } // provide FOR voters to slash. @@ -315,6 +336,8 @@ impl DisputeStateImporter { spam_slot_changes, slash_against, slash_for, + new_participants: self.new_participants, + new_flags: post_flags - pre_flags, } } } @@ -337,15 +360,29 @@ impl Module { // mildly punish all validators involved. they've failed to make // data available to others, so this is most likely spam. SpamSlots::mutate(session_index, |spam_slots| { + let spam_slots = match spam_slots { + Some(ref mut s) => s, + None => return, + }; + + let byzantine_threshold = byzantine_threshold(spam_slots.len()); + let participating = dispute.validators_for | dispute.validators_against; + let decrement_spam = participating.count_ones() <= byzantine_threshold; for validator_index in participating { // TODO [now]: slight punishment. - // also reduce spam slots for all validators involved. this does - // open us up to more spam, but only for validators who are willing + // also reduce spam slots for all validators involved, if the dispute was unconfirmed. + // this does open us up to more spam, but only for validators who are willing // to be punished more. - if let Some(occupied) = spam_slots.get_mut(validator_index as usize) { - *occupied = occupied.saturating_sub(1); + // + // it would be unexpected for this branch to be hit when the dispute has not concluded + // in time, as a dispute guaranteed to have at least one honest participant should + // conclude quickly. + if decrement_spam { + if let Some(occupied) = spam_slots.get_mut(validator_index as usize) { + *occupied = occupied.saturating_sub(1); + } } } }); @@ -456,32 +493,80 @@ impl Module { } }; - // Check all statement signatures - for (statement, validator_index, signature) in &set.statements { - let validator_public = session_info.validators.get(validator_index.0 as usize) - .ok_or(Error::::ValidatorIndexOutOfBounds)?; - - check_signature( - &validator_public, - set.candidate_hash, - set.session, - statement, - signature, - ).map_err(|()| Error::::InvalidSignature)?; - } + // Check and import all votes. + let summary = { + let mut importer = DisputeStateImporter::new(dispute_state, now); + for (statement, validator_index, signature) in &set.statements { + let validator_public = session_info.validators.get(validator_index.0 as usize) + .ok_or(Error::::ValidatorIndexOutOfBounds)?; + + // Check signature before importing. + check_signature( + &validator_public, + set.candidate_hash, + set.session, + statement, + signature, + ).map_err(|()| Error::::InvalidSignature)?; + + let valid = match statement { + DisputeStatement::Valid(_) => true, + DisputeStatement::Invalid(_) => false, + }; + + importer.import(*validator_index, valid).map_err(Error::::from)?; + } + + importer.finish() + }; + + // Apply spam slot changes. Bail early if too many occupied. + { + let mut spam_slots: Vec = SpamSlots::get(&set.session) + .unwrap_or_else(|| vec![0; n_validators]); - let byzantine_threshold = byzantine_threshold(n_validators); - let supermajority_threshold = supermajority_threshold(n_validators); + for (validator_index, spam_slot_change) in summary.spam_slot_changes { + let spam_slot = spam_slots.get_mut(validator_index.0 as usize) + .expect("index is in-bounds, as checked above; qed"); - // TODO [now]: update `DisputeInfo` and determine: - // 1. Spam slot changes. Bail early if too many spam slots occupied. - // 2. Dispute state change. Bail early if duplicate + match spam_slot_change { + SpamSlotChange::Inc => { + ensure!( + *spam_slot < config.dispute_max_spam_slots, + Error::::PotentialSpam, + ); - // TODO [now]: Reward statements based on conclusion. + *spam_slot += 1; + } + SpamSlotChange::Dec => { + *spam_slot = spam_slot.saturating_sub(1); + } + } + } - // TODO [now]: Slash one side if fresh supermajority on the other. + SpamSlots::insert(&set.session, spam_slots); + } - // TODO [now]: Freeze if just concluded local. + // Reward statements. + T::RewardValidators::reward_dispute_statement( + summary.new_participants.iter_ones().map(|i| ValidatorIndex(i as _)) + ); + + // Slash participants on a losing side. + { + // TODO [now]: stash finished dispute and collect `UnsignedTransaction`s later + // with nominator info. + } + + >::insert(&set.session, &set.candidate_hash, &summary.state); + + // Freeze if just concluded against some local candidate + if summary.new_flags.contains(DisputeStateFlags::AGAINST_SUPERMAJORITY) { + if let Some(revert_to) = >::get(&set.session, &set.candidate_hash) { + Frozen::set(true); + // TODO [now]: dispatch reversion log. + } + } Ok(fresh) } diff --git a/runtime/parachains/src/reward_points.rs b/runtime/parachains/src/reward_points.rs index 3fb8435e0916..97dad3c13eb1 100644 --- a/runtime/parachains/src/reward_points.rs +++ b/runtime/parachains/src/reward_points.rs @@ -53,3 +53,5 @@ impl crate::inclusion::RewardValidators for RewardValidatorsWithEraPoints fn reward_bitfields(_validators: impl IntoIterator) { } } + +// TODO [now] implement dispute rewards From 310ee525e1e2c92843de13529c76a857d4d9442c Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 10 May 2021 18:07:09 -0500 Subject: [PATCH 15/67] guide: tweaks --- roadmap/implementers-guide/src/runtime/disputes.md | 2 +- roadmap/implementers-guide/src/types/disputes.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/roadmap/implementers-guide/src/runtime/disputes.md b/roadmap/implementers-guide/src/runtime/disputes.md index a8c608722aaa..3d023f2b45ce 100644 --- a/roadmap/implementers-guide/src/runtime/disputes.md +++ b/roadmap/implementers-guide/src/runtime/disputes.md @@ -42,7 +42,7 @@ Included: double_map (SessionIndex, CandidateHash) -> Option, // fewer than `byzantine_threshold + 1` validators. // // The i'th entry of the vector corresponds to the i'th validator in the session. -SpamSlots: map SessionIndex -> Vec, +SpamSlots: map SessionIndex -> Option>, // Whether the chain is frozen or not. Starts as `false`. When this is `true`, // the chain will not accept any new parachain blocks for backing or inclusion. // It can only be set back to `false` by governance intervention. diff --git a/roadmap/implementers-guide/src/types/disputes.md b/roadmap/implementers-guide/src/types/disputes.md index becace642dfe..36f07e843ea3 100644 --- a/roadmap/implementers-guide/src/types/disputes.md +++ b/roadmap/implementers-guide/src/types/disputes.md @@ -33,8 +33,8 @@ Kinds of dispute statements. Each of these can be combined with a candidate hash ```rust enum ValidDisputeStatementKind { Explicit, - BackingSeconded, - BackingValid, + BackingSeconded(Hash), + BackingValid(Hash), ApprovalChecking, } From 37c3c3aa1b147a8617108282ab22886c6b5c51c2 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 10 May 2021 18:19:08 -0500 Subject: [PATCH 16/67] declare and use punishment trait --- runtime/parachains/src/disputes.rs | 43 +++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 7485e66d1b06..795899e7ea08 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -45,9 +45,29 @@ use crate::{ session_info, }; +/// Reward hooks for disputes. pub trait RewardValidators { // Give each validator a reward, likely small, for participating in the dispute. - fn reward_dispute_statement(validators: impl IntoIterator); + fn reward_dispute_statement(session: SessionIndex, validators: impl IntoIterator); +} + +impl RewardValidators for () { + fn reward_dispute_statement(_: SessionIndex, _: impl IntoIterator) { } +} + +/// Punishment hooks for disputes. +pub trait PunishValidators { + /// Punish a series of validators who were for an invalid parablock. This is expected to be a major + /// punishment. + fn punish_for_invalid(session: SessionIndex, validators: impl IntoIterator); + + /// Punish a series of validators who were against a valid parablock. This is expected to be a minor + /// punishment. + fn punish_against_valid(session: SessionIndex, validators: impl IntoIterator); + + /// Punish a series of validators who were part of a dispute which never concluded. This is expected + /// to be a minor punishment. + fn punish_inconclusive(session: SessionIndex, validators: impl IntoIterator); } pub trait Config: @@ -57,6 +77,7 @@ pub trait Config: { type Event: From + Into<::Event>; type RewardValidators: RewardValidators; + type PunishValidators: PunishValidators; } decl_storage! { @@ -549,13 +570,27 @@ impl Module { // Reward statements. T::RewardValidators::reward_dispute_statement( - summary.new_participants.iter_ones().map(|i| ValidatorIndex(i as _)) + set.session, + summary.new_participants.iter_ones().map(|i| ValidatorIndex(i as _)), ); // Slash participants on a losing side. { - // TODO [now]: stash finished dispute and collect `UnsignedTransaction`s later - // with nominator info. + if summary.new_flags.contains(DisputeStateFlags::FOR_SUPERMAJORITY) { + // a valid candidate, according to 2/3. Punish those on the 'against' side. + T::PunishValidators::punish_against_valid( + set.session, + summary.state.validators_against.iter_ones().map(|i| ValidatorIndex(i as _)), + ); + } + + if summary.new_flags.contains(DisputeStateFlags::AGAINST_SUPERMAJORITY) { + // an invalid candidate, according to 2/3. Punish those on the 'for' side. + T::PunishValidators::punish_against_valid( + set.session, + summary.state.validators_for.iter_ones().map(|i| ValidatorIndex(i as _)), + ); + } } >::insert(&set.session, &set.candidate_hash, &summary.state); From 8180eae8fa375b84bd023cae2d731f2471109e44 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 10 May 2021 18:23:42 -0500 Subject: [PATCH 17/67] punish validators for inconclusive disputes --- runtime/parachains/src/disputes.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 795899e7ea08..18c1e30cca55 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -390,9 +390,7 @@ impl Module { let participating = dispute.validators_for | dispute.validators_against; let decrement_spam = participating.count_ones() <= byzantine_threshold; - for validator_index in participating { - // TODO [now]: slight punishment. - + for validator_index in participating.iter_ones() { // also reduce spam slots for all validators involved, if the dispute was unconfirmed. // this does open us up to more spam, but only for validators who are willing // to be punished more. @@ -406,6 +404,13 @@ impl Module { } } } + + // Slight punishment as these validators have failed to make data available to + // others in a timely manner. + T::PunishValidators::punish_inconclusive( + session_index, + participating.iter_ones().map(|i| ValidatorIndex(i as _)), + ); }); weight += T::DbWeight::get().reads_writes(1, 2); From 2ed7fe5cc2174fbe32a78925f2cdfffc44612220 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 10 May 2021 18:24:40 -0500 Subject: [PATCH 18/67] guide: tiny fix --- roadmap/implementers-guide/src/runtime/disputes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/roadmap/implementers-guide/src/runtime/disputes.md b/roadmap/implementers-guide/src/runtime/disputes.md index 3d023f2b45ce..c5d25dbd18c6 100644 --- a/roadmap/implementers-guide/src/runtime/disputes.md +++ b/roadmap/implementers-guide/src/runtime/disputes.md @@ -96,7 +96,7 @@ Frozen: bool, * `is_frozen()`: Load the value of `Frozen` from storage. -* `revert_and_freeze(BlockNumber): +* `revert_and_freeze(BlockNumber)`: 1. If `is_frozen()` return. 1. issue a digest in the block header which indicates the chain is to be abandoned back to the stored block number. 1. Set `Frozen` to true. From 31c27c04cab839aee523eb54cfa4cf591b5b4018 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 10 May 2021 18:26:04 -0500 Subject: [PATCH 19/67] guide: update docs --- roadmap/implementers-guide/src/runtime/disputes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/roadmap/implementers-guide/src/runtime/disputes.md b/roadmap/implementers-guide/src/runtime/disputes.md index c5d25dbd18c6..41e2fc106809 100644 --- a/roadmap/implementers-guide/src/runtime/disputes.md +++ b/roadmap/implementers-guide/src/runtime/disputes.md @@ -85,7 +85,7 @@ Frozen: bool, 1. Return true if just initiated, false otherwise. * `disputes() -> Vec<(SessionIndex, CandidateHash, DisputeState)>`: Get a list of all disputes and info about dispute state. - 1. Iterate over all disputes in `Disputes`. Set the flag according to `concluded`. + 1. Iterate over all disputes in `Disputes` and collect into a vector. * `note_included(SessionIndex, CandidateHash, included_in: BlockNumber)`: 1. Add `(SessionIndex, CandidateHash)` to the `Included` map with `included_in - 1` as the value. From cfdbebdd41cea703764ecbc54a32f365c01d7ffd Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 10 May 2021 18:27:52 -0500 Subject: [PATCH 20/67] add disputes getter fn --- runtime/parachains/src/disputes.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 18c1e30cca55..5648471211f7 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -610,6 +610,10 @@ impl Module { Ok(fresh) } + + fn disputes() -> Vec<(SessionIndex, CandidateHash, DisputeState)> { + >::iter().collect() + } } fn check_signature( From dcd410ce9cadf8e32e4388976944cf717fe47fb4 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 10 May 2021 18:45:42 -0500 Subject: [PATCH 21/67] guide: small change to spam slots handling --- roadmap/implementers-guide/src/runtime/disputes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/roadmap/implementers-guide/src/runtime/disputes.md b/roadmap/implementers-guide/src/runtime/disputes.md index 41e2fc106809..d91c15b0f9ae 100644 --- a/roadmap/implementers-guide/src/runtime/disputes.md +++ b/roadmap/implementers-guide/src/runtime/disputes.md @@ -74,7 +74,7 @@ Frozen: bool, 1. If there is no dispute under `Disputes`, create a new `DisputeState` with blank bitfields. 1. If `concluded_at` is `Some`, and is `concluded_at + config.post_conclusion_acceptance_period < now`, return false. 1. If the overlap of the validators in the `DisputeStatementSet` and those already present in the `DisputeState` is fewer in number than `byzantine_threshold + 1` and the candidate is not present in the `Included` map - 1. increment `SpamSlots` for each validator in the `DisputeStatementSet` which is not already in the `DisputeState`. Initialize the `SpamSlots` to a zeroed vector first, if necessary. + 1. increment `SpamSlots` for each validator in the `DisputeStatementSet` which is not already in the `DisputeState`. Initialize the `SpamSlots` to a zeroed vector first, if necessary. do not increment `SpamSlots` if the candidate is local. 1. If the value for any spam slot exceeds `config.dispute_max_spam_slots`, return false. 1. If the overlap of the validators in the `DisputeStatementSet` and those already present in the `DisputeState` is at least `byzantine_threshold + 1`, the `DisputeState` has fewer than `byzantine_threshold + 1` validators, and the candidate is not present in the `Included` map, decrement `SpamSlots` for each validator in the `DisputeState`. 1. Import all statements into the dispute. This should fail if any statements are duplicate or if the corresponding bit for the corresponding validator is set in the dispute already. From ebd8b18fd7d77071abfad8da4fd145871ff9bd98 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 10 May 2021 18:45:48 -0500 Subject: [PATCH 22/67] improve spam slots handling and fix some bugs --- runtime/parachains/src/disputes.rs | 85 ++++++++++++++++++++++-------- 1 file changed, 64 insertions(+), 21 deletions(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 5648471211f7..b4cce8ad1a5e 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -27,7 +27,7 @@ use primitives::v1::{ ExplicitDisputeStatement, CompactStatement, SigningContext, ApprovalVote, ValidatorIndex, }; use sp_runtime::{ - traits::{One, Saturating, AppVerify}, + traits::{One, Zero, Saturating, AppVerify}, DispatchResult, DispatchError, SaturatedConversion, }; use frame_system::ensure_root; @@ -378,6 +378,13 @@ impl Module { dispute.concluded_at = Some(now); >::insert(session_index, candidate_hash, &dispute); + if >::contains_key(&session_index, &candidate_hash) { + // Local disputes don't count towards spam. + + weight += T::DbWeight::get().reads_writes(1, 1); + continue; + } + // mildly punish all validators involved. they've failed to make // data available to others, so this is most likely spam. SpamSlots::mutate(session_index, |spam_slots| { @@ -386,24 +393,14 @@ impl Module { None => return, }; - let byzantine_threshold = byzantine_threshold(spam_slots.len()); - - let participating = dispute.validators_for | dispute.validators_against; - let decrement_spam = participating.count_ones() <= byzantine_threshold; - for validator_index in participating.iter_ones() { - // also reduce spam slots for all validators involved, if the dispute was unconfirmed. - // this does open us up to more spam, but only for validators who are willing - // to be punished more. - // - // it would be unexpected for this branch to be hit when the dispute has not concluded - // in time, as a dispute guaranteed to have at least one honest participant should - // conclude quickly. - if decrement_spam { - if let Some(occupied) = spam_slots.get_mut(validator_index as usize) { - *occupied = occupied.saturating_sub(1); - } - } - } + // also reduce spam slots for all validators involved, if the dispute was unconfirmed. + // this does open us up to more spam, but only for validators who are willing + // to be punished more. + // + // it would be unexpected for any change here to occur when the dispute has not concluded + // in time, as a dispute guaranteed to have at least one honest participant should + // conclude quickly. + let participating = decrement_spam(spam_slots, &dispute); // Slight punishment as these validators have failed to make data available to // others in a timely manner. @@ -413,7 +410,7 @@ impl Module { ); }); - weight += T::DbWeight::get().reads_writes(1, 2); + weight += T::DbWeight::get().reads_writes(2, 2); } } @@ -547,7 +544,7 @@ impl Module { }; // Apply spam slot changes. Bail early if too many occupied. - { + if !>::contains_key(&set.session, &set.candidate_hash) { let mut spam_slots: Vec = SpamSlots::get(&set.session) .unwrap_or_else(|| vec![0; n_validators]); @@ -614,6 +611,52 @@ impl Module { fn disputes() -> Vec<(SessionIndex, CandidateHash, DisputeState)> { >::iter().collect() } + + fn note_included(session: SessionIndex, candidate_hash: CandidateHash, included_in: T::BlockNumber) { + if included_in.is_zero() { return } + + let revert_to = included_in - One::one(); + + >::insert(&session, &candidate_hash, revert_to); + + // If we just included a block locally which has a live dispute, decrement spam slots + // for any involved validators, if the dispute is not already confirmed by f + 1. + if let Some(state) = >::get(&session, candidate_hash) { + SpamSlots::mutate(&session, |spam_slots| { + if let Some(ref mut spam_slots) = *spam_slots { + decrement_spam(spam_slots, &state); + } + }); + + let supermajority_threshold = supermajority_threshold(state.validators_against.len()); + if state.validators_against.count_ones() >= supermajority_threshold { + // TODO [now]: revert and freeze. + } + } + } +} + +// If the dispute had not enough validators to confirm, decrement spam slots for all the participating +// validators. +// +// returns the set of participating validators as a bitvec. +fn decrement_spam( + spam_slots: &mut [u32], + dispute: &DisputeState, +) -> bitvec::vec::BitVec { + let byzantine_threshold = byzantine_threshold(spam_slots.len()); + + let participating = dispute.validators_for.clone() | dispute.validators_against.iter().by_val(); + let decrement_spam = participating.count_ones() <= byzantine_threshold; + for validator_index in participating.iter_ones() { + if decrement_spam { + if let Some(occupied) = spam_slots.get_mut(validator_index as usize) { + *occupied = occupied.saturating_sub(1); + } + } + } + + participating } fn check_signature( From da2d43ac9f43728f13f06443f2f50acd6c5e596d Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 10 May 2021 18:55:15 -0500 Subject: [PATCH 23/67] finish API of disputes runtime --- runtime/parachains/src/disputes.rs | 34 +++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index b4cce8ad1a5e..16b210009d1b 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -105,7 +105,7 @@ decl_storage! { // Whether the chain is frozen or not. Starts as `false`. When this is `true`, // the chain will not accept any new parachain blocks for backing or inclusion. // It can only be set back to `false` by governance intervention. - Frozen: bool; + Frozen get(fn is_frozen): bool; } } @@ -600,19 +600,18 @@ impl Module { // Freeze if just concluded against some local candidate if summary.new_flags.contains(DisputeStateFlags::AGAINST_SUPERMAJORITY) { if let Some(revert_to) = >::get(&set.session, &set.candidate_hash) { - Frozen::set(true); - // TODO [now]: dispatch reversion log. + Self::revert_and_freeze(revert_to); } } Ok(fresh) } - fn disputes() -> Vec<(SessionIndex, CandidateHash, DisputeState)> { + pub(crate) fn disputes() -> Vec<(SessionIndex, CandidateHash, DisputeState)> { >::iter().collect() } - fn note_included(session: SessionIndex, candidate_hash: CandidateHash, included_in: T::BlockNumber) { + pub(crate) fn note_included(session: SessionIndex, candidate_hash: CandidateHash, included_in: T::BlockNumber) { if included_in.is_zero() { return } let revert_to = included_in - One::one(); @@ -628,12 +627,31 @@ impl Module { } }); - let supermajority_threshold = supermajority_threshold(state.validators_against.len()); - if state.validators_against.count_ones() >= supermajority_threshold { - // TODO [now]: revert and freeze. + if has_supermajority_against(&state) { + Self::revert_and_freeze(revert_to); } } } + + pub(crate) fn could_be_invalid(session: SessionIndex, candidate_hash: CandidateHash) -> bool { + >::get(&session, &candidate_hash).map_or(false, |dispute| { + // A dispute that is ongoing or has concluded with supermajority-against. + dispute.concluded_at.is_none() || has_supermajority_against(&dispute) + }) + } + + pub(crate) fn revert_and_freeze(revert_to: T::BlockNumber) { + if Self::is_frozen() { return } + + // TODO [now]: issue digest. + + Frozen::set(true); + } +} + +fn has_supermajority_against(dispute: &DisputeState) -> bool { + let supermajority_threshold = supermajority_threshold(dispute.validators_against.len()); + dispute.validators_against.count_ones() >= supermajority_threshold } // If the dispute had not enough validators to confirm, decrement spam slots for all the participating From d09c8515a0881417bb7c456c6214b62bad14c7ba Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 10 May 2021 19:00:39 -0500 Subject: [PATCH 24/67] define and deposit `RevertTo` log --- primitives/src/v1/mod.rs | 4 ++++ runtime/parachains/src/disputes.rs | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/primitives/src/v1/mod.rs b/primitives/src/v1/mod.rs index a4559b6f9dca..c632dee680f3 100644 --- a/primitives/src/v1/mod.rs +++ b/primitives/src/v1/mod.rs @@ -1048,6 +1048,10 @@ pub enum ConsensusLog { /// number in the current chain, inclusive. #[codec(index = 3)] ForceApprove(BlockNumber), + /// The runtime is informing the node to revert the chain back to the given block number + /// in the same chain. + #[codec(index = 4)] + RevertTo(BlockNumber), } impl ConsensusLog { diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 16b210009d1b..61004f481e0e 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -25,6 +25,7 @@ use primitives::v1::{ DisputeState, DisputeStatementSet, MultiDisputeStatementSet, ValidatorId, ValidatorSignature, DisputeStatement, ValidDisputeStatementKind, InvalidDisputeStatementKind, ExplicitDisputeStatement, CompactStatement, SigningContext, ApprovalVote, ValidatorIndex, + ConsensusLog, }; use sp_runtime::{ traits::{One, Zero, Saturating, AppVerify}, @@ -643,7 +644,8 @@ impl Module { pub(crate) fn revert_and_freeze(revert_to: T::BlockNumber) { if Self::is_frozen() { return } - // TODO [now]: issue digest. + let revert_to = revert_to.saturated_into(); + >::deposit_log(ConsensusLog::RevertTo(revert_to).into()); Frozen::set(true); } From cfe3241b01f2b3f6e173c3439fb172089f24378e Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 10 May 2021 19:18:10 -0500 Subject: [PATCH 25/67] begin integrating disputes into para_inherent --- runtime/parachains/src/disputes.rs | 1 + runtime/parachains/src/paras_inherent.rs | 41 +++++++++++++++++++++--- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 61004f481e0e..6a126d9a010f 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -445,6 +445,7 @@ impl Module { } /// Handle sets of dispute statements corresponding to 0 or more candidates. + /// Returns a vector of freshly created disputes. pub(crate) fn provide_multi_dispute_data(statement_sets: MultiDisputeStatementSet) -> Result, DispatchError> { diff --git a/runtime/parachains/src/paras_inherent.rs b/runtime/parachains/src/paras_inherent.rs index c96495d639e3..660ee8d01aee 100644 --- a/runtime/parachains/src/paras_inherent.rs +++ b/runtime/parachains/src/paras_inherent.rs @@ -35,8 +35,10 @@ use frame_support::{ }; use frame_system::ensure_none; use crate::{ + disputes, inclusion, scheduler::{self, FreedReason}, + shared, ump, }; @@ -47,7 +49,7 @@ const INCLUSION_INHERENT_CLAIMED_WEIGHT: Weight = 1_000_000_000; // we assume that 75% of an paras inherent's weight is used processing backed candidates const MINIMAL_INCLUSION_INHERENT_WEIGHT: Weight = INCLUSION_INHERENT_CLAIMED_WEIGHT / 4; -pub trait Config: inclusion::Config + scheduler::Config {} +pub trait Config: disputes::Config + inclusion::Config + scheduler::Config {} decl_storage! { trait Store for Module as ParaInherent { @@ -99,7 +101,7 @@ decl_module! { bitfields: signed_bitfields, backed_candidates, parent_header, - disputes: _, + disputes, } = data; ensure_none(origin)?; @@ -112,6 +114,29 @@ decl_module! { Error::::InvalidParentHeader, ); + // Handle disputes logic. + let freed_disputed: Vec<(_, FreedReason)> = { + let fresh_disputes = >::provide_multi_dispute_data(disputes)?; + if >::is_frozen() { + // The relay chain we are currently on is invalid. Proceed no further on parachains. + Included::set(Some(())); + return Ok(Some( + MINIMAL_INCLUSION_INHERENT_WEIGHT + ).into()); + } + + let current_session = >::session_index(); + let any_current_session_disputes = fresh_disputes.iter() + .any(|(s, _)| s == ¤t_session); + + if any_current_session_disputes { + // TODO [now]: inclusion::collect_disputed. + unimplemented!(); + } else { + Vec::new() + } + }; + // Process new availability bitfields, yielding any availability cores whose // work has now concluded. let expected_bits = >::availability_cores().len(); @@ -121,6 +146,8 @@ decl_module! { >::core_para, )?; + // TODO [now]: Disputes::note_included. + // Handle timeouts for any availability core work. let availability_pred = >::availability_timeout_predicate(); let freed_timeout = if let Some(pred) = availability_pred { @@ -130,8 +157,12 @@ decl_module! { }; // Schedule paras again, given freed cores, and reasons for freeing. - let freed = freed_concluded.into_iter().map(|c| (c, FreedReason::Concluded)) - .chain(freed_timeout.into_iter().map(|c| (c, FreedReason::TimedOut))); + let mut freed = freed_disputed.into_iter() + .chain(freed_concluded.into_iter().map(|c| (c, FreedReason::Concluded))) + .chain(freed_timeout.into_iter().map(|c| (c, FreedReason::TimedOut))) + .collect::>(); + + freed.sort_unstable_by_key(|pair| pair.0); // sort by core index >::clear(); >::schedule( @@ -142,6 +173,8 @@ decl_module! { let backed_candidates = limit_backed_candidates::(backed_candidates); let backed_candidates_len = backed_candidates.len() as Weight; + // TODO [now]: check that `Disputes::could_be_invalid` is false for all backed. + // Process backed candidates according to scheduled cores. let parent_storage_root = parent_header.state_root().clone(); let occupied = >::process_candidates( From d14e9045cc5bfd3be34e8c9ff446677b02e85538 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 10 May 2021 19:19:34 -0500 Subject: [PATCH 26/67] use precomputed slash_for/against --- runtime/parachains/src/disputes.rs | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 6a126d9a010f..a4cc808b2b2c 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -580,21 +580,17 @@ impl Module { // Slash participants on a losing side. { - if summary.new_flags.contains(DisputeStateFlags::FOR_SUPERMAJORITY) { - // a valid candidate, according to 2/3. Punish those on the 'against' side. - T::PunishValidators::punish_against_valid( - set.session, - summary.state.validators_against.iter_ones().map(|i| ValidatorIndex(i as _)), - ); - } - - if summary.new_flags.contains(DisputeStateFlags::AGAINST_SUPERMAJORITY) { - // an invalid candidate, according to 2/3. Punish those on the 'for' side. - T::PunishValidators::punish_against_valid( - set.session, - summary.state.validators_for.iter_ones().map(|i| ValidatorIndex(i as _)), - ); - } + // a valid candidate, according to 2/3. Punish those on the 'against' side. + T::PunishValidators::punish_against_valid( + set.session, + summary.slash_against, + ); + + // an invalid candidate, according to 2/3. Punish those on the 'for' side. + T::PunishValidators::punish_for_invalid( + set.session, + summary.slash_for, + ); } >::insert(&set.session, &set.candidate_hash, &summary.state); From 7ffe889ed7b253d60c823e9bd88b339f41315e13 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 10 May 2021 19:22:50 -0500 Subject: [PATCH 27/67] return candidate hash from process_bitfields --- runtime/parachains/src/inclusion.rs | 8 ++------ runtime/parachains/src/paras_inherent.rs | 2 +- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/runtime/parachains/src/inclusion.rs b/runtime/parachains/src/inclusion.rs index a884656f7ad8..a72d2c7b9315 100644 --- a/runtime/parachains/src/inclusion.rs +++ b/runtime/parachains/src/inclusion.rs @@ -238,7 +238,7 @@ impl Module { expected_bits: usize, unchecked_bitfields: UncheckedSignedAvailabilityBitfields, core_lookup: impl Fn(CoreIndex) -> Option, - ) -> Result, DispatchError> { + ) -> Result, DispatchError> { let validators = shared::Module::::active_validator_keys(); let session_index = shared::Module::::session_index(); @@ -247,7 +247,6 @@ impl Module { .map(|core_para| core_para.map(|p| (p, PendingAvailability::::get(&p)))) .collect(); - // do sanity checks on the bitfields: // 1. no more than one bitfield per validator // 2. bitfields are ascending by validator index. @@ -368,15 +367,12 @@ impl Module { pending_availability.backing_group, ); - freed_cores.push(pending_availability.core); + freed_cores.push((pending_availability.core, pending_availability.hash)); } else { >::insert(¶_id, &pending_availability); } } - // TODO: pass available candidates onwards to validity module once implemented. - // https://github.com/paritytech/polkadot/issues/1251 - Ok(freed_cores) } diff --git a/runtime/parachains/src/paras_inherent.rs b/runtime/parachains/src/paras_inherent.rs index 660ee8d01aee..83f2bbdbfcd1 100644 --- a/runtime/parachains/src/paras_inherent.rs +++ b/runtime/parachains/src/paras_inherent.rs @@ -158,7 +158,7 @@ decl_module! { // Schedule paras again, given freed cores, and reasons for freeing. let mut freed = freed_disputed.into_iter() - .chain(freed_concluded.into_iter().map(|c| (c, FreedReason::Concluded))) + .chain(freed_concluded.into_iter().map(|(c, _hash)| (c, FreedReason::Concluded))) .chain(freed_timeout.into_iter().map(|c| (c, FreedReason::TimedOut))) .collect::>(); From 36388775792089e604d0d9fd4a7eeacadfc3e9a9 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 10 May 2021 19:26:16 -0500 Subject: [PATCH 28/67] implement inclusion::collect_disputed --- runtime/parachains/src/inclusion.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/runtime/parachains/src/inclusion.rs b/runtime/parachains/src/inclusion.rs index a72d2c7b9315..20189b57f596 100644 --- a/runtime/parachains/src/inclusion.rs +++ b/runtime/parachains/src/inclusion.rs @@ -750,6 +750,28 @@ impl Module { cleaned_up_cores } + /// Cleans up all paras pending availability that are in the given list of disputed candidates. + /// + /// Returns a vector of cleaned-up core IDs. + pub(crate) fn collect_disputed(disputed: Vec) -> Vec { + let mut cleaned_up_ids = Vec::new(); + let mut cleaned_up_cores = Vec::new(); + + for (para_id, pending_record) in >::iter() { + if disputed.contains(&pending_record.hash) { + cleaned_up_ids.push(para_id); + cleaned_up_cores.push(pending_record.core); + } + } + + for para_id in cleaned_up_ids { + let _ = >::take(¶_id); + let _ = ::take(¶_id); + } + + cleaned_up_cores + } + /// Forcibly enact the candidate with the given ID as though it had been deemed available /// by bitfields. /// @@ -2549,4 +2571,6 @@ mod tests { assert!(::iter().collect::>().is_empty()); }); } + + // TODO [now]: test `collect_disputed` } From 82e86da4911a905b5d805ea1d80e5af20c15c10e Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 10 May 2021 19:40:33 -0500 Subject: [PATCH 29/67] finish integration into rest of runtime --- runtime/parachains/src/paras_inherent.rs | 32 ++++++++++++++++++++---- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/runtime/parachains/src/paras_inherent.rs b/runtime/parachains/src/paras_inherent.rs index 83f2bbdbfcd1..00d02bc280b4 100644 --- a/runtime/parachains/src/paras_inherent.rs +++ b/runtime/parachains/src/paras_inherent.rs @@ -70,6 +70,8 @@ decl_error! { /// The hash of the submitted parent header doesn't correspond to the saved block hash of /// the parent. InvalidParentHeader, + /// Potentially invalid candidate. + CandidateCouldBeInvalid, } } @@ -115,6 +117,7 @@ decl_module! { ); // Handle disputes logic. + let current_session = >::session_index(); let freed_disputed: Vec<(_, FreedReason)> = { let fresh_disputes = >::provide_multi_dispute_data(disputes)?; if >::is_frozen() { @@ -125,13 +128,19 @@ decl_module! { ).into()); } - let current_session = >::session_index(); let any_current_session_disputes = fresh_disputes.iter() .any(|(s, _)| s == ¤t_session); if any_current_session_disputes { - // TODO [now]: inclusion::collect_disputed. - unimplemented!(); + let current_session_disputes: Vec<_> = fresh_disputes.iter() + .filter(|(s, _)| s == ¤t_session) + .map(|(_, c)| *c) + .collect(); + + >::collect_disputed(current_session_disputes) + .into_iter() + .map(|core| (core, FreedReason::Concluded)) + .collect() } else { Vec::new() } @@ -146,7 +155,11 @@ decl_module! { >::core_para, )?; - // TODO [now]: Disputes::note_included. + // Inform the disputes module of all included candidates. + let now = >::block_number(); + for (_, candidate_hash) in &freed_concluded { + >::note_included(current_session, *candidate_hash, now); + } // Handle timeouts for any availability core work. let availability_pred = >::availability_timeout_predicate(); @@ -173,7 +186,16 @@ decl_module! { let backed_candidates = limit_backed_candidates::(backed_candidates); let backed_candidates_len = backed_candidates.len() as Weight; - // TODO [now]: check that `Disputes::could_be_invalid` is false for all backed. + // Refuse to back any candidates that are disputed or invalid. + for candidate in &backed_candidates { + ensure!( + !>::could_be_invalid( + current_session, + candidate.candidate.hash(), + ), + Error::::CandidateCouldBeInvalid, + ); + } // Process backed candidates according to scheduled cores. let parent_storage_root = parent_header.state_root().clone(); From 955e0c4f6e1da8b67ad8d1b0fde1decdd4b2124f Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 10 May 2021 19:43:13 -0500 Subject: [PATCH 30/67] add Disputes to initializer --- runtime/parachains/src/initializer.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/runtime/parachains/src/initializer.rs b/runtime/parachains/src/initializer.rs index 9eb441fd8c12..a57fa64d5551 100644 --- a/runtime/parachains/src/initializer.rs +++ b/runtime/parachains/src/initializer.rs @@ -29,7 +29,7 @@ use frame_support::{ use parity_scale_codec::{Encode, Decode}; use crate::{ configuration::{self, HostConfiguration}, - shared, paras, scheduler, inclusion, session_info, dmp, ump, hrmp, + shared, paras, scheduler, inclusion, session_info, disputes, dmp, ump, hrmp, }; /// Information about a session change that has just occurred. @@ -77,6 +77,7 @@ pub trait Config: + scheduler::Config + inclusion::Config + session_info::Config + + disputes::Config + dmp::Config + ump::Config + hrmp::Config @@ -125,7 +126,7 @@ decl_module! { // - Scheduler // - Inclusion // - SessionInfo - // - Validity + // - Disputes // - DMP // - UMP // - HRMP @@ -135,6 +136,7 @@ decl_module! { scheduler::Module::::initializer_initialize(now) + inclusion::Module::::initializer_initialize(now) + session_info::Module::::initializer_initialize(now) + + disputes::Module::::initializer_initialize(now) + dmp::Module::::initializer_initialize(now) + ump::Module::::initializer_initialize(now) + hrmp::Module::::initializer_initialize(now); @@ -149,6 +151,7 @@ decl_module! { hrmp::Module::::initializer_finalize(); ump::Module::::initializer_finalize(); dmp::Module::::initializer_finalize(); + disputes::Module::::initializer_finalize(); session_info::Module::::initializer_finalize(); inclusion::Module::::initializer_finalize(); scheduler::Module::::initializer_finalize(); @@ -228,6 +231,7 @@ impl Module { scheduler::Module::::initializer_on_new_session(¬ification); inclusion::Module::::initializer_on_new_session(¬ification); session_info::Module::::initializer_on_new_session(¬ification); + disputes::Module::::initializer_on_new_session(¬ification); dmp::Module::::initializer_on_new_session(¬ification, &outgoing_paras); ump::Module::::initializer_on_new_session(¬ification, &outgoing_paras); hrmp::Module::::initializer_on_new_session(¬ification, &outgoing_paras); From 83e7b627770220d531d2602a1054012a5e58b0f5 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Fri, 21 May 2021 15:53:50 +0200 Subject: [PATCH 31/67] address suggestions --- runtime/parachains/src/disputes.rs | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 61004f481e0e..f88b989b53b3 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -46,6 +46,20 @@ use crate::{ session_info, }; +/// Whereas the dispute is local or remote. +#[derive(Encode, Decode, Clone, PartialEq, Eq)] +pub enum DisputeLocation { + Local, + Remote, +} + +/// The result of a dispute, whether the candidate is deemed valid (for) or invalid (against). +#[derive(Encode, Decode, Clone, PartialEq, Eq)] +pub enum DisputeResult { + Valid, + Invalid, +} + /// Reward hooks for disputes. pub trait RewardValidators { // Give each validator a reward, likely small, for participating in the dispute. @@ -112,12 +126,13 @@ decl_storage! { decl_event! { pub enum Event { - /// A dispute has been initiated. The boolean is true if the dispute is local. \[para_id\] - DisputeInitiated(ParaId, CandidateHash, bool), - /// A dispute has concluded for or against a candidate. The boolean is true if the candidate - /// is deemed valid (for) and false if invalid (against). - DisputeConcluded(ParaId, CandidateHash, bool), + /// A dispute has been initiated. \[para_id, candidate hash, dispute location\] + DisputeInitiated(ParaId, CandidateHash, DisputeLocation), + /// A dispute has concluded for or against a candidate. + /// \[para_id, candidate hash, dispute result\] + DisputeConcluded(ParaId, CandidateHash, DisputeResult), /// A dispute has timed out due to insufficient participation. + /// \[para_id, candidate hash\] DisputeTimedOut(ParaId, CandidateHash), } } @@ -365,7 +380,7 @@ impl DisputeStateImporter { } impl Module { - /// Called by the iniitalizer to initialize the disputes module. + /// Called by the initalizer to initialize the disputes module. pub(crate) fn initializer_initialize(now: T::BlockNumber) -> Weight { let config = >::config(); From fc3ab87c1be122f05158a883614f506c8f2f6cc4 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Fri, 21 May 2021 16:09:36 +0200 Subject: [PATCH 32/67] use pallet macro --- runtime/parachains/src/disputes.rs | 149 ++++++++++++++++------------- 1 file changed, 80 insertions(+), 69 deletions(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index f88b989b53b3..0c750ed14163 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -17,11 +17,8 @@ //! Runtime component for handling disputes of parachain candidates. use sp_std::prelude::*; -use sp_std::result; -#[cfg(feature = "std")] -use sp_std::marker::PhantomData; use primitives::v1::{ - Id as ParaId, ValidationCode, HeadData, SessionIndex, Hash, BlockNumber, CandidateHash, + Id as ParaId, SessionIndex, CandidateHash, DisputeState, DisputeStatementSet, MultiDisputeStatementSet, ValidatorId, ValidatorSignature, DisputeStatement, ValidDisputeStatementKind, InvalidDisputeStatementKind, ExplicitDisputeStatement, CompactStatement, SigningContext, ApprovalVote, ValidatorIndex, @@ -29,32 +26,26 @@ use primitives::v1::{ }; use sp_runtime::{ traits::{One, Zero, Saturating, AppVerify}, - DispatchResult, DispatchError, SaturatedConversion, -}; -use frame_system::ensure_root; -use frame_support::{ - decl_storage, decl_module, decl_error, decl_event, ensure, - traits::Get, - weights::Weight, + DispatchError, SaturatedConversion, RuntimeDebug, }; +use frame_support::{ensure, traits::Get, weights::Weight}; use parity_scale_codec::{Encode, Decode}; use bitvec::{bitvec, order::Lsb0 as BitOrderLsb0}; use crate::{ configuration::{self, HostConfiguration}, initializer::SessionChangeNotification, - shared, session_info, }; /// Whereas the dispute is local or remote. -#[derive(Encode, Decode, Clone, PartialEq, Eq)] +#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug)] pub enum DisputeLocation { Local, Remote, } /// The result of a dispute, whether the candidate is deemed valid (for) or invalid (against). -#[derive(Encode, Decode, Clone, PartialEq, Eq)] +#[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug)] pub enum DisputeResult { Valid, Invalid, @@ -85,46 +76,68 @@ pub trait PunishValidators { fn punish_inconclusive(session: SessionIndex, validators: impl IntoIterator); } -pub trait Config: - frame_system::Config + - configuration::Config + - session_info::Config -{ - type Event: From + Into<::Event>; - type RewardValidators: RewardValidators; - type PunishValidators: PunishValidators; -} - -decl_storage! { - trait Store for Module as ParaDisputes { - /// The last pruneed session, if any. All data stored by this module - /// references sessions. - LastPrunedSession: Option; - // All ongoing or concluded disputes for the last several sessions. - Disputes: double_map - hasher(twox_64_concat) SessionIndex, - hasher(blake2_128_concat) CandidateHash - => Option>; - // All included blocks on the chain, as well as the block number in this chain that - // should be reverted back to if the candidate is disputed and determined to be invalid. - Included: double_map - hasher(twox_64_concat) SessionIndex, - hasher(blake2_128_concat) CandidateHash - => Option; - // Maps session indices to a vector indicating the number of potentially-spam disputes - // each validator is participating in. Potentially-spam disputes are remote disputes which have - // fewer than `byzantine_threshold + 1` validators. - // - // The i'th entry of the vector corresponds to the i'th validator in the session. - SpamSlots: map hasher(twox_64_concat) SessionIndex => Option>; - // Whether the chain is frozen or not. Starts as `false`. When this is `true`, - // the chain will not accept any new parachain blocks for backing or inclusion. - // It can only be set back to `false` by governance intervention. - Frozen get(fn is_frozen): bool; +pub use pallet::*; +#[frame_support::pallet] +pub mod pallet { + use frame_support::pallet_prelude::*; + use frame_system::pallet_prelude::*; + use super::*; + + #[pallet::config] + pub trait Config: + frame_system::Config + + configuration::Config + + session_info::Config + { + type Event: From + IsType<::Event>; + type RewardValidators: RewardValidators; + type PunishValidators: PunishValidators; } -} -decl_event! { + #[pallet::pallet] + pub struct Pallet(_); + + /// The last pruneed session, if any. All data stored by this module + /// references sessions. + #[pallet::storage] + pub(crate) type LastPrunedSession = StorageValue<_, SessionIndex>; + + /// All ongoing or concluded disputes for the last several sessions. + #[pallet::storage] + pub(crate) type Disputes = StorageDoubleMap< + _, + Twox64Concat, SessionIndex, + Blake2_128Concat, CandidateHash, + DisputeState, + >; + + /// All included blocks on the chain, as well as the block number in this chain that + /// should be reverted back to if the candidate is disputed and determined to be invalid. + #[pallet::storage] + pub(crate) type Included = StorageDoubleMap< + _, + Twox64Concat, SessionIndex, + Blake2_128Concat, CandidateHash, + T::BlockNumber, + >; + + /// Maps session indices to a vector indicating the number of potentially-spam disputes + /// each validator is participating in. Potentially-spam disputes are remote disputes which have + /// fewer than `byzantine_threshold + 1` validators. + /// + /// The i'th entry of the vector corresponds to the i'th validator in the session. + #[pallet::storage] + pub(crate) type SpamSlots = StorageMap<_, Twox64Concat, SessionIndex, Vec>; + + /// Whether the chain is frozen or not. Starts as `false`. When this is `true`, + /// the chain will not accept any new parachain blocks for backing or inclusion. + /// It can only be set back to `false` by governance intervention. + #[pallet::storage] + #[pallet::getter(fn is_frozen)] + pub(crate) type Frozen = StorageValue<_, bool, ValueQuery>; + + #[pallet::event] + #[pallet::generate_deposit(fn deposit_event)] pub enum Event { /// A dispute has been initiated. \[para_id, candidate hash, dispute location\] DisputeInitiated(ParaId, CandidateHash, DisputeLocation), @@ -135,17 +148,15 @@ decl_event! { /// \[para_id, candidate hash\] DisputeTimedOut(ParaId, CandidateHash), } -} -decl_module! { - /// The parachains configuration module. - pub struct Module for enum Call where origin: ::Origin { - fn deposit_event() = default; - } -} + #[pallet::hooks] + impl Hooks> for Pallet {} + + #[pallet::call] + impl Pallet {} -decl_error! { - pub enum Error for Module { + #[pallet::error] + pub enum Error { /// Duplicate dispute statement sets provided. DuplicateDisputeStatementSets, /// Ancient dispute statement provided. @@ -379,7 +390,7 @@ impl DisputeStateImporter { } } -impl Module { +impl Pallet { /// Called by the initalizer to initialize the disputes module. pub(crate) fn initializer_initialize(now: T::BlockNumber) -> Weight { let config = >::config(); @@ -403,7 +414,7 @@ impl Module { // mildly punish all validators involved. they've failed to make // data available to others, so this is most likely spam. - SpamSlots::mutate(session_index, |spam_slots| { + SpamSlots::::mutate(session_index, |spam_slots| { let spam_slots = match spam_slots { Some(ref mut s) => s, None => return, @@ -446,12 +457,12 @@ impl Module { let pruning_target = notification.session_index - config.dispute_period - 1; - LastPrunedSession::mutate(|last_pruned| { + LastPrunedSession::::mutate(|last_pruned| { if let Some(last_pruned) = last_pruned { for to_prune in *last_pruned + 1 ..= pruning_target { >::remove_prefix(to_prune); >::remove_prefix(to_prune); - SpamSlots::remove(to_prune); + SpamSlots::::remove(to_prune); } } @@ -561,7 +572,7 @@ impl Module { // Apply spam slot changes. Bail early if too many occupied. if !>::contains_key(&set.session, &set.candidate_hash) { - let mut spam_slots: Vec = SpamSlots::get(&set.session) + let mut spam_slots: Vec = SpamSlots::::get(&set.session) .unwrap_or_else(|| vec![0; n_validators]); for (validator_index, spam_slot_change) in summary.spam_slot_changes { @@ -583,7 +594,7 @@ impl Module { } } - SpamSlots::insert(&set.session, spam_slots); + SpamSlots::::insert(&set.session, spam_slots); } // Reward statements. @@ -637,7 +648,7 @@ impl Module { // If we just included a block locally which has a live dispute, decrement spam slots // for any involved validators, if the dispute is not already confirmed by f + 1. if let Some(state) = >::get(&session, candidate_hash) { - SpamSlots::mutate(&session, |spam_slots| { + SpamSlots::::mutate(&session, |spam_slots| { if let Some(ref mut spam_slots) = *spam_slots { decrement_spam(spam_slots, &state); } @@ -662,7 +673,7 @@ impl Module { let revert_to = revert_to.saturated_into(); >::deposit_log(ConsensusLog::RevertTo(revert_to).into()); - Frozen::set(true); + Frozen::::set(true); } } From 6fa374cea2108787ea05b8b4af9a843367e29821 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Fri, 21 May 2021 16:17:08 +0200 Subject: [PATCH 33/67] fix typo --- runtime/parachains/src/disputes.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 0c750ed14163..7e8db1e372bb 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -444,10 +444,10 @@ impl Pallet { weight } - /// Called by the iniitalizer to finalize the disputes module. + /// Called by the initalizer to finalize the disputes module. pub(crate) fn initializer_finalize() { } - /// Called by the iniitalizer to note a new session in the disputes module. + /// Called by the initalizer to note a new session in the disputes module. pub(crate) fn initializer_on_new_session(notification: &SessionChangeNotification) { let config = >::config(); From 1b4838da93901047e5d914c7f3be850e9be45182 Mon Sep 17 00:00:00 2001 From: Guillaume Thiolliere Date: Thu, 27 May 2021 18:50:48 +0200 Subject: [PATCH 34/67] Update runtime/parachains/src/disputes.rs --- runtime/parachains/src/disputes.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 7e8db1e372bb..642ef521f692 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -615,7 +615,7 @@ impl Pallet { if summary.new_flags.contains(DisputeStateFlags::AGAINST_SUPERMAJORITY) { // an invalid candidate, according to 2/3. Punish those on the 'for' side. - T::PunishValidators::punish_against_valid( + T::PunishValidators::punish_for_invalid( set.session, summary.state.validators_for.iter_ones().map(|i| ValidatorIndex(i as _)), ); From 7ef44b50633cc70219688d79f423a9f3edb5d1f0 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Fri, 28 May 2021 19:24:01 +0200 Subject: [PATCH 35/67] add test: fix pruning --- primitives/src/v1/mod.rs | 2 +- runtime/parachains/src/disputes.rs | 1250 +++++++++++++++++++++- runtime/parachains/src/initializer.rs | 20 +- runtime/parachains/src/mock.rs | 57 +- runtime/parachains/src/paras_inherent.rs | 8 +- 5 files changed, 1316 insertions(+), 21 deletions(-) diff --git a/primitives/src/v1/mod.rs b/primitives/src/v1/mod.rs index c632dee680f3..2c205393c5a5 100644 --- a/primitives/src/v1/mod.rs +++ b/primitives/src/v1/mod.rs @@ -1146,7 +1146,7 @@ pub struct DisputeStatementSet { pub type MultiDisputeStatementSet = Vec; /// The entire state of a dispute. -#[derive(Encode, Decode, Clone, RuntimeDebug)] +#[derive(Encode, Decode, Clone, RuntimeDebug, PartialEq)] pub struct DisputeState { /// A bitfield indicating all validators for the candidate. pub validators_for: BitVec, // one bit per validator. diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index d96b1ff8350c..af582623a042 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -100,11 +100,11 @@ pub mod pallet { /// The last pruneed session, if any. All data stored by this module /// references sessions. #[pallet::storage] - pub(crate) type LastPrunedSession = StorageValue<_, SessionIndex>; + pub(super) type LastPrunedSession = StorageValue<_, SessionIndex>; /// All ongoing or concluded disputes for the last several sessions. #[pallet::storage] - pub(crate) type Disputes = StorageDoubleMap< + pub(super) type Disputes = StorageDoubleMap< _, Twox64Concat, SessionIndex, Blake2_128Concat, CandidateHash, @@ -114,7 +114,7 @@ pub mod pallet { /// All included blocks on the chain, as well as the block number in this chain that /// should be reverted back to if the candidate is disputed and determined to be invalid. #[pallet::storage] - pub(crate) type Included = StorageDoubleMap< + pub(super) type Included = StorageDoubleMap< _, Twox64Concat, SessionIndex, Blake2_128Concat, CandidateHash, @@ -127,14 +127,14 @@ pub mod pallet { /// /// The i'th entry of the vector corresponds to the i'th validator in the session. #[pallet::storage] - pub(crate) type SpamSlots = StorageMap<_, Twox64Concat, SessionIndex, Vec>; + pub(super) type SpamSlots = StorageMap<_, Twox64Concat, SessionIndex, Vec>; /// Whether the chain is frozen or not. Starts as `false`. When this is `true`, /// the chain will not accept any new parachain blocks for backing or inclusion. /// It can only be set back to `false` by governance intervention. #[pallet::storage] #[pallet::getter(fn is_frozen)] - pub(crate) type Frozen = StorageValue<_, bool, ValueQuery>; + pub(super) type Frozen = StorageValue<_, bool, ValueQuery>; #[pallet::event] #[pallet::generate_deposit(fn deposit_event)] @@ -225,6 +225,7 @@ impl DisputeStateFlags { } } +#[derive(PartialEq, RuntimeDebug)] enum SpamSlotChange { Inc, Dec, @@ -245,6 +246,7 @@ struct ImportSummary { new_flags: DisputeStateFlags, } +#[derive(RuntimeDebug, PartialEq, Eq)] enum VoteImportError { ValidatorIndexOutOfBounds, DuplicateStatement, @@ -458,12 +460,16 @@ impl Pallet { let pruning_target = notification.session_index - config.dispute_period - 1; LastPrunedSession::::mutate(|last_pruned| { - if let Some(last_pruned) = last_pruned { - for to_prune in *last_pruned + 1 ..= pruning_target { - >::remove_prefix(to_prune); - >::remove_prefix(to_prune); - SpamSlots::::remove(to_prune); - } + let to_prune = if let Some(last_pruned) = last_pruned { + *last_pruned + 1 ..= pruning_target + } else { + pruning_target ..= pruning_target + }; + + for to_prune in to_prune { + >::remove_prefix(to_prune); + >::remove_prefix(to_prune); + SpamSlots::::remove(to_prune); } *last_pruned = Some(pruning_target); @@ -747,3 +753,1225 @@ fn check_signature( Err(()) } } + +#[cfg(test)] +mod tests { + use super::*; + use frame_system::InitKind; + use frame_support::{assert_ok, assert_err, assert_noop, traits::{OnInitialize, OnFinalize}}; + use crate::mock::{ + new_test_ext, Test, System, AllPallets, Initializer, AccountId, MockGenesisConfig, + REWARD_VALIDATORS, PUNISH_VALIDATORS_FOR, PUNISH_VALIDATORS_AGAINST, + PUNISH_VALIDATORS_INCONCLUSIVE, + }; + use sp_core::{Pair, crypto::CryptoType}; + use primitives::v1::BlockNumber; + + // All arguments for `initializer::on_new_session` + type NewSession<'a> = (bool, SessionIndex, Vec<(&'a AccountId, ValidatorId)>, Option>); + + // Run to specific block, while calling disputes pallet hooks manually, because disputes is not + // integrated in initializer yet. + fn run_to_block<'a>( + to: BlockNumber, + new_session: impl Fn(BlockNumber) -> Option>, + ) { + while System::block_number() < to { + let b = System::block_number(); + if b != 0 { + AllPallets::on_finalize(b); + System::finalize(); + } + + System::initialize(&(b + 1), &Default::default(), &Default::default(), InitKind::Full); + AllPallets::on_initialize(b + 1); + + if let Some(new_session) = new_session(b + 1) { + Initializer::test_trigger_on_new_session( + new_session.0, + new_session.1, + new_session.2.into_iter(), + new_session.3.map(|q| q.into_iter()), + ); + } + } + } + + #[test] + fn test_byzantine_threshold() { + assert_eq!(byzantine_threshold(0), 0); + assert_eq!(byzantine_threshold(1), 0); + assert_eq!(byzantine_threshold(2), 0); + assert_eq!(byzantine_threshold(3), 0); + assert_eq!(byzantine_threshold(4), 1); + assert_eq!(byzantine_threshold(5), 1); + assert_eq!(byzantine_threshold(6), 1); + assert_eq!(byzantine_threshold(7), 2); + } + + #[test] + fn test_supermajority_threshold() { + assert_eq!(supermajority_threshold(0), 0); + assert_eq!(supermajority_threshold(1), 1); + assert_eq!(supermajority_threshold(2), 2); + assert_eq!(supermajority_threshold(3), 3); + assert_eq!(supermajority_threshold(4), 3); + assert_eq!(supermajority_threshold(5), 4); + assert_eq!(supermajority_threshold(6), 5); + assert_eq!(supermajority_threshold(7), 5); + } + + #[test] + fn test_dispute_state_flag_from_state() { + assert_eq!( + DisputeStateFlags::from_state(&DisputeState { + validators_for: bitvec![BitOrderLsb0, u8; 0, 0, 0, 0, 0, 0, 0, 0], + validators_against: bitvec![BitOrderLsb0, u8; 0, 0, 0, 0, 0, 0, 0, 0], + start: 0, + concluded_at: None, + }), + DisputeStateFlags::default(), + ); + + assert_eq!( + DisputeStateFlags::from_state(&DisputeState { + validators_for: bitvec![BitOrderLsb0, u8; 1, 1, 1, 1, 1, 0, 0], + validators_against: bitvec![BitOrderLsb0, u8; 0, 0, 0, 0, 0, 0, 0], + start: 0, + concluded_at: None, + }), + DisputeStateFlags::FOR_SUPERMAJORITY | DisputeStateFlags::CONFIRMED, + ); + + assert_eq!( + DisputeStateFlags::from_state(&DisputeState { + validators_for: bitvec![BitOrderLsb0, u8; 0, 0, 0, 0, 0, 0, 0], + validators_against: bitvec![BitOrderLsb0, u8; 1, 1, 1, 1, 1, 0, 0], + start: 0, + concluded_at: None, + }), + DisputeStateFlags::AGAINST_SUPERMAJORITY | DisputeStateFlags::CONFIRMED, + ); + } + + #[test] + fn test_import_new_participant_spam_inc() { + let mut importer = DisputeStateImporter::new( + DisputeState { + validators_for: bitvec![BitOrderLsb0, u8; 1, 0, 0, 0, 0, 0, 0, 0], + validators_against: bitvec![BitOrderLsb0, u8; 0, 0, 0, 0, 0, 0, 0, 0], + start: 0, + concluded_at: None, + }, + 0, + ); + + assert_err!( + importer.import(ValidatorIndex(9), true), + VoteImportError::ValidatorIndexOutOfBounds, + ); + + assert_err!( + importer.import(ValidatorIndex(0), true), + VoteImportError::DuplicateStatement, + ); + assert_ok!(importer.import(ValidatorIndex(0), false)); + + assert_ok!(importer.import(ValidatorIndex(2), true)); + assert_err!( + importer.import(ValidatorIndex(2), true), + VoteImportError::DuplicateStatement, + ); + + assert_ok!(importer.import(ValidatorIndex(2), false)); + assert_err!( + importer.import(ValidatorIndex(2), false), + VoteImportError::DuplicateStatement, + ); + + let summary = importer.finish(); + assert_eq!(summary.new_flags, DisputeStateFlags::default()); + assert_eq!( + summary.state, + DisputeState { + validators_for: bitvec![BitOrderLsb0, u8; 1, 0, 1, 0, 0, 0, 0, 0], + validators_against: bitvec![BitOrderLsb0, u8; 1, 0, 1, 0, 0, 0, 0, 0], + start: 0, + concluded_at: None, + }, + ); + assert_eq!( + summary.spam_slot_changes, + vec![(ValidatorIndex(2), SpamSlotChange::Inc)], + ); + assert!(summary.slash_for.is_empty()); + assert!(summary.slash_against.is_empty()); + assert_eq!(summary.new_participants, bitvec![BitOrderLsb0, u8; 0, 0, 1, 0, 0, 0, 0, 0]); + } + + #[test] + fn test_import_prev_participant_spam_dec_confirmed() { + let mut importer = DisputeStateImporter::new( + DisputeState { + validators_for: bitvec![BitOrderLsb0, u8; 1, 0, 0, 0, 0, 0, 0, 0], + validators_against: bitvec![BitOrderLsb0, u8; 0, 1, 0, 0, 0, 0, 0, 0], + start: 0, + concluded_at: None, + }, + 0, + ); + + assert_ok!(importer.import(ValidatorIndex(2), true)); + + let summary = importer.finish(); + assert_eq!( + summary.state, + DisputeState { + validators_for: bitvec![BitOrderLsb0, u8; 1, 0, 1, 0, 0, 0, 0, 0], + validators_against: bitvec![BitOrderLsb0, u8; 0, 1, 0, 0, 0, 0, 0, 0], + start: 0, + concluded_at: None, + }, + ); + assert_eq!( + summary.spam_slot_changes, + vec![ + (ValidatorIndex(0), SpamSlotChange::Dec), + (ValidatorIndex(1), SpamSlotChange::Dec), + ], + ); + assert!(summary.slash_for.is_empty()); + assert!(summary.slash_against.is_empty()); + assert_eq!(summary.new_participants, bitvec![BitOrderLsb0, u8; 0, 0, 1, 0, 0, 0, 0, 0]); + assert_eq!(summary.new_flags, DisputeStateFlags::CONFIRMED); + } + + #[test] + fn test_import_prev_participant_spam_dec_confirmed_slash_for() { + let mut importer = DisputeStateImporter::new( + DisputeState { + validators_for: bitvec![BitOrderLsb0, u8; 1, 0, 0, 0, 0, 0, 0, 0], + validators_against: bitvec![BitOrderLsb0, u8; 0, 1, 0, 0, 0, 0, 0, 0], + start: 0, + concluded_at: None, + }, + 0, + ); + + assert_ok!(importer.import(ValidatorIndex(2), true)); + assert_ok!(importer.import(ValidatorIndex(2), false)); + assert_ok!(importer.import(ValidatorIndex(3), false)); + assert_ok!(importer.import(ValidatorIndex(4), false)); + assert_ok!(importer.import(ValidatorIndex(5), false)); + assert_ok!(importer.import(ValidatorIndex(6), false)); + + let summary = importer.finish(); + assert_eq!( + summary.state, + DisputeState { + validators_for: bitvec![BitOrderLsb0, u8; 1, 0, 1, 0, 0, 0, 0, 0], + validators_against: bitvec![BitOrderLsb0, u8; 0, 1, 1, 1, 1, 1, 1, 0], + start: 0, + concluded_at: Some(0), + }, + ); + assert_eq!( + summary.spam_slot_changes, + vec![ + (ValidatorIndex(0), SpamSlotChange::Dec), + (ValidatorIndex(1), SpamSlotChange::Dec), + ], + ); + assert_eq!(summary.slash_for, vec![ValidatorIndex(0), ValidatorIndex(2)]); + assert!(summary.slash_against.is_empty()); + assert_eq!(summary.new_participants, bitvec![BitOrderLsb0, u8; 0, 0, 1, 1, 1, 1, 1, 0]); + assert_eq!( + summary.new_flags, + DisputeStateFlags::CONFIRMED | DisputeStateFlags::AGAINST_SUPERMAJORITY, + ); + } + + #[test] + fn test_import_slash_against() { + let mut importer = DisputeStateImporter::new( + DisputeState { + validators_for: bitvec![BitOrderLsb0, u8; 1, 0, 1, 0, 0, 0, 0, 0], + validators_against: bitvec![BitOrderLsb0, u8; 0, 1, 0, 0, 0, 0, 0, 0], + start: 0, + concluded_at: None, + }, + 0, + ); + + assert_ok!(importer.import(ValidatorIndex(3), true)); + assert_ok!(importer.import(ValidatorIndex(4), true)); + assert_ok!(importer.import(ValidatorIndex(5), false)); + assert_ok!(importer.import(ValidatorIndex(6), true)); + assert_ok!(importer.import(ValidatorIndex(7), true)); + + let summary = importer.finish(); + assert_eq!( + summary.state, + DisputeState { + validators_for: bitvec![BitOrderLsb0, u8; 1, 0, 1, 1, 1, 0, 1, 1], + validators_against: bitvec![BitOrderLsb0, u8; 0, 1, 0, 0, 0, 1, 0, 0], + start: 0, + concluded_at: Some(0), + }, + ); + assert!(summary.spam_slot_changes.is_empty()); + assert!(summary.slash_for.is_empty()); + assert_eq!(summary.slash_against, vec![ValidatorIndex(1), ValidatorIndex(5)]); + assert_eq!(summary.new_participants, bitvec![BitOrderLsb0, u8; 0, 0, 0, 1, 1, 1, 1, 1]); + assert_eq!(summary.new_flags, DisputeStateFlags::FOR_SUPERMAJORITY); + } + + // Test that punish_inconclusive is correctly called. + #[test] + fn test_initializer_initialize() { + let dispute_conclusion_by_time_out_period = 3; + let start = 10; + + let mock_genesis_config = MockGenesisConfig { + configuration: crate::configuration::GenesisConfig { + config: HostConfiguration { + dispute_conclusion_by_time_out_period, + .. Default::default() + }, + .. Default::default() + }, + .. Default::default() + }; + + new_test_ext(mock_genesis_config).execute_with(|| { + let v0 = ::Pair::generate().0; + let v1 = ::Pair::generate().0; + let v2 = ::Pair::generate().0; + let v3 = ::Pair::generate().0; + + // NOTE: v0 index will be 0 + // NOTE: v1 index will be 3 + // NOTE: v2 index will be 2 + // NOTE: v3 index will be 1 + + run_to_block( + start, + |b| { + // a new session at each block + Some(( + true, + b, + vec![(&0, v0.public()), (&1, v1.public()), (&2, v2.public()), (&3, v3.public())], + Some(vec![(&0, v0.public()), (&1, v1.public()), (&2, v2.public()), (&3, v3.public())]), + )) + } + ); + + let candidate_hash = CandidateHash(sp_core::H256::repeat_byte(1)); + + // v0 votes for 3 + let stmts = vec![ + DisputeStatementSet { + candidate_hash: candidate_hash.clone(), + session: start - 1, + statements: vec![ + ( + DisputeStatement::Valid(ValidDisputeStatementKind::Explicit), + ValidatorIndex(0), + v0.sign( + &ExplicitDisputeStatement { + valid: true, + candidate_hash: candidate_hash.clone(), + session: start - 1, + }.signing_payload() + ) + ), + ], + }, + ]; + + assert_ok!( + Pallet::::provide_multi_dispute_data(stmts), + vec![(9, candidate_hash.clone())], + ); + assert_eq!(SpamSlots::::get(start - 1), Some(vec![1, 0, 0, 0])); + + // Run to timeout period + run_to_block(start + dispute_conclusion_by_time_out_period, |_| None); + assert_eq!(SpamSlots::::get(start - 1), Some(vec![1, 0, 0, 0])); + + // Run to timeout + 1 in order to executive on_finalize(timeout) + run_to_block(start + dispute_conclusion_by_time_out_period + 1, |_| None); + assert_eq!(SpamSlots::::get(start - 1), Some(vec![0, 0, 0, 0])); + assert_eq!( + PUNISH_VALIDATORS_INCONCLUSIVE.with(|r| r.borrow()[0].clone()), + (9, vec![ValidatorIndex(0)]), + ); + }); + } + + // Test prunning works + #[test] + fn test_initializer_on_new_session() { + let dispute_period = 3; + + let mock_genesis_config = MockGenesisConfig { + configuration: crate::configuration::GenesisConfig { + config: HostConfiguration { + dispute_period, + .. Default::default() + }, + .. Default::default() + }, + .. Default::default() + }; + + new_test_ext(mock_genesis_config).execute_with(|| { + let v0 = ::Pair::generate().0; + + let candidate_hash = CandidateHash(sp_core::H256::repeat_byte(1)); + Pallet::::note_included(0, candidate_hash.clone(), 0); + Pallet::::note_included(1, candidate_hash.clone(), 1); + Pallet::::note_included(2, candidate_hash.clone(), 2); + Pallet::::note_included(3, candidate_hash.clone(), 3); + Pallet::::note_included(4, candidate_hash.clone(), 4); + Pallet::::note_included(5, candidate_hash.clone(), 5); + Pallet::::note_included(6, candidate_hash.clone(), 5); + + run_to_block( + 7, + |b| { + // a new session at each block + Some(( + true, + b, + vec![(&0, v0.public())], + Some(vec![(&0, v0.public())]), + )) + } + ); + + // current session is 7, + // we keep for dispute_period + 1 session and we remove in on_finalize + // thus we keep info for session 3, 4, 5, 6, 7. + assert_eq!(Included::::iter_prefix(0).count(), 0); + assert_eq!(Included::::iter_prefix(1).count(), 0); + assert_eq!(Included::::iter_prefix(2).count(), 0); + assert_eq!(Included::::iter_prefix(3).count(), 1); + assert_eq!(Included::::iter_prefix(4).count(), 1); + assert_eq!(Included::::iter_prefix(5).count(), 1); + assert_eq!(Included::::iter_prefix(6).count(), 1); + }); + } + + #[test] + fn test_provide_multi_dispute_data_duplicate_error() { + new_test_ext(Default::default()).execute_with(|| { + let candidate_hash_1 = CandidateHash(sp_core::H256::repeat_byte(1)); + let candidate_hash_2 = CandidateHash(sp_core::H256::repeat_byte(2)); + + let stmts = vec![ + DisputeStatementSet { + candidate_hash: candidate_hash_2, + session: 2, + statements: vec![], + }, + DisputeStatementSet { + candidate_hash: candidate_hash_1, + session: 1, + statements: vec![], + }, + DisputeStatementSet { + candidate_hash: candidate_hash_2, + session: 2, + statements: vec![], + }, + ]; + + assert_err!( + Pallet::::provide_multi_dispute_data(stmts), + DispatchError::from(Error::::DuplicateDisputeStatementSets), + ); + }) + } + + #[test] + fn test_provide_multi_dispute_fail_halfway() { + new_test_ext(Default::default()).execute_with(|| { + let v0 = ::Pair::generate().0; + let v1 = ::Pair::generate().0; + + run_to_block( + 100, + |b| { + // a new session at each block + Some(( + true, + b, + vec![(&0, v0.public()), (&1, v1.public())], + Some(vec![(&0, v0.public()), (&1, v1.public())]), + )) + } + ); + + let candidate_hash = CandidateHash(sp_core::H256::repeat_byte(1)); + let stmts = vec![ + // // TODO TODO: this test is currently failing due to fail after writes. + // DisputeStatementSet { + // candidate_hash: candidate_hash.clone(), + // session: 98, + // statements: vec![ + // ( + // DisputeStatement::Valid(ValidDisputeStatementKind::Explicit), + // ValidatorIndex(1), + // v1.sign( + // &ExplicitDisputeStatement { + // valid: true, + // candidate_hash: candidate_hash.clone(), + // session: 98, + // }.signing_payload() + // ), + // ), + // ], + // }, + DisputeStatementSet { + candidate_hash: candidate_hash.clone(), + session: 1, + statements: vec![ + ( + DisputeStatement::Valid(ValidDisputeStatementKind::Explicit), + ValidatorIndex(0), + v0.sign( + &ExplicitDisputeStatement { + valid: true, + candidate_hash: candidate_hash.clone(), + session: 1, + }.signing_payload() + ), + ), + ], + }, + ]; + + assert_noop!( + Pallet::::provide_multi_dispute_data(stmts), + DispatchError::from(Error::::AncientDisputeStatement), + ); + }) + } + + // Test: + // * wrong signature fails + // * signature is checked for correct validator + #[test] + fn test_provide_multi_dispute_is_checking_signature_correctly() { + new_test_ext(Default::default()).execute_with(|| { + let v0 = ::Pair::generate().0; + let v1 = ::Pair::generate().0; + + run_to_block( + 3, + |b| { + // a new session at each block + if b == 1 { + Some(( + true, + b, + vec![(&0, v0.public())], + Some(vec![(&0, v0.public())]), + )) + } else { + Some(( + true, + b, + vec![(&1, v1.public())], + Some(vec![(&1, v1.public())]), + )) + } + } + ); + + + let candidate_hash = CandidateHash(sp_core::H256::repeat_byte(1)); + let stmts = vec![ + DisputeStatementSet { + candidate_hash: candidate_hash.clone(), + session: 1, + statements: vec![ + ( + DisputeStatement::Valid(ValidDisputeStatementKind::Explicit), + ValidatorIndex(0), + v0.sign( + &ExplicitDisputeStatement { + valid: true, + candidate_hash: candidate_hash.clone(), + session: 1, + }.signing_payload() + ), + ), + ], + }, + ]; + + assert_ok!( + Pallet::::provide_multi_dispute_data(stmts), + vec![(1, candidate_hash.clone())], + ); + + let candidate_hash = CandidateHash(sp_core::H256::repeat_byte(1)); + let stmts = vec![ + DisputeStatementSet { + candidate_hash: candidate_hash.clone(), + session: 2, + statements: vec![ + ( + DisputeStatement::Valid(ValidDisputeStatementKind::Explicit), + ValidatorIndex(0), + v0.sign( + &ExplicitDisputeStatement { + valid: true, + candidate_hash: candidate_hash.clone(), + session: 2, + }.signing_payload() + ), + ), + ], + }, + ]; + + assert_noop!( + Pallet::::provide_multi_dispute_data(stmts), + DispatchError::from(Error::::InvalidSignature), + ); + }) + } + + #[test] + fn test_freeze_on_note_included() { + new_test_ext(Default::default()).execute_with(|| { + let v0 = ::Pair::generate().0; + + run_to_block( + 6, + |b| { + // a new session at each block + Some(( + true, + b, + vec![(&0, v0.public())], + Some(vec![(&0, v0.public())]), + )) + } + ); + + let candidate_hash = CandidateHash(sp_core::H256::repeat_byte(1)); + + // v0 votes for 3 + let stmts = vec![ + DisputeStatementSet { + candidate_hash: candidate_hash.clone(), + session: 3, + statements: vec![ + ( + DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit), + ValidatorIndex(0), + v0.sign( + &ExplicitDisputeStatement { + valid: false, + candidate_hash: candidate_hash.clone(), + session: 3, + }.signing_payload() + ) + ), + ], + }, + ]; + assert!(Pallet::::provide_multi_dispute_data(stmts).is_ok()); + + Pallet::::note_included(3, candidate_hash.clone(), 3); + assert_eq!(Frozen::::get(), true); + }); + } + + #[test] + fn test_freeze_provided_against_supermajority_for_included() { + new_test_ext(Default::default()).execute_with(|| { + let v0 = ::Pair::generate().0; + + run_to_block( + 6, + |b| { + // a new session at each block + Some(( + true, + b, + vec![(&0, v0.public())], + Some(vec![(&0, v0.public())]), + )) + } + ); + + let candidate_hash = CandidateHash(sp_core::H256::repeat_byte(1)); + + Pallet::::note_included(3, candidate_hash.clone(), 3); + + // v0 votes for 3 + let stmts = vec![ + DisputeStatementSet { + candidate_hash: candidate_hash.clone(), + session: 3, + statements: vec![ + ( + DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit), + ValidatorIndex(0), + v0.sign( + &ExplicitDisputeStatement { + valid: false, + candidate_hash: candidate_hash.clone(), + session: 3, + }.signing_payload() + ) + ), + ], + }, + ]; + assert!(Pallet::::provide_multi_dispute_data(stmts).is_ok()); + + assert_eq!(Frozen::::get(), true); + }); + } + + // tests for: + // * provide_multi_dispute: with success scenario + // * disputes: correctness of datas + // * could_be_invalid: correctness of datas + // * note_included: decrement spam correctly + // * spam slots: correctly incremented and decremented + // * ensure rewards and punishment are correctly called. + #[test] + fn test_provide_multi_dispute_success_and_other() { + new_test_ext(Default::default()).execute_with(|| { + let v0 = ::Pair::generate().0; + let v1 = ::Pair::generate().0; + let v2 = ::Pair::generate().0; + let v3 = ::Pair::generate().0; + + // NOTE: v0 index will be 0 + // NOTE: v1 index will be 3 + // NOTE: v2 index will be 2 + // NOTE: v3 index will be 1 + + run_to_block( + 6, + |b| { + // a new session at each block + Some(( + true, + b, + vec![(&0, v0.public()), (&1, v1.public()), (&2, v2.public()), (&3, v3.public())], + Some(vec![(&0, v0.public()), (&1, v1.public()), (&2, v2.public()), (&3, v3.public())]), + )) + } + ); + + let candidate_hash = CandidateHash(sp_core::H256::repeat_byte(1)); + + // v0 votes for 3 + let stmts = vec![ + DisputeStatementSet { + candidate_hash: candidate_hash.clone(), + session: 3, + statements: vec![ + ( + DisputeStatement::Valid(ValidDisputeStatementKind::Explicit), + ValidatorIndex(0), + v0.sign( + &ExplicitDisputeStatement { + valid: true, + candidate_hash: candidate_hash.clone(), + session: 3, + }.signing_payload() + ) + ), + ], + }, + ]; + + assert_ok!( + Pallet::::provide_multi_dispute_data(stmts), + vec![(3, candidate_hash.clone())], + ); + assert_eq!(SpamSlots::::get(3), Some(vec![1, 0, 0, 0])); + + // v1 votes for 4 and for 3 + let stmts = vec![ + DisputeStatementSet { + candidate_hash: candidate_hash.clone(), + session: 4, + statements: vec![ + ( + DisputeStatement::Valid(ValidDisputeStatementKind::Explicit), + ValidatorIndex(3), + v1.sign( + &ExplicitDisputeStatement { + valid: true, + candidate_hash: candidate_hash.clone(), + session: 4, + }.signing_payload() + ) + ), + ], + }, + DisputeStatementSet { + candidate_hash: candidate_hash.clone(), + session: 3, + statements: vec![ + ( + DisputeStatement::Valid(ValidDisputeStatementKind::Explicit), + ValidatorIndex(3), + v1.sign( + &ExplicitDisputeStatement { + valid: true, + candidate_hash: candidate_hash.clone(), + session: 3, + }.signing_payload() + ), + ), + ], + }, + ]; + + assert_ok!( + Pallet::::provide_multi_dispute_data(stmts), + vec![(4, candidate_hash.clone())], + ); + assert_eq!(SpamSlots::::get(3), Some(vec![0, 0, 0, 0])); // Confirmed as no longer spam + assert_eq!(SpamSlots::::get(4), Some(vec![0, 0, 0, 1])); + + // v3 votes against 3 and for 5 + let stmts = vec![ + DisputeStatementSet { + candidate_hash: candidate_hash.clone(), + session: 3, + statements: vec![ + ( + DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit), + ValidatorIndex(1), + v3.sign( + &ExplicitDisputeStatement { + valid: false, + candidate_hash: candidate_hash.clone(), + session: 3, + }.signing_payload() + ), + ), + ], + }, + DisputeStatementSet { + candidate_hash: candidate_hash.clone(), + session: 5, + statements: vec![ + ( + DisputeStatement::Valid(ValidDisputeStatementKind::Explicit), + ValidatorIndex(1), + v3.sign( + &ExplicitDisputeStatement { + valid: true, + candidate_hash: candidate_hash.clone(), + session: 5, + }.signing_payload() + ), + ), + ], + }, + ]; + assert_ok!( + Pallet::::provide_multi_dispute_data(stmts), + vec![(5, candidate_hash.clone())], + ); + assert_eq!(SpamSlots::::get(3), Some(vec![0, 0, 0, 0])); + assert_eq!(SpamSlots::::get(4), Some(vec![0, 0, 0, 1])); + assert_eq!(SpamSlots::::get(5), Some(vec![0, 1, 0, 0])); + + // v2 votes for 3 and againt 5 + let stmts = vec![ + DisputeStatementSet { + candidate_hash: candidate_hash.clone(), + session: 3, + statements: vec![ + ( + DisputeStatement::Valid(ValidDisputeStatementKind::Explicit), + ValidatorIndex(2), + v2.sign( + &ExplicitDisputeStatement { + valid: true, + candidate_hash: candidate_hash.clone(), + session: 3, + }.signing_payload() + ) + ), + ], + }, + DisputeStatementSet { + candidate_hash: candidate_hash.clone(), + session: 5, + statements: vec![ + ( + DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit), + ValidatorIndex(2), + v2.sign( + &ExplicitDisputeStatement { + valid: false, + candidate_hash: candidate_hash.clone(), + session: 5, + }.signing_payload() + ), + ), + ], + }, + ]; + assert_ok!(Pallet::::provide_multi_dispute_data(stmts), vec![]); + assert_eq!(SpamSlots::::get(3), Some(vec![0, 0, 0, 0])); + assert_eq!(SpamSlots::::get(4), Some(vec![0, 0, 0, 1])); + assert_eq!(SpamSlots::::get(5), Some(vec![0, 0, 0, 0])); + + // v0 votes for 5 + let stmts = vec![ + DisputeStatementSet { + candidate_hash: candidate_hash.clone(), + session: 5, + statements: vec![ + ( + DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit), + ValidatorIndex(0), + v0.sign( + &ExplicitDisputeStatement { + valid: false, + candidate_hash: candidate_hash.clone(), + session: 5, + }.signing_payload() + ), + ), + ], + }, + ]; + + assert_ok!(Pallet::::provide_multi_dispute_data(stmts), vec![]); + assert_eq!(SpamSlots::::get(3), Some(vec![0, 0, 0, 0])); + assert_eq!(SpamSlots::::get(4), Some(vec![0, 0, 0, 1])); + assert_eq!(SpamSlots::::get(5), Some(vec![0, 0, 0, 0])); + + // v1 votes for 5 + let stmts = vec![ + DisputeStatementSet { + candidate_hash: candidate_hash.clone(), + session: 5, + statements: vec![ + ( + DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit), + ValidatorIndex(3), + v1.sign( + &ExplicitDisputeStatement { + valid: false, + candidate_hash: candidate_hash.clone(), + session: 5, + }.signing_payload() + ) + ), + ], + }, + ]; + + assert_ok!( + Pallet::::provide_multi_dispute_data(stmts), + vec![], + ); + assert_eq!(SpamSlots::::get(3), Some(vec![0, 0, 0, 0])); + assert_eq!(SpamSlots::::get(4), Some(vec![0, 0, 0, 1])); + assert_eq!(SpamSlots::::get(5), Some(vec![0, 0, 0, 0])); + + assert_eq!( + Pallet::::disputes(), + vec![ + ( + 5, + candidate_hash.clone(), + DisputeState { + validators_for: bitvec![BitOrderLsb0, u8; 0, 1, 0, 0], + validators_against: bitvec![BitOrderLsb0, u8; 1, 0, 1, 1], + start: 6, + concluded_at: Some(6), // 3 vote against + } + ), + ( + 3, + candidate_hash.clone(), + DisputeState { + validators_for: bitvec![BitOrderLsb0, u8; 1, 0, 1, 1], + validators_against: bitvec![BitOrderLsb0, u8; 0, 1, 0, 0], + start: 6, + concluded_at: Some(6), // 3 vote for + } + ), + ( + 4, + candidate_hash.clone(), + DisputeState { + validators_for: bitvec![BitOrderLsb0, u8; 0, 0, 0, 1], + validators_against: bitvec![BitOrderLsb0, u8; 0, 0, 0, 0], + start: 6, + concluded_at: None, + } + ), + ] + ); + + assert_eq!(Pallet::::could_be_invalid(3, candidate_hash.clone()), false); // It has 3 votes for + assert_eq!(Pallet::::could_be_invalid(4, candidate_hash.clone()), true); + assert_eq!(Pallet::::could_be_invalid(5, candidate_hash.clone()), true); + + // Ensure inclusion removes spam slots + assert_eq!(SpamSlots::::get(4), Some(vec![0, 0, 0, 1])); + Pallet::::note_included(4, candidate_hash.clone(), 4); + assert_eq!(SpamSlots::::get(4), Some(vec![0, 0, 0, 0])); + + // Ensure the reward_validator function was correctly called + assert_eq!( + REWARD_VALIDATORS.with(|r| r.borrow().clone()), + vec![ + (3, vec![ValidatorIndex(0)]), + (4, vec![ValidatorIndex(3)]), + (3, vec![ValidatorIndex(3)]), + (3, vec![ValidatorIndex(1)]), + (5, vec![ValidatorIndex(1)]), + (3, vec![ValidatorIndex(2)]), + (5, vec![ValidatorIndex(2)]), + (5, vec![ValidatorIndex(0)]), + (5, vec![ValidatorIndex(3)]), + ], + ); + + // Ensure punishment against is called + assert_eq!( + PUNISH_VALIDATORS_AGAINST.with(|r| r.borrow().clone()), + vec![ + (3, vec![]), + (4, vec![]), + (3, vec![]), + (3, vec![]), + (5, vec![]), + (3, vec![ValidatorIndex(1)]), + (5, vec![]), + (5, vec![]), + (5, vec![]), + ], + ); + + // Ensure punishment for is called + assert_eq!( + PUNISH_VALIDATORS_FOR.with(|r| r.borrow().clone()), + vec![ + (3, vec![]), + (4, vec![]), + (3, vec![]), + (3, vec![]), + (5, vec![]), + (3, vec![]), + (5, vec![]), + (5, vec![]), + (5, vec![ValidatorIndex(1)]), + ], + ); + }) + } + + #[test] + fn test_revert_and_freeze() { + new_test_ext(Default::default()).execute_with(|| { + Frozen::::put(true); + assert_noop!( + { + Pallet::::revert_and_freeze(0); + Result::<(), ()>::Err(()) // Just a small trick in order to use assert_noop. + }, + (), + ); + + Frozen::::kill(); + Pallet::::revert_and_freeze(0); + assert_eq!(Frozen::::get(), true); + }) + } + + #[test] + fn test_has_supermajority_against() { + assert_eq!( + has_supermajority_against(&DisputeState { + validators_for: bitvec![BitOrderLsb0, u8; 1, 1, 0, 0, 0, 0, 0, 0], + validators_against: bitvec![BitOrderLsb0, u8; 1, 1, 1, 1, 1, 0, 0, 0], + start: 0, + concluded_at: None, + }), + false, + ); + + assert_eq!( + has_supermajority_against(&DisputeState { + validators_for: bitvec![BitOrderLsb0, u8; 1, 1, 0, 0, 0, 0, 0, 0], + validators_against: bitvec![BitOrderLsb0, u8; 1, 1, 1, 1, 1, 1, 0, 0], + start: 0, + concluded_at: None, + }), + true, + ); + } + + #[test] + fn test_decrement_spam() { + let original_spam_slots = vec![0, 1, 2, 3, 4, 5, 6, 7]; + + // Test confirm is no-op + let mut spam_slots = original_spam_slots.clone(); + let dispute_state_confirm = DisputeState { + validators_for: bitvec![BitOrderLsb0, u8; 1, 1, 0, 0, 0, 0, 0, 0], + validators_against: bitvec![BitOrderLsb0, u8; 1, 0, 1, 0, 0, 0, 0, 0], + start: 0, + concluded_at: None, + }; + assert_eq!( + DisputeStateFlags::from_state(&dispute_state_confirm), + DisputeStateFlags::CONFIRMED + ); + assert_eq!( + decrement_spam(spam_slots.as_mut(), &dispute_state_confirm), + bitvec![BitOrderLsb0, u8; 1, 1, 1, 0, 0, 0, 0, 0], + ); + assert_eq!(spam_slots, original_spam_slots); + + // Test not confirm is decreasing spam + let mut spam_slots = original_spam_slots.clone(); + let dispute_state_no_confirm = DisputeState { + validators_for: bitvec![BitOrderLsb0, u8; 1, 0, 0, 0, 0, 0, 0, 0], + validators_against: bitvec![BitOrderLsb0, u8; 1, 0, 1, 0, 0, 0, 0, 0], + start: 0, + concluded_at: None, + }; + assert_eq!( + DisputeStateFlags::from_state(&dispute_state_no_confirm), + DisputeStateFlags::default() + ); + assert_eq!( + decrement_spam(spam_slots.as_mut(), &dispute_state_no_confirm), + bitvec![BitOrderLsb0, u8; 1, 0, 1, 0, 0, 0, 0, 0], + ); + assert_eq!(spam_slots, vec![0, 1, 1, 3, 4, 5, 6, 7]); + } + + #[test] + fn test_check_signature() { + let validator_id = ::Pair::generate().0; + let wrong_validator_id = ::Pair::generate().0; + + let session = 0; + let wrong_session = 1; + let candidate_hash = CandidateHash(sp_core::H256::repeat_byte(1)); + let wrong_candidate_hash = CandidateHash(sp_core::H256::repeat_byte(2)); + let inclusion_parent = sp_core::H256::repeat_byte(3); + let wrong_inclusion_parent = sp_core::H256::repeat_byte(4); + + let statement_1 = DisputeStatement::Valid(ValidDisputeStatementKind::Explicit); + let statement_2 = DisputeStatement::Valid( + ValidDisputeStatementKind::BackingSeconded(inclusion_parent.clone()) + ); + let wrong_statement_2 = DisputeStatement::Valid( + ValidDisputeStatementKind::BackingSeconded(wrong_inclusion_parent.clone()) + ); + let statement_3 = DisputeStatement::Valid( + ValidDisputeStatementKind::BackingValid(inclusion_parent.clone()) + ); + let wrong_statement_3 = DisputeStatement::Valid( + ValidDisputeStatementKind::BackingValid(wrong_inclusion_parent.clone()) + ); + let statement_4 = DisputeStatement::Valid(ValidDisputeStatementKind::ApprovalChecking); + let statement_5 = DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit); + + let signed_1 = validator_id.sign( + &ExplicitDisputeStatement { + valid: true, + candidate_hash: candidate_hash.clone(), + session, + }.signing_payload() + ); + let signed_2 = validator_id.sign( + &CompactStatement::Seconded(candidate_hash.clone()) + .signing_payload(&SigningContext { + session_index: session, + parent_hash: inclusion_parent.clone() + }) + ); + let signed_3 = validator_id.sign( + &CompactStatement::Valid(candidate_hash.clone()) + .signing_payload(&SigningContext { + session_index: session, + parent_hash: inclusion_parent.clone() + }) + ); + let signed_4 = validator_id.sign( + &ApprovalVote(candidate_hash.clone()).signing_payload(session) + ); + let signed_5 = validator_id.sign( + &ExplicitDisputeStatement { + valid: false, + candidate_hash: candidate_hash.clone(), + session, + }.signing_payload() + ); + + assert!(check_signature(&validator_id.public(), candidate_hash, session, &statement_1, &signed_1).is_ok()); + assert!(check_signature(&wrong_validator_id.public(), candidate_hash, session, &statement_1, &signed_1).is_err()); + assert!(check_signature(&validator_id.public(), wrong_candidate_hash, session, &statement_1, &signed_1).is_err()); + assert!(check_signature(&validator_id.public(), candidate_hash, wrong_session, &statement_1, &signed_1).is_err()); + assert!(check_signature(&validator_id.public(), candidate_hash, session, &statement_2, &signed_1).is_err()); + assert!(check_signature(&validator_id.public(), candidate_hash, session, &statement_3, &signed_1).is_err()); + assert!(check_signature(&validator_id.public(), candidate_hash, session, &statement_4, &signed_1).is_err()); + assert!(check_signature(&validator_id.public(), candidate_hash, session, &statement_5, &signed_1).is_err()); + + assert!(check_signature(&validator_id.public(), candidate_hash, session, &statement_2, &signed_2).is_ok()); + assert!(check_signature(&wrong_validator_id.public(), candidate_hash, session, &statement_2, &signed_2).is_err()); + assert!(check_signature(&validator_id.public(), wrong_candidate_hash, session, &statement_2, &signed_2).is_err()); + assert!(check_signature(&validator_id.public(), candidate_hash, wrong_session, &statement_2, &signed_2).is_err()); + assert!(check_signature(&validator_id.public(), candidate_hash, session, &wrong_statement_2, &signed_2).is_err()); + assert!(check_signature(&validator_id.public(), candidate_hash, session, &statement_1, &signed_2).is_err()); + assert!(check_signature(&validator_id.public(), candidate_hash, session, &statement_3, &signed_2).is_err()); + assert!(check_signature(&validator_id.public(), candidate_hash, session, &statement_4, &signed_2).is_err()); + assert!(check_signature(&validator_id.public(), candidate_hash, session, &statement_5, &signed_2).is_err()); + + assert!(check_signature(&validator_id.public(), candidate_hash, session, &statement_3, &signed_3).is_ok()); + assert!(check_signature(&wrong_validator_id.public(), candidate_hash, session, &statement_3, &signed_3).is_err()); + assert!(check_signature(&validator_id.public(), wrong_candidate_hash, session, &statement_3, &signed_3).is_err()); + assert!(check_signature(&validator_id.public(), candidate_hash, wrong_session, &statement_3, &signed_3).is_err()); + assert!(check_signature(&validator_id.public(), candidate_hash, session, &wrong_statement_3, &signed_3).is_err()); + assert!(check_signature(&validator_id.public(), candidate_hash, session, &statement_1, &signed_3).is_err()); + assert!(check_signature(&validator_id.public(), candidate_hash, session, &statement_2, &signed_3).is_err()); + assert!(check_signature(&validator_id.public(), candidate_hash, session, &statement_4, &signed_3).is_err()); + assert!(check_signature(&validator_id.public(), candidate_hash, session, &statement_5, &signed_3).is_err()); + + assert!(check_signature(&validator_id.public(), candidate_hash, session, &statement_4, &signed_4).is_ok()); + assert!(check_signature(&wrong_validator_id.public(), candidate_hash, session, &statement_4, &signed_4).is_err()); + assert!(check_signature(&validator_id.public(), wrong_candidate_hash, session, &statement_4, &signed_4).is_err()); + assert!(check_signature(&validator_id.public(), candidate_hash, wrong_session, &statement_4, &signed_4).is_err()); + assert!(check_signature(&validator_id.public(), candidate_hash, session, &statement_1, &signed_4).is_err()); + assert!(check_signature(&validator_id.public(), candidate_hash, session, &statement_2, &signed_4).is_err()); + assert!(check_signature(&validator_id.public(), candidate_hash, session, &statement_3, &signed_4).is_err()); + assert!(check_signature(&validator_id.public(), candidate_hash, session, &statement_5, &signed_4).is_err()); + + assert!(check_signature(&validator_id.public(), candidate_hash, session, &statement_5, &signed_5).is_ok()); + assert!(check_signature(&wrong_validator_id.public(), candidate_hash, session, &statement_5, &signed_5).is_err()); + assert!(check_signature(&validator_id.public(), wrong_candidate_hash, session, &statement_5, &signed_5).is_err()); + assert!(check_signature(&validator_id.public(), candidate_hash, wrong_session, &statement_5, &signed_5).is_err()); + assert!(check_signature(&validator_id.public(), candidate_hash, session, &statement_1, &signed_5).is_err()); + assert!(check_signature(&validator_id.public(), candidate_hash, session, &statement_2, &signed_5).is_err()); + assert!(check_signature(&validator_id.public(), candidate_hash, session, &statement_3, &signed_5).is_err()); + assert!(check_signature(&validator_id.public(), candidate_hash, session, &statement_4, &signed_5).is_err()); + } +} diff --git a/runtime/parachains/src/initializer.rs b/runtime/parachains/src/initializer.rs index a57fa64d5551..f7bfe8abfcc8 100644 --- a/runtime/parachains/src/initializer.rs +++ b/runtime/parachains/src/initializer.rs @@ -136,7 +136,7 @@ decl_module! { scheduler::Module::::initializer_initialize(now) + inclusion::Module::::initializer_initialize(now) + session_info::Module::::initializer_initialize(now) + - disputes::Module::::initializer_initialize(now) + + disputes::Pallet::::initializer_initialize(now) + dmp::Module::::initializer_initialize(now) + ump::Module::::initializer_initialize(now) + hrmp::Module::::initializer_initialize(now); @@ -151,7 +151,7 @@ decl_module! { hrmp::Module::::initializer_finalize(); ump::Module::::initializer_finalize(); dmp::Module::::initializer_finalize(); - disputes::Module::::initializer_finalize(); + disputes::Pallet::::initializer_finalize(); session_info::Module::::initializer_finalize(); inclusion::Module::::initializer_finalize(); scheduler::Module::::initializer_finalize(); @@ -231,7 +231,7 @@ impl Module { scheduler::Module::::initializer_on_new_session(¬ification); inclusion::Module::::initializer_on_new_session(¬ification); session_info::Module::::initializer_on_new_session(¬ification); - disputes::Module::::initializer_on_new_session(¬ification); + disputes::Pallet::::initializer_on_new_session(¬ification); dmp::Module::::initializer_on_new_session(¬ification, &outgoing_paras); ump::Module::::initializer_on_new_session(¬ification, &outgoing_paras); hrmp::Module::::initializer_on_new_session(¬ification, &outgoing_paras); @@ -266,6 +266,20 @@ impl Module { } } + + // Allow to trigger on_new_session in tests, this is needed as long as pallet_session is not + // implemented in mock. + #[cfg(test)] + pub(crate) fn test_trigger_on_new_session<'a, I: 'a>( + changed: bool, + session_index: SessionIndex, + validators: I, + queued: Option, + ) + where I: Iterator + { + Self::on_new_session(changed, session_index, validators, queued) + } } impl sp_runtime::BoundToRuntimeAppPublic for Module { diff --git a/runtime/parachains/src/mock.rs b/runtime/parachains/src/mock.rs index 270e93734867..84bd45685d06 100644 --- a/runtime/parachains/src/mock.rs +++ b/runtime/parachains/src/mock.rs @@ -21,14 +21,16 @@ use sp_core::H256; use sp_runtime::traits::{ BlakeTwo256, IdentityLookup, }; -use primitives::v1::{AuthorityDiscoveryId, Balance, BlockNumber, Header, ValidatorIndex}; +use primitives::v1::{ + AuthorityDiscoveryId, Balance, BlockNumber, Header, ValidatorIndex, SessionIndex, +}; use frame_support::parameter_types; use frame_support_test::TestRandomness; use std::cell::RefCell; use std::collections::HashMap; use crate::{ inclusion, scheduler, dmp, ump, hrmp, session_info, paras, configuration, - initializer, shared, + initializer, shared, disputes, }; type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; @@ -52,6 +54,7 @@ frame_support::construct_runtime!( Ump: ump::{Pallet, Call, Storage}, Hrmp: hrmp::{Pallet, Call, Storage, Event}, SessionInfo: session_info::{Pallet, Call, Storage}, + Disputes: disputes::{Pallet, Call, Storage, Event}, } ); @@ -61,6 +64,8 @@ parameter_types! { frame_system::limits::BlockWeights::simple_max(4 * 1024 * 1024); } +pub type AccountId = u64; + impl frame_system::Config for Test { type BaseCallFilter = (); type BlockWeights = BlockWeights; @@ -132,6 +137,54 @@ impl crate::hrmp::Config for Test { type Currency = pallet_balances::Pallet; } +impl crate::disputes::Config for Test { + type Event = Event; + type RewardValidators = Self; + type PunishValidators = Self; +} + +thread_local! { + pub static REWARD_VALIDATORS: RefCell)>> = RefCell::new(Vec::new()); + pub static PUNISH_VALIDATORS_FOR: RefCell)>> = RefCell::new(Vec::new()); + pub static PUNISH_VALIDATORS_AGAINST: RefCell)>> = RefCell::new(Vec::new()); + pub static PUNISH_VALIDATORS_INCONCLUSIVE: RefCell)>> = RefCell::new(Vec::new()); +} + +impl crate::disputes::RewardValidators for Test { + fn reward_dispute_statement( + session: SessionIndex, + validators: impl IntoIterator + ) { + REWARD_VALIDATORS.with(|r| r.borrow_mut().push((session, validators.into_iter().collect()))) + } +} + +impl crate::disputes::PunishValidators for Test { + fn punish_for_invalid( + session: SessionIndex, + validators: impl IntoIterator, + ) { + PUNISH_VALIDATORS_FOR + .with(|r| r.borrow_mut().push((session, validators.into_iter().collect()))) + } + + fn punish_against_valid( + session: SessionIndex, + validators: impl IntoIterator, + ) { + PUNISH_VALIDATORS_AGAINST + .with(|r| r.borrow_mut().push((session, validators.into_iter().collect()))) + } + + fn punish_inconclusive( + session: SessionIndex, + validators: impl IntoIterator, + ) { + PUNISH_VALIDATORS_INCONCLUSIVE + .with(|r| r.borrow_mut().push((session, validators.into_iter().collect()))) + } +} + impl crate::scheduler::Config for Test { } impl crate::inclusion::Config for Test { diff --git a/runtime/parachains/src/paras_inherent.rs b/runtime/parachains/src/paras_inherent.rs index 00d02bc280b4..60c5ced22b37 100644 --- a/runtime/parachains/src/paras_inherent.rs +++ b/runtime/parachains/src/paras_inherent.rs @@ -119,8 +119,8 @@ decl_module! { // Handle disputes logic. let current_session = >::session_index(); let freed_disputed: Vec<(_, FreedReason)> = { - let fresh_disputes = >::provide_multi_dispute_data(disputes)?; - if >::is_frozen() { + let fresh_disputes = >::provide_multi_dispute_data(disputes)?; + if >::is_frozen() { // The relay chain we are currently on is invalid. Proceed no further on parachains. Included::set(Some(())); return Ok(Some( @@ -158,7 +158,7 @@ decl_module! { // Inform the disputes module of all included candidates. let now = >::block_number(); for (_, candidate_hash) in &freed_concluded { - >::note_included(current_session, *candidate_hash, now); + >::note_included(current_session, *candidate_hash, now); } // Handle timeouts for any availability core work. @@ -189,7 +189,7 @@ decl_module! { // Refuse to back any candidates that are disputed or invalid. for candidate in &backed_candidates { ensure!( - !>::could_be_invalid( + !>::could_be_invalid( current_session, candidate.candidate.hash(), ), From 7570236316b22b2d39ee59676b307c2ed0f873a0 Mon Sep 17 00:00:00 2001 From: thiolliere Date: Fri, 28 May 2021 20:00:23 +0200 Subject: [PATCH 36/67] document specific behavior --- runtime/parachains/src/disputes.rs | 71 +++--------------------------- 1 file changed, 6 insertions(+), 65 deletions(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index af582623a042..84d9b0188250 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -478,6 +478,12 @@ impl Pallet { /// Handle sets of dispute statements corresponding to 0 or more candidates. /// Returns a vector of freshly created disputes. + /// + /// # Warning + /// + /// This functions modifies the state when failing. It is expected to be called in inherent, + /// and to fail the extrinsic on error. As invalid inherents are not allowed, the dirty state + /// is not commited. pub(crate) fn provide_multi_dispute_data(statement_sets: MultiDisputeStatementSet) -> Result, DispatchError> { @@ -1195,71 +1201,6 @@ mod tests { }) } - #[test] - fn test_provide_multi_dispute_fail_halfway() { - new_test_ext(Default::default()).execute_with(|| { - let v0 = ::Pair::generate().0; - let v1 = ::Pair::generate().0; - - run_to_block( - 100, - |b| { - // a new session at each block - Some(( - true, - b, - vec![(&0, v0.public()), (&1, v1.public())], - Some(vec![(&0, v0.public()), (&1, v1.public())]), - )) - } - ); - - let candidate_hash = CandidateHash(sp_core::H256::repeat_byte(1)); - let stmts = vec![ - // // TODO TODO: this test is currently failing due to fail after writes. - // DisputeStatementSet { - // candidate_hash: candidate_hash.clone(), - // session: 98, - // statements: vec![ - // ( - // DisputeStatement::Valid(ValidDisputeStatementKind::Explicit), - // ValidatorIndex(1), - // v1.sign( - // &ExplicitDisputeStatement { - // valid: true, - // candidate_hash: candidate_hash.clone(), - // session: 98, - // }.signing_payload() - // ), - // ), - // ], - // }, - DisputeStatementSet { - candidate_hash: candidate_hash.clone(), - session: 1, - statements: vec![ - ( - DisputeStatement::Valid(ValidDisputeStatementKind::Explicit), - ValidatorIndex(0), - v0.sign( - &ExplicitDisputeStatement { - valid: true, - candidate_hash: candidate_hash.clone(), - session: 1, - }.signing_payload() - ), - ), - ], - }, - ]; - - assert_noop!( - Pallet::::provide_multi_dispute_data(stmts), - DispatchError::from(Error::::AncientDisputeStatement), - ); - }) - } - // Test: // * wrong signature fails // * signature is checked for correct validator From 6dd0ad67fdf88ad065abb43fb9861d3b7b075cb2 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 31 May 2021 14:39:19 -0500 Subject: [PATCH 37/67] deposit events on dispute changes --- runtime/parachains/src/disputes.rs | 44 +++++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 84d9b0188250..d046409476f5 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -18,7 +18,7 @@ use sp_std::prelude::*; use primitives::v1::{ - Id as ParaId, SessionIndex, CandidateHash, + SessionIndex, CandidateHash, DisputeState, DisputeStatementSet, MultiDisputeStatementSet, ValidatorId, ValidatorSignature, DisputeStatement, ValidDisputeStatementKind, InvalidDisputeStatementKind, ExplicitDisputeStatement, CompactStatement, SigningContext, ApprovalVote, ValidatorIndex, @@ -137,16 +137,16 @@ pub mod pallet { pub(super) type Frozen = StorageValue<_, bool, ValueQuery>; #[pallet::event] - #[pallet::generate_deposit(fn deposit_event)] + #[pallet::generate_deposit(pub fn deposit_event)] pub enum Event { - /// A dispute has been initiated. \[para_id, candidate hash, dispute location\] - DisputeInitiated(ParaId, CandidateHash, DisputeLocation), + /// A dispute has been initiated. \[candidate hash, dispute location\] + DisputeInitiated(CandidateHash, DisputeLocation), /// A dispute has concluded for or against a candidate. /// \[para_id, candidate hash, dispute result\] - DisputeConcluded(ParaId, CandidateHash, DisputeResult), + DisputeConcluded(CandidateHash, DisputeResult), /// A dispute has timed out due to insufficient participation. /// \[para_id, candidate hash\] - DisputeTimedOut(ParaId, CandidateHash), + DisputeTimedOut(CandidateHash), } #[pallet::hooks] @@ -404,6 +404,8 @@ impl Pallet { if dispute.concluded_at.is_none() && dispute.start + config.dispute_conclusion_by_time_out_period < now { + Self::deposit_event(Event::DisputeTimedOut(candidate_hash)); + dispute.concluded_at = Some(now); >::insert(session_index, candidate_hash, &dispute); @@ -584,7 +586,8 @@ impl Pallet { }; // Apply spam slot changes. Bail early if too many occupied. - if !>::contains_key(&set.session, &set.candidate_hash) { + let is_local = >::contains_key(&set.session, &set.candidate_hash); + if !is_local { let mut spam_slots: Vec = SpamSlots::::get(&set.session) .unwrap_or_else(|| vec![0; n_validators]); @@ -610,6 +613,33 @@ impl Pallet { SpamSlots::::insert(&set.session, spam_slots); } + if fresh { + Self::deposit_event(Event::DisputeInitiated( + set.candidate_hash, + if is_local { DisputeLocation::Local } else { DisputeLocation::Remote }, + )); + } + + { + if summary.new_flags.contains(DisputeStateFlags::FOR_SUPERMAJORITY) { + Self::deposit_event(Event::DisputeConcluded( + set.candidate_hash, + DisputeResult::Valid, + )); + } + + // It is possible, although unexpected, for a dispute to conclude twice. + // This would require f+1 validators to vote in both directions. + // A dispute cannot conclude more than once in each direction. + + if summary.new_flags.contains(DisputeStateFlags::AGAINST_SUPERMAJORITY) { + Self::deposit_event(Event::DisputeConcluded( + set.candidate_hash, + DisputeResult::Invalid, + )); + } + } + // Reward statements. T::RewardValidators::reward_dispute_statement( set.session, From 040cc17764ccfa08d8a06356427b3e027ea74867 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 31 May 2021 14:39:47 -0500 Subject: [PATCH 38/67] add an allow(unused) on fn disputes --- runtime/parachains/src/disputes.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index d046409476f5..dd65db0346c5 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -673,6 +673,7 @@ impl Pallet { Ok(fresh) } + #[allow(unused)] pub(crate) fn disputes() -> Vec<(SessionIndex, CandidateHash, DisputeState)> { >::iter().collect() } From 4a3eeba32d96a731aad84174963905ad9725d203 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 31 May 2021 14:47:22 -0500 Subject: [PATCH 39/67] add a dummy PunishValidators implementation --- runtime/parachains/src/disputes.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index dd65db0346c5..1858a23f4296 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -76,6 +76,20 @@ pub trait PunishValidators { fn punish_inconclusive(session: SessionIndex, validators: impl IntoIterator); } +impl PunishValidators for () { + fn punish_for_invalid(_: SessionIndex, _: impl IntoIterator) { + + } + + fn punish_against_valid(_: SessionIndex, _: impl IntoIterator) { + + } + + fn punish_inconclusive(_: SessionIndex, _: impl IntoIterator) { + + } +} + pub use pallet::*; #[frame_support::pallet] pub mod pallet { From 6f5b27f8658c933fc3b03d26b9554e690d654a0d Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 31 May 2021 14:54:24 -0500 Subject: [PATCH 40/67] add disputes module to Rococo --- runtime/rococo/src/lib.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/runtime/rococo/src/lib.rs b/runtime/rococo/src/lib.rs index da93eb3394dc..b2e7bba53c36 100644 --- a/runtime/rococo/src/lib.rs +++ b/runtime/rococo/src/lib.rs @@ -76,6 +76,7 @@ use runtime_parachains::inclusion as parachains_inclusion; use runtime_parachains::paras_inherent as parachains_paras_inherent; use runtime_parachains::initializer as parachains_initializer; use runtime_parachains::session_info as parachains_session_info; +use runtime_parachains::disputes as parachains_disputes; use runtime_parachains::paras as parachains_paras; use runtime_parachains::dmp as parachains_dmp; use runtime_parachains::ump as parachains_ump; @@ -220,6 +221,7 @@ construct_runtime! { Ump: parachains_ump::{Pallet, Call, Storage}, Hrmp: parachains_hrmp::{Pallet, Call, Storage, Event}, SessionInfo: parachains_session_info::{Pallet, Call, Storage}, + Disputes: parachains_disputes::{Pallet, Call, Storage, Event}, // Parachain Onboarding Pallets Registrar: paras_registrar::{Pallet, Call, Storage, Event}, @@ -736,6 +738,12 @@ impl parachains_hrmp::Config for Runtime { type Currency = Balances; } +impl parachains_disputes::Config for Runtime { + type Event = Event; + type RewardValidators = (); + type PunishValidators = (); +} + impl parachains_paras_inherent::Config for Runtime {} impl parachains_scheduler::Config for Runtime {} From aacc3de14a7c6e55b725862d083554577e417c18 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 31 May 2021 14:59:18 -0500 Subject: [PATCH 41/67] add disputes module to westend runtime --- runtime/westend/src/lib.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/runtime/westend/src/lib.rs b/runtime/westend/src/lib.rs index 23bd2e0cb474..a37706f7bcaa 100644 --- a/runtime/westend/src/lib.rs +++ b/runtime/westend/src/lib.rs @@ -51,6 +51,7 @@ use runtime_parachains::ump as parachains_ump; use runtime_parachains::hrmp as parachains_hrmp; use runtime_parachains::scheduler as parachains_scheduler; use runtime_parachains::reward_points as parachains_reward_points; +use runtime_parachains::disputes as parachains_disputes; use runtime_parachains::runtime_api_impl::v1 as parachains_runtime_api_impl; use xcm::v0::{MultiLocation, NetworkId}; @@ -750,6 +751,12 @@ impl parachains_hrmp::Config for Runtime { type Currency = Balances; } +impl parachains_disputes::Config for Runtime { + type Event = Event; + type RewardValidators = (); + type PunishValidators = (); +} + impl parachains_paras_inherent::Config for Runtime {} impl parachains_scheduler::Config for Runtime {} @@ -926,6 +933,7 @@ construct_runtime! { ParasUmp: parachains_ump::{Pallet, Call, Storage} = 50, ParasHrmp: parachains_hrmp::{Pallet, Call, Storage, Event} = 51, ParasSessionInfo: parachains_session_info::{Pallet, Call, Storage} = 52, + ParasDisputes: parachains_disputes::{Pallet, Call, Storage, Event} = 53, // Parachain Onboarding Pallets. Start indices at 60 to leave room. Registrar: paras_registrar::{Pallet, Call, Storage, Event} = 60, From 993709b95830749507e31ff873e57c2c5743c582 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 31 May 2021 15:01:30 -0500 Subject: [PATCH 42/67] add disputes module to test runtime --- runtime/test-runtime/src/lib.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/runtime/test-runtime/src/lib.rs b/runtime/test-runtime/src/lib.rs index 230c2f2e5f88..fb25ab794128 100644 --- a/runtime/test-runtime/src/lib.rs +++ b/runtime/test-runtime/src/lib.rs @@ -36,6 +36,7 @@ use polkadot_runtime_parachains::dmp as parachains_dmp; use polkadot_runtime_parachains::ump as parachains_ump; use polkadot_runtime_parachains::hrmp as parachains_hrmp; use polkadot_runtime_parachains::scheduler as parachains_scheduler; +use polkadot_runtime_parachains::disputes as parachains_disputes; use polkadot_runtime_parachains::runtime_api_impl::v1 as runtime_impl; use primitives::v1::{ @@ -453,6 +454,12 @@ impl parachains_inclusion::Config for Runtime { type RewardValidators = RewardValidatorsWithEraPoints; } +impl parachains_disputes::Config for Runtime { + type Event = Event; + type RewardValidators = (); + type PunishValidators = (); +} + impl parachains_paras_inherent::Config for Runtime {} impl parachains_initializer::Config for Runtime { @@ -531,6 +538,7 @@ construct_runtime! { ParasSudoWrapper: paras_sudo_wrapper::{Pallet, Call}, SessionInfo: parachains_session_info::{Pallet, Call, Storage}, Hrmp: parachains_hrmp::{Pallet, Call, Storage, Event}, + ParasDisputes: parachains_disputes::{Pallet, Call, Storage, Event}, Sudo: pallet_sudo::{Pallet, Call, Storage, Config, Event}, } From 5b7928a5a79b3a06db3f71a0139d02a38331f8ad Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 31 May 2021 15:02:29 -0500 Subject: [PATCH 43/67] add disputes module to kusama runtime --- runtime/kusama/src/lib.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/runtime/kusama/src/lib.rs b/runtime/kusama/src/lib.rs index 18108fcd94e9..1a82b9f8c6bf 100644 --- a/runtime/kusama/src/lib.rs +++ b/runtime/kusama/src/lib.rs @@ -51,6 +51,7 @@ use runtime_parachains::hrmp as parachains_hrmp; use runtime_parachains::scheduler as parachains_scheduler; use runtime_parachains::reward_points as parachains_reward_points; use runtime_parachains::runtime_api_impl::v1 as parachains_runtime_api_impl; +use runtime_parachains::disputes as parachains_disputes; use xcm::v0::{MultiLocation, NetworkId}; use xcm_builder::{ @@ -1019,6 +1020,12 @@ impl parachains_hrmp::Config for Runtime { type Currency = Balances; } +impl parachains_disputes::Config for Runtime { + type Event = Event; + type RewardValidators = (); + type PunishValidators = (); +} + impl parachains_paras_inherent::Config for Runtime {} impl parachains_scheduler::Config for Runtime {} @@ -1231,6 +1238,7 @@ construct_runtime! { ParasUmp: parachains_ump::{Pallet, Call, Storage} = 59, ParasHrmp: parachains_hrmp::{Pallet, Call, Storage, Event} = 60, ParasSessionInfo: parachains_session_info::{Pallet, Call, Storage} = 61, + ParasDisputes: parachains_disputes::{Pallet, Call, Storage, Event} = 62, // Parachain Onboarding Pallets. Start indices at 70 to leave room. Registrar: paras_registrar::{Pallet, Call, Storage, Event} = 70, From 776b3b65a2ca36e4f5d3a5f1c5fa49e9f106999c Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 2 Jun 2021 16:10:44 -0500 Subject: [PATCH 44/67] guide: prepare for runtime API for checking frozenness --- .../implementers-guide/src/runtime/disputes.md | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/roadmap/implementers-guide/src/runtime/disputes.md b/roadmap/implementers-guide/src/runtime/disputes.md index d91c15b0f9ae..77e9f8b06f8b 100644 --- a/roadmap/implementers-guide/src/runtime/disputes.md +++ b/roadmap/implementers-guide/src/runtime/disputes.md @@ -43,10 +43,11 @@ Included: double_map (SessionIndex, CandidateHash) -> Option, // // The i'th entry of the vector corresponds to the i'th validator in the session. SpamSlots: map SessionIndex -> Option>, -// Whether the chain is frozen or not. Starts as `false`. When this is `true`, -// the chain will not accept any new parachain blocks for backing or inclusion. -// It can only be set back to `false` by governance intervention. -Frozen: bool, +// Whether the chain is frozen or not. Starts as `None`. When this is `Some`, +// the chain will not accept any new parachain blocks for backing or inclusion, +// and its value indicates the last valid block number in the chain. +// It can only be set back to `None` by governance intervention. +Frozen: Option, ``` > `byzantine_threshold` refers to the maximum number `f` of validators which may be byzantine. The total number of validators is `n = 3f + e` where `e in { 1, 2, 3 }`. @@ -94,9 +95,10 @@ Frozen: bool, * `could_be_invalid(SessionIndex, CandidateHash) -> bool`: Returns whether a candidate has a live dispute ongoing or a dispute which has already concluded in the negative. -* `is_frozen()`: Load the value of `Frozen` from storage. +* `is_frozen()`: Load the value of `Frozen` from storage. Return true if `Some` and false if `None`. + +* `last_valid_block()`: Load the value of `Frozen` from storage and return. None indicates that all blocks in the chain are potentially valid. * `revert_and_freeze(BlockNumber)`: 1. If `is_frozen()` return. - 1. issue a digest in the block header which indicates the chain is to be abandoned back to the stored block number. - 1. Set `Frozen` to true. + 1. Set `Frozen` to `Some(BlockNumber)` to indicate a rollback to the given block number is necessary. From a4ee80f5ef2f2ef02d144d327c276d7573c8f783 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 2 Jun 2021 16:15:26 -0500 Subject: [PATCH 45/67] remove revert digests in favor of state variable --- primitives/src/v1/mod.rs | 4 ---- runtime/parachains/src/disputes.rs | 31 +++++++++++++++--------------- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/primitives/src/v1/mod.rs b/primitives/src/v1/mod.rs index 33b6aedec911..421d90fa24ad 100644 --- a/primitives/src/v1/mod.rs +++ b/primitives/src/v1/mod.rs @@ -1055,10 +1055,6 @@ pub enum ConsensusLog { /// number in the current chain, inclusive. #[codec(index = 3)] ForceApprove(BlockNumber), - /// The runtime is informing the node to revert the chain back to the given block number - /// in the same chain. - #[codec(index = 4)] - RevertTo(BlockNumber), } impl ConsensusLog { diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 1858a23f4296..60835fcfb51b 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -22,11 +22,10 @@ use primitives::v1::{ DisputeState, DisputeStatementSet, MultiDisputeStatementSet, ValidatorId, ValidatorSignature, DisputeStatement, ValidDisputeStatementKind, InvalidDisputeStatementKind, ExplicitDisputeStatement, CompactStatement, SigningContext, ApprovalVote, ValidatorIndex, - ConsensusLog, }; use sp_runtime::{ traits::{One, Zero, Saturating, AppVerify}, - DispatchError, SaturatedConversion, RuntimeDebug, + DispatchError, RuntimeDebug, }; use frame_support::{ensure, traits::Get, weights::Weight}; use parity_scale_codec::{Encode, Decode}; @@ -143,12 +142,13 @@ pub mod pallet { #[pallet::storage] pub(super) type SpamSlots = StorageMap<_, Twox64Concat, SessionIndex, Vec>; - /// Whether the chain is frozen or not. Starts as `false`. When this is `true`, - /// the chain will not accept any new parachain blocks for backing or inclusion. - /// It can only be set back to `false` by governance intervention. + /// Whether the chain is frozen. Starts as `None`. When this is `Some`, + /// the chain will not accept any new parachain blocks for backing or inclusion, + /// and its value indicates the last valid block number in the chain. + /// It can only be set back to `None` by governance intervention. #[pallet::storage] - #[pallet::getter(fn is_frozen)] - pub(super) type Frozen = StorageValue<_, bool, ValueQuery>; + #[pallet::getter(fn last_valid_block)] + pub(super) type Frozen = StorageValue<_, Option, ValueQuery>; #[pallet::event] #[pallet::generate_deposit(pub fn deposit_event)] @@ -721,13 +721,14 @@ impl Pallet { }) } + pub(crate) fn is_frozen() -> bool { + Self::last_valid_block().is_some() + } + pub(crate) fn revert_and_freeze(revert_to: T::BlockNumber) { if Self::is_frozen() { return } - let revert_to = revert_to.saturated_into(); - >::deposit_log(ConsensusLog::RevertTo(revert_to).into()); - - Frozen::::set(true); + Frozen::::set(Some(revert_to)); } } @@ -1375,7 +1376,7 @@ mod tests { assert!(Pallet::::provide_multi_dispute_data(stmts).is_ok()); Pallet::::note_included(3, candidate_hash.clone(), 3); - assert_eq!(Frozen::::get(), true); + assert_eq!(Frozen::::get(), Some(2)); }); } @@ -1423,7 +1424,7 @@ mod tests { ]; assert!(Pallet::::provide_multi_dispute_data(stmts).is_ok()); - assert_eq!(Frozen::::get(), true); + assert_eq!(Frozen::::get(), Some(2)); }); } @@ -1774,7 +1775,7 @@ mod tests { #[test] fn test_revert_and_freeze() { new_test_ext(Default::default()).execute_with(|| { - Frozen::::put(true); + Frozen::::put(Some(0)); assert_noop!( { Pallet::::revert_and_freeze(0); @@ -1785,7 +1786,7 @@ mod tests { Frozen::::kill(); Pallet::::revert_and_freeze(0); - assert_eq!(Frozen::::get(), true); + assert_eq!(Frozen::::get(), Some(0)); }) } From b0d579c5bfac456cedaefefc401f12a1f0938ac3 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Thu, 3 Jun 2021 11:46:27 -0500 Subject: [PATCH 46/67] merge reversions --- runtime/parachains/src/disputes.rs | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 60835fcfb51b..ca76f0d2ac81 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -726,9 +726,9 @@ impl Pallet { } pub(crate) fn revert_and_freeze(revert_to: T::BlockNumber) { - if Self::is_frozen() { return } - - Frozen::::set(Some(revert_to)); + if Self::last_valid_block().map_or(true, |last| last > revert_to) { + Frozen::::set(Some(revert_to)); + } } } @@ -1790,6 +1790,23 @@ mod tests { }) } + #[test] + fn test_revert_and_freeze_merges() { + new_test_ext(Default::default()).execute_with(|| { + Frozen::::put(Some(10)); + assert_noop!( + { + Pallet::::revert_and_freeze(10); + Result::<(), ()>::Err(()) // Just a small trick in order to use assert_noop. + }, + (), + ); + + Pallet::::revert_and_freeze(8); + assert_eq!(Frozen::::get(), Some(8)); + }) + } + #[test] fn test_has_supermajority_against() { assert_eq!( From 9430d3cc3038febc443446bc2ebf341223c2e272 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Thu, 3 Jun 2021 11:48:18 -0500 Subject: [PATCH 47/67] Update runtime/parachains/src/disputes.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com> --- runtime/parachains/src/disputes.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index ca76f0d2ac81..b6640e979071 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -36,7 +36,7 @@ use crate::{ session_info, }; -/// Whereas the dispute is local or remote. +/// Whether the dispute is local or remote. #[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug)] pub enum DisputeLocation { Local, From 0680641bedf7324d7d5d3c6d127e92f6d592ad59 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Thu, 3 Jun 2021 11:48:25 -0500 Subject: [PATCH 48/67] Update runtime/parachains/src/disputes.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com> --- runtime/parachains/src/disputes.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index b6640e979071..86dd99b886dd 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -110,7 +110,7 @@ pub mod pallet { #[pallet::pallet] pub struct Pallet(_); - /// The last pruneed session, if any. All data stored by this module + /// The last pruned session, if any. All data stored by this module /// references sessions. #[pallet::storage] pub(super) type LastPrunedSession = StorageValue<_, SessionIndex>; From a715d610d8af92b01fabf0f3a6c0a2e8c1f23572 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Thu, 3 Jun 2021 11:48:33 -0500 Subject: [PATCH 49/67] Update runtime/parachains/src/disputes.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com> --- runtime/parachains/src/disputes.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 86dd99b886dd..70716c3242d5 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -740,7 +740,7 @@ fn has_supermajority_against(dispute: &DisputeState) - // If the dispute had not enough validators to confirm, decrement spam slots for all the participating // validators. // -// returns the set of participating validators as a bitvec. +// Returns the set of participating validators as a bitvec. fn decrement_spam( spam_slots: &mut [u32], dispute: &DisputeState, From 8ebc6350279255d2b2407375522fcf4460a554a3 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 9 Jun 2021 19:56:37 -0500 Subject: [PATCH 50/67] add byzantine_threshold and supermajority_threshold utilities to primitives --- primitives/src/v1/mod.rs | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/primitives/src/v1/mod.rs b/primitives/src/v1/mod.rs index 421d90fa24ad..9341d76a700e 100644 --- a/primitives/src/v1/mod.rs +++ b/primitives/src/v1/mod.rs @@ -1174,6 +1174,19 @@ pub struct InherentData { pub parent_header: HDR, } +/// The maximum number of validators `f` which may safely be faulty. +/// +/// The total number of validators is `n = 3f + e` where `e in { 1, 2, 3 }`. +pub fn byzantine_threshold(n: usize) -> usize { + n.saturating_sub(1) / 3 +} + +/// The supermajority threshold of validators which represents a subset +/// guaranteed to have at least f+1 honest validators. +pub fn supermajority_threshold(n: usize) -> usize { + n - byzantine_threshold(n) +} + #[cfg(test)] mod tests { use super::*; @@ -1224,4 +1237,28 @@ mod tests { &Hash::repeat_byte(4), ); } + + #[test] + fn test_byzantine_threshold() { + assert_eq!(byzantine_threshold(0), 0); + assert_eq!(byzantine_threshold(1), 0); + assert_eq!(byzantine_threshold(2), 0); + assert_eq!(byzantine_threshold(3), 0); + assert_eq!(byzantine_threshold(4), 1); + assert_eq!(byzantine_threshold(5), 1); + assert_eq!(byzantine_threshold(6), 1); + assert_eq!(byzantine_threshold(7), 2); + } + + #[test] + fn test_supermajority_threshold() { + assert_eq!(supermajority_threshold(0), 0); + assert_eq!(supermajority_threshold(1), 1); + assert_eq!(supermajority_threshold(2), 2); + assert_eq!(supermajority_threshold(3), 3); + assert_eq!(supermajority_threshold(4), 3); + assert_eq!(supermajority_threshold(5), 4); + assert_eq!(supermajority_threshold(6), 5); + assert_eq!(supermajority_threshold(7), 5); + } } From f3e22c00bc3fa631b8cab4a4deec35ddbade00d0 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 9 Jun 2021 19:56:44 -0500 Subject: [PATCH 51/67] use primitive helpers --- runtime/parachains/src/disputes.rs | 38 +----------------------------- 1 file changed, 1 insertion(+), 37 deletions(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 70716c3242d5..c05e9735b516 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -22,6 +22,7 @@ use primitives::v1::{ DisputeState, DisputeStatementSet, MultiDisputeStatementSet, ValidatorId, ValidatorSignature, DisputeStatement, ValidDisputeStatementKind, InvalidDisputeStatementKind, ExplicitDisputeStatement, CompactStatement, SigningContext, ApprovalVote, ValidatorIndex, + byzantine_threshold, supermajority_threshold }; use sp_runtime::{ traits::{One, Zero, Saturating, AppVerify}, @@ -186,19 +187,6 @@ pub mod pallet { } } -// The maximum number of validators `f` which may safely be faulty. -// -// The total number of validators is `n = 3f + e` where `e in { 1, 2, 3 }`. -fn byzantine_threshold(n: usize) -> usize { - n.saturating_sub(1) / 3 -} - -// The supermajority threshold of validators which is required to -// conclude a dispute. -fn supermajority_threshold(n: usize) -> usize { - n - byzantine_threshold(n) -} - bitflags::bitflags! { #[derive(Default)] struct DisputeStateFlags: u8 { @@ -849,30 +837,6 @@ mod tests { } } - #[test] - fn test_byzantine_threshold() { - assert_eq!(byzantine_threshold(0), 0); - assert_eq!(byzantine_threshold(1), 0); - assert_eq!(byzantine_threshold(2), 0); - assert_eq!(byzantine_threshold(3), 0); - assert_eq!(byzantine_threshold(4), 1); - assert_eq!(byzantine_threshold(5), 1); - assert_eq!(byzantine_threshold(6), 1); - assert_eq!(byzantine_threshold(7), 2); - } - - #[test] - fn test_supermajority_threshold() { - assert_eq!(supermajority_threshold(0), 0); - assert_eq!(supermajority_threshold(1), 1); - assert_eq!(supermajority_threshold(2), 2); - assert_eq!(supermajority_threshold(3), 3); - assert_eq!(supermajority_threshold(4), 3); - assert_eq!(supermajority_threshold(5), 4); - assert_eq!(supermajority_threshold(6), 5); - assert_eq!(supermajority_threshold(7), 5); - } - #[test] fn test_dispute_state_flag_from_state() { assert_eq!( From 53e2ccb374bf6d36b649120a9b2edb95a64e9810 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Wed, 23 Jun 2021 13:09:44 +0100 Subject: [PATCH 52/67] deposit revert event when freezing chain --- runtime/parachains/src/disputes.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index c05e9735b516..aad3f3544cb8 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -103,7 +103,7 @@ pub mod pallet { configuration::Config + session_info::Config { - type Event: From + IsType<::Event>; + type Event: From> + IsType<::Event>; type RewardValidators: RewardValidators; type PunishValidators: PunishValidators; } @@ -153,7 +153,7 @@ pub mod pallet { #[pallet::event] #[pallet::generate_deposit(pub fn deposit_event)] - pub enum Event { + pub enum Event { /// A dispute has been initiated. \[candidate hash, dispute location\] DisputeInitiated(CandidateHash, DisputeLocation), /// A dispute has concluded for or against a candidate. @@ -162,6 +162,11 @@ pub mod pallet { /// A dispute has timed out due to insufficient participation. /// \[para_id, candidate hash\] DisputeTimedOut(CandidateHash), + /// A dispute has concluded with supermajority against a candidate. + /// Block authors should no longer build on top of this head and should + /// instead revert to the block at the given height which is the last + /// known valid block in this chain. + Revert(T::BlockNumber), } #[pallet::hooks] @@ -716,6 +721,7 @@ impl Pallet { pub(crate) fn revert_and_freeze(revert_to: T::BlockNumber) { if Self::last_valid_block().map_or(true, |last| last > revert_to) { Frozen::::set(Some(revert_to)); + Self::deposit_event(Event::Revert(revert_to)); } } } From 4f4b84b951e5c823ba830757f2f8cb00044c25fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Wed, 23 Jun 2021 13:11:11 +0100 Subject: [PATCH 53/67] deposit revert log when freezing chain --- primitives/src/v1/mod.rs | 5 +++++ runtime/parachains/src/disputes.rs | 14 ++++++++------ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/primitives/src/v1/mod.rs b/primitives/src/v1/mod.rs index 9341d76a700e..bcabb69143a2 100644 --- a/primitives/src/v1/mod.rs +++ b/primitives/src/v1/mod.rs @@ -1055,6 +1055,11 @@ pub enum ConsensusLog { /// number in the current chain, inclusive. #[codec(index = 3)] ForceApprove(BlockNumber), + /// A dispute has concluded with supermajority against a candidate. Block authors + /// should no longer build on top of this head and should instead revert to the + /// block at the given height which is the last known valid block in this chain. + #[codec(index = 4)] + Revert(BlockNumber), } impl ConsensusLog { diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index aad3f3544cb8..2a384347ca70 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -18,15 +18,14 @@ use sp_std::prelude::*; use primitives::v1::{ - SessionIndex, CandidateHash, - DisputeState, DisputeStatementSet, MultiDisputeStatementSet, ValidatorId, ValidatorSignature, - DisputeStatement, ValidDisputeStatementKind, InvalidDisputeStatementKind, - ExplicitDisputeStatement, CompactStatement, SigningContext, ApprovalVote, ValidatorIndex, - byzantine_threshold, supermajority_threshold + byzantine_threshold, supermajority_threshold, ApprovalVote, CandidateHash, CompactStatement, + ConsensusLog, DisputeState, DisputeStatement, DisputeStatementSet, ExplicitDisputeStatement, + InvalidDisputeStatementKind, MultiDisputeStatementSet, SessionIndex, SigningContext, + ValidDisputeStatementKind, ValidatorId, ValidatorIndex, ValidatorSignature, }; use sp_runtime::{ traits::{One, Zero, Saturating, AppVerify}, - DispatchError, RuntimeDebug, + DispatchError, RuntimeDebug, SaturatedConversion, }; use frame_support::{ensure, traits::Get, weights::Weight}; use parity_scale_codec::{Encode, Decode}; @@ -722,6 +721,9 @@ impl Pallet { if Self::last_valid_block().map_or(true, |last| last > revert_to) { Frozen::::set(Some(revert_to)); Self::deposit_event(Event::Revert(revert_to)); + frame_system::Pallet::::deposit_log( + ConsensusLog::Revert(revert_to.saturated_into()).into(), + ); } } } From a7ad1919c7f2a2544595c8d640645142213f9356 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Wed, 23 Jun 2021 13:22:50 +0100 Subject: [PATCH 54/67] test revert event and log are generated when freezing --- runtime/parachains/src/disputes.rs | 6 ++++++ runtime/parachains/src/mock.rs | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 2a384347ca70..cbc466859620 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -1747,6 +1747,9 @@ mod tests { #[test] fn test_revert_and_freeze() { new_test_ext(Default::default()).execute_with(|| { + // events are ignored for genesis block + System::set_block_number(1); + Frozen::::put(Some(0)); assert_noop!( { @@ -1758,7 +1761,10 @@ mod tests { Frozen::::kill(); Pallet::::revert_and_freeze(0); + assert_eq!(Frozen::::get(), Some(0)); + assert_eq!(System::digest().logs[0], ConsensusLog::Revert(0).into()); + System::assert_has_event(Event::Revert(0).into()); }) } diff --git a/runtime/parachains/src/mock.rs b/runtime/parachains/src/mock.rs index 2b4a1905318c..fd601a85e7d9 100644 --- a/runtime/parachains/src/mock.rs +++ b/runtime/parachains/src/mock.rs @@ -54,7 +54,7 @@ frame_support::construct_runtime!( Ump: ump::{Pallet, Call, Storage, Event}, Hrmp: hrmp::{Pallet, Call, Storage, Event}, SessionInfo: session_info::{Pallet, Call, Storage}, - Disputes: disputes::{Pallet, Call, Storage, Event}, + Disputes: disputes::{Pallet, Call, Storage, Event}, } ); From 7b04be67c0a46f6d366f1222cd743993b2875e51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Wed, 23 Jun 2021 13:57:01 +0100 Subject: [PATCH 55/67] add trait to decouple disputes handling from paras inherent handling --- runtime/parachains/src/disputes.rs | 75 ++++++++++++++++++++++++ runtime/parachains/src/paras_inherent.rs | 14 +++-- 2 files changed, 84 insertions(+), 5 deletions(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index cbc466859620..01aab7a2ad37 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -89,6 +89,81 @@ impl PunishValidators for () { } } +/// Hook into disputes handling. +/// +/// Allows decoupling parachains handling from disputes so that it can +/// potentially be disabled when instantiating a specific runtime. +pub trait DisputesHandler { + /// Whether the chain is frozen, if the chain is frozen it will not accept + /// any new parachain blocks for backing or inclusion. + fn is_frozen() -> bool; + + /// Handle sets of dispute statements corresponding to 0 or more candidates. + /// Returns a vector of freshly created disputes. + fn provide_multi_dispute_data( + statement_sets: MultiDisputeStatementSet, + ) -> Result, DispatchError>; + + /// Note that the given candidate has been included. + fn note_included( + session: SessionIndex, + candidate_hash: CandidateHash, + included_in: BlockNumber, + ); + + /// Whether the given candidate could be invalid, i.e. there is an ongoing + /// or concluded dispute with supermajority-against. + fn could_be_invalid(session: SessionIndex, candidate_hash: CandidateHash) -> bool; +} + +impl DisputesHandler for () { + fn is_frozen() -> bool { + false + } + + fn provide_multi_dispute_data( + _statement_sets: MultiDisputeStatementSet, + ) -> Result, DispatchError> { + Ok(Vec::new()) + } + + fn note_included( + _session: SessionIndex, + _candidate_hash: CandidateHash, + _included_in: BlockNumber, + ) { + + } + + fn could_be_invalid(_session: SessionIndex, _candidate_hash: CandidateHash) -> bool { + false + } +} + +impl DisputesHandler for pallet::Pallet { + fn is_frozen() -> bool { + pallet::Pallet::::is_frozen() + } + + fn provide_multi_dispute_data( + statement_sets: MultiDisputeStatementSet, + ) -> Result, DispatchError> { + pallet::Pallet::::provide_multi_dispute_data(statement_sets) + } + + fn note_included( + session: SessionIndex, + candidate_hash: CandidateHash, + included_in: T::BlockNumber, + ) { + pallet::Pallet::::note_included(session, candidate_hash, included_in) + } + + fn could_be_invalid(session: SessionIndex, candidate_hash: CandidateHash) -> bool { + pallet::Pallet::::could_be_invalid(session, candidate_hash) + } +} + pub use pallet::*; #[frame_support::pallet] pub mod pallet { diff --git a/runtime/parachains/src/paras_inherent.rs b/runtime/parachains/src/paras_inherent.rs index 60c5ced22b37..3f90d3cc8116 100644 --- a/runtime/parachains/src/paras_inherent.rs +++ b/runtime/parachains/src/paras_inherent.rs @@ -49,7 +49,9 @@ const INCLUSION_INHERENT_CLAIMED_WEIGHT: Weight = 1_000_000_000; // we assume that 75% of an paras inherent's weight is used processing backed candidates const MINIMAL_INCLUSION_INHERENT_WEIGHT: Weight = INCLUSION_INHERENT_CLAIMED_WEIGHT / 4; -pub trait Config: disputes::Config + inclusion::Config + scheduler::Config {} +pub trait Config: inclusion::Config + scheduler::Config { + type DisputesHandler: disputes::DisputesHandler; +} decl_storage! { trait Store for Module as ParaInherent { @@ -99,6 +101,8 @@ decl_module! { origin, data: ParachainsInherentData, ) -> DispatchResultWithPostInfo { + use disputes::DisputesHandler; + let ParachainsInherentData { bitfields: signed_bitfields, backed_candidates, @@ -119,8 +123,8 @@ decl_module! { // Handle disputes logic. let current_session = >::session_index(); let freed_disputed: Vec<(_, FreedReason)> = { - let fresh_disputes = >::provide_multi_dispute_data(disputes)?; - if >::is_frozen() { + let fresh_disputes = T::DisputesHandler::provide_multi_dispute_data(disputes)?; + if T::DisputesHandler::is_frozen() { // The relay chain we are currently on is invalid. Proceed no further on parachains. Included::set(Some(())); return Ok(Some( @@ -158,7 +162,7 @@ decl_module! { // Inform the disputes module of all included candidates. let now = >::block_number(); for (_, candidate_hash) in &freed_concluded { - >::note_included(current_session, *candidate_hash, now); + T::DisputesHandler::note_included(current_session, *candidate_hash, now); } // Handle timeouts for any availability core work. @@ -189,7 +193,7 @@ decl_module! { // Refuse to back any candidates that are disputed or invalid. for candidate in &backed_candidates { ensure!( - !>::could_be_invalid( + !T::DisputesHandler::could_be_invalid( current_session, candidate.candidate.hash(), ), From c6cf3f7c5ec49db782372f1c5f68a4e31c5f19f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Wed, 23 Jun 2021 13:58:13 +0100 Subject: [PATCH 56/67] runtime: fix compilation and setup dispute handler --- runtime/kusama/src/lib.rs | 6 ++++-- runtime/parachains/src/mock.rs | 4 +++- runtime/rococo/src/lib.rs | 6 ++++-- runtime/test-runtime/src/lib.rs | 6 ++++-- runtime/westend/src/lib.rs | 6 ++++-- 5 files changed, 19 insertions(+), 9 deletions(-) diff --git a/runtime/kusama/src/lib.rs b/runtime/kusama/src/lib.rs index d6af90af6069..219393a38851 100644 --- a/runtime/kusama/src/lib.rs +++ b/runtime/kusama/src/lib.rs @@ -1092,7 +1092,9 @@ impl parachains_disputes::Config for Runtime { type PunishValidators = (); } -impl parachains_paras_inherent::Config for Runtime {} +impl parachains_paras_inherent::Config for Runtime { + type DisputesHandler = (); +} impl parachains_scheduler::Config for Runtime {} @@ -1469,7 +1471,7 @@ construct_runtime! { ParasUmp: parachains_ump::{Pallet, Call, Storage, Event} = 59, ParasHrmp: parachains_hrmp::{Pallet, Call, Storage, Event} = 60, ParasSessionInfo: parachains_session_info::{Pallet, Call, Storage} = 61, - ParasDisputes: parachains_disputes::{Pallet, Call, Storage, Event} = 62, + ParasDisputes: parachains_disputes::{Pallet, Call, Storage, Event} = 62, // Parachain Onboarding Pallets. Start indices at 70 to leave room. Registrar: paras_registrar::{Pallet, Call, Storage, Event} = 70, diff --git a/runtime/parachains/src/mock.rs b/runtime/parachains/src/mock.rs index fd601a85e7d9..13c9f836daa5 100644 --- a/runtime/parachains/src/mock.rs +++ b/runtime/parachains/src/mock.rs @@ -195,7 +195,9 @@ impl crate::inclusion::Config for Test { type RewardValidators = TestRewardValidators; } -impl crate::paras_inherent::Config for Test { } +impl crate::paras_inherent::Config for Test { + type DisputesHandler = Disputes; +} impl crate::session_info::Config for Test { } diff --git a/runtime/rococo/src/lib.rs b/runtime/rococo/src/lib.rs index 709c307861ce..7fb53dce1163 100644 --- a/runtime/rococo/src/lib.rs +++ b/runtime/rococo/src/lib.rs @@ -221,7 +221,7 @@ construct_runtime! { Ump: parachains_ump::{Pallet, Call, Storage, Event}, Hrmp: parachains_hrmp::{Pallet, Call, Storage, Event, Config}, SessionInfo: parachains_session_info::{Pallet, Call, Storage}, - Disputes: parachains_disputes::{Pallet, Call, Storage, Event}, + Disputes: parachains_disputes::{Pallet, Call, Storage, Event}, // Parachain Onboarding Pallets Registrar: paras_registrar::{Pallet, Call, Storage, Event}, @@ -784,7 +784,9 @@ impl parachains_disputes::Config for Runtime { type PunishValidators = (); } -impl parachains_paras_inherent::Config for Runtime {} +impl parachains_paras_inherent::Config for Runtime { + type DisputesHandler = (); +} impl parachains_scheduler::Config for Runtime {} diff --git a/runtime/test-runtime/src/lib.rs b/runtime/test-runtime/src/lib.rs index e8ed5c78a496..261a25340878 100644 --- a/runtime/test-runtime/src/lib.rs +++ b/runtime/test-runtime/src/lib.rs @@ -463,7 +463,9 @@ impl parachains_disputes::Config for Runtime { type PunishValidators = (); } -impl parachains_paras_inherent::Config for Runtime {} +impl parachains_paras_inherent::Config for Runtime { + type DisputesHandler = ParasDisputes; +} impl parachains_initializer::Config for Runtime { type Randomness = pallet_babe::RandomnessFromOneEpochAgo; @@ -543,7 +545,7 @@ construct_runtime! { SessionInfo: parachains_session_info::{Pallet, Call, Storage}, Hrmp: parachains_hrmp::{Pallet, Call, Storage, Event}, Ump: parachains_ump::{Pallet, Call, Storage, Event}, - ParasDisputes: parachains_disputes::{Pallet, Call, Storage, Event}, + ParasDisputes: parachains_disputes::{Pallet, Call, Storage, Event}, Sudo: pallet_sudo::{Pallet, Call, Storage, Config, Event}, } diff --git a/runtime/westend/src/lib.rs b/runtime/westend/src/lib.rs index d36a8d129155..ba5f62cbd7ee 100644 --- a/runtime/westend/src/lib.rs +++ b/runtime/westend/src/lib.rs @@ -771,7 +771,9 @@ impl parachains_disputes::Config for Runtime { type PunishValidators = (); } -impl parachains_paras_inherent::Config for Runtime {} +impl parachains_paras_inherent::Config for Runtime { + type DisputesHandler = (); +} impl parachains_scheduler::Config for Runtime {} @@ -1066,7 +1068,7 @@ construct_runtime! { ParasUmp: parachains_ump::{Pallet, Call, Storage, Event} = 50, ParasHrmp: parachains_hrmp::{Pallet, Call, Storage, Event} = 51, ParasSessionInfo: parachains_session_info::{Pallet, Call, Storage} = 52, - ParasDisputes: parachains_disputes::{Pallet, Call, Storage, Event} = 53, + ParasDisputes: parachains_disputes::{Pallet, Call, Storage, Event} = 53, // Parachain Onboarding Pallets. Start indices at 60 to leave room. Registrar: paras_registrar::{Pallet, Call, Storage, Event} = 60, From 9aa95b4b45192de077dea7c7aa31d36b0d2d5d5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Wed, 23 Jun 2021 20:48:23 +0100 Subject: [PATCH 57/67] disputes: add hook for filtering out dispute statements --- runtime/parachains/src/disputes.rs | 13 +++++++++++++ runtime/parachains/src/paras_inherent.rs | 9 +++++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 2135e14ea7b6..e3c2939f999a 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -98,6 +98,11 @@ pub trait DisputesHandler { /// any new parachain blocks for backing or inclusion. fn is_frozen() -> bool; + /// Handler for filtering any dispute statements before including them as part + /// of inherent data. This can be useful to filter out ancient and duplicate + /// dispute statements. + fn filter_multi_dispute_data(statement_sets: &mut MultiDisputeStatementSet); + /// Handle sets of dispute statements corresponding to 0 or more candidates. /// Returns a vector of freshly created disputes. fn provide_multi_dispute_data( @@ -121,6 +126,10 @@ impl DisputesHandler for () { false } + fn filter_multi_dispute_data(statement_sets: &mut MultiDisputeStatementSet) { + statement_sets.clear() + } + fn provide_multi_dispute_data( _statement_sets: MultiDisputeStatementSet, ) -> Result, DispatchError> { @@ -145,6 +154,10 @@ impl DisputesHandler for pallet::Pallet { pallet::Pallet::::is_frozen() } + fn filter_multi_dispute_data(_statement_sets: &mut MultiDisputeStatementSet) { + // TODO: filter duplicate and ancient dispute statements + } + fn provide_multi_dispute_data( statement_sets: MultiDisputeStatementSet, ) -> Result, DispatchError> { diff --git a/runtime/parachains/src/paras_inherent.rs b/runtime/parachains/src/paras_inherent.rs index 3f90d3cc8116..fad9c06dc8ab 100644 --- a/runtime/parachains/src/paras_inherent.rs +++ b/runtime/parachains/src/paras_inherent.rs @@ -35,7 +35,7 @@ use frame_support::{ }; use frame_system::ensure_none; use crate::{ - disputes, + disputes::{self, DisputesHandler}, inclusion, scheduler::{self, FreedReason}, shared, @@ -101,8 +101,6 @@ decl_module! { origin, data: ParachainsInherentData, ) -> DispatchResultWithPostInfo { - use disputes::DisputesHandler; - let ParachainsInherentData { bitfields: signed_bitfields, backed_candidates, @@ -275,7 +273,7 @@ impl ProvideInherent for Module { const INHERENT_IDENTIFIER: InherentIdentifier = PARACHAINS_INHERENT_IDENTIFIER; fn create_inherent(data: &InherentData) -> Option { - let inherent_data: ParachainsInherentData + let mut inherent_data: ParachainsInherentData = match data.get_data(&Self::INHERENT_IDENTIFIER) { Ok(Some(d)) => d, @@ -290,6 +288,9 @@ impl ProvideInherent for Module { } }; + // filter out any unneeded dispute statements + T::DisputesHandler::filter_multi_dispute_data(&mut inherent_data.disputes); + // Sanity check: session changes can invalidate an inherent, and we _really_ don't want that to happen. // See github.com/paritytech/polkadot/issues/1327 let inherent_data = match Self::enter( From ff06d0f4ac15154bffcaf3382f9e8bedc3b1d7bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Wed, 23 Jun 2021 21:21:23 +0100 Subject: [PATCH 58/67] disputes: add initializer hooks to DisputesHandler --- runtime/parachains/src/disputes.rs | 33 ++++++++++++++++++++++++ runtime/parachains/src/inclusion.rs | 3 ++- runtime/parachains/src/initializer.rs | 10 +++---- runtime/parachains/src/mock.rs | 5 ++-- runtime/parachains/src/paras_inherent.rs | 6 ++--- 5 files changed, 44 insertions(+), 13 deletions(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index e3c2939f999a..8dcf364518ed 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -119,6 +119,15 @@ pub trait DisputesHandler { /// Whether the given candidate could be invalid, i.e. there is an ongoing /// or concluded dispute with supermajority-against. fn could_be_invalid(session: SessionIndex, candidate_hash: CandidateHash) -> bool; + + /// Called by the initializer to initialize the configuration module. + fn initializer_initialize(now: BlockNumber) -> Weight; + + /// Called by the initializer to finalize the configuration module. + fn initializer_finalize(); + + /// Called by the initializer to note that a new session has started. + fn initializer_on_new_session(notification: &SessionChangeNotification); } impl DisputesHandler for () { @@ -147,6 +156,18 @@ impl DisputesHandler for () { fn could_be_invalid(_session: SessionIndex, _candidate_hash: CandidateHash) -> bool { false } + + fn initializer_initialize(_now: BlockNumber) -> Weight { + 0 + } + + fn initializer_finalize() { + + } + + fn initializer_on_new_session(_notification: &SessionChangeNotification) { + + } } impl DisputesHandler for pallet::Pallet { @@ -175,6 +196,18 @@ impl DisputesHandler for pallet::Pallet { fn could_be_invalid(session: SessionIndex, candidate_hash: CandidateHash) -> bool { pallet::Pallet::::could_be_invalid(session, candidate_hash) } + + fn initializer_initialize(now: T::BlockNumber) -> Weight { + pallet::Pallet::::initializer_initialize(now) + } + + fn initializer_finalize() { + pallet::Pallet::::initializer_finalize() + } + + fn initializer_on_new_session(notification: &SessionChangeNotification) { + pallet::Pallet::::initializer_on_new_session(notification) + } } pub use pallet::*; diff --git a/runtime/parachains/src/inclusion.rs b/runtime/parachains/src/inclusion.rs index cdbefa057be2..0d03eafadb70 100644 --- a/runtime/parachains/src/inclusion.rs +++ b/runtime/parachains/src/inclusion.rs @@ -35,7 +35,7 @@ use parity_scale_codec::{Encode, Decode}; use bitvec::{order::Lsb0 as BitOrderLsb0, vec::BitVec}; use sp_runtime::{DispatchError, traits::{One, Saturating}}; -use crate::{configuration, paras, dmp, ump, hrmp, shared, scheduler::CoreAssignment}; +use crate::{configuration, disputes, paras, dmp, ump, hrmp, shared, scheduler::CoreAssignment}; /// A bitfield signed by a validator indicating that it is keeping its piece of the erasure-coding /// for any backed candidates referred to by a `1` bit available. @@ -118,6 +118,7 @@ pub trait Config: + configuration::Config { type Event: From> + Into<::Event>; + type DisputesHandler: disputes::DisputesHandler; type RewardValidators: RewardValidators; } diff --git a/runtime/parachains/src/initializer.rs b/runtime/parachains/src/initializer.rs index f7bfe8abfcc8..40b45e9e32af 100644 --- a/runtime/parachains/src/initializer.rs +++ b/runtime/parachains/src/initializer.rs @@ -29,7 +29,8 @@ use frame_support::{ use parity_scale_codec::{Encode, Decode}; use crate::{ configuration::{self, HostConfiguration}, - shared, paras, scheduler, inclusion, session_info, disputes, dmp, ump, hrmp, + disputes::DisputesHandler, + shared, paras, scheduler, inclusion, session_info, dmp, ump, hrmp, }; /// Information about a session change that has just occurred. @@ -77,7 +78,6 @@ pub trait Config: + scheduler::Config + inclusion::Config + session_info::Config - + disputes::Config + dmp::Config + ump::Config + hrmp::Config @@ -136,7 +136,7 @@ decl_module! { scheduler::Module::::initializer_initialize(now) + inclusion::Module::::initializer_initialize(now) + session_info::Module::::initializer_initialize(now) + - disputes::Pallet::::initializer_initialize(now) + + T::DisputesHandler::initializer_initialize(now) + dmp::Module::::initializer_initialize(now) + ump::Module::::initializer_initialize(now) + hrmp::Module::::initializer_initialize(now); @@ -151,7 +151,7 @@ decl_module! { hrmp::Module::::initializer_finalize(); ump::Module::::initializer_finalize(); dmp::Module::::initializer_finalize(); - disputes::Pallet::::initializer_finalize(); + T::DisputesHandler::initializer_finalize(); session_info::Module::::initializer_finalize(); inclusion::Module::::initializer_finalize(); scheduler::Module::::initializer_finalize(); @@ -231,7 +231,7 @@ impl Module { scheduler::Module::::initializer_on_new_session(¬ification); inclusion::Module::::initializer_on_new_session(¬ification); session_info::Module::::initializer_on_new_session(¬ification); - disputes::Pallet::::initializer_on_new_session(¬ification); + T::DisputesHandler::initializer_on_new_session(¬ification); dmp::Module::::initializer_on_new_session(¬ification, &outgoing_paras); ump::Module::::initializer_on_new_session(¬ification, &outgoing_paras); hrmp::Module::::initializer_on_new_session(¬ification, &outgoing_paras); diff --git a/runtime/parachains/src/mock.rs b/runtime/parachains/src/mock.rs index 13c9f836daa5..5bbc5c9ab7d0 100644 --- a/runtime/parachains/src/mock.rs +++ b/runtime/parachains/src/mock.rs @@ -192,12 +192,11 @@ impl crate::scheduler::Config for Test { } impl crate::inclusion::Config for Test { type Event = Event; + type DisputesHandler = Disputes; type RewardValidators = TestRewardValidators; } -impl crate::paras_inherent::Config for Test { - type DisputesHandler = Disputes; -} +impl crate::paras_inherent::Config for Test { } impl crate::session_info::Config for Test { } diff --git a/runtime/parachains/src/paras_inherent.rs b/runtime/parachains/src/paras_inherent.rs index fad9c06dc8ab..ade7623c7d0a 100644 --- a/runtime/parachains/src/paras_inherent.rs +++ b/runtime/parachains/src/paras_inherent.rs @@ -35,7 +35,7 @@ use frame_support::{ }; use frame_system::ensure_none; use crate::{ - disputes::{self, DisputesHandler}, + disputes::DisputesHandler, inclusion, scheduler::{self, FreedReason}, shared, @@ -49,9 +49,7 @@ const INCLUSION_INHERENT_CLAIMED_WEIGHT: Weight = 1_000_000_000; // we assume that 75% of an paras inherent's weight is used processing backed candidates const MINIMAL_INCLUSION_INHERENT_WEIGHT: Weight = INCLUSION_INHERENT_CLAIMED_WEIGHT / 4; -pub trait Config: inclusion::Config + scheduler::Config { - type DisputesHandler: disputes::DisputesHandler; -} +pub trait Config: inclusion::Config + scheduler::Config {} decl_storage! { trait Store for Module as ParaInherent { From dd35e7f75597d0c3d4762dc882e87d0d709db4b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Wed, 23 Jun 2021 21:22:21 +0100 Subject: [PATCH 59/67] runtime: remove disputes pallet from all runtimes --- runtime/kusama/src/lib.rs | 13 ++----------- runtime/rococo/src/lib.rs | 13 ++----------- runtime/test-runtime/src/lib.rs | 5 ++--- runtime/westend/src/lib.rs | 13 ++----------- 4 files changed, 8 insertions(+), 36 deletions(-) diff --git a/runtime/kusama/src/lib.rs b/runtime/kusama/src/lib.rs index 89c81c660173..70db8f644e51 100644 --- a/runtime/kusama/src/lib.rs +++ b/runtime/kusama/src/lib.rs @@ -52,7 +52,6 @@ use runtime_parachains::hrmp as parachains_hrmp; use runtime_parachains::scheduler as parachains_scheduler; use runtime_parachains::reward_points as parachains_reward_points; use runtime_parachains::runtime_api_impl::v1 as parachains_runtime_api_impl; -use runtime_parachains::disputes as parachains_disputes; use xcm::v0::{MultiLocation::{self, Null, X1}, NetworkId, BodyId, Xcm, Junction::Parachain}; use xcm::v0::MultiAsset::{self, AllConcreteFungible}; @@ -1068,6 +1067,7 @@ impl parachains_session_info::Config for Runtime {} impl parachains_inclusion::Config for Runtime { type Event = Event; + type DisputesHandler = (); type RewardValidators = parachains_reward_points::RewardValidatorsWithEraPoints; } @@ -1094,15 +1094,7 @@ impl parachains_hrmp::Config for Runtime { type Currency = Balances; } -impl parachains_disputes::Config for Runtime { - type Event = Event; - type RewardValidators = (); - type PunishValidators = (); -} - -impl parachains_paras_inherent::Config for Runtime { - type DisputesHandler = (); -} +impl parachains_paras_inherent::Config for Runtime {} impl parachains_scheduler::Config for Runtime {} @@ -1474,7 +1466,6 @@ construct_runtime! { ParasUmp: parachains_ump::{Pallet, Call, Storage, Event} = 59, ParasHrmp: parachains_hrmp::{Pallet, Call, Storage, Event} = 60, ParasSessionInfo: parachains_session_info::{Pallet, Call, Storage} = 61, - ParasDisputes: parachains_disputes::{Pallet, Call, Storage, Event} = 62, // Parachain Onboarding Pallets. Start indices at 70 to leave room. Registrar: paras_registrar::{Pallet, Call, Storage, Event} = 70, diff --git a/runtime/rococo/src/lib.rs b/runtime/rococo/src/lib.rs index cba1c370d354..8f4bdb4f5abf 100644 --- a/runtime/rococo/src/lib.rs +++ b/runtime/rococo/src/lib.rs @@ -76,7 +76,6 @@ use runtime_parachains::inclusion as parachains_inclusion; use runtime_parachains::paras_inherent as parachains_paras_inherent; use runtime_parachains::initializer as parachains_initializer; use runtime_parachains::session_info as parachains_session_info; -use runtime_parachains::disputes as parachains_disputes; use runtime_parachains::paras as parachains_paras; use runtime_parachains::dmp as parachains_dmp; use runtime_parachains::ump as parachains_ump; @@ -221,7 +220,6 @@ construct_runtime! { Ump: parachains_ump::{Pallet, Call, Storage, Event}, Hrmp: parachains_hrmp::{Pallet, Call, Storage, Event, Config}, SessionInfo: parachains_session_info::{Pallet, Call, Storage}, - Disputes: parachains_disputes::{Pallet, Call, Storage, Event}, // Parachain Onboarding Pallets Registrar: paras_registrar::{Pallet, Call, Storage, Event}, @@ -581,6 +579,7 @@ impl runtime_parachains::inclusion::RewardValidators for RewardValidators { impl parachains_inclusion::Config for Runtime { type Event = Event; + type DisputesHandler = (); type RewardValidators = RewardValidators; } @@ -778,15 +777,7 @@ impl parachains_hrmp::Config for Runtime { type Currency = Balances; } -impl parachains_disputes::Config for Runtime { - type Event = Event; - type RewardValidators = (); - type PunishValidators = (); -} - -impl parachains_paras_inherent::Config for Runtime { - type DisputesHandler = (); -} +impl parachains_paras_inherent::Config for Runtime {} impl parachains_scheduler::Config for Runtime {} diff --git a/runtime/test-runtime/src/lib.rs b/runtime/test-runtime/src/lib.rs index 77af5c5c6906..6e252d26f0f4 100644 --- a/runtime/test-runtime/src/lib.rs +++ b/runtime/test-runtime/src/lib.rs @@ -456,6 +456,7 @@ impl parachains_shared::Config for Runtime {} impl parachains_inclusion::Config for Runtime { type Event = Event; + type DisputesHandler = ParasDisputes; type RewardValidators = RewardValidatorsWithEraPoints; } @@ -465,9 +466,7 @@ impl parachains_disputes::Config for Runtime { type PunishValidators = (); } -impl parachains_paras_inherent::Config for Runtime { - type DisputesHandler = ParasDisputes; -} +impl parachains_paras_inherent::Config for Runtime {} impl parachains_initializer::Config for Runtime { type Randomness = pallet_babe::RandomnessFromOneEpochAgo; diff --git a/runtime/westend/src/lib.rs b/runtime/westend/src/lib.rs index f59c9ec0e189..6a51670fb955 100644 --- a/runtime/westend/src/lib.rs +++ b/runtime/westend/src/lib.rs @@ -51,7 +51,6 @@ use runtime_parachains::ump as parachains_ump; use runtime_parachains::hrmp as parachains_hrmp; use runtime_parachains::scheduler as parachains_scheduler; use runtime_parachains::reward_points as parachains_reward_points; -use runtime_parachains::disputes as parachains_disputes; use runtime_parachains::runtime_api_impl::v1 as parachains_runtime_api_impl; use xcm::v0::{MultiLocation::{self, Null, X1}, NetworkId, Xcm, Junction::Parachain}; @@ -741,6 +740,7 @@ impl parachains_session_info::Config for Runtime {} impl parachains_inclusion::Config for Runtime { type Event = Event; + type DisputesHandler = (); type RewardValidators = parachains_reward_points::RewardValidatorsWithEraPoints; } @@ -767,15 +767,7 @@ impl parachains_hrmp::Config for Runtime { type Currency = Balances; } -impl parachains_disputes::Config for Runtime { - type Event = Event; - type RewardValidators = (); - type PunishValidators = (); -} - -impl parachains_paras_inherent::Config for Runtime { - type DisputesHandler = (); -} +impl parachains_paras_inherent::Config for Runtime {} impl parachains_scheduler::Config for Runtime {} @@ -1065,7 +1057,6 @@ construct_runtime! { ParasUmp: parachains_ump::{Pallet, Call, Storage, Event} = 50, ParasHrmp: parachains_hrmp::{Pallet, Call, Storage, Event} = 51, ParasSessionInfo: parachains_session_info::{Pallet, Call, Storage} = 52, - ParasDisputes: parachains_disputes::{Pallet, Call, Storage, Event} = 53, // Parachain Onboarding Pallets. Start indices at 60 to leave room. Registrar: paras_registrar::{Pallet, Call, Storage, Event} = 60, From eb8181188572bc61525d4a68bda1c6480458eeb9 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 14 Jul 2021 21:49:45 -0500 Subject: [PATCH 60/67] tag TODOs --- runtime/parachains/src/disputes.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 8dcf364518ed..9e9627c2c44e 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -596,7 +596,11 @@ impl Pallet { }; for to_prune in to_prune { + // This should be small, as disputes are rare, so `None` is fine. >::remove_prefix(to_prune, None); + + // This is larger, and will be extracted to the `shared` module for more proper pruning. + // TODO: https://github.com/paritytech/polkadot/issues/3469 >::remove_prefix(to_prune, None); SpamSlots::::remove(to_prune); } From c7afa0cf79449f46190c84d4477b70831c7c4119 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 14 Jul 2021 21:57:03 -0500 Subject: [PATCH 61/67] don't import any dispute statements just yet... --- runtime/parachains/src/disputes.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 9e9627c2c44e..1cfbbff402f5 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -175,8 +175,12 @@ impl DisputesHandler for pallet::Pallet { pallet::Pallet::::is_frozen() } - fn filter_multi_dispute_data(_statement_sets: &mut MultiDisputeStatementSet) { - // TODO: filter duplicate and ancient dispute statements + fn filter_multi_dispute_data(statement_sets: &mut MultiDisputeStatementSet) { + // TODO: filter duplicate and ancient dispute statements. For now, don't import anything + // because there will be redundancies. + // + // https://github.com/paritytech/polkadot/issues/3472 + statement_sets.clear(); } fn provide_multi_dispute_data( From 4378b5c505af36ae9038968aeb59ed38c4fa0111 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 16 Jul 2021 21:05:14 -0500 Subject: [PATCH 62/67] address grumbles --- runtime/parachains/src/disputes.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 1cfbbff402f5..23f119b423a8 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -218,7 +218,6 @@ pub use pallet::*; #[frame_support::pallet] pub mod pallet { use frame_support::pallet_prelude::*; - use frame_system::pallet_prelude::*; use super::*; #[pallet::config] @@ -293,12 +292,6 @@ pub mod pallet { Revert(T::BlockNumber), } - #[pallet::hooks] - impl Hooks> for Pallet {} - - #[pallet::call] - impl Pallet {} - #[pallet::error] pub enum Error { /// Duplicate dispute statement sets provided. From 8a7abdbbf395d70fd34dd1e0cf092c979d2d669c Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 16 Jul 2021 21:11:51 -0500 Subject: [PATCH 63/67] fix spellcheck, hopefully --- roadmap/implementers-guide/src/runtime/disputes.md | 7 ++++--- roadmap/implementers-guide/src/types/disputes.md | 10 +++++----- runtime/parachains/src/disputes.rs | 12 ++++++------ 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/roadmap/implementers-guide/src/runtime/disputes.md b/roadmap/implementers-guide/src/runtime/disputes.md index 77e9f8b06f8b..54cb4371c21c 100644 --- a/roadmap/implementers-guide/src/runtime/disputes.md +++ b/roadmap/implementers-guide/src/runtime/disputes.md @@ -55,7 +55,8 @@ Frozen: Option, ## Session Change 1. If the current session is not greater than `config.dispute_period + 1`, nothing to do here. -1. Set `pruning_target = current_session - config.dispute_period - 1`. We add the extra `1` because we want to keep things for `config.dispute_period` _full_ sessions. The stuff at the end of the most recent session has been around for ~0 sessions, not ~1. +1. Set `pruning_target = current_session - config.dispute_period - 1`. We add the extra `1` because we want to keep things for `config.dispute_period` _full_ sessions. + The stuff at the end of the most recent session has been around for a little over 0 sessions, not a little over 1. 1. If `LastPrunedSession` is `None`, then set `LastPrunedSession` to `Some(pruning_target)` and return. 1. Otherwise, clear out all disputes, included candidates, and `SpamSlots` entries in the range `last_pruned..=pruning_target` and set `LastPrunedSession` to `Some(pruning_target)`. @@ -77,7 +78,7 @@ Frozen: Option, 1. If the overlap of the validators in the `DisputeStatementSet` and those already present in the `DisputeState` is fewer in number than `byzantine_threshold + 1` and the candidate is not present in the `Included` map 1. increment `SpamSlots` for each validator in the `DisputeStatementSet` which is not already in the `DisputeState`. Initialize the `SpamSlots` to a zeroed vector first, if necessary. do not increment `SpamSlots` if the candidate is local. 1. If the value for any spam slot exceeds `config.dispute_max_spam_slots`, return false. - 1. If the overlap of the validators in the `DisputeStatementSet` and those already present in the `DisputeState` is at least `byzantine_threshold + 1`, the `DisputeState` has fewer than `byzantine_threshold + 1` validators, and the candidate is not present in the `Included` map, decrement `SpamSlots` for each validator in the `DisputeState`. + 1. If the overlap of the validators in the `DisputeStatementSet` and those already present in the `DisputeState` is at least `byzantine_threshold + 1`, the `DisputeState` has fewer than `byzantine_threshold + 1` validators, and the candidate is not present in the `Included` map, then decrease `SpamSlots` by 1 for each validator in the `DisputeState`. 1. Import all statements into the dispute. This should fail if any statements are duplicate or if the corresponding bit for the corresponding validator is set in the dispute already. 1. If `concluded_at` is `None`, reward all statements. 1. If `concluded_at` is `Some`, reward all statements slightly less. @@ -90,7 +91,7 @@ Frozen: Option, * `note_included(SessionIndex, CandidateHash, included_in: BlockNumber)`: 1. Add `(SessionIndex, CandidateHash)` to the `Included` map with `included_in - 1` as the value. - 1. If there is a dispute under `(Sessionindex, CandidateHash)` with fewer than `byzantine_threshold + 1` participating validators, decrement `SpamSlots` for each validator in the `DisputeState`. + 1. If there is a dispute under `(Sessionindex, CandidateHash)` with fewer than `byzantine_threshold + 1` participating validators, decrease `SpamSlots` by 1 for each validator in the `DisputeState`. 1. If there is a dispute under `(SessionIndex, CandidateHash)` that has concluded against the candidate, invoke `revert_and_freeze` with the stored block number. * `could_be_invalid(SessionIndex, CandidateHash) -> bool`: Returns whether a candidate has a live dispute ongoing or a dispute which has already concluded in the negative. diff --git a/roadmap/implementers-guide/src/types/disputes.md b/roadmap/implementers-guide/src/types/disputes.md index 36f07e843ea3..3043b7615abd 100644 --- a/roadmap/implementers-guide/src/types/disputes.md +++ b/roadmap/implementers-guide/src/types/disputes.md @@ -1,6 +1,6 @@ # Disputes -## DisputeStatementSet +## `DisputeStatementSet` ```rust /// A set of statements about a specific candidate. @@ -11,7 +11,7 @@ struct DisputeStatementSet { } ``` -## DisputeStatement +## `DisputeStatement` ```rust /// A statement about a candidate, to be used within some dispute resolution process. @@ -43,7 +43,7 @@ enum InvalidDisputeStatementKind { } ``` -## ExplicitDisputeStatement +## `ExplicitDisputeStatement` ```rust struct ExplicitDisputeStatement { @@ -53,7 +53,7 @@ struct ExplicitDisputeStatement { } ``` -## MultiDisputeStatementSet +## `MultiDisputeStatementSet` Sets of statements for many (zero or more) disputes. @@ -61,7 +61,7 @@ Sets of statements for many (zero or more) disputes. type MultiDisputeStatementSet = Vec; ``` -## DisputeState +## `DisputeState` ```rust struct DisputeState { diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 23f119b423a8..05f9616071c2 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -280,10 +280,10 @@ pub mod pallet { /// A dispute has been initiated. \[candidate hash, dispute location\] DisputeInitiated(CandidateHash, DisputeLocation), /// A dispute has concluded for or against a candidate. - /// \[para_id, candidate hash, dispute result\] + /// \[para id, candidate hash, dispute result\] DisputeConcluded(CandidateHash, DisputeResult), /// A dispute has timed out due to insufficient participation. - /// \[para_id, candidate hash\] + /// \[para id, candidate hash\] DisputeTimedOut(CandidateHash), /// A dispute has concluded with supermajority against a candidate. /// Block authors should no longer build on top of this head and should @@ -517,7 +517,7 @@ impl DisputeStateImporter { } impl Pallet { - /// Called by the initalizer to initialize the disputes module. + /// Called by the initializer to initialize the disputes module. pub(crate) fn initializer_initialize(now: T::BlockNumber) -> Weight { let config = >::config(); @@ -572,10 +572,10 @@ impl Pallet { weight } - /// Called by the initalizer to finalize the disputes module. + /// Called by the initializer to finalize the disputes module. pub(crate) fn initializer_finalize() { } - /// Called by the initalizer to note a new session in the disputes module. + /// Called by the initializer to note a new session in the disputes module. pub(crate) fn initializer_on_new_session(notification: &SessionChangeNotification) { let config = >::config(); @@ -646,7 +646,7 @@ impl Pallet { /// Handle a set of dispute statements corresponding to a single candidate. /// - /// Fails if the dispute data is invalid. Returns a bool indicating whether the + /// Fails if the dispute data is invalid. Returns a boolean indicating whether the /// dispute is fresh. fn provide_dispute_data(config: &HostConfiguration, set: DisputeStatementSet) -> Result From 663b5ccf007f8bc171a4a4d7b8e123cd0c542200 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 16 Jul 2021 21:13:06 -0500 Subject: [PATCH 64/67] maybe now? --- runtime/parachains/src/disputes.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 05f9616071c2..f7327ad2bd4f 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -280,10 +280,10 @@ pub mod pallet { /// A dispute has been initiated. \[candidate hash, dispute location\] DisputeInitiated(CandidateHash, DisputeLocation), /// A dispute has concluded for or against a candidate. - /// \[para id, candidate hash, dispute result\] + /// `\[para id, candidate hash, dispute result\]` DisputeConcluded(CandidateHash, DisputeResult), /// A dispute has timed out due to insufficient participation. - /// \[para id, candidate hash\] + /// `\[para id, candidate hash\]` DisputeTimedOut(CandidateHash), /// A dispute has concluded with supermajority against a candidate. /// Block authors should no longer build on top of this head and should From 3be97bbad4f3efffb46148dc3a73393097387ccf Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 16 Jul 2021 21:19:16 -0500 Subject: [PATCH 65/67] last spellcheck round --- roadmap/implementers-guide/src/runtime/disputes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/roadmap/implementers-guide/src/runtime/disputes.md b/roadmap/implementers-guide/src/runtime/disputes.md index 54cb4371c21c..79015e4a740f 100644 --- a/roadmap/implementers-guide/src/runtime/disputes.md +++ b/roadmap/implementers-guide/src/runtime/disputes.md @@ -6,7 +6,7 @@ However, this isn't the end of the story. We are working in a forkful blockchain 1. For security, validators that misbehave shouldn't only be slashed on one fork, but on all possible forks. Validators that misbehave shouldn't be able to create a new fork of the chain when caught and get away with their misbehavior. 1. It is possible (and likely) that the parablock being contested has not appeared on all forks. -1. If a block author believes that there is a disputed parablock on a specific fork that will resolve to a reversion of the fork, that block author is better incentivized to build on a different fork which does not include that parablock. +1. If a block author believes that there is a disputed parablock on a specific fork that will resolve to a reversion of the fork, that block author has more incentive to build on a different fork which does not include that parablock. This means that in all likelihood, there is the possibility of disputes that are started on one fork of the relay chain, and as soon as the dispute resolution process starts to indicate that the parablock is indeed invalid, that fork of the relay chain will be abandoned and the dispute will never be fully resolved on that chain. From bb47ed4aa1d88ef0ced5ed6966553fdf9d3bd3c3 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 19 Jul 2021 10:54:36 -0500 Subject: [PATCH 66/67] fix runtime tests --- runtime/parachains/src/mock.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/parachains/src/mock.rs b/runtime/parachains/src/mock.rs index 86cdc2f849ce..1c54ff6c5ae1 100644 --- a/runtime/parachains/src/mock.rs +++ b/runtime/parachains/src/mock.rs @@ -55,7 +55,7 @@ frame_support::construct_runtime!( Ump: ump::{Pallet, Call, Storage, Event}, Hrmp: hrmp::{Pallet, Call, Storage, Event}, SessionInfo: session_info::{Pallet, Call, Storage}, - Disputes: disputes::{Pallet, Call, Storage, Event}, + Disputes: disputes::{Pallet, Storage, Event}, } ); From 5ecf237c51871c0046d9aa3403e99d73f265fa12 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Mon, 19 Jul 2021 11:08:03 -0500 Subject: [PATCH 67/67] fix test-runtime --- runtime/test-runtime/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/test-runtime/src/lib.rs b/runtime/test-runtime/src/lib.rs index aebf6027b64e..5570dbbac74a 100644 --- a/runtime/test-runtime/src/lib.rs +++ b/runtime/test-runtime/src/lib.rs @@ -545,7 +545,7 @@ construct_runtime! { SessionInfo: parachains_session_info::{Pallet, Call, Storage}, Hrmp: parachains_hrmp::{Pallet, Call, Storage, Event}, Ump: parachains_ump::{Pallet, Call, Storage, Event}, - ParasDisputes: parachains_disputes::{Pallet, Call, Storage, Event}, + ParasDisputes: parachains_disputes::{Pallet, Storage, Event}, Sudo: pallet_sudo::{Pallet, Call, Storage, Config, Event}, }