Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Oct 24, 2025
Merged

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

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b7b54ea
allow forward progress through unsettled periods
wjmelements Oct 22, 2025
29cf3b2
add failing test
wjmelements Oct 22, 2025
c448c41
fix test
wjmelements Oct 22, 2025
91f66ec
settleUpTo
wjmelements Oct 23, 2025
a14e411
_provingPeriodForEpoch
wjmelements Oct 23, 2025
4c75bb6
rm redundancy related to provenThisPeriod
wjmelements Oct 23, 2025
93b682d
document test cases
wjmelements Oct 24, 2025
a62e6a7
document why the first period is special
wjmelements Oct 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 5 additions & 20 deletions service_contracts/src/FilecoinWarmStorageService.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -860,7 +860,6 @@ contract FilecoinWarmStorageService is
revert Errors.InvalidChallengeEpoch(dataSetId, minWindow, maxWindow, challengeEpoch);
}
provingDeadlines[dataSetId] = firstDeadline;
provenThisPeriod[dataSetId] = false;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can expect this to already be false when this data set's proving deadline is not initialized


// Initialize the activation epoch when proving first starts
// This marks when the data set became active for proving
Expand Down Expand Up @@ -910,15 +909,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);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is already set by possessionProven

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hah, odd, any idea why it's in here too? hopefully just an oversight and not some functionality I'm not seeing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code has changed over time. Before it was a bitmap, this could set both true and false. Since I also see things initialized to false elsewhere, perhaps the original author did not know that false was the default.

}

provingDeadlines[dataSetId] = nextDeadline;
provenThisPeriod[dataSetId] = false;

Expand Down Expand Up @@ -1469,8 +1459,6 @@ contract FilecoinWarmStorageService is
returns (uint256 provenEpochCount, uint256 settleUpTo)
{
require(toEpoch >= activationEpoch && toEpoch <= block.number, Errors.InvalidEpochRange(fromEpoch, toEpoch));
uint256 currentPeriod = _provingPeriodForEpoch(activationEpoch, block.number);

if (fromEpoch < activationEpoch - 1) {
fromEpoch = activationEpoch - 1;
}
Expand All @@ -1481,39 +1469,36 @@ contract FilecoinWarmStorageService is
uint256 startingPeriodDeadline = _calcPeriodDeadline(activationEpoch, startingPeriod);

if (toEpoch < startingPeriodDeadline) {
if (_isPeriodProven(dataSetId, startingPeriod, currentPeriod)) {
if (_isPeriodProven(dataSetId, startingPeriod)) {
provenEpochCount = toEpoch - fromEpoch;
settleUpTo = toEpoch;
} else {
settleUpTo = fromEpoch;
}
} else {
if (_isPeriodProven(dataSetId, startingPeriod, currentPeriod)) {
if (_isPeriodProven(dataSetId, startingPeriod)) {
provenEpochCount += (startingPeriodDeadline - fromEpoch);
}

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;
}
}
settleUpTo = _calcPeriodDeadline(activationEpoch, endingPeriod - 1);

// handle the last period separately
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// handle the last period separately
// handle the last period separately to account for partial epoch count (toEpoch < deadline)

if (_isPeriodProven(dataSetId, endingPeriod, currentPeriod)) {
if (_isPeriodProven(dataSetId, endingPeriod)) {
provenEpochCount += (toEpoch - settleUpTo);
settleUpTo = toEpoch;
}
}
return (provenEpochCount, settleUpTo);
}

function _isPeriodProven(uint256 dataSetId, uint256 periodId, uint256 currentPeriod) private view returns (bool) {
if (periodId == currentPeriod) {
return provenThisPeriod[dataSetId];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because this is set by possessionProven, we do not need to also check it here. in fact it might be a bug to do so

}
function _isPeriodProven(uint256 dataSetId, uint256 periodId) private view returns (bool) {
uint256 isProven = provenPeriods[dataSetId][periodId >> 8] & (1 << (periodId & 255));
return isProven != 0;
}
Expand Down
Loading