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

Rename enum items in ChallengeGenerator #138

Closed
wants to merge 4 commits into from

Conversation

autquis
Copy link
Contributor

@autquis autquis commented Jan 4, 2024

Description

The names of the enum items in ChallengeGenerator are a bit confusing. This PR is a suggestion for renaming them. I think Independent is a more descriptive name than Multivariate. Ditto for Correlated.

This PR is on top of #137

Pointed out by @mmagician


Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (master)
  • Linked to Github issue with discussion and accepted design OR have an explanation in the PR that describes this work.
  • Wrote unit tests
  • Updated relevant documentation in the code
  • Added a relevant changelog entry to the Pending section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

@autquis autquis requested a review from a team as a code owner January 4, 2024 10:57
@autquis autquis requested review from z-tech, Pratyush and weikengchen and removed request for a team January 4, 2024 10:57
@Pratyush
Copy link
Member

Pratyush commented Jan 4, 2024

I think we should get rid of this enum entirely and replace it with squeezing from the sponge. (Or maybe @mmaker's new crate that handles Fiat-Shamir E2E).

Given this, I am somewhat reluctant to merge this PR, and would instead recommend directing efforts towards replacing the ChallengeGenerator usage with sponge.squeeze. What do you all think?

@mmagician
Copy link
Member

Oh yes, that would make our lives much easier, especially for the refactors coming in later .

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