chore: address PR comments

packages/ERTP/src/amountMath.js

@@ -183,7 +183,7 @@ const AmountMath = {
   isEmpty: (amount, brand = undefined) => {
     assertRecord(amount, 'amount');
     const { brand: allegedBrand, value } = amount;
-    assertRemotable(allegedBrand);
+    assertRemotable(allegedBrand, 'brand');
     optionalBrandCheck(allegedBrand, brand);
     const h = assertValueGetHelpers(value);
     // @ts-ignore Needs better typing to express Value to Helpers relationship

packages/ERTP/src/displayInfo.js

@@ -37,6 +37,12 @@ export const coerceDisplayInfo = (allegedDisplayInfo, assetKind) => {
   // `pureCopy` works in this case because displayInfo is a copyRecord
   // that is pure data, meaning no remotables and no promises.
   allegedDisplayInfo = pureCopy(allegedDisplayInfo);
+  if (allegedDisplayInfo.assetKind !== undefined) {
+    assert(
+      allegedDisplayInfo.assetKind === assetKind,
+      X`displayInfo.assetKind was present (${allegedDisplayInfo.assetKind}) and did not match the assetKind argument (${assetKind})`,
+    );
+  }
   const displayInfo = harden({ ...allegedDisplayInfo, assetKind });
 
   assertSubset(displayInfoKeys, Object.keys(displayInfo));

packages/ERTP/src/paymentLedger.js

@@ -3,7 +3,7 @@
 import { assert, details as X } from '@agoric/assert';
 import { E } from '@agoric/eventual-send';
 import { isPromise } from '@agoric/promise-kit';
-import { Far, assertArray } from '@agoric/marshal';
+import { Far, assertCopyArray } from '@agoric/marshal';
 import { makeWeakStore } from '@agoric/store';
 
 import { AmountMath } from './amountMath.js';
@@ -100,8 +100,8 @@ export const makePaymentLedger = (
    * @returns {Payment[]}
    */
   const moveAssets = (payments, newPaymentBalances) => {
-    assertArray(payments, 'payments');
-    assertArray(newPaymentBalances, 'newPaymentBalances');
+    assertCopyArray(payments, 'payments');
+    assertCopyArray(newPaymentBalances, 'newPaymentBalances');
 
     // There may be zero, one, or many payments as input to
     // moveAssets. We want to protect against someone passing in
@@ -198,7 +198,7 @@ export const makePaymentLedger = (
 
   /** @type {IssuerCombine} */
   const combine = (fromPaymentsPArray, optTotalAmount = undefined) => {
-    assertArray(fromPaymentsPArray, 'fromPaymentsArray');
+    assertCopyArray(fromPaymentsPArray, 'fromPaymentsArray');
     // Payments in `fromPaymentsPArray` must be distinct. Alias
     // checking is delegated to the `moveAssets` function.
     return Promise.all(fromPaymentsPArray).then(fromPaymentsArray => {
@@ -237,7 +237,7 @@ export const makePaymentLedger = (
   const splitMany = (paymentP, amounts) => {
     return E.when(paymentP, srcPayment => {
       assertLivePayment(srcPayment);
-      assertArray(amounts, 'amounts');
+      assertCopyArray(amounts, 'amounts');
       amounts = amounts.map(coerce);
       // Note COMMIT POINT within moveAssets.
       const newPayments = moveAssets(harden([srcPayment]), harden(amounts));

packages/ERTP/src/types.js

@@ -46,27 +46,20 @@
  * This section blindly imitates what Endo's ses/src/error/types.js
  * does to express type overloaded methods.
  *
- * @callback AmountMakeBrandValue
+ * @callback AmountMake
  * @param {Brand} brand
  * @param {Value} allegedValue
  * @returns {Amount}
  *
- * @typedef {AmountMakeBrandValue} AmountMake
- *
- * @callback AmountCoerceBrandAmount
+ * @callback AmountCoerce
  * @param {Brand} brand
  * @param {Amount} allegedAmount
  * @returns {Amount}
  *
- * @typedef {AmountCoerceBrandAmount} AmountCoerce
- *
- * @callback AmountGetValueBrandAmount
+ * @callback AmountGetValue
  * @param {Brand} brand
  * @param {Amount} allegedAmount
  * @returns {Value}
- *
- *
- * @typedef {AmountGetValueBrandAmount} AmountGetValue
  */
 
 /**
@@ -302,7 +295,6 @@
 /**
  * @typedef {Object} AdditionalDisplayInfo
  *
- * Does not include `assetKind`, which is automatically added in MakeIssuerKit
  * @property {number=} decimalPlaces Tells the display software how
  *   many decimal places to move the decimal over to the left, or in
  *   other words, which position corresponds to whole numbers. We
@@ -313,6 +305,7 @@
  *   assets, should not be specified. The decimalPlaces property
  *   should be used for *display purposes only*. Any other use is an
  *   anti-pattern.
+ * @property {AssetKind=} assetKind
  */
 
 /**

packages/ERTP/test/unitTests/test-inputValidation.js

@@ -34,18 +34,21 @@ test('makeIssuerKit bad displayInfo.decimalPlaces', async t => {
 });
 
 test('makeIssuerKit bad displayInfo.assetKind', async t => {
-  // The bad assetKind gets silently overridden
-  const { issuer } = makeIssuerKit(
-    'myTokens',
-    AssetKind.NAT,
-    // @ts-ignore Intentional wrong type for testing
-    harden({
-      assetKind: 'something',
-    }),
+  t.throws(
+    () =>
+      makeIssuerKit(
+        'myTokens',
+        AssetKind.NAT,
+        // @ts-ignore Intentional wrong type for testing
+        harden({
+          assetKind: 'something',
+        }),
+      ),
+    {
+      message:
+        'displayInfo.assetKind was present ("something") and did not match the assetKind argument ("nat")',
+    },
   );
-  t.deepEqual(issuer.getDisplayInfo(), {
-    assetKind: 'nat',
-  });
 });
 
 test('makeIssuerKit bad displayInfo.whatever', async t => {

packages/marshal/index.js

@@ -19,9 +19,9 @@ export {
 } from './src/structure.js';
 export {
   assertRecord,
-  assertArray,
+  assertCopyArray,
   assertRemotable,
   isRemotable,
   isRecord,
-  isArray,
+  isCopyArray,
 } from './src/assertPassStyleOf.js';

packages/marshal/src/assertPassStyleOf.js

@@ -19,8 +19,8 @@ const { details: X, quote: q } = assert;
  * @param {CopyArray} array
  * @returns {boolean}
  */
-const isArray = array => passStyleOf(array) === 'copyArray';
-harden(isArray);
+const isCopyArray = array => passStyleOf(array) === 'copyArray';
+harden(isCopyArray);
 
 /**
  * Check whether the argument is a pass-by-copy record, AKA a
@@ -49,7 +49,7 @@ harden(isRemotable);
  * @param {string=} optNameOfArray
  * @returns {void}
  */
-const assertArray = (array, optNameOfArray = 'Alleged array') => {
+const assertCopyArray = (array, optNameOfArray = 'Alleged array') => {
   const passStyle = passStyleOf(array);
   assert(
     passStyle === 'copyArray',
@@ -58,7 +58,7 @@ const assertArray = (array, optNameOfArray = 'Alleged array') => {
     )} ${array} must be a pass-by-copy array, not ${passStyle}`,
   );
 };
-harden(assertArray);
+harden(assertCopyArray);
 
 /**
  * Assert that the argument is a pass-by-copy record, or a
@@ -102,9 +102,9 @@ harden(assertRemotable);
 
 export {
   assertRecord,
-  assertArray,
+  assertCopyArray,
   assertRemotable,
   isRemotable,
   isRecord,
-  isArray,
+  isCopyArray,
 };

packages/zoe/src/contractSupport/ratio.js

@@ -3,8 +3,8 @@
 import './types.js';
 import { assert, details as X, q } from '@agoric/assert';
 import { AmountMath } from '@agoric/ertp';
-import natMathHelpers from '@agoric/ertp/src/mathHelpers/natMathHelpers.js';
 import { assertRecord } from '@agoric/marshal';
+import { isNat } from '@agoric/nat';
 
 import { natSafeMath } from './safeMath.js';
 
@@ -49,8 +49,16 @@ export const assertIsRatio = ratio => {
       X`Parameter must be a Ratio record, but ${ratio} has ${q(name)}`,
     );
   }
-  natMathHelpers.doCoerce(ratio.numerator.value);
-  natMathHelpers.doCoerce(ratio.denominator.value);
+  const numeratorValue = ratio.numerator.value;
+  const denominatorValue = ratio.denominator.value;
+  assert(
+    isNat(numeratorValue),
+    X`The numerator value must be a NatValue, not ${numeratorValue}`,
+  );
+  assert(
+    isNat(denominatorValue),
+    X`The denominator value must be a NatValue, not ${denominatorValue}`,
+  );
 };
 
 /** @type {MakeRatio} */