fix failing test,formatting, and some minor code quality improvements

Author: parzival1821

service_contracts/src/Errors.sol

@@ -202,7 +202,10 @@ library Errors {
     error RailNotAssociated(uint256 railId);
 
     /// @notice The epoch range is invalid
-    /// @notice Will be emitted if any of the following conditions is NOT met - (fromEpoch must be less than toEpoch), (toEpoch must be greater than block number), (toEpoch must be greater than the activation epoch)
+    /// @notice Will be emitted if any of the following conditions is NOT met :
+    /// @notice 1. fromEpoch must be less than toEpoch
+    /// @notice 2. toEpoch must be greater than block number
+    /// @notice 3. toEpoch must be greater than the activation epoch
     /// @param fromEpoch The starting epoch (exclusive)
     /// @param toEpoch The ending epoch (inclusive)
     error InvalidEpochRange(uint256 fromEpoch, uint256 toEpoch);

service_contracts/src/FilecoinWarmStorageService.sol

@@ -1445,7 +1445,8 @@ contract FilecoinWarmStorageService is
         uint256 provenEpochCount = 0;
         uint256 lastProvenEpoch = fromEpoch;
 
-        (provenEpochCount, lastProvenEpoch) = _findProvenEpochs(dataSetId, fromEpoch, toEpoch, provenEpochCount, lastProvenEpoch);
+        (provenEpochCount, lastProvenEpoch) =
+            _findProvenEpochs(dataSetId, fromEpoch, toEpoch, provenEpochCount, lastProvenEpoch);
 
         // If no epochs are proven, we can't settle anything
         if (provenEpochCount == 0) {
@@ -1469,16 +1470,18 @@ contract FilecoinWarmStorageService is
         });
     }
 
-    function _findProvenEpochs(uint256 dataSetId, uint256 fromEpoch, uint256 toEpoch, uint256 provenEpochCount, uint256 lastProvenEpoch)
-        internal
-        view
-        returns (uint256, uint256)
-    {
+    function _findProvenEpochs(
+        uint256 dataSetId,
+        uint256 fromEpoch,
+        uint256 toEpoch,
+        uint256 provenEpochCount,
+        uint256 lastProvenEpoch
+    ) internal view returns (uint256, uint256) {
         uint256 activationEpoch = provingActivationEpoch[dataSetId];
-        if (toEpoch < activationEpoch){
+        if (toEpoch < activationEpoch) {
             revert Errors.InvalidEpochRange(fromEpoch, toEpoch);
         }
-        if(toEpoch > block.number){
+        if (toEpoch > block.number) {
             revert Errors.InvalidEpochRange(fromEpoch, toEpoch);
         }
         uint256 currentPeriod = getProvingPeriodForEpoch(dataSetId, block.number);
@@ -1506,7 +1509,7 @@ contract FilecoinWarmStorageService is
             }
 
             // now loop through the proving periods between endingPeriod and startingPeriod.
-            for (uint256 period = startingPeriod + 1; period <= endingPeriod - 1; period++) {
+            for (uint256 period = startingPeriod + 1; period < endingPeriod; period++) {
                 if (_isPeriodProven(dataSetId, period, currentPeriod)) {
                     provenEpochCount += maxProvingPeriod;
                     lastProvenEpoch = _calcPeriodDeadline(dataSetId, period);
@@ -1515,7 +1518,7 @@ contract FilecoinWarmStorageService is
 
             // now handle the last period separately
             if (_isPeriodProven(dataSetId, endingPeriod, currentPeriod)) {
-                // then the epochs to add = `endingPeriod_starting` to `toEpoch`. But since `endingPeriod_starting` is simply the ending of its previous period + 1, so  epochs : (`deadline + 1` -> `to`)
+                // then the epochs to add = `endingPeriodStarting` to `toEpoch`. But since `endingPeriodStarting` is simply the ending of its previous period + 1, so  epochs : (`deadline + 1` -> `to`)
                 provenEpochCount += (toEpoch - _calcPeriodDeadline(dataSetId, endingPeriod - 1));
                 lastProvenEpoch = toEpoch;
             }
@@ -1533,7 +1536,7 @@ contract FilecoinWarmStorageService is
     }
 
     function _calcPeriodDeadline(uint256 dataSetId, uint256 periodId) private view returns (uint256) {
-        return provingActivationEpoch[dataSetId] + ((periodId + 1) * maxProvingPeriod); // we need to do `periodId + 1` since it starts from 0
+        return provingActivationEpoch[dataSetId] + (periodId + 1) * maxProvingPeriod;
     }
 
     function railTerminated(uint256 railId, address terminator, uint256 endEpoch) external override {

service_contracts/test/FilecoinWarmStorageService.t.sol

@@ -4432,11 +4432,11 @@ contract ValidatePaymentTest is FilecoinWarmStorageServiceTest {
         uint256 proposedAmount = 1000e6;
 
         vm.prank(address(payments));
+        vm.expectRevert(abi.encodeWithSelector(Errors.InvalidEpochRange.selector, fromEpoch, toEpoch));
         IValidator.ValidationResult memory result =
             pdpServiceWithPayments.validatePayment(info.pdpRailId, proposedAmount, fromEpoch, toEpoch, 0);
 
         assertEq(result.modifiedAmount, 0, "Should pay nothing for pre-activation epochs");
-        assertEq(result.settleUpto, fromEpoch, "Should not settle");
     }
 
     /**