noir-lang / noir

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

Generic traits that return generic arrays don't bind array lengths #4088

Closed sirasistant closed 9 months ago

sirasistant commented 9 months ago

Aim

Doing code like this:

trait Serialize<N> {
    fn serialize(self) -> [Field; N];
}

struct ValueNote {
    value: Field,
}

impl Serialize<1> for ValueNote {
    fn serialize(self) -> [Field; 1] {
        [self.value]
    }
}

fn check<N>(serialized_note: [Field; N]) {
    assert(serialized_note[0] == 0);
}

fn oopsie<Note, N>(note: Note) where Note: Serialize<N> {
    let serialized_note = Note::serialize(note);

    check(serialized_note)
}

fn main(mut note: ValueNote) {
    oopsie(note);
}

Expected Behavior

It should compile!

Bug

Panics with

The application panicked (crashed).
Message:  assertion failed: `(left == right)`
  left: `2`,
 right: `1`
Location: compiler/noirc_evaluator/src/ssa/opt/inlining.rs:420

Workaround: typing annotating the return value:

trait Serialize<N> {
    fn serialize(self) -> [Field; N];
}

struct ValueNote {
    value: Field,
}

impl Serialize<1> for ValueNote {
    fn serialize(self) -> [Field; 1] {
        [self.value]
    }
}

fn check<N>(serialized_note: [Field; N]) {
    assert(serialized_note[0] == 0);
}

fn oopsie<Note, N>(note: Note) where Note: Serialize<N> {
    let serialized_note: [Field; N] = Note::serialize(note); <== This

    check(serialized_note)
}

fn main(mut note: ValueNote) {
    oopsie(note);
}
image

To Reproduce

1. 2. 3. 4.

Installation Method

None

Nargo Version

No response

Additional Context

No response

Would you like to submit a PR for this Issue?

No

Support Needs

No response

Thunkar commented 9 months ago

This issue can also cause the following error (a lot clearer) in the monomorphization step:

Message:  Non-numeric type variable used in expression expecting a value
Location: compiler/noirc_frontend/src/monomorphization/mod.rs:720
sirasistant commented 9 months ago

Another possibly related issue: This fails:

use dep::std::option::Option;

trait MySerialize<N> {
    fn serialize(self) -> [Field; N];
}

trait MyDeserialize<N> {
    fn deserialize(fields: [Field; N]) -> Self;
}

impl MySerialize<1> for Field {
    fn serialize(self) -> [Field; 1] {
        [self]
    }
}

impl MyDeserialize<1> for Field {
    fn deserialize(fields: [Field; 1]) -> Self {
        fields[0]
    }
}

#[oracle(storageRead)]
fn storage_read_oracle<N>(_storage_slot: Field, _number_of_elements: Field) -> [Field; N] {}

unconstrained fn storage_read_oracle_wrapper<N>(_storage_slot: Field) -> [Field; N] {
    storage_read_oracle(_storage_slot, N)
}

pub fn storage_read<N>(storage_slot: Field) -> [Field; N] {
    storage_read_oracle_wrapper(storage_slot)
}

struct PublicState<T> {
    storage_slot: Field,
}

impl<T> PublicState<T> {
    pub fn new(
        storage_slot: Field,
    ) -> Self {
        assert(storage_slot != 0, "Storage slot 0 not allowed. Storage slots must start from 1.");
        PublicState {
            storage_slot,
        }
    }
    pub fn read<T_SERIALIZED_LEN>(self) -> T where T: MyDeserialize<T_SERIALIZED_LEN> {
        let fields: [Field; T_SERIALIZED_LEN] = storage_read(self.storage_slot);
        T::deserialize(fields)
    }
}

fn main(value: Field) {
    let ps: PublicState<Field> = PublicState::new(27);

    assert(ps.read() == value);
}

with

The application panicked (crashed).
Message:  assertion failed: `(left == right)`
  left: `2`,
 right: `1`
Location: compiler/noirc_evaluator/src/ssa/opt/inlining.rs:420

The workaround consists in passing an unused sample array with the length:

use dep::aztec::context::{Context};
use dep::std::option::Option;

trait MySerialize<N> {
    fn serialize(self) -> [Field; N];
}

trait MyDeserialize<N> {
    fn deserialize(fields: [Field; N]) -> Self;
    // Such a hackkity hack!!!!
    fn empty() -> [Field; N];
}

impl MySerialize<1> for Field {
    fn serialize(self) -> [Field; 1] {
        [self]
    }
}

impl MyDeserialize<1> for Field {
    fn deserialize(fields: [Field; 1]) -> Self {
        fields[0]
    }

    fn empty() -> [Field; 1] {
        dep::std::unsafe::zeroed()
    }
}

#[oracle(storageRead)]
fn storage_read_oracle<N>(_storage_slot: Field, _number_of_elements: Field) -> [Field; N] {}

unconstrained fn storage_read_oracle_wrapper<N>(_storage_slot: Field, _sample: [Field; N]) -> [Field; N] {
    storage_read_oracle(_storage_slot, N)
}

pub fn storage_read<N>(storage_slot: Field, _sample: [Field; N]) -> [Field; N] {
    storage_read_oracle_wrapper(storage_slot, _sample)
}

struct PublicState<T> {
    storage_slot: Field,
}

impl<T> PublicState<T> {
    pub fn new(
        storage_slot: Field,
    ) -> Self {
        assert(storage_slot != 0, "Storage slot 0 not allowed. Storage slots must start from 1.");
        PublicState {
            storage_slot,
        }
    }
    pub fn read<T_SERIALIZED_LEN>(self) -> T where T: MyDeserialize<T_SERIALIZED_LEN> {
        let fields: [Field; T_SERIALIZED_LEN] = storage_read(self.storage_slot, T::empty());
        T::deserialize(fields)
    }
}

fn main(value: Field) {
    let ps: PublicState<Field> = PublicState::new(27);

    assert(ps.read() == value);
}

The ugly part of this workaround is that it requires to modify the serialize trait, which would affect users ):

jfecher commented 9 months ago

Workaround: typing annotating the return value:

Another workaround is to call the trait method directly instead of using static method syntax:

trait Serialize<N> {
    fn serialize(self) -> [Field; N];
}

struct ValueNote {
    value: Field,
}

impl Serialize<1> for ValueNote {
    fn serialize(self) -> [Field; 1] {
        [self.value]
    }
}

fn check<N>(serialized_note: [Field; N]) {
    assert(serialized_note[0] == 0);
}

fn oopsie<Note, N>(note: Note) where Note: Serialize<N> {
    let serialized_note = note.serialize(); //ok
    check(serialized_note)
}

fn main(mut note: ValueNote) {
    oopsie(note);
}
jfecher commented 9 months ago

@sirasistant I think your second error is a bit different. Do you mind filing a separate issue?

sirasistant commented 9 months ago

Opened https://github.com/noir-lang/noir/issues/4124