noir-lang / noir

Noir is a domain specific language for zero knowledge proofs
https://noir-lang.org
Apache License 2.0
888 stars 198 forks source link

Arithmetic generics cannot simplify `[Field; (N-1) + 1]` to `[Field; N]` #6495

Closed TomAFrench closed 22 hours ago

TomAFrench commented 2 days ago

On the latest sync we've started getting errors of the form

  ./noir-projects+build-mock-protocol-circuits *failed* | error: expected type [Field; N], found type [Field; ((N - 1) + 1)]
  ./noir-projects+build-mock-protocol-circuits *failed* |    ┌─ /usr/src/noir-projects/noir-protocol-circuits/crates/types/src/abis/side_effect.nr:78:27
  ./noir-projects+build-mock-protocol-circuits *failed* |    │
  ./noir-projects+build-mock-protocol-circuits *failed* | 78 │     fn serialize(self) -> [Field; N] {
  ./noir-projects+build-mock-protocol-circuits *failed* |    │                           ---------- expected [Field; N] because of return type
  ./noir-projects+build-mock-protocol-circuits *failed* | 79 │         array_concat(self.inner.serialize(), [self.counter as Field])
  ./noir-projects+build-mock-protocol-circuits *failed* |    │         ------------------------------------------------------------- [Field; ((N - 1) + 1)] returned here
  ./noir-projects+build-mock-protocol-circuits *failed* |    │

In https://github.com/noir-lang/noir/pull/6494 I've added a unit test showing how we fail to canonicalize the length of these arrays.

We should update the canonicalization logic such that the solves_n_plus_one_minus_one_array_length test passes.

TomAFrench commented 2 days ago

~@michaeljklein potentially related to your recent changes?~ Looks like we just weren't canonicalizing array lengths at all.

jfecher commented 2 days ago

Hmm, it's possible the new CheckedCast variant is messing with recursive matches when we try to check for Infix(Infix(N, -, 1), +, 1) since it now could be Infix(CheckedCast(Infix(N, -, 1)), +, 1) or another variant with CheckedCast wrapping the constants as well.