fix: off-by-one: minimum positive result of getOutputPrice() is 1 (#2198

) * fix: off-by-one: minimum positive result of getOutputPrice() is 1 * fix: add one as BigInt before converting to Nat.
Agoric · Jan 16, 2021 · 82d68d9 · 82d68d9
1 parent 3cb4606
commit 82d68d9
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 9 deletions.
diff --git a/packages/zoe/src/contractSupport/bondingCurves.js b/packages/zoe/src/contractSupport/bondingCurves.js
@@ -8,6 +8,7 @@ const { multiply, floorDivide } = natSafeMath;
 // We use this workaround due to some parser in our toolchain that can't parse
 // bigint literals.
 const BIG_10000 = BigInt(10000);
+const BIG_ONE = BigInt(1);
 
 /**
  * Calculations for constant product markets like Uniswap.
@@ -98,7 +99,7 @@ export const getOutputPrice = (
   const numerator = BigInt(outputValue) * BigInt(inputReserve) * BIG_10000;
   const denominator =
     (BigInt(outputReserve) - BigInt(outputValue)) * oneMinusFeeScaled;
-  return Nat(Number(numerator / denominator));
+  return Nat(Number(numerator / denominator + BIG_ONE));
 };
 
 function assertDefined(label, value) {

diff --git a/packages/zoe/test/autoswapJig.js b/packages/zoe/test/autoswapJig.js
@@ -33,16 +33,18 @@ export const outputFromInputPrice = (xPre, yPre, deltaX, fee) => {
   );
 };
 
-// deltaX = beta * xPre / ( (1 - beta) * gamma )
+// deltaX = (beta * xPre / ( (1 - beta) * gamma )) + 1
 // gamma is (10000 - fee) / 10000
 // beta is deltaY / yPre
 // reducing to a single division:
-//    deltaX = deltaY * xPre * 10000 / (yPre - deltaY ) * gammaNum)
+//    deltaX = (deltaY * xPre * 10000 / (yPre - deltaY ) * gammaNum)) + 1
 export const priceFromTargetOutput = (deltaY, yPre, xPre, fee) => {
   const gammaNumerator = 10000 - fee;
-  return floorDivide(
-    multiply(multiply(deltaY, xPre), 10000),
-    multiply(subtract(yPre, deltaY), gammaNumerator),
+  return (
+    floorDivide(
+      multiply(multiply(deltaY, xPre), 10000),
+      multiply(subtract(yPre, deltaY), gammaNumerator),
+    ) + 1
   );
 };
 

diff --git a/packages/zoe/test/swingsetTests/zoe/test-zoe.js b/packages/zoe/test/swingsetTests/zoe/test-zoe.js
@@ -252,7 +252,7 @@ const expectedAutoswapOkLog = [
   'Swap successfully completed.',
   'bobMoolaPurse: balance {"brand":{},"value":5}',
   'bobSimoleanPurse: balance {"brand":{},"value":5}',
-  'simoleans required {"brand":{},"value":4}',
+  'simoleans required {"brand":{},"value":5}',
   'Liquidity successfully removed.',
   'poolAmounts{"Liquidity":{"brand":{},"value":10},"Central":{"brand":{},"value":0},"Secondary":{"brand":{},"value":0}}',
   'aliceMoolaPurse: balance {"brand":{},"value":8}',

diff --git a/packages/zoe/test/unitTests/contractSupport/test-bondingCurves.js b/packages/zoe/test/unitTests/contractSupport/test-bondingCurves.js
@@ -179,7 +179,7 @@ test('getOutputPrice ok', t => {
     inputReserve: 43,
     outputValue: 37,
   };
-  const expectedOutput = 19;
+  const expectedOutput = 20;
   testGetOutputPrice(t, input, expectedOutput);
 });
 
@@ -231,6 +231,16 @@ test('getOutputPrice big product', t => {
     inputReserve: 100000000,
     outputValue: 1000,
   };
-  const expectedOutput = 1003;
+  const expectedOutput = 1004;
+  testGetOutputPrice(t, input, expectedOutput);
+});
+
+test('getOutputPrice minimum price', t => {
+  const input = {
+    outputReserve: 10,
+    inputReserve: 1,
+    outputValue: 1,
+  };
+  const expectedOutput = 1;
   testGetOutputPrice(t, input, expectedOutput);
 });