[BEEFY] Add runtime support for reporting fork voting (paritytech#4522)

Related to paritytech#4523 Extracting part of paritytech#1903 (credits to @Lederstrumpf for the high-level strategy), but also introducing significant adjustments both to the approach and to the code. The main adjustment is the fact that the `ForkVotingProof` accepts only one vote, compared to the original version which accepted a `vec![]`. With this approach more calls are needed in order to report multiple equivocated votes on the same commit, but it simplifies a lot the checking logic. We can add support for reporting multiple signatures at once in the future. There are 2 things that are missing in order to consider this issue done, but I would propose to do them in a separate PR since this one is already pretty big: - benchmarks/computing a weight for the new extrinsic (this wasn't present in paritytech#1903 either) - exposing an API for generating the ancestry proof. I'm not sure if we should do this in the Mmr pallet or in the Beefy pallet Co-authored-by: Robert Hambrock <[email protected]> --------- Co-authored-by: Adrian Catangiu <[email protected]>
TomaszWaszczyk · Jul 7, 2024 · e5d36e0 · e5d36e0
1 parent e0d51fb
commit e5d36e0
Show file tree

Hide file tree

Showing 25 changed files with 1,378 additions and 400 deletions.
diff --git a/polkadot/node/service/src/fake_runtime_api.rs b/polkadot/node/service/src/fake_runtime_api.rs
@@ -241,7 +241,7 @@ sp_api::impl_runtime_apis! {
 			unimplemented!()
 		}
 
-		fn submit_report_equivocation_unsigned_extrinsic(
+		fn submit_report_double_voting_unsigned_extrinsic(
 			_: sp_consensus_beefy::DoubleVotingProof<
 				BlockNumber,
 				BeefyId,

diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs
@@ -1283,6 +1283,7 @@ impl pallet_beefy::Config for Runtime {
 	type MaxNominators = ConstU32<0>;
 	type MaxSetIdSessionEntries = BeefySetIdSessionEntries;
 	type OnNewValidatorSet = MmrLeaf;
+	type AncestryHelper = MmrLeaf;
 	type WeightInfo = ();
 	type KeyOwnerProof = <Historical as KeyOwnerProofSystem<(KeyTypeId, BeefyId)>>::Proof;
 	type EquivocationReportSystem =
@@ -2052,7 +2053,7 @@ sp_api::impl_runtime_apis! {
 		}
 	}
 
-	#[api_version(3)]
+	#[api_version(4)]
 	impl sp_consensus_beefy::BeefyApi<Block, BeefyId> for Runtime {
 		fn beefy_genesis() -> Option<BlockNumber> {
 			pallet_beefy::GenesisBlock::<Runtime>::get()
@@ -2062,7 +2063,7 @@ sp_api::impl_runtime_apis! {
 			Beefy::validator_set()
 		}
 
-		fn submit_report_equivocation_unsigned_extrinsic(
+		fn submit_report_double_voting_unsigned_extrinsic(
 			equivocation_proof: sp_consensus_beefy::DoubleVotingProof<
 				BlockNumber,
 				BeefyId,
@@ -2072,7 +2073,7 @@ sp_api::impl_runtime_apis! {
 		) -> Option<()> {
 			let key_owner_proof = key_owner_proof.decode()?;
 
-			Beefy::submit_unsigned_equivocation_report(
+			Beefy::submit_unsigned_double_voting_report(
 				equivocation_proof,
 				key_owner_proof,
 			)

diff --git a/polkadot/runtime/test-runtime/src/lib.rs b/polkadot/runtime/test-runtime/src/lib.rs
@@ -1015,7 +1015,7 @@ sp_api::impl_runtime_apis! {
 			None
 		}
 
-		fn submit_report_equivocation_unsigned_extrinsic(
+		fn submit_report_double_voting_unsigned_extrinsic(
 			_equivocation_proof: sp_consensus_beefy::DoubleVotingProof<
 				BlockNumber,
 				BeefyId,

diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs
@@ -328,6 +328,7 @@ impl pallet_beefy::Config for Runtime {
 	type MaxNominators = MaxNominators;
 	type MaxSetIdSessionEntries = BeefySetIdSessionEntries;
 	type OnNewValidatorSet = BeefyMmrLeaf;
+	type AncestryHelper = BeefyMmrLeaf;
 	type WeightInfo = ();
 	type KeyOwnerProof = sp_session::MembershipProof;
 	type EquivocationReportSystem =
@@ -2009,6 +2010,7 @@ sp_api::impl_runtime_apis! {
 		}
 	}
 
+	#[api_version(4)]
 	impl sp_consensus_beefy::BeefyApi<Block, BeefyId> for Runtime {
 		fn beefy_genesis() -> Option<BlockNumber> {
 			pallet_beefy::GenesisBlock::<Runtime>::get()
@@ -2018,7 +2020,7 @@ sp_api::impl_runtime_apis! {
 			Beefy::validator_set()
 		}
 
-		fn submit_report_equivocation_unsigned_extrinsic(
+		fn submit_report_double_voting_unsigned_extrinsic(
 			equivocation_proof: sp_consensus_beefy::DoubleVotingProof<
 				BlockNumber,
 				BeefyId,
@@ -2028,7 +2030,7 @@ sp_api::impl_runtime_apis! {
 		) -> Option<()> {
 			let key_owner_proof = key_owner_proof.decode()?;
 
-			Beefy::submit_unsigned_equivocation_report(
+			Beefy::submit_unsigned_double_voting_report(
 				equivocation_proof,
 				key_owner_proof,
 			)

diff --git a/prdoc/pr_4522.prdoc b/prdoc/pr_4522.prdoc
@@ -0,0 +1,39 @@
+# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
+# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json
+
+title: Added runtime support for reporting BEEFY fork voting
+
+doc:
+  - audience:
+    - Runtime Dev
+    - Runtime User
+    description: |
+      This PR adds the `report_fork_voting`, `report_future_voting` extrinsics to `pallet-beefy`
+      and renames the `report_equivocation` extrinsic to `report_double_voting`.
+      `report_fork_voting` can't be called yet, since it uses `Weight::MAX` weight. We will
+      add benchmarks for it and set the proper weight in a future PR.
+      Also a new `AncestryHelper` associated trait was added to `pallet_beefy::Config`.
+  - audience: Node Dev
+    description: |
+      This PR renames the `submit_report_equivocation_unsigned_extrinsic` in `BeefyApi` to
+      `submit_report_double_voting_unsigned_extrinsic`and bumps the `BeefyApi` version from 3 to 4.
+
+crates: 
+  - name: pallet-beefy
+    bump: major
+  - name: pallet-beefy-mmr
+    bump: minor
+  - name: pallet-mmr
+    bump: major
+  - name: sc-consensus-beefy
+    bump: patch
+  - name: kitchensink-runtime
+    bump: major
+  - name: rococo-runtime
+    bump: major
+  - name: westend-runtime
+    bump: major
+  - name: sp-consensus-beefy
+    bump: major
+  - name: polkadot-service
+    bump: patch
diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs
@@ -2548,6 +2548,7 @@ impl pallet_beefy::Config for Runtime {
 	type MaxNominators = ConstU32<0>;
 	type MaxSetIdSessionEntries = BeefySetIdSessionEntries;
 	type OnNewValidatorSet = MmrLeaf;
+	type AncestryHelper = MmrLeaf;
 	type WeightInfo = ();
 	type KeyOwnerProof = <Historical as KeyOwnerProofSystem<(KeyTypeId, BeefyId)>>::Proof;
 	type EquivocationReportSystem =
@@ -3032,7 +3033,7 @@ impl_runtime_apis! {
 		}
 	}
 
-	#[api_version(3)]
+	#[api_version(4)]
 	impl sp_consensus_beefy::BeefyApi<Block, BeefyId> for Runtime {
 		fn beefy_genesis() -> Option<BlockNumber> {
 			pallet_beefy::GenesisBlock::<Runtime>::get()
@@ -3042,7 +3043,7 @@ impl_runtime_apis! {
 			Beefy::validator_set()
 		}
 
-		fn submit_report_equivocation_unsigned_extrinsic(
+		fn submit_report_double_voting_unsigned_extrinsic(
 			equivocation_proof: sp_consensus_beefy::DoubleVotingProof<
 				BlockNumber,
 				BeefyId,
@@ -3052,7 +3053,7 @@ impl_runtime_apis! {
 		) -> Option<()> {
 			let key_owner_proof = key_owner_proof.decode()?;
 
-			Beefy::submit_unsigned_equivocation_report(
+			Beefy::submit_unsigned_double_voting_report(
 				equivocation_proof,
 				key_owner_proof,
 			)

diff --git a/substrate/client/consensus/beefy/src/fisherman.rs b/substrate/client/consensus/beefy/src/fisherman.rs
@@ -23,7 +23,7 @@ use sp_api::ProvideRuntimeApi;
 use sp_application_crypto::RuntimeAppPublic;
 use sp_blockchain::HeaderBackend;
 use sp_consensus_beefy::{
-	check_equivocation_proof, AuthorityIdBound, BeefyApi, BeefySignatureHasher, DoubleVotingProof,
+	check_double_voting_proof, AuthorityIdBound, BeefyApi, BeefySignatureHasher, DoubleVotingProof,
 	OpaqueKeyOwnershipProof, ValidatorSetId,
 };
 use sp_runtime::{
@@ -132,7 +132,7 @@ where
 			(active_rounds.validators(), active_rounds.validator_set_id());
 		let offender_id = proof.offender_id();
 
-		if !check_equivocation_proof::<_, _, BeefySignatureHasher>(&proof) {
+		if !check_double_voting_proof::<_, _, BeefySignatureHasher>(&proof) {
 			debug!(target: LOG_TARGET, "🥩 Skipping report for bad equivocation {:?}", proof);
 			return Ok(());
 		}
@@ -155,7 +155,7 @@ where
 		for ProvedValidator { key_owner_proof, .. } in key_owner_proofs {
 			self.runtime
 				.runtime_api()
-				.submit_report_equivocation_unsigned_extrinsic(
+				.submit_report_double_voting_unsigned_extrinsic(
 					best_block_hash,
 					proof.clone(),
 					key_owner_proof,

diff --git a/substrate/client/consensus/beefy/src/tests.rs b/substrate/client/consensus/beefy/src/tests.rs
@@ -314,7 +314,7 @@ sp_api::mock_impl_runtime_apis! {
 			self.inner.validator_set.clone()
 		}
 
-		fn submit_report_equivocation_unsigned_extrinsic(
+		fn submit_report_double_voting_unsigned_extrinsic(
 			proof: DoubleVotingProof<NumberFor<Block>, AuthorityId, Signature>,
 			_dummy: OpaqueKeyOwnershipProof,
 		) -> Option<()> {

diff --git a/substrate/client/consensus/beefy/src/worker.rs b/substrate/client/consensus/beefy/src/worker.rs
@@ -1039,7 +1039,7 @@ pub(crate) mod tests {
 		ecdsa_crypto, known_payloads,
 		known_payloads::MMR_ROOT_ID,
 		mmr::MmrRootProvider,
-		test_utils::{generate_equivocation_proof, Keyring},
+		test_utils::{generate_double_voting_proof, Keyring},
 		ConsensusLog, Payload, SignedCommitment,
 	};
 	use sp_runtime::traits::{Header as HeaderT, One};
@@ -1586,7 +1586,7 @@ pub(crate) mod tests {
 		let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]);
 
 		// generate an equivocation proof, with Bob as perpetrator
-		let good_proof = generate_equivocation_proof(
+		let good_proof = generate_double_voting_proof(
 			(block_num, payload1.clone(), set_id, &Keyring::Bob),
 			(block_num, payload2.clone(), set_id, &Keyring::Bob),
 		);
@@ -1618,7 +1618,7 @@ pub(crate) mod tests {
 		assert!(api_alice.reported_equivocations.as_ref().unwrap().lock().is_empty());
 
 		// now let's try reporting a self-equivocation
-		let self_proof = generate_equivocation_proof(
+		let self_proof = generate_double_voting_proof(
 			(block_num, payload1.clone(), set_id, &Keyring::Alice),
 			(block_num, payload2.clone(), set_id, &Keyring::Alice),
 		);

diff --git a/substrate/frame/beefy-mmr/src/lib.rs b/substrate/frame/beefy-mmr/src/lib.rs
@@ -33,20 +33,22 @@
 //!
 //! and thanks to versioning can be easily updated in the future.
 
-use sp_runtime::traits::{Convert, Member};
+use sp_runtime::traits::{Convert, Header, Member};
 use sp_std::prelude::*;
 
 use codec::Decode;
-use pallet_mmr::{LeafDataProvider, ParentNumberAndHash};
+use pallet_mmr::{primitives::AncestryProof, LeafDataProvider, ParentNumberAndHash};
 use sp_consensus_beefy::{
+	known_payloads,
 	mmr::{BeefyAuthoritySet, BeefyDataProvider, BeefyNextAuthoritySet, MmrLeaf, MmrLeafVersion},
-	ValidatorSet as BeefyValidatorSet,
+	AncestryHelper, Commitment, ConsensusLog, ValidatorSet as BeefyValidatorSet,
 };
 
 use frame_support::{crypto::ecdsa::ECDSAExt, traits::Get};
-use frame_system::pallet_prelude::BlockNumberFor;
+use frame_system::pallet_prelude::{BlockNumberFor, HeaderFor};
 
 pub use pallet::*;
+use sp_runtime::generic::OpaqueDigestItemId;
 
 #[cfg(test)]
 mod mock;
@@ -172,6 +174,75 @@ where
 	}
 }
 
+impl<T: Config> AncestryHelper<HeaderFor<T>> for Pallet<T>
+where
+	T: pallet_mmr::Config<Hashing = sp_consensus_beefy::MmrHashing>,
+{
+	type Proof = AncestryProof<MerkleRootOf<T>>;
+	type ValidationContext = MerkleRootOf<T>;
+
+	fn extract_validation_context(header: HeaderFor<T>) -> Option<Self::ValidationContext> {
+		// Check if the provided header is canonical.
+		let expected_hash = frame_system::Pallet::<T>::block_hash(header.number());
+		if expected_hash != header.hash() {
+			return None;
+		}
+
+		// Extract the MMR root from the header digest
+		header.digest().convert_first(|l| {
+			l.try_to(OpaqueDigestItemId::Consensus(&sp_consensus_beefy::BEEFY_ENGINE_ID))
+				.and_then(|log: ConsensusLog<<T as pallet_beefy::Config>::BeefyId>| match log {
+					ConsensusLog::MmrRoot(mmr_root) => Some(mmr_root),
+					_ => None,
+				})
+		})
+	}
+
+	fn is_non_canonical(
+		commitment: &Commitment<BlockNumberFor<T>>,
+		proof: Self::Proof,
+		context: Self::ValidationContext,
+	) -> bool {
+		let commitment_leaf_count =
+			match pallet_mmr::Pallet::<T>::block_num_to_leaf_count(commitment.block_number) {
+				Ok(commitment_leaf_count) => commitment_leaf_count,
+				Err(_) => {
+					// We can't prove that the commitment is non-canonical if the
+					// `commitment.block_number` is invalid.
+					return false
+				},
+			};
+		if commitment_leaf_count != proof.prev_leaf_count {
+			// Can't prove that the commitment is non-canonical if the `commitment.block_number`
+			// doesn't match the ancestry proof.
+			return false;
+		}
+
+		let canonical_mmr_root = context;
+		let canonical_prev_root =
+			match pallet_mmr::Pallet::<T>::verify_ancestry_proof(canonical_mmr_root, proof) {
+				Ok(canonical_prev_root) => canonical_prev_root,
+				Err(_) => {
+					// Can't prove that the commitment is non-canonical if the proof
+					// is invalid.
+					return false
+				},
+			};
+
+		let commitment_root =
+			match commitment.payload.get_decoded::<MerkleRootOf<T>>(&known_payloads::MMR_ROOT_ID) {
+				Some(commitment_root) => commitment_root,
+				None => {
+					// If the commitment doesn't contain any MMR root, while the proof is valid,
+					// the commitment is invalid
+					return true
+				},
+			};
+
+		canonical_prev_root != commitment_root
+	}
+}
+
 impl<T: Config> Pallet<T> {
 	/// Return the currently active BEEFY authority set proof.
 	pub fn authority_set_proof() -> BeefyAuthoritySet<MerkleRootOf<T>> {

diff --git a/substrate/frame/beefy-mmr/src/mock.rs b/substrate/frame/beefy-mmr/src/mock.rs
@@ -101,6 +101,7 @@ impl pallet_beefy::Config for Test {
 	type MaxNominators = ConstU32<1000>;
 	type MaxSetIdSessionEntries = ConstU64<100>;
 	type OnNewValidatorSet = BeefyMmr;
+	type AncestryHelper = BeefyMmr;
 	type WeightInfo = ();
 	type KeyOwnerProof = sp_core::Void;
 	type EquivocationReportSystem = ();