fix!: rights conservation should allow new brands or dropped brands if sum is empty

packages/zoe/src/contractFacet/rightsConservation.js

@@ -34,27 +34,54 @@ const sumByBrand = amounts => {
  *
  * @param  {Store<Brand, Amount>} leftSumsByBrand - a map of brands to sums
  * @param  {Store<Brand, Amount>} rightSumsByBrand - a map of brands to sums
- * indexed by issuer
  */
 const assertEqualPerBrand = (leftSumsByBrand, rightSumsByBrand) => {
-  const leftKeys = leftSumsByBrand.keys();
-  const rightKeys = rightSumsByBrand.keys();
-  assert.equal(
-    leftKeys.length,
-    rightKeys.length,
-    X`${leftKeys.length} should be equal to ${rightKeys.length}`,
-  );
-  leftSumsByBrand
-    .keys()
-    .forEach(brand =>
-      assert(
-        amountMath.isEqual(
-          leftSumsByBrand.get(brand),
-          rightSumsByBrand.get(brand),
-        ),
-        X`rights were not conserved for brand ${brand}`,
-      ),
+  // We cannot assume that all of the brand keys present in
+  // leftSumsByBrand are also present in rightSumsByBrand. A empty
+  // amount could be introduced or dropped, and this should still be
+  // deemed "equal" from the perspective of rights conservation.
+
+  // Thus, we should allow for a brand to be missing from a map, but
+  // only if the sum for the brand in the other map is empty.
+
+  /**
+   * A helper that either gets the sums for the specified brand, or if
+   * the brand is absent in the map, returns an empty amount.
+   *
+   * @param {Brand} brand
+   * @returns {{ leftSum: Amount, rightSum: Amount }}
+   */
+  const getSums = brand => {
+    let leftSum;
+    let rightSum;
+    if (leftSumsByBrand.has(brand)) {
+      leftSum = leftSumsByBrand.get(brand);
+    }
+    if (rightSumsByBrand.has(brand)) {
+      rightSum = rightSumsByBrand.get(brand);
+    }
+    if (leftSum === undefined) {
+      assert(rightSum);
+      leftSum = amountMath.makeEmptyFromAmount(rightSum);
+    }
+    if (rightSum === undefined) {
+      rightSum = amountMath.makeEmptyFromAmount(leftSum);
+    }
+    return { leftSum, rightSum };
+  };
+
+  const allBrands = new Set([
+    ...leftSumsByBrand.keys(),
+    ...rightSumsByBrand.keys(),
+  ]);
+
+  allBrands.forEach(brand => {
+    const { leftSum, rightSum } = getSums(brand);
+    assert(
+      amountMath.isEqual(leftSum, rightSum),
+      X`rights were not conserved for brand ${brand}`,
     );
+  });
 };
 
 /**

packages/zoe/test/unitTests/zcf/test-reallocate-empty.js

@@ -0,0 +1,80 @@
+// @ts-check
+// eslint-disable-next-line import/no-extraneous-dependencies
+import { test } from '@agoric/zoe/tools/prepare-test-env-ava';
+
+import { amountMath } from '@agoric/ertp';
+
+import { setupZCFTest } from './setupZcfTest';
+
+test(`zcfSeat.stage, zcf.reallocate introducing new empty amount`, async t => {
+  const { zcf } = await setupZCFTest();
+  const { zcfSeat: zcfSeat1 } = zcf.makeEmptySeatKit();
+  const { zcfSeat: zcfSeat2 } = zcf.makeEmptySeatKit();
+  const zcfMint = await zcf.makeZCFMint('RUN');
+  const { brand } = zcfMint.getIssuerRecord();
+
+  // Get the amount allocated on zcfSeat1. It is empty for the RUN brand.
+  const allocation = zcfSeat1.getAmountAllocated('RUN', brand);
+  t.true(amountMath.isEmpty(allocation));
+
+  // Stage zcfSeat2 with the allocation from zcfSeat1
+  const zcfSeat2Staging = zcfSeat2.stage({ RUN: allocation });
+
+  // Stage zcfSeat1 with empty
+  const zcfSeat1Staging = zcfSeat1.stage({ RUN: amountMath.makeEmpty(brand) });
+
+  zcf.reallocate(zcfSeat1Staging, zcfSeat2Staging);
+
+  t.deepEqual(zcfSeat1.getCurrentAllocation(), {
+    RUN: amountMath.make(0n, brand),
+  });
+  t.deepEqual(zcfSeat2.getCurrentAllocation(), {
+    RUN: amountMath.make(0n, brand),
+  });
+});
+
+test(`zcfSeat.stage, zcf.reallocate "dropping" empty amount`, async t => {
+  const { zcf } = await setupZCFTest();
+  const { zcfSeat: zcfSeat1 } = zcf.makeEmptySeatKit();
+  const { zcfSeat: zcfSeat2 } = zcf.makeEmptySeatKit();
+  const zcfMint = await zcf.makeZCFMint('RUN');
+  const { brand } = zcfMint.getIssuerRecord();
+
+  zcfMint.mintGains({ RUN: amountMath.make(brand, 0n) }, zcfSeat1);
+  zcfMint.mintGains({ RUN: amountMath.make(brand, 0n) }, zcfSeat2);
+
+  // Now zcfSeat1 and zcfSeat2 both have an empty allocation for RUN.
+  t.deepEqual(zcfSeat1.getCurrentAllocation(), {
+    RUN: amountMath.make(0n, brand),
+  });
+  t.deepEqual(zcfSeat2.getCurrentAllocation(), {
+    RUN: amountMath.make(0n, brand),
+  });
+
+  // Stage zcfSeat1 with an entirely empty allocation
+  const zcfSeat1Staging = zcfSeat2.stage({});
+
+  // Stage zcfSeat2 with an entirely empty allocation
+  const zcfSeat2Staging = zcfSeat1.stage({});
+
+  // Because of how we merge staged allocations with the current
+  // allocation (we don't delete keys), the RUN keyword still remains:
+  t.deepEqual(zcfSeat1Staging.getStagedAllocation(), {
+    RUN: amountMath.make(0n, brand),
+  });
+  t.deepEqual(zcfSeat2Staging.getStagedAllocation(), {
+    RUN: amountMath.make(0n, brand),
+  });
+
+  zcf.reallocate(zcfSeat1Staging, zcfSeat2Staging);
+
+  // The reallocation succeeds without error, but because of how we
+  // merge new allocations with old allocations (we don't delete
+  // keys), the RUN keyword still remains as is.
+  t.deepEqual(zcfSeat1.getCurrentAllocation(), {
+    RUN: amountMath.make(0n, brand),
+  });
+  t.deepEqual(zcfSeat2.getCurrentAllocation(), {
+    RUN: amountMath.make(0n, brand),
+  });
+});