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 aztec-packages #4715

Closed
wants to merge 4 commits into from
Closed

feat: Sync from aztec-packages #4715

wants to merge 4 commits into from

Conversation

AztecBot
Copy link
Collaborator

@AztecBot AztecBot commented Apr 4, 2024

Automated pull of Noir development from aztec-packages.
BEGIN_COMMIT_OVERRIDE
feat!: contract_abi-exports (AztecProtocol/aztec-packages#5386)
feat(avm): integrate AVM with initializers (AztecProtocol/aztec-packages#5469)
feat: Restore hashing args via slice for performance (AztecProtocol/aztec-packages#5539)
END_COMMIT_OVERRIDE

# Goal

This PR aims to expose arbitrary types and values resulting from
contract compilation in the resulting JSON artifact, in a way that is
not tied to aztec-specific features or even smart contracts at all.

# Problem

Up until now, Noir compiled crates that used the `contract` keyword with
a specific flow, which also added additional structs and metadata to the
output such as whatever structs were marked with the `#[event]`
attribute. This coupled Noir to smart contract specific constructs,
which were propagated through the compiler (from the parser to the
actual compilation output). For
AztecProtocol/aztec-packages#5079 and several
other tasks that aim to reduce the mental load and improve the general
devex of our users, we ***need*** to expose several other structs that
are even more specific to aztec, which would only compromise the
generality of the compiler further.

# Proposed solution

The introduction of a new attribute `#[abi(tag)]` that can be applied to
both `structs` and `global` top-level statements, and export types (with
the current `ABIType` format) and values (with the new `ABIValue`
format) in a way that can be interpreted by components further
downstream (for example, our typescript codegen). This way, the noir
compiler doesn't know (or care) about whatever gets included in the
artifact.

The `events` contract artifact key gets replaced by:

```typescript
outputs: {
    structs: Record<string, ABIType[]>;
    globals: Record<string, ABIValue[]>;
};
```

# What this approach allows

- Removing the smart contract specific attribute `#[event]`, replacing
it by a more general `#[abi(events)]`.
- Substantial devex improvements, such as exposing storage layout, note
ids:

![image](https://github.com/AztecProtocol/aztec-packages/assets/5404052/dba1d6ca-1286-4d4d-912e-f222d3414f32)
...or even private function return values prior to macro processing for
decoding `.view` calls
AztecProtocol/aztec-packages#2665

---------

Co-authored-by: esau <152162806+sklppy88@users.noreply.github.com>
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
- update-acvm-workspace-package-versions
- update-docs

env:
# We treat any skipped or failing jobs as a failure for the workflow as a whole.
FAIL: ${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') || contains(needs.*.result, 'skipped') }}
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 reverting a lot of the same things last time we did an aztec-packages sync. Would this be solved with a Noir sync into aztec?

Copy link
Member

Choose a reason for hiding this comment

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

I think so. I opened a PR at AztecProtocol/aztec-packages#5572 but there's a bunch of updates which need to be made on the aztec-packages side.

@sirasistant
Copy link
Contributor

I'll sync them up!

Copy link

socket-security bot commented Apr 8, 2024

No dependency changes detected. Learn more about Socket for GitHub ↗︎

👍 No dependency changes detected in pull request

@vezenovm
Copy link
Contributor

vezenovm commented Apr 8, 2024

Closing to re-sync

@vezenovm vezenovm closed this Apr 8, 2024
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.

4 participants