Beefy equivocation: check all the MMR roots

substrate/frame/beefy-mmr/src/lib.rs

@@ -258,17 +258,31 @@ where
 				},
 			};
 
-		let commitment_root =
-			match commitment.payload.get_decoded::<MerkleRootOf<T>>(&known_payloads::MMR_ROOT_ID) {
-				Some(commitment_root) => commitment_root,
+		let commitment_roots = commitment
+			.payload
+			.get_all_decoded::<MerkleRootOf<T>>(&known_payloads::MMR_ROOT_ID);
+		if commitment_roots.is_empty() {
+			// If the commitment doesn't contain any MMR root, while the proof is valid,
+			// the commitment is invalid
+			return true;
+		}
+		for maybe_commitment_root in commitment_roots {
+			match maybe_commitment_root {
+				Some(commitment_root) => {
+					if canonical_prev_root != commitment_root {
+						// If the commitment contains an MMR root, that is not equal to
+						// `canonical_prev_root`, the commitment is invalid
+						return true;
+					}
+				},
 				None => {
-					// If the commitment doesn't contain any MMR root, while the proof is valid,
-					// the commitment is invalid
-					return true
+					// If the commitment contains an MMR root, that can't be decoded, it is invalid.
+					return true;
 				},
-			};
+			}
+		}
 
-		canonical_prev_root != commitment_root
+		false
 	}
 }
 

substrate/frame/beefy-mmr/src/tests.rs

@@ -278,8 +278,28 @@ fn is_non_canonical_should_work_correctly() {
 				&Commitment {
 					payload: Payload::from_single_entry(
 						known_payloads::MMR_ROOT_ID,
-						H256::repeat_byte(0).encode(),
-					),
+						prev_roots[250 - 1].encode()
+					)
+					.push_raw(known_payloads::MMR_ROOT_ID, H256::repeat_byte(0).encode(),),
+					block_number: 250,
+					validator_set_id: 0,
+				},
+				valid_proof.clone(),
+				Mmr::mmr_root(),
+			),
+			true
+		);
+
+		// If the `commitment.payload` contains an MMR root that can't be decoded,
+		// it's non-canonical.
+		assert_eq!(
+			BeefyMmr::is_non_canonical(
+				&Commitment {
+					payload: Payload::from_single_entry(
+						known_payloads::MMR_ROOT_ID,
+						prev_roots[250 - 1].encode()
+					)
+					.push_raw(known_payloads::MMR_ROOT_ID, vec![],),
 					block_number: 250,
 					validator_set_id: 0,
 				},

substrate/primitives/consensus/beefy/src/payload.rs

@@ -56,13 +56,26 @@ impl Payload {
 		Some(&self.0[index].1)
 	}
 
+	/// Returns all the raw payloads under given `id`.
+	pub fn get_all_raw(&self, id: &BeefyPayloadId) -> Vec<&Vec<u8>> {
+		self.0
+			.iter()
+			.filter_map(|probe| if &probe.0 != id { return None } else { Some(&probe.1) })
+			.collect()
+	}
+
 	/// Returns a decoded payload value under given `id`.
 	///
 	/// In case the value is not there, or it cannot be decoded `None` is returned.
 	pub fn get_decoded<T: Decode>(&self, id: &BeefyPayloadId) -> Option<T> {
 		self.get_raw(id).and_then(|raw| T::decode(&mut &raw[..]).ok())
 	}
 
+	/// Returns all decoded payload values under given `id`.
+	pub fn get_all_decoded<T: Decode>(&self, id: &BeefyPayloadId) -> Vec<Option<T>> {
+		self.get_all_raw(id).iter().map(|raw| T::decode(&mut &raw[..]).ok()).collect()
+	}
+
 	/// Push a `Vec<u8>` with a given id into the payload vec.
 	/// This method will internally sort the payload vec after every push.
 	///