From f61b0de6f99f8ab482897d2877573a01f17d347e Mon Sep 17 00:00:00 2001 From: acatangiu Date: Mon, 4 Apr 2022 14:27:54 +0300 Subject: [PATCH 01/11] beefy-gadget: allow custom runtime api provider --- client/beefy/src/lib.rs | 18 ++++++++++------ client/beefy/src/tests.rs | 34 +++++++++++++++++++++++++++--- client/beefy/src/worker.rs | 42 ++++++++++++++------------------------ 3 files changed, 58 insertions(+), 36 deletions(-) diff --git a/client/beefy/src/lib.rs b/client/beefy/src/lib.rs index 8a6e175f58321..290f63a8cea5e 100644 --- a/client/beefy/src/lib.rs +++ b/client/beefy/src/lib.rs @@ -87,7 +87,7 @@ pub fn beefy_peers_set_config( /// of today, Rust does not allow a type alias to be used as a trait bound. Tracking /// issue is . pub trait Client: - BlockchainEvents + HeaderBackend + Finalizer + ProvideRuntimeApi + Send + Sync + BlockchainEvents + HeaderBackend + Finalizer + Send + Sync where B: Block, BE: Backend, @@ -110,18 +110,21 @@ where } /// BEEFY gadget initialization parameters. -pub struct BeefyParams +pub struct BeefyParams where B: Block, BE: Backend, C: Client, - C::Api: BeefyApi, + R: ProvideRuntimeApi, + R::Api: BeefyApi, N: GossipNetwork + Clone + SyncOracle + Send + Sync + 'static, { /// BEEFY client pub client: Arc, /// Client Backend pub backend: Arc, + /// Runtime Api Provider + pub runtime: Arc, /// Local key store pub key_store: Option, /// Gossip network @@ -142,17 +145,19 @@ where /// Start the BEEFY gadget. /// /// This is a thin shim around running and awaiting a BEEFY worker. -pub async fn start_beefy_gadget(beefy_params: BeefyParams) +pub async fn start_beefy_gadget(beefy_params: BeefyParams) where B: Block, BE: Backend, C: Client, - C::Api: BeefyApi, + R: ProvideRuntimeApi, + R::Api: BeefyApi, N: GossipNetwork + Clone + SyncOracle + Send + Sync + 'static, { let BeefyParams { client, backend, + runtime, key_store, network, signed_commitment_sender, @@ -188,6 +193,7 @@ where let worker_params = worker::WorkerParams { client, backend, + runtime, key_store: key_store.into(), signed_commitment_sender, beefy_best_block_sender, @@ -198,7 +204,7 @@ where sync_oracle, }; - let worker = worker::BeefyWorker::<_, _, _, _>::new(worker_params); + let worker = worker::BeefyWorker::<_, _, _, _, _>::new(worker_params); worker.run().await } diff --git a/client/beefy/src/tests.rs b/client/beefy/src/tests.rs index 92b5ad91c11e1..fabed2a0d84ed 100644 --- a/client/beefy/src/tests.rs +++ b/client/beefy/src/tests.rs @@ -37,9 +37,10 @@ use sc_network_test::{ use sc_utils::notification::NotificationReceiver; use beefy_primitives::{ - crypto::AuthorityId, ConsensusLog, MmrRootHash, ValidatorSet, BEEFY_ENGINE_ID, + crypto::AuthorityId, BeefyApi, ConsensusLog, MmrRootHash, ValidatorSet, BEEFY_ENGINE_ID, KEY_TYPE as BeefyKeyType, }; +use sp_api::{ApiRef, ProvideRuntimeApi}; use sp_consensus::BlockOrigin; use sp_core::H256; use sp_keystore::{SyncCryptoStore, SyncCryptoStorePtr}; @@ -248,11 +249,36 @@ pub(crate) fn create_beefy_keystore(authority: BeefyKeyring) -> SyncCryptoStoreP keystore } +#[derive(Clone)] +pub(crate) struct TestApi {} + +// compiler gets confused and warns us about unused inner +#[allow(dead_code)] +pub(crate) struct RuntimeApi { + inner: TestApi, +} + +impl ProvideRuntimeApi for TestApi { + type Api = RuntimeApi; + + fn runtime_api<'a>(&'a self) -> ApiRef<'a, Self::Api> { + RuntimeApi { inner: self.clone() }.into() + } +} + +sp_api::mock_impl_runtime_apis! { + impl BeefyApi for RuntimeApi { + fn validator_set() -> Option { + BeefyValidatorSet::new(make_beefy_ids(&[BeefyKeyring::Alice, BeefyKeyring::Bob, BeefyKeyring::Charlie]), 0) + } + } +} + pub(crate) fn create_beefy_worker( peer: &BeefyPeer, key: &BeefyKeyring, min_block_delta: u32, -) -> BeefyWorker>> { +) -> BeefyWorker>> { let keystore = create_beefy_keystore(*key); let (signed_commitment_sender, signed_commitment_stream) = @@ -263,6 +289,7 @@ pub(crate) fn create_beefy_worker( let beefy_link_half = BeefyLinkHalf { signed_commitment_stream, beefy_best_block_stream }; *peer.data.beefy_link_half.lock() = Some(beefy_link_half); let test_modifiers = peer.data.test_modifiers.clone().unwrap(); + let api = Arc::new(TestApi {}); let network = peer.network_service().clone(); let sync_oracle = network.clone(); @@ -272,6 +299,7 @@ pub(crate) fn create_beefy_worker( let worker_params = crate::worker::WorkerParams { client: peer.client().as_client(), backend: peer.client().as_backend(), + runtime: api, key_store: Some(keystore).into(), signed_commitment_sender, beefy_best_block_sender, @@ -282,7 +310,7 @@ pub(crate) fn create_beefy_worker( sync_oracle, }; - BeefyWorker::<_, _, _, _>::new(worker_params, test_modifiers) + BeefyWorker::<_, _, _, _, _>::new(worker_params, test_modifiers) } // Spawns beefy voters. Returns a future to spawn on the runtime. diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index 85674c09a278b..d428fdc3fe135 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -26,7 +26,7 @@ use parking_lot::Mutex; use sc_client_api::{Backend, FinalityNotification, FinalityNotifications}; use sc_network_gossip::GossipEngine; -use sp_api::BlockId; +use sp_api::{BlockId, ProvideRuntimeApi}; use sp_arithmetic::traits::AtLeast32Bit; use sp_consensus::SyncOracle; use sp_runtime::{ @@ -52,12 +52,10 @@ use crate::{ Client, }; -pub(crate) struct WorkerParams -where - B: Block, -{ +pub(crate) struct WorkerParams { pub client: Arc, pub backend: Arc, + pub runtime: Arc, pub key_store: BeefyKeystore, pub signed_commitment_sender: BeefySignedCommitmentSender, pub beefy_best_block_sender: BeefyBestBlockSender, @@ -69,15 +67,10 @@ where } /// A BEEFY worker plays the BEEFY protocol -pub(crate) struct BeefyWorker -where - B: Block, - BE: Backend, - C: Client, - SO: SyncOracle + Send + Sync + Clone + 'static, -{ +pub(crate) struct BeefyWorker { client: Arc, backend: Arc, + runtime: Arc, key_store: BeefyKeystore, signed_commitment_sender: BeefySignedCommitmentSender, gossip_engine: Arc>>, @@ -104,12 +97,13 @@ where test_res: tests::TestModifiers, } -impl BeefyWorker +impl BeefyWorker where B: Block + Codec, BE: Backend, C: Client, - C::Api: BeefyApi, + R: ProvideRuntimeApi, + R::Api: BeefyApi, SO: SyncOracle + Send + Sync + Clone + 'static, { /// Return a new BEEFY worker instance. @@ -119,7 +113,7 @@ where /// /// The BEEFY pallet is needed in order to keep track of the BEEFY authority set. pub(crate) fn new( - worker_params: WorkerParams, + worker_params: WorkerParams, #[cfg(test)] // behavior modifiers used in tests test_res: tests::TestModifiers, @@ -127,6 +121,7 @@ where let WorkerParams { client, backend, + runtime, key_store, signed_commitment_sender, beefy_best_block_sender, @@ -144,6 +139,7 @@ where BeefyWorker { client: client.clone(), backend, + runtime, key_store, signed_commitment_sender, gossip_engine: Arc::new(Mutex::new(gossip_engine)), @@ -163,16 +159,7 @@ where test_res, } } -} -impl BeefyWorker -where - B: Block, - BE: Backend, - C: Client, - C::Api: BeefyApi, - SO: SyncOracle + Send + Sync + Clone + 'static, -{ /// Return `Some(number)` if we should be voting on block `number` now, /// return `None` if there is no block we should vote on now. fn current_vote_target(&self) -> Option> { @@ -464,7 +451,7 @@ where .finality_notification_stream() .take_while(|notif| { let at = BlockId::hash(notif.header.hash()); - if let Some(active) = self.client.runtime_api().validator_set(&at).ok().flatten() { + if let Some(active) = self.runtime.runtime_api().validator_set(&at).ok().flatten() { if active.id() == GENESIS_AUTHORITY_SET_ID { // When starting from genesis, there is no session boundary digest. // Just initialize `rounds` to Block #1 as BEEFY mandatory block. @@ -658,7 +645,7 @@ pub(crate) mod tests { use super::*; use crate::{ keystore::tests::Keyring, - tests::{create_beefy_worker, get_beefy_streams, make_beefy_ids, BeefyTestNet}, + tests::{create_beefy_worker, get_beefy_streams, make_beefy_ids, BeefyTestNet, TestApi}, }; use futures::{executor::block_on, future::poll_fn, task::Poll}; @@ -833,8 +820,9 @@ pub(crate) mod tests { let set_up = |worker: &mut BeefyWorker< Block, - PeersFullClient, Backend, + PeersFullClient, + TestApi, Arc>, >, best_grandpa: u64, From 4d4566899cb58e056fdb5112388a856ba11817d8 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Mon, 4 Apr 2022 16:28:45 +0300 Subject: [PATCH 02/11] beefy-gadget: use mock runtime api in tests --- client/beefy/src/lib.rs | 15 ++- client/beefy/src/metrics.rs | 5 +- client/beefy/src/tests.rs | 205 +++++++++++++++++------------------- client/beefy/src/worker.rs | 70 ++++++++---- 4 files changed, 156 insertions(+), 139 deletions(-) diff --git a/client/beefy/src/lib.rs b/client/beefy/src/lib.rs index 290f63a8cea5e..3dd1f4ad2f87e 100644 --- a/client/beefy/src/lib.rs +++ b/client/beefy/src/lib.rs @@ -141,12 +141,15 @@ where pub protocol_name: std::borrow::Cow<'static, str>, } -#[cfg(not(test))] /// Start the BEEFY gadget. /// /// This is a thin shim around running and awaiting a BEEFY worker. -pub async fn start_beefy_gadget(beefy_params: BeefyParams) -where +pub async fn start_beefy_gadget( + beefy_params: BeefyParams, + #[cfg(test)] + // TODO: temporary, remove this + test_res: worker::TestModifiers, +) where B: Block, BE: Backend, C: Client, @@ -204,7 +207,11 @@ where sync_oracle, }; - let worker = worker::BeefyWorker::<_, _, _, _, _>::new(worker_params); + let worker = worker::BeefyWorker::<_, _, _, _, _>::new( + worker_params, + #[cfg(test)] + test_res, + ); worker.run().await } diff --git a/client/beefy/src/metrics.rs b/client/beefy/src/metrics.rs index 20fa98e52fdd5..a6d29dbf88abb 100644 --- a/client/beefy/src/metrics.rs +++ b/client/beefy/src/metrics.rs @@ -18,9 +18,7 @@ //! BEEFY Prometheus metrics definition -#[cfg(not(test))] -use prometheus::{register, PrometheusError, Registry}; -use prometheus::{Counter, Gauge, U64}; +use prometheus::{register, Counter, Gauge, PrometheusError, Registry, U64}; /// BEEFY metrics exposed through Prometheus pub(crate) struct Metrics { @@ -39,7 +37,6 @@ pub(crate) struct Metrics { } impl Metrics { - #[cfg(not(test))] pub(crate) fn register(registry: &Registry) -> Result { Ok(Self { beefy_validator_set_id: register( diff --git a/client/beefy/src/tests.rs b/client/beefy/src/tests.rs index fabed2a0d84ed..7f811b0534327 100644 --- a/client/beefy/src/tests.rs +++ b/client/beefy/src/tests.rs @@ -28,11 +28,10 @@ use sc_chain_spec::{ChainSpec, GenericChainSpec}; use sc_client_api::HeaderBackend; use sc_consensus::BoxJustificationImport; use sc_keystore::LocalKeystore; -use sc_network::{config::ProtocolConfig, NetworkService}; -use sc_network_gossip::GossipEngine; +use sc_network::config::ProtocolConfig; use sc_network_test::{ Block, BlockImportAdapter, FullPeerConfig, PassThroughVerifier, Peer, PeersClient, - PeersFullClient, TestNetFactory, + TestNetFactory, }; use sc_utils::notification::NotificationReceiver; @@ -48,19 +47,17 @@ use sp_runtime::{ codec::Encode, generic::BlockId, traits::Header as HeaderT, BuildStorage, DigestItem, Storage, }; -use substrate_test_runtime_client::{runtime::Header, Backend, ClientExt}; +use substrate_test_runtime_client::{runtime::Header, ClientExt}; use crate::{ - beefy_protocol_name, - keystore::tests::Keyring as BeefyKeyring, - notification::*, - worker::{tests::TestModifiers, BeefyWorker}, + beefy_protocol_name, keystore::tests::Keyring as BeefyKeyring, notification::*, + worker::TestModifiers, }; -const BEEFY_PROTOCOL_NAME: &'static str = "/beefy/1"; +pub(crate) const BEEFY_PROTOCOL_NAME: &'static str = "/beefy/1"; -type BeefyValidatorSet = ValidatorSet; -type BeefyPeer = Peer; +pub(crate) type BeefyValidatorSet = ValidatorSet; +pub(crate) type BeefyPeer = Peer; #[derive(Debug, Serialize, Deserialize)] struct Genesis(std::collections::BTreeMap); @@ -102,27 +99,14 @@ fn beefy_protocol_name() { #[allow(dead_code)] #[derive(Clone)] pub(crate) struct BeefyLinkHalf { - signed_commitment_stream: BeefySignedCommitmentStream, - beefy_best_block_stream: BeefyBestBlockStream, + pub signed_commitment_stream: BeefySignedCommitmentStream, + pub beefy_best_block_stream: BeefyBestBlockStream, } #[derive(Default)] pub(crate) struct PeerData { pub(crate) beefy_link_half: Mutex>, - pub(crate) test_modifiers: Option, -} - -impl PeerData { - pub(crate) fn use_validator_set(&mut self, validator_set: &ValidatorSet) { - if let Some(tm) = self.test_modifiers.as_mut() { - tm.active_validators = validator_set.clone(); - } else { - self.test_modifiers = Some(TestModifiers { - active_validators: validator_set.clone(), - corrupt_mmr_roots: false, - }); - } - } + pub(crate) test_modifiers: TestModifiers, } pub(crate) struct BeefyTestNet { @@ -224,6 +208,46 @@ impl TestNetFactory for BeefyTestNet { } } +macro_rules! create_test_api { + ( $api_name:ident, $($inits:expr),+ ) => { + pub(crate) mod $api_name { + use super::*; + + #[derive(Clone)] + pub(crate) struct TestApi {} + + // compiler gets confused and warns us about unused inner + #[allow(dead_code)] + pub(crate) struct RuntimeApi { + inner: TestApi, + } + + impl ProvideRuntimeApi for TestApi { + type Api = RuntimeApi; + fn runtime_api<'a>(&'a self) -> ApiRef<'a, Self::Api> { + RuntimeApi { inner: self.clone() }.into() + } + } + sp_api::mock_impl_runtime_apis! { + impl BeefyApi for RuntimeApi { + fn validator_set() -> Option { + BeefyValidatorSet::new(make_beefy_ids(&[$($inits),+]), 0) + } + } + } + } + }; +} + +create_test_api!(two_validators, BeefyKeyring::Alice, BeefyKeyring::Bob); +create_test_api!( + four_validators, + BeefyKeyring::Alice, + BeefyKeyring::Bob, + BeefyKeyring::Charlie, + BeefyKeyring::Dave +); + fn add_mmr_digest(header: &mut Header, mmr_hash: MmrRootHash) { header.digest_mut().push(DigestItem::Consensus( BEEFY_ENGINE_ID, @@ -249,81 +273,46 @@ pub(crate) fn create_beefy_keystore(authority: BeefyKeyring) -> SyncCryptoStoreP keystore } -#[derive(Clone)] -pub(crate) struct TestApi {} - -// compiler gets confused and warns us about unused inner -#[allow(dead_code)] -pub(crate) struct RuntimeApi { - inner: TestApi, -} - -impl ProvideRuntimeApi for TestApi { - type Api = RuntimeApi; - - fn runtime_api<'a>(&'a self) -> ApiRef<'a, Self::Api> { - RuntimeApi { inner: self.clone() }.into() - } -} - -sp_api::mock_impl_runtime_apis! { - impl BeefyApi for RuntimeApi { - fn validator_set() -> Option { - BeefyValidatorSet::new(make_beefy_ids(&[BeefyKeyring::Alice, BeefyKeyring::Bob, BeefyKeyring::Charlie]), 0) - } - } -} - -pub(crate) fn create_beefy_worker( - peer: &BeefyPeer, - key: &BeefyKeyring, - min_block_delta: u32, -) -> BeefyWorker>> { - let keystore = create_beefy_keystore(*key); - - let (signed_commitment_sender, signed_commitment_stream) = - BeefySignedCommitmentStream::::channel(); - let (beefy_best_block_sender, beefy_best_block_stream) = - BeefyBestBlockStream::::channel(); - - let beefy_link_half = BeefyLinkHalf { signed_commitment_stream, beefy_best_block_stream }; - *peer.data.beefy_link_half.lock() = Some(beefy_link_half); - let test_modifiers = peer.data.test_modifiers.clone().unwrap(); - let api = Arc::new(TestApi {}); - - let network = peer.network_service().clone(); - let sync_oracle = network.clone(); - let gossip_validator = Arc::new(crate::gossip::GossipValidator::new()); - let gossip_engine = - GossipEngine::new(network, BEEFY_PROTOCOL_NAME, gossip_validator.clone(), None); - let worker_params = crate::worker::WorkerParams { - client: peer.client().as_client(), - backend: peer.client().as_backend(), - runtime: api, - key_store: Some(keystore).into(), - signed_commitment_sender, - beefy_best_block_sender, - gossip_engine, - gossip_validator, - min_block_delta, - metrics: None, - sync_oracle, - }; - - BeefyWorker::<_, _, _, _, _>::new(worker_params, test_modifiers) -} - // Spawns beefy voters. Returns a future to spawn on the runtime. -fn initialize_beefy( +fn initialize_beefy( net: &mut BeefyTestNet, peers: &[BeefyKeyring], + api: Arc, min_block_delta: u32, -) -> impl Future { +) -> impl Future +where + API: ProvideRuntimeApi + Sync + Send, + API::Api: BeefyApi, +{ let voters = FuturesUnordered::new(); for (peer_id, key) in peers.iter().enumerate() { - let worker = create_beefy_worker(&net.peers[peer_id], key, min_block_delta); - let gadget = worker.run(); + let peer = &net.peers[peer_id]; + + let keystore = create_beefy_keystore(*key); + + let (signed_commitment_sender, signed_commitment_stream) = + BeefySignedCommitmentStream::::channel(); + let (beefy_best_block_sender, beefy_best_block_stream) = + BeefyBestBlockStream::::channel(); + let beefy_link_half = BeefyLinkHalf { signed_commitment_stream, beefy_best_block_stream }; + *peer.data.beefy_link_half.lock() = Some(beefy_link_half); + + let test_res = peer.data.test_modifiers.clone(); + + let beefy_params = crate::BeefyParams { + client: peer.client().as_client(), + backend: peer.client().as_backend(), + runtime: api.clone(), + key_store: Some(keystore), + network: peer.network_service().clone(), + signed_commitment_sender, + beefy_best_block_sender, + min_block_delta, + prometheus_registry: None, + protocol_name: BEEFY_PROTOCOL_NAME.into(), + }; + let gadget = crate::start_beefy_gadget::<_, _, _, _, _>(beefy_params, test_res); fn assert_send(_: &T) {} assert_send(&gadget); @@ -471,10 +460,8 @@ fn beefy_finalizing_blocks() { let mut net = BeefyTestNet::new(2, 0); - for i in 0..peers.len() { - net.peer(i).data.use_validator_set(&validator_set); - } - runtime.spawn(initialize_beefy(&mut net, peers, min_block_delta)); + let api = Arc::new(two_validators::TestApi {}); + runtime.spawn(initialize_beefy(&mut net, peers, api, min_block_delta)); // push 42 blocks including `AuthorityChange` digests every 10 blocks. net.generate_blocks(42, session_len, &validator_set); @@ -505,16 +492,14 @@ fn lagging_validators() { sp_tracing::try_init_simple(); let mut runtime = Runtime::new().unwrap(); - let peers = &[BeefyKeyring::Charlie, BeefyKeyring::Dave]; + let peers = &[BeefyKeyring::Alice, BeefyKeyring::Bob]; let validator_set = ValidatorSet::new(make_beefy_ids(peers), 0).unwrap(); let session_len = 30; let min_block_delta = 1; let mut net = BeefyTestNet::new(2, 0); - for i in 0..peers.len() { - net.peer(i).data.use_validator_set(&validator_set); - } - runtime.spawn(initialize_beefy(&mut net, peers, min_block_delta)); + let api = Arc::new(two_validators::TestApi {}); + runtime.spawn(initialize_beefy(&mut net, peers, api, min_block_delta)); // push 42 blocks including `AuthorityChange` digests every 30 blocks. net.generate_blocks(42, session_len, &validator_set); @@ -526,7 +511,7 @@ fn lagging_validators() { // diff-power-of-two rule. finalize_block_and_wait_for_beefy(&net, peers, &mut runtime, &[15], &[1, 9, 13, 14, 15]); - // Charlie finalizes #25, Dave lags behind + // Alice finalizes #25, Bob lags behind let finalize = BlockId::number(25); let (best_blocks, signed_commitments) = get_beefy_streams(&mut *net.lock(), peers); net.lock().peer(0).client().as_client().finalize_block(finalize, None).unwrap(); @@ -535,7 +520,7 @@ fn lagging_validators() { streams_empty_after_timeout(best_blocks, &net, &mut runtime, timeout); streams_empty_after_timeout(signed_commitments, &net, &mut runtime, None); - // Dave catches up and also finalizes #25 + // Bob catches up and also finalizes #25 let (best_blocks, signed_commitments) = get_beefy_streams(&mut *net.lock(), peers); net.lock().peer(1).client().as_client().finalize_block(finalize, None).unwrap(); // expected beefy finalizes block #17 from diff-power-of-two @@ -558,13 +543,11 @@ fn correct_beefy_payload() { let min_block_delta = 2; let mut net = BeefyTestNet::new(4, 0); - for i in 0..peers.len() { - net.peer(i).data.use_validator_set(&validator_set); - } + let api = Arc::new(four_validators::TestApi {}); // Dave will vote on bad mmr roots - net.peer(3).data.test_modifiers.as_mut().map(|tm| tm.corrupt_mmr_roots = true); - runtime.spawn(initialize_beefy(&mut net, peers, min_block_delta)); + net.peer(3).data.test_modifiers.corrupt_mmr_roots = true; + runtime.spawn(initialize_beefy(&mut net, peers, api, min_block_delta)); // push 10 blocks net.generate_blocks(12, session_len, &validator_set); diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index d428fdc3fe135..df53f5d47d33a 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -52,6 +52,12 @@ use crate::{ Client, }; +#[cfg(test)] +#[derive(Clone, Default)] +pub struct TestModifiers { + pub corrupt_mmr_roots: bool, +} + pub(crate) struct WorkerParams { pub client: Arc, pub backend: Arc, @@ -94,7 +100,7 @@ pub(crate) struct BeefyWorker { _backend: PhantomData, #[cfg(test)] // behavior modifiers used in tests - test_res: tests::TestModifiers, + test_res: TestModifiers, } impl BeefyWorker @@ -115,8 +121,8 @@ where pub(crate) fn new( worker_params: WorkerParams, #[cfg(test)] - // behavior modifiers used in tests - test_res: tests::TestModifiers, + // TODO: temporary, remove this + test_res: TestModifiers, ) -> Self { let WorkerParams { client, @@ -445,7 +451,6 @@ where } /// Wait for BEEFY runtime pallet to be available. - #[cfg(not(test))] async fn wait_for_runtime_pallet(&mut self) { self.client .finality_notification_stream() @@ -475,13 +480,6 @@ where self.finality_notifications = self.client.finality_notification_stream(); } - /// For tests don't use runtime pallet. Start rounds from block #1. - #[cfg(test)] - async fn wait_for_runtime_pallet(&mut self) { - let active = self.test_res.active_validators.clone(); - self.init_session_at(active, 1u32.into()); - } - /// Main loop for BEEFY worker. /// /// Wait for BEEFY runtime pallet to be available, then start the main async loop @@ -645,11 +643,16 @@ pub(crate) mod tests { use super::*; use crate::{ keystore::tests::Keyring, - tests::{create_beefy_worker, get_beefy_streams, make_beefy_ids, BeefyTestNet, TestApi}, + notification::{BeefyBestBlockStream, BeefySignedCommitmentStream}, + tests::{ + create_beefy_keystore, get_beefy_streams, make_beefy_ids, two_validators::TestApi, + BeefyPeer, BeefyTestNet, BEEFY_PROTOCOL_NAME, + }, }; use futures::{executor::block_on, future::poll_fn, task::Poll}; + use crate::tests::BeefyLinkHalf; use sc_client_api::HeaderBackend; use sc_network::NetworkService; use sc_network_test::{PeersFullClient, TestNetFactory}; @@ -659,10 +662,41 @@ pub(crate) mod tests { Backend, }; - #[derive(Clone)] - pub struct TestModifiers { - pub active_validators: ValidatorSet, - pub corrupt_mmr_roots: bool, + fn create_beefy_worker( + peer: &BeefyPeer, + key: &Keyring, + min_block_delta: u32, + ) -> BeefyWorker>> { + let keystore = create_beefy_keystore(*key); + + let (signed_commitment_sender, signed_commitment_stream) = + BeefySignedCommitmentStream::::channel(); + let (beefy_best_block_sender, beefy_best_block_stream) = + BeefyBestBlockStream::::channel(); + let beefy_link_half = BeefyLinkHalf { signed_commitment_stream, beefy_best_block_stream }; + *peer.data.beefy_link_half.lock() = Some(beefy_link_half); + + let test_modifiers = peer.data.test_modifiers.clone(); + let api = Arc::new(TestApi {}); + let network = peer.network_service().clone(); + let sync_oracle = network.clone(); + let gossip_validator = Arc::new(crate::gossip::GossipValidator::new()); + let gossip_engine = + GossipEngine::new(network, BEEFY_PROTOCOL_NAME, gossip_validator.clone(), None); + let worker_params = crate::worker::WorkerParams { + client: peer.client().as_client(), + backend: peer.client().as_backend(), + runtime: api, + key_store: Some(keystore).into(), + signed_commitment_sender, + beefy_best_block_sender, + gossip_engine, + gossip_validator, + min_block_delta, + metrics: None, + sync_oracle, + }; + BeefyWorker::<_, _, _, _, _>::new(worker_params, test_modifiers) } #[test] @@ -812,7 +846,6 @@ pub(crate) mod tests { let keys = &[Keyring::Alice]; let validator_set = ValidatorSet::new(make_beefy_ids(keys), 0).unwrap(); let mut net = BeefyTestNet::new(1, 0); - net.peer(0).data.use_validator_set(&validator_set); let mut worker = create_beefy_worker(&net.peer(0), &keys[0], 1); // rounds not initialized -> should vote: `None` @@ -877,7 +910,6 @@ pub(crate) mod tests { let keys = &[Keyring::Alice]; let validator_set = ValidatorSet::new(make_beefy_ids(keys), 0).unwrap(); let mut net = BeefyTestNet::new(1, 0); - net.peer(0).data.use_validator_set(&validator_set); let mut worker = create_beefy_worker(&net.peer(0), &keys[0], 1); // keystore doesn't contain other keys than validators' @@ -901,7 +933,6 @@ pub(crate) mod tests { let keys = &[Keyring::Alice]; let validator_set = ValidatorSet::new(make_beefy_ids(keys), 0).unwrap(); let mut net = BeefyTestNet::new(1, 0); - net.peer(0).data.use_validator_set(&validator_set); let mut worker = create_beefy_worker(&net.peer(0), &keys[0], 1); let (mut best_block_streams, _) = get_beefy_streams(&mut net, keys); @@ -949,7 +980,6 @@ pub(crate) mod tests { let keys = &[Keyring::Alice]; let validator_set = ValidatorSet::new(make_beefy_ids(keys), 0).unwrap(); let mut net = BeefyTestNet::new(1, 0); - net.peer(0).data.use_validator_set(&validator_set); let mut worker = create_beefy_worker(&net.peer(0), &keys[0], 1); assert!(worker.rounds.is_none()); From 7ecaef03573565d07161c3e246f832adff2cb132 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Tue, 5 Apr 2022 13:59:09 +0300 Subject: [PATCH 03/11] pallet-mmr: expose mmr root from state through runtime API --- bin/node/runtime/src/lib.rs | 4 ++++ frame/merkle-mountain-range/primitives/src/lib.rs | 3 +++ frame/merkle-mountain-range/src/lib.rs | 11 ++++++++--- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 565f151ce2a08..1b07581a82096 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1804,6 +1804,10 @@ impl_runtime_apis! { let node = mmr::DataOrHash::Data(leaf.into_opaque_leaf()); pallet_mmr::verify_leaf_proof::(root, node, proof) } + + fn mmr_root() -> Result { + Ok(Mmr::mmr_root()) + } } impl sp_session::SessionKeys for Runtime { diff --git a/frame/merkle-mountain-range/primitives/src/lib.rs b/frame/merkle-mountain-range/primitives/src/lib.rs index cc78dfefefe60..1ac6e533cd0a3 100644 --- a/frame/merkle-mountain-range/primitives/src/lib.rs +++ b/frame/merkle-mountain-range/primitives/src/lib.rs @@ -429,6 +429,9 @@ sp_api::decl_runtime_apis! { /// The leaf data is expected to be encoded in it's compact form. fn verify_proof_stateless(root: Hash, leaf: EncodableOpaqueLeaf, proof: Proof) -> Result<(), Error>; + + /// Return the on-chain MMR root hash. + fn mmr_root() -> Result; } } diff --git a/frame/merkle-mountain-range/src/lib.rs b/frame/merkle-mountain-range/src/lib.rs index f904428e02048..9b3ce44b076bd 100644 --- a/frame/merkle-mountain-range/src/lib.rs +++ b/frame/merkle-mountain-range/src/lib.rs @@ -70,7 +70,7 @@ mod mock; mod tests; pub use pallet::*; -pub use pallet_mmr_primitives::{self as primitives, NodeIndex}; +pub use pallet_mmr_primitives::{self as primitives, LeafIndex, NodeIndex}; pub trait WeightInfo { fn on_initialize(peaks: NodeIndex) -> Weight; @@ -161,7 +161,7 @@ pub mod pallet { /// Current size of the MMR (number of leaves). #[pallet::storage] #[pallet::getter(fn mmr_leaves)] - pub type NumberOfLeaves = StorageValue<_, NodeIndex, ValueQuery>; + pub type NumberOfLeaves = StorageValue<_, LeafIndex, ValueQuery>; /// Hashes of the nodes in the MMR. /// @@ -240,7 +240,7 @@ impl, I: 'static> Pallet { /// all the leaves to be present. /// It may return an error or panic if used incorrectly. pub fn generate_proof( - leaf_index: NodeIndex, + leaf_index: LeafIndex, ) -> Result<(LeafOf, primitives::Proof<>::Hash>), primitives::Error> { let mmr: ModuleMmr = mmr::Mmr::new(Self::mmr_leaves()); mmr.generate_proof(leaf_index) @@ -272,4 +272,9 @@ impl, I: 'static> Pallet { Err(primitives::Error::Verify.log_debug("The proof is incorrect.")) } } + + /// Return the on-chain MMR root hash. + pub fn mmr_root() -> >::Hash { + Self::mmr_root_hash() + } } From bfcdfe5b5b32585c3e2f0dcb2f81956614396b90 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Tue, 5 Apr 2022 16:09:01 +0300 Subject: [PATCH 04/11] beefy-gadget: get mmr root from runtime state --- Cargo.lock | 1 + client/beefy/Cargo.toml | 1 + client/beefy/src/lib.rs | 21 +++----- client/beefy/src/tests.rs | 99 ++++++++++++++++++++++++-------------- client/beefy/src/worker.rs | 46 +++++------------- 5 files changed, 86 insertions(+), 82 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0a1a1e0b17bec..56149944a3584 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -487,6 +487,7 @@ dependencies = [ "futures-timer", "hex", "log 0.4.14", + "pallet-mmr-primitives", "parity-scale-codec", "parking_lot 0.12.0", "sc-chain-spec", diff --git a/client/beefy/Cargo.toml b/client/beefy/Cargo.toml index 02be645b3fc08..3eba8379397ce 100644 --- a/client/beefy/Cargo.toml +++ b/client/beefy/Cargo.toml @@ -38,6 +38,7 @@ sc-network-gossip = { version = "0.10.0-dev", path = "../network-gossip" } sc-utils = { version = "4.0.0-dev", path = "../utils" } beefy-primitives = { version = "4.0.0-dev", path = "../../primitives/beefy" } +pallet-mmr-primitives = { version = "4.0.0-dev", path = "../../frame/merkle-mountain-range/primitives" } [dev-dependencies] sc-consensus = { version = "0.10.0-dev", path = "../consensus/common" } diff --git a/client/beefy/src/lib.rs b/client/beefy/src/lib.rs index 3dd1f4ad2f87e..272ef213038a1 100644 --- a/client/beefy/src/lib.rs +++ b/client/beefy/src/lib.rs @@ -29,7 +29,8 @@ use sp_consensus::SyncOracle; use sp_keystore::SyncCryptoStorePtr; use sp_runtime::traits::Block; -use beefy_primitives::BeefyApi; +use beefy_primitives::{BeefyApi, MmrRootHash}; +use pallet_mmr_primitives::MmrApi; use crate::notification::{BeefyBestBlockSender, BeefySignedCommitmentSender}; @@ -116,7 +117,7 @@ where BE: Backend, C: Client, R: ProvideRuntimeApi, - R::Api: BeefyApi, + R::Api: BeefyApi + MmrApi, N: GossipNetwork + Clone + SyncOracle + Send + Sync + 'static, { /// BEEFY client @@ -144,17 +145,13 @@ where /// Start the BEEFY gadget. /// /// This is a thin shim around running and awaiting a BEEFY worker. -pub async fn start_beefy_gadget( - beefy_params: BeefyParams, - #[cfg(test)] - // TODO: temporary, remove this - test_res: worker::TestModifiers, -) where +pub async fn start_beefy_gadget(beefy_params: BeefyParams) +where B: Block, BE: Backend, C: Client, R: ProvideRuntimeApi, - R::Api: BeefyApi, + R::Api: BeefyApi + MmrApi, N: GossipNetwork + Clone + SyncOracle + Send + Sync + 'static, { let BeefyParams { @@ -207,11 +204,7 @@ pub async fn start_beefy_gadget( sync_oracle, }; - let worker = worker::BeefyWorker::<_, _, _, _, _>::new( - worker_params, - #[cfg(test)] - test_res, - ); + let worker = worker::BeefyWorker::<_, _, _, _, _>::new(worker_params); worker.run().await } diff --git a/client/beefy/src/tests.rs b/client/beefy/src/tests.rs index 7f811b0534327..b45f2365af824 100644 --- a/client/beefy/src/tests.rs +++ b/client/beefy/src/tests.rs @@ -39,6 +39,8 @@ use beefy_primitives::{ crypto::AuthorityId, BeefyApi, ConsensusLog, MmrRootHash, ValidatorSet, BEEFY_ENGINE_ID, KEY_TYPE as BeefyKeyType, }; +use pallet_mmr_primitives::{EncodableOpaqueLeaf, Error as MmrError, LeafIndex, MmrApi, Proof}; + use sp_api::{ApiRef, ProvideRuntimeApi}; use sp_consensus::BlockOrigin; use sp_core::H256; @@ -49,12 +51,11 @@ use sp_runtime::{ use substrate_test_runtime_client::{runtime::Header, ClientExt}; -use crate::{ - beefy_protocol_name, keystore::tests::Keyring as BeefyKeyring, notification::*, - worker::TestModifiers, -}; +use crate::{beefy_protocol_name, keystore::tests::Keyring as BeefyKeyring, notification::*}; pub(crate) const BEEFY_PROTOCOL_NAME: &'static str = "/beefy/1"; +const GOOD_MMR_ROOT: MmrRootHash = MmrRootHash::repeat_byte(0xbf); +const BAD_MMR_ROOT: MmrRootHash = MmrRootHash::repeat_byte(0x42); pub(crate) type BeefyValidatorSet = ValidatorSet; pub(crate) type BeefyPeer = Peer; @@ -106,7 +107,6 @@ pub(crate) struct BeefyLinkHalf { #[derive(Default)] pub(crate) struct PeerData { pub(crate) beefy_link_half: Mutex>, - pub(crate) test_modifiers: TestModifiers, } pub(crate) struct BeefyTestNet { @@ -142,13 +142,7 @@ impl BeefyTestNet { self.peer(0).generate_blocks(count, BlockOrigin::File, |builder| { let mut block = builder.build().unwrap().block; - let block_num = *block.header.number(); - let num_byte = block_num.to_le_bytes().into_iter().next().unwrap(); - let mmr_root = MmrRootHash::repeat_byte(num_byte); - - add_mmr_digest(&mut block.header, mmr_root); - - if block_num % session_length == 0 { + if *block.header.number() % session_length == 0 { add_auth_change_digest(&mut block.header, validator_set.clone()); } @@ -209,11 +203,11 @@ impl TestNetFactory for BeefyTestNet { } macro_rules! create_test_api { - ( $api_name:ident, $($inits:expr),+ ) => { + ( $api_name:ident, mmr_root: $mmr_root:expr, $($inits:expr),+ ) => { pub(crate) mod $api_name { use super::*; - #[derive(Clone)] + #[derive(Clone, Default)] pub(crate) struct TestApi {} // compiler gets confused and warns us about unused inner @@ -234,26 +228,52 @@ macro_rules! create_test_api { BeefyValidatorSet::new(make_beefy_ids(&[$($inits),+]), 0) } } + + impl MmrApi for RuntimeApi { + fn generate_proof(_leaf_index: LeafIndex) + -> Result<(EncodableOpaqueLeaf, Proof), MmrError> { + unimplemented!() + } + + fn verify_proof(_leaf: EncodableOpaqueLeaf, _proof: Proof) + -> Result<(), MmrError> { + unimplemented!() + } + + fn verify_proof_stateless( + _root: MmrRootHash, + _leaf: EncodableOpaqueLeaf, + _proof: Proof + ) -> Result<(), MmrError> { + unimplemented!() + } + + fn mmr_root() -> Result { + Ok($mmr_root) + } + } } } }; } -create_test_api!(two_validators, BeefyKeyring::Alice, BeefyKeyring::Bob); +create_test_api!(two_validators, mmr_root: GOOD_MMR_ROOT, BeefyKeyring::Alice, BeefyKeyring::Bob); create_test_api!( four_validators, + mmr_root: GOOD_MMR_ROOT, + BeefyKeyring::Alice, + BeefyKeyring::Bob, + BeefyKeyring::Charlie, + BeefyKeyring::Dave +); +create_test_api!( + bad_four_validators, + mmr_root: BAD_MMR_ROOT, BeefyKeyring::Alice, BeefyKeyring::Bob, BeefyKeyring::Charlie, BeefyKeyring::Dave ); - -fn add_mmr_digest(header: &mut Header, mmr_hash: MmrRootHash) { - header.digest_mut().push(DigestItem::Consensus( - BEEFY_ENGINE_ID, - ConsensusLog::::MmrRoot(mmr_hash).encode(), - )); -} fn add_auth_change_digest(header: &mut Header, new_auth_set: BeefyValidatorSet) { header.digest_mut().push(DigestItem::Consensus( @@ -276,17 +296,16 @@ pub(crate) fn create_beefy_keystore(authority: BeefyKeyring) -> SyncCryptoStoreP // Spawns beefy voters. Returns a future to spawn on the runtime. fn initialize_beefy( net: &mut BeefyTestNet, - peers: &[BeefyKeyring], - api: Arc, + peers: Vec<(usize, &BeefyKeyring, Arc)>, min_block_delta: u32, ) -> impl Future where - API: ProvideRuntimeApi + Sync + Send, - API::Api: BeefyApi, + API: ProvideRuntimeApi + Default + Sync + Send, + API::Api: BeefyApi + MmrApi, { let voters = FuturesUnordered::new(); - for (peer_id, key) in peers.iter().enumerate() { + for (peer_id, key, api) in peers.into_iter() { let peer = &net.peers[peer_id]; let keystore = create_beefy_keystore(*key); @@ -298,8 +317,6 @@ where let beefy_link_half = BeefyLinkHalf { signed_commitment_stream, beefy_best_block_stream }; *peer.data.beefy_link_half.lock() = Some(beefy_link_half); - let test_res = peer.data.test_modifiers.clone(); - let beefy_params = crate::BeefyParams { client: peer.client().as_client(), backend: peer.client().as_backend(), @@ -312,7 +329,7 @@ where prometheus_registry: None, protocol_name: BEEFY_PROTOCOL_NAME.into(), }; - let gadget = crate::start_beefy_gadget::<_, _, _, _, _>(beefy_params, test_res); + let gadget = crate::start_beefy_gadget::<_, _, _, _, _>(beefy_params); fn assert_send(_: &T) {} assert_send(&gadget); @@ -461,7 +478,8 @@ fn beefy_finalizing_blocks() { let mut net = BeefyTestNet::new(2, 0); let api = Arc::new(two_validators::TestApi {}); - runtime.spawn(initialize_beefy(&mut net, peers, api, min_block_delta)); + let beefy_peers = peers.iter().enumerate().map(|(id, key)| (id, key, api.clone())).collect(); + runtime.spawn(initialize_beefy(&mut net, beefy_peers, min_block_delta)); // push 42 blocks including `AuthorityChange` digests every 10 blocks. net.generate_blocks(42, session_len, &validator_set); @@ -499,7 +517,8 @@ fn lagging_validators() { let mut net = BeefyTestNet::new(2, 0); let api = Arc::new(two_validators::TestApi {}); - runtime.spawn(initialize_beefy(&mut net, peers, api, min_block_delta)); + let beefy_peers = peers.iter().enumerate().map(|(id, key)| (id, key, api.clone())).collect(); + runtime.spawn(initialize_beefy(&mut net, beefy_peers, min_block_delta)); // push 42 blocks including `AuthorityChange` digests every 30 blocks. net.generate_blocks(42, session_len, &validator_set); @@ -543,11 +562,20 @@ fn correct_beefy_payload() { let min_block_delta = 2; let mut net = BeefyTestNet::new(4, 0); - let api = Arc::new(four_validators::TestApi {}); + + // Alice, Bob, Charlie will vote on good payloads + let good_api = Arc::new(four_validators::TestApi {}); + let good_peers = [BeefyKeyring::Alice, BeefyKeyring::Bob, BeefyKeyring::Charlie] + .iter() + .enumerate() + .map(|(id, key)| (id, key, good_api.clone())) + .collect(); + runtime.spawn(initialize_beefy(&mut net, good_peers, min_block_delta)); // Dave will vote on bad mmr roots - net.peer(3).data.test_modifiers.corrupt_mmr_roots = true; - runtime.spawn(initialize_beefy(&mut net, peers, api, min_block_delta)); + let bad_api = Arc::new(bad_four_validators::TestApi {}); + let bad_peers = vec![(3, &BeefyKeyring::Dave, bad_api)]; + runtime.spawn(initialize_beefy(&mut net, bad_peers, min_block_delta)); // push 10 blocks net.generate_blocks(12, session_len, &validator_set); @@ -582,6 +610,7 @@ fn correct_beefy_payload() { // verify consensus is _not_ reached let timeout = Some(Duration::from_millis(500)); + println!("\n\n-------------------\n"); streams_empty_after_timeout(best_blocks, &net, &mut runtime, timeout); streams_empty_after_timeout(signed_commitments, &net, &mut runtime, None); diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index df53f5d47d33a..218f7a50d3a12 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -40,6 +40,7 @@ use beefy_primitives::{ known_payload_ids, BeefyApi, Commitment, ConsensusLog, MmrRootHash, Payload, SignedCommitment, ValidatorSet, VersionedFinalityProof, VoteMessage, BEEFY_ENGINE_ID, GENESIS_AUTHORITY_SET_ID, }; +use pallet_mmr_primitives::MmrApi; use crate::{ error, @@ -52,12 +53,6 @@ use crate::{ Client, }; -#[cfg(test)] -#[derive(Clone, Default)] -pub struct TestModifiers { - pub corrupt_mmr_roots: bool, -} - pub(crate) struct WorkerParams { pub client: Arc, pub backend: Arc, @@ -98,9 +93,6 @@ pub(crate) struct BeefyWorker { sync_oracle: SO, // keep rustc happy _backend: PhantomData, - #[cfg(test)] - // behavior modifiers used in tests - test_res: TestModifiers, } impl BeefyWorker @@ -109,7 +101,7 @@ where BE: Backend, C: Client, R: ProvideRuntimeApi, - R::Api: BeefyApi, + R::Api: BeefyApi + MmrApi, SO: SyncOracle + Send + Sync + Clone + 'static, { /// Return a new BEEFY worker instance. @@ -118,12 +110,7 @@ where /// BEEFY pallet has been deployed on-chain. /// /// The BEEFY pallet is needed in order to keep track of the BEEFY authority set. - pub(crate) fn new( - worker_params: WorkerParams, - #[cfg(test)] - // TODO: temporary, remove this - test_res: TestModifiers, - ) -> Self { + pub(crate) fn new(worker_params: WorkerParams) -> Self { let WorkerParams { client, backend, @@ -161,8 +148,6 @@ where beefy_best_block_sender, sync_oracle, _backend: PhantomData, - #[cfg(test)] - test_res, } } @@ -535,20 +520,16 @@ where } } - /// Simple wrapper over mmr root extraction. - #[cfg(not(test))] - fn extract_mmr_root_digest(&self, header: &B::Header) -> Option { - find_mmr_root_digest::(header) - } - - /// For tests, have the option to modify mmr root. - #[cfg(test)] + /// Simple wrapper over mmr root extraction. Tries from header digest, then from client state. fn extract_mmr_root_digest(&self, header: &B::Header) -> Option { - let mut mmr_root = find_mmr_root_digest::(header); - if self.test_res.corrupt_mmr_roots { - mmr_root.as_mut().map(|hash| *hash ^= MmrRootHash::random()); - } - mmr_root + find_mmr_root_digest::(header).or_else(|| { + self.runtime + .runtime_api() + .mmr_root(&BlockId::hash(header.hash())) + .ok() + .map(|r| r.ok()) + .flatten() + }) } } @@ -676,7 +657,6 @@ pub(crate) mod tests { let beefy_link_half = BeefyLinkHalf { signed_commitment_stream, beefy_best_block_stream }; *peer.data.beefy_link_half.lock() = Some(beefy_link_half); - let test_modifiers = peer.data.test_modifiers.clone(); let api = Arc::new(TestApi {}); let network = peer.network_service().clone(); let sync_oracle = network.clone(); @@ -696,7 +676,7 @@ pub(crate) mod tests { metrics: None, sync_oracle, }; - BeefyWorker::<_, _, _, _, _>::new(worker_params, test_modifiers) + BeefyWorker::<_, _, _, _, _>::new(worker_params) } #[test] From b012c9b2b32295a670ae3f3a6ddfa1ba8dfe0d4d Mon Sep 17 00:00:00 2001 From: acatangiu Date: Thu, 7 Apr 2022 12:58:35 +0300 Subject: [PATCH 05/11] pallet-beefy-mmr: remove MmrRoot from header digests --- client/beefy/src/worker.rs | 56 ++++++------------------------------ frame/beefy-mmr/src/lib.rs | 19 ------------ frame/beefy-mmr/src/mock.rs | 2 +- frame/beefy-mmr/src/tests.rs | 44 ++-------------------------- 4 files changed, 11 insertions(+), 110 deletions(-) diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index 218f7a50d3a12..791a9c68e3cf8 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -374,7 +374,7 @@ where }; let target_hash = target_header.hash(); - let mmr_root = if let Some(hash) = self.extract_mmr_root_digest(&target_header) { + let mmr_root = if let Some(hash) = self.get_mmr_root_digest(&target_header) { hash } else { warn!(target: "beefy", "🥩 No MMR root digest found for: {:?}", target_hash); @@ -521,32 +521,16 @@ where } /// Simple wrapper over mmr root extraction. Tries from header digest, then from client state. - fn extract_mmr_root_digest(&self, header: &B::Header) -> Option { - find_mmr_root_digest::(header).or_else(|| { - self.runtime - .runtime_api() - .mmr_root(&BlockId::hash(header.hash())) - .ok() - .map(|r| r.ok()) - .flatten() - }) + fn get_mmr_root_digest(&self, header: &B::Header) -> Option { + self.runtime + .runtime_api() + .mmr_root(&BlockId::hash(header.hash())) + .ok() + .map(|r| r.ok()) + .flatten() } } -/// Extract the MMR root hash from a digest in the given header, if it exists. -fn find_mmr_root_digest(header: &B::Header) -> Option -where - B: Block, -{ - let id = OpaqueDigestItemId::Consensus(&BEEFY_ENGINE_ID); - - let filter = |log: ConsensusLog| match log { - ConsensusLog::MmrRoot(root) => Some(root), - _ => None, - }; - header.digest().convert_first(|l| l.try_to(id).and_then(filter)) -} - /// Scan the `header` digest log for a BEEFY validator set change. Return either the new /// validator set or `None` in case no validator set change has been signaled. fn find_authorities_change(header: &B::Header) -> Option> @@ -797,30 +781,6 @@ pub(crate) mod tests { assert_eq!(extracted, Some(validator_set)); } - #[test] - fn extract_mmr_root_digest() { - let mut header = Header::new( - 1u32.into(), - Default::default(), - Default::default(), - Default::default(), - Digest::default(), - ); - - // verify empty digest shows nothing - assert!(find_mmr_root_digest::(&header).is_none()); - - let mmr_root_hash = H256::random(); - header.digest_mut().push(DigestItem::Consensus( - BEEFY_ENGINE_ID, - ConsensusLog::::MmrRoot(mmr_root_hash.clone()).encode(), - )); - - // verify validator set is correctly extracted from digest - let extracted = find_mmr_root_digest::(&header); - assert_eq!(extracted, Some(mmr_root_hash)); - } - #[test] fn should_vote_target() { let keys = &[Keyring::Alice]; diff --git a/frame/beefy-mmr/src/lib.rs b/frame/beefy-mmr/src/lib.rs index 9ee7bd770f64c..c7b102cb4a933 100644 --- a/frame/beefy-mmr/src/lib.rs +++ b/frame/beefy-mmr/src/lib.rs @@ -48,25 +48,6 @@ mod mock; #[cfg(test)] mod tests; -/// A BEEFY consensus digest item with MMR root hash. -pub struct DepositBeefyDigest(sp_std::marker::PhantomData); - -impl pallet_mmr::primitives::OnNewRoot for DepositBeefyDigest -where - T: pallet_mmr::Config, - T: pallet_beefy::Config, -{ - fn on_new_root(root: &::Hash) { - let digest = sp_runtime::generic::DigestItem::Consensus( - beefy_primitives::BEEFY_ENGINE_ID, - codec::Encode::encode(&beefy_primitives::ConsensusLog::< - ::BeefyId, - >::MmrRoot(*root)), - ); - >::deposit_log(digest); - } -} - /// Convert BEEFY secp256k1 public keys into Ethereum addresses pub struct BeefyEcdsaToEthereum; impl Convert> for BeefyEcdsaToEthereum { diff --git a/frame/beefy-mmr/src/mock.rs b/frame/beefy-mmr/src/mock.rs index f6a35f68a4a1f..bb426471b50a3 100644 --- a/frame/beefy-mmr/src/mock.rs +++ b/frame/beefy-mmr/src/mock.rs @@ -117,7 +117,7 @@ impl pallet_mmr::Config for Test { type LeafData = BeefyMmr; - type OnNewRoot = pallet_beefy_mmr::DepositBeefyDigest; + type OnNewRoot = (); type WeightInfo = (); } diff --git a/frame/beefy-mmr/src/tests.rs b/frame/beefy-mmr/src/tests.rs index fd383adb1d4a2..12ddf28b3126a 100644 --- a/frame/beefy-mmr/src/tests.rs +++ b/frame/beefy-mmr/src/tests.rs @@ -17,16 +17,13 @@ use std::vec; -use beefy_primitives::{ - mmr::{BeefyNextAuthoritySet, MmrLeafVersion}, - ValidatorSet, -}; +use beefy_primitives::mmr::{BeefyNextAuthoritySet, MmrLeafVersion}; use codec::{Decode, Encode}; use hex_literal::hex; use sp_core::H256; use sp_io::TestExternalities; -use sp_runtime::{traits::Keccak256, DigestItem}; +use sp_runtime::traits::Keccak256; use frame_support::traits::OnInitialize; @@ -40,10 +37,6 @@ fn init_block(block: u64) { BeefyMmr::on_initialize(block); } -pub fn beefy_log(log: ConsensusLog) -> DigestItem { - DigestItem::Consensus(BEEFY_ENGINE_ID, log.encode()) -} - fn offchain_key(pos: usize) -> Vec { (::INDEXING_PREFIX, pos as u64).encode() } @@ -62,39 +55,6 @@ fn read_mmr_leaf(ext: &mut TestExternalities, index: usize) -> MmrLeaf { .unwrap() } -#[test] -fn should_contain_mmr_digest() { - let mut ext = new_test_ext(vec![1, 2, 3, 4]); - ext.execute_with(|| { - init_block(1); - - assert_eq!( - System::digest().logs, - vec![beefy_log(ConsensusLog::MmrRoot( - hex!("fa0275b19b2565089f7e2377ee73b9050e8d53bce108ef722a3251fd9d371d4b").into() - ))] - ); - - // unique every time - init_block(2); - - assert_eq!( - System::digest().logs, - vec![ - beefy_log(ConsensusLog::MmrRoot( - hex!("fa0275b19b2565089f7e2377ee73b9050e8d53bce108ef722a3251fd9d371d4b").into() - )), - beefy_log(ConsensusLog::AuthoritiesChange( - ValidatorSet::new(vec![mock_beefy_id(3), mock_beefy_id(4),], 1,).unwrap() - )), - beefy_log(ConsensusLog::MmrRoot( - hex!("85554fa7d4e863cce3cdce668c1ae82c0174ad37f8d1399284018bec9f9971c3").into() - )), - ] - ); - }); -} - #[test] fn should_contain_valid_leaf_data() { let mut ext = new_test_ext(vec![1, 2, 3, 4]); From 0bf5a33de0e7a6af5015d6d4138bbfd4918d1472 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Fri, 8 Apr 2022 12:11:51 +0300 Subject: [PATCH 06/11] frame/mmr: move mmr primitives out of frame --- Cargo.lock | 21 +- Cargo.toml | 1 + bin/node/runtime/src/lib.rs | 2 +- client/beefy/Cargo.toml | 2 +- client/beefy/src/lib.rs | 2 +- client/beefy/src/worker.rs | 2 +- frame/beefy-mmr/src/lib.rs | 2 +- frame/merkle-mountain-range/Cargo.toml | 2 + .../primitives/Cargo.toml | 6 +- .../primitives/src/lib.rs | 182 +------- frame/merkle-mountain-range/rpc/Cargo.toml | 1 + frame/merkle-mountain-range/rpc/src/lib.rs | 5 +- frame/merkle-mountain-range/src/lib.rs | 9 +- frame/merkle-mountain-range/src/mmr/mod.rs | 4 +- primitives/merkle-mountain-range/Cargo.toml | 37 ++ primitives/merkle-mountain-range/src/lib.rs | 395 ++++++++++++++++++ 16 files changed, 476 insertions(+), 197 deletions(-) create mode 100644 primitives/merkle-mountain-range/Cargo.toml create mode 100644 primitives/merkle-mountain-range/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 56149944a3584..315eab3657ad9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -487,7 +487,6 @@ dependencies = [ "futures-timer", "hex", "log 0.4.14", - "pallet-mmr-primitives", "parity-scale-codec", "parking_lot 0.12.0", "sc-chain-spec", @@ -509,6 +508,7 @@ dependencies = [ "sp-finality-grandpa", "sp-keyring", "sp-keystore", + "sp-mmr-primitives", "sp-runtime", "sp-tracing", "strum", @@ -6100,6 +6100,7 @@ dependencies = [ "scale-info", "sp-core", "sp-io", + "sp-mmr-primitives", "sp-runtime", "sp-std", ] @@ -6108,7 +6109,6 @@ dependencies = [ name = "pallet-mmr-primitives" version = "4.0.0-dev" dependencies = [ - "frame-support", "frame-system", "hex-literal", "log 0.4.14", @@ -6116,6 +6116,8 @@ dependencies = [ "serde", "sp-api", "sp-core", + "sp-debug-derive", + "sp-mmr-primitives", "sp-runtime", "sp-std", ] @@ -6134,6 +6136,7 @@ dependencies = [ "sp-api", "sp-blockchain", "sp-core", + "sp-mmr-primitives", "sp-runtime", ] @@ -10152,6 +10155,20 @@ dependencies = [ "zstd", ] +[[package]] +name = "sp-mmr-primitives" +version = "4.0.0-dev" +dependencies = [ + "hex-literal", + "log 0.4.14", + "parity-scale-codec", + "serde", + "sp-api", + "sp-core", + "sp-debug-derive", + "sp-std", +] + [[package]] name = "sp-npos-elections" version = "4.0.0-dev" diff --git a/Cargo.toml b/Cargo.toml index 13657dd1234a5..4bfbd7ac138b9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -171,6 +171,7 @@ members = [ "primitives/keyring", "primitives/keystore", "primitives/maybe-compressed-blob", + "primitives/merkle-mountain-range", "primitives/npos-elections", "primitives/npos-elections/fuzzer", "primitives/offchain", diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 1b07581a82096..a33b9cf416e1d 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1505,7 +1505,7 @@ pub type Executive = frame_executive::Executive< /// MMR helper types. mod mmr { use super::Runtime; - pub use pallet_mmr::primitives::*; + pub use pallet_mmr::{primitives::*, runtime_primitives::*}; pub type Leaf = <::LeafData as LeafDataProvider>::LeafData; pub type Hash = ::Hash; diff --git a/client/beefy/Cargo.toml b/client/beefy/Cargo.toml index 3eba8379397ce..87c3d5ed71aae 100644 --- a/client/beefy/Cargo.toml +++ b/client/beefy/Cargo.toml @@ -27,6 +27,7 @@ sp-blockchain = { version = "4.0.0-dev", path = "../../primitives/blockchain" } sp-consensus = { version = "0.10.0-dev", path = "../../primitives/consensus/common" } sp-core = { version = "6.0.0", path = "../../primitives/core" } sp-keystore = { version = "0.12.0", path = "../../primitives/keystore" } +sp-mmr-primitives = { version = "4.0.0-dev", default-features = false, path = "../../primitives/merkle-mountain-range" } sp-runtime = { version = "6.0.0", path = "../../primitives/runtime" } sc-chain-spec = { version = "4.0.0-dev", path = "../../client/chain-spec" } @@ -38,7 +39,6 @@ sc-network-gossip = { version = "0.10.0-dev", path = "../network-gossip" } sc-utils = { version = "4.0.0-dev", path = "../utils" } beefy-primitives = { version = "4.0.0-dev", path = "../../primitives/beefy" } -pallet-mmr-primitives = { version = "4.0.0-dev", path = "../../frame/merkle-mountain-range/primitives" } [dev-dependencies] sc-consensus = { version = "0.10.0-dev", path = "../consensus/common" } diff --git a/client/beefy/src/lib.rs b/client/beefy/src/lib.rs index 272ef213038a1..315807ce3aac4 100644 --- a/client/beefy/src/lib.rs +++ b/client/beefy/src/lib.rs @@ -30,7 +30,7 @@ use sp_keystore::SyncCryptoStorePtr; use sp_runtime::traits::Block; use beefy_primitives::{BeefyApi, MmrRootHash}; -use pallet_mmr_primitives::MmrApi; +use sp_mmr_primitives::MmrApi; use crate::notification::{BeefyBestBlockSender, BeefySignedCommitmentSender}; diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index 791a9c68e3cf8..ffca30b0a20d7 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -29,6 +29,7 @@ use sc_network_gossip::GossipEngine; use sp_api::{BlockId, ProvideRuntimeApi}; use sp_arithmetic::traits::AtLeast32Bit; use sp_consensus::SyncOracle; +use sp_mmr_primitives::MmrApi; use sp_runtime::{ generic::OpaqueDigestItemId, traits::{Block, Header, NumberFor}, @@ -40,7 +41,6 @@ use beefy_primitives::{ known_payload_ids, BeefyApi, Commitment, ConsensusLog, MmrRootHash, Payload, SignedCommitment, ValidatorSet, VersionedFinalityProof, VoteMessage, BEEFY_ENGINE_ID, GENESIS_AUTHORITY_SET_ID, }; -use pallet_mmr_primitives::MmrApi; use crate::{ error, diff --git a/frame/beefy-mmr/src/lib.rs b/frame/beefy-mmr/src/lib.rs index c7b102cb4a933..07e4527cf23f6 100644 --- a/frame/beefy-mmr/src/lib.rs +++ b/frame/beefy-mmr/src/lib.rs @@ -37,7 +37,7 @@ use sp_runtime::traits::{Convert, Hash, Member}; use sp_std::prelude::*; use beefy_primitives::mmr::{BeefyDataProvider, BeefyNextAuthoritySet, MmrLeaf, MmrLeafVersion}; -use pallet_mmr::primitives::LeafDataProvider; +use pallet_mmr_primitives::LeafDataProvider; use frame_support::traits::Get; diff --git a/frame/merkle-mountain-range/Cargo.toml b/frame/merkle-mountain-range/Cargo.toml index 796ab98dc2c32..2691804d85524 100644 --- a/frame/merkle-mountain-range/Cargo.toml +++ b/frame/merkle-mountain-range/Cargo.toml @@ -18,6 +18,7 @@ mmr-lib = { package = "ckb-merkle-mountain-range", default-features = false, ver sp-core = { version = "6.0.0", default-features = false, path = "../../primitives/core" } sp-io = { version = "6.0.0", default-features = false, path = "../../primitives/io" } +sp-mmr-primitives = { version = "4.0.0-dev", default-features = false, path = "../../primitives/merkle-mountain-range" } sp-runtime = { version = "6.0.0", default-features = false, path = "../../primitives/runtime" } sp-std = { version = "4.0.0", default-features = false, path = "../../primitives/std" } @@ -39,6 +40,7 @@ std = [ "mmr-lib/std", "sp-core/std", "sp-io/std", + "sp-mmr-primitives/std", "sp-runtime/std", "sp-std/std", "frame-benchmarking/std", diff --git a/frame/merkle-mountain-range/primitives/Cargo.toml b/frame/merkle-mountain-range/primitives/Cargo.toml index 3ce2caa7762c0..ed132c328a68e 100644 --- a/frame/merkle-mountain-range/primitives/Cargo.toml +++ b/frame/merkle-mountain-range/primitives/Cargo.toml @@ -18,10 +18,11 @@ serde = { version = "1.0.136", optional = true, features = ["derive"] } sp-api = { version = "4.0.0-dev", default-features = false, path = "../../../primitives/api" } sp-core = { version = "6.0.0", default-features = false, path = "../../../primitives/core" } +sp-debug-derive = { version = "4.0.0", default-features = false, path = "../../../primitives/debug-derive" } +sp-mmr-primitives = { version = "4.0.0-dev", default-features = false, path = "../../../primitives/merkle-mountain-range" } sp-runtime = { version = "6.0.0", default-features = false, path = "../../../primitives/runtime" } sp-std = { version = "4.0.0", default-features = false, path = "../../../primitives/std" } -frame-support = { version = "4.0.0-dev", default-features = false, path = "../../support" } frame-system = { version = "4.0.0-dev", default-features = false, path = "../../system" } [dev-dependencies] @@ -35,8 +36,9 @@ std = [ "serde", "sp-api/std", "sp-core/std", + "sp-debug-derive/std", + "sp-mmr-primitives/std", "sp-runtime/std", "sp-std/std", - "frame-support/std", "frame-system/std", ] diff --git a/frame/merkle-mountain-range/primitives/src/lib.rs b/frame/merkle-mountain-range/primitives/src/lib.rs index 1ac6e533cd0a3..880f80f29ce13 100644 --- a/frame/merkle-mountain-range/primitives/src/lib.rs +++ b/frame/merkle-mountain-range/primitives/src/lib.rs @@ -20,22 +20,12 @@ #![cfg_attr(not(feature = "std"), no_std)] #![warn(missing_docs)] -use frame_support::RuntimeDebug; +use sp_debug_derive::RuntimeDebug; +use sp_mmr_primitives::FullLeaf; use sp_runtime::traits::{self, One, Saturating}; -use sp_std::fmt; #[cfg(not(feature = "std"))] use sp_std::prelude::Vec; -/// A type to describe node position in the MMR (node index). -pub type NodeIndex = u64; - -/// A type to describe leaf position in the MMR. -/// -/// Note this is different from [`NodeIndex`], which can be applied to -/// both leafs and inner nodes. Leafs will always have consecutive `LeafIndex`, -/// but might be actually at different positions in the MMR `NodeIndex`. -pub type LeafIndex = u64; - /// A provider of the MMR's leaf data. pub trait LeafDataProvider { /// A type that should end up in the leaf of MMR. @@ -83,20 +73,6 @@ impl OnNewRoot for () { fn on_new_root(_root: &Hash) {} } -/// A full leaf content stored in the offchain-db. -pub trait FullLeaf: Clone + PartialEq + fmt::Debug { - /// Encode the leaf either in it's full or compact form. - /// - /// NOTE the encoding returned here MUST be `Decode`able into `FullLeaf`. - fn using_encoded R>(&self, f: F, compact: bool) -> R; -} - -impl FullLeaf for T { - fn using_encoded R>(&self, f: F, _compact: bool) -> R { - codec::Encode::using_encoded(self, f) - } -} - /// An element representing either full data or it's hash. /// /// See [Compact] to see how it may be used in practice to reduce the size @@ -281,160 +257,6 @@ impl_leaf_data_for_tuple!(A:0, B:1, C:2); impl_leaf_data_for_tuple!(A:0, B:1, C:2, D:3); impl_leaf_data_for_tuple!(A:0, B:1, C:2, D:3, E:4); -/// A MMR proof data for one of the leaves. -#[derive(codec::Encode, codec::Decode, RuntimeDebug, Clone, PartialEq, Eq)] -pub struct Proof { - /// The index of the leaf the proof is for. - pub leaf_index: LeafIndex, - /// Number of leaves in MMR, when the proof was generated. - pub leaf_count: NodeIndex, - /// Proof elements (hashes of siblings of inner nodes on the path to the leaf). - pub items: Vec, -} - -/// Merkle Mountain Range operation error. -#[derive(RuntimeDebug, codec::Encode, codec::Decode, PartialEq, Eq)] -pub enum Error { - /// Error while pushing new node. - Push, - /// Error getting the new root. - GetRoot, - /// Error commiting changes. - Commit, - /// Error during proof generation. - GenerateProof, - /// Proof verification error. - Verify, - /// Leaf not found in the storage. - LeafNotFound, -} - -impl Error { - #![allow(unused_variables)] - /// Consume given error `e` with `self` and generate a native log entry with error details. - pub fn log_error(self, e: impl fmt::Debug) -> Self { - log::error!( - target: "runtime::mmr", - "[{:?}] MMR error: {:?}", - self, - e, - ); - self - } - - /// Consume given error `e` with `self` and generate a native log entry with error details. - pub fn log_debug(self, e: impl fmt::Debug) -> Self { - log::debug!( - target: "runtime::mmr", - "[{:?}] MMR error: {:?}", - self, - e, - ); - self - } -} - -/// A helper type to allow using arbitrary SCALE-encoded leaf data in the RuntimeApi. -/// -/// The point is to be able to verify MMR proofs from external MMRs, where we don't -/// know the exact leaf type, but it's enough for us to have it SCALE-encoded. -/// -/// Note the leaf type should be encoded in its compact form when passed through this type. -/// See [FullLeaf] documentation for details. -/// -/// This type does not implement SCALE encoding/decoding on purpose to avoid confusion, -/// it would have to be SCALE-compatible with the concrete leaf type, but due to SCALE limitations -/// it's not possible to know how many bytes the encoding of concrete leaf type uses. -#[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))] -#[derive(RuntimeDebug, Clone, PartialEq)] -pub struct OpaqueLeaf( - /// Raw bytes of the leaf type encoded in its compact form. - /// - /// NOTE it DOES NOT include length prefix (like `Vec` encoding would). - #[cfg_attr(feature = "std", serde(with = "sp_core::bytes"))] - pub Vec, -); - -impl OpaqueLeaf { - /// Convert a concrete MMR leaf into an opaque type. - pub fn from_leaf(leaf: &T) -> Self { - let encoded_leaf = leaf.using_encoded(|d| d.to_vec(), true); - OpaqueLeaf::from_encoded_leaf(encoded_leaf) - } - - /// Create a `OpaqueLeaf` given raw bytes of compact-encoded leaf. - pub fn from_encoded_leaf(encoded_leaf: Vec) -> Self { - OpaqueLeaf(encoded_leaf) - } - - /// Attempt to decode the leaf into expected concrete type. - pub fn try_decode(&self) -> Option { - codec::Decode::decode(&mut &*self.0).ok() - } -} - -impl FullLeaf for OpaqueLeaf { - fn using_encoded R>(&self, f: F, _compact: bool) -> R { - f(&self.0) - } -} - -/// A type-safe wrapper for the concrete leaf type. -/// -/// This structure serves merely to avoid passing raw `Vec` around. -/// It must be `Vec`-encoding compatible. -/// -/// It is different from [`OpaqueLeaf`], because it does implement `Codec` -/// and the encoding has to match raw `Vec` encoding. -#[derive(codec::Encode, codec::Decode, RuntimeDebug, PartialEq, Eq)] -pub struct EncodableOpaqueLeaf(pub Vec); - -impl EncodableOpaqueLeaf { - /// Convert a concrete leaf into encodable opaque version. - pub fn from_leaf(leaf: &T) -> Self { - let opaque = OpaqueLeaf::from_leaf(leaf); - Self::from_opaque_leaf(opaque) - } - - /// Given an opaque leaf, make it encodable. - pub fn from_opaque_leaf(opaque: OpaqueLeaf) -> Self { - Self(opaque.0) - } - - /// Try to convert into a [OpaqueLeaf]. - pub fn into_opaque_leaf(self) -> OpaqueLeaf { - // wrap into `OpaqueLeaf` type - OpaqueLeaf::from_encoded_leaf(self.0) - } -} - -sp_api::decl_runtime_apis! { - /// API to interact with MMR pallet. - pub trait MmrApi { - /// Generate MMR proof for a leaf under given index. - fn generate_proof(leaf_index: LeafIndex) -> Result<(EncodableOpaqueLeaf, Proof), Error>; - - /// Verify MMR proof against on-chain MMR. - /// - /// Note this function will use on-chain MMR root hash and check if the proof - /// matches the hash. - /// See [Self::verify_proof_stateless] for a stateless verifier. - fn verify_proof(leaf: EncodableOpaqueLeaf, proof: Proof) -> Result<(), Error>; - - /// Verify MMR proof against given root hash. - /// - /// Note this function does not require any on-chain storage - the - /// proof is verified against given MMR root hash. - /// - /// The leaf data is expected to be encoded in it's compact form. - fn verify_proof_stateless(root: Hash, leaf: EncodableOpaqueLeaf, proof: Proof) - -> Result<(), Error>; - - /// Return the on-chain MMR root hash. - fn mmr_root() -> Result; - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/frame/merkle-mountain-range/rpc/Cargo.toml b/frame/merkle-mountain-range/rpc/Cargo.toml index 9ac26c2ed54b2..fd5e955064658 100644 --- a/frame/merkle-mountain-range/rpc/Cargo.toml +++ b/frame/merkle-mountain-range/rpc/Cargo.toml @@ -22,6 +22,7 @@ serde = { version = "1.0.136", features = ["derive"] } sp-api = { version = "4.0.0-dev", path = "../../../primitives/api" } sp-blockchain = { version = "4.0.0-dev", path = "../../../primitives/blockchain" } sp-core = { version = "6.0.0", path = "../../../primitives/core" } +sp-mmr-primitives = { version = "4.0.0-dev", default-features = false, path = "../../../primitives/merkle-mountain-range" } sp-runtime = { version = "6.0.0", path = "../../../primitives/runtime" } pallet-mmr-primitives = { version = "4.0.0-dev", path = "../primitives" } diff --git a/frame/merkle-mountain-range/rpc/src/lib.rs b/frame/merkle-mountain-range/rpc/src/lib.rs index bf3eb3b694e39..7de5c3a5e6d53 100644 --- a/frame/merkle-mountain-range/rpc/src/lib.rs +++ b/frame/merkle-mountain-range/rpc/src/lib.rs @@ -26,13 +26,14 @@ use jsonrpc_core::{Error, ErrorCode, Result}; use jsonrpc_derive::rpc; use serde::{Deserialize, Serialize}; -use pallet_mmr_primitives::{Error as MmrError, Proof}; use sp_api::ProvideRuntimeApi; use sp_blockchain::HeaderBackend; use sp_core::Bytes; +use sp_mmr_primitives::{Error as MmrError, LeafIndex, Proof}; use sp_runtime::{generic::BlockId, traits::Block as BlockT}; -pub use pallet_mmr_primitives::{LeafIndex, MmrApi as MmrRuntimeApi}; +// pub use pallet_mmr_primitives::{LeafIndex, MmrApi as MmrRuntimeApi}; +pub use sp_mmr_primitives::MmrApi as MmrRuntimeApi; /// Retrieved MMR leaf and its proof. #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)] diff --git a/frame/merkle-mountain-range/src/lib.rs b/frame/merkle-mountain-range/src/lib.rs index 9b3ce44b076bd..f04edf545bf2a 100644 --- a/frame/merkle-mountain-range/src/lib.rs +++ b/frame/merkle-mountain-range/src/lib.rs @@ -70,7 +70,8 @@ mod mock; mod tests; pub use pallet::*; -pub use pallet_mmr_primitives::{self as primitives, LeafIndex, NodeIndex}; +pub use pallet_mmr_primitives as runtime_primitives; +pub use sp_mmr_primitives::{self as primitives, Error, LeafIndex, NodeIndex}; pub trait WeightInfo { fn on_initialize(peaks: NodeIndex) -> Weight; @@ -138,7 +139,7 @@ pub mod pallet { /// /// Note that the leaf at each block MUST be unique. You may want to include a block hash or /// block number as an easiest way to ensure that. - type LeafData: primitives::LeafDataProvider; + type LeafData: runtime_primitives::LeafDataProvider; /// A hook to act on the new MMR root. /// @@ -175,7 +176,7 @@ pub mod pallet { #[pallet::hooks] impl, I: 'static> Hooks> for Pallet { fn on_initialize(_n: T::BlockNumber) -> Weight { - use primitives::LeafDataProvider; + use runtime_primitives::LeafDataProvider; let leaves = Self::mmr_leaves(); let peaks_before = mmr::utils::NodesUtils::new(leaves).number_of_peaks(); let data = T::LeafData::leaf_data(); @@ -200,7 +201,7 @@ pub mod pallet { type ModuleMmr = mmr::Mmr>; /// Leaf data. -type LeafOf = <>::LeafData as primitives::LeafDataProvider>::LeafData; +type LeafOf = <>::LeafData as runtime_primitives::LeafDataProvider>::LeafData; /// Hashing used for the pallet. pub(crate) type HashingOf = >::Hashing; diff --git a/frame/merkle-mountain-range/src/mmr/mod.rs b/frame/merkle-mountain-range/src/mmr/mod.rs index 1a729b08b966f..f877a9729443e 100644 --- a/frame/merkle-mountain-range/src/mmr/mod.rs +++ b/frame/merkle-mountain-range/src/mmr/mod.rs @@ -19,7 +19,7 @@ mod mmr; pub mod storage; pub mod utils; -use crate::primitives::FullLeaf; +use sp_mmr_primitives::FullLeaf; use sp_runtime::traits; pub use self::mmr::{verify_leaf_proof, Mmr}; @@ -28,7 +28,7 @@ pub use self::mmr::{verify_leaf_proof, Mmr}; pub type NodeOf = Node<>::Hashing, L>; /// A node stored in the MMR. -pub type Node = crate::primitives::DataOrHash; +pub type Node = pallet_mmr_primitives::DataOrHash; /// Default Merging & Hashing behavior for MMR. pub struct Hasher(sp_std::marker::PhantomData<(H, L)>); diff --git a/primitives/merkle-mountain-range/Cargo.toml b/primitives/merkle-mountain-range/Cargo.toml new file mode 100644 index 0000000000000..35c0fdd8057f3 --- /dev/null +++ b/primitives/merkle-mountain-range/Cargo.toml @@ -0,0 +1,37 @@ +[package] +name = "sp-mmr-primitives" +version = "4.0.0-dev" +authors = ["Parity Technologies "] +edition = "2021" +license = "Apache-2.0" +homepage = "https://substrate.io" +repository = "https://github.com/paritytech/substrate/" +description = "Merkle Mountain Range primitives." + +[package.metadata.docs.rs] +targets = ["x86_64-unknown-linux-gnu"] + +[dependencies] +codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false } +log = { version = "0.4.14", default-features = false } +serde = { version = "1.0.136", optional = true, features = ["derive"] } + +sp-api = { version = "4.0.0-dev", default-features = false, path = "../api" } +sp-core = { version = "6.0.0", default-features = false, path = "../core" } +sp-debug-derive = { version = "4.0.0", default-features = false, path = "../debug-derive" } +sp-std = { version = "4.0.0", default-features = false, path = "../std" } + +[dev-dependencies] +hex-literal = "0.3" + +[features] +default = ["std"] +std = [ + "codec/std", + "log/std", + "serde", + "sp-api/std", + "sp-core/std", + "sp-debug-derive/std", + "sp-std/std", +] diff --git a/primitives/merkle-mountain-range/src/lib.rs b/primitives/merkle-mountain-range/src/lib.rs new file mode 100644 index 0000000000000..936d9d78d31a4 --- /dev/null +++ b/primitives/merkle-mountain-range/src/lib.rs @@ -0,0 +1,395 @@ +// This file is part of Substrate. + +// Copyright (C) 2020-2022 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Merkle Mountain Range primitive types. + +#![cfg_attr(not(feature = "std"), no_std)] +#![warn(missing_docs)] + +use sp_debug_derive::RuntimeDebug; +use sp_std::fmt; +#[cfg(not(feature = "std"))] +use sp_std::prelude::Vec; + +/// A type to describe node position in the MMR (node index). +pub type NodeIndex = u64; + +/// A type to describe leaf position in the MMR. +/// +/// Note this is different from [`NodeIndex`], which can be applied to +/// both leafs and inner nodes. Leafs will always have consecutive `LeafIndex`, +/// but might be actually at different positions in the MMR `NodeIndex`. +pub type LeafIndex = u64; + +/// A MMR proof data for one of the leaves. +#[derive(codec::Encode, codec::Decode, RuntimeDebug, Clone, PartialEq, Eq)] +pub struct Proof { + /// The index of the leaf the proof is for. + pub leaf_index: LeafIndex, + /// Number of leaves in MMR, when the proof was generated. + pub leaf_count: NodeIndex, + /// Proof elements (hashes of siblings of inner nodes on the path to the leaf). + pub items: Vec, +} + +/// A full leaf content stored in the offchain-db. +pub trait FullLeaf: Clone + PartialEq + fmt::Debug { + /// Encode the leaf either in it's full or compact form. + /// + /// NOTE the encoding returned here MUST be `Decode`able into `FullLeaf`. + fn using_encoded R>(&self, f: F, compact: bool) -> R; +} + +impl FullLeaf for T { + fn using_encoded R>(&self, f: F, _compact: bool) -> R { + codec::Encode::using_encoded(self, f) + } +} + +/// A helper type to allow using arbitrary SCALE-encoded leaf data in the RuntimeApi. +/// +/// The point is to be able to verify MMR proofs from external MMRs, where we don't +/// know the exact leaf type, but it's enough for us to have it SCALE-encoded. +/// +/// Note the leaf type should be encoded in its compact form when passed through this type. +/// See [FullLeaf] documentation for details. +/// +/// This type does not implement SCALE encoding/decoding on purpose to avoid confusion, +/// it would have to be SCALE-compatible with the concrete leaf type, but due to SCALE limitations +/// it's not possible to know how many bytes the encoding of concrete leaf type uses. +#[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))] +#[derive(RuntimeDebug, Clone, PartialEq)] +pub struct OpaqueLeaf( + /// Raw bytes of the leaf type encoded in its compact form. + /// + /// NOTE it DOES NOT include length prefix (like `Vec` encoding would). + #[cfg_attr(feature = "std", serde(with = "sp_core::bytes"))] + pub Vec, +); + +impl OpaqueLeaf { + /// Convert a concrete MMR leaf into an opaque type. + pub fn from_leaf(leaf: &T) -> Self { + let encoded_leaf = leaf.using_encoded(|d| d.to_vec(), true); + OpaqueLeaf::from_encoded_leaf(encoded_leaf) + } + + /// Create a `OpaqueLeaf` given raw bytes of compact-encoded leaf. + pub fn from_encoded_leaf(encoded_leaf: Vec) -> Self { + OpaqueLeaf(encoded_leaf) + } + + /// Attempt to decode the leaf into expected concrete type. + pub fn try_decode(&self) -> Option { + codec::Decode::decode(&mut &*self.0).ok() + } +} + +impl FullLeaf for OpaqueLeaf { + fn using_encoded R>(&self, f: F, _compact: bool) -> R { + f(&self.0) + } +} + +/// A type-safe wrapper for the concrete leaf type. +/// +/// This structure serves merely to avoid passing raw `Vec` around. +/// It must be `Vec`-encoding compatible. +/// +/// It is different from [`OpaqueLeaf`], because it does implement `Codec` +/// and the encoding has to match raw `Vec` encoding. +#[derive(codec::Encode, codec::Decode, RuntimeDebug, PartialEq, Eq)] +pub struct EncodableOpaqueLeaf(pub Vec); + +impl EncodableOpaqueLeaf { + /// Convert a concrete leaf into encodable opaque version. + pub fn from_leaf(leaf: &T) -> Self { + let opaque = OpaqueLeaf::from_leaf(leaf); + Self::from_opaque_leaf(opaque) + } + + /// Given an opaque leaf, make it encodable. + pub fn from_opaque_leaf(opaque: OpaqueLeaf) -> Self { + Self(opaque.0) + } + + /// Try to convert into a [OpaqueLeaf]. + pub fn into_opaque_leaf(self) -> OpaqueLeaf { + // wrap into `OpaqueLeaf` type + OpaqueLeaf::from_encoded_leaf(self.0) + } +} + +/// Merkle Mountain Range operation error. +#[derive(RuntimeDebug, codec::Encode, codec::Decode, PartialEq, Eq)] +pub enum Error { + /// Error while pushing new node. + Push, + /// Error getting the new root. + GetRoot, + /// Error commiting changes. + Commit, + /// Error during proof generation. + GenerateProof, + /// Proof verification error. + Verify, + /// Leaf not found in the storage. + LeafNotFound, +} + +impl Error { + #![allow(unused_variables)] + /// Consume given error `e` with `self` and generate a native log entry with error details. + pub fn log_error(self, e: impl fmt::Debug) -> Self { + log::error!( + target: "runtime::mmr", + "[{:?}] MMR error: {:?}", + self, + e, + ); + self + } + + /// Consume given error `e` with `self` and generate a native log entry with error details. + pub fn log_debug(self, e: impl fmt::Debug) -> Self { + log::debug!( + target: "runtime::mmr", + "[{:?}] MMR error: {:?}", + self, + e, + ); + self + } +} + +sp_api::decl_runtime_apis! { + /// API to interact with MMR pallet. + pub trait MmrApi { + /// Generate MMR proof for a leaf under given index. + fn generate_proof(leaf_index: LeafIndex) -> Result<(EncodableOpaqueLeaf, Proof), Error>; + + /// Verify MMR proof against on-chain MMR. + /// + /// Note this function will use on-chain MMR root hash and check if the proof + /// matches the hash. + /// See [Self::verify_proof_stateless] for a stateless verifier. + fn verify_proof(leaf: EncodableOpaqueLeaf, proof: Proof) -> Result<(), Error>; + + /// Verify MMR proof against given root hash. + /// + /// Note this function does not require any on-chain storage - the + /// proof is verified against given MMR root hash. + /// + /// The leaf data is expected to be encoded in it's compact form. + fn verify_proof_stateless(root: Hash, leaf: EncodableOpaqueLeaf, proof: Proof) + -> Result<(), Error>; + + /// Return the on-chain MMR root hash. + fn mmr_root() -> Result; + } +} + +/// New MMR root notification hook. +pub trait OnNewRoot { + /// Function called by the pallet in case new MMR root has been computed. + fn on_new_root(root: &Hash); +} + +/// No-op implementation of [OnNewRoot]. +impl OnNewRoot for () { + fn on_new_root(_root: &Hash) {} +} + +#[cfg(test)] +mod tests { + use super::*; + + use codec::Decode; + use sp_core::H256; + use sp_runtime::traits::Keccak256; + + pub(crate) fn hex(s: &str) -> H256 { + s.parse().unwrap() + } + + type Test = DataOrHash; + type TestCompact = Compact; + type TestProof = Proof<::Output>; + + #[test] + fn should_encode_decode_proof() { + // given + let proof: TestProof = Proof { + leaf_index: 5, + leaf_count: 10, + items: vec![ + hex("c3e7ba6b511162fead58f2c8b5764ce869ed1118011ac37392522ed16720bbcd"), + hex("d3e7ba6b511162fead58f2c8b5764ce869ed1118011ac37392522ed16720bbcd"), + hex("e3e7ba6b511162fead58f2c8b5764ce869ed1118011ac37392522ed16720bbcd"), + ], + }; + + // when + let encoded = codec::Encode::encode(&proof); + let decoded = TestProof::decode(&mut &*encoded); + + // then + assert_eq!(decoded, Ok(proof)); + } + + #[test] + fn should_encode_decode_correctly_if_no_compact() { + // given + let cases = vec![ + Test::Data("Hello World!".into()), + Test::Hash(hex("c3e7ba6b511162fead58f2c8b5764ce869ed1118011ac37392522ed16720bbcd")), + Test::Data("".into()), + Test::Data("3e48d6bcd417fb22e044747242451e2c0f3e602d1bcad2767c34808621956417".into()), + ]; + + // when + let encoded = cases.iter().map(codec::Encode::encode).collect::>(); + + let decoded = encoded.iter().map(|x| Test::decode(&mut &**x)).collect::>(); + + // then + assert_eq!( + decoded, + cases.into_iter().map(Result::<_, codec::Error>::Ok).collect::>() + ); + // check encoding correctness + assert_eq!(&encoded[0], &hex_literal::hex!("00343048656c6c6f20576f726c6421")); + assert_eq!( + encoded[1].as_slice(), + hex_literal::hex!("01c3e7ba6b511162fead58f2c8b5764ce869ed1118011ac37392522ed16720bbcd") + .as_ref() + ); + } + + #[test] + fn should_return_the_hash_correctly() { + // given + let a = Test::Data("Hello World!".into()); + let b = Test::Hash(hex("c3e7ba6b511162fead58f2c8b5764ce869ed1118011ac37392522ed16720bbcd")); + + // when + let a = a.hash(); + let b = b.hash(); + + // then + assert_eq!(a, hex("a9c321be8c24ba4dc2bd73f5300bde67dc57228ab8b68b607bb4c39c5374fac9")); + assert_eq!(b, hex("c3e7ba6b511162fead58f2c8b5764ce869ed1118011ac37392522ed16720bbcd")); + } + + #[test] + fn compact_should_work() { + // given + let a = Test::Data("Hello World!".into()); + let b = Test::Data("".into()); + + // when + let c: TestCompact = Compact::new((a.clone(), b.clone())); + let d: TestCompact = Compact::new((Test::Hash(a.hash()), Test::Hash(b.hash()))); + + // then + assert_eq!(c.hash(), d.hash()); + } + + #[test] + fn compact_should_encode_decode_correctly() { + // given + let a = Test::Data("Hello World!".into()); + let b = Test::Data("".into()); + + let c: TestCompact = Compact::new((a.clone(), b.clone())); + let d: TestCompact = Compact::new((Test::Hash(a.hash()), Test::Hash(b.hash()))); + let cases = vec![c, d.clone()]; + + // when + let encoded_compact = + cases.iter().map(|c| c.using_encoded(|x| x.to_vec(), true)).collect::>(); + + let encoded = + cases.iter().map(|c| c.using_encoded(|x| x.to_vec(), false)).collect::>(); + + let decoded_compact = encoded_compact + .iter() + .map(|x| TestCompact::decode(&mut &**x)) + .collect::>(); + + let decoded = encoded.iter().map(|x| TestCompact::decode(&mut &**x)).collect::>(); + + // then + assert_eq!( + decoded, + cases.into_iter().map(Result::<_, codec::Error>::Ok).collect::>() + ); + + assert_eq!(decoded_compact, vec![Ok(d.clone()), Ok(d.clone())]); + } + + #[test] + fn opaque_leaves_should_be_full_leaf_compatible() { + // given + let a = Test::Data("Hello World!".into()); + let b = Test::Data("".into()); + + let c: TestCompact = Compact::new((a.clone(), b.clone())); + let d: TestCompact = Compact::new((Test::Hash(a.hash()), Test::Hash(b.hash()))); + let cases = vec![c, d.clone()]; + + let encoded_compact = cases + .iter() + .map(|c| c.using_encoded(|x| x.to_vec(), true)) + .map(OpaqueLeaf::from_encoded_leaf) + .collect::>(); + + let opaque = cases.iter().map(OpaqueLeaf::from_leaf).collect::>(); + + // then + assert_eq!(encoded_compact, opaque); + } + + #[test] + fn encode_opaque_leaf_should_be_scale_compatible() { + use codec::Encode; + + // given + let a = Test::Data("Hello World!".into()); + let case1 = EncodableOpaqueLeaf::from_leaf(&a); + let case2 = EncodableOpaqueLeaf::from_opaque_leaf(OpaqueLeaf(a.encode())); + let case3 = a.encode().encode(); + + // when + let encoded = vec![&case1, &case2].into_iter().map(|x| x.encode()).collect::>(); + let decoded = vec![&*encoded[0], &*encoded[1], &*case3] + .into_iter() + .map(|x| EncodableOpaqueLeaf::decode(&mut &*x)) + .collect::>(); + + // then + assert_eq!(case1, case2); + assert_eq!(encoded[0], encoded[1]); + // then encoding should also match double-encoded leaf. + assert_eq!(encoded[0], case3); + + assert_eq!(decoded[0], decoded[1]); + assert_eq!(decoded[1], decoded[2]); + assert_eq!(decoded[0], Ok(case2)); + assert_eq!(decoded[1], Ok(case1)); + } +} From f2406b1cabdca7f3615d7de98eb56c37a6b52081 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Fri, 8 Apr 2022 12:58:36 +0300 Subject: [PATCH 07/11] frame/mmr: completely move primitives out of frame --- Cargo.lock | 21 +- Cargo.toml | 1 - bin/node/runtime/src/lib.rs | 4 +- client/beefy/Cargo.toml | 2 +- client/beefy/src/lib.rs | 2 +- client/beefy/src/tests.rs | 2 +- frame/beefy-mmr/Cargo.toml | 2 - frame/beefy-mmr/src/lib.rs | 4 +- frame/beefy-mmr/src/tests.rs | 2 +- frame/merkle-mountain-range/Cargo.toml | 3 - .../primitives/Cargo.toml | 44 -- .../primitives/src/lib.rs | 438 ------------------ frame/merkle-mountain-range/rpc/Cargo.toml | 2 - frame/merkle-mountain-range/rpc/src/lib.rs | 2 +- frame/merkle-mountain-range/src/lib.rs | 33 +- frame/merkle-mountain-range/src/mmr/mod.rs | 4 +- frame/merkle-mountain-range/src/mock.rs | 4 +- frame/merkle-mountain-range/src/tests.rs | 2 +- primitives/merkle-mountain-range/Cargo.toml | 2 + primitives/merkle-mountain-range/src/lib.rs | 228 ++++++++- 20 files changed, 261 insertions(+), 541 deletions(-) delete mode 100644 frame/merkle-mountain-range/primitives/Cargo.toml delete mode 100644 frame/merkle-mountain-range/primitives/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 315eab3657ad9..2e3ad2d227b33 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5666,7 +5666,6 @@ dependencies = [ "log 0.4.14", "pallet-beefy", "pallet-mmr", - "pallet-mmr-primitives", "pallet-session", "parity-scale-codec", "scale-info", @@ -6095,7 +6094,6 @@ dependencies = [ "frame-support", "frame-system", "hex-literal", - "pallet-mmr-primitives", "parity-scale-codec", "scale-info", "sp-core", @@ -6105,23 +6103,6 @@ dependencies = [ "sp-std", ] -[[package]] -name = "pallet-mmr-primitives" -version = "4.0.0-dev" -dependencies = [ - "frame-system", - "hex-literal", - "log 0.4.14", - "parity-scale-codec", - "serde", - "sp-api", - "sp-core", - "sp-debug-derive", - "sp-mmr-primitives", - "sp-runtime", - "sp-std", -] - [[package]] name = "pallet-mmr-rpc" version = "3.0.0" @@ -6129,7 +6110,6 @@ dependencies = [ "jsonrpc-core", "jsonrpc-core-client", "jsonrpc-derive", - "pallet-mmr-primitives", "parity-scale-codec", "serde", "serde_json", @@ -10166,6 +10146,7 @@ dependencies = [ "sp-api", "sp-core", "sp-debug-derive", + "sp-runtime", "sp-std", ] diff --git a/Cargo.toml b/Cargo.toml index 4bfbd7ac138b9..3fea55f433631 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -100,7 +100,6 @@ members = [ "frame/lottery", "frame/membership", "frame/merkle-mountain-range", - "frame/merkle-mountain-range/primitives", "frame/merkle-mountain-range/rpc", "frame/multisig", "frame/nicks", diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index a33b9cf416e1d..3de60e920787d 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -1261,7 +1261,7 @@ impl pallet_mmr::Config for Runtime { const INDEXING_PREFIX: &'static [u8] = b"mmr"; type Hashing = ::Hashing; type Hash = ::Hash; - type LeafData = frame_system::Pallet; + type LeafData = pallet_mmr::ParentNumberAndHash; type OnNewRoot = (); type WeightInfo = (); } @@ -1505,7 +1505,7 @@ pub type Executive = frame_executive::Executive< /// MMR helper types. mod mmr { use super::Runtime; - pub use pallet_mmr::{primitives::*, runtime_primitives::*}; + pub use pallet_mmr::primitives::*; pub type Leaf = <::LeafData as LeafDataProvider>::LeafData; pub type Hash = ::Hash; diff --git a/client/beefy/Cargo.toml b/client/beefy/Cargo.toml index 87c3d5ed71aae..96248249200af 100644 --- a/client/beefy/Cargo.toml +++ b/client/beefy/Cargo.toml @@ -27,7 +27,7 @@ sp-blockchain = { version = "4.0.0-dev", path = "../../primitives/blockchain" } sp-consensus = { version = "0.10.0-dev", path = "../../primitives/consensus/common" } sp-core = { version = "6.0.0", path = "../../primitives/core" } sp-keystore = { version = "0.12.0", path = "../../primitives/keystore" } -sp-mmr-primitives = { version = "4.0.0-dev", default-features = false, path = "../../primitives/merkle-mountain-range" } +sp-mmr-primitives = { version = "4.0.0-dev", path = "../../primitives/merkle-mountain-range" } sp-runtime = { version = "6.0.0", path = "../../primitives/runtime" } sc-chain-spec = { version = "4.0.0-dev", path = "../../client/chain-spec" } diff --git a/client/beefy/src/lib.rs b/client/beefy/src/lib.rs index 315807ce3aac4..5d8aa5c866c4a 100644 --- a/client/beefy/src/lib.rs +++ b/client/beefy/src/lib.rs @@ -27,10 +27,10 @@ use sp_api::ProvideRuntimeApi; use sp_blockchain::HeaderBackend; use sp_consensus::SyncOracle; use sp_keystore::SyncCryptoStorePtr; +use sp_mmr_primitives::MmrApi; use sp_runtime::traits::Block; use beefy_primitives::{BeefyApi, MmrRootHash}; -use sp_mmr_primitives::MmrApi; use crate::notification::{BeefyBestBlockSender, BeefySignedCommitmentSender}; diff --git a/client/beefy/src/tests.rs b/client/beefy/src/tests.rs index b45f2365af824..1601c6af156da 100644 --- a/client/beefy/src/tests.rs +++ b/client/beefy/src/tests.rs @@ -39,7 +39,7 @@ use beefy_primitives::{ crypto::AuthorityId, BeefyApi, ConsensusLog, MmrRootHash, ValidatorSet, BEEFY_ENGINE_ID, KEY_TYPE as BeefyKeyType, }; -use pallet_mmr_primitives::{EncodableOpaqueLeaf, Error as MmrError, LeafIndex, MmrApi, Proof}; +use sp_mmr_primitives::{EncodableOpaqueLeaf, Error as MmrError, LeafIndex, MmrApi, Proof}; use sp_api::{ApiRef, ProvideRuntimeApi}; use sp_consensus::BlockOrigin; diff --git a/frame/beefy-mmr/Cargo.toml b/frame/beefy-mmr/Cargo.toml index 19703fa79863a..a32b1dc6dbd64 100644 --- a/frame/beefy-mmr/Cargo.toml +++ b/frame/beefy-mmr/Cargo.toml @@ -18,7 +18,6 @@ serde = { version = "1.0.136", optional = true } frame-support = { version = "4.0.0-dev", path = "../support", default-features = false } frame-system = { version = "4.0.0-dev", path = "../system", default-features = false } pallet-mmr = { version = "4.0.0-dev", path = "../merkle-mountain-range", default-features = false } -pallet-mmr-primitives = { version = "4.0.0-dev", path = "../merkle-mountain-range/primitives", default-features = false } pallet-session = { version = "4.0.0-dev", path = "../session", default-features = false } sp-core = { version = "6.0.0", path = "../../primitives/core", default-features = false } @@ -46,7 +45,6 @@ std = [ "k256/std", "log/std", "pallet-beefy/std", - "pallet-mmr-primitives/std", "pallet-mmr/std", "pallet-session/std", "serde", diff --git a/frame/beefy-mmr/src/lib.rs b/frame/beefy-mmr/src/lib.rs index 07e4527cf23f6..d10d6bbfaad58 100644 --- a/frame/beefy-mmr/src/lib.rs +++ b/frame/beefy-mmr/src/lib.rs @@ -37,7 +37,7 @@ use sp_runtime::traits::{Convert, Hash, Member}; use sp_std::prelude::*; use beefy_primitives::mmr::{BeefyDataProvider, BeefyNextAuthoritySet, MmrLeaf, MmrLeafVersion}; -use pallet_mmr_primitives::LeafDataProvider; +use pallet_mmr::{LeafDataProvider, ParentNumberAndHash}; use frame_support::traits::Get; @@ -131,7 +131,7 @@ where fn leaf_data() -> Self::LeafData { MmrLeaf { version: T::LeafVersion::get(), - parent_number_and_hash: frame_system::Pallet::::leaf_data(), + parent_number_and_hash: ParentNumberAndHash::::leaf_data(), leaf_extra: T::BeefyDataProvider::extra_data(), beefy_next_authority_set: Pallet::::update_beefy_next_authority_set(), } diff --git a/frame/beefy-mmr/src/tests.rs b/frame/beefy-mmr/src/tests.rs index 12ddf28b3126a..df39be2cd9476 100644 --- a/frame/beefy-mmr/src/tests.rs +++ b/frame/beefy-mmr/src/tests.rs @@ -42,7 +42,7 @@ fn offchain_key(pos: usize) -> Vec { } fn read_mmr_leaf(ext: &mut TestExternalities, index: usize) -> MmrLeaf { - type Node = pallet_mmr_primitives::DataOrHash; + type Node = pallet_mmr::primitives::DataOrHash; ext.persist_offchain_overlay(); let offchain_db = ext.offchain_db(); offchain_db diff --git a/frame/merkle-mountain-range/Cargo.toml b/frame/merkle-mountain-range/Cargo.toml index 2691804d85524..8b67174e8385d 100644 --- a/frame/merkle-mountain-range/Cargo.toml +++ b/frame/merkle-mountain-range/Cargo.toml @@ -26,8 +26,6 @@ frame-benchmarking = { version = "4.0.0-dev", default-features = false, path = " frame-support = { version = "4.0.0-dev", default-features = false, path = "../support" } frame-system = { version = "4.0.0-dev", default-features = false, path = "../system" } -pallet-mmr-primitives = { version = "4.0.0-dev", default-features = false, path = "./primitives" } - [dev-dependencies] env_logger = "0.9" hex-literal = "0.3" @@ -46,7 +44,6 @@ std = [ "frame-benchmarking/std", "frame-support/std", "frame-system/std", - "pallet-mmr-primitives/std", ] runtime-benchmarks = ["frame-benchmarking/runtime-benchmarks"] try-runtime = ["frame-support/try-runtime"] diff --git a/frame/merkle-mountain-range/primitives/Cargo.toml b/frame/merkle-mountain-range/primitives/Cargo.toml deleted file mode 100644 index ed132c328a68e..0000000000000 --- a/frame/merkle-mountain-range/primitives/Cargo.toml +++ /dev/null @@ -1,44 +0,0 @@ -[package] -name = "pallet-mmr-primitives" -version = "4.0.0-dev" -authors = ["Parity Technologies "] -edition = "2021" -license = "Apache-2.0" -homepage = "https://substrate.io" -repository = "https://github.com/paritytech/substrate/" -description = "FRAME Merkle Mountain Range primitives." - -[package.metadata.docs.rs] -targets = ["x86_64-unknown-linux-gnu"] - -[dependencies] -codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false } -log = { version = "0.4.14", default-features = false } -serde = { version = "1.0.136", optional = true, features = ["derive"] } - -sp-api = { version = "4.0.0-dev", default-features = false, path = "../../../primitives/api" } -sp-core = { version = "6.0.0", default-features = false, path = "../../../primitives/core" } -sp-debug-derive = { version = "4.0.0", default-features = false, path = "../../../primitives/debug-derive" } -sp-mmr-primitives = { version = "4.0.0-dev", default-features = false, path = "../../../primitives/merkle-mountain-range" } -sp-runtime = { version = "6.0.0", default-features = false, path = "../../../primitives/runtime" } -sp-std = { version = "4.0.0", default-features = false, path = "../../../primitives/std" } - -frame-system = { version = "4.0.0-dev", default-features = false, path = "../../system" } - -[dev-dependencies] -hex-literal = "0.3" - -[features] -default = ["std"] -std = [ - "codec/std", - "log/std", - "serde", - "sp-api/std", - "sp-core/std", - "sp-debug-derive/std", - "sp-mmr-primitives/std", - "sp-runtime/std", - "sp-std/std", - "frame-system/std", -] diff --git a/frame/merkle-mountain-range/primitives/src/lib.rs b/frame/merkle-mountain-range/primitives/src/lib.rs deleted file mode 100644 index 880f80f29ce13..0000000000000 --- a/frame/merkle-mountain-range/primitives/src/lib.rs +++ /dev/null @@ -1,438 +0,0 @@ -// This file is part of Substrate. - -// Copyright (C) 2020-2022 Parity Technologies (UK) Ltd. -// SPDX-License-Identifier: Apache-2.0 - -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -//! Merkle Mountain Range primitive types. - -#![cfg_attr(not(feature = "std"), no_std)] -#![warn(missing_docs)] - -use sp_debug_derive::RuntimeDebug; -use sp_mmr_primitives::FullLeaf; -use sp_runtime::traits::{self, One, Saturating}; -#[cfg(not(feature = "std"))] -use sp_std::prelude::Vec; - -/// A provider of the MMR's leaf data. -pub trait LeafDataProvider { - /// A type that should end up in the leaf of MMR. - type LeafData: FullLeaf + codec::Decode; - - /// The method to return leaf data that should be placed - /// in the leaf node appended MMR at this block. - /// - /// This is being called by the `on_initialize` method of - /// this pallet at the very beginning of each block. - fn leaf_data() -> Self::LeafData; -} - -impl LeafDataProvider for () { - type LeafData = (); - - fn leaf_data() -> Self::LeafData { - () - } -} - -/// The most common use case for MMRs is to store historical block hashes, -/// so that any point in time in the future we can receive a proof about some past -/// blocks without using excessive on-chain storage. -/// -/// Hence we implement the [LeafDataProvider] for [frame_system::Pallet]. Since the -/// current block hash is not available (since the block is not finished yet), -/// we use the `parent_hash` here along with parent block number. -impl LeafDataProvider for frame_system::Pallet { - type LeafData = (::BlockNumber, ::Hash); - - fn leaf_data() -> Self::LeafData { - (Self::block_number().saturating_sub(One::one()), Self::parent_hash()) - } -} - -/// New MMR root notification hook. -pub trait OnNewRoot { - /// Function called by the pallet in case new MMR root has been computed. - fn on_new_root(root: &Hash); -} - -/// No-op implementation of [OnNewRoot]. -impl OnNewRoot for () { - fn on_new_root(_root: &Hash) {} -} - -/// An element representing either full data or it's hash. -/// -/// See [Compact] to see how it may be used in practice to reduce the size -/// of proofs in case multiple [LeafDataProvider]s are composed together. -/// This is also used internally by the MMR to differentiate leaf nodes (data) -/// and inner nodes (hashes). -/// -/// [DataOrHash::hash] method calculates the hash of this element in it's compact form, -/// so should be used instead of hashing the encoded form (which will always be non-compact). -#[derive(RuntimeDebug, Clone, PartialEq)] -pub enum DataOrHash { - /// Arbitrary data in it's full form. - Data(L), - /// A hash of some data. - Hash(H::Output), -} - -impl From for DataOrHash { - fn from(l: L) -> Self { - Self::Data(l) - } -} - -mod encoding { - use super::*; - - /// A helper type to implement [codec::Codec] for [DataOrHash]. - #[derive(codec::Encode, codec::Decode)] - enum Either { - Left(A), - Right(B), - } - - impl codec::Encode for DataOrHash { - fn encode_to(&self, dest: &mut T) { - match self { - Self::Data(l) => l.using_encoded( - |data| Either::<&[u8], &H::Output>::Left(data).encode_to(dest), - false, - ), - Self::Hash(h) => Either::<&[u8], &H::Output>::Right(h).encode_to(dest), - } - } - } - - impl codec::Decode for DataOrHash { - fn decode(value: &mut I) -> Result { - let decoded: Either, H::Output> = Either::decode(value)?; - Ok(match decoded { - Either::Left(l) => DataOrHash::Data(L::decode(&mut &*l)?), - Either::Right(r) => DataOrHash::Hash(r), - }) - } - } -} - -impl DataOrHash { - /// Retrieve a hash of this item. - /// - /// Depending on the node type it's going to either be a contained value for [DataOrHash::Hash] - /// node, or a hash of SCALE-encoded [DataOrHash::Data] data. - pub fn hash(&self) -> H::Output { - match *self { - Self::Data(ref leaf) => leaf.using_encoded(::hash, true), - Self::Hash(ref hash) => hash.clone(), - } - } -} - -/// A composition of multiple leaf elements with compact form representation. -/// -/// When composing together multiple [LeafDataProvider]s you will end up with -/// a tuple of `LeafData` that each element provides. -/// -/// However this will cause the leaves to have significant size, while for some -/// use cases it will be enough to prove only one element of the tuple. -/// That's the rationale for [Compact] struct. We wrap each element of the tuple -/// into [DataOrHash] and each tuple element is hashed first before constructing -/// the final hash of the entire tuple. This allows you to replace tuple elements -/// you don't care about with their hashes. -#[derive(RuntimeDebug, Clone, PartialEq)] -pub struct Compact { - /// Internal tuple representation. - pub tuple: T, - _hash: sp_std::marker::PhantomData, -} - -impl sp_std::ops::Deref for Compact { - type Target = T; - - fn deref(&self) -> &Self::Target { - &self.tuple - } -} - -impl Compact { - /// Create a new [Compact] wrapper for a tuple. - pub fn new(tuple: T) -> Self { - Self { tuple, _hash: Default::default() } - } -} - -impl codec::Decode for Compact { - fn decode(value: &mut I) -> Result { - T::decode(value).map(Compact::new) - } -} - -macro_rules! impl_leaf_data_for_tuple { - ( $( $name:ident : $id:tt ),+ ) => { - /// [FullLeaf] implementation for `Compact, ...)>` - impl FullLeaf for Compact, )+ )> where - H: traits::Hash, - $( $name: FullLeaf ),+ - { - fn using_encoded R>(&self, f: F, compact: bool) -> R { - if compact { - codec::Encode::using_encoded(&( - $( DataOrHash::::Hash(self.tuple.$id.hash()), )+ - ), f) - } else { - codec::Encode::using_encoded(&self.tuple, f) - } - } - } - - /// [LeafDataProvider] implementation for `Compact, ...)>` - /// - /// This provides a compact-form encoding for tuples wrapped in [Compact]. - impl LeafDataProvider for Compact where - H: traits::Hash, - $( $name: LeafDataProvider ),+ - { - type LeafData = Compact< - H, - ( $( DataOrHash, )+ ), - >; - - fn leaf_data() -> Self::LeafData { - let tuple = ( - $( DataOrHash::Data($name::leaf_data()), )+ - ); - Compact::new(tuple) - } - } - - /// [LeafDataProvider] implementation for `(Tuple, ...)` - /// - /// This provides regular (non-compactable) composition of [LeafDataProvider]s. - impl<$( $name ),+> LeafDataProvider for ( $( $name, )+ ) where - ( $( $name::LeafData, )+ ): FullLeaf, - $( $name: LeafDataProvider ),+ - { - type LeafData = ( $( $name::LeafData, )+ ); - - fn leaf_data() -> Self::LeafData { - ( - $( $name::leaf_data(), )+ - ) - } - } - } -} - -/// Test functions implementation for `Compact, ...)>` -#[cfg(test)] -impl Compact, DataOrHash)> -where - H: traits::Hash, - A: FullLeaf, - B: FullLeaf, -{ - /// Retrieve a hash of this item in it's compact form. - pub fn hash(&self) -> H::Output { - self.using_encoded(::hash, true) - } -} - -impl_leaf_data_for_tuple!(A:0); -impl_leaf_data_for_tuple!(A:0, B:1); -impl_leaf_data_for_tuple!(A:0, B:1, C:2); -impl_leaf_data_for_tuple!(A:0, B:1, C:2, D:3); -impl_leaf_data_for_tuple!(A:0, B:1, C:2, D:3, E:4); - -#[cfg(test)] -mod tests { - use super::*; - - use codec::Decode; - use sp_core::H256; - use sp_runtime::traits::Keccak256; - - pub(crate) fn hex(s: &str) -> H256 { - s.parse().unwrap() - } - - type Test = DataOrHash; - type TestCompact = Compact; - type TestProof = Proof<::Output>; - - #[test] - fn should_encode_decode_proof() { - // given - let proof: TestProof = Proof { - leaf_index: 5, - leaf_count: 10, - items: vec![ - hex("c3e7ba6b511162fead58f2c8b5764ce869ed1118011ac37392522ed16720bbcd"), - hex("d3e7ba6b511162fead58f2c8b5764ce869ed1118011ac37392522ed16720bbcd"), - hex("e3e7ba6b511162fead58f2c8b5764ce869ed1118011ac37392522ed16720bbcd"), - ], - }; - - // when - let encoded = codec::Encode::encode(&proof); - let decoded = TestProof::decode(&mut &*encoded); - - // then - assert_eq!(decoded, Ok(proof)); - } - - #[test] - fn should_encode_decode_correctly_if_no_compact() { - // given - let cases = vec![ - Test::Data("Hello World!".into()), - Test::Hash(hex("c3e7ba6b511162fead58f2c8b5764ce869ed1118011ac37392522ed16720bbcd")), - Test::Data("".into()), - Test::Data("3e48d6bcd417fb22e044747242451e2c0f3e602d1bcad2767c34808621956417".into()), - ]; - - // when - let encoded = cases.iter().map(codec::Encode::encode).collect::>(); - - let decoded = encoded.iter().map(|x| Test::decode(&mut &**x)).collect::>(); - - // then - assert_eq!( - decoded, - cases.into_iter().map(Result::<_, codec::Error>::Ok).collect::>() - ); - // check encoding correctness - assert_eq!(&encoded[0], &hex_literal::hex!("00343048656c6c6f20576f726c6421")); - assert_eq!( - encoded[1].as_slice(), - hex_literal::hex!("01c3e7ba6b511162fead58f2c8b5764ce869ed1118011ac37392522ed16720bbcd") - .as_ref() - ); - } - - #[test] - fn should_return_the_hash_correctly() { - // given - let a = Test::Data("Hello World!".into()); - let b = Test::Hash(hex("c3e7ba6b511162fead58f2c8b5764ce869ed1118011ac37392522ed16720bbcd")); - - // when - let a = a.hash(); - let b = b.hash(); - - // then - assert_eq!(a, hex("a9c321be8c24ba4dc2bd73f5300bde67dc57228ab8b68b607bb4c39c5374fac9")); - assert_eq!(b, hex("c3e7ba6b511162fead58f2c8b5764ce869ed1118011ac37392522ed16720bbcd")); - } - - #[test] - fn compact_should_work() { - // given - let a = Test::Data("Hello World!".into()); - let b = Test::Data("".into()); - - // when - let c: TestCompact = Compact::new((a.clone(), b.clone())); - let d: TestCompact = Compact::new((Test::Hash(a.hash()), Test::Hash(b.hash()))); - - // then - assert_eq!(c.hash(), d.hash()); - } - - #[test] - fn compact_should_encode_decode_correctly() { - // given - let a = Test::Data("Hello World!".into()); - let b = Test::Data("".into()); - - let c: TestCompact = Compact::new((a.clone(), b.clone())); - let d: TestCompact = Compact::new((Test::Hash(a.hash()), Test::Hash(b.hash()))); - let cases = vec![c, d.clone()]; - - // when - let encoded_compact = - cases.iter().map(|c| c.using_encoded(|x| x.to_vec(), true)).collect::>(); - - let encoded = - cases.iter().map(|c| c.using_encoded(|x| x.to_vec(), false)).collect::>(); - - let decoded_compact = encoded_compact - .iter() - .map(|x| TestCompact::decode(&mut &**x)) - .collect::>(); - - let decoded = encoded.iter().map(|x| TestCompact::decode(&mut &**x)).collect::>(); - - // then - assert_eq!( - decoded, - cases.into_iter().map(Result::<_, codec::Error>::Ok).collect::>() - ); - - assert_eq!(decoded_compact, vec![Ok(d.clone()), Ok(d.clone())]); - } - - #[test] - fn opaque_leaves_should_be_full_leaf_compatible() { - // given - let a = Test::Data("Hello World!".into()); - let b = Test::Data("".into()); - - let c: TestCompact = Compact::new((a.clone(), b.clone())); - let d: TestCompact = Compact::new((Test::Hash(a.hash()), Test::Hash(b.hash()))); - let cases = vec![c, d.clone()]; - - let encoded_compact = cases - .iter() - .map(|c| c.using_encoded(|x| x.to_vec(), true)) - .map(OpaqueLeaf::from_encoded_leaf) - .collect::>(); - - let opaque = cases.iter().map(OpaqueLeaf::from_leaf).collect::>(); - - // then - assert_eq!(encoded_compact, opaque); - } - - #[test] - fn encode_opaque_leaf_should_be_scale_compatible() { - use codec::Encode; - - // given - let a = Test::Data("Hello World!".into()); - let case1 = EncodableOpaqueLeaf::from_leaf(&a); - let case2 = EncodableOpaqueLeaf::from_opaque_leaf(OpaqueLeaf(a.encode())); - let case3 = a.encode().encode(); - - // when - let encoded = vec![&case1, &case2].into_iter().map(|x| x.encode()).collect::>(); - let decoded = vec![&*encoded[0], &*encoded[1], &*case3] - .into_iter() - .map(|x| EncodableOpaqueLeaf::decode(&mut &*x)) - .collect::>(); - - // then - assert_eq!(case1, case2); - assert_eq!(encoded[0], encoded[1]); - // then encoding should also match double-encoded leaf. - assert_eq!(encoded[0], case3); - - assert_eq!(decoded[0], decoded[1]); - assert_eq!(decoded[1], decoded[2]); - assert_eq!(decoded[0], Ok(case2)); - assert_eq!(decoded[1], Ok(case1)); - } -} diff --git a/frame/merkle-mountain-range/rpc/Cargo.toml b/frame/merkle-mountain-range/rpc/Cargo.toml index fd5e955064658..94c895ea91517 100644 --- a/frame/merkle-mountain-range/rpc/Cargo.toml +++ b/frame/merkle-mountain-range/rpc/Cargo.toml @@ -25,7 +25,5 @@ sp-core = { version = "6.0.0", path = "../../../primitives/core" } sp-mmr-primitives = { version = "4.0.0-dev", default-features = false, path = "../../../primitives/merkle-mountain-range" } sp-runtime = { version = "6.0.0", path = "../../../primitives/runtime" } -pallet-mmr-primitives = { version = "4.0.0-dev", path = "../primitives" } - [dev-dependencies] serde_json = "1.0.79" diff --git a/frame/merkle-mountain-range/rpc/src/lib.rs b/frame/merkle-mountain-range/rpc/src/lib.rs index 7de5c3a5e6d53..fd957e9783706 100644 --- a/frame/merkle-mountain-range/rpc/src/lib.rs +++ b/frame/merkle-mountain-range/rpc/src/lib.rs @@ -43,7 +43,7 @@ pub struct LeafProof { pub block_hash: BlockHash, /// SCALE-encoded leaf data. pub leaf: Bytes, - /// SCALE-encoded proof data. See [pallet_mmr_primitives::Proof]. + /// SCALE-encoded proof data. See [sp_mmr_primitives::Proof]. pub proof: Bytes, } diff --git a/frame/merkle-mountain-range/src/lib.rs b/frame/merkle-mountain-range/src/lib.rs index f04edf545bf2a..3b8e5c1d64e0d 100644 --- a/frame/merkle-mountain-range/src/lib.rs +++ b/frame/merkle-mountain-range/src/lib.rs @@ -58,7 +58,7 @@ use codec::Encode; use frame_support::weights::Weight; -use sp_runtime::traits; +use sp_runtime::traits::{self, One, Saturating}; #[cfg(any(feature = "runtime-benchmarks", test))] mod benchmarking; @@ -70,8 +70,29 @@ mod mock; mod tests; pub use pallet::*; -pub use pallet_mmr_primitives as runtime_primitives; -pub use sp_mmr_primitives::{self as primitives, Error, LeafIndex, NodeIndex}; +pub use sp_mmr_primitives::{self as primitives, Error, LeafDataProvider, LeafIndex, NodeIndex}; + +/// The most common use case for MMRs is to store historical block hashes, +/// so that any point in time in the future we can receive a proof about some past +/// blocks without using excessive on-chain storage. +/// +/// Hence we implement the [LeafDataProvider] for [ParentNumberAndHash] which is a +/// crate-local wrapper over [frame_system::Pallet]. Since the current block hash +/// is not available (since the block is not finished yet), +/// we use the `parent_hash` here along with parent block number. +pub struct ParentNumberAndHash { + _phanthom: sp_std::marker::PhantomData, +} +impl LeafDataProvider for ParentNumberAndHash { + type LeafData = (::BlockNumber, ::Hash); + + fn leaf_data() -> Self::LeafData { + ( + frame_system::Pallet::::block_number().saturating_sub(One::one()), + frame_system::Pallet::::parent_hash(), + ) + } +} pub trait WeightInfo { fn on_initialize(peaks: NodeIndex) -> Weight; @@ -139,7 +160,7 @@ pub mod pallet { /// /// Note that the leaf at each block MUST be unique. You may want to include a block hash or /// block number as an easiest way to ensure that. - type LeafData: runtime_primitives::LeafDataProvider; + type LeafData: primitives::LeafDataProvider; /// A hook to act on the new MMR root. /// @@ -176,7 +197,7 @@ pub mod pallet { #[pallet::hooks] impl, I: 'static> Hooks> for Pallet { fn on_initialize(_n: T::BlockNumber) -> Weight { - use runtime_primitives::LeafDataProvider; + use primitives::LeafDataProvider; let leaves = Self::mmr_leaves(); let peaks_before = mmr::utils::NodesUtils::new(leaves).number_of_peaks(); let data = T::LeafData::leaf_data(); @@ -201,7 +222,7 @@ pub mod pallet { type ModuleMmr = mmr::Mmr>; /// Leaf data. -type LeafOf = <>::LeafData as runtime_primitives::LeafDataProvider>::LeafData; +type LeafOf = <>::LeafData as primitives::LeafDataProvider>::LeafData; /// Hashing used for the pallet. pub(crate) type HashingOf = >::Hashing; diff --git a/frame/merkle-mountain-range/src/mmr/mod.rs b/frame/merkle-mountain-range/src/mmr/mod.rs index f877a9729443e..1cb4e8535b991 100644 --- a/frame/merkle-mountain-range/src/mmr/mod.rs +++ b/frame/merkle-mountain-range/src/mmr/mod.rs @@ -19,7 +19,7 @@ mod mmr; pub mod storage; pub mod utils; -use sp_mmr_primitives::FullLeaf; +use sp_mmr_primitives::{DataOrHash, FullLeaf}; use sp_runtime::traits; pub use self::mmr::{verify_leaf_proof, Mmr}; @@ -28,7 +28,7 @@ pub use self::mmr::{verify_leaf_proof, Mmr}; pub type NodeOf = Node<>::Hashing, L>; /// A node stored in the MMR. -pub type Node = pallet_mmr_primitives::DataOrHash; +pub type Node = DataOrHash; /// Default Merging & Hashing behavior for MMR. pub struct Hasher(sp_std::marker::PhantomData<(H, L)>); diff --git a/frame/merkle-mountain-range/src/mock.rs b/frame/merkle-mountain-range/src/mock.rs index 56d3c9c0d77d8..b2b6821fcd054 100644 --- a/frame/merkle-mountain-range/src/mock.rs +++ b/frame/merkle-mountain-range/src/mock.rs @@ -20,8 +20,8 @@ use crate::*; use codec::{Decode, Encode}; use frame_support::traits::{ConstU32, ConstU64}; -use pallet_mmr_primitives::{Compact, LeafDataProvider}; use sp_core::H256; +use sp_mmr_primitives::{Compact, LeafDataProvider}; use sp_runtime::{ testing::Header, traits::{BlakeTwo256, IdentityLookup, Keccak256}, @@ -74,7 +74,7 @@ impl Config for Test { type Hashing = Keccak256; type Hash = H256; - type LeafData = Compact, LeafData)>; + type LeafData = Compact, LeafData)>; type OnNewRoot = (); type WeightInfo = (); } diff --git a/frame/merkle-mountain-range/src/tests.rs b/frame/merkle-mountain-range/src/tests.rs index 576a7ace8f1c0..70d1395aa94d5 100644 --- a/frame/merkle-mountain-range/src/tests.rs +++ b/frame/merkle-mountain-range/src/tests.rs @@ -19,11 +19,11 @@ use crate::{mmr::utils, mock::*, *}; use frame_support::traits::OnInitialize; use mmr_lib::helper; -use pallet_mmr_primitives::{Compact, Proof}; use sp_core::{ offchain::{testing::TestOffchainExt, OffchainDbExt, OffchainWorkerExt}, H256, }; +use sp_mmr_primitives::{Compact, Proof}; pub(crate) fn new_test_ext() -> sp_io::TestExternalities { frame_system::GenesisConfig::default().build_storage::().unwrap().into() diff --git a/primitives/merkle-mountain-range/Cargo.toml b/primitives/merkle-mountain-range/Cargo.toml index 35c0fdd8057f3..ddb820f9e6cc4 100644 --- a/primitives/merkle-mountain-range/Cargo.toml +++ b/primitives/merkle-mountain-range/Cargo.toml @@ -19,6 +19,7 @@ serde = { version = "1.0.136", optional = true, features = ["derive"] } sp-api = { version = "4.0.0-dev", default-features = false, path = "../api" } sp-core = { version = "6.0.0", default-features = false, path = "../core" } sp-debug-derive = { version = "4.0.0", default-features = false, path = "../debug-derive" } +sp-runtime = { version = "6.0.0", default-features = false, path = "../runtime" } sp-std = { version = "4.0.0", default-features = false, path = "../std" } [dev-dependencies] @@ -33,5 +34,6 @@ std = [ "sp-api/std", "sp-core/std", "sp-debug-derive/std", + "sp-runtime/std", "sp-std/std", ] diff --git a/primitives/merkle-mountain-range/src/lib.rs b/primitives/merkle-mountain-range/src/lib.rs index 936d9d78d31a4..a27536b8d35b7 100644 --- a/primitives/merkle-mountain-range/src/lib.rs +++ b/primitives/merkle-mountain-range/src/lib.rs @@ -21,6 +21,7 @@ #![warn(missing_docs)] use sp_debug_derive::RuntimeDebug; +use sp_runtime::traits; use sp_std::fmt; #[cfg(not(feature = "std"))] use sp_std::prelude::Vec; @@ -35,6 +36,38 @@ pub type NodeIndex = u64; /// but might be actually at different positions in the MMR `NodeIndex`. pub type LeafIndex = u64; +/// A provider of the MMR's leaf data. +pub trait LeafDataProvider { + /// A type that should end up in the leaf of MMR. + type LeafData: FullLeaf + codec::Decode; + + /// The method to return leaf data that should be placed + /// in the leaf node appended MMR at this block. + /// + /// This is being called by the `on_initialize` method of + /// this pallet at the very beginning of each block. + fn leaf_data() -> Self::LeafData; +} + +impl LeafDataProvider for () { + type LeafData = (); + + fn leaf_data() -> Self::LeafData { + () + } +} + +/// New MMR root notification hook. +pub trait OnNewRoot { + /// Function called by the pallet in case new MMR root has been computed. + fn on_new_root(root: &Hash); +} + +/// No-op implementation of [OnNewRoot]. +impl OnNewRoot for () { + fn on_new_root(_root: &Hash) {} +} + /// A MMR proof data for one of the leaves. #[derive(codec::Encode, codec::Decode, RuntimeDebug, Clone, PartialEq, Eq)] pub struct Proof { @@ -134,6 +167,190 @@ impl EncodableOpaqueLeaf { } } +/// An element representing either full data or it's hash. +/// +/// See [Compact] to see how it may be used in practice to reduce the size +/// of proofs in case multiple [LeafDataProvider]s are composed together. +/// This is also used internally by the MMR to differentiate leaf nodes (data) +/// and inner nodes (hashes). +/// +/// [DataOrHash::hash] method calculates the hash of this element in it's compact form, +/// so should be used instead of hashing the encoded form (which will always be non-compact). +#[derive(RuntimeDebug, Clone, PartialEq)] +pub enum DataOrHash { + /// Arbitrary data in it's full form. + Data(L), + /// A hash of some data. + Hash(H::Output), +} + +impl From for DataOrHash { + fn from(l: L) -> Self { + Self::Data(l) + } +} + +mod encoding { + use super::*; + + /// A helper type to implement [codec::Codec] for [DataOrHash]. + #[derive(codec::Encode, codec::Decode)] + enum Either { + Left(A), + Right(B), + } + + impl codec::Encode for DataOrHash { + fn encode_to(&self, dest: &mut T) { + match self { + Self::Data(l) => l.using_encoded( + |data| Either::<&[u8], &H::Output>::Left(data).encode_to(dest), + false, + ), + Self::Hash(h) => Either::<&[u8], &H::Output>::Right(h).encode_to(dest), + } + } + } + + impl codec::Decode for DataOrHash { + fn decode(value: &mut I) -> Result { + let decoded: Either, H::Output> = Either::decode(value)?; + Ok(match decoded { + Either::Left(l) => DataOrHash::Data(L::decode(&mut &*l)?), + Either::Right(r) => DataOrHash::Hash(r), + }) + } + } +} + +impl DataOrHash { + /// Retrieve a hash of this item. + /// + /// Depending on the node type it's going to either be a contained value for [DataOrHash::Hash] + /// node, or a hash of SCALE-encoded [DataOrHash::Data] data. + pub fn hash(&self) -> H::Output { + match *self { + Self::Data(ref leaf) => leaf.using_encoded(::hash, true), + Self::Hash(ref hash) => hash.clone(), + } + } +} + +/// A composition of multiple leaf elements with compact form representation. +/// +/// When composing together multiple [LeafDataProvider]s you will end up with +/// a tuple of `LeafData` that each element provides. +/// +/// However this will cause the leaves to have significant size, while for some +/// use cases it will be enough to prove only one element of the tuple. +/// That's the rationale for [Compact] struct. We wrap each element of the tuple +/// into [DataOrHash] and each tuple element is hashed first before constructing +/// the final hash of the entire tuple. This allows you to replace tuple elements +/// you don't care about with their hashes. +#[derive(RuntimeDebug, Clone, PartialEq)] +pub struct Compact { + /// Internal tuple representation. + pub tuple: T, + _hash: sp_std::marker::PhantomData, +} + +impl sp_std::ops::Deref for Compact { + type Target = T; + + fn deref(&self) -> &Self::Target { + &self.tuple + } +} + +impl Compact { + /// Create a new [Compact] wrapper for a tuple. + pub fn new(tuple: T) -> Self { + Self { tuple, _hash: Default::default() } + } +} + +impl codec::Decode for Compact { + fn decode(value: &mut I) -> Result { + T::decode(value).map(Compact::new) + } +} + +macro_rules! impl_leaf_data_for_tuple { + ( $( $name:ident : $id:tt ),+ ) => { + /// [FullLeaf] implementation for `Compact, ...)>` + impl FullLeaf for Compact, )+ )> where + H: traits::Hash, + $( $name: FullLeaf ),+ + { + fn using_encoded R>(&self, f: F, compact: bool) -> R { + if compact { + codec::Encode::using_encoded(&( + $( DataOrHash::::Hash(self.tuple.$id.hash()), )+ + ), f) + } else { + codec::Encode::using_encoded(&self.tuple, f) + } + } + } + + /// [LeafDataProvider] implementation for `Compact, ...)>` + /// + /// This provides a compact-form encoding for tuples wrapped in [Compact]. + impl LeafDataProvider for Compact where + H: traits::Hash, + $( $name: LeafDataProvider ),+ + { + type LeafData = Compact< + H, + ( $( DataOrHash, )+ ), + >; + + fn leaf_data() -> Self::LeafData { + let tuple = ( + $( DataOrHash::Data($name::leaf_data()), )+ + ); + Compact::new(tuple) + } + } + + /// [LeafDataProvider] implementation for `(Tuple, ...)` + /// + /// This provides regular (non-compactable) composition of [LeafDataProvider]s. + impl<$( $name ),+> LeafDataProvider for ( $( $name, )+ ) where + ( $( $name::LeafData, )+ ): FullLeaf, + $( $name: LeafDataProvider ),+ + { + type LeafData = ( $( $name::LeafData, )+ ); + + fn leaf_data() -> Self::LeafData { + ( + $( $name::leaf_data(), )+ + ) + } + } + } +} + +/// Test functions implementation for `Compact, ...)>` +#[cfg(test)] +impl Compact, DataOrHash)> +where + H: traits::Hash, + A: FullLeaf, + B: FullLeaf, +{ + /// Retrieve a hash of this item in it's compact form. + pub fn hash(&self) -> H::Output { + self.using_encoded(::hash, true) + } +} + +impl_leaf_data_for_tuple!(A:0); +impl_leaf_data_for_tuple!(A:0, B:1); +impl_leaf_data_for_tuple!(A:0, B:1, C:2); +impl_leaf_data_for_tuple!(A:0, B:1, C:2, D:3); +impl_leaf_data_for_tuple!(A:0, B:1, C:2, D:3, E:4); + /// Merkle Mountain Range operation error. #[derive(RuntimeDebug, codec::Encode, codec::Decode, PartialEq, Eq)] pub enum Error { @@ -203,17 +420,6 @@ sp_api::decl_runtime_apis! { } } -/// New MMR root notification hook. -pub trait OnNewRoot { - /// Function called by the pallet in case new MMR root has been computed. - fn on_new_root(root: &Hash); -} - -/// No-op implementation of [OnNewRoot]. -impl OnNewRoot for () { - fn on_new_root(_root: &Hash) {} -} - #[cfg(test)] mod tests { use super::*; From f0b02fb5beb62e6119982a3fdcc997805a4bf41b Mon Sep 17 00:00:00 2001 From: acatangiu Date: Fri, 8 Apr 2022 13:48:47 +0300 Subject: [PATCH 08/11] address review comments --- client/beefy/src/tests.rs | 1 - client/beefy/src/worker.rs | 2 +- frame/merkle-mountain-range/rpc/src/lib.rs | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/client/beefy/src/tests.rs b/client/beefy/src/tests.rs index 1601c6af156da..96039c8110f4f 100644 --- a/client/beefy/src/tests.rs +++ b/client/beefy/src/tests.rs @@ -610,7 +610,6 @@ fn correct_beefy_payload() { // verify consensus is _not_ reached let timeout = Some(Duration::from_millis(500)); - println!("\n\n-------------------\n"); streams_empty_after_timeout(best_blocks, &net, &mut runtime, timeout); streams_empty_after_timeout(signed_commitments, &net, &mut runtime, None); diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index ffca30b0a20d7..24cf21cd7f674 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -520,7 +520,7 @@ where } } - /// Simple wrapper over mmr root extraction. Tries from header digest, then from client state. + /// Simple wrapper that gets MMR root from client state. fn get_mmr_root_digest(&self, header: &B::Header) -> Option { self.runtime .runtime_api() diff --git a/frame/merkle-mountain-range/rpc/src/lib.rs b/frame/merkle-mountain-range/rpc/src/lib.rs index fd957e9783706..99359bfea8eb6 100644 --- a/frame/merkle-mountain-range/rpc/src/lib.rs +++ b/frame/merkle-mountain-range/rpc/src/lib.rs @@ -32,7 +32,6 @@ use sp_core::Bytes; use sp_mmr_primitives::{Error as MmrError, LeafIndex, Proof}; use sp_runtime::{generic::BlockId, traits::Block as BlockT}; -// pub use pallet_mmr_primitives::{LeafIndex, MmrApi as MmrRuntimeApi}; pub use sp_mmr_primitives::MmrApi as MmrRuntimeApi; /// Retrieved MMR leaf and its proof. From 9363e48aec3ba6aeb854fca426ad59f9042f97d2 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Mon, 11 Apr 2022 14:57:10 +0300 Subject: [PATCH 09/11] beefy-mmr: bring back mmr root from header digest --- client/beefy/src/tests.rs | 21 ++++++++-- client/beefy/src/worker.rs | 56 ++++++++++++++++++++++---- frame/beefy-mmr/src/lib.rs | 19 +++++++++ frame/beefy-mmr/src/mock.rs | 2 +- frame/beefy-mmr/src/tests.rs | 44 +++++++++++++++++++- frame/merkle-mountain-range/src/lib.rs | 1 + 6 files changed, 129 insertions(+), 14 deletions(-) diff --git a/client/beefy/src/tests.rs b/client/beefy/src/tests.rs index 96039c8110f4f..e568daba8e112 100644 --- a/client/beefy/src/tests.rs +++ b/client/beefy/src/tests.rs @@ -138,10 +138,18 @@ impl BeefyTestNet { count: usize, session_length: u64, validator_set: &BeefyValidatorSet, + include_mmr_digest: bool, ) { self.peer(0).generate_blocks(count, BlockOrigin::File, |builder| { let mut block = builder.build().unwrap().block; + if include_mmr_digest { + let block_num = *block.header.number(); + let num_byte = block_num.to_le_bytes().into_iter().next().unwrap(); + let mmr_root = MmrRootHash::repeat_byte(num_byte); + add_mmr_digest(&mut block.header, mmr_root); + } + if *block.header.number() % session_length == 0 { add_auth_change_digest(&mut block.header, validator_set.clone()); } @@ -275,6 +283,13 @@ create_test_api!( BeefyKeyring::Dave ); +fn add_mmr_digest(header: &mut Header, mmr_hash: MmrRootHash) { + header.digest_mut().push(DigestItem::Consensus( + BEEFY_ENGINE_ID, + ConsensusLog::::MmrRoot(mmr_hash).encode(), + )); +} + fn add_auth_change_digest(header: &mut Header, new_auth_set: BeefyValidatorSet) { header.digest_mut().push(DigestItem::Consensus( BEEFY_ENGINE_ID, @@ -482,7 +497,7 @@ fn beefy_finalizing_blocks() { runtime.spawn(initialize_beefy(&mut net, beefy_peers, min_block_delta)); // push 42 blocks including `AuthorityChange` digests every 10 blocks. - net.generate_blocks(42, session_len, &validator_set); + net.generate_blocks(42, session_len, &validator_set, true); net.block_until_sync(); let net = Arc::new(Mutex::new(net)); @@ -521,7 +536,7 @@ fn lagging_validators() { runtime.spawn(initialize_beefy(&mut net, beefy_peers, min_block_delta)); // push 42 blocks including `AuthorityChange` digests every 30 blocks. - net.generate_blocks(42, session_len, &validator_set); + net.generate_blocks(42, session_len, &validator_set, true); net.block_until_sync(); let net = Arc::new(Mutex::new(net)); @@ -578,7 +593,7 @@ fn correct_beefy_payload() { runtime.spawn(initialize_beefy(&mut net, bad_peers, min_block_delta)); // push 10 blocks - net.generate_blocks(12, session_len, &validator_set); + net.generate_blocks(12, session_len, &validator_set, false); net.block_until_sync(); let net = Arc::new(Mutex::new(net)); diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index 24cf21cd7f674..c17698e1e544c 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -520,17 +520,33 @@ where } } - /// Simple wrapper that gets MMR root from client state. + /// Simple wrapper that gets MMR root from header digests or from client state. fn get_mmr_root_digest(&self, header: &B::Header) -> Option { - self.runtime - .runtime_api() - .mmr_root(&BlockId::hash(header.hash())) - .ok() - .map(|r| r.ok()) - .flatten() + find_mmr_root_digest::(header).or_else(|| { + self.runtime + .runtime_api() + .mmr_root(&BlockId::hash(header.hash())) + .ok() + .map(|r| r.ok()) + .flatten() + }) } } +/// Extract the MMR root hash from a digest in the given header, if it exists. +fn find_mmr_root_digest(header: &B::Header) -> Option +where + B: Block, +{ + let id = OpaqueDigestItemId::Consensus(&BEEFY_ENGINE_ID); + + let filter = |log: ConsensusLog| match log { + ConsensusLog::MmrRoot(root) => Some(root), + _ => None, + }; + header.digest().convert_first(|l| l.try_to(id).and_then(filter)) +} + /// Scan the `header` digest log for a BEEFY validator set change. Return either the new /// validator set or `None` in case no validator set change has been signaled. fn find_authorities_change(header: &B::Header) -> Option> @@ -781,6 +797,30 @@ pub(crate) mod tests { assert_eq!(extracted, Some(validator_set)); } + #[test] + fn extract_mmr_root_digest() { + let mut header = Header::new( + 1u32.into(), + Default::default(), + Default::default(), + Default::default(), + Digest::default(), + ); + + // verify empty digest shows nothing + assert!(find_mmr_root_digest::(&header).is_none()); + + let mmr_root_hash = H256::random(); + header.digest_mut().push(DigestItem::Consensus( + BEEFY_ENGINE_ID, + ConsensusLog::::MmrRoot(mmr_root_hash.clone()).encode(), + )); + + // verify validator set is correctly extracted from digest + let extracted = find_mmr_root_digest::(&header); + assert_eq!(extracted, Some(mmr_root_hash)); + } + #[test] fn should_vote_target() { let keys = &[Keyring::Alice]; @@ -898,7 +938,7 @@ pub(crate) mod tests { // generate 2 blocks, try again expect success let (mut best_block_streams, _) = get_beefy_streams(&mut net, keys); let mut best_block_stream = best_block_streams.drain(..).next().unwrap(); - net.generate_blocks(2, 10, &validator_set); + net.generate_blocks(2, 10, &validator_set, false); worker.set_best_beefy_block(2); assert_eq!(worker.best_beefy_block, Some(2)); diff --git a/frame/beefy-mmr/src/lib.rs b/frame/beefy-mmr/src/lib.rs index d10d6bbfaad58..640ebeb7d49cc 100644 --- a/frame/beefy-mmr/src/lib.rs +++ b/frame/beefy-mmr/src/lib.rs @@ -48,6 +48,25 @@ mod mock; #[cfg(test)] mod tests; +/// A BEEFY consensus digest item with MMR root hash. +pub struct DepositBeefyDigest(sp_std::marker::PhantomData); + +impl pallet_mmr::primitives::OnNewRoot for DepositBeefyDigest +where + T: pallet_mmr::Config, + T: pallet_beefy::Config, +{ + fn on_new_root(root: &::Hash) { + let digest = sp_runtime::generic::DigestItem::Consensus( + beefy_primitives::BEEFY_ENGINE_ID, + codec::Encode::encode(&beefy_primitives::ConsensusLog::< + ::BeefyId, + >::MmrRoot(*root)), + ); + >::deposit_log(digest); + } +} + /// Convert BEEFY secp256k1 public keys into Ethereum addresses pub struct BeefyEcdsaToEthereum; impl Convert> for BeefyEcdsaToEthereum { diff --git a/frame/beefy-mmr/src/mock.rs b/frame/beefy-mmr/src/mock.rs index bb426471b50a3..f6a35f68a4a1f 100644 --- a/frame/beefy-mmr/src/mock.rs +++ b/frame/beefy-mmr/src/mock.rs @@ -117,7 +117,7 @@ impl pallet_mmr::Config for Test { type LeafData = BeefyMmr; - type OnNewRoot = (); + type OnNewRoot = pallet_beefy_mmr::DepositBeefyDigest; type WeightInfo = (); } diff --git a/frame/beefy-mmr/src/tests.rs b/frame/beefy-mmr/src/tests.rs index df39be2cd9476..37f571cd842ee 100644 --- a/frame/beefy-mmr/src/tests.rs +++ b/frame/beefy-mmr/src/tests.rs @@ -17,13 +17,16 @@ use std::vec; -use beefy_primitives::mmr::{BeefyNextAuthoritySet, MmrLeafVersion}; +use beefy_primitives::{ + mmr::{BeefyNextAuthoritySet, MmrLeafVersion}, + ValidatorSet, +}; use codec::{Decode, Encode}; use hex_literal::hex; use sp_core::H256; use sp_io::TestExternalities; -use sp_runtime::traits::Keccak256; +use sp_runtime::{traits::Keccak256, DigestItem}; use frame_support::traits::OnInitialize; @@ -37,6 +40,10 @@ fn init_block(block: u64) { BeefyMmr::on_initialize(block); } +pub fn beefy_log(log: ConsensusLog) -> DigestItem { + DigestItem::Consensus(BEEFY_ENGINE_ID, log.encode()) +} + fn offchain_key(pos: usize) -> Vec { (::INDEXING_PREFIX, pos as u64).encode() } @@ -55,6 +62,39 @@ fn read_mmr_leaf(ext: &mut TestExternalities, index: usize) -> MmrLeaf { .unwrap() } +#[test] +fn should_contain_mmr_digest() { + let mut ext = new_test_ext(vec![1, 2, 3, 4]); + ext.execute_with(|| { + init_block(1); + + assert_eq!( + System::digest().logs, + vec![beefy_log(ConsensusLog::MmrRoot( + hex!("fa0275b19b2565089f7e2377ee73b9050e8d53bce108ef722a3251fd9d371d4b").into() + ))] + ); + + // unique every time + init_block(2); + + assert_eq!( + System::digest().logs, + vec![ + beefy_log(ConsensusLog::MmrRoot( + hex!("fa0275b19b2565089f7e2377ee73b9050e8d53bce108ef722a3251fd9d371d4b").into() + )), + beefy_log(ConsensusLog::AuthoritiesChange( + ValidatorSet::new(vec![mock_beefy_id(3), mock_beefy_id(4),], 1,).unwrap() + )), + beefy_log(ConsensusLog::MmrRoot( + hex!("85554fa7d4e863cce3cdce668c1ae82c0174ad37f8d1399284018bec9f9971c3").into() + )), + ] + ); + }); +} + #[test] fn should_contain_valid_leaf_data() { let mut ext = new_test_ext(vec![1, 2, 3, 4]); diff --git a/frame/merkle-mountain-range/src/lib.rs b/frame/merkle-mountain-range/src/lib.rs index 3b8e5c1d64e0d..855eb0a7436dc 100644 --- a/frame/merkle-mountain-range/src/lib.rs +++ b/frame/merkle-mountain-range/src/lib.rs @@ -83,6 +83,7 @@ pub use sp_mmr_primitives::{self as primitives, Error, LeafDataProvider, LeafInd pub struct ParentNumberAndHash { _phanthom: sp_std::marker::PhantomData, } + impl LeafDataProvider for ParentNumberAndHash { type LeafData = (::BlockNumber, ::Hash); From d9e7473cfdfc510424aff0485968c3068ffda22c Mon Sep 17 00:00:00 2001 From: acatangiu Date: Mon, 11 Apr 2022 18:40:12 +0300 Subject: [PATCH 10/11] clippy fixes for rustc 1.60 --- Cargo.lock | 2 +- client/beefy/src/worker.rs | 3 +-- client/db/src/lib.rs | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f26a98cc08528..f46fa5fadc6b6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9090,7 +9090,7 @@ dependencies = [ name = "sc-sysinfo" version = "6.0.0-dev" dependencies = [ - "futures 0.3.19", + "futures 0.3.21", "libc", "log 0.4.14", "rand 0.7.3", diff --git a/client/beefy/src/worker.rs b/client/beefy/src/worker.rs index c17698e1e544c..128850a5f172f 100644 --- a/client/beefy/src/worker.rs +++ b/client/beefy/src/worker.rs @@ -527,8 +527,7 @@ where .runtime_api() .mmr_root(&BlockId::hash(header.hash())) .ok() - .map(|r| r.ok()) - .flatten() + .and_then(|r| r.ok()) }) } } diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index 6a8a025f1f45f..c133dbf44247e 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -107,7 +107,7 @@ pub type DbState = sp_state_machine::TrieBackend>>, HashFor>; /// Length of a [`DbHash`]. -const DB_HASH_LEN: usize = 32; +pub const DB_HASH_LEN: usize = 32; /// Hash type that this backend uses for the database. pub type DbHash = sp_core::H256; From 088de688a26e51303ecf691c8640f6022d770861 Mon Sep 17 00:00:00 2001 From: acatangiu Date: Mon, 11 Apr 2022 19:25:42 +0300 Subject: [PATCH 11/11] address review comments --- client/db/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index c133dbf44247e..4f504651f952c 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -106,8 +106,9 @@ const DEFAULT_CHILD_RATIO: (usize, usize) = (1, 10); pub type DbState = sp_state_machine::TrieBackend>>, HashFor>; +#[cfg(feature = "with-parity-db")] /// Length of a [`DbHash`]. -pub const DB_HASH_LEN: usize = 32; +const DB_HASH_LEN: usize = 32; /// Hash type that this backend uses for the database. pub type DbHash = sp_core::H256;