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.

Sign up for GitHub

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

Jump to bottom

docs(limitations): limitations on ordering and logs of chopped notes #2085

Merged
merged 6 commits into from
Sep 7, 2023
Merged

docs(limitations): limitations on ordering and logs of chopped notes #2085

merged 6 commits into from
Sep 7, 2023

Conversation

jeanmon
Copy link
Contributor

@jeanmon jeanmon commented Sep 7, 2023 •
edited
Loading

Resolves #1652

Checklist:

Remove the checklist to signal you've completed it. Enable auto-merge if the PR is ready to merge.

  • If the pull request requires a cryptography review (e.g. cryptographic algorithm implementations) I have added the 'crypto' tag.
  • I have reviewed my diff in github, line by line and removed unexpected formatting changes, testing logs, or commented-out code.
  • Every change is related to the PR description.
  • I have linked this pull request to relevant issues (if any exist).

@jeanmon
Copy link
Contributor Author

jeanmon commented Sep 7, 2023

@iAmMichaelConnor Is this ok to have both limitations under 'Circuit Limitations'?

@suyash67 @iAmMichaelConnor Please also proofread the language (my english is as good as a french-speaking guy:-))

@jeanmon jeanmon marked this pull request as ready for review September 7, 2023 06:51
@jeanmon jeanmon changed the title documentation: limitations on ordering and logs of chopped notes docs(limitations): limitations on ordering and logs of chopped notes Sep 7, 2023
suyash67
suyash67 reviewed Sep 7, 2023
View reviewed changes
docs/docs/dev_docs/limitations/main.md Outdated
@@ -217,6 +217,18 @@ There are [plans](../../about_aztec/roadmap/engineering_roadmap.md#proper-circui

> **In the mean time**, if you encounter a per-transaction limit when testing, and you're feeling adventurous, you could 'hack' the Sandbox to increase the limits. See here (TODO: link) for a guide. **However**, the limits cannot be increased indefinitely. So although we do anticipate that we'll be able to increase them a little bit, don't go mad and provide yourself with 1 million state transitions per transaction. That would be as unrealistic as artificially increasing Ethereum gas limits to 1 trillion.

### Circuits Processing Order Differ from Execution

Each function call is representing by a circuit with a dedicated zero-knowledge proof of its execution. The [private kernel circuit](../../concepts/advanced/circuits/kernels/private_kernel.md) is in charge of stitching all these proofs together to produce a zero-knowledge proof that the whole execution of all function calls within a transaction is correct. By doing so, the processing order differ from the execution order.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add an example of how the execution order can be different to how the kernel circuit processes these calls? I think David's slides had such an example.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or if there's already an example of function execution order, you can just link it here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added some explanations. @suyash67 Can you please have a look at the second commit?

@jeanmon jeanmon force-pushed the jm/1652-document-limitation-ordering-logs branch from ce9114b to 35a7dcc Compare September 7, 2023 11:31
@jeanmon jeanmon force-pushed the jm/1652-document-limitation-ordering-logs branch 4 times, most recently from 276a271 to ac47593 Compare September 7, 2023 12:24
iAmMichaelConnor
iAmMichaelConnor requested changes Sep 7, 2023
View reviewed changes
Copy link
Contributor

@iAmMichaelConnor iAmMichaelConnor left a comment

Choose a reason for hiding this comment

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

Maybe we should say our plans to resolve these issues.
Do we plan to reorder all side effects to match execution ordering?
I see you've referenced a github issue for the chopping limitation.

docs/docs/dev_docs/limitations/main.md Outdated Show resolved Hide resolved
docs/docs/dev_docs/limitations/main.md Outdated Show resolved Hide resolved
docs/docs/dev_docs/limitations/main.md Outdated Show resolved Hide resolved
@jeanmon jeanmon force-pushed the jm/1652-document-limitation-ordering-logs branch from 4da3dcd to ca01f38 Compare September 7, 2023 14:27
@jeanmon
Copy link
Contributor Author

jeanmon commented Sep 7, 2023

Maybe we should say our plans to resolve these issues. Do we plan to reorder all side effects to match execution ordering? I see you've referenced a github issue for the chopping limitation.

@iAmMichaelConnor I accepted your changes and added a note that we do not plan to change the ordering in the future.

@iAmMichaelConnor iAmMichaelConnor merged commit 315ad3d into master Sep 7, 2023
84 checks passed
@iAmMichaelConnor iAmMichaelConnor deleted the jm/1652-document-limitation-ordering-logs branch September 7, 2023 14:50
@github-actions github-actions bot mentioned this pull request Sep 7, 2023
Merged
PhilWindle pushed a commit that referenced this pull request Sep 8, 2023
@github-actions
chore(master): release 0.1.0-alpha63 (#2078)
a5f2852 
🤖 I have created a new Aztec Packages release
---


##
[0.1.0-alpha63](v0.1.0-alpha62...v0.1.0-alpha63)
(2023-09-08)


### Features

* `GrumpkinScalar` type
([#1919](#1919))
([3a9238a](3a9238a))


### Bug Fixes

* add retry to tag and docker actions
([#2099](#2099))
([9f741f4](9f741f4))
* **breaking change:** change embedded curve scalar mul to use two limbs
to properly encode the scalar field
([#2105](#2105))
([070cc4c](070cc4c))
* broken bootstrap.sh after renaming `aztec-cli` dir as `cli`
([#2097](#2097))
([2386781](2386781))
* browser test in canary flow
([#2102](#2102))
([d52af6c](d52af6c)),
closes
[#2086](#2086)
* check a note is read before nullifying it.
([#2076](#2076))
([aabfb13](aabfb13)),
closes
[#1899](#1899)
* circuits issues when building with gcc
([#2107](#2107))
([4f5c4fe](4f5c4fe))
* COMMIT_TAG arg value in canary Dockerfile
([#2118](#2118))
([a3d6459](a3d6459))
* dont assume safety of nvm
([#2079](#2079))
([a4167e7](a4167e7))
* end-to-end aztec cli dependency issue
([#2092](#2092))
([16ee3e5](16ee3e5))
* minor annoyances
([#2115](#2115))
([a147582](a147582))
* padded printing for e2e-cli
([#2106](#2106))
([5988014](5988014))
* remaining refs to clang15
([#2077](#2077))
([2c16547](2c16547))
* run e2e tests without spot
([#2081](#2081))
([f0aa3ca](f0aa3ca))
* updated CLI readme
([#2098](#2098))
([2226091](2226091)),
closes
[#1784](#1784)


### Miscellaneous

* **circuits:** - remove dead code from cbind of private kernel circuit
([#2088](#2088))
([43dc9d7](43dc9d7))
* **circuits:** remove dead code in cbind.cpp for public kernel
([#2094](#2094))
([861f960](861f960))
* Conservatively raise the minimum supported clang version in CMakeList
([#2023](#2023))
([f49c416](f49c416))
* **constants:** bump number of private reads and writes
([#2062](#2062))
([ab6c6b1](ab6c6b1))
* **contracts:** Use autogenerated Noir interfaces where possible
([#2073](#2073))
([bd6368b](bd6368b)),
closes
[#1604](#1604)
* merge bb release-please
([#2080](#2080))
([e89b043](e89b043))
* move storage into main.nr.
([#2068](#2068))
([2c2d72b](2c2d72b))
* protogalaxy relations
([#1897](#1897))
([35407e2](35407e2))


### Documentation

* **limitations:** limitations on ordering and logs of chopped notes
([#2085](#2085))
([315ad3d](315ad3d)),
closes
[#1652](#1652)

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@suyash67 suyash67 suyash67 approved these changes

@iAmMichaelConnor iAmMichaelConnor iAmMichaelConnor approved these changes

Assignees
No one assigned
Labels
None yet
Projects
A3
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Document limitation: ordering of notes, noteHashes, logs, etc will not be in proper order in block
3 participants
@jeanmon @suyash67 @iAmMichaelConnor