From b580ffd8a3e4ebed76b6dae739aaa3259b57d656 Mon Sep 17 00:00:00 2001 From: Eric Mastro Date: Mon, 22 Aug 2022 16:16:45 +1000 Subject: [PATCH] [refactor] do not expose cancelled logic externally --- contracts/Marketplace.sol | 26 ++++++++++++++++++++------ contracts/Storage.sol | 12 ++++++++++++ test/Marketplace.test.js | 20 -------------------- test/Storage.test.js | 20 ++++++++++++++++++++ 4 files changed, 52 insertions(+), 26 deletions(-) diff --git a/contracts/Marketplace.sol b/contracts/Marketplace.sol index a6127e9..bc0d52d 100644 --- a/contracts/Marketplace.sol +++ b/contracts/Marketplace.sol @@ -124,7 +124,7 @@ contract Marketplace is Collateral, Proofs { /// @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(bytes32 requestId) public view returns (bool) { + function _isCancelled(bytes32 requestId) internal view returns (bool) { RequestContext memory context = requestContexts[requestId]; return context.state == RequestState.Cancelled || @@ -134,15 +134,23 @@ contract Marketplace is Collateral, Proofs { ); } + /// @notice Return id of request that slot belongs to + /// @dev Returns requestId that is mapped to the slotId + /// @param slotId id of the slot + /// @return if of the request the slot belongs to + function _getRequestIdForSlot(bytes32 slotId) internal view returns (bytes32) { + Slot memory slot = _slot(slotId); + require(slot.requestId != 0, "Missing request id"); + return slot.requestId; + } + /// @notice Return true if the request state the slot belongs to 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 slotId the id of the slot /// @return true if request is cancelled - function isSlotCancelled(bytes32 slotId) public view returns (bool) { - Slot memory slot = slots[slotId]; - require(slot.host != address(0), "Slot empty"); - require(slot.requestId != 0, "Missing request id"); - return isCancelled(slot.requestId); + function _isSlotCancelled(bytes32 slotId) internal view returns (bool) { + bytes32 requestId = _getRequestIdForSlot(slotId); + return _isCancelled(requestId); } function _host(bytes32 slotId) internal view returns (address) { @@ -153,6 +161,12 @@ contract Marketplace is Collateral, Proofs { return requests[id]; } + function _slot(bytes32 slotId) internal view returns (Slot memory) { + Slot memory slot = slots[slotId]; + require(slot.host != address(0), "Slot empty"); + return slot; + } + function proofPeriod() public view returns (uint256) { return _period(); } diff --git a/contracts/Storage.sol b/contracts/Storage.sol index 4757f08..c9f6f00 100644 --- a/contracts/Storage.sol +++ b/contracts/Storage.sol @@ -36,6 +36,18 @@ contract Storage is Collateral, Marketplace { return _request(requestId); } + function getSlot(bytes32 slotId) public view returns (Slot memory) { + return _slot(slotId); + } + + function isCancelled(bytes32 requestId) public view returns(bool) { + return _isCancelled(requestId); + } + + function isSlotCancelled(bytes32 slotId) public view returns (bool) { + return _isSlotCancelled(slotId); + } + function getHost(bytes32 requestId) public view returns (address) { return _host(requestId); } diff --git a/test/Marketplace.test.js b/test/Marketplace.test.js index 8b9955e..f8f7b16 100644 --- a/test/Marketplace.test.js +++ b/test/Marketplace.test.js @@ -351,26 +351,6 @@ describe("Marketplace", function () { Failed: 4, } - it("isCancelled is true once request is cancelled", async function () { - await expect(await marketplace.isCancelled(slot.request)).to.equal(false) - await waitUntilExpired(request.expiry) - await expect(await marketplace.isCancelled(slot.request)).to.equal(true) - }) - - it("isSlotCancelled fails when slot is empty", async function () { - await expect( - marketplace.isSlotCancelled(slotId(slot)) - ).to.be.revertedWith("Slot empty") - }) - - it("isSlotCancelled is true once request is cancelled", async function () { - await marketplace.fillSlot(slot.request, slot.index, proof) - await waitUntilExpired(request.expiry) - await expect(await marketplace.isSlotCancelled(slotId(slot))).to.equal( - true - ) - }) - it("state is Cancelled when client withdraws funds", async function () { await expect(await marketplace.state(slot.request)).to.equal( RequestState.New diff --git a/test/Storage.test.js b/test/Storage.test.js index d4de434..8a36682 100644 --- a/test/Storage.test.js +++ b/test/Storage.test.js @@ -7,6 +7,7 @@ const { advanceTime, advanceTimeTo, currentTime } = require("./evm") const { requestId, slotId } = require("./ids") const { periodic } = require("./time") const { price } = require("./price") +const { waitUntilExpired } = require("./marketplace") describe("Storage", function () { const proof = hexlify(randomBytes(42)) @@ -121,6 +122,25 @@ describe("Storage", function () { ).to.be.revertedWith("Request was cancelled") }) }) + describe("contract state", function () { + it("isCancelled is true once request is cancelled", async function () { + await expect(await storage.isCancelled(slot.request)).to.equal(false) + await waitUntilExpired(request.expiry) + await expect(await storage.isCancelled(slot.request)).to.equal(true) + }) + + it("isSlotCancelled fails when slot is empty", async function () { + await expect(storage.isSlotCancelled(slotId(slot))).to.be.revertedWith( + "Slot empty" + ) + }) + + it("isSlotCancelled is true once request is cancelled", async function () { + await storage.fillSlot(slot.request, slot.index, proof) + await waitUntilExpired(request.expiry) + await expect(await storage.isSlotCancelled(slotId(slot))).to.equal(true) + }) + }) }) // TODO: implement checking of actual proofs of storage, instead of dummy bool