improve(perpetual): Modify ConfigStore constraints and params

packages/core/contracts/financial-templates/common/FundingRateApplier.sol

@@ -149,15 +149,13 @@ abstract contract FundingRateApplier is FeePayer {
         returns (FixedPoint.Unsigned memory totalBond)
     {
         require(fundingRate.proposalTime == 0, "Proposal in progress");
+        _validateFundingRate(rate);
 
-        // Timestamp must be after the last funding rate update time, within the last 30 minutes, and it cannot be more
-        // than 90 seconds ahead of the block timestamp.
+        // Timestamp must be after the last funding rate update time, within the last 30 minutes.
         uint256 currentTime = getCurrentTime();
         uint256 updateTime = fundingRate.updateTime;
         require(
-            timestamp > updateTime &&
-                timestamp >= currentTime.sub(_getConfig().proposalTimePastLimit) &&
-                timestamp <= currentTime.add(_getConfig().proposalTimeFutureLimit),
+            timestamp > updateTime && timestamp >= currentTime.sub(_getConfig().proposalTimePastLimit),
             "Invalid proposal time"
         );
 
@@ -169,6 +167,7 @@ abstract contract FundingRateApplier is FeePayer {
         // Set up optimistic oracle.
         bytes32 identifier = fundingRate.identifier;
         bytes memory ancillaryData = _getAncillaryData();
+        // Note: requestPrice will revert if `timestamp` is less than the current block timestamp.
         optimisticOracle.requestPrice(identifier, timestamp, ancillaryData, collateralCurrency, 0);
         totalBond = FixedPoint.Unsigned(
             optimisticOracle.setBond(
@@ -261,6 +260,22 @@ abstract contract FundingRateApplier is FeePayer {
         return fundingRate.rate;
     }
 
+    // Constraining the range of funding rates limits the PfC for any dishonest proposer and enhances the
+    // perpetual's security. For example, let's examine the case where the max and min funding rates
+    // are equivalent to +/- 500%/year. This 1000% funding rate range allows a 8.6% profit from corruption for a
+    // proposer who can deter honest proposers for 74 hours:
+    // 1000%/year / 360 days / 24 hours * 74 hours max attack time = ~ 8.6%.
+    // How would attack work? Imagine that the market is very volatile currently and that the "true" funding
+    // rate for the next 74 hours is -500%, but a dishonest proposer successfully proposes a rate of +500%
+    // (after a two hour liveness) and disputes honest proposers for the next 72 hours. This results in a funding
+    // rate error of 1000% for 74 hours, until the DVM can set the funding rate back to its correct value.
+    function _validateFundingRate(FixedPoint.Signed memory rate) internal view {
+        require(
+            rate.isLessThanOrEqual(_getConfig().maxFundingRate) &&
+                rate.isGreaterThanOrEqual(_getConfig().minFundingRate)
+        );
+    }
+
     // Fetches a funding rate from the Store, determines the period over which to compute an effective fee,
     // and multiplies the current multiplier by the effective fee.
     // A funding rate < 1 will reduce the multiplier, and a funding rate of > 1 will increase the multiplier.

packages/core/contracts/financial-templates/perpetual-multiparty/ConfigStore.sol

@@ -40,15 +40,17 @@ contract ConfigStore is ConfigStoreInterface, Testable, Lockable, Ownable {
         uint256 rewardRate,
         uint256 proposerBond,
         uint256 timelockLiveness,
-        uint256 proposalTimeFutureLimit,
+        int256 maxFundingRate,
+        int256 minFundingRate,
         uint256 proposalTimePastLimit,
         uint256 proposalPassedTimestamp
     );
     event ChangedConfigSettings(
         uint256 rewardRate,
         uint256 proposerBond,
         uint256 timelockLiveness,
-        uint256 proposalTimeFutureLimit,
+        int256 maxFundingRate,
+        int256 minFundingRate,
         uint256 proposalTimePastLimit
     );
 
@@ -112,7 +114,8 @@ contract ConfigStore is ConfigStoreInterface, Testable, Lockable, Ownable {
             newConfig.rewardRatePerSecond.rawValue,
             newConfig.proposerBondPct.rawValue,
             newConfig.timelockLiveness,
-            newConfig.proposalTimeFutureLimit,
+            newConfig.maxFundingRate.rawValue,
+            newConfig.minFundingRate.rawValue,
             newConfig.proposalTimePastLimit,
             pendingPassedTimestamp
         );
@@ -136,7 +139,8 @@ contract ConfigStore is ConfigStoreInterface, Testable, Lockable, Ownable {
                 currentConfig.rewardRatePerSecond.rawValue,
                 currentConfig.proposerBondPct.rawValue,
                 currentConfig.timelockLiveness,
-                currentConfig.proposalTimeFutureLimit,
+                currentConfig.maxFundingRate.rawValue,
+                currentConfig.minFundingRate.rawValue,
                 currentConfig.proposalTimePastLimit
             );
         }
@@ -153,24 +157,33 @@ contract ConfigStore is ConfigStoreInterface, Testable, Lockable, Ownable {
 
     // Use this method to constrain values with which you can set ConfigSettings.
     function _validateConfig(ConfigStoreInterface.ConfigSettings memory config) internal pure {
-        // Its hard to ascertain what are reasonable limits, into the future and the past, for proposal timestamps.
-        // So, we do not set any limits.
+        // We don't set limits on proposal timestamps because there are already natural limits:
+        // - Future: price requests to the OptimisticOracle must be in the past---we can't add further constraints.
+        // - Past: proposal times must always be after the last update time, and  a reasonable past limit would be 30
+        //   mins, meaning that no proposal timestamp can be more than 30 minutes behind the current time.
 
-        // Make sure timelockLiveness is not too long, otherwise contract can might not be able to fix itself
+        // Make sure timelockLiveness is not too long, otherwise contract might not be able to fix itself
         // before a vulnerability drains its collateral.
         require(config.timelockLiveness <= 7 days && config.timelockLiveness >= 1 days, "Invalid timelockLiveness");
 
-        // Upper limits for the reward and bond rates are estimated based on offline discussions,
-        // and it is expected that these hard-coded limits can change in future deployments.
-        // For a discussion thread, go [here](https://github.com/UMAprotocol/protocol/pull/2223#discussion_r530692149).
-
-        // Proposer bond of 0.04% is based on a maximum expected funding rate error of 200%/year.
-        FixedPoint.Unsigned memory maxProposerBond = FixedPoint.fromUnscaledUint(4).div(1e4);
-        require(config.proposerBondPct.isLessThan(maxProposerBond), "Invalid proposerBondPct");
-
-        // Reward rate should be less than 100% a year => 100% / 360 days / 24 hours / 60 mins / 60 secs
+        // The reward rate should be modified as needed to incentivize honest proposers appropriately.
+        // Additionally, the rate should be less than 100% a year => 100% / 360 days / 24 hours / 60 mins / 60 secs
         // = 0.0000033
         FixedPoint.Unsigned memory maxRewardRatePerSecond = FixedPoint.fromUnscaledUint(33).div(1e7);
         require(config.rewardRatePerSecond.isLessThan(maxRewardRatePerSecond), "Invalid rewardRatePerSecond");
+
+        // We don't set a limit on the proposer bond because it is a defense against dishonest proposers. If a proposer
+        // were to successfully propose a very high or low funding rate, then their PfC would be very high. The proposer
+        // could theoretically keep their "evil" funding rate alive indefinitely by continuously disputing honest
+        // proposers, so we would want to be able to set the proposal bond (equal to the dispute bond) higher than their
+        // PfC for each proposal liveness window. The downside of not limiting this is that the config store owner
+        // can set it arbitrarily high and preclude a new funding rate from ever coming in. We suggest setting the proposal
+        // bond based on the configuration's funding rate range like in this discussion:
+        // https://github.com/UMAprotocol/protocol/issues/2039#issuecomment-719734383
+
+        // We also don't set a limit on the funding rate max/min because we might need to allow very high magnitude
+        // funding rates in extraordinarily volatile market situations. Note, that even though we do not bound
+        // the max/min, we still recommend that the deployer of this contract set the funding rate max/min values
+        // to bound the PfC of a dishonest proposer. A reasonable range might be the equivalent of [+200%/year, -200%/year].
     }
 }

packages/core/contracts/financial-templates/perpetual-multiparty/ConfigStoreInterface.sol

@@ -13,8 +13,10 @@ interface ConfigStoreInterface {
         FixedPoint.Unsigned rewardRatePerSecond;
         // Bond % (of given contract's PfC) that must be staked by proposers. Percentage of 1, e.g. 0.0005 is 0.05%.
         FixedPoint.Unsigned proposerBondPct;
-        // Funding rate proposal timestamp cannot be more than this amount of seconds in the future .
-        uint256 proposalTimeFutureLimit;
+        // Maximum funding rate % per second that can be proposed.
+        FixedPoint.Signed maxFundingRate;
+        // Minimum funding rate % per second that can be proposed.
+        FixedPoint.Signed minFundingRate;
         // Funding rate proposal timestamp cannot be more than this amount of seconds in the past from the latest
         // update time.
         uint256 proposalTimePastLimit;

packages/core/contracts/financial-templates/perpetual-multiparty/PerpetualLiquidatable.sol

@@ -241,7 +241,7 @@ contract PerpetualLiquidatable is PerpetualPositionManager {
         PositionData storage positionToLiquidate = _getPositionData(sponsor);
 
         tokensLiquidated = FixedPoint.min(maxTokensToLiquidate, positionToLiquidate.tokensOutstanding);
-        require(tokensLiquidated.isGreaterThan(0), "Liquidating 0 tokens");
+        require(tokensLiquidated.isGreaterThan(0));
 
         // Starting values for the Position being liquidated. If withdrawal request amount is > position's collateral,
         // then set this to 0, otherwise set it to (startCollateral - withdrawal request amount).

packages/core/test/financial-templates/common/FundingRateApplier.js

@@ -37,6 +37,8 @@ contract("FundingRateApplier", function(accounts) {
   const identifier = utf8ToHex("Test Identifier");
   const initialUserBalance = toWei("100");
   const defaultProposal = toWei("0.0000001"); // 1 percent every 100_000 seconds.
+  const maxFundingRate = toWei("0.00001");
+  const minFundingRate = toWei("-0.00001");
   const tokenScaling = toWei("1");
   const proposalTimePastLimit = 1800; // 30 mins.
   const delay = 10000; // 10_000 seconds.
@@ -100,7 +102,8 @@ contract("FundingRateApplier", function(accounts) {
         timelockLiveness: 86400, // 1 day
         rewardRatePerSecond: { rawValue: rewardRate },
         proposerBondPct: { rawValue: bondPercentage },
-        proposalTimeFutureLimit: 90, // Note: this doesn't affect the contract due to the stricter limits imposed by the optimistic oracle.
+        maxFundingRate: { rawValue: maxFundingRate },
+        minFundingRate: { rawValue: minFundingRate },
         proposalTimePastLimit: proposalTimePastLimit // 30 mins
       },
       timer.address
@@ -200,8 +203,15 @@ contract("FundingRateApplier", function(accounts) {
     assert.equal((await fundingRateApplier.fundingRate()).rate.rawValue.toString(), "0");
   });
 
+  it("Funding rate proposal must be within limits", async function() {
+    // Max/min funding rate per second is [< +1e-5, > -1e-5].
+    const currentTime = (await fundingRateApplier.getCurrentTime()).toNumber();
+    assert(await didContractThrow(fundingRateApplier.proposeNewRate({ rawValue: toWei("0.00002") }, currentTime)));
+    assert(await didContractThrow(fundingRateApplier.proposeNewRate({ rawValue: toWei("-0.00002") }, currentTime)));
+  });
+
   it("Proposal time checks", async () => {
-    const newRate = { rawValue: toWei("-0.0001") };
+    const newRate = { rawValue: toWei("-0.000001") };
 
     // Cannot be at or before the last update time.
     assert(await didContractThrow(fundingRateApplier.proposeNewRate(newRate, startTime)));

packages/core/test/financial-templates/perpetual-multiparty/ConfigStore.js

@@ -29,14 +29,16 @@ contract("ConfigStore", function(accounts) {
     timelockLiveness: 86401, // 1 day + 1 second
     rewardRatePerSecond: { rawValue: toWei("0.000001") },
     proposerBondPct: { rawValue: toWei("0.0001") },
-    proposalTimeFutureLimit: 90,
+    maxFundingRate: { rawValue: toWei("0.00001") },
+    minFundingRate: { rawValue: toWei("-0.00001") },
     proposalTimePastLimit: 1800 // 30 mins
   };
   let defaultConfig = {
     timelockLiveness: 86400, // 1 day
     rewardRatePerSecond: { rawValue: "0" },
     proposerBondPct: { rawValue: "0" },
-    proposalTimeFutureLimit: 0,
+    maxFundingRate: { rawValue: toWei("0") },
+    minFundingRate: { rawValue: toWei("0") },
     proposalTimePastLimit: 0
   };
 
@@ -48,7 +50,8 @@ contract("ConfigStore", function(accounts) {
       _inputConfig.rewardRatePerSecond.rawValue.toString()
     );
     assert.equal(currentConfig.proposerBondPct.rawValue.toString(), _inputConfig.proposerBondPct.rawValue.toString());
-    assert.equal(currentConfig.proposalTimeFutureLimit.toString(), _inputConfig.proposalTimeFutureLimit.toString());
+    assert.equal(currentConfig.maxFundingRate.rawValue.toString(), _inputConfig.maxFundingRate.rawValue.toString());
+    assert.equal(currentConfig.minFundingRate.rawValue.toString(), _inputConfig.minFundingRate.rawValue.toString());
     assert.equal(currentConfig.proposalTimePastLimit.toString(), _inputConfig.proposalTimePastLimit.toString());
   }
 
@@ -60,7 +63,8 @@ contract("ConfigStore", function(accounts) {
       _inputConfig.rewardRatePerSecond.rawValue.toString()
     );
     assert.equal(pendingConfig.proposerBondPct.rawValue.toString(), _inputConfig.proposerBondPct.rawValue.toString());
-    assert.equal(pendingConfig.proposalTimeFutureLimit.toString(), _inputConfig.proposalTimeFutureLimit.toString());
+    assert.equal(pendingConfig.maxFundingRate.rawValue.toString(), _inputConfig.maxFundingRate.rawValue.toString());
+    assert.equal(pendingConfig.minFundingRate.rawValue.toString(), _inputConfig.minFundingRate.rawValue.toString());
     assert.equal(pendingConfig.proposalTimePastLimit.toString(), _inputConfig.proposalTimePastLimit.toString());
   }
 
@@ -79,7 +83,8 @@ contract("ConfigStore", function(accounts) {
       assert.equal(config.timelockLiveness.toString(), testConfig.timelockLiveness.toString());
       assert.equal(config.rewardRatePerSecond.rawValue.toString(), testConfig.rewardRatePerSecond.rawValue);
       assert.equal(config.proposerBondPct.rawValue.toString(), testConfig.proposerBondPct.rawValue);
-      assert.equal(config.proposalTimeFutureLimit.toString(), testConfig.proposalTimeFutureLimit.toString());
+      assert.equal(config.maxFundingRate.rawValue.toString(), testConfig.maxFundingRate.rawValue.toString());
+      assert.equal(config.minFundingRate.rawValue.toString(), testConfig.minFundingRate.rawValue.toString());
       assert.equal(config.proposalTimePastLimit.toString(), testConfig.proposalTimePastLimit.toString());
       await storeHasNoPendingConfig(configStore);
     });
@@ -97,13 +102,6 @@ contract("ConfigStore", function(accounts) {
         rewardRatePerSecond: { rawValue: toWei("0.00000331") }
       };
       assert(await didContractThrow(ConfigStore.new(invalidConfig, timer.address)));
-
-      // Invalid proposal bond
-      invalidConfig = {
-        ...testConfig,
-        proposerBondPct: { rawValue: toWei("0.00041") }
-      };
-      assert(await didContractThrow(ConfigStore.new(invalidConfig, timer.address)));
     });
   });
   describe("Proposing a new configuration", function() {
@@ -124,7 +122,8 @@ contract("ConfigStore", function(accounts) {
           ev.proposerBond.toString() === testConfig.proposerBondPct.rawValue &&
           ev.timelockLiveness.toString() === testConfig.timelockLiveness.toString() &&
           ev.proposalPassedTimestamp.toString() === proposeTime.add(toBN(defaultConfig.timelockLiveness)).toString() &&
-          ev.proposalTimeFutureLimit.toString() === testConfig.proposalTimeFutureLimit.toString() &&
+          ev.maxFundingRate.toString() === testConfig.maxFundingRate.rawValue &&
+          ev.minFundingRate.toString() === testConfig.minFundingRate.rawValue &&
           ev.proposalTimePastLimit.toString() === testConfig.proposalTimePastLimit.toString()
         );
       });
@@ -138,7 +137,8 @@ contract("ConfigStore", function(accounts) {
           ev.rewardRate.toString() === testConfig.rewardRatePerSecond.rawValue &&
           ev.proposerBond.toString() === testConfig.proposerBondPct.rawValue &&
           ev.timelockLiveness.toString() === testConfig.timelockLiveness.toString() &&
-          ev.proposalTimeFutureLimit.toString() === testConfig.proposalTimeFutureLimit.toString() &&
+          ev.maxFundingRate.toString() === testConfig.maxFundingRate.rawValue &&
+          ev.minFundingRate.toString() === testConfig.minFundingRate.rawValue &&
           ev.proposalTimePastLimit.toString() === testConfig.proposalTimePastLimit.toString()
         );
       });
@@ -159,7 +159,8 @@ contract("ConfigStore", function(accounts) {
           ev.proposerBond.toString() === testConfig.proposerBondPct.rawValue &&
           ev.timelockLiveness.toString() === testConfig.timelockLiveness.toString() &&
           ev.proposalPassedTimestamp.toString() === proposeTime.add(toBN(defaultConfig.timelockLiveness)).toString() &&
-          ev.proposalTimeFutureLimit.toString() === testConfig.proposalTimeFutureLimit.toString() &&
+          ev.maxFundingRate.toString() === testConfig.maxFundingRate.rawValue &&
+          ev.minFundingRate.toString() === testConfig.minFundingRate.rawValue &&
           ev.proposalTimePastLimit.toString() === testConfig.proposalTimePastLimit.toString()
         );
       });
@@ -199,7 +200,8 @@ contract("ConfigStore", function(accounts) {
           ev.timelockLiveness.toString() === test2Config.timelockLiveness.toString() &&
           ev.proposalPassedTimestamp.toString() ===
             overwriteProposalTime.add(toBN(defaultConfig.timelockLiveness)).toString() &&
-          ev.proposalTimeFutureLimit.toString() === test2Config.proposalTimeFutureLimit.toString() &&
+          ev.maxFundingRate.toString() === test2Config.maxFundingRate.rawValue &&
+          ev.minFundingRate.toString() === test2Config.minFundingRate.rawValue &&
           ev.proposalTimePastLimit.toString() === test2Config.proposalTimePastLimit.toString()
         );
       });
@@ -231,7 +233,8 @@ contract("ConfigStore", function(accounts) {
           ev.rewardRate.toString() === test2Config.rewardRatePerSecond.rawValue &&
           ev.proposerBond.toString() === test2Config.proposerBondPct.rawValue &&
           ev.timelockLiveness.toString() === test2Config.timelockLiveness.toString() &&
-          ev.proposalTimeFutureLimit.toString() === test2Config.proposalTimeFutureLimit.toString() &&
+          ev.maxFundingRate.toString() === test2Config.maxFundingRate.rawValue &&
+          ev.minFundingRate.toString() === test2Config.minFundingRate.rawValue &&
           ev.proposalTimePastLimit.toString() === test2Config.proposalTimePastLimit.toString()
         );
       });

packages/core/test/financial-templates/perpetual-multiparty/Perpetual.js

@@ -26,7 +26,8 @@ contract("Perpetual", function(accounts) {
         timelockLiveness: 86400, // 1 day
         rewardRatePerSecond: { rawValue: "0" },
         proposerBondPct: { rawValue: "0" },
-        proposalTimeFutureLimit: 0,
+        maxFundingRate: { rawValue: "0" },
+        minFundingRate: { rawValue: "0" },
         proposalTimePastLimit: 0
       },
       timer.address