fix: incomparables have no min or max (#6764)

* fix: incomparables have no min or max * fix: boolean expression style test
Agoric · Jan 10, 2023 · 229708b · 229708b
1 parent 1e64d5a
commit 229708b
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 21 deletions.
diff --git a/packages/ERTP/src/amountMath.js b/packages/ERTP/src/amountMath.js
@@ -173,6 +173,24 @@ const coerceLR = (h, leftAmount, rightAmount) => {
   return [h.doCoerce(leftAmount.value), h.doCoerce(rightAmount.value)];
 };
 
+/**
+ * Returns true if the leftAmount is greater than or equal to the
+ * rightAmount. The notion of "greater than or equal to" depends
+ * on the kind of amount, as defined by the MathHelpers. For example,
+ * whether rectangle A is greater than rectangle B depends on whether rectangle
+ * A includes rectangle B as defined by the logic in MathHelpers.
+ *
+ * @template {AssetKind} [K=AssetKind]
+ * @param {Amount<K>} leftAmount
+ * @param {Amount<K>} rightAmount
+ * @param {Brand<K>=} brand
+ * @returns {boolean}
+ */
+const isGTE = (leftAmount, rightAmount, brand = undefined) => {
+  const h = checkLRAndGetHelpers(leftAmount, rightAmount, brand);
+  return h.doIsGTE(...coerceLR(h, leftAmount, rightAmount));
+};
+
 /**
  * Logic for manipulating amounts.
  *
@@ -271,23 +289,7 @@ const AmountMath = {
     const h = assertValueGetHelpers(value);
     return h.doIsEmpty(h.doCoerce(value));
   },
-  /**
-   * Returns true if the leftAmount is greater than or equal to the
-   * rightAmount. For non-scalars, "greater than or equal to" depends
-   * on the kind of amount, as defined by the MathHelpers. For example,
-   * whether rectangle A is greater than rectangle B depends on whether rectangle
-   * A includes rectangle B as defined by the logic in MathHelpers.
-   *
-   * @template {AssetKind} [K=AssetKind]
-   * @param {Amount<K>} leftAmount
-   * @param {Amount<K>} rightAmount
-   * @param {Brand<K>=} brand
-   * @returns {boolean}
-   */
-  isGTE: (leftAmount, rightAmount, brand = undefined) => {
-    const h = checkLRAndGetHelpers(leftAmount, rightAmount, brand);
-    return h.doIsGTE(...coerceLR(h, leftAmount, rightAmount));
-  },
+  isGTE,
   /**
    * Returns true if the leftAmount equals the rightAmount. We assume
    * that if isGTE is true in both directions, isEqual is also true
@@ -348,7 +350,13 @@ const AmountMath = {
    * @param {Brand<K>=} brand
    * @returns {Amount<K>}
    */
-  min: (x, y, brand = undefined) => (AmountMath.isGTE(x, y, brand) ? y : x),
+  min: (x, y, brand = undefined) =>
+    // eslint-disable-next-line no-nested-ternary
+    isGTE(x, y, brand)
+      ? y
+      : isGTE(y, x, brand)
+      ? x
+      : Fail`${x} and ${y} are incomparable`,
   /**
    * Returns the max value between x and y using isGTE
    *
@@ -358,7 +366,13 @@ const AmountMath = {
    * @param {Brand<K>=} brand
    * @returns {Amount<K>}
    */
-  max: (x, y, brand = undefined) => (AmountMath.isGTE(x, y, brand) ? x : y),
+  max: (x, y, brand = undefined) =>
+    // eslint-disable-next-line no-nested-ternary
+    isGTE(x, y, brand)
+      ? x
+      : isGTE(y, x)
+      ? y
+      : Fail`${x} and ${y} are incomparable`,
 };
 harden(AmountMath);
 

diff --git a/packages/ERTP/test/unitTests/test-amountProperties.js b/packages/ERTP/test/unitTests/test-amountProperties.js
@@ -107,8 +107,19 @@ test('subtract: (x + y) - y = x; (y - x) + x = y if y >= x', async t => {
 test('minmax', t => {
   fc.assert(
     fc.property(fc.record({ x: arbAmount, y: arbAmount }), ({ x, y }) => {
-      t.deepEqual(m.min(x, y), m.isGTE(x, y) ? y : x, 'SET: min');
-      t.deepEqual(m.max(x, y), m.isGTE(x, y) ? x : y, 'SET: max');
+      return (
+        (!m.isGTE(x, y) ||
+          (t.deepEqual(m.min(x, y), y, 'min') &&
+            t.deepEqual(m.max(x, y), x, 'max'))) &&
+        (m.isGTE(x, y) ||
+          m.isGTE(y, x) ||
+          (!!t.throws(() => m.min(x, y), {
+            message: /\{"brand":.*?\} and \{"brand":.*?\} are incomparable/,
+          }) &&
+            !!t.throws(() => m.max(x, y), {
+              message: /\{"brand":.*?\} and \{"brand":.*?\} are incomparable/,
+            })))
+      );
     }),
   );
 });