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

Problem with short sized inputs #2

Open
iquerejeta opened this issue Apr 5, 2024 · 1 comment
Open

Problem with short sized inputs #2

iquerejeta opened this issue Apr 5, 2024 · 1 comment
Assignees

Comments

@iquerejeta
Copy link

The implementation of Poseidon has an issue with inputs of size 1.

How to reproduce the bug

Adding the following test to the pow5 test suite will reproduce the error

    #[test]
    fn poseidon_hash_shorter_input() {
        let rng = OsRng;

        let message = [Fp::random(rng)];
        let output =
            poseidon::Hash::<_, OrchardNullifier, ConstantLength<1>, 3, 2>::init().hash(message);

        let k = 7;
        let circuit = HashCircuit::<OrchardNullifier, 3, 2, 1> {
            message: Value::known(message),
            output: Value::known(output),
            _spec: PhantomData,
        };

        // Mock prover runs correctly
        let prover = MockProver::run(k, &circuit, vec![]).unwrap();
        assert_eq!(prover.verify(), Ok(()));

        let params: ParamsIPA<vesta::Affine> = ParamsIPA::new(k);

        let vk = keygen_vk(&params, &circuit).expect("keygen_vk should not fail");
        let pk = keygen_pk(&params, vk, &circuit).expect("keygen_pk should not fail");

        let mut rng = OsRng;
        
        let mut transcript = Blake2bWrite::<_, EqAffine, Challenge255<_>>::init(vec![]);
        create_proof::<IPACommitmentScheme<_>, ProverIPA<_>, _, _, _, _>(
           &params,
           &pk,
           &[circuit],
           &[&[]],
           &mut rng,
           &mut transcript,
        )
           .expect("proof generation should not fail") // <- Proof generation fails with SynthesisError
    }

Fix

Surprisingly enough, the error disappears if we modify the code for load_input_word (line 340) with the following:

                // Load the input into this region.
                let load_input_word = |i: usize| {
                    let (cell, value) = match input.0[i].clone() {
                        Some(PaddedWord::Message(word)) => (word.cell(), word.value().copied()),
                        Some(PaddedWord::Padding(padding_value)) => {
                            let cell = region
                                .assign_fixed(
                                    || format!("load pad_{}", i),
                                    config.rc_b[i],
                                    1,
                                    || Value::known(padding_value),
                                )?
                                .cell();
                            (cell, Value::known(padding_value))
                        }
                        _ => panic!("Input is not padded"),
                    };
                    let var = region.assign_advice(
                        || format!("load input_{}", i),
                        config.state[i],
                        1,
                        || value,
                    )?;
                    region.constrain_equal(cell, var.cell())?;

                    Ok(StateWord(var))
                };

Note that there does not seem to be any apparent difference with the previous code. Previously, we use copy_advice instead of assign_advice and then constrain_equal. However, that is exactly what copy_advice does. I'm quite puzzled, and that is why I haven't opened a PR with the fix, because this might be hiding a bigger issue.

@davidnevadoc
Copy link
Collaborator

I haven't figured out what is going on yet, but I think I narrowed it down a bit:

  // Load the input into this region.
  let load_input_word = |i: usize| {
      let (cell, value) = match input.0[i].clone() {
          Some(PaddedWord::Message(word)) => (word.cell(), word.value().copied()),
          Some(PaddedWord::Padding(padding_value)) => {
              let assigned_cell = region.assign_fixed(
                  || format!("load pad_{}", i),
                  config.rc_b[i],
                  1,
                  || Value::known(padding_value),
              )?;
              // 1. This works
              // (assigned_cell.cell(), Value::known(padding_value))
              // 2. but this doesn't...
              (assigned_cell.cell(), assigned_cell.value().copied())
          }
          _ => panic!("Input is not padded"),
      };
      let var = region.assign_advice(
          || format!("load input_{}", i),
          config.state[i],
          1,
          || value,
      )?;
      region.constrain_equal(cell, var.cell())?;

      Ok(StateWord(var))
  };

Moreover, option 2 fails even if constrain_equal is disabled 🤔

@davidnevadoc davidnevadoc self-assigned this Apr 29, 2024
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