paritytech / trie

Base-16 Modified Patricia Merkle Tree (aka Trie)
Apache License 2.0
258 stars 67 forks source link

Fix `TrieDBRawIterator::prefix_then_seek` #190

Closed koute closed 1 year ago

koute commented 1 year ago

After calling the TrieDBRawIterator::prefix_then_seek the iterator is not supposed to return any keys outside of the requested prefix; this unfortunately is not the case, and this PR fixes the issue.

Implementation notes

Potential future work

cheme commented 1 year ago

From what I read this bug is not in a host function (only clear_prefix v3 can go in the code path but it is "register_only" cc @bkchr is it possible to still use the function? (in this case I am not sure if it can be an issue since runtime will only call it with a cursor if it is not empty, and having cursor at last position do not make sense as we know it is the last position)).

bkchr commented 1 year ago

From what I read this bug is not in a host function (only clear_prefix v3 can go in the code path but it is "register_only" cc @bkchr is it possible to still use the function? (in this case I am not sure if it can be an issue since runtime will only call it with a cursor if it is not empty, and having cursor at last position do not make sense as we know it is the last position)).

Ahh you also came to the same conclusion as me! Good :D

koute commented 1 year ago

I also checked Substrate and the good thing is that we don't have the clear_prefix host functions with maybe_cursor enabled yet. So, we should not be able to have build a block with this bug? @cheme and @koute please check again that I'm right!

From a cursory look this sounds about right.

koute commented 1 year ago

Okay, since I want to put up a substrate PR with this fix I'll merge this now. If you'd still like for me to make any further changes feel free to leave them and I'll do them in a followup.