Performance Fee Calculator Adjust Fee Fix and Scenario Tests (#637)

* Performance fee updates

* handle case

* Set up scenario test

* Add checkpoints

* Commit changes

* Finish testing

* Fix

* Finish assertions

* Fix test

* Fix tests

* Add comment

.env.default

@@ -1,2 +1,4 @@
 # IS DEVELOPMENT AND USING BUIDLER
-IS_BUIDLER=false
+IS_BUIDLER=false
+
+# To use console.log, do `import @nomiclabs/buidler/console.sol`

contracts/core/fee-calculators/PerformanceFeeCalculator.sol

@@ -37,6 +37,11 @@ import { SetUSDValuation } from "../liquidators/impl/SetUSDValuation.sol";
  * Smart contract that stores and returns fees (represented as scaled decimal values). Fees are
  * determined based on performance of the Set and a streaming fee. Set values can be denominated
  * in any any asset based on oracle white list used in deploy.
+ *
+ * CHANGELOG:
+ * - 5/17/2020: Update adjustFee function to update high watermark to prevent unexpected fee actualizations
+ *              when the profitFee was initially 0. We also disallow changing the profit fee if the the fee period
+ *              has not elapsed.
  */
 contract PerformanceFeeCalculator is IFeeCalculator {
 
@@ -258,6 +263,23 @@ contract PerformanceFeeCalculator is IFeeCalculator {
         } else {
             validateProfitFeePercentage(feePercentage);
 
+            // IMPORTANT: In the case that a profit fee is initially 0 and is set to a non-zero number,
+            // the actualizeFee / updateFeeState function does not update the high watermark
+            // Thus, we need to reset the high water mark here so that users do not pay for profit fees
+            // since inception.
+            uint256 rebalancingSetValue = SetUSDValuation.calculateRebalancingSetValue(msg.sender, oracleWhiteList);
+            uint256 existingHighwatermark = feeState[msg.sender].highWatermark;
+            if (rebalancingSetValue > existingHighwatermark) {
+                // In the case the profit fee period hasn't elapsed, disallow changing fees
+                require(
+                    exceedsProfitFeePeriod(msg.sender),
+                    "PerformanceFeeCalculator.adjustFee: ProfitFeePeriod must have elapsed to update fee"
+                );
+
+                feeState[msg.sender].lastProfitFeeTimestamp = block.timestamp;
+                feeState[msg.sender].highWatermark = rebalancingSetValue;
+            }
+
             feeState[msg.sender].profitFeePercentage = feePercentage;
         }
 
@@ -314,6 +336,9 @@ contract PerformanceFeeCalculator is IFeeCalculator {
         validateStreamingFeePercentage(parameters.streamingFeePercentage);
         validateProfitFeePercentage(parameters.profitFeePercentage);
 
+        // WARNING: This require has downstream effects on security assumptions for updating and accruing fees.
+        // Removing it allows highWatermarks to be reset, potentially cancelling fee collections or allowing traders
+        // to apply higher profitFee to Set gains.
         require(
             parameters.highWatermarkResetPeriod >= parameters.profitFeePeriod,
             "PerformanceFeeCalculator.validateFeeParameters: Fee collection frequency must exceed highWatermark reset."

package.json

@@ -1,6 +1,6 @@
 {
   "name": "set-protocol-contracts",
-  "version": "1.4.9-beta",
+  "version": "1.4.10-beta",
   "description": "Smart contracts for {Set} Protocol",
   "main": "dist/artifacts/index.js",
   "typings": "dist/typings/artifacts/index.d.ts",
@@ -20,7 +20,7 @@
     "clean": "rm -rf build; rm -rf transpiled; rm -rf types/generated",
     "clean-chain": "rm -rf blockchain && cp -r snapshots/0x-Kyber-Compound blockchain",
     "buidler-new": "yarn buidler clean && yarn buidler-test",
-    "buidler-test": "yarn buidler-setup && buidler test  transpiled/test/contracts/core/liquidators/twapLiquidator.spec.js",
+    "buidler-test": "yarn buidler-setup && buidler test  transpiled/test/contracts/core/[x].spec.js",
     "buidler-setup": "buidler compile && bash scripts/buidler_deploy_dev.sh && yarn transformJson && yarn generate-typings && yarn transpile",
     "compile": "./node_modules/.bin/truffle compile --all",
     "coverage": "yarn coverage-setup && ./node_modules/.bin/solidity-coverage && yarn coverage-cleanup",

test/contracts/core/fee-calculators/performanceFeeCalculator.spec.ts

@@ -464,6 +464,8 @@ contract('PerformanceFeeCalculator', accounts => {
     let feeType: BigNumber;
     let newFeePercentage: BigNumber;
 
+    let customInitialFeePercentage: BigNumber;
+
     before(async () => {
       addValidSet = true;
 
@@ -506,7 +508,7 @@ contract('PerformanceFeeCalculator', accounts => {
 
       const profitFeePeriod = ONE_DAY_IN_SECONDS.mul(30);
       const highWatermarkResetPeriod = ONE_DAY_IN_SECONDS.mul(365);
-      const profitFeePercentage = ether(.2);
+      const profitFeePercentage = customInitialFeePercentage || ether(.2);
       const streamingFeePercentage = ether(.02);
 
       const feeCalculatorData = feeCalculatorHelper.generatePerformanceFeeCallData(
@@ -566,6 +568,115 @@ contract('PerformanceFeeCalculator', accounts => {
         expect(feeState.profitFeePercentage).to.be.bignumber.equal(newFeePercentage);
       });
 
+      describe('when profit fee is initially 0', async () => {
+        let updatedBTCPrice: BigNumber;
+        let updatedETHPrice: BigNumber;
+
+        let timeElapsed: BigNumber;
+        let customTimeElapsed: BigNumber;
+
+        before(async () => {
+          customInitialFeePercentage = ether(0);
+        });
+
+        after(async () => {
+          customInitialFeePercentage = undefined;
+        });
+
+        beforeEach(async () => {
+          await usdWrappedBTCOracle.updatePrice.sendTransactionAsync(updatedBTCPrice);
+          await ethWrappedBTCOracle.updatePrice.sendTransactionAsync(
+            updatedBTCPrice.mul(ether(1)).div(updatedETHPrice).round(0, 3)
+          );
+
+          await usdWrappedETHOracle.updatePrice.sendTransactionAsync(updatedETHPrice);
+          await ethWrappedETHOracle.updatePrice.sendTransactionAsync(
+            updatedETHPrice.mul(ether(1)).div(updatedETHPrice).round(0, 3)
+          );
+
+          timeElapsed = customTimeElapsed || ONE_YEAR_IN_SECONDS;
+
+          await blockchain.increaseTimeAsync(timeElapsed);
+          await blockchain.mineBlockAsync();
+        });
+
+        describe('and there is a profit', async () => {
+          before(async () => {
+            updatedBTCPrice = ether(8000);
+            updatedETHPrice = ether(140);
+          });
+
+          after(async () => {
+            updatedBTCPrice = undefined;
+            updatedETHPrice = undefined;
+          });
+
+          it('properly sets the fee', async () => {
+            await subject();
+            const feeState: any = await feeCalculator.feeState.callAsync(rebalancingSetToken.address);
+            expect(feeState.profitFeePercentage).to.be.bignumber.equal(newFeePercentage);
+          });
+
+          it('properly resets the watermark', async () => {
+            await subject();
+
+            const rebalancingSetValue = await valuationHelper.calculateRebalancingSetTokenValueAsync(
+              rebalancingSetToken,
+              usdOracleWhiteList,
+            );
+
+            const postFeeState: any = await feeCalculator.feeState.callAsync(rebalancingSetToken.address);
+
+            expect(postFeeState.highWatermark).to.bignumber.equal(rebalancingSetValue);
+          });
+
+          it('sets the last profit fee timestamp correctly', async () => {
+            await subject();
+
+            const lastBlock = await web3.eth.getBlock('latest');
+
+            const feeState: any = await feeCalculator.feeState.callAsync(rebalancingSetToken.address);
+            expect(feeState.lastProfitFeeTimestamp).to.be.bignumber.equal(lastBlock.timestamp);
+          });
+
+          describe('when the profitFeePeriod has not elapsed', async () => {
+            before(async () => {
+              customTimeElapsed = ONE_DAY_IN_SECONDS;
+            });
+
+            after(async () => {
+              customTimeElapsed = undefined;
+            });
+
+            it('should revert', async () => {
+              await expectRevertError(subject());
+            });
+          });
+        });
+
+        describe('when there is no profit', async () => {
+          before(async () => {
+            updatedBTCPrice = ether(7000);
+            updatedETHPrice = ether(100);
+          });
+
+          after(async () => {
+            updatedBTCPrice = undefined;
+            updatedETHPrice = undefined;
+          });
+
+          it('does not reset the watermark', async () => {
+            const preFeeState: any = await feeCalculator.feeState.callAsync(rebalancingSetToken.address);
+
+            await subject();
+
+            const postFeeState: any = await feeCalculator.feeState.callAsync(rebalancingSetToken.address);
+
+            expect(postFeeState.highWatermark).to.bignumber.equal(preFeeState.highWatermark);
+          });
+        });
+      });
+
       describe('when the profit fee is greater than maximumProfitFeePercentage', async () => {
         before(async () => {
           newFeePercentage = ether(.6);

test/contracts/core/integration/performanceFeeIntegration.spec.ts

@@ -27,6 +27,7 @@ import { Blockchain } from '@utils/blockchain';
 import { getWeb3 } from '@utils/web3Helper';
 import { ether } from '@utils/units';
 import {
+  DEFAULT_GAS,
   ONE_DAY_IN_SECONDS,
   ONE_YEAR_IN_SECONDS,
   ZERO
@@ -47,7 +48,7 @@ const blockchain = new Blockchain(web3);
 const Core = artifacts.require('Core');
 const FixedFeeCalculator = artifacts.require('FixedFeeCalculator');
 
-contract('PerformanceFeeCalculator', accounts => {
+contract('PerformanceFeeCalculator Integration Test', accounts => {
   const [
     ownerAccount,
     feeAccount,
@@ -204,22 +205,30 @@ contract('PerformanceFeeCalculator', accounts => {
     await blockchain.revertAsync();
   });
 
-  describe('#updateAndGetFee: USD Denominated', async () => {
+  describe('#updateAndGetFee and adjustFee: USD Denominated', async () => {
     let updatedBTCPrice: BigNumber;
     let updatedETHPrice: BigNumber;
     let chainTimeIncrease: BigNumber;
 
+    let feeType: BigNumber;
+    let newFeePercentage: BigNumber;
+    let subjectNewFeeData: string;
+
+    let customProfitFeePercentage: BigNumber;
+    let customChainTimeIncrease: BigNumber;
+
     before(async () => {
-      chainTimeIncrease = ONE_YEAR_IN_SECONDS;
       updatedBTCPrice = ether(8000);
       updatedETHPrice = ether(140);
     });
 
     beforeEach(async () => {
+      chainTimeIncrease = customChainTimeIncrease || ONE_YEAR_IN_SECONDS;
+
       const calculatorData = feeCalculatorHelper.generatePerformanceFeeCallDataBuffer(
         ONE_DAY_IN_SECONDS.mul(30),
         ONE_YEAR_IN_SECONDS,
-        ether(.2),
+        customProfitFeePercentage || ether(.2),
         ether(.02)
       );
 
@@ -271,6 +280,18 @@ contract('PerformanceFeeCalculator', accounts => {
       return rebalancingSetToken.actualizeFee.sendTransactionAsync();
     }
 
+    async function adjustFeeSubject(): Promise<string> {
+      subjectNewFeeData = feeCalculatorHelper.generateAdjustFeeCallData(
+        feeType,
+        newFeePercentage
+      );
+
+      return rebalancingSetToken.adjustFee.sendTransactionAsync(
+        subjectNewFeeData,
+        { from: ownerAccount, gas: DEFAULT_GAS }
+      );
+    }
+
     it('mints the correct Rebalancing Set to the feeRecipient', async () => {
       const preFeeState: any = await usdFeeCalculator.feeState.callAsync(rebalancingSetToken.address);
       const previousSupply = await rebalancingSetToken.totalSupply.callAsync();
@@ -381,11 +402,11 @@ contract('PerformanceFeeCalculator', accounts => {
 
     describe('when time since last profit fee does not exceed fee frequency', async () => {
       before(async () => {
-        chainTimeIncrease = ONE_DAY_IN_SECONDS.mul(15);
+        customChainTimeIncrease = ONE_DAY_IN_SECONDS.mul(15);
       });
 
       after(async () => {
-        chainTimeIncrease = ONE_YEAR_IN_SECONDS;
+        customChainTimeIncrease = undefined;
       });
 
       it('mints the correct Rebalancing Set to the feeRecipient', async () => {
@@ -604,6 +625,45 @@ contract('PerformanceFeeCalculator', accounts => {
         expect(postFeeState.highWatermark).to.bignumber.equal(expectedHighWatermark);
       });
     });
+
+    describe('when the initial profit fee is 0, there is a profit, and the fees change', async () => {
+      before(async () => {
+        customProfitFeePercentage = ether(0);
+        customChainTimeIncrease = ONE_YEAR_IN_SECONDS.div(2);
+      });
+
+      after(async () => {
+        customProfitFeePercentage = undefined;
+        customChainTimeIncrease = undefined;
+      });
+
+      beforeEach(async () => {
+        feeType = new BigNumber(1);
+        newFeePercentage = ether(0.1);
+      });
+
+      it('resets the highWatermark to the current RebalancingSet value', async () => {
+        await adjustFeeSubject();
+
+        const rebalancingSetValue = await valuationHelper.calculateRebalancingSetTokenValueAsync(
+          rebalancingSetToken,
+          usdOracleWhiteList,
+        );
+
+        const postFeeState: any = await usdFeeCalculator.feeState.callAsync(rebalancingSetToken.address);
+
+        expect(postFeeState.highWatermark).to.bignumber.equal(rebalancingSetValue);
+      });
+
+      it('sets the correct fee', async () => {
+        await adjustFeeSubject();
+
+        const postFeeState: any = await usdFeeCalculator.feeState.callAsync(rebalancingSetToken.address);
+
+        expect(postFeeState.profitFeePercentage).to.bignumber.equal(newFeePercentage);
+      });
+
+    });
   });
 
   describe('#updateAndGetFee: ETH Denominated', async () => {