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

Re-add add_constant_witness method for the composer. #280

Closed
CPerezz opened this issue Aug 11, 2020 · 4 comments
Closed

Re-add add_constant_witness method for the composer. #280

CPerezz opened this issue Aug 11, 2020 · 4 comments
Assignees
Labels
status:invalid covers obsolete tasks, nonsensical issues, stuff that don't make sense (anymore)

Comments

@CPerezz
Copy link
Contributor

CPerezz commented Aug 11, 2020

The method was removed in #278 and there were no issues/actions which support this.
See: a8d8f3e#r41368517

The method was useful and is used across a lot of repos in the v_0.2.6 so I think we should bring it back. Idk if it was an error or not, but definitely should still be in the codebase.

@CPerezz CPerezz added need:investigation status:invalid covers obsolete tasks, nonsensical issues, stuff that don't make sense (anymore) labels Aug 11, 2020
@kevaundray
Copy link
Contributor

Hi,

This method was introduced in 564e969

I believe this method is/will cause a bug;

  • The method states that you are constraining a public input to a witness. However, it is constraining a selector polynomial value to the witness.

  • There is already an identical method named: add_witness_to_circuit_descriptionwhich does the same thing.

The consequence of using add_constant_witness and believing that you are constraining public inputs, will be that each statement will create a new circuit. This is because selector polynomials define the circuit.

Solution

If you need to constrain a public input, I suggest a new method called constrain_public_input or add_public_input or something more similar. Then exposing that instead.

@kevaundray
Copy link
Contributor

Could you possibly link the places which were using this method, so that I can double check the usage later on?

@CPerezz
Copy link
Contributor Author

CPerezz commented Aug 13, 2020

@kevaundray was right and there's a method that does the exact same thing called: add_witness_to_circuit_description but it asks fora variable and a Scalar when it could just ask for a Scalar and return the Variable. So I'll create an issue in order to address that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:invalid covers obsolete tasks, nonsensical issues, stuff that don't make sense (anymore)
Projects
None yet
Development

No branches or pull requests

2 participants