core bootstrap: subsume non-core #4541

dckc · 2022-02-12T04:53:30Z

I (sort of) recommend reviewing the commits separately. In particular, the run-protocol commits are separate from `vats and such. And demo stuff is in a separate commit from production stuff. This strategy fell down a bit with the move from the nameAdmins store to using promise spaces for issuers, brands, etc.

Security Considerations

completes the work of separating demo / test materials from production bootstrap begun in #4309, #4437

Documentation Considerations

and enumeration of what's in home is in here somewhere. (see also #4347)

Testing Considerations

unit test coverage is better but still not good. I'm still struggling to figure out what features to check for regressions.

production:

start VaultFactory, AMM (Bootstrap core: AMM, VaultFactory bootstrap with governance committee #4437 added this in in packages/run-protocol)
test-home.js in packages/solo tests wallet, board, etc. (pre-dates this PR)
- client (solo) bootstrap
- buildZoe
makeFeeCollector (unit tests, separate from bootstrap)
networking, IBC (unit tests, separate from bootstrap)
static checks on initial conditions
- manifest permits are adequate for each bootstrap action
- exactly one Producer for every item consumed.

demo/test:

faucet: demo data into wallet purses (faucet)
demo data for AMM, and VaultFactory
- demo AMM calcluations
- fundAMM unit test - not cost-effective?

Postponed to separate issues:

turadg

I took a pass with some comments and questions. They're mostly superficial as I'm not well acquainted with the bootstrap.

Besides those, nothing stands out to me as problematic but I won't feel comfortable giving an Approval without a walk-through. I'm leaving a Comment review and hoping one of the other reviewers can provide assess whether it's merge-ready.

turadg · 2022-02-15T22:07:07Z

packages/run-protocol/src/econ-behaviors.js

+    epochInterval: 60n * 60n, // 1 hour
+  };
+  const [centralIssuer, centralBrand] = await Promise.all([
+    E(agoricNames).lookup('issuer', 'RUN'), // TODO: constant for RUN


is this to do before merging or in the future? If the latter, it's best to have a ticket or a comment about who will do it.

Good question. It's at least intended to start a conversation with reviewers.

I think last time this came up we discussed a set of "well known names". "RUN" would be one of them. Seems like it wouldn't hurt to have a constant file now for what would eventually be well-known names, as long as we understand what the semantics are of a name being well-known. Though I can't say that I'm certain.

Yesterday I decided that both constants and static types were worth doing, as part of integrating agoricNames with the permits system:

agoric-sdk/packages/vats/src/core/utils.js

Lines 18 to 22 in 1a8fab6

// We reserve these keys in name hubs.

export const agoricNamesReserved = harden({

issuer: {

BLD: 'Agoric staking token',

RUN: 'Agoric RUN currency',

agoric-sdk/packages/vats/src/core/types.js

Lines 119 to 122 in 1a8fab6

/**

* @typedef {{

* issuer: |

* 'RUN' | 'BLD',

...

agoric-sdk/packages/vats/src/core/types.js

Lines 136 to 143 in 1a8fab6

* }} WellKnownName

*

* @typedef {{

* issuer: {

* nameHub: NameHub, nameAdmin: NameAdmin,

* produce: Record<WellKnownName['issuer'], Producer<Issuer>>,

* consume: Record<WellKnownName['issuer'], Promise<Issuer>>,

* },

turadg · 2022-02-15T22:08:59Z

packages/run-protocol/src/econ-behaviors.js

+      epochTimerService,
+      harden(distributorParams),
+    )
+    .catch(e => console.error('Error building fee distributor', e));


why is this better than letting the error propagate?

I'm not sure; I copied / moved this code from elsewhere. @Chris-Hibbert ? @michaelfig ?

I think this catch was done before we "only distribute fees if there is a collector". You can remove it with impunity.

turadg · 2022-02-15T22:11:19Z

packages/run-protocol/src/collect.js

-  allValues: async obj =>
-    fromEntries(Collect.zip(keys(obj), await Promise.all(values(obj)))),
-};
+/** @type { <T, U>(obj: Record<string, T>, f: (t: T) => U) => Record<string, U>} */


👌 I'm a fan of using as little JSDoc as necessary.

Also the named exports!

turadg · 2022-02-15T22:30:11Z

packages/vats/src/core/types.js

 */

 /**
 * @callback CreateUserBundle
 * @param {string} nickname
 * @param {string} clientAddress
 * @param {string[]} powerFlags
- * @returns {Promise<Record<string, unknown>>}
+ * @returns {Promise<Record<string, Promise<any>>>}


This looks like a regression in type safety. How hard would it be to keep the unknown instead of any?

this justification belongs in the code...

The userBundle really is an arbitrary pile of properties. And it's not like you can check them with typeof. You are either in a position to trust them or you're not. With unknown, each client that wants to call a method such as E(userBundle.bank).deposit(payment) has to cast userBundle.bank; ideally, the cast is to some useful type. But unknown can't be cast directly to some other type; it has to be cast to any first. I'm not sure all that hassle is cost-effective.

turadg · 2022-02-15T22:30:35Z

packages/vats/src/core/types.js

+ *     ROLE: string,
+ *     hardcodedClientAddresses: string[],
+ *     noFakeCurrencies: boolean,
+ *     FIXME_GCI: string,


is this FIXME a merge blocker?

it hasn't been for some number of years :)
again, this code is copied / moved from bootstrap.js.

dckc · 2022-02-16T04:08:05Z

I just added several commits based on testing in the context of work on getRUN bootstrap for a demo. Some of these could perhaps go in separate PRs; for example:

fix(vats): reserve: is noop if key already exists (NEEDSTEST)

packages/pegasus/src/install-on-chain.js

Chris-Hibbert

Just a bunch of observations. I don't have any objection to merging these changes in.

I don't feel like I've reviewed all the changes sufficiently to hit approve.

packages/run-protocol/src/econ-behaviors.js

packages/vats/src/core/basic-behaviors.js

Chris-Hibbert · 2022-02-16T19:18:53Z

packages/vats/src/core/client-behaviors.js

+      vatPowers,
+    );
+
+    // TODO: Remove this alias when we can.


Can you add an indication of the time frame or blocking factors?

That comment is unchanged from bootstrap.js. But it's a good question..

Maybe now is the time to remove home.uploads? @michaelfig ?

Maybe now is the time to remove home.uploads?

I think so. We'd need to audit the dapps, but I'm pretty sure it's okay, and we can fix them if they break. But maybe in a follow-on PR.

Chris-Hibbert · 2022-02-16T19:41:25Z

packages/vats/src/core/chain-behaviors.js

+  { template } = {
+    template: {
+      agoricNames: true,
+      bank: true,
+      namesByAddress: true,
+      myAddressNameAdmin: true,
+      board: true,
+      faucet: true,
+      zoe: true,
+    },
+  },


Why is this better than

Suggested change

{ template } = {

template: {

agoricNames: true,

bank: true,

namesByAddress: true,

myAddressNameAdmin: true,

board: true,

faucet: true,

zoe: true,

},

},

template = {

agoricNames: true,

bank: true,

namesByAddress: true,

myAddressNameAdmin: true,

board: true,

faucet: true,

zoe: true,

},

This justification belongs in the code somewhere.

The idea is that the 2nd argument for all boostrap behaviors (functions) is an optional "config" argument that can be set in the bootstrap config file. So template isn't the whole config; it's one option in the config for this behavior.

packages/vats/src/core/chain-behaviors.js

- consume pegasusBundle from the bootstrap promise space. - no need to exclude install-on-chain.js from lint, so move to src/ - there's only one bundle, so never mind the Promise.all loop. - take care not to give Zoe.install a Promise<bundle>; that fails with: unrecognized moduleFormat ' undefined ' - export CONTRACT_NAME = 'Pegasus' (capital P) - since we get nameAdmins from the promise space, use eventual send to call it. - never mind creatorFacet; Pegasus doesn't have one. - prune unused uiConfig - prune import of BootstrapPowers type ran into trouble with ambient types

closes #4021 BREAKING CHANGE: removes getBootstrapPayment from VaultFactory

In order to test the new bootstrap with older versions of the UI, publish the VaultFactory instance and uiConfig under 'Treasury' as well as 'VaultFactory'. - combine awaits for vaultFactoryInstance, vaultFactoryCreator - we share vaultFactory instance in startVaultFactory; no need to do it again in configureVaultFactoryUI - docs: makeCollectFeesInvitation type conflicted with something

- basic-behaviors: - mintInitialSupply, w/unit test - factor out BLD, RUN info as Tokens - example bootMsg for type - addBankAssets, w/unit test - publish runIssuer, runBrand here rather than buildZoe - subsume makeBLDKit - produce namesByAddress, namesByAddressAdmin - makeClientBanks: add home.bank - chain-behaviors: - makeClientManager: delay resolution of chainBundle until expected contents are available - TECHDEBT: factor out expected contents for use in solo's test-home.js - support multiple properties in E(client).assignBundle() - shareEconomyBundles: no getRUNBundle yet - registerNetworkProtocols: IBC, ... - update to use makePegasusConnectionKit - update Collect usage - integrate installPegasusOnChain behavior - manifest: installPegasusOnChain permit - share pegasusBundle from shareBootContractBundles - reserve Pegasus instance in nameHub - client-behaviors: startClient - parameterize logging in makePromiseSpace() - manifest: - track containing vat - TECHDEBT: not enforced as part of permit - track produce / consume for home: installation, instance, issuer, brand - parameterize governance actions by ROLE - core/types.js - factor BootstrapSpace, BootDevices<T> out of BootstrapPowers - chase down lots of vat, device types - SwingsetDevices was a misnomer; devices are chosen by the host application - resolve Device type conflicts

- grantRunBehaviors - test connectFaucet - don't bother with ATOM, moola, simolean faucet payments - coordinate faucet, amm using provideIssuerKit in vat-mints.js - factor out fundAMM calculations for testing - demoIssuers: export utilities for testing - update Collect usage

- don't try to startRewardDistributor on sim-chain

- update CHAIN_BOOTSTRAP_VAT_CONFIG docs in env.md

fixes #4165 BREAKING CHANGE: decentral-config.json config file is no longer available. Use decentral-core-config.js, which starts core services by not RUN protocol etc., or decentral-demo-config.js, which does start the RUN protocol and provides demo funds in wallets, AMM.

Separate demo/test bootstrap from production. BREAKING CHANGE: RUN protocol etc. is not started by default. To start RUN protocol, Pegasus, etc., use: CHAIN_BOOTSTRAP_VAT_CONFIG=@agoric/vats/decentral-demo-config.json \ agoric start local-chain Stay tuned for a mechanism to turn them on via governance.

- shared: factor out duplication

This gives senders (such as a committee electorate sending out voter invitations) a promise for the depositFacet rather than having their lookup fail.

IOU test case

... zoe invitations, in particular.

For static typing and integrating with the bootstrap permit system, we expose { produce, consume } spaces rather than NameAdmins. The idiom `E(agoricNames).lookup('instance', 'economicCommittee')` becomes `instance.consume.economicCommittee`, replacing magic strings with statically checked names. Likewise, the idiom `E(instanceAdmin).update('economicCommittee', instance)` becomes `economicCommittee.resolve(instance)`. chore(vats): avoid magic strings for agoricNames (SQUASHME prod) - manifest: adapt permits from nameHubs to instance.produce and such - configureVaultFactoryUI: no need to produce amm instance; it's done in setupAMM - buildZoe doesn't produce the RUN issuer, brand; no need for agoricNames, nameAdmins, which are now obsolete in any case - test-boot.js to test manifest.js without starting the whole chain: test all 3 roles, with and without post-boot governance actions - makeAddressNameHubs: consume agoricNames, which is produced in boot.js - addBankVats no longer uses bootMsg - no more collectNameAdmins, rarely need to consume agoricNames - makeAgoricNames -> makeAgoricNamesAccess, which connects the promise spaces with the nameAdmins. - chore(pegasus): nameAdmins -> spaces - adapt startRewardDistributor to promise spaces for agoricNames - boot.js: parameterize logging higher - avoid harden() on module object - provide better diagnostic for bad behavior name - produce home.chainTimerService - docs: build scripts for visualizing initial conditions (SQUASHME prod) - vats: genesis typo - style(vats): mgr -> bankMgr - style(run-protocol): vats.distributeFees - makePriceAuthority -> makePriceAuthorityRegistry it's making more than a price authority - docs(vats): cite BLD issuer vat TODO issue - avoid hardening `deprecated` (SQUASHME prod) In a test, some calls that usually cross vats do not, so args get hardened. Since this deprecated thing should go away, let's copy it to make building the test easier. - test-clientBundle for spaces - fix(vats): fix home.chainTimerService - chore(run-protocol): fee distributor etc. - liquidate installation in configureVaultFactoryUI - bundles check is subsumed by permits - obsolete fee distributor .catch() - style(run-protocol): reformat in econ-behaviors - docs(vats): misc. - justify use of any in getChainBundle - TODO: shut down centralSupply contract? - NameHub, NameAdmin are not exposed in WellKnownSpaces Co-authored-by: Chris Hibbert <Chris-Hibbert@users.noreply.github.com>

dckc force-pushed the bootstrap-core branch 10 times, most recently from cdccd78 to 90e8986 Compare February 14, 2022 00:03

dckc marked this pull request as ready for review February 14, 2022 00:32

dckc requested review from michaelfig, Chris-Hibbert and turadg and removed request for michaelfig February 14, 2022 00:32

dckc self-assigned this Feb 14, 2022

turadg reviewed Feb 15, 2022

View reviewed changes

Chris-Hibbert reviewed Feb 16, 2022

View reviewed changes

packages/pegasus/src/install-on-chain.js Outdated Show resolved Hide resolved

dckc changed the title ~~core bootstrap: subsume non-core (WIP)~~ core bootstrap: subsume non-core Feb 16, 2022

Chris-Hibbert reviewed Feb 16, 2022

View reviewed changes

dckc force-pushed the bootstrap-core branch 6 times, most recently from c465d64 to 1a8fab6 Compare February 18, 2022 18:30

dckc added 2 commits February 21, 2022 16:54

chore(run-protocol)!: centralSupply contract for bootstrapPayment

e526a7d

closes #4021 BREAKING CHANGE: removes getBootstrapPayment from VaultFactory

dckc added 16 commits February 21, 2022 19:21

fix(run-protocol): harden results from collection utilities

5d8b4c1

test(solo): decouple CENTRAL_ISSUER_NAME from vats package

02e03dd

build(vats): add @agoric/nat for bootstrapPaymentValue

e11a79e

chore(vats): disentangle chain from sim-chain bootstrap

0a4617b

- don't try to startRewardDistributor on sim-chain

chore(solo): switch bootstrap vat to core/boot.js

2dfe299

chore(cosmic-swingset): use decentral-demo-config.json for sim-chain

7c6e65f

- update CHAIN_BOOTSTRAP_VAT_CONFIG docs in env.md

fix(vats): reserve Treasury uiConfig for compatibility

943fba5

- shared: factor out duplication

feat(vats): reserve depositFacet for each client

806ed5a

This gives senders (such as a committee electorate sending out voter invitations) a promise for the depositFacet rather than having their lookup fail.

fix(vats): don't dup / clobber client/home properties

9ddf290

fix(vats): reserve: is noop if key already exists (NEEDSTEST)

668ef1e

IOU test case

fix(wallet): don't publish depositFacet until we're ready to receive

f448176

... zoe invitations, in particular.

dckc force-pushed the bootstrap-core branch from e08a697 to 15bbf78 Compare February 22, 2022 01:29

dckc added the automerge:no-update (expert!) Automatically merge without updates label Feb 22, 2022

dckc force-pushed the bootstrap-core branch from 15bbf78 to 1b26931 Compare February 22, 2022 01:48

dckc and others added 4 commits February 21, 2022 20:22

build: add yarn.lock entry for @endo/nat@4.1.4

aabf0a6

build(vats): promise-kit moved @Agoric -> @Endo

786c7cc

style(vats): lint

faf037d

dckc force-pushed the bootstrap-core branch from ed963d9 to faf037d Compare February 22, 2022 02:22

Merge branch 'master' into bootstrap-core

008ce3b

mergify bot merged commit 651a96e into master Feb 22, 2022

mergify bot deleted the bootstrap-core branch February 22, 2022 03:02

dckc mentioned this pull request Feb 22, 2022

production bootstrap does not provide trading assets needed by loadgen #4634

Closed

mhofman mentioned this pull request Feb 26, 2022

Perform loadgen tasks with installed asset Agoric/testnet-load-generator#69

Closed

mhofman mentioned this pull request Mar 21, 2022

Rewrite loadgen to use a dedicated asset Agoric/testnet-load-generator#71

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

core bootstrap: subsume non-core #4541

core bootstrap: subsume non-core #4541

dckc commented Feb 12, 2022 •

edited

Loading

turadg left a comment

turadg Feb 15, 2022

dckc Feb 15, 2022

turadg Feb 15, 2022

dckc Feb 18, 2022

turadg Feb 15, 2022

dckc Feb 15, 2022 •

edited

Loading

michaelfig Feb 21, 2022

turadg Feb 15, 2022

turadg Feb 15, 2022

turadg Feb 15, 2022

dckc Feb 15, 2022

turadg Feb 15, 2022

dckc Feb 15, 2022

dckc commented Feb 16, 2022

Chris-Hibbert left a comment

Chris-Hibbert Feb 16, 2022

dckc Feb 17, 2022

michaelfig Feb 21, 2022

Chris-Hibbert Feb 16, 2022

dckc Feb 17, 2022

	// We reserve these keys in name hubs.
	export const agoricNamesReserved = harden({
	issuer: {
	BLD: 'Agoric staking token',
	RUN: 'Agoric RUN currency',

	* }} WellKnownName
	*
	* @typedef {{
	* issuer: {
	* nameHub: NameHub, nameAdmin: NameAdmin,
	* produce: Record<WellKnownName['issuer'], Producer<Issuer>>,
	* consume: Record<WellKnownName['issuer'], Promise<Issuer>>,
	* },

core bootstrap: subsume non-core #4541

core bootstrap: subsume non-core #4541

Conversation

dckc commented Feb 12, 2022 • edited Loading

Security Considerations

Documentation Considerations

Testing Considerations

turadg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dckc Feb 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dckc commented Feb 16, 2022

Chris-Hibbert left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dckc commented Feb 12, 2022 •

edited

Loading

dckc Feb 15, 2022 •

edited

Loading