Skip to content

chore: take proof by reference in verify - #3469

Open
PhilippGackstatter wants to merge 5 commits into
nextfrom
pgackst-small-api-improvements
Open

chore: take proof by reference in verify#3469
PhilippGackstatter wants to merge 5 commits into
nextfrom
pgackst-small-api-improvements

Conversation

@PhilippGackstatter

@PhilippGackstatter PhilippGackstatter commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Describe your changes

Some small improvement suggestions coming from the review of 0xMiden/protocol#3492.

  • verify, Verifier::verify, and Verifier::verify_partial now borrow &ExecutionProof and &ExecutionClaim instead of consuming them. Consuming them forced any caller that still needs them to clone the entire proof (serialized size 114 KB).
    • verify_partial is included for consistency, since it has the same shape and sits next to verify in the public API.
  • Renames two AdviceMutation fields to say what they hold:
    • ExtendMap { other } becomes ExtendMap { map }
    • ExtendMerkleStore { infos } becomes ExtendMerkleStore { inner_nodes }

Checklist before requesting a review

  • Repo forked and branch created from next according to naming convention.
  • Commit messages and codestyle follow conventions.
  • Commits are signed.
  • Relevant issues are linked in the PR description.
  • Tests added for new functionality.
  • Documentation/comments updated according to changes.
  • Updated CHANGELOG.md

@PhilippGackstatter
PhilippGackstatter marked this pull request as ready for review August 5, 2026 11:59

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

Looks good! Thank you!

@huitseeker huitseeker left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

Comment thread benches/synthetic-bench/benches/synthetic_bench.rs Outdated
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