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

In fn logic_gate() line 176, the true branch will never reach. #414

Closed
3for opened this issue Feb 22, 2021 · 6 comments
Closed

In fn logic_gate() line 176, the true branch will never reach. #414

3for opened this issue Feb 22, 2021 · 6 comments

Comments

@3for
Copy link
Contributor

3for commented Feb 22, 2021

In fn logic_gate() line 176, the true branch will never reach, as i will always be small than num_quads.
image

@3for
Copy link
Contributor Author

3for commented Feb 23, 2021

And why we fix the num_bits to be even? I think it can be odd, since 0 ^ 0 = 0 and 0 & 0 = 0, insert a zero at the head of a_bits and b_bits will be ok?

if num_bits & 1 == 1 {
    a_bits.insert(0, false);
    b_bits.insert(0, false);
    num_quads = num_bits >> 1 + 1;
}

@CPerezz
Copy link
Contributor

CPerezz commented Mar 1, 2021

Hey thanks for the comment!

I wrote this some time ago and it was translated from the C++ impl that Aztec has. See https://github.com/AztecProtocol/barretenberg/blob/9846939c0ce8879385664ac7ec723e0ca73ccd24/barretenberg/src/aztec/plonk/proof_system/widgets/turbo_logic_widget_impl.hpp#L133

We mostly guessed the impl from these comments. So I'll need to take a look again.

What I can tell is:

  • The first assumption is totally correct. Indeed send a PR if you want fixing the num_quads to be num_quads - 1 we would appreciate it!
  • In respect to your second comment. The computations are done in base4 but your suggestion would enable to respect it. The problem is that your statement is valid for the 1 case but I'm not sure what would happen for bigger odd numbers. The variation and padding that would need to be added could maybe leak some info about the number. I would need to take a deeper look to. But definitely it might be possible to improve it!

@3for
Copy link
Contributor Author

3for commented Mar 2, 2021

I think we should change the code from:

           let var_c = match i == num_quads {
                true => self.zero_var,
                false => self.add_input(prod_quad_fr),
            };

to:

         let var_c = self.add_input(prod_quad_fr);

As we've actually add zero var for the last row of the program memory:
https://github.com/dusk-network/plonk/blob/master/src/constraint_system/logic.rs#L210

@CPerezz
Copy link
Contributor

CPerezz commented Mar 2, 2021

Makes sense to me. Would you like to open a PR for this? Otherways we'll do so!

@3for
Copy link
Contributor Author

3for commented Mar 2, 2021

I'll send the PR later.

3for added a commit to 3for/plonk that referenced this issue Mar 2, 2021
3for added a commit to 3for/plonk that referenced this issue Mar 2, 2021
3for added a commit to 3for/plonk that referenced this issue Mar 2, 2021
CPerezz added a commit that referenced this issue Mar 3, 2021
rm uncessary match branch mentioned in #414
@CPerezz
Copy link
Contributor

CPerezz commented Mar 8, 2021

Closed via #420

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

No branches or pull requests

2 participants