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

Commit

Permalink
grandpa: don't send equivocation reports for local identities (#7372)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
andresilva authored Oct 26, 2020
1 parent 755514d commit 891900a
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 7 deletions.
22 changes: 15 additions & 7 deletions client/finality-grandpa/src/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ use sp_runtime::traits::{
use sc_telemetry::{telemetry, CONSENSUS_DEBUG, CONSENSUS_INFO};

use crate::{
CommandOrError, Commit, Config, Error, Precommit, Prevote,
PrimaryPropose, SignedMessage, NewAuthoritySet, VoterCommand,
local_authority_id, CommandOrError, Commit, Config, Error, NewAuthoritySet, Precommit, Prevote,
PrimaryPropose, SignedMessage, VoterCommand,
};

use sp_consensus::SelectChain;
Expand Down Expand Up @@ -467,10 +467,18 @@ where
/// 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
/// isn't necessarily the best block if there are pending authority set changes.
fn report_equivocation(
pub(crate) fn report_equivocation(
&self,
equivocation: Equivocation<Block::Hash, NumberFor<Block>>,
) -> 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 best_header = self.select_chain
Expand Down Expand Up @@ -724,7 +732,7 @@ where
let prevote_timer = Delay::new(self.config.gossip_duration * 2);
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) {
HasVoted::Yes(id, vote) => {
Expand Down Expand Up @@ -776,7 +784,7 @@ where
}

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 {
Some(id) => id,
Expand Down Expand Up @@ -815,7 +823,7 @@ where
}

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 {
Some(id) => id,
Expand Down Expand Up @@ -876,7 +884,7 @@ where
round: RoundNumber,
precommit: Precommit<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 {
Some(id) => id,
Expand Down
46 changes: 46 additions & 0 deletions client/finality-grandpa/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1813,3 +1813,49 @@ fn imports_justification_for_regular_blocks_on_import() {
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());
}

0 comments on commit 891900a

Please sign in to comment.