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

Compile time global arithmetic failing in aztec-packages #5269

Closed
michaeljklein opened this issue Jun 17, 2024 · 2 comments
Closed

Compile time global arithmetic failing in aztec-packages #5269

michaeljklein opened this issue Jun 17, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@michaeljklein
Copy link
Contributor

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

  1. checkout fun-with-constants-2
  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"
  1. run aztec-packages/noir-projects/noir-protocol-circuits/bootstrap.sh

Project Impact

Blocker

Impact Context

Currently blocking AztecProtocol/aztec-packages#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

@michaeljklein michaeljklein added the bug Something isn't working label Jun 17, 2024
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Jun 17, 2024
@jfecher
Copy link
Contributor

jfecher commented Jun 17, 2024

Worth noting the elaborator errors have since been fixed.
#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
Copy link
Contributor

jfecher commented Jun 17, 2024

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.

@jfecher jfecher closed this as completed Jun 17, 2024
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

No branches or pull requests

2 participants