diff --git a/Cargo.lock b/Cargo.lock index 0b24f9ef57218..47e05bc08cfe9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4355,8 +4355,10 @@ name = "pallet-grandpa" version = "2.0.0-rc4" dependencies = [ "finality-grandpa", + "frame-benchmarking", "frame-support", "frame-system", + "pallet-authorship", "pallet-balances", "pallet-finality-tracker", "pallet-offences", diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index 85010ba394189..04acf8fa7a904 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -397,7 +397,7 @@ impl_runtime_apis! { Grandpa::grandpa_authorities() } - fn submit_report_equivocation_extrinsic( + fn submit_report_equivocation_unsigned_extrinsic( _equivocation_proof: fg_primitives::EquivocationProof< ::Hash, NumberFor, diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs index 521c5bb078662..5074bda6651ef 100644 --- a/bin/node/cli/src/service.rs +++ b/bin/node/cli/src/service.rs @@ -628,7 +628,6 @@ mod tests { let check_nonce = frame_system::CheckNonce::from(index); let check_weight = frame_system::CheckWeight::new(); let payment = pallet_transaction_payment::ChargeTransactionPayment::from(0); - let validate_grandpa_equivocation = pallet_grandpa::ValidateEquivocationReport::new(); let extra = ( check_spec_version, check_tx_version, @@ -637,12 +636,11 @@ mod tests { check_nonce, check_weight, payment, - validate_grandpa_equivocation, ); let raw_payload = SignedPayload::from_raw( function, extra, - (spec_version, transaction_version, genesis_hash, genesis_hash, (), (), (), ()) + (spec_version, transaction_version, genesis_hash, genesis_hash, (), (), ()) ); let signature = raw_payload.using_encoded(|payload| { signer.sign(payload) diff --git a/bin/node/runtime/Cargo.toml b/bin/node/runtime/Cargo.toml index 1d29a592c414c..406507ab366ba 100644 --- a/bin/node/runtime/Cargo.toml +++ b/bin/node/runtime/Cargo.toml @@ -154,6 +154,7 @@ runtime-benchmarks = [ "pallet-collective/runtime-benchmarks", "pallet-democracy/runtime-benchmarks", "pallet-elections-phragmen/runtime-benchmarks", + "pallet-grandpa/runtime-benchmarks", "pallet-identity/runtime-benchmarks", "pallet-im-online/runtime-benchmarks", "pallet-indices/runtime-benchmarks", diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 6e5a67387c601..3ff4296750d44 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -671,7 +671,6 @@ impl frame_system::offchain::CreateSignedTransaction for R frame_system::CheckNonce::::from(nonce), frame_system::CheckWeight::::new(), pallet_transaction_payment::ChargeTransactionPayment::::from(tip), - pallet_grandpa::ValidateEquivocationReport::::new(), ); let raw_payload = SignedPayload::new(call, extra).map_err(|e| { debug::warn!("Unable to create signed payload: {:?}", e); @@ -734,12 +733,8 @@ impl pallet_grandpa::Trait for Runtime { GrandpaId, )>>::IdentificationTuple; - type HandleEquivocation = pallet_grandpa::EquivocationHandler< - Self::KeyOwnerIdentification, - node_primitives::report::ReporterAppCrypto, - Runtime, - Offences, - >; + type HandleEquivocation = + pallet_grandpa::EquivocationHandler; } parameter_types! { @@ -856,7 +851,7 @@ construct_runtime!( Elections: pallet_elections_phragmen::{Module, Call, Storage, Event, Config}, TechnicalMembership: pallet_membership::::{Module, Call, Storage, Event, Config}, FinalityTracker: pallet_finality_tracker::{Module, Call, Inherent}, - Grandpa: pallet_grandpa::{Module, Call, Storage, Config, Event}, + Grandpa: pallet_grandpa::{Module, Call, Storage, Config, Event, ValidateUnsigned}, Treasury: pallet_treasury::{Module, Call, Storage, Config, Event}, Contracts: pallet_contracts::{Module, Call, Config, Storage, Event}, Sudo: pallet_sudo::{Module, Call, Config, Storage, Event}, @@ -898,7 +893,6 @@ pub type SignedExtra = ( frame_system::CheckNonce, frame_system::CheckWeight, pallet_transaction_payment::ChargeTransactionPayment, - pallet_grandpa::ValidateEquivocationReport, ); /// Unchecked extrinsic type as expected by this runtime. pub type UncheckedExtrinsic = generic::UncheckedExtrinsic; @@ -972,7 +966,7 @@ impl_runtime_apis! { Grandpa::grandpa_authorities() } - fn submit_report_equivocation_extrinsic( + fn submit_report_equivocation_unsigned_extrinsic( equivocation_proof: fg_primitives::EquivocationProof< ::Hash, NumberFor, @@ -981,7 +975,7 @@ impl_runtime_apis! { ) -> Option<()> { let key_owner_proof = key_owner_proof.decode()?; - Grandpa::submit_report_equivocation_extrinsic( + Grandpa::submit_unsigned_equivocation_report( equivocation_proof, key_owner_proof, ) @@ -1161,6 +1155,7 @@ impl_runtime_apis! { add_benchmark!(params, batches, pallet_collective, Council); add_benchmark!(params, batches, pallet_democracy, Democracy); add_benchmark!(params, batches, pallet_elections_phragmen, Elections); + add_benchmark!(params, batches, pallet_grandpa, Grandpa); add_benchmark!(params, batches, pallet_identity, Identity); add_benchmark!(params, batches, pallet_im_online, ImOnline); add_benchmark!(params, batches, pallet_indices, Indices); diff --git a/bin/node/testing/src/keyring.rs b/bin/node/testing/src/keyring.rs index efa47a5982194..3413748563633 100644 --- a/bin/node/testing/src/keyring.rs +++ b/bin/node/testing/src/keyring.rs @@ -77,7 +77,6 @@ pub fn signed_extra(nonce: Index, extra_fee: Balance) -> SignedExtra { frame_system::CheckNonce::from(nonce), frame_system::CheckWeight::new(), pallet_transaction_payment::ChargeTransactionPayment::from(extra_fee), - pallet_grandpa::ValidateEquivocationReport::new(), ) } diff --git a/bin/utils/subkey/src/main.rs b/bin/utils/subkey/src/main.rs index 4153e769c97f5..9455e08175aa7 100644 --- a/bin/utils/subkey/src/main.rs +++ b/bin/utils/subkey/src/main.rs @@ -725,7 +725,6 @@ fn create_extrinsic( frame_system::CheckNonce::::from(i), frame_system::CheckWeight::::new(), pallet_transaction_payment::ChargeTransactionPayment::::from(f), - pallet_grandpa::ValidateEquivocationReport::::new(), ) }; let raw_payload = SignedPayload::from_raw( @@ -739,7 +738,6 @@ fn create_extrinsic( (), (), (), - (), ), ); let signature = raw_payload.using_encoded(|payload| signer.sign(payload)).into_runtime(); diff --git a/client/finality-grandpa/src/environment.rs b/client/finality-grandpa/src/environment.rs index cc6497fc72462..0cfab13a6fa11 100644 --- a/client/finality-grandpa/src/environment.rs +++ b/client/finality-grandpa/src/environment.rs @@ -515,12 +515,14 @@ where equivocation, ); - self.client.runtime_api() - .submit_report_equivocation_extrinsic( + self.client + .runtime_api() + .submit_report_equivocation_unsigned_extrinsic( &BlockId::Hash(best_header.hash()), equivocation_proof, key_owner_proof, - ).map_err(Error::Client)?; + ) + .map_err(Error::Client)?; Ok(()) } diff --git a/client/finality-grandpa/src/tests.rs b/client/finality-grandpa/src/tests.rs index 50f9e8eba2357..e2b9671f04df3 100644 --- a/client/finality-grandpa/src/tests.rs +++ b/client/finality-grandpa/src/tests.rs @@ -217,7 +217,7 @@ sp_api::mock_impl_runtime_apis! { self.inner.genesis_authorities.clone() } - fn submit_report_equivocation_extrinsic( + fn submit_report_equivocation_unsigned_extrinsic( _equivocation_proof: EquivocationProof, _key_owner_proof: OpaqueKeyOwnershipProof, ) -> Option<()> { diff --git a/frame/grandpa/Cargo.toml b/frame/grandpa/Cargo.toml index 0f2477d50e8a9..d1479027505ff 100644 --- a/frame/grandpa/Cargo.toml +++ b/frame/grandpa/Cargo.toml @@ -21,12 +21,15 @@ sp-session = { version = "2.0.0-rc4", default-features = false, path = "../../pr sp-std = { version = "2.0.0-rc4", default-features = false, path = "../../primitives/std" } sp-runtime = { version = "2.0.0-rc4", default-features = false, path = "../../primitives/runtime" } sp-staking = { version = "2.0.0-rc4", default-features = false, path = "../../primitives/staking" } +frame-benchmarking = { version = "2.0.0-rc4", default-features = false, path = "../benchmarking", optional = true } frame-support = { version = "2.0.0-rc4", default-features = false, path = "../support" } frame-system = { version = "2.0.0-rc4", default-features = false, path = "../system" } +pallet-authorship = { version = "2.0.0-rc4", default-features = false, path = "../authorship" } pallet-session = { version = "2.0.0-rc4", default-features = false, path = "../session" } pallet-finality-tracker = { version = "2.0.0-rc4", default-features = false, path = "../finality-tracker" } [dev-dependencies] +frame-benchmarking = { version = "2.0.0-rc4", path = "../benchmarking" } grandpa = { package = "finality-grandpa", version = "0.12.3", features = ["derive-codec"] } sp-io = { version = "2.0.0-rc4", path = "../../primitives/io" } sp-keyring = { version = "2.0.0-rc4", path = "../../primitives/keyring" } @@ -41,6 +44,7 @@ default = ["std"] std = [ "serde", "codec/std", + "frame-benchmarking/std", "sp-application-crypto/std", "sp-core/std", "sp-finality-grandpa/std", @@ -50,6 +54,8 @@ std = [ "sp-runtime/std", "sp-staking/std", "frame-system/std", + "pallet-authorship/std", "pallet-session/std", "pallet-finality-tracker/std", ] +runtime-benchmarks = ["frame-benchmarking"] diff --git a/frame/grandpa/src/benchmarking.rs b/frame/grandpa/src/benchmarking.rs new file mode 100644 index 0000000000000..18f6f62fa44ed --- /dev/null +++ b/frame/grandpa/src/benchmarking.rs @@ -0,0 +1,106 @@ +// This file is part of Substrate. + +// Copyright (C) 2020 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Benchmarks for the GRANDPA pallet. + +#![cfg_attr(not(feature = "std"), no_std)] + +use super::*; +use frame_benchmarking::benchmarks; +use sp_core::H256; + +benchmarks! { + _ { } + + check_equivocation_proof { + let x in 0 .. 1; + + // NOTE: generated with the test below `test_generate_equivocation_report_blob`. + // the output should be deterministic since the keys we use are static. + // with the current benchmark setup it is not possible to generate this + // programatically from the benchmark setup. + const EQUIVOCATION_PROOF_BLOB: [u8; 257] = [ + 1, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 136, 220, 52, 23, + 213, 5, 142, 196, 180, 80, 62, 12, 18, 234, 26, 10, 137, 190, 32, + 15, 233, 137, 34, 66, 61, 67, 52, 1, 79, 166, 176, 238, 207, 48, + 195, 55, 171, 225, 252, 130, 161, 56, 151, 29, 193, 32, 25, 157, + 249, 39, 80, 193, 214, 96, 167, 147, 25, 130, 45, 42, 64, 208, 182, + 164, 10, 0, 0, 0, 0, 0, 0, 0, 234, 236, 231, 45, 70, 171, 135, 246, + 136, 153, 38, 167, 91, 134, 150, 242, 215, 83, 56, 238, 16, 119, 55, + 170, 32, 69, 255, 248, 164, 20, 57, 50, 122, 115, 135, 96, 80, 203, + 131, 232, 73, 23, 149, 86, 174, 59, 193, 92, 121, 76, 154, 211, 44, + 96, 10, 84, 159, 133, 211, 56, 103, 0, 59, 2, 96, 20, 69, 2, 32, + 179, 16, 184, 108, 76, 215, 64, 195, 78, 143, 73, 177, 139, 20, 144, + 98, 231, 41, 117, 255, 220, 115, 41, 59, 27, 75, 56, 10, 0, 0, 0, 0, + 0, 0, 0, 128, 179, 250, 48, 211, 76, 10, 70, 74, 230, 219, 139, 96, + 78, 88, 112, 33, 170, 44, 184, 59, 200, 155, 143, 128, 40, 222, 179, + 210, 190, 84, 16, 182, 21, 34, 94, 28, 193, 163, 226, 51, 251, 134, + 233, 187, 121, 63, 157, 240, 165, 203, 92, 16, 146, 120, 190, 229, + 251, 129, 29, 45, 32, 29, 6 + ]; + + let equivocation_proof1: sp_finality_grandpa::EquivocationProof = + Decode::decode(&mut &EQUIVOCATION_PROOF_BLOB[..]).unwrap(); + + let equivocation_proof2 = equivocation_proof1.clone(); + }: { + sp_finality_grandpa::check_equivocation_proof(equivocation_proof1); + } verify { + assert!(sp_finality_grandpa::check_equivocation_proof(equivocation_proof2)); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::mock::*; + use frame_support::assert_ok; + + #[test] + fn test_benchmarks() { + new_test_ext(vec![(1, 1), (2, 1), (3, 1)]).execute_with(|| { + assert_ok!(test_benchmark_check_equivocation_proof::()); + }) + } + + #[test] + fn test_generate_equivocation_report_blob() { + let authorities = crate::tests::test_authorities(); + + let equivocation_authority_index = 0; + let equivocation_key = &authorities[equivocation_authority_index].0; + let equivocation_keyring = extract_keyring(equivocation_key); + + new_test_ext_raw_authorities(authorities).execute_with(|| { + start_era(1); + + // generate an equivocation proof, with two votes in the same round for + // different block hashes signed by the same key + let equivocation_proof = generate_equivocation_proof( + 1, + (1, H256::random(), 10, &equivocation_keyring), + (1, H256::random(), 10, &equivocation_keyring), + ); + + println!("equivocation_proof: {:?}", equivocation_proof); + println!( + "equivocation_proof.encode(): {:?}", + equivocation_proof.encode() + ); + }); + } +} diff --git a/frame/grandpa/src/equivocation.rs b/frame/grandpa/src/equivocation.rs index 9ac1c12128565..e9662a726c40e 100644 --- a/frame/grandpa/src/equivocation.rs +++ b/frame/grandpa/src/equivocation.rs @@ -24,6 +24,7 @@ //! part of a session); //! - a system for reporting offences; //! - a system for signing and submitting transactions; +//! - a way to get the current block author; //! //! These can be used in an offchain context in order to submit equivocation //! reporting extrinsics (from the client that's running the GRANDPA protocol). @@ -32,165 +33,34 @@ //! //! IMPORTANT: //! When using this module for enabling equivocation reporting it is required -//! that the `ValidateEquivocationReport` signed extension is used in the runtime -//! definition. Failure to do so will allow invalid equivocation reports to be -//! accepted by the runtime. +//! that the `ValidateUnsigned` for the GRANDPA pallet is used in the runtime +//! definition. //! use sp_std::prelude::*; use codec::{self as codec, Decode, Encode}; -use frame_support::{debug, dispatch::IsSubType, traits::KeyOwnerProofSystem}; -use frame_system::offchain::{AppCrypto, CreateSignedTransaction, Signer}; +use frame_support::{debug, traits::KeyOwnerProofSystem}; use sp_finality_grandpa::{EquivocationProof, RoundNumber, SetId}; use sp_runtime::{ - traits::{DispatchInfoOf, SignedExtension}, transaction_validity::{ - InvalidTransaction, TransactionValidity, TransactionValidityError, ValidTransaction, + InvalidTransaction, TransactionPriority, TransactionSource, TransactionValidity, + TransactionValidityError, ValidTransaction, }, DispatchResult, Perbill, }; -use sp_session::GetSessionNumber; use sp_staking::{ offence::{Kind, Offence, OffenceError, ReportOffence}, SessionIndex, }; -/// Ensure that equivocation reports are only processed if valid. -#[derive(Encode, Decode, Clone, Eq, PartialEq)] -pub struct ValidateEquivocationReport(sp_std::marker::PhantomData); - -impl Default for ValidateEquivocationReport { - fn default() -> ValidateEquivocationReport { - ValidateEquivocationReport::new() - } -} - -impl ValidateEquivocationReport { - pub fn new() -> ValidateEquivocationReport { - ValidateEquivocationReport(Default::default()) - } -} - -impl sp_std::fmt::Debug for ValidateEquivocationReport { - fn fmt(&self, f: &mut sp_std::fmt::Formatter) -> sp_std::fmt::Result { - write!(f, "ValidateEquivocationReport") - } -} - -/// Custom validity error used when validating equivocation reports. -#[derive(Debug)] -#[repr(u8)] -pub enum ReportEquivocationValidityError { - /// The proof provided in the report is not valid. - InvalidEquivocationProof = 1, - /// The proof provided in the report is not valid. - InvalidKeyOwnershipProof = 2, - /// The set id provided in the report is not valid. - InvalidSetId = 3, - /// The session index provided in the report is not valid. - InvalidSession = 4, -} - -impl From for TransactionValidityError { - fn from(e: ReportEquivocationValidityError) -> TransactionValidityError { - TransactionValidityError::from(InvalidTransaction::Custom(e as u8)) - } -} - -impl SignedExtension for ValidateEquivocationReport -where - ::Call: IsSubType>, -{ - const IDENTIFIER: &'static str = "ValidateEquivocationReport"; - type AccountId = T::AccountId; - type Call = ::Call; - type AdditionalSigned = (); - type Pre = (); - - fn additional_signed( - &self, - ) -> sp_std::result::Result { - Ok(()) - } - - fn validate( - &self, - _who: &Self::AccountId, - call: &Self::Call, - _info: &DispatchInfoOf, - _len: usize, - ) -> TransactionValidity { - let (equivocation_proof, key_owner_proof) = match call.is_sub_type() { - Some(super::Call::report_equivocation(equivocation_proof, key_owner_proof)) => { - (equivocation_proof, key_owner_proof) - } - _ => return Ok(ValidTransaction::default()), - }; - - // validate the key ownership proof extracting the id of the offender. - if let None = T::KeyOwnerProofSystem::check_proof( - ( - sp_finality_grandpa::KEY_TYPE, - equivocation_proof.offender().clone(), - ), - key_owner_proof.clone(), - ) { - return Err(ReportEquivocationValidityError::InvalidKeyOwnershipProof.into()); - } - - // we check the equivocation within the context of its set id (and - // associated session). - let set_id = equivocation_proof.set_id(); - let session_index = key_owner_proof.session(); - - // validate equivocation proof (check votes are different and - // signatures are valid). - if !sp_finality_grandpa::check_equivocation_proof(equivocation_proof.clone()) { - return Err(ReportEquivocationValidityError::InvalidEquivocationProof.into()); - } - - // fetch the current and previous sets last session index. on the - // genesis set there's no previous set. - let previous_set_id_session_index = if set_id == 0 { - None - } else { - let session_index = - if let Some(session_id) = >::session_for_set(set_id - 1) { - session_id - } else { - return Err(ReportEquivocationValidityError::InvalidSetId.into()); - }; - - Some(session_index) - }; - - let set_id_session_index = - if let Some(session_id) = >::session_for_set(set_id) { - session_id - } else { - return Err(ReportEquivocationValidityError::InvalidSetId.into()); - }; - - // check that the session id for the membership proof is within the - // bounds of the set id reported in the equivocation. - if session_index > set_id_session_index || - previous_set_id_session_index - .map(|previous_index| session_index <= previous_index) - .unwrap_or(false) - { - return Err(ReportEquivocationValidityError::InvalidSession.into()); - } - - Ok(ValidTransaction::default()) - } -} +use super::{Call, Module, Trait}; /// A trait with utility methods for handling equivocation reports in GRANDPA. /// The offence type is generic, and the trait provides , reporting an offence /// triggered by a valid equivocation report, and also for creating and /// submitting equivocation report extrinsics (useful only in offchain context). -pub trait HandleEquivocation { +pub trait HandleEquivocation { /// The offence type used for reporting offences on valid equivocation reports. type Offence: GrandpaOffence; @@ -200,14 +70,23 @@ pub trait HandleEquivocation { offence: Self::Offence, ) -> Result<(), OffenceError>; + /// Returns true if all of the offenders at the given time slot have already been reported. + fn is_known_offence( + offenders: &[T::KeyOwnerIdentification], + time_slot: &>::TimeSlot, + ) -> bool; + /// Create and dispatch an equivocation report extrinsic. - fn submit_equivocation_report( + fn submit_unsigned_equivocation_report( equivocation_proof: EquivocationProof, key_owner_proof: T::KeyOwnerProof, ) -> DispatchResult; + + /// Fetch the current block author id, if defined. + fn block_author() -> Option; } -impl HandleEquivocation for () { +impl HandleEquivocation for () { type Offence = GrandpaEquivocationOffence; fn report_offence( @@ -217,23 +96,34 @@ impl HandleEquivocation for () { Ok(()) } - fn submit_equivocation_report( + fn is_known_offence( + _offenders: &[T::KeyOwnerIdentification], + _time_slot: &GrandpaTimeSlot, + ) -> bool { + true + } + + fn submit_unsigned_equivocation_report( _equivocation_proof: EquivocationProof, _key_owner_proof: T::KeyOwnerProof, ) -> DispatchResult { Ok(()) } + + fn block_author() -> Option { + None + } } /// Generic equivocation handler. This type implements `HandleEquivocation` /// using existing subsystems that are part of frame (type bounds described /// below) and will dispatch to them directly, it's only purpose is to wire all /// subsystems together. -pub struct EquivocationHandler> { - _phantom: sp_std::marker::PhantomData<(I, C, S, R, O)>, +pub struct EquivocationHandler> { + _phantom: sp_std::marker::PhantomData<(I, R, O)>, } -impl Default for EquivocationHandler { +impl Default for EquivocationHandler { fn default() -> Self { Self { _phantom: Default::default(), @@ -241,18 +131,17 @@ impl Default for EquivocationHandler { } } -impl HandleEquivocation - for EquivocationHandler +impl HandleEquivocation for EquivocationHandler where - // A signed transaction creator. Used for signing and submitting equivocation reports. - T: super::Trait + CreateSignedTransaction>, - // Application-specific crypto bindings. - C: AppCrypto, - // The offence type that should be used when reporting. - O: GrandpaOffence, + // We use the authorship pallet to fetch the current block author and use + // `offchain::SendTransactionTypes` for unsigned extrinsic creation and + // submission. + T: Trait + pallet_authorship::Trait + frame_system::offchain::SendTransactionTypes>, // A system for reporting offences after valid equivocation reports are // processed. R: ReportOffence, + // The offence type that should be used when reporting. + O: GrandpaOffence, { type Offence = O; @@ -260,36 +149,29 @@ where R::report_offence(reporters, offence) } - fn submit_equivocation_report( + fn is_known_offence(offenders: &[T::KeyOwnerIdentification], time_slot: &O::TimeSlot) -> bool { + R::is_known_offence(offenders, time_slot) + } + + fn submit_unsigned_equivocation_report( equivocation_proof: EquivocationProof, key_owner_proof: T::KeyOwnerProof, ) -> DispatchResult { - use frame_system::offchain::SendSignedTransaction; + use frame_system::offchain::SubmitTransaction; - let signer = Signer::::all_accounts(); - if !signer.can_sign() { - return Err( - "No local accounts available. Consider adding one via `author_insertKey` RPC.", - )?; - } + let call = Call::report_equivocation_unsigned(equivocation_proof, key_owner_proof); - let results = signer.send_signed_transaction(|_account| { - super::Call::report_equivocation(equivocation_proof.clone(), key_owner_proof.clone()) - }); - - for (acc, res) in &results { - match res { - Ok(()) => debug::info!("[{:?}] Submitted GRANDPA equivocation report.", acc.id), - Err(e) => debug::error!( - "[{:?}] Error submitting equivocation report: {:?}", - acc.id, - e - ), - } + match SubmitTransaction::>::submit_unsigned_transaction(call.into()) { + Ok(()) => debug::info!("Submitted GRANDPA equivocation report."), + Err(e) => debug::error!("Error submitting equivocation report: {:?}", e), } Ok(()) } + + fn block_author() -> Option { + Some(>::author()) + } } /// A round number and set id which point on the time of an offence. @@ -302,6 +184,75 @@ pub struct GrandpaTimeSlot { pub round: RoundNumber, } +/// A `ValidateUnsigned` implementation that restricts calls to `report_equivocation_unsigned` +/// to local calls (i.e. extrinsics generated on this node) or that already in a block. This +/// guarantees that only block authors can include unsigned equivocation reports. +impl frame_support::unsigned::ValidateUnsigned for Module { + type Call = Call; + fn validate_unsigned(source: TransactionSource, call: &Self::Call) -> TransactionValidity { + if let Call::report_equivocation_unsigned(equivocation_proof, _) = call { + // discard equivocation report not coming from the local node + match source { + TransactionSource::Local | TransactionSource::InBlock => { /* allowed */ } + _ => { + debug::warn!( + target: "afg", + "rejecting unsigned report equivocation transaction because it is not local/in-block." + ); + + return InvalidTransaction::Call.into(); + } + } + + ValidTransaction::with_tag_prefix("GrandpaEquivocation") + // We assign the maximum priority for any equivocation report. + .priority(TransactionPriority::max_value()) + // Only one equivocation report for the same offender at the same slot. + .and_provides(( + equivocation_proof.offender().clone(), + equivocation_proof.set_id(), + equivocation_proof.round(), + )) + // We don't propagate this. This can never be included on a remote node. + .propagate(false) + .build() + } else { + InvalidTransaction::Call.into() + } + } + + fn pre_dispatch(call: &Self::Call) -> Result<(), TransactionValidityError> { + if let Call::report_equivocation_unsigned(equivocation_proof, key_owner_proof) = call { + // check the membership proof to extract the offender's id + let key = ( + sp_finality_grandpa::KEY_TYPE, + equivocation_proof.offender().clone(), + ); + + let offender = T::KeyOwnerProofSystem::check_proof(key, key_owner_proof.clone()) + .ok_or(InvalidTransaction::BadProof)?; + + // check if the offence has already been reported, + // and if so then we can discard the report. + let time_slot = + >::Offence::new_time_slot( + equivocation_proof.set_id(), + equivocation_proof.round(), + ); + + let is_known_offence = T::HandleEquivocation::is_known_offence(&[offender], &time_slot); + + if is_known_offence { + Err(InvalidTransaction::Stale.into()) + } else { + Ok(()) + } + } else { + Err(InvalidTransaction::Call.into()) + } + } +} + /// A grandpa equivocation offence report. #[allow(dead_code)] pub struct GrandpaEquivocationOffence { @@ -327,6 +278,9 @@ pub trait GrandpaOffence: Offence { set_id: SetId, round: RoundNumber, ) -> Self; + + /// Create a new GRANDPA offence time slot. + fn new_time_slot(set_id: SetId, round: RoundNumber) -> Self::TimeSlot; } impl GrandpaOffence @@ -346,6 +300,10 @@ impl GrandpaOffence time_slot: GrandpaTimeSlot { set_id, round }, } } + + fn new_time_slot(set_id: SetId, round: RoundNumber) -> Self::TimeSlot { + GrandpaTimeSlot { set_id, round } + } } impl Offence diff --git a/frame/grandpa/src/lib.rs b/frame/grandpa/src/lib.rs index 91d783cb1ad7a..773a78529e372 100644 --- a/frame/grandpa/src/lib.rs +++ b/frame/grandpa/src/lib.rs @@ -43,7 +43,7 @@ use frame_support::{ decl_error, decl_event, decl_module, decl_storage, storage, traits::KeyOwnerProofSystem, Parameter, }; -use frame_system::{ensure_signed, DigestOf}; +use frame_system::{ensure_none, ensure_signed, DigestOf}; use sp_runtime::{ generic::{DigestItem, OpaqueDigestItemId}, traits::Zero, @@ -53,6 +53,9 @@ use sp_session::{GetSessionNumber, GetValidatorCount}; use sp_staking::SessionIndex; mod equivocation; + +#[cfg(any(feature = "runtime-benchmarks", test))] +mod benchmarking; #[cfg(all(feature = "std", test))] mod mock; #[cfg(all(feature = "std", test))] @@ -60,7 +63,7 @@ mod tests; pub use equivocation::{ EquivocationHandler, GrandpaEquivocationOffence, GrandpaOffence, GrandpaTimeSlot, - HandleEquivocation, ValidateEquivocationReport, + HandleEquivocation, }; pub trait Trait: frame_system::Trait { @@ -90,9 +93,8 @@ pub trait Trait: frame_system::Trait { /// offence (after the equivocation has been validated) and for submitting a /// transaction to report an equivocation (from an offchain context). /// NOTE: when enabling equivocation handling (i.e. this type isn't set to - /// `()`) you must add the `equivocation::ValidateEquivocationReport` signed - /// extension to the runtime's `SignedExtra` definition, otherwise - /// equivocation reports won't be properly validated. + /// `()`) you must use this pallet's `ValidateUnsigned` in the runtime + /// definition. type HandleEquivocation: HandleEquivocation; } @@ -190,6 +192,8 @@ decl_error! { TooSoon, /// A key ownership proof provided as part of an equivocation report is invalid. InvalidKeyOwnershipProof, + /// An equivocation proof provided as part of an equivocation report is invalid. + InvalidEquivocationProof, /// A given equivocation report is valid but already previously reported. DuplicateOffenceReport, } @@ -237,46 +241,43 @@ decl_module! { /// equivocation proof and validate the given key ownership proof /// against the extracted offender. If both are valid, the offence /// will be reported. - /// - /// Since the weight of the extrinsic is 0, in order to avoid DoS by - /// submission of invalid equivocation reports, a mandatory pre-validation of - /// the extrinsic is implemented in a `SignedExtension`. - #[weight = 0] + #[weight = weight_for::report_equivocation::(key_owner_proof.validator_count())] fn report_equivocation( origin, equivocation_proof: EquivocationProof, key_owner_proof: T::KeyOwnerProof, ) { - let reporter_id = ensure_signed(origin)?; + let reporter = ensure_signed(origin)?; - let (session_index, validator_set_count) = ( - key_owner_proof.session(), - key_owner_proof.validator_count(), - ); + Self::do_report_equivocation( + Some(reporter), + equivocation_proof, + key_owner_proof, + )?; + } - // we have already checked this proof in `SignedExtension`, we to - // check it again to get the full identification of the offender. - let offender = - T::KeyOwnerProofSystem::check_proof( - (fg_primitives::KEY_TYPE, equivocation_proof.offender().clone()), - key_owner_proof, - ).ok_or(Error::::InvalidKeyOwnershipProof)?; - - // the set id and round when the offence happened - let set_id = equivocation_proof.set_id(); - let round = equivocation_proof.round(); - - // report to the offences module rewarding the sender. - T::HandleEquivocation::report_offence( - vec![reporter_id], - >::Offence::new( - session_index, - validator_set_count, - offender, - set_id, - round, - ), - ).map_err(|_| Error::::DuplicateOffenceReport)?; + /// Report voter equivocation/misbehavior. This method will verify the + /// equivocation proof and validate the given key ownership proof + /// against the extracted offender. If both are valid, the offence + /// will be reported. + /// + /// This extrinsic must be called unsigned and it is expected that only + /// block authors will call it (validated in `ValidateUnsigned`), as such + /// if the block author is defined it will be defined as the equivocation + /// reporter. + #[weight = weight_for::report_equivocation::(key_owner_proof.validator_count())] + fn report_equivocation_unsigned( + origin, + equivocation_proof: EquivocationProof, + key_owner_proof: T::KeyOwnerProof, + ) { + ensure_none(origin)?; + + Self::do_report_equivocation( + T::HandleEquivocation::block_author(), + equivocation_proof, + key_owner_proof, + )?; } fn on_finalize(block_number: T::BlockNumber) { @@ -344,6 +345,40 @@ decl_module! { } } +mod weight_for { + use frame_support::{ + traits::Get, + weights::{ + constants::{WEIGHT_PER_MICROS, WEIGHT_PER_NANOS}, + Weight, + }, + }; + + pub fn report_equivocation(validator_count: u32) -> Weight { + // we take the validator set count from the membership proof to + // calculate the weight but we set a floor of 100 validators. + let validator_count = validator_count.min(100) as u64; + + // worst case we are considering is that the given offender + // is backed by 200 nominators + const MAX_NOMINATORS: u64 = 200; + + // checking membership proof + (35 * WEIGHT_PER_MICROS) + .saturating_add((175 * WEIGHT_PER_NANOS).saturating_mul(validator_count)) + .saturating_add(T::DbWeight::get().reads(5)) + // check equivocation proof + .saturating_add(95 * WEIGHT_PER_MICROS) + // report offence + .saturating_add(110 * WEIGHT_PER_MICROS) + .saturating_add(25 * WEIGHT_PER_MICROS * MAX_NOMINATORS) + .saturating_add(T::DbWeight::get().reads(14 + 3 * MAX_NOMINATORS)) + .saturating_add(T::DbWeight::get().writes(10 + 3 * MAX_NOMINATORS)) + // fetching set id -> session index mappings + .saturating_add(T::DbWeight::get().reads(2)) + } +} + impl Module { /// Get the current set of authorities, along with their respective weights. pub fn grandpa_authorities() -> AuthorityList { @@ -457,15 +492,91 @@ impl Module { SetIdSession::insert(0, 0); } - /// Submits an extrinsic to report an equivocation. This method will sign an - /// extrinsic with a call to `report_equivocation` with any reporting keys - /// available in the keystore and will push the transaction to the pool. - /// Only useful in an offchain context. - pub fn submit_report_equivocation_extrinsic( + fn do_report_equivocation( + reporter: Option, + equivocation_proof: EquivocationProof, + key_owner_proof: T::KeyOwnerProof, + ) -> Result<(), Error> { + // we check the equivocation within the context of its set id (and + // associated session) and round. we also need to know the validator + // set count when the offence since it is required to calculate the + // slash amount. + let set_id = equivocation_proof.set_id(); + let round = equivocation_proof.round(); + let session_index = key_owner_proof.session(); + let validator_count = key_owner_proof.validator_count(); + + // validate the key ownership proof extracting the id of the offender. + let offender = + T::KeyOwnerProofSystem::check_proof( + (fg_primitives::KEY_TYPE, equivocation_proof.offender().clone()), + key_owner_proof, + ).ok_or(Error::::InvalidKeyOwnershipProof)?; + + // validate equivocation proof (check votes are different and + // signatures are valid). + if !sp_finality_grandpa::check_equivocation_proof(equivocation_proof) { + return Err(Error::::InvalidEquivocationProof.into()); + } + + // fetch the current and previous sets last session index. on the + // genesis set there's no previous set. + let previous_set_id_session_index = if set_id == 0 { + None + } else { + let session_index = + if let Some(session_id) = Self::session_for_set(set_id - 1) { + session_id + } else { + return Err(Error::::InvalidEquivocationProof.into()); + }; + + Some(session_index) + }; + + let set_id_session_index = + if let Some(session_id) = Self::session_for_set(set_id) { + session_id + } else { + return Err(Error::::InvalidEquivocationProof.into()); + }; + + // check that the session id for the membership proof is within the + // bounds of the set id reported in the equivocation. + if session_index > set_id_session_index || + previous_set_id_session_index + .map(|previous_index| session_index <= previous_index) + .unwrap_or(false) + { + return Err(Error::::InvalidEquivocationProof.into()); + } + + // report to the offences module rewarding the sender. + T::HandleEquivocation::report_offence( + reporter.into_iter().collect(), + >::Offence::new( + session_index, + validator_count, + offender, + set_id, + round, + ), + ).map_err(|_| Error::::DuplicateOffenceReport) + } + + /// Submits an extrinsic to report an equivocation. This method will create + /// an unsigned extrinsic with a call to `report_equivocation_unsigned` and + /// will push the transaction to the pool. Only useful in an offchain + /// context. + pub fn submit_unsigned_equivocation_report( equivocation_proof: EquivocationProof, key_owner_proof: T::KeyOwnerProof, ) -> Option<()> { - T::HandleEquivocation::submit_equivocation_report(equivocation_proof, key_owner_proof).ok() + T::HandleEquivocation::submit_unsigned_equivocation_report( + equivocation_proof, + key_owner_proof, + ) + .ok() } } diff --git a/frame/grandpa/src/mock.rs b/frame/grandpa/src/mock.rs index 991ada4fbf35b..6291a2f82f102 100644 --- a/frame/grandpa/src/mock.rs +++ b/frame/grandpa/src/mock.rs @@ -19,16 +19,13 @@ #![cfg(test)] -use crate::{ - equivocation::ValidateEquivocationReport, AuthorityId, AuthorityList, Call as GrandpaCall, - ConsensusLog, Module, Trait, -}; +use crate::{AuthorityId, AuthorityList, ConsensusLog, Module, Trait}; use ::grandpa as finality_grandpa; use codec::Encode; use frame_support::{ impl_outer_dispatch, impl_outer_event, impl_outer_origin, parameter_types, traits::{KeyOwnerProofSystem, OnFinalize, OnInitialize}, - weights::{DispatchInfo, Weight}, + weights::Weight, }; use pallet_staking::EraIndex; use sp_core::{crypto::KeyTypeId, H256}; @@ -39,11 +36,7 @@ use sp_runtime::{ curve::PiecewiseLinear, impl_opaque_keys, testing::{Header, TestXt, UintAuthorityId}, - traits::{ - Convert, Extrinsic as ExtrinsicT, IdentityLookup, OpaqueKeys, SaturatedConversion, - SignedExtension, - }, - transaction_validity::TransactionValidityError, + traits::{Convert, IdentityLookup, OpaqueKeys, SaturatedConversion}, DigestItem, Perbill, }; use sp_staking::SessionIndex; @@ -154,6 +147,17 @@ impl session::historical::Trait for Test { type FullIdentificationOf = staking::ExposureOf; } +parameter_types! { + pub const UncleGenerations: u64 = 0; +} + +impl pallet_authorship::Trait for Test { + type FindAuthor = (); + type UncleGenerations = UncleGenerations; + type FilterUncle = (); + type EventHandler = (); +} + parameter_types! { pub const ExistentialDeposit: u128 = 1; } @@ -264,66 +268,7 @@ impl Trait for Test { AuthorityId, )>>::IdentificationTuple; - type HandleEquivocation = super::EquivocationHandler< - Self::KeyOwnerIdentification, - reporting_keys::ReporterAppCrypto, - Test, - Offences, - >; -} - -pub mod reporting_keys { - use sp_core::crypto::KeyTypeId; - - pub const KEY_TYPE: KeyTypeId = KeyTypeId(*b"test"); - - mod app { - use sp_application_crypto::{app_crypto, ed25519}; - app_crypto!(ed25519, super::KEY_TYPE); - - impl sp_runtime::traits::IdentifyAccount for Public { - type AccountId = u64; - fn into_account(self) -> Self::AccountId { - super::super::Grandpa::grandpa_authorities() - .iter() - .map(|(k, _)| k) - .position(|b| *b == self.0.clone().into()) - .unwrap() as u64 - } - } - } - - pub type ReporterId = app::Public; - - pub struct ReporterAppCrypto; - impl frame_system::offchain::AppCrypto - for ReporterAppCrypto - { - type RuntimeAppPublic = ReporterId; - type GenericSignature = sp_core::ed25519::Signature; - type GenericPublic = sp_core::ed25519::Public; - } -} - -type Extrinsic = TestXt; - -impl system::offchain::CreateSignedTransaction for Test -where - Call: From, -{ - fn create_transaction>( - call: Call, - _public: reporting_keys::ReporterId, - _account: ::AccountId, - nonce: ::Index, - ) -> Option<(Call, ::SignaturePayload)> { - Some((call, (nonce, ()))) - } -} - -impl frame_system::offchain::SigningTypes for Test { - type Public = reporting_keys::ReporterId; - type Signature = sp_core::ed25519::Signature; + type HandleEquivocation = super::EquivocationHandler; } mod grandpa { @@ -468,18 +413,6 @@ pub fn initialize_block(number: u64, parent_hash: H256) { ); } -pub fn report_equivocation( - equivocation_proof: sp_finality_grandpa::EquivocationProof, - key_owner_proof: sp_session::MembershipProof, -) -> Result, TransactionValidityError> { - let inner = GrandpaCall::report_equivocation(equivocation_proof, key_owner_proof); - let call = Call::Grandpa(inner.clone()); - - ValidateEquivocationReport::::new().validate(&0, &call, &DispatchInfo::default(), 0)?; - - Ok(inner) -} - pub fn generate_equivocation_proof( set_id: SetId, vote1: (RoundNumber, H256, u64, &Ed25519Keyring), diff --git a/frame/grandpa/src/tests.rs b/frame/grandpa/src/tests.rs index 5f901f227668f..f4b353c0fa0c6 100644 --- a/frame/grandpa/src/tests.rs +++ b/frame/grandpa/src/tests.rs @@ -19,13 +19,13 @@ #![cfg(test)] -use super::*; +use super::{Call, *}; use crate::mock::*; use codec::{Decode, Encode}; use fg_primitives::ScheduledChange; use frame_support::{ assert_err, assert_ok, - traits::{Currency, OnFinalize, UnfilteredDispatchable}, + traits::{Currency, OnFinalize}, }; use frame_system::{EventRecord, Phase}; use sp_core::H256; @@ -316,7 +316,9 @@ fn time_slot_have_sane_ord() { assert!(FIXTURE.windows(2).all(|f| f[0] < f[1])); } -fn test_authorities() -> AuthorityList { +/// Returns a list with 3 authorities with known keys: +/// Alice, Bob and Charlie. +pub fn test_authorities() -> AuthorityList { let authorities = vec![ Ed25519Keyring::Alice, Ed25519Keyring::Bob, @@ -375,8 +377,13 @@ fn report_equivocation_current_set_works() { Historical::prove((sp_finality_grandpa::KEY_TYPE, &equivocation_key)).unwrap(); // report the equivocation and the tx should be dispatched successfully - let inner = report_equivocation(equivocation_proof, key_owner_proof).unwrap(); - assert_ok!(inner.dispatch_bypass_filter(Origin::signed(1))); + assert_ok!( + Grandpa::report_equivocation_unsigned( + Origin::none(), + equivocation_proof, + key_owner_proof, + ), + ); start_era(2); @@ -456,8 +463,13 @@ fn report_equivocation_old_set_works() { // report the equivocation using the key ownership proof generated on // the old set, the tx should be dispatched successfully - let inner = report_equivocation(equivocation_proof, key_owner_proof).unwrap(); - assert_ok!(inner.dispatch_bypass_filter(Origin::signed(1))); + assert_ok!( + Grandpa::report_equivocation_unsigned( + Origin::none(), + equivocation_proof, + key_owner_proof, + ), + ); start_era(3); @@ -516,10 +528,14 @@ fn report_equivocation_invalid_set_id() { (1, H256::random(), 10, &equivocation_keyring), ); - // it should be filtered by the signed extension validation + // the call for reporting the equivocation should error assert_err!( - report_equivocation(equivocation_proof, key_owner_proof), - equivocation::ReportEquivocationValidityError::InvalidSetId, + Grandpa::report_equivocation_unsigned( + Origin::none(), + equivocation_proof, + key_owner_proof, + ), + Error::::InvalidEquivocationProof, ); }); } @@ -555,8 +571,12 @@ fn report_equivocation_invalid_session() { // report an equivocation for the current set using an key ownership // proof from the previous set, the session should be invalid. assert_err!( - report_equivocation(equivocation_proof, key_owner_proof), - equivocation::ReportEquivocationValidityError::InvalidSession, + Grandpa::report_equivocation_unsigned( + Origin::none(), + equivocation_proof, + key_owner_proof, + ), + Error::::InvalidEquivocationProof, ); }); } @@ -596,8 +616,12 @@ fn report_equivocation_invalid_key_owner_proof() { // report an equivocation for the current set using a key ownership // proof for a different key than the one in the equivocation proof. assert_err!( - report_equivocation(equivocation_proof, invalid_key_owner_proof), - equivocation::ReportEquivocationValidityError::InvalidKeyOwnershipProof, + Grandpa::report_equivocation_unsigned( + Origin::none(), + equivocation_proof, + invalid_key_owner_proof, + ), + Error::::InvalidKeyOwnershipProof, ); }); } @@ -623,8 +647,12 @@ fn report_equivocation_invalid_equivocation_proof() { let assert_invalid_equivocation_proof = |equivocation_proof| { assert_err!( - report_equivocation(equivocation_proof, key_owner_proof.clone()), - equivocation::ReportEquivocationValidityError::InvalidEquivocationProof, + Grandpa::report_equivocation_unsigned( + Origin::none(), + equivocation_proof, + key_owner_proof.clone(), + ), + Error::::InvalidEquivocationProof, ); }; @@ -660,3 +688,82 @@ fn report_equivocation_invalid_equivocation_proof() { )); }); } + +#[test] +fn report_equivocation_validate_unsigned_prevents_duplicates() { + use sp_runtime::transaction_validity::{ + InvalidTransaction, TransactionLongevity, TransactionPriority, TransactionSource, + TransactionValidity, ValidTransaction, + }; + + let authorities = test_authorities(); + + new_test_ext_raw_authorities(authorities).execute_with(|| { + start_era(1); + + let authorities = Grandpa::grandpa_authorities(); + + // generate and report an equivocation for the validator at index 0 + let equivocation_authority_index = 0; + let equivocation_key = &authorities[equivocation_authority_index].0; + let equivocation_keyring = extract_keyring(equivocation_key); + let set_id = Grandpa::current_set_id(); + + let equivocation_proof = generate_equivocation_proof( + set_id, + (1, H256::random(), 10, &equivocation_keyring), + (1, H256::random(), 10, &equivocation_keyring), + ); + + let key_owner_proof = + Historical::prove((sp_finality_grandpa::KEY_TYPE, &equivocation_key)).unwrap(); + + let call = Call::report_equivocation_unsigned( + equivocation_proof.clone(), + key_owner_proof.clone(), + ); + + // only local/inblock reports are allowed + assert_eq!( + ::validate_unsigned( + TransactionSource::External, + &call, + ), + InvalidTransaction::Call.into(), + ); + + // the transaction is valid when passed as local + let tx_tag = ( + equivocation_key, + set_id, + 1u64, + ); + + assert_eq!( + ::validate_unsigned( + TransactionSource::Local, + &call, + ), + TransactionValidity::Ok(ValidTransaction { + priority: TransactionPriority::max_value(), + requires: vec![], + provides: vec![("GrandpaEquivocation", tx_tag).encode()], + longevity: TransactionLongevity::max_value(), + propagate: false, + }) + ); + + // the pre dispatch checks should also pass + assert_ok!(::pre_dispatch(&call)); + + // we submit the report + Grandpa::report_equivocation_unsigned(Origin::none(), equivocation_proof, key_owner_proof) + .unwrap(); + + // the report should now be considered stale and the transaction is invalid + assert_err!( + ::pre_dispatch(&call), + InvalidTransaction::Stale, + ); + }); +} diff --git a/frame/offences/benchmarking/src/lib.rs b/frame/offences/benchmarking/src/lib.rs index d0cc1bce225b5..b47c14296a08e 100644 --- a/frame/offences/benchmarking/src/lib.rs +++ b/frame/offences/benchmarking/src/lib.rs @@ -282,21 +282,16 @@ benchmarks! { } report_offence_grandpa { - let r in 1 .. MAX_REPORTERS; let n in 0 .. MAX_NOMINATORS.min(MAX_NOMINATIONS as u32); - let o = 1; - // Make r reporters - let mut reporters = vec![]; - for i in 0 .. r { - let reporter = account("reporter", i, SEED); - reporters.push(reporter); - } + // for grandpa equivocation reports the number of reporters + // and offenders is always 1 + let reporters = vec![account("reporter", 1, SEED)]; // make sure reporters actually get rewarded Staking::::set_slash_reward_fraction(Perbill::one()); - let (mut offenders, raw_offenders) = make_offenders::(o, n)?; + let (mut offenders, raw_offenders) = make_offenders::(1, n)?; let keys = ImOnline::::keys(); let offence = GrandpaEquivocationOffence { @@ -316,9 +311,9 @@ benchmarks! { assert_eq!( System::::event_count(), 0 + 1 // offence - + 2 * r // reporter (reward + endowment) - + o // offenders slashed - + o * n // nominators slashed + + 2 // reporter (reward + endowment) + + 1 // offenders slashed + + n // nominators slashed ); } diff --git a/primitives/finality-grandpa/src/lib.rs b/primitives/finality-grandpa/src/lib.rs index f99880041c0c0..44f5ac8d7b936 100644 --- a/primitives/finality-grandpa/src/lib.rs +++ b/primitives/finality-grandpa/src/lib.rs @@ -497,16 +497,15 @@ sp_api::decl_runtime_apis! { /// is finalized by the authorities from block B-1. fn grandpa_authorities() -> AuthorityList; - /// Submits an extrinsic to report an equivocation. The caller must - /// provide the equivocation proof and a key ownership proof (should be - /// obtained using `generate_key_ownership_proof`). This method will - /// sign the extrinsic with any reporting keys available in the keystore - /// and will push the transaction to the pool. This method returns `None` - /// when creation of the extrinsic fails, either due to unavailability - /// of keys to sign, or because equivocation reporting is disabled for - /// the given runtime (i.e. this method is hardcoded to return `None`). - /// Only useful in an offchain context. - fn submit_report_equivocation_extrinsic( + /// Submits an unsigned extrinsic to report an equivocation. The caller + /// must provide the equivocation proof and a key ownership proof + /// (should be obtained using `generate_key_ownership_proof`). The + /// extrinsic will be unsigned and should only be accepted for local + /// authorship (not to be broadcast to the network). This method returns + /// `None` when creation of the extrinsic fails, e.g. if equivocation + /// reporting is disabled for the given runtime (i.e. this method is + /// hardcoded to return `None`). Only useful in an offchain context. + fn submit_report_equivocation_unsigned_extrinsic( equivocation_proof: EquivocationProof>, key_owner_proof: OpaqueKeyOwnershipProof, ) -> Option<()>; diff --git a/test-utils/runtime/src/lib.rs b/test-utils/runtime/src/lib.rs index 002658fe97784..0ce6ca3c566f0 100644 --- a/test-utils/runtime/src/lib.rs +++ b/test-utils/runtime/src/lib.rs @@ -750,7 +750,7 @@ cfg_if! { Vec::new() } - fn submit_report_equivocation_extrinsic( + fn submit_report_equivocation_unsigned_extrinsic( _equivocation_proof: sp_finality_grandpa::EquivocationProof< ::Hash, NumberFor,