Skip to content

Commit

Permalink
refactor: use Fail template tag in ERTP and zoe (#6709)
Browse files Browse the repository at this point in the history
* refactor: use Fail template tag in ERTP and zoe

* refactor: repair damage
  • Loading branch information
erights authored Dec 22, 2022
1 parent e74278c commit ca0b874
Show file tree
Hide file tree
Showing 37 changed files with 164 additions and 254 deletions.
51 changes: 21 additions & 30 deletions packages/ERTP/src/amountMath.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { setMathHelpers } from './mathHelpers/setMathHelpers.js';
import { copySetMathHelpers } from './mathHelpers/copySetMathHelpers.js';
import { copyBagMathHelpers } from './mathHelpers/copyBagMathHelpers.js';

const { details: X, quote: q } = assert;
const { quote: q, Fail } = assert;

/**
* Constants for the kinds of assets we support.
Expand All @@ -27,9 +27,7 @@ const assetKindNames = harden(Object.values(AssetKind).sort());
*/
const assertAssetKind = allegedAK => {
assetKindNames.includes(allegedAK) ||
assert.fail(
X`The assetKind ${allegedAK} must be one of ${q(assetKindNames)}`,
);
Fail`The assetKind ${allegedAK} must be one of ${q(assetKindNames)}`;
};
harden(assertAssetKind);

Expand Down Expand Up @@ -101,14 +99,14 @@ const assertValueGetAssetKind = value => {
// @ts-expect-error cast
return 'copyBag';
}
assert.fail(
// TODO This isn't quite the right error message, in case valuePassStyle
// is 'tagged'. We would need to distinguish what kind of tagged
// object it is.
// Also, this kind of manual listing is a maintenance hazard we
// (TODO) will encounter when we extend the math helpers further.
X`value ${value} must be a bigint, copySet, copyBag, or an array, not ${passStyle}`,
);
// TODO This isn't quite the right error message, in case valuePassStyle
// is 'tagged'. We would need to distinguish what kind of tagged
// object it is.
// Also, this kind of manual listing is a maintenance hazard we
// (TODO) will encounter when we extend the math helpers further.
throw Fail`value ${value} must be a bigint, copySet, copyBag, or an array, not ${q(
passStyle,
)}`;
};

/**
Expand All @@ -129,11 +127,10 @@ export const assertValueGetHelpers = value =>
const optionalBrandCheck = (allegedBrand, brand) => {
if (brand !== undefined) {
assertRemotable(brand, 'brand');
assert.equal(
allegedBrand,
brand,
X`amount's brand ${allegedBrand} did not match expected brand ${brand}`,
);
allegedBrand === brand ||
Fail`amount's brand ${q(allegedBrand)} did not match expected brand ${q(
brand,
)}`;
}
};

Expand All @@ -153,18 +150,14 @@ const checkLRAndGetHelpers = (leftAmount, rightAmount, brand = undefined) => {
assertRemotable(rightBrand, 'rightBrand');
optionalBrandCheck(leftBrand, brand);
optionalBrandCheck(rightBrand, brand);
assert.equal(
leftBrand,
rightBrand,
X`Brands in left ${leftBrand} and right ${rightBrand} should match but do not`,
);
leftBrand === rightBrand ||
Fail`Brands in left ${q(leftBrand)} and right ${q(
rightBrand,
)} should match but do not`;
const leftHelpers = assertValueGetHelpers(leftValue);
const rightHelpers = assertValueGetHelpers(rightValue);
assert.equal(
leftHelpers,
rightHelpers,
X`The left ${leftAmount} and right amount ${rightAmount} had different assetKinds`,
);
leftHelpers === rightHelpers ||
Fail`The left ${leftAmount} and right amount ${rightAmount} had different assetKinds`;
return leftHelpers;
};

Expand Down Expand Up @@ -219,9 +212,7 @@ const AmountMath = {
assertRecord(allegedAmount, 'amount');
const { brand: allegedBrand, value: allegedValue } = allegedAmount;
brand === allegedBrand ||
assert.fail(
X`The brand in the allegedAmount ${allegedAmount} in 'coerce' didn't match the specified brand ${brand}.`,
);
Fail`The brand in the allegedAmount ${allegedAmount} in 'coerce' didn't match the specified brand ${brand}.`;
// Will throw on inappropriate value
return AmountMath.make(brand, allegedValue);
},
Expand Down
10 changes: 3 additions & 7 deletions packages/ERTP/src/displayInfo.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { assert, details as X } from '@agoric/assert';
import { Fail } from '@agoric/assert';
import { fit } from '@agoric/store';

import { DisplayInfoShape } from './typeGuards.js';
Expand All @@ -13,17 +13,13 @@ export const coerceDisplayInfo = (allegedDisplayInfo, assetKind) => {

if (allegedDisplayInfo.assetKind !== undefined) {
allegedDisplayInfo.assetKind === assetKind ||
assert.fail(
X`displayInfo.assetKind was present (${allegedDisplayInfo.assetKind}) and did not match the assetKind argument (${assetKind})`,
);
Fail`displayInfo.assetKind was present (${allegedDisplayInfo.assetKind}) and did not match the assetKind argument (${assetKind})`;
}
const displayInfo = harden({ ...allegedDisplayInfo, assetKind });

if (displayInfo.decimalPlaces !== undefined) {
Number.isSafeInteger(displayInfo.decimalPlaces) ||
assert.fail(
X`decimalPlaces ${displayInfo.decimalPlaces} is not a safe integer`,
);
Fail`decimalPlaces ${displayInfo.decimalPlaces} is not a safe integer`;
}

return displayInfo;
Expand Down
4 changes: 2 additions & 2 deletions packages/ERTP/src/mathHelpers/natMathHelpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Nat, isNat } from '@agoric/nat';

import '../types-ambient.js';

const { details: X } = assert;
const { Fail } = assert;
const empty = 0n;

/**
Expand All @@ -21,7 +21,7 @@ export const natMathHelpers = harden({
doCoerce: nat => {
// TODO: tighten the definition of Nat in @agoric/nat to throw on `number`
assert.typeof(nat, 'bigint');
assert(isNat(nat), X`value ${nat} must be a natural number`);
isNat(nat) || Fail`value ${nat} must be a natural number`;
return Nat(nat);
},
doMakeEmpty: () => empty,
Expand Down
24 changes: 9 additions & 15 deletions packages/ERTP/src/paymentLedger.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,15 @@ import { BrandI, makeIssuerInterfaces } from './typeGuards.js';

/** @typedef {import('@agoric/vat-data').Baggage} Baggage */

const { details: X, quote: q } = assert;
const { details: X, quote: q, Fail } = assert;

const amountShapeFromElementShape = (brand, assetKind, elementShape) => {
let valueShape;
switch (assetKind) {
case 'nat': {
valueShape = M.nat();
elementShape === undefined ||
assert.fail(
X`Fungible assets cannot have an elementShape: ${q(elementShape)}`,
);
Fail`Fungible assets cannot have an elementShape: ${q(elementShape)}`;
break;
}
case 'set': {
Expand Down Expand Up @@ -53,7 +51,7 @@ const amountShapeFromElementShape = (brand, assetKind, elementShape) => {
break;
}
default: {
assert.fail(X`unexpected asset kind ${q(assetKind)}`);
Fail`unexpected asset kind ${q(assetKind)}`;
}
}

Expand Down Expand Up @@ -230,11 +228,9 @@ export const vivifyPaymentLedger = (
*/
const assertLivePayment = payment => {
paymentLedger.has(payment) ||
assert.fail(
X`${payment} was not a live payment for brand ${q(
brand,
)}. It could be a used-up payment, a payment for another brand, or it might not be a payment at all.`,
);
Fail`${payment} was not a live payment for brand ${q(
brand,
)}. It could be a used-up payment, a payment for another brand, or it might not be a payment at all.`;
};

/**
Expand Down Expand Up @@ -266,7 +262,7 @@ export const vivifyPaymentLedger = (
const antiAliasingStore = new Set();
payments.forEach(payment => {
!antiAliasingStore.has(payment) ||
assert.fail(X`same payment ${payment} seen twice`);
Fail`same payment ${payment} seen twice`;
antiAliasingStore.add(payment);
});
}
Expand All @@ -277,7 +273,7 @@ export const vivifyPaymentLedger = (

// Invariant check
isEqual(total, newTotal) ||
assert.fail(X`rights were not conserved: ${total} vs ${newTotal}`);
Fail`rights were not conserved: ${total} vs ${newTotal}`;

let newPayments;
try {
Expand Down Expand Up @@ -354,9 +350,7 @@ export const vivifyPaymentLedger = (
) => {
amount = coerce(amount);
AmountMath.isGTE(currentBalance, amount) ||
assert.fail(
X`Withdrawal of ${amount} failed because the purse only contained ${currentBalance}`,
);
Fail`Withdrawal of ${amount} failed because the purse only contained ${currentBalance}`;
const newPurseBalance = subtract(currentBalance, amount);

const payment = makePayment();
Expand Down
6 changes: 2 additions & 4 deletions packages/ERTP/src/purse.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { vivifyFarClassKit, makeScalarBigSetStore } from '@agoric/vat-data';
import { AmountMath } from './amountMath.js';
import { makeTransientNotifierKit } from './transientNotifier.js';

const { details: X } = assert;
const { Fail } = assert;

export const vivifyPurseKind = (
issuerBaggage,
Expand Down Expand Up @@ -99,9 +99,7 @@ export const vivifyPurseKind = (
amount = AmountMath.add(amount, delta, brand);
}
state.recoverySet.getSize() === 0 ||
assert.fail(
X`internal: Remaining unrecovered payments: ${facets.purse.getRecoverySet()}`,
);
Fail`internal: Remaining unrecovered payments: ${facets.purse.getRecoverySet()}`;
return amount;
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const doTest = (t, x, y, deltaY, expectedDeltaX) => {

test('0, 0, 0, 0', t => {
t.throws(() => doTest(t, 0n, 0n, 0n, 0n), {
message: 'No infinite ratios! Denominator was 0/"[Alleged: BLD brand]"',
message: 'No infinite ratios! Denominator was 0 "[Alleged: BLD brand]"',
});
});

Expand All @@ -28,7 +28,7 @@ test('0, 0, 1, 0', t => {

test('1, 0, 0, 0', t => {
t.throws(() => doTest(t, 1n, 0n, 0n, 0n), {
message: 'No infinite ratios! Denominator was 0/"[Alleged: BLD brand]"',
message: 'No infinite ratios! Denominator was 0 "[Alleged: BLD brand]"',
});
});

Expand All @@ -42,7 +42,7 @@ test('1, 1, 0, 0', t => {

test('1, 1, 1, 0', t => {
t.throws(() => doTest(t, 1n, 1n, 1n, 0n), {
message: 'No infinite ratios! Denominator was 0/"[Alleged: BLD brand]"',
message: 'No infinite ratios! Denominator was 0 "[Alleged: BLD brand]"',
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const doTest = (t, x, y, deltaX, expectedDeltaY) => {
// deltaXPlusX is 0
test('0, 0, 0, 0', t => {
t.throws(() => doTest(t, 0n, 0n, 0n, 0n), {
message: 'No infinite ratios! Denominator was 0/"[Alleged: RUN brand]"',
message: 'No infinite ratios! Denominator was 0 "[Alleged: RUN brand]"',
});
});

Expand All @@ -32,7 +32,7 @@ test('1, 0, 0, 0', t => {
// deltaXPlusX is 0
test('0, 1, 0, 0', t => {
t.throws(() => doTest(t, 0n, 1n, 0n, 0n), {
message: 'No infinite ratios! Denominator was 0/"[Alleged: RUN brand]"',
message: 'No infinite ratios! Denominator was 0 "[Alleged: RUN brand]"',
});
});

Expand Down
2 changes: 1 addition & 1 deletion packages/ui-components/src/ratio.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export const makeRatio = (
denominatorBrand = numeratorBrand,
) => {
denominator > 0n ||
Fail`No infinite ratios! Denominator was 0/${q(denominatorBrand)}`;
Fail`No infinite ratios! Denominator was 0 ${q(denominatorBrand)}`;

// @ts-expect-error cast to return type because make() ensures
return harden({
Expand Down
32 changes: 12 additions & 20 deletions packages/zoe/src/cleanProposal.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { assert, details as X, q } from '@agoric/assert';
import { assert, q, Fail } from '@agoric/assert';
import { AmountMath, getAssetKind } from '@agoric/ertp';
import { assertRecord } from '@endo/marshal';
import { assertKey, assertPattern, fit, isKey } from '@agoric/store';
Expand All @@ -25,19 +25,15 @@ const firstCapASCII = /^[A-Z][a-zA-Z0-9_$]*$/;
export const assertKeywordName = keyword => {
assert.typeof(keyword, 'string');
keyword.length <= MAX_KEYWORD_LENGTH ||
assert.fail(
X`keyword ${q(keyword)} exceeded maximum length ${q(
MAX_KEYWORD_LENGTH,
)} characters; got ${keyword.length}`,
);
assert(
firstCapASCII.test(keyword),
X`keyword ${q(
Fail`keyword ${q(keyword)} exceeded maximum length ${q(
MAX_KEYWORD_LENGTH,
)} characters; got ${keyword.length}`;
firstCapASCII.test(keyword) ||
Fail`keyword ${q(
keyword,
)} must be an ascii identifier starting with upper case.`,
);
)} must be an ascii identifier starting with upper case.`;
(keyword !== 'NaN' && keyword !== 'Infinity') ||
assert.fail(X`keyword ${q(keyword)} must not be a number's name`);
Fail`keyword ${q(keyword)} must not be a number's name`;
};

/**
Expand Down Expand Up @@ -72,9 +68,7 @@ export const coerceAmountPatternKeywordRecord = (
// TODO: replace this assertion with a check of the assetKind
// property on the brand, when that exists.
assetKind === brandAssetKind ||
assert.fail(
X`The amount ${amount} did not have the assetKind of the brand ${brandAssetKind}`,
);
Fail`The amount ${amount} did not have the assetKind of the brand ${brandAssetKind}`;
return AmountMath.coerce(amount.brand, amount);
} else {
assertPattern(amount);
Expand Down Expand Up @@ -106,7 +100,7 @@ export const coerceAmountKeywordRecord = (
* @param {ExitRule} exit
*/
const assertExit = exit =>
assert(ownKeys(exit).length === 1, X`exit ${exit} should only have one key`);
ownKeys(exit).length === 1 || Fail`exit ${exit} should only have one key`;

/**
* check that keyword is not in both 'want' and 'give'.
Expand All @@ -120,7 +114,7 @@ const assertKeywordNotInBoth = (want, give) => {

giveKeywords.forEach(keyword => {
!wantKeywordSet.has(keyword) ||
assert.fail(X`a keyword cannot be in both 'want' and 'give'`);
Fail`a keyword cannot be in both 'want' and 'give'`;
});
};

Expand Down Expand Up @@ -150,9 +144,7 @@ export const cleanProposal = (proposal, getAssetKindByBrand) => {
...rest
} = proposal;
ownKeys(rest).length === 0 ||
assert.fail(
X`${proposal} - Must only have want:, give:, exit: properties: ${rest}`,
);
Fail`${proposal} - Must only have want:, give:, exit: properties: ${rest}`;

const cleanedWant = coerceAmountPatternKeywordRecord(
want,
Expand Down
18 changes: 7 additions & 11 deletions packages/zoe/src/contractFacet/allocationMath.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { AmountMath } from '@agoric/ertp';

const { details: X, quote: q } = assert;
const { Fail, quote: q } = assert;

/**
* @callback Operation
Expand Down Expand Up @@ -63,18 +63,14 @@ const subtract = (amount, amountToSubtract, keyword) => {
if (amount === undefined) {
// TypeScript confused about `||` control flow so use `if` instead
// https://github.com/microsoft/TypeScript/issues/50739
assert.fail(
X`The amount could not be subtracted from the allocation because the allocation did not have an amount under the keyword ${q(
keyword,
)}.`,
);
throw Fail`The amount could not be subtracted from the allocation because the allocation did not have an amount under the keyword ${q(
keyword,
)}.`;
}
AmountMath.isGTE(amount, amountToSubtract) ||
assert.fail(
X`The amount to be subtracted ${amountToSubtract} was greater than the allocation's amount ${amount} for the keyword ${q(
keyword,
)}`,
);
Fail`The amount to be subtracted ${amountToSubtract} was greater than the allocation's amount ${amount} for the keyword ${q(
keyword,
)}`;
return AmountMath.subtract(amount, amountToSubtract);
}
};
Expand Down
Loading

0 comments on commit ca0b874

Please sign in to comment.