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

Show file tree
Hide file tree
Changes from all commits
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
61 changes: 23 additions & 38 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 @@ -1126,8 +1116,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
Expand Down Expand Up @@ -1442,78 +1434,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
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)) {
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];
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
43 changes: 40 additions & 3 deletions service_contracts/test/FilecoinWarmStorageService.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4312,17 +4312,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");
}

/**
Expand Down
Loading