Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: alt syntax for positive but faster assertions #1280

Merged
merged 2 commits into from
Sep 12, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
30 changes: 14 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,11 @@ export const checkNormalProperty = (
nameType,
)}-named property: ${candidate}`,
) &&
check(
hasOwnPropertyOf(desc, 'value'),
X`${q(propertyName)} must not be an accessor property: ${candidate}`,
) &&
(hasOwnPropertyOf(desc, 'value') ||
check(
false,
X`${q(propertyName)} must not be an accessor property: ${candidate}`,
)) &&
(shouldBeEnumerable
? check(
!!desc.enumerable,
Expand All @@ -111,15 +112,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 +131,11 @@ 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' ||
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
9 changes: 5 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,11 @@ 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 ||
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