Skip to content

feat(state): stitch tree_aux root serving across the VCT upgrade height#410

Merged
ValarDragon merged 6 commits into
ironwood-mainfrom
split/vct-root-serving
Jul 2, 2026
Merged

feat(state): stitch tree_aux root serving across the VCT upgrade height#410
ValarDragon merged 6 commits into
ironwood-mainfrom
split/vct-root-serving

Conversation

@p0mvn

@p0mvn p0mvn commented Jul 2, 2026

Copy link
Copy Markdown

Motivation

Split out of the draft #408 so the verified-commitment-trees (VCT) work can land in small, reviewable slices (follows the merged #402). This slice is the producer/serving half of the fast path: how a node derives and serves the per-block commitment-root payload — read-only, no commit-path changes.

Without it, a node that upgrades mid-chain has a commitment_roots_by_height serving index that only starts at its upgrade height U: a tree_aux BlockRoots request that straddles U would return a short index-only prefix and stall the fetch client's minimum-progress check.

Solution

  • Add commitment_aux.rs with the payload types and producer half:
    • FinalFrontiers + the length-prefixed byte format (to_bytes/from_bytes) used by the embedded checkpoint-handoff frontier, with fail-closed bounds-checked parsing.
    • produce_block_roots: derive per-block roots, shielded tx-counts, and the ZIP-244 auth-data root from an archive database's per-height trees and stored blocks.
    • serve_block_roots: the serving entry point — stitch the per-height-tree run below U with the serving index at/above U into one gap-free batch; both sources stop at the first absent height, and a pre-index database falls back to trees for the whole range.
    • produce_final_frontiers_bytes (exported): derive serialized final-frontier fixtures from a database.
  • Route the tree_aux BlockRoots finalized read in service.rs through serve_block_roots instead of the raw index read.
  • Point the existing serve_block_roots_serves_at_or_above_upgrade_from_index test (from feat(state): add VCT docs config and DB storage #402) at the real entry point.

@v12-auditor

v12-auditor Bot commented Jul 2, 2026

Copy link
Copy Markdown

Note

Complete: Audit complete. V12 found one issue worth reviewing.

Open the full results here.

FindingSeverityDetails
F-95114 🟠 High
Empty Ironwood roots served

The finalized root-serving paths build and persist BlockCommitmentRoots with the empty Ironwood root even when the committed Ironwood tree has advanced. NoteCommitmentTrees collects Ironwood note commitments and updates self.ironwood, and finalized storage writes that actual Ironwood tree when it changes. The serving index is populated with ironwood::tree::NoteCommitmentTree::default().root() instead of note_commitment_trees.ironwood.root(), and the new per-height-tree fallback in produce_block_roots hardcodes the same empty root. ReadRequest::BlockRoots now reaches serve_block_roots, which serves either that index or the faulty fallback, and rollback prefers the indexed roots.ironwood_root over the actual per-height Ironwood tree. For Nu7/Ironwood blocks carrying Ironwood commitments, the served and rollback-rebuild root diverges from the real note-commitment tree used by the block's chain-history commitment.

And two more auto-invalidated findings.

Analyzed five files, diff 848f9a8...e326c5d.

@p0mvn

p0mvn commented Jul 2, 2026

Copy link
Copy Markdown
Author

@cursor review

@cursor cursor 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.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit e326c5d. Configure here.

Comment thread .github/actions/setup-zebra-build/action.yml Outdated
Comment thread zebra-state/src/service.rs Outdated
@p0mvn
p0mvn force-pushed the split/vct-root-serving branch from 0302bc7 to d708185 Compare July 2, 2026 18:28
if: runner.os == 'Linux'
shell: bash
run: |
sudo rm -f /etc/apt/sources.list.d/{microsoft-prod,azure-cli}.{list,sources}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Note to reviewer: drive-by CI fix

Comment on lines -1371 to -1374
/// `serve_block_roots` reads a request that starts at or above the upgrade height `U` straight
/// from the serving index, without touching the per-height trees.
#[test]
fn serve_block_roots_serves_at_or_above_upgrade_from_index() {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Note: moved to commitment_aux.rs

@p0mvn

p0mvn commented Jul 2, 2026

Copy link
Copy Markdown
Author

@cursor review

@cursor cursor 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.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 2ed6bc6. Configure here.

@ValarDragon ValarDragon 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.

Nice! Makes sense to me

@ValarDragon
ValarDragon merged commit c046a43 into ironwood-main Jul 2, 2026
47 checks passed
@ValarDragon
ValarDragon deleted the split/vct-root-serving branch July 2, 2026 19:05
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