optimize justification before submit (#1887)

* optimize justification before submit

* fmt

* spelling

* clippy

* fmt again

* aaand compilation

* clippy

bridges/primitives/header-chain/src/justification.rs

@@ -71,6 +71,14 @@ pub enum Error {
 	ExtraHeadersInVotesAncestries,
 }
 
+/// Given GRANDPA authorities set size, return number of valid authorities votes that the
+/// justification must have to be valid.
+///
+/// This function assumes that all authorities have the same vote weight.
+pub fn required_justification_precommits(authorities_set_length: u32) -> u32 {
+	authorities_set_length - authorities_set_length.saturating_sub(1) / 3
+}
+
 /// Decode justification target.
 pub fn decode_justification_target<Header: HeaderT>(
 	raw_justification: &[u8],
@@ -80,6 +88,27 @@ pub fn decode_justification_target<Header: HeaderT>(
 		.map_err(|_| Error::JustificationDecode)
 }
 
+/// Verify and optimize given justification by removing unknown and duplicate votes.
+pub fn optimize_justification<Header: HeaderT>(
+	finalized_target: (Header::Hash, Header::Number),
+	authorities_set_id: SetId,
+	authorities_set: &VoterSet<AuthorityId>,
+	justification: GrandpaJustification<Header>,
+) -> Result<GrandpaJustification<Header>, Error>
+where
+	Header::Number: finality_grandpa::BlockNumberOps,
+{
+	let mut optimizer = OptimizationCallbacks(Vec::new());
+	verify_justification_with_callbacks(
+		finalized_target,
+		authorities_set_id,
+		authorities_set,
+		&justification,
+		&mut optimizer,
+	)?;
+	Ok(optimizer.optimize(justification))
+}
+
 /// Verify that justification, that is generated by given authority set, finalizes given header.
 pub fn verify_justification<Header: HeaderT>(
 	finalized_target: (Header::Hash, Header::Number),
@@ -87,6 +116,83 @@ pub fn verify_justification<Header: HeaderT>(
 	authorities_set: &VoterSet<AuthorityId>,
 	justification: &GrandpaJustification<Header>,
 ) -> Result<(), Error>
+where
+	Header::Number: finality_grandpa::BlockNumberOps,
+{
+	verify_justification_with_callbacks(
+		finalized_target,
+		authorities_set_id,
+		authorities_set,
+		justification,
+		&mut (),
+	)
+}
+
+/// Verification callbacks.
+trait VerificationCallbacks {
+	/// Called when we see a precommit from unknown authority.
+	fn on_unkown_authority(&mut self, precommit_idx: usize) -> Result<(), Error>;
+	/// Called when we see a precommit with duplicate vote from known authority.
+	fn on_duplicate_authority_vote(&mut self, precommit_idx: usize) -> Result<(), Error>;
+	/// Called when we see a precommit after we've collected enough votes from authorities.
+	fn on_redundant_authority_vote(&mut self, precommit_idx: usize) -> Result<(), Error>;
+}
+
+/// Verification callbacks for justification optimization.
+struct OptimizationCallbacks(Vec<usize>);
+
+impl OptimizationCallbacks {
+	fn optimize<Header: HeaderT>(
+		self,
+		mut justification: GrandpaJustification<Header>,
+	) -> GrandpaJustification<Header> {
+		for invalid_precommit_idx in self.0.into_iter().rev() {
+			justification.commit.precommits.remove(invalid_precommit_idx);
+		}
+		justification
+	}
+}
+
+impl VerificationCallbacks for OptimizationCallbacks {
+	fn on_unkown_authority(&mut self, precommit_idx: usize) -> Result<(), Error> {
+		self.0.push(precommit_idx);
+		Ok(())
+	}
+
+	fn on_duplicate_authority_vote(&mut self, precommit_idx: usize) -> Result<(), Error> {
+		self.0.push(precommit_idx);
+		Ok(())
+	}
+
+	fn on_redundant_authority_vote(&mut self, precommit_idx: usize) -> Result<(), Error> {
+		self.0.push(precommit_idx);
+		Ok(())
+	}
+}
+
+// this implementation will be removed in https://github.com/paritytech/parity-bridges-common/pull/1882
+impl VerificationCallbacks for () {
+	fn on_unkown_authority(&mut self, _precommit_idx: usize) -> Result<(), Error> {
+		Ok(())
+	}
+
+	fn on_duplicate_authority_vote(&mut self, _precommit_idx: usize) -> Result<(), Error> {
+		Ok(())
+	}
+
+	fn on_redundant_authority_vote(&mut self, _precommit_idx: usize) -> Result<(), Error> {
+		Ok(())
+	}
+}
+
+/// Verify that justification, that is generated by given authority set, finalizes given header.
+fn verify_justification_with_callbacks<Header: HeaderT, C: VerificationCallbacks>(
+	finalized_target: (Header::Hash, Header::Number),
+	authorities_set_id: SetId,
+	authorities_set: &VoterSet<AuthorityId>,
+	justification: &GrandpaJustification<Header>,
+	callbacks: &mut C,
+) -> Result<(), Error>
 where
 	Header::Number: finality_grandpa::BlockNumberOps,
 {
@@ -95,17 +201,23 @@ where
 		return Err(Error::InvalidJustificationTarget)
 	}
 
+	let threshold = authorities_set.threshold().0.into();
 	let mut chain = AncestryChain::new(&justification.votes_ancestries);
 	let mut signature_buffer = Vec::new();
 	let mut votes = BTreeSet::new();
 	let mut cumulative_weight = 0u64;
-	for signed in &justification.commit.precommits {
+	for (precommit_idx, signed) in justification.commit.precommits.iter().enumerate() {
+		// if we have collected enough precommits, we probabably want to fail/remove extra
+		// precommits
+		if cumulative_weight > threshold {
+			callbacks.on_redundant_authority_vote(precommit_idx)?;
+		}
+
 		// authority must be in the set
 		let authority_info = match authorities_set.get(&signed.id) {
 			Some(authority_info) => authority_info,
 			None => {
-				// just ignore precommit from unknown authority as
-				// `finality_grandpa::import_precommit` does
+				callbacks.on_unkown_authority(precommit_idx)?;
 				continue
 			},
 		};
@@ -116,6 +228,7 @@ where
 		// `finality-grandpa` crate (mostly related to reporting equivocations). But the only thing
 		// that we care about is that only first vote from the authority is accepted
 		if !votes.insert(signed.id.clone()) {
+			callbacks.on_duplicate_authority_vote(precommit_idx)?;
 			continue
 		}
 
@@ -142,6 +255,7 @@ where
 				thus we'll never overflow the u64::MAX;\
 				qed",
 		);
+
 		// verify authority signature
 		if !sp_finality_grandpa::check_message_signature_with_buffer(
 			&finality_grandpa::Message::Precommit(signed.precommit.clone()),
@@ -162,7 +276,6 @@ where
 
 	// check that the cumulative weight of validators voted for the justification target (or one
 	// of its descendents) is larger than required threshold.
-	let threshold = authorities_set.threshold().0.into();
 	if cumulative_weight >= threshold {
 		Ok(())
 	} else {