From 57e8cd5013325f05e16833a5320b575d32a403f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Uhl=C3=AD=C5=99?= Date: Mon, 6 May 2024 15:13:32 +0200 Subject: [PATCH] feat: expiry specified as duration (#99) --- contracts/Marketplace.sol | 32 +++++++++++++++++++------- contracts/Requests.sol | 2 +- test/Marketplace.test.js | 48 ++++++++++++++++++++++++--------------- test/examples.js | 4 +--- test/marketplace.js | 11 ++++++++- test/requests.js | 7 +++++- 6 files changed, 72 insertions(+), 32 deletions(-) diff --git a/contracts/Marketplace.sol b/contracts/Marketplace.sol index e2f4c5d..e0f3f8d 100644 --- a/contracts/Marketplace.sol +++ b/contracts/Marketplace.sol @@ -32,6 +32,7 @@ contract Marketplace is Proofs, StateRetrieval, Endian { uint256 expiryFundsWithdraw; uint256 startedAt; uint256 endsAt; + uint256 expiresAt; } struct Slot { @@ -90,11 +91,14 @@ contract Marketplace is Proofs, StateRetrieval, Endian { RequestId id = request.id(); require(_requests[id].client == address(0), "Request already exists"); + require( + request.expiry > 0 && request.expiry < request.ask.duration, + "Expiry not in range" + ); _requests[id] = request; - uint256 endsAt = block.timestamp + request.ask.duration; - require(endsAt > request.expiry, "Request end before expiry"); - _requestContexts[id].endsAt = endsAt; + _requestContexts[id].endsAt = block.timestamp + request.ask.duration; + _requestContexts[id].expiresAt = block.timestamp + request.expiry; _addToMyRequests(request.client, id); @@ -103,7 +107,7 @@ contract Marketplace is Proofs, StateRetrieval, Endian { _marketplaceTotals.received += amount; _transferFrom(msg.sender, amount); - emit StorageRequested(id, request.ask, request.expiry); + emit StorageRequested(id, request.ask, _requestContexts[id].expiresAt); } function fillSlot( @@ -206,6 +210,8 @@ contract Marketplace is Proofs, StateRetrieval, Endian { Slot storage slot = _slots[slotId]; Request storage request = _requests[slot.requestId]; + // TODO: Reward for validator that calls this function + if (missingProofs(slotId) % _config.collateral.slashCriterion == 0) { uint256 slashedAmount = (request.ask.collateral * _config.collateral.slashPercentage) / 100; @@ -286,7 +292,10 @@ contract Marketplace is Proofs, StateRetrieval, Endian { /// @param requestId the id of the request function withdrawFunds(RequestId requestId) public { Request storage request = _requests[requestId]; - require(block.timestamp > request.expiry, "Request not yet timed out"); + require( + block.timestamp > requestExpiry(requestId), + "Request not yet timed out" + ); require(request.client == msg.sender, "Invalid client address"); RequestContext storage context = _requestContexts[requestId]; require(context.state == RequestState.New, "Invalid state"); @@ -339,15 +348,22 @@ contract Marketplace is Proofs, StateRetrieval, Endian { } } + function requestExpiry(RequestId requestId) public view returns (uint256) { + return _requestContexts[requestId].expiresAt; + } + /// @notice Calculates the amount that should be payed out to a host if a request expires based on when the host fills the slot function _expiryPayoutAmount( RequestId requestId, uint256 startingTimestamp ) private view returns (uint256) { Request storage request = _requests[requestId]; - require(startingTimestamp < request.expiry, "Start not before expiry"); + require( + startingTimestamp < requestExpiry(requestId), + "Start not before expiry" + ); - return (request.expiry - startingTimestamp) * request.ask.reward; + return (requestExpiry(requestId) - startingTimestamp) * request.ask.reward; } function getHost(SlotId slotId) public view returns (address) { @@ -360,7 +376,7 @@ contract Marketplace is Proofs, StateRetrieval, Endian { RequestContext storage context = _requestContexts[requestId]; if ( context.state == RequestState.New && - block.timestamp > _requests[requestId].expiry + block.timestamp > requestExpiry(requestId) ) { return RequestState.Cancelled; } else if ( diff --git a/contracts/Requests.sol b/contracts/Requests.sol index 090dd17..1771719 100644 --- a/contracts/Requests.sol +++ b/contracts/Requests.sol @@ -8,7 +8,7 @@ struct Request { address client; Ask ask; Content content; - uint256 expiry; // timestamp as seconds since unix epoch at which this request expires + uint256 expiry; // amount of seconds since start of the request at which this request expires bytes32 nonce; // random nonce to differentiate between similar requests } diff --git a/test/Marketplace.test.js b/test/Marketplace.test.js index 94fe6e9..38e75f6 100644 --- a/test/Marketplace.test.js +++ b/test/Marketplace.test.js @@ -140,9 +140,12 @@ describe("Marketplace", function () { it("emits event when storage is requested", async function () { await token.approve(marketplace.address, price(request)) + + // We +1 second to the expiry because the time will advance with the mined transaction for requestStorage because of Hardhat + const expectedExpiry = (await currentTime()) + request.expiry + 1 await expect(marketplace.requestStorage(request)) .to.emit(marketplace, "StorageRequested") - .withArgs(requestId(request), askToArray(request.ask), request.expiry) + .withArgs(requestId(request), askToArray(request.ask), expectedExpiry) }) it("allows retrieval of request details", async function () { @@ -168,11 +171,17 @@ describe("Marketplace", function () { ) }) - it("rejects request when expiry is after request end", async function () { - request.expiry = (await currentTime()) + request.ask.duration + hours(1) + it("rejects request when expiry out of bounds", async function () { await token.approve(marketplace.address, price(request)) + + request.expiry = request.ask.duration + 1 await expect(marketplace.requestStorage(request)).to.be.revertedWith( - "Request end before expiry" + "Expiry not in range" + ) + + request.expiry = 0 + await expect(marketplace.requestStorage(request)).to.be.revertedWith( + "Expiry not in range" ) }) @@ -241,9 +250,10 @@ describe("Marketplace", function () { it("is rejected when request is cancelled", async function () { switchAccount(client) - let expired = { ...request, expiry: (await currentTime()) - hours(1) } + let expired = { ...request, expiry: hours(1) + 1 } await token.approve(marketplace.address, price(request)) await marketplace.requestStorage(expired) + await waitUntilCancelled(expired) switchAccount(host) await expect( marketplace.fillSlot(requestId(expired), slot.index, proof) @@ -465,20 +475,19 @@ describe("Marketplace", function () { }) it("pays the host when contract was cancelled", async function () { - // Lets move the time into middle of the expiry window - const fillTimestamp = - (await currentTime()) + - Math.floor((request.expiry - (await currentTime())) / 2) - - 1 - await advanceTimeToForNextBlock(fillTimestamp) + // Lets advance the time more into the expiry window + const filledAt = (await currentTime()) + Math.floor(request.expiry / 3) + const expiresAt = ( + await marketplace.requestExpiry(requestId(request)) + ).toNumber() + await advanceTimeToForNextBlock(filledAt) await marketplace.fillSlot(slot.request, slot.index, proof) await waitUntilCancelled(request) await marketplace.freeSlot(slotId(slot)) + const expectedPartialPayout = (expiresAt - filledAt) * request.ask.reward const endBalance = await token.balanceOf(host.address) - const expectedPartialPayout = - (request.expiry - fillTimestamp) * request.ask.reward expect(endBalance - ACCOUNT_STARTING_BALANCE).to.be.equal( expectedPartialPayout ) @@ -617,14 +626,17 @@ describe("Marketplace", function () { }) it("withdraws to the client for cancelled requests lowered by hosts payout", async function () { - const fillTimestamp = - (await currentTime()) + - Math.floor((request.expiry - (await currentTime())) / 2) - await advanceTimeToForNextBlock(fillTimestamp) + // Lets advance the time more into the expiry window + const filledAt = (await currentTime()) + Math.floor(request.expiry / 3) + const expiresAt = ( + await marketplace.requestExpiry(requestId(request)) + ).toNumber() + + await advanceTimeToForNextBlock(filledAt) await marketplace.fillSlot(slot.request, slot.index, proof) await waitUntilCancelled(request) const expectedPartialHostPayout = - (request.expiry - fillTimestamp) * request.ask.reward + (expiresAt - filledAt) * request.ask.reward switchAccount(client) await marketplace.withdrawFunds(slot.request) diff --git a/test/examples.js b/test/examples.js index 895f429..ab14677 100644 --- a/test/examples.js +++ b/test/examples.js @@ -1,6 +1,5 @@ const { ethers } = require("hardhat") const { hours } = require("./time") -const { currentTime } = require("./evm") const { hexlify, randomBytes } = ethers.utils const exampleConfiguration = () => ({ @@ -19,7 +18,6 @@ const exampleConfiguration = () => ({ }) const exampleRequest = async () => { - const now = await currentTime() return { client: hexlify(randomBytes(20)), ask: { @@ -35,7 +33,7 @@ const exampleRequest = async () => { cid: "zb2rhheVmk3bLks5MgzTqyznLu1zqGH5jrfTA1eAZXrjx7Vob", merkleRoot: Array.from(randomBytes(32)), }, - expiry: now + hours(1), + expiry: hours(1), nonce: hexlify(randomBytes(32)), } } diff --git a/test/marketplace.js b/test/marketplace.js index e3a9168..1eb641c 100644 --- a/test/marketplace.js +++ b/test/marketplace.js @@ -2,8 +2,16 @@ const { advanceTimeToForNextBlock, currentTime } = require("./evm") const { slotId, requestId } = require("./ids") const { price } = require("./price") +/** + * @dev This will not advance the time right on the "expiry threshold" but will most probably "overshoot it" + * because "currentTime" most probably is not the time at which the request is created, but it is used + * in the next timestamp calculation with `now + expiry`. + * @param request + * @returns {Promise} + */ async function waitUntilCancelled(request) { - await advanceTimeToForNextBlock(request.expiry + 1) + // We do +1, because the expiry check in contract is done as `>` and not `>=`. + await advanceTimeToForNextBlock((await currentTime()) + request.expiry + 1) } async function waitUntilStarted(contract, request, proof, token) { @@ -16,6 +24,7 @@ async function waitUntilStarted(contract, request, proof, token) { async function waitUntilFinished(contract, requestId) { const end = (await contract.requestEnd(requestId)).toNumber() + // We do +1, because the end check in contract is done as `>` and not `>=`. await advanceTimeToForNextBlock(end + 1) } diff --git a/test/requests.js b/test/requests.js index e9c7fb2..7b172fe 100644 --- a/test/requests.js +++ b/test/requests.js @@ -1,4 +1,5 @@ const { Assertion } = require("chai") +const { currentTime } = require("./evm") const RequestState = { New: 0, @@ -46,4 +47,8 @@ const enableRequestAssertions = function () { }) } -module.exports = { RequestState, SlotState, enableRequestAssertions } +module.exports = { + RequestState, + SlotState, + enableRequestAssertions, +}