Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Merkle Mountain Range pallet improvements #7891

Merged
16 commits merged into from
Jan 28, 2021
Merged
Show file tree
Hide file tree
Changes from 9 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
16 changes: 16 additions & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ members = [
"frame/lottery",
"frame/membership",
"frame/merkle-mountain-range",
"frame/merkle-mountain-range/primitives",
"frame/metadata",
"frame/multisig",
"frame/nicks",
Expand Down
39 changes: 38 additions & 1 deletion bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
// and set impl_version to 0. If only runtime
// implementation changes and behavior does not, then leave spec_version as
// is and increment impl_version.
spec_version: 261,
spec_version: 262,
impl_version: 0,
apis: RUNTIME_API_VERSIONS,
transaction_version: 2,
Expand Down Expand Up @@ -1070,6 +1070,20 @@ pub type CheckedExtrinsic = generic::CheckedExtrinsic<AccountId, Call, SignedExt
/// Executive: handles dispatch to the various modules.
pub type Executive = frame_executive::Executive<Runtime, Block, frame_system::ChainContext<Runtime>, Runtime, AllModules>;

/// MMR helper types.
mod mmr {
use super::Runtime;
pub use pallet_mmr::primitives::*;

pub type Leaf = <
<Runtime as pallet_mmr::Config>::LeafData
as
LeafDataProvider
>::LeafData;
pub type Hash = <Runtime as pallet_mmr::Config>::Hash;
pub type Hashing = <Runtime as pallet_mmr::Config>::Hashing;
}

impl_runtime_apis! {
impl sp_api::Core<Block> for Runtime {
fn version() -> RuntimeVersion {
Expand Down Expand Up @@ -1264,6 +1278,29 @@ impl_runtime_apis! {
}
}

impl pallet_mmr::primitives::MmrApi<
Block,
mmr::Leaf,
mmr::Hash,
> for Runtime {
fn generate_proof(leaf_index: u64) -> Result<(mmr::Leaf, mmr::Proof<mmr::Hash>), mmr::Error> {
Mmr::generate_proof(leaf_index)
}

fn verify_proof(leaf: mmr::Leaf, proof: mmr::Proof<mmr::Hash>) -> Result<(), mmr::Error> {
Mmr::verify_leaf(leaf, proof)
}

fn verify_proof_stateless(
root: mmr::Hash,
leaf: Vec<u8>,
Copy link
Contributor

Choose a reason for hiding this comment

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

by curiosity: am I correct saying that user could also give the hash instead of the encoded leaf here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So the leaf has to be encoded in it's compact form, so depending on the types used in the pallet it either has to be the hash (if it's wrapped in Compact) or the full leaf. I don't think passing a hash here would work in case your MMR leaf is configured without Compact (i.e. the "compact" encoding and regular encoding is the same).

proof: mmr::Proof<mmr::Hash>
) -> Result<(), mmr::Error> {
let node = mmr::DataOrHash::Data(mmr::OpaqueLeaf(leaf));
pallet_mmr::verify_leaf_proof::<mmr::Hashing, _>(root, node, proof)
}
}

impl sp_session::SessionKeys<Block> for Runtime {
fn generate_session_keys(seed: Option<Vec<u8>>) -> Vec<u8> {
SessionKeys::generate(seed)
Expand Down
2 changes: 2 additions & 0 deletions frame/merkle-mountain-range/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ frame-benchmarking = { version = "2.0.0", default-features = false, path = "../b
frame-support = { version = "2.0.0", default-features = false, path = "../support" }
frame-system = { version = "2.0.0", default-features = false, path = "../system" }
mmr-lib = { package = "ckb-merkle-mountain-range", default-features = false, version = "0.3.1" }
pallet-mmr-primitives = { version = "2.0.0", default-features = false, path = "./primitives" }
serde = { version = "1.0.101", optional = true }
sp-core = { version = "2.0.0", default-features = false, path = "../../primitives/core" }
sp-io = { version = "2.0.0", default-features = false, path = "../../primitives/io" }
Expand All @@ -35,6 +36,7 @@ std = [
"frame-support/std",
"frame-system/std",
"mmr-lib/std",
"pallet-mmr-primitives/std",
"serde",
"sp-core/std",
"sp-io/std",
Expand Down
38 changes: 38 additions & 0 deletions frame/merkle-mountain-range/primitives/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
[package]
name = "pallet-mmr-primitives"
version = "2.0.0"
authors = ["Parity Technologies <admin@parity.io>"]
edition = "2018"
license = "Apache-2.0"
homepage = "https://substrate.dev"
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 = "1.3.6", default-features = false }
frame-support = { version = "2.0.0", default-features = false, path = "../../support" }
frame-system = { version = "2.0.0", default-features = false, path = "../../system" }
serde = { version = "1.0.101", optional = true, features = ["derive"] }
sp-api = { version = "2.0.0", default-features = false, path = "../../../primitives/api" }
sp-core = { version = "2.0.0", default-features = false, path = "../../../primitives/core" }
sp-runtime = { version = "2.0.0", default-features = false, path = "../../../primitives/runtime" }
sp-std = { version = "2.0.0", default-features = false, path = "../../../primitives/std" }

[dev-dependencies]
hex-literal = "0.3"

[features]
default = ["std"]
std = [
"codec/std",
"frame-support/std",
"frame-system/std",
"serde",
"sp-api/std",
"sp-core/std",
"sp-runtime/std",
"sp-std/std",
]
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@

//! Merkle Mountain Range primitive types.

use frame_support::RuntimeDebug;
#![cfg_attr(not(feature = "std"), no_std)]
#![warn(missing_docs)]

use frame_support::{RuntimeDebug, debug};
use sp_runtime::traits;
use sp_std::fmt;
#[cfg(not(feature = "std"))]
Expand Down Expand Up @@ -127,7 +130,7 @@ mod encoding {
}
}

impl<H: traits::Hash, L: FullLeaf> codec::Decode for DataOrHash<H, L> {
impl<H: traits::Hash, L: FullLeaf + codec::Decode> codec::Decode for DataOrHash<H, L> {
fn decode<I: codec::Input>(value: &mut I) -> Result<Self, codec::Error> {
let decoded: Either<Vec<u8>, H::Output> = Either::decode(value)?;
Ok(match decoded {
Expand Down Expand Up @@ -164,6 +167,7 @@ impl<H: traits::Hash, L: FullLeaf> DataOrHash<H, L> {
/// you don't care about with their hashes.
#[derive(RuntimeDebug, Clone, PartialEq)]
pub struct Compact<H, T> {
/// Internal tuple representation.
pub tuple: T,
_hash: sp_std::marker::PhantomData<H>,
}
Expand All @@ -177,6 +181,7 @@ impl<H, T> sp_std::ops::Deref for Compact<H, T> {
}

impl<H, T> Compact<H, T> {
/// Create a new [Compact] wrapper for a tuple.
pub fn new(tuple: T) -> Self {
Self { tuple, _hash: Default::default() }
}
Expand Down Expand Up @@ -274,15 +279,114 @@ pub struct Proof<Hash> {
pub items: Vec<Hash>,
}

/// 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)]
HCastano marked this conversation as resolved.
Show resolved Hide resolved
/// 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 {
debug::native::error!("[{:?}] 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 {
debug::native::debug!("[{:?}] 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 it's compact form when passed through this type.
tomusdrw marked this conversation as resolved.
Show resolved Hide resolved
/// See [FullLeaf] documentation for details.
///
/// This type is SCALE-compatible with `Vec<u8>` encoding. I.e. you must encode your leaf twice:
/// ```rust,ignore
/// let encoded: Vec<u8> = my_leaf.encode();
/// let opaque: Vec<u8> = encoded.encode();
/// let decoded = OpaqueLeaf::decode(&mut &*opaque);
/// ```
tomusdrw marked this conversation as resolved.
Show resolved Hide resolved
#[cfg_attr(feature = "std", derive(serde::Serialize, serde::Deserialize))]
#[derive(RuntimeDebug, Clone, PartialEq, codec::Decode)]
pub struct OpaqueLeaf(
/// Leaf type encoded in it's compact form.
HCastano marked this conversation as resolved.
Show resolved Hide resolved
#[cfg_attr(feature = "std", serde(with = "sp_core::bytes"))]
pub Vec<u8>
);

impl OpaqueLeaf {
/// Convert a concrete MMR leaf into an opaque type.
pub fn from_leaf<T: FullLeaf>(leaf: T) -> Self {
let encoded_leaf = leaf.using_encoded(|d| d.to_vec(), true);
// OpaqueLeaf must be SCALE-compatible with `Vec<u8>`.
// Simply using raw encoded bytes don't work, cause we don't know the
// length of the expected data.
let encoded_vec = codec::Encode::encode(&encoded_leaf);
OpaqueLeaf(encoded_vec)
}
}

impl FullLeaf for OpaqueLeaf {
fn using_encoded<R, F: FnOnce(&[u8]) -> R>(&self, f: F, _compact: bool) -> R {
f(&self.0)
Copy link
Contributor

Choose a reason for hiding this comment

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

this type encoding/decoding is bit confusing to me.
The encoding is identity but the decoding is not.
e.g. encode(decode(encode(OpaqueLeaf(something))) != encode(OpaqueLeaf(something))

I think it would make more sense to have a manual implementation of Decode.
Or to change to something like this:

Suggested change
pub fn from_leaf<T: FullLeaf>(leaf: T) -> Self {
let encoded_leaf = leaf.using_encoded(|d| d.to_vec(), true);
// OpaqueLeaf must be SCALE-compatible with `Vec<u8>`.
// Simply using raw encoded bytes don't work, cause we don't know the
// length of the expected data.
let encoded_vec = codec::Encode::encode(&encoded_leaf);
OpaqueLeaf(encoded_vec)
}
}
impl FullLeaf for OpaqueLeaf {
fn using_encoded<R, F: FnOnce(&[u8]) -> R>(&self, f: F, _compact: bool) -> R {
f(&self.0)
pub fn from_leaf<T: FullLeaf>(leaf: T) -> Self {
let encoded_leaf = leaf.using_encoded(|d| d.to_vec(), true);
OpaqueLeaf(encoded_leaf)
}
}
impl FullLeaf for OpaqueLeaf {
fn using_encoded<R, F: FnOnce(&[u8]) -> R>(&self, f: F, _compact: bool) -> R {
f(&self.0.encode())

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 was writing an elaborate answer why this is correct, just to learn that it's all wrong :) Thanks for being persistent with this.

So my initialy motivation was to have OpaqueLeaf be Vec<u8> SCALE-compatible, but pretend to be the "unwraped" (no length prefix) encoding for MMR struct. I've realised that this is not really needed, and I can relax the FullLeaf requirement and have OpaqueLeaf be completely independent from SCALE.

Please see the updated version now. The runtime API just accepts a raw Vec<u8>, and we just wrap this into OpaqueLeaf to get custom FullLeaf implementation (the default one (blanket impl for codec::Encode) would re-encode the vec, which is what I wanted to avoid).

}
}

sp_api::decl_runtime_apis! {
/// API to interact with MMR pallet.
pub trait MmrApi<Leaf: codec::Codec, Hash: codec::Codec> {
/// Generate MMR proof for a leaf under given index.
fn generate_proof(leaf_index: u64) -> Result<(Leaf, Proof<Hash>), 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: Leaf, proof: Proof<Hash>) -> 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: Vec<u8>, proof: Proof<Hash>)
-> Result<(), Error>;
}
}

#[cfg(test)]
mod tests {
use super::*;

use codec::Decode;
use crate::tests::hex;
use sp_core::H256;
use sp_runtime::traits::Keccak256;

pub(crate) fn hex(s: &str) -> H256 {
s.parse().unwrap()
}

type Test = DataOrHash<Keccak256, String>;
type TestCompact = Compact<Keccak256, (Test, Test)>;
type TestProof = Proof<<Keccak256 as traits::Hash>::Output>;
Expand Down Expand Up @@ -412,4 +516,57 @@ mod tests {

assert_eq!(decoded_compact, vec![Ok(d.clone()), Ok(d.clone())]);
}

#[test]
fn opaque_leaves_should_be_scale_compatible_with_concrete_ones() {
tomusdrw marked this conversation as resolved.
Show resolved Hide resolved
// 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))
.collect::<Vec<_>>();

let decoded_opaque = encoded_compact
.iter()
// Encode the Vec<u8> again.
.map(codec::Encode::encode)
.map(|x| OpaqueLeaf::decode(&mut &*x))
.collect::<Vec<_>>();

let reencoded = decoded_opaque
.iter()
.map(|x| x.as_ref().map(|x| x.using_encoded(|x| x.to_vec(), false)))
.collect::<Vec<_>>();

let reencoded_compact = decoded_opaque
.iter()
.map(|x| x.as_ref().map(|x| x.using_encoded(|x| x.to_vec(), true)))
.collect::<Vec<_>>();

// then
// make sure that re-encoding opaque leaves end up with the same encoding.
assert_eq!(
reencoded,
encoded_compact.clone().into_iter().map(Ok).collect::<Vec<_>>()
);
// make sure that compact and non-compact encoding is the same.
assert_eq!(
reencoded,
reencoded_compact
);
// make sure that decoded opaque leaves simply contain raw bytes payload.
assert_eq!(
decoded_opaque,
encoded_compact.into_iter().map(OpaqueLeaf).map(Ok).collect::<Vec<_>>()
);
}
}
Loading