feat(examples): add N-of-M multisig treasury demo realm#5951
feat(examples): add N-of-M multisig treasury demo realm#5951zardozmonopoly wants to merge 1 commit into
Conversation
Owner/threshold-based multisig treasury following the cw3-flex-multisig pattern (CosmWasm). Owners propose sending coins from the realm's own treasury balance via chain/banker; proposals execute once enough distinct owners approve. Tested end-to-end on Test13 with 3 real owner addresses (2-of-3 threshold): setup, treasury funding, propose, two independent approvals, and execute all verified on-chain with real coin transfer. Unit tests cover state-machine guards (double-setup, double-approve, execute-below-threshold, unknown proposal).
github-actions
Bot
added
the
🧾 package/realm
Gno2D2
added
the
review/triage-pending
🛠 PR Checks Summary🔴 Pending initial approval by a review team member, or review from tech-staff Manual Checks (for Reviewers):
Read More🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers. ✅ Automated Checks (for Contributors):🟢 Maintainers must be able to edit this pull request (more info) ☑️ Contributor Actions:
☑️ Reviewer Actions:
📚 Resources:Debug
|
|
For context: I run a Test13 testnet validator (moniker: zardozmonopoly). Happy to help test any fix or provide more detail if useful... |
davd-gzl
left a comment
davd-gzl
left a comment
There was a problem hiding this comment.
[AI bot - Automatic review]
Automated technical pass: does the code build, run, and behave as described. No design or scope judgement, and no merge verdict. Posted to give a human reviewer a head start.
Reproduced on 9208bed. The realm has no way back from a bad input: initialized is one-shot, a proposal cannot be cancelled, and the treasury address is fixed by the pkgpath. Every gap below therefore lands as permanent state rather than a retryable error, worth closing before this ships as the reference example.
The red Merge Requirements check is the approval bot, not a code problem.
| func TreasuryAddress() address { | ||
| return unsafe.CurrentRealm().Address() | ||
| } |
There was a problem hiding this comment.
Critical: TreasuryAddress returns the caller's realm address, not the treasury's, so a realm that composes with this one funds itself. unsafe.CurrentRealm() stack-walks, so it resolves to whoever asked. Deriving the address from this package's own pkgpath, or from a threaded cur realm, makes both read paths agree.
repro
# from a local clone of gnolang/gno:
gh pr checkout 5951 -R gnolang/gno
cat > gno.land/pkg/integration/testdata/treasury_addr.txtar <<'EOF'
loadpkg gno.land/r/demo/multisig
gnoland start
gnokey query vm/qeval --data 'gno.land/r/demo/multisig.TreasuryAddress()'
gnokey maketx addpkg -pkgdir $WORK/wrapper -pkgpath gno.land/r/demo/wrapper -gas-fee 1000000ugnot -gas-wanted 10000000 -chainid=tendermint_test test1
gnokey query vm/qeval --data 'gno.land/r/demo/wrapper.Treasury()'
-- wrapper/gnomod.toml --
module = "gno.land/r/demo/wrapper"
gno = "0.9"
-- wrapper/wrapper.gno --
package wrapper
import "gno.land/r/demo/multisig"
func Treasury() string { return multisig.TreasuryAddress().String() }
EOF
go test -v -run 'TestTestdata/treasury_addr' ./gno.land/pkg/integration/ 2>&1 | grep -E 'qeval|data:'
rm gno.land/pkg/integration/testdata/treasury_addr.txtar> gnokey query vm/qeval --data 'gno.land/r/demo/multisig.TreasuryAddress()'
data: ("g17vd2lug0kdaeahm9sv3y0udz7pac9kc0kqs0aa" .uverse.address)
> gnokey query vm/qeval --data 'gno.land/r/demo/wrapper.Treasury()'
data: ("g1ndz9e3hkgyz2xgzs9zuj5au9lf8hfncftf7vwh" string)
The second address is the wrapper's own package address.
| if thresh < 1 || thresh > 3 { | ||
| panic("invalid threshold") | ||
| } | ||
| owners.Set(owner1.String(), true) | ||
| owners.Set(owner2.String(), true) | ||
| owners.Set(owner3.String(), true) | ||
| ownerCount = 3 | ||
| threshold = thresh | ||
| initialized = true |
There was a problem hiding this comment.
Setup accepts three owner addresses without checking they are distinct, then records ownerCount = 3 whatever the tree holds. Setup(A, A, A, 2) leaves one owner and an unreachable threshold, so anything sent to the treasury is locked for good behind the one-shot initialized. The threshold bound at line 47 checks the literal 3 rather than the owner set, so Setup(A, A, B, 3) fails the same way.
repro
# from a local clone of gnolang/gno:
gh pr checkout 5951 -R gnolang/gno
cd examples/gno.land/r/demo/multisig
mv multisig_test.gno /tmp/multisig_test.gno.bak
cat > dup_test.gno <<'EOF'
package multisig_test
import (
"testing"
"gno.land/r/demo/multisig"
)
func TestDupOwners(cur realm, t *testing.T) {
a := address("g1owner1xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")
multisig.Setup(cross(cur), a, a, a, 2)
println("Setup(A,A,A,2) accepted")
testing.SetRealm(testing.NewUserRealm(a))
id := multisig.Propose(cross(cur), "payout", a, 100)
multisig.Approve(cross(cur), id)
println(multisig.Render(""))
multisig.Execute(cross(cur), id)
}
EOF
gno test -v -run TestDupOwners .
rm dup_test.gno && mv /tmp/multisig_test.gno.bak multisig_test.gnoSetup(A,A,A,2) accepted
Multisig Treasury
Initialized: true
Owners: 3 | Threshold: 2
Proposal #1: payout | to: g1owner1xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx | amount: 100 | approvals: 1/2 [pending]
=== RUN TestDupOwners
panic: not enough approvals
# …
| func Propose(cur realm, description string, to address, amount int64) int { | ||
| caller := cur.Previous().Address() | ||
| assertOwner(caller) | ||
|
|
||
| id := nextID | ||
| nextID++ | ||
| p := &Proposal{ | ||
| id: id, | ||
| description: description, | ||
| to: to, | ||
| amount: amount, | ||
| } | ||
| proposals.Set(strconv.Itoa(id), p) | ||
| return id | ||
| } |
There was a problem hiding this comment.
Propose stores the payout amount without checking its sign, so a negative or zero payout is only refused by the bank keeper inside Execute. Both owners pay for an approval transaction on a proposal that can never settle, and with no cancel path it stays in the tree and in Render for good.
repro
# from a local clone of gnolang/gno:
gh pr checkout 5951 -R gnolang/gno
cat > gno.land/pkg/integration/testdata/negamt.txtar <<'EOF'
adduser ownerA
adduser ownerB
loadpkg gno.land/r/demo/multisig
gnoland start
gnokey maketx call -pkgpath gno.land/r/demo/multisig -func Setup -args $test1_user_addr -args $ownerA_user_addr -args $ownerB_user_addr -args 2 -gas-fee 1000000ugnot -gas-wanted 10000000 -chainid=tendermint_test test1
gnokey maketx call -pkgpath gno.land/r/demo/multisig -func Propose -args 'drain' -args $ownerA_user_addr -args -500000 -gas-fee 1000000ugnot -gas-wanted 10000000 -chainid=tendermint_test test1
gnokey maketx call -pkgpath gno.land/r/demo/multisig -func Approve -args 1 -gas-fee 1000000ugnot -gas-wanted 10000000 -chainid=tendermint_test test1
gnokey maketx call -pkgpath gno.land/r/demo/multisig -func Approve -args 1 -gas-fee 1000000ugnot -gas-wanted 10000000 -chainid=tendermint_test ownerA
! gnokey maketx call -pkgpath gno.land/r/demo/multisig -func Execute -args 1 -gas-fee 1000000ugnot -gas-wanted 10000000 -chainid=tendermint_test test1
stderr 'invalid coins error'
EOF
go test -v -run 'TestTestdata/negamt' ./gno.land/pkg/integration/
rm gno.land/pkg/integration/testdata/negamt.txtar> gnokey maketx call ... -func Propose -args 'drain' ... -args -500000 ... test1
(1 int)
OK!
> gnokey maketx call ... -func Approve -args 1 ... test1
OK!
> gnokey maketx call ... -func Approve -args 1 ... ownerA
OK!
> ! gnokey maketx call ... -func Execute -args 1 ... test1
Data: invalid coins error
Execute at gno.land/r/demo/multisig/multisig.gno:117
| func Approve(cur realm, proposalID int) { | ||
| caller := cur.Previous().Address() | ||
| assertOwner(caller) | ||
|
|
||
| raw := proposals.Get(strconv.Itoa(proposalID)) | ||
| if raw == nil { | ||
| panic("proposal does not exist") | ||
| } | ||
| p := raw.(*Proposal) | ||
| if p.executed { | ||
| panic("proposal already executed") | ||
| } | ||
| if p.approvals.Get(caller.String()) != nil { | ||
| panic("already approved") | ||
| } | ||
| p.approvals.Set(caller.String(), true) | ||
| p.approveCnt++ | ||
| } |
There was a problem hiding this comment.
Nothing in the realm removes an approval or a proposal. Once a payout reaches the threshold, Execute settles it at any later block for any caller, so a proposal approved while the treasury was empty fires the moment someone funds the address. cw3-flex-multisig, the model the description names, expires proposals for this reason.
| func TestSetupInitializesCorrectly(cur realm, t *testing.T) { | ||
| owner1 := cur.Previous().Address() | ||
| owner2 := address("g1dummyaddressfortestpurposesonlyxxxxxx1") | ||
| owner3 := address("g1dummyaddressfortestpurposesonlyxxxxxx2") | ||
|
|
||
| Setup(cur, owner1, owner2, owner3, 2) | ||
|
|
||
| if !initialized { | ||
| t.Fatal("expected initialized to be true") | ||
| } | ||
| if threshold != 2 { | ||
| t.Fatalf("expected threshold 2, got %d", threshold) | ||
| } | ||
| if ownerCount != 3 { | ||
| t.Fatalf("expected 3 owners, got %d", ownerCount) | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing test: a threshold reached by distinct owners, and a payout that moves coins. Every test here is in-package and calls through the same cur, so TestApproveIncrementsCount never passes 1 of 2 and only the manual Test13 run exercises the state machine. The suite is also order-dependent: gno test -run TestProposeCreatesProposal panics with multisig not initialized, and six of the eight fail standalone.
test cases
An external test package lets callers differ. Setup is one-shot and package state is shared across files, so this replaces multisig_test.gno rather than sitting beside it.
package multisig_test
import (
"chain"
"chain/banker"
"testing"
"gno.land/r/demo/multisig"
)
func TestExecutePaysRecipient(cur realm, t *testing.T) {
owner1 := address("g1owner1xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")
owner2 := address("g1owner2xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")
owner3 := address("g1owner3xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")
recipient := address("g1recipientxxxxxxxxxxxxxxxxxxxxxxxxxxxx")
multisig.Setup(cross(cur), owner1, owner2, owner3, 2)
treasury := chain.PackageAddress("gno.land/r/demo/multisig")
testing.IssueCoins(treasury, chain.Coins{{Denom: "ugnot", Amount: 1000}})
testing.SetRealm(testing.NewUserRealm(owner1))
id := multisig.Propose(cross(cur), "payout", recipient, 400)
multisig.Approve(cross(cur), id)
testing.SetRealm(testing.NewUserRealm(owner2))
multisig.Approve(cross(cur), id)
testing.SetRealm(testing.NewUserRealm(owner3))
multisig.Execute(cross(cur), id)
got := banker.NewReadonlyBanker().GetCoins(recipient)
if got.String() != "400ugnot" {
t.Fatalf("recipient balance = %s, want 400ugnot", got.String())
}
}The same flow across three real keys on a node, asserting bank/balances, is at tests/multisig_flow.txtar.
| type Proposal struct { | ||
| id int | ||
| description string | ||
| to address | ||
| amount int64 | ||
| executed bool | ||
| approvals avl.Tree | ||
| approveCnt int | ||
| } |
There was a problem hiding this comment.
Nit: Proposal is exported but every field is unexported and there are no accessors, so the only way for another realm to read a proposal is to scrape Render output. approveCnt also duplicates what approvals.Size() already knows, and Execute trusts the counter.
| to: to, | ||
| amount: amount, | ||
| } | ||
| proposals.Set(strconv.Itoa(id), p) |
There was a problem hiding this comment.
Nit: proposal ids are stored as decimal strings, so once there are ten proposals Render lists them as 1, 10, 11, 2.
| return unsafe.CurrentRealm().Address() | ||
| } | ||
|
|
||
| func Render(path string) string { |
There was a problem hiding this comment.
Nit: Render takes path and never reads it, so :1 and :anything both return the full list.
| func init() { | ||
| deployer = unsafe.OriginCaller() | ||
| nextID = 1 | ||
| } |
There was a problem hiding this comment.
Suggestion: realms under examples/ are uploaded at genesis, so on a public chain the account init records is the genesis deployer and no reader can run the demo. Setup refuses everyone else, so one owner set exists for the life of the realm. A factory that mints a multisig per caller would make it something people can try.
| func Execute(cur realm, proposalID int) { | ||
| raw := proposals.Get(strconv.Itoa(proposalID)) | ||
| if raw == nil { | ||
| panic("proposal does not exist") | ||
| } | ||
| p := raw.(*Proposal) | ||
| if p.executed { | ||
| panic("proposal already executed") | ||
| } | ||
| if p.approveCnt < threshold { | ||
| panic("not enough approvals") | ||
| } | ||
| p.executed = true | ||
|
|
||
| bkr := banker.NewBanker(banker.BankerTypeRealmSend, cur) | ||
| bkr.SendCoins(cur.Address(), p.to, chain.Coins{{Denom: "ugnot", Amount: p.amount}}) | ||
| } |
There was a problem hiding this comment.
Suggestion: nothing here emits an event, so following the treasury means polling Render. A chain.Emit on Propose and Execute would let a wallet or indexer show pending payouts.
3 participants
Adds a contract-based multisig treasury realm under examples/gno.land/r/demo/multisig, following the pattern requested in #379 (contract-based multisig patterns).
Design follows the well-established cw3-flex-multisig pattern from CosmWasm/Cosmos:
Setup(owner1, owner2, owner3, threshold)to configure N owners and an M-of-N threshold (init() cannot take parameters, so setup is a guarded one-shot call instead)TreasuryAddress())Proposea payout (description, recipient, amount)Approveproposals; once distinct approvals reach the threshold, any caller canExecute, which uses chain/banker (BankerTypeRealmSend) to move real coins from the realm balance to the recipientTesting:
Open to feedback on API shape (e.g., generalizing Setup to accept a variable number of owners once/if a cleaner way to pass slices via maketx call args is available), naming, or additional edge cases you would want covered.