Skip to content

Carry each node's preprocessed root in PackedNode#392

Open
YairVaknin-starkware wants to merge 1 commit into
yairv/retarget_canonical_to_leaf_blfrom
yairv/packed_node_preprocessed_root
Open

Carry each node's preprocessed root in PackedNode#392
YairVaknin-starkware wants to merge 1 commit into
yairv/retarget_canonical_to_leaf_blfrom
yairv/packed_node_preprocessed_root

Conversation

@YairVaknin-starkware

@YairVaknin-starkware YairVaknin-starkware commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Every PackedNode::Composite carries the preprocessed_root of its node's proof (eight little-endian u32 words: the leaf circuit's for leaves, the multiverifier's for fold nodes; extracted via the new ExtractedProofData). The unpacker's hints stay role-agnostic this way: each node's root is looked up in the supported-roots trust list instead of being derived from the node's role, so admitting new circuit types is just appending to that list.

Stack (3/4): prove_leaf_from_files (#390) ← retarget canonical (#391) ← this ← goldens e2e (#388).

🤖 Generated with Claude Code


This change is Reviewable

@cursor

cursor Bot commented Jul 15, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes the serialized packed-output format and how roots are supplied to downstream verification; behavior is localized but any unpacker or golden expecting the old Composite shape must update.

Overview
PackedNode::Composite now includes an eight-word preprocessed_root on the wire (leaf circuit root for leaves, multiverifier root for fold nodes). Leaf and internal builders take that root explicitly; fold reduction pulls it from each fresh proof via ExtractedProofData and digest_bytes_to_words.

This lets the unpacker use each node’s own root against a supported-roots list instead of inferring roots from node role. root_packed.json shape changes; unit and e2e expectations were updated to match.

Reviewed by Cursor Bugbot for commit ae52f97. Bugbot is set up for automated code reviews on this repo. Configure here.

@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.59%. Comparing base (cde498b) to head (ae52f97).

Additional details and impacted files
@@                           Coverage Diff                           @@
##           yairv/retarget_canonical_to_leaf_bl     #392      +/-   ##
=======================================================================
+ Coverage                                65.47%   65.59%   +0.11%     
=======================================================================
  Files                                       45       45              
  Lines                                     5886     5906      +20     
=======================================================================
+ Hits                                      3854     3874      +20     
  Misses                                    2032     2032              
Files with missing lines Coverage Δ
...ates/stwo_run_and_prove_recursive_tree/src/fold.rs 98.36% <100.00%> (+0.32%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@OmriEshhar1 OmriEshhar1 left a comment

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.

:lgtm:

@OmriEshhar1 reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on Gali-StarkWare).

@YairVaknin-starkware
YairVaknin-starkware force-pushed the yairv/packed_node_preprocessed_root branch from 982b962 to 83646a1 Compare July 16, 2026 21:20
@YairVaknin-starkware
YairVaknin-starkware force-pushed the yairv/retarget_canonical_to_leaf_bl branch from 92490b4 to 8daaad0 Compare July 16, 2026 21:20

@OmriEshhar1 OmriEshhar1 left a comment

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.

@OmriEshhar1 reviewed 2 files and all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on Gali-StarkWare).

Every `PackedNode::Composite` carries the `preprocessed_root` of its node's
proof (eight little-endian u32 words: the leaf circuit's for leaves, the
multiverifier's for fold nodes; extracted via the new `ExtractedProofData`).
The unpacker's hints stay role-agnostic this way: each node's root is looked
up in the supported-roots trust list instead of being derived from the node's
role, so admitting new circuit types is just appending to that list.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@YairVaknin-starkware
YairVaknin-starkware force-pushed the yairv/retarget_canonical_to_leaf_bl branch from 8daaad0 to cde498b Compare July 20, 2026 08:50
@YairVaknin-starkware
YairVaknin-starkware force-pushed the yairv/packed_node_preprocessed_root branch from 83646a1 to ae52f97 Compare July 20, 2026 08:50

@Gali-StarkWare Gali-StarkWare left a comment

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.

@Gali-StarkWare reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on YairVaknin-starkware).


crates/stwo_run_and_prove_recursive_tree/src/fold.rs line 174 at r3 (raw file):

struct ExtractedProofData {
    preprocessed_root: HashValue<QM31>,
    root_words: [u32; N_RESERVED],

Why do we need both?

Code quote:

    preprocessed_root: HashValue<QM31>,
    root_words: [u32; N_RESERVED],

@Gali-StarkWare Gali-StarkWare left a comment

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.

:lgtm: with question

@Gali-StarkWare made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on YairVaknin-starkware).

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