fixup! refactor for containerHasSplit

Author: erights

packages/ERTP/src/amountMath.js

@@ -183,13 +183,13 @@ const coerceLR = (h, leftAmount, rightAmount) => {
  * For non-fungible or sem-fungible amounts, the right operand can also be an
  * `AmountBound` which can a normal concrete `Amount` or a specialized pattern:
  * A `RecordPattern` of a normal concrete `brand: Brand` and a `value:
- * AmountValueHasBound`, as made by `M.has(elementPattern)` or
- * `M.has(elementPattern, bigint)`. This represents those elements of the value
- * collection that match the elementPattern, if that number is exactly the same
- * as the bigint argument. If the second argument of `M.has` is omitted, it
- * defaults to `1n`. IOW, the left operand is `>=` such a bound if the total
- * number of elements in the left operand that match the element pattern is `>=`
- * the bigint argument in the `M.has` pattern.
+ * AmountValueHasBound`, as made by `M.containerHas(elementPattern)` or
+ * `M.containerHas(elementPattern, bigint)`. This represents those elements of
+ * the value collection that match the elementPattern, if that number is exactly
+ * the same as the bigint argument. If the second argument of `M.containerHas`
+ * is omitted, it defaults to `1n`. IOW, the left operand is `>=` such a bound
+ * if the total number of elements in the left operand that match the element
+ * pattern is `>=` the bigint argument in the `M.containerHas` pattern.
  *
  * @template {AssetKind} K
  * @param {Amount<K>} leftAmount

packages/ERTP/src/legacy-payment-helpers.js

@@ -40,6 +40,8 @@ export const claim = async (
 ) => {
   const srcPayment = await srcPaymentP;
   return E.when(E(recoveryPurse).deposit(srcPayment, optAmountShape), amount =>
+    // @ts-expect-error Why doesn't TS know that an Amount is a kind
+    // of AmountBound?
     E(recoveryPurse).withdraw(amount),
   );
 };
@@ -87,6 +89,8 @@ export const combine = async (
   if (optTotalAmount !== undefined) {
     mustMatch(total, optTotalAmount, 'amount');
   }
+  // @ts-expect-error Why doesn't TS know that an Amount is a kind
+  // of AmountBound?
   return E(recoveryPurse).withdraw(total);
 };
 harden(combine);
@@ -111,7 +115,11 @@ export const split = async (recoveryPurse, srcPaymentP, paymentAmountA) => {
   const srcAmount = await E(recoveryPurse).deposit(srcPayment);
   const paymentAmountB = AmountMath.subtract(srcAmount, paymentAmountA);
   return Promise.all([
+    // @ts-expect-error Why doesn't TS know that an Amount is a kind
+    // of AmountBound?
     E(recoveryPurse).withdraw(paymentAmountA),
+    // @ts-expect-error Why doesn't TS know that an Amount is a kind
+    // of AmountBound?
     E(recoveryPurse).withdraw(paymentAmountB),
   ]);
 };
@@ -147,6 +155,12 @@ export const splitMany = async (recoveryPurse, srcPaymentP, amounts) => {
   AmountMath.isEqual(srcAmount, total) ||
     Fail`rights were not conserved: ${total} vs ${srcAmount}`;
 
-  return Promise.all(amounts.map(amount => E(recoveryPurse).withdraw(amount)));
+  return Promise.all(
+    amounts.map(amount =>
+      // @ts-expect-error Why doesn't TS know that an Amount is a kind
+      // of AmountBound?
+      E(recoveryPurse).withdraw(amount),
+    ),
+  );
 };
 harden(splitMany);

packages/ERTP/src/mathHelpers/copySetMathHelpers.js

@@ -1,6 +1,9 @@
 // @jessie-check
 
+import { Fail } from '@endo/errors';
+import { assertChecker } from '@endo/pass-style';
 import {
+  isKey,
   keyEQ,
   makeCopySet,
   mustMatch,
@@ -9,7 +12,9 @@ import {
   setIsSuperset,
   setDisjointUnion,
   setDisjointSubtract,
-} from '@agoric/store';
+  containerHasSplit,
+  kindOf,
+} from '@endo/patterns';
 
 /**
  * @import {Key, CopySet} from '@endo/patterns'
@@ -27,8 +32,38 @@ export const copySetMathHelpers = harden({
   },
   doMakeEmpty: () => empty,
   doIsEmpty: set => getCopySetKeys(set).length === 0,
-  doIsGTE: setIsSuperset,
+  doIsGTE: (left, rightBound) => {
+    if (isKey(rightBound)) {
+      return setIsSuperset(left, rightBound);
+    }
+    kindOf(rightBound) === 'match:containerHas' ||
+      Fail`rightBound must either be a key or an M.containerHas pattern ${rightBound}`;
+    const {
+      payload: [elementPatt, bound],
+    } = rightBound;
+    return !!containerHasSplit(left, elementPatt, bound);
+  },
   doIsEqual: keyEQ,
   doAdd: setDisjointUnion,
-  doSubtract: setDisjointSubtract,
+  doSubtract: (left, rightBound) => {
+    if (isKey(rightBound)) {
+      return setDisjointSubtract(left, rightBound);
+    }
+    kindOf(rightBound) === 'match:containerHas' ||
+      Fail`rightBound must either be a key or an M.containerHas pattern ${rightBound}`;
+    const {
+      payload: [elementPatt, bound],
+    } = rightBound;
+    // Passing in `assertChecker` as the `check` argument should guarantee
+    // that the result is not `undefined`. It would have thrown first.
+    const [_, result] = containerHasSplit(
+      left,
+      elementPatt,
+      bound,
+      false,
+      true,
+      assertChecker,
+    );
+    return harden(result);
+  },
 });

packages/ERTP/src/mathHelpers/setMathHelpers.js

@@ -2,14 +2,17 @@
 
 import { Fail } from '@endo/errors';
 import { passStyleOf, assertChecker } from '@endo/pass-style';
-import { isKey, assertKey, elementsHasSplit, kindOf } from '@endo/patterns';
 import {
+  isKey,
+  assertKey,
   elementsIsSuperset,
   elementsDisjointUnion,
   elementsDisjointSubtract,
   coerceToElements,
   elementsCompare,
-} from '@agoric/store';
+  containerHasSplit,
+  kindOf,
+} from '@endo/patterns';
 
 /**
  * @import {Key} from '@endo/patterns'
@@ -41,31 +44,32 @@ export const setMathHelpers = harden({
     if (isKey(rightBound)) {
       return elementsIsSuperset(left, rightBound);
     }
-    kindOf(rightBound) === 'match:has' ||
-      Fail`rightBound must either be a key or an M.has pattern ${rightBound}`;
+    kindOf(rightBound) === 'match:containerHas' ||
+      Fail`rightBound must either be a key or an M.containerHas pattern ${rightBound}`;
     const {
       payload: [elementPatt, bound],
     } = rightBound;
-    return elementsHasSplit(left, elementPatt, bound);
+    return !!containerHasSplit(left, elementPatt, bound);
   },
   doIsEqual: (x, y) => elementsCompare(x, y) === 0,
   doAdd: elementsDisjointUnion,
   doSubtract: (left, rightBound) => {
     if (isKey(rightBound)) {
       return elementsDisjointSubtract(left, rightBound);
     }
-    kindOf(rightBound) === 'match:has' ||
-      Fail`rightBound must either be a key or an M.has pattern ${rightBound}`;
+    kindOf(rightBound) === 'match:containerHas' ||
+      Fail`rightBound must either be a key or an M.containerHas pattern ${rightBound}`;
     const {
       payload: [elementPatt, bound],
     } = rightBound;
-    const result = [];
-    elementsHasSplit(
+    // Passing in `assertChecker` as the `check` argument should guarantee
+    // that the result is not `undefined`. It would have thrown first.
+    const [_, result] = containerHasSplit(
       left,
       elementPatt,
       bound,
-      undefined,
-      result,
+      false,
+      true,
       assertChecker,
     );
     return harden(result);

packages/ERTP/src/paymentLedger.js

@@ -1,10 +1,8 @@
 // @jessie-check
 
-/// <reference types="@agoric/store/exported.js" />
-
 import { X, q, Fail, annotateError } from '@endo/errors';
 import { isPromise } from '@endo/promise-kit';
-import { mustMatch, M, keyEQ } from '@agoric/store';
+import { mustMatch, M, keyEQ } from '@endo/patterns';
 import { AmountMath } from './amountMath.js';
 import { preparePaymentKind } from './payment.js';
 import { preparePurseKind } from './purse.js';
@@ -13,7 +11,7 @@ import { BrandI, makeIssuerInterfaces } from './typeGuards.js';
 
 /**
  * @import {Key, Pattern} from '@endo/patterns';
- * @import {Amount, AssetKind, DisplayInfo, PaymentLedger, Payment, Brand, RecoverySetsOption, Purse, Issuer, Mint} from './types.js'
+ * @import {Amount, AssetKind, DisplayInfo, PaymentLedger, Payment, Brand, RecoverySetsOption, Purse, Issuer, Mint, AmountBound} from './types.js'
  * @import {ShutdownWithFailure} from '@agoric/swingset-vat'
  * @import {TypedPattern} from '@agoric/internal';
  */

packages/ERTP/src/purse.js

@@ -108,16 +108,16 @@ export const preparePurseKind = (
           updateBalance(purse, balanceStore.getAmount());
           return srcPaymentBalance;
         },
-        withdraw(amount) {
+        withdraw(amountBound) {
           const { state } = this;
           const { purse } = this.facets;
 
           const optRecoverySet = maybeRecoverySet(state);
           const balanceStore = makeAmountStore(state, 'currentBalance');
-          // Note COMMIT POINT within withdraw.
+          // Note COMMIT POINT within withdrawInternal.
           const payment = withdrawInternal(
             balanceStore,
-            amount,
+            amountBound,
             optRecoverySet,
           );
           updateBalance(purse, balanceStore.getAmount());

packages/ERTP/src/typeGuards.js

@@ -98,7 +98,7 @@ harden(AmountShape);
 export const AmountPatternShape = M.pattern();
 
 /** @see {HasBound} */
-export const HasBoundShape = M.tagged('match:has');
+export const HasBoundShape = M.tagged('match:containerHas');
 
 /** @see {AmountValueBound} */
 const AmountValueBoundShape = M.or(AmountValueShape, HasBoundShape);
@@ -242,7 +242,7 @@ export const makeIssuerInterfaces = (
       .optional(AmountPatternShape)
       .returns(amountShape),
     getDepositFacet: M.call().returns(DepositFacetShape),
-    withdraw: M.call(amountShape).returns(PaymentShape),
+    withdraw: M.call(AmountBoundShape).returns(PaymentShape),
     getRecoverySet: M.call().returns(M.setOf(PaymentShape)),
     recoverAll: M.call().returns(amountShape),
   });

packages/ERTP/src/types.ts

@@ -99,7 +99,7 @@ export type AssetKindForValue<V extends AmountValue> = V extends NatValue
         : never;
 
 export type HasBound = CopyTagged<
-  'match:has',
+  'match:containerHas',
   [elementPatt: Pattern, countPatt: bigint, limits?: Limits]
 >;
 
@@ -379,7 +379,7 @@ export type PurseMethods<
    * Withdraw amount
    * from this purse into a new Payment.
    */
-  withdraw: (amount: Amount<K, M>) => Payment<K, M>;
+  withdraw: (amount: AmountBound<K, M>) => Payment<K, M>;
   /**
    * The set of payments
    * withdrawn from this purse that are still live. These are the payments that

packages/ERTP/test/unitTests/mathHelpers/copySetBoundMathHelpers.test.js

@@ -0,0 +1,60 @@
+import { test } from '@agoric/swingset-vat/tools/prepare-test-env-ava.js';
+import { M, makeCopySet } from '@endo/patterns';
+
+import { AmountMath as m } from '../../../src/index.js';
+import { mockCopySetBrand as mockBrand } from './mockBrand.js';
+
+const mock = value => harden({ brand: mockBrand, value });
+const mockCopySet = elements => mock(makeCopySet(elements));
+
+// The "unit tests" for MathHelpers actually make the calls through
+// AmountMath so that we can test that any duplication is handled
+// correctly.
+
+test('set minus setBound', t => {
+  const specimen = mockCopySet(['a', 'b', 'c']);
+  t.deepEqual(
+    m.subtract(specimen, mock(M.containerHas(M.any(), 1n))),
+    mockCopySet(['b', 'c']),
+  );
+  t.deepEqual(
+    m.subtract(specimen, mock(M.containerHas('a', 1n))),
+    mockCopySet(['b', 'c']),
+  );
+  t.deepEqual(
+    m.subtract(specimen, mock(M.containerHas(M.any(), 1n))),
+    mockCopySet(['b', 'c']),
+  );
+  t.deepEqual(
+    m.subtract(specimen, mock(M.containerHas(M.any(), 3n))),
+    mockCopySet([]),
+  );
+  t.deepEqual(
+    m.subtract(specimen, mock(M.containerHas(M.any()))),
+    mockCopySet(['b', 'c']),
+  );
+
+  t.throws(() => m.subtract(specimen, mock(M.containerHas(M.any(), 4n))), {
+    message: 'Has only "[3n]" matches, but needs "[4n]"',
+  });
+
+  t.throws(() => m.subtract(specimen, mock(M.containerHas('d', 1n))), {
+    message: 'Has only "[0n]" matches, but needs "[1n]"',
+  });
+
+  t.throws(() => m.subtract(specimen, mock(M.containerHas('d'))), {
+    message: 'Has only "[0n]" matches, but needs "[1n]"',
+  });
+});
+
+test('set isGTE setBound', t => {
+  const specimen = mockCopySet(['a', 'b', 'c']);
+  t.true(m.isGTE(specimen, mock(M.containerHas(M.any(), 1n))));
+  t.true(m.isGTE(specimen, mock(M.containerHas('a', 1n))));
+  t.true(m.isGTE(specimen, mock(M.containerHas(M.any(), 1n))));
+  t.true(m.isGTE(specimen, mock(M.containerHas(M.any(), 3n))));
+  t.false(m.isGTE(specimen, mock(M.containerHas(M.any(), 4n))));
+  t.false(m.isGTE(specimen, mock(M.containerHas('d', 1n))));
+
+  t.true(m.isGTE(specimen, mock(M.containerHas(M.any()))));
+});

packages/ERTP/test/unitTests/mathHelpers/setBoundMathHelpers.test.js

@@ -11,38 +11,49 @@ const mock = value => harden({ brand: mockBrand, value });
 // correctly.
 
 test('set minus setBound', t => {
+  const specimen = mock(['a', 'b', 'c']);
   t.deepEqual(
-    m.subtract(mock(['a', 'b']), mock(M.has(M.any(), 1n))),
-    mock(['b']),
+    m.subtract(specimen, mock(M.containerHas(M.any(), 1n))),
+    mock(['b', 'c']),
   );
-  t.deepEqual(m.subtract(mock(['a', 'b']), mock(M.has('a', 1n))), mock(['b']));
   t.deepEqual(
-    m.subtract(mock(['a', 'b']), mock(M.has(M.any(), 1n))),
-    mock(['b']),
+    m.subtract(specimen, mock(M.containerHas('a', 1n))),
+    mock(['b', 'c']),
+  );
+  t.deepEqual(
+    m.subtract(specimen, mock(M.containerHas(M.any(), 1n))),
+    mock(['b', 'c']),
+  );
+  t.deepEqual(
+    m.subtract(specimen, mock(M.containerHas(M.any(), 3n))),
+    mock([]),
+  );
+  t.deepEqual(
+    m.subtract(specimen, mock(M.containerHas(M.any()))),
+    mock(['b', 'c']),
   );
-  t.deepEqual(m.subtract(mock(['a', 'b']), mock(M.has(M.any(), 2n))), mock([]));
-  t.deepEqual(m.subtract(mock(['a', 'b']), mock(M.has(M.any()))), mock(['b']));
 
-  t.throws(() => m.subtract(mock(['a', 'b']), mock(M.has(M.any(), 3n))), {
-    message: 'Has only "[2n]" matches, but needs "[3n]"',
+  t.throws(() => m.subtract(specimen, mock(M.containerHas(M.any(), 4n))), {
+    message: 'Has only "[3n]" matches, but needs "[4n]"',
   });
 
-  t.throws(() => m.subtract(mock(['a', 'b']), mock(M.has('c', 1n))), {
+  t.throws(() => m.subtract(specimen, mock(M.containerHas('d', 1n))), {
     message: 'Has only "[0n]" matches, but needs "[1n]"',
   });
 
-  t.throws(() => m.subtract(mock(['a', 'b']), mock(M.has('c'))), {
+  t.throws(() => m.subtract(specimen, mock(M.containerHas('d'))), {
     message: 'Has only "[0n]" matches, but needs "[1n]"',
   });
 });
 
 test('set isGTE setBound', t => {
-  t.true(m.isGTE(mock(['a', 'b']), mock(M.has(M.any(), 1n))));
-  t.true(m.isGTE(mock(['a', 'b']), mock(M.has('a', 1n))));
-  t.true(m.isGTE(mock(['a', 'b']), mock(M.has(M.any(), 1n))));
-  t.true(m.isGTE(mock(['a', 'b']), mock(M.has(M.any(), 2n))));
-  t.false(m.isGTE(mock(['a', 'b']), mock(M.has(M.any(), 3n))));
-  t.false(m.isGTE(mock(['a', 'b']), mock(M.has('c', 1n))));
-
-  t.true(m.isGTE(mock(['a', 'b']), mock(M.has(M.any()))));
+  const specimen = mock(['a', 'b', 'c']);
+  t.true(m.isGTE(specimen, mock(M.containerHas(M.any(), 1n))));
+  t.true(m.isGTE(specimen, mock(M.containerHas('a', 1n))));
+  t.true(m.isGTE(specimen, mock(M.containerHas(M.any(), 1n))));
+  t.true(m.isGTE(specimen, mock(M.containerHas(M.any(), 3n))));
+  t.false(m.isGTE(specimen, mock(M.containerHas(M.any(), 4n))));
+  t.false(m.isGTE(specimen, mock(M.containerHas('d', 1n))));
+
+  t.true(m.isGTE(specimen, mock(M.containerHas(M.any()))));
 });