Skip to content

Commit

Permalink
fix: faster asserts
Browse files Browse the repository at this point in the history
  • Loading branch information
erights committed Sep 10, 2022
1 parent b73a7b8 commit b66bbfc
Show file tree
Hide file tree
Showing 16 changed files with 156 additions and 171 deletions.
16 changes: 8 additions & 8 deletions packages/captp/src/atomics.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ const STATUS_FLAG_REJECT = 4;
* @param {SharedArrayBuffer} transferBuffer the backing buffer
*/
const splitTransferBuffer = transferBuffer => {
assert(
transferBuffer.byteLength >= MIN_TRANSFER_BUFFER_LENGTH,
X`Transfer buffer of ${transferBuffer.byteLength} bytes is smaller than MIN_TRANSFER_BUFFER_LENGTH ${MIN_TRANSFER_BUFFER_LENGTH}`,
);
transferBuffer.byteLength >= MIN_TRANSFER_BUFFER_LENGTH ||
assert.fail(
X`Transfer buffer of ${transferBuffer.byteLength} bytes is smaller than MIN_TRANSFER_BUFFER_LENGTH ${MIN_TRANSFER_BUFFER_LENGTH}`,
);
const lenbuf = new BigUint64Array(transferBuffer, 0, 1);

// The documentation says that this needs to be an Int32Array for use with
Expand All @@ -40,10 +40,10 @@ const splitTransferBuffer = transferBuffer => {
X`Internal error; actual overhead ${overheadLength} of bytes is not TRANSFER_OVERHEAD_LENGTH ${TRANSFER_OVERHEAD_LENGTH}`,
);
const databuf = new Uint8Array(transferBuffer, overheadLength);
assert(
databuf.byteLength >= MIN_DATA_BUFFER_LENGTH,
X`Transfer buffer of size ${transferBuffer.byteLength} only supports ${databuf.byteLength} data bytes; need at least ${MIN_DATA_BUFFER_LENGTH}`,
);
databuf.byteLength >= MIN_DATA_BUFFER_LENGTH ||
assert.fail(
X`Transfer buffer of size ${transferBuffer.byteLength} only supports ${databuf.byteLength} data bytes; need at least ${MIN_DATA_BUFFER_LENGTH}`,
);
return harden({ statusbuf, lenbuf, databuf });
};

Expand Down
48 changes: 24 additions & 24 deletions packages/captp/src/captp.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,10 @@ export const makeCapTP = (
// encounter deadlock. Without a lot more bookkeeping, we can't detect it for
// more general networks of CapTPs, but we are conservative for at least this
// one case.
assert(
!(trapHost && trapGuest),
X`CapTP ${ourId} can only be one of either trapGuest or trapHost`,
);
!(trapHost && trapGuest) ||
assert.fail(
X`CapTP ${ourId} can only be one of either trapGuest or trapHost`,
);

const disconnectReason = id =>
Error(`${JSON.stringify(id)} connection closed`);
Expand Down Expand Up @@ -391,10 +391,10 @@ export const makeCapTP = (
});
};
if (trap) {
assert(
exportedTrapHandlers.has(val),
X`Refused Trap(${val}) because target was not registered with makeTrapHandler`,
);
exportedTrapHandlers.has(val) ||
assert.fail(
X`Refused Trap(${val}) because target was not registered with makeTrapHandler`,
);
assert.typeof(
trapHost,
'function',
Expand Down Expand Up @@ -607,20 +607,18 @@ export const makeCapTP = (

// Create the Trap proxy maker.
const makeTrapImpl = implMethod => (target, ...implArgs) => {
assert(
Promise.resolve(target) !== target,
X`Trap(${target}) target cannot be a promise`,
);
Promise.resolve(target) !== target ||
assert.fail(X`Trap(${target}) target cannot be a promise`);

const slot = valToSlot.get(target);
assert(
slot && slot[1] === '-',
X`Trap(${target}) target was not imported`,
);
assert(
slot[0] === 't',
X`Trap(${target}) imported target was not created with makeTrapHandler`,
);
(slot && slot[1] === '-') ||
assert.fail(X`Trap(${target}) target was not imported`);
// @ts-expect-error TS doesn't realize the above check guarantees
// that slot is not undefined if we reach here.
slot[0] === 't' ||
assert.fail(
X`Trap(${target}) imported target was not created with makeTrapHandler`,
);

// Send a "trap" message.
lastQuestionID += 1;
Expand Down Expand Up @@ -653,6 +651,8 @@ export const makeCapTP = (
// messages over the current CapTP data channel.
const [isException, serialized] = trapGuest({
trapMethod: implMethod,
// @ts-expect-error TS doesn't realize a check above guarantees
// that slot is not undefined if we reach here.
slot,
trapArgs: implArgs,
startTrap: () => {
Expand Down Expand Up @@ -685,10 +685,10 @@ export const makeCapTP = (
});

const value = unserialize(serialized);
assert(
!isThenable(value),
X`Trap(${target}) reply cannot be a Thenable; have ${value}`,
);
!isThenable(value) ||
assert.fail(
X`Trap(${target}) reply cannot be a Thenable; have ${value}`,
);

if (isException) {
throw value;
Expand Down
6 changes: 2 additions & 4 deletions packages/import-bundle/test/test-compartment-wrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,8 @@ const { details: X } = assert;
// use cases that don't involve voluntary participation by oldSrc.

function milageTransform(oldSrc) {
assert(
oldSrc.indexOf('getOdometer') === -1,
X`forbidden access to 'getOdometer' in oldSrc`,
);
oldSrc.indexOf('getOdometer') === -1 ||
assert.fail(X`forbidden access to 'getOdometer' in oldSrc`);
return oldSrc.replace(/addMilage\(\)/g, 'getOdometer().add(1)');
}

Expand Down
22 changes: 10 additions & 12 deletions packages/marshal/src/encodeToCapData.js
Original file line number Diff line number Diff line change
Expand Up @@ -372,10 +372,8 @@ export const makeDecodeFromCapData = ({
// @ts-ignore inadequate type inference
// See https://github.com/endojs/endo/pull/1259#discussion_r954561901
const { original, rest } = jsonEncoded;
assert(
hasOwnPropertyOf(jsonEncoded, 'original'),
X`Invalid Hilbert Hotel encoding ${jsonEncoded}`,
);
hasOwnPropertyOf(jsonEncoded, 'original') ||
assert.fail(X`Invalid Hilbert Hotel encoding ${jsonEncoded}`);
// Don't harden since we're not done mutating it
const result = { [QCLASS]: decodeFromCapData(original) };
if (hasOwnPropertyOf(jsonEncoded, 'rest')) {
Expand All @@ -389,20 +387,20 @@ export const makeDecodeFromCapData = ({
// TODO really should assert that `passStyleOf(rest)` is
// `'copyRecord'` but we'd have to harden it and it is too
// early to do that.
assert(
!hasOwnPropertyOf(restObj, QCLASS),
X`Rest must not contain its own definition of ${q(QCLASS)}`,
);
!hasOwnPropertyOf(restObj, QCLASS) ||
assert.fail(
X`Rest must not contain its own definition of ${q(QCLASS)}`,
);
defineProperties(result, getOwnPropertyDescriptors(restObj));
}
return result;
}

default: {
assert(
qclass !== 'ibid',
X`The protocol no longer supports ibid encoding: ${jsonEncoded}.`,
);
qclass !== 'ibid' ||
assert.fail(
X`The protocol no longer supports ibid encoding: ${jsonEncoded}.`,
);
assert.fail(X`unrecognized ${q(QCLASS)} ${q(qclass)}`, TypeError);
}
}
Expand Down
32 changes: 16 additions & 16 deletions packages/marshal/src/helpers/passStyle-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,12 @@ export const checkNormalProperty = (
nameType,
)}-named property: ${candidate}`,
) &&
check(
hasOwnPropertyOf(desc, 'value'),
X`${q(propertyName)} must not be an accessor property: ${candidate}`,
) &&
(hasOwnPropertyOf(desc, 'value') ||
false ||
check(
false,
X`${q(propertyName)} must not be an accessor property: ${candidate}`,
)) &&
(shouldBeEnumerable
? check(
!!desc.enumerable,
Expand All @@ -111,15 +113,11 @@ harden(getTag);
*/
export const checkTagRecord = (tagRecord, passStyle, check) => {
return (
check(
typeof tagRecord === 'object' && tagRecord !== null,
X`A non-object cannot be a tagRecord: ${tagRecord}`,
) &&
((typeof tagRecord === 'object' && tagRecord !== null) ||
check(false, X`A non-object cannot be a tagRecord: ${tagRecord}`)) &&
check(isFrozen(tagRecord), X`A tagRecord must be frozen: ${tagRecord}`) &&
check(
!isArray(tagRecord),
X`An array cannot be a tagRecords: ${tagRecord}`,
) &&
(!isArray(tagRecord) ||
check(false, X`An array cannot be a tagRecords: ${tagRecord}`)) &&
checkNormalProperty(tagRecord, PASS_STYLE, 'symbol', false, check) &&
check(
tagRecord[PASS_STYLE] === passStyle,
Expand All @@ -134,10 +132,12 @@ export const checkTagRecord = (tagRecord, passStyle, check) => {
false,
check,
) &&
check(
typeof getTag(tagRecord) === 'string',
X`A [Symbol.toString]-named property must be a string: ${tagRecord}`,
)
(typeof getTag(tagRecord) === 'string' ||
false ||
check(
false,
X`A [Symbol.toString]-named property must be a string: ${tagRecord}`,
))
);
};
harden(checkTagRecord);
50 changes: 26 additions & 24 deletions packages/marshal/src/helpers/remotable.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,11 @@ const {
const checkIface = (iface, check) => {
return (
// TODO other possible ifaces, once we have third party veracity
check(
typeof iface === 'string',
X`For now, interface ${iface} must be a string; unimplemented`,
) &&
(typeof iface === 'string' ||
check(
false,
X`For now, interface ${iface} must be a string; unimplemented`,
)) &&
check(
iface === 'Remotable' || iface.startsWith('Alleged: '),
X`For now, iface ${q(
Expand Down Expand Up @@ -144,10 +145,12 @@ const checkRemotableProtoOf = (original, check) => {
} = proto;

return (
check(
ownKeys(rest).length === 0,
X`Unexpected properties on Remotable Proto ${ownKeys(rest)}`,
) && checkIface(iface, check)
(ownKeys(rest).length === 0 ||
check(
false,
X`Unexpected properties on Remotable Proto ${ownKeys(rest)}`,
)) &&
checkIface(iface, check)
);
};

Expand Down Expand Up @@ -195,10 +198,9 @@ export const RemotableHelper = harden({
canBeValid: (candidate, check) => {
if (
!(
check(
isObject(candidate),
X`cannot serialize non-objects like ${candidate}`,
) && check(!isArray(candidate), X`Arrays cannot be pass-by-remote`)
(isObject(candidate) ||
check(false, X`cannot serialize non-objects like ${candidate}`)) &&
check(!isArray(candidate), X`Arrays cannot be pass-by-remote`)
)
) {
return false;
Expand All @@ -222,10 +224,8 @@ export const RemotableHelper = harden({
String(key),
)} in ${candidate}`,
) &&
check(
key !== PASS_STYLE,
X`A pass-by-remote cannot shadow ${q(PASS_STYLE)}`,
),
(key !== PASS_STYLE ||
check(false, X`A pass-by-remote cannot shadow ${q(PASS_STYLE)}`)),
);
} else if (typeof candidate === 'function') {
// Far functions cannot be methods, and cannot have methods.
Expand All @@ -237,14 +237,16 @@ export const RemotableHelper = harden({
nameDesc && typeof nameDesc.value === 'string',
X`Far function name must be a string, in ${candidate}`,
) &&
check(
lengthDesc && typeof lengthDesc.value === 'number',
X`Far function length must be a number, in ${candidate}`,
) &&
check(
restKeys.length === 0,
X`Far functions unexpected properties besides .name and .length ${restKeys}`,
))
((lengthDesc && typeof lengthDesc.value === 'number') ||
check(
false,
X`Far function length must be a number, in ${candidate}`,
)) &&
(restKeys.length === 0 ||
check(
false,
X`Far functions unexpected properties besides .name and .length ${restKeys}`,
)))
);
} else {
return check(false, X`unrecognized typeof ${candidate}`);
Expand Down
10 changes: 6 additions & 4 deletions packages/marshal/src/helpers/safe-promise.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,12 @@ const checkPromiseOwnKeys = (pr, check) => {
const checkSafePromise = (pr, check) =>
check(isFrozen(pr), X`${pr} - Must be frozen`) &&
check(isPromise(pr), X`${pr} - Must be a promise`) &&
check(
getPrototypeOf(pr) === Promise.prototype,
X`${pr} - Must inherit from Promise.prototype: ${q(getPrototypeOf(pr))}`,
) &&
(getPrototypeOf(pr) === Promise.prototype ||
false ||
check(
false,
X`${pr} - Must inherit from Promise.prototype: ${q(getPrototypeOf(pr))}`,
)) &&
checkPromiseOwnKeys(/** @type {Promise} */ (pr), check);
harden(checkSafePromise);

Expand Down
4 changes: 2 additions & 2 deletions packages/marshal/src/helpers/symbol.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ export const isPassableSymbol = sym =>
harden(isPassableSymbol);

export const assertPassableSymbol = sym =>
assert(
isPassableSymbol(sym),
isPassableSymbol(sym) ||
assert.fail(
X`Only registered symbols or well-known symbols are passable: ${q(sym)}`,
);
harden(assertPassableSymbol);
Expand Down
7 changes: 2 additions & 5 deletions packages/marshal/src/helpers/tagged.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,8 @@ export const TaggedHelper = harden({
payload: _payload, // value checked by recursive walk at the end
...rest
} = candidate;

assert(
ownKeys(rest).length === 0,
X`Unexpected properties on Remotable Proto ${ownKeys(rest)}`,
);
(ownKeys(rest).length === 0) ||
assert.fail(X`Unexpected properties on Remotable Proto ${ownKeys(rest)}`);

checkNormalProperty(candidate, 'payload', 'string', true, assertChecker);

Expand Down
17 changes: 9 additions & 8 deletions packages/marshal/src/make-far.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,16 @@ const makeRemotableProto = (remotable, iface) => {
if (oldProto === null) {
oldProto = objectPrototype;
}
assert(
oldProto === objectPrototype || oldProto === null,
X`For now, remotables cannot inherit from anything unusual, in ${remotable}`,
);
oldProto === objectPrototype ||
oldProto === null ||
assert.fail(
X`For now, remotables cannot inherit from anything unusual, in ${remotable}`,
);
} else if (typeof remotable === 'function') {
assert(
oldProto !== null,
X`Original function must not inherit from null: ${remotable}`,
);
oldProto !== null ||
assert.fail(
X`Original function must not inherit from null: ${remotable}`,
);
assert(
oldProto === functionPrototype ||
getPrototypeOf(oldProto) === functionPrototype,
Expand Down
Loading

0 comments on commit b66bbfc

Please sign in to comment.