Skip to content

Commit

Permalink
Passable exo state (#10228)
Browse files Browse the repository at this point in the history
refs: #10170 
refs: endojs/endo#1648

best reviewed by ignoring white space

## Description
After implementing a change to enforce that heap exo are passable, I found that some things currently stored in heap exos, mostly in tests, are not actually passable. Almost all of them are remotable like objects not marked as `Far`, usually related to storage nodes.

While I plan on moving the implementation of the checks into endo instead of landing it in agoric-sdk first, I figured we should already fix the issues found.

### Security Considerations
None

### Scaling Considerations
None

### Documentation Considerations
None

### Testing Considerations
See #10170 for passing tests with checks enabled

### Upgrade Considerations
As it's a heap zone change, cross upgrade behavior is not involved. In the future any code using a stricter heap zone would need to comply. However heaps zones are used as packages bundled by the contract, and are not provided by liveslots or zcf, so there is no compatibility risk.
  • Loading branch information
mergify[bot] authored Oct 5, 2024
2 parents f245412 + 4f07d1c commit 7d1dfc0
Show file tree
Hide file tree
Showing 8 changed files with 128 additions and 109 deletions.
17 changes: 10 additions & 7 deletions packages/cache/test/storage.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,16 @@ harden(makeSimpleMarshaller);

const setup = () => {
const storageNodeState = {};
const chainStorage = makeChainStorageRoot(message => {
assert(message.method === 'set');
assert(message.args.length === 1);
const [[path, value]] = message.args;
assert(path === 'cache');
storageNodeState.cache = value;
}, 'cache');
const chainStorage = makeChainStorageRoot(
Far('ToStorage', message => {
assert(message.method === 'set');
assert(message.args.length === 1);
const [[path, value]] = message.args;
assert(path === 'cache');
storageNodeState.cache = value;
}),
'cache',
);
const cache = makeCache(
makeChainStorageCoordinator(chainStorage, makeSimpleMarshaller()),
);
Expand Down
6 changes: 3 additions & 3 deletions packages/cosmic-swingset/src/chain-main.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { resolve as importMetaResolve } from 'import-meta-resolve';
import tmp from 'tmp';

import { Fail, q } from '@endo/errors';
import { E } from '@endo/far';
import { E, Far } from '@endo/far';
import { makeMarshal } from '@endo/marshal';
import { isNat } from '@endo/nat';
import { M, mustMatch } from '@endo/patterns';
Expand Down Expand Up @@ -432,9 +432,9 @@ export default async function main(progname, args, { env, homedir, agcc }) {
}
}

const toStorage = message => {
const toStorage = Far('BridgeStorageHandler', message => {
return doOutboundBridge(BridgeId.STORAGE, message);
};
});

const makeInstallationPublisher = () => {
const installationStorageNode = makeChainStorageRoot(
Expand Down
7 changes: 5 additions & 2 deletions packages/internal/src/lib-chainStorage.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// @ts-check

import { Fail } from '@endo/errors';
import { E } from '@endo/far';
import { E, Far } from '@endo/far';
import { M } from '@endo/patterns';
import { makeHeapZone } from '@agoric/base-zone/heap.js';
import * as cb from './callback.js';
Expand Down Expand Up @@ -278,7 +278,10 @@ export function makeChainStorageRoot(
*/
const makeNullStorageNode = () => {
// XXX re-use "ChainStorage" methods above which don't actually depend on chains
return makeChainStorageRoot(() => null, 'null');
return makeChainStorageRoot(
Far('NullMessenger', () => null),
'null',
);
};

/**
Expand Down
142 changes: 73 additions & 69 deletions packages/internal/src/storage-test-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,82 +108,86 @@ export const makeFakeStorageKit = (rootPath, rootOptions) => {
};
/** @type {StorageMessage[]} */
const messages = [];
/** @param {StorageMessage} message */

const toStorage = message => {
messages.push(message);
switch (message.method) {
case 'getStoreKey': {
const [key] = message.args;
return { storeName: 'swingset', storeSubkey: `fake:${key}` };
}
case 'get': {
const [key] = message.args;
return data.has(key) ? data.get(key) : null;
}
case 'children': {
const [key] = message.args;
const childEntries = getChildEntries(`${key}.`);
return [...childEntries.keys()];
}
case 'entries': {
const [key] = message.args;
const childEntries = getChildEntries(`${key}.`);
return [...childEntries.entries()].map(entry =>
entry[1] != null ? entry : [entry[0]],
);
}
case 'set':
case 'setWithoutNotify': {
trace('toStorage set', message);
/** @type {StorageEntry[]} */
const newEntries = message.args;
for (const [key, value] of newEntries) {
if (value != null) {
data.set(key, value);
} else {
data.delete(key);
}
const toStorage = Far(
'ToStorage',
/** @param {StorageMessage} message */
message => {
messages.push(message);
switch (message.method) {
case 'getStoreKey': {
const [key] = message.args;
return { storeName: 'swingset', storeSubkey: `fake:${key}` };
}
break;
}
case 'append': {
trace('toStorage append', message);
/** @type {StorageEntry[]} */
const newEntries = message.args;
for (const [key, value] of newEntries) {
value != null || Fail`attempt to append with no value`;
// In the absence of block boundaries, everything goes in a single StreamCell.
const oldVal = data.get(key);
let streamCell;
if (oldVal != null) {
try {
streamCell = JSON.parse(oldVal);
assert(isStreamCell(streamCell));
} catch (_err) {
streamCell = undefined;
case 'get': {
const [key] = message.args;
return data.has(key) ? data.get(key) : null;
}
case 'children': {
const [key] = message.args;
const childEntries = getChildEntries(`${key}.`);
return [...childEntries.keys()];
}
case 'entries': {
const [key] = message.args;
const childEntries = getChildEntries(`${key}.`);
return [...childEntries.entries()].map(entry =>
entry[1] != null ? entry : [entry[0]],
);
}
case 'set':
case 'setWithoutNotify': {
trace('toStorage set', message);
/** @type {StorageEntry[]} */
const newEntries = message.args;
for (const [key, value] of newEntries) {
if (value != null) {
data.set(key, value);
} else {
data.delete(key);
}
}
if (streamCell === undefined) {
streamCell = {
blockHeight: '0',
values: oldVal != null ? [oldVal] : [],
};
break;
}
case 'append': {
trace('toStorage append', message);
/** @type {StorageEntry[]} */
const newEntries = message.args;
for (const [key, value] of newEntries) {
value != null || Fail`attempt to append with no value`;
// In the absence of block boundaries, everything goes in a single StreamCell.
const oldVal = data.get(key);
let streamCell;
if (oldVal != null) {
try {
streamCell = JSON.parse(oldVal);
assert(isStreamCell(streamCell));
} catch (_err) {
streamCell = undefined;
}
}
if (streamCell === undefined) {
streamCell = {
blockHeight: '0',
values: oldVal != null ? [oldVal] : [],
};
}
streamCell.values.push(value);
data.set(key, JSON.stringify(streamCell));
}
streamCell.values.push(value);
data.set(key, JSON.stringify(streamCell));
break;
}
break;
case 'size':
// Intentionally incorrect because it counts non-child descendants,
// but nevertheless supports a "has children" test.
return [...data.keys()].filter(k =>
k.startsWith(`${message.args[0]}.`),
).length;
default:
throw Error(`unsupported method: ${message.method}`);
}
case 'size':
// Intentionally incorrect because it counts non-child descendants,
// but nevertheless supports a "has children" test.
return [...data.keys()].filter(k => k.startsWith(`${message.args[0]}.`))
.length;
default:
throw Error(`unsupported method: ${message.method}`);
}
};
},
);
const rootNode = makeChainStorageRoot(toStorage, rootPath, resolvedOptions);
return {
rootNode,
Expand Down
4 changes: 2 additions & 2 deletions packages/internal/test/callback.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -304,11 +304,11 @@ test('makeAttenuator', async t => {
overrides: {
m1: null,
m2: cb.makeMethodCallback(
{
Far('Abc', {
abc() {
return 'return abc';
},
},
}),
'abc',
),
},
Expand Down
30 changes: 18 additions & 12 deletions packages/vow/test/disconnect.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,23 @@ test('retry on disconnection', async t => {
},
},
);
const makeFinalWatcher = zone.exoClass(
'FinalWatcher',
undefined,
final => ({ final }),
{
onFulfilled(value) {
t.is(this.state.final, 'happy');
t.is(value, 'resolved');
return value;
},
onRejected(reason) {
t.is(this.state.final, 'sad');
t.is(reason && reason.message, 'dejected');
return ['rejected', reason];
},
},
);

const PLANS = [
[0, 'happy'],
Expand Down Expand Up @@ -72,18 +89,7 @@ test('retry on disconnection', async t => {
let resultP;
switch (pattern) {
case 'watch-with-handler': {
const resultW = watch(vow, {
onFulfilled(value) {
t.is(plan[final], 'happy');
t.is(value, 'resolved');
return value;
},
onRejected(reason) {
t.is(plan[final], 'sad');
t.is(reason && reason.message, 'dejected');
return ['rejected', reason];
},
});
const resultW = watch(vow, makeFinalWatcher(plan[final]));
t.is('then' in resultW, false, 'watch resultW.then is undefined');
resultP = when(resultW);
break;
Expand Down
4 changes: 2 additions & 2 deletions packages/vow/test/watch-utils.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -256,14 +256,14 @@ test('asPromise handles watcher arguments', async t => {
const vow = watch(testPromiseP);

let watcherCalled = false;
const watcher = {
const watcher = zone.exo('Watcher', undefined, {
onFulfilled(value, ctx) {
watcherCalled = true;
t.is(value, 'watcher test');
t.deepEqual(ctx, ['ctx']);
return value;
},
};
});

// XXX fix type: `watcherContext` doesn't need to be an array
const result = await asPromise(vow, watcher, ['ctx']);
Expand Down
27 changes: 15 additions & 12 deletions packages/vow/test/watch.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -232,18 +232,21 @@ test('disconnection of non-vow informs watcher', async t => {

// Even though this promise is rejected with a retryable reason, there's no
// vow before it to retry, so we pass the rejection up to the watcher.
/* eslint-disable-next-line prefer-promise-reject-errors */
const vow = watch(Promise.reject('disconnected'), {
onFulfilled(value) {
t.log(`onfulfilled ${value}`);
t.fail('should not fulfil');
return 'fulfilled';
},
onRejected(reason) {
t.is(reason, 'disconnected');
return `rejected ${reason}`;
},
});
const vow = watch(
/* eslint-disable-next-line prefer-promise-reject-errors */
Promise.reject('disconnected'),
zone.exo('Watcher', undefined, {
onFulfilled(value) {
t.log(`onfulfilled ${value}`);
t.fail('should not fulfil');
return 'fulfilled';
},
onRejected(reason) {
t.is(reason, 'disconnected');
return `rejected ${reason}`;
},
}),
);

t.is(await when(vow), 'rejected disconnected');
});

0 comments on commit 7d1dfc0

Please sign in to comment.