From e496ac3550cfc09a0496721b6f2a288539d146f9 Mon Sep 17 00:00:00 2001 From: Mark Spanbroek Date: Mon, 16 Jan 2023 14:14:15 +0100 Subject: [PATCH] [marketplace] remove _isCancelled and _isFinished --- contracts/Marketplace.sol | 39 ++++++++++------------------------- contracts/TestMarketplace.sol | 4 ---- test/Marketplace.test.js | 29 ++------------------------ 3 files changed, 13 insertions(+), 59 deletions(-) diff --git a/contracts/Marketplace.sol b/contracts/Marketplace.sol index 4357ab8..0c31fb6 100644 --- a/contracts/Marketplace.sol +++ b/contracts/Marketplace.sol @@ -179,8 +179,10 @@ contract Marketplace is Collateral, Proofs, StateRetrieval { RequestId requestId, SlotId slotId ) private marketplaceInvariant { + RequestState requestState = state(requestId); require( - _isFinished(requestId) || _isCancelled(requestId), + requestState == RequestState.Finished || + requestState == RequestState.Cancelled, "Contract not ended" ); RequestContext storage context = _context(requestId); @@ -225,30 +227,6 @@ contract Marketplace is Collateral, Proofs, StateRetrieval { require(token.transfer(msg.sender, amount), "Withdraw failed"); } - /// @notice Return true if the request state is RequestState.Cancelled or if the request expiry time has elapsed and the request was never started. - /// @dev Handles the case when a request may have been cancelled, but the client has not withdrawn its funds yet, and therefore the state has not yet been updated. - /// @param requestId the id of the request - /// @return true if request is cancelled - function _isCancelled(RequestId requestId) internal view returns (bool) { - RequestContext storage context = _context(requestId); - return - context.state == RequestState.Cancelled || - (context.state == RequestState.New && - block.timestamp > _request(requestId).expiry); - } - - /// @notice Return true if the request state is RequestState.Finished or if the request duration has elapsed and the request was started. - /// @dev Handles the case when a request may have been finished, but the state has not yet been updated by a transaction. - /// @param requestId the id of the request - /// @return true if request is finished - function _isFinished(RequestId requestId) internal view returns (bool) { - RequestContext memory context = _context(requestId); - return - context.state == RequestState.Finished || - (context.state == RequestState.Started && - block.timestamp > context.endsAt); - } - /// @notice Return id of request that slot belongs to /// @dev Returns requestId that is mapped to the slotId /// @param slotId id of the slot @@ -354,12 +332,17 @@ contract Marketplace is Collateral, Proofs, StateRetrieval { } function state(RequestId requestId) public view returns (RequestState) { - if (_isCancelled(requestId)) { + RequestContext storage context = _context(requestId); + if ( + context.state == RequestState.New && + block.timestamp > _request(requestId).expiry + ) { return RequestState.Cancelled; - } else if (_isFinished(requestId)) { + } else if ( + context.state == RequestState.Started && block.timestamp > context.endsAt + ) { return RequestState.Finished; } else { - RequestContext storage context = _context(requestId); return context.state; } } diff --git a/contracts/TestMarketplace.sol b/contracts/TestMarketplace.sol index 6a32e6d..ec825df 100644 --- a/contracts/TestMarketplace.sol +++ b/contracts/TestMarketplace.sol @@ -30,10 +30,6 @@ contract TestMarketplace is Marketplace { } - function isCancelled(RequestId requestId) public view returns (bool) { - return _isCancelled(requestId); - } - function forciblyFreeSlot(SlotId slotId) public { _forciblyFreeSlot(slotId); } diff --git a/test/Marketplace.test.js b/test/Marketplace.test.js index 470eb4a..87a7b14 100644 --- a/test/Marketplace.test.js +++ b/test/Marketplace.test.js @@ -653,12 +653,6 @@ describe("Marketplace", function () { ) }) - it("changes isCancelled to true once request is cancelled", async function () { - await expect(await marketplace.isCancelled(slot.request)).to.be.false - await waitUntilCancelled(request) - await expect(await marketplace.isCancelled(slot.request)).to.be.true - }) - it("changes proofEnd to the past when request is cancelled", async function () { await marketplace.fillSlot(slot.request, slot.index, proof) await expect(await marketplace.proofEnd(slotId(slot))).to.be.gt( @@ -836,7 +830,7 @@ describe("Marketplace", function () { }) describe("accepting proofs", function () { - it("fails when request Cancelled (isCancelled is true)", async function () { + it("fails when request Cancelled", async function () { await marketplace.fillSlot(slot.request, slot.index, proof) await waitUntilCancelled(request) await expect( @@ -844,17 +838,7 @@ describe("Marketplace", function () { ).to.be.revertedWith("Slot not accepting proofs") }) - it("fails when request Cancelled (state set to Cancelled)", async function () { - await marketplace.fillSlot(slot.request, slot.index, proof) - await waitUntilCancelled(request) - switchAccount(client) - await marketplace.withdrawFunds(slot.request) - await expect( - marketplace.testAcceptsProofs(slotId(slot)) - ).to.be.revertedWith("Slot not accepting proofs") - }) - - it("fails when request Finished (isFinished is true)", async function () { + it("fails when request Finished", async function () { await waitUntilStarted(marketplace, request, proof) await waitUntilFinished(marketplace, requestId(request)) await expect( @@ -862,15 +846,6 @@ describe("Marketplace", function () { ).to.be.revertedWith("Slot not accepting proofs") }) - it("fails when request Finished (state set to Finished)", async function () { - await waitUntilStarted(marketplace, request, proof) - await waitUntilFinished(marketplace, requestId(request)) - await marketplace.freeSlot(slotId(slot)) - await expect( - marketplace.testAcceptsProofs(slotId(slot)) - ).to.be.revertedWith("Slot not accepting proofs") - }) - it("fails when request Failed", async function () { await waitUntilStarted(marketplace, request, proof) await waitUntilFailed(marketplace, request)