Alliance: member can't retire if was set once up for kicking
Description of bug
One of the requirements to retire and return deposit is to be not considered by alliance to be kicked - link. To keep track of all members proposed for kicking there is a storage map with AccountIDs (UpForKicking) - link.
When Alliance proposes proposal with the call to kick_member, the pallet stores the account id in UpForKicking - link. And when member is kicked, the AccountId is removed from UpForKicking storage - link.
But what If the proposal for kicking member is not voted to be executed and closed? The AccountId will not be removed from the UpForKicking storage. And a member wont be able ever to retire.
Just adding a note here from DM so it doesn't get lost:
We could do like Staking and have a "retirement period" (like unbonding period). You'd lose your voting rights during the period, and at the end of it you could actually retire. That would do away with the UpForKicking storage item entirely. This also aligns more with the principle that the system shouldn't need to know about the contents of a proposal until it is time to execute them. I.e. all operations about proposals should be based on their hash, and pallet storage shouldn't change just because something is proposed.
pub trait Config<I: 'static = ()>: frame_system::Config {
type RetirementPeriod: Get<BlockNumber>;
}
enum MemberRole {
Retiring(BlockNumber), // <- add this
}
fn give_retirement_notice(origin) { /* change role to Retiring(current_block_number) */ }
fn retire(origin) { /* check that BlockNumber + RetirementPeriod have passed and retire */ }
We can make the retirement period longer than the Root referendum track such that members cannot just retire to escape being forcibly removed (related to https://github.com/paritytech/substrate/issues/11928).
@joepetrowski There will be one possibility left for a member to retire even though there is approved proposal to kick the member. It can happened if no one executes the proposal for kicking the member within retirement period.
Since the retirement period is supposed to be set greater than the motion duration and alliance will be willing to execute such proposals within motion duration, I think we can tolerate this edge case.
Yeah, I agree. The Alliance members are meant to be active participants. If someone gets away from a slash because everyone in the Alliance was too lazy to close it, well then the Alliance isn't working very well :).