Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inner hash of value with new nodes #142

Merged
merged 209 commits into from
Oct 19, 2021

Conversation

cheme
Copy link
Contributor

@cheme cheme commented Sep 3, 2021

This PR aims at replacing #134.
It achieves the same goals but always use a new db node when inner hashing is use.
This makes code simpler and can be interesting with rc dbs.

Note that compact proof and compact proof with value iterator did change a bit:

  • compact proof: hashed value are following the node (node is switch to a standard node with 0 length value), which is prefixed with a header dead value.
  • compact proof with value iter: hashed value when ignored are replace similarily, just no value node are added afterward.

At this point review can be started as all remaining TODOs are minor.
Codec in substrate_like.rs is the same as the one which will be ported in substrate except for the default threshold size (set to 1 here as most of test only use small values).

TODOs

  • crate versioning

  • write a test for value removal with prefix (check the value node prefix is correct) + a test that insert twice in triedbmut

  • 63e1651 is related to a possible migration optimisation in state machine, does not really have to be in this PR.

  • consider running rust format (little care was used here)

@cheme
Copy link
Contributor Author

cheme commented Oct 5, 2021

b7238ef does remove the hybrid specific compact proof encoding.
@thiolliere , ESCAPE_HEADER was not remove, it is still needed in trie_codec.rs (when header is present we got attached value after the hash). I cannot remove it because the codec is not writing value length when encoding a hashed value so we need to know if value is in proof or not.

trie-db/src/triedbmut.rs Outdated Show resolved Hide resolved
trie-db/src/triedbmut.rs Outdated Show resolved Hide resolved
trie-db/src/triedbmut.rs Outdated Show resolved Hide resolved
trie-db/src/triedbmut.rs Outdated Show resolved Hide resolved
trie-db/src/node.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@gui1117 gui1117 left a comment

Choose a reason for hiding this comment

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

looks good to me, I didn't understood deeply all algorithm, especially the proof in trie-db/src/proof/

But all changes made sense to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants