Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate pallet-mmr to umbrella crate #7081

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,5 @@ substrate.code-workspace
target/
*.scale
justfile
python-venv
.zed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit obscure, maybe add .zed file to your global gitignore configuration?

Suggested change
.zed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

9 changes: 2 additions & 7 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 16 additions & 0 deletions prdoc/pr_7081.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: '[pallet-mmr] Migrate to using frame umbrella crate'

doc:
- audience: Runtime Dev
description: This PR migrates the pallet-mmr to use the frame umbrella crate. This
is part of the ongoing effort to migrate all pallets to use the frame umbrella crate.
The effort is tracked [here](https://github.com/paritytech/polkadot-sdk/issues/6504).

crates:
- name: pallet-mmr
bump: minor
- name: polkadot-sdk-frame
bump: minor
3 changes: 3 additions & 0 deletions substrate/frame/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ sp-consensus-grandpa = { optional = true, workspace = true }
sp-genesis-builder = { optional = true, workspace = true }
sp-inherents = { optional = true, workspace = true }
sp-keyring = { optional = true, workspace = true }
sp-mmr-primitives = { optional = true, workspace = true }
sp-offchain = { optional = true, workspace = true }
sp-session = { optional = true, workspace = true }
sp-storage = { optional = true, workspace = true }
Expand Down Expand Up @@ -78,6 +79,7 @@ runtime = [
"sp-genesis-builder",
"sp-inherents",
"sp-keyring",
"sp-mmr-primitives",
"sp-offchain",
"sp-session",
"sp-storage",
Expand Down Expand Up @@ -105,6 +107,7 @@ std = [
"sp-inherents?/std",
"sp-io/std",
"sp-keyring?/std",
"sp-mmr-primitives?/std",
"sp-offchain?/std",
"sp-runtime/std",
"sp-session?/std",
Expand Down
25 changes: 4 additions & 21 deletions substrate/frame/merkle-mountain-range/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,9 @@ targets = ["x86_64-unknown-linux-gnu"]

[dependencies]
codec = { workspace = true }
frame-benchmarking = { optional = true, workspace = true }
frame-support = { workspace = true }
frame-system = { workspace = true }
frame = { workspace = true, features = ["experimental", "runtime"] }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the experimental feature for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just follow other PRs. Yeah, it's a bit confusing, it won't compile after removing the feature.

polkadot-sdk git:(migrate-pallet-mmr) ✗ cargo check -p pallet-mmr
...
error[E0433]: failed to resolve: could not find `deps` in `frame`
  --> substrate/frame/merkle-mountain-range/src/lib.rs:65:2
   |
65 |     deps::sp_mmr_primitives::{
   |     ^^^^ could not find `deps` in `frame`
...
68 |     prelude::*,
   |     ^^^^^^^ could not find `prelude` in `frame`
...

Same concern in #6504 (comment)

log = { workspace = true }
scale-info = { features = ["derive"], workspace = true }
sp-core = { workspace = true }
sp-io = { workspace = true }
sp-mmr-primitives = { workspace = true }
sp-runtime = { workspace = true }

[dev-dependencies]
array-bytes = { workspace = true, default-features = true }
Expand All @@ -35,24 +29,13 @@ sp-tracing = { workspace = true, default-features = true }
default = ["std"]
std = [
"codec/std",
"frame-benchmarking?/std",
"frame-support/std",
"frame-system/std",
"frame/std",
"log/std",
"scale-info/std",
"sp-core/std",
"sp-io/std",
"sp-mmr-primitives/std",
"sp-runtime/std",
]
runtime-benchmarks = [
"frame-benchmarking/runtime-benchmarks",
"frame-support/runtime-benchmarks",
"frame-system/runtime-benchmarks",
"sp-runtime/runtime-benchmarks",
"frame/runtime-benchmarks",
]
try-runtime = [
"frame-support/try-runtime",
"frame-system/try-runtime",
"sp-runtime/try-runtime",
"frame/try-runtime",
]
10 changes: 6 additions & 4 deletions substrate/frame/merkle-mountain-range/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@
#![cfg(feature = "runtime-benchmarks")]

use crate::*;
use frame_benchmarking::v1::benchmarks_instance_pallet;
use frame_support::traits::OnInitialize;
use frame::{
benchmarking::prelude::v1::benchmarks_instance_pallet,
deps::frame_support::traits::OnInitialize,
};

benchmarks_instance_pallet! {
on_initialize {
Expand All @@ -31,10 +33,10 @@ benchmarks_instance_pallet! {

<<T as pallet::Config::<I>>::BenchmarkHelper as BenchmarkHelper>::setup();
for leaf in 0..(leaves - 1) {
Pallet::<T, I>::on_initialize((leaf as u32).into());
<Pallet::<T, I> as OnInitialize<BlockNumberFor<T>>>::on_initialize((leaf as u32).into());
}
}: {
Pallet::<T, I>::on_initialize((leaves as u32 - 1).into());
<Pallet::<T, I> as OnInitialize<BlockNumberFor<T>>>::on_initialize((leaves as u32 - 1).into());
} verify {
assert_eq!(crate::NumberOfLeaves::<T, I>::get(), leaves);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,13 @@
//! Default weights for the MMR Pallet
//! This file was not auto-generated.

use frame_support::weights::{
constants::{RocksDbWeight as DbWeight, WEIGHT_REF_TIME_PER_NANOS},
Weight,
};
use frame::weights_prelude::{weights_constants::WEIGHT_REF_TIME_PER_NANOS, *};

impl crate::WeightInfo for () {
fn on_initialize(peaks: u32) -> Weight {
let peaks = u64::from(peaks);
// Reading the parent hash.
let leaf_weight = DbWeight::get().reads(1);
let leaf_weight = RocksDbWeight::get().reads(1);
// Blake2 hash cost.
let hash_weight = Weight::from_parts(2u64 * WEIGHT_REF_TIME_PER_NANOS, 0);
// No-op hook.
Expand All @@ -36,6 +33,6 @@ impl crate::WeightInfo for () {
leaf_weight
.saturating_add(hash_weight)
.saturating_add(hook_weight)
.saturating_add(DbWeight::get().reads_writes(2 + peaks, 2 + peaks))
.saturating_add(RocksDbWeight::get().reads_writes(2 + peaks, 2 + peaks))
}
}
42 changes: 18 additions & 24 deletions substrate/frame/merkle-mountain-range/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,19 +59,16 @@
extern crate alloc;

use alloc::vec::Vec;
use frame_support::weights::Weight;
use frame_system::pallet_prelude::{BlockNumberFor, HeaderFor};
use log;
use sp_mmr_primitives::utils;
use sp_runtime::{
traits::{self, One, Saturating},
SaturatedConversion,

pub use frame::{
deps::sp_mmr_primitives::{
self as primitives, utils, utils::NodesUtils, Error, LeafDataProvider, LeafIndex, NodeIndex,
},
prelude::*,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be exported publicly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

};

pub use pallet::*;
pub use sp_mmr_primitives::{
self as primitives, utils::NodesUtils, Error, LeafDataProvider, LeafIndex, NodeIndex,
};

#[cfg(feature = "runtime-benchmarks")]
mod benchmarking;
Expand All @@ -90,11 +87,11 @@ mod tests;
/// 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<T: frame_system::Config> {
_phantom: core::marker::PhantomData<T>,
pub struct ParentNumberAndHash<T: Config> {
_phantom: PhantomData<T>,
}

impl<T: frame_system::Config> LeafDataProvider for ParentNumberAndHash<T> {
impl<T: Config> LeafDataProvider for ParentNumberAndHash<T> {
type LeafData = (BlockNumberFor<T>, <T as frame_system::Config>::Hash);

fn leaf_data() -> Self::LeafData {
Expand All @@ -111,13 +108,11 @@ pub trait BlockHashProvider<BlockNumber, BlockHash> {
}

/// Default implementation of BlockHashProvider using frame_system.
pub struct DefaultBlockHashProvider<T: frame_system::Config> {
pub struct DefaultBlockHashProvider<T: Config> {
_phantom: core::marker::PhantomData<T>,
}

impl<T: frame_system::Config> BlockHashProvider<BlockNumberFor<T>, T::Hash>
for DefaultBlockHashProvider<T>
{
impl<T: Config> BlockHashProvider<BlockNumberFor<T>, T::Hash> for DefaultBlockHashProvider<T> {
fn block_hash(block_number: BlockNumberFor<T>) -> T::Hash {
frame_system::Pallet::<T>::block_hash(block_number)
}
Expand Down Expand Up @@ -147,12 +142,11 @@ type LeafOf<T, I> = <<T as Config<I>>::LeafData as primitives::LeafDataProvider>
/// Hashing used for the pallet.
pub(crate) type HashingOf<T, I> = <T as Config<I>>::Hashing;
/// Hash type used for the pallet.
pub(crate) type HashOf<T, I> = <<T as Config<I>>::Hashing as traits::Hash>::Output;
pub(crate) type HashOf<T, I> = <<T as Config<I>>::Hashing as Hash>::Output;

#[frame_support::pallet]
#[frame::pallet]
pub mod pallet {
use super::*;
use frame_support::pallet_prelude::*;

#[pallet::pallet]
pub struct Pallet<T, I = ()>(PhantomData<(T, I)>);
Expand Down Expand Up @@ -180,7 +174,7 @@ pub mod pallet {
///
/// Then we create a tuple of these two hashes, SCALE-encode it (concatenate) and
/// hash, to obtain a new MMR inner node - the new peak.
type Hashing: traits::Hash;
type Hashing: Hash;

/// Data stored in the leaf nodes.
///
Expand Down Expand Up @@ -250,7 +244,7 @@ pub mod pallet {
fn on_initialize(_n: BlockNumberFor<T>) -> Weight {
use primitives::LeafDataProvider;
let leaves = NumberOfLeaves::<T, I>::get();
let peaks_before = sp_mmr_primitives::utils::NodesUtils::new(leaves).number_of_peaks();
let peaks_before = primitives::utils::NodesUtils::new(leaves).number_of_peaks();
let data = T::LeafData::leaf_data();

// append new leaf to MMR
Expand All @@ -273,7 +267,7 @@ pub mod pallet {
NumberOfLeaves::<T, I>::put(leaves);
RootHash::<T, I>::put(root);

let peaks_after = sp_mmr_primitives::utils::NodesUtils::new(leaves).number_of_peaks();
let peaks_after = primitives::utils::NodesUtils::new(leaves).number_of_peaks();

T::WeightInfo::on_initialize(peaks_before.max(peaks_after) as u32)
}
Expand All @@ -293,7 +287,7 @@ pub fn verify_leaves_proof<H, L>(
proof: primitives::LeafProof<H::Output>,
) -> Result<(), primitives::Error>
where
H: traits::Hash,
H: Hash,
L: primitives::FullLeaf,
{
let is_valid = mmr::verify_leaves_proof::<H, L>(root, leaves, proof)?;
Expand All @@ -310,7 +304,7 @@ pub fn verify_ancestry_proof<H, L>(
ancestry_proof: primitives::AncestryProof<H::Output>,
) -> Result<H::Output, Error>
where
H: traits::Hash,
H: Hash,
L: primitives::FullLeaf,
{
mmr::verify_ancestry_proof::<H, L>(root, ancestry_proof)
Expand Down
23 changes: 12 additions & 11 deletions substrate/frame/merkle-mountain-range/src/mmr/mmr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,14 @@ use crate::{
storage::{OffchainStorage, RuntimeStorage, Storage},
Hasher, Node, NodeOf,
},
primitives::{self, Error, NodeIndex},
primitives::{
self, mmr_lib, mmr_lib::MMRStoreReadOps, utils::NodesUtils, AncestryProof, Error, FullLeaf,
LeafIndex, NodeIndex,
},
Config, HashOf, HashingOf,
};
use alloc::vec::Vec;
use sp_mmr_primitives::{mmr_lib, mmr_lib::MMRStoreReadOps, utils::NodesUtils, LeafIndex};
use frame::prelude::*;

/// Stateless verification of the proof for a batch of leaves.
/// Note, the leaves should be sorted such that corresponding leaves and leaf indices have the
Expand All @@ -36,7 +39,7 @@ pub fn verify_leaves_proof<H, L>(
proof: primitives::LeafProof<H::Output>,
) -> Result<bool, Error>
where
H: sp_runtime::traits::Hash,
H: Hash,
L: primitives::FullLeaf,
{
let size = NodesUtils::new(proof.leaf_count).size();
Expand All @@ -62,11 +65,11 @@ where

pub fn verify_ancestry_proof<H, L>(
root: H::Output,
ancestry_proof: primitives::AncestryProof<H::Output>,
ancestry_proof: AncestryProof<H::Output>,
) -> Result<H::Output, Error>
where
H: sp_runtime::traits::Hash,
L: primitives::FullLeaf,
H: Hash,
L: FullLeaf,
{
let mmr_size = NodesUtils::new(ancestry_proof.leaf_count).size();

Expand Down Expand Up @@ -258,12 +261,10 @@ where
/// The generated proof contains all the leafs in the MMR, so this way we can generate a proof
/// with exactly `leaf_count` items.
#[cfg(feature = "runtime-benchmarks")]
pub fn generate_mock_ancestry_proof(
&self,
) -> Result<sp_mmr_primitives::AncestryProof<HashOf<T, I>>, Error> {
pub fn generate_mock_ancestry_proof(&self) -> Result<AncestryProof<HashOf<T, I>>, Error> {
use crate::ModuleMmr;
use alloc::vec;
use sp_mmr_primitives::mmr_lib::helper;
use mmr_lib::helper;

let mmr: ModuleMmr<OffchainStorage, T, I> = Mmr::new(self.leaves);
let store = <Storage<OffchainStorage, T, I, L>>::default();
Expand All @@ -289,7 +290,7 @@ where
proof_items.push((leaf_pos, leaf));
}

Ok(sp_mmr_primitives::AncestryProof {
Ok(AncestryProof {
prev_peaks,
prev_leaf_count: self.leaves,
leaf_count: self.leaves,
Expand Down
5 changes: 2 additions & 3 deletions substrate/frame/merkle-mountain-range/src/mmr/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,9 @@
mod mmr;
pub mod storage;

use sp_mmr_primitives::{mmr_lib, DataOrHash, FullLeaf};
use sp_runtime::traits;

pub use self::mmr::{verify_ancestry_proof, verify_leaves_proof, Mmr};
use crate::primitives::{mmr_lib, DataOrHash, FullLeaf};
use frame::traits;

/// Node type for runtime `T`.
pub type NodeOf<T, I, L> = Node<<T as crate::Config<I>>::Hashing, L>;
Expand Down
Loading
Loading