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

add method to expose storage_changes.trie_diffs #1226

Closed

Conversation

ermalkaleci
Copy link
Contributor

We need this for chopsticks to get all the storage changes

@ermalkaleci ermalkaleci requested a review from tomaka as a code owner October 17, 2023 08:55
@tomaka
Copy link
Contributor

tomaka commented Oct 17, 2023

There are already functions that expose the list of changes: https://docs.rs/smoldot/latest/smoldot/executor/runtime_host/struct.StorageChanges.html#impl-StorageChanges

Is there something missing?

I'd rather not expose the TrieDiff type directly in the API. (#780)

@ermalkaleci
Copy link
Contributor Author

There are already functions that expose the list of changes: https://docs.rs/smoldot/latest/smoldot/executor/runtime_host/struct.StorageChanges.html#impl-StorageChanges

Is there something missing?

I'd rather not expose the TrieDiff type directly in the API. (#780)

I need to read child storage diffs and I was using trie_changes_iter_ordered but somehow it gives me nothing when child::kill is performed. Having access to trie_diffs, I can iter and read all the storage diffs

@tomaka
Copy link
Contributor

tomaka commented Oct 17, 2023

trie_changes_iter_ordered wraps around trie_diffs, so if it doesn't yield anything that means that trie_diffs doesn't contain anything.

When you do child::kill, all that happens is that all the keys of the child trie are removed. If the child trie was empty, then this does nothing. Is it possible that the child trie was simply empty in the parent block?

@ermalkaleci
Copy link
Contributor Author

we're testing crowdloan refund which calls child:kill and it was refunding the same users because trie_changes_iter_ordered wasn't returning any storage diff for the killed child https://github.com/paritytech/polkadot/blob/52209dcfe546ff39cc031b92d64e787e7e8264d4/runtime/common/src/crowdloan/mod.rs#L708
not sure if there's some bug with trie_changes_iter_ordered but if I reading trie_diffs directly makes things much easier and seems to fix our issue

@tomaka
Copy link
Contributor

tomaka commented Oct 17, 2023

StorageChanges is indeed missing some functions to get the storage changes for child tries, however as mentioned I'd prefer to not expose TrieDiff in the API. I'd prefer to add more functions that allow introspecting the content of the TrieDiffs rather than return a reference to the TrieDiffs.

Also note that StorageChanges has a pretty good fmt::Debug implementation, so you can just print it out in order to be sure whether it contains an item or not.

@ermalkaleci
Copy link
Contributor Author

I'm happy with any solution. I can add a method that iter trie_diffs returning (child, key, value) or if you've better idea just let me know until then I'm using my fork so no rush.

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.

2 participants