Migrate append_lifecycle to TLogPolicy - #1162
Conversation
b68766f to
3157237
Compare
3157237 to
7ff023c
Compare
7ff023c to
38e50ad
Compare
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.
| Benchmark suite | Current: 002758b | Previous: f1f48c4 | Ratio |
|---|---|---|---|
BenchmarkDedup (github.com/transparency-dev/tessera) |
4657141 ns/op 900392 B/op 18768 allocs/op |
2207022 ns/op 886850 B/op 18650 allocs/op |
2.11 |
BenchmarkDedup (github.com/transparency-dev/tessera) - ns/op |
4657141 ns/op |
2207022 ns/op |
2.11 |
BenchmarkLeafBundle_UnmarshalText (github.com/transparency-dev/tessera/api) |
9226 ns/op 6528 B/op 1 allocs/op |
1351 ns/op 6528 B/op 1 allocs/op |
6.83 |
BenchmarkLeafBundle_UnmarshalText (github.com/transparency-dev/tessera/api) - ns/op |
9226 ns/op |
1351 ns/op |
6.83 |
BenchmarkProofBuilder/InclusionProof/WarmCache (github.com/transparency-dev/tessera/client) |
139901 ns/op 15382 B/op 180 allocs/op |
56548 ns/op 15353 B/op 180 allocs/op |
2.47 |
BenchmarkProofBuilder/InclusionProof/WarmCache (github.com/transparency-dev/tessera/client) - ns/op |
139901 ns/op |
56548 ns/op |
2.47 |
BenchmarkProofBuilder/InclusionProof/ColdCache (github.com/transparency-dev/tessera/client) |
1547300 ns/op 689078 B/op 3554 allocs/op |
554836 ns/op 689106 B/op 3555 allocs/op |
2.79 |
BenchmarkProofBuilder/InclusionProof/ColdCache (github.com/transparency-dev/tessera/client) - ns/op |
1547300 ns/op |
554836 ns/op |
2.79 |
BenchmarkProofBuilder/ConsistencyProof/WarmCache (github.com/transparency-dev/tessera/client) |
166212 ns/op 15475 B/op 180 allocs/op |
56420 ns/op 15411 B/op 180 allocs/op |
2.95 |
BenchmarkProofBuilder/ConsistencyProof/WarmCache (github.com/transparency-dev/tessera/client) - ns/op |
166212 ns/op |
56420 ns/op |
2.95 |
This comment was automatically generated by workflow using github-action-benchmark.
38e50ad to
05e7891
Compare
| if w.URL == nil { | ||
| return nil, fmt.Errorf("invalid witness policy: witness %q has no URL", w.Name) | ||
| } |
There was a problem hiding this comment.
Should we also add the nil check for w.Verifier just in case it is nil?
if w.Verifier == nil {
return nil, fmt.Errorf("invalid witness policy: witness %q has no verifier", w.Name)
}There was a problem hiding this comment.
Sure, added, but just wanted to point out that this (URL) one is required because URLs are technically optional in tlog-policy (although, obviously, not for a policy used by a log when signing checkpoints), so we could end up here even though the parser succeeded.
| if len(witnesses) == 0 { | ||
| return nil, nil | ||
| } |
There was a problem hiding this comment.
Should we return an error here if o.witnessPolicy.Quorum is not none but the len(witnesses) is 0?
| 1. Create a new Checkpoint and sign it with the signer provided by [WithCheckpointSigner](https://pkg.go.dev/github.com/transparency-dev/tessera#AppendOptions.WithCheckpointSigner) | ||
| 2. Contact witnesses and collect enough cosignatures to satisfy any witness policy configured by [WithWitnesses](https://pkg.go.dev/github.com/transparency-dev/tessera#AppendOptions.WithWitnesses) | ||
| 2. Contact witnesses and collect enough cosignatures to satisfy any witness policy configured by [WithWitnessPolicy](https://pkg.go.dev/github.com/transparency-dev/tessera#AppendOptions.WithWitnessPolicy) | ||
| 3. If the witness policy is satisfied, make this new Checkpoint public available |
There was a problem hiding this comment.
| 3. If the witness policy is satisfied, make this new Checkpoint public available | |
| 3. If the witness policy is satisfied, make this new Checkpoint publicly available |
| return wit | ||
| } | ||
|
|
||
| type keyUrl struct { |
There was a problem hiding this comment.
This PR migrates the append lifecycle code to use the
formats/policypackage.Towards #1152