Skip to content

Migrate append_lifecycle to TLogPolicy - #1162

Merged
AlCutter merged 4 commits into
migrate_tlog_policy_2from
migrate_tlog_policy_3
Sep 10, 2026
Merged

AlCutter merged 4 commits into
migrate_tlog_policy_2from
migrate_tlog_policy_3

Conversation

@AlCutter

@AlCutter AlCutter commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

This PR migrates the append lifecycle code to use the formats/policy package.

Towards #1152

@AlCutter
AlCutter force-pushed the migrate_tlog_policy_3 branch 2 times, most recently from b68766f to 3157237 Compare September 8, 2026 15:09
@AlCutter AlCutter changed the title migrate tlog policy 3 Migrate append_lifecycle to TLogPolicy Sep 8, 2026
@AlCutter
AlCutter force-pushed the migrate_tlog_policy_3 branch from 3157237 to 7ff023c Compare September 8, 2026 15:23
@AlCutter
AlCutter requested a review from roger2hk September 8, 2026 15:24
@AlCutter
AlCutter marked this pull request as ready for review September 8, 2026 16:28
@AlCutter
AlCutter requested a review from a team as a code owner September 8, 2026 16:28
@AlCutter
AlCutter force-pushed the migrate_tlog_policy_3 branch from 7ff023c to 38e50ad Compare September 9, 2026 11:27

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ 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.

@AlCutter
AlCutter force-pushed the migrate_tlog_policy_3 branch from 38e50ad to 05e7891 Compare September 9, 2026 11:31
Comment thread append_lifecycle.go
Comment on lines +841 to 843
if w.URL == nil {
return nil, fmt.Errorf("invalid witness policy: witness %q has no URL", w.Name)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread append_lifecycle.go
Comment on lines 849 to 851
if len(witnesses) == 0 {
return nil, nil
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we return an error here if o.witnessPolicy.Quorum is not none but the len(witnesses) is 0?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, done.

Comment thread README.md Outdated
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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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

Comment thread append_lifecycle_test.go Outdated
return wit
}

type keyUrl struct {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlCutter
AlCutter merged commit 1a0eb0b into main Sep 10, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants