diff --git a/substrate/core/finality-grandpa/src/communication/mod.rs b/substrate/core/finality-grandpa/src/communication/mod.rs index 6f43b1106a54e8aaf161f32f008131966b0bdb7f..9a6a40fb8d26b18df20d908449cf542de1b0f450 100644 --- a/substrate/core/finality-grandpa/src/communication/mod.rs +++ b/substrate/core/finality-grandpa/src/communication/mod.rs @@ -683,7 +683,7 @@ impl<B: BlockT, N: Network<B>> Clone for NetworkBridge<B, N> { } } -fn localized_payload<E: Encode>(round: RoundNumber, set_id: SetIdNumber, message: &E) -> Vec<u8> { +pub(crate) fn localized_payload<E: Encode>(round: RoundNumber, set_id: SetIdNumber, message: &E) -> Vec<u8> { (message, round, set_id).encode() } diff --git a/substrate/core/finality-grandpa/src/import.rs b/substrate/core/finality-grandpa/src/import.rs index 758f6f18dbb0100b07c1ee4be85e9e1dd0e8ec5f..8fbe0791e8c058132e7c091f623b8162dadfc47e 100644 --- a/substrate/core/finality-grandpa/src/import.rs +++ b/substrate/core/finality-grandpa/src/import.rs @@ -465,17 +465,15 @@ impl<B, E, Block: BlockT<Hash=H256>, RA, SC> BlockImport<Block> _ => {}, } - if !needs_justification && !enacts_consensus_change { - return Ok(ImportResult::Imported(imported_aux)); - } - match justification { Some(justification) => { self.import_justification(hash, number, justification, needs_justification).unwrap_or_else(|err| { - debug!(target: "finality", "Imported block #{} that enacts authority set change with \ - invalid justification: {:?}, requesting justification from peers.", number, err); - imported_aux.bad_justification = true; - imported_aux.needs_justification = true; + if needs_justification || enacts_consensus_change { + debug!(target: "finality", "Imported block #{} that enacts authority set change with \ + invalid justification: {:?}, requesting justification from peers.", number, err); + imported_aux.bad_justification = true; + imported_aux.needs_justification = true; + } }); }, None => { diff --git a/substrate/core/finality-grandpa/src/justification.rs b/substrate/core/finality-grandpa/src/justification.rs index b4de8ff058684dbba3f452b7c65096143c685bb0..f5965df3e1228b38eea7b44d50929727872df688 100644 --- a/substrate/core/finality-grandpa/src/justification.rs +++ b/substrate/core/finality-grandpa/src/justification.rs @@ -39,7 +39,7 @@ use crate::communication; /// This is meant to be stored in the db and passed around the network to other /// nodes, and are used by syncing nodes to prove authority set handoffs. #[derive(Encode, Decode)] -pub(crate) struct GrandpaJustification<Block: BlockT> { +pub struct GrandpaJustification<Block: BlockT> { round: u64, pub(crate) commit: Commit<Block>, votes_ancestries: Vec<Block::Header>, diff --git a/substrate/core/finality-grandpa/src/lib.rs b/substrate/core/finality-grandpa/src/lib.rs index 63eddfd3f33fb27b62da9cd81877d034958495b3..68019dc8eb21b71df9cd7cdc332052d8ef41c104 100644 --- a/substrate/core/finality-grandpa/src/lib.rs +++ b/substrate/core/finality-grandpa/src/lib.rs @@ -96,6 +96,7 @@ mod voting_rule; pub use communication::Network; pub use finality_proof::FinalityProofProvider; +pub use justification::GrandpaJustification; pub use light_import::light_block_import; pub use observer::run_grandpa_observer; pub use voting_rule::{ diff --git a/substrate/core/finality-grandpa/src/tests.rs b/substrate/core/finality-grandpa/src/tests.rs index 8c0047e38bdbfccf76f5f3dcbab64eca37ecad87..2767c14b274314a15657de7e4502b51ed248ac2f 100644 --- a/substrate/core/finality-grandpa/src/tests.rs +++ b/substrate/core/finality-grandpa/src/tests.rs @@ -1627,3 +1627,84 @@ fn grandpa_environment_respects_voting_rules() { 19, ); } + +#[test] +fn imports_justification_for_regular_blocks_on_import() { + // NOTE: this is a regression test since initially we would only import + // justifications for authority change blocks, and would discard any + // existing justification otherwise. + let peers = &[Ed25519Keyring::Alice]; + let voters = make_ids(peers); + let api = TestApi::new(voters); + let mut net = GrandpaTestNet::new(api.clone(), 1); + + let client = net.peer(0).client().clone(); + let (mut block_import, ..) = net.make_block_import(client.clone()); + + let full_client = client.as_full().expect("only full clients are used in test"); + let builder = full_client.new_block_at(&BlockId::Number(0), Default::default()).unwrap(); + let block = builder.bake().unwrap(); + + let block_hash = block.hash(); + + // create a valid justification, with one precommit targeting the block + let justification = { + let round = 1; + let set_id = 0; + + let precommit = grandpa::Precommit { + target_hash: block_hash, + target_number: *block.header.number(), + }; + + let msg = grandpa::Message::Precommit(precommit.clone()); + let encoded = communication::localized_payload(round, set_id, &msg); + let signature = peers[0].sign(&encoded[..]).into(); + + let precommit = grandpa::SignedPrecommit { + precommit, + signature, + id: peers[0].public().into(), + }; + + let commit = grandpa::Commit { + target_hash: block_hash, + target_number: *block.header.number(), + precommits: vec![precommit], + }; + + GrandpaJustification::from_commit( + &full_client, + round, + commit, + ).unwrap() + }; + + // we import the block with justification attached + let block = BlockImportParams { + origin: BlockOrigin::File, + header: block.header, + justification: Some(justification.encode()), + post_digests: Vec::new(), + body: Some(block.extrinsics), + finalized: false, + auxiliary: Vec::new(), + fork_choice: ForkChoiceStrategy::LongestChain, + }; + + assert_eq!( + block_import.import_block(block, HashMap::new()).unwrap(), + ImportResult::Imported(ImportedAux { + needs_justification: false, + clear_justification_requests: false, + bad_justification: false, + is_new_best: true, + ..Default::default() + }), + ); + + // the justification should be imported and available from the client + assert!( + client.justification(&BlockId::Hash(block_hash)).unwrap().is_some(), + ); +}