From 5c39d56a22464e839fd5a772fdfd74bd7ef20dda Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Wed, 19 Jun 2024 18:58:02 -0700 Subject: [PATCH] test(liveslots): relaxDurabilityRules should not make promises durable We have "fake vat" testing environments which configure their copy of liveslots with a setting named `relaxDurabilityRules = true`. This changes the rules for durable objects, which normally refuse to accept non-durable state. When the rules are relaxed, they accept ephemeral and merely-virtual objects as well. Promises were never supposed to be accepted, even when the rules are relaxed. But liveslots had a bug, and accepted Promises in the relaxed mode. This PR changes liveslots to correctly reject them. Lacking this enforcement, some other packages had mistakenly stored Promises in durable objects, and gotten away with it. Now that liveslots is fixed, their tests would fail. So this PR also fixes those packages. One case is the smart-wallet, whose tests which use contexts.js will get a Zoe that uses fully-resolved `agoricNames` and `board`. But some tests are deliberately not using that context, to exercise what happens when E(zoe).startInstance() is given bad arguments. This changes the tests to use bad *resolved* arguments, as to Promises for bad arguments, because the latter now fails in a different way. Giving Zoe a promise in `terms` causes "value for instanceRecord is not durable", which is not what this test is trying to exercise. In addition, the expected error message was updated, because it now mentions things like "NameHubKit" instead of "Promise" in the unrelated (good) arguments. The provisionPool test was fixed with an unsatisfying `await null` stall. See #9598 for notes and plans for a better fix. --- .../inter-protocol/test/provisionPool.test.js | 4 ++++ .../test/smartWallet/boot-test-utils.js | 4 +++- .../test/smartWallet/contexts.js | 15 ++++++++++----- packages/pegasus/test/fakeVatAdmin.js | 19 +++++++++++-------- packages/smart-wallet/test/contexts.js | 15 ++++++++++----- .../test/startWalletFactory.test.js | 13 +++++++++---- .../src/virtualReferences.js | 5 ++++- .../test/durabilityChecks.test.js | 2 +- packages/vats/tools/boot-test-utils.js | 4 +++- packages/zoe/tools/fakeVatAdmin.js | 17 +++++++++-------- 10 files changed, 64 insertions(+), 34 deletions(-) diff --git a/packages/inter-protocol/test/provisionPool.test.js b/packages/inter-protocol/test/provisionPool.test.js index 7f2d3798609..48ea08f7312 100644 --- a/packages/inter-protocol/test/provisionPool.test.js +++ b/packages/inter-protocol/test/provisionPool.test.js @@ -571,11 +571,15 @@ test('provisionPool publishes metricsOverride promptly', async t => { }, ); + // FIXME: remove the 'await null', + // https://github.com/Agoric/agoric-sdk/issues/9598 + await null; const metrics = E(facets.publicFacet).getMetrics(); const { head: { value: initialMetrics }, } = await E(metrics).subscribeAfter(); + // FIXME fails when fakeVatAdmin.js fixed for non-promise root. Why? t.deepEqual(initialMetrics, { totalMintedConverted: minted.make(20_000_000n), totalMintedProvided: minted.make(750_000n), diff --git a/packages/inter-protocol/test/smartWallet/boot-test-utils.js b/packages/inter-protocol/test/smartWallet/boot-test-utils.js index c4674281b69..ae1231d5781 100644 --- a/packages/inter-protocol/test/smartWallet/boot-test-utils.js +++ b/packages/inter-protocol/test/smartWallet/boot-test-utils.js @@ -5,6 +5,7 @@ */ // @ts-check import { Fail } from '@endo/errors'; +import { E } from '@endo/eventual-send'; import { makeFakeVatAdmin, zcfBundleCap, @@ -109,7 +110,8 @@ export const makePopulatedFakeVatAdmin = () => { const baggage = makeScalarBigMapStore('baggage'); const adminNode = /** @type {import('@agoric/swingset-vat').VatAdminFacet} */ ({}); - return { root: buildRoot({}, vatParameters, baggage), adminNode }; + const rootP = buildRoot({}, vatParameters, baggage); + return E.when(rootP, root => harden({ root, adminNode })); }; const createVatByName = async name => { return createVat(fakeNameToCap.get(name) || Fail`unknown vat ${name}`); diff --git a/packages/inter-protocol/test/smartWallet/contexts.js b/packages/inter-protocol/test/smartWallet/contexts.js index fb0872219f0..7f6bca7ed05 100644 --- a/packages/inter-protocol/test/smartWallet/contexts.js +++ b/packages/inter-protocol/test/smartWallet/contexts.js @@ -111,14 +111,19 @@ export const makeDefaultTestContext = async (t, makeSpace) => { */ const walletBridgeManager = await (bridgeManager && makeScopedBridge(bridgeManager, BridgeId.WALLET)); + + const customTerms = await deeplyFulfilledObject( + harden({ + agoricNames, // may be a promise + board: consume.board, // may be a promise + assetPublisher, + }), + ); + const walletFactory = await E(zoe).startInstance( installation, {}, - { - agoricNames, - board: consume.board, - assetPublisher, - }, + customTerms, { storageNode, walletBridgeManager }, ); diff --git a/packages/pegasus/test/fakeVatAdmin.js b/packages/pegasus/test/fakeVatAdmin.js index 5c483621338..80996bdfb55 100644 --- a/packages/pegasus/test/fakeVatAdmin.js +++ b/packages/pegasus/test/fakeVatAdmin.js @@ -7,15 +7,18 @@ export default harden({ createMeter: () => {}, createUnlimitedMeter: () => {}, createVat: bundle => { - return harden({ - root: E(evalContractBundle(bundle)).buildRootObject(), - adminNode: { - done: () => { - return makePromiseKit().promise; + const rootP = E(evalContractBundle(bundle)).buildRootObject(); + return E.when(rootP, root => + harden({ + root, + adminNode: { + done: () => { + return makePromiseKit().promise; + }, + terminate: () => {}, }, - terminate: () => {}, - }, - }); + }), + ); }, createVatByName: _name => { throw Error(`createVatByName not supported in fake mode`); diff --git a/packages/smart-wallet/test/contexts.js b/packages/smart-wallet/test/contexts.js index 0afb7e22b49..4034e4c2f42 100644 --- a/packages/smart-wallet/test/contexts.js +++ b/packages/smart-wallet/test/contexts.js @@ -41,14 +41,19 @@ export const makeDefaultTestContext = async (t, makeSpace) => { const bridgeManager = await consume.bridgeManager; const walletBridgeManager = await (bridgeManager && makeScopedBridge(bridgeManager, BridgeId.WALLET)); - const walletFactory = await E(zoe).startInstance( - installation, - {}, - { + + const customTerms = await deeplyFulfilledObject( + harden({ agoricNames, board: consume.board, assetPublisher, - }, + }), + ); + + const walletFactory = await E(zoe).startInstance( + installation, + {}, + customTerms, { storageNode, walletBridgeManager }, ); diff --git a/packages/smart-wallet/test/startWalletFactory.test.js b/packages/smart-wallet/test/startWalletFactory.test.js index b427a7c31cb..528b70655fb 100644 --- a/packages/smart-wallet/test/startWalletFactory.test.js +++ b/packages/smart-wallet/test/startWalletFactory.test.js @@ -54,9 +54,12 @@ test.before(async t => { test('customTermsShape', async t => { const { consume, bridgeManager, installation, storageNode } = t.context; - const { agoricNames, board, zoe } = consume; + const { zoe } = consume; const privateArgs = { bridgeManager, storageNode }; + const agoricNames = await consume.agoricNames; + const board = await consume.board; + // extra term await t.throwsAsync( E(zoe).startInstance( @@ -73,7 +76,7 @@ test('customTermsShape', async t => { ), { message: - 'customTerms: {"agoricNames":"[Promise]","assetPublisher":{},"board":"[Promise]","extra":"[Seen]"} - Must not have unexpected properties: ["extra"]', + 'customTerms: {"agoricNames":"[Alleged: NameHubKit nameHub]","assetPublisher":{},"board":"[Alleged: Board board]","extra":"[Seen]"} - Must not have unexpected properties: ["extra"]', }, ); @@ -90,14 +93,16 @@ test('customTermsShape', async t => { ), { message: - 'customTerms: {"agoricNames":"[Promise]"} - Must have missing properties ["board","assetPublisher"]', + 'customTerms: {"agoricNames":"[Alleged: NameHubKit nameHub]"} - Must have missing properties ["board","assetPublisher"]', }, ); }); test('privateArgsShape', async t => { const { consume, bridgeManager, installation } = t.context; - const { agoricNames, board, zoe } = consume; + const { zoe } = consume; + const agoricNames = await consume.agoricNames; + const board = await consume.board; const terms = { agoricNames, board, diff --git a/packages/swingset-liveslots/src/virtualReferences.js b/packages/swingset-liveslots/src/virtualReferences.js index 10ed3785140..09f78ff2ac9 100644 --- a/packages/swingset-liveslots/src/virtualReferences.js +++ b/packages/swingset-liveslots/src/virtualReferences.js @@ -290,7 +290,10 @@ export function makeVirtualReferenceManager( */ function isDurable(vref) { const { type, id, virtual, durable, allocatedByVat } = parseVatSlot(vref); - if (relaxDurabilityRules) { + if (type === 'promise') { + // promises are not durable even if `relaxDurabilityRules === true` + return false; + } else if (relaxDurabilityRules) { // we'll pretend an object is durable if running with relaxed rules return true; } else if (type === 'device') { diff --git a/packages/swingset-liveslots/test/durabilityChecks.test.js b/packages/swingset-liveslots/test/durabilityChecks.test.js index a939a681213..dafb3408598 100644 --- a/packages/swingset-liveslots/test/durabilityChecks.test.js +++ b/packages/swingset-liveslots/test/durabilityChecks.test.js @@ -262,7 +262,7 @@ async function runDurabilityCheckTest(t, relaxDurabilityRules) { passVal(() => virtualMap.init('non-scalar key', aNonScalarKey)); passVal(() => virtualMap.init('non-scalar non-key', aNonScalarNonKey)); - condVal(() => durableMap.init('promise', aPassablePromise)); + failVal(() => durableMap.init('promise', aPassablePromise)); passVal(() => durableMap.init('error', aPassableError)); passVal(() => durableMap.init('non-scalar key', aNonScalarKey)); passVal(() => durableMap.init('non-scalar non-key', aNonScalarNonKey)); diff --git a/packages/vats/tools/boot-test-utils.js b/packages/vats/tools/boot-test-utils.js index cb8fc7b9cf6..f43f6e15552 100644 --- a/packages/vats/tools/boot-test-utils.js +++ b/packages/vats/tools/boot-test-utils.js @@ -1,4 +1,5 @@ import { Fail } from '@endo/errors'; +import { E } from '@endo/far'; import { Far } from '@endo/marshal'; import { @@ -126,7 +127,8 @@ export const makePopulatedFakeVatAdmin = () => { const baggage = makeScalarBigMapStore('baggage'); const adminNode = /** @type {import('@agoric/swingset-vat').VatAdminFacet} */ ({}); - return { root: buildRoot({}, vatParameters, baggage), adminNode }; + const rootP = buildRoot({}, vatParameters, baggage); + return E.when(rootP, root => harden({ root, adminNode })); }; const createVatByName = async name => { return createVat(fakeNameToCap.get(name) || Fail`unknown vat ${name}`); diff --git a/packages/zoe/tools/fakeVatAdmin.js b/packages/zoe/tools/fakeVatAdmin.js index d56565079b5..b174a067309 100644 --- a/packages/zoe/tools/fakeVatAdmin.js +++ b/packages/zoe/tools/fakeVatAdmin.js @@ -103,15 +103,16 @@ function makeFakeVatAdmin(testContextSetter = undefined, makeRemote = x => x) { // buildRootObject: vp => ns.buildRootObject(vpow, vp, vatBaggage), // }), // ); - return Promise.resolve( + const rootP = makeRemote( + E(evalContractBundle(zcfBundle)).buildRootObject( + vpow, + vatParameters, + vatBaggage, + ), + ); + return E.when(rootP, root => harden({ - root: makeRemote( - E(evalContractBundle(zcfBundle)).buildRootObject( - vpow, - vatParameters, - vatBaggage, - ), - ), + root, adminNode: Far('adminNode', { done: () => { return exitKit.promise;