diff --git a/contracts/Marketplace.sol b/contracts/Marketplace.sol index cf1d448..2c076ac 100644 --- a/contracts/Marketplace.sol +++ b/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; diff --git a/contracts/Storage.sol b/contracts/Storage.sol index b0572dc..7699c45 100644 --- a/contracts/Storage.sol +++ b/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); diff --git a/contracts/TestMarketplace.sol b/contracts/TestMarketplace.sol index 524904b..e15be96 100644 --- a/contracts/TestMarketplace.sol +++ b/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); } -} \ No newline at end of file +} diff --git a/test/Marketplace.test.js b/test/Marketplace.test.js index b0b309a..e0893f5 100644 --- a/test/Marketplace.test.js +++ b/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( diff --git a/test/marketplace.js b/test/marketplace.js index a67715f..35ceb3b 100644 --- a/test/marketplace.js +++ b/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) } }