Skip to content

Commit

Permalink
Challenge window restriction (#77)
Browse files Browse the repository at this point in the history 
* Tested and working

* Testing nextChallengeWindow added useful subfunction + refactor

---------

Co-authored-by: zenground0 <ZenGround0@users.noreply.github.com>
  • Loading branch information
@ZenGround0
ZenGround0 and ZenGround0 authored Dec 3, 2024
1 parent 19993af commit c994801
Show file tree
Hide file tree
Showing 4 changed files with 252 additions and 41 deletions.
14 changes: 9 additions & 5 deletions src/PDPVerifier.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ interface PDPListener {
function rootsAdded(uint256 proofSetId, uint256 firstAdded, PDPVerifier.RootData[] memory rootData) external;
function rootsScheduledRemove(uint256 proofSetId, uint256[] memory rootIds) external;
function posessionProven(uint256 proofSetId, uint256 challengedLeafCount, uint256 seed, uint256 challengeCount) external;
function nextProvingPeriod(uint256 proofSetId, uint256 leafCount) external;
function nextProvingPeriod(uint256 proofSetId, uint256 challengeEpoch, uint256 leafCount) external;
}

contract PDPVerifier is Initializable, UUPSUpgradeable, OwnableUpgradeable {
Expand Down Expand Up @@ -411,11 +411,12 @@ contract PDPVerifier is Initializable, UUPSUpgradeable, OwnableUpgradeable {
// So after this method is called roots scheduled for removal are no longer eligible for challenging
// and can be deleted. And roots added in the last proving period must be available for challenging.
//
// Additionally this method forces sampling of a new challenge `challengeFinality` epochs in the future.
// Additionally this method forces sampling of a new challenge. It enforces that the new
// challenge epoch is at least `challengeFinality` epochs in the future.
//
// Note that this method can be called at any time but the pdpListener will likely consider it
// a "fault" or other penalizeable behavior to call this method before calling provePossesion.
function nextProvingPeriod(uint256 setId) public {
function nextProvingPeriod(uint256 setId, uint256 challengeEpoch) public {
require(msg.sender == proofSetOwner[setId], "only the owner can move to next proving period");
// Take removed roots out of proving set
uint256[] storage removals = scheduledRemovals[setId];
Expand All @@ -429,7 +430,10 @@ contract PDPVerifier is Initializable, UUPSUpgradeable, OwnableUpgradeable {
removeRoots(setId, removalsToProcess);
// Bring added roots into proving set
challengeRange[setId] = proofSetLeafCount[setId];
nextChallengeEpoch[setId] = block.number + challengeFinality;
if (challengeEpoch - block.number < challengeFinality) {
revert("challenge epoch must be at least challengeFinality epochs in the future");
}
nextChallengeEpoch[setId] = challengeEpoch;

// Clear next challenge epoch if the set is now empty.
// It will be re-set when new data is added.
Expand All @@ -439,7 +443,7 @@ contract PDPVerifier is Initializable, UUPSUpgradeable, OwnableUpgradeable {

address listenerAddr = proofSetListener[setId];
if (listenerAddr != address(0)) {
PDPListener(listenerAddr).nextProvingPeriod(setId, proofSetLeafCount[setId]);
PDPListener(listenerAddr).nextProvingPeriod(setId, challengeEpoch,proofSetLeafCount[setId]);
}
}

Expand Down
50 changes: 46 additions & 4 deletions src/SimplePDPService.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,42 @@ contract SimplePDPService is PDPListener, PDPRecordKeeper, Initializable, UUPSUp
return 2880;
}

// Number of epochs at the end of a proving period during which a
// proof of possession can be submitted
function challengeWindow() public pure returns (uint256) {
return 60;
}

// The start of the challenge window for the current proving period
function thisChallengeWindowStart(uint256 setId) public view returns (uint256) {
if (provingDeadlines[setId] == 0) {
revert("Proving not yet started");
}

uint256 periodsSkipped;
// Proving period is open 0 skipped periods
if (block.number <= provingDeadlines[setId]) {
periodsSkipped = 0;
} else { // Proving period has closed possibly some skipped periods
periodsSkipped = 1 + (block.number - (provingDeadlines[setId] + 1)) / getMaxProvingPeriod();
}
return provingDeadlines[setId] + periodsSkipped*getMaxProvingPeriod() - challengeWindow();
}

// The start of the NEXT OPEN proving period's challenge window
// Useful for querying before nextProvingPeriod to determine challengeEpoch to submit for nextProvingPeriod
function nextChallengeWindowStart(uint256 setId) public view returns (uint256) {
if (provingDeadlines[setId] == 0) {
revert("Proving not yet started");
}
// If the current period is open this is the next period's challenge window
if (block.number <= provingDeadlines[setId]) {
return thisChallengeWindowStart(setId) + getMaxProvingPeriod();
}
// If the current period is not yet open this is the current period's challenge window
return thisChallengeWindowStart(setId);
}

// Challenges / merkle inclusion proofs provided per proof set
function getChallengesPerProof() public pure returns (uint64) {
return 5;
Expand Down Expand Up @@ -151,8 +187,8 @@ contract SimplePDPService is PDPListener, PDPRecordKeeper, Initializable, UUPSUp
}

// nextProvingPeriod checks for unsubmitted proof and emits a fault if so
function nextProvingPeriod(uint256 proofSetId, uint256 leafCount) external onlyPDPVerifier {
receiveProofSetEvent(proofSetId, OperationType.NEXT_PROVING_PERIOD, abi.encode(leafCount));
function nextProvingPeriod(uint256 proofSetId, uint256 challengeEpoch, uint256 leafCount) external onlyPDPVerifier {
receiveProofSetEvent(proofSetId, OperationType.NEXT_PROVING_PERIOD, abi.encode(challengeEpoch, leafCount));
// Noop when proving period not yet open
// Can only get here if calling nextProvingPeriod multiple times within the same proving period
uint256 prevDeadline = provingDeadlines[proofSetId] - getMaxProvingPeriod();
Expand All @@ -164,9 +200,15 @@ contract SimplePDPService is PDPListener, PDPRecordKeeper, Initializable, UUPSUp
// Proving period is open 0 skipped periods
if (block.number <= provingDeadlines[proofSetId]) {
periodsSkipped = 0;
} else { // Proving period has closed possible some skipped periods
} else { // Proving period has closed possibly some skipped periods
periodsSkipped = (block.number - (provingDeadlines[proofSetId] + 1)) / getMaxProvingPeriod();
}
// ensure next challenge epoch falls within the next challenge window.
// The next challenge window immediately precedes the next deadline
uint256 nextDeadline = provingDeadlines[proofSetId] + getMaxProvingPeriod()*(periodsSkipped+1);
if (challengeEpoch < nextDeadline - challengeWindow() || challengeEpoch > nextDeadline) {
revert("Next challenge epoch must fall within the next challenge window");
}
uint256 faultPeriods = periodsSkipped;
if (!provenThisPeriod[proofSetId]) {
// include previous unproven period
Expand All @@ -175,7 +217,7 @@ contract SimplePDPService is PDPListener, PDPRecordKeeper, Initializable, UUPSUp
if (faultPeriods > 0) {
emit FaultRecord(faultPeriods);
}
provingDeadlines[proofSetId] = provingDeadlines[proofSetId] + getMaxProvingPeriod()*(periodsSkipped+1);
provingDeadlines[proofSetId] = nextDeadline;
provenThisPeriod[proofSetId] = false;
}
}
45 changes: 23 additions & 22 deletions test/PDPVerifier.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ contract PDPVerifierProofSetMutateTest is Test {
uint256 rootId = pdpVerifier.addRoots(setId, roots);
listenerAssert.expectEvent(PDPRecordKeeper.OperationType.ADD, setId);
// flush add
pdpVerifier.nextProvingPeriod(setId);
pdpVerifier.nextProvingPeriod(setId, block.number + challengeFinalityDelay);
listenerAssert.expectEvent(PDPRecordKeeper.OperationType.NEXT_PROVING_PERIOD, setId);

uint256 leafCount = roots[0].rawSize / 32;
Expand All @@ -278,7 +278,7 @@ contract PDPVerifierProofSetMutateTest is Test {
listenerAssert.expectEvent(PDPRecordKeeper.OperationType.ADD, setId);
assertEq(firstId, 0);
// flush add
pdpVerifier.nextProvingPeriod(setId);
pdpVerifier.nextProvingPeriod(setId, block.number + challengeFinalityDelay);
listenerAssert.expectEvent(PDPRecordKeeper.OperationType.NEXT_PROVING_PERIOD, setId);

uint256 expectedLeafCount = roots[0].rawSize / 32 + roots[1].rawSize / 32;
Expand Down Expand Up @@ -365,7 +365,7 @@ contract PDPVerifierProofSetMutateTest is Test {
pdpVerifier.scheduleRemovals(setId, toRemove);
listenerAssert.expectEvent(PDPRecordKeeper.OperationType.REMOVE_SCHEDULED, setId);

pdpVerifier.nextProvingPeriod(setId); // flush
pdpVerifier.nextProvingPeriod(setId, block.number + challengeFinalityDelay); // flush
listenerAssert.expectEvent(PDPRecordKeeper.OperationType.NEXT_PROVING_PERIOD, setId);

assertEq(pdpVerifier.getNextChallengeEpoch(setId), 0);
Expand All @@ -391,7 +391,7 @@ contract PDPVerifierProofSetMutateTest is Test {
toRemove[1] = 2;
pdpVerifier.scheduleRemovals(setId, toRemove);
listenerAssert.expectEvent(PDPRecordKeeper.OperationType.REMOVE_SCHEDULED, setId);
pdpVerifier.nextProvingPeriod(setId); // flush
pdpVerifier.nextProvingPeriod(setId, block.number + challengeFinalityDelay); // flush
listenerAssert.expectEvent(PDPRecordKeeper.OperationType.NEXT_PROVING_PERIOD, setId);

assertEq(pdpVerifier.rootLive(setId, 0), false);
Expand Down Expand Up @@ -431,7 +431,7 @@ contract PDPVerifierProofSetMutateTest is Test {
pdpVerifier.scheduleRemovals(setId, toRemove);
listenerAssert.expectEvent(PDPRecordKeeper.OperationType.REMOVE_SCHEDULED, setId);
// Actual removal does not fail
pdpVerifier.nextProvingPeriod(setId);
pdpVerifier.nextProvingPeriod(setId, block.number + challengeFinalityDelay);
listenerAssert.expectEvent(PDPRecordKeeper.OperationType.NEXT_PROVING_PERIOD, setId);

// Scheduling both unchallengeable and challengeable roots for removal succeeds
Expand All @@ -451,7 +451,7 @@ contract PDPVerifierProofSetMutateTest is Test {
assertEq(false, pdpVerifier.rootChallengable(setId, 1));
pdpVerifier.scheduleRemovals(setId, toRemove2);
listenerAssert.expectEvent(PDPRecordKeeper.OperationType.REMOVE_SCHEDULED, setId);
pdpVerifier.nextProvingPeriod(setId);
pdpVerifier.nextProvingPeriod(setId, block.number + challengeFinalityDelay);
listenerAssert.expectEvent(PDPRecordKeeper.OperationType.NEXT_PROVING_PERIOD, setId);

assertEq(false, pdpVerifier.rootLive(setId, 0));
Expand Down Expand Up @@ -526,8 +526,8 @@ contract TestingRecordKeeperService is PDPListener, PDPRecordKeeper {
receiveProofSetEvent(proofSetId, PDPRecordKeeper.OperationType.PROVE_POSSESSION, abi.encode(challengedLeafCount, seed, challengeCount));
}

function nextProvingPeriod(uint256 proofSetId, uint256 leafCount) external override {
receiveProofSetEvent(proofSetId, PDPRecordKeeper.OperationType.NEXT_PROVING_PERIOD, abi.encode(leafCount));
function nextProvingPeriod(uint256 proofSetId, uint256 challengeEpoch, uint256 leafCount) external override {
receiveProofSetEvent(proofSetId, PDPRecordKeeper.OperationType.NEXT_PROVING_PERIOD, abi.encode(challengeEpoch, leafCount));
}
}

Expand Down Expand Up @@ -576,7 +576,7 @@ contract PDPVerifierProofTest is Test, ProofBuilderHelper {
assertEq(pdpVerifier.getNextChallengeEpoch(setId), challengeEpoch);

// Verify the next challenge is in a subsequent epoch after nextProvingPeriod
pdpVerifier.nextProvingPeriod(setId);
pdpVerifier.nextProvingPeriod(setId, block.number + challengeFinalityDelay);
listenerAssert.expectEvent(PDPRecordKeeper.OperationType.NEXT_PROVING_PERIOD, setId);

assertEq(pdpVerifier.getNextChallengeEpoch(setId), block.number + challengeFinalityDelay);
Expand Down Expand Up @@ -688,7 +688,7 @@ contract PDPVerifierProofTest is Test, ProofBuilderHelper {
vm.mockCall(pdpVerifier.RANDOMNESS_PRECOMPILE(), abi.encode(challengeEpoch), abi.encode(challengeEpoch));
pdpVerifier.provePossession{value: PDPFees.proofFee(proofs.length)}(setId, proofs);
listenerAssert.expectEvent(PDPRecordKeeper.OperationType.PROVE_POSSESSION, setId);
pdpVerifier.nextProvingPeriod(setId); // resample
pdpVerifier.nextProvingPeriod(setId, block.number + challengeFinalityDelay); // resample
listenerAssert.expectEvent(PDPRecordKeeper.OperationType.NEXT_PROVING_PERIOD, setId);

uint nextChallengeEpoch = pdpVerifier.getNextChallengeEpoch(setId);
Expand Down Expand Up @@ -734,7 +734,7 @@ contract PDPVerifierProofTest is Test, ProofBuilderHelper {
pdpVerifier.scheduleRemovals(setId, removeRoots);
listenerAssert.expectEvent(PDPRecordKeeper.OperationType.REMOVE_SCHEDULED, setId);
// flush removes
pdpVerifier.nextProvingPeriod(setId);
pdpVerifier.nextProvingPeriod(setId, block.number + challengeFinalityDelay);
listenerAssert.expectEvent(PDPRecordKeeper.OperationType.NEXT_PROVING_PERIOD, setId);

// Make a new proof that is valid with two roots
Expand Down Expand Up @@ -798,7 +798,7 @@ contract PDPVerifierProofTest is Test, ProofBuilderHelper {
listenerAssert.expectEvent(PDPRecordKeeper.OperationType.CREATE, setId);
pdpVerifier.addRoots(setId, roots);
listenerAssert.expectEvent(PDPRecordKeeper.OperationType.ADD, setId);
pdpVerifier.nextProvingPeriod(setId); // flush adds
pdpVerifier.nextProvingPeriod(setId, block.number + challengeFinalityDelay); // flush adds
listenerAssert.expectEvent(PDPRecordKeeper.OperationType.NEXT_PROVING_PERIOD, setId);
return (setId, trees);
}
Expand All @@ -819,7 +819,7 @@ contract PDPVerifierProofTest is Test, ProofBuilderHelper {
roots[0] = makeRoot(tree, leafCount);
uint256 rootId = pdpVerifier.addRoots(setId, roots);
listenerAssert.expectEvent(PDPRecordKeeper.OperationType.ADD, setId);
pdpVerifier.nextProvingPeriod(setId); // flush adds
pdpVerifier.nextProvingPeriod(setId, block.number + challengeFinalityDelay); // flush adds
listenerAssert.expectEvent(PDPRecordKeeper.OperationType.NEXT_PROVING_PERIOD, setId);
return (tree, rootId);
}
Expand Down Expand Up @@ -889,12 +889,13 @@ contract SumTreeAddTest is Test {
SumTreeInternalTestPDPVerifier pdpVerifier;
TestingRecordKeeperService listener;
uint256 testSetId;
uint256 challengeFinalityDelay = 100;

function setUp() public {
PDPVerifier pdpVerifierImpl = new SumTreeInternalTestPDPVerifier();
bytes memory initializeData = abi.encodeWithSelector(
PDPVerifier.initialize.selector,
100
challengeFinalityDelay
);
MyERC1967Proxy proxy = new MyERC1967Proxy(address(pdpVerifierImpl), initializeData);
pdpVerifier = SumTreeInternalTestPDPVerifier(address(proxy));
Expand Down Expand Up @@ -969,7 +970,7 @@ contract SumTreeAddTest is Test {
// Remove roots in batch
pdpVerifier.scheduleRemovals(testSetId, rootIdsToRemove);
// flush adds and removals
pdpVerifier.nextProvingPeriod(testSetId);
pdpVerifier.nextProvingPeriod(testSetId, block.number + challengeFinalityDelay);
for (uint256 i = 0; i < rootIdsToRemove.length; i++) {
bytes memory zeroBytes;
assertEq(pdpVerifier.getRootCid(testSetId, rootIdsToRemove[i]).data, zeroBytes);
Expand Down Expand Up @@ -1106,7 +1107,7 @@ contract SumTreeAddTest is Test {
pdpVerifier.addRoots(testSetId, rootDataArray);
}
pdpVerifier.scheduleRemovals(testSetId, rootIdsToRemove);
pdpVerifier.nextProvingPeriod(testSetId); //flush removals
pdpVerifier.nextProvingPeriod(testSetId, block.number + challengeFinalityDelay); //flush removals

assertFindRootAndOffset(testSetId, 0, 3, 0);
assertFindRootAndOffset(testSetId, 1, 4, 0);
Expand Down Expand Up @@ -1197,8 +1198,8 @@ contract BadListener is PDPListener {
receiveProofSetEvent(proofSetId, PDPRecordKeeper.OperationType.PROVE_POSSESSION, abi.encode(challengedLeafCount, seed, challengeCount));
}

function nextProvingPeriod(uint256 proofSetId, uint256 leafCount) external view {
receiveProofSetEvent(proofSetId, PDPRecordKeeper.OperationType.NEXT_PROVING_PERIOD, abi.encode(leafCount));
function nextProvingPeriod(uint256 proofSetId, uint256 challengeEpoch, uint256 leafCount) external view {
receiveProofSetEvent(proofSetId, PDPRecordKeeper.OperationType.NEXT_PROVING_PERIOD, abi.encode(challengeEpoch, leafCount));
}

function receiveProofSetEvent(
Expand All @@ -1215,12 +1216,12 @@ contract BadListener is PDPListener {
contract PDPListenerIntegrationTest is Test {
PDPVerifier pdpVerifier;
BadListener badListener;

uint256 constant challengeFinalityDelay = 2;
function setUp() public {
PDPVerifier pdpVerifierImpl = new PDPVerifier();
bytes memory initializeData = abi.encodeWithSelector(
PDPVerifier.initialize.selector,
2
challengeFinalityDelay
);
MyERC1967Proxy proxy = new MyERC1967Proxy(address(pdpVerifierImpl), initializeData);
pdpVerifier = PDPVerifier(address(proxy));
Expand Down Expand Up @@ -1254,7 +1255,7 @@ contract PDPListenerIntegrationTest is Test {

badListener.setBadOperation(PDPRecordKeeper.OperationType.NEXT_PROVING_PERIOD);
vm.expectRevert("Failing operation");
pdpVerifier.nextProvingPeriod(0);
pdpVerifier.nextProvingPeriod(0, block.number + challengeFinalityDelay);
}
}

Expand Down Expand Up @@ -1337,7 +1338,7 @@ contract PDPVerifierE2ETest is Test, ProofBuilderHelper {

vm.mockCall(pdpVerifier.RANDOMNESS_PRECOMPILE(), abi.encode(pdpVerifier.getNextChallengeEpoch(setId)), abi.encode(pdpVerifier.getNextChallengeEpoch(setId)));
pdpVerifier.provePossession{value: PDPFees.proofFee(proofsPP1.length)}(setId, proofsPP1);
pdpVerifier.nextProvingPeriod(setId);
pdpVerifier.nextProvingPeriod(setId, block.number + challengeFinalityDelay);
// CHECK: leaf counts
assertEq(pdpVerifier.getRootLeafCount(setId, 0), leafCountsA[0], "First root leaf count should be the set leaf count");
assertEq(pdpVerifier.getRootLeafCount(setId, 1), 0, "Second root leaf count should be zeroed after removal");
Expand Down
Loading

0 comments on commit c994801

Please sign in to comment.