Merge pull request #582 from keep-network/constitutionally-limited

Constitutionally Limited: Impose certain additional limits on governance actions

Three limitations on governance actions:
- Prevents lot sizes less than 0.0005 BTC.
- Prevents lot sizes greater than 10 BTC.
- Prevents signer fees below 5bps.

solidity/contracts/system/TBTCSystem.sol

@@ -169,30 +169,49 @@ contract TBTCSystem is Ownable, ITBTCSystem, DepositLog {
     function beginSignerFeeDivisorUpdate(uint16 _signerFeeDivisor)
         external onlyOwner
     {
-        require(_signerFeeDivisor > 9, "Signer fee divisor must be greater than 9, for a signer fee that is <= 10%.");
+        require(
+            _signerFeeDivisor > 9,
+            "Signer fee divisor must be greater than 9, for a signer fee that is <= 10%."
+        );
+        require(
+            _signerFeeDivisor < 2000,
+            "Signer fee divisor must be less than 2000, for a signer fee that is > 0.05%."
+        );
+
         newSignerFeeDivisor = _signerFeeDivisor;
         signerFeeDivisorChangeInitiated = block.timestamp;
         emit SignerFeeDivisorUpdateStarted(_signerFeeDivisor, block.timestamp);
     }
 
     /// @notice Set the allowed deposit lot sizes.
-    /// @dev    Lot size array should always contain 10**8 satoshis (1BTC value)
+    /// @dev    Lot size array should always contain 10**8 satoshis (1 BTC) and
+    ///         cannot contain values less than 50000 satoshis (0.0005 BTC) or
+    ///         greater than 10**9 satoshis (10 BTC).
     ///         This can be finalized by calling `finalizeLotSizesUpdate`
-    ///         Anytime after `governanceTimeDelay` has elapsed.
+    ///         anytime after `governanceTimeDelay` has elapsed.
     /// @param _lotSizes Array of allowed lot sizes.
     function beginLotSizesUpdate(uint64[] calldata _lotSizes)
         external onlyOwner
     {
-        for( uint i = 0; i < _lotSizes.length; i++){
-            if (_lotSizes[i] == 10**8){
-                lotSizesSatoshis = _lotSizes;
-                emit LotSizesUpdateStarted(_lotSizes, block.timestamp);
-                newLotSizesSatoshis = _lotSizes;
-                lotSizesChangeInitiated = block.timestamp;
-                return;
+        bool hasSingleBitcoin = false;
+        for (uint i = 0; i < _lotSizes.length; i++) {
+            if (_lotSizes[i] == 10**8) {
+                hasSingleBitcoin = true;
+            } else if (_lotSizes[i] < 50 * 10**3) {
+                // Failed the minimum requirement, break on out.
+                revert("Lot sizes less than 0.0005 BTC are not allowed");
+            } else if (_lotSizes[i] > 10 * 10**8) {
+                // Failed the maximum requirement, break on out.
+                revert("Lot sizes greater than 10 BTC are not allowed");
             }
         }
-        revert("Lot size array must always contain 1BTC");
+
+        require(hasSingleBitcoin, "Lot size array must always contain 1 BTC");
+
+        lotSizesSatoshis = _lotSizes;
+        emit LotSizesUpdateStarted(_lotSizes, block.timestamp);
+        newLotSizesSatoshis = _lotSizes;
+        lotSizesChangeInitiated = block.timestamp;
     }
 
     /// @notice Set the system collateralization levels

solidity/test/TBTCSystemTest.js

@@ -278,12 +278,19 @@ describe("TBTCSystem", async function() {
         )
       })
 
-      it("reverts if fee divisor is smaller than 10", async () => {
+      it("reverts if fee divisor is smaller than or equal to 9", async () => {
         await expectRevert(
           tbtcSystem.beginSignerFeeDivisorUpdate(new BN("9")),
           "Signer fee divisor must be greater than 9, for a signer fee that is <= 10%.",
         )
       })
+
+      it("reverts if fee divisor is greater than or equal to 2000", async () => {
+        await expectRevert(
+          tbtcSystem.beginSignerFeeDivisorUpdate(new BN("2000")),
+          "Signer fee divisor must be less than 2000, for a signer fee that is > 0.05%.",
+        )
+      })
     })
 
     describe("finalizeSignerFeeDivisorUpdate", async () => {
@@ -319,7 +326,12 @@ describe("TBTCSystem", async function() {
   })
 
   describe("update lot sizes", async () => {
-    const lotSizes = [new BN(10 ** 8), new BN(10 ** 6)]
+    const lotSizes = [
+      new BN(10 ** 8), // required
+      new BN(10 ** 6),
+      new BN(10 ** 9), // upper bound
+      new BN(50 * 10 ** 3), // lower bound
+    ]
     describe("beginLotSizesUpdate", async () => {
       it("executes and emits a LotSizesUpdateStarted event", async () => {
         const testSizes = [new BN(10 ** 8), new BN(10 ** 6)]
@@ -358,15 +370,31 @@ describe("TBTCSystem", async function() {
         const lotSizes = []
         await expectRevert(
           tbtcSystem.beginLotSizesUpdate(lotSizes),
-          "Lot size array must always contain 1BTC",
+          "Lot size array must always contain 1 BTC",
         )
       })
 
-      it("reverts if lot size array does not contain a 1BTC lot size", async () => {
+      it("reverts if lot size array does not contain a 1 BTC lot size", async () => {
         const lotSizes = [10 ** 7]
         await expectRevert(
           tbtcSystem.beginLotSizesUpdate(lotSizes),
-          "Lot size array must always contain 1BTC",
+          "Lot size array must always contain 1 BTC",
+        )
+      })
+
+      it("reverts if lot size array contains a lot size < 0.0005 BTC", async () => {
+        const lotSizes = [10 ** 7, 10 ** 8, 5 * 10 ** 3 - 1]
+        await expectRevert(
+          tbtcSystem.beginLotSizesUpdate(lotSizes),
+          "Lot sizes less than 0.0005 BTC are not allowed",
+        )
+      })
+
+      it("reverts if lot size array contains a lot size > 10 BTC", async () => {
+        const lotSizes = [10 ** 7, 10 ** 9 + 1, 10 ** 8]
+        await expectRevert(
+          tbtcSystem.beginLotSizesUpdate(lotSizes),
+          "Lot sizes greater than 10 BTC are not allowed",
         )
       })
     })