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

Group module spec #5236

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions x/group/spec/decision_protocols.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Decision Protocols

This documents the decision protocols to be included by default in the SDK.

## `ThresholdDecisionPolicy`

```go
// ThresholdDecisionPolicy is a decision policy that supports decisions
// based on a simple threshold
type ThresholdDecisionPolicy struct {
// Specifies the number of votes that must be accumulated in order for a decision to be made by the group.
// A member gets as many votes as is indicated by their Weight field.
// A big integer is used here to avoid any potential vulnerabilities from overflow errors
// where large weight and threshold values are used.
DecisionThreshold sdk.Int `json:"decision_threshold"`
}
```

## `PercentageDecisionPolicy`

```go
type PercentageDecisionPolicy struct {
Percent sdk.Dec `json:"percent"`
}
```
79 changes: 79 additions & 0 deletions x/group/spec/group.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
# Group Specification

## Types

### `GroupID`

```go
// GroupID is the auto-generated ID of the group
type GroupID uint64
```


### `Member`

```go
// Member specifies the and power of a group member
type Member struct {
// The address of a group member. Can be another group or a contract
Address sdk.AccAddress `json:"address"`
// The integral weight of this member with respect to other members
Weight sdk.Int `json:"weight"`
Description string `json:"description"`
}
```

## Messages

### `MsgCreateGroup`

```go
type MsgCreateGroup struct {
// The admin of the group is allowed to change the group structure. A group account
// can own a group in order for the group to be able to manage its own members
Admin sdk.AccAddress `json:"admin"`
// The members of the group and their associated power
Members []Member `json:"members,omitempty"`
Description string `json:"description,omitempty"`
}
```

*Returns:* `GroupID` based on an auto-incrementing `uint64`.

### `MsgUpdateGroup

```go
// MsgUpdateGroupMembers updates the members of the group, adding, removing,
// and updating members as needed. To remove an existing member set its Power to 0.
type MsgUpdateGroupMembers struct {
Admin sdk.AccAddress `json:"admin"`
Group GroupID `json:"group"`
// NewAdmin sets a new admin for the group. If this is left empty, the
// current admin is not changed.
NewAdmin sdk.AccAddress `json:"new_admin"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please define behavior if NewAdmin is unset (I assume don't change Admin).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can either make it a no-op like you're proposing or this would seal the group and make it impossible to change? Is that desirable? I think that would probably be unexpected behavior so for now I will define it as a no-op. If there is a use case for that, this could be a separate "seal group" message.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree

// Description sets a new description if the string point is non-nil,
// otherwise the description isn't changed
Description *string `json:"description,omitempty"`
MemberUpdates []Member `json:"member_updates,omitempty"`
}
```

## Keeper

### Constructor: ` NewKeeper(groupStoreKey sdk.StoreKey, cdc *codec.Codec, accountKeeper auth.AccountKeeper, dispatcher msg_delegation.Keeper)`

The group keeper gets a reference to the `auth.AccountKeeper` in order to create
accounts for new groups, and a reference to the `msg_delegation.Keeper` in order
to authorize messages send back to the router.

### Query Methods

```go
type GroupKeeper interface {
IterateGroupMembers(group GroupID, fn func (member sdk.AccAddress, weight sdk.Int, description string) (stop bool))
IterateGroupsByMember(member sdk.Address, fn func (group GroupID) (stop bool))
IterateGroupsByAdmin(member sdk.Address, fn func (group Group) (stop bool))
GetGroupDescription(group GroupID) string
GetTotalWeight(group GroupID) sdk.Int
}
```
80 changes: 80 additions & 0 deletions x/group/spec/group_account.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
# Group Policy Specification

## Types

### `DecisionPolicy`

```go
// DecisionPolicy allows for flexibility in decision policy based both on
// weights (the tally of yes, no, abstain, and veto votes) and votingDuration -
// the amount of time that has been allowed for voting. A zero votingDuration
// means this proposal is being executed as basically a single multi-sig
// transaction with no voting window. This may be okay for some DecisionPolicy's.
// Other policies may stipulate that at least a few hours or days of voting
// must occur to pass a proposal.
type DecisionPolicy interface {
Allow(tally Tally, totalPower sdk.Int, votingDuration time.Duration)
}
```

### `Tally`

```go
type Tally struct {
YesCount sdk.Int
NoCount sdk.Int
AbstainCount sdk.Int
VetoCount sdk.Int
}
```


## Messages

### `MsgCreateGroupAccount
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name is a bit unwieldy. I don't have a great alternative, but should be refined. Group makes clear sense and is what it says. GroupAccount is not clear.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, GroupAccount is a bit unwieldy but I also was unable to come up with a better name. I am open to proposals.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will give one when I have one.... wish I was good at naming

```go
// MsgCreateGroupPolicy creates a new group policy with the provided
// DecisionPolicy. A group policy allows groups to set different decision
// policys for different types of action.
type MsgCreateGroupAccount struct {
Admin sdk.AccAddress `json:"admin"`
Group GroupID `json:"group"`
DecisionPolicy DecisionPolicy `json:"decision_policy"`
Description string `json:"Description,omitempty"`
}
```

*Returns:* a new auto-generated `sdk.AccAddress` for this group account.

### `MsgGroupExec`

```go
// MsgGroupExec executed the provided messages using the groups account if the
// provided signers pass the group's DecisionPolicy. This is essentially a
// basic multi-signature execution method.
type MsgGroupExec struct {
GroupAccount sdk.AccAddress `json:"group_account"`
Signers []sdk.AccAddress `json:"signers"`
Msgs []sdk.Msg `json:"msgs"`
}
```

### Update messages

```go
type MsgUpdateGroupAccount struct {
Admin sdk.AccAddress `json:"admin"`
GroupAccount sdk.AccAddress `json:"group_account"`
// NewAdmin sets a new admin for the group. If this is left empty, the
// current admin is not changed.
NewAdmin sdk.AccAddress `json:"new_admin"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question as above about omitted fields and zero-values - do we set to zero value, do we leave them unchanged? Both have issues, maybe consider pointers, even if a bit funny looking here, they render to the same json.

Copy link
Contributor

@Hyung-bharvest Hyung-bharvest Nov 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to suggest two functionality to MsgUpdateGroupAccount.

  1. There can be a GroupAccount without Admin
  2. Members of the GroupAccount can vote for MsgUpdateGroupAccount

This way, we can create a GroupAccount which ownership is fully decentralized but modifiable.

Another question.
Why MsgUpdateGroupAccount does not have a field Members to be updated?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

questions resolved. thank you @aaronc

// Description sets a new description if the string point is non-nil,
// otherwise the description isn't changed
Description *string `json:"description,omitempty"`
// DecisionPolicy changes the GroupAccount DecisionPolicy, if left to nil,
// the DecisionPolicy isn't changed
DecisionPolicy DecisionPolicy `json:"decision_policy"`
}

```

97 changes: 97 additions & 0 deletions x/group/spec/proposal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
# Group Proposal Specification

Groups can submit proposals in order to execute arbitrary transactions.
Authorization to execute messages is performed by the `msg_delegation.Keeper`'s
`DispatchActions` method. The actual protocol that a group uses to determine
whether a certain proposal passes are based on the group accounts's
`DecisionPolicy`.

## Types

### `ProposalID`

```go
type ProposalID uint64
```

An auto-incremented integer ID for each proposal.

### `Vote`

```go
type Vote int

const (
No Vote = iota
Yes = 1
Abstain = 2
Veto = 3
)
```

## Messages

### `MsgPropose`

```go
type MsgPropose struct {
Proposer sdk.AccAddress `json:"proposer"`
GroupAccount sdk.AccAddress `json:"group_account"`
Msgs []sdk.Msg `json:"msgs"`
Description string `json:"Description,omitempty"`
}
```

*Returns:* a new `ProposalID` based on an auto-incremented integer.

### `MsgVote`

Votes are counted as `MsgVote`s are submitted. The final tally of votes should
always be based on the state of the group at the end of the proposal. As a result,
whenever group membership is changed via `MsgUpdateGroupMembers`, existing vote
counts for open proposals must be updated when a member's power is changed.

```go
type MsgVote struct {
ProposalID ProposalID `json:"proposal_id"`
Voter sdk.AccAddress `json:"voter"`
Vote Vote `json:"vote"`
Comment string `json:"comment,omitempty"`
}
```

### `MsgExecProposal`

```go
// MsgExecProposal attempts to execute and close an open proposal based on
// the current Tally of votes and the amount of time that the proposal has
// been open for voting. If the proposal can pass based on those parameters,
// its Msgs will be passed to the router via the msg_delegation.Keeper and the
// proposal will be closed. The signer of this message will pay any gas costs.
// If the proposal cannot pass based on the current parameters, it will remain
// open until the MaxVotingPeriod time is reached and then automatically closed.
type MsgExecProposal struct {
ProposalID ProposalID `json:"proposal_id"`
Signer sdk.AccAddress `json:"signer"`
}
```
### `MsgWithdrawProposal`

```go
// MsgWithdrawProposal allows the original proposer to withdraw an open proposal.
type MsgWithdrawProposal struct {
ProposalID ProposalID `json:"proposal_id"`
Proposer sdk.AccAddress `json:"proposer"`
}
```

## Params

The group module contains the following parameters:

| Key | Type | Usage | Example |
|------------------------|-----------------|---------|-----|
| MaxVotingPeriod | string (time ns) | Proposals older than this time will be garbage-collected | "172800000000000" |
| MaxDescriptionCharacters | string (uint64) | | "256" |
| MaxCommentCharacters | string (uint64) | | "256" |