-
Notifications
You must be signed in to change notification settings - Fork 234
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
Plumbs noir (for review only) NO MERGE #3427
Conversation
nvm install | ||
set -eu | ||
|
||
# if [ "$(uname)" = "Darwin" ]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's up with this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed commented in "full" pr.
Depending on nvm is nasty. It's now assumed the user has node 18 installed and available.
I believe we get appropriate version warnings if they dont.
@@ -247,7 +247,7 @@ async function executePrivateKernelInitWithACVM(input: InitInputType): Promise<R | |||
); | |||
|
|||
// Decode the witness map into two fields, the return values and the inputs | |||
const decodedInputs: DecodedInputs = abiDecode(PrivateKernelInitSimulatedJson.abi, _witnessMap); | |||
const decodedInputs: DecodedInputs = abiDecode(PrivateKernelInitSimulatedJson.abi as Abi, _witnessMap); | |||
|
|||
// Cast the inputs as the return type | |||
return decodedInputs.return_value as ReturnType; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not getting why this one has typecasts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know why it wasn't an issue before.
This is the fact that JSON doesnt have type info so string != 'blah'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm other than comment
For a less noisey review: #3427 It contains everything prior to the noir pulls. This is PR we actually want to merge though. * Adds `noir-packages` job to build the ts packages. * Fix `sed` use in bb.js to work on Mac. * Bootstrap scripts for noir, call it from bootstrap.sh. * Fix to `build_local`. * Use the noir bootstrap scripts to build the docker containers. * `yarn-project` now builds the protocol circuits from the noir build in bootstrap and dockerfile. * `yarn-project` portals the noir packages in. **What does this mean for releases!?* * Fixes as we're now on noir 19. * Remove `noir-protocol-circuits/src/target` from repo as they're now built. * Bootstrap improvements: * Defaults to a "lighter" bootstrap. * `bootstrap.sh clean` resets repo to as if fresh clone. (be careful). * Introduces ability to test the bootstrap process via a docker container. (might add a nightly job, plus alternative ubuntu releases).
For a less noisey review: AztecProtocol/aztec-packages#3427 It contains everything prior to the noir pulls. This is PR we actually want to merge though. * Adds `noir-packages` job to build the ts packages. * Fix `sed` use in bb.js to work on Mac. * Bootstrap scripts for noir, call it from bootstrap.sh. * Fix to `build_local`. * Use the noir bootstrap scripts to build the docker containers. * `yarn-project` now builds the protocol circuits from the noir build in bootstrap and dockerfile. * `yarn-project` portals the noir packages in. **What does this mean for releases!?* * Fixes as we're now on noir 19. * Remove `noir-protocol-circuits/src/target` from repo as they're now built. * Bootstrap improvements: * Defaults to a "lighter" bootstrap. * `bootstrap.sh clean` resets repo to as if fresh clone. (be careful). * Introduces ability to test the bootstrap process via a docker container. (might add a nightly job, plus alternative ubuntu releases).
For a less noisey review: AztecProtocol/aztec-packages#3427 It contains everything prior to the noir pulls. This is PR we actually want to merge though. * Adds `noir-packages` job to build the ts packages. * Fix `sed` use in bb.js to work on Mac. * Bootstrap scripts for noir, call it from bootstrap.sh. * Fix to `build_local`. * Use the noir bootstrap scripts to build the docker containers. * `yarn-project` now builds the protocol circuits from the noir build in bootstrap and dockerfile. * `yarn-project` portals the noir packages in. **What does this mean for releases!?* * Fixes as we're now on noir 19. * Remove `noir-protocol-circuits/src/target` from repo as they're now built. * Bootstrap improvements: * Defaults to a "lighter" bootstrap. * `bootstrap.sh clean` resets repo to as if fresh clone. (be careful). * Introduces ability to test the bootstrap process via a docker container. (might add a nightly job, plus alternative ubuntu releases).
This PR is here simply to provide something cleaner to review, otherwise we see all the noir pull changes.
Actual passing PR to review is here: #3420
noir-packages
job to build the ts packages.sed
use in bb.js to work on Mac.build_local
.yarn-project
now builds the protocol circuits from the noir build in bootstrap and dockerfile.yarn-project
portals the noir packages in. *What does this mean for releases!?noir-protocol-circuits/src/target
from repo as they're now built.bootstrap.sh clean
resets repo to as if fresh clone. (be careful).