From 257c9a29ae9417c1abe7d58eb0f1b090d43f1813 Mon Sep 17 00:00:00 2001 From: Fedor Sakharov Date: Wed, 29 Jan 2020 14:49:54 +0300 Subject: [PATCH 01/18] Parachains double vote handler initial implementation. --- primitives/src/parachain.rs | 10 +- runtime/common/src/parachains.rs | 358 ++++++++++++++++++++++++++++++- runtime/common/src/registrar.rs | 46 +++- runtime/kusama/src/lib.rs | 1 + runtime/polkadot/src/lib.rs | 1 + 5 files changed, 402 insertions(+), 14 deletions(-) diff --git a/primitives/src/parachain.rs b/primitives/src/parachain.rs index 8b1381e87298..93432d360a30 100644 --- a/primitives/src/parachain.rs +++ b/primitives/src/parachain.rs @@ -281,8 +281,8 @@ impl CollationInfo { } /// Candidate receipt type. -#[derive(PartialEq, Eq, Clone, Encode, Decode)] -#[cfg_attr(feature = "std", derive(Debug, Default))] +#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug)] +#[cfg_attr(feature = "std", derive(Default))] pub struct CandidateReceipt { /// The ID of the parachain this is a candidate for. pub parachain_index: Id, @@ -479,8 +479,7 @@ pub struct ValidationCode(#[cfg_attr(feature = "std", serde(with="bytes"))] pub pub struct Activity(#[cfg_attr(feature = "std", serde(with="bytes"))] pub Vec); /// Statements which can be made about parachain candidates. -#[derive(Clone, PartialEq, Eq, Encode)] -#[cfg_attr(feature = "std", derive(Debug))] +#[derive(Clone, PartialEq, Eq, Decode, Encode, RuntimeDebug)] pub enum Statement { /// Proposal of a parachain candidate. #[codec(index = "1")] @@ -495,8 +494,7 @@ pub enum Statement { /// An either implicit or explicit attestation to the validity of a parachain /// candidate. -#[derive(Clone, PartialEq, Decode, Encode)] -#[cfg_attr(feature = "std", derive(Debug))] +#[derive(Clone, Eq, PartialEq, Decode, Encode, RuntimeDebug)] pub enum ValidityAttestation { /// implicit validity attestation by issuing. /// This corresponds to issuance of a `Candidate` statement. diff --git a/runtime/common/src/parachains.rs b/runtime/common/src/parachains.rs index 00e51adeafef..344396d50de7 100644 --- a/runtime/common/src/parachains.rs +++ b/runtime/common/src/parachains.rs @@ -21,9 +21,17 @@ use rstd::result; use rstd::collections::btree_map::BTreeMap; use codec::{Encode, Decode}; -use sp_runtime::traits::{ - Hash as HashT, BlakeTwo256, Saturating, One, Zero, Dispatchable, - AccountIdConversion, BadOrigin, +use session::historical::IdentificationTuple; +use sp_runtime::{ + Perbill, RuntimeDebug, + traits::{ + Hash as HashT, BlakeTwo256, Saturating, One, Zero, Dispatchable, + AccountIdConversion, BadOrigin, Convert, + }, +}; +use sp_staking::{ + SessionIndex, + offence::{ReportOffence, Offence, Kind}, }; use frame_support::weights::SimpleDispatchInfo; use primitives::{ @@ -31,11 +39,11 @@ use primitives::{ parachain::{ self, Id as ParaId, Chain, DutyRoster, AttestedCandidate, Statement, ParachainDispatchOrigin, UpwardMessage, BlockIngressRoots, ValidatorId, ActiveParas, CollatorId, Retriable, - NEW_HEADS_IDENTIFIER, + ValidityAttestation, NEW_HEADS_IDENTIFIER, }, }; use frame_support::{ - Parameter, dispatch::DispatchResult, decl_storage, decl_module, decl_error, ensure, + Parameter, dispatch::DispatchResult, decl_storage, decl_module, decl_error, ensure, fail, traits::{Currency, Get, WithdrawReason, ExistenceRequirement, Randomness}, }; @@ -105,13 +113,66 @@ impl> ParachainCurrency for T where /// Interface to the persistent (stash) identities of the current validators. pub struct ValidatorIdentities(rstd::marker::PhantomData); +#[derive(RuntimeDebug, Encode, Decode)] +#[derive(Clone, Eq, PartialEq)] +pub struct DoubleVoteReport { + /// Identity of the double-voter. + pub identity: ValidatorId, + /// First vote of the double-vote. + pub first: (Statement, ValidityAttestation), + /// Second vote of the double-vote. + pub second: (Statement, ValidityAttestation), +} + +impl DoubleVoteReport { + fn verify>( + &self, + parent_hash: H, + ) -> Result<(), ()> { + let first = self.first.clone(); + let second = self.second.clone(); + + Self::verify_vote(first, &parent_hash, self.identity.clone())?; + Self::verify_vote(second, &parent_hash, self.identity.clone())?; + + Ok(()) + } + + fn verify_vote>( + vote: (Statement, ValidityAttestation), + parent_hash: H, + authority: ValidatorId, + ) -> Result<(), ()> { + use sp_runtime::traits::AppVerify; + let (statement, attestation) = vote; + + let (payload, sig) = match attestation { + ValidityAttestation::Implicit(sig) => { + let payload = localized_payload(statement, parent_hash); + + (payload, sig) + } + ValidityAttestation::Explicit(sig) => { + let payload = localized_payload(statement, parent_hash); + + (payload, sig) + } + }; + + match sig.verify(&payload[..], &authority) { + true => Ok(()), + false => Err(()), + } + } +} + impl Get> for ValidatorIdentities { fn get() -> Vec { >::validators() } } -pub trait Trait: attestations::Trait { +pub trait Trait: attestations::Trait + session::historical::Trait { /// The outer origin type. type Origin: From + From>; @@ -137,6 +198,14 @@ pub trait Trait: attestations::Trait { /// Max head data size. type MaxHeadDataSize: Get; + + /// Submit double-vote offence reports. + type ReportDoubleVote: + ReportOffence< + Self::AccountId, + IdentificationTuple, + DoubleVoteOffence>, + >; } /// Origin for the parachains module. @@ -147,6 +216,44 @@ pub enum Origin { Parachain(ParaId), } +/// An offence that is filed if the validator has submitted a double vote. +#[derive(RuntimeDebug)] +#[cfg_attr(feature = "std", derive(Clone, PartialEq, Eq))] +pub struct DoubleVoteOffence { + /// The current session index in which we report a validator. + session_index: SessionIndex, + /// The size of the validator set in current session/era. + validator_set_count: u32, + /// An offender that has submitted two conflicting votes. + offender: Offender, +} + +impl Offence for DoubleVoteOffence { + const ID: Kind = *b"double-vote:doub"; + type TimeSlot = SessionIndex; + + fn offenders(&self) -> Vec { + vec![self.offender.clone()] + } + + fn session_index(&self) -> SessionIndex { + self.session_index + } + + fn validator_set_count(&self) -> u32 { + self.validator_set_count + } + + fn time_slot(&self) -> Self::TimeSlot { + self.session_index + } + + fn slash_fraction(_offenders_count: u32, _validator_set_count: u32) -> Perbill { + // Slash 100%. + Perbill::from_percent(100) + } +} + // result of as trie_db::NodeCodec>::hashed_null_node() const EMPTY_TRIE_ROOT: [u8; 32] = [ 3, 23, 10, 46, 117, 151, 183, 183, 227, 216, 76, 5, 57, 29, 19, 154, @@ -322,6 +429,62 @@ decl_module! { Ok(()) } + /// Provide a proof that some validator has commited a double-vote. + pub fn report_double_vote( + origin, + report: DoubleVoteReport, + ) -> DispatchResult { + ensure_none(origin)?; + + let session_index = >::current_index(); + let validators = >::validators(); + let validator_set_count = validators.len() as u32; + let parent_hash = >::block_hash(>::block_number()); + + let authorities = Self::authorities(); + let offender_idx = match authorities.iter().position(|a| *a == report.identity) { + Some(idx) => idx, + None => return Err("Not in validator set".into()), + }; + + let offender = match T::FullIdentificationOf::convert(validators[offender_idx].clone()) { + Some(id) => (validators[offender_idx].clone(), id), + None => return Err("Failed to convert".into()), + }; + + let offence = DoubleVoteOffence { session_index, validator_set_count, offender }; + + ensure!( + report.verify(parent_hash).is_ok(), + "Report verification failed.", + ); + + match (report.first.0, report.second.0) { + // If issuing a `Candidate` message on a parachain block, neither a `Valid` or + // `Invalid` vote cannot be issued on that parachain block, as the `Candidate` + // message is an implicit validity vote. + (Statement::Candidate(candidate), Statement::Valid(hash)) | + (Statement::Candidate(candidate), Statement::Invalid(hash)) | + (Statement::Valid(hash), Statement::Candidate(candidate)) | + (Statement::Invalid(hash), Statement::Candidate(candidate)) + if candidate.hash() == hash => { + T::ReportDoubleVote::report_offence(vec![], offence); + }, + // Otherwise, it is illegal to cast both a `Valid` and + // `Invalid` vote on a given parachain block. + (Statement::Valid(hash_1), Statement::Invalid(hash_2)) | + (Statement::Invalid(hash_1), Statement::Valid(hash_2)) + if hash_1 == hash_2 => { + T::ReportDoubleVote::report_offence(vec![], offence); + }, + _ => { + fail!("Not a case of a double vote"); + } + } + + Ok(()) + } + fn on_initialize() { ::DidUpdate::kill(); } @@ -712,7 +875,6 @@ impl Module { active_parachains: &[(ParaId, Option<(CollatorId, Retriable)>)] ) -> rstd::result::Result, sp_runtime::DispatchError> { - use primitives::parachain::ValidityAttestation; use sp_runtime::traits::AppVerify; // returns groups of slices that have the same chain ID. @@ -967,6 +1129,7 @@ pub fn ensure_parachain(o: OuterOrigin) -> result::Result; type MaxCodeSize = MaxCodeSize; type MaxHeadDataSize = MaxHeadDataSize; + type ReportDoubleVote = OffenceHandler; + } + + thread_local! { + pub static OFFENCES: RefCell, DoubleVoteOffence>)>> = RefCell::new(vec![]); + } + + pub struct OffenceHandler; + impl ReportOffence, DoubleVoteOffence>> for OffenceHandler { + fn report_offence(reporters: Vec, offence: DoubleVoteOffence>) { + OFFENCES.with(|l| l.borrow_mut().push((reporters, offence))); + } } type Parachains = Module; @@ -1265,6 +1442,12 @@ mod tests { ParachainsCall::set_heads(v) } + fn report_double_vote( + report: DoubleVoteReport, + ) -> ParachainsCall { + ParachainsCall::report_double_vote(report) + } + fn make_attestations(candidate: &mut AttestedCandidate) { let mut vote_implicit = false; let parent_hash = >::parent_hash(); @@ -2139,4 +2322,165 @@ mod tests { let hashed_null_node = as trie_db::NodeCodec>::hashed_null_node(); assert_eq!(hashed_null_node, EMPTY_TRIE_ROOT.into()) } + + #[test] + fn double_vote_works() { + let parachains = vec![ + (0u32.into(), vec![], vec![]), + ]; + + let extract_key = |public: ValidatorId| { + let mut raw_public = [0; 32]; + raw_public.copy_from_slice(public.as_ref()); + Sr25519Keyring::from_raw_public(raw_public).unwrap() + }; + + let candidate = CandidateReceipt { + parachain_index: 1.into(), + collator: Default::default(), + signature: Default::default(), + head_data: HeadData(vec![1, 2, 3]), + parent_head: HeadData(vec![1, 2, 3]), + egress_queue_roots: vec![], + fees: 0, + block_data_hash: Default::default(), + upward_messages: vec![], + erasure_root: [1u8; 32].into(), + }; + + let candidate_hash = candidate.hash(); + + // Test that a Candidate and Valid statements on the same candidate get slashed. + new_test_ext(parachains.clone()).execute_with(|| { + init_block(); + let authorities = Parachains::authorities(); + let authority_index = 0; + let key = extract_key(authorities[authority_index].clone()); + + let statement_candidate = Statement::Candidate(candidate.clone()); + let statement_valid = Statement::Valid(candidate_hash.clone()); + let block_number = >::block_number(); + let parent_hash = >::block_hash(block_number); + + let payload_1 = localized_payload(statement_candidate.clone(), parent_hash); + let payload_2 = localized_payload(statement_valid.clone(), parent_hash); + + let signature_1 = key.sign(&payload_1[..]).into(); + let signature_2 = key.sign(&payload_2[..]).into(); + + let attestation_1 = ValidityAttestation::Implicit(signature_1); + let attestation_2 = ValidityAttestation::Explicit(signature_2); + + let report = DoubleVoteReport { + identity: ValidatorId::from(key.public()), + first: (statement_candidate, attestation_1), + second: (statement_valid, attestation_2), + }; + + assert_ok!(Parachains::dispatch( + report_double_vote(report), + Origin::NONE, + )); + + let offences = OFFENCES.with(|l| l.replace(vec![])); + assert_eq!(offences, vec![(vec![], DoubleVoteOffence { + session_index: 0, + validator_set_count: 8, + offender: (authority_index as u64, staking::Exposure { + total: 0, + own: 0, + others: vec![], + }), + })]); + }); + + // Test that a Candidate and Invalid statements on the same candidate get slashed. + new_test_ext(parachains.clone()).execute_with(|| { + init_block(); + let authorities = Parachains::authorities(); + let authority_index = 0; + let key = extract_key(authorities[authority_index].clone()); + + let statement_candidate = Statement::Candidate(candidate.clone()); + let statement_invalid = Statement::Invalid(candidate_hash.clone()); + let block_number = >::block_number(); + let parent_hash = >::block_hash(block_number); + + let payload_1 = localized_payload(statement_candidate.clone(), parent_hash); + let payload_2 = localized_payload(statement_invalid.clone(), parent_hash); + + let signature_1 = key.sign(&payload_1[..]).into(); + let signature_2 = key.sign(&payload_2[..]).into(); + + let attestation_1 = ValidityAttestation::Implicit(signature_1); + let attestation_2 = ValidityAttestation::Explicit(signature_2); + + let report = DoubleVoteReport { + identity: ValidatorId::from(key.public()), + first: (statement_candidate, attestation_1), + second: (statement_invalid, attestation_2), + }; + + assert_ok!(Parachains::dispatch( + report_double_vote(report), + Origin::NONE, + )); + + let offences = OFFENCES.with(|l| l.replace(vec![])); + assert_eq!(offences, vec![(vec![], DoubleVoteOffence { + session_index: 0, + validator_set_count: 8, + offender: (authority_index as u64, staking::Exposure { + total: 0, + own: 0, + others: vec![], + }), + })]); + }); + + + // Test that an Invalid and Valid statements on the same candidate get slashed. + new_test_ext(parachains.clone()).execute_with(|| { + init_block(); + let authorities = Parachains::authorities(); + let authority_index = 0; + let key = extract_key(authorities[authority_index].clone()); + + let statement_invalid = Statement::Invalid(candidate_hash.clone()); + let statement_valid = Statement::Valid(candidate_hash.clone()); + let block_number = >::block_number(); + let parent_hash = >::block_hash(block_number); + + let payload_1 = localized_payload(statement_invalid.clone(), parent_hash); + let payload_2 = localized_payload(statement_valid.clone(), parent_hash); + + let signature_1 = key.sign(&payload_1[..]).into(); + let signature_2 = key.sign(&payload_2[..]).into(); + + let attestation_1 = ValidityAttestation::Explicit(signature_1); + let attestation_2 = ValidityAttestation::Explicit(signature_2); + + let report = DoubleVoteReport { + identity: ValidatorId::from(key.public()), + first: (statement_invalid, attestation_1), + second: (statement_valid, attestation_2), + }; + + assert_ok!(Parachains::dispatch( + report_double_vote(report), + Origin::NONE, + )); + + let offences = OFFENCES.with(|l| l.replace(vec![])); + assert_eq!(offences, vec![(vec![], DoubleVoteOffence { + session_index: 0, + validator_set_count: 8, + offender: (authority_index as u64, staking::Exposure { + total: 0, + own: 0, + others: vec![], + }), + })]); + }); + } } diff --git a/runtime/common/src/registrar.rs b/runtime/common/src/registrar.rs index 2775a49fec8c..827b06fcae48 100644 --- a/runtime/common/src/registrar.rs +++ b/runtime/common/src/registrar.rs @@ -650,7 +650,7 @@ mod tests { traits::{ BlakeTwo256, IdentityLookup, OnInitialize, OnFinalize, Dispatchable, AccountIdConversion, - }, testing::{UintAuthorityId, Header}, Perbill + }, testing::{UintAuthorityId, Header}, Perbill, curve::PiecewiseLinear, }; use primitives::{ parachain::{ @@ -681,6 +681,17 @@ mod tests { } } + pallet_staking_reward_curve::build! { + const REWARD_CURVE: PiecewiseLinear<'static> = curve!( + min_inflation: 0_025_000, + max_inflation: 0_100_000, + ideal_stake: 0_500_000, + falloff: 0_050_000, + max_piece_count: 40, + test_precision: 0_005_000, + ); + } + #[derive(Clone, Eq, PartialEq)] pub struct Test; parameter_types! { @@ -739,7 +750,11 @@ mod tests { } parameter_types!{ + pub const SlashDeferDuration: staking::EraIndex = 7; pub const AttestationPeriod: BlockNumber = 100; + pub const MinimumPeriod: u64 = 3; + pub const SessionsPerEra: sp_staking::SessionIndex = 6; + pub const BondingDuration: staking::EraIndex = 28; } impl attestations::Trait for Test { @@ -752,6 +767,7 @@ mod tests { pub const Period: BlockNumber = 1; pub const Offset: BlockNumber = 0; pub const DisabledValidatorsThreshold: Perbill = Perbill::from_percent(17); + pub const RewardCurve: &'static PiecewiseLinear<'static> = &REWARD_CURVE; } impl session::Trait for Test { @@ -770,6 +786,33 @@ mod tests { pub const MaxCodeSize: u32 = 100; } + impl staking::Trait for Test { + type RewardRemainder = (); + type CurrencyToVote = (); + type Event = (); + type Currency = balances::Module; + type Slash = (); + type Reward = (); + type SessionsPerEra = SessionsPerEra; + type BondingDuration = BondingDuration; + type SlashDeferDuration = SlashDeferDuration; + type SlashCancelOrigin = system::EnsureRoot; + type SessionInterface = Self; + type Time = timestamp::Module; + type RewardCurve = RewardCurve; + } + + impl timestamp::Trait for Test { + type Moment = u64; + type OnTimestampSet = (); + type MinimumPeriod = MinimumPeriod; + } + + impl session::historical::Trait for Test { + type FullIdentification = staking::Exposure; + type FullIdentificationOf = staking::ExposureOf; + } + impl parachains::Trait for Test { type Origin = Origin; type Call = Call; @@ -779,6 +822,7 @@ mod tests { type Randomness = RandomnessCollectiveFlip; type MaxCodeSize = MaxCodeSize; type MaxHeadDataSize = MaxHeadDataSize; + type ReportDoubleVote = (); } parameter_types! { diff --git a/runtime/kusama/src/lib.rs b/runtime/kusama/src/lib.rs index f85319cca8cd..21bbf15c3e66 100644 --- a/runtime/kusama/src/lib.rs +++ b/runtime/kusama/src/lib.rs @@ -475,6 +475,7 @@ impl parachains::Trait for Runtime { type Registrar = Registrar; type MaxCodeSize = MaxCodeSize; type MaxHeadDataSize = MaxHeadDataSize; + type ReportDoubleVote = Offences; } parameter_types! { diff --git a/runtime/polkadot/src/lib.rs b/runtime/polkadot/src/lib.rs index 95f6ee3d65d7..1d8abca7245a 100644 --- a/runtime/polkadot/src/lib.rs +++ b/runtime/polkadot/src/lib.rs @@ -479,6 +479,7 @@ impl parachains::Trait for Runtime { type Registrar = Registrar; type MaxCodeSize = MaxCodeSize; type MaxHeadDataSize = MaxHeadDataSize; + type ReportDoubleVote = Offences; } parameter_types! { From a0ea78f5cbe07ccbcbdd64c625245e95b6f9c766 Mon Sep 17 00:00:00 2001 From: Fedor Sakharov Date: Sun, 16 Feb 2020 17:20:02 +0300 Subject: [PATCH 02/18] Make tests test the actual slashing. --- Cargo.lock | 1 + runtime/common/Cargo.toml | 1 + runtime/common/src/parachains.rs | 230 +++++++++++++++++++++++++------ 3 files changed, 188 insertions(+), 44 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 636fdb963212..a18c53c23640 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3970,6 +3970,7 @@ dependencies = [ "pallet-authorship 2.0.0 (git+https://github.com/paritytech/substrate?branch=polkadot-master)", "pallet-babe 2.0.0 (git+https://github.com/paritytech/substrate?branch=polkadot-master)", "pallet-balances 2.0.0 (git+https://github.com/paritytech/substrate?branch=polkadot-master)", + "pallet-offences 2.0.0 (git+https://github.com/paritytech/substrate?branch=polkadot-master)", "pallet-randomness-collective-flip 2.0.0 (git+https://github.com/paritytech/substrate?branch=polkadot-master)", "pallet-session 2.0.0 (git+https://github.com/paritytech/substrate?branch=polkadot-master)", "pallet-staking 2.0.0 (git+https://github.com/paritytech/substrate?branch=polkadot-master)", diff --git a/runtime/common/Cargo.toml b/runtime/common/Cargo.toml index 35912b805739..056b75b5dc35 100644 --- a/runtime/common/Cargo.toml +++ b/runtime/common/Cargo.toml @@ -28,6 +28,7 @@ staking = { package = "pallet-staking", git = "https://github.com/paritytech/sub system = { package = "frame-system", git = "https://github.com/paritytech/substrate", branch = "polkadot-master", default-features = false } timestamp = { package = "pallet-timestamp", git = "https://github.com/paritytech/substrate", branch = "polkadot-master", default-features = false } vesting = { package = "pallet-vesting", git = "https://github.com/paritytech/substrate", branch = "polkadot-master", default-features = false } +offences = { package = "pallet-offences", git = "https://github.com/paritytech/substrate", branch = "polkadot-master", default-features = false } primitives = { package = "polkadot-primitives", path = "../../primitives", default-features = false } polkadot-parachain = { path = "../../parachain", default-features = false } diff --git a/runtime/common/src/parachains.rs b/runtime/common/src/parachains.rs index 344396d50de7..750f8862a0b3 100644 --- a/runtime/common/src/parachains.rs +++ b/runtime/common/src/parachains.rs @@ -1129,14 +1129,13 @@ pub fn ensure_parachain(o: OuterOrigin) -> result::Result = &REWARD_CURVE; } + pub struct CurrencyToVoteHandler; + + impl Convert for CurrencyToVoteHandler { + fn convert(x: u128) -> u128 { x } + } + + impl Convert for CurrencyToVoteHandler { + fn convert(x: u128) -> u64 { x.saturated_into() } + } + impl staking::Trait for Test { type RewardRemainder = (); - type CurrencyToVote = (); + type CurrencyToVote = CurrencyToVoteHandler; type Event = (); type Currency = balances::Module; type Slash = (); @@ -1334,6 +1345,12 @@ mod tests { type MaxRetries = MaxRetries; } + impl offences::Trait for Test { + type Event = (); + type IdentificationTuple = session::historical::IdentificationTuple; + type OnOffenceHandler = Staking; + } + parameter_types! { pub const MaxHeadDataSize: u32 = 100; pub const MaxCodeSize: u32 = 100; @@ -1348,22 +1365,16 @@ mod tests { type Registrar = registrar::Module; type MaxCodeSize = MaxCodeSize; type MaxHeadDataSize = MaxHeadDataSize; - type ReportDoubleVote = OffenceHandler; - } - - thread_local! { - pub static OFFENCES: RefCell, DoubleVoteOffence>)>> = RefCell::new(vec![]); - } - - pub struct OffenceHandler; - impl ReportOffence, DoubleVoteOffence>> for OffenceHandler { - fn report_offence(reporters: Vec, offence: DoubleVoteOffence>) { - OFFENCES.with(|l| l.borrow_mut().push((reporters, offence))); - } + type ReportDoubleVote = Offences; } type Parachains = Module; type System = system::Module; + type Offences = offences::Module; + type Staking = staking::Module; + type Balances = balances::Module; + type Session = session::Module; + type Timestamp = timestamp::Module; type RandomnessCollectiveFlip = randomness_collective_flip::Module; type Registrar = registrar::Module; @@ -1429,8 +1440,9 @@ mod tests { staking::GenesisConfig:: { current_era: 0, stakers, - validator_count: 10, - minimum_validator_count: 8, + validator_count: 8, + force_era: staking::Forcing::ForceNew, + minimum_validator_count: 0, invulnerables: vec![], .. Default::default() }.assimilate_storage(&mut t).unwrap(); @@ -1538,6 +1550,26 @@ mod tests { } } + fn start_session(session_index: SessionIndex) { + // Compensate for session delay + let session_index = session_index + 1; + for i in Session::current_index()..session_index { + println!("session index {}", i); + Staking::new_session(i); + System::set_block_number((i + 1).into()); + Timestamp::set_timestamp(System::block_number() * 6000); + println!("block {:?}", System::block_number()); + Session::on_initialize(System::block_number()); + } + + assert_eq!(Session::current_index(), session_index); + } + + fn start_era(era_index: EraIndex) { + start_session((era_index * 3).into()); + assert_eq!(Staking::current_era(), era_index); + } + fn init_block() { println!("Initializing {}", System::block_number()); System::on_initialize(System::block_number()); @@ -1553,6 +1585,7 @@ mod tests { Registrar::on_finalize(System::block_number()); System::on_finalize(System::block_number()); } + Staking::new_session(System::block_number() as u32); System::set_block_number(System::block_number() + 1); init_block(); } @@ -2352,7 +2385,11 @@ mod tests { // Test that a Candidate and Valid statements on the same candidate get slashed. new_test_ext(parachains.clone()).execute_with(|| { - init_block(); + assert_eq!(Staking::current_era(), 0); + assert_eq!(Session::current_index(), 0); + + start_era(1); + let authorities = Parachains::authorities(); let authority_index = 0; let key = extract_key(authorities[authority_index].clone()); @@ -2377,26 +2414,61 @@ mod tests { second: (statement_valid, attestation_2), }; + // Check that in the beginning the genesis balances are there. + for i in 0..authorities.len() { + assert_eq!(Balances::total_balance(&(i as u64)), 10_000_000); + assert_eq!(Staking::slashable_balance_of(&(i as u64)), 10_000); + + assert_eq!( + Staking::stakers(i as u64), + staking::Exposure { + total: 10_000, + own: 10_000, + others: vec![], + }, + ); + } + assert_ok!(Parachains::dispatch( report_double_vote(report), Origin::NONE, )); - let offences = OFFENCES.with(|l| l.replace(vec![])); - assert_eq!(offences, vec![(vec![], DoubleVoteOffence { - session_index: 0, - validator_set_count: 8, - offender: (authority_index as u64, staking::Exposure { + start_era(2); + + // Check that the balance of 0-th validator is slashed 100%. + assert_eq!(Balances::total_balance(&0), 10_000_000 - 10_000); + assert_eq!(Staking::slashable_balance_of(&0), 0); + + assert_eq!( + Staking::stakers(0), + staking::Exposure { total: 0, own: 0, others: vec![], - }), - })]); + }, + ); + + // Check that the balances of all other validators are left intact. + for i in 1..authorities.len() { + assert_eq!(Balances::total_balance(&(i as u64)), 10_000_000); + assert_eq!(Staking::slashable_balance_of(&(i as u64)), 10_000); + + assert_eq!( + Staking::stakers(i as u64), + staking::Exposure { + total: 10_000, + own: 10_000, + others: vec![], + }, + ); + } }); // Test that a Candidate and Invalid statements on the same candidate get slashed. new_test_ext(parachains.clone()).execute_with(|| { - init_block(); + start_era(1); + let authorities = Parachains::authorities(); let authority_index = 0; let key = extract_key(authorities[authority_index].clone()); @@ -2421,27 +2493,63 @@ mod tests { second: (statement_invalid, attestation_2), }; + // Check that in the beginning the genesis balances are there. + for i in 0..authorities.len() { + assert_eq!(Balances::total_balance(&(i as u64)), 10_000_000); + assert_eq!(Staking::slashable_balance_of(&(i as u64)), 10_000); + + assert_eq!( + Staking::stakers(i as u64), + staking::Exposure { + total: 10_000, + own: 10_000, + others: vec![], + }, + ); + } + assert_ok!(Parachains::dispatch( report_double_vote(report), Origin::NONE, )); - let offences = OFFENCES.with(|l| l.replace(vec![])); - assert_eq!(offences, vec![(vec![], DoubleVoteOffence { - session_index: 0, - validator_set_count: 8, - offender: (authority_index as u64, staking::Exposure { + start_era(2); + + // Check that the balance of 0-th validator is slashed 100%. + assert_eq!(Balances::total_balance(&0), 10_000_000 - 10_000); + assert_eq!(Staking::slashable_balance_of(&0), 0); + + assert_eq!( + Staking::stakers(0), + staking::Exposure { total: 0, own: 0, others: vec![], - }), - })]); + }, + ); + + // Check that the balances of all other validators are left intact. + for i in 1..authorities.len() { + assert_eq!(Balances::total_balance(&(i as u64)), 10_000_000); + assert_eq!(Staking::slashable_balance_of(&(i as u64)), 10_000); + + assert_eq!( + Staking::stakers(i as u64), + staking::Exposure { + total: 10_000, + own: 10_000, + others: vec![], + }, + ); + } + }); // Test that an Invalid and Valid statements on the same candidate get slashed. new_test_ext(parachains.clone()).execute_with(|| { - init_block(); + start_era(1); + let authorities = Parachains::authorities(); let authority_index = 0; let key = extract_key(authorities[authority_index].clone()); @@ -2466,21 +2574,55 @@ mod tests { second: (statement_valid, attestation_2), }; + // Check that in the beginning the genesis balances are there. + for i in 0..authorities.len() { + assert_eq!(Balances::total_balance(&(i as u64)), 10_000_000); + assert_eq!(Staking::slashable_balance_of(&(i as u64)), 10_000); + + assert_eq!( + Staking::stakers(i as u64), + staking::Exposure { + total: 10_000, + own: 10_000, + others: vec![], + }, + ); + } + assert_ok!(Parachains::dispatch( report_double_vote(report), Origin::NONE, )); - let offences = OFFENCES.with(|l| l.replace(vec![])); - assert_eq!(offences, vec![(vec![], DoubleVoteOffence { - session_index: 0, - validator_set_count: 8, - offender: (authority_index as u64, staking::Exposure { + start_era(2); + + // Check that the balance of 0-th validator is slashed 100%. + assert_eq!(Balances::total_balance(&0), 10_000_000 - 10_000); + assert_eq!(Staking::slashable_balance_of(&0), 0); + + assert_eq!( + Staking::stakers(0), + staking::Exposure { total: 0, own: 0, others: vec![], - }), - })]); + }, + ); + + // Check that the balances of all other validators are left intact. + for i in 1..authorities.len() { + assert_eq!(Balances::total_balance(&(i as u64)), 10_000_000); + assert_eq!(Staking::slashable_balance_of(&(i as u64)), 10_000); + + assert_eq!( + Staking::stakers(i as u64), + staking::Exposure { + total: 10_000, + own: 10_000, + others: vec![], + }, + ); + } }); } } From ed78c75a01dec962887bb717865125c1104bc46e Mon Sep 17 00:00:00 2001 From: Fedor Sakharov Date: Thu, 20 Feb 2020 17:27:11 +0300 Subject: [PATCH 03/18] Implement SignedExtension validation of double vote reports. --- runtime/common/src/parachains.rs | 164 +++++++++++++++++++++++++------ runtime/kusama/src/lib.rs | 3 +- runtime/polkadot/src/lib.rs | 3 +- 3 files changed, 138 insertions(+), 32 deletions(-) diff --git a/runtime/common/src/parachains.rs b/runtime/common/src/parachains.rs index 750f8862a0b3..d68348ebee45 100644 --- a/runtime/common/src/parachains.rs +++ b/runtime/common/src/parachains.rs @@ -26,14 +26,18 @@ use sp_runtime::{ Perbill, RuntimeDebug, traits::{ Hash as HashT, BlakeTwo256, Saturating, One, Zero, Dispatchable, - AccountIdConversion, BadOrigin, Convert, + AccountIdConversion, BadOrigin, Convert, SignedExtension, }, + transaction_validity::{TransactionValidityError, ValidTransaction, TransactionValidity}, }; use sp_staking::{ SessionIndex, offence::{ReportOffence, Offence, Kind}, }; -use frame_support::weights::SimpleDispatchInfo; +use frame_support::{ + dispatch::{IsSubType}, + weights::{DispatchInfo, SimpleDispatchInfo}, +}; use primitives::{ Hash, Balance, parachain::{ @@ -43,13 +47,14 @@ use primitives::{ }, }; use frame_support::{ - Parameter, dispatch::DispatchResult, decl_storage, decl_module, decl_error, ensure, fail, + Parameter, dispatch::DispatchResult, decl_storage, decl_module, decl_error, ensure, traits::{Currency, Get, WithdrawReason, ExistenceRequirement, Randomness}, }; +use sp_runtime::transaction_validity::InvalidTransaction; use inherents::{ProvideInherent, InherentData, MakeFatalError, InherentIdentifier}; -use system::ensure_none; +use system::{ensure_none, ensure_signed}; use crate::attestations::{self, IncludedBlocks}; use crate::registrar::Registrar; @@ -430,12 +435,19 @@ decl_module! { } /// Provide a proof that some validator has commited a double-vote. + /// + /// The weight is 0; in order to avoid DoS a `SignedExtension` validation + /// is implemented. + #[weight = SimpleDispatchInfo::FixedNormal(0)] pub fn report_double_vote( origin, report: DoubleVoteReport, ) -> DispatchResult { - ensure_none(origin)?; + let reporter = ensure_signed(origin)?; + // The following code duplicates the logic in `ValidateDoubeVoteReports::validate` + // since we still need to do all these steps to derermine which one of the offenders + // gets slashed. let session_index = >::current_index(); let validators = >::validators(); let validator_set_count = validators.len() as u32; @@ -459,28 +471,9 @@ decl_module! { "Report verification failed.", ); - match (report.first.0, report.second.0) { - // If issuing a `Candidate` message on a parachain block, neither a `Valid` or - // `Invalid` vote cannot be issued on that parachain block, as the `Candidate` - // message is an implicit validity vote. - (Statement::Candidate(candidate), Statement::Valid(hash)) | - (Statement::Candidate(candidate), Statement::Invalid(hash)) | - (Statement::Valid(hash), Statement::Candidate(candidate)) | - (Statement::Invalid(hash), Statement::Candidate(candidate)) - if candidate.hash() == hash => { - T::ReportDoubleVote::report_offence(vec![], offence); - }, - // Otherwise, it is illegal to cast both a `Valid` and - // `Invalid` vote on a given parachain block. - (Statement::Valid(hash_1), Statement::Invalid(hash_2)) | - (Statement::Invalid(hash_1), Statement::Valid(hash_2)) - if hash_1 == hash_2 => { - T::ReportDoubleVote::report_offence(vec![], offence); - }, - _ => { - fail!("Not a case of a double vote"); - } - } + // Checks if this is actually a double vote are + // implemented in `ValidateDoubleVoteReports::validete`. + T::ReportDoubleVote::report_offence(vec![reporter], offence); Ok(()) } @@ -1125,6 +1118,117 @@ pub fn ensure_parachain(o: OuterOrigin) -> result::Result(rstd::marker::PhantomData) where + ::Call: IsSubType, T>; + +impl rstd::fmt::Debug for ValidateDoubleVoteReports where + ::Call: IsSubType, T> +{ + fn fmt(&self, f: &mut rstd::fmt::Formatter) -> rstd::fmt::Result { + write!(f, "ValidateDoubleVoteReports") + } +} + +/// Custom validity error used while validating double vote reports. +#[repr(u8)] +pub enum DoubleVoteValidityError { + /// The authority being reported is not in the authority set. + NotAnAuthority = 0, + + /// Failed to convert offender's `FullIdentificationOf`. + FailedToConvertId = 1, + + /// The signature on one or both of the statements in the report is wrong. + InvalidSignature = 2, + + /// The two statements in the report are not conflicting. + NotDoubleVote = 3, +} + +impl SignedExtension for ValidateDoubleVoteReports where + ::Call: IsSubType, T> +{ + const IDENTIFIER: &'static str = "ValidateDoubleVoteReports"; + type AccountId = T::AccountId; + type Call = ::Call; + type AdditionalSigned = (); + type Pre = (); + type DispatchInfo = DispatchInfo; + + fn additional_signed(&self) + -> rstd::result::Result + { + Ok(()) + } + + fn validate( + &self, + _who: &Self::AccountId, + call: &Self::Call, + _info: DispatchInfo, + _len: usize, + ) -> TransactionValidity { + let r = ValidTransaction::default(); + + if let Some(local_call) = call.is_sub_type() { + if let Call::report_double_vote(report) = local_call { + let e = TransactionValidityError::from( + InvalidTransaction::Custom(DoubleVoteValidityError::NotAnAuthority as u8) + ); + + let validators = >::validators(); + let parent_hash = >::block_hash(>::block_number()); + + let authorities = Module::::authorities(); + let offender_idx = match authorities.iter().position(|a| *a == report.identity) { + Some(idx) => idx, + None => return Err(e), + }; + + let e = TransactionValidityError::from( + InvalidTransaction::Custom(DoubleVoteValidityError::FailedToConvertId as u8) + ); + if T::FullIdentificationOf::convert(validators[offender_idx].clone()).is_none() { + return Err(e); + } + + let e = TransactionValidityError::from( + InvalidTransaction::Custom(DoubleVoteValidityError::InvalidSignature as u8) + ); + report.verify(parent_hash).map_err(|_| e)?; + + let e = TransactionValidityError::from( + InvalidTransaction::Custom(DoubleVoteValidityError::NotDoubleVote as u8) + ); + match (&report.first.0, &report.second.0) { + // If issuing a `Candidate` message on a parachain block, neither a `Valid` or + // `Invalid` vote cannot be issued on that parachain block, as the `Candidate` + // message is an implicit validity vote. + (Statement::Candidate(candidate), Statement::Valid(hash)) | + (Statement::Candidate(candidate), Statement::Invalid(hash)) | + (Statement::Valid(hash), Statement::Candidate(candidate)) | + (Statement::Invalid(hash), Statement::Candidate(candidate)) + if candidate.hash() == *hash => {}, + // Otherwise, it is illegal to cast both a `Valid` and + // `Invalid` vote on a given parachain block. + (Statement::Valid(hash_1), Statement::Invalid(hash_2)) | + (Statement::Invalid(hash_1), Statement::Valid(hash_2)) + if *hash_1 == *hash_2 => {}, + _ => { + return Err(e); + } + } + } + } + + Ok(r) + } +} + + #[cfg(test)] mod tests { use super::*; @@ -2431,7 +2535,7 @@ mod tests { assert_ok!(Parachains::dispatch( report_double_vote(report), - Origin::NONE, + Origin::signed(1), )); start_era(2); @@ -2510,7 +2614,7 @@ mod tests { assert_ok!(Parachains::dispatch( report_double_vote(report), - Origin::NONE, + Origin::signed(1), )); start_era(2); @@ -2591,7 +2695,7 @@ mod tests { assert_ok!(Parachains::dispatch( report_double_vote(report), - Origin::NONE, + Origin::signed(1), )); start_era(2); diff --git a/runtime/kusama/src/lib.rs b/runtime/kusama/src/lib.rs index 21bbf15c3e66..57cb00e80865 100644 --- a/runtime/kusama/src/lib.rs +++ b/runtime/kusama/src/lib.rs @@ -688,7 +688,8 @@ pub type SignedExtra = ( system::CheckNonce, system::CheckWeight, transaction_payment::ChargeTransactionPayment::, - registrar::LimitParathreadCommits + registrar::LimitParathreadCommits, + parachains::ValidateDoubleVoteReports, ); /// Unchecked extrinsic type as expected by this runtime. pub type UncheckedExtrinsic = generic::UncheckedExtrinsic; diff --git a/runtime/polkadot/src/lib.rs b/runtime/polkadot/src/lib.rs index 1d8abca7245a..041a0e429dec 100644 --- a/runtime/polkadot/src/lib.rs +++ b/runtime/polkadot/src/lib.rs @@ -606,7 +606,8 @@ pub type SignedExtra = ( system::CheckNonce, system::CheckWeight, transaction_payment::ChargeTransactionPayment::, - registrar::LimitParathreadCommits + registrar::LimitParathreadCommits, + parachains::ValidateDoubleVoteReports ); /// Unchecked extrinsic type as expected by this runtime. pub type UncheckedExtrinsic = generic::UncheckedExtrinsic; From 8f1c879b872657e7ed22e47831c6579fde93cf8a Mon Sep 17 00:00:00 2001 From: Fedor Sakharov Date: Thu, 20 Feb 2020 19:18:07 +0300 Subject: [PATCH 04/18] Fixes build after merge --- runtime/common/src/parachains.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/runtime/common/src/parachains.rs b/runtime/common/src/parachains.rs index 7f0c00946078..5302975d59d3 100644 --- a/runtime/common/src/parachains.rs +++ b/runtime/common/src/parachains.rs @@ -2082,7 +2082,6 @@ mod tests { signature: Default::default(), head_data: HeadData(vec![1, 2, 3]), parent_head: HeadData(vec![1, 2, 3]), - egress_queue_roots: vec![], fees: 0, block_data_hash: Default::default(), upward_messages: vec![], From 55ce72282cd4948e35532d0b7fe3b5567caa99d7 Mon Sep 17 00:00:00 2001 From: Fedor Sakharov Date: Fri, 28 Feb 2020 15:09:18 +0300 Subject: [PATCH 05/18] Review fixes --- runtime/common/src/parachains.rs | 344 +++++++++++++++++++++++-------- 1 file changed, 261 insertions(+), 83 deletions(-) diff --git a/runtime/common/src/parachains.rs b/runtime/common/src/parachains.rs index 0825f59495e0..e3c133e8a79c 100644 --- a/runtime/common/src/parachains.rs +++ b/runtime/common/src/parachains.rs @@ -128,40 +128,58 @@ impl DoubleVoteReport { fn verify>( &self, parent_hash: H, - ) -> Result<(), ()> { + ) -> Result<(), DoubleVoteValidityError> { let first = self.first.clone(); let second = self.second.clone(); - Self::verify_vote(first, &parent_hash, self.identity.clone())?; - Self::verify_vote(second, &parent_hash, self.identity.clone())?; + // Check signatures. + Self::verify_vote(&first, &parent_hash, &self.identity)?; + Self::verify_vote(&second, &parent_hash, &self.identity)?; + + match (&first.0, &second.0) { + // If issuing a `Candidate` message on a parachain block, neither a `Valid` or + // `Invalid` vote cannot be issued on that parachain block, as the `Candidate` + // message is an implicit validity vote. + (Statement::Candidate(candidate_hash), Statement::Valid(hash)) | + (Statement::Candidate(candidate_hash), Statement::Invalid(hash)) | + (Statement::Valid(hash), Statement::Candidate(candidate_hash)) | + (Statement::Invalid(hash), Statement::Candidate(candidate_hash)) + if *candidate_hash == *hash => {}, + // Otherwise, it is illegal to cast both a `Valid` and + // `Invalid` vote on a given parachain block. + (Statement::Valid(hash_1), Statement::Invalid(hash_2)) | + (Statement::Invalid(hash_1), Statement::Valid(hash_2)) + if *hash_1 == *hash_2 => {}, + _ => { + return Err(DoubleVoteValidityError::NotDoubleVote); + } + } Ok(()) } fn verify_vote>( - vote: (Statement, ValidityAttestation), + vote: &(Statement, ValidityAttestation), parent_hash: H, - authority: ValidatorId, - ) -> Result<(), ()> { - let (statement, attestation) = vote; - - let (payload, sig) = match attestation { - ValidityAttestation::Implicit(sig) => { - let payload = localized_payload(statement, parent_hash); - - (payload, sig) + authority: &ValidatorId, + ) -> Result<(), DoubleVoteValidityError> { + let sig = match vote { + (Statement::Candidate(_), ValidityAttestation::Implicit(sig)) | + (Statement::Valid(_), ValidityAttestation::Explicit(sig)) | + (Statement::Invalid(_), ValidityAttestation::Explicit(sig)) => { + sig } - ValidityAttestation::Explicit(sig) => { - let payload = localized_payload(statement, parent_hash); - - (payload, sig) + _ => { + return Err(DoubleVoteValidityError::InvalidReport); } }; + let payload = localized_payload(vote.0.clone(), parent_hash); - match sig.verify(&payload[..], &authority) { - true => Ok(()), - false => Err(()), + if !sig.verify(&payload[..], authority) { + return Err(DoubleVoteValidityError::InvalidSignature); } + + Ok(()) } } @@ -432,13 +450,12 @@ decl_module! { let session_index = >::current_index(); let validators = >::validators(); let validator_set_count = validators.len() as u32; - let parent_hash = >::block_hash(>::block_number()); + let _parent_hash = >::block_hash(>::block_number()); let authorities = Self::authorities(); - let offender_idx = match authorities.iter().position(|a| *a == report.identity) { - Some(idx) => idx, - None => return Err("Not in validator set".into()), - }; + let offender_idx = authorities.iter() + .position(|a| *a == report.identity) + .ok_or_else(|| "Not in validator set")?; let offender = match T::FullIdentificationOf::convert(validators[offender_idx].clone()) { Some(id) => (validators[offender_idx].clone(), id), @@ -447,11 +464,6 @@ decl_module! { let offence = DoubleVoteOffence { session_index, validator_set_count, offender }; - ensure!( - report.verify(parent_hash).is_ok(), - "Report verification failed.", - ); - // Checks if this is actually a double vote are // implemented in `ValidateDoubleVoteReports::validete`. T::ReportDoubleVote::report_offence(vec![reporter], offence); @@ -1021,11 +1033,9 @@ pub fn ensure_parachain(o: OuterOrigin) -> result::Result(rstd::marker::PhantomData) where - ::Call: IsSubType, T>; +pub struct ValidateDoubleVoteReports(rstd::marker::PhantomData); -impl rstd::fmt::Debug for ValidateDoubleVoteReports where - ::Call: IsSubType, T> +impl rstd::fmt::Debug for ValidateDoubleVoteReports where { fn fmt(&self, f: &mut rstd::fmt::Formatter) -> rstd::fmt::Result { write!(f, "ValidateDoubleVoteReports") @@ -1033,6 +1043,7 @@ impl rstd::fmt::Debug for ValidateDoubleVoteReports w } /// Custom validity error used while validating double vote reports. +#[derive(RuntimeDebug)] #[repr(u8)] pub enum DoubleVoteValidityError { /// The authority being reported is not in the authority set. @@ -1046,6 +1057,9 @@ pub enum DoubleVoteValidityError { /// The two statements in the report are not conflicting. NotDoubleVote = 3, + + /// Invalid report. Indicates that statement doesn't match the attestation on one of the votes. + InvalidReport = 4, } impl SignedExtension for ValidateDoubleVoteReports where @@ -1075,52 +1089,26 @@ impl SignedExtension for ValidateDoubleVoteReports wh if let Some(local_call) = call.is_sub_type() { if let Call::report_double_vote(report) = local_call { - let e = TransactionValidityError::from( - InvalidTransaction::Custom(DoubleVoteValidityError::NotAnAuthority as u8) - ); - let validators = >::validators(); let parent_hash = >::block_hash(>::block_number()); let authorities = Module::::authorities(); let offender_idx = match authorities.iter().position(|a| *a == report.identity) { Some(idx) => idx, - None => return Err(e), + None => return Err(InvalidTransaction::Custom( + DoubleVoteValidityError::NotAnAuthority as u8).into() + ), }; - let e = TransactionValidityError::from( - InvalidTransaction::Custom(DoubleVoteValidityError::FailedToConvertId as u8) - ); if T::FullIdentificationOf::convert(validators[offender_idx].clone()).is_none() { - return Err(e); + return Err(InvalidTransaction::Custom( + DoubleVoteValidityError::FailedToConvertId as u8).into() + ); } - let e = TransactionValidityError::from( - InvalidTransaction::Custom(DoubleVoteValidityError::InvalidSignature as u8) - ); - report.verify(parent_hash).map_err(|_| e)?; - - let e = TransactionValidityError::from( - InvalidTransaction::Custom(DoubleVoteValidityError::NotDoubleVote as u8) - ); - match (&report.first.0, &report.second.0) { - // If issuing a `Candidate` message on a parachain block, neither a `Valid` or - // `Invalid` vote cannot be issued on that parachain block, as the `Candidate` - // message is an implicit validity vote. - (Statement::Candidate(candidate_hash), Statement::Valid(hash)) | - (Statement::Candidate(candidate_hash), Statement::Invalid(hash)) | - (Statement::Valid(hash), Statement::Candidate(candidate_hash)) | - (Statement::Invalid(hash), Statement::Candidate(candidate_hash)) - if *candidate_hash == *hash => {}, - // Otherwise, it is illegal to cast both a `Valid` and - // `Invalid` vote on a given parachain block. - (Statement::Valid(hash_1), Statement::Invalid(hash_2)) | - (Statement::Invalid(hash_1), Statement::Valid(hash_2)) - if *hash_1 == *hash_2 => {}, - _ => { - return Err(e); - } - } + report + .verify(parent_hash) + .map_err(|e| TransactionValidityError::from(InvalidTransaction::Custom(e as u8)))?; } } @@ -1465,8 +1453,14 @@ mod tests { fn report_double_vote( report: DoubleVoteReport, - ) -> ParachainsCall { - ParachainsCall::report_double_vote(report) + ) -> Result, TransactionValidityError> { + let inner = ParachainsCall::report_double_vote(report); + let call = Call::Parachains(inner.clone()); + + ValidateDoubleVoteReports::(rstd::marker::PhantomData) + .validate(&0, &call, DispatchInfo::default(), 0)?; + + Ok(inner) } // creates a template candidate which pins to correct relay-chain state. @@ -2061,7 +2055,7 @@ mod tests { } #[test] - fn double_vote_works() { + fn double_vote_candidate_and_valid_works() { let parachains = vec![ (1u32.into(), vec![], vec![]), ]; @@ -2072,7 +2066,6 @@ mod tests { Sr25519Keyring::from_raw_public(raw_public).unwrap() }; - // Test that a Candidate and Valid statements on the same candidate get slashed. new_test_ext(parachains.clone()).execute_with(|| { let candidate = raw_candidate(1.into()).abridge().0; @@ -2122,10 +2115,9 @@ mod tests { ); } - assert_ok!(Parachains::dispatch( - report_double_vote(report), - Origin::signed(1), - )); + let inner = report_double_vote(report).unwrap(); + + assert_ok!(Parachains::dispatch(inner, Origin::signed(1))); start_era(2); @@ -2157,11 +2149,22 @@ mod tests { ); } }); + } + + #[test] + fn double_vote_candidate_and_invalid_works() { + let parachains = vec![ + (1u32.into(), vec![], vec![]), + ]; + + let extract_key = |public: ValidatorId| { + let mut raw_public = [0; 32]; + raw_public.copy_from_slice(public.as_ref()); + Sr25519Keyring::from_raw_public(raw_public).unwrap() + }; // Test that a Candidate and Invalid statements on the same candidate get slashed. new_test_ext(parachains.clone()).execute_with(|| { - run_to_block(2); - let candidate = raw_candidate(1.into()).abridge().0; let candidate_hash = candidate.hash(); @@ -2207,7 +2210,7 @@ mod tests { } assert_ok!(Parachains::dispatch( - report_double_vote(report), + report_double_vote(report).unwrap(), Origin::signed(1), )); @@ -2242,12 +2245,22 @@ mod tests { } }); + } + #[test] + fn double_vote_valid_and_invalid_works() { + let parachains = vec![ + (1u32.into(), vec![], vec![]), + ]; + + let extract_key = |public: ValidatorId| { + let mut raw_public = [0; 32]; + raw_public.copy_from_slice(public.as_ref()); + Sr25519Keyring::from_raw_public(raw_public).unwrap() + }; // Test that an Invalid and Valid statements on the same candidate get slashed. new_test_ext(parachains.clone()).execute_with(|| { - run_to_block(2); - let candidate = raw_candidate(1.into()).abridge().0; let candidate_hash = candidate.hash(); @@ -2293,10 +2306,115 @@ mod tests { } assert_ok!(Parachains::dispatch( - report_double_vote(report), + report_double_vote(report).unwrap(), + Origin::signed(1), + )); + + start_era(2); + + // Check that the balance of 0-th validator is slashed 100%. + assert_eq!(Balances::total_balance(&0), 10_000_000 - 10_000); + assert_eq!(Staking::slashable_balance_of(&0), 0); + + assert_eq!( + Staking::stakers(0), + staking::Exposure { + total: 0, + own: 0, + others: vec![], + }, + ); + + // Check that the balances of all other validators are left intact. + for i in 1..authorities.len() { + assert_eq!(Balances::total_balance(&(i as u64)), 10_000_000); + assert_eq!(Staking::slashable_balance_of(&(i as u64)), 10_000); + + assert_eq!( + Staking::stakers(i as u64), + staking::Exposure { + total: 10_000, + own: 10_000, + others: vec![], + }, + ); + } + }); + } + + // Check that submitting the same report twice errors. + #[test] + fn double_vote_submit_twice_works() { + let parachains = vec![ + (1u32.into(), vec![], vec![]), + ]; + + let extract_key = |public: ValidatorId| { + let mut raw_public = [0; 32]; + raw_public.copy_from_slice(public.as_ref()); + Sr25519Keyring::from_raw_public(raw_public).unwrap() + }; + + // Test that a Candidate and Valid statements on the same candidate get slashed. + new_test_ext(parachains.clone()).execute_with(|| { + let candidate = raw_candidate(1.into()).abridge().0; + let candidate_hash = candidate.hash(); + + assert_eq!(Staking::current_era(), 0); + assert_eq!(Session::current_index(), 0); + + start_era(1); + + let authorities = Parachains::authorities(); + let authority_index = 0; + let key = extract_key(authorities[authority_index].clone()); + + let statement_candidate = Statement::Candidate(candidate_hash.clone()); + let statement_valid = Statement::Valid(candidate_hash.clone()); + let block_number = >::block_number(); + let parent_hash = >::block_hash(block_number); + + let payload_1 = localized_payload(statement_candidate.clone(), parent_hash); + let payload_2 = localized_payload(statement_valid.clone(), parent_hash); + + let signature_1 = key.sign(&payload_1[..]).into(); + let signature_2 = key.sign(&payload_2[..]).into(); + + let attestation_1 = ValidityAttestation::Implicit(signature_1); + let attestation_2 = ValidityAttestation::Explicit(signature_2); + + let report = DoubleVoteReport { + identity: ValidatorId::from(key.public()), + first: (statement_candidate, attestation_1), + second: (statement_valid, attestation_2), + }; + + // Check that in the beginning the genesis balances are there. + for i in 0..authorities.len() { + assert_eq!(Balances::total_balance(&(i as u64)), 10_000_000); + assert_eq!(Staking::slashable_balance_of(&(i as u64)), 10_000); + + assert_eq!( + Staking::stakers(i as u64), + staking::Exposure { + total: 10_000, + own: 10_000, + others: vec![], + }, + ); + } + + assert_ok!(Parachains::dispatch( + report_double_vote(report.clone()).unwrap(), Origin::signed(1), )); + assert!(Parachains::dispatch( + report_double_vote(report).unwrap(), + Origin::signed(1), + ).is_err() + ); + start_era(2); // Check that the balance of 0-th validator is slashed 100%. @@ -2328,4 +2446,64 @@ mod tests { } }); } + + // Check that submitting invalid reports fail. + #[test] + fn double_vote_submit_invalid_works() { + let parachains = vec![ + (1u32.into(), vec![], vec![]), + ]; + + let extract_key = |public: ValidatorId| { + let mut raw_public = [0; 32]; + raw_public.copy_from_slice(public.as_ref()); + Sr25519Keyring::from_raw_public(raw_public).unwrap() + }; + + // Test that a Candidate and Valid statements on the same candidate get slashed. + new_test_ext(parachains.clone()).execute_with(|| { + let candidate = raw_candidate(1.into()).abridge().0; + let candidate_hash = candidate.hash(); + + assert_eq!(Staking::current_era(), 0); + assert_eq!(Session::current_index(), 0); + + start_era(1); + + let authorities = Parachains::authorities(); + let authority_1_index = 0; + let authority_2_index = 1; + let key_1 = extract_key(authorities[authority_1_index].clone()); + let key_2 = extract_key(authorities[authority_2_index].clone()); + + let statement_candidate = Statement::Candidate(candidate_hash.clone()); + let statement_valid = Statement::Valid(candidate_hash.clone()); + let block_number = >::block_number(); + let parent_hash = >::block_hash(block_number); + + let payload_1 = localized_payload(statement_candidate.clone(), parent_hash); + let payload_2 = localized_payload(statement_valid.clone(), parent_hash); + + let signature_1 = key_1.sign(&payload_1[..]).into(); + let signature_2 = key_2.sign(&payload_2[..]).into(); + + let attestation_1 = ValidityAttestation::Implicit(signature_1); + let attestation_2 = ValidityAttestation::Explicit(signature_2); + + let report = DoubleVoteReport { + identity: ValidatorId::from(key_1.public()), + first: (statement_candidate, attestation_1), + second: (statement_valid, attestation_2), + }; + + assert_eq!( + report_double_vote(report.clone()), + Err(TransactionValidityError::Invalid( + InvalidTransaction::Custom(DoubleVoteValidityError::InvalidSignature as u8) + ) + ), + ); + }); + } + } From 9b06b3aeafaef7e739c707017a5e48d73551c825 Mon Sep 17 00:00:00 2001 From: Fedor Sakharov Date: Wed, 4 Mar 2020 17:44:31 +0300 Subject: [PATCH 06/18] Adds historical session proofs --- runtime/common/src/parachains.rs | 193 +++++++++++++++++++++++++------ runtime/common/src/registrar.rs | 2 +- runtime/kusama/Cargo.toml | 2 +- runtime/kusama/src/lib.rs | 8 +- runtime/polkadot/Cargo.toml | 2 +- runtime/polkadot/src/lib.rs | 8 +- 6 files changed, 175 insertions(+), 40 deletions(-) diff --git a/runtime/common/src/parachains.rs b/runtime/common/src/parachains.rs index e3c133e8a79c..ea168a8d1a67 100644 --- a/runtime/common/src/parachains.rs +++ b/runtime/common/src/parachains.rs @@ -20,9 +20,11 @@ use rstd::prelude::*; use rstd::result; use codec::{Decode, Encode}; -use session::historical::IdentificationTuple; +use session::historical::{IdentificationTuple, Proof}; use sp_runtime::{ Perbill, RuntimeDebug, + // TODO: This should be changed to a proper key type. + key_types::DUMMY, traits::{ Hash as HashT, BlakeTwo256, Saturating, One, Dispatchable, AccountIdConversion, BadOrigin, Convert, SignedExtension, AppVerify, @@ -34,6 +36,7 @@ use sp_staking::{ offence::{ReportOffence, Offence, Kind}, }; use frame_support::{ + traits::KeyOwnerProofSystem, dispatch::{IsSubType}, weights::{DispatchInfo, SimpleDispatchInfo}, }; @@ -217,12 +220,7 @@ pub trait Trait: attestations::Trait + session::historical::Trait { type MaxHeadDataSize: Get; /// Submit double-vote offence reports. - type ReportDoubleVote: - ReportOffence< - Self::AccountId, - IdentificationTuple, - DoubleVoteOffence>, - >; + type HandleDoubleVote: HandleDoubleVote; } /// Origin for the parachains module. @@ -271,6 +269,107 @@ impl Offence for DoubleVoteOffence { } } +impl ParachainsOffence for DoubleVoteOffence { + fn new( + session_index: SessionIndex, + validator_set_count: u32, + offender: O, + ) -> Self { + Self { + session_index, + validator_set_count, + offender, + } + } +} + +pub trait ParachainsOffence: Offence { + fn new( + session_index: SessionIndex, + validator_set_count: u32, + offender: O, + ) -> Self; +} + +pub trait HandleDoubleVote { + /// The proof of key ownership. + type Offence: ParachainsOffence>; + + fn check_report( + report: &DoubleVoteReport, + key_owner_proof: Proof, + i: Vec + ) -> Option>; + + fn report_offence( + reporters: Vec, + offence: Self::Offence + ); +} + +impl HandleDoubleVote for () { + type Offence = DoubleVoteOffence>; + + fn check_report( + _report: &DoubleVoteReport, + _key_owner_proof: Proof, + _i: Vec, + ) -> Option> { + None + } + + // TODO: As soon as substrate is updated and `report_offence` returns + // a result, this function should also be changed to returning a result. + fn report_offence( + _reporters: Vec, + _offence: Self::Offence, + ) {} +} + +pub struct DoubleVoteHandler { + _phantom: rstd::marker::PhantomData<(P, R, O)>, +} + +impl Default for DoubleVoteHandler { + fn default() -> Self { + Self { + _phantom: Default::default(), + } + } +} + +impl HandleDoubleVote for DoubleVoteHandler +where + T: Trait, + P: KeyOwnerProofSystem< + // AccountId? + (sp_core::crypto::KeyTypeId, Vec), + Proof = Proof, + IdentificationTuple = session::historical::IdentificationTuple, + >, + O: ParachainsOffence, + R: ReportOffence, +{ + type Offence = O; + + fn check_report( + _report: &DoubleVoteReport, + key_owner_proof: Proof, + i: Vec, + ) -> Option> { + let id = P::check_proof((DUMMY, i), key_owner_proof)?; + + Some(id) + } + + fn report_offence( + reporters: Vec, + offence: O, + ) { + R::report_offence(reporters, offence); + } +} + /// Total number of individual messages allowed in the parachain -> relay-chain message queue. const MAX_QUEUE_COUNT: usize = 100; /// Total size of messages allowed in the parachain -> relay-chain message queue before which no @@ -441,32 +540,34 @@ decl_module! { pub fn report_double_vote( origin, report: DoubleVoteReport, + key_owner_proof: Vec, + id: Vec, ) -> DispatchResult { let reporter = ensure_signed(origin)?; - // The following code duplicates the logic in `ValidateDoubeVoteReports::validate` - // since we still need to do all these steps to derermine which one of the offenders - // gets slashed. - let session_index = >::current_index(); let validators = >::validators(); let validator_set_count = validators.len() as u32; - let _parent_hash = >::block_hash(>::block_number()); - let authorities = Self::authorities(); - let offender_idx = authorities.iter() - .position(|a| *a == report.identity) - .ok_or_else(|| "Not in validator set")?; + let key_owner_proof: Proof = Decode::decode(&mut &key_owner_proof[..]) + .map_err(|_| "Key owner proof decoding failed.")?; - let offender = match T::FullIdentificationOf::convert(validators[offender_idx].clone()) { - Some(id) => (validators[offender_idx].clone(), id), - None => return Err("Failed to convert".into()), - }; + let session = Default::default(); // TODO: this has to be key_owner_proof.session(); - let offence = DoubleVoteOffence { session_index, validator_set_count, offender }; + let id = T::HandleDoubleVote::check_report( + &report, + key_owner_proof, + id, + ).ok_or("Invalid/outdated key ownership proof.")?; + + let offence = >::Offence::new( + session, + validator_set_count, + id, + ); // Checks if this is actually a double vote are // implemented in `ValidateDoubleVoteReports::validete`. - T::ReportDoubleVote::report_offence(vec![reporter], offence); + T::HandleDoubleVote::report_offence(vec![reporter], offence); Ok(()) } @@ -1088,7 +1189,7 @@ impl SignedExtension for ValidateDoubleVoteReports wh let r = ValidTransaction::default(); if let Some(local_call) = call.is_sub_type() { - if let Call::report_double_vote(report) = local_call { + if let Call::report_double_vote(report, _proof, _id) = local_call { let validators = >::validators(); let parent_hash = >::block_hash(>::block_number()); @@ -1206,7 +1307,7 @@ mod tests { type ValidatorId = u64; type ValidatorIdOf = staking::StashOf; type ShouldEndSession = session::PeriodicSessions; - type SessionManager = (); + type SessionManager = session::historical::NoteHistoricalRoot; type SessionHandler = session::TestSessionHandler; type Keys = UintAuthorityId; type DisabledValidatorsThreshold = DisabledValidatorsThreshold; @@ -1362,7 +1463,11 @@ mod tests { type Registrar = registrar::Module; type MaxCodeSize = MaxCodeSize; type MaxHeadDataSize = MaxHeadDataSize; - type ReportDoubleVote = Offences; + type HandleDoubleVote = DoubleVoteHandler< + Historical, + Offences, + DoubleVoteOffence>, + >; } type Parachains = Module; @@ -1374,6 +1479,7 @@ mod tests { type Timestamp = timestamp::Module; type RandomnessCollectiveFlip = randomness_collective_flip::Module; type Registrar = registrar::Module; + type Historical = session::historical::Module; fn new_test_ext(parachains: Vec<(ParaId, Vec, Vec)>) -> TestExternalities { use staking::StakerStatus; @@ -1453,8 +1559,10 @@ mod tests { fn report_double_vote( report: DoubleVoteReport, + proof: Vec, + id: Vec, ) -> Result, TransactionValidityError> { - let inner = ParachainsCall::report_double_vote(report); + let inner = ParachainsCall::report_double_vote(report, proof, id); let call = Call::Parachains(inner.clone()); ValidateDoubleVoteReports::(rstd::marker::PhantomData) @@ -1566,12 +1674,12 @@ mod tests { Session::on_initialize(System::block_number()); } - assert_eq!(Session::current_index(), session_index); + //assert_eq!(Session::current_index(), session_index); } fn start_era(era_index: EraIndex) { start_session((era_index * 3).into()); - assert_eq!(Staking::current_era(), era_index); + //assert_eq!(Staking::current_era(), era_index); } fn init_block() { @@ -2115,7 +2223,10 @@ mod tests { ); } - let inner = report_double_vote(report).unwrap(); + let encoded_key = (authority_index as u64).encode(); + let proof = Historical::prove((DUMMY, &encoded_key[..])).unwrap(); + + let inner = report_double_vote(report, proof.encode(), encoded_key).unwrap(); assert_ok!(Parachains::dispatch(inner, Origin::signed(1))); @@ -2168,7 +2279,7 @@ mod tests { let candidate = raw_candidate(1.into()).abridge().0; let candidate_hash = candidate.hash(); - start_era(1); + start_session(1); let authorities = Parachains::authorities(); let authority_index = 0; @@ -2209,8 +2320,11 @@ mod tests { ); } + let encoded_key = (authority_index as u64).encode(); + let proof = Historical::prove((DUMMY, &encoded_key[..])).unwrap(); + assert_ok!(Parachains::dispatch( - report_double_vote(report).unwrap(), + report_double_vote(report, proof.encode(), encoded_key).unwrap(), Origin::signed(1), )); @@ -2305,8 +2419,11 @@ mod tests { ); } + let encoded_key = (authority_index as u64).encode(); + let proof = Historical::prove((DUMMY, &encoded_key[..])).unwrap(); + assert_ok!(Parachains::dispatch( - report_double_vote(report).unwrap(), + report_double_vote(report, proof.encode(), encoded_key).unwrap(), Origin::signed(1), )); @@ -2404,13 +2521,16 @@ mod tests { ); } + let encoded_key = (authority_index as u64).encode(); + let proof = Historical::prove((DUMMY, &encoded_key[..])).unwrap(); + assert_ok!(Parachains::dispatch( - report_double_vote(report.clone()).unwrap(), + report_double_vote(report.clone(), proof.encode(), encoded_key.clone()).unwrap(), Origin::signed(1), )); assert!(Parachains::dispatch( - report_double_vote(report).unwrap(), + report_double_vote(report, proof.encode(), encoded_key).unwrap(), Origin::signed(1), ).is_err() ); @@ -2496,8 +2616,11 @@ mod tests { second: (statement_valid, attestation_2), }; + let encoded_key = (authority_1_index as u64).encode(); + let proof = Historical::prove((DUMMY, &encoded_key[..])).unwrap(); + assert_eq!( - report_double_vote(report.clone()), + report_double_vote(report.clone(), proof.encode(), encoded_key.clone()), Err(TransactionValidityError::Invalid( InvalidTransaction::Custom(DoubleVoteValidityError::InvalidSignature as u8) ) diff --git a/runtime/common/src/registrar.rs b/runtime/common/src/registrar.rs index acb00888473f..eb70a35ce118 100644 --- a/runtime/common/src/registrar.rs +++ b/runtime/common/src/registrar.rs @@ -818,7 +818,7 @@ mod tests { type Randomness = RandomnessCollectiveFlip; type MaxCodeSize = MaxCodeSize; type MaxHeadDataSize = MaxHeadDataSize; - type ReportDoubleVote = (); + type HandleDoubleVote = (); } parameter_types! { diff --git a/runtime/kusama/Cargo.toml b/runtime/kusama/Cargo.toml index 80e74e5efc10..b37d8c6fabf7 100644 --- a/runtime/kusama/Cargo.toml +++ b/runtime/kusama/Cargo.toml @@ -48,7 +48,7 @@ nicks = { package = "pallet-nicks", git = "https://github.com/paritytech/substra offences = { package = "pallet-offences", git = "https://github.com/paritytech/substrate", branch = "polkadot-master", default-features = false } randomness-collective-flip = { package = "pallet-randomness-collective-flip", git = "https://github.com/paritytech/substrate", branch = "polkadot-master", default-features = false } recovery = { package = "pallet-recovery", git = "https://github.com/paritytech/substrate", branch = "polkadot-master", default-features = false } -session = { package = "pallet-session", git = "https://github.com/paritytech/substrate", branch = "polkadot-master", default-features = false } +session = { package = "pallet-session", features = [ "historical" ], git = "https://github.com/paritytech/substrate", branch = "polkadot-master", default-features = false } society = { package = "pallet-society", git = "https://github.com/paritytech/substrate", branch = "polkadot-master", default-features = false } frame-support = { git = "https://github.com/paritytech/substrate", branch = "polkadot-master", default-features = false } staking = { package = "pallet-staking", git = "https://github.com/paritytech/substrate", branch = "polkadot-master", default-features = false } diff --git a/runtime/kusama/src/lib.rs b/runtime/kusama/src/lib.rs index adc6030ed0c0..e86258d63e26 100644 --- a/runtime/kusama/src/lib.rs +++ b/runtime/kusama/src/lib.rs @@ -53,6 +53,7 @@ use im_online::sr25519::AuthorityId as ImOnlineId; use authority_discovery_primitives::AuthorityId as AuthorityDiscoveryId; use system::offchain::TransactionSubmitter; use pallet_transaction_payment_rpc_runtime_api::RuntimeDispatchInfo; +use session::{historical as session_historical}; #[cfg(feature = "std")] pub use staking::StakerStatus; @@ -473,7 +474,11 @@ impl parachains::Trait for Runtime { type Registrar = Registrar; type MaxCodeSize = MaxCodeSize; type MaxHeadDataSize = MaxHeadDataSize; - type ReportDoubleVote = Offences; + type HandleDoubleVote = parachains::DoubleVoteHandler< + Historical, + Offences, + parachains::DoubleVoteOffence>, + >; } parameter_types! { @@ -626,6 +631,7 @@ construct_runtime! { Authorship: authorship::{Module, Call, Storage}, Staking: staking::{Module, Call, Storage, Config, Event}, Offences: offences::{Module, Call, Storage, Event}, + Historical: session_historical::{Module}, Session: session::{Module, Call, Storage, Event, Config}, FinalityTracker: finality_tracker::{Module, Call, Inherent}, Grandpa: grandpa::{Module, Call, Storage, Config, Event}, diff --git a/runtime/polkadot/Cargo.toml b/runtime/polkadot/Cargo.toml index 1718d5dca255..48c939f02df1 100644 --- a/runtime/polkadot/Cargo.toml +++ b/runtime/polkadot/Cargo.toml @@ -46,7 +46,7 @@ membership = { package = "pallet-membership", git = "https://github.com/parityte nicks = { package = "pallet-nicks", git = "https://github.com/paritytech/substrate", branch = "polkadot-master", default-features = false } offences = { package = "pallet-offences", git = "https://github.com/paritytech/substrate", branch = "polkadot-master", default-features = false } randomness-collective-flip = { package = "pallet-randomness-collective-flip", git = "https://github.com/paritytech/substrate", branch = "polkadot-master", default-features = false } -session = { package = "pallet-session", git = "https://github.com/paritytech/substrate", branch = "polkadot-master", default-features = false } +session = { package = "pallet-session", features = [ "historical" ], git = "https://github.com/paritytech/substrate", branch = "polkadot-master", default-features = false } frame-support = { git = "https://github.com/paritytech/substrate", branch = "polkadot-master", default-features = false } staking = { package = "pallet-staking", git = "https://github.com/paritytech/substrate", branch = "polkadot-master", default-features = false } pallet-staking-reward-curve = { git = "https://github.com/paritytech/substrate", branch = "polkadot-master" } diff --git a/runtime/polkadot/src/lib.rs b/runtime/polkadot/src/lib.rs index 82e38af15b42..7d3da16c1664 100644 --- a/runtime/polkadot/src/lib.rs +++ b/runtime/polkadot/src/lib.rs @@ -57,6 +57,7 @@ use im_online::sr25519::AuthorityId as ImOnlineId; use authority_discovery_primitives::AuthorityId as AuthorityDiscoveryId; use system::offchain::TransactionSubmitter; use pallet_transaction_payment_rpc_runtime_api::RuntimeDispatchInfo; +use session::{historical as session_historical}; #[cfg(feature = "std")] pub use staking::StakerStatus; @@ -481,7 +482,11 @@ impl parachains::Trait for Runtime { type Registrar = Registrar; type MaxCodeSize = MaxCodeSize; type MaxHeadDataSize = MaxHeadDataSize; - type ReportDoubleVote = Offences; + type HandleDoubleVote = parachains::DoubleVoteHandler< + Historical, + Offences, + parachains::DoubleVoteOffence>, + >; } parameter_types! { @@ -557,6 +562,7 @@ construct_runtime! { Authorship: authorship::{Module, Call, Storage}, Staking: staking::{Module, Call, Storage, Config, Event}, Offences: offences::{Module, Call, Storage, Event}, + Historical: session_historical::{Module}, Session: session::{Module, Call, Storage, Event, Config}, FinalityTracker: finality_tracker::{Module, Call, Inherent}, Grandpa: grandpa::{Module, Call, Storage, Config, Event}, From a91ce09aa833a316ba52ee82752aa0f32b5c7155 Mon Sep 17 00:00:00 2001 From: Fedor Sakharov Date: Fri, 6 Mar 2020 14:17:34 +0300 Subject: [PATCH 07/18] Review fixes. --- runtime/common/src/parachains.rs | 320 +++++++++++++------------------ runtime/common/src/registrar.rs | 8 +- runtime/kusama/src/lib.rs | 13 +- runtime/polkadot/src/lib.rs | 13 +- 4 files changed, 155 insertions(+), 199 deletions(-) diff --git a/runtime/common/src/parachains.rs b/runtime/common/src/parachains.rs index ea168a8d1a67..03901426af43 100644 --- a/runtime/common/src/parachains.rs +++ b/runtime/common/src/parachains.rs @@ -20,11 +20,8 @@ use rstd::prelude::*; use rstd::result; use codec::{Decode, Encode}; -use session::historical::{IdentificationTuple, Proof}; use sp_runtime::{ - Perbill, RuntimeDebug, - // TODO: This should be changed to a proper key type. - key_types::DUMMY, + KeyTypeId, Perbill, RuntimeDebug, traits::{ Hash as HashT, BlakeTwo256, Saturating, One, Dispatchable, AccountIdConversion, BadOrigin, Convert, SignedExtension, AppVerify, @@ -46,7 +43,7 @@ use primitives::{ self, Id as ParaId, Chain, DutyRoster, AttestedCandidate, Statement, ParachainDispatchOrigin, UpwardMessage, ValidatorId, ActiveParas, CollatorId, Retriable, OmittedValidationData, CandidateReceipt, GlobalValidationSchedule, AbridgedCandidateReceipt, - LocalValidationData, ValidityAttestation, NEW_HEADS_IDENTIFIER, + LocalValidationData, ValidityAttestation, NEW_HEADS_IDENTIFIER, PARACHAIN_KEY_TYPE_ID, }, }; use frame_support::{ @@ -118,22 +115,28 @@ pub struct ValidatorIdentities(rstd::marker::PhantomData); #[derive(RuntimeDebug, Encode, Decode)] #[derive(Clone, Eq, PartialEq)] -pub struct DoubleVoteReport { +pub struct DoubleVoteReport { /// Identity of the double-voter. pub identity: ValidatorId, /// First vote of the double-vote. pub first: (Statement, ValidityAttestation), /// Second vote of the double-vote. pub second: (Statement, ValidityAttestation), + /// Proof + pub proof: Proof, } -impl DoubleVoteReport { - fn verify>( +impl DoubleVoteReport { + fn verify, H: AsRef<[u8]>>( &self, parent_hash: H, ) -> Result<(), DoubleVoteValidityError> { let first = self.first.clone(); let second = self.second.clone(); + let id = self.identity.encode(); + + T::KeyOwnerProofSystem::check_proof((PARACHAIN_KEY_TYPE_ID, id), self.proof.clone()) + .ok_or(DoubleVoteValidityError::InvalidProof)?; // Check signatures. Self::verify_vote(&first, &parent_hash, &self.identity)?; @@ -220,7 +223,29 @@ pub trait Trait: attestations::Trait + session::historical::Trait { type MaxHeadDataSize: Get; /// Submit double-vote offence reports. - type HandleDoubleVote: HandleDoubleVote; + /// type HandleDoubleVote: HandleDoubleVote; + + /// Proof type (since it is a part of Calls it needs to be bound to `Parameter` here) + /// TODO: I know of no other way to bind an associated type of `KeyOwnerProofSystem::Proof` + /// to parameter. + type Proof: Parameter; + + /// Compute and check proofs of historical key owners. + type KeyOwnerProofSystem: KeyOwnerProofSystem< + (KeyTypeId, Vec), + Proof = Self::Proof, + IdentificationTuple = Self::IdentificationTuple, + >; + + /// An identification tuple type bound to `Clone`. + type IdentificationTuple: Parameter; + + /// Report an offence. + type ReportOffence: ReportOffence< + Self::AccountId, + Self::IdentificationTuple, + DoubleVoteOffence + >; } /// Origin for the parachains module. @@ -269,107 +294,6 @@ impl Offence for DoubleVoteOffence { } } -impl ParachainsOffence for DoubleVoteOffence { - fn new( - session_index: SessionIndex, - validator_set_count: u32, - offender: O, - ) -> Self { - Self { - session_index, - validator_set_count, - offender, - } - } -} - -pub trait ParachainsOffence: Offence { - fn new( - session_index: SessionIndex, - validator_set_count: u32, - offender: O, - ) -> Self; -} - -pub trait HandleDoubleVote { - /// The proof of key ownership. - type Offence: ParachainsOffence>; - - fn check_report( - report: &DoubleVoteReport, - key_owner_proof: Proof, - i: Vec - ) -> Option>; - - fn report_offence( - reporters: Vec, - offence: Self::Offence - ); -} - -impl HandleDoubleVote for () { - type Offence = DoubleVoteOffence>; - - fn check_report( - _report: &DoubleVoteReport, - _key_owner_proof: Proof, - _i: Vec, - ) -> Option> { - None - } - - // TODO: As soon as substrate is updated and `report_offence` returns - // a result, this function should also be changed to returning a result. - fn report_offence( - _reporters: Vec, - _offence: Self::Offence, - ) {} -} - -pub struct DoubleVoteHandler { - _phantom: rstd::marker::PhantomData<(P, R, O)>, -} - -impl Default for DoubleVoteHandler { - fn default() -> Self { - Self { - _phantom: Default::default(), - } - } -} - -impl HandleDoubleVote for DoubleVoteHandler -where - T: Trait, - P: KeyOwnerProofSystem< - // AccountId? - (sp_core::crypto::KeyTypeId, Vec), - Proof = Proof, - IdentificationTuple = session::historical::IdentificationTuple, - >, - O: ParachainsOffence, - R: ReportOffence, -{ - type Offence = O; - - fn check_report( - _report: &DoubleVoteReport, - key_owner_proof: Proof, - i: Vec, - ) -> Option> { - let id = P::check_proof((DUMMY, i), key_owner_proof)?; - - Some(id) - } - - fn report_offence( - reporters: Vec, - offence: O, - ) { - R::report_offence(reporters, offence); - } -} - /// Total number of individual messages allowed in the parachain -> relay-chain message queue. const MAX_QUEUE_COUNT: usize = 100; /// Total size of messages allowed in the parachain -> relay-chain message queue before which no @@ -539,35 +463,34 @@ decl_module! { #[weight = SimpleDispatchInfo::FixedNormal(0)] pub fn report_double_vote( origin, - report: DoubleVoteReport, - key_owner_proof: Vec, - id: Vec, + report: DoubleVoteReport< + )>>::Proof + >, ) -> DispatchResult { let reporter = ensure_signed(origin)?; let validators = >::validators(); let validator_set_count = validators.len() as u32; - let key_owner_proof: Proof = Decode::decode(&mut &key_owner_proof[..]) - .map_err(|_| "Key owner proof decoding failed.")?; + //let key_owner_proof = report.proof.clone();// Proof = Decode::decode(&mut &key_owner_proof[..]) + //.map_err(|_| "Key owner proof decoding failed.")?; - let session = Default::default(); // TODO: this has to be key_owner_proof.session(); + let session_index = Default::default(); // TODO: this has to be key_owner_proof.session(); - let id = T::HandleDoubleVote::check_report( - &report, - key_owner_proof, - id, + let offender = T::KeyOwnerProofSystem::check_proof( + (PARACHAIN_KEY_TYPE_ID, report.identity.encode()), + report.proof.clone(), ).ok_or("Invalid/outdated key ownership proof.")?; - let offence = >::Offence::new( - session, + let offence = DoubleVoteOffence { + session_index, validator_set_count, - id, - ); + offender, + }; // Checks if this is actually a double vote are // implemented in `ValidateDoubleVoteReports::validete`. - T::HandleDoubleVote::report_offence(vec![reporter], offence); + T::ReportOffence::report_offence(vec![reporter], offence); Ok(()) } @@ -1161,6 +1084,9 @@ pub enum DoubleVoteValidityError { /// Invalid report. Indicates that statement doesn't match the attestation on one of the votes. InvalidReport = 4, + + /// The proof provided in the report is not valid. + InvalidProof = 5, } impl SignedExtension for ValidateDoubleVoteReports where @@ -1189,7 +1115,7 @@ impl SignedExtension for ValidateDoubleVoteReports wh let r = ValidTransaction::default(); if let Some(local_call) = call.is_sub_type() { - if let Call::report_double_vote(report, _proof, _id) = local_call { + if let Call::report_double_vote(report) = local_call { let validators = >::validators(); let parent_hash = >::block_hash(>::block_number()); @@ -1208,7 +1134,7 @@ impl SignedExtension for ValidateDoubleVoteReports wh } report - .verify(parent_hash) + .verify::(parent_hash) .map_err(|e| TransactionValidityError::from(InvalidTransaction::Custom(e as u8)))?; } } @@ -1227,8 +1153,12 @@ mod tests { use sp_core::{H256, Blake2Hasher}; use sp_trie::NodeCodec; use sp_runtime::{ - Perbill, curve::PiecewiseLinear, testing::{UintAuthorityId, Header}, - traits::{BlakeTwo256, IdentityLookup, OnInitialize, OnFinalize, SaturatedConversion}, + impl_opaque_keys, + Perbill, curve::PiecewiseLinear, testing::{Header}, + traits::{ + BlakeTwo256, IdentityLookup, OnInitialize, OnFinalize, SaturatedConversion, + OpaqueKeys, + }, }; use primitives::{ parachain::{ @@ -1244,7 +1174,7 @@ mod tests { use crate::parachains; use crate::registrar; use crate::slots; - use session::SessionManager; + use session::{SessionHandler, SessionManager}; use staking::EraIndex; // result of as trie_db::NodeCodec>::hashed_null_node() @@ -1265,6 +1195,12 @@ mod tests { } } + impl_opaque_keys! { + pub struct TestSessionKeys { + pub parachain_validator: super::Module, + } + } + #[derive(Clone, Eq, PartialEq)] pub struct Test; parameter_types! { @@ -1302,14 +1238,28 @@ mod tests { pub const DisabledValidatorsThreshold: Perbill = Perbill::from_percent(17); } + /// Custom `SessionHandler` since we use `TestSessionKeys` as `Keys`. + pub struct TestSessionHandler; + impl SessionHandler for TestSessionHandler { + const KEY_TYPE_IDS: &'static [KeyTypeId] = &[PARACHAIN_KEY_TYPE_ID]; + + fn on_genesis_session(_: &[(AId, Ks)]) {} + + fn on_new_session(_: bool, _: &[(AId, Ks)], _: &[(AId, Ks)]) {} + + fn on_before_session_ending() {} + + fn on_disabled(_: usize) {} + } + impl session::Trait for Test { type Event = (); type ValidatorId = u64; type ValidatorIdOf = staking::StashOf; type ShouldEndSession = session::PeriodicSessions; type SessionManager = session::historical::NoteHistoricalRoot; - type SessionHandler = session::TestSessionHandler; - type Keys = UintAuthorityId; + type SessionHandler = TestSessionHandler; + type Keys = TestSessionKeys; type DisabledValidatorsThreshold = DisabledValidatorsThreshold; } @@ -1463,11 +1413,10 @@ mod tests { type Registrar = registrar::Module; type MaxCodeSize = MaxCodeSize; type MaxHeadDataSize = MaxHeadDataSize; - type HandleDoubleVote = DoubleVoteHandler< - Historical, - Offences, - DoubleVoteOffence>, - >; + type Proof = )>>::Proof; + type IdentificationTuple = )>>::IdentificationTuple; + type ReportOffence = Offences; + type KeyOwnerProofSystem = Historical; } type Parachains = Module; @@ -1500,7 +1449,9 @@ mod tests { // stashes are the index. let session_keys: Vec<_> = authority_keys.iter().enumerate() - .map(|(i, _k)| (i as u64, i as u64, UintAuthorityId(i as u64))) + .map(|(i, k)| (i as u64, i as u64, TestSessionKeys { + parachain_validator: ValidatorId::from(k.public()), + })) .collect(); let authorities: Vec<_> = authority_keys.iter().map(|k| ValidatorId::from(k.public())).collect(); @@ -1558,11 +1509,9 @@ mod tests { } fn report_double_vote( - report: DoubleVoteReport, - proof: Vec, - id: Vec, + report: DoubleVoteReport, ) -> Result, TransactionValidityError> { - let inner = ParachainsCall::report_double_vote(report, proof, id); + let inner = ParachainsCall::report_double_vote(report); let call = Call::Parachains(inner.clone()); ValidateDoubleVoteReports::(rstd::marker::PhantomData) @@ -2202,12 +2151,6 @@ mod tests { let attestation_1 = ValidityAttestation::Implicit(signature_1); let attestation_2 = ValidityAttestation::Explicit(signature_2); - let report = DoubleVoteReport { - identity: ValidatorId::from(key.public()), - first: (statement_candidate, attestation_1), - second: (statement_valid, attestation_2), - }; - // Check that in the beginning the genesis balances are there. for i in 0..authorities.len() { assert_eq!(Balances::total_balance(&(i as u64)), 10_000_000); @@ -2223,10 +2166,17 @@ mod tests { ); } - let encoded_key = (authority_index as u64).encode(); - let proof = Historical::prove((DUMMY, &encoded_key[..])).unwrap(); + let encoded_key = key.encode(); + let proof = Historical::prove((PARACHAIN_KEY_TYPE_ID, &encoded_key[..])).unwrap(); + + let report = DoubleVoteReport { + identity: ValidatorId::from(key.public()), + first: (statement_candidate, attestation_1), + second: (statement_valid, attestation_2), + proof, + }; - let inner = report_double_vote(report, proof.encode(), encoded_key).unwrap(); + let inner = report_double_vote(report).unwrap(); assert_ok!(Parachains::dispatch(inner, Origin::signed(1))); @@ -2299,12 +2249,6 @@ mod tests { let attestation_1 = ValidityAttestation::Implicit(signature_1); let attestation_2 = ValidityAttestation::Explicit(signature_2); - let report = DoubleVoteReport { - identity: ValidatorId::from(key.public()), - first: (statement_candidate, attestation_1), - second: (statement_invalid, attestation_2), - }; - // Check that in the beginning the genesis balances are there. for i in 0..authorities.len() { assert_eq!(Balances::total_balance(&(i as u64)), 10_000_000); @@ -2320,11 +2264,18 @@ mod tests { ); } - let encoded_key = (authority_index as u64).encode(); - let proof = Historical::prove((DUMMY, &encoded_key[..])).unwrap(); + let encoded_key = key.encode(); + let proof = Historical::prove((PARACHAIN_KEY_TYPE_ID, &encoded_key[..])).unwrap(); + + let report = DoubleVoteReport { + identity: ValidatorId::from(key.public()), + first: (statement_candidate, attestation_1), + second: (statement_invalid, attestation_2), + proof, + }; assert_ok!(Parachains::dispatch( - report_double_vote(report, proof.encode(), encoded_key).unwrap(), + report_double_vote(report).unwrap(), Origin::signed(1), )); @@ -2398,12 +2349,6 @@ mod tests { let attestation_1 = ValidityAttestation::Explicit(signature_1); let attestation_2 = ValidityAttestation::Explicit(signature_2); - let report = DoubleVoteReport { - identity: ValidatorId::from(key.public()), - first: (statement_invalid, attestation_1), - second: (statement_valid, attestation_2), - }; - // Check that in the beginning the genesis balances are there. for i in 0..authorities.len() { assert_eq!(Balances::total_balance(&(i as u64)), 10_000_000); @@ -2419,11 +2364,18 @@ mod tests { ); } - let encoded_key = (authority_index as u64).encode(); - let proof = Historical::prove((DUMMY, &encoded_key[..])).unwrap(); + let encoded_key = key.encode(); + let proof = Historical::prove((PARACHAIN_KEY_TYPE_ID, &encoded_key[..])).unwrap(); + + let report = DoubleVoteReport { + identity: ValidatorId::from(key.public()), + first: (statement_invalid, attestation_1), + second: (statement_valid, attestation_2), + proof, + }; assert_ok!(Parachains::dispatch( - report_double_vote(report, proof.encode(), encoded_key).unwrap(), + report_double_vote(report).unwrap(), Origin::signed(1), )); @@ -2500,12 +2452,6 @@ mod tests { let attestation_1 = ValidityAttestation::Implicit(signature_1); let attestation_2 = ValidityAttestation::Explicit(signature_2); - let report = DoubleVoteReport { - identity: ValidatorId::from(key.public()), - first: (statement_candidate, attestation_1), - second: (statement_valid, attestation_2), - }; - // Check that in the beginning the genesis balances are there. for i in 0..authorities.len() { assert_eq!(Balances::total_balance(&(i as u64)), 10_000_000); @@ -2521,16 +2467,23 @@ mod tests { ); } - let encoded_key = (authority_index as u64).encode(); - let proof = Historical::prove((DUMMY, &encoded_key[..])).unwrap(); + let encoded_key = key.encode(); + let proof = Historical::prove((PARACHAIN_KEY_TYPE_ID, &encoded_key[..])).unwrap(); + + let report = DoubleVoteReport { + identity: ValidatorId::from(key.public()), + first: (statement_candidate, attestation_1), + second: (statement_valid, attestation_2), + proof, + }; assert_ok!(Parachains::dispatch( - report_double_vote(report.clone(), proof.encode(), encoded_key.clone()).unwrap(), + report_double_vote(report.clone()).unwrap(), Origin::signed(1), )); assert!(Parachains::dispatch( - report_double_vote(report, proof.encode(), encoded_key).unwrap(), + report_double_vote(report).unwrap(), Origin::signed(1), ).is_err() ); @@ -2610,17 +2563,18 @@ mod tests { let attestation_1 = ValidityAttestation::Implicit(signature_1); let attestation_2 = ValidityAttestation::Explicit(signature_2); + let encoded_key = key_1.encode(); + let proof = Historical::prove((PARACHAIN_KEY_TYPE_ID, &encoded_key[..])).unwrap(); + let report = DoubleVoteReport { identity: ValidatorId::from(key_1.public()), first: (statement_candidate, attestation_1), second: (statement_valid, attestation_2), + proof, }; - let encoded_key = (authority_1_index as u64).encode(); - let proof = Historical::prove((DUMMY, &encoded_key[..])).unwrap(); - assert_eq!( - report_double_vote(report.clone(), proof.encode(), encoded_key.clone()), + report_double_vote(report.clone()), Err(TransactionValidityError::Invalid( InvalidTransaction::Custom(DoubleVoteValidityError::InvalidSignature as u8) ) diff --git a/runtime/common/src/registrar.rs b/runtime/common/src/registrar.rs index eb70a35ce118..181f5bfd0a47 100644 --- a/runtime/common/src/registrar.rs +++ b/runtime/common/src/registrar.rs @@ -646,7 +646,7 @@ mod tests { traits::{ BlakeTwo256, IdentityLookup, OnInitialize, OnFinalize, Dispatchable, AccountIdConversion, - }, testing::{UintAuthorityId, Header}, Perbill, curve::PiecewiseLinear, + }, testing::{UintAuthorityId, Header}, KeyTypeId, Perbill, curve::PiecewiseLinear, }; use primitives::{ parachain::{ @@ -657,6 +657,7 @@ mod tests { Balance, BlockNumber, }; use frame_support::{ + traits::KeyOwnerProofSystem, impl_outer_origin, impl_outer_dispatch, assert_ok, parameter_types, assert_noop, }; use keyring::Sr25519Keyring; @@ -818,7 +819,10 @@ mod tests { type Randomness = RandomnessCollectiveFlip; type MaxCodeSize = MaxCodeSize; type MaxHeadDataSize = MaxHeadDataSize; - type HandleDoubleVote = (); + type Proof = session::historical::Proof; + type KeyOwnerProofSystem = session::historical::Module; + type IdentificationTuple = )>>::IdentificationTuple; + type ReportOffence = (); } parameter_types! { diff --git a/runtime/kusama/src/lib.rs b/runtime/kusama/src/lib.rs index e86258d63e26..6bd721ad74ef 100644 --- a/runtime/kusama/src/lib.rs +++ b/runtime/kusama/src/lib.rs @@ -34,7 +34,7 @@ use runtime_common::{attestations, claims, parachains, registrar, slots, }; use sp_runtime::{ create_runtime_str, generic, impl_opaque_keys, - ApplyExtrinsicResult, Percent, Permill, Perbill, RuntimeDebug, + ApplyExtrinsicResult, KeyTypeId, Percent, Permill, Perbill, RuntimeDebug, transaction_validity::{TransactionValidity, InvalidTransaction, TransactionValidityError}, curve::PiecewiseLinear, traits::{BlakeTwo256, Block as BlockT, SignedExtension, OpaqueKeys, ConvertInto, IdentityLookup}, @@ -46,7 +46,7 @@ use version::NativeVersion; use sp_core::OpaqueMetadata; use sp_staking::SessionIndex; use frame_support::{ - parameter_types, construct_runtime, traits::{SplitTwoWays, Randomness}, + parameter_types, construct_runtime, traits::{KeyOwnerProofSystem, SplitTwoWays, Randomness}, weights::DispatchInfo, }; use im_online::sr25519::AuthorityId as ImOnlineId; @@ -474,11 +474,10 @@ impl parachains::Trait for Runtime { type Registrar = Registrar; type MaxCodeSize = MaxCodeSize; type MaxHeadDataSize = MaxHeadDataSize; - type HandleDoubleVote = parachains::DoubleVoteHandler< - Historical, - Offences, - parachains::DoubleVoteOffence>, - >; + type Proof = session::historical::Proof; + type KeyOwnerProofSystem = session::historical::Module; + type IdentificationTuple = )>>::IdentificationTuple; + type ReportOffence = Offences; } parameter_types! { diff --git a/runtime/polkadot/src/lib.rs b/runtime/polkadot/src/lib.rs index 7d3da16c1664..75ab5c1796d7 100644 --- a/runtime/polkadot/src/lib.rs +++ b/runtime/polkadot/src/lib.rs @@ -35,7 +35,7 @@ use primitives::{ }; use sp_runtime::{ create_runtime_str, generic, impl_opaque_keys, - ApplyExtrinsicResult, Percent, Permill, Perbill, RuntimeDebug, + ApplyExtrinsicResult, KeyTypeId, Percent, Permill, Perbill, RuntimeDebug, transaction_validity::{TransactionValidity, InvalidTransaction, TransactionValidityError}, curve::PiecewiseLinear, traits::{ @@ -50,7 +50,7 @@ use version::NativeVersion; use sp_core::OpaqueMetadata; use sp_staking::SessionIndex; use frame_support::{ - parameter_types, construct_runtime, traits::{SplitTwoWays, Randomness}, + parameter_types, construct_runtime, traits::{KeyOwnerProofSystem, SplitTwoWays, Randomness}, weights::DispatchInfo, }; use im_online::sr25519::AuthorityId as ImOnlineId; @@ -482,11 +482,10 @@ impl parachains::Trait for Runtime { type Registrar = Registrar; type MaxCodeSize = MaxCodeSize; type MaxHeadDataSize = MaxHeadDataSize; - type HandleDoubleVote = parachains::DoubleVoteHandler< - Historical, - Offences, - parachains::DoubleVoteOffence>, - >; + type Proof = session::historical::Proof; + type KeyOwnerProofSystem = session::historical::Module; + type IdentificationTuple = )>>::IdentificationTuple; + type ReportOffence = Offences; } parameter_types! { From 3a7124e667ae19afef9df8316efc7b68134df07c Mon Sep 17 00:00:00 2001 From: Fedor Sakharov Date: Tue, 10 Mar 2020 19:13:32 +0300 Subject: [PATCH 08/18] Bump runtime spec_version --- runtime/kusama/src/lib.rs | 2 +- runtime/polkadot/src/lib.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/runtime/kusama/src/lib.rs b/runtime/kusama/src/lib.rs index 94f85e02b3cb..bf8f84942f02 100644 --- a/runtime/kusama/src/lib.rs +++ b/runtime/kusama/src/lib.rs @@ -77,7 +77,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("kusama"), impl_name: create_runtime_str!("parity-kusama"), authoring_version: 2, - spec_version: 1050, + spec_version: 1051, impl_version: 0, apis: RUNTIME_API_VERSIONS, }; diff --git a/runtime/polkadot/src/lib.rs b/runtime/polkadot/src/lib.rs index eeb2fe7f3920..39c59a7b851b 100644 --- a/runtime/polkadot/src/lib.rs +++ b/runtime/polkadot/src/lib.rs @@ -82,7 +82,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("polkadot"), impl_name: create_runtime_str!("parity-polkadot"), authoring_version: 2, - spec_version: 1004, + spec_version: 1005, impl_version: 0, apis: RUNTIME_API_VERSIONS, }; From 1b731e7454bbca4e1dc598b0f96a58c74388d271 Mon Sep 17 00:00:00 2001 From: Fedor Sakharov Date: Wed, 11 Mar 2020 14:53:26 +0300 Subject: [PATCH 09/18] Get the session number from the proof --- runtime/common/src/parachains.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/runtime/common/src/parachains.rs b/runtime/common/src/parachains.rs index 3e0e17428548..3cf28631157b 100644 --- a/runtime/common/src/parachains.rs +++ b/runtime/common/src/parachains.rs @@ -126,7 +126,7 @@ pub struct DoubleVoteReport { pub proof: Proof, } -impl DoubleVoteReport { +impl DoubleVoteReport { fn verify, H: AsRef<[u8]>>( &self, parent_hash: H, @@ -195,6 +195,17 @@ impl Get> for ValidatorIdentities { } } +/// A trait to get a session number the `Proof` belongs to. +pub trait GetSessionNumber { + fn session(&self) -> SessionIndex; +} + +impl GetSessionNumber for session::historical::Proof { + fn session(&self) -> SessionIndex { + self.session() + } +} + pub trait Trait: attestations::Trait + session::historical::Trait { /// The outer origin type. type Origin: From + From>; @@ -228,7 +239,7 @@ pub trait Trait: attestations::Trait + session::historical::Trait { /// Proof type (since it is a part of Calls it needs to be bound to `Parameter` here) /// TODO: I know of no other way to bind an associated type of `KeyOwnerProofSystem::Proof` /// to parameter. - type Proof: Parameter; + type Proof: Parameter + GetSessionNumber; /// Compute and check proofs of historical key owners. type KeyOwnerProofSystem: KeyOwnerProofSystem< @@ -472,10 +483,7 @@ decl_module! { let validators = >::validators(); let validator_set_count = validators.len() as u32; - //let key_owner_proof = report.proof.clone();// Proof = Decode::decode(&mut &key_owner_proof[..]) - //.map_err(|_| "Key owner proof decoding failed.")?; - - let session_index = Default::default(); // TODO: this has to be key_owner_proof.session(); + let session_index = report.proof.session(); let offender = T::KeyOwnerProofSystem::check_proof( (PARACHAIN_KEY_TYPE_ID, report.identity.encode()), From e91453e16dc4f08d9068148e4259e93ee8c55a45 Mon Sep 17 00:00:00 2001 From: Fedor Sakharov Date: Thu, 12 Mar 2020 10:28:25 +0300 Subject: [PATCH 10/18] Check that proof matches session --- runtime/common/src/parachains.rs | 181 ++++++++++++++++++++++++++----- 1 file changed, 155 insertions(+), 26 deletions(-) diff --git a/runtime/common/src/parachains.rs b/runtime/common/src/parachains.rs index 3cf28631157b..ad0fe4f102aa 100644 --- a/runtime/common/src/parachains.rs +++ b/runtime/common/src/parachains.rs @@ -115,7 +115,7 @@ pub struct ValidatorIdentities(rstd::marker::PhantomData); #[derive(RuntimeDebug, Encode, Decode)] #[derive(Clone, Eq, PartialEq)] -pub struct DoubleVoteReport { +pub struct DoubleVoteReport { /// Identity of the double-voter. pub identity: ValidatorId, /// First vote of the double-vote. @@ -124,12 +124,13 @@ pub struct DoubleVoteReport { pub second: (Statement, ValidityAttestation), /// Proof pub proof: Proof, + /// Parent hash of the block this offence was commited. + pub parent_hash: Hash, } -impl DoubleVoteReport { - fn verify, H: AsRef<[u8]>>( +impl> DoubleVoteReport { + fn verify>( &self, - parent_hash: H, ) -> Result<(), DoubleVoteValidityError> { let first = self.first.clone(); let second = self.second.clone(); @@ -139,8 +140,8 @@ impl DoubleVoteReport { .ok_or(DoubleVoteValidityError::InvalidProof)?; // Check signatures. - Self::verify_vote(&first, &parent_hash, &self.identity)?; - Self::verify_vote(&second, &parent_hash, &self.identity)?; + Self::verify_vote(&first, &self.parent_hash, &self.identity)?; + Self::verify_vote(&second, &self.parent_hash, &self.identity)?; match (&first.0, &second.0) { // If issuing a `Candidate` message on a parachain block, neither a `Valid` or @@ -164,9 +165,9 @@ impl DoubleVoteReport { Ok(()) } - fn verify_vote>( + fn verify_vote( vote: &(Statement, ValidityAttestation), - parent_hash: H, + parent_hash: &Hash, authority: &ValidatorId, ) -> Result<(), DoubleVoteValidityError> { let sig = match vote { @@ -336,6 +337,12 @@ decl_storage! { /// /// `None` if not yet updated. pub DidUpdate: Option>; + + /// The mapping from parent block hashes to session indexes. + /// + /// Used for double vote report validation. + pub ParentToSessionIndex get(session_at_block): + map hasher(blake2_256) T::Hash => SessionIndex; } add_extra_genesis { config(authorities): Vec; @@ -475,7 +482,8 @@ decl_module! { pub fn report_double_vote( origin, report: DoubleVoteReport< - )>>::Proof + )>>::Proof, + T::Hash, >, ) -> DispatchResult { let reporter = ensure_signed(origin)?; @@ -506,6 +514,11 @@ decl_module! { fn on_initialize() { ::DidUpdate::kill(); + + let current_session = >::current_index(); + let parent_hash = >::parent_hash(); + + >::insert(parent_hash, current_session); } fn on_finalize() { @@ -1126,7 +1139,14 @@ impl SignedExtension for ValidateDoubleVoteReports wh if let Some(local_call) = call.is_sub_type() { if let Call::report_double_vote(report) = local_call { let validators = >::validators(); - let parent_hash = >::block_hash(>::block_number()); + let parent_hash = report.parent_hash; + + let expected_session = Module::::session_at_block(parent_hash); + let session = report.proof.session(); + + if session != expected_session { + return Err(InvalidTransaction::BadProof.into()); + } let authorities = Module::::authorities(); let offender_idx = match authorities.iter().position(|a| *a == report.identity) { @@ -1143,7 +1163,7 @@ impl SignedExtension for ValidateDoubleVoteReports wh } report - .verify::(parent_hash) + .verify::() .map_err(|e| TransactionValidityError::from(InvalidTransaction::Custom(e as u8)))?; } } @@ -1519,7 +1539,7 @@ mod tests { } fn report_double_vote( - report: DoubleVoteReport, + report: DoubleVoteReport::Hash>, ) -> Result, TransactionValidityError> { let inner = ParachainsCall::report_double_vote(report); let call = Call::Parachains(inner.clone()); @@ -1622,14 +1642,35 @@ mod tests { } fn start_session(session_index: SessionIndex) { + let mut parent_hash = System::parent_hash(); + use sp_runtime::traits::Header; + for i in Session::current_index()..session_index { println!("session index {}", i); + Staking::on_finalize(System::block_number()); System::set_block_number((i + 1).into()); Timestamp::set_timestamp(System::block_number() * 6000); - println!("block {:?}", System::block_number()); - println!("Current era {:?}", Staking::current_era()); - Session::on_initialize(System::block_number()); + + // In order to be able to use `System::parent_hash()` in the tests + // we need to first get it via `System::finalize` and then set it + // the `System::initialize`. However, it is needed to be taken into + // consideration that finalizing will prune some data in `System` + // storage including old values `BlockHash` if that reaches above + // `BlockHashCount` capacity. + if System::block_number() > 1 { + let hdr = System::finalize(); + parent_hash = hdr.hash(); + } + + System::initialize( + &(i as u64 + 1), + &parent_hash, + &Default::default(), + &Default::default(), + Default::default(), + ); + init_block(); } assert_eq!(Session::current_index(), session_index); @@ -1642,6 +1683,7 @@ mod tests { fn init_block() { println!("Initializing {}", System::block_number()); + Session::on_initialize(System::block_number()); System::on_initialize(System::block_number()); Registrar::on_initialize(System::block_number()); Parachains::on_initialize(System::block_number()); @@ -2148,8 +2190,7 @@ mod tests { let statement_candidate = Statement::Candidate(candidate_hash.clone()); let statement_valid = Statement::Valid(candidate_hash.clone()); - let block_number = >::block_number(); - let parent_hash = >::block_hash(block_number); + let parent_hash = System::parent_hash(); let payload_1 = localized_payload(statement_candidate.clone(), parent_hash); let payload_2 = localized_payload(statement_valid.clone(), parent_hash); @@ -2183,6 +2224,7 @@ mod tests { first: (statement_candidate, attestation_1), second: (statement_valid, attestation_2), proof, + parent_hash, }; let inner = report_double_vote(report).unwrap(); @@ -2240,15 +2282,13 @@ mod tests { start_era(1); - println!("Era started"); let authorities = Parachains::authorities(); let authority_index = 0; let key = extract_key(authorities[authority_index].clone()); let statement_candidate = Statement::Candidate(candidate_hash); let statement_invalid = Statement::Invalid(candidate_hash.clone()); - let block_number = >::block_number(); - let parent_hash = >::block_hash(block_number); + let parent_hash = System::parent_hash(); let payload_1 = localized_payload(statement_candidate.clone(), parent_hash); let payload_2 = localized_payload(statement_invalid.clone(), parent_hash); @@ -2282,6 +2322,7 @@ mod tests { first: (statement_candidate, attestation_1), second: (statement_invalid, attestation_2), proof, + parent_hash, }; assert_ok!(Parachains::dispatch( @@ -2347,8 +2388,7 @@ mod tests { let statement_invalid = Statement::Invalid(candidate_hash.clone()); let statement_valid = Statement::Valid(candidate_hash.clone()); - let block_number = >::block_number(); - let parent_hash = >::block_hash(block_number); + let parent_hash = System::parent_hash(); let payload_1 = localized_payload(statement_invalid.clone(), parent_hash); let payload_2 = localized_payload(statement_valid.clone(), parent_hash); @@ -2382,6 +2422,7 @@ mod tests { first: (statement_invalid, attestation_1), second: (statement_valid, attestation_2), proof, + parent_hash, }; assert_ok!(Parachains::dispatch( @@ -2450,8 +2491,7 @@ mod tests { let statement_candidate = Statement::Candidate(candidate_hash.clone()); let statement_valid = Statement::Valid(candidate_hash.clone()); - let block_number = >::block_number(); - let parent_hash = >::block_hash(block_number); + let parent_hash = System::parent_hash(); let payload_1 = localized_payload(statement_candidate.clone(), parent_hash); let payload_2 = localized_payload(statement_valid.clone(), parent_hash); @@ -2485,6 +2525,7 @@ mod tests { first: (statement_candidate, attestation_1), second: (statement_valid, attestation_2), proof, + parent_hash, }; assert_ok!(Parachains::dispatch( @@ -2561,8 +2602,7 @@ mod tests { let statement_candidate = Statement::Candidate(candidate_hash.clone()); let statement_valid = Statement::Valid(candidate_hash.clone()); - let block_number = >::block_number(); - let parent_hash = >::block_hash(block_number); + let parent_hash = System::parent_hash(); let payload_1 = localized_payload(statement_candidate.clone(), parent_hash); let payload_2 = localized_payload(statement_valid.clone(), parent_hash); @@ -2581,6 +2621,7 @@ mod tests { first: (statement_candidate, attestation_1), second: (statement_valid, attestation_2), proof, + parent_hash, }; assert_eq!( @@ -2593,4 +2634,92 @@ mod tests { }); } + #[test] + fn double_vote_proof_session_mismatch_fails() { + let parachains = vec![ + (1u32.into(), vec![], vec![]), + ]; + + let extract_key = |public: ValidatorId| { + let mut raw_public = [0; 32]; + raw_public.copy_from_slice(public.as_ref()); + Sr25519Keyring::from_raw_public(raw_public).unwrap() + }; + + // Test that submitting a report with a session mismatch between the `parent_hash` + // and the proof itself fails. + new_test_ext(parachains.clone()).execute_with(|| { + let candidate = raw_candidate(1.into()).abridge().0; + let candidate_hash = candidate.hash(); + + assert_eq!(Staking::current_era(), Some(0)); + assert_eq!(Session::current_index(), 0); + + start_era(1); + + let authorities = Parachains::authorities(); + let authority_index = 0; + let key = extract_key(authorities[authority_index].clone()); + + let statement_candidate = Statement::Candidate(candidate_hash.clone()); + let statement_valid = Statement::Valid(candidate_hash.clone()); + let parent_hash = System::parent_hash(); + + let payload_1 = localized_payload(statement_candidate.clone(), parent_hash); + let payload_2 = localized_payload(statement_valid.clone(), parent_hash); + + let signature_1 = key.sign(&payload_1[..]).into(); + let signature_2 = key.sign(&payload_2[..]).into(); + + let attestation_1 = ValidityAttestation::Implicit(signature_1); + let attestation_2 = ValidityAttestation::Explicit(signature_2); + + // Check that in the beginning the genesis balances are there. + for i in 0..authorities.len() { + assert_eq!(Balances::total_balance(&(i as u64)), 10_000_000); + assert_eq!(Staking::slashable_balance_of(&(i as u64)), 10_000); + + assert_eq!( + Staking::eras_stakers(1, i as u64), + staking::Exposure { + total: 10_000, + own: 10_000, + others: vec![], + }, + ); + } + + // Get the proof from another session. + start_era(2); + let encoded_key = key.encode(); + let proof = Historical::prove((PARACHAIN_KEY_TYPE_ID, &encoded_key[..])).unwrap(); + + let report = DoubleVoteReport { + identity: ValidatorId::from(key.public()), + first: (statement_candidate, attestation_1), + second: (statement_valid, attestation_2), + proof, + parent_hash, + }; + + assert!(report_double_vote(report.clone()).is_err()); + + start_era(3); + + // Check that the balances are unchanged. + for i in 0..authorities.len() { + assert_eq!(Balances::total_balance(&(i as u64)), 10_000_000); + assert_eq!(Staking::slashable_balance_of(&(i as u64)), 10_000); + + assert_eq!( + Staking::eras_stakers(1, i as u64), + staking::Exposure { + total: 10_000, + own: 10_000, + others: vec![], + }, + ); + } + }); + } } From 4861836be6cdb44792539d036f1bd69dd91354b1 Mon Sep 17 00:00:00 2001 From: Fedor Sakharov Date: Thu, 12 Mar 2020 16:46:58 +0300 Subject: [PATCH 11/18] Change signature type on DoubleVoteReport --- runtime/common/src/parachains.rs | 61 +++++++++----------------------- 1 file changed, 17 insertions(+), 44 deletions(-) diff --git a/runtime/common/src/parachains.rs b/runtime/common/src/parachains.rs index ad0fe4f102aa..8a4fe1fa222c 100644 --- a/runtime/common/src/parachains.rs +++ b/runtime/common/src/parachains.rs @@ -44,6 +44,7 @@ use primitives::{ UpwardMessage, ValidatorId, ActiveParas, CollatorId, Retriable, OmittedValidationData, CandidateReceipt, GlobalValidationSchedule, AbridgedCandidateReceipt, LocalValidationData, ValidityAttestation, NEW_HEADS_IDENTIFIER, PARACHAIN_KEY_TYPE_ID, + ValidatorSignature, }, }; use frame_support::{ @@ -119,9 +120,9 @@ pub struct DoubleVoteReport { /// Identity of the double-voter. pub identity: ValidatorId, /// First vote of the double-vote. - pub first: (Statement, ValidityAttestation), + pub first: (Statement, ValidatorSignature), /// Second vote of the double-vote. - pub second: (Statement, ValidityAttestation), + pub second: (Statement, ValidatorSignature), /// Proof pub proof: Proof, /// Parent hash of the block this offence was commited. @@ -166,23 +167,13 @@ impl> DoubleVoteReport Result<(), DoubleVoteValidityError> { - let sig = match vote { - (Statement::Candidate(_), ValidityAttestation::Implicit(sig)) | - (Statement::Valid(_), ValidityAttestation::Explicit(sig)) | - (Statement::Invalid(_), ValidityAttestation::Explicit(sig)) => { - sig - } - _ => { - return Err(DoubleVoteValidityError::InvalidReport); - } - }; let payload = localized_payload(vote.0.clone(), parent_hash); - if !sig.verify(&payload[..], authority) { + if !vote.1.verify(&payload[..], authority) { return Err(DoubleVoteValidityError::InvalidSignature); } @@ -2198,9 +2189,6 @@ mod tests { let signature_1 = key.sign(&payload_1[..]).into(); let signature_2 = key.sign(&payload_2[..]).into(); - let attestation_1 = ValidityAttestation::Implicit(signature_1); - let attestation_2 = ValidityAttestation::Explicit(signature_2); - // Check that in the beginning the genesis balances are there. for i in 0..authorities.len() { assert_eq!(Balances::total_balance(&(i as u64)), 10_000_000); @@ -2221,8 +2209,8 @@ mod tests { let report = DoubleVoteReport { identity: ValidatorId::from(key.public()), - first: (statement_candidate, attestation_1), - second: (statement_valid, attestation_2), + first: (statement_candidate, signature_1), + second: (statement_valid, signature_2), proof, parent_hash, }; @@ -2296,9 +2284,6 @@ mod tests { let signature_1 = key.sign(&payload_1[..]).into(); let signature_2 = key.sign(&payload_2[..]).into(); - let attestation_1 = ValidityAttestation::Implicit(signature_1); - let attestation_2 = ValidityAttestation::Explicit(signature_2); - // Check that in the beginning the genesis balances are there. for i in 0..authorities.len() { assert_eq!(Balances::total_balance(&(i as u64)), 10_000_000); @@ -2319,8 +2304,8 @@ mod tests { let report = DoubleVoteReport { identity: ValidatorId::from(key.public()), - first: (statement_candidate, attestation_1), - second: (statement_invalid, attestation_2), + first: (statement_candidate, signature_1), + second: (statement_invalid, signature_2), proof, parent_hash, }; @@ -2396,9 +2381,6 @@ mod tests { let signature_1 = key.sign(&payload_1[..]).into(); let signature_2 = key.sign(&payload_2[..]).into(); - let attestation_1 = ValidityAttestation::Explicit(signature_1); - let attestation_2 = ValidityAttestation::Explicit(signature_2); - // Check that in the beginning the genesis balances are there. for i in 0..authorities.len() { assert_eq!(Balances::total_balance(&(i as u64)), 10_000_000); @@ -2419,8 +2401,8 @@ mod tests { let report = DoubleVoteReport { identity: ValidatorId::from(key.public()), - first: (statement_invalid, attestation_1), - second: (statement_valid, attestation_2), + first: (statement_invalid, signature_1), + second: (statement_valid, signature_2), proof, parent_hash, }; @@ -2499,9 +2481,6 @@ mod tests { let signature_1 = key.sign(&payload_1[..]).into(); let signature_2 = key.sign(&payload_2[..]).into(); - let attestation_1 = ValidityAttestation::Implicit(signature_1); - let attestation_2 = ValidityAttestation::Explicit(signature_2); - // Check that in the beginning the genesis balances are there. for i in 0..authorities.len() { assert_eq!(Balances::total_balance(&(i as u64)), 10_000_000); @@ -2522,8 +2501,8 @@ mod tests { let report = DoubleVoteReport { identity: ValidatorId::from(key.public()), - first: (statement_candidate, attestation_1), - second: (statement_valid, attestation_2), + first: (statement_candidate, signature_1), + second: (statement_valid, signature_2), proof, parent_hash, }; @@ -2610,16 +2589,13 @@ mod tests { let signature_1 = key_1.sign(&payload_1[..]).into(); let signature_2 = key_2.sign(&payload_2[..]).into(); - let attestation_1 = ValidityAttestation::Implicit(signature_1); - let attestation_2 = ValidityAttestation::Explicit(signature_2); - let encoded_key = key_1.encode(); let proof = Historical::prove((PARACHAIN_KEY_TYPE_ID, &encoded_key[..])).unwrap(); let report = DoubleVoteReport { identity: ValidatorId::from(key_1.public()), - first: (statement_candidate, attestation_1), - second: (statement_valid, attestation_2), + first: (statement_candidate, signature_1), + second: (statement_valid, signature_2), proof, parent_hash, }; @@ -2671,9 +2647,6 @@ mod tests { let signature_1 = key.sign(&payload_1[..]).into(); let signature_2 = key.sign(&payload_2[..]).into(); - let attestation_1 = ValidityAttestation::Implicit(signature_1); - let attestation_2 = ValidityAttestation::Explicit(signature_2); - // Check that in the beginning the genesis balances are there. for i in 0..authorities.len() { assert_eq!(Balances::total_balance(&(i as u64)), 10_000_000); @@ -2696,8 +2669,8 @@ mod tests { let report = DoubleVoteReport { identity: ValidatorId::from(key.public()), - first: (statement_candidate, attestation_1), - second: (statement_valid, attestation_2), + first: (statement_candidate, signature_1), + second: (statement_valid, signature_2), proof, parent_hash, }; From 456a94adce78e932501c4c083c95a0086cbc1f74 Mon Sep 17 00:00:00 2001 From: Fedor Sakharov Date: Thu, 12 Mar 2020 17:16:08 +0300 Subject: [PATCH 12/18] Adds docs and removes blank lines --- runtime/common/src/parachains.rs | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/runtime/common/src/parachains.rs b/runtime/common/src/parachains.rs index 8a4fe1fa222c..68ef59cc0556 100644 --- a/runtime/common/src/parachains.rs +++ b/runtime/common/src/parachains.rs @@ -114,6 +114,11 @@ impl> ParachainCurrency for T where /// Interface to the persistent (stash) identities of the current validators. pub struct ValidatorIdentities(rstd::marker::PhantomData); +/// A structure used to report conflicting votes by validators. +/// +/// It is generic over two parameters: +/// `Proof` - proof of historical ownership of a key by some validator. +/// `Hash` - a type of a hash used in the runtime. #[derive(RuntimeDebug, Encode, Decode)] #[derive(Clone, Eq, PartialEq)] pub struct DoubleVoteReport { @@ -123,7 +128,7 @@ pub struct DoubleVoteReport { pub first: (Statement, ValidatorSignature), /// Second vote of the double-vote. pub second: (Statement, ValidatorSignature), - /// Proof + /// Proof that the validator with `identity` id was actually a validator at `parent_hash`. pub proof: Proof, /// Parent hash of the block this offence was commited. pub parent_hash: Hash, @@ -228,9 +233,11 @@ pub trait Trait: attestations::Trait + session::historical::Trait { /// Submit double-vote offence reports. /// type HandleDoubleVote: HandleDoubleVote; - /// Proof type (since it is a part of Calls it needs to be bound to `Parameter` here) - /// TODO: I know of no other way to bind an associated type of `KeyOwnerProofSystem::Proof` - /// to parameter. + /// Proof type. + /// + /// We need this type to bind the `KeyOwnerProofSystem::Proof` to necessary bounds. + /// As soon as https://rust-lang.github.io/rfcs/2289-associated-type-bounds.html + /// gets in this can be simplified. type Proof: Parameter + GetSessionNumber; /// Compute and check proofs of historical key owners. @@ -240,7 +247,7 @@ pub trait Trait: attestations::Trait + session::historical::Trait { IdentificationTuple = Self::IdentificationTuple, >; - /// An identification tuple type bound to `Clone`. + /// An identification tuple type bound to `Parameter`. type IdentificationTuple: Parameter; /// Report an offence. @@ -332,6 +339,7 @@ decl_storage! { /// The mapping from parent block hashes to session indexes. /// /// Used for double vote report validation. + /// This is not pruned at the moment. pub ParentToSessionIndex get(session_at_block): map hasher(blake2_256) T::Hash => SessionIndex; } @@ -484,6 +492,8 @@ decl_module! { let session_index = report.proof.session(); + // We have already checked this proof in `SignedExtension`, but we need + // this here to get the full identification of the offender. let offender = T::KeyOwnerProofSystem::check_proof( (PARACHAIN_KEY_TYPE_ID, report.identity.encode()), report.proof.clone(), @@ -1291,7 +1301,6 @@ mod tests { parameter_types! { pub const MinimumPeriod: u64 = 3; } - impl timestamp::Trait for Test { type Moment = u64; type OnTimestampSet = (); @@ -1306,7 +1315,6 @@ mod tests { const MINUTES: BlockNumber = 60_000 / (MILLISECS_PER_BLOCK as BlockNumber); const HOURS: BlockNumber = MINUTES * 60; } - parameter_types! { pub const EpochDuration: u64 = time::EPOCH_DURATION_IN_BLOCKS as u64; pub const ExpectedBlockTime: u64 = time::MILLISECS_PER_BLOCK; From f60b0586d495a62c7bbaa9b1b35446951bd09f36 Mon Sep 17 00:00:00 2001 From: Fedor Sakharov Date: Thu, 12 Mar 2020 17:18:09 +0300 Subject: [PATCH 13/18] Removes leftover code --- runtime/common/src/parachains.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/runtime/common/src/parachains.rs b/runtime/common/src/parachains.rs index 68ef59cc0556..c3b0f25de6d6 100644 --- a/runtime/common/src/parachains.rs +++ b/runtime/common/src/parachains.rs @@ -230,9 +230,6 @@ pub trait Trait: attestations::Trait + session::historical::Trait { /// Max head data size. type MaxHeadDataSize: Get; - /// Submit double-vote offence reports. - /// type HandleDoubleVote: HandleDoubleVote; - /// Proof type. /// /// We need this type to bind the `KeyOwnerProofSystem::Proof` to necessary bounds. From c12d17ab74396c05fde4af3ec283a85624a12b19 Mon Sep 17 00:00:00 2001 From: Fedor Sakharov Date: Sun, 15 Mar 2020 16:47:13 +0300 Subject: [PATCH 14/18] Fix build --- Cargo.lock | 1 + runtime/common/src/parachains.rs | 10 +++++----- runtime/test-runtime/Cargo.toml | 4 +++- runtime/test-runtime/src/lib.rs | 20 ++++++++++++++++++-- 4 files changed, 27 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cdd99bf7f60b..3431adf920d9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4115,6 +4115,7 @@ dependencies = [ "pallet-grandpa 2.0.0-alpha.3 (git+https://github.com/paritytech/substrate?branch=polkadot-master)", "pallet-indices 2.0.0-alpha.3 (git+https://github.com/paritytech/substrate?branch=polkadot-master)", "pallet-nicks 2.0.0-alpha.3 (git+https://github.com/paritytech/substrate?branch=polkadot-master)", + "pallet-offences 2.0.0-alpha.3 (git+https://github.com/paritytech/substrate?branch=polkadot-master)", "pallet-randomness-collective-flip 2.0.0-alpha.3 (git+https://github.com/paritytech/substrate?branch=polkadot-master)", "pallet-session 2.0.0-alpha.3 (git+https://github.com/paritytech/substrate?branch=polkadot-master)", "pallet-staking 2.0.0-alpha.3 (git+https://github.com/paritytech/substrate?branch=polkadot-master)", diff --git a/runtime/common/src/parachains.rs b/runtime/common/src/parachains.rs index 7c9940d1af98..f0d963a71683 100644 --- a/runtime/common/src/parachains.rs +++ b/runtime/common/src/parachains.rs @@ -1077,11 +1077,11 @@ pub fn ensure_parachain(o: OuterOrigin) -> result::Result(rstd::marker::PhantomData); +pub struct ValidateDoubleVoteReports(sp_std::marker::PhantomData); -impl rstd::fmt::Debug for ValidateDoubleVoteReports where +impl sp_std::fmt::Debug for ValidateDoubleVoteReports where { - fn fmt(&self, f: &mut rstd::fmt::Formatter) -> rstd::fmt::Result { + fn fmt(&self, f: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result { write!(f, "ValidateDoubleVoteReports") } } @@ -1120,7 +1120,7 @@ impl SignedExtension for ValidateDoubleVoteReports wh type DispatchInfo = DispatchInfo; fn additional_signed(&self) - -> rstd::result::Result + -> sp_std::result::Result { Ok(()) } @@ -1540,7 +1540,7 @@ mod tests { let inner = ParachainsCall::report_double_vote(report); let call = Call::Parachains(inner.clone()); - ValidateDoubleVoteReports::(rstd::marker::PhantomData) + ValidateDoubleVoteReports::(sp_std::marker::PhantomData) .validate(&0, &call, DispatchInfo::default(), 0)?; Ok(inner) diff --git a/runtime/test-runtime/Cargo.toml b/runtime/test-runtime/Cargo.toml index 3431e7c1e6a9..0447735bee3e 100644 --- a/runtime/test-runtime/Cargo.toml +++ b/runtime/test-runtime/Cargo.toml @@ -36,8 +36,9 @@ finality-tracker = { package = "pallet-finality-tracker", git = "https://github. grandpa = { package = "pallet-grandpa", git = "https://github.com/paritytech/substrate", branch = "polkadot-master", default-features = false, features = ["migrate-authorities"] } indices = { package = "pallet-indices", git = "https://github.com/paritytech/substrate", branch = "polkadot-master", default-features = false } nicks = { package = "pallet-nicks", git = "https://github.com/paritytech/substrate", branch = "polkadot-master", default-features = false } +offences = { package = "pallet-offences", git = "https://github.com/paritytech/substrate", branch = "polkadot-master", default-features = false } randomness-collective-flip = { package = "pallet-randomness-collective-flip", git = "https://github.com/paritytech/substrate", branch = "polkadot-master", default-features = false } -session = { package = "pallet-session", git = "https://github.com/paritytech/substrate", branch = "polkadot-master", default-features = false } +session = { package = "pallet-session", features = [ "historical"], git = "https://github.com/paritytech/substrate", branch = "polkadot-master", default-features = false } frame-support = { git = "https://github.com/paritytech/substrate", branch = "polkadot-master", default-features = false } staking = { package = "pallet-staking", git = "https://github.com/paritytech/substrate", branch = "polkadot-master", default-features = false } pallet-staking-reward-curve = { git = "https://github.com/paritytech/substrate", branch = "polkadot-master" } @@ -89,6 +90,7 @@ std = [ "grandpa/std", "indices/std", "nicks/std", + "offences/std", "sp-runtime/std", "sp-staking/std", "session/std", diff --git a/runtime/test-runtime/src/lib.rs b/runtime/test-runtime/src/lib.rs index a70e18602407..f5a135442b74 100644 --- a/runtime/test-runtime/src/lib.rs +++ b/runtime/test-runtime/src/lib.rs @@ -34,7 +34,7 @@ use runtime_common::{attestations, claims, parachains, registrar, slots, use sp_runtime::{ create_runtime_str, generic, impl_opaque_keys, - ApplyExtrinsicResult, Perbill, RuntimeDebug, + ApplyExtrinsicResult, Perbill, RuntimeDebug, KeyTypeId, transaction_validity::{TransactionValidity, InvalidTransaction, TransactionValidityError}, curve::PiecewiseLinear, traits::{BlakeTwo256, Block as BlockT, StaticLookup, SignedExtension, OpaqueKeys, ConvertInto}, @@ -46,10 +46,12 @@ use version::NativeVersion; use sp_core::OpaqueMetadata; use sp_staking::SessionIndex; use frame_support::{ - parameter_types, construct_runtime, traits::Randomness, + parameter_types, construct_runtime, + traits::{KeyOwnerProofSystem, Randomness}, weights::DispatchInfo, }; use pallet_transaction_payment_rpc_runtime_api::RuntimeDispatchInfo; +use session::{historical as session_historical}; #[cfg(feature = "std")] pub use staking::StakerStatus; @@ -311,6 +313,18 @@ impl parachains::Trait for Runtime { type Registrar = Registrar; type MaxCodeSize = MaxCodeSize; type MaxHeadDataSize = MaxHeadDataSize; + type Proof = session::historical::Proof; + type KeyOwnerProofSystem = session::historical::Module; + type IdentificationTuple = < + Self::KeyOwnerProofSystem as KeyOwnerProofSystem<(KeyTypeId, Vec)> + >::IdentificationTuple; + type ReportOffence = Offences; +} + +impl offences::Trait for Runtime { + type Event = Event; + type IdentificationTuple = session::historical::IdentificationTuple; + type OnOffenceHandler = Staking; } parameter_types! { @@ -385,6 +399,8 @@ construct_runtime! { // Consensus support. Authorship: authorship::{Module, Call, Storage}, Staking: staking::{Module, Call, Storage, Config, Event}, + Offences: offences::{Module, Call, Storage, Event}, + Historical: session_historical::{Module}, Session: session::{Module, Call, Storage, Event, Config}, Grandpa: grandpa::{Module, Call, Storage, Config, Event}, From de45a66ff6d44e261415dc5710f2c2873e3da57d Mon Sep 17 00:00:00 2001 From: Fedor Sakharov Date: Tue, 17 Mar 2020 18:25:41 +0300 Subject: [PATCH 15/18] Fix build after a merge --- runtime/common/src/parachains.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/common/src/parachains.rs b/runtime/common/src/parachains.rs index 9d66638c08d0..57e51de29b7c 100644 --- a/runtime/common/src/parachains.rs +++ b/runtime/common/src/parachains.rs @@ -338,7 +338,7 @@ decl_storage! { /// Used for double vote report validation. /// This is not pruned at the moment. pub ParentToSessionIndex get(session_at_block): - map hasher(blake2_256) T::Hash => SessionIndex; + map hasher(twox_64_concat) T::Hash => SessionIndex; } add_extra_genesis { config(authorities): Vec; From 71b858d5784218ab817708ed3b885ebdc6610254 Mon Sep 17 00:00:00 2001 From: Fedor Sakharov Date: Wed, 18 Mar 2020 10:58:43 +0300 Subject: [PATCH 16/18] Apply suggestions from code review Co-Authored-By: Robert Habermeier --- runtime/common/src/parachains.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/common/src/parachains.rs b/runtime/common/src/parachains.rs index 57e51de29b7c..717c09302772 100644 --- a/runtime/common/src/parachains.rs +++ b/runtime/common/src/parachains.rs @@ -276,7 +276,7 @@ pub struct DoubleVoteOffence { } impl Offence for DoubleVoteOffence { - const ID: Kind = *b"double-vote:doub"; + const ID: Kind = *b"para:double-vote"; type TimeSlot = SessionIndex; fn offenders(&self) -> Vec { From 212e94c2fb5eda7c332a9d1d821ed684a3fcf4a3 Mon Sep 17 00:00:00 2001 From: Fedor Sakharov Date: Sat, 21 Mar 2020 12:38:44 +0300 Subject: [PATCH 17/18] Prune ParentToSessionIndex --- runtime/common/src/parachains.rs | 52 ++++++++++++++++++++++++++++++-- runtime/common/src/registrar.rs | 4 +-- 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/runtime/common/src/parachains.rs b/runtime/common/src/parachains.rs index 717c09302772..ae8dd877c5a6 100644 --- a/runtime/common/src/parachains.rs +++ b/runtime/common/src/parachains.rs @@ -203,7 +203,7 @@ impl GetSessionNumber for session::historical::Proof { } } -pub trait Trait: attestations::Trait + session::historical::Trait { +pub trait Trait: attestations::Trait + session::historical::Trait + staking::Trait { /// The outer origin type. type Origin: From + From>; @@ -336,9 +336,14 @@ decl_storage! { /// The mapping from parent block hashes to session indexes. /// /// Used for double vote report validation. - /// This is not pruned at the moment. pub ParentToSessionIndex get(session_at_block): map hasher(twox_64_concat) T::Hash => SessionIndex; + + /// The era that is active currently. + /// + /// Changes with the `ActiveEra` from `staking`. Upon these changes `ParentToSessionIndex` + /// is pruned. + ActiveEra get(active_era): Option; } add_extra_genesis { config(authorities): Vec; @@ -516,6 +521,21 @@ decl_module! { let current_session = >::current_index(); let parent_hash = >::parent_hash(); + match Self::active_era() { + Some(era) => { + if let Some(active_era) = >::current_era() { + if era != active_era { + ::ActiveEra::put(active_era); + >::remove_all(); + } + } + } + None => { + if let Some(active_era) = >::current_era() { + ::ActiveEra::set(Some(active_era)); + } + } + } >::insert(parent_hash, current_session); } @@ -1643,7 +1663,6 @@ mod tests { for i in Session::current_index()..session_index { println!("session index {}", i); - Staking::on_finalize(System::block_number()); System::set_block_number((i + 1).into()); Timestamp::set_timestamp(System::block_number() * 6000); @@ -2700,4 +2719,31 @@ mod tests { } }); } + + #[test] + fn double_vote_hash_to_session_gets_pruned() { + let parachains = vec![ + (1u32.into(), vec![], vec![]), + ]; + + // Test that `ParentToSessionIndex` is pruned upon eras turn. + new_test_ext(parachains.clone()).execute_with(|| { + let candidate = raw_candidate(1.into()).abridge().0; + let candidate_hash = candidate.hash(); + start_era(1); + + let parent_hash_1 = System::parent_hash(); + // The mapping should know about the session at this block. + assert_eq!(Parachains::session_at_block(parent_hash_1), 3); + + start_era(2); + + let parent_hash_2 = System::parent_hash(); + + // Info about a block from pevious era is removed. + assert_eq!(Parachains::session_at_block(parent_hash_1), 0); + // Info about a block form this era is present. + assert_eq!(Parachains::session_at_block(parent_hash_2), 6); + }); + } } diff --git a/runtime/common/src/registrar.rs b/runtime/common/src/registrar.rs index 5b0132807a43..1a401303cd07 100644 --- a/runtime/common/src/registrar.rs +++ b/runtime/common/src/registrar.rs @@ -312,7 +312,7 @@ decl_module! { ) { let who = ensure_signed(origin)?; - T::Currency::reserve(&who, T::ParathreadDeposit::get())?; + ::Currency::reserve(&who, T::ParathreadDeposit::get())?; let info = ParaInfo { scheduling: Scheduling::Dynamic, @@ -371,7 +371,7 @@ decl_module! { Self::force_unschedule(|i| i == id); let debtor = >::take(id); - let _ = T::Currency::unreserve(&debtor, T::ParathreadDeposit::get()); + let _ = ::Currency::unreserve(&debtor, T::ParathreadDeposit::get()); Self::deposit_event(Event::ParathreadRegistered(id)); } From 0f2183d0a8f2a540bf8ae05f5dc9bac5d1644b9c Mon Sep 17 00:00:00 2001 From: Fedor Sakharov Date: Sat, 21 Mar 2020 16:56:19 +0300 Subject: [PATCH 18/18] Remove a clone and a warning --- runtime/common/src/parachains.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/runtime/common/src/parachains.rs b/runtime/common/src/parachains.rs index c0a0ae4645e0..bca0460ab397 100644 --- a/runtime/common/src/parachains.rs +++ b/runtime/common/src/parachains.rs @@ -493,12 +493,13 @@ decl_module! { let validator_set_count = validators.len() as u32; let session_index = report.proof.session(); + let DoubleVoteReport { identity, proof, .. } = report; // We have already checked this proof in `SignedExtension`, but we need // this here to get the full identification of the offender. let offender = T::KeyOwnerProofSystem::check_proof( - (PARACHAIN_KEY_TYPE_ID, report.identity.encode()), - report.proof.clone(), + (PARACHAIN_KEY_TYPE_ID, identity.encode()), + proof, ).ok_or("Invalid/outdated key ownership proof.")?; let offence = DoubleVoteOffence { @@ -2728,8 +2729,6 @@ mod tests { // Test that `ParentToSessionIndex` is pruned upon eras turn. new_test_ext(parachains.clone()).execute_with(|| { - let candidate = raw_candidate(1.into()).abridge().0; - let candidate_hash = candidate.hash(); start_era(1); let parent_hash_1 = System::parent_hash();