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

@@ -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))

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 {
+  function _startRequiringProofs(SlotId id) internal {
-    if (probability == 0) {
-      revert Proofs_InvalidProbability();
-    }
     _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;
   }
 

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 {
+  function startRequiringProofs(SlotId slot) public {
-    _startRequiringProofs(slot, probability);
+    _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;
+  }
 }

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
 

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.setSlotProbability(slotId, probability)
-      await proofs.startRequiringProofs(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")
-    })
   })
 })