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

Enable Governance to add Collateral type to Treasury #3924

Closed
4 tasks done
Chris-Hibbert opened this issue Oct 4, 2021 · 19 comments
Closed
4 tasks done

Enable Governance to add Collateral type to Treasury #3924

Chris-Hibbert opened this issue Oct 4, 2021 · 19 comments
Assignees
Labels
AMM Core Economy OBSOLETE in favor of INTER-protocol enhancement New feature or request Governance Governance Inter-protocol Overarching Inter Protocol restival to be done before RUN Protocol Purple Team festival security Vaults VaultFactor (née Treasury)
Milestone

Comments

@Chris-Hibbert
Copy link
Contributor

Chris-Hibbert commented Oct 4, 2021

What is the Problem Being Solved?

Adding a Collateral type to the Treasury should be under the control of governance.

Description of the Design

I think this requires a special purpose function, and some general purpose controls to manage and make visible the control over when the function gets invoked.

The special purpose function includes creating a vaultManager, endowing/finding an appropriate AMM, and connecting the two up as well as setting up the parameters for the new pool & vaultMgr.

Security Considerations

This involves governance, so the standard legibility targets apply.

Test Plan

Probably requires SwingSet tests including multiple voters, and a demonstration of someone validating the chain of responsibility.

as part of RUN Protocol preview:

@Chris-Hibbert Chris-Hibbert added enhancement New feature or request security Medium Core Economy OBSOLETE in favor of INTER-protocol Inter-protocol Overarching Inter Protocol Governance Governance AMM labels Oct 4, 2021
@Chris-Hibbert Chris-Hibbert added this to the Beta Phase 4: Governance milestone Oct 4, 2021
@Chris-Hibbert Chris-Hibbert self-assigned this Oct 4, 2021
@Chris-Hibbert
Copy link
Contributor Author

Because

  • this impacts the interactions between the Treasury and the AMM
  • it may require some updates on the AMM side to support automated changes
  • setting up parameters for the AMM is currently tangled in the Treasury and will eventually be separate.

I think the following order will reduce cross-dependencies most quickly:

  • add parameter governance to the constantProduct AMM
  • start up constant product AMM stand-alone
  • remove AMM creation from Treasury; make it rely on a specified AMM
  • add governance for new collateral types to Treasury, depending on the new AMM. The Treasury will assume it doesn't need to create a new pool

The plan for governance control of adding new collateral types is

  • a generic mechanism in the governance package for queuing up a question that will invoke contract-provide code if it passes, and
  • code in the treasury to plug into that mechanism and add a new collateral type. I expect the code in the treasury to
    • check preconditions
    • format a question spec
    • carry out the vote if successful.
  • The new mechanism will expect to rely on the same electorate used for parameter governance.

I think this approach leaves paramManager untouched. It may add functionality to contractHelper.

@dckc
Copy link
Member

dckc commented Oct 8, 2021

@Chris-Hibbert for the demo (#3913) the thing I need most urgently is a test case - or at least a somewhat detailed sketch of it.

Would you please draft something?

p.s. we talked thru enough details for now.

@rowgraus rowgraus added the MN-1 label Jan 19, 2022
@Tartuffo Tartuffo removed the MN-1 label Feb 7, 2022
@Tartuffo Tartuffo removed this from the Beta Phase 4: Governance milestone Feb 8, 2022
@Tartuffo Tartuffo added the restival to be done before RUN Protocol Purple Team festival label Feb 8, 2022
@Tartuffo
Copy link
Contributor

This is a bootstrap value that Governance can change, consumed by code in the top of the Vault Factory.

@dckc
Copy link
Member

dckc commented Feb 14, 2022

The special purpose function includes creating a vaultManager, endowing/finding an appropriate AMM, and connecting the two up as well as setting up the parameters for the new pool & vaultMgr.

I don't see why it's any more than just calling addVaultType, as in:

return E(vaultFactoryCreator).addVaultType(
kit.issuer,
issuerName,
rates,
);

One thought I had was: we have Issuer parameters. Maybe add a new parameter for a list of Issuers, and have governance vote to change that? But that's kinda klunky. And it doesn't the keyword nor rates.

So... how about some "call method X on the creatorFacet with args (a1, a2, ...)" question structure?

@Chris-Hibbert
Copy link
Contributor Author

So... how about some "call method X on the creatorFacet with args (a1, a2, ...)" question structure?

I think that's the right answer. When I created this issue, there was more work involved, because creating a new vaultManager included adding and endowing a new AMM pool. It's much simpler since that's been refactored out.

@Chris-Hibbert
Copy link
Contributor Author

wrong button. sorry.

@Chris-Hibbert Chris-Hibbert reopened this Feb 14, 2022
@Chris-Hibbert Chris-Hibbert added the Vaults VaultFactor (née Treasury) label Mar 10, 2022
@Tartuffo Tartuffo added this to the Mainnet 1 milestone Mar 23, 2022
@Tartuffo Tartuffo modified the milestones: Mainnet 1, RUN Protocol RC0 Apr 6, 2022
@Tartuffo Tartuffo modified the milestones: Mainnet 1, RUN Protocol RC0 Apr 6, 2022
@Tartuffo Tartuffo removed the Medium label Apr 8, 2022
@Tartuffo Tartuffo modified the milestones: Mainnet 1, RUN Protocol RC0 Apr 11, 2022
@Chris-Hibbert
Copy link
Contributor Author

The process of adding a collateral type to the VaultFactory under governance is a matter of holding a cosmos-level vote, which invokes a script that runs in the bootstrap context.

As I understand it, this can be done using the scenario2-core-eval target in cosmic-swingset/Makefile. The script would be in a file, and EVAL_CODE would be set to the filename before running make.

The script would look approximately like

/* global E */

/** Add a new collateral type as directed by governance */

import { makeRatio } from '@agoric/zoe/src/contractSupport/index.js';
import { AmountMath } from '@agoric/ertp';

import { CENTRAL_ISSUER_NAME } from '../../../vats/src/core/utils.js';

const BASIS_POINTS = 10_000n;

/** @param {BootstrapPowers} p */
const addCollateralType = ({
  brand: {
    consume: { [CENTRAL_ISSUER_NAME]: centralBrandP },
  },
  consume: { vaultFactoryCreatorFacet, board },
}) => {
  const centralBrand = await centralBrandP;
  /** @type {VaultManagerParamValues} */
  const vaultManagerParams = {
    debtLimit: AmountMath.make(centralBrand, 0n),
    liquidationMargin: makeRatio(0n, centralBrand),
    liquidationPenalty: makeRatio(10n, centralBrand),
    interestRate: makeRatio(0n, centralBrand, BASIS_POINTS),
    loanFee: makeRatio(0n, centralBrand, BASIS_POINTS),
  };

  // Prerequisite step: register an issuer on the Board under this name.
  const issuerName = 'newIssuer';
  const issuer = E(board).getValue(issuerName);

  vaultFactoryCreatorFacet.addVaultType(issuer, issuerName, vaultManagerParams);
};

// "export" our behavior by way of the completion value of this script.
addCollateralType;

Before running it, someone should ensure that an AMM pool exists for the collateral type, and the issuer for the type is registered on the board under newIssuer. Or more likely, the script is edited to include the collateral's actual name and BoardID.

I have not tested the script.

@dckc
Copy link
Member

dckc commented Apr 16, 2022

Note that import is a syntax error in evaluated scripts. So things like makeRatio and AmountMath.make have to be copy-pasted in.

In cases where this becomes unmanageable, you can E(zoe).install() a bundle of modules and refer to them from a short script. Stay tuned for details near #5029 , which may become a bit more complicated when installation is permissioned (#4395).

@dckc
Copy link
Member

dckc commented Apr 28, 2022

drafted with @michaelfig : gov-ibc-to-vault.js

  • addAtomishAsset to VBank
  • registerScaledPriceAuthority
  • addAssetToVault

see also #5169 stuff in gov-inviteCommittee.js

  • inviteCommitteeMembers

with @michaelfig , @Chris-Hibbert :

to get past "no collateral types":

  1. some tokens, say, 100_000_000 uphoton, come over IBC (from faucet) to benefactors’ accounts, to

    • establish the IBC denom in the cosmos layer
    • get funds
  2. agd tx gov submit-proposal swingset-core-eval proposal ...

    1. Send out econ committee (Econ committee for Devnet #5169)
      1. voter invitations
      2. poser invitations
      3. Invitation to “agenda” contract (feature: add a contract that supports voting on economic contracts #5245), from which they can get the public API:
        1. per governed contract: Far(‘agendaMakers’, { voteOnApiInvocation, voteOnParamChanges })
    2. E(vaultFactoryCreatorFacet).addVaultType(ibcATOMS.issuer, { debtCeiling: 0, ...})
    3. register scaled price authority from ATOM:USD to ibcATOM:RUN
      1. addBankAsset(denom, ...) <- we can choose decimal places here so that we have, say, 10M "ibc/ATOMs"
    4. addPool(ibc/ATOMs.brand, 'ibcATOM')
  3. benefactors deposit ibcATOM in reserve

  4. Gov cttee moves money from reserve to AMM pool

    E(reserveGovernorCreatorFacet).voteOnApiInvocation(‘addLiquidityToAmmPool’, [ibcAtom(100), run(1000)], binaryVoteCounterInstall, Deadline,)

  5. econ committee raises ibcATOM vault debt ceiling (open for business)

    1. E(vaultFactoryCreatorFacet).voteOnParamChanges({paramPath: { key: { collateralBrand } }, Changes: { DebtLimit: AmountMath.make(RUNBrand, 1000n) }}

@dckc
Copy link
Member

dckc commented May 3, 2022

Waiting for Oracles?

We (@michaelfig @Chris-Hibbert and I) got very close yesterday. The E(bankManager).addAsset() call to create an ERTP issuer out of the interchain IbcATOM denom worked, and the proposal added it to the AMM:

command[13] E(ammAPI).getAllPoolBrands()
history[13] [[Object Alleged: IbcATOM brand]{}]

but when we try to confirm that it's available as a collateral, the promise doesn't resolve:

command[24] E(home.agoricNames).lookup('instance', 'VaultFactory')
history[24] [Object Alleged: InstanceHandle]{}
command[25] E(home.zoe).getPublicFacet(history[24])
history[25] [Object Alleged: VaultDirector public]{}
E(history[25]).getCollaterals()
history[27] unresolved Promise

We suspect the VaultFactory is trying to get a price from the Oracles, and that step is waiting for colleagues in Europe.

@dckc
Copy link
Member

dckc commented May 3, 2022

IbcATOM did get added to VaultFactory

We can getGovernedParams for the relevant collateralBrand:

command[34] history[13][0]
history[34] [Object Alleged: IbcATOM brand]{}
command[36] E(history[25]).getGovernedParams({collateralBrand: history[34]})
history[36] {"DebtLimit":{"type":"amount","value":{"brand":[Object Alleged: RUN brand]{},"value":0n}},"InterestRate":{"type":"ratio","value":{"denominator":{"brand":[Object Alleged: RUN brand]{},"value":100n},"numerator":{"brand":[Object Alleged: RUN brand]{},"value":1n}}},"LiquidationMargin":{"type":"ratio","value":{"denominator":{"brand":[Object Alleged: RUN brand]{},"value":100n},"numerator":{"brand":[Object Alleged: RUN brand]{},"value":1n}}},"LiquidationPenalty":{"type":"ratio","value":{"denominator":{"brand":[Object Alleged: RUN brand]{},"value":100n},"numerator":{"brand":[Object Alleged: RUN brand]{},"value":1n}}},"LoanFee":{"type":"ratio","value":{"denominator":{"brand":[Object Alleged: RUN brand]{},"value":100n},"numerator":{"brand":[Object Alleged: RUN brand]{},"value":1n}}}}

@dckc
Copy link
Member

dckc commented May 6, 2022

voteOnVaultParamChanges failing

same symptoms as with the reserve #5298 (comment) : undefined is a undefined but must be a bigint or a number

https://gist.github.com/dckc/2b0d26f81c8c838d4641ad8edbaff650#file-symptoms-vaults-txt

@dckc
Copy link
Member

dckc commented May 11, 2022

The debt limit is 0 until the gov cttee raises it, but we have a collateral type:

image

@dckc
Copy link
Member

dckc commented May 12, 2022

making progress on raising the debt limit via a crude voting gui...

dckc/gov-demo1#1

@dckc
Copy link
Member

dckc commented May 13, 2022

gak! vote failed: No quorum again, even though both @rowgraus and I voted.

I'm inclined to try the configuration from #5362 to debug this.

Struggling to diagnose

The vote counter stats show 0 votes no matter what I do.

image

repl session:

command[231]
E(econ.pub).getOpenQuestions()
history[231]
[[Object Alleged: QuestionHandle]{}]
command[232]
E(econ.pub).getQuestion(h[231][0])
history[232]
[Object Alleged: question details]{}
command[233]
E(h[232]).sdf()
history[233]
Promise.reject("TypeError: target has no method \"sdf\", has [\"getDetails\",\"getVoteCounter\"]")
command[234]
E(h[232]).getVoteCounter()
history[234]
[Object Alleged: InstanceHandle]{}
command[235]
E(zoe).getInstallation(h[234])
history[235]
Promise.reject("Error: A Zoe invitation is required, not (an object)")
command[236]
E(h[232]).getDetails()
history[236]
{"closingRule":{"deadline":1652381659n,"timer":[Object Alleged: timerService]{}},"counterInstance":[Object Alleged: InstanceHandle]{},"electionType":"param_change","issue":{"contract":[Object Alleged: InstanceHandle]{},"spec":{"changes":{"DebtLimit":{"brand":[Object Alleged: RUN brand]{},"value":10100000000000n}},"paramPath":{"key":{"collateralBrand":[Object Alleged: IbcATOM brand]{}}}}},"maxChoices":1,"method":"unranked","positions":[{"changes":{"DebtLimit":{"brand":[Object Alleged: RUN brand]{},"value":10100000000000n}}},{"noChange":["DebtLimit"]}],"questionHandle":[Object Alleged: QuestionHandle]{},"quorumRule":"majority","tieOutcome":{"noChange":["DebtLimit"]}}
command[237]
E(zoe).getPublicFacet(h[234])
history[237]
[Object Alleged: publicFacet]{}
command[238]
E(h[237]).lkdsjf()
history[238]
Promise.reject("TypeError: target has no method \"lkdsjf\", has [\"getDetails\",\"getInstance\",\"getOutcome\",\"getQuestion\",\"getStats\",\"isOpen\"]")
command[239]
E(h[237]).getStats()
history[239]
{"results":[{"position":{"changes":{"DebtLimit":{"brand":[Object Alleged: RUN brand]{},"value":10100000000000n}}},"total":0n},{"position":{"noChange":["DebtLimit"]},"total":0n}],"spoiled":0n,"votes":0}
command[240]
E(h[237]).getStats()
history[240]
{"results":[{"position":{"changes":{"DebtLimit":{"brand":[Object Alleged: RUN brand]{},"value":10100000000000n}}},"total":0n},{"position":{"noChange":["DebtLimit"]},"total":0n}],"spoiled":0n,"votes":0}
command[241]
E(h[237]).getStats()
history[241]
{"results":[{"position":{"changes":{"DebtLimit":{"brand":[Object Alleged: RUN brand]{},"value":10100000000000n}}},"total":0n},{"position":{"noChange":["DebtLimit"]},"total":0n}],"spoiled":0n,"votes":0}
command[242]
E(h[237]).getStats()
history[242]
{"results":[{"position":{"changes":{"DebtLimit":{"brand":[Object Alleged: RUN brand]{},"value":10100000000000n}}},"total":0n},{"position":{"noChange":["DebtLimit"]},"total":0n}],"spoiled":0n,"votes":0}
command[243]
E(home.myAddressNameAdmin).getMyAddress9)
history[243]
exception: [SyntaxError: missing ;]
command[244]
E(home.myAddressNameAdmin).getMyAddress()
history[244]
"agoric1j5sm4kr2mdae6sfey6qjm9vaamddmp0yuxs6kz"
command[245]
E(h[237]).getStats()
history[245]
{"results":[{"position":{"changes":{"DebtLimit":{"brand":[Object Alleged: RUN brand]{},"value":10100000000000n}}},"total":0n},{"position":{"noChange":["DebtLimit"]},"total":0n}],"spoiled":0n,"votes":0}
command[246]
voterFacet
history[246]
[Object Alleged: voter0]{}
command[247]
E(voterFacet).lsdkjfldskj()
history[247]
Promise.reject("TypeError: target has no method \"lsdkjfldskj\", has [\"castBallotFor\"]")
command[248]
E(econ.pub).getOpenQuestions()
history[248]
[[Object Alleged: QuestionHandle]{}]
command[249]
E(econ.pub).getQuestion(h[248][0])
history[249]
[Object Alleged: question details]{}
command[250]
E(h[249]).getVoteCounter()
history[250]
[Object Alleged: InstanceHandle]{}
command[251]
E(zoe).getPublicFacet(h[250])
history[251]
[Object Alleged: publicFacet]{}
command[252]
E(h[251]).getStats()
history[252]
{"results":[{"position":{"changes":{"DebtLimit":{"brand":[Object Alleged: RUN brand]{},"value":10200000000000n}}},"total":0n},{"position":{"noChange":["DebtLimit"]},"total":0n}],"spoiled":0n,"votes":0}
command[253]
E(h[251]).getStats()
history[253]
{"results":[{"position":{"changes":{"DebtLimit":{"brand":[Object Alleged: RUN brand]{},"value":10200000000000n}}},"total":0n},{"position":{"noChange":["DebtLimit"]},"total":0n}],"spoiled":0n,"votes":0}
command[254]
E(h[251]).getStats()
history[254]
{"results":[{"position":{"changes":{"DebtLimit":{"brand":[Object Alleged: RUN brand]{},"value":10200000000000n}}},"total":0n},{"position":{"noChange":["DebtLimit"]},"total":0n}],"spoiled":0n,"votes":0}

@Tartuffo
Copy link
Contributor

Update - this works in unit testing, but not in devnet / instagoric.

@dckc
Copy link
Member

dckc commented May 16, 2022

but not in devnet / instagoric

We haven't tried it in instagoric; that's up next / soon.

@dckc
Copy link
Member

dckc commented May 21, 2022

@arirubinstein that WIP deploy script for raising the debt limit that I mentioned... I cleaned it up a bit and tested it:

@dckc
Copy link
Member

dckc commented May 27, 2022

Let's reduce the scope of this to be independent of any particular network deployment. gov-test-collateral.js shows that the feature is there.

For instagoric networks with a single-member committee, we have

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AMM Core Economy OBSOLETE in favor of INTER-protocol enhancement New feature or request Governance Governance Inter-protocol Overarching Inter Protocol restival to be done before RUN Protocol Purple Team festival security Vaults VaultFactor (née Treasury)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants