-
Notifications
You must be signed in to change notification settings - Fork 4
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.
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
Enforce challenge count in simple pdp service #63
Enforce challenge count in simple pdp service #63
Conversation
@anorth please take a look at this next week and leave review / PR improvements. |
function posessionProven(uint256 proofSetId, uint256 challengedLeafCount, uint256 seed, uint256 challengeCount) external onlyPDPVerifier { | ||
receiveProofSetEvent(proofSetId, OperationType.PROVE_POSSESSION, abi.encode(challengedLeafCount, seed, challengeCount)); | ||
emit Debug("Here we go", 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the debug events when you come back to clean up.
} | ||
emit FaultRecord(FaultType.SKIPPED, periodsSkipped); | ||
} | ||
provenThisPeriod[proofSetId] = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This appears to be missing re-setting provingDeadlines[proofSetId]
to a future epoch.
There's a policy decision to be made about which future epoch. I reckon that if it was proven this period, we should add the full proving period to the deadline (so it stays the same time of day). So proving early doesn't lead to disadvantage of needing to prove more frequently over the long term.
But if this period wasn't proven, we probably want to set the deadline based on current epoch, like when the first root is added.
// Warp to just before the deadline | ||
vm.warp(block.number + pdpService.getMaxProvingPeriod() - 1); | ||
pdpService.posessionProven(proofSetId, challengedLeafCount, seed, challengeCount); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the proof should also be accepted on the deadline, so exercise that here.
As part of requirements we want to allow on chain enforcement of challenge count during proving without baking assumptions directly in the pdp verifier.
So we want to put this in the simple pdp service. To do this well wrt testing we want to refactor simple pdp service a bit. This PR extracts record keeper functionality to a base contract that can be shared by simple pdp service and a testing contract that forgoes the complexity of simple pdp service.
This PR also introduces functions on simple pdp service that make discovery of faults easy for off chain observers.