feat: expiry specified as duration (#99)

contracts/Marketplace.sol

@@ -32,6 +32,7 @@ contract Marketplace is Proofs, StateRetrieval, Endian {
     uint256 expiryFundsWithdraw;
     uint256 startedAt;
     uint256 endsAt;
+    uint256 expiresAt;
   }
 
   struct Slot {
@@ -90,11 +91,14 @@ contract Marketplace is Proofs, StateRetrieval, Endian {
 
     RequestId id = request.id();
     require(_requests[id].client == address(0), "Request already exists");
+    require(
+      request.expiry > 0 && request.expiry < request.ask.duration,
+      "Expiry not in range"
+    );
 
     _requests[id] = request;
-    uint256 endsAt = block.timestamp + request.ask.duration;
+    _requestContexts[id].endsAt = block.timestamp + request.ask.duration;
-    require(endsAt > request.expiry, "Request end before expiry");
+    _requestContexts[id].expiresAt = block.timestamp + request.expiry;
-    _requestContexts[id].endsAt = endsAt;
 
     _addToMyRequests(request.client, id);
 
@@ -103,7 +107,7 @@ contract Marketplace is Proofs, StateRetrieval, Endian {
     _marketplaceTotals.received += amount;
     _transferFrom(msg.sender, amount);
 
-    emit StorageRequested(id, request.ask, request.expiry);
+    emit StorageRequested(id, request.ask, _requestContexts[id].expiresAt);
   }
 
   function fillSlot(
@@ -206,6 +210,8 @@ contract Marketplace is Proofs, StateRetrieval, Endian {
     Slot storage slot = _slots[slotId];
     Request storage request = _requests[slot.requestId];
 
+    // TODO: Reward for validator that calls this function
+
     if (missingProofs(slotId) % _config.collateral.slashCriterion == 0) {
       uint256 slashedAmount = (request.ask.collateral *
         _config.collateral.slashPercentage) / 100;
@@ -286,7 +292,10 @@ contract Marketplace is Proofs, StateRetrieval, Endian {
   /// @param requestId the id of the request
   function withdrawFunds(RequestId requestId) public {
     Request storage request = _requests[requestId];
-    require(block.timestamp > request.expiry, "Request not yet timed out");
+    require(
+      block.timestamp > requestExpiry(requestId),
+      "Request not yet timed out"
+    );
     require(request.client == msg.sender, "Invalid client address");
     RequestContext storage context = _requestContexts[requestId];
     require(context.state == RequestState.New, "Invalid state");
@@ -339,15 +348,22 @@ contract Marketplace is Proofs, StateRetrieval, Endian {
     }
   }
 
+  function requestExpiry(RequestId requestId) public view returns (uint256) {
+    return _requestContexts[requestId].expiresAt;
+  }
+
   /// @notice Calculates the amount that should be payed out to a host if a request expires based on when the host fills the slot
   function _expiryPayoutAmount(
     RequestId requestId,
     uint256 startingTimestamp
   ) private view returns (uint256) {
     Request storage request = _requests[requestId];
-    require(startingTimestamp < request.expiry, "Start not before expiry");
+    require(
+      startingTimestamp < requestExpiry(requestId),
+      "Start not before expiry"
+    );
 
-    return (request.expiry - startingTimestamp) * request.ask.reward;
+    return (requestExpiry(requestId) - startingTimestamp) * request.ask.reward;
   }
 
   function getHost(SlotId slotId) public view returns (address) {
@@ -360,7 +376,7 @@ contract Marketplace is Proofs, StateRetrieval, Endian {
     RequestContext storage context = _requestContexts[requestId];
     if (
       context.state == RequestState.New &&
-      block.timestamp > _requests[requestId].expiry
+      block.timestamp > requestExpiry(requestId)
     ) {
       return RequestState.Cancelled;
     } else if (

contracts/Requests.sol

@@ -8,7 +8,7 @@ struct Request {
   address client;
   Ask ask;
   Content content;
-  uint256 expiry; // timestamp as seconds since unix epoch at which this request expires
+  uint256 expiry; // amount of seconds since start of the request at which this request expires
   bytes32 nonce; // random nonce to differentiate between similar requests
 }
 

test/Marketplace.test.js

@@ -140,9 +140,12 @@ describe("Marketplace", function () {
 
     it("emits event when storage is requested", async function () {
       await token.approve(marketplace.address, price(request))
+
+      // We +1 second to the expiry because the time will advance with the mined transaction for requestStorage because of Hardhat
+      const expectedExpiry = (await currentTime()) + request.expiry + 1
       await expect(marketplace.requestStorage(request))
         .to.emit(marketplace, "StorageRequested")
-        .withArgs(requestId(request), askToArray(request.ask), request.expiry)
+        .withArgs(requestId(request), askToArray(request.ask), expectedExpiry)
     })
 
     it("allows retrieval of request details", async function () {
@@ -168,11 +171,17 @@ describe("Marketplace", function () {
       )
     })
 
-    it("rejects request when expiry is after request end", async function () {
+    it("rejects request when expiry out of bounds", async function () {
-      request.expiry = (await currentTime()) + request.ask.duration + hours(1)
       await token.approve(marketplace.address, price(request))
+
+      request.expiry = request.ask.duration + 1
       await expect(marketplace.requestStorage(request)).to.be.revertedWith(
-        "Request end before expiry"
+        "Expiry not in range"
+      )
+
+      request.expiry = 0
+      await expect(marketplace.requestStorage(request)).to.be.revertedWith(
+        "Expiry not in range"
       )
     })
 
@@ -241,9 +250,10 @@ describe("Marketplace", function () {
 
     it("is rejected when request is cancelled", async function () {
       switchAccount(client)
-      let expired = { ...request, expiry: (await currentTime()) - hours(1) }
+      let expired = { ...request, expiry: hours(1) + 1 }
       await token.approve(marketplace.address, price(request))
       await marketplace.requestStorage(expired)
+      await waitUntilCancelled(expired)
       switchAccount(host)
       await expect(
         marketplace.fillSlot(requestId(expired), slot.index, proof)
@@ -465,20 +475,19 @@ describe("Marketplace", function () {
     })
 
     it("pays the host when contract was cancelled", async function () {
-      // Lets move the time into middle of the expiry window
+      // Lets advance the time more into the expiry window
-      const fillTimestamp =
+      const filledAt = (await currentTime()) + Math.floor(request.expiry / 3)
-        (await currentTime()) +
+      const expiresAt = (
-        Math.floor((request.expiry - (await currentTime())) / 2) -
+        await marketplace.requestExpiry(requestId(request))
-        1
+      ).toNumber()
-      await advanceTimeToForNextBlock(fillTimestamp)
 
+      await advanceTimeToForNextBlock(filledAt)
       await marketplace.fillSlot(slot.request, slot.index, proof)
       await waitUntilCancelled(request)
       await marketplace.freeSlot(slotId(slot))
 
+      const expectedPartialPayout = (expiresAt - filledAt) * request.ask.reward
       const endBalance = await token.balanceOf(host.address)
-      const expectedPartialPayout =
-        (request.expiry - fillTimestamp) * request.ask.reward
       expect(endBalance - ACCOUNT_STARTING_BALANCE).to.be.equal(
         expectedPartialPayout
       )
@@ -617,14 +626,17 @@ describe("Marketplace", function () {
     })
 
     it("withdraws to the client for cancelled requests lowered by hosts payout", async function () {
-      const fillTimestamp =
+      // Lets advance the time more into the expiry window
-        (await currentTime()) +
+      const filledAt = (await currentTime()) + Math.floor(request.expiry / 3)
-        Math.floor((request.expiry - (await currentTime())) / 2)
+      const expiresAt = (
-      await advanceTimeToForNextBlock(fillTimestamp)
+        await marketplace.requestExpiry(requestId(request))
+      ).toNumber()
+
+      await advanceTimeToForNextBlock(filledAt)
       await marketplace.fillSlot(slot.request, slot.index, proof)
       await waitUntilCancelled(request)
       const expectedPartialHostPayout =
-        (request.expiry - fillTimestamp) * request.ask.reward
+        (expiresAt - filledAt) * request.ask.reward
 
       switchAccount(client)
       await marketplace.withdrawFunds(slot.request)

test/examples.js

@@ -1,6 +1,5 @@
 const { ethers } = require("hardhat")
 const { hours } = require("./time")
-const { currentTime } = require("./evm")
 const { hexlify, randomBytes } = ethers.utils
 
 const exampleConfiguration = () => ({
@@ -19,7 +18,6 @@ const exampleConfiguration = () => ({
 })
 
 const exampleRequest = async () => {
-  const now = await currentTime()
   return {
     client: hexlify(randomBytes(20)),
     ask: {
@@ -35,7 +33,7 @@ const exampleRequest = async () => {
       cid: "zb2rhheVmk3bLks5MgzTqyznLu1zqGH5jrfTA1eAZXrjx7Vob",
       merkleRoot: Array.from(randomBytes(32)),
     },
-    expiry: now + hours(1),
+    expiry: hours(1),
     nonce: hexlify(randomBytes(32)),
   }
 }

test/marketplace.js

@@ -2,8 +2,16 @@ const { advanceTimeToForNextBlock, currentTime } = require("./evm")
 const { slotId, requestId } = require("./ids")
 const { price } = require("./price")
 
+/**
+ * @dev This will not advance the time right on the "expiry threshold" but will most probably "overshoot it"
+ *      because "currentTime" most probably is not the time at which the request is created, but it is used
+ *      in the next timestamp calculation with `now + expiry`.
+ * @param request
+ * @returns {Promise<void>}
+ */
 async function waitUntilCancelled(request) {
-  await advanceTimeToForNextBlock(request.expiry + 1)
+  // We do +1, because the expiry check in contract is done as `>` and not `>=`.
+  await advanceTimeToForNextBlock((await currentTime()) + request.expiry + 1)
 }
 
 async function waitUntilStarted(contract, request, proof, token) {
@@ -16,6 +24,7 @@ async function waitUntilStarted(contract, request, proof, token) {
 
 async function waitUntilFinished(contract, requestId) {
   const end = (await contract.requestEnd(requestId)).toNumber()
+  // We do +1, because the end check in contract is done as `>` and not `>=`.
   await advanceTimeToForNextBlock(end + 1)
 }
 

test/requests.js

@@ -1,4 +1,5 @@
 const { Assertion } = require("chai")
+const { currentTime } = require("./evm")
 
 const RequestState = {
   New: 0,
@@ -46,4 +47,8 @@ const enableRequestAssertions = function () {
   })
 }
 
-module.exports = { RequestState, SlotState, enableRequestAssertions }
+module.exports = {
+  RequestState,
+  SlotState,
+  enableRequestAssertions,
+}