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

Rk/clean collect logs #1026

Merged
merged 2 commits into from
Jul 18, 2023
Merged

Rk/clean collect logs #1026

merged 2 commits into from
Jul 18, 2023

Conversation

rahul-kothari
Copy link
Contributor

Description

I have updated it with your comments - so feel free to just review the 2nd commit

Stack is basically a queue in a reverse order. So doing what santiago originally did but adding a .reverse() would work.
Added some tests

Please sanity check that the ordering I have written in the tests are similar to what you would expect from ACVM or kernel

Checklist:

  • I have reviewed my diff in github, line by line.
  • Every change is related to the PR description.
  • I have linked this pull request to the issue(s) that it resolves.
  • There are no unexpected formatting changes, superfluous debug logs, or commented-out code.
  • The branch has been merged or rebased against the head of its merge target.
  • I'm happy for the PR to be merged at the reviewer's next convenience.

});

it('collect unencrypted logs with multiple logs each function call', () => {
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rephrase the name of this test as fnA and fnB do not emit logs. Maybe sthg like:
'collect unencrypted logs with multiple logs in each function call leaves'

@rahul-kothari rahul-kothari force-pushed the rk/clean_collect_logs branch from 497af6f to 9369285 Compare July 18, 2023 10:21
@rahul-kothari rahul-kothari force-pushed the rk/clean_collect_logs branch from 9369285 to ad99dfd Compare July 18, 2023 10:26
@rahul-kothari rahul-kothari merged commit 4b4ea52 into master Jul 18, 2023
@rahul-kothari rahul-kothari deleted the rk/clean_collect_logs branch July 18, 2023 10:42
suyash67 added a commit that referenced this pull request Aug 18, 2023
# Description

Multi-transfer contract demonstrates upto 12 transfers per transaction
for applications like payroll. We add two e2e tests, one that spends
notes to transfer tokens to 12 unique recipients, other that splits a
single note into 12 smaller denomination notes. The second test also
tests creation and spending of value notes in the same transaction.

Co-credit: @rahul-kothari, @jeanmon, @Cooper-Kunz, @jfecher.
Thanks to @LeilaWang for helping fix `get_balance` in aztec-noir.

Also resolves #987
(This PR ensures that we emit different unencrypted logs in the same
transaction, @rahul-kothari wrote specific execution tests for this
issue in #1026)

# Checklist:

- [x] I have reviewed my diff in github, line by line.
- [x] Every change is related to the PR description.
- [x] I have
[linked](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue)
this pull request to the issue(s) that it resolves.
- [x] There are no unexpected formatting changes, superfluous debug
logs, or commented-out code.
- [x] The branch has been merged or rebased against the head of its
merge target.
- [x] I'm happy for the PR to be merged at the reviewer's next
convenience.

---------

Co-authored-by: Rahul Kothari <[email protected]>
superstar0402 added a commit to superstar0402/aztec-nr that referenced this pull request Aug 16, 2024
# Description

Multi-transfer contract demonstrates upto 12 transfers per transaction
for applications like payroll. We add two e2e tests, one that spends
notes to transfer tokens to 12 unique recipients, other that splits a
single note into 12 smaller denomination notes. The second test also
tests creation and spending of value notes in the same transaction.

Co-credit: @rahul-kothari, @jeanmon, @Cooper-Kunz, @jfecher.
Thanks to @LeilaWang for helping fix `get_balance` in aztec-noir.

Also resolves AztecProtocol/aztec-packages#987
(This PR ensures that we emit different unencrypted logs in the same
transaction, @rahul-kothari wrote specific execution tests for this
issue in AztecProtocol/aztec-packages#1026)

# Checklist:

- [x] I have reviewed my diff in github, line by line.
- [x] Every change is related to the PR description.
- [x] I have
[linked](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue)
this pull request to the issue(s) that it resolves.
- [x] There are no unexpected formatting changes, superfluous debug
logs, or commented-out code.
- [x] The branch has been merged or rebased against the head of its
merge target.
- [x] I'm happy for the PR to be merged at the reviewer's next
convenience.

---------

Co-authored-by: Rahul Kothari <[email protected]>
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.

3 participants