fix: fullOrder leak. Semi-fungibles via CopyBags (#4305)

packages/ERTP/package.json

@@ -48,7 +48,8 @@
   "devDependencies": {
     "@agoric/bundle-source": "^2.0.1",
     "@agoric/swingset-vat": "^0.24.1",
-    "ava": "^3.12.1"
+    "ava": "^3.12.1",
+    "fast-check": "^2.21.0"
   },
   "files": [
     "src",

packages/ERTP/src/amountMath.js

@@ -7,18 +7,20 @@ import { M, matches } from '@agoric/store';
 import { natMathHelpers } from './mathHelpers/natMathHelpers.js';
 import { setMathHelpers } from './mathHelpers/setMathHelpers.js';
 import { copySetMathHelpers } from './mathHelpers/copySetMathHelpers.js';
+import { copyBagMathHelpers } from './mathHelpers/copyBagMathHelpers.js';
 
 const { details: X, quote: q } = assert;
 
 /**
  * Constants for the kinds of assets we support.
  *
- * @type {{ NAT: 'nat', SET: 'set', COPY_SET: 'copySet' }}
+ * @type {{ NAT: 'nat', SET: 'set', COPY_SET: 'copySet', COPY_BAG: 'copyBag' }}
  */
 const AssetKind = harden({
   NAT: 'nat',
   SET: 'set',
   COPY_SET: 'copySet',
+  COPY_BAG: 'copyBag',
 });
 const assetKindNames = harden(Object.values(AssetKind).sort());
 
@@ -72,12 +74,14 @@ harden(assertAssetKind);
 /** @type {{
  *   nat: NatMathHelpers,
  *   set: SetMathHelpers,
- *   copySet: CopySetMathHelpers }
- * } */
+ *   copySet: CopySetMathHelpers,
+ *   copyBag: CopyBagMathHelpers
+ * }} */
 const helpers = {
   nat: natMathHelpers,
   set: setMathHelpers,
   copySet: copySetMathHelpers,
+  copyBag: copyBagMathHelpers,
 };
 
 /** @type {(value: AmountValue) => AssetKind} */
@@ -92,25 +96,30 @@ const assertValueGetAssetKind = value => {
   if (matches(value, M.set())) {
     return 'copySet';
   }
+  if (matches(value, M.bag())) {
+    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 to
-    // include CopyMaps.
-    X`value ${value} must be a bigint, copySet, or an array, not ${passStyle}`,
+    // (TODO) will encounter when we extend the math helpers further.
+    X`value ${value} must be a bigint, copySet, copyBag, or an array, not ${passStyle}`,
   );
 };
 
 /**
  *
  * Asserts that value is a valid AmountMath and returns the appropriate helpers.
  *
+ * Made available only for testing, but it is harmless for other uses.
+ *
  * @param {AmountValue} value
  * @returns {MathHelpers<*>}
  */
-const assertValueGetHelpers = value => helpers[assertValueGetAssetKind(value)];
+export const assertValueGetHelpers = value =>
+  helpers[assertValueGetAssetKind(value)];
 
 /** @type {(allegedBrand: Brand, brand?: Brand) => void} */
 const optionalBrandCheck = (allegedBrand, brand) => {

packages/ERTP/src/mathHelpers/copyBagMathHelpers.js

@@ -0,0 +1,32 @@
+// @ts-check
+
+import {
+  keyEQ,
+  makeCopyBag,
+  fit,
+  M,
+  getCopyBagEntries,
+  bagIsSuperbag,
+  bagUnion,
+  bagDisjointSubtract,
+} from '@agoric/store';
+import '../types.js';
+
+/** @type {CopyBagValue} */
+const empty = makeCopyBag([]);
+
+/**
+ * @type {CopyBagMathHelpers}
+ */
+export const copyBagMathHelpers = harden({
+  doCoerce: bag => {
+    fit(bag, M.bag());
+    return bag;
+  },
+  doMakeEmpty: () => empty,
+  doIsEmpty: bag => getCopyBagEntries(bag).length === 0,
+  doIsGTE: bagIsSuperbag,
+  doIsEqual: keyEQ,
+  doAdd: bagUnion,
+  doSubtract: bagDisjointSubtract,
+});

packages/ERTP/src/mathHelpers/copySetMathHelpers.js

@@ -1,30 +1,20 @@
 // @ts-check
 
 import {
-  makeSetOps,
   keyEQ,
   makeCopySet,
   fit,
   M,
   getCopySetKeys,
-  makeFullOrderComparatorKit,
+  setIsSuperset,
+  setDisjointUnion,
+  setDisjointSubtract,
 } from '@agoric/store';
 import '../types.js';
 
 /** @type {CopySetValue} */
 const empty = makeCopySet([]);
 
-/**
- * TODO SECURITY BUG: /~https://github.com/Agoric/agoric-sdk/issues/4261
- * This creates observable mutable static state, in the
- * history-based ordering of remotables.
- */
-const fullCompare = makeFullOrderComparatorKit(true).antiComparator;
-
-const { isSetSuperset, setDisjointUnion, setDisjointSubtract } = makeSetOps(
-  fullCompare,
-);
-
 /**
  * @type {CopySetMathHelpers}
  */
@@ -35,7 +25,7 @@ export const copySetMathHelpers = harden({
   },
   doMakeEmpty: () => empty,
   doIsEmpty: set => getCopySetKeys(set).length === 0,
-  doIsGTE: isSetSuperset,
+  doIsGTE: setIsSuperset,
   doIsEqual: keyEQ,
   doAdd: setDisjointUnion,
   doSubtract: setDisjointSubtract,

packages/ERTP/src/mathHelpers/setMathHelpers.js

@@ -1,12 +1,13 @@
 // @ts-check
 
-import { assertChecker, passStyleOf } from '@agoric/marshal';
+import { passStyleOf } from '@agoric/marshal';
 import {
   assertKey,
-  makeSetOps,
-  sortByRank,
-  keyEQ,
-  makeFullOrderComparatorKit,
+  elementsIsSuperset,
+  elementsDisjointUnion,
+  elementsDisjointSubtract,
+  coerceToElements,
+  elementsCompare,
 } from '@agoric/store';
 import '../types.js';
 
@@ -15,47 +16,24 @@ import '../types.js';
 /** @type {SetValue} */
 const empty = harden([]);
 
-/**
- * TODO SECURITY BUG: /~https://github.com/Agoric/agoric-sdk/issues/4261
- * This creates observable mutable static state, in the
- * history-based ordering of remotables.
- */
-const fullCompare = makeFullOrderComparatorKit(true).antiComparator;
-
-const {
-  checkNoDuplicates,
-  isListSuperset,
-  listDisjointUnion,
-  listDisjointSubtract,
-} = makeSetOps(fullCompare);
-
-const assertNoDuplicates = list => checkNoDuplicates(list, assertChecker);
-
 /**
  * @deprecated Replace array-based SetMath with CopySet-based CopySetMath
  * @type {SetMathHelpers}
  */
 export const setMathHelpers = harden({
   doCoerce: list => {
-    assert(
-      passStyleOf(list) === 'copyArray',
-      `The value of a non-fungible token must be an array but was ${list}`,
-    );
+    list = coerceToElements(list);
     // Assert that list contains only
     //   * pass-by-copy primitives,
     //   * pass-by-copy containers containing keys,
     //   * remotables.
     assertKey(list);
-    list = sortByRank(list, fullCompare);
-    // As a coercion, should we also deduplicate here? Might mask bugs
-    // elsewhere though, so probably not.
-    assertNoDuplicates(list);
     return list;
   },
   doMakeEmpty: () => empty,
   doIsEmpty: list => passStyleOf(list) === 'copyArray' && list.length === 0,
-  doIsGTE: isListSuperset,
-  doIsEqual: keyEQ,
-  doAdd: listDisjointUnion,
-  doSubtract: listDisjointSubtract,
+  doIsGTE: elementsIsSuperset,
+  doIsEqual: (x, y) => elementsCompare(x, y) === 0,
+  doAdd: elementsDisjointUnion,
+  doSubtract: elementsDisjointSubtract,
 });

packages/ERTP/src/typeGuards.js

@@ -12,7 +12,7 @@ const NatValueShape = M.nat();
 /**
  * When the AmountValue of an Amount fits the CopySetValueShape, i.e., when it
  * is a CopySet, then it represents the set of those
- * keys, where each key represents some individual non fungible
+ * keys, where each key represents some individual non-fungible
  * item, like a concert ticket, from the non-fungible asset class
  * represented by that amount's brand. The amount itself represents
  * the set of these items, as opposed to any of the other items
@@ -38,7 +38,34 @@ const CopySetValueShape = M.set();
  */
 const SetValueShape = M.arrayOf(M.key());
 
-const AmountValueShape = M.or(NatValueShape, CopySetValueShape, SetValueShape);
+/**
+ * When the AmountValue of an Amount fits the CopyBagValueShape, i.e., when it
+ * is a CopyBag, then it represents the bag (multiset) of those
+ * keys, where each key represents some individual semi-fungible
+ * item, like a concert ticket, from the semi-fungible asset class
+ * represented by that amount's brand. The number of times that key
+ * appears in the bag is the number of fungible units of that key.
+ * The amount itself represents
+ * the bag of these items, as opposed to any of the other items
+ * from the same asset class.
+ *
+ * If a given value class represents concert tickets, it seems bizarre
+ * that we can form amounts of any key. The hard constraint is that
+ * the code that holds the mint for that asset class---the one associated
+ * with that brand, only mints the items representing the real units
+ * of that asset class as defined by it. Anyone else can put together
+ * an amount expressing, for example, that they "want" some items that
+ * will never be minted. That want will never be satisfied.
+ * "You can't always get..."
+ */
+const CopyBagValueShape = M.bagOf();
+
+const AmountValueShape = M.or(
+  NatValueShape,
+  CopySetValueShape,
+  SetValueShape,
+  CopyBagValueShape,
+);
 
 export const AmountShape = harden({
   brand: M.remotable(),
@@ -74,3 +101,12 @@ harden(isCopySetValue);
  */
 export const isSetValue = value => matches(value, SetValueShape);
 harden(isSetValue);
+
+/**
+ * Returns true if value is a CopyBag
+ *
+ * @param {AmountValue} value
+ * @returns {value is CopyBagValue}
+ */
+export const isCopyBagValue = value => matches(value, CopyBagValueShape);
+harden(isCopyBagValue);

packages/ERTP/src/types.js

@@ -21,7 +21,7 @@
  */
 
 /**
- * @typedef {NatValue | SetValue | CopySetValue} AmountValue
+ * @typedef {NatValue | SetValue | CopySetValue | CopyBagValue} AmountValue
  * An `AmountValue` describes a set or quantity of assets that can be owned or
  * shared.
  *
@@ -37,7 +37,7 @@
  * to represent the array of its elements. `CopySetValue` is the proper
  * representation using a CopySet.
  *
- * TODO Eventually add `CopyBagValue` for semi-fungible rights represented as a
+ * A semi-fungible `CopyBagValue` is represented as a
  * `CopyBag` of `Key` objects. "Bag" is synonymous with MultiSet, where an
  * element of a bag can be present once or more times, i.e., some positive
  * bigint number of times, representing that quantity of the asset represented
@@ -51,7 +51,7 @@
  */
 
 /**
- * @typedef {'nat' | 'set' | 'copySet' } AssetKind
+ * @typedef {'nat' | 'set' | 'copySet' | 'copyBag' } AssetKind
  *
  * See doc-comment for `AmountValue`.
  */
@@ -556,6 +556,14 @@
  * @typedef {MathHelpers<CopySetValue>} CopySetMathHelpers
  */
 
+/**
+ * @typedef {CopyBag<Key>} CopyBagValue
+ */
+
+/**
+ * @typedef {MathHelpers<CopyBagValue>} CopyBagMathHelpers
+ */
+
 /**
  * @callback AssertAssetKind
  * @param {AssetKind} allegedAK