From e31e39f22cd1b57c37f4399c1484c4001cc35c6a Mon Sep 17 00:00:00 2001 From: Mark Spanbroek Date: Thu, 30 Jan 2025 09:34:42 +0100 Subject: [PATCH] simplify time-based logic in tests, and fix requestEnd() - use the `allowBlocksWithSameTimestamp` hardhat option - remove block time gymnastics from marketplace tests - fix erroneous implementation of requestEnd() which surfaced because of the the improved tests --- contracts/Marketplace.sol | 10 +++-- hardhat.config.js | 1 + test/Marketplace.test.js | 78 +++++++++++++-------------------------- test/Proofs.test.js | 49 ++++++++---------------- test/evm.js | 37 ++++--------------- test/marketplace.js | 8 ++-- 6 files changed, 60 insertions(+), 123 deletions(-) diff --git a/contracts/Marketplace.sol b/contracts/Marketplace.sol index 283a393..8c44fbf 100644 --- a/contracts/Marketplace.sol +++ b/contracts/Marketplace.sol @@ -558,13 +558,15 @@ contract Marketplace is SlotReservations, Proofs, StateRetrieval, Endian { } function requestEnd(RequestId requestId) public view returns (uint64) { - uint64 end = _requestContexts[requestId].endsAt; RequestState state = requestState(requestId); if (state == RequestState.New || state == RequestState.Started) { - return end; - } else { - return uint64(Math.min(end, block.timestamp - 1)); + return _requestContexts[requestId].endsAt; } + if (state == RequestState.Cancelled) { + return _requestContexts[requestId].expiresAt; + } + return + uint64(Math.min(_requestContexts[requestId].endsAt, block.timestamp)); } function requestExpiry(RequestId requestId) public view returns (uint64) { diff --git a/hardhat.config.js b/hardhat.config.js index b29d98c..e35006a 100644 --- a/hardhat.config.js +++ b/hardhat.config.js @@ -24,6 +24,7 @@ module.exports = { networks: { hardhat: { tags: ["local"], + allowBlocksWithSameTimestamp: true }, codexdisttestnetwork: { url: `${process.env.DISTTEST_NETWORK_URL}`, diff --git a/test/Marketplace.test.js b/test/Marketplace.test.js index 2f996f5..76e5085 100644 --- a/test/Marketplace.test.js +++ b/test/Marketplace.test.js @@ -32,10 +32,9 @@ const { collateralPerSlot } = require("./collateral") const { snapshot, revert, - mine, ensureMinimumBlockHeight, - advanceTimeForNextBlock, - advanceTimeToForNextBlock, + advanceTime, + advanceTimeTo, currentTime, } = require("./evm") const { arrayify } = require("ethers/lib/utils") @@ -180,9 +179,7 @@ describe("Marketplace", function () { it("emits event when storage is requested", async function () { await token.approve(marketplace.address, maxPrice(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 + const expectedExpiry = (await currentTime()) + request.expiry await expect(marketplace.requestStorage(request)) .to.emit(marketplace, "StorageRequested") .withArgs(requestId(request), askToArray(request.ask), expectedExpiry) @@ -328,7 +325,7 @@ describe("Marketplace", function () { // We need to advance the time to next period, because filling slot // must not be done in the same period as for that period there was already proof // submitted with the previous `fillSlot` and the transaction would revert with "Proof already submitted". - await advanceTimeForNextBlock(config.proofs.period + 1) + await advanceTime(config.proofs.period + 1) const startBalance = await token.balanceOf(host.address) const collateral = collateralPerSlot(request) @@ -478,7 +475,7 @@ describe("Marketplace", function () { await token.approve(marketplace.address, collateral) await marketplace.reserveSlot(slot.request, slot.index) await marketplace.fillSlot(slot.request, slot.index, proof) - await advanceTimeForNextBlock(config.proofs.period) + await advanceTime(config.proofs.period) }) it("allows proofs to be submitted", async function () { @@ -522,13 +519,12 @@ describe("Marketplace", function () { await marketplace.fillSlot(slot.request, slot.index, proof) await expect( (await marketplace.requestEnd(requestId(request))).toNumber() - ).to.be.closeTo(requestTime + request.ask.duration, 1) + ).to.equal(requestTime + request.ask.duration) }) it("sets request end time to the past once failed", async function () { await waitUntilStarted(marketplace, request, proof, token) await waitUntilFailed(marketplace, request) - let slot0 = { ...slot, index: request.ask.maxSlotLoss + 1 } const now = await currentTime() await expect(await marketplace.requestEnd(requestId(request))).to.be.eq( now - 1 @@ -539,7 +535,6 @@ describe("Marketplace", function () { await marketplace.reserveSlot(slot.request, slot.index) await marketplace.fillSlot(slot.request, slot.index, proof) await waitUntilCancelled(request) - await mine() const now = await currentTime() await expect(await marketplace.requestEnd(requestId(request))).to.be.eq( now - 1 @@ -549,11 +544,7 @@ describe("Marketplace", function () { it("checks that request end time is in the past once finished", async function () { await waitUntilStarted(marketplace, request, proof, token) await waitUntilFinished(marketplace, requestId(request)) - await mine() const now = await currentTime() - // 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( now - 1 ) @@ -618,7 +609,7 @@ describe("Marketplace", function () { // We are advancing the time because most of the slots will be filled somewhere // in the "expiry window" and not at its beginning. This is more "real" setup // and demonstrates the partial payout feature better. - await advanceTimeForNextBlock(request.expiry / 2) + await advanceTime(request.expiry / 2) const expectedPayouts = await waitUntilStarted( marketplace, @@ -702,7 +693,7 @@ describe("Marketplace", function () { ).toNumber() await marketplace.reserveSlot(slot.request, slot.index) - await advanceTimeToForNextBlock(filledAt) + await advanceTimeTo(filledAt) await marketplace.fillSlot(slot.request, slot.index, proof) await waitUntilCancelled(request) await marketplace.freeSlot(slotId(slot)) @@ -723,7 +714,7 @@ describe("Marketplace", function () { ).toNumber() await marketplace.reserveSlot(slot.request, slot.index) - await advanceTimeToForNextBlock(filledAt) + await advanceTimeTo(filledAt) await marketplace.fillSlot(slot.request, slot.index, proof) await waitUntilCancelled(request) const startBalanceHost = await token.balanceOf(host.address) @@ -869,6 +860,10 @@ describe("Marketplace", function () { switchAccount(client) await token.approve(marketplace.address, maxPrice(request)) await marketplace.requestStorage(request) + + // wait a bit, so that there are funds for the client to withdraw + await advanceTime(10) + switchAccount(host) const collateral = collateralPerSlot(request) await token.approve(marketplace.address, collateral) @@ -1011,7 +1006,7 @@ describe("Marketplace", function () { ).toNumber() await marketplace.reserveSlot(slot.request, slot.index) - await advanceTimeToForNextBlock(filledAt) + await advanceTimeTo(filledAt) await marketplace.fillSlot(slot.request, slot.index, proof) await waitUntilCancelled(request) const expectedPartialhostRewardRecipient = @@ -1069,7 +1064,6 @@ describe("Marketplace", function () { it("changes to 'Cancelled' once request is cancelled", async function () { await waitUntilCancelled(request) - await mine() expect(await marketplace.requestState(slot.request)).to.equal(Cancelled) }) @@ -1091,7 +1085,6 @@ describe("Marketplace", function () { it("changes to 'Failed' once too many slots are freed", async function () { await waitUntilStarted(marketplace, request, proof, token) await waitUntilFailed(marketplace, request) - await mine() expect(await marketplace.requestState(slot.request)).to.equal(Failed) }) @@ -1115,7 +1108,6 @@ describe("Marketplace", function () { it("changes to 'Finished' when the request ends", async function () { await waitUntilStarted(marketplace, request, proof, token) await waitUntilFinished(marketplace, requestId(request)) - await mine() expect(await marketplace.requestState(slot.request)).to.equal(Finished) }) @@ -1145,16 +1137,14 @@ describe("Marketplace", function () { }) async function waitUntilProofIsRequired(id) { - await advanceTimeToForNextBlock(periodEnd(periodOf(await currentTime()))) - await mine() + await advanceTimeTo(periodEnd(periodOf(await currentTime()))) while ( !( (await marketplace.isProofRequired(id)) && (await marketplace.getPointer(id)) < 250 ) ) { - await advanceTimeForNextBlock(period) - await mine() + await advanceTime(period) } } @@ -1171,7 +1161,6 @@ describe("Marketplace", function () { it("changes to 'Finished' when request finishes", async function () { await waitUntilStarted(marketplace, request, proof, token) await waitUntilFinished(marketplace, slot.request) - await mine() expect(await marketplace.slotState(slotId(slot))).to.equal(Finished) }) @@ -1179,7 +1168,6 @@ describe("Marketplace", function () { await marketplace.reserveSlot(slot.request, slot.index) await marketplace.fillSlot(slot.request, slot.index, proof) await waitUntilCancelled(request) - await mine() expect(await marketplace.slotState(slotId(slot))).to.equal(Cancelled) }) @@ -1195,8 +1183,7 @@ describe("Marketplace", function () { while ((await marketplace.slotState(slotId(slot))) === Filled) { await waitUntilProofIsRequired(slotId(slot)) const missedPeriod = periodOf(await currentTime()) - await advanceTimeForNextBlock(period) - await mine() + await advanceTime(period + 1) await marketplace.markProofAsMissing(slotId(slot), missedPeriod) } expect(await marketplace.slotState(slotId(slot))).to.equal(Repair) @@ -1205,7 +1192,6 @@ describe("Marketplace", function () { it("changes to 'Failed' when request fails", async function () { await waitUntilStarted(marketplace, request, proof, token) await waitUntilSlotFailed(marketplace, request, slot) - await mine() expect(await marketplace.slotState(slotId(slot))).to.equal(Failed) }) @@ -1258,20 +1244,19 @@ describe("Marketplace", function () { async function waitUntilProofWillBeRequired(id) { while (!(await marketplace.willProofBeRequired(id))) { - await mine() + await advanceTime(period) } } async function waitUntilProofIsRequired(id) { - await advanceTimeToForNextBlock(periodEnd(periodOf(await currentTime()))) + await advanceTimeTo(periodEnd(periodOf(await currentTime()))) while ( !( (await marketplace.isProofRequired(id)) && (await marketplace.getPointer(id)) < 250 ) ) { - await advanceTimeForNextBlock(period) - await mine() + await advanceTime(period) } } @@ -1289,7 +1274,6 @@ describe("Marketplace", function () { await waitUntilProofWillBeRequired(id) await expect(await marketplace.willProofBeRequired(id)).to.be.true await waitUntilCancelled(request) - await mine() await expect(await marketplace.willProofBeRequired(id)).to.be.false }) @@ -1300,7 +1284,6 @@ describe("Marketplace", function () { await waitUntilProofIsRequired(id) await expect(await marketplace.isProofRequired(id)).to.be.true await waitUntilCancelled(request) - await mine() await expect(await marketplace.isProofRequired(id)).to.be.false }) @@ -1309,11 +1292,9 @@ describe("Marketplace", function () { await marketplace.reserveSlot(slot.request, slot.index) await marketplace.fillSlot(slot.request, slot.index, proof) await waitUntilProofIsRequired(id) - await mine() const challenge1 = await marketplace.getChallenge(id) expect(BigNumber.from(challenge1).gt(0)) await waitUntilCancelled(request) - await mine() const challenge2 = await marketplace.getChallenge(id) expect(BigNumber.from(challenge2).isZero()) }) @@ -1323,11 +1304,9 @@ describe("Marketplace", function () { await marketplace.reserveSlot(slot.request, slot.index) await marketplace.fillSlot(slot.request, slot.index, proof) await waitUntilProofIsRequired(id) - await mine() const challenge1 = await marketplace.getChallenge(id) expect(BigNumber.from(challenge1).gt(0)) await waitUntilCancelled(request) - await mine() const challenge2 = await marketplace.getChallenge(id) expect(BigNumber.from(challenge2).isZero()) }) @@ -1349,16 +1328,14 @@ describe("Marketplace", function () { }) async function waitUntilProofIsRequired(id) { - await advanceTimeToForNextBlock(periodEnd(periodOf(await currentTime()))) - await mine() + await advanceTimeTo(periodEnd(periodOf(await currentTime()))) while ( !( (await marketplace.isProofRequired(id)) && (await marketplace.getPointer(id)) < 250 ) ) { - await advanceTimeForNextBlock(period) - await mine() + await advanceTime(period) } } @@ -1381,7 +1358,7 @@ describe("Marketplace", function () { await waitUntilProofIsRequired(id) let missedPeriod = periodOf(await currentTime()) - await advanceTimeForNextBlock(period + 1) + await advanceTime(period + 1) await marketplace.markProofAsMissing(id, missedPeriod) const collateral = collateralPerSlot(request) @@ -1408,7 +1385,7 @@ describe("Marketplace", function () { await waitUntilProofIsRequired(id) let missedPeriod = periodOf(await currentTime()) - await advanceTimeForNextBlock(period + 1) + await advanceTime(period + 1) await marketplace.markProofAsMissing(id, missedPeriod) const endBalance = await token.balanceOf(validatorRecipient.address) @@ -1441,7 +1418,7 @@ describe("Marketplace", function () { ) await waitUntilProofIsRequired(slotId(slot)) const missedPeriod = periodOf(await currentTime()) - await advanceTimeForNextBlock(period + 1) + await advanceTime(period + 1) await marketplace.markProofAsMissing(slotId(slot), missedPeriod) } expect(await marketplace.slotState(slotId(slot))).to.equal( @@ -1468,7 +1445,7 @@ describe("Marketplace", function () { ) await waitUntilProofIsRequired(slotId(slot)) const missedPeriod = periodOf(await currentTime()) - await advanceTimeForNextBlock(period + 1) + await advanceTime(period + 1) expect(await marketplace.missingProofs(slotId(slot))).to.equal( missedProofs ) @@ -1502,7 +1479,6 @@ describe("Marketplace", function () { it("keeps request in list when cancelled", async function () { await marketplace.requestStorage(request) await waitUntilCancelled(request) - await mine() expect(await marketplace.myRequests()).to.deep.equal([requestId(request)]) }) @@ -1521,7 +1497,6 @@ describe("Marketplace", function () { switchAccount(host) await waitUntilStarted(marketplace, request, proof, token) await waitUntilFailed(marketplace, request) - await mine() switchAccount(client) expect(await marketplace.myRequests()).to.deep.equal([requestId(request)]) }) @@ -1584,7 +1559,6 @@ describe("Marketplace", function () { await marketplace.reserveSlot(slot.request, slot1.index) await marketplace.fillSlot(slot.request, slot1.index, proof) await waitUntilCancelled(request) - await mine() expect(await marketplace.mySlots()).to.have.members([ slotId(slot), slotId(slot1), diff --git a/test/Proofs.test.js b/test/Proofs.test.js index e2bc0d9..ce1a937 100644 --- a/test/Proofs.test.js +++ b/test/Proofs.test.js @@ -7,8 +7,8 @@ const { mine, ensureMinimumBlockHeight, currentTime, - advanceTimeForNextBlock, - advanceTimeToForNextBlock, + advanceTime, + advanceTimeTo, } = require("./evm") const { periodic } = require("./time") const { loadProof, loadPublicInput } = require("../verifier/verifier") @@ -52,15 +52,13 @@ describe("Proofs", function () { const samples = 256 // 256 samples avoids bias due to pointer downtime await proofs.setSlotProbability(slotId, probability) await proofs.startRequiringProofs(slotId) - await advanceTimeForNextBlock(period) - await mine() + await advanceTime(period) let amount = 0 for (let i = 0; i < samples; i++) { if (await proofs.isProofRequired(slotId)) { amount += 1 } - await advanceTimeForNextBlock(period) - await mine() + await advanceTime(period) } const p = 1 / probability // expected probability @@ -74,8 +72,7 @@ describe("Proofs", function () { const probability = 1 await proofs.setSlotProbability(slotId, probability) await proofs.startRequiringProofs(slotId) - await advanceTimeForNextBlock(period) - await mine() + await advanceTime(period) while ((await proofs.getPointer(slotId)) < downtime) { await mine() } @@ -89,8 +86,7 @@ describe("Proofs", function () { await proofs.startRequiringProofs(slotId) while (Math.floor((await currentTime()) / period) == startPeriod) { expect(await proofs.isProofRequired(slotId)).to.be.false - await advanceTimeForNextBlock(Math.floor(period / 10)) - await mine() + await advanceTime(Math.floor(period / 10)) } }) @@ -108,14 +104,11 @@ describe("Proofs", function () { req1 = await proofs.isProofRequired(id1) req2 = await proofs.isProofRequired(id2) req3 = await proofs.isProofRequired(id3) - await advanceTimeForNextBlock(period) - await mine() + await advanceTime(period) } }) it("moves pointer one block at a time", async function () { - await advanceTimeToForNextBlock(periodEnd(periodOf(await currentTime()))) - await mine() for (let i = 0; i < 256; i++) { let previous = await proofs.getPointer(slotId) await mine() @@ -128,7 +121,7 @@ describe("Proofs", function () { describe("when proof requirement is upcoming", function () { async function waitUntilProofWillBeRequired() { while (!(await proofs.willProofBeRequired(slotId))) { - await mine() + await advanceTime(period) } } @@ -136,7 +129,6 @@ describe("Proofs", function () { await proofs.setSlotState(slotId, SlotState.Filled) await proofs.setSlotProbability(slotId, probability) await proofs.startRequiringProofs(slotId) - await advanceTimeToForNextBlock(periodEnd(periodOf(await currentTime()))) await waitUntilProofWillBeRequired() }) @@ -175,23 +167,20 @@ describe("Proofs", function () { }) async function waitUntilProofIsRequired(slotId) { - await advanceTimeToForNextBlock(periodEnd(periodOf(await currentTime()))) - await mine() - while ( !( (await proofs.isProofRequired(slotId)) && (await proofs.getPointer(slotId)) < 250 ) ) { - await advanceTimeForNextBlock(period) - await mine() + await advanceTime(period) } } it("provides different challenges per period", async function () { await waitUntilProofIsRequired(slotId) const challenge1 = await proofs.getChallenge(slotId) + await advanceTime(period) await waitUntilProofIsRequired(slotId) const challenge2 = await proofs.getChallenge(slotId) expect(challenge2).not.to.equal(challenge1) @@ -231,7 +220,6 @@ describe("Proofs", function () { }) it("fails proof submission when already submitted", async function () { - await advanceTimeToForNextBlock(periodEnd(periodOf(await currentTime()))) await proofs.proofReceived(slotId, proof, pubSignals) await expect( proofs.proofReceived(slotId, proof, pubSignals) @@ -242,8 +230,7 @@ describe("Proofs", function () { expect(await proofs.missingProofs(slotId)).to.equal(0) await waitUntilProofIsRequired(slotId) let missedPeriod = periodOf(await currentTime()) - await advanceTimeToForNextBlock(periodEnd(missedPeriod)) - await mine() + await advanceTimeTo(periodEnd(missedPeriod) + 1) await proofs.markProofAsMissing(slotId, missedPeriod) expect(await proofs.missingProofs(slotId)).to.equal(1) }) @@ -259,7 +246,7 @@ describe("Proofs", function () { it("does not mark a proof as missing after timeout", async function () { await waitUntilProofIsRequired(slotId) let currentPeriod = periodOf(await currentTime()) - await advanceTimeToForNextBlock(periodEnd(currentPeriod) + timeout) + await advanceTimeTo(periodEnd(currentPeriod) + timeout + 1) await expect( proofs.markProofAsMissing(slotId, currentPeriod) ).to.be.revertedWith("Proofs_ValidationTimedOut") @@ -269,8 +256,7 @@ describe("Proofs", function () { await waitUntilProofIsRequired(slotId) let receivedPeriod = periodOf(await currentTime()) await proofs.proofReceived(slotId, proof, pubSignals) - await advanceTimeToForNextBlock(periodEnd(receivedPeriod)) - await mine() + await advanceTimeTo(periodEnd(receivedPeriod) + 1) await expect( proofs.markProofAsMissing(slotId, receivedPeriod) ).to.be.revertedWith("Proofs_ProofNotMissing") @@ -278,12 +264,10 @@ describe("Proofs", function () { it("does not mark proof as missing when not required", async function () { while (await proofs.isProofRequired(slotId)) { - await advanceTimeForNextBlock(period) - await mine() + await advanceTime(period) } let currentPeriod = periodOf(await currentTime()) - await advanceTimeToForNextBlock(periodEnd(currentPeriod)) - await mine() + await advanceTimeTo(periodEnd(currentPeriod) + 1) await expect( proofs.markProofAsMissing(slotId, currentPeriod) ).to.be.revertedWith("Proofs_ProofNotRequired") @@ -292,8 +276,7 @@ describe("Proofs", function () { it("does not mark proof as missing twice", async function () { await waitUntilProofIsRequired(slotId) let missedPeriod = periodOf(await currentTime()) - await advanceTimeToForNextBlock(periodEnd(missedPeriod)) - await mine() + await advanceTimeTo(periodEnd(missedPeriod) + 1) await proofs.markProofAsMissing(slotId, missedPeriod) await expect( proofs.markProofAsMissing(slotId, missedPeriod) diff --git a/test/evm.js b/test/evm.js index e02c4fc..885c3a7 100644 --- a/test/evm.js +++ b/test/evm.js @@ -11,16 +11,9 @@ async function snapshot() { async function revert() { const { id, time } = snapshots.pop() await ethers.provider.send("evm_revert", [id]) - await ethers.provider.send("evm_setNextBlockTimestamp", [time + 1]) + await ethers.provider.send("evm_setNextBlockTimestamp", [time]) } -/** - * Mines new block. - * - * This call increases the block's timestamp by 1! - * - * @returns {Promise} - */ async function mine() { await ethers.provider.send("evm_mine") } @@ -36,30 +29,14 @@ async function currentTime() { return block.timestamp } -/** - * Function that advances time by adding seconds to current timestamp for **next block**. - * - * If you need the timestamp to be already applied for current block then mine a new block with `mine()` after this call. - * This is mainly needed when doing assertions on top of view calls that does not create transactions and mine new block. - * - * @param timestamp - * @returns {Promise} - */ -async function advanceTimeForNextBlock(seconds) { +async function advanceTime(seconds) { await ethers.provider.send("evm_increaseTime", [seconds]) + await mine() } -/** - * Function that sets specific timestamp for **next block**. - * - * If you need the timestamp to be already applied for current block then mine a new block with `mine()` after this call. - * This is mainly needed when doing assertions on top of view calls that does not create transactions and mine new block. - * - * @param timestamp - * @returns {Promise} - */ -async function advanceTimeToForNextBlock(timestamp) { +async function advanceTimeTo(timestamp) { await ethers.provider.send("evm_setNextBlockTimestamp", [timestamp]) + await mine() } module.exports = { @@ -68,6 +45,6 @@ module.exports = { mine, ensureMinimumBlockHeight, currentTime, - advanceTimeForNextBlock, - advanceTimeToForNextBlock, + advanceTime, + advanceTimeTo, } diff --git a/test/marketplace.js b/test/marketplace.js index 5eaef19..4ed338f 100644 --- a/test/marketplace.js +++ b/test/marketplace.js @@ -1,6 +1,6 @@ -const { advanceTimeToForNextBlock, currentTime } = require("./evm") +const { advanceTimeTo, currentTime, mine } = require("./evm") const { slotId, requestId } = require("./ids") -const { maxPrice, payoutForDuration } = require("./price") +const { payoutForDuration } = require("./price") const { collateralPerSlot } = require("./collateral") /** @@ -12,7 +12,7 @@ const { collateralPerSlot } = require("./collateral") */ async function waitUntilCancelled(request) { // We do +1, because the expiry check in contract is done as `>` and not `>=`. - await advanceTimeToForNextBlock((await currentTime()) + request.expiry + 1) + await advanceTimeTo((await currentTime()) + request.expiry + 1) } async function waitUntilSlotsFilled(contract, request, proof, token, slots) { @@ -48,7 +48,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) + await advanceTimeTo(end + 1) } async function waitUntilFailed(contract, request) {