-
Notifications
You must be signed in to change notification settings - Fork 10
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
Add WithCheckpointVerifier
to tessera.StorageOptions
for parsing checkpoints
#83
Add WithCheckpointVerifier
to tessera.StorageOptions
for parsing checkpoints
#83
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #83 +/- ##
==========================================
- Coverage 35.80% 32.67% -3.13%
==========================================
Files 16 18 +2
Lines 1363 1469 +106
==========================================
- Hits 488 480 -8
- Misses 801 917 +116
+ Partials 74 72 -2 ☔ View full report in Codecov by Sentry. |
0847da1
to
9408476
Compare
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.
Just a thought, would it make sense to roll this into the existing WithCheckpointSigner()
option?
The existing storage implementations don't need a verifier since they store state internally and just "render" checkpoint from that so never need to verify it, but if there will be other patterns where this isn't case (POSIX too, presumably?) then it'd probably make sense to pop signer & verifier in the same option; it'll avoid bugs where someone passes a signer but not a verifier, and then they only find out when their binary crashes when it tries to integrate.
83d2852
to
dff5805
Compare
dff5805
to
0201b00
Compare
Good point. I have refactored the |
#21