From f8e9f3c848d4229c09c490935f63a4da970397d7 Mon Sep 17 00:00:00 2001 From: Mark Spanbroek Date: Tue, 10 Jan 2023 12:11:20 +0100 Subject: [PATCH] [Proofs] Remove double administration of request end The timestamp when proofs no longer are required was stored in both the Marketplace and Proofs. It is now only stored in the Marketplace. --- contracts/Marketplace.sol | 8 ++---- contracts/Proofs.sol | 43 ++++------------------------ contracts/TestProofs.sol | 22 +++++++------- test/Proofs.test.js | 60 ++++++++++----------------------------- 4 files changed, 34 insertions(+), 99 deletions(-) diff --git a/contracts/Marketplace.sol b/contracts/Marketplace.sol index 2900fb2..659102d 100644 --- a/contracts/Marketplace.sol +++ b/contracts/Marketplace.sol @@ -71,7 +71,6 @@ contract Marketplace is Collateral, Proofs { RequestContext storage context = _context(id); // set contract end time to `duration` from now (time request was created) context.endsAt = block.timestamp + request.ask.duration; - _setProofEnd(id, context.endsAt); requestsPerClient[request.client].add(RequestId.unwrap(id)); @@ -97,7 +96,7 @@ contract Marketplace is Collateral, Proofs { require(balanceOf(msg.sender) >= collateral, "Insufficient collateral"); - _expectProofs(slotId, requestId, request.ask.proofProbability); + _expectProofs(slotId, request.ask.proofProbability); submitProof(slotId, proof); slot.host = msg.sender; @@ -179,7 +178,6 @@ contract Marketplace is Collateral, Proofs { context.state == RequestState.Started ) { context.state = RequestState.Failed; - _setProofEnd(requestId, block.timestamp - 1); context.endsAt = block.timestamp - 1; emit RequestFailed(requestId); @@ -322,12 +320,12 @@ contract Marketplace is Collateral, Proofs { return _timeout(); } - function proofEnd(SlotId slotId) public view returns (uint256) { + function proofEnd(SlotId slotId) public view override returns (uint256) { return requestEnd(_slot(slotId).requestId); } function requestEnd(RequestId requestId) public view returns (uint256) { - uint256 end = _end(requestId); + uint256 end = _context(requestId).endsAt; if (_requestAcceptsProofs(requestId)) { return end; } else { diff --git a/contracts/Proofs.sol b/contracts/Proofs.sol index 9b6f1be..fb5f3a7 100644 --- a/contracts/Proofs.sol +++ b/contracts/Proofs.sol @@ -3,7 +3,7 @@ pragma solidity ^0.8.8; import "./Requests.sol"; -contract Proofs { +abstract contract Proofs { uint256 private immutable period; uint256 private immutable timeout; uint8 private immutable downtime; @@ -17,8 +17,6 @@ contract Proofs { mapping(SlotId => bool) private slotIds; mapping(SlotId => uint256) private starts; - mapping(RequestId => uint256) private ends; - mapping(SlotId => RequestId) private requestForSlot; mapping(SlotId => uint256) private probabilities; mapping(SlotId => uint256) private markers; mapping(SlotId => uint256) private missed; @@ -33,21 +31,9 @@ contract Proofs { return timeout; } - function _end(RequestId requestId) internal view returns (uint256) { - uint256 end = ends[requestId]; - require(end > 0, "Proof ending doesn't exist"); - return end; - } - - function _requestId(SlotId id) internal view returns (RequestId) { - RequestId requestId = requestForSlot[id]; - require(RequestId.unwrap(requestId) > 0, "request for slot doesn't exist"); - return requestId; - } - - function _end(SlotId id) internal view returns (uint256) { - return _end(_requestId(id)); - } + // 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 missingProofs(SlotId slotId) public view returns (uint256) { return missed[slotId]; @@ -63,19 +49,13 @@ contract Proofs { /// @notice Informs the contract that proofs should be expected for id /// @dev Requires that the id is not already in use - /// @param id identifies the proof expectation, typically a slot id /// @param probability The probability that a proof should be expected - function _expectProofs( - SlotId id, // typically slot id - RequestId request, // typically request id, used so that the ending is global for all slots - uint256 probability - ) internal { + function _expectProofs(SlotId id, uint256 probability) internal { require(!slotIds[id], "Slot id already in use"); slotIds[id] = true; starts[id] = block.timestamp; probabilities[id] = probability; markers[id] = uint256(blockhash(block.number - 1)) % period; - requestForSlot[id] = request; } function _unexpectProofs(SlotId id) internal { @@ -122,7 +102,7 @@ contract Proofs { if (proofPeriod <= periodOf(starts[id])) { return (false, 0); } - uint256 end = _end(id); + uint256 end = proofEnd(id); if (proofPeriod >= periodOf(end)) { return (false, 0); } @@ -171,16 +151,5 @@ contract Proofs { missed[id] += 1; } - /// @notice Sets the proof end time - /// @dev Can only be set once - /// @param ending the new end time (in seconds) - function _setProofEnd(RequestId request, uint256 ending) internal { - require( - ends[request] == 0 || ending < block.timestamp, - "End exists or must be past" - ); - ends[request] = ending; - } - event ProofSubmitted(SlotId id, bytes proof); } diff --git a/contracts/TestProofs.sol b/contracts/TestProofs.sol index 8ef95a6..634d3b6 100644 --- a/contracts/TestProofs.sol +++ b/contracts/TestProofs.sol @@ -5,6 +5,8 @@ import "./Proofs.sol"; // exposes internal functions of Proofs for testing contract TestProofs is Proofs { + mapping(SlotId => uint256) private ends; + constructor( uint256 __period, uint256 __timeout, @@ -16,6 +18,10 @@ contract TestProofs is Proofs { } + function proofEnd(SlotId slotId) public view override returns (uint256) { + return ends[slotId]; + } + function period() public view returns (uint256) { return _period(); } @@ -24,16 +30,8 @@ contract TestProofs is Proofs { return _timeout(); } - function end(RequestId id) public view returns (uint256) { - return _end(id); - } - - function expectProofs( - SlotId slot, - RequestId request, - uint256 _probability - ) public { - _expectProofs(slot, request, _probability); + function expectProofs(SlotId slot, uint256 _probability) public { + _expectProofs(slot, _probability); } function unexpectProofs(SlotId id) public { @@ -60,7 +58,7 @@ contract TestProofs is Proofs { _markProofAsMissing(id, _period); } - function setProofEnd(RequestId id, uint256 ending) public { - _setProofEnd(id, ending); + function setProofEnd(SlotId id, uint256 end) public { + ends[id] = end; } } diff --git a/test/Proofs.test.js b/test/Proofs.test.js index d2df518..3fda82d 100644 --- a/test/Proofs.test.js +++ b/test/Proofs.test.js @@ -37,18 +37,18 @@ describe("Proofs", function () { describe("general", function () { beforeEach(async function () { - await proofs.setProofEnd(requestId, (await currentTime()) + duration) + await proofs.setProofEnd(slotId, (await currentTime()) + duration) }) it("does not allow ids to be reused", async function () { - await proofs.expectProofs(slotId, requestId, probability) - await expect( - proofs.expectProofs(slotId, requestId, probability) - ).to.be.revertedWith("Slot id already in use") + await proofs.expectProofs(slotId, probability) + await expect(proofs.expectProofs(slotId, probability)).to.be.revertedWith( + "Slot id already in use" + ) }) it("requires proofs with an agreed upon probability", async function () { - await proofs.expectProofs(slotId, requestId, probability) + await proofs.expectProofs(slotId, probability) let amount = 0 for (let i = 0; i < 100; i++) { if (await proofs.isProofRequired(slotId)) { @@ -63,7 +63,7 @@ describe("Proofs", function () { it("requires no proofs in the start period", async function () { const startPeriod = Math.floor((await currentTime()) / period) const probability = 1 - await proofs.expectProofs(slotId, requestId, probability) + await proofs.expectProofs(slotId, probability) while (Math.floor((await currentTime()) / period) == startPeriod) { expect(await proofs.isProofRequired(slotId)).to.be.false await advanceTime(Math.floor(period / 10)) @@ -72,14 +72,14 @@ describe("Proofs", function () { it("requires no proofs in the end period", async function () { const probability = 1 - await proofs.expectProofs(slotId, requestId, probability) + await proofs.expectProofs(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.expectProofs(slotId, requestId, probability) + await proofs.expectProofs(slotId, probability) await advanceTime(duration + timeout) expect(await proofs.isProofRequired(slotId)).to.be.false }) @@ -89,7 +89,8 @@ describe("Proofs", function () { let id2 = hexlify(randomBytes(32)) let id3 = hexlify(randomBytes(32)) for (let slotId of [id1, id2, id3]) { - await proofs.expectProofs(slotId, requestId, probability) + await proofs.setProofEnd(slotId, (await currentTime()) + duration) + await proofs.expectProofs(slotId, probability) } let req1, req2, req3 while (req1 === req2 && req2 === req3) { @@ -119,8 +120,8 @@ describe("Proofs", function () { } beforeEach(async function () { - await proofs.setProofEnd(requestId, (await currentTime()) + duration) - await proofs.expectProofs(slotId, requestId, probability) + await proofs.setProofEnd(slotId, (await currentTime()) + duration) + await proofs.expectProofs(slotId, probability) await advanceTimeTo(periodEnd(periodOf(await currentTime()))) await waitUntilProofWillBeRequired() }) @@ -153,8 +154,8 @@ describe("Proofs", function () { const proof = hexlify(randomBytes(42)) beforeEach(async function () { - await proofs.setProofEnd(requestId, (await currentTime()) + duration) - await proofs.expectProofs(slotId, requestId, probability) + await proofs.setProofEnd(slotId, (await currentTime()) + duration) + await proofs.expectProofs(slotId, probability) }) async function waitUntilProofIsRequired(slotId) { @@ -273,35 +274,4 @@ describe("Proofs", function () { await expect(await proofs.isProofRequired(slotId)).to.be.false }) }) - - describe("set proof end", function () { - const proof = hexlify(randomBytes(42)) - - it("fails to get proof end when proof ending doesn't exist", async function () { - await expect(proofs.end(requestId)).to.be.revertedWith( - "Proof ending doesn't exist" - ) - }) - - it("sets proof end when proof ending doesn't already exist", async function () { - let ending = (await currentTime()) + duration - await expect(proofs.setProofEnd(requestId, ending)).not.to.be.reverted - await expect(await proofs.end(requestId)).to.equal(ending) - }) - - it("sets proof end when proof ending exists and ending set to past", async function () { - // let ending = (await currentTime()) + duration - // await proofs.setProofEnd(requestId, ending) - let past = (await currentTime()) - 1 - await expect(proofs.setProofEnd(requestId, past)).not.to.be.reverted - }) - - it("fails when proof ending already exists and ending set to future", async function () { - let ending = (await currentTime()) + duration - await proofs.setProofEnd(requestId, ending) - await expect(proofs.setProofEnd(requestId, ending)).to.be.revertedWith( - "End exists or must be past" - ) - }) - }) })