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

Corner cases about (child) storage #241

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Conversation

lamafab
Copy link
Contributor

@lamafab lamafab commented Oct 10, 2022

Regarding w3f/polkadot-spec#540

What happens if the runtime calls the ext_storage_get_version_1 or ext_storage_read_version_1 host function with a key K, where K is in fact a child trie? As far as I understand, this would return the trie root hash of the child trie, but this isn't really clear in the spec.

Current behavior: child keys cannot be retrieved via ext_storage_* functions, meaning the functions return None.

What happens if the runtime calls the ext_storage_set_version_1 host function with a key K, where K is in fact a child trie? I assume that it destroys the child trie, but this isn't really clear.

Current behavior: the keys can co-exist, meaning that you can use K for both as a regular storage key and as a child key (does not destroy the child storage).

EDIT: Actually, it does look like when you call ext_default_child_storage_set_version_1 first, it does not allow you to use the same key for rtm_ext_storage_set_version_1. Not sure if this is intentional. See https://github.com/w3f/polkadot-tests/pull/241/files#diff-dab36c8cbf0299b6a8ed1f420f3ab4d641873ca8573c24a1089a70a4d5803004R337

EDIT-2: Emeric confirmed that ext_storage_set_* silently drops :child_storage:default:* keys.

What happens if the runtime calls any of the ext_default_child_storage_* host functions with a child_storage_key that is in fact not a child trie? I assume that this is the equivalent of a runtime panic?

Current behavior: No panic, it just returns None.

(btw I'm still having issues with setting up a container for all of this, there' some weird behavior going on. Will follow up later)

@cheme
Copy link
Contributor

cheme commented Oct 10, 2022

Current behavior: child keys cannot be retrieved via ext_storage_* functions, meaning the functions return None.

this is a bit odd, I would expect that we could get them by querying with the prefix appended to the storage_key. Maybe it is filtered at a different level than where I did look, but on principle I see no issue in allowing it (quite the opposite in fact).

What happens if the runtime calls the ext_storage_set_version_1 host function with a key K, where K is in fact a child trie? I assume that it destroys the child trie, but this isn't really clear.

as mention in dm the operation is skipped (a warning is emitted but the set_storage operation is ignored in: https://github.com/paritytech/substrate/blob/df81976c40e2e0573d59c51e12eb3c15c3ff3057/primitives/state-machine/src/ext.rs#L394 ).

What happens if the runtime calls any of the ext_default_child_storage_* host functions with a child_storage_key that is in fact not a child trie? I assume that this is the equivalent of a runtime panic?

the host function is using the part of the key after :child_prefix:default: so it is actually not doable (the prefix is added afterward).

@cheme
Copy link
Contributor

cheme commented Oct 10, 2022

While looking for a reason get should fail, I realize that another one that is block is using remove_prefix if prefix do contains :child_storage so key as ``, : , `:ch`, `:child_storage:default:anything`.

Copy link
Contributor

@FlorianFranzen FlorianFranzen left a comment

Choose a reason for hiding this comment

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

I am not sure I would start moving away from the current naming scheme and just extend the existing tests for now.

You will also have to implement these changes in the other adapter as well.

Lastly, please do not touch the submodules unless there is a good reason. It is probably also why the CI is broken,

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

Successfully merging this pull request may close these issues.

3 participants