fix: multipoolAutoswap was calculating output prices incorrectly. (#2166)

* fix: multipoolAutoswap was calculating output prices incorrectly.

It effectively had the two pools swapped. We didn't notice because all our tests had the two pools
at rough parity. While trying to make the staking contract work, I kept geting prices that were 2x
the input rather than half when I expected the opposite.

The tests were correspondingly broken. The tests called priceFromTargetOutput() to calculate
expected values, and consistently had the pools reversed in the call. The correct call is
`priceFromTargetOutput(deltaY, yPre, xPre, fee)`. Notice that the first two parameters are both in
`Y` units. The calls from the test have been corrected so the second parameter is the pool that
matches the requested output amount.

In the newly added test, the pool is set up with a ratio of 500 Moola to 1000 Central tokens. When
Alice asks the price for 300 Central, the price will now be 155. When she trades back, it takes
roughly twice the number of central tokens to purchase the moola she wants.

* refactor: Vault uses a priceAuthority to schedule liquidation

getPriceGivenAvailableInput and getPriceGivenRequiredOutput
added tests
renamed userSeat in addLiquidityActual() to zcfSeat. That cost me time!

* refactor: update getCurrentPrice

update prices that changed in tests.

* refactor: reduce four methods in pool for calculating prices to two

And use price improvements in multipoolAutoswap

* refactor: share & publish price calculations from getCurrentPrice.js

* fix: add a test, fix a bug, remove redundant code
* docs: add a description of the four methods for getting prices.

Author: Chris-Hibbert

packages/zoe/src/contracts/multipoolAutoswap/getCurrentPrice.js

@@ -1,16 +1,118 @@
 import '../../../exported';
 
 /**
- * Build functions to calculate prices for multipoolAutoswap. Two methods are
- * returned. In one the caller specifies the amount they will pay, and in the
- * other they specify the amount they wish to receive.
+ * Build functions to calculate prices for multipoolAutoswap. Four methods are
+ * returned, as two complementary pairs. In one pair of methods the caller
+ * specifies the amount they will pay, and in the other they specify the amount
+ * they wish to receive. The two with shorter names (getOutputForGivenInput,
+ * getInputForGivenOutput) are consistent with the uniswap interface. They each
+ * return a single amount. The other two return { amountIn, centralAmount,
+ * amountOut }, which specifies the best exchange consistent with the request.
+ * centralAmount is omitted from these methods' results in the publicFacet.
  *
  * @param {(brand: Brand) => boolean} isSecondary
  * @param {(brand: Brand) => boolean} isCentral
  * @param {(brand: Brand) => Pool} getPool
+ * @param {Brand} centralBrand
  */
 
-export const makeGetCurrentPrice = (isSecondary, isCentral, getPool) => {
+export const makeGetCurrentPrice = (
+  isSecondary,
+  isCentral,
+  getPool,
+  centralBrand,
+) => {
+  const getPriceGivenAvailableInput = (amountIn, brandOut) => {
+    const { brand: brandIn } = amountIn;
+
+    if (isCentral(brandIn) && isSecondary(brandOut)) {
+      return getPool(brandOut).getPriceGivenAvailableInput(amountIn, brandOut);
+    } else if (isSecondary(brandIn) && isCentral(brandOut)) {
+      return getPool(brandIn).getPriceGivenAvailableInput(amountIn, brandOut);
+    } else if (isSecondary(brandIn) && isSecondary(brandOut)) {
+      // We must do two consecutive getPriceGivenAvailableInput() calls,
+      // followed by a call to getPriceGivenRequiredOutput().
+      // 1) from amountIn to the central token, which tells us how much central
+      // would be provided for amountIn (centralAmount)
+      // 2) from centralAmount to brandOut, which tells us how much of brandOut
+      // will be provided (amountOut) as well as the minimum price in central
+      // tokens (reducedCentralAmount), then finally
+      // 3) call getPriceGivenRequiredOutput() to see if the same proceeds can
+      // be purchased for less (reducedAmountIn).
+
+      const brandInPool = getPool(brandIn);
+      const brandOutPool = getPool(brandOut);
+      const {
+        amountOut: centralAmount,
+      } = brandInPool.getPriceGivenAvailableInput(amountIn, centralBrand);
+      const {
+        amountIn: reducedCentralAmount,
+        amountOut,
+      } = brandOutPool.getPriceGivenAvailableInput(centralAmount, brandOut);
+
+      // propagate reduced prices back to the first pool
+      const {
+        amountIn: reducedAmountIn,
+      } = brandInPool.getPriceGivenRequiredOutput(
+        brandIn,
+        reducedCentralAmount,
+      );
+      return {
+        amountIn: reducedAmountIn,
+        amountOut,
+        centralAmount: reducedCentralAmount,
+      };
+    }
+
+    throw new Error(`brands were not recognized`);
+  };
+
+  const getPriceGivenRequiredOutput = (brandIn, amountOut) => {
+    const { brand: brandOut } = amountOut;
+
+    if (isCentral(brandIn) && isSecondary(brandOut)) {
+      return getPool(brandOut).getPriceGivenRequiredOutput(brandIn, amountOut);
+    } else if (isSecondary(brandIn) && isCentral(brandOut)) {
+      return getPool(brandIn).getPriceGivenRequiredOutput(brandIn, amountOut);
+    } else if (isSecondary(brandIn) && isSecondary(brandOut)) {
+      // We must do two consecutive getPriceGivenRequiredOutput() calls,
+      // followed by a call to getPriceGivenAvailableInput().
+      // 1) from amountOut to the central token, which tells us how much central
+      // is required to obtain amountOut (centralAmount)
+      // 2) from centralAmount to brandIn, which tells us how much of brandIn
+      // is required (amountIn) as well as the max proceeds in central
+      // tokens (improvedCentralAmount), then finally
+      // 3) call getPriceGivenAvailableInput() to see if improvedCentralAmount
+      // produces a larger amount (improvedAmountOut)
+
+      const brandInPool = getPool(brandIn);
+      const brandOutPool = getPool(brandOut);
+
+      const {
+        amountIn: centralAmount,
+      } = brandOutPool.getPriceGivenRequiredOutput(centralBrand, amountOut);
+      const {
+        amountIn,
+        amountOut: improvedCentralAmount,
+      } = brandInPool.getPriceGivenRequiredOutput(brandIn, centralAmount);
+
+      // propagate improved prices
+      const {
+        amountOut: improvedAmountOut,
+      } = brandOutPool.getPriceGivenAvailableInput(
+        improvedCentralAmount,
+        brandOut,
+      );
+      return {
+        amountIn,
+        amountOut: improvedAmountOut,
+        centralAmount: improvedCentralAmount,
+      };
+    }
+
+    throw new Error(`brands were not recognized`);
+  };
+
   /**
    * `getOutputForGivenInput` calculates the result of a trade, given a certain
    * amount of digital assets in.
@@ -21,29 +123,7 @@ export const makeGetCurrentPrice = (isSecondary, isCentral, getPool) => {
    * @returns {Amount} the amount that would be paid out at the current price.
    */
   const getOutputForGivenInput = (amountIn, brandOut) => {
-    const { brand: brandIn, value: inputValue } = amountIn;
-
-    if (isCentral(brandIn) && isSecondary(brandOut)) {
-      return getPool(brandOut).getCentralToSecondaryInputPrice(inputValue);
-    }
-
-    if (isSecondary(brandIn) && isCentral(brandOut)) {
-      return getPool(brandIn).getSecondaryToCentralInputPrice(inputValue);
-    }
-
-    if (isSecondary(brandIn) && isSecondary(brandOut)) {
-      // We must do two consecutive calls to get the price: from
-      // the brandIn to the central token, then from the central
-      // token to the brandOut.
-      const centralTokenAmount = getPool(
-        brandIn,
-      ).getSecondaryToCentralInputPrice(inputValue);
-      return getPool(brandOut).getCentralToSecondaryInputPrice(
-        centralTokenAmount.value,
-      );
-    }
-
-    throw new Error(`brands were not recognized`);
+    return getPriceGivenAvailableInput(amountIn, brandOut).amountOut;
   };
 
   /**
@@ -55,30 +135,13 @@ export const makeGetCurrentPrice = (isSecondary, isCentral, getPool) => {
    * @returns {Amount} The amount required to be paid in order to gain amountOut
    */
   const getInputForGivenOutput = (amountOut, brandIn) => {
-    const { brand: brandOut, value: outputValue } = amountOut;
-
-    if (isCentral(brandIn) && isSecondary(brandOut)) {
-      return getPool(brandOut).getCentralToSecondaryOutputPrice(outputValue);
-    }
-
-    if (isSecondary(brandIn) && isCentral(brandOut)) {
-      return getPool(brandIn).getSecondaryToCentralOutputPrice(outputValue);
-    }
-
-    if (isSecondary(brandIn) && isSecondary(brandOut)) {
-      // We must do two consecutive calls to get the price: from
-      // the brandIn to the central token, then from the central
-      // token to the brandOut.
-      const centralTokenAmount = getPool(
-        brandIn,
-      ).getSecondaryToCentralOutputPrice(outputValue);
-      return getPool(brandOut).getCentralToSecondaryOutputPrice(
-        centralTokenAmount.value,
-      );
-    }
-
-    throw new Error(`brands were not recognized`);
+    return getPriceGivenRequiredOutput(brandIn, amountOut).amountIn;
   };
 
-  return { getOutputForGivenInput, getInputForGivenOutput };
+  return {
+    getOutputForGivenInput,
+    getInputForGivenOutput,
+    getPriceGivenRequiredOutput,
+    getPriceGivenAvailableInput,
+  };
 };

packages/zoe/src/contracts/multipoolAutoswap/multipoolAutoswap.js

@@ -80,11 +80,39 @@ const start = zcf => {
   const {
     getOutputForGivenInput,
     getInputForGivenOutput,
-  } = makeGetCurrentPrice(isSecondary, isCentral, getPool);
+    getPriceGivenAvailableInput: getInternalPriceGivenAvailableInput,
+    getPriceGivenRequiredOutput: getInternalPriceGivenRequiredOutput,
+  } = makeGetCurrentPrice(isSecondary, isCentral, getPool, centralBrand);
+
+  const getPriceGivenRequiredOutput = (brandIn, amountOutInitial) => {
+    const { amountIn, amountOut } = getInternalPriceGivenRequiredOutput(
+      brandIn,
+      amountOutInitial,
+    );
+    // dropping centralAmount from the public method
+    return { amountIn, amountOut };
+  };
+
+  const getPriceGivenAvailableInput = (amountInInitial, brandOut) => {
+    const { amountIn, amountOut } = getInternalPriceGivenAvailableInput(
+      amountInInitial,
+      brandOut,
+    );
+    // dropping centralAmount from the public method
+    return { amountIn, amountOut };
+  };
+
   const {
     makeSwapInInvitation,
     makeSwapOutInvitation,
-  } = makeMakeSwapInvitation(zcf, isSecondary, isCentral, getPool);
+  } = makeMakeSwapInvitation(
+    zcf,
+    isSecondary,
+    isCentral,
+    getPool,
+    getInternalPriceGivenAvailableInput,
+    getInternalPriceGivenRequiredOutput,
+  );
   const makeAddLiquidityInvitation = makeMakeAddLiquidityInvitation(
     zcf,
     getPool,
@@ -103,6 +131,8 @@ const start = zcf => {
     getLiquiditySupply,
     getInputPrice: getOutputForGivenInput,
     getOutputPrice: getInputForGivenOutput,
+    getPriceGivenRequiredOutput,
+    getPriceGivenAvailableInput,
     makeSwapInvitation: makeSwapInInvitation,
     makeSwapInInvitation,
     makeSwapOutInvitation,