fix(slot-reservations): ensure slot is free (#196)

Ensure that the slot state is free before allowing reservations

contracts/Marketplace.sol

@@ -457,6 +457,10 @@ contract Marketplace is SlotReservations, Proofs, StateRetrieval, Endian {
     _;
   }
 
+  function _slotIsFree(SlotId slotId) internal view override returns (bool) {
+    return _slots[slotId].state == SlotState.Free;
+  }
+
   function requestEnd(RequestId requestId) public view returns (uint256) {
     uint256 end = _requestContexts[requestId].endsAt;
     RequestState state = requestState(requestId);

contracts/SlotReservations.sol

@@ -5,7 +5,7 @@ import "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
 import "./Requests.sol";
 import "./Configuration.sol";
 
-contract SlotReservations {
+abstract contract SlotReservations {
   using EnumerableSet for EnumerableSet.AddressSet;
 
   mapping(SlotId => EnumerableSet.AddressSet) internal _reservations;
@@ -15,6 +15,8 @@ contract SlotReservations {
     _config = config;
   }
 
+  function _slotIsFree(SlotId slotId) internal view virtual returns (bool);
+
   function reserveSlot(RequestId requestId, uint256 slotIndex) public {
     require(canReserveSlot(requestId, slotIndex), "Reservation not allowed");
 
@@ -34,6 +36,7 @@ contract SlotReservations {
     SlotId slotId = Requests.slotId(requestId, slotIndex);
     return
       // TODO: add in check for address inside of expanding window
+      _slotIsFree(slotId) &&
       (_reservations[slotId].length() < _config.maxReservations) &&
       (!_reservations[slotId].contains(host));
   }

contracts/TestSlotReservations.sol

@@ -6,6 +6,8 @@ import "./SlotReservations.sol";
 contract TestSlotReservations is SlotReservations {
   using EnumerableSet for EnumerableSet.AddressSet;
 
+  mapping(SlotId => SlotState) private _states;
+
   // solhint-disable-next-line no-empty-blocks
   constructor(SlotReservationsConfig memory config) SlotReservations(config) {}
 
@@ -16,4 +18,12 @@ contract TestSlotReservations is SlotReservations {
   function length(SlotId slotId) public view returns (uint256) {
     return _reservations[slotId].length();
   }
+
+  function _slotIsFree(SlotId slotId) internal view override returns (bool) {
+    return _states[slotId] == SlotState.Free;
+  }
+
+  function setSlotState(SlotId id, SlotState state) public {
+    _states[id] = state;
+  }
 }

test/SlotReservations.test.js

@@ -2,6 +2,7 @@ const { expect } = require("chai")
 const { ethers } = require("hardhat")
 const { exampleRequest, exampleConfiguration } = require("./examples")
 const { requestId, slotId } = require("./ids")
+const { SlotState } = require("./requests")
 
 describe("SlotReservations", function () {
   let reservations
@@ -28,6 +29,8 @@ describe("SlotReservations", function () {
       index: slotIndex,
     }
     id = slotId(slot)
+
+    await reservations.setSlotState(id, SlotState.Free)
   })
 
   function switchAccount(account) {
@@ -99,6 +102,19 @@ describe("SlotReservations", function () {
     expect(await reservations.canReserveSlot(reqId, slotIndex)).to.be.false
   })
 
+  it("cannot reserve a slot if not free", async function () {
+    await reservations.setSlotState(id, SlotState.Filled)
+    await expect(reservations.reserveSlot(reqId, slotIndex)).to.be.revertedWith(
+      "Reservation not allowed"
+    )
+    expect(await reservations.length(id)).to.equal(0)
+  })
+
+  it("reports a slot cannot be reserved if not free", async function () {
+    await reservations.setSlotState(id, SlotState.Filled)
+    expect(await reservations.canReserveSlot(reqId, slotIndex)).to.be.false
+  })
+
   it("should emit an event when slot reservations are full", async function () {
     await reservations.reserveSlot(reqId, slotIndex)
     switchAccount(address1)