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

feat: Sync from noir #7134

Closed
wants to merge 36 commits into from
Closed

feat: Sync from noir #7134

wants to merge 36 commits into from

Conversation

AztecBot
Copy link
Collaborator

Automated pull of development from the noir programming language, a dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix: update in_contract flag before handling function metadata in elaborator (noir-lang/noir#5292)
fix: fix incorrect return type being applied to stdlib functions modulus_be_bytes(), modulus_be_bits(), etc. (noir-lang/noir#5278)
chore: refactor test case generation in build.rs (noir-lang/noir#5280)
fix: handle struct with nested arrays in oracle return values (noir-lang/noir#5244)
feat: build simple dictionary from inspecting ACIR program (noir-lang/noir#5264)
fix: Add more thorough check for whether a type is valid when passing it from constrained code to unconstrained code (noir-lang/noir#5009)
chore: Pedersen commitment in Noir (noir-lang/noir#5221)
chore: pedersen hash in Noir (noir-lang/noir#5217)
fix: Don't lazily elaborate functions (noir-lang/noir#5282)
fix: avoid unnecessarily splitting expressions with multiplication terms with a shared term (noir-lang/noir#5291)
fix: avoid duplicating constant arrays (noir-lang/noir#5287)
feat: add fuzzer for Noir programs (noir-lang/noir#5251)
feat: Run comptime code from annotations on a type definition (noir-lang/noir#5256)
feat: implement comptime support for as_slice builtin (noir-lang/noir#5276)
chore: create separate crate just for noir artifacts (noir-lang/noir#5162)
feat: add support for wildcard types (noir-lang/noir#5275)
chore: replace logical operators with bitwise in DebugToString (noir-lang/noir#5236)
fix: use proper serialization in AbiValue (noir-lang/noir#5270)
chore: simplify compilation flow to write to file immediately (noir-lang/noir#5265)
feat: implement comptime support for array_len builtin (noir-lang/noir#5272)
chore: Use the elaborator by default (noir-lang/noir#5246)
chore: Release Noir(0.31.0) (noir-lang/noir#5166)
feat!: remove dep:: prefix (noir-lang/noir#4946)
feat: Sync from aztec-packages (noir-lang/noir#5242)
chore: replace is_bn254 implementation to not rely on truncation of literals (noir-lang/noir#5247)
chore: add no-predicate to hash implementations (noir-lang/noir#5253)
feat(experimental): Implement macro calls & splicing into Expr values (noir-lang/noir#5203)
feat: add BoundedVec::map (noir-lang/noir#5250)
chore: add no predicate to poseidon2 (noir-lang/noir#5252)
feat: add set and set_unchecked methods to Vec and BoundedVec (noir-lang/noir#5241)
fix: Disable if optimization (noir-lang/noir#5240)
chore: redo typo PR by dropbigfish (noir-lang/noir#5234)
chore: add property tests for ABI encoding (noir-lang/noir#5216)
chore: thread generics through ACIR/brillig gen (noir-lang/noir#5120)
chore: copy across typo PR script from aztec-packages (noir-lang/noir#5235)
chore(docs): fixing trailing slash issue (noir-lang/noir#5233)
fix: add support for nested arrays returned by oracles (noir-lang/noir#5132)
chore: Parse macros (noir-lang/noir#5229)
chore: Optimize the elaborator (noir-lang/noir#5230)
fix(elaborator): Fix regression introduced by lazy-global changes (noir-lang/noir#5223)
fix(elaborator): Fix duplicate methods error (noir-lang/noir#5225)
chore: fixing all relative paths (noir-lang/noir#5220)
chore: push code related to ABI gen into noirc_driver (noir-lang/noir#5218)
END_COMMIT_OVERRIDE

AztecBot and others added 18 commits June 20, 2024 14:55
…etadata in elaborator (noir-lang/noir#5292)

fix: fix incorrect return type being applied to stdlib functions `modulus_be_bytes()`, `modulus_be_bits()`, etc. (noir-lang/noir#5278)
chore: refactor test case generation in build.rs (noir-lang/noir#5280)
fix: handle struct with nested arrays in oracle return values (noir-lang/noir#5244)
feat: build simple dictionary from inspecting ACIR program (noir-lang/noir#5264)
fix: Add more thorough check for whether a type is valid when passing it from constrained code to unconstrained code (noir-lang/noir#5009)
chore: Pedersen commitment in Noir (noir-lang/noir#5221)
chore: pedersen hash in Noir (noir-lang/noir#5217)
fix: Don't lazily elaborate functions (noir-lang/noir#5282)
fix: avoid unnecessarily splitting expressions with multiplication terms with a shared term (noir-lang/noir#5291)
fix: avoid duplicating constant arrays (noir-lang/noir#5287)
feat: add fuzzer for Noir programs (noir-lang/noir#5251)
feat: Run `comptime` code from annotations on a type definition (noir-lang/noir#5256)
feat: implement comptime support for `as_slice` builtin (noir-lang/noir#5276)
chore: create separate crate just for noir artifacts (noir-lang/noir#5162)
feat: add support for wildcard types (noir-lang/noir#5275)
chore: replace logical operators with bitwise in `DebugToString` (noir-lang/noir#5236)
fix: use proper serialization in `AbiValue` (noir-lang/noir#5270)
chore: simplify compilation flow to write to file immediately (noir-lang/noir#5265)
feat: implement comptime support for `array_len` builtin (noir-lang/noir#5272)
chore: Use the elaborator by default (noir-lang/noir#5246)
chore: Release Noir(0.31.0) (noir-lang/noir#5166)
feat!: remove `dep::` prefix (noir-lang/noir#4946)
feat: Sync from aztec-packages (noir-lang/noir#5242)
chore: replace `is_bn254` implementation to not rely on truncation of literals (noir-lang/noir#5247)
chore: add no-predicate to hash implementations (noir-lang/noir#5253)
feat(experimental): Implement macro calls & splicing into `Expr` values (noir-lang/noir#5203)
feat: add BoundedVec::map (noir-lang/noir#5250)
chore: add no predicate to poseidon2 (noir-lang/noir#5252)
feat: add `set` and `set_unchecked` methods to `Vec` and `BoundedVec` (noir-lang/noir#5241)
fix: Disable `if` optimization (noir-lang/noir#5240)
chore: redo typo PR by dropbigfish (noir-lang/noir#5234)
chore: add property tests for ABI encoding (noir-lang/noir#5216)
chore: thread generics through ACIR/brillig gen (noir-lang/noir#5120)
chore: copy across typo PR script from aztec-packages (noir-lang/noir#5235)
chore(docs): fixing trailing slash issue (noir-lang/noir#5233)
fix: add support for nested arrays returned by oracles (noir-lang/noir#5132)
chore: Parse macros (noir-lang/noir#5229)
chore: Optimize the elaborator (noir-lang/noir#5230)
fix(elaborator): Fix regression introduced by lazy-global changes (noir-lang/noir#5223)
fix(elaborator): Fix duplicate methods error (noir-lang/noir#5225)
chore: fixing all relative paths (noir-lang/noir#5220)
chore: push code related to ABI gen into `noirc_driver` (noir-lang/noir#5218)
* master: (62 commits)
  refactor: note hashing gate optimizations (#7130)
  chore: fix migration notes (#7133)
  feat(p2p): more comprehensive peer management, dial retries, persistence fix (#6953)
  feat: devnet deployments (#7024)
  chore(avm): renamings and comments (#7128)
  refactor: `destroy_note(...)` optimization (#7103)
  git subrepo push --branch=master noir-projects/aztec-nr
  git_subrepo.sh: Fix parent in .gitrepo file. [skip ci]
  chore: replace relative paths to noir-protocol-circuits
  git subrepo push --branch=master barretenberg
  chore: Ultra flavor cleanup (#7070)
  feat: Several updates in SMT verification module (#7105)
  fix: Fix bug for a unit test in full proving mode repated to MSM (#7104)
  feat!: make note_getter return BoundedVec instead of an Option array (#7050)
  chore: Remove unneeded public input folding (#7094)
  chore: reads the return data (#6669)
  test: Create workflow for full AVM tests (#7051)
  chore: Fix noir-projects dockerfile for CircleCI (#7093)
  fix: export event selector and replace function selector with event selector where appropriate (#7095)
  chore(avm): remove avm prefix from pil and executor (#7099)
  ...
Copy link
Contributor

github-actions bot commented Jun 20, 2024

Changes to circuit sizes

Generated at commit: 2ead89a47ad98753494b07f5ee15c5d686cb67a7, compared to commit: 33ccf1b61260868e6bb027b7838ef530717bff01

🧾 Summary (100% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
rollup_base +393,777 ❌ +116.12% +21,006,387 ❌ +678.33%
private_kernel_reset +299,292 ❌ +123.64% +17,169,149 ❌ +674.72%
private_kernel_reset_big +149,532 ❌ +83.60% +8,584,389 ❌ +504.10%
public_kernel_tail +516,811 ❌ +42.47% +27,090,896 ❌ +499.18%
private_kernel_reset_medium +74,652 ❌ +50.69% +4,292,009 ❌ +334.78%
private_kernel_reset_small +37,212 ❌ +28.30% +2,145,819 ❌ +200.24%
private_kernel_tail +11,999 ❌ +54.61% +484,639 ❌ +36.25%
public_kernel_app_logic +24,261 ❌ +7.12% +463,098 ❌ +22.86%
public_kernel_teardown +24,285 ❌ +7.13% +463,140 ❌ +22.86%
private_kernel_tail_to_public +11,944 ❌ +2.60% +482,811 ❌ +22.79%
public_kernel_setup +11,373 ❌ +4.05% +306,968 ❌ +17.03%
rollup_root +2,023 ❌ +94.31% +144,483 ❌ +15.86%
private_kernel_inner +5,793 ❌ +10.83% +120,721 ❌ +9.34%
private_kernel_init +498 ❌ +1.62% +25,488 ❌ +6.52%
parity_root +99 ❌ +4.63% +7,411 ❌ +0.66%
parity_base +102 ❌ +29.48% -28,854 ✅ -36.15%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
rollup_base 732,876 (+393,777) +116.12% 24,103,174 (+21,006,387) +678.33%
private_kernel_reset 541,352 (+299,292) +123.64% 19,713,792 (+17,169,149) +674.72%
private_kernel_reset_big 328,405 (+149,532) +83.60% 10,287,293 (+8,584,389) +504.10%
public_kernel_tail 1,733,658 (+516,811) +42.47% 32,517,991 (+27,090,896) +499.18%
private_kernel_reset_medium 221,933 (+74,652) +50.69% 5,574,045 (+4,292,009) +334.78%
private_kernel_reset_small 168,696 (+37,212) +28.30% 3,217,420 (+2,145,819) +200.24%
private_kernel_tail 33,971 (+11,999) +54.61% 1,821,451 (+484,639) +36.25%
public_kernel_app_logic 364,783 (+24,261) +7.12% 2,489,280 (+463,098) +22.86%
public_kernel_teardown 365,005 (+24,285) +7.13% 2,489,561 (+463,140) +22.86%
private_kernel_tail_to_public 470,511 (+11,944) +2.60% 2,600,970 (+482,811) +22.79%
public_kernel_setup 292,333 (+11,373) +4.05% 2,109,666 (+306,968) +17.03%
rollup_root 4,168 (+2,023) +94.31% 1,055,264 (+144,483) +15.86%
private_kernel_inner 59,304 (+5,793) +10.83% 1,413,370 (+120,721) +9.34%
private_kernel_init 31,185 (+498) +1.62% 416,264 (+25,488) +6.52%
parity_root 2,238 (+99) +4.63% 1,136,706 (+7,411) +0.66%
parity_base 448 (+102) +29.48% 50,957 (-28,854) -36.15%

@benesjan
Copy link
Contributor

I tried fixing the remaining issues and the problem is that the resulting contract artifacts now have null as a return_type and we try to map it to our TS interfaces which do not support that.

image

I see that for other contracts and for functions which actually return something the return_type is not null so this looks like a conscious change and we should just update our TS interface.

@TomAFrench can you please confirm that null return type is expected? Once I have the confirmation I'll update the types.

@TomAFrench
Copy link
Member

Yes this is expected in the case where we don't have a return value as we can't assign it a type.

@TomAFrench
Copy link
Member

The only thing that makes sense a potential cause for this issue is that we have an issue in how the new pedersen hash implementation deals with predicates. This could potentially screw up the calculation of the args_hash in the below code.

pub fn hash_args(args: [Field]) -> Field {
if args.len() == 0 {
0
} else {
assert(args.len() < ARGS_HASH_CHUNK_COUNT * ARGS_HASH_CHUNK_LENGTH);
let mut chunks_hashes = [0; ARGS_HASH_CHUNK_COUNT];
let mut current_chunk_values = [0; ARGS_HASH_CHUNK_LENGTH];
let mut current_chunk_index = 0;
let mut index_inside_current_chunk = 0;
for i in 0..args.len() {
current_chunk_values[index_inside_current_chunk] = args[i];
index_inside_current_chunk+=1;
if index_inside_current_chunk == ARGS_HASH_CHUNK_LENGTH {
chunks_hashes[current_chunk_index] = pedersen_hash(current_chunk_values, GENERATOR_INDEX__FUNCTION_ARGS);
current_chunk_values = [0; ARGS_HASH_CHUNK_LENGTH];
current_chunk_index+=1;
index_inside_current_chunk = 0;
}
}
if index_inside_current_chunk > 0 {
chunks_hashes[current_chunk_index] = pedersen_hash(current_chunk_values, GENERATOR_INDEX__FUNCTION_ARGS);
}
pedersen_hash(chunks_hashes, GENERATOR_INDEX__FUNCTION_ARGS)
}
}

@sirasistant
Copy link
Contributor

One question, are the generators the same in noir's implementation of pedersen vs BB implementation of pedersen? we might be having a mismatch between typescript computed pedersen hashes vs noir computed pedersens

@TomAFrench
Copy link
Member

Yeah, the issue seems to be that the separator was being applied to the length generator rather than the generators for the commitment.

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

Successfully merging this pull request may close these issues.

None yet

4 participants