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.

Sign up for GitHub

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

Jump to bottom

MPCS: Rename CommitmentWithData to CommitmentWithWitness & remove unnecessary trait requirement. #745

Merged
merged 3 commits into from
Dec 13, 2024
Merged

MPCS: Rename CommitmentWithData to CommitmentWithWitness & remove unnecessary trait requirement. #745

merged 3 commits into from
Dec 13, 2024

Conversation

yczhangsjtu
Copy link
Collaborator

@yczhangsjtu yczhangsjtu commented Dec 13, 2024 •
edited
Loading

  • The name "CommitmentWithData" is vague, renamed to CommitmentWithWitness to be clearer
  • It doesn't actually need to be serialized anywhere in the protocol, and never need to create a default instance, remove the unnecessary trait bounds (which doesn't cause any compilation error at all), and to avoid implementing these trait unnecessarily in WHIR wrapper

@yczhangsjtu yczhangsjtu changed the title MPCS: Rename CommitmentWithData to CommitmentWithWitness & remove Serialization requirement. MPCS: Rename CommitmentWithData to CommitmentWithWitness & remove unnecessary trait requirement. Dec 13, 2024
matthiasgoergens
matthiasgoergens approved these changes Dec 13, 2024
View reviewed changes
Copy link
Collaborator

@matthiasgoergens matthiasgoergens left a comment

Choose a reason for hiding this comment

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

Looks good.

@matthiasgoergens
Copy link
Collaborator

Thanks for providing a detailed motivation in the PR description.

hero78119
hero78119 approved these changes Dec 13, 2024
View reviewed changes
Copy link
Collaborator

@hero78119 hero78119 left a comment

Choose a reason for hiding this comment

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

👍

@hero78119 hero78119 merged commit 027f7b0 into master Dec 13, 2024
6 checks passed
@hero78119 hero78119 deleted the feat/mpcs-comm-with-data-no-serialize branch December 13, 2024 02:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hero78119 hero78119 hero78119 approved these changes

@matthiasgoergens matthiasgoergens matthiasgoergens approved these changes

@kunxian-xia kunxian-xia Awaiting requested review from kunxian-xia

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@yczhangsjtu @matthiasgoergens @hero78119