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

feat(vats): facilitate launching additional PSMs #6142

Merged
merged 12 commits into from
Sep 8, 2022
Merged

Conversation

dckc
Copy link
Member

@dckc dckc commented Sep 6, 2022

goal:
closes: #6021

Description

This exposes the startPSM and makeAnchorAsset functions to the core-eval scope; as a result, a proposal to start a PSM for DAI is just a few lines.

TODO:

  • handle naming issues around multiple PSMs

Security Considerations

TODO:

  • refine permit of example proposal
  • set default fee limit to 0 IST

Documentation Considerations

@rowgraus should we put something about this process in some Inter Protocol docs? Or just wait until it's time to discuss an actual proposal with the community?

Testing Considerations

I did local-chain end-to-end integration testing, as noted below

@dckc
Copy link
Member Author

dckc commented Sep 6, 2022

How to introduce additional PSMs to the psmCharter?

@dtribble @Chris-Hibbert bootstrap wires up "the" PSM to psmCharter for governing minting limits and such. When we start another PSM, I just realized we need to introduce it to the psmCharter somehow too.

@dckc dckc marked this pull request as ready for review September 6, 2022 04:03
@dckc dckc requested a review from turadg as a code owner September 6, 2022 04:03
Chris-Hibbert added a commit that referenced this pull request Sep 6, 2022
This helps with #6142, but might be merged into that or #6139.
Chris-Hibbert added a commit that referenced this pull request Sep 7, 2022
This helps with #6142, but might be merged into that or #6139.
Copy link
Member

@dtribble dtribble left a comment

Choose a reason for hiding this comment

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

I have some questions in the comments.

"economicCommittee": true
},
"produce": {
"$ISSUE": "These names assume there can be only one psm",
Copy link
Member

Choose a reason for hiding this comment

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

Are these intended to remain?

Copy link
Member Author

@dckc dckc Sep 7, 2022

Choose a reason for hiding this comment

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

no. hence #6142 (comment) and #6146 and such.

@dckc dckc marked this pull request as draft September 7, 2022 05:43
mergify bot added a commit that referenced this pull request Sep 7, 2022
* refactor: make PSM bootstrap support multiple brands

This helps with #6142, but might be merged into that or #6139.

* chore(inter-protocol): register PSM under anchor keyword

* chore: clean-ups and comments suggested in review.

* build(ERTP): increase test timeout to mitigate CI failure

Co-authored-by: Dan Connolly <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
@Chris-Hibbert
Copy link
Contributor

I think #6146 addressed

  • handle naming issues around multiple PSMs
    Can we check that off in the top-comment, or is there more to be done for naming?

@dckc
Copy link
Member Author

dckc commented Sep 7, 2022

connection to the psmCharter is working, finally:

2022-09-07T16:33:46.716Z block-manager: block 28 begin
2022-09-07T16:33:46.721Z SwingSet: vat: v1: issuer DAI: new Promise
2022-09-07T16:33:46.722Z SwingSet: vat: v1: brand DAI: new Promise
2022-09-07T16:33:46.727Z SwingSet: vat: v1: starting PSM: { keyword: 'DAI', decimalPlaces: 18, denom: 'ibc/toydai', proposedName: 'Maker DAI' }
XS snapshot written to /home/connolly/projects/agoric-sdk/packages/cosmic-swingset/t1/n0/data/ag-cosmos-chain-state/xs-snapshots/47d5177d084b205d875cf75dec79225aa57cf2e61d2584
0f4baa9f1aa4af9da5.gz : 1635995 bytes compressed, 7494503 raw
2022-09-07T16:33:47.902Z block-manager: block 28 commit
2022-09-07T16:33:51.736Z block-manager: block 29 begin
2022-09-07T16:33:51.897Z block-manager: block 29 commit
2022-09-07T16:33:56.749Z block-manager: block 30 begin
XS snapshot written to /home/connolly/projects/agoric-sdk/packages/cosmic-swingset/t1/n0/data/ag-cosmos-chain-state/xs-snapshots/0a106938f822316b12246b919fa522475b154b526938f5
1914863e2f0475b33f.gz : 1555397 bytes compressed, 7046479 raw
2022-09-07T16:33:58.057Z block-manager: block 30 commit
2022-09-07T16:34:01.759Z block-manager: block 31 begin
2022-09-07T16:34:02.874Z SwingSet: vat: v20: PSM Starting Object [Alleged: DAI brand] {} { denominator: { brand: Object [Alleged: IST brand] {}, value: 100n }, numerator: { br
and: Object [Alleged: DAI brand] {}, value: 100n } }
XS snapshot written to /home/connolly/projects/agoric-sdk/packages/cosmic-swingset/t1/n0/data/ag-cosmos-chain-state/xs-snapshots/75d15629896ebebc681d8bf282946327b7749da2578774
1ea2e72abdd9d22f10.gz : 1690238 bytes compressed, 7657983 raw
2022-09-07T16:34:03.158Z block-manager: block 31 commit
2022-09-07T16:34:06.772Z block-manager: block 32 begin
2022-09-07T16:34:06.868Z block-manager: block 32 commit
2022-09-07T16:34:11.783Z block-manager: block 33 begin
2022-09-07T16:34:11.922Z block-manager: block 33 commit
2022-09-07T16:34:16.798Z block-manager: block 34 begin
2022-09-07T16:34:16.970Z block-manager: block 34 commit
2022-09-07T16:34:21.815Z block-manager: block 35 begin
2022-09-07T16:34:21.927Z SwingSet: vat: v11: psmCharter: adding instance { minted: Object [Alleged: IST brand] {}, anchor: Object [Alleged: DAI brand] {} }
2022-09-07T16:34:21.937Z SwingSet: vat: v1: started PSM: { options: { anchorOptions: { keyword: 'DAI', decimalPlaces: 18, denom: 'ibc/toydai', proposedName: 'Maker DAI' } }, W
antStableFeeBP: 1n, GiveStableFeeBP: 3n, MINT_LIMIT: 0n }
2022-09-07T16:34:21.964Z block-manager: block 35 commit

@dckc dckc marked this pull request as ready for review September 7, 2022 16:48
@dckc
Copy link
Member Author

dckc commented Sep 7, 2022

@Chris-Hibbert and co, PTAL.

I think #6146 addressed "handle naming issues around multiple PSMs"
Can we check that off in the top-comment, or is there more to be done for naming?

There was some wiring up the psmCharter, but it's done now and checked off.

@dckc
Copy link
Member Author

dckc commented Sep 7, 2022

re docs: I can imagine the chainStorage and agoricNames stuff should be documented.

for now:

$ agoric follow :published.agoricNames.instance
[
  [
    "economicCommittee",
    slot(0,"Alleged: InstanceHandle"),
  ],
  [
    "psm-IST-AUSD",
    slot(1,"Alleged: InstanceHandle"),
  ],
  [
    "psm-IST-DAI",
    slot(2,"Alleged: InstanceHandle"),
  ],
]
$ agd query vstorage children published.psm.IST
children:
- AUSD
- DAI

@@ -46,7 +49,9 @@ export const start = async (zcf, privateArgs) => {
);
};

const makeOfferFilterInvitation = () => {
/** @param {Instance} instance */
Copy link
Member Author

Choose a reason for hiding this comment

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

@turadg I'm interested to get some training on using Instance types parameterized by the contract start function and such.

Copy link
Member

Choose a reason for hiding this comment

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

@dckc
Copy link
Member Author

dckc commented Sep 7, 2022

oops! I force-pushed after requesting review. I hope I didn't make too much of a mess.

Copy link
Contributor

@Chris-Hibbert Chris-Hibbert left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +11 to +12
import * as startPSMmod from '@agoric/inter-protocol/src/proposals/startPSM.js';
import * as ERTPmod from '@agoric/ertp';
Copy link
Contributor

Choose a reason for hiding this comment

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

Capitalization nit: shouldn't these be startPSMMod and ERTPMod?

Copy link
Member Author

Choose a reason for hiding this comment

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

could be. if this version doesn't pass CI, I'll tweak it.

 - provide startPSM module to core-eval
 - support proposedName, decimalPlaces in anchorOptions
   - check shape of anchorOptions
... in order to avoid importing sim-behaviors.js into boot-psm.js
@dckc dckc added the automerge:no-update (expert!) Automatically merge without updates label Sep 8, 2022
@mergify mergify bot merged commit bd7ae9c into master Sep 8, 2022
@mergify mergify bot deleted the 6021-more-psm-assets branch September 8, 2022 01:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:no-update (expert!) Automatically merge without updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BLDer DAO can start PSMs for more reference stable interchain assets
4 participants