Explore replacing counters with CountedStorageMap

[shannonwells]

We have a few counters for separate storage of items in storage. CountedStorageMap won't apply to all but we could see how well it works various storage like CurrentSchemaIdentifierMaximum, CurrentMsaIdentifierMaximum, PublicKeyToMsaId + PublicKeyCountForMsaId, and PayloadSignatureBucketCount.

https://paritytech.github.io/substrate/master/frame_support/storage/types/struct.CountedStorageMap.html

[allanperlee]

Hi @shannonwells, it's me again! I could probably address this issue. Regardless of the upgrade, can a pull-request be submitted?

[wilwade]

@allanperlee If you want to take a swing at it, feel free! We'll be upgrading to 0.9.41 soonish I think.

#1332

[allanperlee]

Hi @wilwade, I am working full-time now, I want to still work this issue, but this will take me about two weeks, is that fine?

[wilwade]

@allanperlee (sorry for the lag) Yes and congratulations on the job! That is fine, but also make sure to take care. Starting new job can be a lot.

[allanperlee]

Thanks, @wilwade! Let me attempt starting this weekend. A medical emergency is getting in the way too. 🤦‍♂️

[wilwade]

Unblocked with the update to 0.9.42 complete!

[allanperlee]

Hi @wilwade! I resumed this issue and the CountedStorageMap only takes items that implement certain traitts such as EncodeLIke. On https://github.com/allanperlee/frequency/blob/main/pallets/msa/src/lib.rs#L172, the ValueQuery is "maximum MSA Id"...

[aramikm]

Hi @allanperlee! glad to see you are back. A number of things to consider before working on this

• This ticket was created when we didn't have tens of thousands of MSA ids on mainnnet and changing the storage for msa pallet would require a multi-block storage migration which would be a complicated process and I'm not sure if we would like to attempt something like that currently.
• The signature bucket related things might change based on some limitations we found during MEWE user migrations so I won't think it would make sense to spend time on it currently.
• It might makes sense to narrow the scope to only schema pallet for start and even though by changing storage we would need to create storage migration for that but at least we know there are not that many schemas registered and it might fit in a single block on Mainnet. (On Rococo this might not be the case but we would need to check)

I'm not sure if I answered your question but please let me know.

[allanperlee]

Hi @aramikm! I am currently working on the schema pallet, and I misunderstood the issue with the trait binding; some storages can be easily converted. Addressing your concern for storage migration, since we aren't changing the values in the converted storages, is a migration necessary?

[aramikm]

I think so, the reason is that we would need to replace the both of these storages with one CountedStorageMap. And based on the way CountedStorageMap is stored (storing count + map) we would need to migrate items from Schemas and into that counted one and finally kill these two storages at the end of migration.

	/// Storage type for current number of schemas
	/// Useful for retrieving latest schema id
	/// - Value: Last Schema Id
	#[pallet::storage]
	#[pallet::getter(fn get_current_schema_identifier_maximum)]
	pub(super) type CurrentSchemaIdentifierMaximum<T: Config> =
		StorageValue<_, SchemaId, ValueQuery>;

	/// Storage for message schema struct data
	/// - Key: Schema Id
	/// - Value: [`Schema`](Schema)
	#[pallet::storage]
	#[pallet::getter(fn get_schema)]
	pub(super) type Schemas<T: Config> = StorageMap<
		_,
		Twox64Concat,
		SchemaId,
		Schema<T::SchemaModelMaxBytesBoundedVecLimit>,
		OptionQuery,
	>;

[allanperlee]

@aramikm I have a draft finished, but I am struggling with compilation errors upon reaching the polkadot-cli:

I followed all steps for Ubuntu building including clang, curl, git, sudo updates, and everything is updated, What other steps can I take? Thanks!

[aramikm]

Hi @allanperlee , so we had a number of changes recently that might contribute to that.

• We upgraded underlying substrate and cumulus libraries to version release-polkadot-v1.1.0 which is using the https://github.com/paritytech/polkadot-sdk library.

• To have a better support for ProofOfValidity in our chain. We are doing some review and refactoring in Messages, Schemas and Stateful-storage pallets. One of these refactors is in schemas: PoV compatible changes #1743 which splits the schema into two different sections for storage. This change unfortunately will make this issue obsolete.

Our team is moving fast and unfortunately this current ticket do not make sense anymore and I apologize if we were not able to communicate that effectively before your efforts. We have a number of other tickets that might be good starting tickets and still valid such as

• Optimization: Hashset for delegation mutation #1155
• Prepaid (free) transactions should still increment the nonce for accounts that have an Account #1766
• Switch CLI parameter parsing from Clap derive macros to builder pattern #1521

please take a look and ask any related questions if needed.

[allanperlee]

Hello, @aramikm, it's all good, at least we experimented and realized we can close this issue. #1776 looks appealing, can I get that assigned? Then probably explore the other issues?

[aramikm]

@allanperlee sure.

[wilwade]

Closing this per discussion in https://github.com/LibertyDSNP/frequency/pull/1743/files#r1391162531