From 068f4b141c21d2fdf9958e69f35a614fa0899da5 Mon Sep 17 00:00:00 2001 From: Dean Tribble Date: Mon, 14 Sep 2020 15:35:42 -0700 Subject: [PATCH] fix: simplify helper APIs (#1732) Switch the calling convention for autoswap helpers to passing positional arguments. The use of a params object doesn't work very well in JS. It's exacerbated if we need to use `harden`, and the value is reduced now that we have pervasive types on helpers. Co-authored-by: Dean Tribble --- .../zoe/src/contractSupport/bondingCurves.js | 52 ++++++----- packages/zoe/src/contracts/autoswap.js | 70 +++++++-------- .../src/contracts/multipoolAutoswap/pool.js | 86 +++++++++---------- .../contractSupport/test-bondingCurves.js | 41 ++------- 4 files changed, 105 insertions(+), 144 deletions(-) diff --git a/packages/zoe/src/contractSupport/bondingCurves.js b/packages/zoe/src/contractSupport/bondingCurves.js index 7e318bd0c4b..b1f943f08bb 100644 --- a/packages/zoe/src/contractSupport/bondingCurves.js +++ b/packages/zoe/src/contractSupport/bondingCurves.js @@ -15,24 +15,23 @@ const { add, subtract, multiply, floorDivide } = natSafeMath; * request, and to do the actual reallocation after an offer has * been made. * - * @param {Object} params - * @param {number} params.inputValue - the value of the asset sent + * @param {number} inputValue - the value of the asset sent * in to be swapped - * @param {number} params.inputReserve - the value in the liquidity + * @param {number} inputReserve - the value in the liquidity * pool of the kind of asset sent in - * @param {number} params.outputReserve - the value in the liquidity + * @param {number} outputReserve - the value in the liquidity * pool of the kind of asset to be sent out - * @param {number} [params.feeBasisPoints=30] - the fee taken in - * basis points. The default is 0.3% or 30 basis points. The fee is taken from - * inputValue + * @param {number} [feeBasisPoints=30] - the fee taken in + * basis points. The default is 0.3% or 30 basis points. The fee + * is taken from inputValue * @returns {number} outputValue - the current price, in value form */ -export const getInputPrice = ({ +export const getInputPrice = ( inputValue, inputReserve, outputReserve, feeBasisPoints = 30, -}) => { +) => { const oneMinusFeeInTenThousandths = subtract(10000, feeBasisPoints); const inputWithFee = multiply(inputValue, oneMinusFeeInTenThousandths); const numerator = multiply(inputWithFee, outputReserve); @@ -49,24 +48,23 @@ export const getInputPrice = ({ * request, and to do the actual reallocation after an offer has * been made. * - * @param {Object} params - * @param {number} params.outputValue - the value of the asset the user wants + * @param {number} outputValue - the value of the asset the user wants * to get - * @param {number} params.inputReserve - the value in the liquidity + * @param {number} inputReserve - the value in the liquidity * pool of the asset being spent - * @param {number} params.outputReserve - the value in the liquidity + * @param {number} outputReserve - the value in the liquidity * pool of the kind of asset to be sent out - * @param {number} [params.feeBasisPoints=30] - the fee taken in + * @param {number} [feeBasisPoints=30] - the fee taken in * basis points. The default is 0.3% or 30 basis points. The fee is taken from * outputValue * @returns {number} inputValue - the value of input required to purchase output */ -export const getOutputPrice = ({ +export const getOutputPrice = ( outputValue, inputReserve, outputReserve, feeBasisPoints = 30, -}) => { +) => { const oneMinusFeeInTenThousandths = subtract(10000, feeBasisPoints); const numerator = multiply(multiply(outputValue, inputReserve), 10000); const denominator = multiply( @@ -86,11 +84,11 @@ function assertDefined(label, value) { // liquidity multiplied by the ratio of new central tokens to central tokens // already held. If the current supply is zero, return the inputValue as the // initial liquidity to mint is arbitrary. -export const calcLiqValueToMint = ({ +export const calcLiqValueToMint = ( liqTokenSupply, inputValue, inputReserve, -}) => { +) => { assertDefined('liqTokenSupply', liqTokenSupply); assertDefined('inputValue', inputValue); assertDefined('inputReserve', inputReserve); @@ -98,7 +96,6 @@ export const calcLiqValueToMint = ({ if (liqTokenSupply === 0) { return inputValue; } - return floorDivide(multiply(inputValue, liqTokenSupply), inputReserve); }; @@ -107,20 +104,19 @@ export const calcLiqValueToMint = ({ * adding liquidity. We require that the deposited ratio of central to secondary * match the current ratio of holdings in the pool. * - * @param {Object} params - * @param {number} params.centralIn - The value of central assets being deposited - * @param {number} params.centralPool - The value of central assets in the pool - * @param {number} params.secondaryPool - The value of secondary assets in the pool - * @param {number} params.secondaryIn - The value of secondary assets provided. If + * @param {number} centralIn - The value of central assets being deposited + * @param {number} centralPool - The value of central assets in the pool + * @param {number} secondaryPool - The value of secondary assets in the pool + * @param {number} secondaryIn - The value of secondary assets provided. If * the pool is empty, the entire amount will be accepted * @returns {number} - the amount of secondary required */ -export const calcSecondaryRequired = ({ +export const calcSecondaryRequired = ( centralIn, centralPool, secondaryPool, secondaryIn, -}) => { +) => { assertDefined('centralIn', centralIn); assertDefined('centralPool', centralPool); assertDefined('secondaryReserve', secondaryPool); @@ -143,11 +139,11 @@ export const calcSecondaryRequired = ({ // Calculate how many underlying tokens (in the form of a value) should be // returned when removing liquidity. -export const calcValueToRemove = ({ +export const calcValueToRemove = ( liqTokenSupply, poolValue, liquidityValueIn, -}) => { +) => { assertDefined('liqTokenSupply', liqTokenSupply); assertDefined('liquidityValueIn', liquidityValueIn); assertDefined('poolValue', poolValue); diff --git a/packages/zoe/src/contracts/autoswap.js b/packages/zoe/src/contracts/autoswap.js index 940db33b93c..2b33db4dcef 100644 --- a/packages/zoe/src/contracts/autoswap.js +++ b/packages/zoe/src/contracts/autoswap.js @@ -143,11 +143,9 @@ const start = async zcf => { } = swapSeat.getProposal(); const outputValue = getInputPrice( - harden({ - inputValue: amountIn.value, - inputReserve: getPoolAmount(amountIn.brand).value, - outputReserve: getPoolAmount(wantedAmountOut.brand).value, - }), + amountIn.value, + getPoolAmount(amountIn.brand).value, + getPoolAmount(wantedAmountOut.brand).value, ); const outAmountMath = zcf.getAmountMath(wantedAmountOut.brand); const tradeAmountOut = outAmountMath.make(outputValue); @@ -173,11 +171,11 @@ const start = async zcf => { want: { Out: wantedAmountOut }, } = swapSeat.getProposal(); - const tradePrice = getOutputPrice({ - outputValue: wantedAmountOut.value, - inputReserve: getPoolAmount(amountIn.brand).value, - outputReserve: getPoolAmount(wantedAmountOut.brand).value, - }); + const tradePrice = getOutputPrice( + wantedAmountOut.value, + getPoolAmount(amountIn.brand).value, + getPoolAmount(wantedAmountOut.brand).value, + ); assert(tradePrice <= amountIn.value, 'amountIn insufficient'); const inAmountMath = zcf.getAmountMath(amountIn.brand); const tradeAmountIn = inAmountMath.make(tradePrice); @@ -190,11 +188,9 @@ const start = async zcf => { const centralPool = getPoolAmount(brands.Central).value; const centralIn = userAllocation.Central.value; const liquidityValueOut = calcLiqValueToMint( - harden({ - liqTokenSupply, - inputValue: centralIn, - inputReserve: centralPool, - }), + liqTokenSupply, + centralIn, + centralPool, ); const liquidityAmountOut = liquidityMath.make(liquidityValueOut); liquidityMint.mintGains({ Liquidity: liquidityAmountOut }, poolSeat); @@ -248,12 +244,12 @@ const start = async zcf => { // To calculate liquidity, we'll need to calculate alpha from the primary // token's value before, and the value that will be added to the pool const secondaryOut = secondaryMath.make( - calcSecondaryRequired({ - centralIn: userAllocation.Central.value, - centralPool: getPoolAmount(brands.Central).value, - secondaryPool: getPoolAmount(brands.Secondary).value, - secondaryIn: secondaryIn.value, - }), + calcSecondaryRequired( + userAllocation.Central.value, + getPoolAmount(brands.Central).value, + getPoolAmount(brands.Secondary).value, + secondaryIn.value, + ), ); // Central was specified precisely so offer must provide enough secondary. @@ -277,20 +273,16 @@ const start = async zcf => { const newUserCentralAmount = centralMath.make( calcValueToRemove( - harden({ - liqTokenSupply, - poolValue: getPoolAmount(brands.Central).value, - liquidityValueIn, - }), + liqTokenSupply, + getPoolAmount(brands.Central).value, + liquidityValueIn, ), ); const newUserSecondaryAmount = secondaryMath.make( calcValueToRemove( - harden({ - liqTokenSupply, - poolValue: getPoolAmount(brands.Secondary).value, - liquidityValueIn, - }), + liqTokenSupply, + getPoolAmount(brands.Secondary).value, + liquidityValueIn, ), ); @@ -339,11 +331,9 @@ const start = async zcf => { const inputReserve = getPoolAmount(amountIn.brand).value; const outputReserve = getPoolAmount(brandOut).value; const outputValue = getInputPrice( - harden({ - inputValue: amountIn.value, - inputReserve, - outputReserve, - }), + amountIn.value, + inputReserve, + outputReserve, ); return zcf.getAmountMath(brandOut).make(outputValue); }; @@ -359,11 +349,9 @@ const start = async zcf => { const inputReserve = getPoolAmount(brandIn).value; const outputReserve = getPoolAmount(amountOut.brand).value; const outputValue = getOutputPrice( - harden({ - outputValue: amountOut.value, - inputReserve, - outputReserve, - }), + amountOut.value, + inputReserve, + outputReserve, ); return zcf.getAmountMath(brandIn).make(outputValue); }; diff --git a/packages/zoe/src/contracts/multipoolAutoswap/pool.js b/packages/zoe/src/contracts/multipoolAutoswap/pool.js index 8f2e7f000ff..daf61ca0566 100644 --- a/packages/zoe/src/contracts/multipoolAutoswap/pool.js +++ b/packages/zoe/src/contracts/multipoolAutoswap/pool.js @@ -31,11 +31,9 @@ export const makeAddPool = (zcf, isSecondary, initPool, centralBrand) => { const addLiquidityActual = (pool, userSeat, secondaryAmount) => { const liquidityValueOut = calcLiqValueToMint( - harden({ - liqTokenSupply, - inputValue: userSeat.getAmountAllocated('Central').value, - inputReserve: pool.getCentralAmount().value, - }), + liqTokenSupply, + userSeat.getAmountAllocated('Central').value, + pool.getCentralAmount().value, ); const liquidityAmountOut = liquidityAmountMath.make(liquidityValueOut); @@ -78,38 +76,38 @@ export const makeAddPool = (zcf, isSecondary, initPool, centralBrand) => { poolSeat.getAmountAllocated('Secondary', secondaryBrand), getCentralToSecondaryInputPrice: inputValue => { assertPoolInitialized(pool); - const result = getInputPrice({ + const result = getInputPrice( inputValue, - inputReserve: pool.getCentralAmount().value, - outputReserve: pool.getSecondaryAmount().value, - }); + pool.getCentralAmount().value, + pool.getSecondaryAmount().value, + ); return pool.getAmountMath().make(result); }, getSecondaryToCentralInputPrice: inputValue => { assertPoolInitialized(pool); - const result = getInputPrice({ + const result = getInputPrice( inputValue, - inputReserve: pool.getSecondaryAmount().value, - outputReserve: pool.getCentralAmount().value, - }); + pool.getSecondaryAmount().value, + pool.getCentralAmount().value, + ); return pool.getCentralAmountMath().make(result); }, getCentralToSecondaryOutputPrice: outputValue => { assertPoolInitialized(pool); - const result = getOutputPrice({ + const result = getOutputPrice( outputValue, - inputReserve: pool.getCentralAmount().value, - outputReserve: pool.getSecondaryAmount().value, - }); + pool.getCentralAmount().value, + pool.getSecondaryAmount().value, + ); return pool.getAmountMath().make(result); }, getSecondaryToCentralOutputPrice: outputValue => { assertPoolInitialized(pool); - const result = getOutputPrice({ + const result = getOutputPrice( outputValue, - inputReserve: pool.getSecondaryAmount().value, - outputReserve: pool.getCentralAmount().value, - }); + pool.getSecondaryAmount().value, + pool.getCentralAmount().value, + ); return pool.getCentralAmountMath().make(result); }, addLiquidity: userSeat => { @@ -123,14 +121,16 @@ export const makeAddPool = (zcf, isSecondary, initPool, centralBrand) => { // To calculate liquidity, we'll need to calculate alpha from the primary // token's value before, and the value that will be added to the pool - const secondaryOut = pool.getAmountMath().make( - calcSecondaryRequired({ - centralIn: userAllocation.Central.value, - centralPool: pool.getCentralAmount().value, - secondaryPool: pool.getSecondaryAmount().value, - secondaryIn: secondaryIn.value, - }), - ); + const secondaryOut = pool + .getAmountMath() + .make( + calcSecondaryRequired( + userAllocation.Central.value, + pool.getCentralAmount().value, + pool.getSecondaryAmount().value, + secondaryIn.value, + ), + ); // Central was specified precisely so offer must provide enough secondary. assert( @@ -146,25 +146,25 @@ export const makeAddPool = (zcf, isSecondary, initPool, centralBrand) => { liquidityBrand, ); const liquidityValueIn = liquidityIn.value; - const centralTokenAmountOut = pool.getCentralAmountMath().make( - calcValueToRemove( - harden({ + const centralTokenAmountOut = pool + .getCentralAmountMath() + .make( + calcValueToRemove( liqTokenSupply, - poolValue: pool.getCentralAmount().value, + pool.getCentralAmount().value, liquidityValueIn, - }), - ), - ); + ), + ); - const tokenKeywordAmountOut = pool.getAmountMath().make( - calcValueToRemove( - harden({ + const tokenKeywordAmountOut = pool + .getAmountMath() + .make( + calcValueToRemove( liqTokenSupply, - poolValue: pool.getSecondaryAmount().value, + pool.getSecondaryAmount().value, liquidityValueIn, - }), - ), - ); + ), + ); liqTokenSupply -= liquidityValueIn; diff --git a/packages/zoe/test/unitTests/contractSupport/test-bondingCurves.js b/packages/zoe/test/unitTests/contractSupport/test-bondingCurves.js index aa2a0c84f3c..d45f6642359 100644 --- a/packages/zoe/test/unitTests/contractSupport/test-bondingCurves.js +++ b/packages/zoe/test/unitTests/contractSupport/test-bondingCurves.js @@ -7,8 +7,12 @@ import { calcLiqValueToMint, } from '../../../src/contractSupport'; -const testGetPrice = (t, input, expectedOutput) => { - const output = getInputPrice(input); +const testGetPrice = ( + t, + { inputReserve, outputReserve, inputValue }, + expectedOutput, +) => { + const output = getInputPrice(inputValue, inputReserve, outputReserve); t.deepEqual(output, expectedOutput); }; @@ -87,43 +91,16 @@ test('getInputPrice ok 6', t => { }); test('calculate value to mint - positive supply 1', t => { - const res = calcLiqValueToMint({ - liqTokenSupply: 20, - inputValue: 30, - inputReserve: 5, - }); + const res = calcLiqValueToMint(20, 30, 5); t.is(res, (20 * 30) / 5, 'When supply is present, floor(x*y/z)'); }); -test('calculate value to mint - mispelled key', t => { - t.throws( - () => - calcLiqValueToMint({ - liquidityTokenSupply: 20, - inputValue: 30, - inputReserve: 5, - }), - { - message: /value required/, - }, - `calcLiqValueToMint should throw if a key is misspelled`, - ); -}); - test('calculate value to mint - positive supply 2', t => { - const res = calcLiqValueToMint({ - liqTokenSupply: 5, - inputValue: 8, - inputReserve: 7, - }); + const res = calcLiqValueToMint(5, 8, 7); t.is(res, 5, 'When supply is present, floor(x*y/z)'); }); test('calculate value to mint - no supply', t => { - const res = calcLiqValueToMint({ - liqTokenSupply: 0, - inputValue: 30, - inputReserve: 5, - }); + const res = calcLiqValueToMint(0, 30, 5); t.is(res, 30, 'When the supply is empty, return inputValue'); });