Compare Swap and Trade Helpers and make more consistent (#1667)

* refactor: rewrite trade and swap

* chore: update outdated comment

packages/zoe/src/contractSupport/types.js

@@ -4,19 +4,48 @@
 /**
  * @callback Trade
  * Trade between left and right so that left and right end up with
- * the declared gains.
+ * the declared gains and losses.
  * @param {ContractFacet} zcf
- * @param {SeatGainsLossesRecord} keepLeft
- * @param {SeatGainsLossesRecord} tryRight
+ * @param {SeatGainsLossesRecord} left
+ * @param {SeatGainsLossesRecord} right
+ * @param {String} [leftHasExitedMsg] A custom error message if
+ * the left seat has been exited already
+ * @param {String} [rightHasExitedMsg] A custom error message if the
+ * right seat has been exited already
  * @returns {void}
  *
  * @typedef {Object} SeatGainsLossesRecord
  * @property {ZCFSeat} seat
- * @property {AmountKeywordRecord} gains - what the offer will
+ * @property {AmountKeywordRecord} gains - what the seat will
  * gain as a result of this trade
- * @property {AmountKeywordRecord=} losses - what the offer will
+ * @property {AmountKeywordRecord=} losses - what the seat will
  * give up as a result of this trade. Losses is optional, but can
- * only be omitted if the keywords for both offers are the same.
- * If losses is not defined, the gains of the other offer is
+ * only be omitted if the keywords for both seats are the same.
+ * If losses is not defined, the gains of the other seat is
  * subtracted.
  */
+
+/**
+ * @callback Swap
+ * If two seats can satisfy each other's wants, trade enough to
+ * satisfy the wants of both seats and exit both seats.
+ *
+ * The surplus remains with the original seat. For example if seat A
+ * gives 5 moola and seat B only wants 3 moola, seat A retains 2
+ * moola.
+ *
+ * If the leftSeat has exited already, both seats will be kicked out
+ * with an error message (provided by 'leftHasExitedMsg'). Similarly,
+ * if the rightSeat has exited already, both seats will be kicked out
+ * with an error message (provided by 'rightHasExitedMsg').
+ *
+ * If the swap fails, no assets are transferred, both seats are kicked
+ * out, and the function throws.
+ *
+ * @param {ContractFacet} zcf
+ * @param {ZCFSeat} leftSeat
+ * @param {ZCFSeat} rightSeat
+ * @param {String} [leftHasExitedMsg]
+ * @param {String} [rightHasExitedMsg]
+ * @returns {string}
+ */

packages/zoe/src/contractSupport/zoeHelpers.js

@@ -113,116 +113,102 @@ export const satisfies = (zcf, seat, update) => {
 };
 
 /** @type {Trade} */
-export const trade = (zcf, keepLeft, tryRight) => {
-  assert(
-    keepLeft.seat !== tryRight.seat,
-    details`an offer cannot trade with itself`,
-  );
-  let leftAllocation = keepLeft.seat.getCurrentAllocation();
-  let rightAllocation = tryRight.seat.getCurrentAllocation();
+export const trade = (
+  zcf,
+  left,
+  right,
+  leftHasExitedMsg,
+  rightHasExitedMsg,
+) => {
+  assert(left.seat !== right.seat, details`a seat cannot trade with itself`);
+  assert(!left.seat.hasExited(), leftHasExitedMsg);
+  assert(!right.seat.hasExited(), rightHasExitedMsg);
+  let leftAllocation = left.seat.getCurrentAllocation();
+  let rightAllocation = right.seat.getCurrentAllocation();
   try {
-    // for all the keywords and amounts in leftGains, transfer from
+    // for all the keywords and amounts in left.gains, transfer from
     // right to left
     ({ from: rightAllocation, to: leftAllocation } = calcNewAllocations(
       zcf,
       { from: rightAllocation, to: leftAllocation },
-      keepLeft.gains,
-      tryRight.losses,
+      left.gains,
+      right.losses,
     ));
-    // For all the keywords and amounts in rightGains, transfer from
+    // For all the keywords and amounts in right.gains, transfer from
     // left to right
     ({ from: leftAllocation, to: rightAllocation } = calcNewAllocations(
       zcf,
       { from: leftAllocation, to: rightAllocation },
-      tryRight.gains,
-      keepLeft.losses,
+      right.gains,
+      left.losses,
     ));
   } catch (err) {
     console.log(err);
-    throw tryRight.seat.kickOut(
-      new Error(
-        `The trade between left ${keepLeft} and right ${tryRight} failed. Please check the log for more information`,
-      ),
+    throw new Error(
+      `The trade between left ${left} and right ${right} failed. Please check the log for more information`,
     );
   }
 
   // Check whether reallocate would error before calling. If
-  // it would error, reject the right offer and return.
-  const offerSafeForLeft = keepLeft.seat.isOfferSafe(leftAllocation);
-  const offerSafeForRight = tryRight.seat.isOfferSafe(rightAllocation);
+  // it would error, log information and throw.
+  const offerSafeForLeft = left.seat.isOfferSafe(leftAllocation);
+  const offerSafeForRight = right.seat.isOfferSafe(rightAllocation);
   if (!(offerSafeForLeft && offerSafeForRight)) {
-    console.log(`currentLeftAllocation`, keepLeft.seat.getCurrentAllocation());
-    console.log(`currentRightAllocation`, tryRight.seat.getCurrentAllocation());
+    console.log(`currentLeftAllocation`, left.seat.getCurrentAllocation());
+    console.log(`currentRightAllocation`, right.seat.getCurrentAllocation());
     console.log(`proposed left reallocation`, leftAllocation);
     console.log(`proposed right reallocation`, rightAllocation);
     // show the constraints
-    console.log(`left want`, keepLeft.seat.getProposal().want);
-    console.log(`right want`, tryRight.seat.getProposal().want);
+    console.log(`left want`, left.seat.getProposal().want);
+    console.log(`right want`, right.seat.getProposal().want);
 
     if (!offerSafeForLeft) {
       console.log(`offer not safe for left`);
     }
     if (!offerSafeForRight) {
       console.log(`offer not safe for right`);
     }
-    throw tryRight.seat.kickOut(
-      new Error(
-        `The trade between left ${keepLeft} and right ${tryRight} failed offer safety. Please check the log for more information`,
-      ),
+    throw new Error(
+      `The trade between left ${left} and right ${right} failed offer safety. Please check the log for more information`,
     );
   }
 
   return zcf.reallocate(
-    keepLeft.seat.stage(leftAllocation),
-    tryRight.seat.stage(rightAllocation),
+    left.seat.stage(leftAllocation),
+    right.seat.stage(rightAllocation),
   );
 };
 
-/**
- * If the two handles can trade, then swap their compatible assets,
- * marking both offers as complete.
- *
- * The surplus remains with the original offer. For example if
- * offer A gives 5 moola and offer B only wants 3 moola, offer A
- * retains 2 moola.
- *
- * If the keep offer is no longer active (it was already completed), the try
- * offer will be rejected with a message (provided by 'keepHandleInactiveMsg').
- *
- * TODO: If the try offer is no longer active, swap() should terminate with
- * a useful error message.
- *
- * If the swap fails, no assets are transferred, and the 'try' offer is rejected.
- *
- * @param {ContractFacet} zcf
- * @param {ZCFSeat} keepSeat
- * @param {ZCFSeat} trySeat
- * @param {String} [keepHandleInactiveMsg]
- */
+/** @type Swap */
 export const swap = (
   zcf,
-  keepSeat,
-  trySeat,
-  keepHandleInactiveMsg = 'prior offer is unavailable',
+  leftSeat,
+  rightSeat,
+  leftHasExitedMsg = 'the left seat in swap() has exited',
+  rightHasExitedMsg = 'the right seat in swap() has exited',
 ) => {
-  if (keepSeat.hasExited()) {
-    throw trySeat.kickOut(new Error(keepHandleInactiveMsg));
+  try {
+    trade(
+      zcf,
+      {
+        seat: leftSeat,
+        gains: leftSeat.getProposal().want,
+      },
+      {
+        seat: rightSeat,
+        gains: rightSeat.getProposal().want,
+      },
+      leftHasExitedMsg,
+      rightHasExitedMsg,
+    );
+  } catch (err) {
+    leftSeat.kickOut(err);
+    rightSeat.kickOut(err);
+    throw err;
   }
 
-  trade(
-    zcf,
-    {
-      seat: keepSeat,
-      gains: keepSeat.getProposal().want,
-    },
-    {
-      seat: trySeat,
-      gains: trySeat.getProposal().want,
-    },
-  );
-
-  keepSeat.exit();
-  trySeat.exit();
+  leftSeat.exit();
+  rightSeat.exit();
   return defaultAcceptanceMsg;
 };