From 716b864f02febdfeac4a5076836e8dd5273285b8 Mon Sep 17 00:00:00 2001 From: Eric Mastro Date: Thu, 8 Sep 2022 17:56:01 +1000 Subject: [PATCH] [marketplace] update state getter Update `Marketplace.state` getter to to take `isCancelled` into account. This state can then be used internally and externally. Add checks to `proofEnd`, `isProofRequired`, `willProofBeRequired`, and `getChallenge` that understands if the request is in a state to accept proofs. If not, return other values. Add `slotMustAcceptProofs` modifier, originally introduced in a later PR, which requires the request to be in state of the request accepting proofs. Add tests for all the above. --- contracts/Marketplace.sol | 41 ++++++++++++++++- contracts/Storage.sol | 23 +++++----- contracts/TestMarketplace.sol | 28 ++++++++++++ test/Marketplace.test.js | 44 ++++++++++++++++-- test/Storage.test.js | 85 +++++++++++++++++++++++++++-------- 5 files changed, 189 insertions(+), 32 deletions(-) create mode 100644 contracts/TestMarketplace.sol diff --git a/contracts/Marketplace.sol b/contracts/Marketplace.sol index 1b5545c..a61fd42 100644 --- a/contracts/Marketplace.sol +++ b/contracts/Marketplace.sol @@ -169,6 +169,10 @@ contract Marketplace is Collateral, Proofs { return slot; } + function _context(bytes32 requestId) internal view returns (RequestContext storage) { + return requestContexts[requestId]; + } + function proofPeriod() public view returns (uint256) { return _period(); } @@ -178,6 +182,9 @@ contract Marketplace is Collateral, Proofs { } function proofEnd(bytes32 slotId) public view returns (uint256) { + if (!_slotAcceptsProofs(slotId)) { + return block.timestamp - 1; + } return _end(slotId); } @@ -202,7 +209,30 @@ contract Marketplace is Collateral, Proofs { } function state(bytes32 requestId) public view returns (RequestState) { - return requestContexts[requestId].state; + // TODO: add check for _isFinished + if (_isCancelled(requestId)) { + return RequestState.Cancelled; + } else { + RequestContext storage context = _context(requestId); + return context.state; + } + } + + + /// @notice returns true when the request 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 + function _requestAcceptsProofs(bytes32 requestId) internal view returns (bool) { + RequestState s = state(requestId); + return s == RequestState.New || s == RequestState.Started; + } + + /// @notice returns true when the request 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 slotId id of the slot, that is mapped to a request, for which to obtain state info + function _slotAcceptsProofs(bytes32 slotId) internal view returns (bool) { + bytes32 requestId = _getRequestIdForSlot(slotId); + return _requestAcceptsProofs(requestId); } struct Request { @@ -273,6 +303,15 @@ contract Marketplace is Collateral, Proofs { assert(funds.received == funds.balance + funds.sent); } + /// @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 slotId id of the slot, that is mapped to a request, for which to obtain state info + modifier slotMustAcceptProofs(bytes32 slotId) { + bytes32 requestId = _getRequestIdForSlot(slotId); + require(_requestAcceptsProofs(requestId), "Slot not accepting proofs"); + _; + } + struct MarketplaceFunds { uint256 balance; uint256 received; diff --git a/contracts/Storage.sol b/contracts/Storage.sol index c9f6f00..2c0ea79 100644 --- a/contracts/Storage.sol +++ b/contracts/Storage.sol @@ -40,14 +40,6 @@ contract Storage is Collateral, Marketplace { 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); } @@ -57,14 +49,23 @@ contract Storage is Collateral, Marketplace { } function isProofRequired(bytes32 slotId) public view returns (bool) { + if(!_slotAcceptsProofs(slotId)) { + return false; + } return _isProofRequired(slotId); } function willProofBeRequired(bytes32 slotId) public view returns (bool) { + if(!_slotAcceptsProofs(slotId)) { + return false; + } return _willProofBeRequired(slotId); } function getChallenge(bytes32 slotId) public view returns (bytes32) { + if(!_slotAcceptsProofs(slotId)) { + return bytes32(0); + } return _getChallenge(slotId); } @@ -76,8 +77,10 @@ contract Storage is Collateral, Marketplace { _submitProof(slotId, proof); } - function markProofAsMissing(bytes32 slotId, uint256 period) public { - require(!isSlotCancelled(slotId), "Request was cancelled"); + function markProofAsMissing(bytes32 slotId, uint256 period) + public + slotMustAcceptProofs(slotId) + { _markProofAsMissing(slotId, period); if (_missed(slotId) % slashMisses == 0) { _slash(_host(slotId), slashPercentage); diff --git a/contracts/TestMarketplace.sol b/contracts/TestMarketplace.sol new file mode 100644 index 0000000..c4a83eb --- /dev/null +++ b/contracts/TestMarketplace.sol @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import "./Marketplace.sol"; + +// exposes internal functions of Marketplace for testing +contract TestMarketplace is Marketplace { + constructor( + IERC20 _token, + uint256 _collateral, + uint256 _proofPeriod, + uint256 _proofTimeout, + uint8 _proofDowntime + ) + Marketplace(_token, _collateral, _proofPeriod,_proofTimeout,_proofDowntime) + // solhint-disable-next-line no-empty-blocks + { + + } + + function isCancelled(bytes32 requestId) public view returns (bool) { + return _isCancelled(requestId); + } + + function isSlotCancelled(bytes32 slotId) public view returns (bool) { + return _isSlotCancelled(slotId); + } +} \ No newline at end of file diff --git a/test/Marketplace.test.js b/test/Marketplace.test.js index 8191991..4c0af4c 100644 --- a/test/Marketplace.test.js +++ b/test/Marketplace.test.js @@ -11,6 +11,7 @@ const { revert, ensureMinimumBlockHeight, advanceTimeTo, + currentTime, } = require("./evm") describe("Marketplace", function () { @@ -38,7 +39,7 @@ describe("Marketplace", function () { await token.mint(account.address, 1_000_000_000) } - const Marketplace = await ethers.getContractFactory("Marketplace") + const Marketplace = await ethers.getContractFactory("TestMarketplace") marketplace = await Marketplace.deploy( token.address, collateral, @@ -338,7 +339,7 @@ describe("Marketplace", function () { await marketplace.deposit(collateral) }) - it("state is Cancelled when client withdraws funds", async function () { + it("changes state to Cancelled when client withdraws funds", async function () { await expect(await marketplace.state(slot.request)).to.equal( RequestState.New ) @@ -350,7 +351,7 @@ describe("Marketplace", function () { ) }) - it("state is Started once all slots are filled", async function () { + it("changes state to Started once all slots are filled", async function () { await expect(await marketplace.state(slot.request)).to.equal( RequestState.New ) @@ -363,5 +364,42 @@ describe("Marketplace", function () { RequestState.Started ) }) + + it("changes state to Cancelled once request is cancelled", async function () { + await waitUntilExpired(request.expiry) + await expect(await marketplace.state(slot.request)).to.equal( + RequestState.Cancelled + ) + }) + + it("changes isCancelled to true once request is cancelled", async function () { + await expect(await marketplace.isCancelled(slot.request)).to.be.false + await waitUntilExpired(request.expiry) + await expect(await marketplace.isCancelled(slot.request)).to.be.true + }) + + it("rejects isSlotCancelled when slot is empty", async function () { + await expect( + marketplace.isSlotCancelled(slotId(slot)) + ).to.be.revertedWith("Slot empty") + }) + + it("changes isSlotCancelled to true once request is cancelled", async function () { + await marketplace.fillSlot(slot.request, slot.index, proof) + await expect(await marketplace.isSlotCancelled(slotId(slot))).to.be.false + await waitUntilExpired(request.expiry) + await expect(await marketplace.isSlotCancelled(slotId(slot))).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( + await currentTime() + ) + await waitUntilExpired(request.expiry) + await expect(await marketplace.proofEnd(slotId(slot))).to.be.lt( + await currentTime() + ) + }) }) }) diff --git a/test/Storage.test.js b/test/Storage.test.js index 8a36682..e1ee4bb 100644 --- a/test/Storage.test.js +++ b/test/Storage.test.js @@ -1,9 +1,10 @@ const { expect } = require("chai") const { ethers, deployments } = require("hardhat") +const { BigNumber } = ethers const { hexlify, randomBytes } = ethers.utils const { AddressZero } = ethers.constants const { exampleRequest } = require("./examples") -const { advanceTime, advanceTimeTo, currentTime } = require("./evm") +const { advanceTime, advanceTimeTo, currentTime, mine } = require("./evm") const { requestId, slotId } = require("./ids") const { periodic } = require("./time") const { price } = require("./price") @@ -112,6 +113,33 @@ describe("Storage", function () { const expectedBalance = (collateralAmount * (100 - slashPercentage)) / 100 expect(await storage.balanceOf(host.address)).to.equal(expectedBalance) }) + }) + + describe("contract state", function () { + let period, periodOf, periodEnd + + beforeEach(async function () { + period = (await storage.proofPeriod()).toNumber() + ;({ periodOf, periodEnd } = periodic(period)) + }) + + async function waitUntilProofWillBeRequired(id) { + while (!(await storage.willProofBeRequired(id))) { + await mine() + } + } + + async function waitUntilProofIsRequired(id) { + await advanceTimeTo(periodEnd(periodOf(await currentTime()))) + while ( + !( + (await storage.isProofRequired(id)) && + (await storage.getPointer(id)) < 250 + ) + ) { + await advanceTime(period) + } + } it("fails to mark proof as missing when cancelled", async function () { await storage.fillSlot(slot.request, slot.index, proof) @@ -119,26 +147,47 @@ describe("Storage", function () { let missedPeriod = periodOf(await currentTime()) await expect( storage.markProofAsMissing(slotId(slot), missedPeriod) - ).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) + ).to.be.revertedWith("Slot not accepting proofs") }) - 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 () { + it("will not require proofs once cancelled", async function () { + const id = slotId(slot) await storage.fillSlot(slot.request, slot.index, proof) - await waitUntilExpired(request.expiry) - await expect(await storage.isSlotCancelled(slotId(slot))).to.equal(true) + await waitUntilProofWillBeRequired(id) + await expect(await storage.willProofBeRequired(id)).to.be.true + await advanceTimeTo(request.expiry + 1) + await expect(await storage.willProofBeRequired(id)).to.be.false + }) + + it("does not require proofs once cancelled", async function () { + const id = slotId(slot) + await storage.fillSlot(slot.request, slot.index, proof) + await waitUntilProofIsRequired(id) + await expect(await storage.isProofRequired(id)).to.be.true + await advanceTimeTo(request.expiry + 1) + await expect(await storage.isProofRequired(id)).to.be.false + }) + + it("does not provide challenges once cancelled", async function () { + const id = slotId(slot) + await storage.fillSlot(slot.request, slot.index, proof) + await waitUntilProofIsRequired(id) + const challenge1 = await storage.getChallenge(id) + expect(BigNumber.from(challenge1).gt(0)) + await advanceTimeTo(request.expiry + 1) + const challenge2 = await storage.getChallenge(id) + expect(BigNumber.from(challenge2).isZero()) + }) + + it("does not provide pointer once cancelled", async function () { + const id = slotId(slot) + await storage.fillSlot(slot.request, slot.index, proof) + await waitUntilProofIsRequired(id) + const challenge1 = await storage.getChallenge(id) + expect(BigNumber.from(challenge1).gt(0)) + await advanceTimeTo(request.expiry + 1) + const challenge2 = await storage.getChallenge(id) + expect(BigNumber.from(challenge2).isZero()) }) }) })