From 78c15710f308dc546c9b453bf2947ae09ce57828 Mon Sep 17 00:00:00 2001 From: Arnaud Date: Mon, 27 Jan 2025 11:02:27 +0100 Subject: [PATCH] Remove the mapping _probabilities (#215) * Remove the mapping _probabilities * Fix the slot propability calculation test by filling slot only instead of requiring proofs * Remove custom errorr Proofs_InvalidProbability not used anymore --- contracts/Marketplace.sol | 11 ++++++++++- contracts/Proofs.sol | 17 ++++++++--------- contracts/TestProofs.sol | 24 +++++++++++++++++++++--- test/Marketplace.test.js | 24 ++++++++++++++++++++++++ test/Proofs.test.js | 27 +++++++++++++-------------- 5 files changed, 76 insertions(+), 27 deletions(-) diff --git a/contracts/Marketplace.sol b/contracts/Marketplace.sol index 58c8854..4271617 100644 --- a/contracts/Marketplace.sol +++ b/contracts/Marketplace.sol @@ -205,7 +205,7 @@ contract Marketplace is SlotReservations, Proofs, StateRetrieval, Endian { revert Marketplace_SlotNotFree(); } - _startRequiringProofs(slotId, request.ask.proofProbability); + _startRequiringProofs(slotId); submitProof(slotId, proof); slot.host = msg.sender; @@ -646,6 +646,15 @@ contract Marketplace is SlotReservations, Proofs, StateRetrieval, Endian { return slot.state; } + function slotProbability( + SlotId slotId + ) public view override returns (uint256) { + Slot storage slot = _slots[slotId]; + Request storage request = _requests[slot.requestId]; + return + (request.ask.proofProbability * (256 - _config.proofs.downtime)) / 256; + } + function _transferFrom(address sender, uint256 amount) internal { address receiver = address(this); if (!_token.transferFrom(sender, receiver, amount)) diff --git a/contracts/Proofs.sol b/contracts/Proofs.sol index 3b9d267..4a70fa3 100644 --- a/contracts/Proofs.sol +++ b/contracts/Proofs.sol @@ -19,7 +19,6 @@ abstract contract Proofs is Periods { error Proofs_ProofNotMissing(); error Proofs_ProofNotRequired(); error Proofs_ProofAlreadyMarkedMissing(); - error Proofs_InvalidProbability(); ProofConfig private _config; IGroth16Verifier private _verifier; @@ -41,13 +40,18 @@ abstract contract Proofs is Periods { } mapping(SlotId => uint256) private _slotStarts; // TODO: Should be smaller than uint256 - mapping(SlotId => uint256) private _probabilities; mapping(SlotId => uint256) private _missed; // TODO: Should be smaller than uint256 mapping(SlotId => mapping(Period => bool)) private _received; mapping(SlotId => mapping(Period => bool)) private _missing; function slotState(SlotId id) public view virtual returns (SlotState); + /** + * @param id Slot's ID + * @return Integer which specifies the probability of how often the proofs will be required. Lower number means higher probability. + */ + function slotProbability(SlotId id) public view virtual returns (uint256); + /** * @return Number of missed proofs since Slot was Filled */ @@ -65,16 +69,11 @@ abstract contract Proofs is Periods { /** * @param id Slot's ID for which the proofs should be started to require - * @param probability Integer which specifies the probability of how often the proofs will be required. Lower number means higher probability. * @notice Notes down the block's timestamp as Slot's starting time for requiring proofs * and saves the required probability. */ - function _startRequiringProofs(SlotId id, uint256 probability) internal { - if (probability == 0) { - revert Proofs_InvalidProbability(); - } + function _startRequiringProofs(SlotId id) internal { _slotStarts[id] = block.timestamp; - _probabilities[id] = probability; } /** @@ -149,7 +148,7 @@ abstract contract Proofs is Periods { /// Scaling of the probability according the downtime configuration /// See: https://github.com/codex-storage/codex-research/blob/41c4b4409d2092d0a5475aca0f28995034e58d14/design/storage-proof-timing.md#pointer-downtime - uint256 probability = (_probabilities[id] * (256 - _config.downtime)) / 256; + uint256 probability = slotProbability(id); isRequired = probability == 0 || uint256(challenge) % probability == 0; } diff --git a/contracts/TestProofs.sol b/contracts/TestProofs.sol index ae34b24..240f568 100644 --- a/contracts/TestProofs.sol +++ b/contracts/TestProofs.sol @@ -6,18 +6,26 @@ import "./Proofs.sol"; // exposes internal functions of Proofs for testing contract TestProofs is Proofs { mapping(SlotId => SlotState) private _states; + mapping(SlotId => uint256) private _probabilities; + // A _config object exist in Proofs but it is private. + // Better to duplicate this config in the test implementation + // rather than modifiying the existing implementation and change + // private to internal, which may cause problems in the Marketplace contract. + ProofConfig private _proofConfig; constructor( ProofConfig memory config, IGroth16Verifier verifier - ) Proofs(config, verifier) {} // solhint-disable-line no-empty-blocks + ) Proofs(config, verifier) { + _proofConfig = config; + } function slotState(SlotId slotId) public view override returns (SlotState) { return _states[slotId]; } - function startRequiringProofs(SlotId slot, uint256 probability) public { - _startRequiringProofs(slot, probability); + function startRequiringProofs(SlotId slot) public { + _startRequiringProofs(slot); } function markProofAsMissing(SlotId id, Period period) public { @@ -35,4 +43,14 @@ contract TestProofs is Proofs { function setSlotState(SlotId id, SlotState state) public { _states[id] = state; } + + function slotProbability( + SlotId id + ) public view virtual override returns (uint256) { + return (_probabilities[id] * (256 - _proofConfig.downtime)) / 256; + } + + function setSlotProbability(SlotId id, uint256 probability) public { + _probabilities[id] = probability; + } } diff --git a/test/Marketplace.test.js b/test/Marketplace.test.js index 7e8254f..f8fd6f2 100644 --- a/test/Marketplace.test.js +++ b/test/Marketplace.test.js @@ -1206,6 +1206,30 @@ describe("Marketplace", function () { }) }) + describe("slot probability", function () { + beforeEach(async function () { + switchAccount(client) + await token.approve(marketplace.address, maxPrice(request)) + await marketplace.requestStorage(request) + switchAccount(host) + const collateral = collateralPerSlot(request) + await token.approve(marketplace.address, collateral) + }) + + it("calculates correctly the slot probability", async function () { + await marketplace.reserveSlot(slot.request, slot.index) + await marketplace.fillSlot(slot.request, slot.index, proof) + + // request.ask.proofProbability = 4 + // config.proofs.downtime = 64 + // 4 * (256 - 64) / 256 + const expectedProbability = 3 + expect(await marketplace.slotProbability(slotId(slot))).to.equal( + expectedProbability + ) + }) + }) + describe("proof requirements", function () { let period, periodOf, periodEnd diff --git a/test/Proofs.test.js b/test/Proofs.test.js index 1c6931b..e2bc0d9 100644 --- a/test/Proofs.test.js +++ b/test/Proofs.test.js @@ -50,8 +50,8 @@ describe("Proofs", function () { it("requires proofs with an agreed upon probability", async function () { const samples = 256 // 256 samples avoids bias due to pointer downtime - - await proofs.startRequiringProofs(slotId, probability) + await proofs.setSlotProbability(slotId, probability) + await proofs.startRequiringProofs(slotId) await advanceTimeForNextBlock(period) await mine() let amount = 0 @@ -71,7 +71,9 @@ describe("Proofs", function () { }) it("supports probability 1 (proofs are always required)", async function () { - await proofs.startRequiringProofs(slotId, 1) + const probability = 1 + await proofs.setSlotProbability(slotId, probability) + await proofs.startRequiringProofs(slotId) await advanceTimeForNextBlock(period) await mine() while ((await proofs.getPointer(slotId)) < downtime) { @@ -83,7 +85,8 @@ 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.startRequiringProofs(slotId, probability) + await proofs.setSlotProbability(slotId, probability) + 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)) @@ -97,7 +100,8 @@ describe("Proofs", function () { let id3 = hexlify(randomBytes(32)) for (let slotId of [id1, id2, id3]) { await proofs.setSlotState(slotId, SlotState.Filled) - await proofs.startRequiringProofs(slotId, probability) + await proofs.setSlotProbability(slotId, probability) + await proofs.startRequiringProofs(slotId) } let req1, req2, req3 while (req1 === req2 && req2 === req3) { @@ -130,7 +134,8 @@ describe("Proofs", function () { beforeEach(async function () { await proofs.setSlotState(slotId, SlotState.Filled) - await proofs.startRequiringProofs(slotId, probability) + await proofs.setSlotProbability(slotId, probability) + await proofs.startRequiringProofs(slotId) await advanceTimeToForNextBlock(periodEnd(periodOf(await currentTime()))) await waitUntilProofWillBeRequired() }) @@ -165,7 +170,8 @@ describe("Proofs", function () { beforeEach(async function () { await proofs.setSlotState(slotId, SlotState.Filled) - await proofs.startRequiringProofs(slotId, probability) + await proofs.setSlotProbability(slotId, probability) + await proofs.startRequiringProofs(slotId) }) async function waitUntilProofIsRequired(slotId) { @@ -299,12 +305,5 @@ describe("Proofs", function () { await proofs.setSlotState(slotId, SlotState.Finished) expect(await proofs.isProofRequired(slotId)).to.be.false }) - - it("is rejected when probability is 0", async function () { - const probability = 0 - await expect( - proofs.startRequiringProofs(slotId, probability) - ).to.be.revertedWith("Proofs_InvalidProbability") - }) }) })