transparency-dev / trillian-tessera

Go library for building tile-based transparency logs (tlogs)
Apache License 2.0
11 stars 11 forks source link

Add `WithCheckpointVerifier` to `tessera.StorageOptions` for parsing checkpoints #83

Closed roger2hk closed 4 months ago

roger2hk commented 4 months ago

21

codecov-commenter commented 4 months ago

Codecov Report

Attention: Patch coverage is 0% with 15 lines in your changes missing coverage. Please review.

Project coverage is 32.67%. Comparing base (46ec9c2) to head (0201b00). Report is 7 commits behind head on main.

Files Patch % Lines
cmd/example-mysql/main.go 0.00% 7 Missing :warning:
log.go 0.00% 7 Missing :warning:
cmd/example-gcp/main.go 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ 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 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

roger2hk commented 4 months ago

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.

Good point. I have refactored the WithCheckpointSigner to WithCheckpointSignerVerifier.