Skip to content

Add subtree signer support - #579

Merged
AlCutter merged 2 commits into
transparency-dev:mainfrom
AlCutter:sign_subtree
Jul 20, 2026
Merged

Add subtree signer support#579
AlCutter merged 2 commits into
transparency-dev:mainfrom
AlCutter:sign_subtree

Conversation

@AlCutter

Copy link
Copy Markdown
Contributor

This PR adds support for the new sign-subtree API recently added to tlog-witness.

@codecov-commenter

codecov-commenter commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 57.53425% with 62 lines in your changes missing coverage. Please review.
✅ Project coverage is 27.40%. Comparing base (7c01d92) to head (1b8d9a4).

Files with missing lines Patch % Lines
witness/http.go 51.42% 28 Missing and 6 partials ⚠️
witness/witness.go 64.86% 19 Missing and 7 partials ⚠️
omniwitness/omniwitness.go 0.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #579      +/-   ##
==========================================
+ Coverage   25.40%   27.40%   +2.00%     
==========================================
  Files          28       28              
  Lines        2047     2189     +142     
==========================================
+ Hits          520      600      +80     
- Misses       1413     1461      +48     
- Partials      114      128      +14     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@AlCutter
AlCutter force-pushed the sign_subtree branch 3 times, most recently from a14f500 to edafca5 Compare July 10, 2026 16:52
@AlCutter
AlCutter requested a review from roger2hk July 10, 2026 16:52
@AlCutter
AlCutter marked this pull request as ready for review July 10, 2026 16:53
Comment thread witness/http.go Outdated
}
var start, end uint64
if n, err := fmt.Sscanf(string(rangeLine), "subtree %d %d", &start, &end); err != nil || n != 2 {
return 0, 0, nil, nil, nil, err

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.

If err is nil and n != 2, the err returned will be nil. Would this cause any issue?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch, updated to ensure we return a non-nil error.

Comment thread witness/witness.go
Comment on lines +451 to +452
name := s.Name()
hash := s.KeyHash()

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.

In the signChkpt(), the following code is called to verify the signer name. I think we need to have the same validation here.

if !isValidSignerName(name) {
	return nil, nil, errors.New("invalid signer")
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added.

Comment thread witness/witness.go Outdated
return len(w.subtreeSigners) > 0
}

func (w *Witness) subtreeVerifiers() []note.Verifier {

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.

Are the verifiers the same for all requests in the same witness? If so, we can initialize them once and store it somewhere to reuse.

Comment thread witness/witness.go Outdated
}

// isSubtreeValid returns whether a subtree covers a valid range.
// A subtree is valid if there exist a parent tree node to:

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
// A subtree is valid if there exist a parent tree node to:
// A subtree is valid if there exists a parent tree node to:

Comment thread witness/witness.go Outdated
return fmt.Errorf("start %d must be 0 when subtree length %d > 1<<63", start, l)
}
if bc := bitCeil(l); start&(bc-1) != 0 {
return fmt.Errorf("start %d not a multiple of bit_ceil(end - start) = %d", start, bc)

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
return fmt.Errorf("start %d not a multiple of bit_ceil(end - start) = %d", start, bc)
return fmt.Errorf("start %d not a multiple of bitCeil(end - start) = %d", start, bc)

@AlCutter
AlCutter merged commit 2e1c697 into transparency-dev:main Jul 20, 2026
14 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.

3 participants