[marketplace] update state getter

Update `Marketplace.state` getter to to take `isCancelled` into account. This state can then be used internally and externally.

Add checks to `proofEnd`, `isProofRequired`, `willProofBeRequired`, and `getChallenge` that understands if the request is in a state to accept proofs. If not, return other values.

Add `slotMustAcceptProofs` modifier, originally introduced in a later PR, which requires the request to be in state of the request accepting proofs.

Add tests for all the above.

contracts/Marketplace.sol

@@ -169,6 +169,10 @@ contract Marketplace is Collateral, Proofs {
     return slot;
   }
 
+  function _context(bytes32 requestId) internal view returns (RequestContext storage) {
+    return requestContexts[requestId];
+  }
+
   function proofPeriod() public view returns (uint256) {
     return _period();
   }
@@ -178,6 +182,9 @@ contract Marketplace is Collateral, Proofs {
   }
 
   function proofEnd(bytes32 slotId) public view returns (uint256) {
+    if (!_slotAcceptsProofs(slotId)) {
+      return block.timestamp - 1;
+    }
     return _end(slotId);
   }
 
@@ -202,7 +209,30 @@ contract Marketplace is Collateral, Proofs {
   }
 
   function state(bytes32 requestId) public view returns (RequestState) {
-    return requestContexts[requestId].state;
+    // TODO: add check for _isFinished
+    if (_isCancelled(requestId)) {
+      return RequestState.Cancelled;
+    } else {
+      RequestContext storage context = _context(requestId);
+      return context.state;
+    }
+  }
+
+
+  /// @notice returns true when the request 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
+  function _requestAcceptsProofs(bytes32 requestId) internal view returns (bool) {
+    RequestState s = state(requestId);
+    return s == RequestState.New || s == RequestState.Started;
+  }
+
+  /// @notice returns true when the request 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 slotId id of the slot, that is mapped to a request, for which to obtain state info
+  function _slotAcceptsProofs(bytes32 slotId) internal view returns (bool) {
+    bytes32 requestId = _getRequestIdForSlot(slotId);
+    return _requestAcceptsProofs(requestId);
   }
 
   struct Request {
@@ -273,6 +303,15 @@ contract Marketplace is Collateral, Proofs {
     assert(funds.received == funds.balance + funds.sent);
   }
 
+  /// @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 slotId id of the slot, that is mapped to a request, for which to obtain state info
+  modifier slotMustAcceptProofs(bytes32 slotId) {
+    bytes32 requestId = _getRequestIdForSlot(slotId);
+    require(_requestAcceptsProofs(requestId), "Slot not accepting proofs");
+    _;
+  }
+
   struct MarketplaceFunds {
     uint256 balance;
     uint256 received;

contracts/Storage.sol

@@ -40,14 +40,6 @@ contract Storage is Collateral, Marketplace {
     return _slot(slotId);
   }
 
-  function isCancelled(bytes32 requestId) public view returns(bool) {
-    return _isCancelled(requestId);
-  }
-
-  function isSlotCancelled(bytes32 slotId) public view returns (bool) {
-    return _isSlotCancelled(slotId);
-  }
-
   function getHost(bytes32 requestId) public view returns (address) {
     return _host(requestId);
   }
@@ -57,14 +49,23 @@ contract Storage is Collateral, Marketplace {
   }
 
   function isProofRequired(bytes32 slotId) public view returns (bool) {
+    if(!_slotAcceptsProofs(slotId)) {
+      return false;
+    }
     return _isProofRequired(slotId);
   }
 
   function willProofBeRequired(bytes32 slotId) public view returns (bool) {
+    if(!_slotAcceptsProofs(slotId)) {
+      return false;
+    }
     return _willProofBeRequired(slotId);
   }
 
   function getChallenge(bytes32 slotId) public view returns (bytes32) {
+    if(!_slotAcceptsProofs(slotId)) {
+      return bytes32(0);
+    }
     return _getChallenge(slotId);
   }
 
@@ -76,8 +77,10 @@ contract Storage is Collateral, Marketplace {
     _submitProof(slotId, proof);
   }
 
-  function markProofAsMissing(bytes32 slotId, uint256 period) public {
+  function markProofAsMissing(bytes32 slotId, uint256 period)
-    require(!isSlotCancelled(slotId), "Request was cancelled");
+    public
+    slotMustAcceptProofs(slotId)
+  {
     _markProofAsMissing(slotId, period);
     if (_missed(slotId) % slashMisses == 0) {
       _slash(_host(slotId), slashPercentage);

contracts/TestMarketplace.sol

@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.0;
+
+import "./Marketplace.sol";
+
+// exposes internal functions of Marketplace for testing
+contract TestMarketplace is Marketplace {
+  constructor(
+    IERC20 _token,
+    uint256 _collateral,
+    uint256 _proofPeriod,
+    uint256 _proofTimeout,
+    uint8 _proofDowntime
+  )
+    Marketplace(_token, _collateral, _proofPeriod,_proofTimeout,_proofDowntime)
+  // solhint-disable-next-line no-empty-blocks
+  {
+
+  }
+
+  function isCancelled(bytes32 requestId) public view returns (bool) {
+    return _isCancelled(requestId);
+  }
+
+  function isSlotCancelled(bytes32 slotId) public view returns (bool) {
+    return _isSlotCancelled(slotId);
+  }
+}

test/Marketplace.test.js

@@ -11,6 +11,7 @@ const {
   revert,
   ensureMinimumBlockHeight,
   advanceTimeTo,
+  currentTime,
 } = require("./evm")
 
 describe("Marketplace", function () {
@@ -38,7 +39,7 @@ describe("Marketplace", function () {
       await token.mint(account.address, 1_000_000_000)
     }
 
-    const Marketplace = await ethers.getContractFactory("Marketplace")
+    const Marketplace = await ethers.getContractFactory("TestMarketplace")
     marketplace = await Marketplace.deploy(
       token.address,
       collateral,
@@ -338,7 +339,7 @@ describe("Marketplace", function () {
       await marketplace.deposit(collateral)
     })
 
-    it("state is Cancelled when client withdraws funds", async function () {
+    it("changes state to Cancelled when client withdraws funds", async function () {
       await expect(await marketplace.state(slot.request)).to.equal(
         RequestState.New
       )
@@ -350,7 +351,7 @@ describe("Marketplace", function () {
       )
     })
 
-    it("state is Started once all slots are filled", async function () {
+    it("changes state to Started once all slots are filled", async function () {
       await expect(await marketplace.state(slot.request)).to.equal(
         RequestState.New
       )
@@ -363,5 +364,42 @@ describe("Marketplace", function () {
         RequestState.Started
       )
     })
+
+    it("changes state to Cancelled once request is cancelled", async function () {
+      await waitUntilExpired(request.expiry)
+      await expect(await marketplace.state(slot.request)).to.equal(
+        RequestState.Cancelled
+      )
+    })
+
+    it("changes isCancelled to true once request is cancelled", async function () {
+      await expect(await marketplace.isCancelled(slot.request)).to.be.false
+      await waitUntilExpired(request.expiry)
+      await expect(await marketplace.isCancelled(slot.request)).to.be.true
+    })
+
+    it("rejects isSlotCancelled when slot is empty", async function () {
+      await expect(
+        marketplace.isSlotCancelled(slotId(slot))
+      ).to.be.revertedWith("Slot empty")
+    })
+
+    it("changes isSlotCancelled to true once request is cancelled", async function () {
+      await marketplace.fillSlot(slot.request, slot.index, proof)
+      await expect(await marketplace.isSlotCancelled(slotId(slot))).to.be.false
+      await waitUntilExpired(request.expiry)
+      await expect(await marketplace.isSlotCancelled(slotId(slot))).to.be.true
+    })
+
+    it("changes proofEnd to the past when request is cancelled", async function () {
+      await marketplace.fillSlot(slot.request, slot.index, proof)
+      await expect(await marketplace.proofEnd(slotId(slot))).to.be.gt(
+        await currentTime()
+      )
+      await waitUntilExpired(request.expiry)
+      await expect(await marketplace.proofEnd(slotId(slot))).to.be.lt(
+        await currentTime()
+      )
+    })
   })
 })

test/Storage.test.js

@@ -1,9 +1,10 @@
 const { expect } = require("chai")
 const { ethers, deployments } = require("hardhat")
+const { BigNumber } = ethers
 const { hexlify, randomBytes } = ethers.utils
 const { AddressZero } = ethers.constants
 const { exampleRequest } = require("./examples")
-const { advanceTime, advanceTimeTo, currentTime } = require("./evm")
+const { advanceTime, advanceTimeTo, currentTime, mine } = require("./evm")
 const { requestId, slotId } = require("./ids")
 const { periodic } = require("./time")
 const { price } = require("./price")
@@ -112,6 +113,33 @@ describe("Storage", function () {
       const expectedBalance = (collateralAmount * (100 - slashPercentage)) / 100
       expect(await storage.balanceOf(host.address)).to.equal(expectedBalance)
     })
+  })
+
+  describe("contract state", function () {
+    let period, periodOf, periodEnd
+
+    beforeEach(async function () {
+      period = (await storage.proofPeriod()).toNumber()
+      ;({ periodOf, periodEnd } = periodic(period))
+    })
+
+    async function waitUntilProofWillBeRequired(id) {
+      while (!(await storage.willProofBeRequired(id))) {
+        await mine()
+      }
+    }
+
+    async function waitUntilProofIsRequired(id) {
+      await advanceTimeTo(periodEnd(periodOf(await currentTime())))
+      while (
+        !(
+          (await storage.isProofRequired(id)) &&
+          (await storage.getPointer(id)) < 250
+        )
+      ) {
+        await advanceTime(period)
+      }
+    }
 
     it("fails to mark proof as missing when cancelled", async function () {
       await storage.fillSlot(slot.request, slot.index, proof)
@@ -119,26 +147,47 @@ describe("Storage", function () {
       let missedPeriod = periodOf(await currentTime())
       await expect(
         storage.markProofAsMissing(slotId(slot), missedPeriod)
-      ).to.be.revertedWith("Request was cancelled")
+      ).to.be.revertedWith("Slot not accepting proofs")
-    })
-  })
-  describe("contract state", function () {
-    it("isCancelled is true once request is cancelled", async function () {
-      await expect(await storage.isCancelled(slot.request)).to.equal(false)
-      await waitUntilExpired(request.expiry)
-      await expect(await storage.isCancelled(slot.request)).to.equal(true)
     })
 
-    it("isSlotCancelled fails when slot is empty", async function () {
+    it("will not require proofs once cancelled", async function () {
-      await expect(storage.isSlotCancelled(slotId(slot))).to.be.revertedWith(
+      const id = slotId(slot)
-        "Slot empty"
-      )
-    })
-
-    it("isSlotCancelled is true once request is cancelled", async function () {
       await storage.fillSlot(slot.request, slot.index, proof)
-      await waitUntilExpired(request.expiry)
+      await waitUntilProofWillBeRequired(id)
-      await expect(await storage.isSlotCancelled(slotId(slot))).to.equal(true)
+      await expect(await storage.willProofBeRequired(id)).to.be.true
+      await advanceTimeTo(request.expiry + 1)
+      await expect(await storage.willProofBeRequired(id)).to.be.false
+    })
+
+    it("does not require proofs once cancelled", async function () {
+      const id = slotId(slot)
+      await storage.fillSlot(slot.request, slot.index, proof)
+      await waitUntilProofIsRequired(id)
+      await expect(await storage.isProofRequired(id)).to.be.true
+      await advanceTimeTo(request.expiry + 1)
+      await expect(await storage.isProofRequired(id)).to.be.false
+    })
+
+    it("does not provide challenges once cancelled", async function () {
+      const id = slotId(slot)
+      await storage.fillSlot(slot.request, slot.index, proof)
+      await waitUntilProofIsRequired(id)
+      const challenge1 = await storage.getChallenge(id)
+      expect(BigNumber.from(challenge1).gt(0))
+      await advanceTimeTo(request.expiry + 1)
+      const challenge2 = await storage.getChallenge(id)
+      expect(BigNumber.from(challenge2).isZero())
+    })
+
+    it("does not provide pointer once cancelled", async function () {
+      const id = slotId(slot)
+      await storage.fillSlot(slot.request, slot.index, proof)
+      await waitUntilProofIsRequired(id)
+      const challenge1 = await storage.getChallenge(id)
+      expect(BigNumber.from(challenge1).gt(0))
+      await advanceTimeTo(request.expiry + 1)
+      const challenge2 = await storage.getChallenge(id)
+      expect(BigNumber.from(challenge2).isZero())
     })
   })
 })