update fillSlot and freeSlot state requirements

`fillSlot` and `freeSlot` must both be in a state of New or Started and not Cancelled (Finished to be added later).

Update tests to support the above.

contracts/Marketplace.sol

@@ -51,14 +51,9 @@ contract Marketplace is Collateral, Proofs {
     bytes32 requestId,
     uint256 slotIndex,
     bytes calldata proof
-  ) public marketplaceInvariant {
+  ) public requestMustAcceptProofs(requestId) marketplaceInvariant {
     Request storage request = _request(requestId);
     require(slotIndex < request.ask.slots, "Invalid slot");
-    RequestContext storage context = requestContexts[requestId];
-    require(!_isCancelled(requestId), "Request cancelled");
-    // TODO: in the case of repair, update below require condition by adding
-    // || context.state == RequestState.Started
-    require(context.state == RequestState.New, "Invalid state");
 
     bytes32 slotId = keccak256(abi.encode(requestId, slotIndex));
     Slot storage slot = slots[slotId];
@@ -72,6 +67,7 @@ contract Marketplace is Collateral, Proofs {
 
     slot.host = msg.sender;
     slot.requestId = requestId;
+    RequestContext storage context = requestContexts[requestId];
     context.slotsFilled += 1;
     emit SlotFilled(requestId, slotIndex, slotId);
     if (context.slotsFilled == request.ask.slots) {
@@ -83,11 +79,10 @@ contract Marketplace is Collateral, Proofs {
 
   function _freeSlot(
     bytes32 slotId
-  ) internal marketplaceInvariant {
+  ) internal slotMustAcceptProofs(slotId) marketplaceInvariant {
     Slot storage slot = _slot(slotId);
     bytes32 requestId = slot.requestId;
     RequestContext storage context = requestContexts[requestId];
-    require(context.state == RequestState.Started, "Invalid state");
 
     // TODO: burn host's slot collateral except for repair costs + mark proof
     // missing reward
@@ -101,9 +96,11 @@ contract Marketplace is Collateral, Proofs {
     context.slotsFilled -= 1;
     emit SlotFreed(requestId, slotId);
 
-    Request memory request = _request(requestId);
+    Request storage request = _request(requestId);
     uint256 slotsLost = request.ask.slots - context.slotsFilled;
-    if (slotsLost > request.ask.maxSlotLoss) {
+    if (slotsLost > request.ask.maxSlotLoss &&
+        context.state == RequestState.Started) {
+
       context.state = RequestState.Failed;
       emit RequestFailed(requestId);
 
@@ -158,12 +155,12 @@ contract Marketplace is Collateral, Proofs {
   /// @param requestId the id of the request
   /// @return true if request is cancelled
   function _isCancelled(bytes32 requestId) internal view returns (bool) {
-    RequestContext memory context = requestContexts[requestId];
+    RequestContext storage context = _context(requestId);
     return
       context.state == RequestState.Cancelled ||
       (
         context.state == RequestState.New &&
-        block.timestamp > requests[requestId].expiry
+        block.timestamp > _request(requestId).expiry
       );
   }
 
@@ -349,6 +346,14 @@ contract Marketplace is Collateral, Proofs {
     _;
   }
 
+  /// @notice Modifier that requires the request state to be that which is accepting proof submissions from hosts occupying slots.
+  /// @dev Request state must be new or started, and must not be cancelled, finished, or failed.
+  /// @param requestId id of the request, for which to obtain state info
+  modifier requestMustAcceptProofs(bytes32 requestId) {
+    require(_requestAcceptsProofs(requestId), "Request not accepting proofs");
+    _;
+  }
+
   struct MarketplaceFunds {
     uint256 balance;
     uint256 received;

contracts/Storage.sol

@@ -90,8 +90,7 @@ contract Storage is Collateral, Marketplace {
         _slash(host, slashPercentage);
 
       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
+        // When the collateral drops 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);

contracts/TestMarketplace.sol

@@ -25,7 +25,7 @@ contract TestMarketplace is Marketplace {
   function isSlotCancelled(bytes32 slotId) public view returns (bool) {
     return _isSlotCancelled(slotId);
   }
-  
+
   function freeSlot(bytes32 slotId) public {
     _freeSlot(slotId);
   }
@@ -33,4 +33,4 @@ contract TestMarketplace is Marketplace {
   function slot(bytes32 slotId) public view returns (Slot memory) {
     return _slot(slotId);
   }
-}
+}

test/Marketplace.test.js

@@ -103,7 +103,6 @@ describe("Marketplace", function () {
     })
   })
 
-
   describe("filling a slot", function () {
     beforeEach(async function () {
       switchAccount(client)
@@ -161,7 +160,7 @@ describe("Marketplace", function () {
       ).to.be.revertedWith("Unknown request")
     })
 
-    it("is rejected when request is expired/cancelled", async function () {
+    it("is rejected when request is cancelled", async function () {
       switchAccount(client)
       let expired = { ...request, expiry: now() - hours(1) }
       await token.approve(marketplace.address, price(request))
@@ -169,7 +168,7 @@ describe("Marketplace", function () {
       switchAccount(host)
       await expect(
         marketplace.fillSlot(requestId(expired), slot.index, proof)
-      ).to.be.revertedWith("Request cancelled")
+      ).to.be.revertedWith("Request not accepting proofs")
     })
 
     it("is rejected when slot index not in range", async function () {
@@ -186,7 +185,7 @@ describe("Marketplace", function () {
       }
       await expect(
         marketplace.fillSlot(slot.request, lastSlot, proof)
-      ).to.be.revertedWith("Invalid state")
+      ).to.be.revertedWith("Slot already filled")
     })
   })
 
@@ -212,11 +211,6 @@ describe("Marketplace", function () {
       )
     })
 
-    it("fails to free slot when not started", async function () {
-      await marketplace.fillSlot(slot.request, slot.index, proof)
-      await expect(marketplace.freeSlot(id)).to.be.revertedWith("Invalid state")
-    })
-
     it("successfully frees slot", async function () {
       await waitUntilAllSlotsFilled(
         marketplace,
@@ -445,6 +439,20 @@ describe("Marketplace", function () {
       )
     })
 
+    it("does not change state to Failed if too many slots freed but contract not started", async function () {
+      for (let i = 0; i <= request.ask.maxSlotLoss; i++) {
+        await marketplace.fillSlot(slot.request, i, proof)
+      }
+      for (let i = 0; i <= request.ask.maxSlotLoss; i++) {
+        slot.index = i
+        let id = slotId(slot)
+        await marketplace.freeSlot(id)
+      }
+      await expect(await marketplace.state(slot.request)).to.equal(
+        RequestState.New
+      )
+    })
+
     it("changes state to Cancelled once request is cancelled", async function () {
       await waitUntilExpired(request.expiry)
       await expect(await marketplace.state(slot.request)).to.equal(

test/marketplace.js

@@ -11,8 +11,7 @@ async function waitUntilExpired(expiry) {
 }
 
 async function waitUntilAllSlotsFilled(contract, numSlots, requestId, proof) {
-  const lastSlot = numSlots - 1
-  for (let i = 0; i <= lastSlot; i++) {
+  for (let i = 0; i < numSlots; i++) {
     await contract.fillSlot(requestId, i, proof)
   }
 }