grandpa: don't send equivocation reports for local identities (#7372)

* grandpa: don't send equivocation reports for local identities

* grandpa: add test for self-report

* grandpa: fix test compilation

this works on rust nightly but breaks on ci which is using rust stable
This commit is contained in:
André Silva
2020-10-26 12:47:39 +00:00
committed by GitHub
parent 568dd6fd42
commit f2925f96a2
2 changed files with 61 additions and 7 deletions
@@ -42,8 +42,8 @@ use sp_runtime::traits::{
use sc_telemetry::{telemetry, CONSENSUS_DEBUG, CONSENSUS_INFO}; use sc_telemetry::{telemetry, CONSENSUS_DEBUG, CONSENSUS_INFO};
use crate::{ use crate::{
CommandOrError, Commit, Config, Error, Precommit, Prevote, local_authority_id, CommandOrError, Commit, Config, Error, NewAuthoritySet, Precommit, Prevote,
PrimaryPropose, SignedMessage, NewAuthoritySet, VoterCommand, PrimaryPropose, SignedMessage, VoterCommand,
}; };
use sp_consensus::SelectChain; use sp_consensus::SelectChain;
@@ -467,10 +467,18 @@ where
/// extrinsic to report the equivocation. In particular, the session membership /// extrinsic to report the equivocation. In particular, the session membership
/// proof must be generated at the block at which the given set was active which /// proof must be generated at the block at which the given set was active which
/// isn't necessarily the best block if there are pending authority set changes. /// isn't necessarily the best block if there are pending authority set changes.
fn report_equivocation( pub(crate) fn report_equivocation(
&self, &self,
equivocation: Equivocation<Block::Hash, NumberFor<Block>>, equivocation: Equivocation<Block::Hash, NumberFor<Block>>,
) -> Result<(), Error> { ) -> Result<(), Error> {
if let Some(local_id) = local_authority_id(&self.voters, self.config.keystore.as_ref()) {
if *equivocation.offender() == local_id {
return Err(Error::Safety(
"Refraining from sending equivocation report for our own equivocation.".into(),
));
}
}
let is_descendent_of = is_descendent_of(&*self.client, None); let is_descendent_of = is_descendent_of(&*self.client, None);
let best_header = self.select_chain let best_header = self.select_chain
@@ -724,7 +732,7 @@ where
let prevote_timer = Delay::new(self.config.gossip_duration * 2); let prevote_timer = Delay::new(self.config.gossip_duration * 2);
let precommit_timer = Delay::new(self.config.gossip_duration * 4); let precommit_timer = Delay::new(self.config.gossip_duration * 4);
let local_id = crate::local_authority_id(&self.voters, self.config.keystore.as_ref()); let local_id = local_authority_id(&self.voters, self.config.keystore.as_ref());
let has_voted = match self.voter_set_state.has_voted(round) { let has_voted = match self.voter_set_state.has_voted(round) {
HasVoted::Yes(id, vote) => { HasVoted::Yes(id, vote) => {
@@ -776,7 +784,7 @@ where
} }
fn proposed(&self, round: RoundNumber, propose: PrimaryPropose<Block>) -> Result<(), Self::Error> { fn proposed(&self, round: RoundNumber, propose: PrimaryPropose<Block>) -> Result<(), Self::Error> {
let local_id = crate::local_authority_id(&self.voters, self.config.keystore.as_ref()); let local_id = local_authority_id(&self.voters, self.config.keystore.as_ref());
let local_id = match local_id { let local_id = match local_id {
Some(id) => id, Some(id) => id,
@@ -815,7 +823,7 @@ where
} }
fn prevoted(&self, round: RoundNumber, prevote: Prevote<Block>) -> Result<(), Self::Error> { fn prevoted(&self, round: RoundNumber, prevote: Prevote<Block>) -> Result<(), Self::Error> {
let local_id = crate::local_authority_id(&self.voters, self.config.keystore.as_ref()); let local_id = local_authority_id(&self.voters, self.config.keystore.as_ref());
let local_id = match local_id { let local_id = match local_id {
Some(id) => id, Some(id) => id,
@@ -876,7 +884,7 @@ where
round: RoundNumber, round: RoundNumber,
precommit: Precommit<Block>, precommit: Precommit<Block>,
) -> Result<(), Self::Error> { ) -> Result<(), Self::Error> {
let local_id = crate::local_authority_id(&self.voters, self.config.keystore.as_ref()); let local_id = local_authority_id(&self.voters, self.config.keystore.as_ref());
let local_id = match local_id { let local_id = match local_id {
Some(id) => id, Some(id) => id,
@@ -1813,3 +1813,49 @@ fn imports_justification_for_regular_blocks_on_import() {
client.justification(&BlockId::Hash(block_hash)).unwrap().is_some(), client.justification(&BlockId::Hash(block_hash)).unwrap().is_some(),
); );
} }
#[test]
fn grandpa_environment_doesnt_send_equivocation_reports_for_itself() {
let alice = Ed25519Keyring::Alice;
let voters = make_ids(&[alice]);
let environment = {
let mut net = GrandpaTestNet::new(TestApi::new(voters), 1);
let peer = net.peer(0);
let network_service = peer.network_service().clone();
let link = peer.data.lock().take().unwrap();
let (keystore, _keystore_path) = create_keystore(alice);
test_environment(&link, Some(keystore), network_service.clone(), ())
};
let signed_prevote = {
let prevote = finality_grandpa::Prevote {
target_hash: H256::random(),
target_number: 1,
};
let signed = alice.sign(&[]).into();
(prevote, signed)
};
let mut equivocation = finality_grandpa::Equivocation {
round_number: 1,
identity: alice.public().into(),
first: signed_prevote.clone(),
second: signed_prevote.clone(),
};
// reporting the equivocation should fail since the offender is a local
// authority (i.e. we have keys in our keystore for the given id)
let equivocation_proof = sp_finality_grandpa::Equivocation::Prevote(equivocation.clone());
assert!(matches!(
environment.report_equivocation(equivocation_proof),
Err(Error::Safety(_))
));
// if we set the equivocation offender to another id for which we don't have
// keys it should work
equivocation.identity = Default::default();
let equivocation_proof = sp_finality_grandpa::Equivocation::Prevote(equivocation);
assert!(environment.report_equivocation(equivocation_proof).is_ok());
}