feat(IValidator): Allow Forward Progress Despite Missing Proofs (#321)

wjmelements · web-flow · commit e7c609e5c01c · 2025-10-24T13:50:02.000-05:00
Reviewer @rvagg It is only important to block forward progress through the current proving period. Currently we don't allow forward progress through unsettled periods without proofs. This could be a problem if there was a rail unproven for a long time but proofs were later resumed. Though much less likely after #258 and #267, we would not want it to be possible for the gap to exceed the block gas limit. Also important to recall that the `fromEpoch` is an exclusive index while `toEpoch` is an inclusive index. #### Changes * allow to settleUpTo previous period when unproven * define settledUpTo in previously irrelevant case
diff --git a/service_contracts/src/FilecoinWarmStorageService.sol b/service_contracts/src/FilecoinWarmStorageService.sol
@@ -876,7 +876,6 @@ contract FilecoinWarmStorageService is
                 revert Errors.InvalidChallengeEpoch(dataSetId, minWindow, maxWindow, challengeEpoch);
             }
             provingDeadlines[dataSetId] = firstDeadline;
-            provenThisPeriod[dataSetId] = false;
 
             // Initialize the activation epoch when proving first starts
             // This marks when the data set became active for proving
@@ -926,15 +925,6 @@ contract FilecoinWarmStorageService is
             emit FaultRecord(dataSetId, faultPeriods, provingDeadlines[dataSetId]);
         }
 
-        // Record the status of the current/previous proving period that's ending
-        if (provingDeadlines[dataSetId] != NO_PROVING_DEADLINE && provenThisPeriod[dataSetId]) {
-            // Determine the period ID that just completed
-            uint256 completedPeriodId = getProvingPeriodForEpoch(dataSetId, provingDeadlines[dataSetId] - 1);
-
-            // Record whether this period was proven
-            provenPeriods[dataSetId][completedPeriodId >> 8] |= 1 << (completedPeriodId & 255);
-        }
-
         provingDeadlines[dataSetId] = nextDeadline;
         provenThisPeriod[dataSetId] = false;
 
@@ -1179,8 +1169,10 @@ contract FilecoinWarmStorageService is
      * @return The period ID this epoch belongs to, or type(uint256).max if before activation
      */
     function getProvingPeriodForEpoch(uint256 dataSetId, uint256 epoch) public view returns (uint256) {
-        uint256 activationEpoch = provingActivationEpoch[dataSetId];
+        return _provingPeriodForEpoch(provingActivationEpoch[dataSetId], epoch);
+    }
 
+    function _provingPeriodForEpoch(uint256 activationEpoch, uint256 epoch) internal view returns (uint256) {
         // If proving wasn't activated or epoch is before activation
         if (activationEpoch == 0 || epoch < activationEpoch) {
             return type(uint256).max; // Invalid period
@@ -1484,78 +1476,71 @@ contract FilecoinWarmStorageService is
             });
         }
 
-        // Count proven epochs and find the last proven epoch
-        (uint256 provenEpochCount, uint256 lastProvenEpoch) =
+        // Count proven epochs up to toEpoch, possibly stopping earlier if unresolved
+        (uint256 provenEpochCount, uint256 settleUpTo) =
             _findProvenEpochs(dataSetId, fromEpoch, toEpoch, activationEpoch);
 
         // If no epochs are proven, we can't settle anything
         if (provenEpochCount == 0) {
             return ValidationResult({
                 modifiedAmount: 0,
-                settleUpto: fromEpoch,
+                settleUpto: settleUpTo,
                 note: "No proven epochs in the requested range"
             });
         }
 
         // Calculate the modified amount based on proven epochs
         uint256 modifiedAmount = (proposedAmount * provenEpochCount) / totalEpochsRequested;
 
-        return ValidationResult({
-            modifiedAmount: modifiedAmount,
-            settleUpto: lastProvenEpoch, // Settle up to the last proven epoch
-            note: ""
-        });
+        return ValidationResult({modifiedAmount: modifiedAmount, settleUpto: settleUpTo, note: ""});
     }
 
     function _findProvenEpochs(uint256 dataSetId, uint256 fromEpoch, uint256 toEpoch, uint256 activationEpoch)
         internal
         view
-        returns (uint256 provenEpochCount, uint256 settledUpTo)
+        returns (uint256 provenEpochCount, uint256 settleUpTo)
     {
         require(toEpoch >= activationEpoch && toEpoch <= block.number, Errors.InvalidEpochRange(fromEpoch, toEpoch));
-        uint256 currentPeriod = getProvingPeriodForEpoch(dataSetId, block.number);
-
         if (fromEpoch < activationEpoch - 1) {
             fromEpoch = activationEpoch - 1;
         }
 
-        uint256 startingPeriod = getProvingPeriodForEpoch(dataSetId, fromEpoch + 1);
+        uint256 startingPeriod = _provingPeriodForEpoch(activationEpoch, fromEpoch + 1);
 
-        // handle first period separately
+        // handle first period separately; it may be partially settled already
         uint256 startingPeriodDeadline = _calcPeriodDeadline(activationEpoch, startingPeriod);
 
         if (toEpoch < startingPeriodDeadline) {
-            if (_isPeriodProven(dataSetId, startingPeriod, currentPeriod)) {
+            if (_isPeriodProven(dataSetId, startingPeriod)) {
                 provenEpochCount = toEpoch - fromEpoch;
-                settledUpTo = toEpoch;
+                settleUpTo = toEpoch;
+            } else {
+                settleUpTo = fromEpoch;
             }
         } else {
-            if (_isPeriodProven(dataSetId, startingPeriod, currentPeriod)) {
+            if (_isPeriodProven(dataSetId, startingPeriod)) {
                 provenEpochCount += (startingPeriodDeadline - fromEpoch);
             }
 
-            uint256 endingPeriod = getProvingPeriodForEpoch(dataSetId, toEpoch);
+            uint256 endingPeriod = _provingPeriodForEpoch(activationEpoch, toEpoch);
             // loop through the proving periods between startingPeriod and endingPeriod
             for (uint256 period = startingPeriod + 1; period < endingPeriod; period++) {
-                if (_isPeriodProven(dataSetId, period, currentPeriod)) {
+                if (_isPeriodProven(dataSetId, period)) {
                     provenEpochCount += maxProvingPeriod;
                 }
             }
-            settledUpTo = _calcPeriodDeadline(activationEpoch, endingPeriod - 1);
+            settleUpTo = _calcPeriodDeadline(activationEpoch, endingPeriod - 1);
 
             // handle the last period separately
-            if (_isPeriodProven(dataSetId, endingPeriod, currentPeriod)) {
-                provenEpochCount += (toEpoch - settledUpTo);
-                settledUpTo = toEpoch;
+            if (_isPeriodProven(dataSetId, endingPeriod)) {
+                provenEpochCount += (toEpoch - settleUpTo);
+                settleUpTo = toEpoch;
             }
         }
-        return (provenEpochCount, settledUpTo);
+        return (provenEpochCount, settleUpTo);
     }
 
-    function _isPeriodProven(uint256 dataSetId, uint256 periodId, uint256 currentPeriod) private view returns (bool) {
-        if (periodId == currentPeriod) {
-            return provenThisPeriod[dataSetId];
-        }
+    function _isPeriodProven(uint256 dataSetId, uint256 periodId) private view returns (bool) {
         uint256 isProven = provenPeriods[dataSetId][periodId >> 8] & (1 << (periodId & 255));
         return isProven != 0;
     }
diff --git a/service_contracts/test/FilecoinWarmStorageService.t.sol b/service_contracts/test/FilecoinWarmStorageService.t.sol
@@ -5057,17 +5057,54 @@ contract ValidatePaymentTest is FilecoinWarmStorageServiceTest {
         // Validate payment
         FilecoinWarmStorageService.DataSetInfoView memory info = viewContract.getDataSet(dataSetId);
         uint256 fromEpoch = activationEpoch - 1; // exclusive
-        uint256 toEpoch = activationEpoch + (maxProvingPeriod * 3) - 1;
+        uint256 toEpoch = vm.getBlockNumber() - 1;
         uint256 proposedAmount = 1000e6;
 
-        vm.prank(address(payments));
         IValidator.ValidationResult memory result =
             pdpServiceWithPayments.validatePayment(info.pdpRailId, proposedAmount, fromEpoch, toEpoch, 0);
 
-        // Should pay nothing
+        // Should settle two unproven periods
+        assertEq(result.modifiedAmount, 0, "Should pay nothing");
+        assertEq(result.settleUpto, activationEpoch + (maxProvingPeriod * 2), "Should not settle last period");
+        assertEq(result.note, "No proven epochs in the requested range");
+
+        vm.prank(address(mockPDPVerifier));
+        pdpServiceWithPayments.nextProvingPeriod(dataSetId, challengeEpoch + maxProvingPeriod * 2, 100, "");
+
+        // Should settle up to start of current period
+        result = pdpServiceWithPayments.validatePayment(info.pdpRailId, proposedAmount, activationEpoch, toEpoch, 0);
+        assertEq(result.modifiedAmount, 0, "Should pay nothing");
+        assertEq(result.settleUpto, activationEpoch + (maxProvingPeriod * 2), "Should not settle last period");
+        assertEq(result.note, "No proven epochs in the requested range");
+
+        // Never settle less than 1 proving period when that period is unproven
+        toEpoch = activationEpoch + 1;
+        result = pdpServiceWithPayments.validatePayment(info.pdpRailId, proposedAmount, activationEpoch, toEpoch, 0);
+        assertEq(result.modifiedAmount, 0, "Should pay nothing");
+        assertEq(result.settleUpto, activationEpoch, "Should not settle");
+        assertEq(result.note, "No proven epochs in the requested range");
+
+        // Never settle less than 1 proving period when that period is unproven
+        fromEpoch = activationEpoch + maxProvingPeriod * 2 - 1;
+        toEpoch = activationEpoch + maxProvingPeriod * 2 + 1;
+        result = pdpServiceWithPayments.validatePayment(info.pdpRailId, proposedAmount, fromEpoch, toEpoch, 0);
         assertEq(result.modifiedAmount, 0, "Should pay nothing");
         assertEq(result.settleUpto, fromEpoch, "Should not settle");
         assertEq(result.note, "No proven epochs in the requested range");
+
+        // Settle only up to the start of current period
+        fromEpoch = activationEpoch + maxProvingPeriod * 2 - 2;
+        result = pdpServiceWithPayments.validatePayment(info.pdpRailId, proposedAmount, fromEpoch, toEpoch, 0);
+        assertEq(result.modifiedAmount, 0, "Should pay nothing");
+        assertEq(result.settleUpto, activationEpoch + maxProvingPeriod * 2, "Should not settle into last period");
+        assertEq(result.note, "No proven epochs in the requested range");
+
+        // Settle only up to the start of current period
+        fromEpoch = activationEpoch + maxProvingPeriod / 2;
+        result = pdpServiceWithPayments.validatePayment(info.pdpRailId, proposedAmount, fromEpoch, toEpoch, 0);
+        assertEq(result.modifiedAmount, 0, "Should pay nothing");
+        assertEq(result.settleUpto, activationEpoch + maxProvingPeriod * 2, "Should not settle into last period");
+        assertEq(result.note, "No proven epochs in the requested range");
     }
 
     /**
