6880 bootstrap integration test of vaultFactory with smart wallet #7021

turadg · 2023-02-17T00:29:16Z

closes: #6880

Description

Tests Vault Factory integration with Smart Wallet, using a new kind of Swingset test.

vs contract tests:

integrates the full bootstrap so there's no config maintenance
runs on top of the same decentral-test-vaults-config.json as testnet
takes ~1m to run

vs chain tests:

fakes the Cosmos bridge
uses local instead of xs-worker so it's faster (e.g. xs about 50% slower than Node in local dev) and enables use of the Node debugger
can use Ava test runner and assertions

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

turadg · 2023-02-24T23:23:49Z

packages/SwingSet/tools/bundleTool.js

   * @param {(message: *) => void} [log]
   */
  const load = async (rootPath, targetName, log = console.debug) => {
+    targetName ||= pathAmbient.basename(rootPath, '.js');


the ||= is why this PR is stacked on #7073

@dckc I know you'll have thoughts about pathAmbient. It was a hassle to wire it through the functions and they don't have types so that would be lost. I figure the module is like a closure in which an authority is available so this is not a violation of ocap.

I figure at some point we'll have things like Node's path not importable. Until then this feels pragmatic.

It was a hassle to wire it through the functions ...

Adding basename to readPowers isn't straightforward?

I figure the module is like a closure in which an authority is available so this is not a violation of ocap.

Hm. Exporting functions that close over ambient authority such as path.basename is one of the few crisp prohibitions we have managed to articulate (or... I thought we had managed...):

No modules should export powerful objects (for example, objects that close over fs or process.env).
-- OCap Discipline

Adding basename to readPowers isn't straightforward?

Adding it to makeReadPowers isn't because that's in Endo. This works,

const readPowers = { ...makeReadPowers({ fs, url, crypto }), basename: fs.basename, };

But it's confusing the makeReadPowers doesn't make readPowers. And the type information is lost. The net cost of repeating every fs function that we need strikes me as greater than its benefit.

No modules should export powerful objects (for example, objects that close over fs or process.env).

I don't think we fully follow that edict. A search of process.env shows 77 results in 40 files. Not all are functions closing over it but I see 10-20 that are.

I'll remove pathAmbient to get this PR in but I think the ocap coding guidelines could use some refinement.

No modules should export powerful objects (for example, objects that close over fs or process.env).

I don't think we fully follow that edict.

Indeed; #2160 represents the technical-debt to clean that up.

And the type information is lost.

That seems straightforward to address.

The net cost of repeating every fs function that we need strikes me as greater than its benefit.

It is unfortunate that node's fs API is awkward w.r.t. the principle of least authority. I hope we can use something shaped like this more widely...

agoric-sdk/packages/SwingSet/tools/bundleTool.js

Lines 9 to 31 in 010105a

export const makeFileReader = (fileName, { fs, path }) => {

const make = there => makeFileReader(there, { fs, path });

return harden({

toString: () => fileName,

readText: () => fs.promises.readFile(fileName, 'utf-8'),

neighbor: ref => make(path.resolve(fileName, ref)),

stat: () => fs.promises.stat(fileName),

absolute: () => path.normalize(fileName),

relative: there => path.relative(fileName, there),

exists: () => fs.existsSync(fileName),

});

};

export const makeFileWriter = (fileName, { fs, path }) => {

const make = there => makeFileWriter(there, { fs, path });

return harden({

toString: () => fileName,

writeText: txt => fs.promises.writeFile(fileName, txt),

readOnly: () => makeFileReader(fileName, { fs, path }),

neighbor: ref => make(path.resolve(fileName, ref)),

mkdir: opts => fs.promises.mkdir(fileName, opts),

});

};

Adding it to makeReadPowers isn't because that's in Endo.

Ah... I missed that point earlier. Now I appreciate why it's awkward.

Here's hoping I can sync up with endo on POLA-shaped IO soonish. Thanks for ef90a4f meanwhile.

packages/SwingSet/tools/bundleTool.js

packages/SwingSet/src/controller/initializeSwingset.js

turadg · 2023-02-25T17:12:09Z

Since these integrate bootstrap I'm wondering if the tests should be in 'packages/vats' instead. At least the supports module.

In some way "vats" is the product and these are functional tests of user flows so it makes sense for them to be there.

packages/SwingSet/src/controller/initializeSwingset.js

dckc

very nice!

some stuff to consider, but nothing critical

dckc · 2023-02-27T21:36:27Z

packages/SwingSet/src/controller/initializeSwingset.js

-
-  async function getBundle(desc, mode, nameToBundle) {
-    if (mode === 'bundle') {
+  const bundleCache = await (config.bundleCachePath


packages/SwingSet/src/controller/initializeSwingset.js

dckc · 2023-02-27T21:42:43Z

packages/SwingSet/src/devices/mailbox/mailbox.js

+/**
+ *
+ * @param {Map<*, *>} state
+ */


Suggested change

/**

*

* @param {Map<*, *>} state

*/

/** @param {Map<*, *>} state */

is that type not inferred?

good point, this doesn't add anything. removed.

dckc · 2023-02-27T21:44:24Z

packages/SwingSet/src/lib/kmarshal.js

@@ -71,6 +86,10 @@ const kmarshal = makeMarshal(krefOf, kslot, {

 export const kser = value => kmarshal.serialize(harden(value));

+/**
+ * @param {import('@endo/marshal').CapData<any>} serializedValue


I think kernel slots are strings...

Suggested change

* @param {import('@endo/marshal').CapData<any>} serializedValue

* @param {import('@endo/marshal').CapData<string>} serializedValue

dckc · 2023-02-27T21:50:19Z

packages/SwingSet/test/bootstrap-relay.js

 import { buildManualTimer } from '../tools/manual-timer.js';
+import { makePassableEncoding } from '../tools/passableEncoding.js';


something to consider for this file:

agoric-sdk/packages/vats/test/upgrading/test-upgrade-vats.js

Lines 66 to 94 in c0af2a6

const relayRoot = new Proxy(

{},

{

get:

(_t, prop, _rx) =>

(...args) =>

run(prop, args),

},

);

const EV = name =>

new Proxy(

{},

{

get:

(_t, prop, _rx) =>

(...args) =>

messageVat(name, prop, args),

},

);

const EP = presence =>

new Proxy(

{},

{

get:

(_t, prop, _rx) =>

(...args) =>

messageObject(presence, prop, args),

},

);

nifty! TALA 38cd282

@gibson042 check this out ^

packages/vats/test/bootstrapTests/supports.js

dckc · 2023-02-27T22:45:44Z

packages/vats/test/bootstrapTests/test-vaults-integration.js

+  const wd = await factoryDriver.provideSmartWallet('agoric1open');
+
+  const { agoricNamesRemotes } = t.context;
+  await wd.executeOffer({


static typechecking this big offer expression would be nifty

it checks the types of the properties, but for some reason it doesn't check the top level properties. gonna punt on that.

packages/vats/test/bootstrapTests/test-vaults-integration.js

dckc · 2023-02-27T22:51:38Z

packages/vats/test/bootstrapTests/test-vaults-integration.js

+    status: { id: 'open-vault', numWantsSatisfied: 1 },
+  });
+
+  // try giving more than is available in the purse/vbank


dckc · 2023-02-27T22:55:15Z

packages/vats/test/snapshots/test-board-utils.js.md

+
+Generated by [AVA](https://avajs.dev).
+
+## makeAgoricNamesRemotesFromFakeStorage


It would be great to promote this to https://docs.agoric.com/

cut build time from 60s to 10s on my machine

Co-Authored-By: Dan Connolly <dckc@madmode.com>

mhofman · 2023-02-28T21:55:39Z

packages/cosmic-swingset/src/launch-chain.js

-async function buildSwingset(
+/**
+ * @param {Map<*, *>} mailboxStorage
+ * @param {*} bridgeOutbound
+ * @param {SwingStoreKernelStorage} kernelStorage
+ * @param {string} vatconfig absolute path
+ * @param {Record<string, any>} argv XXX argv should be an array but it's being called with object
+ * @param {{ ROLE: string }} env
+ * @param {*} options
+ */
+export async function buildSwingset(


I really wasn't expecting this to be exported, and to stay an internal concern.

I'm not clear on what the intent of this comment is. Do you want it to be factored differently? If so, how?

I just have 3 pending PRs that are now failing/conflicting because of this change. I'm not sure if there is a better way to factor this, haven't looked closely yet. It's more of a process question, I wish I had known about this change coming.

mhofman · 2023-02-28T21:58:04Z

fakes the Cosmos bridge

I hope this will be compatible with the bridge changes in #6741

Edit: confirmed to be compatible!

turadg force-pushed the 6880-vaultFactory-wallets branch 5 times, most recently from 18998f8 to 137a8af Compare February 24, 2023 23:19

turadg changed the base branch from master to ta/drop-node-14 February 24, 2023 23:20

turadg changed the title ~~6880 test integration of vaultFactory with smart wallet~~ 6880 bootstrap integration test of vaultFactory with smart wallet Feb 24, 2023

turadg requested review from warner and dckc February 24, 2023 23:21

turadg marked this pull request as ready for review February 24, 2023 23:21

turadg commented Feb 24, 2023

View reviewed changes

turadg requested a review from gibson042 February 24, 2023 23:24

Base automatically changed from ta/drop-node-14 to master February 24, 2023 23:38

dckc reviewed Feb 24, 2023

View reviewed changes

packages/SwingSet/tools/bundleTool.js Outdated Show resolved Hide resolved

dckc reviewed Feb 25, 2023

View reviewed changes

packages/SwingSet/src/controller/initializeSwingset.js Outdated Show resolved Hide resolved

turadg force-pushed the 6880-vaultFactory-wallets branch from 137a8af to ef90a4f Compare February 25, 2023 00:51

ivanlei reviewed Feb 26, 2023

View reviewed changes

packages/SwingSet/src/controller/initializeSwingset.js Outdated Show resolved Hide resolved

turadg added 6 commits February 27, 2023 14:06

refactor: Fail assertion style

bdb3b3f

refactor: extract makePassableEncoding

0b6cca8

feat: export buildSwingset

28eae2b

feat: testing supports in bootstrap

197a6e3

refactor: VBankAccount constants

cba34ec

chore(bundleTool): ocap discipline

d376d0f

dckc approved these changes Feb 27, 2023

View reviewed changes

turadg added 5 commits February 27, 2023 15:14

feat: board-utils

4f80ad3

chore(types): ts-check launch-chain

eaa090a

chore: debugging and docs

e69a5f4

chore: log more

09f5b30

chore(types): ConfigProposal

86e4d32

turadg and others added 4 commits February 27, 2023 15:14

test: expose toStorage() from FakeStorageKit

665d6de

test: test vaults integration with bootstrap

5ed1dda

feat: cache bundles in initializeSwingset

6e57171

cut build time from 60s to 10s on my machine

test(bootstrap): proxies

38cd282

Co-Authored-By: Dan Connolly <dckc@madmode.com>

turadg force-pushed the 6880-vaultFactory-wallets branch from 5ebd303 to 38cd282 Compare February 28, 2023 00:05

turadg added the automerge:no-update (expert!) Automatically merge without updates label Feb 28, 2023

mergify bot merged commit f8ca721 into master Feb 28, 2023

mergify bot deleted the 6880-vaultFactory-wallets branch February 28, 2023 00:56

turadg mentioned this pull request Feb 28, 2023

clientSupport for CLI and bootstrap tests #7091

Merged

mhofman reviewed Feb 28, 2023

View reviewed changes

arirubinstein mentioned this pull request Apr 4, 2023

Add Vaults Deployment Test for vaults #6955

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

6880 bootstrap integration test of vaultFactory with smart wallet #7021

6880 bootstrap integration test of vaultFactory with smart wallet #7021

turadg commented Feb 17, 2023 •

edited

Loading

turadg Feb 24, 2023

dckc Feb 24, 2023

turadg Feb 25, 2023 •

edited

Loading

dckc Feb 25, 2023

dckc Feb 25, 2023

turadg commented Feb 25, 2023

dckc left a comment

dckc Feb 27, 2023

dckc Feb 27, 2023

turadg Feb 27, 2023

dckc Feb 27, 2023

dckc Feb 27, 2023

turadg Feb 28, 2023

dckc Feb 28, 2023

dckc Feb 27, 2023

turadg Feb 27, 2023

dckc Feb 27, 2023

dckc Feb 27, 2023

mhofman Feb 28, 2023

turadg Feb 28, 2023

mhofman Feb 28, 2023

mhofman commented Feb 28, 2023 •

edited

Loading

	export const makeFileReader = (fileName, { fs, path }) => {
	const make = there => makeFileReader(there, { fs, path });
	return harden({
	toString: () => fileName,
	readText: () => fs.promises.readFile(fileName, 'utf-8'),
	neighbor: ref => make(path.resolve(fileName, ref)),
	stat: () => fs.promises.stat(fileName),
	absolute: () => path.normalize(fileName),
	relative: there => path.relative(fileName, there),
	exists: () => fs.existsSync(fileName),
	});
	};

	export const makeFileWriter = (fileName, { fs, path }) => {
	const make = there => makeFileWriter(there, { fs, path });
	return harden({
	toString: () => fileName,
	writeText: txt => fs.promises.writeFile(fileName, txt),
	readOnly: () => makeFileReader(fileName, { fs, path }),
	neighbor: ref => make(path.resolve(fileName, ref)),
	mkdir: opts => fs.promises.mkdir(fileName, opts),
	});
	};

	* @param {import('@endo/marshal').CapData<any>} serializedValue
	* @param {import('@endo/marshal').CapData<string>} serializedValue

		import { buildManualTimer } from '../tools/manual-timer.js';
		import { makePassableEncoding } from '../tools/passableEncoding.js';

	const relayRoot = new Proxy(
	{},
	{
	get:
	(_t, prop, _rx) =>
	(...args) =>
	run(prop, args),
	},
	);
	const EV = name =>
	new Proxy(
	{},
	{
	get:
	(_t, prop, _rx) =>
	(...args) =>
	messageVat(name, prop, args),
	},
	);
	const EP = presence =>
	new Proxy(
	{},
	{
	get:
	(_t, prop, _rx) =>
	(...args) =>
	messageObject(presence, prop, args),
	},
	);


		Generated by [AVA](https://avajs.dev).

		## makeAgoricNamesRemotesFromFakeStorage

6880 bootstrap integration test of vaultFactory with smart wallet #7021

6880 bootstrap integration test of vaultFactory with smart wallet #7021

Conversation

turadg commented Feb 17, 2023 • edited Loading

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turadg Feb 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turadg commented Feb 25, 2023

dckc 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

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

Choose a reason for hiding this comment

mhofman commented Feb 28, 2023 • edited Loading

turadg commented Feb 17, 2023 •

edited

Loading

turadg Feb 25, 2023 •

edited

Loading

mhofman commented Feb 28, 2023 •

edited

Loading