From e2b72b95dc8b53bac47662ab6d8ee6b9e2dbf5a8 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Mon, 13 May 2024 16:46:18 +0300 Subject: [PATCH 01/14] Add Beefy pallet hook for checking ancestry --- polkadot/runtime/rococo/src/lib.rs | 1 + polkadot/runtime/westend/src/lib.rs | 1 + substrate/bin/node/runtime/src/lib.rs | 1 + substrate/frame/beefy-mmr/src/lib.rs | 49 +++++- substrate/frame/beefy-mmr/src/mock.rs | 1 + substrate/frame/beefy-mmr/src/tests.rs | 162 ++++++++++++++++-- substrate/frame/beefy/src/lib.rs | 7 +- substrate/frame/beefy/src/mock.rs | 14 ++ .../frame/merkle-mountain-range/src/lib.rs | 52 +++--- .../merkle-mountain-range/src/mmr/mmr.rs | 74 ++++---- .../merkle-mountain-range/src/mmr/mod.rs | 2 +- .../merkle-mountain-range/src/mmr/storage.rs | 3 +- .../frame/merkle-mountain-range/src/tests.rs | 21 --- .../primitives/consensus/beefy/src/lib.rs | 10 ++ .../primitives/consensus/beefy/src/payload.rs | 2 +- 15 files changed, 294 insertions(+), 106 deletions(-) diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index c22d5c39b233..1f7f485e75ae 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -1270,6 +1270,7 @@ impl pallet_beefy::Config for Runtime { type MaxNominators = ConstU32<0>; type MaxSetIdSessionEntries = BeefySetIdSessionEntries; type OnNewValidatorSet = MmrLeaf; + type AncestryHelper = MmrLeaf; type WeightInfo = (); type KeyOwnerProof = >::Proof; type EquivocationReportSystem = diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index b62c6d08201c..1dc372bcacaf 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -326,6 +326,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 = diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index b1f948afa561..a89e7b314f0b 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -2561,6 +2561,7 @@ impl pallet_beefy::Config for Runtime { type MaxNominators = ConstU32<0>; type MaxSetIdSessionEntries = BeefySetIdSessionEntries; type OnNewValidatorSet = MmrLeaf; + type AncestryHelper = MmrLeaf; type WeightInfo = (); type KeyOwnerProof = >::Proof; type EquivocationReportSystem = diff --git a/substrate/frame/beefy-mmr/src/lib.rs b/substrate/frame/beefy-mmr/src/lib.rs index e423f1b342f2..4bd03360c281 100644 --- a/substrate/frame/beefy-mmr/src/lib.rs +++ b/substrate/frame/beefy-mmr/src/lib.rs @@ -37,10 +37,11 @@ use sp_runtime::traits::{Convert, 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, ValidatorSet as BeefyValidatorSet, }; use frame_support::{crypto::ecdsa::ECDSAExt, traits::Get}; @@ -172,6 +173,50 @@ where } } +impl AncestryHelper> for Pallet { + type Proof = AncestryProof>; + + fn is_non_canonical(commitment: &Commitment>, proof: Self::Proof) -> bool { + let commitment_leaf_count = + match pallet_mmr::Pallet::::block_num_to_leaf_count(commitment.block_number) { + Ok(commitment_leaf_count) => commitment_leaf_count, + Err(_) => { + // We consider the commitment non-canonical if the `commitment.block_number` + // is invalid. + return true + }, + }; + 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 = pallet_mmr::Pallet::::mmr_root(); + let canonical_prev_root = + match pallet_mmr::Pallet::::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::>(&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 Pallet { /// Return the currently active BEEFY authority set proof. pub fn authority_set_proof() -> BeefyAuthoritySet> { diff --git a/substrate/frame/beefy-mmr/src/mock.rs b/substrate/frame/beefy-mmr/src/mock.rs index d59c219d3e71..0521bdabbe49 100644 --- 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 = (); diff --git a/substrate/frame/beefy-mmr/src/tests.rs b/substrate/frame/beefy-mmr/src/tests.rs index fac799bf64e4..83a6292295b7 100644 --- a/substrate/frame/beefy-mmr/src/tests.rs +++ b/substrate/frame/beefy-mmr/src/tests.rs @@ -19,11 +19,15 @@ use std::vec; use codec::{Decode, Encode}; use sp_consensus_beefy::{ + known_payloads, mmr::{BeefyNextAuthoritySet, MmrLeafVersion}, - ValidatorSet, + AncestryHelper, Commitment, Payload, ValidatorSet, }; -use sp_core::H256; +use sp_core::{ + offchain::{testing::TestOffchainExt, OffchainDbExt, OffchainWorkerExt}, + H256, +}; use sp_io::TestExternalities; use sp_runtime::{traits::Keccak256, DigestItem}; @@ -32,7 +36,8 @@ use frame_support::traits::OnInitialize; use crate::mock::*; fn init_block(block: u64) { - System::set_block_number(block); + let hash = H256::repeat_byte(block as u8); + System::initialize(&block, &hash, &Default::default()); Session::on_initialize(block); Mmr::on_initialize(block); Beefy::on_initialize(block); @@ -62,37 +67,31 @@ fn should_contain_mmr_digest() { let mut ext = new_test_ext(vec![1, 2, 3, 4]); ext.execute_with(|| { init_block(1); - assert_eq!( System::digest().logs, vec![ beefy_log(ConsensusLog::AuthoritiesChange( ValidatorSet::new(vec![mock_beefy_id(1), mock_beefy_id(2)], 1).unwrap() )), - beefy_log(ConsensusLog::MmrRoot(array_bytes::hex_n_into_unchecked( - "95803defe6ea9f41e7ec6afa497064f21bfded027d8812efacbdf984e630cbdc" - ))) + beefy_log(ConsensusLog::MmrRoot(H256::from_slice(&[ + 117, 0, 56, 25, 185, 195, 71, 232, 67, 213, 27, 178, 64, 168, 137, 220, 64, + 184, 64, 240, 83, 245, 18, 93, 185, 202, 125, 205, 17, 254, 18, 143 + ]))) ] ); // unique every time init_block(2); - assert_eq!( System::digest().logs, vec![ - beefy_log(ConsensusLog::AuthoritiesChange( - ValidatorSet::new(vec![mock_beefy_id(1), mock_beefy_id(2)], 1).unwrap() - )), - beefy_log(ConsensusLog::MmrRoot(array_bytes::hex_n_into_unchecked( - "95803defe6ea9f41e7ec6afa497064f21bfded027d8812efacbdf984e630cbdc" - ))), beefy_log(ConsensusLog::AuthoritiesChange( ValidatorSet::new(vec![mock_beefy_id(3), mock_beefy_id(4)], 2).unwrap() )), - beefy_log(ConsensusLog::MmrRoot(array_bytes::hex_n_into_unchecked( - "a73271a0974f1e67d6e9b8dd58e506177a2e556519a330796721e98279a753e2" - ))), + beefy_log(ConsensusLog::MmrRoot(H256::from_slice(&[ + 193, 246, 48, 7, 89, 204, 186, 109, 167, 226, 188, 211, 8, 243, 203, 154, 234, + 235, 136, 210, 245, 7, 209, 27, 241, 90, 156, 113, 137, 65, 191, 139 + ]))), ] ); }); @@ -115,7 +114,7 @@ fn should_contain_valid_leaf_data() { mmr_leaf, MmrLeaf { version: MmrLeafVersion::new(1, 5), - parent_number_and_hash: (0_u64, H256::repeat_byte(0x45)), + parent_number_and_hash: (0_u64, H256::repeat_byte(1)), beefy_next_authority_set: BeefyNextAuthoritySet { id: 2, len: 2, @@ -140,7 +139,7 @@ fn should_contain_valid_leaf_data() { mmr_leaf, MmrLeaf { version: MmrLeafVersion::new(1, 5), - parent_number_and_hash: (1_u64, H256::repeat_byte(0x45)), + parent_number_and_hash: (1_u64, H256::repeat_byte(2)), beefy_next_authority_set: BeefyNextAuthoritySet { id: 3, len: 2, @@ -207,3 +206,128 @@ fn should_update_authorities() { assert_eq!(want, next_auth_set.keyset_commitment); }); } + +#[test] +fn is_non_canonical_should_work_correctly() { + let mut ext = new_test_ext(vec![1, 2]); + + let mut prev_roots = vec![]; + ext.execute_with(|| { + for block_num in 1..=500 { + init_block(block_num); + prev_roots.push(Mmr::mmr_root()) + } + }); + ext.persist_offchain_overlay(); + + // Register offchain ext. + let (offchain, _offchain_state) = TestOffchainExt::with_offchain_db(ext.offchain_db()); + ext.register_extension(OffchainDbExt::new(offchain.clone())); + ext.register_extension(OffchainWorkerExt::new(offchain)); + + ext.execute_with(|| { + let valid_proof = Mmr::generate_ancestry_proof(250, None).unwrap(); + let mut invalid_proof = valid_proof.clone(); + invalid_proof.items.push((300, Default::default())); + + // The commitment is invalid if it has no MMR root payload and the proof is valid. + assert_eq!( + BeefyMmr::is_non_canonical( + &Commitment { + payload: Payload::from_single_entry([0, 0], vec![]), + block_number: 250, + validator_set_id: 0 + }, + valid_proof.clone(), + ), + true + ); + + // If the `commitment.payload` contains an MMR root that doesn't match the ancestry proof, + // it's non-canonical. + assert_eq!( + BeefyMmr::is_non_canonical( + &Commitment { + payload: Payload::from_single_entry( + known_payloads::MMR_ROOT_ID, + H256::repeat_byte(0).encode(), + ), + block_number: 250, + validator_set_id: 0, + }, + valid_proof.clone(), + ), + true + ); + + // Should return false if the proof is invalid, no matter the payload. + assert_eq!( + BeefyMmr::is_non_canonical( + &Commitment { + payload: Payload::from_single_entry( + known_payloads::MMR_ROOT_ID, + H256::repeat_byte(0).encode(), + ), + block_number: 250, + validator_set_id: 0 + }, + invalid_proof, + ), + false + ); + + // Can't prove that the commitment is non-canonical if the `commitment.block_number` + // doesn't match the ancestry proof. + assert_eq!( + BeefyMmr::is_non_canonical( + &Commitment { + payload: Payload::from_single_entry( + known_payloads::MMR_ROOT_ID, + prev_roots[250 - 1].encode(), + ), + block_number: 300, + validator_set_id: 0, + }, + valid_proof, + ), + false + ); + + // For each previous block, the check: + // - should return false, if the commitment is targeting the canonical chain + // - should return true if the commitment is NOT targeting the canonical chain + for prev_block_number in 1usize..=500 { + let proof = Mmr::generate_ancestry_proof(prev_block_number as u64, None).unwrap(); + + assert_eq!( + BeefyMmr::is_non_canonical( + &Commitment { + payload: Payload::from_single_entry( + known_payloads::MMR_ROOT_ID, + prev_roots[prev_block_number - 1].encode(), + ), + block_number: prev_block_number as u64, + validator_set_id: 0, + }, + proof.clone(), + ), + false + ); + + assert_eq!( + BeefyMmr::is_non_canonical( + &Commitment { + payload: Payload::from_single_entry( + known_payloads::MMR_ROOT_ID, + H256::repeat_byte(0).encode(), + ), + block_number: prev_block_number as u64, + validator_set_id: 0, + }, + proof, + ), + true + ) + } + }); +} diff --git a/substrate/frame/beefy/src/lib.rs b/substrate/frame/beefy/src/lib.rs index 63f3e9bb309c..4554392829e0 100644 --- a/substrate/frame/beefy/src/lib.rs +++ b/substrate/frame/beefy/src/lib.rs @@ -41,8 +41,8 @@ use sp_staking::{offence::OffenceReportSystem, SessionIndex}; use sp_std::prelude::*; use sp_consensus_beefy::{ - AuthorityIndex, BeefyAuthorityId, ConsensusLog, DoubleVotingProof, OnNewValidatorSet, - ValidatorSet, BEEFY_ENGINE_ID, GENESIS_AUTHORITY_SET_ID, + AncestryHelper, AuthorityIndex, BeefyAuthorityId, ConsensusLog, DoubleVotingProof, + OnNewValidatorSet, ValidatorSet, BEEFY_ENGINE_ID, GENESIS_AUTHORITY_SET_ID, }; mod default_weights; @@ -98,6 +98,9 @@ pub mod pallet { /// weight MMR root over validators and make it available for Light Clients. type OnNewValidatorSet: OnNewValidatorSet<::BeefyId>; + /// Hook for checking commitment canonicity. + type AncestryHelper: AncestryHelper>; + /// Weights for this pallet. type WeightInfo: WeightInfo; diff --git a/substrate/frame/beefy/src/mock.rs b/substrate/frame/beefy/src/mock.rs index 0b87de6bf5d7..d769b2bcdacd 100644 --- a/substrate/frame/beefy/src/mock.rs +++ b/substrate/frame/beefy/src/mock.rs @@ -37,6 +37,7 @@ use sp_state_machine::BasicExternalities; use crate as pallet_beefy; pub use sp_consensus_beefy::{ecdsa_crypto::AuthorityId as BeefyId, ConsensusLog, BEEFY_ENGINE_ID}; +use sp_consensus_beefy::{AncestryHelper, Commitment}; impl_opaque_keys! { pub struct MockSessionKeys { @@ -80,6 +81,18 @@ parameter_types! { pub const ReportLongevity: u64 = BondingDuration::get() as u64 * SessionsPerEra::get() as u64 * Period::get(); pub const MaxSetIdSessionEntries: u32 = BondingDuration::get() * SessionsPerEra::get(); + + pub storage CommitmentIsTargetingFork: bool = true; +} + +pub struct MockAncestryHelper; + +impl AncestryHelper for MockAncestryHelper { + type Proof = (); + + fn is_non_canonical(_commitment: &Commitment, _proof: Self::Proof) -> bool { + CommitmentIsTargetingFork::get() + } } impl pallet_beefy::Config for Test { @@ -88,6 +101,7 @@ impl pallet_beefy::Config for Test { type MaxNominators = ConstU32<1000>; type MaxSetIdSessionEntries = MaxSetIdSessionEntries; type OnNewValidatorSet = (); + type AncestryHelper = MockAncestryHelper; type WeightInfo = (); type KeyOwnerProof = >::Proof; type EquivocationReportSystem = diff --git a/substrate/frame/merkle-mountain-range/src/lib.rs b/substrate/frame/merkle-mountain-range/src/lib.rs index a86443f2e011..47a325db605d 100644 --- a/substrate/frame/merkle-mountain-range/src/lib.rs +++ b/substrate/frame/merkle-mountain-range/src/lib.rs @@ -282,6 +282,19 @@ where } } +/// Stateless ancestry proof verification. +pub fn verify_ancestry_proof( + root: H::Output, + ancestry_proof: primitives::AncestryProof, +) -> Result +where + H: traits::Hash, + L: primitives::FullLeaf, +{ + mmr::verify_ancestry_proof::(root, ancestry_proof) + .map_err(|_| Error::Verify.log_debug(("The ancestry proof is incorrect.", root))) +} + impl, I: 'static> Pallet { /// Build offchain key from `parent_hash` of block that originally added node `pos` to MMR. /// @@ -303,17 +316,14 @@ impl, I: 'static> Pallet { } /// Provide the parent number for the block that added `leaf_index` to the MMR. - fn leaf_index_to_parent_block_num( - leaf_index: LeafIndex, - leaves_count: LeafIndex, - ) -> BlockNumberFor { + fn leaf_index_to_parent_block_num(leaf_index: LeafIndex) -> BlockNumberFor { // leaves are zero-indexed and were added one per block since pallet activation, // while block numbers are one-indexed, so block number that added `leaf_idx` is: // `block_num = block_num_when_pallet_activated + leaf_idx + 1` // `block_num = (current_block_num - leaves_count) + leaf_idx + 1` // `parent_block_num = current_block_num - leaves_count + leaf_idx`. >::block_number() - .saturating_sub(leaves_count.saturated_into()) + .saturating_sub(Self::mmr_leaves().saturated_into()) .saturating_add(leaf_index.saturated_into()) } @@ -330,6 +340,15 @@ impl, I: 'static> Pallet { utils::block_num_to_leaf_index::>(block_num, first_mmr_block) } + /// Convert a block number into a leaf index. + pub fn block_num_to_leaf_count(block_num: BlockNumberFor) -> Result + where + T: frame_system::Config, + { + let leaf_index = Self::block_num_to_leaf_index(block_num)?; + Ok(leaf_index.saturating_add(1)) + } + /// Generate an MMR proof for the given `block_numbers`. /// If `best_known_block_number = Some(n)`, this generates a historical proof for /// the chain with head at height `n`. @@ -347,8 +366,7 @@ impl, I: 'static> Pallet { let best_known_block_number = best_known_block_number.unwrap_or_else(|| >::block_number()); - let leaves_count = - Self::block_num_to_leaf_index(best_known_block_number)?.saturating_add(1); + let leaf_count = Self::block_num_to_leaf_count(best_known_block_number)?; // we need to translate the block_numbers into leaf indices. let leaf_indices = block_numbers @@ -358,7 +376,7 @@ impl, I: 'static> Pallet { }) .collect::, _>>()?; - let mmr: ModuleMmr = mmr::Mmr::new(leaves_count); + let mmr: ModuleMmr = mmr::Mmr::new(leaf_count); mmr.generate_proof(leaf_indices) } @@ -374,7 +392,7 @@ impl, I: 'static> Pallet { ) -> Result<(), primitives::Error> { if proof.leaf_count > NumberOfLeaves::::get() || proof.leaf_count == 0 || - (proof.items.len().saturating_add(leaves.len())) as u64 > proof.leaf_count + proof.items.len().saturating_add(leaves.len()) as u64 > proof.leaf_count { return Err(primitives::Error::Verify .log_debug("The proof has incorrect number of leaves or proof items.")) @@ -397,24 +415,18 @@ impl, I: 'static> Pallet { let best_known_block_number = best_known_block_number.unwrap_or_else(|| >::block_number()); - let leaf_count = Self::block_num_to_leaf_index(best_known_block_number)?.saturating_add(1); - let prev_leaf_count = Self::block_num_to_leaf_index(prev_block_number)?.saturating_add(1); + let leaf_count = Self::block_num_to_leaf_count(best_known_block_number)?; + let prev_leaf_count = Self::block_num_to_leaf_count(prev_block_number)?; let mmr: ModuleMmr = mmr::Mmr::new(leaf_count); mmr.generate_ancestry_proof(prev_leaf_count) } pub fn verify_ancestry_proof( + root: HashOf, ancestry_proof: primitives::AncestryProof>, - ) -> Result<(), Error> { - let mmr: ModuleMmr = - mmr::Mmr::new(ancestry_proof.leaf_count); - let is_valid = mmr.verify_ancestry_proof(ancestry_proof)?; - if is_valid { - Ok(()) - } else { - Err(Error::Verify.log_debug("The ancestry proof is incorrect.")) - } + ) -> Result, Error> { + verify_ancestry_proof::, LeafOf>(root, ancestry_proof) } /// Return the on-chain MMR root hash. diff --git a/substrate/frame/merkle-mountain-range/src/mmr/mmr.rs b/substrate/frame/merkle-mountain-range/src/mmr/mmr.rs index 5efc172d1e93..8a99f4d87deb 100644 --- a/substrate/frame/merkle-mountain-range/src/mmr/mmr.rs +++ b/substrate/frame/merkle-mountain-range/src/mmr/mmr.rs @@ -60,6 +60,42 @@ where .map_err(|e| Error::Verify.log_debug(e)) } +pub fn verify_ancestry_proof( + root: H::Output, + ancestry_proof: primitives::AncestryProof, +) -> Result +where + H: sp_runtime::traits::Hash, + L: primitives::FullLeaf, +{ + let mmr_size = NodesUtils::new(ancestry_proof.leaf_count).size(); + + let prev_peaks_proof = mmr_lib::NodeMerkleProof::, Hasher>::new( + mmr_size, + ancestry_proof + .items + .into_iter() + .map(|(index, hash)| (index, Node::Hash(hash))) + .collect(), + ); + + let raw_ancestry_proof = mmr_lib::AncestryProof::, Hasher> { + prev_peaks: ancestry_proof.prev_peaks.into_iter().map(|hash| Node::Hash(hash)).collect(), + prev_size: mmr_lib::helper::leaf_index_to_mmr_size(ancestry_proof.prev_leaf_count - 1), + proof: prev_peaks_proof, + }; + + let prev_root = mmr_lib::ancestry_proof::bagging_peaks_hashes::, Hasher>( + raw_ancestry_proof.prev_peaks.clone(), + ) + .map_err(|e| Error::Verify.log_debug(e))?; + raw_ancestry_proof + .verify_ancestor(Node::Hash(root), prev_root.clone()) + .map_err(|e| Error::Verify.log_debug(e))?; + + Ok(prev_root.hash()) +} + /// A wrapper around an MMR library to expose limited functionality. /// /// Available functions depend on the storage kind ([Runtime](crate::mmr::storage::RuntimeStorage) @@ -119,44 +155,6 @@ where .map_err(|e| Error::Verify.log_debug(e)) } - pub fn verify_ancestry_proof( - &self, - ancestry_proof: primitives::AncestryProof>, - ) -> Result { - let prev_peaks_proof = - mmr_lib::NodeMerkleProof::, Hasher, L>>::new( - self.mmr.mmr_size(), - ancestry_proof - .items - .into_iter() - .map(|(index, hash)| (index, Node::Hash(hash))) - .collect(), - ); - - let raw_ancestry_proof = mmr_lib::AncestryProof::< - NodeOf, - Hasher, L>, - > { - prev_peaks: ancestry_proof - .prev_peaks - .into_iter() - .map(|hash| Node::Hash(hash)) - .collect(), - prev_size: mmr_lib::helper::leaf_index_to_mmr_size(ancestry_proof.prev_leaf_count - 1), - proof: prev_peaks_proof, - }; - - let prev_root = mmr_lib::ancestry_proof::bagging_peaks_hashes::< - NodeOf, - Hasher, L>, - >(raw_ancestry_proof.prev_peaks.clone()) - .map_err(|e| Error::Verify.log_debug(e))?; - let root = self.mmr.get_root().map_err(|e| Error::GetRoot.log_error(e))?; - raw_ancestry_proof - .verify_ancestor(root, prev_root) - .map_err(|e| Error::Verify.log_debug(e)) - } - /// Return the internal size of the MMR (number of nodes). #[cfg(test)] pub fn size(&self) -> NodeIndex { diff --git a/substrate/frame/merkle-mountain-range/src/mmr/mod.rs b/substrate/frame/merkle-mountain-range/src/mmr/mod.rs index 93fefe910e45..5b73f53506e9 100644 --- a/substrate/frame/merkle-mountain-range/src/mmr/mod.rs +++ b/substrate/frame/merkle-mountain-range/src/mmr/mod.rs @@ -21,7 +21,7 @@ pub mod storage; use sp_mmr_primitives::{mmr_lib, DataOrHash, FullLeaf}; use sp_runtime::traits; -pub use self::mmr::{verify_leaves_proof, Mmr}; +pub use self::mmr::{verify_ancestry_proof, verify_leaves_proof, Mmr}; /// Node type for runtime `T`. pub type NodeOf = Node<>::Hashing, L>; diff --git a/substrate/frame/merkle-mountain-range/src/mmr/storage.rs b/substrate/frame/merkle-mountain-range/src/mmr/storage.rs index 6848b8f1b990..e27440be35c4 100644 --- a/substrate/frame/merkle-mountain-range/src/mmr/storage.rs +++ b/substrate/frame/merkle-mountain-range/src/mmr/storage.rs @@ -67,7 +67,6 @@ where L: primitives::FullLeaf + codec::Decode, { fn get_elem(&self, pos: NodeIndex) -> mmr_lib::Result>> { - let leaves = NumberOfLeaves::::get(); // Find out which leaf added node `pos` in the MMR. let ancestor_leaf_idx = NodesUtils::leaf_index_that_added_node(pos); @@ -86,7 +85,7 @@ where // Fall through to searching node using fork-specific key. let ancestor_parent_block_num = - Pallet::::leaf_index_to_parent_block_num(ancestor_leaf_idx, leaves); + Pallet::::leaf_index_to_parent_block_num(ancestor_leaf_idx); let ancestor_parent_hash = T::BlockHashProvider::block_hash(ancestor_parent_block_num); let temp_key = Pallet::::node_temp_offchain_key(pos, ancestor_parent_hash); debug!( diff --git a/substrate/frame/merkle-mountain-range/src/tests.rs b/substrate/frame/merkle-mountain-range/src/tests.rs index f8cfcb4e2c28..e83efaac2c6e 100644 --- a/substrate/frame/merkle-mountain-range/src/tests.rs +++ b/substrate/frame/merkle-mountain-range/src/tests.rs @@ -787,24 +787,3 @@ fn does_not_panic_when_generating_historical_proofs() { ); }); } - -#[test] -fn generating_and_verifying_ancestry_proofs_works_correctly() { - let _ = env_logger::try_init(); - let mut ext = new_test_ext(); - ext.execute_with(|| add_blocks(500)); - ext.persist_offchain_overlay(); - register_offchain_ext(&mut ext); - - ext.execute_with(|| { - // Check that generating and verifying ancestry proofs works correctly - // for each previous block - for prev_block_number in 1..501 { - let proof = Pallet::::generate_ancestry_proof(prev_block_number, None).unwrap(); - Pallet::::verify_ancestry_proof(proof).unwrap(); - } - - // Check that we can't generate ancestry proofs for a future block. - assert_eq!(Pallet::::generate_ancestry_proof(501, None), Err(Error::GenerateProof)); - }); -} diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index 390c0ff71273..349c1c0c2e84 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -393,6 +393,16 @@ impl OnNewValidatorSet for () { fn on_new_validator_set(_: &ValidatorSet, _: &ValidatorSet) {} } +/// Hook containing helper methods for proving/checking commitment canonicity. +pub trait AncestryHelper { + /// Type containing proved info about the canonical chain at a certain height. + type Proof; + + /// Check if a commitment is pointing to a header on a non-canonical chain + /// against a canonicity proof generated at the same header height. + fn is_non_canonical(commitment: &Commitment, proof: Self::Proof) -> bool; +} + /// An opaque type used to represent the key ownership proof at the runtime API /// boundary. The inner value is an encoded representation of the actual key /// ownership proof which will be parameterized when defining the runtime. At diff --git a/substrate/primitives/consensus/beefy/src/payload.rs b/substrate/primitives/consensus/beefy/src/payload.rs index 1a06e620e7ad..d22255c384bc 100644 --- a/substrate/primitives/consensus/beefy/src/payload.rs +++ b/substrate/primitives/consensus/beefy/src/payload.rs @@ -58,7 +58,7 @@ impl Payload { /// Returns a decoded payload value under given `id`. /// - /// In case the value is not there or it cannot be decoded does not match `None` is returned. + /// In case the value is not there, or it cannot be decoded `None` is returned. pub fn get_decoded(&self, id: &BeefyPayloadId) -> Option { self.get_raw(id).and_then(|raw| T::decode(&mut &raw[..]).ok()) } From 8a7ac614cc734c3641e65c3429908681450dba0f Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Fri, 17 May 2024 17:18:21 +0300 Subject: [PATCH 02/14] Renamings report_equivocation -> report_double_voting --- polkadot/node/service/src/fake_runtime_api.rs | 2 +- polkadot/runtime/rococo/src/lib.rs | 6 ++--- polkadot/runtime/test-runtime/src/lib.rs | 2 +- polkadot/runtime/westend/src/lib.rs | 5 ++-- substrate/bin/node/runtime/src/lib.rs | 6 ++--- .../client/consensus/beefy/src/fisherman.rs | 2 +- substrate/client/consensus/beefy/src/tests.rs | 2 +- substrate/frame/beefy/src/default_weights.rs | 2 +- substrate/frame/beefy/src/equivocation.rs | 6 ++--- substrate/frame/beefy/src/lib.rs | 12 ++++----- substrate/frame/beefy/src/tests.rs | 26 +++++++++---------- .../primitives/consensus/beefy/src/lib.rs | 4 +-- 12 files changed, 38 insertions(+), 37 deletions(-) diff --git a/polkadot/node/service/src/fake_runtime_api.rs b/polkadot/node/service/src/fake_runtime_api.rs index 03c4836020d9..a2e1e0115476 100644 --- 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( _: beefy_primitives::DoubleVotingProof< BlockNumber, BeefyId, diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index 1f7f485e75ae..c8ac5b5bba2c 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -2084,7 +2084,7 @@ sp_api::impl_runtime_apis! { } } - #[api_version(3)] + #[api_version(4)] impl beefy_primitives::BeefyApi for Runtime { fn beefy_genesis() -> Option { pallet_beefy::GenesisBlock::::get() @@ -2094,7 +2094,7 @@ sp_api::impl_runtime_apis! { Beefy::validator_set() } - fn submit_report_equivocation_unsigned_extrinsic( + fn submit_report_double_voting_unsigned_extrinsic( equivocation_proof: beefy_primitives::DoubleVotingProof< BlockNumber, BeefyId, @@ -2104,7 +2104,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 index 9eb0fcca6678..e18c2bff9f39 100644 --- a/polkadot/runtime/test-runtime/src/lib.rs +++ b/polkadot/runtime/test-runtime/src/lib.rs @@ -1013,7 +1013,7 @@ sp_api::impl_runtime_apis! { None } - fn submit_report_equivocation_unsigned_extrinsic( + fn submit_report_double_voting_unsigned_extrinsic( _equivocation_proof: beefy_primitives::DoubleVotingProof< BlockNumber, BeefyId, diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index 1dc372bcacaf..34e8c53b2ad1 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -1965,6 +1965,7 @@ sp_api::impl_runtime_apis! { } } + #[api_version(4)] impl beefy_primitives::BeefyApi for Runtime { fn beefy_genesis() -> Option { pallet_beefy::GenesisBlock::::get() @@ -1974,7 +1975,7 @@ sp_api::impl_runtime_apis! { Beefy::validator_set() } - fn submit_report_equivocation_unsigned_extrinsic( + fn submit_report_double_voting_unsigned_extrinsic( equivocation_proof: beefy_primitives::DoubleVotingProof< BlockNumber, BeefyId, @@ -1984,7 +1985,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/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index a89e7b314f0b..086b95e2bde4 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -3030,7 +3030,7 @@ impl_runtime_apis! { } } - #[api_version(3)] + #[api_version(4)] impl sp_consensus_beefy::BeefyApi for Runtime { fn beefy_genesis() -> Option { pallet_beefy::GenesisBlock::::get() @@ -3040,7 +3040,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, @@ -3050,7 +3050,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 index a2b4c8f945d1..991f936fabe2 100644 --- a/substrate/client/consensus/beefy/src/fisherman.rs +++ b/substrate/client/consensus/beefy/src/fisherman.rs @@ -149,7 +149,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 index 2bb145d660df..5a370298a209 100644 --- a/substrate/client/consensus/beefy/src/tests.rs +++ b/substrate/client/consensus/beefy/src/tests.rs @@ -312,7 +312,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, AuthorityId, Signature>, _dummy: OpaqueKeyOwnershipProof, ) -> Option<()> { diff --git a/substrate/frame/beefy/src/default_weights.rs b/substrate/frame/beefy/src/default_weights.rs index 8042f0c932eb..6d0c941081e8 100644 --- a/substrate/frame/beefy/src/default_weights.rs +++ b/substrate/frame/beefy/src/default_weights.rs @@ -24,7 +24,7 @@ use frame_support::weights::{ }; impl crate::WeightInfo for () { - fn report_equivocation(validator_count: u32, max_nominators_per_validator: u32) -> Weight { + fn report_double_voting(validator_count: u32, max_nominators_per_validator: u32) -> Weight { // we take the validator set count from the membership proof to // calculate the weight but we set a floor of 100 validators. let validator_count = validator_count.max(100) as u64; diff --git a/substrate/frame/beefy/src/equivocation.rs b/substrate/frame/beefy/src/equivocation.rs index aecc9e721d5c..b3ea9df49789 100644 --- a/substrate/frame/beefy/src/equivocation.rs +++ b/substrate/frame/beefy/src/equivocation.rs @@ -150,7 +150,7 @@ where use frame_system::offchain::SubmitTransaction; let (equivocation_proof, key_owner_proof) = evidence; - let call = Call::report_equivocation_unsigned { + let call = Call::report_double_voting_unsigned { equivocation_proof: Box::new(equivocation_proof), key_owner_proof, }; @@ -239,7 +239,7 @@ where /// unsigned equivocation reports. impl Pallet { pub fn validate_unsigned(source: TransactionSource, call: &Call) -> TransactionValidity { - if let Call::report_equivocation_unsigned { equivocation_proof, key_owner_proof } = call { + if let Call::report_double_voting_unsigned { equivocation_proof, key_owner_proof } = call { // discard equivocation report not coming from the local node match source { TransactionSource::Local | TransactionSource::InBlock => { /* allowed */ }, @@ -277,7 +277,7 @@ impl Pallet { } pub fn pre_dispatch(call: &Call) -> Result<(), TransactionValidityError> { - if let Call::report_equivocation_unsigned { equivocation_proof, key_owner_proof } = call { + if let Call::report_double_voting_unsigned { equivocation_proof, key_owner_proof } = call { let evidence = (*equivocation_proof.clone(), key_owner_proof.clone()); T::EquivocationReportSystem::check_evidence(evidence) } else { diff --git a/substrate/frame/beefy/src/lib.rs b/substrate/frame/beefy/src/lib.rs index 4554392829e0..06987052ce99 100644 --- a/substrate/frame/beefy/src/lib.rs +++ b/substrate/frame/beefy/src/lib.rs @@ -206,11 +206,11 @@ pub mod pallet { /// against the extracted offender. If both are valid, the offence /// will be reported. #[pallet::call_index(0)] - #[pallet::weight(T::WeightInfo::report_equivocation( + #[pallet::weight(T::WeightInfo::report_double_voting( key_owner_proof.validator_count(), T::MaxNominators::get(), ))] - pub fn report_equivocation( + pub fn report_double_voting( origin: OriginFor, equivocation_proof: Box< DoubleVotingProof< @@ -241,11 +241,11 @@ pub mod pallet { /// if the block author is defined it will be defined as the equivocation /// reporter. #[pallet::call_index(1)] - #[pallet::weight(T::WeightInfo::report_equivocation( + #[pallet::weight(T::WeightInfo::report_double_voting( key_owner_proof.validator_count(), T::MaxNominators::get(), ))] - pub fn report_equivocation_unsigned( + pub fn report_double_voting_unsigned( origin: OriginFor, equivocation_proof: Box< DoubleVotingProof< @@ -370,7 +370,7 @@ impl Pallet { /// Submits an extrinsic to report an equivocation. This method will create /// an unsigned extrinsic with a call to `report_equivocation_unsigned` and /// will push the transaction to the pool. Only useful in an offchain context. - pub fn submit_unsigned_equivocation_report( + pub fn submit_unsigned_double_voting_report( equivocation_proof: DoubleVotingProof< BlockNumberFor, T::BeefyId, @@ -529,6 +529,6 @@ impl IsMember for Pallet { } pub trait WeightInfo { - fn report_equivocation(validator_count: u32, max_nominators_per_validator: u32) -> Weight; + fn report_double_voting(validator_count: u32, max_nominators_per_validator: u32) -> Weight; fn set_new_genesis() -> Weight; } diff --git a/substrate/frame/beefy/src/tests.rs b/substrate/frame/beefy/src/tests.rs index 6a6aa245ce1f..8b7358fc7d80 100644 --- a/substrate/frame/beefy/src/tests.rs +++ b/substrate/frame/beefy/src/tests.rs @@ -310,7 +310,7 @@ fn report_equivocation_current_set_works() { let key_owner_proof = Historical::prove((BEEFY_KEY_TYPE, &equivocation_key)).unwrap(); // report the equivocation and the tx should be dispatched successfully - assert_ok!(Beefy::report_equivocation_unsigned( + assert_ok!(Beefy::report_double_voting_unsigned( RuntimeOrigin::none(), Box::new(equivocation_proof), key_owner_proof, @@ -393,7 +393,7 @@ fn report_equivocation_old_set_works() { ); // report the equivocation and the tx should be dispatched successfully - assert_ok!(Beefy::report_equivocation_unsigned( + assert_ok!(Beefy::report_double_voting_unsigned( RuntimeOrigin::none(), Box::new(equivocation_proof), key_owner_proof, @@ -456,7 +456,7 @@ fn report_equivocation_invalid_set_id() { // the call for reporting the equivocation should error assert_err!( - Beefy::report_equivocation_unsigned( + Beefy::report_double_voting_unsigned( RuntimeOrigin::none(), Box::new(equivocation_proof), key_owner_proof, @@ -499,7 +499,7 @@ fn report_equivocation_invalid_session() { // report an equivocation for the current set using an key ownership // proof from the previous set, the session should be invalid. assert_err!( - Beefy::report_equivocation_unsigned( + Beefy::report_double_voting_unsigned( RuntimeOrigin::none(), Box::new(equivocation_proof), key_owner_proof, @@ -547,7 +547,7 @@ fn report_equivocation_invalid_key_owner_proof() { // report an equivocation for the current set using a key ownership // proof for a different key than the one in the equivocation proof. assert_err!( - Beefy::report_equivocation_unsigned( + Beefy::report_double_voting_unsigned( RuntimeOrigin::none(), Box::new(equivocation_proof), invalid_key_owner_proof, @@ -578,7 +578,7 @@ fn report_equivocation_invalid_equivocation_proof() { let assert_invalid_equivocation_proof = |equivocation_proof| { assert_err!( - Beefy::report_equivocation_unsigned( + Beefy::report_double_voting_unsigned( RuntimeOrigin::none(), Box::new(equivocation_proof), key_owner_proof.clone(), @@ -656,7 +656,7 @@ fn report_equivocation_validate_unsigned_prevents_duplicates() { let key_owner_proof = Historical::prove((BEEFY_KEY_TYPE, &equivocation_key)).unwrap(); - let call = Call::report_equivocation_unsigned { + let call = Call::report_double_voting_unsigned { equivocation_proof: Box::new(equivocation_proof.clone()), key_owner_proof: key_owner_proof.clone(), }; @@ -691,7 +691,7 @@ fn report_equivocation_validate_unsigned_prevents_duplicates() { assert_ok!(::pre_dispatch(&call)); // we submit the report - Beefy::report_equivocation_unsigned( + Beefy::report_double_voting_unsigned( RuntimeOrigin::none(), Box::new(equivocation_proof), key_owner_proof, @@ -720,7 +720,7 @@ fn report_equivocation_has_valid_weight() { // the weight depends on the size of the validator set, // but there's a lower bound of 100 validators. assert!((1..=100) - .map(|validators| ::WeightInfo::report_equivocation(validators, 1000)) + .map(|validators| ::WeightInfo::report_double_voting(validators, 1000)) .collect::>() .windows(2) .all(|w| w[0] == w[1])); @@ -728,7 +728,7 @@ fn report_equivocation_has_valid_weight() { // after 100 validators the weight should keep increasing // with every extra validator. assert!((100..=1000) - .map(|validators| ::WeightInfo::report_equivocation(validators, 1000)) + .map(|validators| ::WeightInfo::report_double_voting(validators, 1000)) .collect::>() .windows(2) .all(|w| w[0].ref_time() < w[1].ref_time())); @@ -762,7 +762,7 @@ fn valid_equivocation_reports_dont_pay_fees() { let key_owner_proof = Historical::prove((BEEFY_KEY_TYPE, &equivocation_key)).unwrap(); // check the dispatch info for the call. - let info = Call::::report_equivocation_unsigned { + let info = Call::::report_double_voting_unsigned { equivocation_proof: Box::new(equivocation_proof.clone()), key_owner_proof: key_owner_proof.clone(), } @@ -773,7 +773,7 @@ fn valid_equivocation_reports_dont_pay_fees() { assert_eq!(info.pays_fee, Pays::Yes); // report the equivocation. - let post_info = Beefy::report_equivocation_unsigned( + let post_info = Beefy::report_double_voting_unsigned( RuntimeOrigin::none(), Box::new(equivocation_proof.clone()), key_owner_proof.clone(), @@ -787,7 +787,7 @@ fn valid_equivocation_reports_dont_pay_fees() { // report the equivocation again which is invalid now since it is // duplicate. - let post_info = Beefy::report_equivocation_unsigned( + let post_info = Beefy::report_double_voting_unsigned( RuntimeOrigin::none(), Box::new(equivocation_proof), key_owner_proof, diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index 349c1c0c2e84..8c376ef3c750 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -427,7 +427,7 @@ impl OpaqueKeyOwnershipProof { sp_api::decl_runtime_apis! { /// API necessary for BEEFY voters. - #[api_version(3)] + #[api_version(4)] pub trait BeefyApi where AuthorityId : Codec + RuntimeAppPublic, { @@ -445,7 +445,7 @@ sp_api::decl_runtime_apis! { /// `None` when creation of the extrinsic fails, e.g. if equivocation /// reporting is disabled for the given runtime (i.e. this method is /// hardcoded to return `None`). Only useful in an offchain context. - fn submit_report_equivocation_unsigned_extrinsic( + fn submit_report_double_voting_unsigned_extrinsic( equivocation_proof: DoubleVotingProof, AuthorityId, ::Signature>, key_owner_proof: OpaqueKeyOwnershipProof, From 1ff72ef209a9ed002d539dc3205ee8643d1b5d19 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Fri, 17 May 2024 14:30:47 +0300 Subject: [PATCH 03/14] Add support for reporting fork voting --- .../client/consensus/beefy/src/worker.rs | 6 +- substrate/frame/beefy/src/default_weights.rs | 5 + substrate/frame/beefy/src/equivocation.rs | 250 +++++--- substrate/frame/beefy/src/lib.rs | 119 +++- substrate/frame/beefy/src/mock.rs | 13 +- substrate/frame/beefy/src/tests.rs | 543 +++++++++++++++++- .../primitives/consensus/beefy/src/lib.rs | 24 +- .../consensus/beefy/src/test_utils.rs | 39 +- 8 files changed, 850 insertions(+), 149 deletions(-) diff --git a/substrate/client/consensus/beefy/src/worker.rs b/substrate/client/consensus/beefy/src/worker.rs index cfbb3d63aea4..78c0639a7218 100644 --- a/substrate/client/consensus/beefy/src/worker.rs +++ b/substrate/client/consensus/beefy/src/worker.rs @@ -1014,7 +1014,7 @@ pub(crate) mod tests { 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}; @@ -1555,7 +1555,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), ); @@ -1587,7 +1587,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/src/default_weights.rs b/substrate/frame/beefy/src/default_weights.rs index 6d0c941081e8..5eec3c145391 100644 --- a/substrate/frame/beefy/src/default_weights.rs +++ b/substrate/frame/beefy/src/default_weights.rs @@ -50,6 +50,11 @@ impl crate::WeightInfo for () { .saturating_add(DbWeight::get().reads(2)) } + // TODO: Calculate + fn report_fork_voting(_validator_count: u32, _max_nominators_per_validator: u32) -> Weight { + Weight::MAX + } + fn set_new_genesis() -> Weight { DbWeight::get().writes(1) } diff --git a/substrate/frame/beefy/src/equivocation.rs b/substrate/frame/beefy/src/equivocation.rs index b3ea9df49789..b066891d5815 100644 --- a/substrate/frame/beefy/src/equivocation.rs +++ b/substrate/frame/beefy/src/equivocation.rs @@ -38,7 +38,10 @@ use codec::{self as codec, Decode, Encode}; use frame_support::traits::{Get, KeyOwnerProofSystem}; use frame_system::pallet_prelude::BlockNumberFor; use log::{error, info}; -use sp_consensus_beefy::{DoubleVotingProof, ValidatorSetId, KEY_TYPE as BEEFY_KEY_TYPE}; +use sp_consensus_beefy::{ + check_commitment_signature, AncestryHelper, DoubleVotingProof, ForkVotingProof, ValidatorSetId, + KEY_TYPE as BEEFY_KEY_TYPE, +}; use sp_runtime::{ transaction_validity::{ InvalidTransaction, TransactionPriority, TransactionSource, TransactionValidity, @@ -118,18 +121,110 @@ where /// `offchain::SendTransactionTypes`. /// - On-chain validity checks and processing are mostly delegated to the user provided generic /// types implementing `KeyOwnerProofSystem` and `ReportOffence` traits. -/// - Offence reporter for unsigned transactions is fetched via the the authorship pallet. +/// - Offence reporter for unsigned transactions is fetched via the authorship pallet. pub struct EquivocationReportSystem(sp_std::marker::PhantomData<(T, R, P, L)>); /// Equivocation evidence convenience alias. -pub type EquivocationEvidenceFor = ( - DoubleVotingProof< - BlockNumberFor, - ::BeefyId, - <::BeefyId as RuntimeAppPublic>::Signature, - >, - ::KeyOwnerProof, -); +pub enum EquivocationEvidenceFor { + DoubleVotingProof( + DoubleVotingProof< + BlockNumberFor, + T::BeefyId, + ::Signature, + >, + T::KeyOwnerProof, + ), + ForkVotingProof( + ForkVotingProof< + BlockNumberFor, + T::BeefyId, + >>::Proof, + >, + T::KeyOwnerProof, + ), +} + +impl EquivocationEvidenceFor { + /// Returns the authority id of the equivocator. + fn offender_id(&self) -> &T::BeefyId { + match self { + EquivocationEvidenceFor::DoubleVotingProof(equivocation_proof, _) => + equivocation_proof.offender_id(), + EquivocationEvidenceFor::ForkVotingProof(equivocation_proof, _) => + &equivocation_proof.vote.id, + } + } + + /// Returns the round number at which the equivocation occurred. + fn round_number(&self) -> &BlockNumberFor { + match self { + EquivocationEvidenceFor::DoubleVotingProof(equivocation_proof, _) => + equivocation_proof.round_number(), + EquivocationEvidenceFor::ForkVotingProof(equivocation_proof, _) => + &equivocation_proof.vote.commitment.block_number, + } + } + + /// Returns the set id at which the equivocation occurred. + fn set_id(&self) -> ValidatorSetId { + match self { + EquivocationEvidenceFor::DoubleVotingProof(equivocation_proof, _) => + equivocation_proof.set_id(), + EquivocationEvidenceFor::ForkVotingProof(equivocation_proof, _) => + equivocation_proof.vote.commitment.validator_set_id, + } + } + + /// Returns the set id at which the equivocation occurred. + fn key_owner_proof(&self) -> &T::KeyOwnerProof { + match self { + EquivocationEvidenceFor::DoubleVotingProof(_, key_owner_proof) => key_owner_proof, + EquivocationEvidenceFor::ForkVotingProof(_, key_owner_proof) => key_owner_proof, + } + } + + fn checked_offender

(&self) -> Option + where + P: KeyOwnerProofSystem<(KeyTypeId, T::BeefyId), Proof = T::KeyOwnerProof>, + { + let key = (BEEFY_KEY_TYPE, self.offender_id().clone()); + P::check_proof(key, self.key_owner_proof().clone()) + } + + fn check_equivocation_proof(self) -> Result<(), Error> { + match self { + EquivocationEvidenceFor::DoubleVotingProof(equivocation_proof, _) => { + // Validate equivocation proof (check votes are different and signatures are valid). + if !sp_consensus_beefy::check_equivocation_proof(&equivocation_proof) { + return Err(Error::::InvalidDoubleVotingProof); + } + + return Ok(()) + }, + EquivocationEvidenceFor::ForkVotingProof(equivocation_proof, _) => { + let (vote, ancestry_proof) = + (equivocation_proof.vote, equivocation_proof.ancestry_proof); + + let is_non_canonical = + >>::is_non_canonical( + &vote.commitment, + ancestry_proof, + ); + if !is_non_canonical { + return Err(Error::::InvalidForkVotingProof); + } + + let is_signature_valid = + check_commitment_signature(&vote.commitment, &vote.id, &vote.signature); + if !is_signature_valid { + return Err(Error::::InvalidForkVotingProof); + } + + Ok(()) + }, + } + } +} impl OffenceReportSystem, EquivocationEvidenceFor> for EquivocationReportSystem @@ -148,13 +243,8 @@ where fn publish_evidence(evidence: EquivocationEvidenceFor) -> Result<(), ()> { use frame_system::offchain::SubmitTransaction; - let (equivocation_proof, key_owner_proof) = evidence; - - let call = Call::report_double_voting_unsigned { - equivocation_proof: Box::new(equivocation_proof), - key_owner_proof, - }; + let call: Call = evidence.into(); let res = SubmitTransaction::>::submit_unsigned_transaction(call.into()); match res { Ok(_) => info!(target: LOG_TARGET, "Submitted equivocation report."), @@ -166,18 +256,10 @@ where fn check_evidence( evidence: EquivocationEvidenceFor, ) -> Result<(), TransactionValidityError> { - let (equivocation_proof, key_owner_proof) = evidence; - - // Check the membership proof to extract the offender's id - let key = (BEEFY_KEY_TYPE, equivocation_proof.offender_id().clone()); - let offender = P::check_proof(key, key_owner_proof).ok_or(InvalidTransaction::BadProof)?; + let offender = evidence.checked_offender::

().ok_or(InvalidTransaction::BadProof)?; // Check if the offence has already been reported, and if so then we can discard the report. - let time_slot = TimeSlot { - set_id: equivocation_proof.set_id(), - round: *equivocation_proof.round_number(), - }; - + let time_slot = TimeSlot { set_id: evidence.set_id(), round: *evidence.round_number() }; if R::is_known_offence(&[offender], &time_slot) { Err(InvalidTransaction::Stale.into()) } else { @@ -189,47 +271,37 @@ where reporter: Option, evidence: EquivocationEvidenceFor, ) -> Result<(), DispatchError> { - let (equivocation_proof, key_owner_proof) = evidence; let reporter = reporter.or_else(|| pallet_authorship::Pallet::::author()); - let offender = equivocation_proof.offender_id().clone(); - - // We check the equivocation within the context of its set id (and - // associated session) and round. We also need to know the validator - // set count at the time of the offence since it is required to calculate - // the slash amount. - let set_id = equivocation_proof.set_id(); - let round = *equivocation_proof.round_number(); - let session_index = key_owner_proof.session(); - let validator_set_count = key_owner_proof.validator_count(); - // Validate the key ownership proof extracting the id of the offender. - let offender = P::check_proof((BEEFY_KEY_TYPE, offender), key_owner_proof) - .ok_or(Error::::InvalidKeyOwnershipProof)?; + // We check the equivocation within the context of its set id (and associated session). + let set_id = evidence.set_id(); + let round = *evidence.round_number(); + let set_id_session_index = crate::SetIdSession::::get(set_id) + .ok_or(Error::::InvalidEquivocationProofSession)?; - // Validate equivocation proof (check votes are different and signatures are valid). - if !sp_consensus_beefy::check_equivocation_proof(&equivocation_proof) { - return Err(Error::::InvalidEquivocationProof.into()) - } - - // Check that the session id for the membership proof is within the - // bounds of the set id reported in the equivocation. - let set_id_session_index = - crate::SetIdSession::::get(set_id).ok_or(Error::::InvalidEquivocationProof)?; + // Check that the session id for the membership proof is within the bounds + // of the set id reported in the equivocation. + let key_owner_proof = evidence.key_owner_proof(); + let validator_count = key_owner_proof.validator_count(); + let session_index = key_owner_proof.session(); if session_index != set_id_session_index { - return Err(Error::::InvalidEquivocationProof.into()) + return Err(Error::::InvalidEquivocationProofSession.into()) } + // Validate the key ownership proof extracting the id of the offender. + let offender = + evidence.checked_offender::

().ok_or(Error::::InvalidKeyOwnershipProof)?; + + evidence.check_equivocation_proof()?; + let offence = EquivocationOffence { time_slot: TimeSlot { set_id, round }, session_index, - validator_set_count, + validator_set_count: validator_count, offender, }; - R::report_offence(reporter.into_iter().collect(), offence) - .map_err(|_| Error::::DuplicateOffenceReport)?; - - Ok(()) + .map_err(|_| Error::::DuplicateOffenceReport.into()) } } @@ -239,49 +311,37 @@ where /// unsigned equivocation reports. impl Pallet { pub fn validate_unsigned(source: TransactionSource, call: &Call) -> TransactionValidity { - if let Call::report_double_voting_unsigned { equivocation_proof, key_owner_proof } = call { - // discard equivocation report not coming from the local node - match source { - TransactionSource::Local | TransactionSource::InBlock => { /* allowed */ }, - _ => { - log::warn!( - target: LOG_TARGET, - "rejecting unsigned report equivocation transaction because it is not local/in-block." - ); - return InvalidTransaction::Call.into() - }, - } - - let evidence = (*equivocation_proof.clone(), key_owner_proof.clone()); - T::EquivocationReportSystem::check_evidence(evidence)?; - - let longevity = - >::Longevity::get(); - - ValidTransaction::with_tag_prefix("BeefyEquivocation") - // We assign the maximum priority for any equivocation report. - .priority(TransactionPriority::MAX) - // Only one equivocation report for the same offender at the same slot. - .and_provides(( - equivocation_proof.offender_id().clone(), - equivocation_proof.set_id(), - *equivocation_proof.round_number(), - )) - .longevity(longevity) - // We don't propagate this. This can never be included on a remote node. - .propagate(false) - .build() - } else { - InvalidTransaction::Call.into() + // discard equivocation report not coming from the local node + match source { + TransactionSource::Local | TransactionSource::InBlock => { /* allowed */ }, + _ => { + log::warn!( + target: LOG_TARGET, + "rejecting unsigned report equivocation transaction because it is not local/in-block." + ); + return InvalidTransaction::Call.into() + }, } + + let evidence = call.to_equivocation_evidence_for().ok_or(InvalidTransaction::Call)?; + let tag = (evidence.offender_id().clone(), evidence.set_id(), *evidence.round_number()); + T::EquivocationReportSystem::check_evidence(evidence)?; + + let longevity = + >::Longevity::get(); + ValidTransaction::with_tag_prefix("BeefyEquivocation") + // We assign the maximum priority for any equivocation report. + .priority(TransactionPriority::MAX) + // Only one equivocation report for the same offender at the same slot. + .and_provides(tag) + .longevity(longevity) + // We don't propagate this. This can never be included on a remote node. + .propagate(false) + .build() } pub fn pre_dispatch(call: &Call) -> Result<(), TransactionValidityError> { - if let Call::report_double_voting_unsigned { equivocation_proof, key_owner_proof } = call { - let evidence = (*equivocation_proof.clone(), key_owner_proof.clone()); - T::EquivocationReportSystem::check_evidence(evidence) - } else { - Err(InvalidTransaction::Call.into()) - } + let evidence = call.to_equivocation_evidence_for().ok_or(InvalidTransaction::Call)?; + T::EquivocationReportSystem::check_evidence(evidence) } } diff --git a/substrate/frame/beefy/src/lib.rs b/substrate/frame/beefy/src/lib.rs index 06987052ce99..753d7dcab240 100644 --- a/substrate/frame/beefy/src/lib.rs +++ b/substrate/frame/beefy/src/lib.rs @@ -42,7 +42,7 @@ use sp_std::prelude::*; use sp_consensus_beefy::{ AncestryHelper, AuthorityIndex, BeefyAuthorityId, ConsensusLog, DoubleVotingProof, - OnNewValidatorSet, ValidatorSet, BEEFY_ENGINE_ID, GENESIS_AUTHORITY_SET_ID, + ForkVotingProof, OnNewValidatorSet, ValidatorSet, BEEFY_ENGINE_ID, GENESIS_AUTHORITY_SET_ID, }; mod default_weights; @@ -191,8 +191,12 @@ pub mod pallet { pub enum Error { /// A key ownership proof provided as part of an equivocation report is invalid. InvalidKeyOwnershipProof, - /// An equivocation proof provided as part of an equivocation report is invalid. - InvalidEquivocationProof, + /// A double voting proof provided as part of an equivocation report is invalid. + InvalidDoubleVotingProof, + /// A fork voting proof provided as part of an equivocation report is invalid. + InvalidForkVotingProof, + /// The session of the equivocation proof is invalid + InvalidEquivocationProofSession, /// A given equivocation report is valid but already previously reported. DuplicateOffenceReport, /// Submitted configuration is invalid. @@ -225,7 +229,7 @@ pub mod pallet { T::EquivocationReportSystem::process_evidence( Some(reporter), - (*equivocation_proof, key_owner_proof), + EquivocationEvidenceFor::DoubleVotingProof(*equivocation_proof, key_owner_proof), )?; // Waive the fee since the report is valid and beneficial Ok(Pays::No.into()) @@ -260,7 +264,7 @@ pub mod pallet { T::EquivocationReportSystem::process_evidence( None, - (*equivocation_proof, key_owner_proof), + EquivocationEvidenceFor::DoubleVotingProof(*equivocation_proof, key_owner_proof), )?; Ok(Pays::No.into()) } @@ -281,6 +285,69 @@ pub mod pallet { GenesisBlock::::put(Some(genesis_block)); Ok(()) } + + /// Report fork voting equivocation. This method will verify the equivocation proof + /// and validate the given key ownership proof against the extracted offender. + /// If both are valid, the offence will be reported. + #[pallet::call_index(3)] + #[pallet::weight(T::WeightInfo::report_fork_voting( + key_owner_proof.validator_count(), + T::MaxNominators::get(), + ))] + pub fn report_fork_voting( + origin: OriginFor, + equivocation_proof: Box< + ForkVotingProof< + BlockNumberFor, + T::BeefyId, + >>::Proof, + >, + >, + key_owner_proof: T::KeyOwnerProof, + ) -> DispatchResultWithPostInfo { + let reporter = ensure_signed(origin)?; + + T::EquivocationReportSystem::process_evidence( + Some(reporter), + EquivocationEvidenceFor::ForkVotingProof(*equivocation_proof, key_owner_proof), + )?; + // Waive the fee since the report is valid and beneficial + Ok(Pays::No.into()) + } + + /// Report fork voting equivocation. This method will verify the equivocation proof + /// and validate the given key ownership proof against the extracted offender. + /// If both are valid, the offence will be reported. + /// + /// This extrinsic must be called unsigned and it is expected that only + /// block authors will call it (validated in `ValidateUnsigned`), as such + /// if the block author is defined it will be defined as the equivocation + /// reporter. + #[pallet::call_index(4)] + #[pallet::weight(T::WeightInfo::report_fork_voting( + key_owner_proof.validator_count(), + T::MaxNominators::get(), + ))] + pub fn report_fork_voting_unsigned( + origin: OriginFor, + equivocation_proof: Box< + ForkVotingProof< + BlockNumberFor, + T::BeefyId, + >>::Proof, + >, + >, + key_owner_proof: T::KeyOwnerProof, + ) -> DispatchResultWithPostInfo { + ensure_none(origin)?; + + T::EquivocationReportSystem::process_evidence( + None, + EquivocationEvidenceFor::ForkVotingProof(*equivocation_proof, key_owner_proof), + )?; + // Waive the fee since the report is valid and beneficial + Ok(Pays::No.into()) + } } #[pallet::hooks] @@ -303,6 +370,41 @@ pub mod pallet { Self::validate_unsigned(source, call) } } + + impl Call { + pub fn to_equivocation_evidence_for(&self) -> Option> { + match self { + Call::report_double_voting_unsigned { equivocation_proof, key_owner_proof } => + Some(EquivocationEvidenceFor::::DoubleVotingProof( + *equivocation_proof.clone(), + key_owner_proof.clone(), + )), + Call::report_fork_voting_unsigned { equivocation_proof, key_owner_proof } => + Some(EquivocationEvidenceFor::::ForkVotingProof( + *equivocation_proof.clone(), + key_owner_proof.clone(), + )), + _ => None, + } + } + } + + impl From> for Call { + fn from(evidence: EquivocationEvidenceFor) -> Self { + match evidence { + EquivocationEvidenceFor::DoubleVotingProof(equivocation_proof, key_owner_proof) => + Call::report_double_voting_unsigned { + equivocation_proof: Box::new(equivocation_proof), + key_owner_proof, + }, + EquivocationEvidenceFor::ForkVotingProof(equivocation_proof, key_owner_proof) => + Call::report_fork_voting_unsigned { + equivocation_proof: Box::new(equivocation_proof), + key_owner_proof, + }, + } + } + } } #[cfg(any(feature = "try-runtime", test))] @@ -378,7 +480,11 @@ impl Pallet { >, key_owner_proof: T::KeyOwnerProof, ) -> Option<()> { - T::EquivocationReportSystem::publish_evidence((equivocation_proof, key_owner_proof)).ok() + T::EquivocationReportSystem::publish_evidence(EquivocationEvidenceFor::DoubleVotingProof( + equivocation_proof, + key_owner_proof, + )) + .ok() } fn change_authorities( @@ -530,5 +636,6 @@ impl IsMember for Pallet { pub trait WeightInfo { fn report_double_voting(validator_count: u32, max_nominators_per_validator: u32) -> Weight; + fn report_fork_voting(validator_count: u32, max_nominators_per_validator: u32) -> Weight; fn set_new_genesis() -> Weight; } diff --git a/substrate/frame/beefy/src/mock.rs b/substrate/frame/beefy/src/mock.rs index d769b2bcdacd..0271c48f8476 100644 --- a/substrate/frame/beefy/src/mock.rs +++ b/substrate/frame/beefy/src/mock.rs @@ -15,6 +15,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +use codec::{Decode, Encode}; +use scale_info::TypeInfo; use std::vec; use frame_election_provider_support::{ @@ -81,17 +83,20 @@ parameter_types! { pub const ReportLongevity: u64 = BondingDuration::get() as u64 * SessionsPerEra::get() as u64 * Period::get(); pub const MaxSetIdSessionEntries: u32 = BondingDuration::get() * SessionsPerEra::get(); +} - pub storage CommitmentIsTargetingFork: bool = true; +#[derive(Clone, Debug, Decode, Encode, PartialEq, TypeInfo)] +pub struct MockAncestryProof { + pub is_non_canonical: bool, } pub struct MockAncestryHelper; impl AncestryHelper for MockAncestryHelper { - type Proof = (); + type Proof = MockAncestryProof; - fn is_non_canonical(_commitment: &Commitment, _proof: Self::Proof) -> bool { - CommitmentIsTargetingFork::get() + fn is_non_canonical(_commitment: &Commitment, proof: Self::Proof) -> bool { + proof.is_non_canonical } } diff --git a/substrate/frame/beefy/src/tests.rs b/substrate/frame/beefy/src/tests.rs index 8b7358fc7d80..5dbff88bc86a 100644 --- a/substrate/frame/beefy/src/tests.rs +++ b/substrate/frame/beefy/src/tests.rs @@ -26,7 +26,9 @@ use frame_support::{ use sp_consensus_beefy::{ check_equivocation_proof, known_payloads::MMR_ROOT_ID, - test_utils::{generate_equivocation_proof, Keyring as BeefyKeyring}, + test_utils::{ + generate_double_voting_proof, generate_fork_voting_proof, Keyring as BeefyKeyring, + }, Payload, ValidatorSet, KEY_TYPE as BEEFY_KEY_TYPE, }; use sp_runtime::DigestItem; @@ -222,7 +224,7 @@ fn should_sign_and_verify() { // generate an equivocation proof, with two votes in the same round for // same payload signed by the same key - let equivocation_proof = generate_equivocation_proof( + let equivocation_proof = generate_double_voting_proof( (1, payload1.clone(), set_id, &BeefyKeyring::Bob), (1, payload1.clone(), set_id, &BeefyKeyring::Bob), ); @@ -231,7 +233,7 @@ fn should_sign_and_verify() { // generate an equivocation proof, with two votes in different rounds for // different payloads signed by the same key - let equivocation_proof = generate_equivocation_proof( + let equivocation_proof = generate_double_voting_proof( (1, payload1.clone(), set_id, &BeefyKeyring::Bob), (2, payload2.clone(), set_id, &BeefyKeyring::Bob), ); @@ -239,7 +241,7 @@ fn should_sign_and_verify() { assert!(!check_equivocation_proof::<_, _, Keccak256>(&equivocation_proof)); // generate an equivocation proof, with two votes by different authorities - let equivocation_proof = generate_equivocation_proof( + let equivocation_proof = generate_double_voting_proof( (1, payload1.clone(), set_id, &BeefyKeyring::Alice), (1, payload2.clone(), set_id, &BeefyKeyring::Bob), ); @@ -247,7 +249,7 @@ fn should_sign_and_verify() { assert!(!check_equivocation_proof::<_, _, Keccak256>(&equivocation_proof)); // generate an equivocation proof, with two votes in different set ids - let equivocation_proof = generate_equivocation_proof( + let equivocation_proof = generate_double_voting_proof( (1, payload1.clone(), set_id, &BeefyKeyring::Bob), (1, payload2.clone(), set_id + 1, &BeefyKeyring::Bob), ); @@ -257,7 +259,7 @@ fn should_sign_and_verify() { // generate an equivocation proof, with two votes in the same round for // different payloads signed by the same key let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); - let equivocation_proof = generate_equivocation_proof( + let equivocation_proof = generate_double_voting_proof( (1, payload1, set_id, &BeefyKeyring::Bob), (1, payload2, set_id, &BeefyKeyring::Bob), ); @@ -301,7 +303,7 @@ fn report_equivocation_current_set_works() { let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); // generate an equivocation proof, with two votes in the same round for // different payloads signed by the same key - let equivocation_proof = generate_equivocation_proof( + let equivocation_proof = generate_double_voting_proof( (block_num, payload1, set_id, &equivocation_keyring), (block_num, payload2, set_id, &equivocation_keyring), ); @@ -387,7 +389,7 @@ fn report_equivocation_old_set_works() { let payload1 = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); // generate an equivocation proof for the old set, - let equivocation_proof = generate_equivocation_proof( + let equivocation_proof = generate_double_voting_proof( (block_num, payload1, old_set_id, &equivocation_keyring), (block_num, payload2, old_set_id, &equivocation_keyring), ); @@ -449,7 +451,7 @@ fn report_equivocation_invalid_set_id() { let payload1 = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); // generate an equivocation for a future set - let equivocation_proof = generate_equivocation_proof( + let equivocation_proof = generate_double_voting_proof( (block_num, payload1, set_id + 1, &equivocation_keyring), (block_num, payload2, set_id + 1, &equivocation_keyring), ); @@ -461,7 +463,7 @@ fn report_equivocation_invalid_set_id() { Box::new(equivocation_proof), key_owner_proof, ), - Error::::InvalidEquivocationProof, + Error::::InvalidEquivocationProofSession, ); }); } @@ -491,7 +493,7 @@ fn report_equivocation_invalid_session() { let payload1 = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); // generate an equivocation proof at following era set id = 2 - let equivocation_proof = generate_equivocation_proof( + let equivocation_proof = generate_double_voting_proof( (block_num, payload1, set_id, &equivocation_keyring), (block_num, payload2, set_id, &equivocation_keyring), ); @@ -504,7 +506,7 @@ fn report_equivocation_invalid_session() { Box::new(equivocation_proof), key_owner_proof, ), - Error::::InvalidEquivocationProof, + Error::::InvalidEquivocationProofSession, ); }); } @@ -535,9 +537,9 @@ fn report_equivocation_invalid_key_owner_proof() { let payload1 = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); // generate an equivocation proof for the authority at index 0 - let equivocation_proof = generate_equivocation_proof( - (block_num, payload1, set_id + 1, &equivocation_keyring), - (block_num, payload2, set_id + 1, &equivocation_keyring), + let equivocation_proof = generate_double_voting_proof( + (block_num, payload1, set_id, &equivocation_keyring), + (block_num, payload2, set_id, &equivocation_keyring), ); // we need to start a new era otherwise the key ownership proof won't be @@ -583,7 +585,7 @@ fn report_equivocation_invalid_equivocation_proof() { Box::new(equivocation_proof), key_owner_proof.clone(), ), - Error::::InvalidEquivocationProof, + Error::::InvalidDoubleVotingProof, ); }; @@ -594,31 +596,31 @@ fn report_equivocation_invalid_equivocation_proof() { // both votes target the same block number and payload, // there is no equivocation. - assert_invalid_equivocation_proof(generate_equivocation_proof( + assert_invalid_equivocation_proof(generate_double_voting_proof( (block_num, payload1.clone(), set_id, &equivocation_keyring), (block_num, payload1.clone(), set_id, &equivocation_keyring), )); // votes targeting different rounds, there is no equivocation. - assert_invalid_equivocation_proof(generate_equivocation_proof( + assert_invalid_equivocation_proof(generate_double_voting_proof( (block_num, payload1.clone(), set_id, &equivocation_keyring), (block_num + 1, payload2.clone(), set_id, &equivocation_keyring), )); // votes signed with different authority keys - assert_invalid_equivocation_proof(generate_equivocation_proof( + assert_invalid_equivocation_proof(generate_double_voting_proof( (block_num, payload1.clone(), set_id, &equivocation_keyring), (block_num, payload1.clone(), set_id, &BeefyKeyring::Charlie), )); // votes signed with a key that isn't part of the authority set - assert_invalid_equivocation_proof(generate_equivocation_proof( + assert_invalid_equivocation_proof(generate_double_voting_proof( (block_num, payload1.clone(), set_id, &equivocation_keyring), (block_num, payload1.clone(), set_id, &BeefyKeyring::Dave), )); // votes targeting different set ids - assert_invalid_equivocation_proof(generate_equivocation_proof( + assert_invalid_equivocation_proof(generate_double_voting_proof( (block_num, payload1, set_id, &equivocation_keyring), (block_num, payload2, set_id + 1, &equivocation_keyring), )); @@ -649,7 +651,7 @@ fn report_equivocation_validate_unsigned_prevents_duplicates() { let payload1 = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); - let equivocation_proof = generate_equivocation_proof( + let equivocation_proof = generate_double_voting_proof( (block_num, payload1, set_id, &equivocation_keyring), (block_num, payload2, set_id, &equivocation_keyring), ); @@ -753,7 +755,7 @@ fn valid_equivocation_reports_dont_pay_fees() { // generate equivocation proof let payload1 = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); - let equivocation_proof = generate_equivocation_proof( + let equivocation_proof = generate_double_voting_proof( (block_num, payload1, set_id, &equivocation_keyring), (block_num, payload2, set_id, &equivocation_keyring), ); @@ -802,6 +804,501 @@ fn valid_equivocation_reports_dont_pay_fees() { }) } +#[test] +fn report_fork_equivocation_vote_current_set_works() { + let authorities = test_authorities(); + + let mut ext = ExtBuilder::default().add_authorities(authorities).build(); + + let mut era = 1; + let block_num = ext.execute_with(|| { + assert_eq!(Staking::current_era(), Some(0)); + assert_eq!(Session::current_index(), 0); + start_era(era); + + let block_num = System::block_number(); + era += 1; + start_era(era); + block_num + }); + ext.persist_offchain_overlay(); + + ext.execute_with(|| { + let validator_set = Beefy::validator_set().unwrap(); + let authorities = validator_set.validators(); + let set_id = validator_set.id(); + let validators = Session::validators(); + + // make sure that all validators have the same balance + for validator in &validators { + assert_eq!(Balances::total_balance(validator), 10_000_000); + assert_eq!(Staking::slashable_balance_of(validator), 10_000); + + assert_eq!( + Staking::eras_stakers(era, validator), + pallet_staking::Exposure { total: 10_000, own: 10_000, others: vec![] }, + ); + } + + assert_eq!(authorities.len(), 2); + let equivocation_authority_index = 1; + let equivocation_key = &authorities[equivocation_authority_index]; + let equivocation_keyring = BeefyKeyring::from_public(equivocation_key).unwrap(); + + let payload = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); + + // generate a fork equivocation proof, with a vote in the same round for a + // different payload than finalized + let equivocation_proof = generate_fork_voting_proof( + (block_num, payload, set_id, &equivocation_keyring), + MockAncestryProof { is_non_canonical: true }, + ); + + // create the key ownership proof + let key_owner_proof = Historical::prove((BEEFY_KEY_TYPE, &equivocation_key)).unwrap(); + + // report the equivocation and the tx should be dispatched successfully + assert_ok!(Beefy::report_fork_voting_unsigned( + RuntimeOrigin::none(), + Box::new(equivocation_proof), + key_owner_proof, + ),); + + era += 1; + start_era(era); + + // check that the balance of 0-th validator is slashed 100%. + let equivocation_validator_id = validators[equivocation_authority_index]; + + assert_eq!(Balances::total_balance(&equivocation_validator_id), 10_000_000 - 10_000); + assert_eq!(Staking::slashable_balance_of(&equivocation_validator_id), 0); + assert_eq!( + Staking::eras_stakers(era, &equivocation_validator_id), + pallet_staking::Exposure { total: 0, own: 0, others: vec![] }, + ); + + // check that the balances of all other validators are left intact. + for validator in &validators { + if *validator == equivocation_validator_id { + continue + } + + assert_eq!(Balances::total_balance(validator), 10_000_000); + assert_eq!(Staking::slashable_balance_of(validator), 10_000); + + assert_eq!( + Staking::eras_stakers(era, validator), + pallet_staking::Exposure { total: 10_000, own: 10_000, others: vec![] }, + ); + } + }); +} + +#[test] +fn report_fork_equivocation_vote_old_set_works() { + let authorities = test_authorities(); + + let mut ext = ExtBuilder::default().add_authorities(authorities).build(); + + let mut era = 1; + let ( + block_num, + validators, + old_set_id, + equivocation_authority_index, + equivocation_key, + key_owner_proof, + ) = ext.execute_with(|| { + start_era(era); + let block_num = System::block_number(); + era += 1; + start_era(era); + + let validator_set = Beefy::validator_set().unwrap(); + let authorities = validator_set.validators(); + let validators = Session::validators(); + let old_set_id = validator_set.id(); + + assert_eq!(authorities.len(), 2); + let equivocation_authority_index = 0; + let equivocation_key = authorities[equivocation_authority_index].clone(); + + // create the key ownership proof in the "old" set + let key_owner_proof = Historical::prove((BEEFY_KEY_TYPE, &&equivocation_key)).unwrap(); + + era += 1; + start_era(era); + ( + block_num, + validators, + old_set_id, + equivocation_authority_index, + equivocation_key, + key_owner_proof, + ) + }); + ext.persist_offchain_overlay(); + + ext.execute_with(|| { + // make sure that all authorities have the same balance + for validator in &validators { + assert_eq!(Balances::total_balance(validator), 10_000_000); + assert_eq!(Staking::slashable_balance_of(validator), 10_000); + + assert_eq!( + Staking::eras_stakers(2, validator), + pallet_staking::Exposure { total: 10_000, own: 10_000, others: vec![] }, + ); + } + + let validator_set = Beefy::validator_set().unwrap(); + let new_set_id = validator_set.id(); + assert_eq!(old_set_id + 3, new_set_id); + + let equivocation_keyring = BeefyKeyring::from_public(&equivocation_key).unwrap(); + + let payload = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); + + // generate a fork equivocation proof, with a vote in the same round for a + // different payload than finalized + let equivocation_proof = generate_fork_voting_proof( + (block_num, payload, old_set_id, &equivocation_keyring), + MockAncestryProof { is_non_canonical: true }, + ); + + // report the equivocation and the tx should be dispatched successfully + assert_ok!(Beefy::report_fork_voting_unsigned( + RuntimeOrigin::none(), + Box::new(equivocation_proof), + key_owner_proof, + ),); + + era += 1; + start_era(era); + + // check that the balance of 0-th validator is slashed 100%. + let equivocation_validator_id = validators[equivocation_authority_index]; + + assert_eq!(Balances::total_balance(&equivocation_validator_id), 10_000_000 - 10_000); + assert_eq!(Staking::slashable_balance_of(&equivocation_validator_id), 0); + assert_eq!( + Staking::eras_stakers(era, &equivocation_validator_id), + pallet_staking::Exposure { total: 0, own: 0, others: vec![] }, + ); + + // check that the balances of all other validators are left intact. + for validator in &validators { + if *validator == equivocation_validator_id { + continue + } + + assert_eq!(Balances::total_balance(validator), 10_000_000); + assert_eq!(Staking::slashable_balance_of(validator), 10_000); + + assert_eq!( + Staking::eras_stakers(3, validator), + pallet_staking::Exposure { total: 10_000, own: 10_000, others: vec![] }, + ); + } + }); +} + +#[test] +fn report_fork_equivocation_vote_invalid_set_id() { + let authorities = test_authorities(); + + let mut ext = ExtBuilder::default().add_authorities(authorities).build(); + + let block_num = ext.execute_with(|| { + let mut era = 1; + start_era(era); + let block_num = System::block_number(); + + era += 1; + start_era(era); + block_num + }); + ext.persist_offchain_overlay(); + + ext.execute_with(|| { + let validator_set = Beefy::validator_set().unwrap(); + let authorities = validator_set.validators(); + let set_id = validator_set.id(); + + let equivocation_authority_index = 0; + let equivocation_key = &authorities[equivocation_authority_index]; + let equivocation_keyring = BeefyKeyring::from_public(equivocation_key).unwrap(); + + let key_owner_proof = Historical::prove((BEEFY_KEY_TYPE, &equivocation_key)).unwrap(); + + let payload = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); + // generate an equivocation for a future set + let equivocation_proof = generate_fork_voting_proof( + (block_num, payload, set_id + 1, &equivocation_keyring), + MockAncestryProof { is_non_canonical: true }, + ); + + // the call for reporting the equivocation should error + assert_err!( + Beefy::report_fork_voting_unsigned( + RuntimeOrigin::none(), + Box::new(equivocation_proof), + key_owner_proof, + ), + Error::::InvalidEquivocationProofSession, + ); + }); +} + +#[test] +fn report_fork_equivocation_vote_invalid_session() { + let authorities = test_authorities(); + + let mut ext = ExtBuilder::default().add_authorities(authorities).build(); + + let mut era = 1; + let (block_num, equivocation_keyring, key_owner_proof) = ext.execute_with(|| { + start_era(era); + let block_num = System::block_number(); + + era += 1; + start_era(era); + + let validator_set = Beefy::validator_set().unwrap(); + let authorities = validator_set.validators(); + + let equivocation_authority_index = 0; + let equivocation_key = &authorities[equivocation_authority_index]; + let equivocation_keyring = BeefyKeyring::from_public(equivocation_key).unwrap(); + + // generate a key ownership proof at current era set id + let key_owner_proof = Historical::prove((BEEFY_KEY_TYPE, &equivocation_key)).unwrap(); + + era += 1; + start_era(era); + (block_num, equivocation_keyring, key_owner_proof) + }); + ext.persist_offchain_overlay(); + + ext.execute_with(|| { + let set_id = Beefy::validator_set().unwrap().id(); + + let payload = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); + // generate an equivocation proof at following era set id = 3 + let equivocation_proof = generate_fork_voting_proof( + (block_num, payload, set_id, &equivocation_keyring), + MockAncestryProof { is_non_canonical: true }, + ); + + // report an equivocation for the current set using a key ownership + // proof from the previous set, the session should be invalid. + assert_err!( + Beefy::report_fork_voting_unsigned( + RuntimeOrigin::none(), + Box::new(equivocation_proof), + key_owner_proof, + ), + Error::::InvalidEquivocationProofSession, + ); + }); +} + +#[test] +fn report_fork_equivocation_vote_invalid_key_owner_proof() { + let authorities = test_authorities(); + + let mut ext = ExtBuilder::default().add_authorities(authorities).build(); + + let mut era = 1; + let block_num = ext.execute_with(|| { + start_era(era); + let block_num = System::block_number(); + + era += 1; + start_era(era); + block_num + }); + ext.persist_offchain_overlay(); + + ext.execute_with(|| { + let validator_set = Beefy::validator_set().unwrap(); + let authorities = validator_set.validators(); + let set_id = validator_set.id(); + + let invalid_owner_authority_index = 1; + let invalid_owner_key = &authorities[invalid_owner_authority_index]; + + // generate a key ownership proof for the authority at index 1 + let invalid_key_owner_proof = + Historical::prove((BEEFY_KEY_TYPE, &invalid_owner_key)).unwrap(); + + let equivocation_authority_index = 0; + let equivocation_key = &authorities[equivocation_authority_index]; + let equivocation_keyring = BeefyKeyring::from_public(equivocation_key).unwrap(); + + let payload = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); + // generate an equivocation for a future set + let equivocation_proof = generate_fork_voting_proof( + (block_num, payload, set_id, &equivocation_keyring), + MockAncestryProof { is_non_canonical: true }, + ); + + // we need to start a new era otherwise the key ownership proof won't be + // checked since the authorities are part of the current session + era += 1; + start_era(era); + + // report an equivocation for the current set using a key ownership + // proof for a different key than the one in the equivocation proof. + assert_err!( + Beefy::report_fork_voting_unsigned( + RuntimeOrigin::none(), + Box::new(equivocation_proof), + invalid_key_owner_proof, + ), + Error::::InvalidKeyOwnershipProof, + ); + }); +} + +#[test] +fn report_fork_equivocation_vote_invalid_equivocation_proof() { + let authorities = test_authorities(); + + let mut ext = ExtBuilder::default().add_authorities(authorities).build(); + + let mut era = 1; + let (block_num, set_id, equivocation_keyring, key_owner_proof) = ext.execute_with(|| { + start_era(era); + let block_num = System::block_number(); + + let validator_set = Beefy::validator_set().unwrap(); + let authorities = validator_set.validators(); + let set_id = validator_set.id(); + + let equivocation_authority_index = 0; + let equivocation_key = &authorities[equivocation_authority_index]; + let equivocation_keyring = BeefyKeyring::from_public(equivocation_key).unwrap(); + + // generate a key ownership proof at set id in era 1 + let key_owner_proof = Historical::prove((BEEFY_KEY_TYPE, &equivocation_key)).unwrap(); + + era += 1; + start_era(era); + (block_num, set_id, equivocation_keyring, key_owner_proof) + }); + ext.persist_offchain_overlay(); + + ext.execute_with(|| { + let payload = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); + + // vote signed with a key that isn't part of the authority set + let equivocation_proof = generate_fork_voting_proof( + (block_num, payload.clone(), set_id, &BeefyKeyring::Dave), + MockAncestryProof { is_non_canonical: true }, + ); + assert_err!( + Beefy::report_fork_voting_unsigned( + RuntimeOrigin::none(), + Box::new(equivocation_proof), + key_owner_proof.clone(), + ), + Error::::InvalidKeyOwnershipProof, + ); + + // Simulate InvalidForkVotingProof error. + let equivocation_proof = generate_fork_voting_proof( + (block_num + 1, payload.clone(), set_id, &equivocation_keyring), + MockAncestryProof { is_non_canonical: false }, + ); + assert_err!( + Beefy::report_fork_voting_unsigned( + RuntimeOrigin::none(), + Box::new(equivocation_proof), + key_owner_proof.clone(), + ), + Error::::InvalidForkVotingProof, + ); + }); +} + +#[test] +fn valid_fork_equivocation_vote_reports_dont_pay_fees() { + let authorities = test_authorities(); + + let mut ext = ExtBuilder::default().add_authorities(authorities).build(); + + let mut era = 1; + let block_num = ext.execute_with(|| { + start_era(era); + let block_num = System::block_number(); + + era += 1; + start_era(era); + block_num + }); + ext.persist_offchain_overlay(); + + ext.execute_with(|| { + let validator_set = Beefy::validator_set().unwrap(); + let authorities = validator_set.validators(); + let set_id = validator_set.id(); + + let equivocation_authority_index = 0; + let equivocation_key = &authorities[equivocation_authority_index]; + let equivocation_keyring = BeefyKeyring::from_public(equivocation_key).unwrap(); + + // generate equivocation proof + let payload = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); + let equivocation_proof = generate_fork_voting_proof( + (block_num, payload, set_id, &equivocation_keyring), + MockAncestryProof { is_non_canonical: true }, + ); + + // create the key ownership proof. + let key_owner_proof = Historical::prove((BEEFY_KEY_TYPE, &equivocation_key)).unwrap(); + + // check the dispatch info for the call. + let info = Call::::report_fork_voting_unsigned { + equivocation_proof: Box::new(equivocation_proof.clone()), + key_owner_proof: key_owner_proof.clone(), + } + .get_dispatch_info(); + + // it should have non-zero weight and the fee has to be paid. + assert!(info.weight.any_gt(Weight::zero())); + assert_eq!(info.pays_fee, Pays::Yes); + + // report the equivocation. + let post_info = Beefy::report_fork_voting_unsigned( + RuntimeOrigin::none(), + Box::new(equivocation_proof.clone()), + key_owner_proof.clone(), + ) + .unwrap(); + + // the original weight should be kept, but given that the report + // is valid the fee is waived. + assert!(post_info.actual_weight.is_none()); + assert_eq!(post_info.pays_fee, Pays::No); + + // report the equivocation again which is invalid now since it is + // duplicate. + let post_info = Beefy::report_fork_voting_unsigned( + RuntimeOrigin::none(), + Box::new(equivocation_proof.clone()), + key_owner_proof.clone(), + ) + .err() + .unwrap() + .post_info; + + // the fee is not waived and the original weight is kept. + assert!(post_info.actual_weight.is_none()); + assert_eq!(post_info.pays_fee, Pays::Yes); + }) +} + #[test] fn set_new_genesis_works() { let authorities = test_authorities(); diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index 8c376ef3c750..0c87621d32e3 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -302,8 +302,10 @@ pub struct VoteMessage { pub signature: Signature, } -/// Proof of voter misbehavior on a given set id. Misbehavior/equivocation in -/// BEEFY happens when a voter votes on the same round/block for different payloads. +/// Proof showing that an authority voted twice in the same round. +/// +/// One type of misbehavior in BEEFY happens when an authority votes in the same round/block +/// for different payloads. /// Proving is achieved by collecting the signed commitments of conflicting votes. #[derive(Clone, Debug, Decode, Encode, PartialEq, TypeInfo)] pub struct DoubleVotingProof { @@ -328,6 +330,18 @@ impl DoubleVotingProof { } } +/// Proof showing that an authority voted for a non-canonical chain. +/// +/// Proving is achieved by providing a proof that contains relevant info about the canonical chain +/// at `commitment.block_number`. The `commitment` can be checked against this info. +#[derive(Clone, Debug, Decode, Encode, PartialEq, TypeInfo)] +pub struct ForkVotingProof { + /// The equivocated vote. + pub vote: VoteMessage, + /// Proof containing info about the canonical chain at `commitment.block_number`. + pub ancestry_proof: AncestryProof, +} + /// Check a commitment signature by encoding the commitment and /// verifying the provided signature using the expected authority id. pub fn check_commitment_signature( @@ -396,7 +410,7 @@ impl OnNewValidatorSet for () { /// Hook containing helper methods for proving/checking commitment canonicity. pub trait AncestryHelper { /// Type containing proved info about the canonical chain at a certain height. - type Proof; + type Proof: Clone + Debug + Decode + Encode + PartialEq + TypeInfo; /// Check if a commitment is pointing to a header on a non-canonical chain /// against a canonicity proof generated at the same header height. @@ -437,8 +451,8 @@ sp_api::decl_runtime_apis! { /// Return the current active BEEFY validator set fn validator_set() -> Option>; - /// Submits an unsigned extrinsic to report an equivocation. The caller - /// must provide the equivocation proof and a key ownership proof + /// Submits an unsigned extrinsic to report a double voting equivocation. The caller + /// must provide the double voting proof and a key ownership proof /// (should be obtained using `generate_key_ownership_proof`). The /// extrinsic will be unsigned and should only be accepted for local /// authorship (not to be broadcast to the network). This method returns diff --git a/substrate/primitives/consensus/beefy/src/test_utils.rs b/substrate/primitives/consensus/beefy/src/test_utils.rs index d7fd49214f12..c210de0ca750 100644 --- a/substrate/primitives/consensus/beefy/src/test_utils.rs +++ b/substrate/primitives/consensus/beefy/src/test_utils.rs @@ -18,12 +18,12 @@ #[cfg(feature = "bls-experimental")] use crate::ecdsa_bls_crypto; use crate::{ - ecdsa_crypto, AuthorityIdBound, BeefySignatureHasher, Commitment, DoubleVotingProof, Payload, - ValidatorSetId, VoteMessage, + ecdsa_crypto, AuthorityIdBound, BeefySignatureHasher, Commitment, DoubleVotingProof, + ForkVotingProof, Payload, ValidatorSetId, VoteMessage, }; use sp_application_crypto::{AppCrypto, AppPair, RuntimeAppPublic, Wraps}; use sp_core::{ecdsa, Pair}; -use sp_runtime::traits::Hash; +use sp_runtime::traits::{BlockNumber, Hash}; use codec::Encode; use std::{collections::HashMap, marker::PhantomData}; @@ -136,20 +136,33 @@ impl From> for ecdsa_crypto::Public { } } -/// Create a new `EquivocationProof` based on given arguments. -pub fn generate_equivocation_proof( +/// Create a new `VoteMessage` from commitment primitives and keyring +fn signed_vote( + block_number: Number, + payload: Payload, + validator_set_id: ValidatorSetId, + keyring: &Keyring, +) -> VoteMessage { + let commitment = Commitment { validator_set_id, block_number, payload }; + let signature = keyring.sign(&commitment.encode()); + VoteMessage { commitment, id: keyring.public(), signature } +} + +/// Create a new `DoubleVotingProof` based on given arguments. +pub fn generate_double_voting_proof( vote1: (u64, Payload, ValidatorSetId, &Keyring), vote2: (u64, Payload, ValidatorSetId, &Keyring), ) -> DoubleVotingProof { - let signed_vote = |block_number: u64, - payload: Payload, - validator_set_id: ValidatorSetId, - keyring: &Keyring| { - let commitment = Commitment { validator_set_id, block_number, payload }; - let signature = keyring.sign(&commitment.encode()); - VoteMessage { commitment, id: keyring.public(), signature } - }; let first = signed_vote(vote1.0, vote1.1, vote1.2, vote1.3); let second = signed_vote(vote2.0, vote2.1, vote2.2, vote2.3); DoubleVotingProof { first, second } } + +/// Create a new `ForkVotingProof` based on vote & canonical header. +pub fn generate_fork_voting_proof( + vote: (u64, Payload, ValidatorSetId, &Keyring), + ancestry_proof: AncestryProof, +) -> ForkVotingProof { + let signed_vote = signed_vote(vote.0, vote.1, vote.2, vote.3); + ForkVotingProof { vote: signed_vote, ancestry_proof } +} From 4b69c20fdd5de77b043c5fc601b4a9d111a94645 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Mon, 20 May 2024 17:48:18 +0300 Subject: [PATCH 04/14] Address code review comments --- substrate/frame/beefy-mmr/src/lib.rs | 6 ++-- .../frame/merkle-mountain-range/src/tests.rs | 33 +++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/substrate/frame/beefy-mmr/src/lib.rs b/substrate/frame/beefy-mmr/src/lib.rs index 4bd03360c281..3805dc5d02ea 100644 --- a/substrate/frame/beefy-mmr/src/lib.rs +++ b/substrate/frame/beefy-mmr/src/lib.rs @@ -181,9 +181,9 @@ impl AncestryHelper> for Pallet { match pallet_mmr::Pallet::::block_num_to_leaf_count(commitment.block_number) { Ok(commitment_leaf_count) => commitment_leaf_count, Err(_) => { - // We consider the commitment non-canonical if the `commitment.block_number` - // is invalid. - return true + // 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 { diff --git a/substrate/frame/merkle-mountain-range/src/tests.rs b/substrate/frame/merkle-mountain-range/src/tests.rs index e83efaac2c6e..b8c9d54db820 100644 --- a/substrate/frame/merkle-mountain-range/src/tests.rs +++ b/substrate/frame/merkle-mountain-range/src/tests.rs @@ -787,3 +787,36 @@ fn does_not_panic_when_generating_historical_proofs() { ); }); } + +#[test] +fn generating_and_verifying_ancestry_proofs_works_correctly() { + let _ = env_logger::try_init(); + let mut ext = new_test_ext(); + + let mut prev_roots = vec![]; + ext.execute_with(|| { + for _ in 1..=500 { + add_blocks(1); + prev_roots.push(Pallet::::mmr_root()) + } + }); + ext.persist_offchain_overlay(); + register_offchain_ext(&mut ext); + + ext.execute_with(|| { + let root = Pallet::::mmr_root(); + // Check that generating and verifying ancestry proofs works correctly + // for each previous block + for prev_block_number in 1usize..=500 { + let proof = + Pallet::::generate_ancestry_proof(prev_block_number as u64, None).unwrap(); + assert_eq!( + Pallet::::verify_ancestry_proof(root, proof), + Ok(prev_roots[prev_block_number - 1]) + ); + } + + // Check that we can't generate ancestry proofs for a future block. + assert_eq!(Pallet::::generate_ancestry_proof(501, None), Err(Error::GenerateProof)); + }); +} From f709cef74f6cab056396bf563ea11f98953af8d2 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Thu, 23 May 2024 17:02:09 +0300 Subject: [PATCH 05/14] Validate ancestry proofs generated at older blocks --- substrate/frame/beefy-mmr/src/lib.rs | 38 ++++++-- substrate/frame/beefy-mmr/src/tests.rs | 68 ++++++++++++--- substrate/frame/beefy/src/equivocation.rs | 22 +++-- substrate/frame/beefy/src/lib.rs | 12 +-- substrate/frame/beefy/src/mock.rs | 42 +++++++-- substrate/frame/beefy/src/tests.rs | 87 +++++++++++++++++++ .../primitives/consensus/beefy/src/lib.rs | 21 +++-- .../consensus/beefy/src/test_utils.rs | 9 +- 8 files changed, 253 insertions(+), 46 deletions(-) diff --git a/substrate/frame/beefy-mmr/src/lib.rs b/substrate/frame/beefy-mmr/src/lib.rs index 3805dc5d02ea..18ebc9d8f38a 100644 --- a/substrate/frame/beefy-mmr/src/lib.rs +++ b/substrate/frame/beefy-mmr/src/lib.rs @@ -33,7 +33,7 @@ //! //! 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; @@ -41,13 +41,14 @@ use pallet_mmr::{primitives::AncestryProof, LeafDataProvider, ParentNumberAndHas use sp_consensus_beefy::{ known_payloads, mmr::{BeefyAuthoritySet, BeefyDataProvider, BeefyNextAuthoritySet, MmrLeaf, MmrLeafVersion}, - AncestryHelper, Commitment, 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; @@ -173,10 +174,35 @@ where } } -impl AncestryHelper> for Pallet { +impl AncestryHelper> for Pallet +where + T: pallet_mmr::Config, +{ type Proof = AncestryProof>; + type ValidationContext = MerkleRootOf; + + fn extract_validation_context(header: HeaderFor) -> Option { + // Check if the provided header is canonical. + let expected_hash = frame_system::Pallet::::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<::BeefyId>| match log { + ConsensusLog::MmrRoot(mmr_root) => Some(mmr_root), + _ => None, + }) + }) + } - fn is_non_canonical(commitment: &Commitment>, proof: Self::Proof) -> bool { + fn is_non_canonical( + commitment: &Commitment>, + proof: Self::Proof, + context: Self::ValidationContext, + ) -> bool { let commitment_leaf_count = match pallet_mmr::Pallet::::block_num_to_leaf_count(commitment.block_number) { Ok(commitment_leaf_count) => commitment_leaf_count, @@ -192,7 +218,7 @@ impl AncestryHelper> for Pallet { return false; } - let canonical_mmr_root = pallet_mmr::Pallet::::mmr_root(); + let canonical_mmr_root = context; let canonical_prev_root = match pallet_mmr::Pallet::::verify_ancestry_proof(canonical_mmr_root, proof) { Ok(canonical_prev_root) => canonical_prev_root, diff --git a/substrate/frame/beefy-mmr/src/tests.rs b/substrate/frame/beefy-mmr/src/tests.rs index 83a6292295b7..f99835a1dc0a 100644 --- a/substrate/frame/beefy-mmr/src/tests.rs +++ b/substrate/frame/beefy-mmr/src/tests.rs @@ -35,9 +35,9 @@ use frame_support::traits::OnInitialize; use crate::mock::*; -fn init_block(block: u64) { - let hash = H256::repeat_byte(block as u8); - System::initialize(&block, &hash, &Default::default()); +fn init_block(block: u64, maybe_parent_hash: Option) { + let parent_hash = maybe_parent_hash.unwrap_or(H256::repeat_byte(block as u8)); + System::initialize(&block, &parent_hash, &Default::default()); Session::on_initialize(block); Mmr::on_initialize(block); Beefy::on_initialize(block); @@ -66,7 +66,7 @@ fn read_mmr_leaf(ext: &mut TestExternalities, key: Vec) -> MmrLeaf { fn should_contain_mmr_digest() { let mut ext = new_test_ext(vec![1, 2, 3, 4]); ext.execute_with(|| { - init_block(1); + init_block(1, None); assert_eq!( System::digest().logs, vec![ @@ -81,7 +81,7 @@ fn should_contain_mmr_digest() { ); // unique every time - init_block(2); + init_block(2, None); assert_eq!( System::digest().logs, vec![ @@ -105,7 +105,7 @@ fn should_contain_valid_leaf_data() { let mut ext = new_test_ext(vec![1, 2, 3, 4]); let parent_hash = ext.execute_with(|| { - init_block(1); + init_block(1, None); frame_system::Pallet::::parent_hash() }); @@ -130,7 +130,7 @@ fn should_contain_valid_leaf_data() { // build second block on top let parent_hash = ext.execute_with(|| { - init_block(2); + init_block(2, None); frame_system::Pallet::::parent_hash() }); @@ -174,7 +174,7 @@ fn should_update_authorities() { assert_eq!(auth_set.keyset_commitment, next_auth_set.keyset_commitment); let announced_set = next_auth_set; - init_block(1); + init_block(1, None); let auth_set = BeefyMmr::authority_set_proof(); let next_auth_set = BeefyMmr::next_authority_set_proof(); @@ -190,7 +190,7 @@ fn should_update_authorities() { assert_eq!(want, next_auth_set.keyset_commitment); let announced_set = next_auth_set; - init_block(2); + init_block(2, None); let auth_set = BeefyMmr::authority_set_proof(); let next_auth_set = BeefyMmr::next_authority_set_proof(); @@ -207,6 +207,48 @@ fn should_update_authorities() { }); } +#[test] +fn extract_validation_context_should_work_correctly() { + let mut ext = new_test_ext(vec![1, 2]); + + // Register offchain ext. + let (offchain, _offchain_state) = TestOffchainExt::with_offchain_db(ext.offchain_db()); + ext.register_extension(OffchainDbExt::new(offchain.clone())); + ext.register_extension(OffchainWorkerExt::new(offchain)); + + ext.execute_with(|| { + init_block(1, None); + let h1 = System::finalize(); + init_block(2, Some(h1.hash())); + let h2 = System::finalize(); + + // Check the MMR root log + let expected_mmr_root: [u8; 32] = array_bytes::hex_n_into_unchecked( + "b2106eff9894288bc212b3a9389caa54efd37962c3a7b71b3b0b06a0911b88a5", + ); + assert_eq!( + System::digest().logs, + vec![beefy_log(ConsensusLog::MmrRoot(H256::from_slice(&expected_mmr_root)))] + ); + + // Make sure that all the info about h2 was stored on-chain + init_block(3, Some(h2.hash())); + + // `extract_validation_context` should return the MMR root when the provided header + // is part of the chain, + assert_eq!( + BeefyMmr::extract_validation_context(h2.clone()), + Some(H256::from_slice(&expected_mmr_root)) + ); + + // `extract_validation_context` should return `None` when the provided header + // is not part of the chain. + let mut fork_h2 = h2; + fork_h2.state_root = H256::repeat_byte(0); + assert_eq!(BeefyMmr::extract_validation_context(fork_h2), None); + }); +} + #[test] fn is_non_canonical_should_work_correctly() { let mut ext = new_test_ext(vec![1, 2]); @@ -214,7 +256,7 @@ fn is_non_canonical_should_work_correctly() { let mut prev_roots = vec![]; ext.execute_with(|| { for block_num in 1..=500 { - init_block(block_num); + init_block(block_num, None); prev_roots.push(Mmr::mmr_root()) } }); @@ -239,6 +281,7 @@ fn is_non_canonical_should_work_correctly() { validator_set_id: 0 }, valid_proof.clone(), + Mmr::mmr_root(), ), true ); @@ -256,6 +299,7 @@ fn is_non_canonical_should_work_correctly() { validator_set_id: 0, }, valid_proof.clone(), + Mmr::mmr_root(), ), true ); @@ -272,6 +316,7 @@ fn is_non_canonical_should_work_correctly() { validator_set_id: 0 }, invalid_proof, + Mmr::mmr_root(), ), false ); @@ -289,6 +334,7 @@ fn is_non_canonical_should_work_correctly() { validator_set_id: 0, }, valid_proof, + Mmr::mmr_root(), ), false ); @@ -310,6 +356,7 @@ fn is_non_canonical_should_work_correctly() { validator_set_id: 0, }, proof.clone(), + Mmr::mmr_root(), ), false ); @@ -325,6 +372,7 @@ fn is_non_canonical_should_work_correctly() { validator_set_id: 0, }, proof, + Mmr::mmr_root(), ), true ) diff --git a/substrate/frame/beefy/src/equivocation.rs b/substrate/frame/beefy/src/equivocation.rs index b066891d5815..4f3d49481cef 100644 --- a/substrate/frame/beefy/src/equivocation.rs +++ b/substrate/frame/beefy/src/equivocation.rs @@ -36,7 +36,7 @@ use codec::{self as codec, Decode, Encode}; use frame_support::traits::{Get, KeyOwnerProofSystem}; -use frame_system::pallet_prelude::BlockNumberFor; +use frame_system::pallet_prelude::{BlockNumberFor, HeaderFor}; use log::{error, info}; use sp_consensus_beefy::{ check_commitment_signature, AncestryHelper, DoubleVotingProof, ForkVotingProof, ValidatorSetId, @@ -136,9 +136,9 @@ pub enum EquivocationEvidenceFor { ), ForkVotingProof( ForkVotingProof< - BlockNumberFor, + HeaderFor, T::BeefyId, - >>::Proof, + >>::Proof, >, T::KeyOwnerProof, ), @@ -202,13 +202,23 @@ impl EquivocationEvidenceFor { return Ok(()) }, EquivocationEvidenceFor::ForkVotingProof(equivocation_proof, _) => { - let (vote, ancestry_proof) = - (equivocation_proof.vote, equivocation_proof.ancestry_proof); + let ForkVotingProof { vote, ancestry_proof, header } = equivocation_proof; + + let maybe_validation_context = , + >>::extract_validation_context(header); + let validation_context = match maybe_validation_context { + Some(validation_context) => validation_context, + None => { + return Err(Error::::InvalidForkVotingProof); + }, + }; let is_non_canonical = - >>::is_non_canonical( + >>::is_non_canonical( &vote.commitment, ancestry_proof, + validation_context, ); if !is_non_canonical { return Err(Error::::InvalidForkVotingProof); diff --git a/substrate/frame/beefy/src/lib.rs b/substrate/frame/beefy/src/lib.rs index 753d7dcab240..b48452a9b78c 100644 --- a/substrate/frame/beefy/src/lib.rs +++ b/substrate/frame/beefy/src/lib.rs @@ -28,7 +28,7 @@ use frame_support::{ }; use frame_system::{ ensure_none, ensure_signed, - pallet_prelude::{BlockNumberFor, OriginFor}, + pallet_prelude::{BlockNumberFor, HeaderFor, OriginFor}, }; use log; use sp_runtime::{ @@ -99,7 +99,7 @@ pub mod pallet { type OnNewValidatorSet: OnNewValidatorSet<::BeefyId>; /// Hook for checking commitment canonicity. - type AncestryHelper: AncestryHelper>; + type AncestryHelper: AncestryHelper>; /// Weights for this pallet. type WeightInfo: WeightInfo; @@ -298,9 +298,9 @@ pub mod pallet { origin: OriginFor, equivocation_proof: Box< ForkVotingProof< - BlockNumberFor, + HeaderFor, T::BeefyId, - >>::Proof, + >>::Proof, >, >, key_owner_proof: T::KeyOwnerProof, @@ -332,9 +332,9 @@ pub mod pallet { origin: OriginFor, equivocation_proof: Box< ForkVotingProof< - BlockNumberFor, + HeaderFor, T::BeefyId, - >>::Proof, + >>::Proof, >, >, key_owner_proof: T::KeyOwnerProof, diff --git a/substrate/frame/beefy/src/mock.rs b/substrate/frame/beefy/src/mock.rs index 0271c48f8476..117b9b9430ec 100644 --- a/substrate/frame/beefy/src/mock.rs +++ b/substrate/frame/beefy/src/mock.rs @@ -30,8 +30,12 @@ use frame_support::{ use pallet_session::historical as pallet_session_historical; use sp_core::{crypto::KeyTypeId, ConstU128}; use sp_runtime::{ - app_crypto::ecdsa::Public, curve::PiecewiseLinear, impl_opaque_keys, testing::TestXt, - traits::OpaqueKeys, BuildStorage, Perbill, + app_crypto::ecdsa::Public, + curve::PiecewiseLinear, + impl_opaque_keys, + testing::TestXt, + traits::{Header as HeaderT, OpaqueKeys}, + BuildStorage, Perbill, }; use sp_staking::{EraIndex, SessionIndex}; use sp_state_machine::BasicExternalities; @@ -78,25 +82,45 @@ where type Extrinsic = TestXt; } +#[derive(Clone, Debug, Decode, Encode, PartialEq, TypeInfo)] +pub struct MockAncestryProofContext { + pub is_valid: bool, +} + +#[derive(Clone, Debug, Decode, Encode, PartialEq, TypeInfo)] +pub struct MockAncestryProof { + pub is_non_canonical: bool, +} + parameter_types! { pub const Period: u64 = 1; pub const ReportLongevity: u64 = BondingDuration::get() as u64 * SessionsPerEra::get() as u64 * Period::get(); pub const MaxSetIdSessionEntries: u32 = BondingDuration::get() * SessionsPerEra::get(); -} -#[derive(Clone, Debug, Decode, Encode, PartialEq, TypeInfo)] -pub struct MockAncestryProof { - pub is_non_canonical: bool, + pub storage AncestryProofContext: Option = Some( + MockAncestryProofContext { + is_valid: true, + } + ); } pub struct MockAncestryHelper; -impl AncestryHelper for MockAncestryHelper { +impl AncestryHelper

for MockAncestryHelper { type Proof = MockAncestryProof; + type ValidationContext = MockAncestryProofContext; + + fn extract_validation_context(_header: Header) -> Option { + AncestryProofContext::get() + } - fn is_non_canonical(_commitment: &Commitment, proof: Self::Proof) -> bool { - proof.is_non_canonical + fn is_non_canonical( + _commitment: &Commitment, + proof: Self::Proof, + context: Self::ValidationContext, + ) -> bool { + context.is_valid && proof.is_non_canonical } } diff --git a/substrate/frame/beefy/src/tests.rs b/substrate/frame/beefy/src/tests.rs index 5dbff88bc86a..d819ceaa163c 100644 --- a/substrate/frame/beefy/src/tests.rs +++ b/substrate/frame/beefy/src/tests.rs @@ -852,6 +852,7 @@ fn report_fork_equivocation_vote_current_set_works() { let equivocation_proof = generate_fork_voting_proof( (block_num, payload, set_id, &equivocation_keyring), MockAncestryProof { is_non_canonical: true }, + System::finalize(), ); // create the key ownership proof @@ -964,6 +965,7 @@ fn report_fork_equivocation_vote_old_set_works() { let equivocation_proof = generate_fork_voting_proof( (block_num, payload, old_set_id, &equivocation_keyring), MockAncestryProof { is_non_canonical: true }, + System::finalize(), ); // report the equivocation and the tx should be dispatched successfully @@ -1036,6 +1038,7 @@ fn report_fork_equivocation_vote_invalid_set_id() { let equivocation_proof = generate_fork_voting_proof( (block_num, payload, set_id + 1, &equivocation_keyring), MockAncestryProof { is_non_canonical: true }, + System::finalize(), ); // the call for reporting the equivocation should error @@ -1088,6 +1091,7 @@ fn report_fork_equivocation_vote_invalid_session() { let equivocation_proof = generate_fork_voting_proof( (block_num, payload, set_id, &equivocation_keyring), MockAncestryProof { is_non_canonical: true }, + System::finalize(), ); // report an equivocation for the current set using a key ownership @@ -1141,6 +1145,7 @@ fn report_fork_equivocation_vote_invalid_key_owner_proof() { let equivocation_proof = generate_fork_voting_proof( (block_num, payload, set_id, &equivocation_keyring), MockAncestryProof { is_non_canonical: true }, + System::finalize(), ); // we need to start a new era otherwise the key ownership proof won't be @@ -1161,6 +1166,85 @@ fn report_fork_equivocation_vote_invalid_key_owner_proof() { }); } +#[test] +fn report_fork_equivocation_vote_invalid_context() { + let authorities = test_authorities(); + + let mut ext = ExtBuilder::default().add_authorities(authorities).build(); + + let mut era = 1; + let block_num = ext.execute_with(|| { + assert_eq!(Staking::current_era(), Some(0)); + assert_eq!(Session::current_index(), 0); + start_era(era); + + let block_num = System::block_number(); + era += 1; + start_era(era); + block_num + }); + ext.persist_offchain_overlay(); + + ext.execute_with(|| { + let validator_set = Beefy::validator_set().unwrap(); + let authorities = validator_set.validators(); + let set_id = validator_set.id(); + let validators = Session::validators(); + + // make sure that all validators have the same balance + for validator in &validators { + assert_eq!(Balances::total_balance(validator), 10_000_000); + assert_eq!(Staking::slashable_balance_of(validator), 10_000); + + assert_eq!( + Staking::eras_stakers(era, validator), + pallet_staking::Exposure { total: 10_000, own: 10_000, others: vec![] }, + ); + } + + assert_eq!(authorities.len(), 2); + let equivocation_authority_index = 1; + let equivocation_key = &authorities[equivocation_authority_index]; + let equivocation_keyring = BeefyKeyring::from_public(equivocation_key).unwrap(); + + let payload = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); + + // generate a fork equivocation proof, with a vote in the same round for a + // different payload than finalized + let equivocation_proof = generate_fork_voting_proof( + (block_num, payload, set_id, &equivocation_keyring), + MockAncestryProof { is_non_canonical: true }, + System::finalize(), + ); + + // create the key ownership proof + let key_owner_proof = Historical::prove((BEEFY_KEY_TYPE, &equivocation_key)).unwrap(); + + // report an equivocation for the current set. Simulate a failure of + // `extract_validation_context` + AncestryProofContext::set(&None); + assert_err!( + Beefy::report_fork_voting_unsigned( + RuntimeOrigin::none(), + Box::new(equivocation_proof.clone()), + key_owner_proof.clone(), + ), + Error::::InvalidForkVotingProof, + ); + + // report an equivocation for the current set. Simulate an invalid context. + AncestryProofContext::set(&Some(MockAncestryProofContext { is_valid: false })); + assert_err!( + Beefy::report_fork_voting_unsigned( + RuntimeOrigin::none(), + Box::new(equivocation_proof), + key_owner_proof, + ), + Error::::InvalidForkVotingProof, + ); + }); +} + #[test] fn report_fork_equivocation_vote_invalid_equivocation_proof() { let authorities = test_authorities(); @@ -1196,6 +1280,7 @@ fn report_fork_equivocation_vote_invalid_equivocation_proof() { let equivocation_proof = generate_fork_voting_proof( (block_num, payload.clone(), set_id, &BeefyKeyring::Dave), MockAncestryProof { is_non_canonical: true }, + System::finalize(), ); assert_err!( Beefy::report_fork_voting_unsigned( @@ -1210,6 +1295,7 @@ fn report_fork_equivocation_vote_invalid_equivocation_proof() { let equivocation_proof = generate_fork_voting_proof( (block_num + 1, payload.clone(), set_id, &equivocation_keyring), MockAncestryProof { is_non_canonical: false }, + System::finalize(), ); assert_err!( Beefy::report_fork_voting_unsigned( @@ -1253,6 +1339,7 @@ fn valid_fork_equivocation_vote_reports_dont_pay_fees() { let equivocation_proof = generate_fork_voting_proof( (block_num, payload, set_id, &equivocation_keyring), MockAncestryProof { is_non_canonical: true }, + System::finalize(), ); // create the key ownership proof. diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index 0c87621d32e3..7bdf5d522d4b 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -52,7 +52,7 @@ use core::fmt::{Debug, Display}; use scale_info::TypeInfo; use sp_application_crypto::{AppCrypto, AppPublic, ByteArray, RuntimeAppPublic}; use sp_core::H256; -use sp_runtime::traits::{Hash, Keccak256, NumberFor}; +use sp_runtime::traits::{Hash, Header as HeaderT, Keccak256, NumberFor}; /// Key type for BEEFY module. pub const KEY_TYPE: sp_core::crypto::KeyTypeId = sp_application_crypto::key_types::BEEFY; @@ -335,11 +335,13 @@ impl DoubleVotingProof { /// Proving is achieved by providing a proof that contains relevant info about the canonical chain /// at `commitment.block_number`. The `commitment` can be checked against this info. #[derive(Clone, Debug, Decode, Encode, PartialEq, TypeInfo)] -pub struct ForkVotingProof { +pub struct ForkVotingProof { /// The equivocated vote. - pub vote: VoteMessage, + pub vote: VoteMessage, /// Proof containing info about the canonical chain at `commitment.block_number`. pub ancestry_proof: AncestryProof, + /// The header of the block where the ancestry proof was generated + pub header: Header, } /// Check a commitment signature by encoding the commitment and @@ -408,13 +410,22 @@ impl OnNewValidatorSet for () { } /// Hook containing helper methods for proving/checking commitment canonicity. -pub trait AncestryHelper { +pub trait AncestryHelper { /// Type containing proved info about the canonical chain at a certain height. type Proof: Clone + Debug + Decode + Encode + PartialEq + TypeInfo; + /// The data needed for validating the proof. + type ValidationContext; + + /// Extract the validation context from the provided header. + fn extract_validation_context(header: Header) -> Option; /// Check if a commitment is pointing to a header on a non-canonical chain /// against a canonicity proof generated at the same header height. - fn is_non_canonical(commitment: &Commitment, proof: Self::Proof) -> bool; + fn is_non_canonical( + commitment: &Commitment, + proof: Self::Proof, + context: Self::ValidationContext, + ) -> bool; } /// An opaque type used to represent the key ownership proof at the runtime API diff --git a/substrate/primitives/consensus/beefy/src/test_utils.rs b/substrate/primitives/consensus/beefy/src/test_utils.rs index c210de0ca750..cba3dde7ec96 100644 --- a/substrate/primitives/consensus/beefy/src/test_utils.rs +++ b/substrate/primitives/consensus/beefy/src/test_utils.rs @@ -23,7 +23,7 @@ use crate::{ }; use sp_application_crypto::{AppCrypto, AppPair, RuntimeAppPublic, Wraps}; use sp_core::{ecdsa, Pair}; -use sp_runtime::traits::{BlockNumber, Hash}; +use sp_runtime::traits::{BlockNumber, Hash, Header as HeaderT}; use codec::Encode; use std::{collections::HashMap, marker::PhantomData}; @@ -159,10 +159,11 @@ pub fn generate_double_voting_proof( } /// Create a new `ForkVotingProof` based on vote & canonical header. -pub fn generate_fork_voting_proof( +pub fn generate_fork_voting_proof, AncestryProof>( vote: (u64, Payload, ValidatorSetId, &Keyring), ancestry_proof: AncestryProof, -) -> ForkVotingProof { + header: Header, +) -> ForkVotingProof { let signed_vote = signed_vote(vote.0, vote.1, vote.2, vote.3); - ForkVotingProof { vote: signed_vote, ancestry_proof } + ForkVotingProof { vote: signed_vote, ancestry_proof, header } } From 4f85eaa13ed1f7638fae080c9190fc4a785da862 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Fri, 7 Jun 2024 09:54:17 +0300 Subject: [PATCH 06/14] Add future block voting offence --- substrate/frame/beefy/src/default_weights.rs | 11 +- substrate/frame/beefy/src/equivocation.rs | 27 +- substrate/frame/beefy/src/lib.rs | 84 +- substrate/frame/beefy/src/tests.rs | 908 ++++++------------ .../primitives/consensus/beefy/src/lib.rs | 7 + .../consensus/beefy/src/test_utils.rs | 12 +- 6 files changed, 429 insertions(+), 620 deletions(-) diff --git a/substrate/frame/beefy/src/default_weights.rs b/substrate/frame/beefy/src/default_weights.rs index 5eec3c145391..70dd3bb02bf1 100644 --- a/substrate/frame/beefy/src/default_weights.rs +++ b/substrate/frame/beefy/src/default_weights.rs @@ -24,7 +24,11 @@ use frame_support::weights::{ }; impl crate::WeightInfo for () { - fn report_double_voting(validator_count: u32, max_nominators_per_validator: u32) -> Weight { + fn report_voting_equivocation( + votes_count: u32, + validator_count: u32, + max_nominators_per_validator: u32, + ) -> Weight { // we take the validator set count from the membership proof to // calculate the weight but we set a floor of 100 validators. let validator_count = validator_count.max(100) as u64; @@ -37,7 +41,10 @@ impl crate::WeightInfo for () { ) .saturating_add(DbWeight::get().reads(5)) // check equivocation proof - .saturating_add(Weight::from_parts(95u64 * WEIGHT_REF_TIME_PER_MICROS, 0)) + .saturating_add(Weight::from_parts( + (50u64 * WEIGHT_REF_TIME_PER_MICROS).saturating_mul(votes_count as u64), + 0, + )) // report offence .saturating_add(Weight::from_parts(110u64 * WEIGHT_REF_TIME_PER_MICROS, 0)) .saturating_add(Weight::from_parts( diff --git a/substrate/frame/beefy/src/equivocation.rs b/substrate/frame/beefy/src/equivocation.rs index 4f3d49481cef..e07ceb226314 100644 --- a/substrate/frame/beefy/src/equivocation.rs +++ b/substrate/frame/beefy/src/equivocation.rs @@ -39,8 +39,8 @@ use frame_support::traits::{Get, KeyOwnerProofSystem}; use frame_system::pallet_prelude::{BlockNumberFor, HeaderFor}; use log::{error, info}; use sp_consensus_beefy::{ - check_commitment_signature, AncestryHelper, DoubleVotingProof, ForkVotingProof, ValidatorSetId, - KEY_TYPE as BEEFY_KEY_TYPE, + check_commitment_signature, AncestryHelper, DoubleVotingProof, ForkVotingProof, + FutureBlockVotingProof, ValidatorSetId, KEY_TYPE as BEEFY_KEY_TYPE, }; use sp_runtime::{ transaction_validity::{ @@ -142,6 +142,7 @@ pub enum EquivocationEvidenceFor { >, T::KeyOwnerProof, ), + FutureBlockVotingProof(FutureBlockVotingProof, T::BeefyId>, T::KeyOwnerProof), } impl EquivocationEvidenceFor { @@ -152,6 +153,8 @@ impl EquivocationEvidenceFor { equivocation_proof.offender_id(), EquivocationEvidenceFor::ForkVotingProof(equivocation_proof, _) => &equivocation_proof.vote.id, + EquivocationEvidenceFor::FutureBlockVotingProof(equivocation_proof, _) => + &equivocation_proof.vote.id, } } @@ -162,6 +165,8 @@ impl EquivocationEvidenceFor { equivocation_proof.round_number(), EquivocationEvidenceFor::ForkVotingProof(equivocation_proof, _) => &equivocation_proof.vote.commitment.block_number, + EquivocationEvidenceFor::FutureBlockVotingProof(equivocation_proof, _) => + &equivocation_proof.vote.commitment.block_number, } } @@ -172,6 +177,8 @@ impl EquivocationEvidenceFor { equivocation_proof.set_id(), EquivocationEvidenceFor::ForkVotingProof(equivocation_proof, _) => equivocation_proof.vote.commitment.validator_set_id, + EquivocationEvidenceFor::FutureBlockVotingProof(equivocation_proof, _) => + equivocation_proof.vote.commitment.validator_set_id, } } @@ -180,6 +187,7 @@ impl EquivocationEvidenceFor { match self { EquivocationEvidenceFor::DoubleVotingProof(_, key_owner_proof) => key_owner_proof, EquivocationEvidenceFor::ForkVotingProof(_, key_owner_proof) => key_owner_proof, + EquivocationEvidenceFor::FutureBlockVotingProof(_, key_owner_proof) => key_owner_proof, } } @@ -230,6 +238,21 @@ impl EquivocationEvidenceFor { return Err(Error::::InvalidForkVotingProof); } + Ok(()) + }, + EquivocationEvidenceFor::FutureBlockVotingProof(equivocation_proof, _) => { + let FutureBlockVotingProof { vote } = equivocation_proof; + // Check if the commitment actually targets a future block + if vote.commitment.block_number < frame_system::Pallet::::block_number() { + return Err(Error::::InvalidFutureBlockVotingProof); + } + + let is_signature_valid = + check_commitment_signature(&vote.commitment, &vote.id, &vote.signature); + if !is_signature_valid { + return Err(Error::::InvalidForkVotingProof); + } + Ok(()) }, } diff --git a/substrate/frame/beefy/src/lib.rs b/substrate/frame/beefy/src/lib.rs index b48452a9b78c..a49f5d28f455 100644 --- a/substrate/frame/beefy/src/lib.rs +++ b/substrate/frame/beefy/src/lib.rs @@ -42,7 +42,8 @@ use sp_std::prelude::*; use sp_consensus_beefy::{ AncestryHelper, AuthorityIndex, BeefyAuthorityId, ConsensusLog, DoubleVotingProof, - ForkVotingProof, OnNewValidatorSet, ValidatorSet, BEEFY_ENGINE_ID, GENESIS_AUTHORITY_SET_ID, + ForkVotingProof, FutureBlockVotingProof, OnNewValidatorSet, ValidatorSet, BEEFY_ENGINE_ID, + GENESIS_AUTHORITY_SET_ID, }; mod default_weights; @@ -195,6 +196,8 @@ pub mod pallet { InvalidDoubleVotingProof, /// A fork voting proof provided as part of an equivocation report is invalid. InvalidForkVotingProof, + /// A future block voting proof provided as part of an equivocation report is invalid. + InvalidFutureBlockVotingProof, /// The session of the equivocation proof is invalid InvalidEquivocationProofSession, /// A given equivocation report is valid but already previously reported. @@ -348,6 +351,63 @@ pub mod pallet { // Waive the fee since the report is valid and beneficial Ok(Pays::No.into()) } + + /// Report future block voting equivocation. This method will verify the equivocation proof + /// and validate the given key ownership proof against the extracted offender. + /// If both are valid, the offence will be reported. + #[pallet::call_index(5)] + #[pallet::weight(T::WeightInfo::report_fork_voting( + key_owner_proof.validator_count(), + T::MaxNominators::get(), + ))] + pub fn report_future_block_voting( + origin: OriginFor, + equivocation_proof: Box, T::BeefyId>>, + key_owner_proof: T::KeyOwnerProof, + ) -> DispatchResultWithPostInfo { + let reporter = ensure_signed(origin)?; + + T::EquivocationReportSystem::process_evidence( + Some(reporter), + EquivocationEvidenceFor::FutureBlockVotingProof( + *equivocation_proof, + key_owner_proof, + ), + )?; + // Waive the fee since the report is valid and beneficial + Ok(Pays::No.into()) + } + + /// Report future block voting equivocation. This method will verify the equivocation proof + /// and validate the given key ownership proof against the extracted offender. + /// If both are valid, the offence will be reported. + /// + /// This extrinsic must be called unsigned and it is expected that only + /// block authors will call it (validated in `ValidateUnsigned`), as such + /// if the block author is defined it will be defined as the equivocation + /// reporter. + #[pallet::call_index(6)] + #[pallet::weight(T::WeightInfo::report_fork_voting( + key_owner_proof.validator_count(), + T::MaxNominators::get(), + ))] + pub fn report_future_block_voting_unsigned( + origin: OriginFor, + equivocation_proof: Box, T::BeefyId>>, + key_owner_proof: T::KeyOwnerProof, + ) -> DispatchResultWithPostInfo { + ensure_none(origin)?; + + T::EquivocationReportSystem::process_evidence( + None, + EquivocationEvidenceFor::FutureBlockVotingProof( + *equivocation_proof, + key_owner_proof, + ), + )?; + // Waive the fee since the report is valid and beneficial + Ok(Pays::No.into()) + } } #[pallet::hooks] @@ -402,6 +462,13 @@ pub mod pallet { equivocation_proof: Box::new(equivocation_proof), key_owner_proof, }, + EquivocationEvidenceFor::FutureBlockVotingProof( + equivocation_proof, + key_owner_proof, + ) => Call::report_future_block_voting_unsigned { + equivocation_proof: Box::new(equivocation_proof), + key_owner_proof, + }, } } } @@ -635,7 +702,20 @@ impl IsMember for Pallet { } pub trait WeightInfo { - fn report_double_voting(validator_count: u32, max_nominators_per_validator: u32) -> Weight; + fn report_voting_equivocation( + votes_count: u32, + validator_count: u32, + max_nominators_per_validator: u32, + ) -> Weight; + fn report_double_voting(validator_count: u32, max_nominators_per_validator: u32) -> Weight { + Self::report_voting_equivocation(2, validator_count, max_nominators_per_validator) + } fn report_fork_voting(validator_count: u32, max_nominators_per_validator: u32) -> Weight; + fn report_future_block_voting( + validator_count: u32, + max_nominators_per_validator: u32, + ) -> Weight { + Self::report_voting_equivocation(1, validator_count, max_nominators_per_validator) + } fn set_new_genesis() -> Weight; } diff --git a/substrate/frame/beefy/src/tests.rs b/substrate/frame/beefy/src/tests.rs index d819ceaa163c..33577439ac1a 100644 --- a/substrate/frame/beefy/src/tests.rs +++ b/substrate/frame/beefy/src/tests.rs @@ -20,20 +20,22 @@ use std::vec; use frame_support::{ assert_err, assert_ok, - dispatch::{GetDispatchInfo, Pays}, + dispatch::{DispatchResultWithPostInfo, Pays}, traits::{Currency, KeyOwnerProofSystem, OnInitialize}, }; use sp_consensus_beefy::{ - check_equivocation_proof, + check_equivocation_proof, ecdsa_crypto, known_payloads::MMR_ROOT_ID, test_utils::{ - generate_double_voting_proof, generate_fork_voting_proof, Keyring as BeefyKeyring, + generate_double_voting_proof, generate_fork_voting_proof, + generate_future_block_voting_proof, Keyring as BeefyKeyring, }, - Payload, ValidatorSet, KEY_TYPE as BEEFY_KEY_TYPE, + Payload, ValidatorSet, ValidatorSetId, KEY_TYPE as BEEFY_KEY_TYPE, }; use sp_runtime::DigestItem; +use sp_session::MembershipProof; -use crate::{self as beefy, mock::*, Call, Config, Error, Weight, WeightInfo}; +use crate::{self as beefy, mock::*, Call, Config, Error, WeightInfo}; fn init_block(block: u64) { System::set_block_number(block); @@ -267,8 +269,47 @@ fn should_sign_and_verify() { assert!(check_equivocation_proof::<_, _, Keccak256>(&equivocation_proof)); } -#[test] -fn report_equivocation_current_set_works() { +trait ReportEquivocationFn: + FnMut( + u64, + ValidatorSetId, + &BeefyKeyring, + MembershipProof, +) -> DispatchResultWithPostInfo +{ +} + +impl ReportEquivocationFn for F where + F: FnMut( + u64, + ValidatorSetId, + &BeefyKeyring, + MembershipProof, + ) -> DispatchResultWithPostInfo +{ +} + +fn report_double_voting( + block_num: u64, + set_id: ValidatorSetId, + equivocation_keyring: &BeefyKeyring, + key_owner_proof: MembershipProof, +) -> DispatchResultWithPostInfo { + let payload1 = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); + let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); + let equivocation_proof = generate_double_voting_proof( + (block_num, payload1, set_id, &equivocation_keyring), + (block_num, payload2, set_id, &equivocation_keyring), + ); + + Beefy::report_double_voting_unsigned( + RuntimeOrigin::none(), + Box::new(equivocation_proof), + key_owner_proof, + ) +} + +fn report_equivocation_current_set_works(mut f: impl ReportEquivocationFn) { let authorities = test_authorities(); ExtBuilder::default().add_authorities(authorities).build_and_execute(|| { @@ -299,24 +340,11 @@ fn report_equivocation_current_set_works() { let equivocation_key = &authorities[equivocation_authority_index]; let equivocation_keyring = BeefyKeyring::from_public(equivocation_key).unwrap(); - let payload1 = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); - let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); - // generate an equivocation proof, with two votes in the same round for - // different payloads signed by the same key - let equivocation_proof = generate_double_voting_proof( - (block_num, payload1, set_id, &equivocation_keyring), - (block_num, payload2, set_id, &equivocation_keyring), - ); - // create the key ownership proof let key_owner_proof = Historical::prove((BEEFY_KEY_TYPE, &equivocation_key)).unwrap(); // report the equivocation and the tx should be dispatched successfully - assert_ok!(Beefy::report_double_voting_unsigned( - RuntimeOrigin::none(), - Box::new(equivocation_proof), - key_owner_proof, - ),); + assert_ok!(f(block_num, set_id, &equivocation_keyring, key_owner_proof)); start_era(2); @@ -347,8 +375,7 @@ fn report_equivocation_current_set_works() { }); } -#[test] -fn report_equivocation_old_set_works() { +fn report_equivocation_old_set_works(mut f: impl ReportEquivocationFn) { let authorities = test_authorities(); ExtBuilder::default().add_authorities(authorities).build_and_execute(|| { @@ -386,20 +413,8 @@ fn report_equivocation_old_set_works() { let equivocation_keyring = BeefyKeyring::from_public(equivocation_key).unwrap(); - let payload1 = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); - let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); - // generate an equivocation proof for the old set, - let equivocation_proof = generate_double_voting_proof( - (block_num, payload1, old_set_id, &equivocation_keyring), - (block_num, payload2, old_set_id, &equivocation_keyring), - ); - // report the equivocation and the tx should be dispatched successfully - assert_ok!(Beefy::report_double_voting_unsigned( - RuntimeOrigin::none(), - Box::new(equivocation_proof), - key_owner_proof, - ),); + assert_ok!(f(block_num, old_set_id, &equivocation_keyring, key_owner_proof)); start_era(3); @@ -430,8 +445,7 @@ fn report_equivocation_old_set_works() { }); } -#[test] -fn report_equivocation_invalid_set_id() { +fn report_equivocation_invalid_set_id(mut f: impl ReportEquivocationFn) { let authorities = test_authorities(); ExtBuilder::default().add_authorities(authorities).build_and_execute(|| { @@ -448,28 +462,15 @@ fn report_equivocation_invalid_set_id() { let key_owner_proof = Historical::prove((BEEFY_KEY_TYPE, &equivocation_key)).unwrap(); - let payload1 = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); - let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); - // generate an equivocation for a future set - let equivocation_proof = generate_double_voting_proof( - (block_num, payload1, set_id + 1, &equivocation_keyring), - (block_num, payload2, set_id + 1, &equivocation_keyring), - ); - // the call for reporting the equivocation should error assert_err!( - Beefy::report_double_voting_unsigned( - RuntimeOrigin::none(), - Box::new(equivocation_proof), - key_owner_proof, - ), + f(block_num, set_id + 1, &equivocation_keyring, key_owner_proof), Error::::InvalidEquivocationProofSession, ); }); } -#[test] -fn report_equivocation_invalid_session() { +fn report_equivocation_invalid_session(mut f: impl ReportEquivocationFn) { let authorities = test_authorities(); ExtBuilder::default().add_authorities(authorities).build_and_execute(|| { @@ -490,29 +491,16 @@ fn report_equivocation_invalid_session() { let set_id = Beefy::validator_set().unwrap().id(); - let payload1 = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); - let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); - // generate an equivocation proof at following era set id = 2 - let equivocation_proof = generate_double_voting_proof( - (block_num, payload1, set_id, &equivocation_keyring), - (block_num, payload2, set_id, &equivocation_keyring), - ); - // report an equivocation for the current set using an key ownership // proof from the previous set, the session should be invalid. assert_err!( - Beefy::report_double_voting_unsigned( - RuntimeOrigin::none(), - Box::new(equivocation_proof), - key_owner_proof, - ), + f(block_num, set_id + 1, &equivocation_keyring, key_owner_proof), Error::::InvalidEquivocationProofSession, ); }); } -#[test] -fn report_equivocation_invalid_key_owner_proof() { +fn report_equivocation_invalid_key_owner_proof(mut f: impl ReportEquivocationFn) { let authorities = test_authorities(); ExtBuilder::default().add_authorities(authorities).build_and_execute(|| { @@ -534,14 +522,6 @@ fn report_equivocation_invalid_key_owner_proof() { let equivocation_key = &authorities[equivocation_authority_index]; let equivocation_keyring = BeefyKeyring::from_public(equivocation_key).unwrap(); - let payload1 = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); - let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); - // generate an equivocation proof for the authority at index 0 - let equivocation_proof = generate_double_voting_proof( - (block_num, payload1, set_id, &equivocation_keyring), - (block_num, payload2, set_id, &equivocation_keyring), - ); - // we need to start a new era otherwise the key ownership proof won't be // checked since the authorities are part of the current session start_era(2); @@ -549,18 +529,81 @@ fn report_equivocation_invalid_key_owner_proof() { // report an equivocation for the current set using a key ownership // proof for a different key than the one in the equivocation proof. assert_err!( - Beefy::report_double_voting_unsigned( - RuntimeOrigin::none(), - Box::new(equivocation_proof), - invalid_key_owner_proof, - ), + f(block_num, set_id, &equivocation_keyring, invalid_key_owner_proof), Error::::InvalidKeyOwnershipProof, ); }); } +fn valid_equivocation_reports_dont_pay_fees(mut f: impl ReportEquivocationFn) { + let authorities = test_authorities(); + + ExtBuilder::default().add_authorities(authorities).build_and_execute(|| { + start_era(1); + + let block_num = System::block_number(); + let validator_set = Beefy::validator_set().unwrap(); + let authorities = validator_set.validators(); + let set_id = validator_set.id(); + + let equivocation_authority_index = 0; + let equivocation_key = &authorities[equivocation_authority_index]; + let equivocation_keyring = BeefyKeyring::from_public(equivocation_key).unwrap(); + + // create the key ownership proof. + let key_owner_proof = Historical::prove((BEEFY_KEY_TYPE, &equivocation_key)).unwrap(); + + // report the equivocation. + let post_info = + f(block_num, set_id, &equivocation_keyring, key_owner_proof.clone()).unwrap(); + + // the original weight should be kept, but given that the report + // is valid the fee is waived. + assert!(post_info.actual_weight.is_none()); + assert_eq!(post_info.pays_fee, Pays::No); + + // report the equivocation again which is invalid now since it is + // duplicate. + let post_info = f(block_num, set_id, &equivocation_keyring, key_owner_proof) + .err() + .unwrap() + .post_info; + + // the fee is not waived and the original weight is kept. + assert!(post_info.actual_weight.is_none()); + assert_eq!(post_info.pays_fee, Pays::Yes); + }) +} + +// Test double voting reporting logic. + +#[test] +fn report_double_voting_current_set_works() { + report_equivocation_current_set_works(report_double_voting); +} + +#[test] +fn report_double_voting_old_set_works() { + report_equivocation_old_set_works(report_double_voting); +} + +#[test] +fn report_double_voting_invalid_set_id() { + report_equivocation_invalid_set_id(report_double_voting); +} + +#[test] +fn report_double_voting_invalid_session() { + report_equivocation_invalid_session(report_double_voting); +} + #[test] -fn report_equivocation_invalid_equivocation_proof() { +fn report_double_voting_invalid_key_owner_proof() { + report_equivocation_invalid_key_owner_proof(report_double_voting); +} + +#[test] +fn report_double_voting_invalid_equivocation_proof() { let authorities = test_authorities(); ExtBuilder::default().add_authorities(authorities).build_and_execute(|| { @@ -628,7 +671,7 @@ fn report_equivocation_invalid_equivocation_proof() { } #[test] -fn report_equivocation_validate_unsigned_prevents_duplicates() { +fn report_double_voting_validate_unsigned_prevents_duplicates() { use sp_runtime::transaction_validity::{ InvalidTransaction, TransactionPriority, TransactionSource, TransactionValidity, ValidTransaction, @@ -718,7 +761,7 @@ fn report_equivocation_validate_unsigned_prevents_duplicates() { } #[test] -fn report_equivocation_has_valid_weight() { +fn report_double_voting_has_valid_weight() { // the weight depends on the size of the validator set, // but there's a lower bound of 100 validators. assert!((1..=100) @@ -737,13 +780,68 @@ fn report_equivocation_has_valid_weight() { } #[test] -fn valid_equivocation_reports_dont_pay_fees() { +fn valid_double_voting_reports_dont_pay_fees() { + valid_equivocation_reports_dont_pay_fees(report_double_voting) +} + +// Test fork voting reporting logic. + +fn report_fork_voting( + block_num: u64, + set_id: ValidatorSetId, + equivocation_keyring: &BeefyKeyring, + key_owner_proof: MembershipProof, +) -> DispatchResultWithPostInfo { + let payload = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); + let equivocation_proof = generate_fork_voting_proof( + (block_num, payload, set_id, &equivocation_keyring), + MockAncestryProof { is_non_canonical: true }, + System::finalize(), + ); + + Beefy::report_fork_voting_unsigned( + RuntimeOrigin::none(), + Box::new(equivocation_proof), + key_owner_proof, + ) +} + +#[test] +fn report_fork_voting_current_set_works() { + report_equivocation_current_set_works(report_fork_voting); +} + +#[test] +fn report_fork_voting_old_set_works() { + report_equivocation_old_set_works(report_fork_voting); +} + +#[test] +fn report_fork_voting_invalid_set_id() { + report_equivocation_invalid_set_id(report_fork_voting); +} + +#[test] +fn report_fork_voting_invalid_session() { + report_equivocation_invalid_session(report_fork_voting); +} + +#[test] +fn report_fork_voting_invalid_key_owner_proof() { + report_equivocation_invalid_key_owner_proof(report_fork_voting); +} + +#[test] +fn report_fork_voting_invalid_equivocation_proof() { let authorities = test_authorities(); - ExtBuilder::default().add_authorities(authorities).build_and_execute(|| { - start_era(1); + let mut ext = ExtBuilder::default().add_authorities(authorities).build(); + let mut era = 1; + let (block_num, set_id, equivocation_keyring, key_owner_proof) = ext.execute_with(|| { + start_era(era); let block_num = System::block_number(); + let validator_set = Beefy::validator_set().unwrap(); let authorities = validator_set.validators(); let set_id = validator_set.id(); @@ -752,60 +850,52 @@ fn valid_equivocation_reports_dont_pay_fees() { let equivocation_key = &authorities[equivocation_authority_index]; let equivocation_keyring = BeefyKeyring::from_public(equivocation_key).unwrap(); - // generate equivocation proof - let payload1 = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); - let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); - let equivocation_proof = generate_double_voting_proof( - (block_num, payload1, set_id, &equivocation_keyring), - (block_num, payload2, set_id, &equivocation_keyring), - ); - - // create the key ownership proof. + // generate a key ownership proof at set id in era 1 let key_owner_proof = Historical::prove((BEEFY_KEY_TYPE, &equivocation_key)).unwrap(); - // check the dispatch info for the call. - let info = Call::::report_double_voting_unsigned { - equivocation_proof: Box::new(equivocation_proof.clone()), - key_owner_proof: key_owner_proof.clone(), - } - .get_dispatch_info(); - - // it should have non-zero weight and the fee has to be paid. - assert!(info.weight.any_gt(Weight::zero())); - assert_eq!(info.pays_fee, Pays::Yes); - - // report the equivocation. - let post_info = Beefy::report_double_voting_unsigned( - RuntimeOrigin::none(), - Box::new(equivocation_proof.clone()), - key_owner_proof.clone(), - ) - .unwrap(); + era += 1; + start_era(era); + (block_num, set_id, equivocation_keyring, key_owner_proof) + }); + ext.persist_offchain_overlay(); - // the original weight should be kept, but given that the report - // is valid the fee is waived. - assert!(post_info.actual_weight.is_none()); - assert_eq!(post_info.pays_fee, Pays::No); + ext.execute_with(|| { + let payload = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); - // report the equivocation again which is invalid now since it is - // duplicate. - let post_info = Beefy::report_double_voting_unsigned( - RuntimeOrigin::none(), - Box::new(equivocation_proof), - key_owner_proof, - ) - .err() - .unwrap() - .post_info; + // vote signed with a key that isn't part of the authority set + let equivocation_proof = generate_fork_voting_proof( + (block_num, payload.clone(), set_id, &BeefyKeyring::Dave), + MockAncestryProof { is_non_canonical: true }, + System::finalize(), + ); + assert_err!( + Beefy::report_fork_voting_unsigned( + RuntimeOrigin::none(), + Box::new(equivocation_proof), + key_owner_proof.clone(), + ), + Error::::InvalidKeyOwnershipProof, + ); - // the fee is not waived and the original weight is kept. - assert!(post_info.actual_weight.is_none()); - assert_eq!(post_info.pays_fee, Pays::Yes); - }) + // Simulate InvalidForkVotingProof error. + let equivocation_proof = generate_fork_voting_proof( + (block_num + 1, payload.clone(), set_id, &equivocation_keyring), + MockAncestryProof { is_non_canonical: false }, + System::finalize(), + ); + assert_err!( + Beefy::report_fork_voting_unsigned( + RuntimeOrigin::none(), + Box::new(equivocation_proof), + key_owner_proof.clone(), + ), + Error::::InvalidForkVotingProof, + ); + }); } #[test] -fn report_fork_equivocation_vote_current_set_works() { +fn report_fork_voting_invalid_context() { let authorities = test_authorities(); let mut ext = ExtBuilder::default().add_authorities(authorities).build(); @@ -858,403 +948,90 @@ fn report_fork_equivocation_vote_current_set_works() { // create the key ownership proof let key_owner_proof = Historical::prove((BEEFY_KEY_TYPE, &equivocation_key)).unwrap(); - // report the equivocation and the tx should be dispatched successfully - assert_ok!(Beefy::report_fork_voting_unsigned( - RuntimeOrigin::none(), - Box::new(equivocation_proof), - key_owner_proof, - ),); - - era += 1; - start_era(era); - - // check that the balance of 0-th validator is slashed 100%. - let equivocation_validator_id = validators[equivocation_authority_index]; + // report an equivocation for the current set. Simulate a failure of + // `extract_validation_context` + AncestryProofContext::set(&None); + assert_err!( + Beefy::report_fork_voting_unsigned( + RuntimeOrigin::none(), + Box::new(equivocation_proof.clone()), + key_owner_proof.clone(), + ), + Error::::InvalidForkVotingProof, + ); - assert_eq!(Balances::total_balance(&equivocation_validator_id), 10_000_000 - 10_000); - assert_eq!(Staking::slashable_balance_of(&equivocation_validator_id), 0); - assert_eq!( - Staking::eras_stakers(era, &equivocation_validator_id), - pallet_staking::Exposure { total: 0, own: 0, others: vec![] }, + // report an equivocation for the current set. Simulate an invalid context. + AncestryProofContext::set(&Some(MockAncestryProofContext { is_valid: false })); + assert_err!( + Beefy::report_fork_voting_unsigned( + RuntimeOrigin::none(), + Box::new(equivocation_proof), + key_owner_proof, + ), + Error::::InvalidForkVotingProof, ); + }); +} - // check that the balances of all other validators are left intact. - for validator in &validators { - if *validator == equivocation_validator_id { - continue - } +#[test] +fn valid_fork_voting_reports_dont_pay_fees() { + valid_equivocation_reports_dont_pay_fees(report_fork_voting) +} - assert_eq!(Balances::total_balance(validator), 10_000_000); - assert_eq!(Staking::slashable_balance_of(validator), 10_000); - - assert_eq!( - Staking::eras_stakers(era, validator), - pallet_staking::Exposure { total: 10_000, own: 10_000, others: vec![] }, - ); - } - }); +// Test future block voting reporting logic. + +fn report_future_block_voting( + block_num: u64, + set_id: ValidatorSetId, + equivocation_keyring: &BeefyKeyring, + key_owner_proof: MembershipProof, +) -> DispatchResultWithPostInfo { + let payload = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); + let equivocation_proof = generate_future_block_voting_proof(( + block_num + 100, + payload, + set_id, + &equivocation_keyring, + )); + + Beefy::report_future_block_voting_unsigned( + RuntimeOrigin::none(), + Box::new(equivocation_proof), + key_owner_proof, + ) } #[test] -fn report_fork_equivocation_vote_old_set_works() { - let authorities = test_authorities(); - - let mut ext = ExtBuilder::default().add_authorities(authorities).build(); - - let mut era = 1; - let ( - block_num, - validators, - old_set_id, - equivocation_authority_index, - equivocation_key, - key_owner_proof, - ) = ext.execute_with(|| { - start_era(era); - let block_num = System::block_number(); - era += 1; - start_era(era); - - let validator_set = Beefy::validator_set().unwrap(); - let authorities = validator_set.validators(); - let validators = Session::validators(); - let old_set_id = validator_set.id(); - - assert_eq!(authorities.len(), 2); - let equivocation_authority_index = 0; - let equivocation_key = authorities[equivocation_authority_index].clone(); - - // create the key ownership proof in the "old" set - let key_owner_proof = Historical::prove((BEEFY_KEY_TYPE, &&equivocation_key)).unwrap(); - - era += 1; - start_era(era); - ( - block_num, - validators, - old_set_id, - equivocation_authority_index, - equivocation_key, - key_owner_proof, - ) - }); - ext.persist_offchain_overlay(); - - ext.execute_with(|| { - // make sure that all authorities have the same balance - for validator in &validators { - assert_eq!(Balances::total_balance(validator), 10_000_000); - assert_eq!(Staking::slashable_balance_of(validator), 10_000); - - assert_eq!( - Staking::eras_stakers(2, validator), - pallet_staking::Exposure { total: 10_000, own: 10_000, others: vec![] }, - ); - } - - let validator_set = Beefy::validator_set().unwrap(); - let new_set_id = validator_set.id(); - assert_eq!(old_set_id + 3, new_set_id); - - let equivocation_keyring = BeefyKeyring::from_public(&equivocation_key).unwrap(); - - let payload = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); - - // generate a fork equivocation proof, with a vote in the same round for a - // different payload than finalized - let equivocation_proof = generate_fork_voting_proof( - (block_num, payload, old_set_id, &equivocation_keyring), - MockAncestryProof { is_non_canonical: true }, - System::finalize(), - ); - - // report the equivocation and the tx should be dispatched successfully - assert_ok!(Beefy::report_fork_voting_unsigned( - RuntimeOrigin::none(), - Box::new(equivocation_proof), - key_owner_proof, - ),); - - era += 1; - start_era(era); - - // check that the balance of 0-th validator is slashed 100%. - let equivocation_validator_id = validators[equivocation_authority_index]; - - assert_eq!(Balances::total_balance(&equivocation_validator_id), 10_000_000 - 10_000); - assert_eq!(Staking::slashable_balance_of(&equivocation_validator_id), 0); - assert_eq!( - Staking::eras_stakers(era, &equivocation_validator_id), - pallet_staking::Exposure { total: 0, own: 0, others: vec![] }, - ); - - // check that the balances of all other validators are left intact. - for validator in &validators { - if *validator == equivocation_validator_id { - continue - } - - assert_eq!(Balances::total_balance(validator), 10_000_000); - assert_eq!(Staking::slashable_balance_of(validator), 10_000); - - assert_eq!( - Staking::eras_stakers(3, validator), - pallet_staking::Exposure { total: 10_000, own: 10_000, others: vec![] }, - ); - } - }); +fn report_future_block_voting_current_set_works() { + report_equivocation_current_set_works(report_future_block_voting); } #[test] -fn report_fork_equivocation_vote_invalid_set_id() { - let authorities = test_authorities(); - - let mut ext = ExtBuilder::default().add_authorities(authorities).build(); - - let block_num = ext.execute_with(|| { - let mut era = 1; - start_era(era); - let block_num = System::block_number(); - - era += 1; - start_era(era); - block_num - }); - ext.persist_offchain_overlay(); - - ext.execute_with(|| { - let validator_set = Beefy::validator_set().unwrap(); - let authorities = validator_set.validators(); - let set_id = validator_set.id(); - - let equivocation_authority_index = 0; - let equivocation_key = &authorities[equivocation_authority_index]; - let equivocation_keyring = BeefyKeyring::from_public(equivocation_key).unwrap(); - - let key_owner_proof = Historical::prove((BEEFY_KEY_TYPE, &equivocation_key)).unwrap(); - - let payload = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); - // generate an equivocation for a future set - let equivocation_proof = generate_fork_voting_proof( - (block_num, payload, set_id + 1, &equivocation_keyring), - MockAncestryProof { is_non_canonical: true }, - System::finalize(), - ); - - // the call for reporting the equivocation should error - assert_err!( - Beefy::report_fork_voting_unsigned( - RuntimeOrigin::none(), - Box::new(equivocation_proof), - key_owner_proof, - ), - Error::::InvalidEquivocationProofSession, - ); - }); +fn report_future_block_voting_old_set_works() { + report_equivocation_old_set_works(report_future_block_voting); } #[test] -fn report_fork_equivocation_vote_invalid_session() { - let authorities = test_authorities(); - - let mut ext = ExtBuilder::default().add_authorities(authorities).build(); - - let mut era = 1; - let (block_num, equivocation_keyring, key_owner_proof) = ext.execute_with(|| { - start_era(era); - let block_num = System::block_number(); - - era += 1; - start_era(era); - - let validator_set = Beefy::validator_set().unwrap(); - let authorities = validator_set.validators(); - - let equivocation_authority_index = 0; - let equivocation_key = &authorities[equivocation_authority_index]; - let equivocation_keyring = BeefyKeyring::from_public(equivocation_key).unwrap(); - - // generate a key ownership proof at current era set id - let key_owner_proof = Historical::prove((BEEFY_KEY_TYPE, &equivocation_key)).unwrap(); - - era += 1; - start_era(era); - (block_num, equivocation_keyring, key_owner_proof) - }); - ext.persist_offchain_overlay(); - - ext.execute_with(|| { - let set_id = Beefy::validator_set().unwrap().id(); - - let payload = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); - // generate an equivocation proof at following era set id = 3 - let equivocation_proof = generate_fork_voting_proof( - (block_num, payload, set_id, &equivocation_keyring), - MockAncestryProof { is_non_canonical: true }, - System::finalize(), - ); - - // report an equivocation for the current set using a key ownership - // proof from the previous set, the session should be invalid. - assert_err!( - Beefy::report_fork_voting_unsigned( - RuntimeOrigin::none(), - Box::new(equivocation_proof), - key_owner_proof, - ), - Error::::InvalidEquivocationProofSession, - ); - }); +fn report_future_block_voting_invalid_set_id() { + report_equivocation_invalid_set_id(report_future_block_voting); } #[test] -fn report_fork_equivocation_vote_invalid_key_owner_proof() { - let authorities = test_authorities(); - - let mut ext = ExtBuilder::default().add_authorities(authorities).build(); - - let mut era = 1; - let block_num = ext.execute_with(|| { - start_era(era); - let block_num = System::block_number(); - - era += 1; - start_era(era); - block_num - }); - ext.persist_offchain_overlay(); - - ext.execute_with(|| { - let validator_set = Beefy::validator_set().unwrap(); - let authorities = validator_set.validators(); - let set_id = validator_set.id(); - - let invalid_owner_authority_index = 1; - let invalid_owner_key = &authorities[invalid_owner_authority_index]; - - // generate a key ownership proof for the authority at index 1 - let invalid_key_owner_proof = - Historical::prove((BEEFY_KEY_TYPE, &invalid_owner_key)).unwrap(); - - let equivocation_authority_index = 0; - let equivocation_key = &authorities[equivocation_authority_index]; - let equivocation_keyring = BeefyKeyring::from_public(equivocation_key).unwrap(); - - let payload = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); - // generate an equivocation for a future set - let equivocation_proof = generate_fork_voting_proof( - (block_num, payload, set_id, &equivocation_keyring), - MockAncestryProof { is_non_canonical: true }, - System::finalize(), - ); - - // we need to start a new era otherwise the key ownership proof won't be - // checked since the authorities are part of the current session - era += 1; - start_era(era); - - // report an equivocation for the current set using a key ownership - // proof for a different key than the one in the equivocation proof. - assert_err!( - Beefy::report_fork_voting_unsigned( - RuntimeOrigin::none(), - Box::new(equivocation_proof), - invalid_key_owner_proof, - ), - Error::::InvalidKeyOwnershipProof, - ); - }); +fn report_future_block_voting_invalid_session() { + report_equivocation_invalid_session(report_future_block_voting); } #[test] -fn report_fork_equivocation_vote_invalid_context() { - let authorities = test_authorities(); - - let mut ext = ExtBuilder::default().add_authorities(authorities).build(); - - let mut era = 1; - let block_num = ext.execute_with(|| { - assert_eq!(Staking::current_era(), Some(0)); - assert_eq!(Session::current_index(), 0); - start_era(era); - - let block_num = System::block_number(); - era += 1; - start_era(era); - block_num - }); - ext.persist_offchain_overlay(); - - ext.execute_with(|| { - let validator_set = Beefy::validator_set().unwrap(); - let authorities = validator_set.validators(); - let set_id = validator_set.id(); - let validators = Session::validators(); - - // make sure that all validators have the same balance - for validator in &validators { - assert_eq!(Balances::total_balance(validator), 10_000_000); - assert_eq!(Staking::slashable_balance_of(validator), 10_000); - - assert_eq!( - Staking::eras_stakers(era, validator), - pallet_staking::Exposure { total: 10_000, own: 10_000, others: vec![] }, - ); - } - - assert_eq!(authorities.len(), 2); - let equivocation_authority_index = 1; - let equivocation_key = &authorities[equivocation_authority_index]; - let equivocation_keyring = BeefyKeyring::from_public(equivocation_key).unwrap(); - - let payload = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); - - // generate a fork equivocation proof, with a vote in the same round for a - // different payload than finalized - let equivocation_proof = generate_fork_voting_proof( - (block_num, payload, set_id, &equivocation_keyring), - MockAncestryProof { is_non_canonical: true }, - System::finalize(), - ); - - // create the key ownership proof - let key_owner_proof = Historical::prove((BEEFY_KEY_TYPE, &equivocation_key)).unwrap(); - - // report an equivocation for the current set. Simulate a failure of - // `extract_validation_context` - AncestryProofContext::set(&None); - assert_err!( - Beefy::report_fork_voting_unsigned( - RuntimeOrigin::none(), - Box::new(equivocation_proof.clone()), - key_owner_proof.clone(), - ), - Error::::InvalidForkVotingProof, - ); - - // report an equivocation for the current set. Simulate an invalid context. - AncestryProofContext::set(&Some(MockAncestryProofContext { is_valid: false })); - assert_err!( - Beefy::report_fork_voting_unsigned( - RuntimeOrigin::none(), - Box::new(equivocation_proof), - key_owner_proof, - ), - Error::::InvalidForkVotingProof, - ); - }); +fn report_future_block_voting_invalid_key_owner_proof() { + report_equivocation_invalid_key_owner_proof(report_future_block_voting); } #[test] -fn report_fork_equivocation_vote_invalid_equivocation_proof() { +fn report_future_block_voting_invalid_equivocation_proof() { let authorities = test_authorities(); - let mut ext = ExtBuilder::default().add_authorities(authorities).build(); - - let mut era = 1; - let (block_num, set_id, equivocation_keyring, key_owner_proof) = ext.execute_with(|| { - start_era(era); - let block_num = System::block_number(); + ExtBuilder::default().add_authorities(authorities).build_and_execute(|| { + start_era(1); let validator_set = Beefy::validator_set().unwrap(); let authorities = validator_set.validators(); @@ -1264,126 +1041,33 @@ fn report_fork_equivocation_vote_invalid_equivocation_proof() { let equivocation_key = &authorities[equivocation_authority_index]; let equivocation_keyring = BeefyKeyring::from_public(equivocation_key).unwrap(); - // generate a key ownership proof at set id in era 1 + // create the key ownership proof let key_owner_proof = Historical::prove((BEEFY_KEY_TYPE, &equivocation_key)).unwrap(); - era += 1; - start_era(era); - (block_num, set_id, equivocation_keyring, key_owner_proof) - }); - ext.persist_offchain_overlay(); + start_era(2); - ext.execute_with(|| { let payload = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); - // vote signed with a key that isn't part of the authority set - let equivocation_proof = generate_fork_voting_proof( - (block_num, payload.clone(), set_id, &BeefyKeyring::Dave), - MockAncestryProof { is_non_canonical: true }, - System::finalize(), - ); + // vote targeting old block assert_err!( - Beefy::report_fork_voting_unsigned( + Beefy::report_future_block_voting_unsigned( RuntimeOrigin::none(), - Box::new(equivocation_proof), + Box::new(generate_future_block_voting_proof(( + 1, + payload.clone(), + set_id, + &equivocation_keyring, + ))), key_owner_proof.clone(), ), - Error::::InvalidKeyOwnershipProof, - ); - - // Simulate InvalidForkVotingProof error. - let equivocation_proof = generate_fork_voting_proof( - (block_num + 1, payload.clone(), set_id, &equivocation_keyring), - MockAncestryProof { is_non_canonical: false }, - System::finalize(), - ); - assert_err!( - Beefy::report_fork_voting_unsigned( - RuntimeOrigin::none(), - Box::new(equivocation_proof), - key_owner_proof.clone(), - ), - Error::::InvalidForkVotingProof, + Error::::InvalidFutureBlockVotingProof, ); }); } #[test] -fn valid_fork_equivocation_vote_reports_dont_pay_fees() { - let authorities = test_authorities(); - - let mut ext = ExtBuilder::default().add_authorities(authorities).build(); - - let mut era = 1; - let block_num = ext.execute_with(|| { - start_era(era); - let block_num = System::block_number(); - - era += 1; - start_era(era); - block_num - }); - ext.persist_offchain_overlay(); - - ext.execute_with(|| { - let validator_set = Beefy::validator_set().unwrap(); - let authorities = validator_set.validators(); - let set_id = validator_set.id(); - - let equivocation_authority_index = 0; - let equivocation_key = &authorities[equivocation_authority_index]; - let equivocation_keyring = BeefyKeyring::from_public(equivocation_key).unwrap(); - - // generate equivocation proof - let payload = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); - let equivocation_proof = generate_fork_voting_proof( - (block_num, payload, set_id, &equivocation_keyring), - MockAncestryProof { is_non_canonical: true }, - System::finalize(), - ); - - // create the key ownership proof. - let key_owner_proof = Historical::prove((BEEFY_KEY_TYPE, &equivocation_key)).unwrap(); - - // check the dispatch info for the call. - let info = Call::::report_fork_voting_unsigned { - equivocation_proof: Box::new(equivocation_proof.clone()), - key_owner_proof: key_owner_proof.clone(), - } - .get_dispatch_info(); - - // it should have non-zero weight and the fee has to be paid. - assert!(info.weight.any_gt(Weight::zero())); - assert_eq!(info.pays_fee, Pays::Yes); - - // report the equivocation. - let post_info = Beefy::report_fork_voting_unsigned( - RuntimeOrigin::none(), - Box::new(equivocation_proof.clone()), - key_owner_proof.clone(), - ) - .unwrap(); - - // the original weight should be kept, but given that the report - // is valid the fee is waived. - assert!(post_info.actual_weight.is_none()); - assert_eq!(post_info.pays_fee, Pays::No); - - // report the equivocation again which is invalid now since it is - // duplicate. - let post_info = Beefy::report_fork_voting_unsigned( - RuntimeOrigin::none(), - Box::new(equivocation_proof.clone()), - key_owner_proof.clone(), - ) - .err() - .unwrap() - .post_info; - - // the fee is not waived and the original weight is kept. - assert!(post_info.actual_weight.is_none()); - assert_eq!(post_info.pays_fee, Pays::Yes); - }) +fn valid_future_block_voting_reports_dont_pay_fees() { + valid_equivocation_reports_dont_pay_fees(report_future_block_voting) } #[test] diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index c3396448e70a..e5ffa3033fa0 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -347,6 +347,13 @@ pub struct ForkVotingProof pub header: Header, } +/// Proof showing that an authority voted for a future block. +#[derive(Clone, Debug, Decode, Encode, PartialEq, TypeInfo)] +pub struct FutureBlockVotingProof { + /// The equivocated vote. + pub vote: VoteMessage, +} + /// Check a commitment signature by encoding the commitment and /// verifying the provided signature using the expected authority id. pub fn check_commitment_signature( diff --git a/substrate/primitives/consensus/beefy/src/test_utils.rs b/substrate/primitives/consensus/beefy/src/test_utils.rs index cba3dde7ec96..bd335ede4893 100644 --- a/substrate/primitives/consensus/beefy/src/test_utils.rs +++ b/substrate/primitives/consensus/beefy/src/test_utils.rs @@ -19,7 +19,7 @@ use crate::ecdsa_bls_crypto; use crate::{ ecdsa_crypto, AuthorityIdBound, BeefySignatureHasher, Commitment, DoubleVotingProof, - ForkVotingProof, Payload, ValidatorSetId, VoteMessage, + ForkVotingProof, FutureBlockVotingProof, Payload, ValidatorSetId, VoteMessage, }; use sp_application_crypto::{AppCrypto, AppPair, RuntimeAppPublic, Wraps}; use sp_core::{ecdsa, Pair}; @@ -137,7 +137,7 @@ impl From> for ecdsa_crypto::Public { } /// Create a new `VoteMessage` from commitment primitives and keyring -fn signed_vote( +pub fn signed_vote( block_number: Number, payload: Payload, validator_set_id: ValidatorSetId, @@ -167,3 +167,11 @@ pub fn generate_fork_voting_proof, AncestryProof>( let signed_vote = signed_vote(vote.0, vote.1, vote.2, vote.3); ForkVotingProof { vote: signed_vote, ancestry_proof, header } } + +/// Create a new `ForkVotingProof` based on vote & canonical header. +pub fn generate_future_block_voting_proof( + vote: (u64, Payload, ValidatorSetId, &Keyring), +) -> FutureBlockVotingProof { + let signed_vote = signed_vote(vote.0, vote.1, vote.2, vote.3); + FutureBlockVotingProof { vote: signed_vote } +} From 2450e770e6dcd51b8a50c8612aed11aef16bffd8 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Wed, 3 Jul 2024 13:42:52 +0300 Subject: [PATCH 07/14] prdoc --- prdoc/pr_4522.prdoc | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 prdoc/pr_4522.prdoc diff --git a/prdoc/pr_4522.prdoc b/prdoc/pr_4522.prdoc new file mode 100644 index 000000000000..72cacfd6bdd7 --- /dev/null +++ b/prdoc/pr_4522.prdoc @@ -0,0 +1,25 @@ +# 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. + - 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 + - name: pallet-beefy-mmr + - name: pallet-mmr + - name: sc-consensus-beefy + - name: kitchensink-runtime + - name: rococo-runtime + - name: westend-runtime From 7445ec45ac700da1113639ed375f3378225f6109 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Wed, 3 Jul 2024 13:58:50 +0300 Subject: [PATCH 08/14] fix prdoc --- prdoc/pr_4522.prdoc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/prdoc/pr_4522.prdoc b/prdoc/pr_4522.prdoc index 72cacfd6bdd7..47ffbc2c57c0 100644 --- a/prdoc/pr_4522.prdoc +++ b/prdoc/pr_4522.prdoc @@ -4,7 +4,9 @@ title: Added runtime support for reporting BEEFY fork voting doc: - - audience: Runtime Dev | Runtime User + - 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`. From 89a78d555ed4aa23410ba4c1932edd8196a5bb0a Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Wed, 3 Jul 2024 14:13:22 +0300 Subject: [PATCH 09/14] Update prdoc Co-authored-by: Adrian Catangiu --- prdoc/pr_4522.prdoc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/prdoc/pr_4522.prdoc b/prdoc/pr_4522.prdoc index 47ffbc2c57c0..684c2a161e9a 100644 --- a/prdoc/pr_4522.prdoc +++ b/prdoc/pr_4522.prdoc @@ -19,9 +19,16 @@ doc: crates: - name: pallet-beefy + bump: major - name: pallet-beefy-mmr + bump: major - name: pallet-mmr + bump: major - name: sc-consensus-beefy + bump: minor - name: kitchensink-runtime + bump: major - name: rococo-runtime + bump: major - name: westend-runtime + bump: major From 80b4c6f8869fbc6332f249785780accbf7a6da8a Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Wed, 3 Jul 2024 14:41:15 +0300 Subject: [PATCH 10/14] check_equivocation_proof -> check_double_voting_proof --- substrate/client/consensus/beefy/src/fisherman.rs | 4 ++-- substrate/frame/beefy/src/equivocation.rs | 2 +- substrate/frame/beefy/src/tests.rs | 12 ++++++------ substrate/primitives/consensus/beefy/src/lib.rs | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/substrate/client/consensus/beefy/src/fisherman.rs b/substrate/client/consensus/beefy/src/fisherman.rs index ec838b48a394..faa4d34eff5a 100644 --- 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(()); } diff --git a/substrate/frame/beefy/src/equivocation.rs b/substrate/frame/beefy/src/equivocation.rs index e07ceb226314..a1526e781111 100644 --- a/substrate/frame/beefy/src/equivocation.rs +++ b/substrate/frame/beefy/src/equivocation.rs @@ -203,7 +203,7 @@ impl EquivocationEvidenceFor { match self { EquivocationEvidenceFor::DoubleVotingProof(equivocation_proof, _) => { // Validate equivocation proof (check votes are different and signatures are valid). - if !sp_consensus_beefy::check_equivocation_proof(&equivocation_proof) { + if !sp_consensus_beefy::check_double_voting_proof(&equivocation_proof) { return Err(Error::::InvalidDoubleVotingProof); } diff --git a/substrate/frame/beefy/src/tests.rs b/substrate/frame/beefy/src/tests.rs index 33577439ac1a..a63b3532b698 100644 --- a/substrate/frame/beefy/src/tests.rs +++ b/substrate/frame/beefy/src/tests.rs @@ -24,7 +24,7 @@ use frame_support::{ traits::{Currency, KeyOwnerProofSystem, OnInitialize}, }; use sp_consensus_beefy::{ - check_equivocation_proof, ecdsa_crypto, + check_double_voting_proof, ecdsa_crypto, known_payloads::MMR_ROOT_ID, test_utils::{ generate_double_voting_proof, generate_fork_voting_proof, @@ -231,7 +231,7 @@ fn should_sign_and_verify() { (1, payload1.clone(), set_id, &BeefyKeyring::Bob), ); // expect invalid equivocation proof - assert!(!check_equivocation_proof::<_, _, Keccak256>(&equivocation_proof)); + assert!(!check_double_voting_proof::<_, _, Keccak256>(&equivocation_proof)); // generate an equivocation proof, with two votes in different rounds for // different payloads signed by the same key @@ -240,7 +240,7 @@ fn should_sign_and_verify() { (2, payload2.clone(), set_id, &BeefyKeyring::Bob), ); // expect invalid equivocation proof - assert!(!check_equivocation_proof::<_, _, Keccak256>(&equivocation_proof)); + assert!(!check_double_voting_proof::<_, _, Keccak256>(&equivocation_proof)); // generate an equivocation proof, with two votes by different authorities let equivocation_proof = generate_double_voting_proof( @@ -248,7 +248,7 @@ fn should_sign_and_verify() { (1, payload2.clone(), set_id, &BeefyKeyring::Bob), ); // expect invalid equivocation proof - assert!(!check_equivocation_proof::<_, _, Keccak256>(&equivocation_proof)); + assert!(!check_double_voting_proof::<_, _, Keccak256>(&equivocation_proof)); // generate an equivocation proof, with two votes in different set ids let equivocation_proof = generate_double_voting_proof( @@ -256,7 +256,7 @@ fn should_sign_and_verify() { (1, payload2.clone(), set_id + 1, &BeefyKeyring::Bob), ); // expect invalid equivocation proof - assert!(!check_equivocation_proof::<_, _, Keccak256>(&equivocation_proof)); + assert!(!check_double_voting_proof::<_, _, Keccak256>(&equivocation_proof)); // generate an equivocation proof, with two votes in the same round for // different payloads signed by the same key @@ -266,7 +266,7 @@ fn should_sign_and_verify() { (1, payload2, set_id, &BeefyKeyring::Bob), ); // expect valid equivocation proof - assert!(check_equivocation_proof::<_, _, Keccak256>(&equivocation_proof)); + assert!(check_double_voting_proof::<_, _, Keccak256>(&equivocation_proof)); } trait ReportEquivocationFn: diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index c630dfb1b1ea..7f6f733d0e39 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -374,7 +374,7 @@ where /// Verifies the equivocation proof by making sure that both votes target /// different blocks and that its signatures are valid. -pub fn check_equivocation_proof( +pub fn check_double_voting_proof( report: &DoubleVotingProof::Signature>, ) -> bool where From 95c82ff4273b4a6b58627060c3ab6bc9e3269896 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Wed, 3 Jul 2024 15:34:47 +0300 Subject: [PATCH 11/14] Update prdoc --- prdoc/pr_4522.prdoc | 1 + 1 file changed, 1 insertion(+) diff --git a/prdoc/pr_4522.prdoc b/prdoc/pr_4522.prdoc index 684c2a161e9a..a88fe68af7b5 100644 --- a/prdoc/pr_4522.prdoc +++ b/prdoc/pr_4522.prdoc @@ -12,6 +12,7 @@ doc: 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 From e8822cabbb1bed280fdce00d4b8ebab682017100 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Wed, 3 Jul 2024 15:55:32 +0300 Subject: [PATCH 12/14] Fix semver --- prdoc/pr_4522.prdoc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/prdoc/pr_4522.prdoc b/prdoc/pr_4522.prdoc index a88fe68af7b5..4d4e479f46ae 100644 --- a/prdoc/pr_4522.prdoc +++ b/prdoc/pr_4522.prdoc @@ -22,7 +22,7 @@ crates: - name: pallet-beefy bump: major - name: pallet-beefy-mmr - bump: major + bump: minor - name: pallet-mmr bump: major - name: sc-consensus-beefy @@ -30,6 +30,8 @@ crates: - name: kitchensink-runtime bump: major - name: rococo-runtime - bump: major + bump: minor - name: westend-runtime - bump: major + bump: minor + - name: sp-consensus-beefy + - name: polkadot-service From fb12a8aada8285e68ce26ee41a95c4aa4b5aaef2 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Wed, 3 Jul 2024 15:56:40 +0300 Subject: [PATCH 13/14] Leftover semver fix --- prdoc/pr_4522.prdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prdoc/pr_4522.prdoc b/prdoc/pr_4522.prdoc index 4d4e479f46ae..e1a781087f13 100644 --- a/prdoc/pr_4522.prdoc +++ b/prdoc/pr_4522.prdoc @@ -26,7 +26,7 @@ crates: - name: pallet-mmr bump: major - name: sc-consensus-beefy - bump: minor + bump: patch - name: kitchensink-runtime bump: major - name: rococo-runtime From c1f97b55d993cd384a96fd105328b2e3174cee73 Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Wed, 3 Jul 2024 16:17:42 +0300 Subject: [PATCH 14/14] upate prdoc bumps --- prdoc/pr_4522.prdoc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/prdoc/pr_4522.prdoc b/prdoc/pr_4522.prdoc index e1a781087f13..c8fdcfa51a41 100644 --- a/prdoc/pr_4522.prdoc +++ b/prdoc/pr_4522.prdoc @@ -30,8 +30,10 @@ crates: - name: kitchensink-runtime bump: major - name: rococo-runtime - bump: minor + bump: major - name: westend-runtime - bump: minor + bump: major - name: sp-consensus-beefy + bump: major - name: polkadot-service + bump: patch