From f51d3270a8a59b4a5fdcac029f6f752fbad3ad59 Mon Sep 17 00:00:00 2001
From: Kate Sills <kate@agoric.com>
Date: Fri, 18 Jun 2021 10:59:05 -0700
Subject: [PATCH] refactor(zoe)!: handle subtracting empty in a less fragile
 way (#3345)

* chore: handle subtracting empty in a less fragile way
---
 .../zoe/src/contractFacet/allocationMath.js   | 29 +++++++++++++------
 .../zoe/src/contracts/newSwap/collectFees.js  |  5 ----
 .../unitTests/zcf/test-reallocate-empty.js    |  8 ++---
 3 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/packages/zoe/src/contractFacet/allocationMath.js b/packages/zoe/src/contractFacet/allocationMath.js
index 32e8c6209b1..2ddd34be6f0 100644
--- a/packages/zoe/src/contractFacet/allocationMath.js
+++ b/packages/zoe/src/contractFacet/allocationMath.js
@@ -25,12 +25,19 @@ import { AmountMath } from '@agoric/ertp';
  */
 const doOperation = (allocation, amountKeywordRecord, operationFn) => {
   const allKeywords = Object.keys({ ...allocation, ...amountKeywordRecord });
-  return Object.fromEntries(
-    allKeywords.map(keyword => [
-      keyword,
-      operationFn(allocation[keyword], amountKeywordRecord[keyword], keyword),
-    ]),
-  );
+
+  const entries = allKeywords
+    .map(keyword => {
+      const result = operationFn(
+        allocation[keyword],
+        amountKeywordRecord[keyword],
+        keyword,
+      );
+      return [keyword, result];
+    })
+    .filter(([_keyword, result]) => result !== undefined);
+
+  return Object.fromEntries(entries);
 };
 
 /** @type {Operation} */
@@ -46,7 +53,13 @@ const add = (amount, amountToAdd, _keyword) => {
 
 /** @type {Operation} */
 const subtract = (amount, amountToSubtract, keyword) => {
-  if (amountToSubtract !== undefined) {
+  // if amountToSubtract is undefined OR is defined, but empty
+  if (amountToSubtract === undefined || AmountMath.isEmpty(amountToSubtract)) {
+    // Subtracting undefined is equivalent to subtracting empty, so in
+    // both cases we can return the original amount. If the original
+    // amount is undefined, it is filtered out in doOperation.
+    return /** @type {Amount} */ (amount);
+  } else {
     assert(
       amount !== undefined,
       X`The amount could not be subtracted from the allocation because the allocation did not have an amount under the keyword ${keyword}.`,
@@ -57,8 +70,6 @@ const subtract = (amount, amountToSubtract, keyword) => {
     );
     return AmountMath.subtract(amount, amountToSubtract);
   }
-  // Subtracting undefined is equivalent to subtracting empty.
-  return /** @type {Amount} */ (amount);
 };
 
 /**
diff --git a/packages/zoe/src/contracts/newSwap/collectFees.js b/packages/zoe/src/contracts/newSwap/collectFees.js
index b66e4b92876..af7cc30e23e 100644
--- a/packages/zoe/src/contracts/newSwap/collectFees.js
+++ b/packages/zoe/src/contracts/newSwap/collectFees.js
@@ -1,13 +1,8 @@
 // @ts-check
 
-import { AmountMath } from '@agoric/ertp';
-
 export const makeMakeCollectFeesInvitation = (zcf, feeSeat, centralBrand) => {
   const collectFees = seat => {
-    // Ensure that the feeSeat has a stagedAllocation with a RUN keyword
-    feeSeat.incrementBy({ RUN: AmountMath.makeEmpty(centralBrand) });
     const amount = feeSeat.getAmountAllocated('RUN', centralBrand);
-
     seat.incrementBy(feeSeat.decrementBy({ RUN: amount }));
     zcf.reallocate(seat, feeSeat);
 
diff --git a/packages/zoe/test/unitTests/zcf/test-reallocate-empty.js b/packages/zoe/test/unitTests/zcf/test-reallocate-empty.js
index 36b6f944751..f6bc9b293aa 100644
--- a/packages/zoe/test/unitTests/zcf/test-reallocate-empty.js
+++ b/packages/zoe/test/unitTests/zcf/test-reallocate-empty.js
@@ -19,11 +19,9 @@ test(`zcf.reallocate introducing new empty amount`, async t => {
 
   const empty = AmountMath.makeEmpty(brand, AssetKind.NAT);
 
-  // decrementBy empty
-  t.throws(() => zcfSeat1.decrementBy({ RUN: empty }), {
-    message:
-      'The amount could not be subtracted from the allocation because the allocation did not have an amount under the keyword "RUN".',
-  });
+  // decrementBy empty does not throw, and does not add a keyword
+  zcfSeat1.decrementBy({ RUN: empty });
+  t.deepEqual(zcfSeat1.getStagedAllocation(), {});
 
   // Try to incrementBy empty. This succeeds, and the keyword is added
   // with an empty amount.