Skip to content

Commit af644df

Browse files
perf(validatePayment): loop over proving periods (#267)
## Related Issue Closes 252 ( Once completed) ## Current Progress I've implemented the updated looping algorithm in validatePayment as described in the issue - [x] Replaced the existing loop with new algorithm - [ ] Convert provenPeriods to a bitmap - [ ] Remove provenThisPeriod ## What I Need Feedback On **Before I continue with the rest of the issue, I'd like to get feedback on my implementation of the looping algorithm.** Since there were no existing tests for this specific part of the code, I want to make sure my approach is correct before implementing the remaining parts of the issue. Also I would love to help you guys write some tests for the same if required ## Questions for Maintainers - Does the new looping logic match what you had in mind? - Are there any edge cases I should consider? - Any suggestions for improvement before I move forward? ## Changes Made - Modified `FilecoinWarmStorageService.sol` to implement the new loop structure ## Next Steps (after approval) Once you confirm this approach is correct, I'll: - Complete the remaining tasks in #252 - Request a full review **Note:** This is a WIP/Draft PR specifically for early feedback on the algorithm implementation. (Also, just a heads-up, I changed my username from `dev-n-dough` to `parzival1821` --------- Co-authored-by: William Morriss <[email protected]>
1 parent 15eeb6d commit af644df

File tree

5 files changed

+351
-80
lines changed

5 files changed

+351
-80
lines changed

service_contracts/.gitignore

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ node_modules/
1010

1111
# Lock files
1212
package-lock.json
13+
foundry.lock
1314

1415
# Ignore IDEs
15-
.idea
16+
.idea

service_contracts/abi/FilecoinWarmStorageService.abi.json

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -401,30 +401,6 @@
401401
"outputs": [],
402402
"stateMutability": "nonpayable"
403403
},
404-
{
405-
"type": "function",
406-
"name": "isEpochProven",
407-
"inputs": [
408-
{
409-
"name": "dataSetId",
410-
"type": "uint256",
411-
"internalType": "uint256"
412-
},
413-
{
414-
"name": "epoch",
415-
"type": "uint256",
416-
"internalType": "uint256"
417-
}
418-
],
419-
"outputs": [
420-
{
421-
"name": "",
422-
"type": "bool",
423-
"internalType": "bool"
424-
}
425-
],
426-
"stateMutability": "view"
427-
},
428404
{
429405
"type": "function",
430406
"name": "migrate",

service_contracts/src/Errors.sol

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,11 @@ library Errors {
201201
/// @param railId The rail ID
202202
error RailNotAssociated(uint256 railId);
203203

204-
/// @notice The epoch range is invalid (toEpoch must be > fromEpoch)
204+
/// @notice The epoch range is invalid
205+
/// @notice Will be emitted if any of the following conditions is NOT met:
206+
/// @notice 1. fromEpoch must be less than toEpoch
207+
/// @notice 2. toEpoch must be less than block number
208+
/// @notice 3. toEpoch must be greater than the activation epoch
205209
/// @param fromEpoch The starting epoch (exclusive)
206210
/// @param toEpoch The ending epoch (inclusive)
207211
error InvalidEpochRange(uint256 fromEpoch, uint256 toEpoch);

service_contracts/src/FilecoinWarmStorageService.sol

Lines changed: 61 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1141,43 +1141,6 @@ contract FilecoinWarmStorageService is
11411141
return (epoch - activationEpoch) / maxProvingPeriod;
11421142
}
11431143

1144-
/**
1145-
* @notice Checks if a specific epoch has been proven
1146-
* @dev Returns true only if the epoch belongs to a proven proving period
1147-
* @param dataSetId The ID of the data set to check
1148-
* @param epoch The epoch to check
1149-
* @return True if the epoch has been proven, false otherwise
1150-
*/
1151-
function isEpochProven(uint256 dataSetId, uint256 epoch) public view returns (bool) {
1152-
// Check if data set is active
1153-
if (provingActivationEpoch[dataSetId] == 0) {
1154-
return false;
1155-
}
1156-
1157-
// Check if this epoch is before activation
1158-
if (epoch < provingActivationEpoch[dataSetId]) {
1159-
return false;
1160-
}
1161-
1162-
// Check if this epoch is in the future (beyond current block)
1163-
if (epoch > block.number) {
1164-
return false;
1165-
}
1166-
1167-
// Get the period this epoch belongs to
1168-
uint256 periodId = getProvingPeriodForEpoch(dataSetId, epoch);
1169-
1170-
// Special case: current ongoing proving period
1171-
uint256 currentPeriod = getProvingPeriodForEpoch(dataSetId, block.number);
1172-
if (periodId == currentPeriod) {
1173-
// For the current period, check if it has been proven already
1174-
return provenThisPeriod[dataSetId];
1175-
}
1176-
1177-
// For past periods, check the provenPeriods bitmapping
1178-
return 0 != provenPeriods[dataSetId][periodId >> 8] & (1 << (periodId & 255));
1179-
}
1180-
11811144
function max(uint256 a, uint256 b) internal pure returns (uint256) {
11821145
return a > b ? a : b;
11831146
}
@@ -1470,7 +1433,8 @@ contract FilecoinWarmStorageService is
14701433
require(totalEpochsRequested > 0, Errors.InvalidEpochRange(fromEpoch, toEpoch));
14711434

14721435
// If proving wasn't ever activated for this data set, don't pay anything
1473-
if (provingActivationEpoch[dataSetId] == 0) {
1436+
uint256 activationEpoch = provingActivationEpoch[dataSetId];
1437+
if (activationEpoch == 0) {
14741438
return ValidationResult({
14751439
modifiedAmount: 0,
14761440
settleUpto: fromEpoch,
@@ -1479,18 +1443,8 @@ contract FilecoinWarmStorageService is
14791443
}
14801444

14811445
// Count proven epochs and find the last proven epoch
1482-
uint256 provenEpochCount = 0;
1483-
uint256 lastProvenEpoch = fromEpoch;
1484-
1485-
// Check each epoch in the range
1486-
for (uint256 epoch = fromEpoch + 1; epoch <= toEpoch; epoch++) {
1487-
bool isProven = isEpochProven(dataSetId, epoch);
1488-
1489-
if (isProven) {
1490-
provenEpochCount++;
1491-
lastProvenEpoch = epoch;
1492-
}
1493-
}
1446+
(uint256 provenEpochCount, uint256 lastProvenEpoch) =
1447+
_findProvenEpochs(dataSetId, fromEpoch, toEpoch, activationEpoch);
14941448

14951449
// If no epochs are proven, we can't settle anything
14961450
if (provenEpochCount == 0) {
@@ -1504,16 +1458,70 @@ contract FilecoinWarmStorageService is
15041458
// Calculate the modified amount based on proven epochs
15051459
uint256 modifiedAmount = (proposedAmount * provenEpochCount) / totalEpochsRequested;
15061460

1507-
// Calculate how many epochs were not proven (faulted)
1508-
uint256 faultedEpochs = totalEpochsRequested - provenEpochCount;
1509-
15101461
return ValidationResult({
15111462
modifiedAmount: modifiedAmount,
15121463
settleUpto: lastProvenEpoch, // Settle up to the last proven epoch
15131464
note: ""
15141465
});
15151466
}
15161467

1468+
function _findProvenEpochs(uint256 dataSetId, uint256 fromEpoch, uint256 toEpoch, uint256 activationEpoch)
1469+
internal
1470+
view
1471+
returns (uint256 provenEpochCount, uint256 settledUpTo)
1472+
{
1473+
require(toEpoch >= activationEpoch && toEpoch <= block.number, Errors.InvalidEpochRange(fromEpoch, toEpoch));
1474+
uint256 currentPeriod = getProvingPeriodForEpoch(dataSetId, block.number);
1475+
1476+
if (fromEpoch < activationEpoch - 1) {
1477+
fromEpoch = activationEpoch - 1;
1478+
}
1479+
1480+
uint256 startingPeriod = getProvingPeriodForEpoch(dataSetId, fromEpoch + 1);
1481+
1482+
// handle first period separately
1483+
uint256 startingPeriodDeadline = _calcPeriodDeadline(activationEpoch, startingPeriod);
1484+
1485+
if (toEpoch < startingPeriodDeadline) {
1486+
if (_isPeriodProven(dataSetId, startingPeriod, currentPeriod)) {
1487+
provenEpochCount = toEpoch - fromEpoch;
1488+
settledUpTo = toEpoch;
1489+
}
1490+
} else {
1491+
if (_isPeriodProven(dataSetId, startingPeriod, currentPeriod)) {
1492+
provenEpochCount += (startingPeriodDeadline - fromEpoch);
1493+
}
1494+
1495+
uint256 endingPeriod = getProvingPeriodForEpoch(dataSetId, toEpoch);
1496+
// loop through the proving periods between startingPeriod and endingPeriod
1497+
for (uint256 period = startingPeriod + 1; period < endingPeriod; period++) {
1498+
if (_isPeriodProven(dataSetId, period, currentPeriod)) {
1499+
provenEpochCount += maxProvingPeriod;
1500+
}
1501+
}
1502+
settledUpTo = _calcPeriodDeadline(activationEpoch, endingPeriod - 1);
1503+
1504+
// handle the last period separately
1505+
if (_isPeriodProven(dataSetId, endingPeriod, currentPeriod)) {
1506+
provenEpochCount += (toEpoch - settledUpTo);
1507+
settledUpTo = toEpoch;
1508+
}
1509+
}
1510+
return (provenEpochCount, settledUpTo);
1511+
}
1512+
1513+
function _isPeriodProven(uint256 dataSetId, uint256 periodId, uint256 currentPeriod) private view returns (bool) {
1514+
if (periodId == currentPeriod) {
1515+
return provenThisPeriod[dataSetId];
1516+
}
1517+
uint256 isProven = provenPeriods[dataSetId][periodId >> 8] & (1 << (periodId & 255));
1518+
return isProven != 0;
1519+
}
1520+
1521+
function _calcPeriodDeadline(uint256 activationEpoch, uint256 periodId) private view returns (uint256) {
1522+
return activationEpoch + (periodId + 1) * maxProvingPeriod;
1523+
}
1524+
15171525
function railTerminated(uint256 railId, address terminator, uint256 endEpoch) external override {
15181526
require(msg.sender == paymentsContractAddress, Errors.CallerNotPayments(paymentsContractAddress, msg.sender));
15191527

0 commit comments

Comments
 (0)