From c8dda373006673ea17f1e1e5c8ebdceb72834b61 Mon Sep 17 00:00:00 2001 From: Mark Spanbroek Date: Mon, 16 Jan 2023 16:31:04 +0100 Subject: [PATCH] [marketplace] introduce SlotState - replace proofEnd() by slotState() - replace _slotAcceptsProofs() by slotState() - remove _stopRequiringProofs() --- contracts/Marketplace.sol | 42 +++++++++------- contracts/Proofs.sol | 18 ++----- contracts/Requests.sol | 6 +++ contracts/TestProofs.sol | 14 ++---- test/Marketplace.test.js | 101 +++----------------------------------- test/Proofs.test.js | 40 ++++----------- test/marketplace.js | 9 ---- test/requests.js | 15 ++++++ 8 files changed, 71 insertions(+), 174 deletions(-) create mode 100644 test/requests.js diff --git a/contracts/Marketplace.sol b/contracts/Marketplace.sol index c89b605..b078886 100644 --- a/contracts/Marketplace.sol +++ b/contracts/Marketplace.sol @@ -80,7 +80,9 @@ contract Marketplace is Collateral, Proofs, StateRetrieval { SlotId slotId = Requests.slotId(requestId, slotIndex); Slot storage slot = slots[slotId]; - require(slot.host == address(0), "Slot already filled"); + slot.requestId = requestId; + + require(slotState(slotId) == SlotState.Free, "Slot already filled"); require(balanceOf(msg.sender) >= collateral, "Insufficient collateral"); @@ -88,7 +90,7 @@ contract Marketplace is Collateral, Proofs, StateRetrieval { submitProof(slotId, proof); slot.host = msg.sender; - slot.requestId = requestId; + slot.state = SlotState.Filled; RequestContext storage context = _context(requestId); context.slotsFilled += 1; @@ -150,10 +152,9 @@ contract Marketplace is Collateral, Proofs, StateRetrieval { // Slot collateral is not yet implemented as the design decision was // not finalised. - _stopRequiringProofs(slotId); - removeFromMySlots(slot.host, slotId); + slot.state = SlotState.Free; slot.host = address(0); slot.requestId = RequestId.wrap(0); context.slotsFilled -= 1; @@ -258,7 +259,7 @@ contract Marketplace is Collateral, Proofs, StateRetrieval { function _slot(SlotId slotId) internal view returns (Slot storage) { Slot storage slot = slots[slotId]; - require(slot.host != address(0), "Slot empty"); + require(slot.state == SlotState.Filled, "Slot empty"); return slot; } @@ -272,10 +273,6 @@ contract Marketplace is Collateral, Proofs, StateRetrieval { return secondsPerPeriod; } - function proofEnd(SlotId slotId) public view override returns (uint256) { - return requestEnd(_slot(slotId).requestId); - } - function requestEnd(RequestId requestId) public view returns (uint256) { uint256 end = _context(requestId).endsAt; if (_requestAcceptsProofs(requestId)) { @@ -286,21 +283,21 @@ contract Marketplace is Collateral, Proofs, StateRetrieval { } function isProofRequired(SlotId slotId) public view returns (bool) { - if (!_slotAcceptsProofs(slotId)) { + if (slotState(slotId) != SlotState.Filled) { return false; } return _isProofRequired(slotId); } function willProofBeRequired(SlotId slotId) public view returns (bool) { - if (!_slotAcceptsProofs(slotId)) { + if (slotState(slotId) != SlotState.Filled) { return false; } return _willProofBeRequired(slotId); } function getChallenge(SlotId slotId) public view returns (bytes32) { - if (!_slotAcceptsProofs(slotId)) { + if (slotState(slotId) != SlotState.Filled) { return bytes32(0); } return _getChallenge(slotId); @@ -348,12 +345,18 @@ contract Marketplace is Collateral, Proofs, StateRetrieval { } } - /// @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(SlotId slotId) internal view returns (bool) { - RequestId requestId = _getRequestIdForSlot(slotId); - return _requestAcceptsProofs(requestId); + function slotState(SlotId slotId) internal view override returns (SlotState) { + Slot storage slot = slots[slotId]; + RequestState reqState = requestState(slot.requestId); + if ( + reqState == RequestState.Finished || + reqState == RequestState.Cancelled || + reqState == RequestState.Failed + ) { + return SlotState.Finished; + } else { + return slot.state; + } } /// @notice returns true when the request is accepting proof submissions from hosts occupying slots. @@ -367,13 +370,14 @@ contract Marketplace is Collateral, Proofs, StateRetrieval { } struct RequestContext { - uint256 slotsFilled; RequestState state; + uint256 slotsFilled; uint256 startedAt; uint256 endsAt; } struct Slot { + SlotState state; address host; bool hostPaid; RequestId requestId; diff --git a/contracts/Proofs.sol b/contracts/Proofs.sol index 1f9b4bf..ac25f96 100644 --- a/contracts/Proofs.sol +++ b/contracts/Proofs.sol @@ -18,16 +18,13 @@ abstract contract Proofs is Periods { downtime = __downtime; } - mapping(SlotId => bool) private slotIds; mapping(SlotId => uint256) private slotStarts; mapping(SlotId => uint256) private probabilities; mapping(SlotId => uint256) private missed; mapping(SlotId => mapping(Period => bool)) private received; mapping(SlotId => mapping(Period => bool)) private missing; - // Override this to let the proving system know when proofs for a - // slot are no longer required. - function proofEnd(SlotId id) public view virtual returns (uint256); + function slotState(SlotId id) internal view virtual returns (SlotState); function missingProofs(SlotId slotId) public view returns (uint256) { return missed[slotId]; @@ -37,17 +34,10 @@ abstract contract Proofs is Periods { /// @dev Requires that the id is not already in use /// @param probability The probability that a proof should be expected function _startRequiringProofs(SlotId id, uint256 probability) internal { - require(!slotIds[id], "Proofs already required for slot"); - slotIds[id] = true; slotStarts[id] = block.timestamp; probabilities[id] = probability; } - function _stopRequiringProofs(SlotId id) internal { - require(slotIds[id], "Proofs not required for slot"); - slotIds[id] = false; - } - function _getPointer( SlotId id, Period proofPeriod @@ -84,15 +74,15 @@ abstract contract Proofs is Periods { SlotId id, Period proofPeriod ) internal view returns (bool isRequired, uint8 pointer) { + SlotState state = slotState(id); Period start = periodOf(slotStarts[id]); - Period end = periodOf(proofEnd(id)); - if (!isAfter(proofPeriod, start) || !isBefore(proofPeriod, end)) { + if (state != SlotState.Filled || !isAfter(proofPeriod, start)) { return (false, 0); } pointer = _getPointer(id, proofPeriod); bytes32 challenge = _getChallenge(pointer); uint256 probability = (probabilities[id] * (256 - downtime)) / 256; - isRequired = slotIds[id] && uint256(challenge) % probability == 0; + isRequired = uint256(challenge) % probability == 0; } function _isProofRequired( diff --git a/contracts/Requests.sol b/contracts/Requests.sol index a648bdf..43ff627 100644 --- a/contracts/Requests.sol +++ b/contracts/Requests.sol @@ -45,6 +45,12 @@ enum RequestState { Failed // too many nodes have failed to provide proofs, data lost } +enum SlotState { + Free, // [default] not filled yet, or host has freed slot + Filled, // host has filled slot + Finished // successfully or unsuccessfully completed +} + library Requests { function id(Request memory request) internal pure returns (RequestId) { return RequestId.wrap(keccak256(abi.encode(request))); diff --git a/contracts/TestProofs.sol b/contracts/TestProofs.sol index f1a43b4..356073b 100644 --- a/contracts/TestProofs.sol +++ b/contracts/TestProofs.sol @@ -5,7 +5,7 @@ import "./Proofs.sol"; // exposes internal functions of Proofs for testing contract TestProofs is Proofs { - mapping(SlotId => uint256) private ends; + mapping(SlotId => SlotState) private states; constructor( uint256 __period, @@ -18,18 +18,14 @@ contract TestProofs is Proofs { } - function proofEnd(SlotId slotId) public view override returns (uint256) { - return ends[slotId]; + function slotState(SlotId slotId) internal view override returns (SlotState) { + return states[slotId]; } function startRequiringProofs(SlotId slot, uint256 _probability) public { _startRequiringProofs(slot, _probability); } - function stopRequiringProofs(SlotId id) public { - _stopRequiringProofs(id); - } - function isProofRequired(SlotId id) public view returns (bool) { return _isProofRequired(id); } @@ -50,7 +46,7 @@ contract TestProofs is Proofs { _markProofAsMissing(id, _period); } - function setProofEnd(SlotId id, uint256 end) public { - ends[id] = end; + function setSlotState(SlotId id, SlotState state) public { + states[id] = state; } } diff --git a/test/Marketplace.test.js b/test/Marketplace.test.js index 0152ed9..f863cfa 100644 --- a/test/Marketplace.test.js +++ b/test/Marketplace.test.js @@ -6,13 +6,13 @@ const { expect } = require("chai") const { exampleRequest } = require("./examples") const { periodic, hours, minutes } = require("./time") const { requestId, slotId, askToArray } = require("./ids") +const { RequestState } = require("./requests") const { waitUntilCancelled, waitUntilStarted, waitUntilFinished, waitUntilFailed, waitUntilSlotFailed, - RequestState, } = require("./marketplace") const { price, pricePerSlot } = require("./price") const { @@ -152,11 +152,6 @@ describe("Marketplace", function () { expect(await marketplace.getHost(slotId(slot))).to.equal(host.address) }) - it("starts requiring storage proofs", async function () { - await marketplace.fillSlot(slot.request, slot.index, proof) - expect(await marketplace.proofEnd(slotId(slot))).to.be.gt(0) - }) - it("is rejected when proof is incorrect", async function () { let invalid = hexlify([]) await expect( @@ -233,66 +228,6 @@ describe("Marketplace", function () { }) }) - describe("proof end", function () { - var requestTime - beforeEach(async function () { - switchAccount(client) - await token.approve(marketplace.address, price(request)) - await marketplace.requestStorage(request) - requestTime = await currentTime() - switchAccount(host) - await token.approve(marketplace.address, collateral) - await marketplace.deposit(collateral) - }) - - it("shares proof end time for all slots in request", async function () { - const lastSlot = request.ask.slots - 1 - for (let i = 0; i < lastSlot; i++) { - await marketplace.fillSlot(slot.request, i, proof) - } - advanceTime(minutes(10)) - await marketplace.fillSlot(slot.request, lastSlot, proof) - let slot0 = { ...slot, index: 0 } - let end = await marketplace.proofEnd(slotId(slot0)) - for (let i = 1; i <= lastSlot; i++) { - let sloti = { ...slot, index: i } - await expect((await marketplace.proofEnd(slotId(sloti))) === end) - } - }) - - it("sets the proof end time to now + duration", async function () { - await marketplace.fillSlot(slot.request, slot.index, proof) - await expect( - (await marketplace.proofEnd(slotId(slot))).toNumber() - ).to.be.closeTo(requestTime + request.ask.duration, 1) - }) - - it("sets proof end time to the past once failed", async function () { - await waitUntilStarted(marketplace, request, proof) - await waitUntilFailed(marketplace, request) - let slot0 = { ...slot, index: request.ask.maxSlotLoss + 1 } - const now = await currentTime() - await expect(await marketplace.proofEnd(slotId(slot0))).to.be.eq(now - 1) - }) - - it("sets proof end time to the past once cancelled", async function () { - await marketplace.fillSlot(slot.request, slot.index, proof) - await waitUntilCancelled(request) - const now = await currentTime() - await expect(await marketplace.proofEnd(slotId(slot))).to.be.eq(now - 1) - }) - - it("checks that proof end time is in the past once finished", async function () { - await waitUntilStarted(marketplace, request, proof) - await waitUntilFinished(marketplace, requestId(request)) - const now = await currentTime() - // in the process of calling currentTime and proofEnd, - // block.timestamp has advanced by 1, so the expected proof end time will - // be block.timestamp - 1. - await expect(await marketplace.proofEnd(slotId(slot))).to.be.eq(now - 1) - }) - }) - describe("request end", function () { var requestTime beforeEach(async function () { @@ -305,21 +240,6 @@ describe("Marketplace", function () { await marketplace.deposit(collateral) }) - it("shares request end time for all slots in request", async function () { - const lastSlot = request.ask.slots - 1 - for (let i = 0; i < lastSlot; i++) { - await marketplace.fillSlot(slot.request, i, proof) - } - advanceTime(minutes(10)) - await marketplace.fillSlot(slot.request, lastSlot, proof) - let slot0 = { ...slot, index: 0 } - let end = await marketplace.requestEnd(requestId(request)) - for (let i = 1; i <= lastSlot; i++) { - let sloti = { ...slot, index: i } - await expect((await marketplace.proofEnd(slotId(sloti))) === end) - } - }) - it("sets the request end time to now + duration", async function () { await marketplace.fillSlot(slot.request, slot.index, proof) await expect( @@ -350,7 +270,7 @@ describe("Marketplace", function () { await waitUntilStarted(marketplace, request, proof) await waitUntilFinished(marketplace, requestId(request)) const now = await currentTime() - // in the process of calling currentTime and proofEnd, + // in the process of calling currentTime and requestEnd, // block.timestamp has advanced by 1, so the expected proof end time will // be block.timestamp - 1. await expect(await marketplace.requestEnd(requestId(request))).to.be.eq( @@ -652,17 +572,6 @@ describe("Marketplace", function () { RequestState.Cancelled ) }) - - 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 waitUntilCancelled(request) - await expect(await marketplace.proofEnd(slotId(slot))).to.be.lt( - await currentTime() - ) - }) }) describe("proof requirements", function () { @@ -698,6 +607,12 @@ describe("Marketplace", function () { } } + it("requires proofs when slot is filled", async function () { + const id = slotId(slot) + await marketplace.fillSlot(slot.request, slot.index, proof) + await waitUntilProofWillBeRequired(id) + }) + it("will not require proofs once cancelled", async function () { const id = slotId(slot) await marketplace.fillSlot(slot.request, slot.index, proof) diff --git a/test/Proofs.test.js b/test/Proofs.test.js index 727b7b0..444923a 100644 --- a/test/Proofs.test.js +++ b/test/Proofs.test.js @@ -11,6 +11,7 @@ const { advanceTimeTo, } = require("./evm") const { periodic } = require("./time") +const { SlotState } = require("./requests") describe("Proofs", function () { const slotId = hexlify(randomBytes(32)) @@ -37,14 +38,7 @@ describe("Proofs", function () { describe("general", function () { beforeEach(async function () { - await proofs.setProofEnd(slotId, (await currentTime()) + duration) - }) - - it("does not allow ids to be reused", async function () { - await proofs.startRequiringProofs(slotId, probability) - await expect( - proofs.startRequiringProofs(slotId, probability) - ).to.be.revertedWith("Proofs already required for slot") + await proofs.setSlotState(slotId, SlotState.Filled) }) it("requires proofs with an agreed upon probability", async function () { @@ -70,26 +64,12 @@ describe("Proofs", function () { } }) - it("requires no proofs in the end period", async function () { - const probability = 1 - await proofs.startRequiringProofs(slotId, probability) - await advanceTime(duration) - expect(await proofs.isProofRequired(slotId)).to.be.false - }) - - it("requires no proofs after the end time", async function () { - const probability = 1 - await proofs.startRequiringProofs(slotId, probability) - await advanceTime(duration + timeout) - expect(await proofs.isProofRequired(slotId)).to.be.false - }) - it("requires proofs for different ids at different times", async function () { let id1 = hexlify(randomBytes(32)) let id2 = hexlify(randomBytes(32)) let id3 = hexlify(randomBytes(32)) for (let slotId of [id1, id2, id3]) { - await proofs.setProofEnd(slotId, (await currentTime()) + duration) + await proofs.setSlotState(slotId, SlotState.Filled) await proofs.startRequiringProofs(slotId, probability) } let req1, req2, req3 @@ -120,7 +100,7 @@ describe("Proofs", function () { } beforeEach(async function () { - await proofs.setProofEnd(slotId, (await currentTime()) + duration) + await proofs.setSlotState(slotId, SlotState.Filled) await proofs.startRequiringProofs(slotId, probability) await advanceTimeTo(periodEnd(periodOf(await currentTime()))) await waitUntilProofWillBeRequired() @@ -142,10 +122,10 @@ describe("Proofs", function () { expect(await proofs.isProofRequired(slotId)).to.be.true }) - it("will not require proofs when no longer expected", async function () { + it("will not require proofs when slot is finished", async function () { expect(await proofs.getPointer(slotId)).to.be.lt(downtime) expect(await proofs.willProofBeRequired(slotId)).to.be.true - await proofs.stopRequiringProofs(slotId) + await proofs.setSlotState(slotId, SlotState.Finished) expect(await proofs.willProofBeRequired(slotId)).to.be.false }) }) @@ -154,7 +134,7 @@ describe("Proofs", function () { const proof = hexlify(randomBytes(42)) beforeEach(async function () { - await proofs.setProofEnd(slotId, (await currentTime()) + duration) + await proofs.setSlotState(slotId, SlotState.Filled) await proofs.startRequiringProofs(slotId, probability) }) @@ -268,10 +248,10 @@ describe("Proofs", function () { ).to.be.revertedWith("Proof already marked as missing") }) - it("requires no proofs when no longer expected", async function () { + it("requires no proofs when slot is finished", async function () { await waitUntilProofIsRequired(slotId) - await proofs.stopRequiringProofs(slotId) - await expect(await proofs.isProofRequired(slotId)).to.be.false + await proofs.setSlotState(slotId, SlotState.Finished) + expect(await proofs.isProofRequired(slotId)).to.be.false }) }) }) diff --git a/test/marketplace.js b/test/marketplace.js index 0be9def..245de35 100644 --- a/test/marketplace.js +++ b/test/marketplace.js @@ -37,19 +37,10 @@ async function waitUntilSlotFailed(contract, request, slot) { } } -const RequestState = { - New: 0, - Started: 1, - Cancelled: 2, - Finished: 3, - Failed: 4, -} - module.exports = { waitUntilCancelled, waitUntilStarted, waitUntilFinished, waitUntilFailed, waitUntilSlotFailed, - RequestState, } diff --git a/test/requests.js b/test/requests.js new file mode 100644 index 0000000..2a5ade8 --- /dev/null +++ b/test/requests.js @@ -0,0 +1,15 @@ +const RequestState = { + New: 0, + Started: 1, + Cancelled: 2, + Finished: 3, + Failed: 4, +} + +const SlotState = { + Free: 0, + Filled: 1, + Finished: 2, +} + +module.exports = { RequestState, SlotState }