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

Another take at adjusting the host functions for child tries #673

Merged
merged 2 commits into from
Jun 6, 2023

Conversation

tomaka
Copy link
Contributor

@tomaka tomaka commented Jun 6, 2023

cc #166

This is an alternative approach to #167

In the main branch and in #167, we treat the child trie entries in the main trie as "virtual". In other words, if the runtime asks for a child trie entry in the main trie, the value is built on the fly.
While this is IMO the best approach theoretically speaking, this unfortunately also leads to a complicated API.

This PR changes that approach and instead stores a proper value in the main trie whenever a child trie root is re-calculated.
This leads to a more simple API because it seems that Substrate also uses this approach in its implementation.
Many weirdnesses in the child tries design are easily explained if you consider these weirdnesses as corner cases that aren't properly handled and that the implementation writes to the main trie when a child trie root is re-calculated.

What this PR does is revert many of the changes of #167, and instead make ext_default_child_storage_root_version_1 do two things instead of one: calculate the root then write it to the main trie.

This changes the behavior in a subtle way: before this PR, if you write to a non-existing child trie but don't call ext_default_child_storage_root_version_1, the child trie is never actually created. After this PR, if you do the same thing, the child trie is created (and stored in the database) but has no corresponding entry in the main trie.
I can't tell whether the same happens in Substrate, because again child tries are a clusterfuck.
While this could create a leak, in practice it is a bug in the runtime, and it is no different than if the runtime wrote an entry in the main trie and never ever read it back.

@tomaka tomaka added this pull request to the merge queue Jun 6, 2023
Merged via the queue into smol-dot:main with commit c7ccda8 Jun 6, 2023
@tomaka tomaka deleted the child-trie-hosts-v2 branch June 6, 2023 08:23
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.

1 participant