From 51498b40e8153b4837196d71293847acc8f635cd Mon Sep 17 00:00:00 2001 From: Serban Iorga Date: Wed, 3 Jul 2024 16:44:21 +0300 Subject: [PATCH] [BEEFY] Add runtime support for reporting fork voting (#4522) Related to https://github.com/paritytech/polkadot-sdk/issues/4523 Extracting part of https://github.com/paritytech/polkadot-sdk/pull/1903 (credits to @Lederstrumpf for the high-level strategy), but also introducing significant adjustments both to the approach and to the code. The main adjustment is the fact that the `ForkVotingProof` accepts only one vote, compared to the original version which accepted a `vec![]`. With this approach more calls are needed in order to report multiple equivocated votes on the same commit, but it simplifies a lot the checking logic. We can add support for reporting multiple signatures at once in the future. There are 2 things that are missing in order to consider this issue done, but I would propose to do them in a separate PR since this one is already pretty big: - benchmarks/computing a weight for the new extrinsic (this wasn't present in https://github.com/paritytech/polkadot-sdk/pull/1903 either) - exposing an API for generating the ancestry proof. I'm not sure if we should do this in the Mmr pallet or in the Beefy pallet Co-authored-by: Robert Hambrock --------- Co-authored-by: Adrian Catangiu --- polkadot/node/service/src/fake_runtime_api.rs | 2 +- polkadot/runtime/rococo/src/lib.rs | 7 +- polkadot/runtime/test-runtime/src/lib.rs | 2 +- polkadot/runtime/westend/src/lib.rs | 6 +- prdoc/pr_4522.prdoc | 39 ++ substrate/bin/node/runtime/src/lib.rs | 7 +- .../client/consensus/beefy/src/fisherman.rs | 6 +- substrate/client/consensus/beefy/src/tests.rs | 2 +- .../client/consensus/beefy/src/worker.rs | 6 +- substrate/frame/beefy-mmr/src/lib.rs | 79 ++- substrate/frame/beefy-mmr/src/mock.rs | 1 + substrate/frame/beefy-mmr/src/tests.rs | 224 ++++++- substrate/frame/beefy/src/default_weights.rs | 16 +- substrate/frame/beefy/src/equivocation.rs | 285 ++++++--- substrate/frame/beefy/src/lib.rs | 218 ++++++- substrate/frame/beefy/src/mock.rs | 47 +- substrate/frame/beefy/src/tests.rs | 572 +++++++++++++----- .../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 | 20 +- .../primitives/consensus/beefy/src/lib.rs | 58 +- .../primitives/consensus/beefy/src/payload.rs | 2 +- .../consensus/beefy/src/test_utils.rs | 48 +- 25 files changed, 1378 insertions(+), 400 deletions(-) create mode 100644 prdoc/pr_4522.prdoc diff --git a/polkadot/node/service/src/fake_runtime_api.rs b/polkadot/node/service/src/fake_runtime_api.rs index debf3690aead9..e971830c95cb2 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( _: sp_consensus_beefy::DoubleVotingProof< BlockNumber, BeefyId, diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index c91a712cce0df..015e433382c8d 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -1283,6 +1283,7 @@ impl pallet_beefy::Config for Runtime { type MaxNominators = ConstU32<0>; type MaxSetIdSessionEntries = BeefySetIdSessionEntries; type OnNewValidatorSet = MmrLeaf; + type AncestryHelper = MmrLeaf; type WeightInfo = (); type KeyOwnerProof = >::Proof; type EquivocationReportSystem = @@ -2052,7 +2053,7 @@ sp_api::impl_runtime_apis! { } } - #[api_version(3)] + #[api_version(4)] impl sp_consensus_beefy::BeefyApi for Runtime { fn beefy_genesis() -> Option { pallet_beefy::GenesisBlock::::get() @@ -2062,7 +2063,7 @@ sp_api::impl_runtime_apis! { Beefy::validator_set() } - fn submit_report_equivocation_unsigned_extrinsic( + fn submit_report_double_voting_unsigned_extrinsic( equivocation_proof: sp_consensus_beefy::DoubleVotingProof< BlockNumber, BeefyId, @@ -2072,7 +2073,7 @@ sp_api::impl_runtime_apis! { ) -> Option<()> { let key_owner_proof = key_owner_proof.decode()?; - Beefy::submit_unsigned_equivocation_report( + Beefy::submit_unsigned_double_voting_report( equivocation_proof, key_owner_proof, ) diff --git a/polkadot/runtime/test-runtime/src/lib.rs b/polkadot/runtime/test-runtime/src/lib.rs index 334c6eb733a1d..96392c026d5c9 100644 --- a/polkadot/runtime/test-runtime/src/lib.rs +++ b/polkadot/runtime/test-runtime/src/lib.rs @@ -1015,7 +1015,7 @@ sp_api::impl_runtime_apis! { None } - fn submit_report_equivocation_unsigned_extrinsic( + fn submit_report_double_voting_unsigned_extrinsic( _equivocation_proof: sp_consensus_beefy::DoubleVotingProof< BlockNumber, BeefyId, diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index 5b50a078539ef..ca58a6390109d 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -328,6 +328,7 @@ impl pallet_beefy::Config for Runtime { type MaxNominators = MaxNominators; type MaxSetIdSessionEntries = BeefySetIdSessionEntries; type OnNewValidatorSet = BeefyMmrLeaf; + type AncestryHelper = BeefyMmrLeaf; type WeightInfo = (); type KeyOwnerProof = sp_session::MembershipProof; type EquivocationReportSystem = @@ -2009,6 +2010,7 @@ sp_api::impl_runtime_apis! { } } + #[api_version(4)] impl sp_consensus_beefy::BeefyApi for Runtime { fn beefy_genesis() -> Option { pallet_beefy::GenesisBlock::::get() @@ -2018,7 +2020,7 @@ sp_api::impl_runtime_apis! { Beefy::validator_set() } - fn submit_report_equivocation_unsigned_extrinsic( + fn submit_report_double_voting_unsigned_extrinsic( equivocation_proof: sp_consensus_beefy::DoubleVotingProof< BlockNumber, BeefyId, @@ -2028,7 +2030,7 @@ sp_api::impl_runtime_apis! { ) -> Option<()> { let key_owner_proof = key_owner_proof.decode()?; - Beefy::submit_unsigned_equivocation_report( + Beefy::submit_unsigned_double_voting_report( equivocation_proof, key_owner_proof, ) diff --git a/prdoc/pr_4522.prdoc b/prdoc/pr_4522.prdoc new file mode 100644 index 0000000000000..c8fdcfa51a419 --- /dev/null +++ b/prdoc/pr_4522.prdoc @@ -0,0 +1,39 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: Added runtime support for reporting BEEFY fork voting + +doc: + - audience: + - Runtime Dev + - Runtime User + description: | + This PR adds the `report_fork_voting`, `report_future_voting` extrinsics to `pallet-beefy` + and renames the `report_equivocation` extrinsic to `report_double_voting`. + `report_fork_voting` can't be called yet, since it uses `Weight::MAX` weight. We will + add benchmarks for it and set the proper weight in a future PR. + Also a new `AncestryHelper` associated trait was added to `pallet_beefy::Config`. + - audience: Node Dev + description: | + This PR renames the `submit_report_equivocation_unsigned_extrinsic` in `BeefyApi` to + `submit_report_double_voting_unsigned_extrinsic`and bumps the `BeefyApi` version from 3 to 4. + +crates: + - name: pallet-beefy + bump: major + - name: pallet-beefy-mmr + bump: minor + - name: pallet-mmr + bump: major + - name: sc-consensus-beefy + bump: patch + - name: kitchensink-runtime + bump: major + - name: rococo-runtime + bump: major + - name: westend-runtime + bump: major + - name: sp-consensus-beefy + bump: major + - name: polkadot-service + bump: patch diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 839e157ae7645..fc87fea57ba2e 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -2548,6 +2548,7 @@ impl pallet_beefy::Config for Runtime { type MaxNominators = ConstU32<0>; type MaxSetIdSessionEntries = BeefySetIdSessionEntries; type OnNewValidatorSet = MmrLeaf; + type AncestryHelper = MmrLeaf; type WeightInfo = (); type KeyOwnerProof = >::Proof; type EquivocationReportSystem = @@ -3032,7 +3033,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() @@ -3042,7 +3043,7 @@ impl_runtime_apis! { Beefy::validator_set() } - fn submit_report_equivocation_unsigned_extrinsic( + fn submit_report_double_voting_unsigned_extrinsic( equivocation_proof: sp_consensus_beefy::DoubleVotingProof< BlockNumber, BeefyId, @@ -3052,7 +3053,7 @@ impl_runtime_apis! { ) -> Option<()> { let key_owner_proof = key_owner_proof.decode()?; - Beefy::submit_unsigned_equivocation_report( + Beefy::submit_unsigned_double_voting_report( equivocation_proof, key_owner_proof, ) diff --git a/substrate/client/consensus/beefy/src/fisherman.rs b/substrate/client/consensus/beefy/src/fisherman.rs index 073fee0bdbdbe..faa4d34eff5ac 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(()); } @@ -155,7 +155,7 @@ where for ProvedValidator { key_owner_proof, .. } in key_owner_proofs { self.runtime .runtime_api() - .submit_report_equivocation_unsigned_extrinsic( + .submit_report_double_voting_unsigned_extrinsic( best_block_hash, proof.clone(), key_owner_proof, diff --git a/substrate/client/consensus/beefy/src/tests.rs b/substrate/client/consensus/beefy/src/tests.rs index 681e11a0c5310..d8f5b39dbbaaa 100644 --- a/substrate/client/consensus/beefy/src/tests.rs +++ b/substrate/client/consensus/beefy/src/tests.rs @@ -314,7 +314,7 @@ sp_api::mock_impl_runtime_apis! { self.inner.validator_set.clone() } - fn submit_report_equivocation_unsigned_extrinsic( + fn submit_report_double_voting_unsigned_extrinsic( proof: DoubleVotingProof, AuthorityId, Signature>, _dummy: OpaqueKeyOwnershipProof, ) -> Option<()> { diff --git a/substrate/client/consensus/beefy/src/worker.rs b/substrate/client/consensus/beefy/src/worker.rs index 3ce4da7ecd56a..4a9f7a2d0e3b0 100644 --- a/substrate/client/consensus/beefy/src/worker.rs +++ b/substrate/client/consensus/beefy/src/worker.rs @@ -1039,7 +1039,7 @@ pub(crate) mod tests { ecdsa_crypto, known_payloads, known_payloads::MMR_ROOT_ID, mmr::MmrRootProvider, - test_utils::{generate_equivocation_proof, Keyring}, + test_utils::{generate_double_voting_proof, Keyring}, ConsensusLog, Payload, SignedCommitment, }; use sp_runtime::traits::{Header as HeaderT, One}; @@ -1586,7 +1586,7 @@ pub(crate) mod tests { let payload2 = Payload::from_single_entry(MMR_ROOT_ID, vec![128]); // generate an equivocation proof, with Bob as perpetrator - let good_proof = generate_equivocation_proof( + let good_proof = generate_double_voting_proof( (block_num, payload1.clone(), set_id, &Keyring::Bob), (block_num, payload2.clone(), set_id, &Keyring::Bob), ); @@ -1618,7 +1618,7 @@ pub(crate) mod tests { assert!(api_alice.reported_equivocations.as_ref().unwrap().lock().is_empty()); // now let's try reporting a self-equivocation - let self_proof = generate_equivocation_proof( + let self_proof = generate_double_voting_proof( (block_num, payload1.clone(), set_id, &Keyring::Alice), (block_num, payload2.clone(), set_id, &Keyring::Alice), ); diff --git a/substrate/frame/beefy-mmr/src/lib.rs b/substrate/frame/beefy-mmr/src/lib.rs index e423f1b342f2f..18ebc9d8f38a7 100644 --- a/substrate/frame/beefy-mmr/src/lib.rs +++ b/substrate/frame/beefy-mmr/src/lib.rs @@ -33,20 +33,22 @@ //! //! and thanks to versioning can be easily updated in the future. -use sp_runtime::traits::{Convert, Member}; +use sp_runtime::traits::{Convert, Header, Member}; use sp_std::prelude::*; use codec::Decode; -use pallet_mmr::{LeafDataProvider, ParentNumberAndHash}; +use pallet_mmr::{primitives::AncestryProof, LeafDataProvider, ParentNumberAndHash}; use sp_consensus_beefy::{ + known_payloads, mmr::{BeefyAuthoritySet, BeefyDataProvider, BeefyNextAuthoritySet, MmrLeaf, MmrLeafVersion}, - ValidatorSet as BeefyValidatorSet, + AncestryHelper, Commitment, ConsensusLog, ValidatorSet as BeefyValidatorSet, }; use frame_support::{crypto::ecdsa::ECDSAExt, traits::Get}; -use frame_system::pallet_prelude::BlockNumberFor; +use frame_system::pallet_prelude::{BlockNumberFor, HeaderFor}; pub use pallet::*; +use sp_runtime::generic::OpaqueDigestItemId; #[cfg(test)] mod mock; @@ -172,6 +174,75 @@ where } } +impl 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, + 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, + Err(_) => { + // We can't prove that the commitment is non-canonical if the + // `commitment.block_number` is invalid. + return false + }, + }; + if commitment_leaf_count != proof.prev_leaf_count { + // Can't prove that the commitment is non-canonical if the `commitment.block_number` + // doesn't match the ancestry proof. + return false; + } + + let canonical_mmr_root = context; + let canonical_prev_root = + match pallet_mmr::Pallet::::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 d59c219d3e71e..0521bdabbe495 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 fac799bf64e43..f99835a1dc0a5 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}; @@ -31,8 +35,9 @@ use frame_support::traits::OnInitialize; use crate::mock::*; -fn init_block(block: u64) { - System::set_block_number(block); +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); @@ -61,38 +66,32 @@ 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![ 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); - + init_block(2, None); 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 + ]))), ] ); }); @@ -106,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() }); @@ -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, @@ -131,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() }); @@ -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, @@ -175,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(); @@ -191,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,3 +206,176 @@ fn should_update_authorities() { assert_eq!(want, next_auth_set.keyset_commitment); }); } + +#[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]); + + let mut prev_roots = vec![]; + ext.execute_with(|| { + for block_num in 1..=500 { + init_block(block_num, None); + 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(), + Mmr::mmr_root(), + ), + 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(), + Mmr::mmr_root(), + ), + 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, + Mmr::mmr_root(), + ), + 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, + Mmr::mmr_root(), + ), + 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(), + Mmr::mmr_root(), + ), + 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, + Mmr::mmr_root(), + ), + true + ) + } + }); +} diff --git a/substrate/frame/beefy/src/default_weights.rs b/substrate/frame/beefy/src/default_weights.rs index 8042f0c932eb6..70dd3bb02bf1e 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_equivocation(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( @@ -50,6 +57,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 aecc9e721d5c4..a1526e7811111 100644 --- a/substrate/frame/beefy/src/equivocation.rs +++ b/substrate/frame/beefy/src/equivocation.rs @@ -36,9 +36,12 @@ 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::{DoubleVotingProof, ValidatorSetId, KEY_TYPE as BEEFY_KEY_TYPE}; +use sp_consensus_beefy::{ + check_commitment_signature, AncestryHelper, DoubleVotingProof, ForkVotingProof, + FutureBlockVotingProof, ValidatorSetId, KEY_TYPE as BEEFY_KEY_TYPE, +}; use sp_runtime::{ transaction_validity::{ InvalidTransaction, TransactionPriority, TransactionSource, TransactionValidity, @@ -118,18 +121,143 @@ 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< + HeaderFor, + T::BeefyId, + >>::Proof, + >, + T::KeyOwnerProof, + ), + FutureBlockVotingProof(FutureBlockVotingProof, T::BeefyId>, 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, + EquivocationEvidenceFor::FutureBlockVotingProof(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, + EquivocationEvidenceFor::FutureBlockVotingProof(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, + EquivocationEvidenceFor::FutureBlockVotingProof(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, + EquivocationEvidenceFor::FutureBlockVotingProof(_, 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_double_voting_proof(&equivocation_proof) { + return Err(Error::::InvalidDoubleVotingProof); + } + + return Ok(()) + }, + EquivocationEvidenceFor::ForkVotingProof(equivocation_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( + &vote.commitment, + ancestry_proof, + validation_context, + ); + 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(()) + }, + 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(()) + }, + } + } +} impl OffenceReportSystem, EquivocationEvidenceFor> for EquivocationReportSystem @@ -148,13 +276,8 @@ where fn publish_evidence(evidence: EquivocationEvidenceFor) -> Result<(), ()> { use frame_system::offchain::SubmitTransaction; - let (equivocation_proof, key_owner_proof) = evidence; - - let call = Call::report_equivocation_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 +289,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 +304,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 +344,37 @@ 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 { - // 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_equivocation_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 63f3e9bb309c6..a49f5d28f455a 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::{ @@ -41,8 +41,9 @@ 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, + ForkVotingProof, FutureBlockVotingProof, OnNewValidatorSet, ValidatorSet, BEEFY_ENGINE_ID, + GENESIS_AUTHORITY_SET_ID, }; mod default_weights; @@ -98,6 +99,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; @@ -188,8 +192,14 @@ 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, + /// 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. DuplicateOffenceReport, /// Submitted configuration is invalid. @@ -203,11 +213,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< @@ -222,7 +232,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()) @@ -238,11 +248,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< @@ -257,7 +267,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()) } @@ -278,6 +288,126 @@ 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< + HeaderFor, + 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< + HeaderFor, + 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()) + } + + /// 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] @@ -300,6 +430,48 @@ 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, + }, + EquivocationEvidenceFor::FutureBlockVotingProof( + equivocation_proof, + key_owner_proof, + ) => Call::report_future_block_voting_unsigned { + equivocation_proof: Box::new(equivocation_proof), + key_owner_proof, + }, + } + } + } } #[cfg(any(feature = "try-runtime", test))] @@ -367,7 +539,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, @@ -375,7 +547,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( @@ -526,6 +702,20 @@ impl IsMember for Pallet { } pub trait WeightInfo { - fn report_equivocation(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/mock.rs b/substrate/frame/beefy/src/mock.rs index 35bf172d60632..03efccff76430 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::{ @@ -28,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; @@ -37,6 +43,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 { @@ -75,11 +82,46 @@ 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(); + + pub storage AncestryProofContext: Option = Some( + MockAncestryProofContext { + is_valid: true, + } + ); +} + +pub struct 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, + context: Self::ValidationContext, + ) -> bool { + context.is_valid && proof.is_non_canonical + } } impl pallet_beefy::Config for Test { @@ -88,6 +130,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/beefy/src/tests.rs b/substrate/frame/beefy/src/tests.rs index 6a6aa245ce1f9..a63b3532b6983 100644 --- a/substrate/frame/beefy/src/tests.rs +++ b/substrate/frame/beefy/src/tests.rs @@ -20,18 +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_double_voting_proof, ecdsa_crypto, known_payloads::MMR_ROOT_ID, - test_utils::{generate_equivocation_proof, Keyring as BeefyKeyring}, - Payload, ValidatorSet, KEY_TYPE as BEEFY_KEY_TYPE, + test_utils::{ + generate_double_voting_proof, generate_fork_voting_proof, + generate_future_block_voting_proof, Keyring as BeefyKeyring, + }, + 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); @@ -222,51 +226,90 @@ 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), ); // 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 - 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), ); // 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_equivocation_proof( + let equivocation_proof = generate_double_voting_proof( (1, payload1.clone(), set_id, &BeefyKeyring::Alice), (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_equivocation_proof( + let equivocation_proof = generate_double_voting_proof( (1, payload1.clone(), set_id, &BeefyKeyring::Bob), (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 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), ); // expect valid equivocation proof - assert!(check_equivocation_proof::<_, _, Keccak256>(&equivocation_proof)); + assert!(check_double_voting_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(|| { @@ -297,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_equivocation_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_equivocation_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); @@ -345,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(|| { @@ -384,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_equivocation_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_equivocation_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); @@ -428,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(|| { @@ -446,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_equivocation_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_equivocation_unsigned( - RuntimeOrigin::none(), - Box::new(equivocation_proof), - key_owner_proof, - ), - Error::::InvalidEquivocationProof, + 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(|| { @@ -488,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_equivocation_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_equivocation_unsigned( - RuntimeOrigin::none(), - Box::new(equivocation_proof), - key_owner_proof, - ), - Error::::InvalidEquivocationProof, + 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(|| { @@ -532,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_equivocation_proof( - (block_num, payload1, set_id + 1, &equivocation_keyring), - (block_num, payload2, set_id + 1, &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); @@ -547,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_equivocation_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_equivocation_invalid_equivocation_proof() { +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_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(|| { @@ -578,12 +623,12 @@ 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(), ), - Error::::InvalidEquivocationProof, + Error::::InvalidDoubleVotingProof, ); }; @@ -594,31 +639,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), )); @@ -626,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, @@ -649,14 +694,14 @@ 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), ); 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 +736,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, @@ -716,11 +761,11 @@ 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) - .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,20 +773,75 @@ 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())); } #[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(); @@ -750,56 +850,224 @@ 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_equivocation_proof( - (block_num, payload1, set_id, &equivocation_keyring), - (block_num, payload2, set_id, &equivocation_keyring), + // 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 }, + System::finalize(), + ); + assert_err!( + Beefy::report_fork_voting_unsigned( + RuntimeOrigin::none(), + Box::new(equivocation_proof), + key_owner_proof.clone(), + ), + Error::::InvalidKeyOwnershipProof, ); - // create the key ownership proof. - let key_owner_proof = Historical::prove((BEEFY_KEY_TYPE, &equivocation_key)).unwrap(); + // 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, + ); + }); +} - // check the dispatch info for the call. - let info = Call::::report_equivocation_unsigned { - equivocation_proof: Box::new(equivocation_proof.clone()), - key_owner_proof: key_owner_proof.clone(), +#[test] +fn report_fork_voting_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![] }, + ); } - .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); + 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(); - // report the equivocation. - let post_info = Beefy::report_equivocation_unsigned( - RuntimeOrigin::none(), - Box::new(equivocation_proof.clone()), - key_owner_proof.clone(), - ) - .unwrap(); + let payload = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); - // 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); + // 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(), + ); - // report the equivocation again which is invalid now since it is - // duplicate. - let post_info = Beefy::report_equivocation_unsigned( - RuntimeOrigin::none(), - Box::new(equivocation_proof), - key_owner_proof, - ) - .err() - .unwrap() - .post_info; + // create the key ownership proof + let key_owner_proof = Historical::prove((BEEFY_KEY_TYPE, &equivocation_key)).unwrap(); - // 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); - }) + // 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 valid_fork_voting_reports_dont_pay_fees() { + valid_equivocation_reports_dont_pay_fees(report_fork_voting) +} + +// 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_future_block_voting_current_set_works() { + report_equivocation_current_set_works(report_future_block_voting); +} + +#[test] +fn report_future_block_voting_old_set_works() { + report_equivocation_old_set_works(report_future_block_voting); +} + +#[test] +fn report_future_block_voting_invalid_set_id() { + report_equivocation_invalid_set_id(report_future_block_voting); +} + +#[test] +fn report_future_block_voting_invalid_session() { + report_equivocation_invalid_session(report_future_block_voting); +} + +#[test] +fn report_future_block_voting_invalid_key_owner_proof() { + report_equivocation_invalid_key_owner_proof(report_future_block_voting); +} + +#[test] +fn report_future_block_voting_invalid_equivocation_proof() { + let authorities = test_authorities(); + + ExtBuilder::default().add_authorities(authorities).build_and_execute(|| { + start_era(1); + + 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(); + + start_era(2); + + let payload = Payload::from_single_entry(MMR_ROOT_ID, vec![42]); + + // vote targeting old block + assert_err!( + Beefy::report_future_block_voting_unsigned( + RuntimeOrigin::none(), + Box::new(generate_future_block_voting_proof(( + 1, + payload.clone(), + set_id, + &equivocation_keyring, + ))), + key_owner_proof.clone(), + ), + Error::::InvalidFutureBlockVotingProof, + ); + }); +} + +#[test] +fn valid_future_block_voting_reports_dont_pay_fees() { + valid_equivocation_reports_dont_pay_fees(report_future_block_voting) } #[test] diff --git a/substrate/frame/merkle-mountain-range/src/lib.rs b/substrate/frame/merkle-mountain-range/src/lib.rs index a86443f2e0114..47a325db605d8 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 5efc172d1e93f..8a99f4d87deb0 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 93fefe910e45d..5b73f53506e92 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 6848b8f1b9906..e27440be35c45 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 f8cfcb4e2c286..b8c9d54db8209 100644 --- a/substrate/frame/merkle-mountain-range/src/tests.rs +++ b/substrate/frame/merkle-mountain-range/src/tests.rs @@ -792,16 +792,28 @@ fn does_not_panic_when_generating_historical_proofs() { 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)); + + 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 1..501 { - let proof = Pallet::::generate_ancestry_proof(prev_block_number, None).unwrap(); - Pallet::::verify_ancestry_proof(proof).unwrap(); + 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. diff --git a/substrate/primitives/consensus/beefy/src/lib.rs b/substrate/primitives/consensus/beefy/src/lib.rs index 913184402aef7..7f6f733d0e39a 100644 --- a/substrate/primitives/consensus/beefy/src/lib.rs +++ b/substrate/primitives/consensus/beefy/src/lib.rs @@ -53,7 +53,7 @@ use scale_info::TypeInfo; use sp_application_crypto::{AppPublic, RuntimeAppPublic}; use sp_core::H256; use sp_runtime::{ - traits::{Hash, Keccak256, NumberFor}, + traits::{Hash, Header as HeaderT, Keccak256, NumberFor}, OpaqueValue, }; @@ -307,8 +307,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 { @@ -333,6 +335,27 @@ 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, + /// The header of the block where the ancestry proof was generated + 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( @@ -351,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 @@ -398,6 +421,25 @@ 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: 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, + context: Self::ValidationContext, + ) -> 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 @@ -408,7 +450,7 @@ pub type OpaqueKeyOwnershipProof = OpaqueValue; sp_api::decl_runtime_apis! { /// API necessary for BEEFY voters. - #[api_version(3)] + #[api_version(4)] pub trait BeefyApi where AuthorityId : Codec + RuntimeAppPublic, { @@ -418,15 +460,15 @@ 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 /// `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, diff --git a/substrate/primitives/consensus/beefy/src/payload.rs b/substrate/primitives/consensus/beefy/src/payload.rs index 1a06e620e7ad4..d22255c384bc2 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()) } diff --git a/substrate/primitives/consensus/beefy/src/test_utils.rs b/substrate/primitives/consensus/beefy/src/test_utils.rs index d7fd49214f12f..bd335ede48938 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, FutureBlockVotingProof, 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, Header as HeaderT}; use codec::Encode; use std::{collections::HashMap, marker::PhantomData}; @@ -136,20 +136,42 @@ 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 +pub 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, AncestryProof>( + vote: (u64, Payload, ValidatorSetId, &Keyring), + ancestry_proof: AncestryProof, + header: Header, +) -> ForkVotingProof { + 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 } +}