Companion for substrate#11302 (#5667)

* bridges: bump finality-grandpa version

* bridges: fix tests

* update lockfile for {"substrate"}

Co-authored-by: parity-processbot <>

polkadot/bridges/modules/grandpa/Cargo.toml

@@ -9,7 +9,7 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0"
 
 [dependencies]
 codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false }
-finality-grandpa = { version = "0.15.0", default-features = false }
+finality-grandpa = { version = "0.16.0", default-features = false }
 log = { version = "0.4.14", default-features = false }
 num-traits = { version = "0.2", default-features = false }
 scale-info = { version = "2.0.1", default-features = false, features = ["derive"] }

polkadot/bridges/primitives/header-chain/Cargo.toml

@@ -8,7 +8,7 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0"
 
 [dependencies]
 codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false }
-finality-grandpa = { version = "0.15.0", default-features = false }
+finality-grandpa = { version = "0.16.0", default-features = false }
 scale-info = { version = "2.0.1", default-features = false, features = ["derive"] }
 serde = { version = "1.0", optional = true }
 
@@ -25,7 +25,6 @@ sp-runtime = { git = "https://github.com/paritytech/substrate", branch = "master
 sp-std = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
 
 [dev-dependencies]
-assert_matches = "1.5"
 bp-test-utils = { path = "../test-utils" }
 hex = "0.4"
 hex-literal = "0.3"

polkadot/bridges/primitives/header-chain/tests/implementation_match.rs

@@ -20,7 +20,6 @@
 //! Some of tests in this module may partially duplicate tests from `justification.rs`,
 //! but their purpose is different.
 
-use assert_matches::assert_matches;
 use bp_header_chain::justification::{verify_justification, Error, GrandpaJustification};
 use bp_test_utils::{
 	header_id, make_justification_for_header, signed_precommit, test_header, Account,
@@ -106,7 +105,7 @@ pub fn make_default_justification(header: &TestHeader) -> GrandpaJustification<T
 //
 // 1) to return `Err()` (which only may happen if `finality_grandpa::Chain` implementation
 //    returns an error);
-// 2) to return `Ok(validation_result) if validation_result.ghost().is_none()`.
+// 2) to return `Ok(validation_result)` if `validation_result.is_valid()` is false.
 //
 // Our implementation would just return error in both cases.
 
@@ -126,16 +125,17 @@ fn same_result_when_precommit_target_has_lower_number_than_commit_target() {
 		),
 		Err(Error::PrecommitIsNotCommitDescendant),
 	);
-	// original implementation returns empty GHOST
+
-	assert_matches!(
+	// original implementation returns `Ok(validation_result)`
-		finality_grandpa::validate_commit(
+	// with `validation_result.is_valid() == false`.
-			&justification.commit,
+	let result = finality_grandpa::validate_commit(
-			&full_voter_set(),
+		&justification.commit,
-			&AncestryChain::new(&justification.votes_ancestries),
+		&full_voter_set(),
-		)
+		&AncestryChain::new(&justification.votes_ancestries),
-		.map(|result| result.ghost().cloned()),
+	)
-		Ok(None)
+	.unwrap();
-	);
+
+	assert!(!result.is_valid());
 }
 
 #[test]
@@ -158,21 +158,28 @@ fn same_result_when_precommit_target_is_not_descendant_of_commit_target() {
 		),
 		Err(Error::PrecommitIsNotCommitDescendant),
 	);
-	// original implementation returns empty GHOST
+
-	assert_matches!(
+	// original implementation returns `Ok(validation_result)`
-		finality_grandpa::validate_commit(
+	// with `validation_result.is_valid() == false`.
-			&justification.commit,
+	let result = finality_grandpa::validate_commit(
-			&full_voter_set(),
+		&justification.commit,
-			&AncestryChain::new(&justification.votes_ancestries),
+		&full_voter_set(),
-		)
+		&AncestryChain::new(&justification.votes_ancestries),
-		.map(|result| result.ghost().cloned()),
+	)
-		Ok(None)
+	.unwrap();
-	);
+
+	assert!(!result.is_valid());
 }
 
 #[test]
 fn same_result_when_justification_contains_duplicate_vote() {
-	let mut justification = make_default_justification(&test_header(1));
+	let mut justification = make_justification_for_header(JustificationGeneratorParams {
+		header: test_header(1),
+		authorities: minimal_accounts_set(),
+		ancestors: 0,
+		..Default::default()
+	});
+
 	// the justification may contain exactly the same vote (i.e. same precommit and same signature)
 	// multiple times && it isn't treated as an error by original implementation
 	justification.commit.precommits.push(justification.commit.precommits[0].clone());
@@ -188,21 +195,28 @@ fn same_result_when_justification_contains_duplicate_vote() {
 		),
 		Ok(()),
 	);
-	// original implementation returns non-empty GHOST
+
-	assert_matches!(
+	// original implementation returns `Ok(validation_result)`
-		finality_grandpa::validate_commit(
+	// with `validation_result.is_valid() == true`.
-			&justification.commit,
+	let result = finality_grandpa::validate_commit(
-			&full_voter_set(),
+		&justification.commit,
-			&AncestryChain::new(&justification.votes_ancestries),
+		&full_voter_set(),
-		)
+		&AncestryChain::new(&justification.votes_ancestries),
-		.map(|result| result.ghost().cloned()),
+	)
-		Ok(Some(_))
+	.unwrap();
-	);
+
+	assert!(result.is_valid());
 }
 
 #[test]
 fn same_result_when_authority_equivocates_once_in_a_round() {
-	let mut justification = make_default_justification(&test_header(1));
+	let mut justification = make_justification_for_header(JustificationGeneratorParams {
+		header: test_header(1),
+		authorities: minimal_accounts_set(),
+		ancestors: 0,
+		..Default::default()
+	});
+
 	// the justification original implementation allows authority to submit two different
 	// votes in a single round, of which only first is 'accepted'
 	justification.commit.precommits.push(signed_precommit::<TestHeader>(
@@ -222,21 +236,28 @@ fn same_result_when_authority_equivocates_once_in_a_round() {
 		),
 		Ok(()),
 	);
-	// original implementation returns non-empty GHOST
+
-	assert_matches!(
+	// original implementation returns `Ok(validation_result)`
-		finality_grandpa::validate_commit(
+	// with `validation_result.is_valid() == true`.
-			&justification.commit,
+	let result = finality_grandpa::validate_commit(
-			&full_voter_set(),
+		&justification.commit,
-			&AncestryChain::new(&justification.votes_ancestries),
+		&full_voter_set(),
-		)
+		&AncestryChain::new(&justification.votes_ancestries),
-		.map(|result| result.ghost().cloned()),
+	)
-		Ok(Some(_))
+	.unwrap();
-	);
+
+	assert!(result.is_valid());
 }
 
 #[test]
 fn same_result_when_authority_equivocates_twice_in_a_round() {
-	let mut justification = make_default_justification(&test_header(1));
+	let mut justification = make_justification_for_header(JustificationGeneratorParams {
+		header: test_header(1),
+		authorities: minimal_accounts_set(),
+		ancestors: 0,
+		..Default::default()
+	});
+
 	// there's some code in the original implementation that should return an error when
 	// same authority submits more than two different votes in a single round:
 	// https://github.com/paritytech/finality-grandpa/blob/6aeea2d1159d0f418f0b86e70739f2130629ca09/src/lib.rs#L473
@@ -266,16 +287,17 @@ fn same_result_when_authority_equivocates_twice_in_a_round() {
 		),
 		Ok(()),
 	);
-	// original implementation returns non-empty GHOST
+
-	assert_matches!(
+	// original implementation returns `Ok(validation_result)`
-		finality_grandpa::validate_commit(
+	// with `validation_result.is_valid() == true`.
-			&justification.commit,
+	let result = finality_grandpa::validate_commit(
-			&full_voter_set(),
+		&justification.commit,
-			&AncestryChain::new(&justification.votes_ancestries),
+		&full_voter_set(),
-		)
+		&AncestryChain::new(&justification.votes_ancestries),
-		.map(|result| result.ghost().cloned()),
+	)
-		Ok(Some(_))
+	.unwrap();
-	);
+
+	assert!(result.is_valid());
 }
 
 #[test]
@@ -299,14 +321,15 @@ fn same_result_when_there_are_not_enough_cumulative_weight_to_finalize_commit_ta
 		),
 		Err(Error::TooLowCumulativeWeight),
 	);
-	// original implementation returns empty GHOST
+
-	assert_matches!(
+	// original implementation returns `Ok(validation_result)`
-		finality_grandpa::validate_commit(
+	// with `validation_result.is_valid() == false`.
-			&justification.commit,
+	let result = finality_grandpa::validate_commit(
-			&full_voter_set(),
+		&justification.commit,
-			&AncestryChain::new(&justification.votes_ancestries),
+		&full_voter_set(),
-		)
+		&AncestryChain::new(&justification.votes_ancestries),
-		.map(|result| result.ghost().cloned()),
+	)
-		Ok(None)
+	.unwrap();
-	);
+
+	assert!(!result.is_valid());
 }

polkadot/bridges/primitives/test-utils/Cargo.toml

@@ -9,7 +9,7 @@ license = "GPL-3.0-or-later WITH Classpath-exception-2.0"
 bp-header-chain = { path = "../header-chain", default-features = false  }
 codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false }
 ed25519-dalek = { version = "1.0", default-features = false, features = ["u64_backend"] }
-finality-grandpa = { version = "0.15.0", default-features = false }
+finality-grandpa = { version = "0.16.0", default-features = false }
 sp-application-crypto = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }
 sp-finality-grandpa = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false  }
 sp-runtime = { git = "https://github.com/paritytech/substrate", branch = "master", default-features = false }

polkadot/bridges/relays/bin-substrate/Cargo.toml

@@ -79,4 +79,4 @@ hex-literal = "0.3"
 pallet-bridge-grandpa = { path = "../../modules/grandpa" }
 sp-keyring = { git = "https://github.com/paritytech/substrate", branch = "master" }
 tempfile = "3.2"
-finality-grandpa = { version = "0.15.0" }
+finality-grandpa = { version = "0.16.0" }

polkadot/bridges/relays/lib-substrate-relay/Cargo.toml

@@ -20,7 +20,7 @@ log = "0.4.14"
 bp-header-chain = { path = "../../primitives/header-chain" }
 bridge-runtime-common = { path = "../../bin/runtime-common" }
 
-finality-grandpa = { version = "0.15.0" }
+finality-grandpa = { version = "0.16.0" }
 finality-relay = { path = "../finality" }
 relay-utils = { path = "../utils" }
 messages-relay = { path = "../messages" }