From baded845f3fe71bf9f13b39330b3bf6813b66765 Mon Sep 17 00:00:00 2001 From: Eric <5089238+emizzle@users.noreply.github.com> Date: Thu, 15 May 2025 11:37:50 +1000 Subject: [PATCH] fix(slot-reservations): Allows slot to be reserved when in repair (#234) Previous to when SlotState.Repair was implemented, slots in repair would be considered free and the slots could be reserved in this state. Now that SlotState.Repair has been implemented, the `canReserveSlot` needs to check that the SlotState is in Repair or is Free before allowing reservation. --- contracts/Marketplace.sol | 8 +++----- contracts/SlotReservations.sol | 5 +++-- contracts/TestSlotReservations.sol | 4 ++-- test/SlotReservations.test.js | 19 +++++++++++++++---- 4 files changed, 23 insertions(+), 13 deletions(-) diff --git a/contracts/Marketplace.sol b/contracts/Marketplace.sol index d8c23ae..60d8aef 100644 --- a/contracts/Marketplace.sol +++ b/contracts/Marketplace.sol @@ -559,10 +559,6 @@ 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 (uint64) { RequestState state = requestState(requestId); if (state == RequestState.New || state == RequestState.Started) { @@ -636,7 +632,9 @@ contract Marketplace is SlotReservations, Proofs, StateRetrieval, Endian { } } - function slotState(SlotId slotId) public view override returns (SlotState) { + function slotState( + SlotId slotId + ) public view override(Proofs, SlotReservations) returns (SlotState) { Slot storage slot = _slots[slotId]; if (RequestId.unwrap(slot.requestId) == 0) { return SlotState.Free; diff --git a/contracts/SlotReservations.sol b/contracts/SlotReservations.sol index c1359a8..8faa87b 100644 --- a/contracts/SlotReservations.sol +++ b/contracts/SlotReservations.sol @@ -16,7 +16,7 @@ abstract contract SlotReservations { _config = config; } - function _slotIsFree(SlotId slotId) internal view virtual returns (bool); + function slotState(SlotId id) public view virtual returns (SlotState); function reserveSlot(RequestId requestId, uint64 slotIndex) public { if (!canReserveSlot(requestId, slotIndex)) @@ -36,9 +36,10 @@ abstract contract SlotReservations { ) public view returns (bool) { address host = msg.sender; SlotId slotId = Requests.slotId(requestId, slotIndex); + SlotState state = slotState(slotId); return // TODO: add in check for address inside of expanding window - _slotIsFree(slotId) && + (state == SlotState.Free || state == SlotState.Repair) && (_reservations[slotId].length() < _config.maxReservations) && (!_reservations[slotId].contains(host)); } diff --git a/contracts/TestSlotReservations.sol b/contracts/TestSlotReservations.sol index ae2d528..3793223 100644 --- a/contracts/TestSlotReservations.sol +++ b/contracts/TestSlotReservations.sol @@ -19,8 +19,8 @@ contract TestSlotReservations is SlotReservations { return _reservations[slotId].length(); } - function _slotIsFree(SlotId slotId) internal view override returns (bool) { - return _states[slotId] == SlotState.Free; + function slotState(SlotId slotId) public view override returns (SlotState) { + return _states[slotId]; } function setSlotState(SlotId id, SlotState state) public { diff --git a/test/SlotReservations.test.js b/test/SlotReservations.test.js index 04f4ced..cc91bad 100644 --- a/test/SlotReservations.test.js +++ b/test/SlotReservations.test.js @@ -37,7 +37,12 @@ describe("SlotReservations", function () { reservations = reservations.connect(account) } - it("allows a slot to be reserved", async function () { + it("allows a slot to be reserved if free", async function () { + expect(reservations.reserveSlot(reqId, slotIndex)).to.not.be.reverted + }) + + it("allows a slot to be reserved if in repair", async function () { + await reservations.setSlotState(id, SlotState.Repair) expect(reservations.reserveSlot(reqId, slotIndex)).to.not.be.reverted }) @@ -59,7 +64,13 @@ describe("SlotReservations", function () { expect(await reservations.length(id)).to.equal(2) }) - it("reports a slot can be reserved", async function () { + it("reports a slot can be reserved if free", async function () { + await reservations.setSlotState(id, SlotState.Free) + expect(await reservations.canReserveSlot(reqId, slotIndex)).to.be.true + }) + + it("reports a slot can be reserved if in repair", async function () { + await reservations.setSlotState(id, SlotState.Repair) expect(await reservations.canReserveSlot(reqId, slotIndex)).to.be.true }) @@ -102,7 +113,7 @@ describe("SlotReservations", function () { expect(await reservations.canReserveSlot(reqId, slotIndex)).to.be.false }) - it("cannot reserve a slot if not free", async function () { + it("cannot reserve a slot if not free or not in repair", async function () { await reservations.setSlotState(id, SlotState.Filled) await expect(reservations.reserveSlot(reqId, slotIndex)).to.be.revertedWith( "SlotReservations_ReservationNotAllowed" @@ -110,7 +121,7 @@ describe("SlotReservations", function () { expect(await reservations.length(id)).to.equal(0) }) - it("reports a slot cannot be reserved if not free", async function () { + it("reports a slot cannot be reserved if not free or not in repair", async function () { await reservations.setSlotState(id, SlotState.Filled) expect(await reservations.canReserveSlot(reqId, slotIndex)).to.be.false })