search

anilhelvaci / dapp-pool-lending-protocol

The core of the Agoric economy
9 stars 1 forks source link

Governance Analysis #41

Closed anilhelvaci closed 2 years ago

anilhelvaci commented 2 years ago

Requirements

The parameters to be controlled by governance are listed in the bounty specification. Seems like they all can be controlled by one ParamManager controlled by a contractHelper.

Current Structure

There's a ParamManager for every pool in the protocol. These ParamManagers are stored in the LendingPool contract. Ideally I want to use the contractHelper.js but not sure how I can make that happen in a scenario where we are required to update the parameters that are directly related to pools.

Steps

anilhelvaci commented 2 years ago

Development Notes

These are notes that I'm leaving to myself as I'm exploring the governace package.

  • getParamMgrRetriever, what does this return?
  • ElectionType.offer_filter needs further explanation
  • When governing an API invocation, how do we see the arguments that will be passed to the API to be invoked?
  • a property called methodArgs is present inside issue object.
  • When opening a question on whether or not mark an asset as collateral, does it make sense to open this question with the ElectionType as an api_invocation or a param_change?
  • Who holds the contractGovernor's creatorFacet?
  • How can a simple voter get to vote on a question?
  • Logic behind electrateTools/getPoserInvitation?
  • Structure of offerConfig for makeVoteInvitation?
  • How do voters get the questionHandle? Is there another way other than using getOpenQuestions?
  • How does the poserInvitation get updated? Related Methods
  • getUpdatedPoserFacet
  • What happens if I use paramChangeSpec that does not have a [CONTRACT_ELECTORATE] property?
anilhelvaci commented 2 years ago

Initial Design

Below is the initial design I've come to after going over Agoric's existing governance package and Compound Finance's governance method.

I'll refer to the governance token as GOV. This name is arbitrary, might change later.

The general rules around the initial governance design is;

The way I'm planning to implement above behaviors in agoric-sdk governance package is;

Users should lock their GOV tokens inside a ZCFSeat so that we're sure they actually have those tokens.

Electorate

The existing implementation of the an Electorate is the committee.js contract which stores all posed questions in a map that has structure like this;

const allQuestions = MapStore<QuestionHandle, VoteCounterFacets>

Since users will lock their GOV tokens into our contract we're gonna have to use a ZCFSeat to escrow those tokens. The way I imagine how to integrate a ZCFSeat into the existing structure above is;

/**
@type {{
voteCounterFacet: VoteCounterFacet,
tokenSeat: ZCFSeat
}} QuestionFacet
*/
const allQuestions = MapStore<QuestionHandle, QuestionFacet>

To achieve desired behavior a collection of methods like below should be implemented;

const verifyCanPoseQuestion = () => {
// Checks if the user has enough GOV tokens to pose a question. This method will be added to the existing `addQuestion` implementation in the committee contract.
};

const vote = (questionHandle) => {
// Enables users to vote on a particular question at a the weight they posses GOV tokens. 
};

const redeem = (questionHandle) => {
// Lets users redeem the GOV tokens they locked for a particular question.
};

const electorateFacet = {
    addQuestion,
    vote,
    redeem,
};

const creatorFacet = {
.
.
.
    getElectorateFacetInvitation: () => getElectorateFacetInvitation(zcf, electorateFacet),  // See ElectorateTools
};

ElectorateTools

This module is gonna be like the existing ElectorateTools but we'll store this in our package and will implement required utility methods for our Electorate here. One obvious method is;

/**
 * @param {ZCF} zcf
 * @param {ElectorateFacet} electorateFacet
 */
const getElectorateFacetInvitation = (zcf, electorateFacet) => {
  const electorateFacetHandler = () => Far(`questionPoser`, electorateFacet);
  return zcf.makeInvitation(electorateFacetHandler, `electorateFacet`);
};

ElectionManager

Work_In_Progress

Below is a very high level overview of the design that aims to show which methods will be added to which component. lending-protocol-tests-governance-design

Chris-Hibbert commented 2 years ago

getParamMgrRetriever, what does this return?

See README.md

ElectionType.offer_filter needs further explanation

Zoe now has filters that allow a contract or its governance to limit which offers can be exercised. The purpose of this is to allow governance to disable access to the contract in emergencies. My apologies that this hasn't been added to the documentation yet.

https://github.com/Agoric/agoric-sdk/commit/732e229ca5d3f3bb196b8bcaa442be3f03f99a62

Who holds the contractGovernor's creatorFacet?

Our bootstrap environment holds those. I suspect this is insufficiently documented. You might ask about it in office hours.

How can a simple voter get to vote on a question?

Their voterFacet can be used to cast ballots on questions they know about. The harder question is how they come to know about votes in progress. The committee's publicFacet has a questionSubscriber that notifies about new questions, and a getOpenQuestions() method that allows one to ask directly. Our voting UI makes use of these.

Logic behind electrateTools/getPoserInvitation?

The creatorFacet in committee.js has a method getPoserInvitation() that allows its caller to get an invitation that can be used to create a new question that the electorate can vote on. When the committee is controlled by an electionManager (e.g. psmCharter.js) that method is tightly held and only a limited set of people can invoke it.

Users should lock their GOV tokens inside a ZCFSeat so that we're sure they actually have those tokens.

I recommend following the attestation model. This allows holders of some tokens to get attestations that they can use to vote and doesn't require locking up the tokens.

anilhelvaci commented 2 years ago

Thanks @Chris-Hibbert !!!

anilhelvaci commented 2 years ago

Question Dump On attestation

2022-11-11

Related Code my-lien.js attestation.js bridge.js attestation/test-userFlow.js

I'd be glad if you could answer above questions 🙏 @Chris-Hibbert @dckc

dckc commented 2 years ago
  • how can I create the attMaker and therefore lienBridge objects for my own governance token?

The lienBridge makes sense only for tokens where an ERTP brand (such as BLD) represents a cosmos denom (such as ubld). Does the GOV token necessarily have a cosmos denom?

If so, then it would make sense to expand makeStakeReporter to work not just for BLD but for all brands registered with E(bankManager).addAsset(). Then the home.attMaker would work for GOV as well as BLD, provided GOV is registered.

  • As a dApp developer do I have access to a bridgeManager or bridgeDevice from a deploy script?

No; that would allow dApp developers to interfere with chain-wide services.

  • Where should we create the attestationFacets: Electorate, ElectionManager or the governedContract(LendingPool)?

Probably LendingPool. @Chris-Hibbert , does that make sense to you?

Another possibility is a separate contract. BLD Boost was originally a separate contract from the attestation maker, but we folded them together to simplify some things. I can probably dig up the separate contract and you might want to use that.

Chris-Hibbert commented 2 years ago

Where should we create the attestationFacets: Electorate, ElectionManager or the governedContract(LendingPool)? Probably LendingPool. @Chris-Hibbert , does that make sense to you?

certainly not the electorate or electionManager. Whatever can validate ownership of the underlying token, which makes some sense for the lendingPool.

Another possibility is a separate contract. BLD Boost was originally a separate contract from the attestation maker, but we folded them together to simplify some things. I can probably dig up the separate contract and you might want to use that.

Yes, that would be helpful. I recall the original version (or at least an earlier version) having tests that started with assets created in the JS layer.

Anil Helvaci => Found it: https://github.com/Agoric/agoric-sdk/pull/3475

anilhelvaci commented 2 years ago

The lienBridge makes sense only for tokens where an ERTP brand (such as BLD) represents a cosmos denom (such as ubld). Does the GOV token necessarily have a cosmos denom?

I don't know what it means for an ERTP brand to represent a cosmos denom @dckc. What I had in mind is a simple ERTP fungible token created with zcf.makeZCFMint(). If having a cosmos denom just means that it should have 6 decimal places like BLD then I can definitely do that. But according to:

If so, then it would make sense to expand makeStakeReporter to work not just for BLD but for all brands registered with E(bankManager).addAsset(). Then the home.attMaker would work for GOV as well as BLD, provided GOV is registered.

Requires some changes to the agoric-sdk as well and waiting for this change to come alive probably going to block me for some time.

Another possibility is a separate contract. BLD Boost was originally a separate contract from the attestation maker, but we folded them together to simplify some things. I can probably dig up the separate contract and you might want to use that.

Yes, that would be helpful. I recall the original version (or at least an earlier version) having tests that started with assets created in the JS layer.

This seems like a quicker approach. I would be so glad if you could provide me the separate contract and the tests you mention here.

I want to sum up what I understand just to make sure we're on the same page;

Am I correct? @Chris-Hibbert @dckc ?

I'm still not sure how this separate contract will make sure users actually have the GOV tokens without making them lock their GOV tokens?

dckc commented 2 years ago

The lienBridge makes sense only for tokens where an ERTP brand (such as BLD) represents a cosmos denom (such as ubld). Does the GOV token necessarily have a cosmos denom?

I don't know what it means for an ERTP brand to represent a cosmos denom @dckc. What I had in mind is a simple ERTP fungible token created with zcf.makeZCFMint().

Oops. I'm afraid the whole discussion of attestations has been a red herring. You did say that you plan for users to put their tokens in escrow with Zoe in order to prove ownership. Attestations are a way to avoid that, since BLD boost is designed to let people participate without transferring their BLD tokens to Zoe.

anilhelvaci commented 2 years ago

You did say that you plan for users to put their tokens in escrow with Zoe in order to prove ownership. Attestations are a way to avoid that, since BLD boost is designed to let people participate without transferring their BLD tokens to Zoe.

I need to make sure users have the governance token of my protocol before I let them do any governance action. One possible solution I had in mind was to escrow the tokens in Zoe. But this is not mandatory I just need to make sure they have the tokens. You and Chris suggested attestations saying that it is more flexible and users might need their tokens for something else. This makes sense to me. But from what I can understand; attestations, as they are right now, only work for BLD brand and no other ERTP brand. If there's a solution for this, I want to use attestations for governance tokens distributed from lending protocol.

dckc commented 2 years ago

The attestations used in BLD boost attest that some cosmos-level tokens have a lien against them.

In order to get further in this discussion, I suppose we need to clear this up...

I don't know what it means for an ERTP brand to represent a cosmos denom @dckc.

I'm not sure how to do that briefly. Perhaps in office hours tomorrow?

anilhelvaci commented 2 years ago

I'm not sure how to do that briefly. Perhaps in office hours tomorrow?

That'd be great, looking forward to it!

anilhelvaci commented 2 years ago

Rethink

dckc commented 2 years ago

@Chris-Hibbert says it has to be a contract

ref: coreArchitecture.png (office hours recording to appear)

anilhelvaci commented 2 years ago

In this session of the office hours it has been clarified that there's no easy way for an ordinary ERTP token can be attested. So moving forward with the escrowing strategy.

anilhelvaci commented 2 years ago

Governance Implementation Initial Review

Instructions

Context

As a result of the conversation above and my own work, I've made the below design