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

state_machine no_std witness externalities #6934

Merged
24 commits merged into from
Sep 11, 2020
Merged

state_machine no_std witness externalities #6934

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
ea03567
checkpoint before removing CT from change trie
cheme Aug 21, 2020
c0e25ae
before trie backend without tx
cheme Aug 21, 2020
2cb6378
undo
cheme Aug 21, 2020
b0e04a4
Started no transaction, but would need using a different root
cheme Aug 21, 2020
a9aa27c
Remove NoTransaction.
cheme Aug 21, 2020
2747339
Merge branch 'master' into witness_backend
cheme Aug 21, 2020
cac2280
partially address review.
cheme Aug 21, 2020
11305d1
Remove ChangeTrieOverlay.
cheme Aug 24, 2020
4f05206
modified function
cheme Aug 25, 2020
d4ce6dd
Merge branch 'master' into witness_backend
cheme Aug 25, 2020
147ef68
Remove witness_ext
cheme Aug 25, 2020
d067b0c
need noops changes root
cheme Aug 25, 2020
0e6b642
update from cumulus branch
cheme Aug 25, 2020
4c454cc
line break
cheme Aug 25, 2020
d32e3fd
remove warning
cheme Aug 26, 2020
753a8d7
line break
cheme Aug 26, 2020
2d63cf7
From review: renamings and stats active in no std (except time).
cheme Sep 8, 2020
1fadc14
Merge branch 'master' into witness_backend
cheme Sep 8, 2020
9b3146d
include cache, exclude change trie cache with individual temporary ba…
cheme Sep 8, 2020
bdbaefc
little test
cheme Sep 8, 2020
d1b10c8
fuse imports and filter_map prepare_extrinsics_input_inner fold.
cheme Sep 10, 2020
14e40b6
put back ExtInner into Ext, awkward double proto for new function.
cheme Sep 10, 2020
e459698
Apply suggestions from code review
bkchr Sep 11, 2020
bef5997
Update primitives/state-machine/Cargo.toml
bkchr Sep 11, 2020
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
3 changes: 2 additions & 1 deletion Cargo.lock
View file
Open in desktop

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

17 changes: 13 additions & 4 deletions primitives/externalities/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,16 @@ documentation = "https://docs.rs/sp-externalities"
targets = ["x86_64-unknown-linux-gnu"]

[dependencies]
sp-storage = { version = "2.0.0-rc6", path = "../storage" }
sp-std = { version = "2.0.0-rc6", path = "../std" }
environmental = { version = "1.1.1" }
codec = { package = "parity-scale-codec", version = "1.3.1" }
sp-storage = { version = "2.0.0-rc6", path = "../storage", default-features = false }
sp-std = { version = "2.0.0-rc6", path = "../std", default-features = false }
environmental = { version = "1.1.1", default-features = false }
codec = { package = "parity-scale-codec", version = "1.3.1", default-features = false }

[features]
default = ["std"]
std = [
"codec/std",
"environmental/std",
"sp-std/std",
"sp-storage/std",
]
7 changes: 5 additions & 2 deletions primitives/externalities/src/extensions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
//!
//! It is required that each extension implements the [`Extension`] trait.

use std::{collections::HashMap, collections::hash_map::Entry, any::{Any, TypeId}, ops::DerefMut};
use sp_std::{
collections::btree_map::{BTreeMap, Entry}, any::{Any, TypeId}, ops::DerefMut, boxed::Box,
};
use crate::Error;

/// Marker trait for types that should be registered as [`Externalities`](crate::Externalities) extension.
Expand Down Expand Up @@ -104,9 +106,10 @@ pub trait ExtensionStore {
/// Stores extensions that should be made available through the externalities.
#[derive(Default)]
pub struct Extensions {
extensions: HashMap<TypeId, Box<dyn Extension>>,
extensions: BTreeMap<TypeId, Box<dyn Extension>>,
}

#[cfg(feature = "std")]
impl std::fmt::Debug for Extensions {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "Extensions: ({})", self.extensions.len())
Expand Down
4 changes: 3 additions & 1 deletion primitives/externalities/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.

#![cfg_attr(not(feature = "std"), no_std)]

//! Substrate externalities abstraction
//!
//! The externalities mainly provide access to storage and to registered extensions. Extensions
Expand All @@ -23,7 +25,7 @@
//!
//! This crate exposes the main [`Externalities`] trait.

use std::any::{Any, TypeId};
use sp_std::{any::{Any, TypeId}, vec::Vec, boxed::Box};

use sp_storage::{ChildInfo, TrackedStorageKey};

Expand Down
43 changes: 29 additions & 14 deletions primitives/state-machine/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,25 +13,40 @@ documentation = "https://docs.rs/sp-state-machine"
targets = ["x86_64-unknown-linux-gnu"]

[dependencies]
log = "0.4.8"
parking_lot = "0.10.0"
hash-db = "0.15.2"
trie-db = "0.22.0"
trie-root = "0.16.0"
sp-trie = { version = "2.0.0-rc6", path = "../trie" }
sp-core = { version = "2.0.0-rc6", path = "../core" }
sp-panic-handler = { version = "2.0.0-rc6", path = "../panic-handler" }
codec = { package = "parity-scale-codec", version = "1.3.1" }
num-traits = "0.2.8"
rand = "0.7.2"
sp-externalities = { version = "0.8.0-rc6", path = "../externalities" }
itertools = "0.9"
log = { version = "0.4.8", optional = true }
parking_lot = { version = "0.10.0", optional = true }
hash-db = { version = "0.15.2", default-features = false }
trie-db = { version = "0.22.0", default-features = false }
trie-root = { version = "0.16.0", default-features = false }
sp-trie = { version = "2.0.0-rc6", path = "../trie", default-features = false }
sp-core = { version = "2.0.0-rc6", path = "../core", default-features = false }
sp-panic-handler = { version = "2.0.0-rc6", path = "../panic-handler", optional = true }
codec = { package = "parity-scale-codec", version = "1.3.1", default-features = false }
num-traits = { version = "0.2.8", default-features = false }
rand = { version = "0.7.2", optional = true }
sp-externalities = { version = "0.8.0-rc6", path = "../externalities", default-features = false }
smallvec = "1.4.1"
sp-std = { version = "2.0.0-rc6", default-features = false, path = "../std" }

[dev-dependencies]
hex-literal = "0.3.1"
sp-runtime = { version = "2.0.0-rc6", path = "../runtime" }
pretty_assertions = "0.6.1"

[features]
default = []
default = ["std"]
std = [
"codec/std",
"hash-db/std",
"num-traits/std",
"sp-core/std",
"sp-externalities/std",
"sp-std/std",
"sp-trie/std",
"trie-db/std",
"trie-root/std",
"log",
"parking_lot",
"rand",
"sp-panic-handler",
]
9 changes: 7 additions & 2 deletions primitives/state-machine/src/backend.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,22 @@
use hash_db::Hasher;
use codec::{Decode, Encode};
use sp_core::{
traits::RuntimeCode,
storage::{ChildInfo, well_known_keys, TrackedStorageKey}
};
use crate::{
trie_backend::TrieBackend,
trie_backend_essence::TrieBackendStorage,
UsageInfo, StorageKey, StorageValue, StorageCollection, ChildStorageCollection,
};
use sp_std::vec::Vec;
#[cfg(feature = "std")]
use sp_core::traits::RuntimeCode;

/// A state backend is used to read state data and can have changes committed
/// to it.
///
/// The clone operation (if implemented) should be cheap.
pub trait Backend<H: Hasher>: std::fmt::Debug {
pub trait Backend<H: Hasher>: sp_std::fmt::Debug {
/// An error type when fetching data is not possible.
type Error: super::Error;

Expand Down Expand Up @@ -375,11 +377,13 @@ pub(crate) fn insert_into_memory_db<H, I>(mdb: &mut sp_trie::MemoryDB<H>, input:
}

/// Wrapper to create a [`RuntimeCode`] from a type that implements [`Backend`].
#[cfg(feature = "std")]
pub struct BackendRuntimeCode<'a, B, H> {
backend: &'a B,
_marker: std::marker::PhantomData<H>,
}

#[cfg(feature = "std")]
impl<'a, B: Backend<H>, H: Hasher> sp_core::traits::FetchRuntimeCode for
BackendRuntimeCode<'a, B, H>
{
Expand All @@ -388,6 +392,7 @@ impl<'a, B: Backend<H>, H: Hasher> sp_core::traits::FetchRuntimeCode for
}
}

#[cfg(feature = "std")]
impl<'a, B: Backend<H>, H: Hasher> BackendRuntimeCode<'a, B, H> where H::Out: Encode {
/// Create a new instance.
pub fn new(backend: &'a B) -> Self {
Expand Down
21 changes: 14 additions & 7 deletions primitives/state-machine/src/changes_trie/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,15 @@ fn prepare_extrinsics_input_inner<'a, B, H, Number>(
Number: BlockNumber,
{
changes
.filter(|( _, v)| v.extrinsics().next().is_some())
Copy link
Member

Choose a reason for hiding this comment

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

Why did you change this code?

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 did modify the 'extrinsics' prototype to stop returning an iterator (before it was fusing iterator with itertools unique but I am do not like the implementation a lot (internal hashmap), so I simply return a btreeset instead, changing the code here calling 'extrinsics' multiple times, so avoid fusing the iterator multiple time).
But I think I will remove the ChangeTrieOverlay trait that only try to avoid some foot print and is leaking a bit too much everywhere for my taste).

Copy link
Member

Choose a reason for hiding this comment

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

If you change this to a filter_map, you almost need no changes below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well it avoid a tab, will change (easier to review, even if I like it better without the filter map, I am really waiting for rustc "Tracking issue for methods converting bool to Option<T> #64260" to get stable).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I remember, 'extrinsics' function is not a getter, it have an associated cost, that is the main reason (even if optimized by the compilator, this way of putting it makes it clear that we don't want to call the function multiple times, but I can write it with combinator too).

Copy link
Member

Choose a reason for hiding this comment

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

I see the problem. However the old implementation was also an Iterator that did not do anything when not called.

.try_fold(BTreeMap::new(), |mut map: BTreeMap<&[u8], (ExtrinsicIndex<Number>, Vec<u32>)>, (k, v)| {
.filter_map(|(k, v)| {
Copy link
Member

Choose a reason for hiding this comment

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

Looks much better!

let extrinsics = v.extrinsics();
if !extrinsics.is_empty() {
Some((k, extrinsics))
} else {
None
}
})
.try_fold(BTreeMap::new(), |mut map: BTreeMap<&[u8], (ExtrinsicIndex<Number>, Vec<u32>)>, (k, extrinsics)| {
match map.entry(k) {
Entry::Vacant(entry) => {
// ignore temporary values (values that have null value at the end of operation
Expand All @@ -161,7 +168,7 @@ fn prepare_extrinsics_input_inner<'a, B, H, Number>(
}
};

let extrinsics = v.extrinsics().cloned().collect();
let extrinsics = extrinsics.into_iter().collect();
entry.insert((ExtrinsicIndex {
block: block.clone(),
key: k.to_vec(),
Expand All @@ -170,11 +177,11 @@ fn prepare_extrinsics_input_inner<'a, B, H, Number>(
Entry::Occupied(mut entry) => {
// we do not need to check for temporary values here, because entry is Occupied
// AND we are checking it before insertion
let extrinsics = &mut entry.get_mut().1;
extrinsics.extend(
v.extrinsics().cloned()
let entry_extrinsics = &mut entry.get_mut().1;
entry_extrinsics.extend(
extrinsics.into_iter()
);
extrinsics.sort();
entry_extrinsics.sort();
},
}

Expand Down
3 changes: 0 additions & 3 deletions primitives/state-machine/src/changes_trie/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,6 @@ use crate::{
},
};

/// Changes that are made outside of extrinsics are marked with this index;
pub const NO_EXTRINSIC_INDEX: u32 = 0xffffffff;

/// Requirements for block number that can be used with changes tries.
pub trait BlockNumber:
Send + Sync + 'static +
Expand Down
4 changes: 2 additions & 2 deletions primitives/state-machine/src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

/// State Machine Errors

use std::fmt;
use sp_std::fmt;

/// State Machine Error bound.
///
Expand All @@ -34,7 +34,7 @@ impl<T: 'static + fmt::Debug + fmt::Display + Send> Error for T {}
#[derive(Debug, Eq, PartialEq)]
pub enum ExecutionError {
/// Backend error.
Backend(String),
Backend(crate::DefaultError),
/// The entry `:code` doesn't exist in storage so there's no way we can execute anything.
CodeEntryDoesNotExist,
/// Backend is incompatible with execution proof generation process.
Expand Down
Loading