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

Expects negative public inputs #717

Closed
ureeves opened this issue Dec 12, 2022 · 2 comments · Fixed by #718
Closed

Expects negative public inputs #717

ureeves opened this issue Dec 12, 2022 · 2 comments · Fixed by #718
Assignees
Labels
fix:bug Something isn't working team:research

Comments

@ureeves
Copy link
Member

ureeves commented Dec 12, 2022

Describe the bug
The current implementation will produce negative public inputs(PIs). This is incredibly confusing and unexpected. The proving algorithm in the Prover::prove functions produces PIs that are the negative of what is expected, and Verifier::verify expects them to be negative as well.

To Reproduce
Run cargo t with this code to reproduce:

use dusk_plonk::prelude::*;

/// Simple circuit proving that `a + b = c`, with `c` as a public input.
#[derive(Default)]
struct Circ {
    a: BlsScalar,
    b: BlsScalar,
    c: BlsScalar,
}

impl Circuit for Circ {
    fn circuit<C>(&self, composer: &mut C) -> Result<(), Error>
    where
        C: Composer,
    {
        let a = composer.append_witness(self.a);
        let b = composer.append_witness(self.b);

        let constraint = Constraint::new().a(a).b(b).left(1).right(1).public(-self.c);
        composer.append_gate(constraint);

        Ok(())
    }
}

#[test]
fn big_whoops() {
    use rand::prelude::*;

    let rng = &mut StdRng::seed_from_u64(0xbeef);
    let pp = PublicParameters::setup(1 << 4, rng).unwrap();

    let (prover, verifier) =
        Compiler::compile(&pp, b"ureeves").expect("The circuit should compile");

    // 1 + 2 = 3
    let circ = Circ {
        a: BlsScalar::from(1),
        b: BlsScalar::from(2),
        c: BlsScalar::from(3),
    };

    let (proof, pis) = prover
        .prove(rng, &circ)
        .expect("The circuit proves successfully");

    assert_eq!(pis.len(), 1, "There should be one public input");

    // FIXME
    assert_eq!(
        -pis[0],
        BlsScalar::from(3),
        "Plonk expects the PIs to be negative"
    );

    verifier
        .verify(&proof, &pis)
        .expect("Verifying the circuit should succeed");
}

Expected behaviour
I would expect the test to fail on the assert_eq marked with the FIXME comment, but this is not the case.

Additional context
This was discovered during an effort to port of the transfer contract to piecrust.

@ureeves ureeves added fix:bug Something isn't working team:research labels Dec 12, 2022
@moCello
Copy link
Member

moCello commented Dec 21, 2022

the above example does not reproduce the bug since it appends the public input to the circuit in its negative form and so the returned public inputs can be expected to be negative as well.

to reproduce run:

use dusk_plonk::prelude::*;
use rand::rngs::StdRng;
use rand::SeedableRng;

#[derive(Debug, Default)]
pub struct TestCircuit {
    a: BlsScalar,
    b: BlsScalar,
    c: BlsScalar,
}

impl Circuit for TestCircuit {
    fn circuit<C>(&self, composer: &mut C) -> Result<(), Error>
    where
        C: Composer,
    {
        let a = composer.append_witness(self.a);
        let b = composer.append_witness(self.b);

        // append the gate a + b = c twice in a slightly different way:

        // This will result in a positive public input value
        let constraint = Constraint::new().left(1).right(1).a(a).b(b).public(self.c);
        composer.gate_add(constraint);

        // This will result in a negative public input value
        let c = composer.append_public(self.c);
        let constraint = Constraint::new().left(1).right(1).a(a).b(b);
        let result = composer.gate_add(constraint);
        composer.assert_equal(result, c);

        Ok(())
    }
}

fn main() {
    let label = b"transcript-arguments";
    let mut rng = StdRng::seed_from_u64(0xbeef);
    let pp = PublicParameters::setup(1 << 5, &mut rng).expect("failed to setup");

    let (prover, verifier) =
        Compiler::compile::<TestCircuit>(&pp, label).expect("failed to compile circuit");

    let circuit = TestCircuit {
        a: BlsScalar::from(2),
        b: BlsScalar::from(2),
        c: BlsScalar::from(4),
    };

    // Generate the proof and its public inputs
    let (proof, pi) = prover.prove(&mut rng, &circuit).expect("failed to prove");

    // Here the inconsistency becomes apparent:
    // the first public input is as it should, the second is negated
    assert_eq!(pi[0], BlsScalar::from(4));
    assert_eq!(pi[1], -BlsScalar::from(4));

    // Verify the generated proof
    verifier
        .verify(&proof, &pi)
        .expect("failed to verify proof");
}

@moCello
Copy link
Member

moCello commented Dec 21, 2022

In my example above, we want to express the same constraint a + b = c in two different gates.

In the first gate, we append the public input value c directly to the constraint. This results in the first entry of the public inputs vector to be positive as well.

In the second gate we first append the public value to the circuit and then constrain it to be a + b. This makes for the same constraint but at the same time negates the public inputs value for that gate.

This shows that the use of public inputs is inconsistent which results in unexpected behavior.

@moCello moCello self-assigned this Dec 21, 2022
moCello added a commit that referenced this issue Dec 21, 2022
@moCello moCello mentioned this issue Apr 6, 2023
moCello added a commit to dusk-network/rusk that referenced this issue Jul 6, 2023
moCello added a commit to dusk-network/rusk that referenced this issue Jul 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix:bug Something isn't working team:research
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants