From 097a888b1f60d285595dbae6ebac5af32f9ace67 Mon Sep 17 00:00:00 2001 From: spypsy Date: Mon, 26 Feb 2024 13:08:05 +0000 Subject: [PATCH 1/5] fix: AZTEC_PORT variable for devnet (#4700) - `AZTEC_NODE_PORT` -> `AZTEC_PORT` - Log Availability Oracle contract address on node start --- yarn-project/aztec-node/src/aztec-node/server.ts | 3 ++- yarn-project/aztec-node/terraform/main.tf | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/yarn-project/aztec-node/src/aztec-node/server.ts b/yarn-project/aztec-node/src/aztec-node/server.ts index 4d182caaa5ef..d31b95afce2c 100644 --- a/yarn-project/aztec-node/src/aztec-node/server.ts +++ b/yarn-project/aztec-node/src/aztec-node/server.ts @@ -87,7 +87,8 @@ export class AztecNodeService implements AztecNode { `Registry: ${config.l1Contracts.registryAddress.toString()}\n` + `Inbox: ${config.l1Contracts.inboxAddress.toString()}\n` + `Outbox: ${config.l1Contracts.outboxAddress.toString()}\n` + - `Contract Emitter: ${config.l1Contracts.contractDeploymentEmitterAddress.toString()}`; + `Contract Emitter: ${config.l1Contracts.contractDeploymentEmitterAddress.toString()}\n` + + `Availability Oracle: ${config.l1Contracts.availabilityOracleAddress.toString()}`; this.log(message); } diff --git a/yarn-project/aztec-node/terraform/main.tf b/yarn-project/aztec-node/terraform/main.tf index aa030c163d71..ce2e3b42bffc 100644 --- a/yarn-project/aztec-node/terraform/main.tf +++ b/yarn-project/aztec-node/terraform/main.tf @@ -180,7 +180,7 @@ resource "aws_ecs_task_definition" "aztec-node" { "value": "false" }, { - "name": "AZTEC_NODE_PORT", + "name": "AZTEC_PORT", "value": "80" }, { From c2027e3f58279fc9fa7c8e5c1b7fdcf832555d90 Mon Sep 17 00:00:00 2001 From: spypsy Date: Mon, 26 Feb 2024 13:16:41 +0000 Subject: [PATCH 2/5] fix: PXE devnet connectivity (#4759) - Turn on retries for PXE client talking to Aztec node - turn on `pxe_` namespacing by default when running PXE outside of the `aztec` binary context --- .../aztec.js/src/rpc_clients/pxe_client.ts | 22 +++++++++---------- yarn-project/end-to-end/src/fixtures/utils.ts | 3 ++- .../src/json-rpc/client/json_rpc_client.ts | 2 +- .../pxe/src/pxe_http/pxe_http_server.ts | 5 +++-- 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/yarn-project/aztec.js/src/rpc_clients/pxe_client.ts b/yarn-project/aztec.js/src/rpc_clients/pxe_client.ts index 296daead28bc..61a7a480985c 100644 --- a/yarn-project/aztec.js/src/rpc_clients/pxe_client.ts +++ b/yarn-project/aztec.js/src/rpc_clients/pxe_client.ts @@ -32,28 +32,28 @@ import { createJsonRpcClient, makeFetch } from '@aztec/foundation/json-rpc/clien * @param fetch - The fetch implementation to use. * @returns A JSON-RPC client of PXE. */ -export const createPXEClient = (url: string, fetch = makeFetch([1, 2, 3], true)): PXE => +export const createPXEClient = (url: string, fetch = makeFetch([1, 2, 3], false)): PXE => createJsonRpcClient( url, { - CompleteAddress, - FunctionSelector, + AuthWitness, AztecAddress, - TxExecutionRequest, + CompleteAddress, ContractData, + FunctionSelector, + EthAddress, ExtendedContractData, + ExtendedNote, ExtendedUnencryptedL2Log, - TxHash, - EthAddress, - Point, Fr, GrumpkinScalar, - Note, - ExtendedNote, - AuthWitness, + L2Block, L2Tx, LogId, - L2Block, + Note, + Point, + TxExecutionRequest, + TxHash, }, { Tx, TxReceipt, L2BlockL2Logs }, false, diff --git a/yarn-project/end-to-end/src/fixtures/utils.ts b/yarn-project/end-to-end/src/fixtures/utils.ts index 8ac596c5faa7..2edd58fb46ee 100644 --- a/yarn-project/end-to-end/src/fixtures/utils.ts +++ b/yarn-project/end-to-end/src/fixtures/utils.ts @@ -19,6 +19,7 @@ import { createDebugLogger, createPXEClient, deployL1Contracts, + makeFetch, waitForPXE, } from '@aztec/aztec.js'; import { @@ -164,7 +165,7 @@ async function setupWithRemoteEnvironment( logger(`Creating Aztec Node client to remote host ${aztecNodeUrl}`); const aztecNode = createAztecNodeClient(aztecNodeUrl); logger(`Creating PXE client to remote host ${PXE_URL}`); - const pxeClient = createPXEClient(PXE_URL); + const pxeClient = createPXEClient(PXE_URL, makeFetch([1, 2, 3], true)); await waitForPXE(pxeClient, logger); logger('JSON RPC client connected to PXE'); logger(`Retrieving contract addresses from ${PXE_URL}`); diff --git a/yarn-project/foundation/src/json-rpc/client/json_rpc_client.ts b/yarn-project/foundation/src/json-rpc/client/json_rpc_client.ts index c62b20d66f73..1e0c69d25794 100644 --- a/yarn-project/foundation/src/json-rpc/client/json_rpc_client.ts +++ b/yarn-project/foundation/src/json-rpc/client/json_rpc_client.ts @@ -77,7 +77,7 @@ export function makeFetch(retries: number[], noRetry: boolean, log?: DebugLogger return async (host: string, rpcMethod: string, body: any, useApiEndpoints: boolean) => { return await retry( () => defaultFetch(host, rpcMethod, body, useApiEndpoints, noRetry), - `JsonRpcClient request to ${host}`, + `JsonRpcClient request ${rpcMethod} to ${host}`, makeBackoff(retries), log, true, diff --git a/yarn-project/pxe/src/pxe_http/pxe_http_server.ts b/yarn-project/pxe/src/pxe_http/pxe_http_server.ts index 300da326589c..1a379c9d0a99 100644 --- a/yarn-project/pxe/src/pxe_http/pxe_http_server.ts +++ b/yarn-project/pxe/src/pxe_http/pxe_http_server.ts @@ -20,7 +20,7 @@ import { FunctionSelector } from '@aztec/circuits.js'; import { AztecAddress } from '@aztec/foundation/aztec-address'; import { EthAddress } from '@aztec/foundation/eth-address'; import { Fr, GrumpkinScalar, Point } from '@aztec/foundation/fields'; -import { JsonRpcServer } from '@aztec/foundation/json-rpc/server'; +import { JsonRpcServer, createNamespacedJsonRpcServer } from '@aztec/foundation/json-rpc/server'; import http from 'http'; @@ -63,7 +63,8 @@ export function createPXERpcServer(pxeService: PXE): JsonRpcServer { * @returns A running http server. */ export function startPXEHttpServer(pxeService: PXE, port: string | number): http.Server { - const rpcServer = createPXERpcServer(pxeService); + const pxeServer = createPXERpcServer(pxeService); + const rpcServer = createNamespacedJsonRpcServer([{ pxe: pxeServer }]); const app = rpcServer.getApp(); const httpServer = http.createServer(app.callback()); From f3c65ce75637bd47aca849a08b567b06a69318b0 Mon Sep 17 00:00:00 2001 From: Charlie Lye Date: Mon, 26 Feb 2024 14:06:09 +0000 Subject: [PATCH 3/5] chore: decouple ypb (#4749) This: * Decouples (partially) yarn-project-base build from rest of pipeline. Actually it's not quite what I hoped as I realised that at present to build ypb we need bb.js and noir packages to have been built first (which in turn require bb and noir). This maybe able to be worked around. My hope was that I fully decouple it so that we would only perform the expensive yarn install, if the package.json files had changed. Practically this partial decoupling probably won't improve CI times that much, but it still makes sense to do what's done here. * Fixes bug in boxes whereby it didn't wait for aztec to fully start before running test. * Remove `prep` call which compiles from `test`. Think this was added by @signorecello but running the container tests should not be doing any builds. --- .circleci/config.yml | 7 +++---- boxes/.dockerignore | 3 ++- boxes/Dockerfile | 3 ++- boxes/docker-compose.yml | 6 +++++- boxes/react/package.json | 2 +- build_manifest.yml | 8 +++----- yarn-project/Dockerfile | 15 ++++++++++----- yarn-project/yarn-project-base/Dockerfile | 20 ++------------------ 8 files changed, 28 insertions(+), 36 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index eb276b39bdf7..435d9f88a99e 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1389,16 +1389,15 @@ workflows: # Yarn Project - yarn-project-base: requires: - - avm-transpiler - - l1-contracts - bb-js - noir-packages - - noir-projects - - boxes-files <<: *defaults - yarn-project: requires: - yarn-project-base + - l1-contracts + - noir-projects + - boxes-files <<: *defaults - yarn-project-prod: *defaults_yarn_project - yarn-project-formatting: *defaults_yarn_project diff --git a/boxes/.dockerignore b/boxes/.dockerignore index bde85ad2dcda..7eab5f5a5cbb 100644 --- a/boxes/.dockerignore +++ b/boxes/.dockerignore @@ -5,4 +5,5 @@ node_modules .tsbuildinfo Dockerfile* .dockerignore -docker-compose.yml \ No newline at end of file +docker-compose.yml +**/artifacts \ No newline at end of file diff --git a/boxes/Dockerfile b/boxes/Dockerfile index 9241170b544c..ef381f2fff51 100644 --- a/boxes/Dockerfile +++ b/boxes/Dockerfile @@ -6,6 +6,7 @@ FROM 278380418400.dkr.ecr.eu-west-2.amazonaws.com/noir-projects as noir-projects # We need yarn. Start fresh container. FROM node:18.19.0 +RUN apt update && apt install netcat-openbsd COPY --from=aztec /usr/src /usr/src COPY --from=noir /usr/src/noir/target/release/nargo /usr/src/noir/target/release/nargo COPY --from=noir-projects /usr/src/noir-projects/aztec-nr /usr/src/noir-projects/aztec-nr @@ -13,4 +14,4 @@ WORKDIR /usr/src/boxes ENV AZTEC_NARGO=/usr/src/noir/target/release/nargo ENV AZTEC_CLI=/usr/src/yarn-project/cli/aztec-cli-dest RUN yarn && yarn build -ENTRYPOINT ["yarn", "workspace"] +ENTRYPOINT ["/bin/sh", "-c"] diff --git a/boxes/docker-compose.yml b/boxes/docker-compose.yml index 957c8ee68528..18ab65d443fd 100644 --- a/boxes/docker-compose.yml +++ b/boxes/docker-compose.yml @@ -18,7 +18,11 @@ services: boxes: image: aztecprotocol/boxes - command: "@aztec/$BOX test" + entrypoint: > + sh -c ' + while ! nc -z aztec 8080; do sleep 1; done; + yarn workspace @aztec/$BOX test + ' environment: DEBUG: "aztec:*" DEBUG_COLORS: "true" diff --git a/boxes/react/package.json b/boxes/react/package.json index 1d859052a837..9a5969152b6a 100644 --- a/boxes/react/package.json +++ b/boxes/react/package.json @@ -14,7 +14,7 @@ "serve": "serve -p 3000 ./dist", "formatting": "prettier --check ./src && eslint ./src", "formatting:fix": "prettier -w ./src", - "test": "yarn prep && NODE_NO_WARNINGS=1 node --experimental-vm-modules $(yarn bin jest) --runInBand" + "test": "NODE_NO_WARNINGS=1 node --experimental-vm-modules $(yarn bin jest) --runInBand" }, "jest": { "preset": "ts-jest/presets/default-esm", diff --git a/build_manifest.yml b/build_manifest.yml index 665e16f6b240..7f29658c01e1 100644 --- a/build_manifest.yml +++ b/build_manifest.yml @@ -135,13 +135,8 @@ yarn-project-base: - ^yarn-project/yarn.lock - ^yarn-project/.*/package.json$ dependencies: - - l1-contracts - bb.js - - noir - noir-packages - - boxes-files - - avm-transpiler - - noir-projects yarn-project: buildDir: yarn-project @@ -151,6 +146,9 @@ yarn-project: - ^yarn-project/cli/aztec-cli dependencies: - yarn-project-base + - l1-contracts + - boxes-files + - noir-projects yarn-project-prod: buildDir: yarn-project diff --git a/yarn-project/Dockerfile b/yarn-project/Dockerfile index 9de7d8776b82..aea67d0c5886 100644 --- a/yarn-project/Dockerfile +++ b/yarn-project/Dockerfile @@ -4,19 +4,24 @@ # - Run the tests. # - Run the formatter checks. # Any subsequent build steps needed to support downstream containers should be done in those containers build files. -FROM 278380418400.dkr.ecr.eu-west-2.amazonaws.com/yarn-project-base as builder +FROM 278380418400.dkr.ecr.eu-west-2.amazonaws.com/l1-contracts as contracts +FROM 278380418400.dkr.ecr.eu-west-2.amazonaws.com/noir-projects as noir-projects +FROM 278380418400.dkr.ecr.eu-west-2.amazonaws.com/boxes-files as boxes-files -# Copy in the entire workspace. +FROM 278380418400.dkr.ecr.eu-west-2.amazonaws.com/yarn-project-base +COPY --from=contracts /usr/src/l1-contracts /usr/src/l1-contracts +COPY --from=noir-projects /usr/src/noir-projects /usr/src/noir-projects +COPY --from=boxes-files /usr/src/boxes /usr/src/boxes COPY . . - +# Generate L1 contract TypeScript artifacts. +RUN cd l1-artifacts && ./scripts/generate-artifacts.sh && rm -rf /usr/src/l1-contracts # This is actually our code generation tool. Needed to build contract typescript wrappers. RUN yarn workspace @aztec/noir-compiler build -# Builds noir contracts (TODO: move this stage pre yarn-project). Generates typescript wrappers. +# Generates typescript wrappers. RUN yarn workspace @aztec/noir-contracts.js build:contracts # We need to build accounts as it needs to copy in account contracts from noir-contracts. RUN yarn workspace @aztec/accounts build:copy-contracts RUN yarn workspace @aztec/protocol-contracts build:copy-contracts RUN yarn workspace @aztec/noir-protocol-circuits-types build RUN yarn tsc -b - ENTRYPOINT ["yarn"] diff --git a/yarn-project/yarn-project-base/Dockerfile b/yarn-project/yarn-project-base/Dockerfile index 933ec230be2a..b53ed652bc0b 100644 --- a/yarn-project/yarn-project-base/Dockerfile +++ b/yarn-project/yarn-project-base/Dockerfile @@ -42,25 +42,15 @@ # RUN yarn workspaces focus --production && yarn cache clean && rm -rf ../**/src # - Create final slim image by copying needed dirs into a fresh image. # -FROM 278380418400.dkr.ecr.eu-west-2.amazonaws.com/l1-contracts as contracts FROM 278380418400.dkr.ecr.eu-west-2.amazonaws.com/bb.js as bb.js -FROM 278380418400.dkr.ecr.eu-west-2.amazonaws.com/noir-projects as noir-projects FROM 278380418400.dkr.ecr.eu-west-2.amazonaws.com/noir-packages as noir-packages -FROM 278380418400.dkr.ecr.eu-west-2.amazonaws.com/boxes-files as boxes-files FROM node:18.19.0 RUN apt update && apt install -y jq curl perl && rm -rf /var/lib/apt/lists/* && apt-get clean -# Copy L1 contracts. -COPY --from=contracts /usr/src/l1-contracts /usr/src/l1-contracts -# Copy in bb.js +# Copy in portalled packages. COPY --from=bb.js /usr/src/barretenberg/ts /usr/src/barretenberg/ts -# Copy in noir packages COPY --from=noir-packages /usr/src/noir/packages /usr/src/noir/packages -# Copy in noir projects -COPY --from=noir-projects /usr/src/noir-projects /usr/src/noir-projects -# Copy in boxes -COPY --from=boxes-files /usr/src/boxes /usr/src/boxes # We install a symlink to yarn-project's node_modules at a location that all portalled packages can find as they # walk up the tree as part of module resolution. The supposedly idiomatic way of supporting module resolution @@ -88,11 +78,5 @@ RUN yarn --immutable && rm -rf /root/.cache/puppeteer && /bin/bash -c '\ [[ $F =~ (.*-) ]] && ln $F /root/.yarn/berry/cache/${BASH_REMATCH[1]}8.zip; \ done' -# If everything's worked properly, we should no longer need access to the network. -RUN echo "enableNetwork: false" >> .yarnrc.yml - # Check package.json inheritance and tsconfig project references. -RUN yarn prepare:check - -# Generate L1 contract TypeScript artifacts. -RUN cd l1-artifacts && ./scripts/generate-artifacts.sh && rm -rf /usr/src/l1-contracts \ No newline at end of file +RUN yarn prepare:check \ No newline at end of file From d9a1853cc0474050f40ef52b196568b711f7eb07 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Mon, 26 Feb 2024 14:50:46 +0000 Subject: [PATCH 4/5] chore(ci): enforce formatting of noir rust code (#4765) Closes #4763 --- noir/aztec_macros/src/lib.rs | 107 ++++++++++++------ .../src/hir/def_collector/dc_crate.rs | 13 ++- noir/compiler/noirc_frontend/src/lib.rs | 2 +- noir/noirc_macros/src/lib.rs | 2 +- noir/scripts/test_native.sh | 4 +- 5 files changed, 86 insertions(+), 42 deletions(-) diff --git a/noir/aztec_macros/src/lib.rs b/noir/aztec_macros/src/lib.rs index a0a63d8a9a8e..8f70d5b5e86e 100644 --- a/noir/aztec_macros/src/lib.rs +++ b/noir/aztec_macros/src/lib.rs @@ -3,8 +3,10 @@ use std::vec; use convert_case::{Case, Casing}; use iter_extended::vecmap; +use noirc_errors::Location; use noirc_frontend::hir::def_collector::dc_crate::{UnresolvedFunctions, UnresolvedTraitImpl}; use noirc_frontend::hir::def_map::{LocalModuleId, ModuleId}; +use noirc_frontend::macros_api::parse_program; use noirc_frontend::macros_api::FieldElement; use noirc_frontend::macros_api::{ BlockExpression, CallExpression, CastExpression, Distinctness, Expression, ExpressionKind, @@ -20,8 +22,6 @@ use noirc_frontend::macros_api::{MacroError, MacroProcessor}; use noirc_frontend::macros_api::{ModuleDefId, NodeInterner, SortedModule, StructId}; use noirc_frontend::node_interner::{FuncId, TraitId, TraitImplId, TraitImplKind}; use noirc_frontend::Lambda; -use noirc_frontend::macros_api::parse_program; -use noirc_errors::Location; pub struct AztecMacro; impl MacroProcessor for AztecMacro { @@ -38,11 +38,16 @@ impl MacroProcessor for AztecMacro { &self, crate_id: &CrateId, context: &mut HirContext, - unresolved_traits_impls: &Vec, + unresolved_traits_impls: &[UnresolvedTraitImpl], collected_functions: &mut Vec, ) -> Result<(), (MacroError, FileId)> { if has_aztec_dependency(crate_id, context) { - inject_compute_note_hash_and_nullifier(crate_id, context, unresolved_traits_impls, collected_functions) + inject_compute_note_hash_and_nullifier( + crate_id, + context, + unresolved_traits_impls, + collected_functions, + ) } else { Ok(()) } @@ -351,7 +356,10 @@ fn check_for_storage_implementation(module: &SortedModule) -> bool { } // Check if "compute_note_hash_and_nullifier(AztecAddress,Field,Field,Field,[Field; N]) -> [Field; 4]" is defined -fn check_for_compute_note_hash_and_nullifier_definition(functions_data: &Vec<(LocalModuleId, FuncId, NoirFunction)>, module_id: LocalModuleId) -> bool { +fn check_for_compute_note_hash_and_nullifier_definition( + functions_data: &[(LocalModuleId, FuncId, NoirFunction)], + module_id: LocalModuleId, +) -> bool { functions_data.iter().filter(|func_data| func_data.0 == module_id).any(|func_data| { func_data.2.def.name.0.contents == "compute_note_hash_and_nullifier" && func_data.2.def.parameters.len() == 5 @@ -1611,18 +1619,21 @@ fn event_signature(event: &StructType) -> String { fn inject_compute_note_hash_and_nullifier( crate_id: &CrateId, context: &mut HirContext, - unresolved_traits_impls: &Vec, - collected_functions: &mut Vec, + unresolved_traits_impls: &[UnresolvedTraitImpl], + collected_functions: &mut [UnresolvedFunctions], ) -> Result<(), (MacroError, FileId)> { // We first fetch modules in this crate which correspond to contracts, along with their file id. - let contract_module_file_ids: Vec<(LocalModuleId, FileId)> = context.def_map(crate_id).expect("ICE: Missing crate in def_map") - .modules().iter() + let contract_module_file_ids: Vec<(LocalModuleId, FileId)> = context + .def_map(crate_id) + .expect("ICE: Missing crate in def_map") + .modules() + .iter() .filter(|(_, module)| module.is_contract) .map(|(idx, module)| (LocalModuleId(idx), module.location.file)) .collect(); // If the current crate does not contain a contract module we simply skip it. - if contract_module_file_ids.len() == 0 { + if contract_module_file_ids.is_empty() { return Ok(()); } else if contract_module_file_ids.len() != 1 { panic!("Found multiple contracts in the same crate"); @@ -1633,7 +1644,9 @@ fn inject_compute_note_hash_and_nullifier( // If compute_note_hash_and_nullifier is already defined by the user, we skip auto-generation in order to provide an // escape hatch for this mechanism. // TODO(#4647): improve this diagnosis and error messaging. - if collected_functions.iter().any(|coll_funcs_data| check_for_compute_note_hash_and_nullifier_definition(&coll_funcs_data.functions, module_id)) { + if collected_functions.iter().any(|coll_funcs_data| { + check_for_compute_note_hash_and_nullifier_definition(&coll_funcs_data.functions, module_id) + }) { return Ok(()); } @@ -1647,7 +1660,7 @@ fn inject_compute_note_hash_and_nullifier( // And inject the newly created function into the contract. - // TODO(#4373): We don't have a reasonable location for the source code of this autogenerated function, so we simply + // TODO(#4373): We don't have a reasonable location for the source code of this autogenerated function, so we simply // pass an empty span. This function should not produce errors anyway so this should not matter. let location = Location::new(Span::empty(0), file_id); @@ -1656,7 +1669,12 @@ fn inject_compute_note_hash_and_nullifier( // on collected but unresolved functions. let func_id = context.def_interner.push_empty_fn(); - context.def_interner.push_function(func_id, &func.def, ModuleId { krate: *crate_id, local_id: module_id }, location); + context.def_interner.push_function( + func_id, + &func.def, + ModuleId { krate: *crate_id, local_id: module_id }, + location, + ); context.def_map_mut(crate_id).unwrap() .modules_mut()[module_id.0] @@ -1666,8 +1684,10 @@ fn inject_compute_note_hash_and_nullifier( "Failed to declare the autogenerated compute_note_hash_and_nullifier function, likely due to a duplicate definition. See https://github.com/AztecProtocol/aztec-packages/issues/4647." ); - collected_functions.iter_mut() - .find(|fns| fns.file_id == file_id).expect("ICE: no functions found in contract file") + collected_functions + .iter_mut() + .find(|fns| fns.file_id == file_id) + .expect("ICE: no functions found in contract file") .push_fn(module_id, func_id, func.clone()); Ok(()) @@ -1676,15 +1696,15 @@ fn inject_compute_note_hash_and_nullifier( // Fetches the name of all structs that implement trait_name, both in the current crate and all of its dependencies. fn fetch_struct_trait_impls( context: &mut HirContext, - unresolved_traits_impls: &Vec, - trait_name: &str + unresolved_traits_impls: &[UnresolvedTraitImpl], + trait_name: &str, ) -> Vec { let mut struct_typenames: Vec = Vec::new(); - // These structs can be declared in either external crates or the current one. External crates that contain + // These structs can be declared in either external crates or the current one. External crates that contain // dependencies have already been processed and resolved, but are available here via the NodeInterner. Note that // crates on which the current crate does not depend on may not have been processed, and will be ignored. - for trait_impl_id in 0..(&context.def_interner.next_trait_impl_id()).0 { + for trait_impl_id in 0..context.def_interner.next_trait_impl_id().0 { let trait_impl = &context.def_interner.get_trait_implementation(TraitImplId(trait_impl_id)); if trait_impl.borrow().ident.0.contents == *trait_name { @@ -1697,13 +1717,26 @@ fn fetch_struct_trait_impls( } // This crate's traits and impls have not yet been resolved, so we look for impls in unresolved_trait_impls. - struct_typenames.extend(unresolved_traits_impls.iter() - .filter(|trait_impl| - trait_impl.trait_path.segments.last().expect("ICE: empty trait_impl path").0.contents == *trait_name) - .filter_map(|trait_impl| match &trait_impl.object_type.typ { - UnresolvedTypeData::Named(path, _, _) => Some(path.segments.last().unwrap().0.contents.clone()), - _ => None, - })); + struct_typenames.extend( + unresolved_traits_impls + .iter() + .filter(|trait_impl| { + trait_impl + .trait_path + .segments + .last() + .expect("ICE: empty trait_impl path") + .0 + .contents + == *trait_name + }) + .filter_map(|trait_impl| match &trait_impl.object_type.typ { + UnresolvedTypeData::Named(path, _, _) => { + Some(path.segments.last().unwrap().0.contents.clone()) + } + _ => None, + }), + ); struct_typenames } @@ -1722,11 +1755,11 @@ fn generate_compute_note_hash_and_nullifier(note_types: &Vec) -> NoirFun } fn generate_compute_note_hash_and_nullifier_source(note_types: &Vec) -> String { - // TODO(#4649): The serialized_note parameter is a fixed-size array, but we don't know what length it should have. + // TODO(#4649): The serialized_note parameter is a fixed-size array, but we don't know what length it should have. // For now we hardcode it to 20, which is the same as MAX_NOTE_FIELDS_LENGTH. - if note_types.len() == 0 { - // TODO(#4520): Even if the contract does not include any notes, other parts of the stack expect for this + if note_types.is_empty() { + // TODO(#4520): Even if the contract does not include any notes, other parts of the stack expect for this // function to exist, so we include a dummy version. We likely should error out here instead. " unconstrained fn compute_note_hash_and_nullifier( @@ -1737,7 +1770,8 @@ fn generate_compute_note_hash_and_nullifier_source(note_types: &Vec) -> serialized_note: [Field; 20] ) -> pub [Field; 4] { [0, 0, 0, 0] - }".to_string() + }" + .to_string() } else { // For contracts that include notes we do a simple if-else chain comparing note_type_id with the different // get_note_type_id of each of the note types. @@ -1749,12 +1783,14 @@ fn generate_compute_note_hash_and_nullifier_source(note_types: &Vec) -> , note_type)).collect(); // TODO(#4520): error out on the else instead of returning a zero array - let full_if_statement = if_statements.join(" else ") + " + let full_if_statement = if_statements.join(" else ") + + " else { [0, 0, 0, 0] }"; - format!(" + format!( + " unconstrained fn compute_note_hash_and_nullifier( contract_address: AztecAddress, nonce: Field, @@ -1765,7 +1801,8 @@ fn generate_compute_note_hash_and_nullifier_source(note_types: &Vec) -> let note_header = NoteHeader::new(contract_address, nonce, storage_slot); {} - }}", full_if_statement) + }}", + full_if_statement + ) + } } - -} \ No newline at end of file diff --git a/noir/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/noir/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index b10ca0f07693..7f36af5b30e0 100644 --- a/noir/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/noir/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -258,11 +258,16 @@ impl DefCollector { // TODO(#4653): generalize this function for macro_processor in ¯o_processors { - macro_processor.process_unresolved_traits_impls(&crate_id, context, &def_collector.collected_traits_impls, &mut def_collector.collected_functions).unwrap_or_else( - |(macro_err, file_id)| { + macro_processor + .process_unresolved_traits_impls( + &crate_id, + context, + &def_collector.collected_traits_impls, + &mut def_collector.collected_functions, + ) + .unwrap_or_else(|(macro_err, file_id)| { errors.push((macro_err.into(), file_id)); - }, - ); + }); } inject_prelude(crate_id, context, crate_root, &mut def_collector.collected_imports); diff --git a/noir/compiler/noirc_frontend/src/lib.rs b/noir/compiler/noirc_frontend/src/lib.rs index 55f5fb1ca404..be007929fc43 100644 --- a/noir/compiler/noirc_frontend/src/lib.rs +++ b/noir/compiler/noirc_frontend/src/lib.rs @@ -81,7 +81,7 @@ pub mod macros_api { &self, _crate_id: &CrateId, _context: &mut HirContext, - _unresolved_traits_impls: &Vec, + _unresolved_traits_impls: &[UnresolvedTraitImpl], _collected_functions: &mut Vec, ) -> Result<(), (MacroError, FileId)>; diff --git a/noir/noirc_macros/src/lib.rs b/noir/noirc_macros/src/lib.rs index 55109dbfba09..9a916843200d 100644 --- a/noir/noirc_macros/src/lib.rs +++ b/noir/noirc_macros/src/lib.rs @@ -22,7 +22,7 @@ impl MacroProcessor for AssertMessageMacro { &self, _crate_id: &CrateId, _context: &mut HirContext, - _unresolevd_traits_impls: &Vec, + _unresolved_traits_impls: &[UnresolvedTraitImpl], _collected_functions: &mut Vec, ) -> Result<(), (MacroError, FileId)> { Ok(()) diff --git a/noir/scripts/test_native.sh b/noir/scripts/test_native.sh index bc1c47ecf12a..9b9aa0ce4d70 100755 --- a/noir/scripts/test_native.sh +++ b/noir/scripts/test_native.sh @@ -12,4 +12,6 @@ else export GIT_COMMIT=$(git rev-parse --verify HEAD) fi -cargo test --workspace --locked --release \ No newline at end of file +cargo fmt --all --check +cargo clippy --workspace --locked --release +cargo test --workspace --locked --release From 74025170288e39e1d7516f57df94f22bc30f663c Mon Sep 17 00:00:00 2001 From: Innokentii Sennovskii Date: Mon, 26 Feb 2024 15:21:51 +0000 Subject: [PATCH 5/5] feat: Goblin Translator Fuzzer (#4752) A small and dumb fuzzer for Translator circuit builder and composer, which helped find the finicky test issue. Also adds a preset to compile with fuzzing+debug+asan --- barretenberg/cpp/CMakePresets.json | 22 +++++ .../goblin_translator.fuzzer.hpp | 84 +++++++++++++++++++ ...blin_translator_circuit_builder.fuzzer.cpp | 46 ++++++++++ .../goblin_translator_composer.fuzzer.cpp | 47 +++++++++++ 4 files changed, 199 insertions(+) create mode 100644 barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_translator.fuzzer.hpp create mode 100644 barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_translator_circuit_builder.fuzzer.cpp create mode 100644 barretenberg/cpp/src/barretenberg/translator_vm/goblin_translator_composer.fuzzer.cpp diff --git a/barretenberg/cpp/CMakePresets.json b/barretenberg/cpp/CMakePresets.json index 002a19d456c1..85e8c0c94908 100644 --- a/barretenberg/cpp/CMakePresets.json +++ b/barretenberg/cpp/CMakePresets.json @@ -131,6 +131,18 @@ "FUZZING": "ON" } }, + { + "name": "fuzzing-asan", + "displayName": "Build with fuzzing", + "description": "Build default preset but with fuzzing enabled", + "inherits": "clang16-dbg", + "binaryDir": "build-fuzzing", + "cacheVariables": { + "FUZZING": "ON", + "ENABLE_ASAN": "ON", + "DISABLE_ASM": "ON" + } + }, { "name": "smt-verification", "displayName": "Build with smt verificaiton", @@ -377,6 +389,11 @@ "inherits": "clang16", "configurePreset": "fuzzing" }, +{ + "name": "fuzzing-asan", + "inherits": "clang16-dbg", + "configurePreset": "fuzzing-asan" + }, { "name": "gperftools", "inherits": "clang16", @@ -501,6 +518,11 @@ "inherits": "default", "configurePreset": "fuzzing" }, +{ + "name": "fuzzing-asan", + "inherits": "clang16-dbg", + "configurePreset": "fuzzing-asan" + }, { "name": "smt-verification", "inherits": "clang16", diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_translator.fuzzer.hpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_translator.fuzzer.hpp new file mode 100644 index 000000000000..57d54288c168 --- /dev/null +++ b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_translator.fuzzer.hpp @@ -0,0 +1,84 @@ +/** + * @file goblin_translator.fuzzer.hpp + * @author Rumata888 + * @brief Contains common procedures used by the circuit builder fuzzer and the composer fuzzer + * @date 2024-02-25 + * + */ +#include "barretenberg/numeric/uint256/uint256.hpp" +#include "goblin_translator_circuit_builder.hpp" + +using namespace bb; + +using Fr = curve::BN254::ScalarField; +using Fq = curve::BN254::BaseField; +using G1 = curve::BN254::AffineElement; + +/** + * @brief Parse raw operations for ECCOpQueue from the data stream + * + * @param data pointer to data + * @param size size in bytes + * @return std::vector + */ +std::vector parse_operations(const unsigned char* data, size_t size) +{ + std::vector raw_ops; + + size_t size_left = size; + // Just iterate and parse until there's no data left + while (size_left >= sizeof(ECCOpQueue::ECCVMOperation)) { + raw_ops.emplace_back((ECCOpQueue::ECCVMOperation*)(data + (size - size_left))); + size_left -= sizeof(ECCOpQueue::ECCVMOperation); + } + return raw_ops; +} + +/** + * @brief Try to parse out the batching and evaluating challenges and then the ECCOpQUeue from the data + * + * @param data pointer to the buffer + * @param size size of the buffer + * @return std::optional>> + */ +std::optional>> parse_and_construct_opqueue(const unsigned char* data, + size_t size) +{ + std::vector raw_ops; + + // Try to parse batching challenge + size_t size_left = size; + if (size_left < sizeof(uint256_t)) { + return {}; + } + const auto batching_challenge = Fq(*((uint256_t*)data)); + + // Try to parse evaluation challenge + size_left -= sizeof(uint256_t); + if (size_left < sizeof(uint256_t)) { + return {}; + } + const auto x = Fq(*((uint256_t*)data)); + if (x.is_zero()) { + return {}; + } + size_left -= sizeof(uint256_t); + + // Try to parse operations + raw_ops = parse_operations(data + (size - size_left), size_left); + if (raw_ops.empty()) { + return {}; + } + + // Add a padding element to avoid non-zero commitments + const auto p_x = uint256_t(0xd3c208c16d87cfd3, 0xd97816a916871ca8, 0x9b85045b68181585, 0x30644e72e131a02); + const auto p_y = uint256_t(0x3ce1cc9c7e645a83, 0x2edac647851e3ac5, 0xd0cbe61fced2bc53, 0x1a76dae6d3272396); + auto padding_element = G1(p_x, p_y); + auto padding_scalar = -Fr::one(); + auto ecc_op_queue = std::make_shared(); + ecc_op_queue->raw_ops = raw_ops; + ecc_op_queue->mul_accumulate(padding_element, padding_scalar); + + // Create circuit builder and feed the queue inside + return std::make_tuple(batching_challenge, x, ecc_op_queue); +} diff --git a/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_translator_circuit_builder.fuzzer.cpp b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_translator_circuit_builder.fuzzer.cpp new file mode 100644 index 000000000000..216eea10ca1a --- /dev/null +++ b/barretenberg/cpp/src/barretenberg/proof_system/circuit_builder/goblin_translator_circuit_builder.fuzzer.cpp @@ -0,0 +1,46 @@ +#include "barretenberg/proof_system/circuit_builder/goblin_translator.fuzzer.hpp" +/** + * @brief A very primitive fuzzing harness, no interesting mutations, just parse and throw at the circuit builder + * + */ +extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data, size_t size) +{ + // Parse the queue and challenges + // TODO(Rumata888): composer generates the initial challenge through FS, so we have to do that, too + auto parsing_result = parse_and_construct_opqueue(data, size); + if (!parsing_result.has_value()) { + return 0; + } + auto [batching_challenge, x, op_queue] = parsing_result.value(); + // Construct the circuit + auto circuit_builder = GoblinTranslatorCircuitBuilder(batching_challenge, x, op_queue); + + Fq x_inv = x.invert(); + auto op_accumulator = Fq(0); + auto p_x_accumulator = Fq(0); + auto p_y_accumulator = Fq(0); + auto z_1_accumulator = Fq(0); + auto z_2_accumulator = Fq(0); + // Compute the batched evaluation of polynomials (multiplying by inverse to go from lower to higher) + for (auto& ecc_op : op_queue->raw_ops) { + op_accumulator = op_accumulator * x_inv + ecc_op.get_opcode_value(); + p_x_accumulator = p_x_accumulator * x_inv + ecc_op.base_point.x; + p_y_accumulator = p_y_accumulator * x_inv + ecc_op.base_point.y; + z_1_accumulator = z_1_accumulator * x_inv + ecc_op.z1; + z_2_accumulator = z_2_accumulator * x_inv + ecc_op.z2; + } + Fq x_pow = x.pow(op_queue->raw_ops.size() - 1); + + // Multiply by an appropriate power of x to get rid of the inverses + Fq result = ((((z_2_accumulator * batching_challenge + z_1_accumulator) * batching_challenge + p_y_accumulator) * + batching_challenge + + p_x_accumulator) * + batching_challenge + + op_accumulator) * + x_pow; + + // The data is malformed, so just call check_circuit, but ignore the output + circuit_builder.check_circuit(); + (void)result; + return 0; +} \ No newline at end of file diff --git a/barretenberg/cpp/src/barretenberg/translator_vm/goblin_translator_composer.fuzzer.cpp b/barretenberg/cpp/src/barretenberg/translator_vm/goblin_translator_composer.fuzzer.cpp new file mode 100644 index 000000000000..4e2ad5080633 --- /dev/null +++ b/barretenberg/cpp/src/barretenberg/translator_vm/goblin_translator_composer.fuzzer.cpp @@ -0,0 +1,47 @@ +#include "barretenberg/translator_vm/goblin_translator_composer.hpp" +#include "barretenberg/proof_system/circuit_builder/goblin_translator.fuzzer.hpp" +#include "barretenberg/translator_vm/goblin_translator_prover.hpp" +extern "C" void LLVMFuzzerInitialize(int*, char***) +{ + srs::init_crs_factory("../srs_db/ignition"); +} +/** + * @brief A very primitive fuzzer for the composer + * + * @details Super-slow. Shouldn't be run on its own. First you need to run the circuit builder fuzzer, then minimize the + * corpus and then just use that corpus + * + */ +extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data, size_t size) +{ + // Parse challenges and opqueue from data + auto parsing_result = parse_and_construct_opqueue(data, size); + if (!parsing_result.has_value()) { + return 0; + } + auto [batching_challenge_init, x, op_queue] = parsing_result.value(); + auto prover_transcript = std::make_shared(); + prover_transcript->send_to_verifier("init", batching_challenge_init); + prover_transcript->export_proof(); + Fq translation_batching_challenge = prover_transcript->template get_challenge("Translation:batching_challenge"); + + // Construct circuit + auto circuit_builder = GoblinTranslatorCircuitBuilder(translation_batching_challenge, x, op_queue); + + // Check that the circuit passes + bool checked = circuit_builder.check_circuit(); + + // Construct proof + auto composer = bb::GoblinTranslatorComposer(); + auto prover = composer.create_prover(circuit_builder, prover_transcript); + auto proof = prover.construct_proof(); + + // Verify proof + auto verifier_transcript = std::make_shared(prover_transcript->proof_data); + verifier_transcript->template receive_from_prover("init"); + auto verifier = composer.create_verifier(circuit_builder, verifier_transcript); + bool verified = verifier.verify_proof(proof); + (void)checked; + (void)verified; + return 0; +} \ No newline at end of file