Improve sp_io::storage::clear_prefix API to return the number of key deleted

[gui1117]

Currently the api is:

substrate/primitives/io/src/lib.rs

Lines 143 to 175 in 9a00c43

	/// Clear the storage of each key-value pair where the key starts with the given `prefix`.
	///
	/// # Limit
	///
	/// Deletes all keys from the overlay and up to `limit` keys from the backend if
	/// it is set to `Some`. No limit is applied when `limit` is set to `None`.
	///
	/// The limit can be used to partially delete a prefix storage in case it is too large
	/// to delete in one go (block).
	///
	/// It returns a boolean false iff some keys are remaining in
	/// the prefix after the functions returns. Also returns a `u32` with
	/// the number of keys removed from the process.
	///
	/// # Note
	///
	/// Please note that keys that are residing in the overlay for that prefix when
	/// issuing this call are all deleted without counting towards the `limit`. Only keys
	/// written during the current block are part of the overlay. Deleting with a `limit`
	/// mostly makes sense with an empty overlay for that prefix.
	///
	/// Calling this function multiple times per block for the same `prefix` does
	/// not make much sense because it is not cumulative when called inside the same block.
	/// Use this function to distribute the deletion of a single child trie across multiple
	/// blocks.
	#[version(2)]
	fn clear_prefix(&mut self, prefix: &[u8], limit: Option<u32>) -> KillStorageResult {
	let (all_removed, num_removed) = Externalities::clear_prefix(*self, prefix, limit);
	match all_removed {
	true => KillStorageResult::AllRemoved(num_removed),
	false => KillStorageResult::SomeRemaining(num_removed),
	}
	}

So the return type KillStorageResult only return the number of keys deleted from the backend.
But sometime we also want to know the number of key deleted from the overlay. Like in #10231 to implement CountedStorageMap::remove_all with some limit on the number of key to remove in backend.

To implement this we need to create the version 3 of clear_prefix. returning all the key removed to overlay.
Then we can implement the ability to have a limit in CountedStorageMap::remove_all similarly to StorageMap::remove_all.

[dharjeezy]

Picking this up @thiolliere

[dharjeezy]

Hello @thiolliere is version 3 of clear_prefix just going to return all the key removed from overlay or return both that of backend and overlay?

[gui1117]

ideally I think we should return:
The number of key removed from overlay, the number of key removed from backend, and if all the keys at the prefix were removed or if there is some remaining ones. So the only information to add from current type is the number of key removed from overlay

[dharjeezy]

I just made a PR for this issue. Can you help check to see what i did @thiolliere

[gui1117]

ah sorry I have mistaken, even if we return the number of key deleted from the overlay, the method CountedStorageMap::remove_all still cannot support any limit. Because some keys can be deleted from both overlay and backend and thuswe cannot update the counter correctly :-/

For example if both backend and overlay contains some key "abc", doing a clear prefix with a limit of one will delete the key from and overlay and the key from backend, and thus we would decrease the counter by 2. but actually it should be decreased only by one because the key deleted from overlay and the key deleted from backend correspond to the same key in the storage counted map.

Considering this I'm not sure implementing the new API is very useful. At least I don't have any usecase for the new API suggested in the issue.
And solving the issue about allowing CountedStorageMap::remove_all to have some limit similarly to StorageMap::remove_all would require more work.

I don't know if it worth pursuing the implementation, sorry @dharjeezy

[dharjeezy]

uhmmm... So are you saying we should scrap(delete) the implementation that i am doing currently and you'd close this issue? @thiolliere

[dharjeezy]

Should i go ahead and close my PR while you close this issue? @thiolliere

[gui1117]

Yes I think I can close the issue, if people feel like the propose API have usecases then we can reopen.

Sorry for having mistaken.