From f56ff7a76587fc36bcaf5a88efe663b3b1f974e8 Mon Sep 17 00:00:00 2001
From: Chris Hibbert <hibbert@agoric.com>
Date: Tue, 4 May 2021 14:58:49 -0700
Subject: [PATCH] fix: test and workaround for a reallocate issue in zoe

issue 3033 shows that reallocate fails unnecessarily if asked to
reallocate to seats that had no previous allocation, even if the new
allocations are empty.

This works around that for the collectFees contract until a fix can be prepared.
---
 .../zoe/src/contracts/newSwap/collectFees.js  | 15 +++++--
 .../contracts/newSwap/test-newSwap-swap.js    | 42 +++++++++++++++++++
 2 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/packages/zoe/src/contracts/newSwap/collectFees.js b/packages/zoe/src/contracts/newSwap/collectFees.js
index c5ce9950e31..234fc87bf75 100644
--- a/packages/zoe/src/contracts/newSwap/collectFees.js
+++ b/packages/zoe/src/contracts/newSwap/collectFees.js
@@ -5,10 +5,17 @@ import { amountMath } from '@agoric/ertp';
 export const makeMakeCollectFeesInvitation = (zcf, feeSeat, centralBrand) => {
   const collectFees = seat => {
     const allocation = feeSeat.getAmountAllocated('RUN', centralBrand);
-    zcf.reallocate(
-      seat.stage({ RUN: allocation }),
-      feeSeat.stage({ RUN: amountMath.makeEmpty(centralBrand) }),
-    );
+
+    // This check works around
+    // https://github.com/Agoric/agoric-sdk/issues/3033
+    // when that bug is fixed, the reallocate can be moved outside the check and
+    // the check dropped.
+    if (!amountMath.isEmpty(allocation)) {
+      zcf.reallocate(
+        seat.stage({ RUN: allocation }),
+        feeSeat.stage({ RUN: amountMath.makeEmpty(centralBrand) }),
+      );
+    }
     seat.exit();
     return `paid out ${allocation.value}`;
   };
diff --git a/packages/zoe/test/unitTests/contracts/newSwap/test-newSwap-swap.js b/packages/zoe/test/unitTests/contracts/newSwap/test-newSwap-swap.js
index 70bede059fb..ac217d6428e 100644
--- a/packages/zoe/test/unitTests/contracts/newSwap/test-newSwap-swap.js
+++ b/packages/zoe/test/unitTests/contracts/newSwap/test-newSwap-swap.js
@@ -420,6 +420,13 @@ test('newSwap doubleSwap', async t => {
     publicFacet,
   ).makeAddLiquidityInvitation();
 
+  const collectFeesInvitation2 = E(creatorFacet).makeCollectFeesInvitation();
+  const collectFeesSeat2 = await zoe.offer(
+    collectFeesInvitation2,
+    undefined,
+    undefined,
+  );
+
   const moolaLiquidityIssuer = await E(publicFacet).addPool(
     moolaR.issuer,
     'Moola',
@@ -902,3 +909,38 @@ test('newSwap jig - swapOut uneven', async t => {
     RUN: amountMath.makeEmpty(centralR.brand),
   });
 });
+
+// This demonstrates that we've worked around
+// https://github.com/Agoric/agoric-sdk/issues/3033.  When that is fixed, the
+// work-around in collectFees should be cleaned up.
+test('newSwap workaround zoe Bug', async t => {
+  const zoe = makeZoe(fakeVatAdmin);
+
+  // Set up central token
+  const centralR = makeIssuerKit('central');
+
+  // Alice creates an autoswap instance
+  const bundle = await bundleSource(newSwapRoot);
+
+  const installation = await zoe.install(bundle);
+  // This timer is only used to build quotes. Let's make it non-zero
+  const fakeTimer = buildManualTimer(console.log, 30);
+  const { creatorFacet } = await zoe.startInstance(
+    installation,
+    harden({ Central: centralR.issuer }),
+    { timer: fakeTimer, poolFee: 24n, protocolFee: 6n },
+  );
+
+  const collectFeesInvitation2 = E(creatorFacet).makeCollectFeesInvitation();
+  const collectFeesSeat2 = await zoe.offer(
+    collectFeesInvitation2,
+    undefined,
+    undefined,
+  );
+
+  const payout = await E(collectFeesSeat2).getPayout('RUN');
+  const result = await E(collectFeesSeat2).getOfferResult();
+
+  t.deepEqual(payout, undefined);
+  t.deepEqual(result, 'paid out 0');
+});