Improve docs of remove_all and remove_prefix

[bkchr]

These docs didn't mention how the removal works internally. This is important for the user to know
that calling such a method multiple times in the same block leads always to the same result.

[cheme]

Looks like a good idea to stress this fact. Maybe for frame doc the comment could be shorter (like just apply the largest limit).

[cheme]

Then Note is more about the underlying reason (no persistence of the applied limit in the overlay).

Wonder if renaming limit to limit_backend or backend_fix_limit could be a first hint.

[cheme]

Actually I like your comment on sp-io better.

[bkchr]

Actually I like your comment on sp-io better.

Which part exactly? Could you add some suggestions to the pr?

[cheme]

The note that describe the way overlay is handled:

    /// The deletion would always start from `prefix` resulting in the same keys being deleted
	/// every time this function is called with the exact same arguments per block. This happens
	/// because the keys in the overlay are not taken into account when deleting keys in the
	/// backend.

Is more clear to me, but that may be because I know the problematic.
I was thinking a bit about it during lunch and another way to put it would be: limit is only applied over state as it is at the start of the block, ignoring previous calls or something like this.

[bkchr]

Talked to @shawntabrizi and he said we can continue with the current docs.

[cheme]

Could add For multiple calls the largest limit is applied.

[kianenigma]

Is there anyway to change this behavior? It is rather counter intuitive now.

[cheme]

Is there anyway to change this behavior? It is rather counter intuitive now.

you would need to manage prefix removal progress from the state machine change overlay. But thing like remove at key "a" then remove at key "ab" with limits will look like a nightmare.

Generally solution, as I see it, is to use prefix removal the same way we add or remove value: need to be in the overlay, then calculation of storage root from runtime (and in trie crate) need to have this prefix removal in the delta but not produce the change set immediately (as querying the change set would need to be either at pruning or in an asynch way in client/db). I remember having implemented pieces of it and the bad thing are: need new trie api to detach/remove a prefix, break the substrate design (trie related query of prefix nodes leaks in client code).

[bkchr]

We could change the implementation on the host side to take into account the keys in the overlay and don't let them count towards the limit when we iterate over them in the backend. However, that would mean the more often you call the method the more costly it gets. For parachains that could also be bad for example, because it could happen that you are required to include data in the proof which was set as None already in the overlay.

I thought about doing the changes on the client side before changing the documentation. I'm not really sold on anything, but if we change the host side it may results in assuming too much about the runtime side and we break something else.

Calling remove_all with the same limit multiple times is already something that is against the reason for having the limit. Then you could either directly call with limit = None or limit = 200 instead of just 100.

This https://substrate.stackexchange.com/questions/1646/multiple-remove-all-calls-in-same-block post brought this up here.