Compile time global arithmetic failing in aztec-packages

michaeljklein commented 3 months ago

Aim

Attempting to compile noir-protocol-circuits' types package with Jan's modifications: https://github.com/AztecProtocol/aztec-packages/blob/fun-with-constants-2/noir-projects/noir-protocol-circuits/crates/types/src/constants.nr#L50

// DOES NOT COMPILE
global MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX: u32 = 32 - 1;
// COMPILES
// global MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX: u32 = 31;

Expected Behavior

global Foo: u32 = 32 - 1; should evaluate to a constant and behave equivalently to global Foo: u32 = 31;

Bug

Currently, it appears that multiple errors are being thrown and only some of them are making it to the user:

When running on master, it fails with two groups of errors:

Global failed to resolve:

error: Cannot find a global or generic type parameter named `plain::MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX`
   ┌─ /Users/michaelklein/Coding/rust/aztec-packages/noir-projects/noir-protocol-circuits/crates/types/src/abis/accumulated_data/combined_accumulated_data.nr:23:67
   │
23 │     sorted_public_data_update_requests: [PublicDataUpdateRequest; MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX],
   │                                                                   -------------------------------------- Only globals or generic type parameters are allowed to be used as an array type's length
   │

Array length failed to resolve (and defaulted to 0):

error: Expected type [u32; 0], found type [u32; 31]
    ┌─ /Users/michaelklein/Coding/rust/aztec-packages/noir-projects/noir-protocol-circuits/crates/public-kernel-lib/src/public_kernel_tail.nr:335:33
    │  
335 │               let combine_hints = CombineHints {
    │ ╭─────────────────────────────────'
336 │ │                 sorted_note_hashes,
337 │ │                 sorted_note_hashes_indexes,
338 │ │                 sorted_unencrypted_logs_hashes,
    · │
343 │ │                 deduped_public_data_update_requests_runs
344 │ │             };
    │ ╰─────────────'
    │

When using --use-elaborator, it fails with other errors (known to occur in the elaborator):

error: duplicate definitions of to_field found
   ┌─ /Users/michaelklein/Coding/rust/aztec-packages/noir-projects/noir-protocol-circuits/crates/types/src/address/public_keys_hash.nr:9:8
   │
 9 │     fn to_field(self) -> Field {
   │        -------- first definition found here
   ·
37 │     pub fn to_field(self) -> Field {
   │            -------- second definition found here

..

error: No method named 'serialize' found for type 'PublicCircuitPublicInputs'
    ┌─ /Users/michaelklein/Coding/rust/aztec-packages/noir-projects/noir-protocol-circuits/crates/types/src/abis/public_circuit_public_inputs.nr:156:23
    │
156 │         pedersen_hash(self.serialize(), GENERATOR_INDEX__PUBLIC_CIRCUIT_PUBLIC_INPUTS)
    │                       ----------------
    │

error: No method named 'serialize' found for type 'GlobalVariables'
   ┌─ /Users/michaelklein/Coding/rust/aztec-packages/noir-projects/noir-protocol-circuits/crates/types/src/header.nr:38:34
   │
38 │         fields.extend_from_array(self.global_variables.serialize());
   │                                  ---------------------------------

To Reproduce

checkout fun-with-constants-2

modify this line in bootstrap.sh to point to nargo on master

# NARGO=${NARGO:-../../noir/noir-repo/target/release/nargo}
NARGO="YOUR_PATH/noir/target/debug/nargo"

run aztec-packages/noir-projects/noir-protocol-circuits/bootstrap.sh

Project Impact

Blocker

Impact Context

Currently blocking https://github.com/AztecProtocol/aztec-packages/issues/6817

Workaround

Yes

Workaround Description

Manually calculate and replace the 32 - 1 with 31

Additional Context

No response

Installation Method

None

Nargo Version

No response

NoirJS Version

No response

Would you like to submit a PR for this Issue?

None

Support Needs

No response

jfecher commented 3 months ago

Worth noting the elaborator errors have since been fixed. https://github.com/noir-lang/noir/pull/5246 has just merged as well, so the elaborator is now the default. I've verified the code in question tests successfully on current master

jfecher commented 3 months ago

Going to close this issue due to the comment above. I don't think we need to spend time debugging code that only fails while using the legacy compiler passes.

noir-lang / noir