-
Notifications
You must be signed in to change notification settings - Fork 370
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
feat: r/gov/dao
v2
#2581
base: master
Are you sure you want to change the base?
feat: r/gov/dao
v2
#2581
Changes from all commits
4dd7c58
aad859b
81396bf
dd5d64b
c342741
3d16ba6
8f2e3ac
ae0639a
3a7ba00
00056a5
209ee06
54c555d
84aeb30
322bc8c
794a353
8c45039
268021d
43e5c66
e1cd617
67e4535
8fbbd5e
fd13d02
ebfd3d2
b3856c7
4d3d94e
1f1d5d7
7fa7530
ef299bd
3d0c056
4396c48
24e0b08
4827d0b
26f21d8
71e8644
93d2b70
efe35cc
63cf23f
4520593
af65764
f567394
4ec9f3b
30ac5ff
c05289f
d8e5b7d
d320493
d33b0df
79be838
8c52893
74948fc
c9b4cc4
93ca6a8
ee77102
dbf0966
2f337a6
31d265f
908b949
6b3b81c
480a287
06fe4fe
dba0379
873ba9b
6ceb72a
8198c7f
8a51452
b0fa6b7
676b202
852291c
ae650d2
aa4f9ef
33343d7
c3c33a2
30b80f7
7addbd5
2f2632b
671872a
3f196b2
c4b3970
38e2eae
82745d9
15404f5
2f19233
9cbc56a
3858a3c
ee989ff
af75547
d98d780
7171651
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
package combinederr | ||
|
||
import "strings" | ||
|
||
// CombinedError is a combined execution error | ||
type CombinedError struct { | ||
errors []error | ||
} | ||
|
||
// Error returns the combined execution error | ||
func (e *CombinedError) Error() string { | ||
if len(e.errors) == 0 { | ||
return "" | ||
} | ||
|
||
var sb strings.Builder | ||
|
||
for _, err := range e.errors { | ||
sb.WriteString(err.Error() + "; ") | ||
} | ||
|
||
// Remove the last semicolon and space | ||
result := sb.String() | ||
|
||
return result[:len(result)-2] | ||
} | ||
|
||
// Add adds a new error to the execution error | ||
func (e *CombinedError) Add(err error) { | ||
if err == nil { | ||
return | ||
} | ||
|
||
e.errors = append(e.errors, err) | ||
} | ||
|
||
// Size returns a | ||
func (e *CombinedError) Size() int { | ||
return len(e.errors) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
module gno.land/p/combinederr |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
package dao | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add a package-level comment to explain the role of this p/ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added: |
||
|
||
const ( | ||
ProposalAddedEvent = "ProposalAdded" // emitted when a new proposal has been added | ||
ProposalAcceptedEvent = "ProposalAccepted" // emitted when a proposal has been accepted | ||
ProposalNotAcceptedEvent = "ProposalNotAccepted" // emitted when a proposal has not been accepted | ||
ProposalExecutedEvent = "ProposalExecuted" // emitted when a proposal has been executed | ||
|
||
ProposalEventIDKey = "proposal-id" | ||
ProposalEventAuthorKey = "proposal-author" | ||
ProposalEventExecutionKey = "exec-status" | ||
) | ||
|
||
// ProposalRequest is a single govdao proposal request | ||
// that contains the necessary information to | ||
// log and generate a valid proposal | ||
type ProposalRequest struct { | ||
Description string // the description associated with the proposal | ||
Executor Executor // the proposal executor | ||
} | ||
|
||
// DAO defines the DAO abstraction | ||
type DAO interface { | ||
// PropStore is the DAO proposal storage | ||
PropStore | ||
|
||
// Propose adds a new proposal to the executor-based GOVDAO. | ||
// Returns the generated proposal ID | ||
Propose(request ProposalRequest) (uint64, error) | ||
|
||
// ExecuteProposal executes the proposal with the given ID | ||
ExecuteProposal(id uint64) error | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
// Package dao houses common DAO building blocks (framework), which can be used or adopted by any | ||
// specific DAO implementation. By design, the DAO should house the proposals it receives, but not the actual | ||
// DAO members or proposal votes. These abstractions should be implemented by a separate entity, to keep the DAO | ||
// agnostic of implementation details such as these (member / vote management). | ||
package dao |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
package dao | ||
|
||
import ( | ||
"std" | ||
|
||
"gno.land/p/demo/ufmt" | ||
) | ||
|
||
// EmitProposalAdded emits an event signaling that | ||
// a given proposal was added | ||
func EmitProposalAdded(id uint64, proposer std.Address) { | ||
std.Emit( | ||
ProposalAddedEvent, | ||
ProposalEventIDKey, ufmt.Sprintf("%d", id), | ||
ProposalEventAuthorKey, proposer.String(), | ||
) | ||
} | ||
|
||
// EmitProposalAccepted emits an event signaling that | ||
// a given proposal was accepted | ||
func EmitProposalAccepted(id uint64) { | ||
std.Emit( | ||
ProposalAcceptedEvent, | ||
ProposalEventIDKey, ufmt.Sprintf("%d", id), | ||
) | ||
} | ||
|
||
// EmitProposalNotAccepted emits an event signaling that | ||
// a given proposal was not accepted | ||
func EmitProposalNotAccepted(id uint64) { | ||
std.Emit( | ||
ProposalNotAcceptedEvent, | ||
ProposalEventIDKey, ufmt.Sprintf("%d", id), | ||
) | ||
} | ||
|
||
// EmitProposalExecuted emits an event signaling that | ||
// a given proposal was executed, with the given status | ||
func EmitProposalExecuted(id uint64, status ProposalStatus) { | ||
std.Emit( | ||
ProposalExecutedEvent, | ||
ProposalEventIDKey, ufmt.Sprintf("%d", id), | ||
ProposalEventExecutionKey, status.String(), | ||
) | ||
} | ||
|
||
// EmitVoteAdded emits an event signaling that | ||
// a vote was cast for a given proposal | ||
func EmitVoteAdded(id uint64, voter std.Address, option VoteOption) { | ||
std.Emit( | ||
VoteAddedEvent, | ||
VoteAddedIDKey, ufmt.Sprintf("%d", id), | ||
VoteAddedAuthorKey, voter.String(), | ||
VoteAddedOptionKey, option.String(), | ||
) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
package dao | ||
|
||
// Executor represents a minimal closure-oriented proposal design. | ||
// It is intended to be used by a govdao governance proposal (v1, v2, etc) | ||
type Executor interface { | ||
// Execute executes the given proposal, and returns any error encountered | ||
// during the execution | ||
Execute() error | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
module gno.land/p/demo/dao | ||
|
||
require gno.land/p/demo/ufmt v0.0.0-latest |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
package dao | ||
|
||
import "std" | ||
|
||
type ProposalStatus string | ||
|
||
// ACTIVE -> ACCEPTED -> EXECUTION(SUCCEEDED/FAILED) | ||
// ACTIVE -> NOT ACCEPTED | ||
Comment on lines
+7
to
+8
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: these comment lines should be part of a comment block for We should use a linter for missing comments for exports. |
||
var ( | ||
Active ProposalStatus = "active" // proposal is still active | ||
Accepted ProposalStatus = "accepted" // proposal gathered quorum | ||
NotAccepted ProposalStatus = "not accepted" // proposal failed to gather quorum | ||
ExecutionSuccessful ProposalStatus = "execution successful" // proposal is executed successfully | ||
ExecutionFailed ProposalStatus = "execution failed" // proposal is failed during execution | ||
) | ||
|
||
func (s ProposalStatus) String() string { | ||
return string(s) | ||
} | ||
|
||
// PropStore defines the proposal storage abstraction | ||
type PropStore interface { | ||
// Proposals returns the given paginated proposals | ||
Proposals(offset, count uint64) []Proposal | ||
|
||
// ProposalByID returns the proposal associated with | ||
// the given ID, if any | ||
ProposalByID(id uint64) (Proposal, error) | ||
|
||
// Size returns the number of proposals in | ||
// the proposal store | ||
Size() int | ||
} | ||
|
||
// Proposal is the single proposal abstraction | ||
type Proposal interface { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note to self: Wouldn't it be better and more secure to use a struct here? What if we define a custom struct that implements the interface for an attack? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's fine to leave the implementation details of the proposal up to the package / realm creator: // Proposal is the single proposal abstraction
type Proposal interface {
// Author returns the author of the proposal
Author() std.Address
// Description returns the description of the proposal
Description() string
// Status returns the status of the proposal
Status() ProposalStatus
// Executor returns the proposal executor
Executor() Executor
// Stats returns the voting stats of the proposal
Stats() Stats
// IsExpired returns a flag indicating if the proposal expired
IsExpired() bool
// Render renders the proposal in a readable format
Render() string
} The |
||
// Author returns the author of the proposal | ||
Author() std.Address | ||
|
||
// Description returns the description of the proposal | ||
Description() string | ||
|
||
// Status returns the status of the proposal | ||
Status() ProposalStatus | ||
|
||
// Executor returns the proposal executor | ||
Executor() Executor | ||
|
||
// Stats returns the voting stats of the proposal | ||
Stats() Stats | ||
|
||
// IsExpired returns a flag indicating if the proposal expired | ||
IsExpired() bool | ||
|
||
// Render renders the proposal in a readable format | ||
Render() string | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
package dao | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a comment to explain that the vote will be removed from this p/demo/dao package. A DAO shouldn't have to comply with or define how the voting mechanism works internally; it should be viewed as an entity that makes decisions. Therefore, we should focus on the proposal rather than the votes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I want to merge this PR soon, so I'm in favor of just adding the comment. However, please note that this is the biggest weakness of this implementation, and it's the only issue I see that will require us to create a v2. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I checked, it turns out the extent of "votes" being present in the dao package is just in the context of a single type
I've added a comment to make it clear: |
||
|
||
// NOTE: | ||
// This voting pods will be removed in a future version of the | ||
// p/demo/dao package. A DAO shouldn't have to comply with or define how the voting mechanism works internally; | ||
// it should be viewed as an entity that makes decisions | ||
// | ||
// The extent of "votes being enforced" in this implementation is just in the context | ||
// of types a DAO can use (import), and in the context of "Stats", where | ||
// there is a notion of "Yay", "Nay" and "Abstain" votes. | ||
const ( | ||
VoteAddedEvent = "VoteAdded" // emitted when a vote was cast for a proposal | ||
|
||
VoteAddedIDKey = "proposal-id" | ||
VoteAddedAuthorKey = "author" | ||
VoteAddedOptionKey = "option" | ||
) | ||
|
||
type VoteOption string | ||
|
||
const ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should it be an option I see that later in the code you keep track of missing votes (it is always useful), but it is not clear to me if, when and why missing vote == abstain, in the context of a generic govDAO where provided specific governance rules could be different. |
||
YesVote VoteOption = "YES" | ||
NoVote VoteOption = "NO" | ||
AbstainVote VoteOption = "ABSTAIN" | ||
) | ||
|
||
func (v VoteOption) String() string { | ||
return string(v) | ||
} | ||
|
||
// Stats encompasses the proposal voting stats | ||
type Stats struct { | ||
YayVotes uint64 | ||
NayVotes uint64 | ||
AbstainVotes uint64 | ||
moul marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
TotalVotingPower uint64 | ||
} | ||
|
||
// YayPercent returns the percentage (0-100) of the yay votes | ||
// in relation to the total voting power | ||
func (v Stats) YayPercent() uint64 { | ||
return v.YayVotes * 100 / v.TotalVotingPower | ||
} | ||
|
||
// NayPercent returns the percentage (0-100) of the nay votes | ||
// in relation to the total voting power | ||
func (v Stats) NayPercent() uint64 { | ||
return v.NayVotes * 100 / v.TotalVotingPower | ||
} | ||
|
||
// AbstainPercent returns the percentage (0-100) of the abstain votes | ||
// in relation to the total voting power | ||
func (v Stats) AbstainPercent() uint64 { | ||
return v.AbstainVotes * 100 / v.TotalVotingPower | ||
} | ||
|
||
// MissingVotes returns the summed voting power that has not | ||
// participated in proposal voting yet | ||
func (v Stats) MissingVotes() uint64 { | ||
return v.TotalVotingPower - (v.YayVotes + v.NayVotes + v.AbstainVotes) | ||
} | ||
|
||
// MissingVotesPercent returns the percentage (0-100) of the missing votes | ||
// in relation to the total voting power | ||
func (v Stats) MissingVotesPercent() uint64 { | ||
return v.MissingVotes() * 100 / v.TotalVotingPower | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,9 @@ | ||
module gno.land/r/gov/dao | ||
module gno.land/p/demo/membstore | ||
|
||
require ( | ||
gno.land/p/demo/avl v0.0.0-latest | ||
gno.land/p/demo/testutils v0.0.0-latest | ||
gno.land/p/demo/uassert v0.0.0-latest | ||
gno.land/p/demo/ufmt v0.0.0-latest | ||
gno.land/p/demo/urequire v0.0.0-latest | ||
gno.land/p/gov/proposal v0.0.0-latest | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
package membstore | ||
|
||
import ( | ||
"std" | ||
) | ||
|
||
// MemberStore defines the member storage abstraction | ||
type MemberStore interface { | ||
// Members returns all members in the store | ||
Members(offset, count uint64) []Member | ||
|
||
// Size returns the current size of the store | ||
Size() int | ||
|
||
// IsMember returns a flag indicating if the given address | ||
// belongs to a member | ||
IsMember(address std.Address) bool | ||
|
||
// TotalPower returns the total voting power of the member store | ||
TotalPower() uint64 | ||
|
||
// Member returns the requested member | ||
Member(address std.Address) (Member, error) | ||
|
||
// AddMember adds a member to the store | ||
AddMember(member Member) error | ||
|
||
// UpdateMember updates the member in the store. | ||
// If updating a member's voting power to 0, | ||
// the member will be removed | ||
UpdateMember(address std.Address, member Member) error | ||
} | ||
|
||
// Member holds the relevant member information | ||
type Member struct { | ||
Address std.Address // bech32 gno address of the member (unique) | ||
VotingPower uint64 // the voting power of the member | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: in comments, sentences end with a period. It can be enforced by a linter. I only reported this line, but there are many other occurences of missing period.