From 22b86994e74f66ba4df864874d6474990ba3a7b6 Mon Sep 17 00:00:00 2001 From: cheme Date: Wed, 27 Jan 2021 17:23:34 +0100 Subject: [PATCH 01/25] validation extension in sp_io --- primitives/io/Cargo.toml | 18 ++++++++-------- primitives/io/src/lib.rs | 44 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 10 deletions(-) diff --git a/primitives/io/Cargo.toml b/primitives/io/Cargo.toml index 01ea58e87e3e2..42355e6c58a9f 100644 --- a/primitives/io/Cargo.toml +++ b/primitives/io/Cargo.toml @@ -17,16 +17,16 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] codec = { package = "parity-scale-codec", version = "1.3.6", default-features = false } hash-db = { version = "0.15.2", default-features = false } -sp-core = { version = "2.0.0", default-features = false, path = "../core" } -sp-keystore = { version = "0.8.0", default-features = false, optional = true, path = "../keystore" } -sp-std = { version = "2.0.0", default-features = false, path = "../std" } +sp-core = { version = "2.0.0", default-features = false } +sp-keystore = { version = "0.8.0", default-features = false, optional = true } +sp-std = { version = "2.0.0", default-features = false } libsecp256k1 = { version = "0.3.4", optional = true } -sp-state-machine = { version = "0.8.0", optional = true, path = "../state-machine" } -sp-wasm-interface = { version = "2.0.0", path = "../wasm-interface", default-features = false } -sp-runtime-interface = { version = "2.0.0", default-features = false, path = "../runtime-interface" } -sp-trie = { version = "2.0.0", optional = true, path = "../trie" } -sp-externalities = { version = "0.8.0", optional = true, path = "../externalities" } -sp-tracing = { version = "2.0.0", default-features = false, path = "../tracing" } +sp-state-machine = { version = "0.8.0", optional = true } +sp-wasm-interface = { version = "2.0.0", default-features = false } +sp-runtime-interface = { version = "2.0.0", default-features = false } +sp-trie = { version = "2.0.0", optional = true } +sp-externalities = { version = "0.8.0", optional = true } +sp-tracing = { version = "2.0.0", default-features = false } log = { version = "0.4.8", optional = true } futures = { version = "0.3.1", features = ["thread-pool"], optional = true } parking_lot = { version = "0.11.1", optional = true } diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index 397dd3c21712a..5b9ac70feb3e5 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -1292,7 +1292,49 @@ pub trait RuntimeTasks { runtime_spawn.join(handle) }).expect("`RuntimeTasks::join`: called outside of externalities context") } - } +} + +use sp_std::boxed::Box; + + +#[runtime_interface] +pub trait Validation { + /// TODO + fn validation_code(&mut self) -> Vec { + use sp_externalities::ExternalitiesExt; + let extension = self.extension::() + .expect("Validation extension missing, this should only be call in a validation context."); + extension.validation_code() + } +} + +#[cfg(feature = "std")] +sp_externalities::decl_extension! { + /// executor extension. + pub struct ValidationExt(Box); +} + +#[cfg(feature = "std")] +impl ValidationExt { + /// New instance of task executor extension. + pub fn new(validation_ext: impl Validation) -> Self { + Self(Box::new(validation_ext)) + } +} + +/// Base methods to implement validation extension. +pub trait Validation: Send + 'static { + /// Get the validation code currently running. + /// This can be use to check validity or to complete + /// proofs. + fn validation_code(&self) -> Vec; +} + +impl Validation for &'static [u8] { + fn validation_code(&self) -> Vec { + self.to_vec() + } +} /// Allocator used by Substrate when executing the Wasm runtime. #[cfg(not(feature = "std"))] From 4de9f7a30f60fe56e247cf32332016c7d2620baa Mon Sep 17 00:00:00 2001 From: cheme Date: Wed, 27 Jan 2021 17:36:40 +0100 Subject: [PATCH 02/25] need paths --- primitives/io/Cargo.toml | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/primitives/io/Cargo.toml b/primitives/io/Cargo.toml index 42355e6c58a9f..01ea58e87e3e2 100644 --- a/primitives/io/Cargo.toml +++ b/primitives/io/Cargo.toml @@ -17,16 +17,16 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] codec = { package = "parity-scale-codec", version = "1.3.6", default-features = false } hash-db = { version = "0.15.2", default-features = false } -sp-core = { version = "2.0.0", default-features = false } -sp-keystore = { version = "0.8.0", default-features = false, optional = true } -sp-std = { version = "2.0.0", default-features = false } +sp-core = { version = "2.0.0", default-features = false, path = "../core" } +sp-keystore = { version = "0.8.0", default-features = false, optional = true, path = "../keystore" } +sp-std = { version = "2.0.0", default-features = false, path = "../std" } libsecp256k1 = { version = "0.3.4", optional = true } -sp-state-machine = { version = "0.8.0", optional = true } -sp-wasm-interface = { version = "2.0.0", default-features = false } -sp-runtime-interface = { version = "2.0.0", default-features = false } -sp-trie = { version = "2.0.0", optional = true } -sp-externalities = { version = "0.8.0", optional = true } -sp-tracing = { version = "2.0.0", default-features = false } +sp-state-machine = { version = "0.8.0", optional = true, path = "../state-machine" } +sp-wasm-interface = { version = "2.0.0", path = "../wasm-interface", default-features = false } +sp-runtime-interface = { version = "2.0.0", default-features = false, path = "../runtime-interface" } +sp-trie = { version = "2.0.0", optional = true, path = "../trie" } +sp-externalities = { version = "0.8.0", optional = true, path = "../externalities" } +sp-tracing = { version = "2.0.0", default-features = false, path = "../tracing" } log = { version = "0.4.8", optional = true } futures = { version = "0.3.1", features = ["thread-pool"], optional = true } parking_lot = { version = "0.11.1", optional = true } From f0ea2af03f1bd078a0b55c191eea2488da638363 Mon Sep 17 00:00:00 2001 From: cheme Date: Wed, 27 Jan 2021 17:43:38 +0100 Subject: [PATCH 03/25] arc impl --- primitives/io/src/lib.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index 5b9ac70feb3e5..42d1409ec4f14 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -1336,6 +1336,13 @@ impl Validation for &'static [u8] { } } +#[cfg(feature = "std")] +impl Validation for std::sync::Arc { + fn validation_code(&self) -> Vec { + self.deref().validation_code() + } +} + /// Allocator used by Substrate when executing the Wasm runtime. #[cfg(not(feature = "std"))] struct WasmAllocator; From 71202a940060d875543d24a430ec60cae2ca1e9c Mon Sep 17 00:00:00 2001 From: cheme Date: Thu, 28 Jan 2021 10:09:56 +0100 Subject: [PATCH 04/25] missing host function in executor --- primitives/io/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index 42d1409ec4f14..ee7550aae1a7e 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -1411,6 +1411,7 @@ pub type SubstrateHostFunctions = ( crate::trie::HostFunctions, offchain_index::HostFunctions, runtime_tasks::HostFunctions, + validation::HostFunctions, ); #[cfg(test)] From f848b4434fa2925f7b7f4b31b3d2023e7e58a7f8 Mon Sep 17 00:00:00 2001 From: cheme Date: Mon, 1 Feb 2021 10:17:33 +0100 Subject: [PATCH 05/25] io to pkdot --- primitives/io/src/lib.rs | 52 +--------------------------------------- 1 file changed, 1 insertion(+), 51 deletions(-) diff --git a/primitives/io/src/lib.rs b/primitives/io/src/lib.rs index ee7550aae1a7e..397dd3c21712a 100644 --- a/primitives/io/src/lib.rs +++ b/primitives/io/src/lib.rs @@ -1292,56 +1292,7 @@ pub trait RuntimeTasks { runtime_spawn.join(handle) }).expect("`RuntimeTasks::join`: called outside of externalities context") } -} - -use sp_std::boxed::Box; - - -#[runtime_interface] -pub trait Validation { - /// TODO - fn validation_code(&mut self) -> Vec { - use sp_externalities::ExternalitiesExt; - let extension = self.extension::() - .expect("Validation extension missing, this should only be call in a validation context."); - extension.validation_code() - } -} - -#[cfg(feature = "std")] -sp_externalities::decl_extension! { - /// executor extension. - pub struct ValidationExt(Box); -} - -#[cfg(feature = "std")] -impl ValidationExt { - /// New instance of task executor extension. - pub fn new(validation_ext: impl Validation) -> Self { - Self(Box::new(validation_ext)) - } -} - -/// Base methods to implement validation extension. -pub trait Validation: Send + 'static { - /// Get the validation code currently running. - /// This can be use to check validity or to complete - /// proofs. - fn validation_code(&self) -> Vec; -} - -impl Validation for &'static [u8] { - fn validation_code(&self) -> Vec { - self.to_vec() - } -} - -#[cfg(feature = "std")] -impl Validation for std::sync::Arc { - fn validation_code(&self) -> Vec { - self.deref().validation_code() - } -} + } /// Allocator used by Substrate when executing the Wasm runtime. #[cfg(not(feature = "std"))] @@ -1411,7 +1362,6 @@ pub type SubstrateHostFunctions = ( crate::trie::HostFunctions, offchain_index::HostFunctions, runtime_tasks::HostFunctions, - validation::HostFunctions, ); #[cfg(test)] From 1979b6a2e2f34be00f4f27572453beea811e4a70 Mon Sep 17 00:00:00 2001 From: cheme Date: Mon, 1 Feb 2021 13:03:31 +0100 Subject: [PATCH 06/25] decode function. --- Cargo.lock | 8 +- Cargo.toml | 4 + primitives/trie/Cargo.toml | 2 +- primitives/trie/src/lib.rs | 5 ++ primitives/trie/src/trie_codec.rs | 140 ++++++++++++++++++++++++++++++ 5 files changed, 153 insertions(+), 6 deletions(-) create mode 100644 primitives/trie/src/trie_codec.rs diff --git a/Cargo.lock b/Cargo.lock index 7421af5ff1b5e..06e430bb97469 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2143,8 +2143,7 @@ dependencies = [ [[package]] name = "hash-db" version = "0.15.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d23bd4e7b5eda0d0f3a307e8b381fdc8ba9000f26fbe912250c0a4cc3956364a" +source = "git+https://github.com/cheme/trie?branch=skip_compact_values_alt#ccb001e53234adb85b0c00b031bbc56151018d43" [[package]] name = "hash256-std-hasher" @@ -9755,9 +9754,8 @@ dependencies = [ [[package]] name = "trie-db" -version = "0.22.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5cc176c377eb24d652c9c69c832c832019011b6106182bf84276c66b66d5c9a6" +version = "0.22.3" +source = "git+https://github.com/cheme/trie?branch=skip_compact_values_alt#ccb001e53234adb85b0c00b031bbc56151018d43" dependencies = [ "hash-db", "hashbrown", diff --git a/Cargo.toml b/Cargo.toml index 12e79490ef6b0..59f5f09e1e975 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -255,3 +255,7 @@ zeroize = { opt-level = 3 } [profile.release] # Substrate runtime requires unwinding. panic = "unwind" + +[patch.crates-io] +trie-db = { git = 'https://github.com/cheme/trie', branch = 'skip_compact_values_alt' } +hash-db = { git = 'https://github.com/cheme/trie', branch = 'skip_compact_values_alt' } diff --git a/primitives/trie/Cargo.toml b/primitives/trie/Cargo.toml index 4392f01d222aa..bf52f08c3fc12 100644 --- a/primitives/trie/Cargo.toml +++ b/primitives/trie/Cargo.toml @@ -21,7 +21,7 @@ harness = false codec = { package = "parity-scale-codec", version = "1.3.6", default-features = false } sp-std = { version = "2.0.0", default-features = false, path = "../std" } hash-db = { version = "0.15.2", default-features = false } -trie-db = { version = "0.22.2", default-features = false } +trie-db = { version = "0.22.3", default-features = false } trie-root = { version = "0.16.0", default-features = false } memory-db = { version = "0.25.0", default-features = false } sp-core = { version = "2.0.0", default-features = false, path = "../core" } diff --git a/primitives/trie/src/lib.rs b/primitives/trie/src/lib.rs index 572283f1c027e..3ecad1d5b0318 100644 --- a/primitives/trie/src/lib.rs +++ b/primitives/trie/src/lib.rs @@ -23,6 +23,7 @@ mod error; mod node_header; mod node_codec; mod storage_proof; +mod trie_codec; mod trie_stream; use sp_std::{boxed::Box, marker::PhantomData, vec::Vec, borrow::Borrow}; @@ -39,12 +40,16 @@ pub use storage_proof::StorageProof; /// Various re-exports from the `trie-db` crate. pub use trie_db::{ Trie, TrieMut, DBValue, Recorder, CError, Query, TrieLayout, TrieConfiguration, nibble_ops, TrieDBIterator, + LazyFetcher, }; /// Various re-exports from the `memory-db` crate. pub use memory_db::KeyFunction; pub use memory_db::prefixed_key; /// Various re-exports from the `hash-db` crate. pub use hash_db::{HashDB as HashDBT, EMPTY_PREFIX}; +/// Trie codec reexport, mainly child trie support +/// for trie compact proof. +pub use trie_codec::{decode_compact}; #[derive(Default)] /// substrate trie layout diff --git a/primitives/trie/src/trie_codec.rs b/primitives/trie/src/trie_codec.rs new file mode 100644 index 0000000000000..711b59c7dcadd --- /dev/null +++ b/primitives/trie/src/trie_codec.rs @@ -0,0 +1,140 @@ +// This file is part of Substrate. + +// Copyright (C) 2015-2021 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. + +//! Compact proof support. +//! +//! This uses compact proof from trie crate and extends +//! it to substrate specific layout and child trie system. + +use crate::{EMPTY_PREFIX, HashDBT, LazyFetcher, + TrieHash, TrieError, TrieConfiguration}; + +type VerifyError = crate::VerifyError, Box>>; + +fn verify_error(error: Box>) -> VerifyError { + VerifyError::::DecodeError(error) +} + +/// Decode a compact proof. +/// +/// Takes as input a destination `db` for decoded node and `encoded` +/// an iterator of compact encoded nodes. +/// +/// Also allows optionally injecting specified value in +/// top trie proof with `know_keys` and the lazy +/// associated `fetcher`. +/// +/// Child trie are decoded in order of child trie root present +/// in the top trie. +pub fn decode_compact<'a, L, DB, I, F, K>( + db: &mut DB, + encoded: I, + fetcher: F, + known_keys: Option, + expected_root: Option<&TrieHash>, +) -> Result, VerifyError> + where + L: TrieConfiguration, + DB: HashDBT + hash_db::HashDBRef, + I: IntoIterator, + F: LazyFetcher<'a>, + K: IntoIterator, +{ + let mut nodes_iter = encoded.into_iter(); + let (top_root, _nb_used) = if let Some(known_keys) = known_keys { + trie_db::decode_compact_with_known_values::( + db, + &mut nodes_iter, + fetcher, + known_keys, + false, // current use of compact do to escape empty value. + ).map_err(verify_error::)? + } else { + trie_db::decode_compact_from_iter::( + db, + &mut nodes_iter, + ).map_err(verify_error::)? + }; + + if let Some(expected_root) = expected_root { + if expected_root != &top_root { + return Err(VerifyError::::RootMismatch(expected_root.clone())); + } + } + + let mut child_tries = Vec::new(); + { + // fetch child trie roots + let trie = crate::TrieDB::::new(db, &top_root).map_err(verify_error::)?; + + use trie_db::Trie; + let mut iter = trie.iter().map_err(verify_error::)?; + + let childtrie_roots = sp_core::storage::well_known_keys::DEFAULT_CHILD_STORAGE_KEY_PREFIX; + if iter.seek(childtrie_roots).is_ok() { + loop { + match iter.next() { + Some(Ok((key, value))) if key.starts_with(childtrie_roots) => { + // we expect all default child trie root to be correctly encoded. + // see other child trie functions. + let mut root = TrieHash::::default(); + // still in a proof so prevent panic + if root.as_mut().len() != value.as_slice().len() { + return Err(VerifyError::::RootMismatch(Default::default())); + } + root.as_mut().copy_from_slice(value.as_ref()); + child_tries.push(root); + }, + // allow incomplete database error: we only + // require access to data in the proof. + Some(Err(error)) => match *error { + trie_db::TrieError::IncompleteDatabase(..) => (), + e => panic!("{:?}", e), + }, + _ => break, + } + } + } + } + + if !HashDBT::::contains(db, &top_root, EMPTY_PREFIX) { + return Err(VerifyError::::IncompleteProof); + } + + for child_root in child_tries.into_iter() { + if !HashDBT::::contains(db, &child_root, EMPTY_PREFIX) { + // child proof are allowed to be missing (unused root can be included + // due to trie structure modification). + continue; + } + + let (top_root, _nb_used) = trie_db::decode_compact_from_iter::( + db, + &mut nodes_iter, + ).map_err(verify_error::)?; + + if child_root != top_root { + return Err(VerifyError::::RootMismatch(child_root)); + } + } + + if nodes_iter.next().is_some() { + return Err(VerifyError::::ExtraneousNode); + } + + Ok(top_root) +} From 927a096737416c5e83587eb0b7b9b06bae2da83d Mon Sep 17 00:00:00 2001 From: cheme Date: Mon, 1 Feb 2021 14:53:06 +0100 Subject: [PATCH 07/25] encode primitive. --- primitives/trie/src/lib.rs | 4 +- primitives/trie/src/storage_proof.rs | 13 ++++++ primitives/trie/src/trie_codec.rs | 69 +++++++++++++++++++++++++++- 3 files changed, 82 insertions(+), 4 deletions(-) diff --git a/primitives/trie/src/lib.rs b/primitives/trie/src/lib.rs index 3ecad1d5b0318..5a4cff6c9d5be 100644 --- a/primitives/trie/src/lib.rs +++ b/primitives/trie/src/lib.rs @@ -36,7 +36,7 @@ pub use error::Error; pub use trie_stream::TrieStream; /// The Substrate format implementation of `NodeCodec`. pub use node_codec::NodeCodec; -pub use storage_proof::StorageProof; +pub use storage_proof::{StorageProof, CompactProof}; /// Various re-exports from the `trie-db` crate. pub use trie_db::{ Trie, TrieMut, DBValue, Recorder, CError, Query, TrieLayout, TrieConfiguration, nibble_ops, TrieDBIterator, @@ -49,7 +49,7 @@ pub use memory_db::prefixed_key; pub use hash_db::{HashDB as HashDBT, EMPTY_PREFIX}; /// Trie codec reexport, mainly child trie support /// for trie compact proof. -pub use trie_codec::{decode_compact}; +pub use trie_codec::{decode_compact, encode_compact}; #[derive(Default)] /// substrate trie layout diff --git a/primitives/trie/src/storage_proof.rs b/primitives/trie/src/storage_proof.rs index f0b2bfd4bc3d3..96c2be3460393 100644 --- a/primitives/trie/src/storage_proof.rs +++ b/primitives/trie/src/storage_proof.rs @@ -31,6 +31,12 @@ pub struct StorageProof { trie_nodes: Vec>, } +/// Storage proof in compact form. +#[derive(Debug, PartialEq, Eq, Clone, Encode, Decode)] +pub struct CompactProof { + pub encoded_nodes: Vec>, +} + impl StorageProof { /// Constructs a storage proof from a subset of encoded trie nodes in a storage backend. pub fn new(trie_nodes: Vec>) -> Self { @@ -77,6 +83,13 @@ impl StorageProof { } } +impl CompactProof { + /// Return an iterator on the compact encoded nodes. + pub fn iter_compact_encoded_nodes(&self) -> impl Iterator { + self.encoded_nodes.iter().map(Vec::as_slice) + } +} + /// An iterator over trie nodes constructed from a storage proof. The nodes are not guaranteed to /// be traversed in any particular order. pub struct StorageProofNodeIterator { diff --git a/primitives/trie/src/trie_codec.rs b/primitives/trie/src/trie_codec.rs index 711b59c7dcadd..7c0e9d3460b4b 100644 --- a/primitives/trie/src/trie_codec.rs +++ b/primitives/trie/src/trie_codec.rs @@ -21,7 +21,9 @@ //! it to substrate specific layout and child trie system. use crate::{EMPTY_PREFIX, HashDBT, LazyFetcher, - TrieHash, TrieError, TrieConfiguration}; + TrieHash, TrieError, TrieConfiguration, CompactProof, StorageProof}; +use sp_std::boxed::Box; +use sp_std::vec::Vec; type VerifyError = crate::VerifyError, Box>>; @@ -103,7 +105,7 @@ pub fn decode_compact<'a, L, DB, I, F, K>( // require access to data in the proof. Some(Err(error)) => match *error { trie_db::TrieError::IncompleteDatabase(..) => (), - e => panic!("{:?}", e), + e => return Err(VerifyError::::DecodeError(Box::new(e))), }, _ => break, } @@ -138,3 +140,66 @@ pub fn decode_compact<'a, L, DB, I, F, K>( Ok(top_root) } + +pub fn encode_compact<'a, L, I>( + proof: StorageProof, + root: TrieHash, + to_skip: I, +) -> Result>> + where + L: TrieConfiguration, + I: IntoIterator + 'a, +{ + let mut child_tries = Vec::new(); + let partial_db = proof.into_memory_db(); + let mut compact_proof = { + let trie = crate::TrieDB::::new(&partial_db, &root)?; + + use trie_db::Trie; + let mut iter = trie.iter()?; + + let childtrie_roots = sp_core::storage::well_known_keys::DEFAULT_CHILD_STORAGE_KEY_PREFIX; + if iter.seek(childtrie_roots).is_ok() { + loop { + match iter.next() { + Some(Ok((key, value))) if key.starts_with(childtrie_roots) => { + let mut root = TrieHash::::default(); + if root.as_mut().len() != value.as_slice().len() { + return Err(Box::new(trie_db::TrieError::InvalidStateRoot(Default::default()))); + } + root.as_mut().copy_from_slice(value.as_ref()); + child_tries.push(root); + }, + // allow incomplete database error: we only + // require access to data in the proof. + Some(Err(error)) => match *error { + trie_db::TrieError::IncompleteDatabase(..) => (), + e => return Err(Box::new(e)), + }, + _ => break, + } + } + } + + trie_db::encode_compact_skip_conditional_with_key::( + &trie, + trie_db::compact_conditions::skip_given_ordered_keys(to_skip), + false, // We do not escape empty value. + )? + }; + + for child_root in child_tries { + if !HashDBT::::contains(&partial_db, &child_root, EMPTY_PREFIX) { + // child proof are allowed to be missing (unused root can be included + // due to trie structure modification). + continue; + } + + let trie = crate::TrieDB::::new(&partial_db, &child_root)?; + let child_proof = trie_db::encode_compact::(&trie)?; + + compact_proof.extend(child_proof); + } + + Ok(CompactProof { encoded_nodes: compact_proof }) +} From b034ec0b7b451be829fadd1939a9f8d5e8178cf9 Mon Sep 17 00:00:00 2001 From: cheme Date: Mon, 1 Feb 2021 15:34:23 +0100 Subject: [PATCH 08/25] trailing tab --- primitives/trie/src/trie_codec.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/primitives/trie/src/trie_codec.rs b/primitives/trie/src/trie_codec.rs index 7c0e9d3460b4b..bfeea8e0293d4 100644 --- a/primitives/trie/src/trie_codec.rs +++ b/primitives/trie/src/trie_codec.rs @@ -1,6 +1,6 @@ // This file is part of Substrate. -// Copyright (C) 2015-2021 Parity Technologies (UK) Ltd. +// Copyright (C) 2021-2021 Parity Technologies (UK) Ltd. // SPDX-License-Identifier: Apache-2.0 // Licensed under the Apache License, Version 2.0 (the "License"); @@ -149,7 +149,7 @@ pub fn encode_compact<'a, L, I>( where L: TrieConfiguration, I: IntoIterator + 'a, -{ +{ let mut child_tries = Vec::new(); let partial_db = proof.into_memory_db(); let mut compact_proof = { From 757ff301ee60ce69a17546ac709bca0e2f00e472 Mon Sep 17 00:00:00 2001 From: cheme Date: Mon, 1 Feb 2021 16:46:20 +0100 Subject: [PATCH 09/25] multiple patch --- utils/wasm-builder/src/wasm_project.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/wasm-builder/src/wasm_project.rs b/utils/wasm-builder/src/wasm_project.rs index 73dc2e13af348..ea28b1dfb9f2c 100644 --- a/utils/wasm-builder/src/wasm_project.rs +++ b/utils/wasm-builder/src/wasm_project.rs @@ -224,7 +224,7 @@ fn create_project_cargo_toml( wasm_workspace_toml.insert("profile".into(), profile.into()); // Add patch section from the project root `Cargo.toml` - if let Some(mut patch) = workspace_toml.remove("patch").and_then(|p| p.try_into::().ok()) { + while let Some(mut patch) = workspace_toml.remove("patch").and_then(|p| p.try_into::
().ok()) { // Iterate over all patches and make the patch path absolute from the workspace root path. patch.iter_mut() .filter_map(|p| From 7dba01bb0cc4929a75012af56c0fbd53fb59ffe3 Mon Sep 17 00:00:00 2001 From: cheme Date: Tue, 2 Feb 2021 11:29:51 +0100 Subject: [PATCH 10/25] fix child trie logic --- Cargo.lock | 4 ++-- primitives/state-machine/src/lib.rs | 29 +++++++++++++++++++++++++++-- primitives/trie/src/trie_codec.rs | 27 +++++++++++++++------------ 3 files changed, 44 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 06e430bb97469..cf927a2e0eca7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2143,7 +2143,7 @@ dependencies = [ [[package]] name = "hash-db" version = "0.15.2" -source = "git+https://github.com/cheme/trie?branch=skip_compact_values_alt#ccb001e53234adb85b0c00b031bbc56151018d43" +source = "git+https://github.com/cheme/trie?branch=skip_compact_values_alt#8b8c6481c3360678b734b0f08a5360216bb53039" [[package]] name = "hash256-std-hasher" @@ -9755,7 +9755,7 @@ dependencies = [ [[package]] name = "trie-db" version = "0.22.3" -source = "git+https://github.com/cheme/trie?branch=skip_compact_values_alt#ccb001e53234adb85b0c00b031bbc56151018d43" +source = "git+https://github.com/cheme/trie?branch=skip_compact_values_alt#8b8c6481c3360678b734b0f08a5360216bb53039" dependencies = [ "hash-db", "hashbrown", diff --git a/primitives/state-machine/src/lib.rs b/primitives/state-machine/src/lib.rs index 6d85b56f8aae2..b8b777fd4f2c3 100644 --- a/primitives/state-machine/src/lib.rs +++ b/primitives/state-machine/src/lib.rs @@ -1434,12 +1434,36 @@ mod tests { #[test] fn prove_read_and_proof_check_works() { + fn test_compact(remote_proof: StorageProof, remote_root: &sp_core::H256) -> StorageProof { + type Layout = sp_trie::Layout; + let compact_remote_proof = sp_trie::encode_compact::( + remote_proof, + remote_root.clone(), + std::iter::empty(), + ).unwrap(); + let mut db = sp_trie::MemoryDB::::new(&[]); + sp_trie::decode_compact::>( + &mut db, + compact_remote_proof.iter_compact_encoded_nodes(), + (), + None, + Some(remote_root), + ).unwrap(); + StorageProof::new(db.drain().into_iter().filter_map(|kv| + if (kv.1).1 > 0 { + Some((kv.1).0) + } else { + None + } + ).collect()) + } let child_info = ChildInfo::new_default(b"sub1"); let child_info = &child_info; // fetch read proof from 'remote' full node let remote_backend = trie_backend::tests::test_trie(); - let remote_root = remote_backend.storage_root(::std::iter::empty()).0; + let remote_root = remote_backend.storage_root(std::iter::empty()).0; let remote_proof = prove_read(remote_backend, &[b"value2"]).unwrap(); + let remote_proof = test_compact(remote_proof, &remote_root); // check proof locally let local_result1 = read_proof_check::( remote_root, @@ -1459,12 +1483,13 @@ mod tests { assert_eq!(local_result2, false); // on child trie let remote_backend = trie_backend::tests::test_trie(); - let remote_root = remote_backend.storage_root(::std::iter::empty()).0; + let remote_root = remote_backend.storage_root(std::iter::empty()).0; let remote_proof = prove_child_read( remote_backend, child_info, &[b"value3"], ).unwrap(); + let remote_proof = test_compact(remote_proof, &remote_root); let local_result1 = read_child_proof_check::( remote_root, remote_proof.clone(), diff --git a/primitives/trie/src/trie_codec.rs b/primitives/trie/src/trie_codec.rs index bfeea8e0293d4..43d630c7f0246 100644 --- a/primitives/trie/src/trie_codec.rs +++ b/primitives/trie/src/trie_codec.rs @@ -117,22 +117,25 @@ pub fn decode_compact<'a, L, DB, I, F, K>( return Err(VerifyError::::IncompleteProof); } + let mut previous_extracted_child_trie = None; for child_root in child_tries.into_iter() { - if !HashDBT::::contains(db, &child_root, EMPTY_PREFIX) { - // child proof are allowed to be missing (unused root can be included - // due to trie structure modification). - continue; + if previous_extracted_child_trie == None { + let (top_root, _) = trie_db::decode_compact_from_iter::( + db, + &mut nodes_iter, + ).map_err(verify_error::)?; + previous_extracted_child_trie = Some(top_root); } - - let (top_root, _nb_used) = trie_db::decode_compact_from_iter::( - db, - &mut nodes_iter, - ).map_err(verify_error::)?; - - if child_root != top_root { - return Err(VerifyError::::RootMismatch(child_root)); + + // we allow skipping child root by only + // decoding next on match. + if Some(child_root) == previous_extracted_child_trie { + previous_extracted_child_trie = None; } } + if let Some(child_root) = previous_extracted_child_trie { + return Err(VerifyError::::RootMismatch(child_root)); + } if nodes_iter.next().is_some() { return Err(VerifyError::::ExtraneousNode); From 7ef5a41a04e01b31f2a145915a6a0657dd90733d Mon Sep 17 00:00:00 2001 From: cheme Date: Thu, 8 Apr 2021 16:08:02 +0200 Subject: [PATCH 11/25] restore master versionning --- primitives/trie/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/primitives/trie/Cargo.toml b/primitives/trie/Cargo.toml index 9584ae678d409..4396550a48a8f 100644 --- a/primitives/trie/Cargo.toml +++ b/primitives/trie/Cargo.toml @@ -21,7 +21,7 @@ harness = false codec = { package = "parity-scale-codec", version = "2.0.0", default-features = false } sp-std = { version = "3.0.0", default-features = false, path = "../std" } hash-db = { version = "0.15.2", default-features = false } -trie-db = { version = "0.22.3", default-features = false } +trie-db = { version = "0.22.2", default-features = false } trie-root = { version = "0.16.0", default-features = false } memory-db = { version = "0.26.0", default-features = false } sp-core = { version = "3.0.0", default-features = false, path = "../core" } From 360c3240a55861e70d67d95b09813ce5ec013411 Mon Sep 17 00:00:00 2001 From: cheme Date: Thu, 8 Apr 2021 17:04:11 +0200 Subject: [PATCH 12/25] bench compact proof size --- client/db/src/bench.rs | 12 +++++++++--- frame/benchmarking/src/analysis.rs | 5 +++++ frame/benchmarking/src/lib.rs | 8 ++++++-- frame/benchmarking/src/utils.rs | 3 ++- primitives/externalities/src/lib.rs | 2 +- primitives/state-machine/src/backend.rs | 2 +- primitives/state-machine/src/ext.rs | 2 +- utils/frame/benchmarking-cli/src/command.rs | 5 +++-- utils/frame/benchmarking-cli/src/writer.rs | 1 + 9 files changed, 29 insertions(+), 11 deletions(-) diff --git a/client/db/src/bench.rs b/client/db/src/bench.rs index 2704676207b00..763aadf8d6053 100644 --- a/client/db/src/bench.rs +++ b/client/db/src/bench.rs @@ -23,7 +23,7 @@ use std::cell::{Cell, RefCell}; use std::collections::HashMap; use hash_db::{Prefix, Hasher}; -use sp_trie::{MemoryDB, prefixed_key, StorageProof}; +use sp_trie::{MemoryDB, prefixed_key, StorageProof, encode_compact}; use sp_core::{ storage::{ChildInfo, TrackedStorageKey}, hexdisplay::HexDisplay @@ -517,14 +517,20 @@ impl StateBackend> for BenchmarkingState { self.state.borrow().as_ref().map_or(sp_state_machine::UsageInfo::empty(), |s| s.usage_info()) } - fn proof_size(&self) -> Option { + fn proof_size(&self) -> Option<(u32, u32)> { self.proof_recorder.as_ref().map(|recorder| { let proof = StorageProof::new(recorder .read() .iter() .filter_map(|(_k, v)| v.as_ref().map(|v| v.to_vec())) .collect()); - proof.encoded_size() as u32 + let proof_size = proof.encoded_size() as u32; + let compact_proof = encode_compact::>>( + proof, + self.root.get(), + ).unwrap(); + + (proof_size, compact_proof.encoded_size() as u32) }) } } diff --git a/frame/benchmarking/src/analysis.rs b/frame/benchmarking/src/analysis.rs index 7b6d8838fd21c..e1a49c4a66962 100644 --- a/frame/benchmarking/src/analysis.rs +++ b/frame/benchmarking/src/analysis.rs @@ -39,6 +39,7 @@ pub enum BenchmarkSelector { Reads, Writes, ProofSize, + CompactProofSize, } #[derive(Debug)] @@ -88,6 +89,7 @@ impl Analysis { BenchmarkSelector::Reads => result.reads.into(), BenchmarkSelector::Writes => result.writes.into(), BenchmarkSelector::ProofSize => result.proof_size.into(), + BenchmarkSelector::CompactProofSize => result.compact_proof_size.into(), } ).collect(); @@ -129,6 +131,7 @@ impl Analysis { BenchmarkSelector::Reads => result.reads.into(), BenchmarkSelector::Writes => result.writes.into(), BenchmarkSelector::ProofSize => result.proof_size.into(), + BenchmarkSelector::CompactProofSize => result.compact_proof_size.into(), }; (result.components[i].1, data) }) @@ -194,6 +197,7 @@ impl Analysis { BenchmarkSelector::Reads => result.reads.into(), BenchmarkSelector::Writes => result.writes.into(), BenchmarkSelector::ProofSize => result.proof_size.into(), + BenchmarkSelector::CompactProofSize => result.compact_proof_size.into(), }) } @@ -375,6 +379,7 @@ mod tests { writes, repeat_writes: 0, proof_size: 0, + compact_proof_size: 0, } } diff --git a/frame/benchmarking/src/lib.rs b/frame/benchmarking/src/lib.rs index ea1bfbd68104e..f56001d9a1d78 100644 --- a/frame/benchmarking/src/lib.rs +++ b/frame/benchmarking/src/lib.rs @@ -774,8 +774,11 @@ macro_rules! impl_benchmark { // Calculate the diff caused by the benchmark. let elapsed_extrinsic = finish_extrinsic.saturating_sub(start_extrinsic); - let diff_pov = match (start_pov, end_pov) { - (Some(start), Some(end)) => end.saturating_sub(start), + let (diff_pov, compact_diff_pov) = match (start_pov, end_pov) { + (Some((start, compact_start)), Some((end, compact_end))) => ( + end.saturating_sub(start), + compact_end.saturating_sub(compact_start), + ), _ => Default::default(), }; @@ -806,6 +809,7 @@ macro_rules! impl_benchmark { writes: read_write_count.2, repeat_writes: read_write_count.3, proof_size: diff_pov, + compact_proof_size: compact_diff_pov, }); } diff --git a/frame/benchmarking/src/utils.rs b/frame/benchmarking/src/utils.rs index 2db7b2e95d9d4..e051e9b6ed980 100644 --- a/frame/benchmarking/src/utils.rs +++ b/frame/benchmarking/src/utils.rs @@ -63,6 +63,7 @@ pub struct BenchmarkResults { pub writes: u32, pub repeat_writes: u32, pub proof_size: u32, + pub compact_proof_size: u32, } /// Configuration used to setup and run runtime benchmarks. @@ -165,7 +166,7 @@ pub trait Benchmarking { } /// Get current estimated proof size. - fn proof_size(&self) -> Option { + fn proof_size(&self) -> Option<(u32, u32)> { self.proof_size() } } diff --git a/primitives/externalities/src/lib.rs b/primitives/externalities/src/lib.rs index c90881b76e26c..783f65fdbca10 100644 --- a/primitives/externalities/src/lib.rs +++ b/primitives/externalities/src/lib.rs @@ -288,7 +288,7 @@ pub trait Externalities: ExtensionStore { /// /// Returns estimated proof size for the state queries so far. /// Proof is reset on commit and wipe. - fn proof_size(&self) -> Option { + fn proof_size(&self) -> Option<(u32, u32)> { None } } diff --git a/primitives/state-machine/src/backend.rs b/primitives/state-machine/src/backend.rs index 1a8892f8dd141..579872d3f4d6f 100644 --- a/primitives/state-machine/src/backend.rs +++ b/primitives/state-machine/src/backend.rs @@ -247,7 +247,7 @@ pub trait Backend: sp_std::fmt::Debug { fn set_whitelist(&self, _: Vec) {} /// Estimate proof size - fn proof_size(&self) -> Option { + fn proof_size(&self) -> Option<(u32, u32)> { unimplemented!() } } diff --git a/primitives/state-machine/src/ext.rs b/primitives/state-machine/src/ext.rs index 424a3c6c421a2..5ca526de37012 100644 --- a/primitives/state-machine/src/ext.rs +++ b/primitives/state-machine/src/ext.rs @@ -713,7 +713,7 @@ where self.backend.set_whitelist(new) } - fn proof_size(&self) -> Option { + fn proof_size(&self) -> Option<(u32, u32)> { self.backend.proof_size() } } diff --git a/utils/frame/benchmarking-cli/src/command.rs b/utils/frame/benchmarking-cli/src/command.rs index 80d95d1c86dcf..6974b85978077 100644 --- a/utils/frame/benchmarking-cli/src/command.rs +++ b/utils/frame/benchmarking-cli/src/command.rs @@ -126,13 +126,13 @@ impl BenchmarkCmd { // Print the table header batch.results[0].components.iter().for_each(|param| print!("{:?},", param.0)); - print!("extrinsic_time_ns,storage_root_time_ns,reads,repeat_reads,writes,repeat_writes,proof_size_bytes\n"); + print!("extrinsic_time_ns,storage_root_time_ns,reads,repeat_reads,writes,repeat_writes,proof_size_bytes,compact_proof_size_bytes\n"); // Print the values batch.results.iter().for_each(|result| { let parameters = &result.components; parameters.iter().for_each(|param| print!("{:?},", param.1)); // Print extrinsic time and storage root time - print!("{:?},{:?},{:?},{:?},{:?},{:?},{:?}\n", + print!("{:?},{:?},{:?},{:?},{:?},{:?},{:?},{:?}\n", result.extrinsic_time, result.storage_root_time, result.reads, @@ -140,6 +140,7 @@ impl BenchmarkCmd { result.writes, result.repeat_writes, result.proof_size, + result.compact_proof_size, ); }); diff --git a/utils/frame/benchmarking-cli/src/writer.rs b/utils/frame/benchmarking-cli/src/writer.rs index 6fd6cc6eefdc6..a15e4b7e716bc 100644 --- a/utils/frame/benchmarking-cli/src/writer.rs +++ b/utils/frame/benchmarking-cli/src/writer.rs @@ -422,6 +422,7 @@ mod test { writes: (base + slope * i).into(), repeat_writes: 0, proof_size: 0, + compact_proof_size: 0, } ) } From 4edc38aadc66412e82cca9a32ec0c0041bda23e1 Mon Sep 17 00:00:00 2001 From: cheme Date: Thu, 8 Apr 2021 17:10:12 +0200 Subject: [PATCH 13/25] trie-db 22.3 is needed --- primitives/trie/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/primitives/trie/Cargo.toml b/primitives/trie/Cargo.toml index 4396550a48a8f..9584ae678d409 100644 --- a/primitives/trie/Cargo.toml +++ b/primitives/trie/Cargo.toml @@ -21,7 +21,7 @@ harness = false codec = { package = "parity-scale-codec", version = "2.0.0", default-features = false } sp-std = { version = "3.0.0", default-features = false, path = "../std" } hash-db = { version = "0.15.2", default-features = false } -trie-db = { version = "0.22.2", default-features = false } +trie-db = { version = "0.22.3", default-features = false } trie-root = { version = "0.16.0", default-features = false } memory-db = { version = "0.26.0", default-features = false } sp-core = { version = "3.0.0", default-features = false, path = "../core" } From 6f899c0f8c8d016d2161c88d361faf5a341e9706 Mon Sep 17 00:00:00 2001 From: cheme Date: Thu, 8 Apr 2021 17:22:39 +0200 Subject: [PATCH 14/25] line width --- utils/wasm-builder/src/wasm_project.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/utils/wasm-builder/src/wasm_project.rs b/utils/wasm-builder/src/wasm_project.rs index 2ad220f97246c..4787358dcdd78 100644 --- a/utils/wasm-builder/src/wasm_project.rs +++ b/utils/wasm-builder/src/wasm_project.rs @@ -232,7 +232,8 @@ fn create_project_cargo_toml( wasm_workspace_toml.insert("profile".into(), profile.into()); // Add patch section from the project root `Cargo.toml` - while let Some(mut patch) = workspace_toml.remove("patch").and_then(|p| p.try_into::
().ok()) { + while let Some(mut patch) = workspace_toml.remove("patch") + .and_then(|p| p.try_into::
().ok()) { // Iterate over all patches and make the patch path absolute from the workspace root path. patch.iter_mut() .filter_map(|p| From d3eb91c2dfaf72c12881d692131eb818b3a9bdf2 Mon Sep 17 00:00:00 2001 From: cheme Date: Thu, 8 Apr 2021 17:37:20 +0200 Subject: [PATCH 15/25] split line --- utils/frame/benchmarking-cli/src/command.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/utils/frame/benchmarking-cli/src/command.rs b/utils/frame/benchmarking-cli/src/command.rs index 6974b85978077..3dfa9bfea794d 100644 --- a/utils/frame/benchmarking-cli/src/command.rs +++ b/utils/frame/benchmarking-cli/src/command.rs @@ -126,7 +126,8 @@ impl BenchmarkCmd { // Print the table header batch.results[0].components.iter().for_each(|param| print!("{:?},", param.0)); - print!("extrinsic_time_ns,storage_root_time_ns,reads,repeat_reads,writes,repeat_writes,proof_size_bytes,compact_proof_size_bytes\n"); + print!("extrinsic_time_ns,storage_root_time_ns,reads,repeat_reads, + \\writes,repeat_writes,proof_size_bytes,compact_proof_size_bytes\n"); // Print the values batch.results.iter().for_each(|result| { let parameters = &result.components; From 4475edd658b97c4fb3e1c11540d5c6a680d02b5a Mon Sep 17 00:00:00 2001 From: Emeric Chevalier Date: Fri, 9 Apr 2021 17:35:09 +0200 Subject: [PATCH 16/25] fixes for bench (additional root may not be needed as original issue was with empty proof). --- client/db/src/bench.rs | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/client/db/src/bench.rs b/client/db/src/bench.rs index 763aadf8d6053..62c67575b1525 100644 --- a/client/db/src/bench.rs +++ b/client/db/src/bench.rs @@ -118,6 +118,7 @@ pub struct BenchmarkingState { read_write_tracker: RefCell, whitelist: RefCell>, proof_recorder: Option>>, + proof_recorder_root: Cell, } impl BenchmarkingState { @@ -130,7 +131,7 @@ impl BenchmarkingState { let mut state = BenchmarkingState { state: RefCell::new(None), db: Cell::new(None), - root: Cell::new(root), + root: Cell::new(root.clone()), genesis: Default::default(), genesis_root: Default::default(), record: Default::default(), @@ -140,6 +141,7 @@ impl BenchmarkingState { read_write_tracker: Default::default(), whitelist: Default::default(), proof_recorder: record_proof.then(Default::default), + proof_recorder_root: Cell::new(root.clone()), }; state.add_whitelist_to_tracker(); @@ -169,6 +171,7 @@ impl BenchmarkingState { self.db.set(Some(db.clone())); if let Some(recorder) = &self.proof_recorder { recorder.write().clear(); + self.proof_recorder_root.set(self.root.get()); } let storage_db = Arc::new(StorageDb:: { db, @@ -525,12 +528,29 @@ impl StateBackend> for BenchmarkingState { .filter_map(|(_k, v)| v.as_ref().map(|v| v.to_vec())) .collect()); let proof_size = proof.encoded_size() as u32; - let compact_proof = encode_compact::>>( - proof, - self.root.get(), - ).unwrap(); + let proof_recorder_root = self.proof_recorder_root.get(); + let compact_size = if proof_recorder_root == Default::default() || proof_size == 1 { + // empty trie + proof_size + } else { + let compact_proof = encode_compact::>>( + proof, + proof_recorder_root, + ); + if compact_proof.is_err() { + panic!( + "proof rec root {:?}, root {:?}, genesis {:?}, rec_len {:?}", + self.proof_recorder_root.get(), + self.root.get(), + self.genesis_root, + proof_size, + ); + } + let compact_proof = compact_proof.unwrap(); + compact_proof.encoded_size() as u32 + }; - (proof_size, compact_proof.encoded_size() as u32) + (proof_size, compact_size) }) } } From c0fc8fd1140bf29fed471cd3a6264e36d2d7669b Mon Sep 17 00:00:00 2001 From: cheme Date: Mon, 3 May 2021 19:18:41 +0200 Subject: [PATCH 17/25] revert compact from block size calculation. --- .../basic-authorship/src/basic_authorship.rs | 28 ++++--------------- client/block-builder/src/lib.rs | 21 ++------------ 2 files changed, 7 insertions(+), 42 deletions(-) diff --git a/client/basic-authorship/src/basic_authorship.rs b/client/basic-authorship/src/basic_authorship.rs index 851a4f0c8ffc9..c8277d3b5d32c 100644 --- a/client/basic-authorship/src/basic_authorship.rs +++ b/client/basic-authorship/src/basic_authorship.rs @@ -51,16 +51,6 @@ use sc_proposer_metrics::MetricsLink as PrometheusMetrics; /// transferred to other nodes. pub const DEFAULT_BLOCK_SIZE_LIMIT: usize = 4 * 1024 * 1024 + 512; -/// Should proof size be included. -pub enum IncludeProofSize { - /// Include full proof size. - True, - /// Do not include proof size. - False, - /// Include proof size in compact form. - Compacted, -} - /// Proposer factory. pub struct ProposerFactory { spawn_handle: Box, @@ -77,7 +67,7 @@ pub struct ProposerFactory { default_block_size_limit: usize, telemetry: Option, /// When estimating the block size, should the proof be included? - include_proof_in_block_size_estimation: IncludeProofSize, + include_proof_in_block_size_estimation: bool, /// phantom member to pin the `Backend`/`ProofRecording` type. _phantom: PhantomData<(B, PR)>, } @@ -100,7 +90,7 @@ impl ProposerFactory { default_block_size_limit: DEFAULT_BLOCK_SIZE_LIMIT, telemetry, client, - include_proof_in_block_size_estimation: IncludeProofSize::False, + include_proof_in_block_size_estimation: false, _phantom: PhantomData, } } @@ -127,22 +117,14 @@ impl ProposerFactory { metrics: PrometheusMetrics::new(prometheus), default_block_size_limit: DEFAULT_BLOCK_SIZE_LIMIT, telemetry, - include_proof_in_block_size_estimation: IncludeProofSize::True, + include_proof_in_block_size_estimation: true, _phantom: PhantomData, } } /// Disable the proof inclusion when estimating the block size. pub fn disable_proof_in_block_size_estimation(&mut self) { - self.include_proof_in_block_size_estimation = IncludeProofSize::False; - } - - /// Use the size of the compact proof in block size estimation. - /// Generally this is highly discourage as it can be slow. - /// Therefore using non compact proof as an over-estimation can - /// be better when slow is not possible. - pub fn use_compact_proof_in_block_size_estimation(&mut self) { - self.include_proof_in_block_size_estimation = IncludeProofSize::Compacted; + self.include_proof_in_block_size_estimation = false; } } @@ -233,7 +215,7 @@ pub struct Proposer { now: Box time::Instant + Send + Sync>, metrics: PrometheusMetrics, default_block_size_limit: usize, - include_proof_in_block_size_estimation: IncludeProofSize, + include_proof_in_block_size_estimation: bool, telemetry: Option, _phantom: PhantomData<(B, PR)>, } diff --git a/client/block-builder/src/lib.rs b/client/block-builder/src/lib.rs index 9c904fb6c3c37..7d391f8fb85b3 100644 --- a/client/block-builder/src/lib.rs +++ b/client/block-builder/src/lib.rs @@ -280,27 +280,10 @@ where /// /// If `include_proof` is `true`, the estimated size of the storage proof will be added /// to the estimation. - /// If `compact_proof` is `true`, the size of the compacted storage proof will be added - /// to the estimation. This can be slow. - pub fn estimate_block_size(&self, include_proof: bool, compact_proof: bool) -> usize { + pub fn estimate_block_size(&self, include_proof: bool) -> usize { let size = self.estimated_header_size + self.extrinsics.encoded_size(); - if compact_proof { - let proof = self.api.proof_recorder().to_storage_proof(); - let proof_recorder_root = self.proof_recorder_root.get(); - let compact_size = if proof_recorder_root == Default::default() || proof_size == 1 { - // empty trie - proof_size - } else { - if let Some(size) = CompactProof::encoded_compact_size::>( - proof, - proof_recorder_root, - ) { - size as u32 - } else { - } - size + CompactProof:: Date: Fri, 4 Jun 2021 15:03:00 +0200 Subject: [PATCH 18/25] New error type for compression. --- primitives/trie/src/error.rs | 2 +- primitives/trie/src/lib.rs | 2 +- primitives/trie/src/trie_codec.rs | 107 ++++++++++++++++++++++++------ 3 files changed, 89 insertions(+), 22 deletions(-) diff --git a/primitives/trie/src/error.rs b/primitives/trie/src/error.rs index 8e1d9b974ffd5..bdaa49b1156f7 100644 --- a/primitives/trie/src/error.rs +++ b/primitives/trie/src/error.rs @@ -26,7 +26,7 @@ pub enum Error { /// Bad format. BadFormat, /// Decoding error. - Decode(codec::Error) + Decode(codec::Error), } impl From for Error { diff --git a/primitives/trie/src/lib.rs b/primitives/trie/src/lib.rs index 2977b8b655d7c..89bef715ba99a 100644 --- a/primitives/trie/src/lib.rs +++ b/primitives/trie/src/lib.rs @@ -48,7 +48,7 @@ pub use memory_db::prefixed_key; pub use hash_db::{HashDB as HashDBT, EMPTY_PREFIX}; /// Trie codec reexport, mainly child trie support /// for trie compact proof. -pub use trie_codec::{decode_compact, encode_compact}; +pub use trie_codec::{decode_compact, encode_compact, Error as CompactProofError}; #[derive(Default)] /// substrate trie layout diff --git a/primitives/trie/src/trie_codec.rs b/primitives/trie/src/trie_codec.rs index 0e950883562c6..1c7d224cccbc2 100644 --- a/primitives/trie/src/trie_codec.rs +++ b/primitives/trie/src/trie_codec.rs @@ -27,11 +27,72 @@ use crate::{ use sp_std::boxed::Box; use sp_std::vec::Vec; use trie_db::Trie; +#[cfg(feature="std")] +use std::fmt; +#[cfg(feature="std")] +use std::error::Error as StdError; -type VerifyError = crate::VerifyError, Box>>; -fn verify_error(error: Box>) -> VerifyError { - VerifyError::::DecodeError(error) +/// Error for trie node decoding. +pub enum Error { + /// Verification failed due to root mismatch. + RootMismatch(TrieHash, TrieHash), + /// Missing nodes in proof. + IncompleteProof, + /// Compact node is not needed. + ExtraneousChildNode, + /// Child content with root not in proof. + ExtraneousChildProof(TrieHash), + /// Bad child trie root. + InvalidChildRoot(Vec, Vec), + /// Errors from trie crate. + TrieError(Box>), +} + +impl From>> for Error { + fn from(error: Box>) -> Self { + Error::TrieError(error) + } +} + +#[cfg(feature="std")] +impl StdError for Error { + fn description(&self) -> &str { + match self { + Error::InvalidChildRoot(..) => "Invalid child root error", + Error::TrieError(..) => "Trie db error", + Error::RootMismatch(..) => "Trie db error", + Error::IncompleteProof => "Incomplete proof", + Error::ExtraneousChildNode => "Extraneous child node", + Error::ExtraneousChildProof(..) => "Extraneous child proof", + } + } +} + +#[cfg(feature="std")] +impl fmt::Debug for Error { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + ::fmt(&self, f) + } +} + +#[cfg(feature="std")] +impl fmt::Display for Error { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Error::InvalidChildRoot(k, v) => write!(f, "InvalidChildRoot at {:x?}: {:x?}", k, v), + Error::TrieError(e) => write!(f, "Trie error: {}", e), + Error::IncompleteProof => write!(f, "Incomplete proof"), + Error::ExtraneousChildNode => write!(f, "Child node content with no root in proof"), + Error::ExtraneousChildProof(root) => write!(f, "Proof of child trie {:x?} not in parent proof", root.as_ref()), + Error::RootMismatch(root, expected) => write!( + f, + "Verification error, root is {:x?}, expected: {:x?}", + root.as_ref(), + expected.as_ref(), + ), + } + } } /// Decode a compact proof. @@ -45,7 +106,7 @@ pub fn decode_compact<'a, L, DB, I>( db: &mut DB, encoded: I, expected_root: Option<&TrieHash>, -) -> Result, VerifyError> +) -> Result, Error> where L: TrieConfiguration, DB: HashDBT + hash_db::HashDBRef, @@ -55,20 +116,21 @@ pub fn decode_compact<'a, L, DB, I>( let (top_root, _nb_used) = trie_db::decode_compact_from_iter::( db, &mut nodes_iter, - ).map_err(verify_error::)?; + )?; + // Only check root if expected root is passed as argument. if let Some(expected_root) = expected_root { if expected_root != &top_root { - return Err(VerifyError::::RootMismatch(expected_root.clone())); + return Err(Error::RootMismatch(top_root.clone(), expected_root.clone())); } } let mut child_tries = Vec::new(); { // fetch child trie roots - let trie = crate::TrieDB::::new(db, &top_root).map_err(verify_error::)?; + let trie = crate::TrieDB::::new(db, &top_root)?; - let mut iter = trie.iter().map_err(verify_error::)?; + let mut iter = trie.iter()?; let childtrie_roots = sp_core::storage::well_known_keys::DEFAULT_CHILD_STORAGE_KEY_PREFIX; if iter.seek(childtrie_roots).is_ok() { @@ -80,7 +142,7 @@ pub fn decode_compact<'a, L, DB, I>( let mut root = TrieHash::::default(); // still in a proof so prevent panic if root.as_mut().len() != value.as_slice().len() { - return Err(VerifyError::::RootMismatch(Default::default())); + return Err(Error::InvalidChildRoot(key, value)); } root.as_mut().copy_from_slice(value.as_ref()); child_tries.push(root); @@ -89,7 +151,7 @@ pub fn decode_compact<'a, L, DB, I>( // require access to data in the proof. Some(Err(error)) => match *error { trie_db::TrieError::IncompleteDatabase(..) => (), - e => return Err(VerifyError::::DecodeError(Box::new(e))), + e => return Err(Box::new(e).into()), }, _ => break, } @@ -98,31 +160,35 @@ pub fn decode_compact<'a, L, DB, I>( } if !HashDBT::::contains(db, &top_root, EMPTY_PREFIX) { - return Err(VerifyError::::IncompleteProof); + return Err(Error::IncompleteProof); } let mut previous_extracted_child_trie = None; for child_root in child_tries.into_iter() { - if previous_extracted_child_trie == None { + if previous_extracted_child_trie.is_none() { let (top_root, _) = trie_db::decode_compact_from_iter::( db, &mut nodes_iter, - ).map_err(verify_error::)?; + )?; previous_extracted_child_trie = Some(top_root); } - // we allow skipping child root by only - // decoding next on match. + // we do not early exit on root mismatch but try the + // other read from proof (some child root may be + // in proof without actual child content). if Some(child_root) == previous_extracted_child_trie { previous_extracted_child_trie = None; } } + if let Some(child_root) = previous_extracted_child_trie { - return Err(VerifyError::::RootMismatch(child_root)); + // A child root was read from proof but is not present + // in top trie. + return Err(Error::ExtraneousChildProof(child_root)); } if nodes_iter.next().is_some() { - return Err(VerifyError::::ExtraneousNode); + return Err(Error::ExtraneousChildNode); } Ok(top_root) @@ -138,7 +204,7 @@ pub fn decode_compact<'a, L, DB, I>( pub fn encode_compact<'a, L>( proof: StorageProof, root: TrieHash, -) -> Result>> +) -> Result> where L: TrieConfiguration, { @@ -156,7 +222,8 @@ pub fn encode_compact<'a, L>( Some(Ok((key, value))) if key.starts_with(childtrie_roots) => { let mut root = TrieHash::::default(); if root.as_mut().len() != value.as_slice().len() { - return Err(Box::new(trie_db::TrieError::InvalidStateRoot(Default::default()))); + // some child trie root in top trie are not an encoded hash. + return Err(Error::InvalidChildRoot(key.to_vec(), value.to_vec())); } root.as_mut().copy_from_slice(value.as_ref()); child_tries.push(root); @@ -165,7 +232,7 @@ pub fn encode_compact<'a, L>( // require access to data in the proof. Some(Err(error)) => match *error { trie_db::TrieError::IncompleteDatabase(..) => (), - e => return Err(Box::new(e)), + e => return Err(Box::new(e).into()), }, _ => break, } From c37046e63e37a7536fff7e60ab1f0a83321a7be3 Mon Sep 17 00:00:00 2001 From: cheme Date: Fri, 4 Jun 2021 15:47:37 +0200 Subject: [PATCH 19/25] Adding test (incomplete (failing)). Also lacking real proof checking (no good primitives in sp-trie crate). --- primitives/trie/src/lib.rs | 60 ++++++++++++++++++++++++++++ primitives/trie/src/storage_proof.rs | 13 +++--- 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/primitives/trie/src/lib.rs b/primitives/trie/src/lib.rs index 89bef715ba99a..be23270af0b39 100644 --- a/primitives/trie/src/lib.rs +++ b/primitives/trie/src/lib.rs @@ -894,4 +894,64 @@ mod tests { assert_eq!(first_storage_root, second_storage_root); } + + #[test] + fn compact_proof_processing() { + let mut pairs = vec![ + (hex!("dd8502").to_vec(), vec![1u8; 32]), + (hex!("ff0203").to_vec(), vec![2u8; 32]), + ]; + let pairs_child = vec![ + (hex!("e502").to_vec(), vec![1u8; 32]), + (hex!("0203").to_vec(), vec![2u8; 32]), + ]; + + let mut memdb = MemoryDB::default(); + let mut populate_child_trie = |key: &[u8], content: Vec<(Vec, Vec)>| -> (Vec, Vec) { + let mut root = Default::default(); + populate_trie::(&mut memdb, &mut root, &content); + let mut child_key = sp_core::storage::well_known_keys::DEFAULT_CHILD_STORAGE_KEY_PREFIX.to_vec(); + child_key.extend_from_slice(key); + pairs.push((child_key.clone(), root.as_ref().to_vec())); + (child_key, root.as_ref().to_vec()) + }; + let (child, child_root) = populate_child_trie(b"child", pairs_child); + + let mut root = Default::default(); + populate_trie::(&mut memdb, &mut root, &pairs); + + let read_key_set = &[ + (hex!("dd8502").to_vec(), Some(vec![1u8; 32])), + (child, Some(child_root)), + (b"dummy".to_vec(), None), + ]; + + let proof = generate_trie_proof::( + &memdb, + root, + read_key_set.iter().map(|kv| &kv.0), + ).unwrap(); + + let compact_proof = crate::trie_codec::encode_compact::( + StorageProof::new(proof), + root.clone(), + ).unwrap(); + let mut memdb = MemoryDB::default(); + let _ = crate::trie_codec::decode_compact::( + &mut memdb, + compact_proof.encoded_nodes.iter().map(|n| n.as_slice()), + Some(&root), + ).unwrap(); + let nodes: Vec> = memdb.drain().into_iter().filter_map(|(_, v)| { + (v.1 > 0).then(|| v.0) + }).collect(); + // Check that a K, V included into the proof are verified. + assert!(verify_trie_proof::( + &root, + nodes.as_slice(), + read_key_set, + ).is_ok()); + } + + } diff --git a/primitives/trie/src/storage_proof.rs b/primitives/trie/src/storage_proof.rs index 227c21fcbb191..ce56caa070b13 100644 --- a/primitives/trie/src/storage_proof.rs +++ b/primitives/trie/src/storage_proof.rs @@ -106,11 +106,7 @@ impl CompactProof { proof, root, ); - if let Ok(proof) = compact_proof { - Some(proof.encoded_size()) - } else { - None - } + compact_proof.ok().map(|p| p.encoded_size()) } } @@ -145,3 +141,10 @@ impl From for crate::MemoryDB { db } } + +#[cfg(test)] +mod test { + #[test] + fn test_compact_proof() { + } +} From cfd923a4e1f8fe0ad283702dab5ce4c3132ea058 Mon Sep 17 00:00:00 2001 From: Emeric Chevalier Date: Fri, 4 Jun 2021 17:49:40 +0200 Subject: [PATCH 20/25] There is currently no proof recording utility in sp_trie, removing test. --- primitives/trie/src/lib.rs | 60 -------------------------------------- 1 file changed, 60 deletions(-) diff --git a/primitives/trie/src/lib.rs b/primitives/trie/src/lib.rs index be23270af0b39..89bef715ba99a 100644 --- a/primitives/trie/src/lib.rs +++ b/primitives/trie/src/lib.rs @@ -894,64 +894,4 @@ mod tests { assert_eq!(first_storage_root, second_storage_root); } - - #[test] - fn compact_proof_processing() { - let mut pairs = vec![ - (hex!("dd8502").to_vec(), vec![1u8; 32]), - (hex!("ff0203").to_vec(), vec![2u8; 32]), - ]; - let pairs_child = vec![ - (hex!("e502").to_vec(), vec![1u8; 32]), - (hex!("0203").to_vec(), vec![2u8; 32]), - ]; - - let mut memdb = MemoryDB::default(); - let mut populate_child_trie = |key: &[u8], content: Vec<(Vec, Vec)>| -> (Vec, Vec) { - let mut root = Default::default(); - populate_trie::(&mut memdb, &mut root, &content); - let mut child_key = sp_core::storage::well_known_keys::DEFAULT_CHILD_STORAGE_KEY_PREFIX.to_vec(); - child_key.extend_from_slice(key); - pairs.push((child_key.clone(), root.as_ref().to_vec())); - (child_key, root.as_ref().to_vec()) - }; - let (child, child_root) = populate_child_trie(b"child", pairs_child); - - let mut root = Default::default(); - populate_trie::(&mut memdb, &mut root, &pairs); - - let read_key_set = &[ - (hex!("dd8502").to_vec(), Some(vec![1u8; 32])), - (child, Some(child_root)), - (b"dummy".to_vec(), None), - ]; - - let proof = generate_trie_proof::( - &memdb, - root, - read_key_set.iter().map(|kv| &kv.0), - ).unwrap(); - - let compact_proof = crate::trie_codec::encode_compact::( - StorageProof::new(proof), - root.clone(), - ).unwrap(); - let mut memdb = MemoryDB::default(); - let _ = crate::trie_codec::decode_compact::( - &mut memdb, - compact_proof.encoded_nodes.iter().map(|n| n.as_slice()), - Some(&root), - ).unwrap(); - let nodes: Vec> = memdb.drain().into_iter().filter_map(|(_, v)| { - (v.1 > 0).then(|| v.0) - }).collect(); - // Check that a K, V included into the proof are verified. - assert!(verify_trie_proof::( - &root, - nodes.as_slice(), - read_key_set, - ).is_ok()); - } - - } From f25fc34643c254ed79971d4244290a17d34222b7 Mon Sep 17 00:00:00 2001 From: Emeric Chevalier Date: Fri, 4 Jun 2021 18:43:55 +0200 Subject: [PATCH 21/25] small test of child root in proof without a child proof. --- primitives/state-machine/src/lib.rs | 85 ++++++++++++++++++++++------- 1 file changed, 65 insertions(+), 20 deletions(-) diff --git a/primitives/state-machine/src/lib.rs b/primitives/state-machine/src/lib.rs index 23057e208b04b..fa1944c8954e6 100644 --- a/primitives/state-machine/src/lib.rs +++ b/primitives/state-machine/src/lib.rs @@ -1402,28 +1402,29 @@ mod tests { } } + fn test_compact(remote_proof: StorageProof, remote_root: &sp_core::H256) -> StorageProof { + type Layout = sp_trie::Layout; + let compact_remote_proof = sp_trie::encode_compact::( + remote_proof, + remote_root.clone(), + ).unwrap(); + let mut db = sp_trie::MemoryDB::::new(&[]); + sp_trie::decode_compact::( + &mut db, + compact_remote_proof.iter_compact_encoded_nodes(), + Some(remote_root), + ).unwrap(); + StorageProof::new(db.drain().into_iter().filter_map(|kv| + if (kv.1).1 > 0 { + Some((kv.1).0) + } else { + None + } + ).collect()) + } + #[test] fn prove_read_and_proof_check_works() { - fn test_compact(remote_proof: StorageProof, remote_root: &sp_core::H256) -> StorageProof { - type Layout = sp_trie::Layout; - let compact_remote_proof = sp_trie::encode_compact::( - remote_proof, - remote_root.clone(), - ).unwrap(); - let mut db = sp_trie::MemoryDB::::new(&[]); - sp_trie::decode_compact::( - &mut db, - compact_remote_proof.iter_compact_encoded_nodes(), - Some(remote_root), - ).unwrap(); - StorageProof::new(db.drain().into_iter().filter_map(|kv| - if (kv.1).1 > 0 { - Some((kv.1).0) - } else { - None - } - ).collect()) - } let child_info = ChildInfo::new_default(b"sub1"); let child_info = &child_info; // fetch read proof from 'remote' full node @@ -1479,6 +1480,50 @@ mod tests { ); } + #[test] + fn compact_multiple_child_trie() { + // this root will be queried + let child_info1 = ChildInfo::new_default(b"sub1"); + // this root will not be include in proof + let child_info2 = ChildInfo::new_default(b"sub2"); + // this root will be include in proof + let child_info3 = ChildInfo::new_default(b"sub"); + let mut remote_backend = trie_backend::tests::test_trie(); + let (remote_root, transaction) = remote_backend.full_storage_root( + std::iter::empty(), + vec![ + (&child_info1, vec![ + (&b"key1"[..], Some(&b"val2"[..])), + (&b"key2"[..], Some(&b"val3"[..])), + ].into_iter()), + (&child_info2, vec![ + (&b"key3"[..], Some(&b"val4"[..])), + (&b"key4"[..], Some(&b"val5"[..])), + ].into_iter()), + (&child_info3, vec![ + (&b"key5"[..], Some(&b"val6"[..])), + (&b"key6"[..], Some(&b"val7"[..])), + ].into_iter()), + ].into_iter(), + ); + remote_backend.backend_storage_mut().consolidate(transaction); + remote_backend.essence.set_root(remote_root.clone()); + let remote_proof = prove_child_read( + remote_backend, + &child_info1, + &[b"key1"], + ).unwrap(); + let remote_proof = test_compact(remote_proof, &remote_root); + let local_result1 = read_child_proof_check::( + remote_root, + remote_proof.clone(), + &child_info1, + &[b"key1"], + ).unwrap(); + assert_eq!(local_result1.len(), 1); + assert_eq!(local_result1.get(&b"key1"[..]), Some(&Some(b"val2".to_vec()))); + } + #[test] fn child_storage_uuid() { From 19c132ed042e29d6d40cb997b43a57173d831ea6 Mon Sep 17 00:00:00 2001 From: Emeric Chevalier Date: Sun, 6 Jun 2021 09:23:36 +0200 Subject: [PATCH 22/25] remove empty test. --- primitives/trie/src/storage_proof.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/primitives/trie/src/storage_proof.rs b/primitives/trie/src/storage_proof.rs index ce56caa070b13..1cea1b963ff73 100644 --- a/primitives/trie/src/storage_proof.rs +++ b/primitives/trie/src/storage_proof.rs @@ -141,10 +141,3 @@ impl From for crate::MemoryDB { db } } - -#[cfg(test)] -mod test { - #[test] - fn test_compact_proof() { - } -} From 515c2fd666154a83a8bb945793448bbc1a04168d Mon Sep 17 00:00:00 2001 From: Emeric Chevalier Date: Sun, 6 Jun 2021 10:06:13 +0200 Subject: [PATCH 23/25] remove non compact proof size --- client/db/src/bench.rs | 8 +++----- frame/benchmarking/src/analysis.rs | 5 ----- frame/benchmarking/src/lib.rs | 8 ++------ frame/benchmarking/src/utils.rs | 3 +-- primitives/externalities/src/lib.rs | 2 +- primitives/state-machine/src/backend.rs | 2 +- primitives/state-machine/src/ext.rs | 2 +- utils/frame/benchmarking-cli/src/command.rs | 3 +-- utils/frame/benchmarking-cli/src/writer.rs | 1 - 9 files changed, 10 insertions(+), 24 deletions(-) diff --git a/client/db/src/bench.rs b/client/db/src/bench.rs index 1f5357b3ffc89..e490014ed96c4 100644 --- a/client/db/src/bench.rs +++ b/client/db/src/bench.rs @@ -520,12 +520,12 @@ impl StateBackend> for BenchmarkingState { self.state.borrow().as_ref().map_or(sp_state_machine::UsageInfo::empty(), |s| s.usage_info()) } - fn proof_size(&self) -> Option<(u32, u32)> { + fn proof_size(&self) -> Option { self.proof_recorder.as_ref().map(|recorder| { let proof_size = recorder.estimate_encoded_size() as u32; let proof = recorder.to_storage_proof(); let proof_recorder_root = self.proof_recorder_root.get(); - let compact_size = if proof_recorder_root == Default::default() || proof_size == 1 { + if proof_recorder_root == Default::default() || proof_size == 1 { // empty trie proof_size } else { @@ -543,9 +543,7 @@ impl StateBackend> for BenchmarkingState { proof_size, ); } - }; - - (proof_size, compact_size) + } }) } } diff --git a/frame/benchmarking/src/analysis.rs b/frame/benchmarking/src/analysis.rs index e1a49c4a66962..7b6d8838fd21c 100644 --- a/frame/benchmarking/src/analysis.rs +++ b/frame/benchmarking/src/analysis.rs @@ -39,7 +39,6 @@ pub enum BenchmarkSelector { Reads, Writes, ProofSize, - CompactProofSize, } #[derive(Debug)] @@ -89,7 +88,6 @@ impl Analysis { BenchmarkSelector::Reads => result.reads.into(), BenchmarkSelector::Writes => result.writes.into(), BenchmarkSelector::ProofSize => result.proof_size.into(), - BenchmarkSelector::CompactProofSize => result.compact_proof_size.into(), } ).collect(); @@ -131,7 +129,6 @@ impl Analysis { BenchmarkSelector::Reads => result.reads.into(), BenchmarkSelector::Writes => result.writes.into(), BenchmarkSelector::ProofSize => result.proof_size.into(), - BenchmarkSelector::CompactProofSize => result.compact_proof_size.into(), }; (result.components[i].1, data) }) @@ -197,7 +194,6 @@ impl Analysis { BenchmarkSelector::Reads => result.reads.into(), BenchmarkSelector::Writes => result.writes.into(), BenchmarkSelector::ProofSize => result.proof_size.into(), - BenchmarkSelector::CompactProofSize => result.compact_proof_size.into(), }) } @@ -379,7 +375,6 @@ mod tests { writes, repeat_writes: 0, proof_size: 0, - compact_proof_size: 0, } } diff --git a/frame/benchmarking/src/lib.rs b/frame/benchmarking/src/lib.rs index 79ade2eefa043..8160bd5d1dd21 100644 --- a/frame/benchmarking/src/lib.rs +++ b/frame/benchmarking/src/lib.rs @@ -787,11 +787,8 @@ macro_rules! impl_benchmark { // Calculate the diff caused by the benchmark. let elapsed_extrinsic = finish_extrinsic.saturating_sub(start_extrinsic); - let (diff_pov, compact_diff_pov) = match (start_pov, end_pov) { - (Some((start, compact_start)), Some((end, compact_end))) => ( - end.saturating_sub(start), - compact_end.saturating_sub(compact_start), - ), + let diff_pov = match (start_pov, end_pov) { + (Some(start), Some(end)) => end.saturating_sub(start), _ => Default::default(), }; @@ -836,7 +833,6 @@ macro_rules! impl_benchmark { writes: read_write_count.2, repeat_writes: read_write_count.3, proof_size: diff_pov, - compact_proof_size: compact_diff_pov, }); } diff --git a/frame/benchmarking/src/utils.rs b/frame/benchmarking/src/utils.rs index e051e9b6ed980..2db7b2e95d9d4 100644 --- a/frame/benchmarking/src/utils.rs +++ b/frame/benchmarking/src/utils.rs @@ -63,7 +63,6 @@ pub struct BenchmarkResults { pub writes: u32, pub repeat_writes: u32, pub proof_size: u32, - pub compact_proof_size: u32, } /// Configuration used to setup and run runtime benchmarks. @@ -166,7 +165,7 @@ pub trait Benchmarking { } /// Get current estimated proof size. - fn proof_size(&self) -> Option<(u32, u32)> { + fn proof_size(&self) -> Option { self.proof_size() } } diff --git a/primitives/externalities/src/lib.rs b/primitives/externalities/src/lib.rs index 1e77a3832c226..14145e8798498 100644 --- a/primitives/externalities/src/lib.rs +++ b/primitives/externalities/src/lib.rs @@ -288,7 +288,7 @@ pub trait Externalities: ExtensionStore { /// /// Returns estimated proof size for the state queries so far. /// Proof is reset on commit and wipe. - fn proof_size(&self) -> Option<(u32, u32)> { + fn proof_size(&self) -> Option { None } } diff --git a/primitives/state-machine/src/backend.rs b/primitives/state-machine/src/backend.rs index 990fdec589c8c..92b4c83314e72 100644 --- a/primitives/state-machine/src/backend.rs +++ b/primitives/state-machine/src/backend.rs @@ -247,7 +247,7 @@ pub trait Backend: sp_std::fmt::Debug { fn set_whitelist(&self, _: Vec) {} /// Estimate proof size - fn proof_size(&self) -> Option<(u32, u32)> { + fn proof_size(&self) -> Option { unimplemented!() } } diff --git a/primitives/state-machine/src/ext.rs b/primitives/state-machine/src/ext.rs index f7ff76c047351..8bcf1f28a0778 100644 --- a/primitives/state-machine/src/ext.rs +++ b/primitives/state-machine/src/ext.rs @@ -775,7 +775,7 @@ where self.backend.set_whitelist(new) } - fn proof_size(&self) -> Option<(u32, u32)> { + fn proof_size(&self) -> Option { self.backend.proof_size() } } diff --git a/utils/frame/benchmarking-cli/src/command.rs b/utils/frame/benchmarking-cli/src/command.rs index 3dfa9bfea794d..83f1d9e5b7e88 100644 --- a/utils/frame/benchmarking-cli/src/command.rs +++ b/utils/frame/benchmarking-cli/src/command.rs @@ -133,7 +133,7 @@ impl BenchmarkCmd { let parameters = &result.components; parameters.iter().for_each(|param| print!("{:?},", param.1)); // Print extrinsic time and storage root time - print!("{:?},{:?},{:?},{:?},{:?},{:?},{:?},{:?}\n", + print!("{:?},{:?},{:?},{:?},{:?},{:?},{:?}\n", result.extrinsic_time, result.storage_root_time, result.reads, @@ -141,7 +141,6 @@ impl BenchmarkCmd { result.writes, result.repeat_writes, result.proof_size, - result.compact_proof_size, ); }); diff --git a/utils/frame/benchmarking-cli/src/writer.rs b/utils/frame/benchmarking-cli/src/writer.rs index a15e4b7e716bc..6fd6cc6eefdc6 100644 --- a/utils/frame/benchmarking-cli/src/writer.rs +++ b/utils/frame/benchmarking-cli/src/writer.rs @@ -422,7 +422,6 @@ mod test { writes: (base + slope * i).into(), repeat_writes: 0, proof_size: 0, - compact_proof_size: 0, } ) } From f867d7f6353aac19a2e96b11b47886090006d4ef Mon Sep 17 00:00:00 2001 From: Emeric Chevalier Date: Sun, 6 Jun 2021 10:16:56 +0200 Subject: [PATCH 24/25] Missing revert. --- primitives/state-machine/src/lib.rs | 2 +- utils/frame/benchmarking-cli/src/command.rs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/primitives/state-machine/src/lib.rs b/primitives/state-machine/src/lib.rs index fa1944c8954e6..75f6cd1f9b87a 100644 --- a/primitives/state-machine/src/lib.rs +++ b/primitives/state-machine/src/lib.rs @@ -1523,7 +1523,7 @@ mod tests { assert_eq!(local_result1.len(), 1); assert_eq!(local_result1.get(&b"key1"[..]), Some(&Some(b"val2".to_vec()))); } - + #[test] fn child_storage_uuid() { diff --git a/utils/frame/benchmarking-cli/src/command.rs b/utils/frame/benchmarking-cli/src/command.rs index 83f1d9e5b7e88..80d95d1c86dcf 100644 --- a/utils/frame/benchmarking-cli/src/command.rs +++ b/utils/frame/benchmarking-cli/src/command.rs @@ -126,8 +126,7 @@ impl BenchmarkCmd { // Print the table header batch.results[0].components.iter().for_each(|param| print!("{:?},", param.0)); - print!("extrinsic_time_ns,storage_root_time_ns,reads,repeat_reads, - \\writes,repeat_writes,proof_size_bytes,compact_proof_size_bytes\n"); + print!("extrinsic_time_ns,storage_root_time_ns,reads,repeat_reads,writes,repeat_writes,proof_size_bytes\n"); // Print the values batch.results.iter().for_each(|result| { let parameters = &result.components; From d639940e46045159de9a9e792a1b633f3b6a311c Mon Sep 17 00:00:00 2001 From: Emeric Chevalier Date: Mon, 7 Jun 2021 09:51:41 +0200 Subject: [PATCH 25/25] proof method to encode decode. --- client/db/src/bench.rs | 7 ++-- primitives/state-machine/src/lib.rs | 18 ++-------- primitives/trie/src/storage_proof.rs | 53 +++++++++++++++++++++------- primitives/trie/src/trie_codec.rs | 2 +- 4 files changed, 45 insertions(+), 35 deletions(-) diff --git a/client/db/src/bench.rs b/client/db/src/bench.rs index e490014ed96c4..c198fb400408e 100644 --- a/client/db/src/bench.rs +++ b/client/db/src/bench.rs @@ -23,7 +23,7 @@ use std::cell::{Cell, RefCell}; use std::collections::HashMap; use hash_db::{Prefix, Hasher}; -use sp_trie::{MemoryDB, prefixed_key, CompactProof}; +use sp_trie::{MemoryDB, prefixed_key}; use sp_core::{ storage::{ChildInfo, TrackedStorageKey}, hexdisplay::HexDisplay @@ -529,10 +529,7 @@ impl StateBackend> for BenchmarkingState { // empty trie proof_size } else { - if let Some(size) = CompactProof::encoded_compact_size::>( - proof, - proof_recorder_root, - ) { + if let Some(size) = proof.encoded_compact_size::>(proof_recorder_root) { size as u32 } else { panic!( diff --git a/primitives/state-machine/src/lib.rs b/primitives/state-machine/src/lib.rs index 75f6cd1f9b87a..0508bfb780929 100644 --- a/primitives/state-machine/src/lib.rs +++ b/primitives/state-machine/src/lib.rs @@ -1403,24 +1403,10 @@ mod tests { } fn test_compact(remote_proof: StorageProof, remote_root: &sp_core::H256) -> StorageProof { - type Layout = sp_trie::Layout; - let compact_remote_proof = sp_trie::encode_compact::( - remote_proof, + let compact_remote_proof = remote_proof.into_compact_proof::( remote_root.clone(), ).unwrap(); - let mut db = sp_trie::MemoryDB::::new(&[]); - sp_trie::decode_compact::( - &mut db, - compact_remote_proof.iter_compact_encoded_nodes(), - Some(remote_root), - ).unwrap(); - StorageProof::new(db.drain().into_iter().filter_map(|kv| - if (kv.1).1 > 0 { - Some((kv.1).0) - } else { - None - } - ).collect()) + compact_remote_proof.to_storage_proof::(Some(remote_root)).unwrap().0 } #[test] diff --git a/primitives/trie/src/storage_proof.rs b/primitives/trie/src/storage_proof.rs index 1cea1b963ff73..03668920509b8 100644 --- a/primitives/trie/src/storage_proof.rs +++ b/primitives/trie/src/storage_proof.rs @@ -85,6 +85,26 @@ impl StorageProof { Self { trie_nodes } } + + /// Encode as a compact proof with default + /// trie layout. + pub fn into_compact_proof( + self, + root: H::Out, + ) -> Result>> { + crate::encode_compact::>(self, root) + } + + /// Returns the estimated encoded size of the compact proof. + /// + /// Runing this operation is a slow operation (build the whole compact proof) and should only be + /// in non sensitive path. + /// Return `None` on error. + pub fn encoded_compact_size(self, root: H::Out) -> Option { + let compact_proof = self.into_compact_proof::(root); + compact_proof.ok().map(|p| p.encoded_size()) + } + } impl CompactProof { @@ -93,20 +113,27 @@ impl CompactProof { self.encoded_nodes.iter().map(Vec::as_slice) } - /// Returns the estimated encoded size of the compact proof. + /// Decode to a full storage_proof. /// - /// Runing this operation is a slow operation (build the whole compact proof) and should only be - /// in non sensitive path. - /// Return `None` on error. - pub fn encoded_compact_size( - proof: StorageProof, - root: H::Out, - ) -> Option { - let compact_proof = crate::encode_compact::>( - proof, - root, - ); - compact_proof.ok().map(|p| p.encoded_size()) + /// Method use a temporary `HashDB`, and `sp_trie::decode_compact` + /// is often better. + pub fn to_storage_proof( + &self, + expected_root: Option<&H::Out>, + ) -> Result<(StorageProof, H::Out), crate::CompactProofError>> { + let mut db = crate::MemoryDB::::new(&[]); + let root = crate::decode_compact::, _, _>( + &mut db, + self.iter_compact_encoded_nodes(), + expected_root, + )?; + Ok((StorageProof::new(db.drain().into_iter().filter_map(|kv| + if (kv.1).1 > 0 { + Some((kv.1).0) + } else { + None + } + ).collect()), root)) } } diff --git a/primitives/trie/src/trie_codec.rs b/primitives/trie/src/trie_codec.rs index 1c7d224cccbc2..efe3223580f3f 100644 --- a/primitives/trie/src/trie_codec.rs +++ b/primitives/trie/src/trie_codec.rs @@ -201,7 +201,7 @@ pub fn decode_compact<'a, L, DB, I>( /// Then parse all child trie root and compress main trie content first /// then all child trie contents. /// Child trie are ordered by the order of their roots in the top trie. -pub fn encode_compact<'a, L>( +pub fn encode_compact( proof: StorageProof, root: TrieHash, ) -> Result>