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
+63 −41
Merged

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

Show file tree
Hide file tree
Changes from 3 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
12 changes: 5 additions & 7 deletions service_contracts/src/FilecoinWarmStorageService.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1443,26 +1443,22 @@ contract FilecoinWarmStorageService is
}

// Count proven epochs and find the last proven epoch
(uint256 provenEpochCount, uint256 lastProvenEpoch) =
(uint256 provenEpochCount, uint256 settledUpTo) =
Copy link
Collaborator

@rvagg rvagg Oct 23, 2025

Choose a reason for hiding this comment

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

I feel like "settled" isn't the right language for (and in) _findProvenEpochs, maybe "settleUpTo" as an instruction would be more accurate? Or even "provenUpTo" as a description—although that's tricky because it's not necessarily all proven, it's more like "this is the last historical epoch within which you can make a settlement". The event itself has "settle" so I think it's just this internal usage that's making me trip over what on earth _findProvenEpochs is trying to do for us.

Copy link
Contributor Author

@wjmelements wjmelements Oct 23, 2025

Choose a reason for hiding this comment

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

I think settle is the right word based on how it's used in settleRail. Once the period is faulted it's not going to become unfaulted, so we should mark progress, and that does settle it.

Copy link
Collaborator

@rvagg rvagg Oct 23, 2025

Choose a reason for hiding this comment

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

k, well let's remove the "d" from here and in _findProvenEpochs, it's odd

Copy link
Contributor Author

@wjmelements wjmelements Oct 23, 2025

Choose a reason for hiding this comment

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

91f66ec

_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: settledUpTo,
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: settledUpTo, note: ""});
}

function _findProvenEpochs(uint256 dataSetId, uint256 fromEpoch, uint256 toEpoch, uint256 activationEpoch)
Expand All @@ -1486,6 +1482,8 @@ contract FilecoinWarmStorageService is
if (_isPeriodProven(dataSetId, startingPeriod, currentPeriod)) {
provenEpochCount = toEpoch - fromEpoch;
settledUpTo = toEpoch;
} else {
settledUpTo = fromEpoch;
}
} else {
if (_isPeriodProven(dataSetId, startingPeriod, currentPeriod)) {
Expand Down
30 changes: 28 additions & 2 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,43 @@ 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
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, "");

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");

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");

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");

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");
}

/**
Expand Down
Loading