Add MerkleMountainRangeRoot digest item #5479
Conversation
* add new merkle mountain range digest item for header * introduce merkle mountain range lib * add mmr root to digest in System::finalize(), waiting mmr crate supporting T:Hash to make compile pass * upgrade to use mmr lib from ckb * move mmr logic to seprate darwinia pallet * remove useless lines * add tests for header mmr digest * update test comment
|
It looks like @hackfisher hasn't signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement at the following URL: https://cla.parity.io Once you've signed, please reply to this thread with Many thanks, Parity Technologies CLA Bot |
|
[clabot:check]. |
|
It looks like @hackfisher signed our Contributor License Agreement. 👍 Many thanks, Parity Technologies CLA Bot |
|
Can't you use Also Can we be sure that MMR root hash is even related to this and be the same? |
| @@ -173,6 +178,10 @@ pub enum DigestItemRef<'a, Hash: 'a> { | |||
| /// Digest item that contains signal from changes tries manager to the | |||
| /// native code. | |||
| ChangesTrieSignal(&'a ChangesTrieSignal), | |||
|
|
|||
| /// Reference to `DigestItem::MerkleMountainRangeRoot`. | |||
| MerkleMountainRangeRoot(&'a Hash), | |||
There was a problem hiding this comment.
Why isn't this done via Other case? You can create your own extension trait in your own code.
To handle the conversion like so:
trait AsMerkleMountainRangeRoot<Hash> {
fn as_merkle_mountain_range_root(&self) -> Option<&Hash>;
}
impl<'a, Hash: 'a> AsMerkleMountainRangeRoot<Hash> for DigestItemRef<'a, Hash> {
fn as_merkle_mountain_range_root(&self) -> Option<&Hash> {
match *self {
DigestItemRef::Other(ref data) => Hash::from(&data).ok(),
_ => None,
}
}
}There was a problem hiding this comment.
It makes sense.
I was thinking that if we use Other, the data may conflict with digest item having the same struct which could be introduced in future modules.
I think can resolve this by using a specific prefix in Other data.
Thanks, I'm closing this PR.
There was a problem hiding this comment.
AFAICT Other is meant to be exactly this - a non-conflicting way to add your own thing to Digest. Unless someone corrects me ;)
It is indeed to only one element though, but internally you can then encode multiple different variants.
Perhaps we are lacking some documentation here to make it clear what's the intention of that variant.
This is to add a new type of digest item in digest,
This is used for future light-client enhancement, and here is a sample pallet how this digest log will be deposited:
https://github.com/darwinia-network/darwinia-common/blob/v0.5.0/frame/header-mmr/src/lib.rs#L85
Related darwinia-network#1 and darwinia-network/darwinia#358
DigestItemType in darwinia's fork was using 18 to avoid future conflicts with the potential reserved types, this PR is going to use 8 as the type index, and will sync darwinia's forking to the same 8 after this PR get merged.
Thanks.