-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Storage Map with Counters #8605
Comments
What I like about this idea, implemented as a generic |
I like the idea to introduce a Hooks to extend possibilities on insert and on remove, then we can introduce a type alias to be able to create some storage with counters with: pub type StorageMapCounted<P, H, V, Q, E> = StorageMap<Prefix, Hasher, Key, Value, QueryKind=OptionQuery, OnEmpty=GetDefault, Hooks=Counter>;
#[pallet::storage]
type MyStorage<T: Config> = StorageMapCounted<_, Twox128Concat, u32, 32>; Note that pallet macro should support this, or with minor change. Though in the metadata it will be seen as a storage map. |
Why not
? I'm not aware of any length limit for keys, and this would let us use common-prefix methods to manage the counter with the storage. [edit] Oh, it looks like that's what you already implemented in the diff. |
IMO any key with the prefix Better to store the counter in another place with the name |
I don't think there's any restriction on the length, I was just trying to stick to the size of the rest of the keys in the map. Both this, and
Work fine. I am working on a draft here: https://github.com/paritytech/substrate/compare/kiz-bounded-staking?expand=1 It is not ready for a draft PR ready yet, but please provide feedback if you have any. So far I've tried to not change any of the base trait ( |
Seems very sensible to me. I agree with Gui that the hooks seems more flexible and that we can always typedef a |
Current working branch is pretty generic over this and just provides post-hooks, one implementation of which is a |
@kianenigma Is the I say this because you'll be able to implement |
I'd be fine with switching the generic for an associated type if it fits better. Admittedly, this work in that branch is pretty much WIP. I really like the whole idea: don't just implement a counter, implement generic hooks, one function of which can be a counter. But the details can vary. |
I am trying to see how this can be implemented, one difficulty is that this Hook would need to be triggered in lots of places and the storage traits in Also we currently have 3 implementation of maps: map, double-map and n-map. Ideally map and double-map should directly use n-map implementation so that we don't need to duplicate this implementation 3 times. (sidenote: to factorize the code I first thought about doing:
but we can't do this if we want to keep the interface of storage map (like being able to call So I think we can reorganize the code in frame_support::storage::types as such:
once we remove decl_storage, we can delete all the code in |
Thanks for the detailed explanations @thiolliere Before reading further, I want to add my current 2 cents that I think however we implement this, for now adding manual counters is worthwhile since it will be easy to migrate them later to a more macro-enhanced approach. and I agree that the hook prototype was quite incomplete, and while expanding the idea I was also quite worried that if we miss triggering the hook somewhere, it would be quite a hard bug to track down. Ideally, the hooks must be somewhere inside |
Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions. |
The more I think of it the less I'm convinced it is doable, for instance there is stuff implemented for the StorageMap trait like https://github.com/open-web3-stack/open-runtime-module-library/blob/443ee91bc2ca5f1fc155c0378eef6e89b67e2e97/utilities/src/iterator.rs#L81 |
This is already done. |
For works such as staking, we want to be able to easily track the count of items of a map without the need to go through the expensive
::iter().count()
.Historically, the pattern to do this was to manually do this. I think now is a good time to start elaborating on how we can achieve a nicer way to do this. The ideal syntax for me would be to do:
Essentially, this would use the name of the counter function given to generate a new unique storage key:
And store a new u32 under this key. Then we need to somehow override
insert
,mutate
,remove
and all other mutating calls, or have hooks for them, that would update the counter accordingly. I started prototyping this, and came up with something along these lines:And then we'd need to sprinkle the usage of
Hooks
into the impl ofStorageMap
and call it in appropriate places.But I haven't got to work with either of
decl
or the pallet macros, so not sure if it is correct. And even so, this is aligned with the opinion that a counter-map should be just an extension (like what I namedHooks
) to the normalStorageMap
. Another idea is to have a makegenerator::StorageMap
trait a super-trait, have a sub-trait calledCountedStorageMap
or something like this. In this case, the counter map would be a new storage type, so you probably will see something like this:The text was updated successfully, but these errors were encountered: