Skip to content

Generate subtree inclusion proofs#226

Merged
phbnf merged 5 commits into
transparency-dev:mainfrom
phbnf:subtreeinclusion
Jun 3, 2026
Merged

Generate subtree inclusion proofs#226
phbnf merged 5 commits into
transparency-dev:mainfrom
phbnf:subtreeinclusion

Conversation

@phbnf
Copy link
Copy Markdown
Contributor

@phbnf phbnf commented Jun 1, 2026

Towards #225.

This PR introduces SubtreeInclusion and allows generating inclusion proofs for a leaf in a given subtree. Verification will come in a later PR.

This implementation:

  • checks that the inputs are valid
  • shifts the subtree to make it start at 0
  • fetches the corresponding inclusion proof nodes
  • shifts all nodes back to their original position

The tests for this function are based on the existing tests of Inclusion, with the addition of:

  • Pathological cases
  • Existing test cases, all shifted by bit_ceil(end-start). I believe this is enough. Specifically, I don't think that we need to add tests shifted by x*bit_ceil(end-start). We can always add more tests later if need be. We should probably add subtree tests inside testonly/ as well.

Alternatives explored:

  • non-shifting: by modifying the nodes function to accept start and end index. It added complexity to that function without clear benefits. I doubt that the performance would be noticeable.
  • with the current approach, make Inclusion call SubtreeInclusion: this works, but adds unnecessary checks to Inclusion. I like how simple Inclusion is today.
  • with the current approach, in SubtreeInclusion, run some input validation checks, then call Inclusion which would call further input validation tests, then shift nodes back. This would just make error message more confusing, with little benefits.

Let me know what you think, and we'll take it from there.

@phbnf phbnf requested a review from AlCutter June 1, 2026 09:05
@phbnf phbnf requested a review from a team as a code owner June 1, 2026 09:05
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 1, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 57.68%. Comparing base (4ebea17) to head (ac00877).
⚠️ Report is 122 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #226       +/-   ##
===========================================
- Coverage   89.33%   57.68%   -31.66%     
===========================================
  Files           7        8        +1     
  Lines         497      605      +108     
===========================================
- Hits          444      349       -95     
- Misses         48      251      +203     
  Partials        5        5               

☔ View full report in Codecov by Sentry.
📢 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
Copy link
Copy Markdown
Contributor Author

phbnf commented Jun 1, 2026

codecov is complaining, but it's using 4ebea17 as a baseline, which is over two years old.

@phbnf phbnf force-pushed the subtreeinclusion branch 5 times, most recently from 7b03e29 to ebffc12 Compare June 2, 2026 17:03
@phbnf phbnf force-pushed the subtreeinclusion branch from ebffc12 to ac00877 Compare June 3, 2026 09:02
@phbnf
Copy link
Copy Markdown
Contributor Author

phbnf commented Jun 3, 2026

I had to re-push the branch because commit 604509d6be7b9cdf194a24c213fad2c0a3972d85 still included some failing tests I added yesterday when we ere chatting to trigger edge cases:

TestInclusion
...
		{size: 8, index: 2, want: rehash(1, 2, id(0, 0), id(0, 2))}, // a c
		{size: 4, index: 1, want: rehash(1, 2, id(0, 0), id(0, 2))}, // a c

I've now removed them, but I need another approval.

@phbnf phbnf merged commit 610b863 into transparency-dev:main Jun 3, 2026
16 of 17 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