DigestItem trait (v2) #650
Conversation
substrate/runtime-support/src/lib.rs
Outdated
|
|
||
| #[macro_export] | ||
| macro_rules! impl_outer_log { | ||
| ($(#[$attr:meta])* pub enum $name:ident for $trait:ident { $( $module:ident($( $log_type_name:ident => $log_type:ty => $as_log:ident ),*) ),* }) => { |
There was a problem hiding this comment.
looks like there's always parens after the $module:ident (though potentially empty). prefer no parens to empty parens.
There was a problem hiding this comment.
Removed this at all, because of manual DigestItem impl
demo/runtime/src/lib.rs
Outdated
| @@ -176,6 +176,12 @@ impl_outer_event! { | |||
| } | |||
| } | |||
|
|
|||
| impl_outer_log! { | |||
| pub enum Log for Concrete { | |||
| consensus(AuthoritiesChange => consensus::AuthoritiesChange<SessionKey> => as_authorities_change) | |||
There was a problem hiding this comment.
consider just introducing trait AsAuthoritiesChange { fn as_authorities_change(self) -> Result<Vec<SessionKey>, Self> } and then manually making an impl of it here. might save quite a lot of somewhat opaque syntax.
There was a problem hiding this comment.
Substrate has an access to digest items only through DigestItem trait, which is shared among all possible runtimes (declared in primitives). Extracting as_authorities_change (and other similar conversions later) into separate conversion trait would require DiggestItem to inherit from all those traits + will make it impossible to use default implementation (which returns None right now) => all runtimes have to explicitly declare support even for those 'system' digest items, which they are not willing to support (by simply using StubDigestItem and returning None).
I'm not sure it is better - this is an additional overhead for runtimes development. From the other side, it will force runtime developers to explicitly throw away items the do not want to support. Left as is for now, but could extract if you insist.
There was a problem hiding this comment.
Probably also could remove AuthoritiesChangeDigest trait in favor of Vec<AuthorityId>.
| /// Should be used as a stub for items that are not supported by runtimes. | ||
| #[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] | ||
| #[derive(Encode, Decode, PartialEq, Eq, Clone)] | ||
| pub struct StubDigestItem; |
gavofyork
added
A7-looksgoodcantmerge
and removed
A0-please_review
|
will need resolving |
…rs-generic-over-hasher-and-rlpcodec * origin/master: Fixed sync stalling when import queue is full (#691) New extrinsic dispatch model (#678) remove parachain's Cargo.lock (#682) Implement json metadata for outer events (#672) Improvements to the Kademlia system (#688) Use BufReader and BufWriter (#684) Switch to using parity/rust:substrate which has rust nightly-2018-08-31 (#686) Update to latest libp2p (#673) Implement storage json metadata (#670) impl MaybeEmpty for H256 and u64 (aka AccountId in prod/tests) (#665) Speedup compilation (#671) Remove requirement of function indices for decl_module! (#666) DigestItem trait (v2) (#650)
Updates the requirements on [criterion](https://github.com/bheisler/criterion.rs) to permit the latest version. - [Release notes](https://github.com/bheisler/criterion.rs/releases) - [Changelog](https://github.com/bheisler/criterion.rs/blob/master/CHANGELOG.md) - [Commits](bheisler/criterion.rs@0.3.0...0.4.0) --- updated-dependencies: - dependency-name: criterion dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Replaces #636 (messed up my local branches => new PR).
Required for #269 and #628 (comment)
This PR doesn't change anything - just adds some traits + example implementation + usage.
Traits are:
DigestItem+AuthoritiesChangeDigest. More (likeChangesTrieRootDigest) are to follow in next PRs.The main difference from #636 is that I've tried to use
Event-s infrastructure as suggested (specifically - macro for constructing 'total'enum). Still the main difference is that we want to parseLogs from theClient=>AuthoritiesChangeDigest+ differences in macro.