Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Alliance: member can't retire if was set once up for kicking #11936

Closed
muharem opened this issue Jul 29, 2022 · 3 comments · Fixed by #11970
Closed

Alliance: member can't retire if was set once up for kicking #11936

muharem opened this issue Jul 29, 2022 · 3 comments · Fixed by #11970
Assignees
Labels
I3-bug The node fails to follow expected behavior. Z1-easy Can be fixed primarily by duplicating and adapting code by an intermediate coder

Comments

@muharem
Copy link
Contributor

muharem commented Jul 29, 2022

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.

@muharem muharem added I3-bug The node fails to follow expected behavior. Z1-easy Can be fixed primarily by duplicating and adapting code by an intermediate coder labels Jul 29, 2022
@joepetrowski
Copy link
Contributor

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 #11928).

@muharem muharem self-assigned this Jul 29, 2022
@muharem
Copy link
Contributor Author

muharem commented Aug 2, 2022

@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.

@joepetrowski
Copy link
Contributor

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 :).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
I3-bug The node fails to follow expected behavior. Z1-easy Can be fixed primarily by duplicating and adapting code by an intermediate coder
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants