Generalize VariantCount into InstanceCount

[NingLin-P]

Is there an existing issue?

• I have searched the existing issues

Experiencing problems? Have you tried our Stack Exchange first?

• This is not a support question.

Motivation

#2657 replaces MaxHolds with VariantCount::VARIANT_COUNT it works fine with RuntimeHoldReason enum that has a static number of instances like:

enum RuntimeHoldReason {
    Reason1,
    Reason2,
}

but it is not general enough to work fine with enums that have data attached to the variant like:

enum RuntimeHoldReason {
    Reason1(Id),
    Reason2(Id),
}

which still has 2 variants but can construct much more than 2 instances with different data, and at the end it still requires something similar to MaxHolds

Request

To cover both use cases, generalize VariantCount into InstanceCount:

/// Trait to get the max number of instances that are allowed to be constructed in any enum.
pub trait InstanceCount {
	/// Get the max number of instances that are allowed to be constructed.
	const INSTANCE_COUNT: u32;
}

for enum that has a static number of instances the INSTANCE_COUNT value will be the same as VariantCount:: VARIANT_COUNT, while for enum that doesn't have a static number of instances INSTANCE_COUNT works similar to MaxHolds

Solution

No response

Are you willing to help with this request?

Yes!

[bkchr]

while for enum that doesn't have a static number of instances INSTANCE_COUNT works similar to MaxHolds

Not sure what you mean by this. This was always "static". The only change this linked pr is bringing is that the number of locks is calculated automatically for you.

enum RuntimeHoldReason {
    Reason1(Id),
    Reason2(Id),
}

This looks problematic to me. Why is this unbounded? How does this works? What is ID?

[ggwpez]

I asked Gav once on whether we should allow dynamically generated RuntimeHoldReason with attached data and he said no.
I think the intention behind these Reason enums is to have an unambiguous mapping from a RuntimeHoldReason to a location in the code where it was set. Parametrizing those with arbitrary values mostly circumvents that.
Not sure what to say in the case that ID is also an enum...

[NingLin-P]

Why is this unbounded? How does this works? What is ID?

It is bounded by other logic to something like 100. In our usage in subspace, ID is the operator id, a nominator can nominate multiple operators at the same time and result in multiple different RuntimeHoldReasons (e.g. RuntimeHoldReason::Reason1(1u32) and RuntimeHoldReason::Reason1(2u32)).

[bkchr]

Why do you need one exclusive hold per operator/nominator?

[nazar-pc]

Why do you need one exclusive hold per operator/nominator?

I guess it is convenient to do. Otherwise the list will have to be additionally maintained in another place with some reference counting of sorts.

[NingLin-P]

Why do you need one exclusive hold per operator/nominator?

Yeah, it is convenient, so we don't need to keep track of how many balances are on hold for a particular operator and use balance_on_hold instead.

[ggwpez]

Okay i guess just adding a new trait like InstanceCount and adding a blanked impl for VariantCount should not be too intrusive?

[xlc]

this is designed to only have very limited and static number of reasons. I will suggest a new pallet or something for this requested feature, instead of modify the existing code because it breaks some of the original assumptions

yeah it maybe possible to do it in such why that is still somewhat compatible, but it can’t scale, it can easily misused. we should aim for a proper solution instead