Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add MsgLeaveGroup to group module #9657

Closed
4 tasks
aaronc opened this issue Jul 8, 2021 · 5 comments · Fixed by #10887
Closed
4 tasks

Add MsgLeaveGroup to group module #9657

aaronc opened this issue Jul 8, 2021 · 5 comments · Fixed by #10887

Comments

@aaronc
Copy link
Member

aaronc commented Jul 8, 2021

Summary

An ability for a member to unilaterally leave a group.

Problem Definition

Normally in a social group, if one member decides to leave a group, they simply can. Nobody else needs to agree.

Also, if I am using a group to manage a single wallet with multiple keys. I would like the ability to "log out" of one device without needing the other devices to approve.

Proposal

Add MsgLeaveGroup:

message MsgLeaveGroup {
  string member_address = 1;
  uint64 group_id = 2;
}

One difficulty with implementing this is that it could make ThresholdDecisionPolicy fail if there are not enough members to meet the threshold if a member leaves. To address this, we either need to:

  • add validation logic preventing members from leaving if that would break the threshold, or
  • switch to a percentage decision policy or hybrid threshold/decision policy to accomodate members leaving

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@aaronc aaronc changed the title Add MsgLeaveGroup Add MsgLeaveGroup to group module Jul 8, 2021
@aaronc
Copy link
Member Author

aaronc commented Jul 8, 2021

@blushi @frumioj thoughts?

@frumioj
Copy link
Contributor

frumioj commented Jul 8, 2021

Is "logout" the same as "leave group"? Why?
Can the group admin leave the group?
Are the members of the group actual users (ie. social group case) or simply "wielders of a key" (on behalf of users)?
I see these groups as being like the members of a household, with a shared kitty for household expenses. No problem to leave as such, but what happens to their money (if invested) and their vote as to what is a valid expense?

@blushi
Copy link
Contributor

blushi commented Jul 9, 2021

Can the group admin leave the group?

A group admin is not necessarily part of the group members. But a group admin can update the group with a new admin.

@blushi
Copy link
Contributor

blushi commented Jul 9, 2021

One difficulty with implementing this is that it could make ThresholdDecisionPolicy fail if there are not enough members to meet the threshold if a member leaves. To address this, we either need to:

add validation logic preventing members from leaving if that would break the threshold, or
switch to a percentage decision policy or hybrid threshold/decision policy to accomodate members leaving

The first option would be the most straightforward to implement for now but then I agree this could come with a bad UX because that would require the group account admin to update the decision policy beforehand.
But that's also the option that would work best for any kind of decision policy because even if we switch to a percentage decision policy, we cannot assume that it will always be the case.

@blushi
Copy link
Contributor

blushi commented Aug 30, 2021

should we reopen this? cc/ @frumioj @aaronc

@frumioj frumioj reopened this Aug 30, 2021
@github-actions github-actions bot removed the stale label Aug 31, 2021
@aleem1314 aleem1314 self-assigned this Jan 4, 2022
@amaury1093 amaury1093 moved this from Ready to In Progress in Cosmos SDK: Gov & Group WG Feb 7, 2022
@amaury1093 amaury1093 moved this from In Progress to In Review in Cosmos SDK: Gov & Group WG Feb 15, 2022
@mergify mergify bot closed this as completed in #10887 Mar 4, 2022
Repository owner moved this from In Review to Done in Cosmos SDK: Gov & Group WG Mar 4, 2022
mergify bot pushed a commit that referenced this issue Mar 4, 2022
## Description

Closes: #9657 



---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

4 participants