Conversation
aztec-nargo
e7bc033 to
e31046b
Compare
| # Should this just be aztec-test? It's like, a new command that doesn't exist on aztec cli. | ||
| # Or just make this a first class command on aztec cli? |
There was a problem hiding this comment.
This comment was no longer relevant - now we know we want aztec test.
| @@ -1,25 +0,0 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
aztec compile handles this automatically so no need to have a separate command.
7b961e4 to
7bcf731
Compare
aztec-up/bin/aztec
Outdated
| # Note: I allow devs to use the `aztec` command to also create bin and lib projects because we currently rely | ||
| # on the dockerized nargo and I am not sure if it's expected they will have a native nargo on path. If we decide | ||
| # to use native nargo I think we can just hardcode --contract here and not bother with the other options. |
There was a problem hiding this comment.
The implementation of the new command is basically the same as init but I didn't bother reusing the code here as the complexity is low and this comment might make me change quite a few things here.
Feel like the design choice of allowing devs to also create bin and lib project with aztec command is controversial so please chime in if you disagree with the choice.
| preload-crs: Downloads and caches the Common Reference String (CRS) data required for zero-knowledge proofs. | ||
| Example: | ||
| $ aztec preload-crs # preloads CRS data |
There was a problem hiding this comment.
This is unrelated to the PR - it was undocumented here and AI added it here when updating the docs.
aztec-nargoaztec-nargo and aztec-postprocess-contract to aztec
| $DOCKER_REPO:$VERSION "$@" | ||
| } | ||
|
|
||
| function setup_aztec_contract_project { |
There was a problem hiding this comment.
Added this function and parse_project_flags_and_build_nargo_args because the functionality is shared between nargo new and nargo init commands.
| --lib) | ||
| NARGO_ARGS+=("--lib") | ||
| HAS_TYPE_FLAG=true | ||
| shift | ||
| ;; | ||
| --bin) | ||
| NARGO_ARGS+=("--bin") | ||
| HAS_TYPE_FLAG=true | ||
| shift | ||
| ;; | ||
| --contract) | ||
| NARGO_ARGS+=("--contract") | ||
| HAS_TYPE_FLAG=true | ||
| IS_CONTRACT=true | ||
| shift | ||
| ;; |
There was a problem hiding this comment.
I allow devs to use the aztec command to also create bin and lib projects because we currently rely on the dockerized nargo and I am not sure if it's expected they will have a native nargo on path. If we decide to use native nargo I think we can just hardcode --contract here and not bother with the other options.
I think this is quite a controversial decision so if the reviewer disagrees LMK.
| lsp) | ||
| # TODO: Drop this (and the corresponding docs in cli.ts) in case `nargo` is to be used natively (not containerized). | ||
|
|
||
| # Special-case nargo's LSP command: | ||
| # 1. include --rm for cleanup | ||
| # 2. don't specify a user (run as root) | ||
| # 3. don't specify a workdir | ||
| validate_working_directory | ||
|
|
||
| docker run --rm -i \ | ||
| --name aztec-nargo-lsp \ | ||
| -v $HOME:$HOME \ | ||
| -e HOME=$HOME \ | ||
| --entrypoint=/usr/src/noir/noir-repo/target/release/nargo \ | ||
| $DOCKER_REPO:$VERSION lsp | ||
| ;; |
There was a problem hiding this comment.
This is a copy-paste from the original aztec-nargo command. Might be dropped in the future if it's decided we don't want to use containerized nargo.
aztec-nargo and aztec-postprocess-contract to aztecaztec command
4ef46de to
1cdec92
Compare
boxes/boxes/react/package.json
Outdated
| "compile": "cd src/contracts && ${AZTEC_NARGO:-aztec-nargo} compile --silence-warnings && ${AZTEC_POSTPROCESS_CONTRACT:-aztec-postprocess-contract}", | ||
| "codegen": "${AZTEC_BUILDER:-aztec} codegen src/contracts/target -o artifacts", | ||
| "compile": "cd src/contracts && ${AZTEC:-aztec} compile --silence-warnings", | ||
| "codegen": "${AZTEC:-aztec} codegen src/contracts/target -o artifacts", |
There was a problem hiding this comment.
The codegen command is also exposed on aztec directly so I used it here.
Think it might make sense to just drop the builder cli and have the functionality exposed just on the aztec command but that's out of scope of this PR.
| export TRANSPILER=$PWD/../avm-transpiler/target/release/avm-transpiler | ||
| export BB=$PWD/../barretenberg/cpp/build/bin/bb | ||
| export NARGO=$PWD/../noir/noir-repo/target/release/nargo | ||
| export AZTEC_NARGO=$PWD/../aztec-up/bin/aztec-nargo | ||
| export AZTEC_POSTPROCESS_CONTRACT=$PWD/../aztec-up/bin/aztec-postprocess-contract | ||
| export AZTEC_BUILDER=$PWD/../yarn-project/builder/aztec-builder-dest |
There was a problem hiding this comment.
These were stale and not used so I dropped it.
eab3a42 to
b0bd5bd
Compare
Flakey Tests🤖 says: This CI run detected 1 tests that failed, but were tolerated due to a .test_patterns.yml entry. |
7ff9178 to
80e26bf
Compare
80e26bf to
2102b17
Compare
2102b17 to
69a40c3
Compare
nventuro
left a comment
There was a problem hiding this comment.
Thanks! Reading bash breaks my brain but the functionality seems to be there. If we find small issues here we can address them as we go.
69a40c3 to
c263145
Compare
The title of the PR is vague because there is a lot of changes in this PR. Other than tackling these issues here: Closes https://linear.app/aztec-labs/issue/F-41/replace-aztec-nargo-init-with-aztec-init Closes https://linear.app/aztec-labs/issue/F-43/replace-aztec-nargo-compile-with-aztec-compile Closes https://linear.app/aztec-labs/issue/F-44/aztec-compile-help-will-override-the-nargo-compile-help-with-aztec Closes https://linear.app/aztec-labs/issue/F-48/add-aztec-check-and-aztec-fmt I also: - Added `aztec new` command that is like `aztec init` but instead calls `nargo new`. This was necessary as we used `aztec-nargo new` in docs etc. Hence I needed the `aztec new counterpart`. - I dropped the `aztec-postprocess-contract` directory containing the `transpile_contract_and_gen_vks.sh` script because it now only called the aztec_process command in bb which made this basically a tech debt from time where we forced devs to call this script manually. - Went to occurrences of "aztec-nargo" and "aztec-postprocess-contract" strings in the codebase and docs and did the relevant updates. - I replace the default contract template that `nargo init --contract` and `nargo new --contract` commands spit out with: ``` use aztec::macros::aztec; #[aztec] contract Main {} ``` This results in the `aztec-nargo` and `aztec-postprocess-contract` being deprecated (and deleted). Thought it's fine to do all these changes in one PR because the complexity of the changes is low.
c263145 to
1e6a6a6
Compare

The title of the PR is vague because there is a lot of changes in this PR.
Other than tackling these issues here:
Closes https://linear.app/aztec-labs/issue/F-41/replace-aztec-nargo-init-with-aztec-init
Closes https://linear.app/aztec-labs/issue/F-43/replace-aztec-nargo-compile-with-aztec-compile
Closes https://linear.app/aztec-labs/issue/F-44/aztec-compile-help-will-override-the-nargo-compile-help-with-aztec
Closes https://linear.app/aztec-labs/issue/F-48/add-aztec-check-and-aztec-fmt
I also:
aztec newcommand that is likeaztec initbut instead callsnargo new. This was necessary as we usedaztec-nargo newin docs etc. Hence I needed theaztec new counterpart.aztec-postprocess-contractdirectory containing thetranspile_contract_and_gen_vks.shscript because it now only called the aztec_process command in bb which made this basically a tech debt from time where we forced devs to call this script manually.nargo init --contractandnargo new --contractcommands spit out with:This results in the
aztec-nargoandaztec-postprocess-contractbeing deprecated (and deleted).Thought it's fine to do all these changes in one PR because the complexity of the changes is low.