Skip to content

Commit

Permalink
test(liveslots): relaxDurabilityRules should not make promises durable
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
erights authored and warner committed Jul 10, 2024
1 parent bc48a8b commit 5c39d56
Show file tree
Hide file tree
Showing 10 changed files with 64 additions and 34 deletions.
4 changes: 4 additions & 0 deletions packages/inter-protocol/test/provisionPool.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
4 changes: 3 additions & 1 deletion packages/inter-protocol/test/smartWallet/boot-test-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/
// @ts-check
import { Fail } from '@endo/errors';
import { E } from '@endo/eventual-send';
import {
makeFakeVatAdmin,
zcfBundleCap,
Expand Down Expand Up @@ -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}`);
Expand Down
15 changes: 10 additions & 5 deletions packages/inter-protocol/test/smartWallet/contexts.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
);

Expand Down
19 changes: 11 additions & 8 deletions packages/pegasus/test/fakeVatAdmin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`);
Expand Down
15 changes: 10 additions & 5 deletions packages/smart-wallet/test/contexts.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
);

Expand Down
13 changes: 9 additions & 4 deletions packages/smart-wallet/test/startWalletFactory.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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"]',
},
);

Expand All @@ -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,
Expand Down
5 changes: 4 additions & 1 deletion packages/swingset-liveslots/src/virtualReferences.js
Original file line number Diff line number Diff line change
Expand Up @@ -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') {
Expand Down
2 changes: 1 addition & 1 deletion packages/swingset-liveslots/test/durabilityChecks.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
4 changes: 3 additions & 1 deletion packages/vats/tools/boot-test-utils.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Fail } from '@endo/errors';
import { E } from '@endo/far';
import { Far } from '@endo/marshal';

import {
Expand Down Expand Up @@ -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}`);
Expand Down
17 changes: 9 additions & 8 deletions packages/zoe/tools/fakeVatAdmin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 5c39d56

Please sign in to comment.