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(avm-simulator): consider previous pending nullifiers across enqueued calls #6188

Merged
merged 7 commits into from
May 7, 2024

Conversation

fcarreiro
Copy link
Contributor

No description provided.

Copy link
Contributor Author

fcarreiro commented May 3, 2024

@fcarreiro fcarreiro force-pushed the fc/avm-storage-across-enqueued-calls branch from 569386b to d966123 Compare May 3, 2024 14:49
@fcarreiro fcarreiro force-pushed the fc/avm-nullifiers-across-enqueued-calls branch from 6753ba3 to 78fdfc8 Compare May 3, 2024 14:49
@fcarreiro fcarreiro requested review from dbanks12 and removed request for Maddiaa0 May 3, 2024 16:00
@fcarreiro fcarreiro force-pushed the fc/avm-storage-across-enqueued-calls branch from d966123 to f29b328 Compare May 3, 2024 16:13
@fcarreiro fcarreiro force-pushed the fc/avm-nullifiers-across-enqueued-calls branch from 78fdfc8 to f1afce6 Compare May 3, 2024 16:13
Comment on lines +75 to +84
for (const nullifier of executionContext.pendingNullifiers) {
worldStateJournal.nullifiers.cache.appendSiloed(nullifier.value);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably fine for now.... But if we plan to use this long-term (as opposed to having another layer of caching like the WorldStatePublicDB), maybe this should be a constructor argument to the state manager?

Copy link
Contributor

@dbanks12 dbanks12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One comment

Copy link
Contributor

@dbanks12 dbanks12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments but lgtm

@fcarreiro fcarreiro force-pushed the fc/avm-storage-across-enqueued-calls branch from f29b328 to b9acabf Compare May 7, 2024 10:25
@fcarreiro fcarreiro force-pushed the fc/avm-nullifiers-across-enqueued-calls branch from f1afce6 to 6212576 Compare May 7, 2024 10:35
Copy link
Contributor

github-actions bot commented May 7, 2024

Changes to circuit sizes

Generated at commit: ab3e2f1cda10dd79d4d095c6e1449aa8cd5b8372, compared to commit: 62043e0448f3dfe07016900f3e333c0286819860

🧾 Summary (100% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
private_kernel_tail +1 ❌ +0.00% +659 ❌ +0.06%
private_kernel_tail_to_public +1 ❌ +0.00% +659 ❌ +0.04%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
private_kernel_tail 135,880 (+1) +0.00% 1,063,378 (+659) +0.06%
private_kernel_tail_to_public 349,078 (+1) +0.00% 1,536,194 (+659) +0.04%

@fcarreiro fcarreiro force-pushed the fc/avm-storage-across-enqueued-calls branch from 62043e0 to 46c74f8 Compare May 7, 2024 14:54
@fcarreiro fcarreiro force-pushed the fc/avm-nullifiers-across-enqueued-calls branch from 6212576 to 09a6736 Compare May 7, 2024 14:54
Copy link
Contributor Author

fcarreiro commented May 7, 2024

Merge activity

Base automatically changed from fc/avm-storage-across-enqueued-calls to master May 7, 2024 16:05
@fcarreiro fcarreiro merged commit 4676431 into master May 7, 2024
64 of 87 checks passed
@fcarreiro fcarreiro deleted the fc/avm-nullifiers-across-enqueued-calls branch May 7, 2024 16:06
TomAFrench added a commit that referenced this pull request May 8, 2024
* master: (25 commits)
  fix: Enable client proof tests (#6249)
  chore: update cspell for abi demonomorphizer (#6258)
  feat(aztec-nr): add 'with_gas()' function to avm call interface (#6256)
  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
  fix: Pw/update merge check (#6201)
  chore(master): Release 0.37.1 (#6148)
  fix: Cl/split out e2e tests (#6242)
  feat: Typings generator with generics (#6235)
  chore(ci): fix restarts with fresh spot, acir test fixes, non-mandatory benches (#6226)
  chore: misc AVM migration prep changes (#6253)
  feat!: AES blackbox (#6016)
  chore(docs): Fix some typos in specs of private kernel initial (#6224)
  chore(aztec-macros): avm function return types are auto tagged as `pub` (#6250)
  chore(aztec-nr): create a 'with_selector' version of `emit_unencrypted_log` in avm context (#6248)
  fix: registering PublicDataWitness in JsonRpcServer (#6243)
  feat: Sync from noir (#6234)
  feat(avm-simulator): consider previous pending nullifiers across enqueued calls (#6188)
  ...
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.

2 participants