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

fix: auto update contract compilation artifacts in boxes #2635

Closed
wants to merge 59 commits into from

Conversation

dan-aztec
Copy link
Contributor

@dan-aztec dan-aztec commented Oct 3, 2023

separately build the contracts in boxes, to keep bytecode up to date as things change upstream.

slight artifact nameing change due to the difference in how noir-contracts compilation generated artifacts, vs the aztec-cli compile command that box users would run locally. but, fixed the relative import bug on generated typesript classes, in the case the artifact is in the same directory!

moved lingering exports to AztecJs to reduce dependencies on the frontend only apps

@dan-aztec dan-aztec changed the title add new blank contract fix: auto update + copy contract artifacts to boxes from noir-contracts Oct 3, 2023
@dan-aztec dan-aztec marked this pull request as ready for review October 3, 2023 13:29
@LeilaWang
Copy link
Collaborator

Looks like we've duplicated the contract source files and are copying the artefacts from noir-contracts while the "real" contract source files are still in boxes?

@dan-aztec
Copy link
Contributor Author

yes, that's right. should we copy the source code as well? (I didn't think those would change very often, we can copy the main.nr but easier to not do Nargo.toml because of monorepo differences)

@LeilaWang
Copy link
Collaborator

Maybe change the toml files in boxes to contain relative paths. We are already modifying the paths in the unbox command so it should only require small adjustment. And add a bootstrap script to boxes to rebuild the artefacts all at once. Should also try to run the compile script in CI, so that the job will fail if a contract doesn't adopt a breaking change in aztec.nr or noir. Will be great if we can make the job fail if the artefacts are not being updated. But will have to spend some time to figure out the best way to do that.

@dan-aztec
Copy link
Contributor Author

ok seems reasonable. updating the Nargo.toml and added a script to copy the source code, gotta think about detection "missed recompilation" in CI

@LeilaWang
Copy link
Collaborator

The point of updating the paths in Nargo.toml is that we can just compile from there. So we don't have to copy the source code to noir-contracts and then copy it back to boxes.

@dan-aztec
Copy link
Contributor Author

ah OK, I thought it was to try to do a last minute compile and check results against the copied artifacts. remove the link to noir-contracts then, and just do everything in boxes. i think this will revert from kebab-case back to UpperCamel, because the noir-cointracts output script changes the naming to kebab

@dan-aztec dan-aztec changed the title fix: auto update + copy contract artifacts to boxes from noir-contracts fix: auto update contract compilation artifacts in boxes Oct 4, 2023
@dan-aztec
Copy link
Contributor Author

this one got in a really bad state re: SentTx

@dan-aztec dan-aztec closed this Oct 6, 2023
dan-aztec added a commit that referenced this pull request Oct 10, 2023
…s import (#2727)

[forking prior PR to see where e2e tests
failed](#2635), got
in a bad state so splitting into smaller PRs.

This will help keep the boxes up to date by introducing a `bootstrap.sh`
for boxes, which recompiles within the monorepo using the latest local
`aztec-cli compile` command.

The artifact casing changes because `noir-contracts` compile script does
some manual changing to kebab case.

--> changes box contract Nargo.toml to be buildable within monorepo by
using relative paths for deps. These get updated now in the `unbox`
command.

Also discovered `CompleteAddress` was broken in the frontend, Leila
helped resolve import errors. added a test for the in e2e browser. This
means boxes are broken on current release of our packages, so need to
release again after this lands.

Will do a follow up PR for reducing dependencies to just
`@aztec/aztec.js`, as this PR is getting big
@dan-aztec dan-aztec deleted the dan/auto-copy-box-artifacts branch October 10, 2023 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Noir Contract Typescript generated class's contract ABI import path bug
2 participants