diff --git a/contracts/Marketplace.sol b/contracts/Marketplace.sol index d13b4fc..dc01305 100644 --- a/contracts/Marketplace.sol +++ b/contracts/Marketplace.sol @@ -14,6 +14,7 @@ contract Marketplace is Collateral, Proofs { type RequestId is bytes32; type SlotId is bytes32; + type ActiveSlotId is bytes32; uint256 public immutable collateral; MarketplaceFunds private funds; @@ -21,8 +22,8 @@ contract Marketplace is Collateral, Proofs { mapping(RequestId => RequestContext) private requestContexts; mapping(SlotId => Slot) private slots; mapping(address => EnumerableSet.Bytes32Set) private activeRequests; - mapping(address => EnumerableSet.Bytes32Set) private activeSlots; - mapping(address => uint8) private activeSlotsIdx; + mapping(RequestId => mapping(ActiveSlotId => EnumerableSet.Bytes32Set)) private activeSlots; + mapping(RequestId => uint8) private activeSlotsIdx; constructor( IERC20 _token, @@ -42,36 +43,39 @@ contract Marketplace is Collateral, Proofs { return _toRequestIds(activeRequests[msg.sender].values()); } - function mySlots() public view returns (SlotId[] memory) { - return _activeSlotsForHost(msg.sender); + function mySlots(RequestId requestId) + public + view + returns (SlotId[] memory) + { + EnumerableSet.Bytes32Set storage slotIds = _activeSlotsForHost(msg.sender, + requestId); + return _toSlotIds(slotIds.values()); } - function _activeSlotsForHost(address host) + + function _activeSlotsForHost(address host, RequestId requestId) internal view - returns (SlotId[] memory) + returns (EnumerableSet.Bytes32Set storage) { - return _toSlotIds(activeSlots[host].values()); + mapping(ActiveSlotId => EnumerableSet.Bytes32Set) storage activeForReq = + activeSlots[requestId]; + ActiveSlotId id = _toActiveSlotId(host, requestId); + return activeForReq[id]; + } + + /// @notice Clears active slots for a request + /// @dev Because there are no efficient ways to clear an EnumerableSet, an index is updated that points to a new instance. + /// @param requestId request for which to clear the active slots + function _clearActiveSlots(RequestId requestId) internal { + activeSlotsIdx[requestId]++; } function _equals(RequestId a, RequestId b) internal pure returns (bool) { return RequestId.unwrap(a) == RequestId.unwrap(b); } - /// @notice Clears active slots for a host for a given request - /// @dev WARNING: This could potentially run out of gas if the number of slots is too high. Calling .values() copies to memory. - /// @param host address of the host for which to clear the active slots - /// @param requestId identifies the request that the slot must belong to for it to be cleared - function _clearActiveSlotsForRequest(address host, RequestId requestId) internal { - SlotId[] memory slotIds = _activeSlotsForHost(host); - for (uint8 i = 0; i < slotIds.length; i++) { - RequestId slotRequestId = _getRequestIdForSlot(slotIds[i]); - if (_equals(slotRequestId, requestId)) { - activeSlots[host].remove(SlotId.unwrap(slotIds[i])); - } - } - } - function requestStorage(Request calldata request) public marketplaceInvariant @@ -123,7 +127,7 @@ contract Marketplace is Collateral, Proofs { slot.requestId = requestId; RequestContext storage context = _context(requestId); context.slotsFilled += 1; - activeSlots[slot.host].add(SlotId.unwrap(slotId)); + _activeSlotsForHost(slot.host, requestId).add(SlotId.unwrap(slotId)); emit SlotFilled(requestId, slotIndex, slotId); if (context.slotsFilled == request.ask.slots) { context.state = RequestState.Started; @@ -150,8 +154,7 @@ contract Marketplace is Collateral, Proofs { _unexpectProofs(_toProofId(slotId)); - address slotHost = slot.host; - activeSlots[slotHost].remove(SlotId.unwrap(slotId)); + _activeSlotsForHost(slot.host, requestId).remove(SlotId.unwrap(slotId)); slot.host = address(0); slot.requestId = RequestId.wrap(0); context.slotsFilled -= 1; @@ -167,7 +170,7 @@ contract Marketplace is Collateral, Proofs { _setProofEnd(_toEndId(requestId), block.timestamp - 1); context.endsAt = block.timestamp - 1; activeRequests[request.client].remove(RequestId.unwrap(requestId)); - _clearActiveSlotsForRequest(slotHost, requestId); + _clearActiveSlots(requestId); emit RequestFailed(requestId); // TODO: burn all remaining slot collateral (note: slot collateral not @@ -188,7 +191,7 @@ contract Marketplace is Collateral, Proofs { SlotId slotId = _toSlotId(requestId, slotIndex); Slot storage slot = _slot(slotId); require(!slot.hostPaid, "Already paid"); - activeSlots[slot.host].remove(SlotId.unwrap(slotId)); + _activeSlotsForHost(slot.host, requestId).remove(SlotId.unwrap(slotId)); uint256 amount = pricePerSlot(requests[requestId]); funds.sent += amount; funds.balance -= amount; @@ -406,6 +409,17 @@ contract Marketplace is Collateral, Proofs { return SlotId.wrap(keccak256(abi.encode(requestId, slotIndex))); } + function _toActiveSlotId(address host, RequestId requestId) + internal + view + returns (ActiveSlotId) + { + uint8 activeSlotIdx = activeSlotsIdx[requestId]; + return ActiveSlotId.wrap( + keccak256(abi.encode(host, activeSlotIdx)) + ); + } + function _toLockId(RequestId requestId) internal pure returns (LockId) { return LockId.wrap(RequestId.unwrap(requestId)); } diff --git a/test/Marketplace.test.js b/test/Marketplace.test.js index e86008a..41d8522 100644 --- a/test/Marketplace.test.js +++ b/test/Marketplace.test.js @@ -746,7 +746,7 @@ describe("Marketplace", function () { await marketplace.fillSlot(slot.request, slot.index, proof) let slot1 = { ...slot, index: slot.index + 1 } await marketplace.fillSlot(slot.request, slot1.index, proof) - expect(await marketplace.mySlots()).to.deep.equal([ + expect(await marketplace.mySlots(slot.request)).to.deep.equal([ slotId(slot), slotId(slot1), ]) @@ -757,10 +757,34 @@ describe("Marketplace", function () { let slot1 = { ...slot, index: slot.index + 1 } await marketplace.fillSlot(slot.request, slot1.index, proof) await marketplace.freeSlot(slotId(slot)) - expect(await marketplace.mySlots()).to.deep.equal([slotId(slot1)]) + expect(await marketplace.mySlots(slot.request)).to.deep.equal([ + slotId(slot1), + ]) }) - it("removes all request's active slots when request fails", async function () { + it("removes active slots for all hosts in a request when it fails", async function () { + let halfOfSlots = request.ask.slots / 2 + + // fill half the slots with host1 + for (let i = 0; i < Math.floor(halfOfSlots); i++) { + await marketplace.fillSlot(requestId(request), i, proof) + } + + // fill other half of slots with host2 + switchAccount(host2) + await token.approve(marketplace.address, collateral) + await marketplace.deposit(collateral) + for (let i = Math.floor(halfOfSlots); i < request.ask.slots; i++) { + await marketplace.fillSlot(requestId(request), i, proof) + } + + await waitUntilFailed(marketplace, request, slot) + expect(await marketplace.mySlots(slot.request)).to.deep.equal([]) + switchAccount(host) + expect(await marketplace.mySlots(slot.request)).to.deep.equal([]) + }) + + it("doesn't remove active slots for hosts in request that didn't fail", async function () { // start first request await waitUntilStarted(marketplace, request, proof) @@ -784,14 +808,19 @@ describe("Marketplace", function () { let id = slotId(expectedSlot) expected.push(id) } - expect(await marketplace.mySlots()).to.deep.equal(expected.reverse()) + expect(await marketplace.mySlots(slot.request)).to.deep.equal([]) + expect(await marketplace.mySlots(expectedSlot.request)).to.deep.equal( + expected + ) }) it("removes slots from list when request finishes", async function () { await waitUntilStarted(marketplace, request, proof) await waitUntilFinished(marketplace, requestId(request)) await marketplace.payoutSlot(slot.request, slot.index) - expect(await marketplace.mySlots()).to.not.contain(slotId(slot)) + expect(await marketplace.mySlots(slot.request)).to.not.contain( + slotId(slot) + ) }) }) })