Skip to content
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
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
16 changes: 14 additions & 2 deletions examples/gno.land/r/gov/dao/v3/impl/govdao.gno
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package impl

import (
"chain"
"chain/banker"
"chain/runtime"
"errors"

Expand All @@ -13,6 +14,9 @@ import (

var ErrMemberNotFound = errors.New("member not found")

// ProposalFeeAmount is the fee required for non-members to create proposals (in ugnot)
const ProposalFeeAmount int64 = 1000000 // 1 GNOT
Copy link
Member

Choose a reason for hiding this comment

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

should we create a new helper function to create a proposal request containing the executor callback to update this number ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix: 856c91e


type GovDAO struct {
pss ProposalsStatuses
render *render
Expand Down Expand Up @@ -64,11 +68,19 @@ func (g *GovDAO) PreCreateProposal(r dao.ProposalRequest) (address, error) {
runtime.CurrentRealm(), runtime.PreviousRealm()))
}

// Verify that the one creating the proposal is a member.
caller := runtime.OriginCaller()
mem, _ := getMembers(cross).GetMember(caller)

// If the caller is not a member, they must pay a fee
if mem == nil {
return caller, errors.New("only members can create new proposals")
sent := banker.OriginSend()
feeRequired := chain.NewCoin("ugnot", ProposalFeeAmount)
if len(sent) == 0 || sent.AmountOf("ugnot") < ProposalFeeAmount {
return caller, errors.New(ufmt.Sprintf(
"non-members must send %s to create a proposal",
feeRequired.String(),
))
}
}

return caller, nil
Expand Down
46 changes: 42 additions & 4 deletions examples/gno.land/r/gov/dao/v3/impl/govdao_test.gno
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ import (
"strings"
"testing"

"chain"

"gno.land/p/nt/testutils"
"gno.land/p/nt/uassert"
"gno.land/p/nt/urequire"
"gno.land/r/gov/dao"
"gno.land/r/gov/dao/v3/memberstore"
Expand Down Expand Up @@ -87,7 +90,7 @@ func TestCreateProposalAndVote(cur realm, t *testing.T) {
dao.MustCreateProposal(cross, NewAddMemberRequest(cur, nm1, memberstore.T3, portfolio))
})

urequire.AbortsWithMessage(t, "proposer is not a member", func(cur realm) {
uassert.AbortsContains(t, "proposal creation must be done directly by a user or through the r/gov/dao proxy", func(cur realm) {
dao.MustCreateProposal(cross, NewAddMemberRequest(cur, nm1, memberstore.T2, portfolio))
})

Expand Down Expand Up @@ -216,9 +219,8 @@ func TestUpgradeDaoImplementation(t *testing.T) {
testing.SetOriginCaller(noMember)
testing.SetRealm(testing.NewCodeRealm("gno.land/r/gov/dao/v3/impl"))

urequire.PanicsWithMessage(t, "proposer is not a member", func() {
NewUpgradeDaoImplRequest(govDAO, "gno.land/r/gov/dao/v4/impl", "Something happened and we have to fix it.")
})
testing.SetOriginSend(chain.Coins{chain.Coin{Denom: "ugnot", Amount: ProposalFeeAmount}}) // 1 GNOT
NewUpgradeDaoImplRequest(govDAO, "gno.land/r/gov/dao/v4/impl", "Something happened and we have to fix it.")

testing.SetOriginCaller(m1)
testing.SetRealm(testing.NewCodeRealm("gno.land/r/gov/dao/v3/impl"))
Expand Down Expand Up @@ -275,6 +277,42 @@ func TestUpgradeDaoImplementation(t *testing.T) {
urequire.Equal(t, true, contains(dao.Render("7"), "YES PERCENT: 68.42105263157895%"))
}

func TestNonMemberProposalFee(cur realm, t *testing.T) {
loadMembers()

portfolio := "# This is my portfolio:\n\n- THINGS"
nm1 := testutils.TestAddress("nm1")

testing.SetOriginCaller(noMember)
testing.SetRealm(testing.NewCodeRealm("gno.land/r/gov/dao/v3/impl"))

// Test: Non-member without fee should fail with realm validation error
uassert.AbortsContains(t, "proposal creation must be done directly by a user or through the r/gov/dao proxy", func(cur realm) {
dao.MustCreateProposal(cross, NewAddMemberRequest(cur, nm1, memberstore.T2, portfolio))
})

// Test: Non-member with insufficient fee should also fail with realm validation error
testing.SetOriginSend(chain.Coins{chain.Coin{Denom: "ugnot", Amount: 500000}}) // 0.5 GNOT - insufficient
uassert.AbortsContains(t, "proposal creation must be done directly by a user or through the r/gov/dao proxy", func(cur realm) {
Copy link
Member

Choose a reason for hiding this comment

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

why the error here should say "proposal creation must be done directly by a user or through the r/gov/dao proxy", i would expect the test to ensure there is a "insufficient fee, user must send 1_000_000 gnot" or smth like this, same for all your test in this function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix: 856c91e

dao.MustCreateProposal(cross, NewAddMemberRequest(cur, nm1, memberstore.T2, portfolio))
})

// Test: Non-member with correct fee should also fail with realm validation error
testing.SetOriginSend(chain.Coins{chain.Coin{Denom: "ugnot", Amount: ProposalFeeAmount}}) // 1 GNOT
uassert.AbortsContains(t, "proposal creation must be done directly by a user or through the r/gov/dao proxy", func(cur realm) {
dao.MustCreateProposal(cross, NewAddMemberRequest(cur, nm1, memberstore.T2, portfolio))
})

// Reset OrigSend
testing.SetOriginSend(chain.Coins{})

// Test: Members also fail due to realm validation in tests
testing.SetOriginCaller(m1)
uassert.AbortsContains(t, "proposal creation must be done directly by a user or through the r/gov/dao proxy", func(cur realm) {
dao.MustCreateProposal(cross, NewAddMemberRequest(cur, testutils.TestAddress("nm2"), memberstore.T2, portfolio))
})
}

func contains(s, substr string) bool {
return strings.Index(s, substr) >= 0
}
22 changes: 0 additions & 22 deletions examples/gno.land/r/gov/dao/v3/impl/prop_requests.gno
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package impl

import (
"chain/runtime"

"gno.land/p/aeddi/panictoerr"
"gno.land/p/moul/md"
trs_pkg "gno.land/p/nt/treasury"
Expand All @@ -14,11 +12,6 @@ import (
)

func NewChangeLawRequest(_ realm, newLaw *Law) dao.ProposalRequest {
member, _ := memberstore.Get().GetMember(runtime.OriginCaller())
if member == nil {
panic("proposer is not a member")
}

cb := func(_ realm) error {
law = newLaw
return nil
Expand All @@ -30,11 +23,6 @@ func NewChangeLawRequest(_ realm, newLaw *Law) dao.ProposalRequest {
}

func NewUpgradeDaoImplRequest(newDao dao.DAO, realmPkg, reason string) dao.ProposalRequest {
member, _ := memberstore.Get().GetMember(runtime.OriginCaller())
if member == nil {
panic("proposer is not a member")
}

cb := func(_ realm) error {
// dao.UpdateImpl() must be cross-called from v3/impl but
// what calls this cb function is r/gov/dao.
Expand Down Expand Up @@ -66,17 +54,7 @@ func NewAddMemberRequest(_ realm, addr address, tier string, portfolio string) d
panic("A portfolio for the proposed member is required")
}

member, _ := memberstore.Get().GetMember(runtime.OriginCaller())
if member == nil {
panic("proposer is not a member")
}

if member.InvitationPoints <= 0 {
panic("proposer does not have enough invitation points for inviting new people to the board")
}
Comment on lines -74 to -76
Copy link
Member

Choose a reason for hiding this comment

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

for me this line should never have been there, invitations point should be only for the T3 AddMember function but since it was here it makes me wondering if we should keep it in case the proposal is coming from a member ?

Copy link
Contributor Author

@Davphla Davphla Dec 11, 2025

Choose a reason for hiding this comment

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

I agree, in the end this is just a helper function to create a ProposalRequest object, any non-member could bypass this condition by creating their own ProposalRequest object.
But I don't think we should keep it as it is not protective, or we should rethink how invitationPoint work in GovDAO.


cb := func(_ realm) error {
member.RemoveInvitationPoint()
err := memberstore.Get().SetMember(tier, addr, memberByTier(tier))

return err
Expand Down
12 changes: 10 additions & 2 deletions examples/gno.land/r/gov/dao/v3/impl/render.gno
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func (ren *render) renderProposalPage(sPid string, d *GovDAO) string {

ps := d.pss.GetStatus(dao.ProposalID(pid))
out := ufmt.Sprintf("## Prop #%v - %v\n", pid, p.Title())
out += "Author: " + tryResolveAddr(p.Author()) + "\n\n"
out += ufmt.Sprintf("Author: %s %s\n\n", tryResolveAddr(p.Author()), renderMembershipBadge(p.Author(), d))

out += p.Description()
out += "\n\n"
Expand Down Expand Up @@ -122,7 +122,7 @@ func (ren *render) renderProposalListItem(sPid string, d *GovDAO) string {

ps := d.pss.GetStatus(dao.ProposalID(pid))
out := ufmt.Sprintf("### [Prop #%v - %v](%v:%v)\n", pid, p.Title(), ren.relativeRealmPath, pid)
out += ufmt.Sprintf("Author: %s\n\n", tryResolveAddr(p.Author()))
out += ufmt.Sprintf("Author: %s %s\n\n", tryResolveAddr(p.Author()), renderMembershipBadge(p.Author(), d))

out += "Status: " + getPropStatus(ps)
out += "\n\n"
Expand Down Expand Up @@ -189,3 +189,11 @@ func tryResolveAddr(addr address) string {
}
return userData.RenderLink("")
}

func renderMembershipBadge(addr address, d *GovDAO) string {
mem, _ := getMembers(cross).GetMember(addr)
if mem == nil {
return "`[non-member]`"
}
return ""
}
21 changes: 21 additions & 0 deletions examples/gno.land/r/gov/dao/v3/impl/render_test.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package impl

import (
"testing"

"gno.land/p/nt/urequire"
)

// TestRenderMembershipBadge verifies that the non-member badge is displayed correctly
func TestRenderMembershipBadge(t *testing.T) {
loadMembers()
dao := NewGovDAO()

// Test member (should not show badge)
badge := renderMembershipBadge(m1, dao)
urequire.Equal(t, "", badge, "Members should not have a badge")

// Test non-member (should show badge)
badge = renderMembershipBadge(noMember, dao)
urequire.Equal(t, "`[non-member]`", badge, "Non-members should have a badge")
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ gnoland start
# call gov/dao render to check everything is working as expected and the loader worked
gnokey query vm/qrender --data 'gno.land/r/gov/dao:'

# try to add the proposal using a non-member
# try to add the proposal using a non-member (should fail because no fee was sent)
gnokey maketx run -gas-fee 1000000ugnot -gas-wanted 100000000 -broadcast -chainid=tendermint_test newmember $WORK/proposer/create_proposal.gno
stdout 'only members can create new proposals'
stdout 'non-members must send 1000000ugnot to create a proposal'

-- proposer/create_proposal.gno --
package main
Expand Down
Loading