Allow capping the amount of work performed when deleting a child trie

[athei]

This adds a new version of the storage_kill runtime interface that allows limiting the amount of work that is performed when deleting a child trie:

substrate/primitives/io/src/lib.rs

Lines 307 to 311 in d6ff136

	#[version(2)]
	fn storage_kill(&mut self, storage_key: &[u8], limit: Option<u32>) -> bool {
	let child_info = ChildInfo::new_default(storage_key);
	self.kill_child_storage(&child_info, limit)
	}

We need this feature for removing contracts that are too big to be removed in one block but need to be deleted because of missing rent payments.

polkadot companion: paritytech/polkadot#2077

[cheme]

Code LGTM, some minor interrogation about naming and also wondering if versioned 'kill_storage' could be a good idea.

About this new function, I am still stuck with my previous idea #5280 (have a specific change sent upward and only iterate all keys when pruning state (and not at all for archive node), but it did not go through review at the time.
I think I am a bit too opinionated on this point (even if #5280 (comment) seems to agree??), so I am letting the final word to the other reviewers.

[athei]

About this new function, I am still stuck with my previous idea #5280 (have a specific change sent upward and only iterate all keys when pruning state (and not at all for archive node), but it did not go through review at the time.
I think I am a bit too opinionated on this point (even if #5280 (comment) seems to agree??), so I am letting the final word to the other reviewers.

I am happy to replace the lazy removal with a constant time direct removal when it comes around. But it is up in the air for so long that I don't have any hope that it will happen soon. I cannot wait for this to happen because we need a solution now.

[cheme]

I am happy to replace the lazy removal with a constant time direct removal when it comes around. But it is up in the air for so long that I don't have any hope that it will happen soon. I cannot wait for this to happen because we need a solution now.

Yes I am aware of that, and I don't really think this will come around either (I did it once and unless there is an explicit decision to do it again, I won't push effort in a unwanted direction).
Morning idea, it is also possible to use the existing host function 'default_child_storage_next_key' and 'default_child_storage_clear' to implement it, but it seems like a terrible idea to me (way to much sp_io calls), I like this PR version better.

[athei]

Morning idea, it is also possible to use the existing host function 'default_child_storage_next_key' and 'default_child_storage_clear' to implement it, but it seems like a terrible idea to me (way to much sp_io calls), I like this PR version better.

Yeah this would result in a terrible overhead where a lot of keys are copied over the client <-> runtime interface. Hence this PR.

[cheme]

LGTM.
As mentioned before, I do not really agree on the design, but the other option did not reach consensus previously.

[gui1117]

code looks good to me,

I don't think people will request a storage_kill with limit which includes overlay, but maybe there is some usecase. Anyway people can stil do (in a less efficient way) with next_key if they want to limit also in the overlay.

[athei]

The runtime generally controls what is in the overlay by deciding when to call this function. In my case this will always be with an empty overlay for that cild trie.

[athei]

bot merge

[ghost]

Checks failed; merge aborted.

[athei]

bot merge force

[ghost]

Trying merge.

[bkchr]

Sorry for being late. The only change that I would have proposed would be to use your KillOutcome enum here directly.

[athei]

Decided against it because everyone else there seems to use primitive types. I think it is not too bad to leave it this way now?

[bkchr]

It is fine :)