-
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
feat: SimplePDPService: improve the fault record event #84
Conversation
src/SimplePDPService.sol
Outdated
faultPeriods += 1; | ||
} | ||
if (faultPeriods > 0) { | ||
emit FaultRecord(faultPeriods); | ||
uint256[] memory faultedPeriodsList = new uint256[](faultPeriods); |
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 don't think this is good. This event should be constant sized. It is trivial to reconstruct the faultedPeriodsList offchain from constant size. And then there's no issues with the event growing very big when recovering from long standing faults. We should do (provingDeadlines[proofSetId], faultPeriods) or (nextDeadline, faultPeriods)
@@ -163,25 +163,29 @@ contract SimplePDPServiceFaultsTest is Test { | |||
|
|||
function testFaultWithinOpenPeriod() public { | |||
pdpService.rootsAdded(proofSetId, 0, new PDPVerifier.RootData[](0), empty); | |||
|
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.
Where are these noisy edits coming from? If we want to include them we should be running a linting check on CI and fix all of them at once.
Closing in favour of #87 |
Add the
proofsetID
and the actual faulted deadlines to theFaultRecordEvent
.