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

Passing an array of booleans as input to the circuit is not implemented. #844

Closed
Mickanos opened this issue Feb 14, 2023 · 2 comments · Fixed by #900
Closed

Passing an array of booleans as input to the circuit is not implemented. #844

Mickanos opened this issue Feb 14, 2023 · 2 comments · Fixed by #900
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@Mickanos
Copy link

Problem

Although the documentation does not forbid to pass arrays of booleans as input to the main function of the circuit, the noirc_abi does not handle this situation.
The problem is easy to reproduce:
Create a noir project with main.nr:

fn main(sel: [bool;2], y : pub Field) {
        if sel[0] {
                if sel[1] {
                        constrain y==0;
                } else {
                        constrain y==1;
                }
        } else {
                if sel[1] {
                        constrain y==2;
                } else {
                        constrain y==3;
                }
        }
}

now try the following Prover.toml files:

sel = ["true","false"] 
y = 1
sel = [true,false]
y=1

and

sel = "[true,false]"
y=1

Running nargo prove results in three different error messages depending on each case. In the first case, we get Expected witness values to be integers, provided value causes `invalid digit found in string` error. With the second approach, we get input.toml file is badly formed, could not parse, data did not match any variant of untagged enum TomlTypes for key `sel` at line 1 column 1. Finally, the third approach yields cannot parse a string toml type into Array { length: 2, typ: Boolean }.

Solution

Getting each approach to work requires different fixes.
The first format requires implementing a function parse_str_to_bool and using it in the TomlTypes::ArrayString(arr_str) case of the match in function try_from_toml.

The second format is simpler to implement. Add a variant ArrayBool(Vec<bool>) in the definition of the TomlTypes enum.

I am not sure that the third format is desired, since it is not implemented for arrays of integers either. This would require parsing a string of the form "[,...,]" into an array of the appropriate type. Note however that using nargo check when an array is part of the input of the main function creates a line of the form key = "[]" in Prover.toml, thus encouraging usage of an unsupported syntax.

Alternatives considered

Specify in the Noir book that arrays of booleans are not supported as input to the main function.

@Mickanos Mickanos added the enhancement New feature or request label Feb 14, 2023
@Mickanos Mickanos changed the title Passing an array of booleans as input to the circuit causes an error. Passing an array of booleans as input to the circuit is not implemented. Feb 14, 2023
@TomAFrench
Copy link
Member

Hey @Mickanos, thanks for opening this issue! I think that the second approach would be best and would fit most with the current style.

Would you like to open a PR to address this issue? Ideally the fix would also include a test case for a circuit which takes an array of bools as an argument (You can see examples of this sort of test in https://github.com/noir-lang/noir/tree/master/crates/nargo/tests/test_data)

@TomAFrench TomAFrench added the good first issue Good for newcomers label Feb 14, 2023
@Mickanos
Copy link
Author

Sure, I'll look into it.
This is my first time opening a pull request, so please bear with me while I figure it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants