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

Build a basic stake controlled governance contract #36

Closed
eloylp opened this issue Aug 2, 2023 · 5 comments · Fixed by #51 or #87
Closed

Build a basic stake controlled governance contract #36

eloylp opened this issue Aug 2, 2023 · 5 comments · Fixed by #51 or #87
Assignees
Milestone

Comments

@eloylp
Copy link
Member

eloylp commented Aug 2, 2023

Context

Possible interesting links:

Goal

  • Build a basic stake controlled governance contract that lets holders vote on code upgrades.

Governance contract invariants

  • The contract should be able to work with any token that accomplishes the token interface.
  • The contract will have a single curator or admin, that will:
    • Whitelist participants.
    • Whitelist proposals created by participants.
  • Curator can be replaced by participants any time by creating a special proposal.
  • A participant can stake an arbitrary quantity of funds.
  • A participant can withdraw all its funds at any moment. This implies the account needs to be whitelisted by curators again if wants to return.
  • A participant can withdraw an arbitrary amount of funds at any moment.
  • A participant can stake more funds at any moment.
  • The contract needs to track balances of all participants. (Address => Balance).
  • Any participant account with non zero balance in the contract address, can create and vote on "proposals".
  • Proposals can be in status:
    • Waiting - Proposal needs curator approval.
    • Voting - Voting is open.
    • Voted - Deadline was reached, results are available.
    • Cancelled - Proposal cancelled by proposer.
    • Executed - Once approved, only the proposer can execute the code upgrade.
  • Each proposal has a deadline, by default, 10 days after creation. No more votes are accepted after this deadline is reached.
  • Quorum in proposals is calculated by:
    • Proportionally to each participants staked funds in the moment of voting.
    • Only positive and negative votes are available.
    • A proposal is approved only if positive votes > negative votes when the voting deadline is reached.
  • The contract must emit enough events in order to keep informed other dapps:
    • On each proposal status change.
    • On code upgrades (WasmHash, ProposalId, ApprovalRate).
    • On funds staking (Address, Amount, OpType) where OpType is "stake".
    • On funds withdrawal (Address, Amount, OpType) where OpType is "withdraw".
    • On curator change.

Aside thoughts:

  • We could now check if the bits of our voting example is re-usable enough for being used by this contract.
  • Currently we are going to only implement one kind of voting mechanism for keeping it simple, but we should code it in a way voting mechanism can be easily extended.
  • Soroban has the the ability to not only use "persistent" storage, but also "emporary" one. We should use this wisely as we can reduce fees and the "state bloat" problem.
  • Once a code update proposal updates the code of the DAO contract, the contract ID will remain the same (see docs).
@eloylp eloylp added this to the Milestone 1 milestone Aug 2, 2023
@eloylp eloylp self-assigned this Aug 22, 2023
@eloylp
Copy link
Member Author

eloylp commented Aug 29, 2023

Hello @mariopil and @geofmureithi , from now you can follow up the work on this draft PR: #51 . I will try to push commits frequently. Feel free to comment anything you would like to change at any point :)

@eloylp
Copy link
Member Author

eloylp commented Sep 4, 2023

Alright ! here is an status update of the work done till now:

Currently, the contract can manage the staking part, which means the following API is available:

#[contractimpl]
impl GovernanceContract {
    pub fn init(env: Env, curator: Address, token: Address) {
        // ...
    }

    pub fn join(env: Env, participant_addr: Address, amount: i128) -> Result<(), Error> {
       // ...
    }

    pub fn stake(env: Env, participant: Address, amount: i128) -> Result<(), Error> {
       // ...
    }

    pub fn leave(env: Env, participant: Address) -> Result<(), Error> {
       // ...
    }    

    pub fn withdraw(env: Env, participant: Address, amount: i128) -> Result<(), Error> {
       // ... 
    }
}

I am now continuing implementing the proposal/voting API, taking into account the voting contract we did before, lets see how much we can reuse from there. After that, i will proceed with the code upgrade feature, once a proposal is approved.

I think i am going to proceed and implement all the functionality first. Probably composability for the UI is not going to be the best in the first POC. But i just want to see the patterns in code. Then we can discuss how to refactor it.

I will continue pushing this little summaries for keeping you up to date @geofmureithi @mariopil

@eloylp
Copy link
Member Author

eloylp commented Sep 12, 2023

First POC version (proposal)

During the last days we were able to implement a first version of a stake controlled governance contract that accomplishes most of the previously commented objectives.

All the code is available in the related PR, but lest do a walk through the most important aspects, so the later review will be easier.

Overview

The proposal is to re-use our current voting contract in a way the call tree is like this:

graph LR;
user--> GovernanceContract -- only_admin_mode --> VotingContract
Loading

The governance contract is responsible of also initialise the voting one. This is being done by the env.deployer() API and provides a way to deploy the contracts that are dependencies of a bigger system. Under the eyes of the user of the Governance contract, it would only be deploying one contract.

The voting contract is intializated in "only admin mode" , so it will only accept calls from the Governance contract, which would be the admin for that contract. No one else can invoke the voting contract.

Authorizations from participants are being forwarded accordingly during the cross contract calls.

API surface

Heres is the updated API surface. Hope is enough intuititve.

#[contractimpl]
impl GovernanceContract {
    pub fn init(env: Env, curator: Address, token: Address) {
        // ...
    }

    pub fn join(env: Env, participant_addr: Address, amount: i128) -> Result<(), Error> {
       // ...
    }

    pub fn stake(env: Env, participant: Address, amount: i128) -> Result<(), Error> {
       // ...
    }

    pub fn leave(env: Env, participant: Address) -> Result<(), Error> {
       // ...
    }    

    pub fn withdraw(env: Env, participant: Address, amount: i128) -> Result<(), Error> {
       // ... 
    }
   
   pub fn new_proposal(env: Env, participant: Address,  id: u64, kind: ProposalType, new_contract_hash: BytesN<32>) -> Result<(), Error> { 
     // ...
   }
   pub fn vote(env: Env, participant: Address, id: u64) -> Result<(), Error> {
     // ...
   }
   pub fn execute_proposal(env: Env, participant: Address, id: u64) -> Result<(), Error> {
    // ...
   }
}

Regarding the state

During the voting phase, the voting contract only acts as a registry for proposals, but it doesnt hold any current status of them regarding approval rate. Thats because the staking is dynamic, and voting results are only definitive once the proposal is executed. In that moment, the governance contract forwards the staking data to the voting contract, which finally stores the final state of the proposal.

I have certain concerns here regarding how this would work at scale. Maybe a good test for this would be to simulate a contract with millions of participants and proposals, while drawing a fee curve x->(participants+proposals) y->(fee per operation) and see if it grows.

Published events should be used by Quasar (indexer) to provide a near-realtime view of the status of a proposal, as all staked amounts and voting operations are being reflected in published events (see below).

Supported proposals

The current design should allow the code to be easily extended with more proposal types. Here are the current ones:

  • Standard - Does nothing but saving state on execution.
  • Code upgrade - A proposer deploys a new version of the governance contract obtaining a wasm hash, and creates a new proposal with the wasm hash as comment. Once the proposal is executed, the code upgrade takes place.
  • Curator change - At any moment the participants can decide curator is not doing well and that they need a change. They can create new proposal with a representation of the address as comment (see PR on how we are actually encoding this). Once the proposal is executed, the curator change takes place.

Generalization of the Voting contract

Some changes have been done in the voting contract. They are only generalizations and improvements this new use case brings with it, but it should be perfectly usable standalone. See the linked PR for more information. The commits start with the prefix [VC] of "voting contract".

Strategy with events

We have 2 contracts on this equation (Governance and voting) and events should not overlap. That means each contract has the resposibility of publishing its own related operations events.

There is an initial event published by the governance contract, which tells whats the address of the deployed voting contract. So Quasar (an indexer) could use it for correlating events of the contracts from that initial point.

The following events should be synchronized with the developments in Quasar and the following ones should be taken as a proposal, subject to changes.

Events of the Governance contract

env.events().publish(
            (Symbol::new(&env, "participant_joined"), participant_addr),
            (),
        );
env.events()
            .publish((Symbol::new(env, "stake"), participant.address()), amount);
env.events()
            .publish((Symbol::new(&env, "participant_left"), &participant), ());
env.events().publish(
            (Symbol::new(env, "withdraw"), participant.address()),
            amount,
        );
env.events()
            .publish((Symbol::new(&env, "participant_whitelisted"),), participant);
env.events().publish(
            (Symbol::new(&env, "proposal_executed"), &participant, id),
            (),
        );

Events of the Voting contract

env.events().publish(
            (Symbol::new(&env, "proposal_created"), id, kind, proposer),
            ()
        );
env.events().publish(
            (Symbol::new(&env, "proposal_voted"), id, voter),
            updated_approval_rate,
        );

Deviations from the initial plan (see issue description)

Proposals can be in status:
Waiting - Proposal needs curator approval.
Voting - Voting is open.
Voted - Deadline was reached, results are available.
Cancelled - Proposal cancelled by proposer.
Executed - Once approved, only the proposer can execute the code upgrade.

Having a status field on a proposal is tricky. Because we would depend in contract invokations in order to update it. So the status is calculated each time someone invokes is_closed() or similar methods. On this calculations, checking the expiration date is normally what we do to determine the status.

Quorum in proposals is calculated by:
Proportionally to each participants staked funds in the moment of voting.
Only positive and negative votes are available.
A proposal is approved only if positive votes > negative votes when the voting deadline is reached.

The current approach is to preserve the current participation system, as it is naturally aligned with the current voting system of the voting contract. So instead of storing votes, the voting contract stores participations , which could be votes anyways, so the original behaviour is preserved - The proposal needs to reach a configured approval rate for being considered "approved" inside the configured time window. Would be good to think how to implement other mechanisms BTW.

The contract will have a single curator or admin, that will:

  • Whitelist participants.
  • Whitelist proposals created by participants.

Yes, for whitelisting participants. But whitelisting proposals is probably too much, so it was initially not implemented. Only the whitelisted participants balance will be taken into account for approval rates calculation.

Exceeding the Fee budget

While testing the execute_proposal(..) Governance API function, we started to errors related to budget exceeded. We solved that remporary by calling in tests:

env.budget().reset_unlimited()

This could be seen as a bad practice, but taking into account Stellar network is still determining this limits (See #42 ) , i think the proper thing should be to check Discord regarding this. IMO, we are not doing many calls, but limits looks too much conservative.

New tools creation

Some little experimentation has been done in order to improve the readbility of the code (see linked PR) like:

  • Created a utils crate for intially grouping this tools, without creating a mesh of dependency graph.
  • Added the idea of repositories for storage, so we dont need to mess with storage keys along all the code.

Other ideas and thoughts

  • As we can see in the public API, the satking part is very determined. Would it make sense to extract that logic in a way that can be reused by other contracts ? i.e Adding a staking layer ? Part of a second iteration maybe.
  • Currently, a proposer can execute an approved proposal anytime. But maybe we should put a limit on this. Would be weird to see someone applying a code upgrade 2 years later.
  • Should the creator of a proposal vote on its own proposal ?

Conclusions

  • This is a first proposal of the governance contract and should be carefully reviewed.
  • It allowed us to see a bit of how to integrate 2 contracts and reuse their functionality.
  • It also served as good test for the voting contract, to see how to make it more generic.
  • The team gained more experience with a more complex contract overall.

Possible next steps (Heading to production code)

In order of priority i would:

  • Hardening the code. I would not believe all the cases are contemplated. We were adding a lot of features, now comes the consolidation/adjustments part.
  • Add more code level Rust docs.
  • More code refactor in both contracts. Now i think its decent, but can be improved.
  • Tests in futurenet . We didnt do any ! Maybe would be good to start writing tests with the outcome of E2E testing tools for Soroban contracts #34 .
    • Code upgrade needs to be tested in futurenet. There is currently a unit test (ignored), but i am getting a cryptic error i am not able to debug.
  • Test behaviour of data structures and fees with Millions of participants and see where it breaks (state section for more details).
  • Solve the problem regarding exceeding the fees (See dedicated section above).
  • Overall review of corner cases.
  • Once the contract is more stabilised, try Publishing and importing contracts via OCI #58 , as we have a good use case here.

Let me know your thoughts ! Any feedback is welcome. Moving this issue and PR to review ⏩

@eloylp
Copy link
Member Author

eloylp commented Oct 8, 2023

Hello @geofmureithi @mariopil , there are some improvements in the governance contract. Please check last commits in the PR they have reasonable descriptions and are atomic enough IMO. Moving #51 for review again ! thanks ! ⏩

The main difference is that now cross contract calls are performed through env.invoke_contract(...) instead of contract clients, as when using clients one cannot invoke struct methods and also problems with shared types (See this discord discussion) . The only downside, is that we need to encode the types before sending them as soroban_sdk::Val , so we lose type checking in cross contract calls. But clearly solved commented (discord discussion) runtime issues + now we can use methods in structs, which in our case, reduced a lot cross contract calls, that made us resolve the fees issue we had pending to resolve.

@eloylp
Copy link
Member Author

eloylp commented Oct 8, 2023

While adding docs i felt i was repeating things and thought about this #86 , let me know your thoughts there :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant