fix(#34): updates slippage tolerance to use correct multiplier

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@primitivefi/rmm-sdk",
-  "version": "1.1.0-beta.5",
+  "version": "1.1.0-rc.3",
   "description": "∇ A Software Development Kit for Primitive RMM-01.",
   "main": "dist/index.js",
   "typings": "dist/index.d.ts",
@@ -33,8 +33,8 @@
     "prettier": ">=2.0.0"
   },
   "dependencies": {
-    "@primitivefi/rmm-core": "^1.0.0-rc.1",
-    "@primitivefi/rmm-manager": "^1.0.0-rc.1",
+    "@primitivefi/rmm-core": "^1.0.0-rc.2",
+    "@primitivefi/rmm-manager": "^1.0.0-rc.2",
     "@primitivefi/rmm-math": "^2.0.0-rc.1",
     "@uniswap/sdk-core": "^3.0.1",
     "shelljs": "0.8.5",

src/entities/pool.ts

@@ -387,11 +387,19 @@ export class Pool extends Calibration {
 
     switch (sideOfPool) {
       case PoolSides.RISKY:
+        invariant(
+          reserveRisky.gt(0),
+          `Reserve risky is 0. It must be greater than 0 because its being used as a denominator to compute LP tokens to mint.`
+        )
         delRisky = amount
         delLiquidity = liquidity.mul(delRisky).div(reserveRisky)
         delStable = reserveStable.mul(delLiquidity).div(liquidity)
         break
       case PoolSides.STABLE:
+        invariant(
+          reserveStable.gt(0),
+          `Reserve stable is 0. It must be greater than 0 because its being used as a denominator to compute LP tokens to mint.`
+        )
         delStable = amount
         delLiquidity = liquidity.mul(delStable).div(reserveStable)
         delRisky = reserveRisky.mul(delLiquidity).div(liquidity)

src/peripheryManager.ts

@@ -58,24 +58,47 @@ export interface LiquidityOptions {
 /**
  * Provide liquidity argument details.
  *
- * @param recipient Target account that receives liquidity.
+ * @remarks
+ * Slippage tolerance can be safely set to 0,
+ * which will cause the transaction to revert in the case the expected `delLiquidity`
+ * is not granted to the transaction sender.
+ *
+ * @param recipient Address that will be granted the minted Primitive liquidity pool tokens.
  * @param delRisky Amount of risky tokens to provide as liquidity.
  * @param delStable Amount of stable tokens to provide as Liquidity.
  * @param delLiquidity Desired amount of liquidity to mint.
  * @param fromMargin Use margin balance to pay for liquidity deposit.
  * @param slippageTolerance Maximum difference in liquidity received from expected liquidity.
  * @param createPool Create a pool and allocate liquidity to it.
+ * @param useNative Whether or not a native protocol token (e.g. Ether) should be used with a wrapped token version.
  *
  * @beta
  */
-export interface AllocateOptions extends PermitTokens, LiquidityOptions, RecipientOptions, NativeOptions, Deadline {
+export interface AllocateOptions extends PermitTokens, LiquidityOptions, NativeOptions, RecipientOptions {
   fromMargin: boolean
   slippageTolerance: Percentage
   createPool?: boolean
 }
 
-/** Remove liquidity argument details. */
-export interface RemoveOptions extends LiquidityOptions, RecipientOptions, NativeOptions, Deadline {
+/**
+ * Remove liquidity argument details.
+ *
+ * @remarks
+ * Expected risky and stable amounts should be defaulted to 0.
+ *
+ * @param expectedRisky Amount of risky tokens to withdraw from margin account, minRisky is added to this.
+ * @param expectedStable Amount of stable tokens to withdraw from margin account, minStable is added to this.
+ * @param toMargin Whether or not to keep tokens withdrawn from liquidity in margin.
+ * @param slippageTolerance Percentage deviation from the expected token amounts being removed from the pool.
+ * @param recipient Address that will be granted the minted Primitive liquidity pool tokens.
+ * @param delRisky Amount of risky tokens to provide as liquidity.
+ * @param delStable Amount of stable tokens to provide as Liquidity.
+ * @param delLiquidity Desired amount of liquidity to mint.
+ * @param useNative Whether or not a native protocol token (e.g. Ether) should be used with a wrapped token version.
+ *
+ * @beta
+ */
+export interface RemoveOptions extends LiquidityOptions, RecipientOptions, NativeOptions {
   expectedRisky: Wei
   expectedStable: Wei
   toMargin: boolean
@@ -125,12 +148,13 @@ export abstract class PeripheryManager extends SelfPermit {
    * @throws
    * Throws if liquidity amount decimals and pool decimals are not equal.
    * Throws if pool has an undefined {@link IPool.referencePriceOfRisky}.
+   * Throws if `liquidity` is less than {@link IEngine.MIN_LIQUIDITY}.
    *
    * @beta
    */
   public static encodeCreate(pool: Pool, liquidity: Wei): string {
     validateDecimals(liquidity, pool)
-    invariant(typeof pool.referencePriceOfRisky !== 'undefined', `Attempting to create a pool without reference price`)
+    invariant(typeof pool.referencePriceOfRisky !== 'undefined', `Attempting to create a pool without reference price.`)
     const riskyPerLp = parseWei(
       Swaps.getRiskyReservesGivenReferencePrice(
         pool.strike.float,
@@ -141,6 +165,18 @@ export abstract class PeripheryManager extends SelfPermit {
       pool.risky.decimals
     )
 
+    invariant(
+      riskyPerLp.gt(0),
+      `Attempting to create a pool that has 0 risky tokens in the reserves. This is only possible after a pool has expired, has the maturity timestamp already been passed?`
+    )
+    invariant(
+      riskyPerLp.lt(parseWei(1, riskyPerLp.decimals)),
+      `Attempting to create a pool that has 1 risky token per liquidity in the reserves. This is only possible after a pool has expired, has the maturity timestamp already been passed?`
+    )
+    invariant(
+      liquidity.gte(pool.MIN_LIQUIDITY),
+      `Attempting to create a pool and allocating less than MIN_LIQUIDITY amount of liquidity.`
+    )
     return PeripheryManager.INTERFACE.encodeFunctionData('create', [
       pool.risky.address,
       pool.stable.address,
@@ -333,6 +369,8 @@ export abstract class PeripheryManager extends SelfPermit {
    * Throws if any {@link LiquidityOptions} amount decimals does not match respective token decimals.
    * Throws if depositing a currency and the token has an undefined `wrapped` attribute.
    * Throws if attempting to create a pool from a margin balance.
+   * Throws if computed minimum liquidity to receive is 0.
+   * Throws if recipient is AddressZero.
    *
    * @beta
    */
@@ -344,6 +382,9 @@ export abstract class PeripheryManager extends SelfPermit {
     validateDecimals(options.delRisky, pool.risky)
     validateDecimals(options.delStable, pool.stable)
 
+    const recipient: string = validateAndParseAddress(options.recipient)
+    invariant(recipient !== AddressZero, 'Zero Address Recipient')
+
     let calldatas: string[] = []
 
     // if permits
@@ -355,11 +396,16 @@ export abstract class PeripheryManager extends SelfPermit {
       calldatas.push(PeripheryManager.encodePermit(pool.stable, options.permitStable))
     }
 
-    const minLiquidity = options.delLiquidity.mul(options.slippageTolerance.raw).div(Percentage.BasisPoints)
+    const slippageMultiplier = Percentage.BasisPoints - options.slippageTolerance.bps // 100% - slippage%
+    const minLiquidity = options.delLiquidity.mul(slippageMultiplier).div(Percentage.BasisPoints)
+    invariant(
+      minLiquidity.gt(0),
+      `Slippage parameter minLiquidity cannot be zero! ${options.delLiquidity.display} * ${slippageMultiplier} / ${Percentage.BasisPoints} = ${minLiquidity.display} `
+    )
 
     // if curve should be created
     if (options.createPool) {
-      invariant(!options.fromMargin, 'Cannot pay from margin when creating')
+      invariant(!options.fromMargin, 'Cannot pay from margin when creating, set fromMargin to false.')
       calldatas.push(PeripheryManager.encodeCreate(pool, options.delLiquidity))
     } else {
       calldatas.push(
@@ -404,6 +450,13 @@ export abstract class PeripheryManager extends SelfPermit {
    * @param pool {@link IPool} Uses poolId and tokens of Pool entity for remove arguments.
    * @param options {@link RemoveOptions} Remove argument details.
    *
+   * @remarks
+   * Computed min token amounts to receive are used to withdraw them from margin, and expected amounts are added to them.
+   * A high slippageTolerance when removing liquidity could cause a discrepancy in the withdrawn tokens and tokens received in margin.
+   * For this reason, it's best to have a 0 slippage tolerance.
+   * In most cases, 0 slippage tolerance and 0 expected amounts should be used.
+   * Potentially fails if there is not enough margin balance after removing liquidity when attempting to withdraw expected amounts.
+   *
    * @throws
    * Throws if {@link LiquidityOptions.delLiquidity} is zero.
    * Throws if {@link LiquidityOptions} amount decimals does not match respective token decimals.
@@ -419,8 +472,9 @@ export abstract class PeripheryManager extends SelfPermit {
     let calldatas: string[] = []
 
     const { delRisky, delStable } = pool.liquidityQuote(options.delLiquidity, PoolSides.RMM_LP)
-    const minRisky = delRisky.mul(options.slippageTolerance.bps).div(Percentage.BasisPoints)
-    const minStable = delStable.mul(options.slippageTolerance.bps).div(Percentage.BasisPoints)
+    const slippageMultiplier = Percentage.BasisPoints - options.slippageTolerance.bps // 100% - slippage%
+    const minRisky = delRisky.mul(slippageMultiplier).div(Percentage.BasisPoints)
+    const minStable = delStable.mul(slippageMultiplier).div(Percentage.BasisPoints)
 
     // tokens are by default removed from curve and deposited to margin
     calldatas.push(
@@ -438,8 +492,8 @@ export abstract class PeripheryManager extends SelfPermit {
       calldatas.push(
         ...PeripheryManager.encodeWithdraw(pool, {
           recipient: options.recipient,
-          amountRisky: options.expectedRisky,
-          amountStable: options.expectedStable,
+          amountRisky: minRisky.add(options.expectedRisky),
+          amountStable: minStable.add(options.expectedStable),
           useNative: options.useNative
         })
       )

test/PeripheryManager.spec.ts

@@ -1,14 +1,14 @@
-import { parsePercentage, parseWei } from 'web3-units'
+import { BigNumber, ContractFactory } from 'ethers'
+import { parsePercentage, parseWei, Percentage } from 'web3-units'
 import { AddressZero } from '@ethersproject/constants'
 import { Ether, NativeCurrency } from '@uniswap/sdk-core'
 
-import { Pool } from '../src/entities/pool'
+import { Pool, PoolSides } from '../src/entities/pool'
 import { Swaps } from '../src/entities/swaps'
 import { PeripheryManager } from '../src/peripheryManager'
 
 import { AddressOne } from './shared/constants'
 import { usePool, usePoolWithDecimals, useWethPool } from './shared/fixture'
-import { BigNumber, ContractFactory } from 'ethers'
 
 function decode(frag: string, data: any) {
   return PeripheryManager.INTERFACE.decodeFunctionData(frag, data)
@@ -331,6 +331,28 @@ describe('Periphery Manager', function() {
       expect(value).toBe('0x00')
     })
 
+    it('should have a minLiquidity equal to delLiquidity when slippageTolerance is 0', async function() {
+      const recipient = from
+      const fromMargin = false
+      const delRisky = parseWei(0.3, pool.risky.decimals)
+      const delStable = parseWei(3, pool.stable.decimals)
+      const delLiquidity = parseWei(1, 18)
+
+      const { calldata, value } = PeripheryManager.allocateCallParameters(pool, {
+        recipient,
+        fromMargin,
+        delRisky,
+        delStable,
+        delLiquidity,
+        slippageTolerance: parsePercentage(0)
+      })
+      const data = [pool.poolId, pool.risky.address, pool.stable.address, delRisky.raw, delStable.raw, fromMargin]
+      const decoded = decode('allocate', calldata)
+      data.forEach((item, i) => expect(item.toString()).toStrictEqual(decoded[i].toString()))
+      expect(value).toBe('0x00')
+      expect(decoded[decoded.length - 1].toString()).toStrictEqual(delLiquidity.toString())
+    })
+
     it('successful using native', async function() {
       const recipient = from
       const fromMargin = false
@@ -365,6 +387,12 @@ describe('Periphery Manager', function() {
       const refundETHDecoded = decode('refundETH', multicall.data[1])
       expect(refundETHDecoded).toBeDefined()
       expect(value).toBe(delRisky.raw.toHexString())
+      expect(allocateDecoded[allocateDecoded.length - 1].toString()).toStrictEqual(
+        delLiquidity
+          .mul(Percentage.BasisPoints - slippageTolerance.bps)
+          .div(Percentage.BasisPoints)
+          .toString()
+      )
     })
 
     it('successful when creating pool instead', async function() {
@@ -559,6 +587,33 @@ describe('Periphery Manager', function() {
       expect(value).toBe('0x00')
     })
 
+    it('should have same minRisky as expectedRisky and same minStable as expectedStable with 0 slippage tolerance', async function() {
+      const recipient = from
+      const toMargin = true
+      const delLiquidity = parseWei(1, 18)
+      const { delRisky, delStable } = pool.liquidityQuote(delLiquidity, PoolSides.RMM_LP)
+      const expectedRisky = delRisky
+      const expectedStable = delStable
+
+      const { calldata, value } = PeripheryManager.removeCallParameters(pool, {
+        delLiquidity,
+        expectedRisky,
+        expectedStable,
+        toMargin,
+        delRisky,
+        delStable,
+        recipient,
+        slippageTolerance: parsePercentage(0)
+      })
+
+      const data = [pool.address, pool.poolId, delLiquidity.raw]
+      const decoded = decode('remove', calldata)
+      data.forEach((item, i) => expect(item.toString()).toStrictEqual(decoded[i].toString()))
+      expect(value).toBe('0x00')
+      expect(decoded[decoded.length - 2].toString()).toStrictEqual(delRisky.toString())
+      expect(decoded[decoded.length - 1].toString()).toStrictEqual(delStable.toString())
+    })
+
     it('fails if delLiquidity is zero', async function() {
       const recipient = from
       const toMargin = false

yarn.lock

@@ -1740,18 +1740,18 @@
   resolved "https://registry.yarnpkg.com/@openzeppelin/contracts/-/contracts-4.4.1.tgz#3382db2cd83ab565ed9626765e7da92944b45de8"
   integrity sha512-o+pHCf/yMLSlV5MkDQEzEQL402i6SoRnktru+0rdSxVEFZcTzzGhZCAtZjUFyKGazMSv1TilzMg+RbED1N8XHQ==
 
-"@primitivefi/rmm-core@^1.0.0-rc.1":
-  version "1.0.0-rc.1"
-  resolved "https://registry.yarnpkg.com/@primitivefi/rmm-core/-/rmm-core-1.0.0-rc.1.tgz#42e378e38d2f8655507c7e790de71c0a10622ff9"
-  integrity sha512-q9Syd9nSWv3xpfMIkPikmUDfDILubtWg8U1xHdlxOwxgQ/f33ZxnA0zje8XoAoce6utwTOn3MAbGpvxe4MIN6g==
+"@primitivefi/rmm-core@^1.0.0-rc.2":
+  version "1.0.0-rc.2"
+  resolved "https://registry.yarnpkg.com/@primitivefi/rmm-core/-/rmm-core-1.0.0-rc.2.tgz#b30741108d1bc1fe52dd1db32b449cdf7919ffd6"
+  integrity sha512-dX58g1WcKfm+nWNZtWtPPMJp1GYe6Py0PxDbc7tvcTEXTYgf2/FHYUjEO9KEKqrFKgPLHh3B2NQrCIiXvHE/DQ==
 
-"@primitivefi/rmm-manager@^1.0.0-rc.1":
-  version "1.0.0-rc.1"
-  resolved "https://registry.yarnpkg.com/@primitivefi/rmm-manager/-/rmm-manager-1.0.0-rc.1.tgz#6ca21e96d14f304e56e02aefaaea15d5a3352a2a"
-  integrity sha512-+aJOw8eU9b0B1dZOWkIWoAbApVOMf3DGzmKRBSV4ugSd2mMdlgMMoFosVpd3AZU4n79yBzM5LgvxM7y6Ley+sA==
+"@primitivefi/rmm-manager@^1.0.0-rc.2":
+  version "1.0.0-rc.2"
+  resolved "https://registry.yarnpkg.com/@primitivefi/rmm-manager/-/rmm-manager-1.0.0-rc.2.tgz#5740eceafdb17fa7f24b835eb43afaf1e9b3eb95"
+  integrity sha512-hrWxaTE7Zgh29MR2DSjxJcjWC4xRQygg4YtHmvC4rMVwGw3VGRjsXWg3qITUsor8xza0WYMQPFGohYmGT/tKGQ==
   dependencies:
     "@openzeppelin/contracts" "^4.1.0"
-    "@primitivefi/rmm-core" "^1.0.0-rc.1"
+    "@primitivefi/rmm-core" "^1.0.0-rc.2"
     base64-sol "^1.1.0"
 
 "@primitivefi/rmm-math@^2.0.0-rc.1":