slash first then check collateral threshold

Once a proof is marked as missing, if that missing proof is enough to slash a host, first slash the host, then check the hosts balance. If the balance has dropped below the minimum allowable collateral threshold, then remove them from the slot.

contracts/Collateral.sol

@@ -47,19 +47,11 @@ contract Collateral is AccountLocks {
     assert(token.transfer(msg.sender, amount));
   }
 
-  function _slashAmount(address account, uint256 percentage)
-    internal
-    view
-    returns (uint256)
-  {
-    return (balanceOf(account) * percentage) / 100;
-  }
-
   function _slash(address account, uint256 percentage)
     internal
     collateralInvariant
   {
-    uint256 amount = _slashAmount(account, percentage);
+    uint256 amount = (balanceOf(account) * percentage) / 100;
     funds.slashed += amount;
     subtract(account, amount);
   }

contracts/Storage.sol

@@ -87,18 +87,15 @@ contract Storage is Collateral, Marketplace {
     _markProofAsMissing(slotId, period);
     address host = _host(slotId);
     if (_missed(slotId) % slashMisses == 0) {
+        _slash(host, slashPercentage);
 
-      uint256 slashAmount = _slashAmount(host, slashPercentage);
-      if (balanceOf(host) - slashAmount < minCollateralThreshold) {
+      if (balanceOf(host) < minCollateralThreshold) {
         // If host has been slashed enough such that the next slashing would
         // cause the collateral to drop below the minimum threshold, the slot
         // needs to be freed so that there is enough remaining collateral to be
         // distributed for repairs and rewards (with any leftover to be burnt).
         _freeSlot(slotId);
       }
-      else {
-        _slash(host, slashPercentage);
-      }
     }
   }
 }

test/Storage.test.js

@@ -82,7 +82,7 @@ describe("Storage", function () {
     })
   })
 
-  describe("slashing when missing proofs", function () {
+  describe("missing proofs", function () {
     let period, periodOf, periodEnd
 
     beforeEach(async function () {
@@ -102,6 +102,7 @@ describe("Storage", function () {
       }
     }
 
+    describe("slashing when missing proofs", function () {
       it("reduces collateral when too many proofs are missing", async function () {
         const id = slotId(slot)
         await storage.fillSlot(slot.request, slot.index, proof)
@@ -111,11 +112,44 @@ describe("Storage", function () {
           await advanceTime(period)
           await storage.markProofAsMissing(id, missedPeriod)
         }
-      const expectedBalance = (collateralAmount * (100 - slashPercentage)) / 100
+        const expectedBalance =
+          (collateralAmount * (100 - slashPercentage)) / 100
         expect(await storage.balanceOf(host.address)).to.equal(expectedBalance)
       })
     })
 
+    describe("freeing a slot", function () {
+      it("frees slot when collateral slashed below minimum threshold", async function () {
+        const id = slotId(slot)
+
+        await waitUntilAllSlotsFilled(
+          storage,
+          request.ask.slots,
+          slot.request,
+          proof
+        )
+
+        // max slashes before dropping below collateral threshold
+        const maxSlashes = 10
+        for (let i = 0; i < maxSlashes; i++) {
+          for (let j = 0; j < slashMisses; j++) {
+            await waitUntilProofIsRequired(id)
+            let missedPeriod = periodOf(await currentTime())
+            await advanceTime(period)
+            if (i === maxSlashes - 1 && j === slashMisses - 1) {
+              await expect(
+                await storage.markProofAsMissing(id, missedPeriod)
+              ).to.emit(storage, "SlotFreed")
+              await expect(storage.getSlot(id)).to.be.revertedWith("Slot empty")
+            } else {
+              await storage.markProofAsMissing(id, missedPeriod)
+            }
+          }
+        }
+      })
+    })
+  })
+
   describe("contract state", function () {
     let period, periodOf, periodEnd
 
@@ -192,53 +226,7 @@ describe("Storage", function () {
     })
   })
   
-  describe("freeing a slot", function () {
-    beforeEach(async function () {
-      period = (await storage.proofPeriod()).toNumber()
-      ;({ periodOf, periodEnd } = periodic(period))
-    })
 
-    async function waitUntilProofIsRequired(id) {
-      await advanceTimeTo(periodEnd(periodOf(await currentTime())))
-      while (
-        !(
-          (await storage.isProofRequired(id)) &&
-          (await storage.getPointer(id)) < 250
-        )
-      ) {
-        await advanceTime(period)
-      }
-    }
-
-    it("frees slot when collateral slashed below minimum threshold", async function () {
-      const id = slotId(slot)
-
-      await waitUntilAllSlotsFilled(
-        storage,
-        request.ask.slots,
-        slot.request,
-        proof
-      )
-
-      // max slashes before dropping below collateral threshold
-      const maxSlashes = 10
-      for (let i = 0; i < maxSlashes; i++) {
-        for (let j = 0; j < slashMisses; j++) {
-          await waitUntilProofIsRequired(id)
-          let missedPeriod = periodOf(await currentTime())
-          await advanceTime(period)
-          if (i === maxSlashes - 1 && j === slashMisses - 1) {
-            await expect(
-              await storage.markProofAsMissing(id, missedPeriod)
-            ).to.emit(storage, "SlotFreed")
-            await expect(storage.getSlot(id)).to.be.revertedWith("Slot empty")
-          } else {
-            await storage.markProofAsMissing(id, missedPeriod)
-          }
-        }
-      }
-    })
-  })
 })
 
 // TODO: implement checking of actual proofs of storage, instead of dummy bool