Skip to content

Commit

Permalink
fix: review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
erights committed Feb 13, 2021
1 parent 7db7e5c commit 17d7df6
Show file tree
Hide file tree
Showing 14 changed files with 40 additions and 46 deletions.
12 changes: 5 additions & 7 deletions packages/SwingSet/src/initializeSwingset.js
Original file line number Diff line number Diff line change
Expand Up @@ -204,13 +204,11 @@ export function loadSwingsetConfigFile(configPath) {
normalizeConfigDescriptor(config.vats, dirname, true);
normalizeConfigDescriptor(config.bundles, dirname, false);
// normalizeConfigDescriptor(config.devices, dirname, true); // TODO: represent devices
if (!config.bootstrap) {
assert.fail(X`no designated bootstrap vat in ${configPath}`);
} else if (!config.vats[config.bootstrap]) {
throw Error(
`bootstrap vat ${config.bootstrap} not found in ${configPath}`,
);
}
assert(config.bootstrap, X`no designated bootstrap vat in ${configPath}`);
assert(
config.vats[config.bootstrap],
X`bootstrap vat ${config.bootstrap} not found in ${configPath}`,
);
return config;
} catch (e) {
if (e.code === 'ENOENT') {
Expand Down
4 changes: 1 addition & 3 deletions packages/SwingSet/src/kernel/deviceSlots.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,7 @@ export function makeDeviceSlots(
// since devices don't accept Promises either, SO(x) must be given a
// presence, not a promise that might resolve to a presence.

if (outstandingProxies.has(x)) {
throw new Error('SO(SO(x)) is invalid');
}
assert(!outstandingProxies.has(x), X`SO(SO(x)) is invalid`);
const slot = valToSlot.get(x);
assert(
slot && parseVatSlot(slot).type === 'object',
Expand Down
10 changes: 7 additions & 3 deletions packages/SwingSet/src/kernel/id.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { assert, details as X } from '@agoric/assert';
*/
export function insistVatID(s) {
try {
assert(s === `${s}`, X`not a string`);
assert.typeof(s, 'string', X`not a string`);
assert(s.startsWith(`v`), X`does not start with 'v'`);
Nat(Number(s.slice(1)));
} catch (e) {
Expand Down Expand Up @@ -55,7 +55,7 @@ export function makeVatID(index) {
*/
export function insistDeviceID(s) {
try {
assert(s === `${s}`, X`not a string`);
assert.typeof(s, 'string', X`not a string`);
assert(s.startsWith(`d`), X`does not start with 'd'`);
Nat(Number(s.slice(1)));
} catch (e) {
Expand Down Expand Up @@ -87,7 +87,11 @@ export function makeDeviceID(index) {
* @throws {Error} if the parameter is not a string or is malformed.
*/
export function parseVatOrDeviceID(s) {
assert(s === `${s}`, X`${s} is not a string, so cannot be a VatID/DeviceID`);
assert.typeof(
s,
'string',
X`${s} is not a string, so cannot be a VatID/DeviceID`,
);
s = `${s}`;
let type;
let idSuffix;
Expand Down
4 changes: 3 additions & 1 deletion packages/SwingSet/src/kernel/initializeKernel.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ export function initializeKernel(config, hostStorage, verbose = false) {
'object',
X`bundle is not an object, rather ${bundle}`,
);
} else assert(bundleName, X`no bundle specified for vat ${name}`);
} else {
assert(bundleName, X`no bundle specified for vat ${name}`);
}

// todo: consider having vats indicate 'enablePipelining' by exporting a
// boolean, rather than options= . We'd have to retrieve the flag from
Expand Down
4 changes: 2 additions & 2 deletions packages/SwingSet/src/kernel/state/deviceKeeper.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export function makeDeviceKeeper(storage, deviceID, addKernelDeviceNode) {
* or is otherwise invalid.
*/
function mapDeviceSlotToKernelSlot(devSlot) {
assert(`${devSlot}` === devSlot, X`non-string devSlot: ${devSlot}`);
assert.typeof(devSlot, 'string', X`non-string devSlot: ${devSlot}`);
// kdebug(`mapOutbound ${devSlot}`);
const devKey = `${deviceID}.c.${devSlot}`;
if (!storage.has(devKey)) {
Expand Down Expand Up @@ -101,7 +101,7 @@ export function makeDeviceKeeper(storage, deviceID, addKernelDeviceNode) {
* devices or is otherwise invalid.
*/
function mapKernelSlotToDeviceSlot(kernelSlot) {
assert(`${kernelSlot}` === kernelSlot, 'non-string kernelSlot');
assert.typeof(kernelSlot, 'string', 'non-string kernelSlot');
const kernelKey = `${deviceID}.c.${kernelSlot}`;
if (!storage.has(kernelKey)) {
const { type } = parseKernelSlot(kernelSlot);
Expand Down
6 changes: 3 additions & 3 deletions packages/SwingSet/src/kernel/state/kernelKeeper.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import Nat from '@agoric/nat';
import { assert, details as X, q } from '@agoric/assert';
import { assert, details as X } from '@agoric/assert';
import { initializeVatState, makeVatKeeper } from './vatKeeper';
import { initializeDeviceState, makeDeviceKeeper } from './deviceKeeper';
import { insistEnhancedStorageAPI } from '../../storageAPI';
Expand Down Expand Up @@ -589,7 +589,7 @@ export default function makeKernelKeeper(storage, kernelSlog) {
function getVatIDForName(name) {
assert.typeof(name, 'string');
const k = `vat.name.${name}`;
assert(storage.has(k), X`vat name ${q(name)} must exist, but doesn't`);
assert(storage.has(k), X`vat name ${name} must exist, but doesn't`);
return storage.get(k);
}

Expand Down Expand Up @@ -752,7 +752,7 @@ export default function makeKernelKeeper(storage, kernelSlog) {
function getDeviceIDForName(name) {
assert.typeof(name, 'string');
const k = `device.name.${name}`;
assert(storage.has(k), X`device name ${q(name)} must exist, but doesn't`);
assert(storage.has(k), X`device name ${name} must exist, but doesn't`);
return storage.get(k);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/SwingSet/src/kernel/state/vatKeeper.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export function makeVatKeeper(
* or is otherwise invalid.
*/
function mapVatSlotToKernelSlot(vatSlot) {
assert(`${vatSlot}` === vatSlot, X`non-string vatSlot: ${vatSlot}`);
assert.typeof(vatSlot, 'string', X`non-string vatSlot: ${vatSlot}`);
const vatKey = `${vatID}.c.${vatSlot}`;
if (!storage.has(vatKey)) {
const { type, allocatedByVat } = parseVatSlot(vatSlot);
Expand Down Expand Up @@ -140,7 +140,7 @@ export function makeVatKeeper(
* or is otherwise invalid.
*/
function mapKernelSlotToVatSlot(kernelSlot) {
assert(`${kernelSlot}` === kernelSlot, 'non-string kernelSlot');
assert.typeof(kernelSlot, 'string', 'non-string kernelSlot');
const kernelKey = `${vatID}.c.${kernelSlot}`;
if (!storage.has(kernelKey)) {
const { type } = parseKernelSlot(kernelSlot);
Expand Down
2 changes: 1 addition & 1 deletion packages/SwingSet/src/kernel/vatTranslator.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ function makeTranslateVatSyscallToKernelSyscall(vatID, kernelKeeper) {
const { mapVatSlotToKernelSlot } = vatKeeper;

function translateSend(targetSlot, method, args, resultSlot) {
assert(`${targetSlot}` === targetSlot, 'non-string targetSlot');
assert.typeof(targetSlot, 'string', 'non-string targetSlot');
insistCapData(args);
// TODO: disable send-to-self for now, qv issue #43
const target = mapVatSlotToKernelSlot(targetSlot);
Expand Down
2 changes: 1 addition & 1 deletion packages/SwingSet/test/test-devices.js
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,6 @@ test.serial('syscall.callNow(promise) is vat-fatal', async t => {
// now check that the vat was terminated: this should throw an exception
// because the entire bootstrap vat was deleted
t.throws(() => c.queueToVatExport('bootstrap', 'o+0', 'ping', capargs([])), {
message: `vat name "bootstrap" must exist, but doesn't`,
message: /vat name .* must exist, but doesn't/,
});
});
4 changes: 2 additions & 2 deletions packages/SwingSet/test/test-vpid-kernel.js
Original file line number Diff line number Diff line change
Expand Up @@ -582,9 +582,9 @@ test(`kernel vpid handling crossing resolutions`, async t => {
await kernel.start(undefined); // no bootstrapVatName, so no bootstrap call
// vatX controls the scenario, vatA and vatB are the players
let onDispatchCallback;
function odc(dp) {
function odc(d) {
if (onDispatchCallback) {
onDispatchCallback(dp);
onDispatchCallback(d);
}
}
const { log: logA, getSyscall: getSyscallA } = await buildRawVat(
Expand Down
6 changes: 0 additions & 6 deletions packages/assert/src/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,6 @@ const { details, quote } = globalAssert;

export { globalAssert as assert, details, quote };

// DEPRECATED: Going forward we encourage the pattern over importing the
// abbreviation 'q' for quote.
//
// ```js
// import { quote as q, details as X } from '@agoric/assert';
// ```
export { quote as q };

/**
Expand Down
10 changes: 4 additions & 6 deletions packages/assert/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,6 @@
*/

/**
* @callback AssertQuote
*
* To "declassify" and quote a substitution value used in a
* details`...` template literal, enclose that substitution expression
* in a call to `quote`. This states that the argument should appear quoted
Expand All @@ -171,11 +169,10 @@
* details`${sky.color} should be ${quote(color)}`,
* );
* ```
*
* The normal convention is to locally rename `quote` to `q` and
* `details` to `X`
* The normal convention is to locally rename `details` to `X` and import `q`
* and `assert` unmodified.
* ```js
* const { details: X, quote: q } = assert;
* import { assert, details as X, q } from \'@agoric/assert\';
* ```
* so the above example would then be
* ```js
Expand All @@ -186,6 +183,7 @@
* );
* ```
*
* @callback AssertQuote
* @param {*} payload What to declassify
* @returns {StringablePayload} The declassified payload
*/
Expand Down
4 changes: 2 additions & 2 deletions packages/cosmic-swingset/lib/ag-solo/vats/vat-http.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ export function buildRootObject(vatPowers) {
}

return harden({
setCommandDevice(dp) {
commandDevice = dp;
setCommandDevice(d) {
commandDevice = d;

const replHandler = getReplHandler(replObjects, send, vatPowers);
registerURLHandler(replHandler, '/private/repl');
Expand Down
14 changes: 7 additions & 7 deletions packages/swing-store-lmdb/lmdbSwingStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ function makeSwingStore(dirPath, forceReset = false) {
* @throws if key is not a string.
*/
function get(key) {
assert(`${key}` === key, X`non-string key ${key}`);
assert.typeof(key, 'string', X`non-string key ${key}`);
ensureTxn();
let result = txn.getString(dbi, key);
if (result === null) {
Expand All @@ -90,8 +90,8 @@ function makeSwingStore(dirPath, forceReset = false) {
* @throws if either parameter is not a string.
*/
function* getKeys(start, end) {
assert(`${start}` === start, X`non-string start ${start}`);
assert(`${end}` === end, X`non-string end ${end}`);
assert.typeof(start, 'string', X`non-string start ${start}`);
assert.typeof(end, 'string', X`non-string end ${end}`);

ensureTxn();
const cursor = new lmdb.Cursor(txn, dbi);
Expand All @@ -113,7 +113,7 @@ function makeSwingStore(dirPath, forceReset = false) {
* @throws if key is not a string.
*/
function has(key) {
assert(`${key}` === key, X`non-string key ${key}`);
assert.typeof(key, 'string', X`non-string key ${key}`);
return get(key) !== undefined;
}

Expand All @@ -127,8 +127,8 @@ function makeSwingStore(dirPath, forceReset = false) {
* @throws if either parameter is not a string.
*/
function set(key, value) {
assert(`${key}` === key, X`non-string key ${key}`);
assert(`${value}` === value, X`non-string value ${value}`);
assert.typeof(key, 'string', X`non-string key ${key}`);
assert.typeof(value, 'string', X`non-string value ${value}`);
ensureTxn();
txn.putString(dbi, key, value);
}
Expand All @@ -142,7 +142,7 @@ function makeSwingStore(dirPath, forceReset = false) {
* @throws if key is not a string.
*/
function del(key) {
assert(`${key}` === key, X`non-string key ${key}`);
assert.typeof(key, 'string', X`non-string key ${key}`);
if (has(key)) {
ensureTxn();
txn.del(dbi, key);
Expand Down

0 comments on commit 17d7df6

Please sign in to comment.