Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: put merlin behind feature #180

Merged
merged 7 commits into from
Sep 26, 2024
Merged

Conversation

JayWhite2357
Copy link
Contributor

@JayWhite2357 JayWhite2357 commented Sep 24, 2024

Rationale for this change

See #161 for rationale.

What changes are included in this PR?

NOTE: the individual commits may be good to review individually

  • DoryMessages accepts impl Transcript instead of merlin::Transcript. This is a breaking change, making the proofs non-backward compatible. The API deos not change.

  • Changed CommitmentEvaluationProof to accept impl Transcript rather than merlin::Transcript.

  • Replaced merlin::Transcript with Keccak256Transcript within QueryProof. Although the proof could be made generic, there is no real use-case for merlin. In the future, we can easily support other transcripts, but I think it is best to keep it concrete for now.

  • Dropped dead code and renamed the old merlin module.

  • Put merlin behind the blitzar feature flag since it is still required for the blitzar::InnerProductProof.

  • The tests are NOT refactored to remove merlin. This can be done in a separate PR.

Are these changes tested?

Yes. Existing tests cover the changes.

@JayWhite2357 JayWhite2357 force-pushed the feat/merlin-behind-feature branch 2 times, most recently from 0c6a460 to 0834695 Compare September 24, 2024 05:04
@JayWhite2357 JayWhite2357 marked this pull request as ready for review September 24, 2024 05:17
@JayWhite2357 JayWhite2357 linked an issue Sep 24, 2024 that may be closed by this pull request
6 tasks
@JayWhite2357 JayWhite2357 changed the title feat: put merlin behind feature feat!: put merlin behind feature Sep 24, 2024
@JayWhite2357 JayWhite2357 force-pushed the feat/merlin-behind-feature branch from 0834695 to d59ec73 Compare September 26, 2024 15:34
Copy link
Contributor

@tlovell-sxt tlovell-sxt left a comment

Choose a reason for hiding this comment

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

LGTM. I wasn't aware of the new transcript/core traits but their improved implementation/ergonomics are definitely on display here

@JayWhite2357 JayWhite2357 merged commit 057edde into main Sep 26, 2024
10 checks passed
@JayWhite2357 JayWhite2357 deleted the feat/merlin-behind-feature branch September 26, 2024 19:38
Copy link

🎉 This PR is included in version 0.24.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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.

Enable using transcripts other than merlin::Transcript.
3 participants