Add FindSubtrees by AlCutter · Pull Request #250 · transparency-dev/merkle

AlCutter · 2026-07-16T14:36:14Z

This PR adds a FindSubtrees func which implements the MTC algorithm for determining the one or two subtrees which efficiently cover a range of entries.

codecov · 2026-07-16T14:38:39Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 48.01%. Comparing base (600c703) to head (68cd6f2).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #250      +/-   ##
==========================================
+ Coverage   47.16%   48.01%   +0.84%     
==========================================
  Files           8        8              
  Lines         918      933      +15     
==========================================
+ Hits          433      448      +15     
  Misses        476      476              
  Partials        9        9

☔ 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.

phbnf · 2026-07-17T09:16:02Z

+	}{
+		{start: 5, end: 5, wantErr: true},
+		{start: 6, end: 5, wantErr: true},
+		{start: 0, end: 1, want: []Subtree{{Start: 0, End: 1}}},


The FindSubtrees implementation matches 1:1 the reference implementation, so I've got not doubt that it works.

For completeness though, maybe add:

{start: 3, end: 4}: i.e one entry that doesn't start at 0 or at a power of 2

{start:4, end: 6}: i.e a perfect subtree, that still returns two subtrees

Why not? Done :)

phbnf · 2026-07-17T09:34:05Z

+					t.Fatalf("FindSubtrees(%d, %d): %v", start, end, err)
+				}
+				switch l := len(subtrees); l {
+				case 1:


Will this branch ever be executed?

Good spot, removed.

phbnf · 2026-07-17T09:41:58Z

+// FindSubtrees returns one or two subtrees that efficiently cover [start, end).
+// It corresponds to the "Selecting Two Subtrees" procedure described in Section 4.5.1
+// of draft-ietf-plants-merkle-tree-certs.
+func FindSubtrees(start, end uint64) ([]Subtree, error) {


This matches the reference implementation 1:1, and so it makes sense.

However, I fail to understand why the specs are written like this, do you?

§4.5.1 The procedure either returns [start, end) as a subtree, or two subtrees, left and right, that satisfy the following properties, so if [start, end) is a perfect subtree, then the proof will only include one subtree.

The algorithm below already special cases len([start, end)) == 1 and already supports returning a single subtree. Why not replace If end - start is one, return a single subtree, [start, end) with if [start, end) is a valid subtree, return a single subtree [start, end)?

I presume (maybe I'm missing on something?) that all callers of FindSubtrees will call isSubtreeValid first. Should we provide function that does that? (Alternatively, I'm happy to ask on Slack / push to change the specs... but I'm looking forward to hearing what you think first!)

Drive-by. Looks like it does something related to Decompose:

merkle/compact/range.go

Line 248 in 600c703

func Decompose(begin, end uint64) (uint64, uint64) {

but rounds up left and right to a power of 2.

Something like:

left, right := Decompose(begin, end) mid := begin + left left, right = roundPow2(left), roundPow2(right) return mid - left, mid, mid + right

The Decompose function splits the range in a way that left contains all the subtrees whose left sibling is out-of-bounds from the left side. Nicely, all these subtrees are in a prefix of [begin, end). Same, right has all subtrees whose right sibling is out-of-bounds from the right side; and it's a suffix. One of them can be empty/zero (when we are a perfect subtree, or leaning one way).

Because of the siblings property, we can round up and be sure that the left and right subtree still ends/begins at the mid point.

Well, it doesn't round up end, so I guess it's something slightly different.

When you/they say "subtree", is it a perfect subtree? If so, I struggle to see how it's always possible to cover a range with 2 perfect subtrees if the right one is not allowed to cross end. As an example, [0, 7) or [8,15).

So it looks like the 2 subtrees can be non-perfect. Then "efficiently cover [start, end)" means something very specific to this spec. Maybe mention what it means for clarity?

I presume (maybe I'm missing on something?) that all callers of FindSubtrees will call isSubtreeValid first.

I don't think so - there's no requirement that start, end passed into FindSubtrees already describes a valid subtree, or did I miss something?

Yeah, the spec behaves weirdly. For example, it decides to decompose [0,2) even though it's not needed

https://go.dev/play/p/bci8ykKpLiO

I think that all callers who want to decompose a range will have to wrap the call in something along the lines of

Ah, I see what you mean now 👍.
Then it seems like this function should just do that too, inline, and fall-back to the two subtree algo? Otherwise this is a bit of a sharp edge.
I've added a commit which does that and adjusted the test + comments, wdyt?

The thing is: it's not only the power-of-2 that's the corner case. If [begin, end) is already a "right wing" tree, the algorithm will decompose it, but it's possible not to.

For example, for all trees like [0,n), or all trees in [8,n) /* n <= 16 */ it returns 2 subtress, while one fits the definition.

https://go.dev/play/p/JtFPP6oUVsV (same code, commented out the panics)

Hm, I see, that's what isSubtreeValid checks.

I think the specs needs to be disambiguated, I'll ask folks and see what we can do.

AlCutter · 2026-07-17T11:36:02Z

+//   - the returned subtree(s) fully cover the [start, end) range.
+//   - there are no "extra" entries covered past end, but there may be covered entries prior to start.
+//   - the number of entries covered before start is always less than half the size of the first returned subtree.
+func FindSubtrees(start, end uint64) ([]Subtree, error) {


@phbnf @pav-kv Any opinions on returning (start, mid, end, error) instead of adding the struct?

I think the struct is better - otherwise the client would need some extra logic to understand what to do when mid=end.

Returning end seems redundant.

Also, anything breaks with start == end? Empty range -> empty decomposition start == mid == end.

Maybe in some extreme, the error could also be omitted. I imagine almost any func in this stack is checking the bounds. A common way to replace such checks/errors is a type that carries some invariant:

// Span is blah. // Invariant: Begin <= End type Span struct { Begin uint64 End uint64 } func (s Span) check() error { if s.Begin > s.End { // bark } return nil }

Then anyone who accepts this param as an input has been promised the invariant and doesn't need to check it again. Only the topmost caller will check it once.

@pav-kv Sorry, was on autopilot and merged when I got back from lunch - didn't mean to ignore your comments!

Yeah, it would be nice to drop the error and it's a good point about end, and the two do kinda go hand in hand.

@phbnf let's see how you end up using this in the MTC log, perhaps we can circle back and change this if it ends up fitting well there. Maybe the equality checks vs the err check end up being a wash, but worth a look.

@AlCutter np, I am but a driver-by here :D

AlCutter · 2026-07-17T12:27:24Z

+// FindSubtrees returns one or two subtrees that efficiently cover [start, end).
+// It corresponds to the "Selecting Two Subtrees" procedure described in Section 4.5.1
+// of draft-ietf-plants-merkle-tree-certs.
+func FindSubtrees(start, end uint64) ([]Subtree, error) {


I think that all callers who want to decompose a range will have to wrap the call in something along the lines of

Ah, I see what you mean now 👍.
Then it seems like this function should just do that too, inline, and fall-back to the two subtree algo? Otherwise this is a bit of a sharp edge.
I've added a commit which does that and adjusted the test + comments, wdyt?

phbnf · 2026-07-17T12:52:59Z

+//   - the returned subtree(s) fully cover the [start, end) range.
+//   - there are no "extra" entries covered past end, but there may be covered entries prior to start.
+//   - the number of entries covered before start is always less than half the size of the first returned subtree.
+func FindSubtrees(start, end uint64) ([]Subtree, error) {


I think the struct is better - otherwise the client would need some extra logic to understand what to do when mid=end.

phbnf · 2026-07-17T12:53:37Z

+// FindSubtrees returns one or two subtrees that efficiently cover [start, end).
+// It corresponds to the "Selecting Two Subtrees" procedure described in Section 4.5.1
+// of draft-ietf-plants-merkle-tree-certs.
+func FindSubtrees(start, end uint64) ([]Subtree, error) {


I think the specs needs to be disambiguated, I'll ask folks and see what we can do.

Add FindSubtrees support

1c24304

AlCutter requested a review from phbnf July 16, 2026 14:36

AlCutter assigned phbnf Jul 16, 2026

AlCutter requested a review from a team as a code owner July 16, 2026 14:36

phbnf reviewed Jul 17, 2026

View reviewed changes

Address comments

a1f0945

AlCutter force-pushed the find_subtrees branch from 9ddfb4d to a1f0945 Compare July 17, 2026 11:34

Handle case where [start, end) is already a valid subtree

68cd6f2

AlCutter commented Jul 17, 2026

View reviewed changes

phbnf approved these changes Jul 17, 2026

View reviewed changes

AlCutter merged commit 76c956a into transparency-dev:main Jul 17, 2026
18 checks passed

AlCutter deleted the find_subtrees branch July 17, 2026 13:34

Uh oh!

Conversation

AlCutter commented Jul 16, 2026

Uh oh!

codecov Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Jul 16, 2026 •

edited

Loading