diff --git a/.noir-sync-commit b/.noir-sync-commit index 4ebd701fd95e..58cd9057d55a 100644 --- a/.noir-sync-commit +++ b/.noir-sync-commit @@ -1 +1 @@ -49a095ded5cd33795bcdac60cbd98ce7c5ab9198 +fdfe2bf752771b9611dc71953d50423b4ae7ec44 diff --git a/noir/noir-repo/.github/benchmark_projects.yml b/noir/noir-repo/.github/benchmark_projects.yml index 17eb7aa96a67..e1bcb080829b 100644 --- a/noir/noir-repo/.github/benchmark_projects.yml +++ b/noir/noir-repo/.github/benchmark_projects.yml @@ -1,4 +1,4 @@ -define: &AZ_COMMIT 6c0b83d4b73408f87acfa080d52a81c411e47336 +define: &AZ_COMMIT 1350f93c3e9af8f601ca67ca3e67d0127c9767b6 projects: private-kernel-inner: repo: AztecProtocol/aztec-packages @@ -73,7 +73,7 @@ projects: path: noir-projects/noir-protocol-circuits/crates/rollup-block-root num_runs: 1 timeout: 60 - compilation-timeout: 100 + compilation-timeout: 110 execution-timeout: 40 compilation-memory-limit: 7000 execution-memory-limit: 1500 diff --git a/noir/noir-repo/.github/workflows/reports.yml b/noir/noir-repo/.github/workflows/reports.yml index 4e49b2507527..37f313259ad3 100644 --- a/noir/noir-repo/.github/workflows/reports.yml +++ b/noir/noir-repo/.github/workflows/reports.yml @@ -282,9 +282,9 @@ jobs: strategy: fail-fast: false matrix: - project: ${{ fromJson( needs.benchmark-projects-list.outputs.projects )}} + include: ${{ fromJson( needs.benchmark-projects-list.outputs.projects )}} - name: External repo compilation and execution reports - ${{ matrix.project.repo }}/${{ matrix.project.path }} + name: External repo compilation and execution reports - ${{ matrix.repo }}/${{ matrix.path }} steps: - uses: actions/checkout@v4 with: @@ -302,32 +302,77 @@ jobs: - name: Checkout uses: actions/checkout@v4 with: - repository: ${{ matrix.project.repo }} + repository: ${{ matrix.repo }} path: test-repo - ref: ${{ matrix.project.ref }} + ref: ${{ matrix.ref }} - name: Fetch noir dependencies - working-directory: ./test-repo/${{ matrix.project.path }} + working-directory: ./test-repo/${{ matrix.path }} run: | # We run `nargo check` to pre-fetch any dependencies so we don't measure the time to download these # when benchmarking. nargo check - name: Generate compilation report - working-directory: ./test-repo/${{ matrix.project.path }} + id: compilation_report + working-directory: ./test-repo/${{ matrix.path }} run: | mv /home/runner/work/noir/noir/scripts/test_programs/compilation_report.sh ./compilation_report.sh touch parse_time.sh chmod +x parse_time.sh cp /home/runner/work/noir/noir/scripts/test_programs/parse_time.sh ./parse_time.sh - ./compilation_report.sh 1 ${{ matrix.project.num_runs }} + ./compilation_report.sh 1 ${{ matrix.num_runs }} + + PACKAGE_NAME=${{ matrix.path }} + PACKAGE_NAME=$(basename $PACKAGE_NAME) + REPORT_NAME=compilation_report_$PACKAGE_NAME.json + REPORT_PATH=$(pwd)/$REPORT_NAME + mv ./compilation_report.json $REPORT_PATH + + echo "report_name=$REPORT_NAME" >> $GITHUB_OUTPUT + echo "report_path=$REPORT_PATH" >> $GITHUB_OUTPUT env: - FLAGS: ${{ matrix.project.flags }} + FLAGS: ${{ matrix.flags }} + + - name: Upload compilation report + uses: actions/upload-artifact@v4 + with: + name: ${{ steps.compilation_report.outputs.report_name }} + path: ${{ steps.compilation_report.outputs.report_path }} + retention-days: 3 + overwrite: true + + - name: Generate execution report + id: execution_report + working-directory: ./test-repo/${{ matrix.path }} + if: ${{ !matrix.cannot_execute }} + run: | + mv /home/runner/work/noir/noir/scripts/test_programs/execution_report.sh ./execution_report.sh + mv /home/runner/work/noir/noir/scripts/test_programs/parse_time.sh ./parse_time.sh + ./execution_report.sh 1 ${{ matrix.num_runs }} + + PACKAGE_NAME=${{ matrix.path }} + PACKAGE_NAME=$(basename $PACKAGE_NAME) + REPORT_NAME=execution_report_$PACKAGE_NAME.json + REPORT_PATH=$(pwd)/$REPORT_NAME + mv ./execution_report.json $REPORT_PATH + + echo "report_name=$REPORT_NAME" >> $GITHUB_OUTPUT + echo "report_path=$REPORT_PATH" >> $GITHUB_OUTPUT + + - name: Upload execution report + if: ${{ !matrix.cannot_execute }} + uses: actions/upload-artifact@v4 + with: + name: ${{ steps.execution_report.outputs.report_name }} + path: ${{ steps.execution_report.outputs.report_path }} + retention-days: 3 + overwrite: true - name: Check compilation time limit run: | - TIME=$(jq '.[0].value' ./test-repo/${{ matrix.project.path }}/compilation_report.json) - TIME_LIMIT=${{ matrix.project.compilation-timeout }} + TIME=$(jq '.[0].value' ${{ steps.compilation_report.outputs.report_path }}) + TIME_LIMIT=${{ matrix.compilation-timeout }} if awk 'BEGIN{exit !(ARGV[1]>ARGV[2])}' "$TIME" "$TIME_LIMIT"; then # Don't bump this timeout without understanding why this has happened and confirming that you're not the cause. echo "Failing due to compilation exceeding timeout..." @@ -336,19 +381,11 @@ jobs: exit 1 fi - - name: Generate execution report - working-directory: ./test-repo/${{ matrix.project.path }} - if: ${{ !matrix.project.cannot_execute }} - run: | - mv /home/runner/work/noir/noir/scripts/test_programs/execution_report.sh ./execution_report.sh - mv /home/runner/work/noir/noir/scripts/test_programs/parse_time.sh ./parse_time.sh - ./execution_report.sh 1 ${{ matrix.project.num_runs }} - - name: Check execution time limit - if: ${{ !matrix.project.cannot_execute }} + if: ${{ !matrix.cannot_execute }} run: | - TIME=$(jq '.[0].value' ./test-repo/${{ matrix.project.path }}/execution_report.json) - TIME_LIMIT=${{ matrix.project.execution-timeout }} + TIME=$(jq '.[0].value' ${{ steps.execution_report.outputs.report_path }}) + TIME_LIMIT=${{ matrix.execution-timeout }} if awk 'BEGIN{exit !(ARGV[1]>ARGV[2])}' "$TIME" "$TIME_LIMIT"; then # Don't bump this timeout without understanding why this has happened and confirming that you're not the cause. echo "Failing due to execution exceeding timeout..." @@ -357,41 +394,6 @@ jobs: exit 1 fi - - name: Move compilation report - id: compilation_report - shell: bash - run: | - PACKAGE_NAME=${{ matrix.project.path }} - PACKAGE_NAME=$(basename $PACKAGE_NAME) - mv ./test-repo/${{ matrix.project.path }}/compilation_report.json ./compilation_report_$PACKAGE_NAME.json - echo "compilation_report_name=$PACKAGE_NAME" >> $GITHUB_OUTPUT - - - name: Move execution report - id: execution_report - shell: bash - if: ${{ !matrix.project.cannot_execute }} - run: | - PACKAGE_NAME=${{ matrix.project.path }} - PACKAGE_NAME=$(basename $PACKAGE_NAME) - mv ./test-repo/${{ matrix.project.path }}/execution_report.json ./execution_report_$PACKAGE_NAME.json - echo "execution_report_name=$PACKAGE_NAME" >> $GITHUB_OUTPUT - - - name: Upload compilation report - uses: actions/upload-artifact@v4 - with: - name: compilation_report_${{ steps.compilation_report.outputs.compilation_report_name }} - path: compilation_report_${{ steps.compilation_report.outputs.compilation_report_name }}.json - retention-days: 3 - overwrite: true - - - name: Upload execution report - uses: actions/upload-artifact@v4 - with: - name: execution_report_${{ steps.execution_report.outputs.execution_report_name }} - path: execution_report_${{ steps.execution_report.outputs.execution_report_name }}.json - retention-days: 3 - overwrite: true - external_repo_memory_report: needs: [build-nargo, benchmark-projects-list] runs-on: ubuntu-22.04 @@ -415,6 +417,9 @@ jobs: - name: Download nargo binary uses: ./scripts/.github/actions/download-nargo + - name: Download nargo binary + uses: ./scripts/.github/actions/download-nargo + - name: Checkout uses: actions/checkout@v4 with: @@ -423,44 +428,62 @@ jobs: ref: ${{ matrix.ref }} - name: Generate compilation memory report + id: compilation_report working-directory: ./test-repo/${{ matrix.path }} run: | mv /home/runner/work/noir/noir/scripts/test_programs/memory_report.sh ./memory_report.sh mv /home/runner/work/noir/noir/scripts/test_programs/parse_memory.sh ./parse_memory.sh ./memory_report.sh 1 + # Rename the memory report as the execution report is about to write to the same file - cp memory_report.json compilation_memory_report.json + PACKAGE_NAME=${{ matrix.path }} + PACKAGE_NAME=$(basename $PACKAGE_NAME) + REPORT_NAME=compilation_mem_report_$PACKAGE_NAME.json + REPORT_PATH=$(pwd)/$REPORT_NAME + mv ./memory_report.json $REPORT_PATH + + echo "report_name=$REPORT_NAME" >> $GITHUB_OUTPUT + echo "report_path=$REPORT_PATH" >> $GITHUB_OUTPUT env: FLAGS: ${{ matrix.flags }} - - name: Check compilation memory limit - run: | - MEMORY=$(jq '.[0].value' ./test-repo/${{ matrix.path }}/compilation_memory_report.json) - MEMORY_LIMIT=${{ matrix.compilation-memory-limit }} - if awk 'BEGIN{exit !(ARGV[1]>ARGV[2])}' "$MEMORY" "$MEMORY_LIMIT"; then - # Don't bump this limit without understanding why this has happened and confirming that you're not the cause. - echo "Failing due to compilation exceeding memory limit..." - echo "Limit: "$MEMORY_LIMIT"MB" - echo "Compilation took: "$MEMORY"MB". - exit 1 - fi + - name: Upload compilation memory report + uses: actions/upload-artifact@v4 + with: + name: ${{ steps.compilation_report.outputs.report_name }} + path: ${{ steps.compilation_report.outputs.report_path }} + retention-days: 3 + overwrite: true - - name: Check compilation memory limit + - name: Generate execution memory report + id: execution_report + working-directory: ./test-repo/${{ matrix.path }} + if: ${{ !matrix.cannot_execute }} run: | - MEMORY=$(jq '.[0].value' ./test-repo/${{ matrix.project.path }}/compilation_memory_report.json) - MEMORY_LIMIT=6000 - if awk 'BEGIN{exit !(ARGV[1]>ARGV[2])}' "$MEMORY" "$MEMORY_LIMIT"; then - # Don't bump this limit without understanding why this has happened and confirming that you're not the cause. - echo "Failing due to compilation exceeding memory limit..." - echo "Limit: "$MEMORY_LIMIT"MB" - echo "Compilation took: "$MEMORY"MB". - exit 1 - fi + ./memory_report.sh 1 1 + + PACKAGE_NAME=${{ matrix.path }} + PACKAGE_NAME=$(basename $PACKAGE_NAME) + REPORT_NAME=execution_mem_report_$PACKAGE_NAME.json + REPORT_PATH=$(pwd)/$REPORT_NAME + mv ./memory_report.json $REPORT_PATH + + echo "report_name=$REPORT_NAME" >> $GITHUB_OUTPUT + echo "report_path=$REPORT_PATH" >> $GITHUB_OUTPUT + + - name: Upload execution memory report + if: ${{ !matrix.cannot_execute }} + uses: actions/upload-artifact@v4 + with: + name: ${{ steps.execution_report.outputs.report_name }} + path: ${{ steps.execution_report.outputs.report_path }} + retention-days: 3 + overwrite: true - name: Check compilation memory limit run: | - MEMORY=$(jq '.[0].value' ./test-repo/${{ matrix.project.path }}/compilation_memory_report.json) - MEMORY_LIMIT=6000 + MEMORY=$(jq '.[0].value' ${{ steps.compilation_report.outputs.report_path }}) + MEMORY_LIMIT=${{ matrix.compilation-memory-limit }} if awk 'BEGIN{exit !(ARGV[1]>ARGV[2])}' "$MEMORY" "$MEMORY_LIMIT"; then # Don't bump this limit without understanding why this has happened and confirming that you're not the cause. echo "Failing due to compilation exceeding memory limit..." @@ -469,16 +492,10 @@ jobs: exit 1 fi - - name: Generate execution memory report - working-directory: ./test-repo/${{ matrix.path }} - if: ${{ !matrix.cannot_execute }} - run: | - ./memory_report.sh 1 1 - - name: Check execution memory limit if: ${{ !matrix.cannot_execute }} run: | - MEMORY=$(jq '.[0].value' ./test-repo/${{ matrix.path }}/memory_report.json) + MEMORY=$(jq '.[0].value' ${{ steps.execution_report.outputs.report_path }}) MEMORY_LIMIT=${{ matrix.execution-memory-limit }} if awk 'BEGIN{exit !(ARGV[1]>ARGV[2])}' "$MEMORY" "$MEMORY_LIMIT"; then # Don't bump this limit without understanding why this has happened and confirming that you're not the cause. @@ -488,40 +505,6 @@ jobs: exit 1 fi - - name: Move compilation report - id: compilation_mem_report - shell: bash - run: | - PACKAGE_NAME=${{ matrix.path }} - PACKAGE_NAME=$(basename $PACKAGE_NAME) - mv ./test-repo/${{ matrix.path }}/compilation_memory_report.json ./memory_report_$PACKAGE_NAME.json - echo "memory_report_name=$PACKAGE_NAME" >> $GITHUB_OUTPUT - - - name: Upload compilation memory report - uses: actions/upload-artifact@v4 - with: - name: compilation_mem_report_${{ steps.compilation_mem_report.outputs.memory_report_name }} - path: memory_report_${{ steps.compilation_mem_report.outputs.memory_report_name }}.json - retention-days: 3 - overwrite: true - - - name: Move execution report - id: execution_mem_report - if: ${{ !matrix.cannot_execute }} - run: | - PACKAGE_NAME=${{ matrix.path }} - PACKAGE_NAME=$(basename $PACKAGE_NAME) - mv ./test-repo/${{ matrix.path }}/memory_report.json ./memory_report_$PACKAGE_NAME.json - echo "memory_report_name=$PACKAGE_NAME" >> $GITHUB_OUTPUT - - - name: Upload execution memory report - uses: actions/upload-artifact@v4 - with: - name: execution_mem_report_${{ steps.execution_mem_report.outputs.memory_report_name }} - path: memory_report_${{ steps.execution_mem_report.outputs.memory_report_name }}.json - retention-days: 3 - overwrite: true - upload_compilation_report: name: Upload compilation report needs: [generate_compilation_and_execution_report, external_repo_compilation_and_execution_report] diff --git a/noir/noir-repo/.github/workflows/test-js-packages.yml b/noir/noir-repo/.github/workflows/test-js-packages.yml index fe573b608893..0af0e41d46ae 100644 --- a/noir/noir-repo/.github/workflows/test-js-packages.yml +++ b/noir/noir-repo/.github/workflows/test-js-packages.yml @@ -541,18 +541,6 @@ jobs: fi env: NARGO_IGNORE_TEST_FAILURES_FROM_FOREIGN_CALLS: true - - - name: Check test time limit - run: | - TIME=$(jq '.[0].value' ./test-repo/${{ matrix.path }}/${{ steps.test_report.outputs.test_report_name }}.json) - if awk 'BEGIN{exit !(ARGV[1]>ARGV[2])}' "$TIME" "${{ matrix.timeout }}"; then - # Don't bump this timeout without understanding why this has happened and confirming that you're not the cause. - echo "Failing due to test suite exceeding timeout..." - echo "Timeout: ${{ matrix.timeout }}" - echo "Test suite took: $TIME". - exit 1 - fi - - name: Compare test results working-directory: ./noir-repo run: .github/scripts/check_test_results.sh .github/critical_libraries_status/${{ matrix.repo }}/${{ matrix.path }}.failures.jsonl .github/critical_libraries_status/${{ matrix.repo }}/${{ matrix.path }}.actual.jsonl @@ -566,6 +554,17 @@ jobs: retention-days: 3 overwrite: true + - name: Check test time limit + run: | + TIME=$(jq '.[0].value' ./test-repo/${{ matrix.path }}/${{ steps.test_report.outputs.test_report_name }}.json) + if awk 'BEGIN{exit !(ARGV[1]>ARGV[2])}' "$TIME" "${{ matrix.timeout }}"; then + # Don't bump this timeout without understanding why this has happened and confirming that you're not the cause. + echo "Failing due to test suite exceeding timeout..." + echo "Timeout: ${{ matrix.timeout }}" + echo "Test suite took: $TIME". + exit 1 + fi + compile-noir-contracts: needs: [build-nargo] runs-on: ubuntu-22.04 diff --git a/noir/noir-repo/Cargo.lock b/noir/noir-repo/Cargo.lock index a522ccb3bed5..8b6912452f04 100644 --- a/noir/noir-repo/Cargo.lock +++ b/noir/noir-repo/Cargo.lock @@ -90,10 +90,10 @@ dependencies = [ "color-eyre", "const_format", "nargo", + "noir_artifact_cli", "paste", "proptest", "rand", - "thiserror", "toml", "tracing-appender", "tracing-subscriber", @@ -3155,6 +3155,30 @@ dependencies = [ "libc", ] +[[package]] +name = "noir_artifact_cli" +version = "1.0.0-beta.2" +dependencies = [ + "acir", + "acvm", + "bn254_blackbox_solver", + "clap", + "color-eyre", + "const_format", + "fm", + "nargo", + "noirc_abi", + "noirc_artifacts", + "noirc_artifacts_info", + "noirc_driver", + "noirc_errors", + "serde", + "serde_json", + "thiserror", + "toml", + "tracing-subscriber", +] + [[package]] name = "noir_debugger" version = "1.0.0-beta.2" diff --git a/noir/noir-repo/Cargo.toml b/noir/noir-repo/Cargo.toml index 02c4592a5d4c..c91eb816083f 100644 --- a/noir/noir-repo/Cargo.toml +++ b/noir/noir-repo/Cargo.toml @@ -23,6 +23,7 @@ members = [ "tooling/noirc_abi", "tooling/noirc_abi_wasm", "tooling/acvm_cli", + "tooling/artifact_cli", "tooling/profiler", "tooling/inspector", # ACVM @@ -40,6 +41,7 @@ members = [ default-members = [ "tooling/nargo_cli", "tooling/acvm_cli", + "tooling/artifact_cli", "tooling/profiler", "tooling/inspector", ] @@ -91,6 +93,7 @@ noir_debugger = { path = "tooling/debugger" } noirc_abi = { path = "tooling/noirc_abi" } noirc_artifacts = { path = "tooling/noirc_artifacts" } noirc_artifacts_info = { path = "tooling/noirc_artifacts_info" } +noir_artifact_cli = { path = "tooling/artifact_cli" } # Arkworks ark-bn254 = { version = "^0.5.0", default-features = false, features = [ diff --git a/noir/noir-repo/EXTERNAL_NOIR_LIBRARIES.yml b/noir/noir-repo/EXTERNAL_NOIR_LIBRARIES.yml index 0ea374928031..fcb6885c1659 100644 --- a/noir/noir-repo/EXTERNAL_NOIR_LIBRARIES.yml +++ b/noir/noir-repo/EXTERNAL_NOIR_LIBRARIES.yml @@ -1,4 +1,4 @@ -define: &AZ_COMMIT 6c0b83d4b73408f87acfa080d52a81c411e47336 +define: &AZ_COMMIT 1350f93c3e9af8f601ca67ca3e67d0127c9767b6 libraries: noir_check_shuffle: repo: noir-lang/noir_check_shuffle diff --git a/noir/noir-repo/acvm-repo/acir/src/circuit/mod.rs b/noir/noir-repo/acvm-repo/acir/src/circuit/mod.rs index e651e6998a4a..091a3dcb0e52 100644 --- a/noir/noir-repo/acvm-repo/acir/src/circuit/mod.rs +++ b/noir/noir-repo/acvm-repo/acir/src/circuit/mod.rs @@ -272,6 +272,7 @@ impl Deserialize<'a>> Program { .map_err(|err| std::io::Error::new(std::io::ErrorKind::InvalidInput, err)) } + /// Deserialize bytecode. pub fn deserialize_program(serialized_circuit: &[u8]) -> std::io::Result { Program::read(serialized_circuit) } diff --git a/noir/noir-repo/compiler/noirc_errors/src/reporter.rs b/noir/noir-repo/compiler/noirc_errors/src/reporter.rs index 20a765fdaa5f..30bf49348b6a 100644 --- a/noir/noir-repo/compiler/noirc_errors/src/reporter.rs +++ b/noir/noir-repo/compiler/noirc_errors/src/reporter.rs @@ -211,6 +211,7 @@ pub fn report_all<'files>( } impl FileDiagnostic { + /// Print the report; return true if it was an error. pub fn report<'files>( &self, files: &'files impl Files<'files, FileId = fm::FileId>, diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/constrain.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/constrain.rs index a3881419a830..8b3b897088a2 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/constrain.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/constrain.rs @@ -123,6 +123,39 @@ pub(super) fn decompose_constrain( } } + (Value::NumericConstant { constant, .. }, Value::Instruction { instruction, .. }) + | (Value::Instruction { instruction, .. }, Value::NumericConstant { constant, .. }) => { + match dfg[*instruction] { + Instruction::Binary(Binary { lhs, rhs, operator: BinaryOp::Mul { .. } }) + if constant.is_zero() && lhs == rhs => + { + // Replace an assertion that a squared value is zero + // + // v1 = mul v0, v0 + // constrain v1 == u1 0 + // + // with a direct assertion that value being squared is equal to 0 + // + // v1 = mul v0, v0 + // constrain v0 == u1 0 + // + // This is due to the fact that for `v1` to be 0 then `v0` is 0. + // + // Note that this doesn't remove the value `v1` as it may be used in other instructions, but it + // will likely be removed through dead instruction elimination. + // + // This is safe for all numeric types as the underlying field has a prime modulus so squaring + // a non-zero value should never result in zero. + + let zero = FieldElement::zero(); + let zero = dfg.make_constant(zero, dfg.type_of_value(lhs).unwrap_numeric()); + decompose_constrain(lhs, zero, msg, dfg) + } + + _ => vec![Instruction::Constrain(lhs, rhs, msg.clone())], + } + } + ( Value::Instruction { instruction: instruction_lhs, .. }, Value::Instruction { instruction: instruction_rhs, .. }, @@ -144,3 +177,31 @@ pub(super) fn decompose_constrain( } } } + +#[cfg(test)] +mod tests { + use crate::ssa::{opt::assert_normalized_ssa_equals, ssa_gen::Ssa}; + + #[test] + fn simplifies_assertions_that_squared_values_are_equal_to_zero() { + let src = " + acir(inline) fn main f0 { + b0(v0: Field): + v1 = mul v0, v0 + constrain v1 == Field 0 + return + } + "; + let ssa = Ssa::from_str_simplifying(src).unwrap(); + + let expected = " + acir(inline) fn main f0 { + b0(v0: Field): + v1 = mul v0, v0 + constrain v0 == Field 0 + return + } + "; + assert_normalized_ssa_equals(ssa, expected); + } +} diff --git a/noir/noir-repo/compiler/noirc_frontend/src/ast/expression.rs b/noir/noir-repo/compiler/noirc_frontend/src/ast/expression.rs index 15b781051bc0..01705e0af881 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/ast/expression.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/ast/expression.rs @@ -245,6 +245,45 @@ impl Expression { pub fn new(kind: ExpressionKind, span: Span) -> Expression { Expression { kind, span } } + + /// Returns the innermost span that gives this expression its type. + pub fn type_span(&self) -> Span { + match &self.kind { + ExpressionKind::Block(block_expression) + | ExpressionKind::Comptime(block_expression, _) + | ExpressionKind::Unsafe(block_expression, _) => { + if let Some(statement) = block_expression.statements.last() { + statement.type_span() + } else { + self.span + } + } + ExpressionKind::Parenthesized(expression) => expression.type_span(), + ExpressionKind::Literal(..) + | ExpressionKind::Prefix(..) + | ExpressionKind::Index(..) + | ExpressionKind::Call(..) + | ExpressionKind::MethodCall(..) + | ExpressionKind::Constrain(..) + | ExpressionKind::Constructor(..) + | ExpressionKind::MemberAccess(..) + | ExpressionKind::Cast(..) + | ExpressionKind::Infix(..) + | ExpressionKind::If(..) + | ExpressionKind::Match(..) + | ExpressionKind::Variable(..) + | ExpressionKind::Tuple(..) + | ExpressionKind::Lambda(..) + | ExpressionKind::Quote(..) + | ExpressionKind::Unquote(..) + | ExpressionKind::AsTraitPath(..) + | ExpressionKind::TypePath(..) + | ExpressionKind::Resolved(..) + | ExpressionKind::Interned(..) + | ExpressionKind::InternedStatement(..) + | ExpressionKind::Error => self.span, + } + } } pub type BinaryOp = Spanned; diff --git a/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs b/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs index 145cff0a3415..372f20f87800 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/ast/statement.rs @@ -73,6 +73,24 @@ impl Statement { self.kind = self.kind.add_semicolon(semi, span, last_statement_in_block, emit_error); self } + + /// Returns the innermost span that gives this statement its type. + pub fn type_span(&self) -> Span { + match &self.kind { + StatementKind::Expression(expression) => expression.type_span(), + StatementKind::Comptime(statement) => statement.type_span(), + StatementKind::Let(..) + | StatementKind::Assign(..) + | StatementKind::For(..) + | StatementKind::Loop(..) + | StatementKind::While(..) + | StatementKind::Break + | StatementKind::Continue + | StatementKind::Semi(..) + | StatementKind::Interned(..) + | StatementKind::Error => self.span, + } + } } impl StatementKind { diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/enums.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/enums.rs index ffc2f0221209..02d9bfae4946 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/enums.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/enums.rs @@ -274,7 +274,7 @@ impl Elaborator<'_> { let columns = vec![Column::new(variable_to_match, pattern)]; let guard = None; - let body_span = branch.span; + let body_span = branch.type_span(); let (body, body_type) = self.elaborate_expression(branch); self.unify(&body_type, &result_type, || TypeCheckError::TypeMismatch { @@ -291,7 +291,7 @@ impl Elaborator<'_> { /// Convert an expression into a Pattern, defining any variables within. fn expression_to_pattern(&mut self, expression: Expression, expected_type: &Type) -> Pattern { - let expr_span = expression.span; + let expr_span = expression.type_span(); let unify_with_expected_type = |this: &mut Self, actual| { this.unify(actual, expected_type, || TypeCheckError::TypeMismatch { expected_typ: expected_type.to_string(), diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/expressions.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/expressions.rs index 18d5e3be82ed..3b25f85a25c2 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -971,8 +971,8 @@ impl<'context> Elaborator<'context> { if_expr: IfExpression, target_type: Option<&Type>, ) -> (HirExpression, Type) { - let expr_span = if_expr.condition.span; - let consequence_span = if_expr.consequence.span; + let expr_span = if_expr.condition.type_span(); + let consequence_span = if_expr.consequence.type_span(); let (condition, cond_type) = self.elaborate_expression(if_expr.condition); let (consequence, mut ret_type) = self.elaborate_expression_with_target_type(if_expr.consequence, target_type); @@ -984,9 +984,10 @@ impl<'context> Elaborator<'context> { }); let (alternative, else_type, error_span) = if let Some(alternative) = if_expr.alternative { + let alternative_span = alternative.type_span(); let (else_, else_type) = self.elaborate_expression_with_target_type(alternative, target_type); - (Some(else_), else_type, expr_span) + (Some(else_), else_type, alternative_span) } else { (None, Type::Unit, consequence_span) }; diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/statements.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/statements.rs index 8b60a660a161..3379db4aa668 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/statements.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/statements.rs @@ -219,7 +219,14 @@ impl<'context> Elaborator<'context> { self.interner.push_definition_type(identifier.id, start_range_type); - let (block, _block_type) = self.elaborate_expression(block); + let block_span = block.type_span(); + let (block, block_type) = self.elaborate_expression(block); + + self.unify(&block_type, &Type::Unit, || TypeCheckError::TypeMismatch { + expected_typ: Type::Unit.to_string(), + expr_typ: block_type.to_string(), + expr_span: block_span, + }); self.pop_scope(); self.current_loop = old_loop; @@ -244,7 +251,14 @@ impl<'context> Elaborator<'context> { self.current_loop = Some(Loop { is_for: false, has_break: false }); self.push_scope(); - let (block, _block_type) = self.elaborate_expression(block); + let block_span = block.type_span(); + let (block, block_type) = self.elaborate_expression(block); + + self.unify(&block_type, &Type::Unit, || TypeCheckError::TypeMismatch { + expected_typ: Type::Unit.to_string(), + expr_typ: block_type.to_string(), + expr_span: block_span, + }); self.pop_scope(); @@ -269,9 +283,8 @@ impl<'context> Elaborator<'context> { self.current_loop = Some(Loop { is_for: false, has_break: false }); self.push_scope(); - let condition_span = while_.condition.span; + let condition_span = while_.condition.type_span(); let (condition, cond_type) = self.elaborate_expression(while_.condition); - let (block, _block_type) = self.elaborate_expression(while_.body); self.unify(&cond_type, &Type::Bool, || TypeCheckError::TypeMismatch { expected_typ: Type::Bool.to_string(), @@ -279,6 +292,15 @@ impl<'context> Elaborator<'context> { expr_span: condition_span, }); + let block_span = while_.body.type_span(); + let (block, block_type) = self.elaborate_expression(while_.body); + + self.unify(&block_type, &Type::Unit, || TypeCheckError::TypeMismatch { + expected_typ: Type::Unit.to_string(), + expr_typ: block_type.to_string(), + expr_span: block_span, + }); + self.pop_scope(); std::mem::replace(&mut self.current_loop, old_loop).expect("Expected a loop"); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index 1679c069bd56..279d176ad33b 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -715,6 +715,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { let location = self.elaborator.interner.expr_location(&id); if let Type::FieldElement = &typ { + let value = if is_negative { -value } else { value }; Ok(Value::Field(value)) } else if let Type::Integer(sign, bit_size) = &typ { match (sign, bit_size) { diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs index 13c855c6fe7d..5af0cf41a627 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs @@ -419,7 +419,7 @@ impl<'a> From<&'a ResolverError> for Diagnostic { "Usage of the `#[foreign]` or `#[builtin]` function attributes are not allowed outside of the Noir standard library".into(), ident.span(), ), - ResolverError::OracleMarkedAsConstrained { ident } => Diagnostic::simple_warning( + ResolverError::OracleMarkedAsConstrained { ident } => Diagnostic::simple_error( error.to_string(), "Oracle functions must have the `unconstrained` keyword applied".into(), ident.span(), diff --git a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/global.rs b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/global.rs index 3a4f6d922a4d..2cd8343fe31d 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/global.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/global.rs @@ -70,9 +70,12 @@ fn ident_to_pattern(ident: Ident, mutable: bool) -> Pattern { #[cfg(test)] mod tests { + use acvm::FieldElement; + use crate::{ ast::{ - IntegerBitSize, ItemVisibility, LetStatement, Pattern, Signedness, UnresolvedTypeData, + ExpressionKind, IntegerBitSize, ItemVisibility, LetStatement, Literal, Pattern, + Signedness, UnresolvedTypeData, }, parser::{ parser::{ @@ -171,4 +174,27 @@ mod tests { let error = get_single_error(&errors, span); assert_eq!(error.to_string(), "Expected a ';' but found end of input"); } + + #[test] + fn parse_negative_field_global() { + let src = " + global foo: Field = -17; + "; + let (let_statement, _visibility) = parse_global_no_errors(src); + let Pattern::Identifier(name) = &let_statement.pattern else { + panic!("Expected identifier pattern"); + }; + assert_eq!("foo", name.to_string()); + assert_eq!(let_statement.pattern.span().start(), 16); + assert_eq!(let_statement.pattern.span().end(), 19); + + let ExpressionKind::Literal(Literal::Integer(abs_value, is_negative)) = + let_statement.expression.kind + else { + panic!("Expected integer literal expression, got {:?}", let_statement.expression.kind); + }; + + assert!(is_negative); + assert_eq!(abs_value, FieldElement::from(17u128)); + } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests.rs index 1b3a19a5cfc3..6ff3b204f2e0 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests.rs @@ -4352,8 +4352,8 @@ fn call_function_alias_type() { fn errors_on_if_without_else_type_mismatch() { let src = r#" fn main() { - if true { - 1 + if true { + 1 } } "#; @@ -4371,3 +4371,56 @@ fn does_not_stack_overflow_on_many_comments_in_a_row() { let src = "//\n".repeat(10_000); assert_no_errors(&src); } + +#[test] +fn errors_if_for_body_type_is_not_unit() { + let src = r#" + fn main() { + for _ in 0..1 { + 1 + } + } + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + let CompilationError::TypeError(TypeCheckError::TypeMismatch { .. }) = &errors[0].0 else { + panic!("Expected a TypeMismatch error"); + }; +} + +#[test] +fn errors_if_loop_body_type_is_not_unit() { + let src = r#" + unconstrained fn main() { + loop { + if false { break; } + + 1 + } + } + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + let CompilationError::TypeError(TypeCheckError::TypeMismatch { .. }) = &errors[0].0 else { + panic!("Expected a TypeMismatch error"); + }; +} + +#[test] +fn errors_if_while_body_type_is_not_unit() { + let src = r#" + unconstrained fn main() { + while 1 == 1 { + 1 + } + } + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + let CompilationError::TypeError(TypeCheckError::TypeMismatch { .. }) = &errors[0].0 else { + panic!("Expected a TypeMismatch error"); + }; +} diff --git a/noir/noir-repo/docs/docs/tooling/cspell.json b/noir/noir-repo/docs/docs/tooling/cspell.json new file mode 100644 index 000000000000..e8126ba0874b --- /dev/null +++ b/noir/noir-repo/docs/docs/tooling/cspell.json @@ -0,0 +1,5 @@ +{ + "words": [ + "lookback" + ] +} diff --git a/noir/noir-repo/docs/docs/tooling/security.md b/noir/noir-repo/docs/docs/tooling/security.md new file mode 100644 index 000000000000..e14481efc317 --- /dev/null +++ b/noir/noir-repo/docs/docs/tooling/security.md @@ -0,0 +1,61 @@ +--- +title: Security checks +description: Security checks currently provided by the compiler +keywords: [Nargo, Security, Brillig, Unconstrained] +sidebar_position: 2 +--- + +# Security checks + +Two compilation security passes exist currently to ensure soundness of compiled code. Problems they catch are reported as "bugs" (as opposed to errors) in the compiler output. For example: + +``` +**bug**: Brillig function call isn't properly covered by a manual constraint +``` + +### Independent subgraph detection + +This pass examines the instruction flow graph to see if the final function would involve values that don't come from any provided inputs and don't result in the outputs. That would mean there are no constraints ensuring the required continuity. + +This check is enabled by default and can be disabled by passing the `--skip-underconstrained-check` option to `nargo`. + +### Brillig manual constraint coverage + +The results of a Brillig function call must be constrained to ensure security, adhering to these rules: every resulting value (including every array element of a resulting array) has to be involved in a later constraint (i.e. assert, range check) against either one of the arguments of the call, or a constant. In this context, involvement means that a descendant value (e.g. a result of a chain of operations over the value) of a result has to be checked against a descendant value of an argument. For example: + +```rust +unconstrained fn factor(v0: Field) -> [Field; 2] { + ... +} + +fn main f0 (foo: Field) -> [Field; 2] { + let factored = unsafe { factor(foo) }; + assert(factored[0] * factored[1] == foo); + return factored +} +``` + +Here, the results of `factor` are two elements of the returned array. The value `factored[0] * factored[1]` is a descendant of both of them, so both are involved in a constraint against the argument value in the `assert`. Hence, the call to an unconstrained function is properly covered. + +This pass checks if the constraint coverage of Brillig calls is sufficient in these terms. + +The check is at the moment disabled by default due to performance concerns and can be enabled by passing the `--enable-brillig-constraints-check` option to `nargo`. + +#### Lookback option + +Certain false positives of this check can be avoided by providing the `--enable-brillig-constraints-check-lookback` option to `nargo`, which can be slower at compile-time but additionally ensures that descendants of call argument values coming from operations *preceding* the call itself would be followed. For example, consider this case: + +```rust +unconstrained fn unconstrained_add(v0: Field, v1: Field) -> Field { + v0 + v1 +} + +fn main f0 (v0: Field, v1: Field) { + let foo = v0 + v1; + let bar = unsafe { unconstrained_add(v0, v1) }; + assert(foo == bar); + return bar +} +``` + +Normally, the addition operation over `v0` and `v1` happening before the call itself would prevent the call from being (correctly) considered properly constrained. With this option enabled, the false positive goes away at the cost of the check becoming somewhat less performant on large unrolled loops. diff --git a/noir/noir-repo/test_programs/compile_success_empty/regression_7433/Nargo.toml b/noir/noir-repo/test_programs/compile_success_empty/regression_7433/Nargo.toml new file mode 100644 index 000000000000..207dd9f39de1 --- /dev/null +++ b/noir/noir-repo/test_programs/compile_success_empty/regression_7433/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "regression_7433" +type = "bin" +authors = [""] + +[dependencies] diff --git a/noir/noir-repo/test_programs/compile_success_empty/regression_7433/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/regression_7433/src/main.nr new file mode 100644 index 000000000000..03788a84227b --- /dev/null +++ b/noir/noir-repo/test_programs/compile_success_empty/regression_7433/src/main.nr @@ -0,0 +1,7 @@ +// These two globals should cancel out. +global positive_global: Field = 17; +global negative_global: Field = -17; // Previously the negative sign here would be dropped so this became 17. + +fn main() { + assert_eq(positive_global + negative_global, 0); +} diff --git a/noir/noir-repo/test_programs/execution_success/multi_scalar_mul/Prover.toml b/noir/noir-repo/test_programs/execution_success/multi_scalar_mul/Prover.toml index 88c54b6be455..3c8375e22c53 100644 --- a/noir/noir-repo/test_programs/execution_success/multi_scalar_mul/Prover.toml +++ b/noir/noir-repo/test_programs/execution_success/multi_scalar_mul/Prover.toml @@ -1,4 +1,10 @@ -scalars = ["0", "0", "0", "0", "0"] +scalars = [ + "0x20644e72e131a029b85045b68181585d2833e84879b9709143e1f593f0000001", + "0x27241797651f4adc1f04da8e1aeb", + "0x00", + "0x30644e72e131a029b85045b68181585d2833e84879b9709143e1f593f0000000", + "0x1b", +] [[points]] is_infinite = "0" diff --git a/noir/noir-repo/test_programs/execution_success/multi_scalar_mul/src/main.nr b/noir/noir-repo/test_programs/execution_success/multi_scalar_mul/src/main.nr index f19391a6a4cb..b191fb7aa1e7 100644 --- a/noir/noir-repo/test_programs/execution_success/multi_scalar_mul/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/multi_scalar_mul/src/main.nr @@ -8,12 +8,23 @@ unconstrained fn main( points: [EmbeddedCurvePoint; 5], scalars: [Field; 5], ) -> pub EmbeddedCurvePoint { + double_then_add_msm(points, scalars) +} + +unconstrained fn double_then_add_msm( + points: [EmbeddedCurvePoint; N], + scalars: [Field; N], +) -> EmbeddedCurvePoint { // EmbeddedCurveScalar are two 128-bit numbers let mut acc = EmbeddedCurvePoint::point_at_infinity(); - for i in 0..1 { + for i in 0..N { // These should probably be EmbeddedCurveScalars // let full_scalar: Field = scalars[i].hi * 2.pow_32(128) + scalars[i].lo; let full_scalar = scalars[i]; + // If the scalar is zero we won't add anything to acc + if full_scalar == 0 { + continue; + } let full_scalar_bits: [u1; 254] = full_scalar.to_be_bits(); let mut index_of_msb = 0; // Iterates in BE @@ -24,22 +35,19 @@ unconstrained fn main( } } - let mut temp = points[i]; - let mut res = EmbeddedCurvePoint::point_at_infinity(); - // When iterative backwards we want to go to bits.len() - 2 - for j in 0..(254 - index_of_msb) { - let k = 253 - j; - + let temp = points[i]; + let mut res = points[i]; + // traversing from second MSB to LSB + for j in (index_of_msb + 1)..(254) { + // Double + res = embedded_curve_add_unsafe(res, res); // Add - if full_scalar_bits[k] == 1 { + if full_scalar_bits[j] == 1 { res = embedded_curve_add_unsafe(res, temp); } - // Double - temp = embedded_curve_add_unsafe(temp, temp); } acc = embedded_curve_add_unsafe(acc, res); } acc } - diff --git a/noir/noir-repo/tooling/acvm_cli/Cargo.toml b/noir/noir-repo/tooling/acvm_cli/Cargo.toml index 06dd5e676bd1..d3906175df3b 100644 --- a/noir/noir-repo/tooling/acvm_cli/Cargo.toml +++ b/noir/noir-repo/tooling/acvm_cli/Cargo.toml @@ -21,15 +21,16 @@ name = "acvm" path = "src/main.rs" [dependencies] -thiserror.workspace = true -toml.workspace = true color-eyre.workspace = true clap.workspace = true -acvm.workspace = true -nargo.workspace = true const_format.workspace = true -bn254_blackbox_solver.workspace = true +toml.workspace = true + acir.workspace = true +acvm.workspace = true +bn254_blackbox_solver.workspace = true +nargo.workspace = true +noir_artifact_cli.workspace = true # Logs tracing-subscriber.workspace = true diff --git a/noir/noir-repo/tooling/acvm_cli/src/cli/execute_cmd.rs b/noir/noir-repo/tooling/acvm_cli/src/cli/execute_cmd.rs index e5d48073ca8b..6f6fba7cd1e7 100644 --- a/noir/noir-repo/tooling/acvm_cli/src/cli/execute_cmd.rs +++ b/noir/noir-repo/tooling/acvm_cli/src/cli/execute_cmd.rs @@ -1,4 +1,5 @@ use std::io::{self, Write}; +use std::path::PathBuf; use acir::circuit::Program; use acir::native_types::{WitnessMap, WitnessStack}; @@ -7,11 +8,12 @@ use bn254_blackbox_solver::Bn254BlackBoxSolver; use clap::Args; use nargo::PrintOutput; -use crate::cli::fs::inputs::{read_bytecode_from_file, read_inputs_from_file}; -use crate::errors::CliError; use nargo::{foreign_calls::DefaultForeignCallBuilder, ops::execute_program}; +use noir_artifact_cli::errors::CliError; +use noir_artifact_cli::fs::artifact::read_bytecode_from_file; +use noir_artifact_cli::fs::witness::save_witness_to_dir; -use super::fs::witness::{create_output_witness_string, save_witness_to_dir}; +use crate::fs::witness::{create_output_witness_string, read_witness_from_file}; /// Executes a circuit to calculate its return value #[derive(Debug, Clone, Args)] @@ -30,7 +32,7 @@ pub(crate) struct ExecuteCommand { /// The working directory #[clap(long, short)] - working_directory: String, + working_directory: PathBuf, /// Set to print output witness to stdout #[clap(long, short, action)] @@ -45,9 +47,9 @@ pub(crate) struct ExecuteCommand { fn run_command(args: ExecuteCommand) -> Result { let bytecode = read_bytecode_from_file(&args.working_directory, &args.bytecode)?; - let circuit_inputs = read_inputs_from_file(&args.working_directory, &args.input_witness)?; + let input_witness = read_witness_from_file(&args.working_directory.join(&args.input_witness))?; let output_witness = - execute_program_from_witness(circuit_inputs, &bytecode, args.pedantic_solving)?; + execute_program_from_witness(input_witness, &bytecode, args.pedantic_solving)?; assert_eq!(output_witness.length(), 1, "ACVM CLI only supports a witness stack of size 1"); let output_witness_string = create_output_witness_string( &output_witness.peek().expect("Should have a witness stack item").witness, @@ -76,8 +78,8 @@ pub(crate) fn execute_program_from_witness( bytecode: &[u8], pedantic_solving: bool, ) -> Result, CliError> { - let program: Program = Program::deserialize_program(bytecode) - .map_err(|_| CliError::CircuitDeserializationError())?; + let program: Program = + Program::deserialize_program(bytecode).map_err(CliError::CircuitDeserializationError)?; execute_program( &program, inputs_map, diff --git a/noir/noir-repo/tooling/acvm_cli/src/cli/fs/inputs.rs b/noir/noir-repo/tooling/acvm_cli/src/cli/fs/inputs.rs deleted file mode 100644 index a0b6e3a95451..000000000000 --- a/noir/noir-repo/tooling/acvm_cli/src/cli/fs/inputs.rs +++ /dev/null @@ -1,54 +0,0 @@ -use acir::{ - native_types::{Witness, WitnessMap}, - AcirField, FieldElement, -}; -use toml::Table; - -use crate::errors::{CliError, FilesystemError}; -use std::{fs::read, path::Path}; - -/// Returns the circuit's parameters parsed from a toml file at the given location -pub(crate) fn read_inputs_from_file>( - working_directory: P, - file_name: &String, -) -> Result, CliError> { - let file_path = working_directory.as_ref().join(file_name); - if !file_path.exists() { - return Err(CliError::FilesystemError(FilesystemError::MissingTomlFile( - file_name.to_owned(), - file_path, - ))); - } - - let input_string = std::fs::read_to_string(file_path) - .map_err(|_| FilesystemError::InvalidTomlFile(file_name.clone()))?; - let input_map = input_string - .parse::() - .map_err(|_| FilesystemError::InvalidTomlFile(file_name.clone()))?; - let mut witnesses: WitnessMap = WitnessMap::new(); - for (key, value) in input_map.into_iter() { - let index = - Witness(key.trim().parse().map_err(|_| CliError::WitnessIndexError(key.clone()))?); - if !value.is_str() { - return Err(CliError::WitnessValueError(key.clone())); - } - let field = FieldElement::from_hex(value.as_str().unwrap()).unwrap(); - witnesses.insert(index, field); - } - - Ok(witnesses) -} - -/// Returns the circuit's bytecode read from the file at the given location -pub(crate) fn read_bytecode_from_file>( - working_directory: P, - file_name: &String, -) -> Result, FilesystemError> { - let file_path = working_directory.as_ref().join(file_name); - if !file_path.exists() { - return Err(FilesystemError::MissingBytecodeFile(file_name.to_owned(), file_path)); - } - let bytecode: Vec = - read(file_path).map_err(|_| FilesystemError::InvalidBytecodeFile(file_name.clone()))?; - Ok(bytecode) -} diff --git a/noir/noir-repo/tooling/acvm_cli/src/cli/fs/mod.rs b/noir/noir-repo/tooling/acvm_cli/src/cli/fs/mod.rs deleted file mode 100644 index f23ba06fd8bc..000000000000 --- a/noir/noir-repo/tooling/acvm_cli/src/cli/fs/mod.rs +++ /dev/null @@ -1,2 +0,0 @@ -pub(super) mod inputs; -pub(super) mod witness; diff --git a/noir/noir-repo/tooling/acvm_cli/src/cli/fs/witness.rs b/noir/noir-repo/tooling/acvm_cli/src/cli/fs/witness.rs deleted file mode 100644 index 6ecba9792c3c..000000000000 --- a/noir/noir-repo/tooling/acvm_cli/src/cli/fs/witness.rs +++ /dev/null @@ -1,63 +0,0 @@ -use std::{ - collections::BTreeMap, - fs::File, - io::Write, - path::{Path, PathBuf}, -}; - -use acir::FieldElement; -use acvm::acir::{ - native_types::{WitnessMap, WitnessStack}, - AcirField, -}; - -use crate::errors::{CliError, FilesystemError}; - -fn create_named_dir(named_dir: &Path, name: &str) -> PathBuf { - std::fs::create_dir_all(named_dir) - .unwrap_or_else(|_| panic!("could not create the `{name}` directory")); - - PathBuf::from(named_dir) -} - -fn write_to_file(bytes: &[u8], path: &Path) -> String { - let display = path.display(); - - let mut file = match File::create(path) { - Err(why) => panic!("couldn't create {display}: {why}"), - Ok(file) => file, - }; - - match file.write_all(bytes) { - Err(why) => panic!("couldn't write to {display}: {why}"), - Ok(_) => display.to_string(), - } -} - -/// Creates a toml representation of the provided witness map -pub(crate) fn create_output_witness_string( - witnesses: &WitnessMap, -) -> Result { - let mut witness_map: BTreeMap = BTreeMap::new(); - for (key, value) in witnesses.clone().into_iter() { - witness_map.insert(key.0.to_string(), format!("0x{}", value.to_hex())); - } - - toml::to_string(&witness_map).map_err(|_| CliError::OutputWitnessSerializationFailed()) -} - -pub(crate) fn save_witness_to_dir>( - witnesses: WitnessStack, - witness_name: &str, - witness_dir: P, -) -> Result { - create_named_dir(witness_dir.as_ref(), "witness"); - let witness_path = witness_dir.as_ref().join(witness_name).with_extension("gz"); - - let buf: Vec = witnesses - .try_into() - .map_err(|_op| FilesystemError::OutputWitnessCreationFailed(witness_name.to_string()))?; - write_to_file(buf.as_slice(), &witness_path); - - Ok(witness_path) -} diff --git a/noir/noir-repo/tooling/acvm_cli/src/cli/mod.rs b/noir/noir-repo/tooling/acvm_cli/src/cli/mod.rs index f31e123d0cd2..1459ae430142 100644 --- a/noir/noir-repo/tooling/acvm_cli/src/cli/mod.rs +++ b/noir/noir-repo/tooling/acvm_cli/src/cli/mod.rs @@ -3,7 +3,6 @@ use color_eyre::eyre; use const_format::formatcp; mod execute_cmd; -mod fs; const ACVM_VERSION: &str = env!("CARGO_PKG_VERSION"); diff --git a/noir/noir-repo/tooling/acvm_cli/src/errors.rs b/noir/noir-repo/tooling/acvm_cli/src/errors.rs deleted file mode 100644 index 886c1bf80f29..000000000000 --- a/noir/noir-repo/tooling/acvm_cli/src/errors.rs +++ /dev/null @@ -1,50 +0,0 @@ -use acir::FieldElement; -use nargo::NargoError; -use std::path::PathBuf; -use thiserror::Error; - -#[derive(Debug, Error)] -pub(crate) enum FilesystemError { - #[error( - " Error: cannot find {0} in expected location {1:?}.\n Please generate this file at the expected location." - )] - MissingTomlFile(String, PathBuf), - #[error(" Error: failed to parse toml file {0}.")] - InvalidTomlFile(String), - #[error( - " Error: cannot find {0} in expected location {1:?}.\n Please generate this file at the expected location." - )] - MissingBytecodeFile(String, PathBuf), - - #[error(" Error: failed to read bytecode file {0}.")] - InvalidBytecodeFile(String), - - #[error(" Error: failed to create output witness file {0}.")] - OutputWitnessCreationFailed(String), -} - -#[derive(Debug, Error)] -pub(crate) enum CliError { - /// Filesystem errors - #[error(transparent)] - FilesystemError(#[from] FilesystemError), - - /// Error related to circuit deserialization - #[error("Error: failed to deserialize circuit in ACVM CLI")] - CircuitDeserializationError(), - - /// Error related to circuit execution - #[error(transparent)] - CircuitExecutionError(#[from] NargoError), - - /// Input Witness Value Error - #[error("Error: failed to parse witness value {0}")] - WitnessValueError(String), - - /// Input Witness Index Error - #[error("Error: failed to parse witness index {0}")] - WitnessIndexError(String), - - #[error(" Error: failed to serialize output witness.")] - OutputWitnessSerializationFailed(), -} diff --git a/noir/noir-repo/tooling/acvm_cli/src/fs/mod.rs b/noir/noir-repo/tooling/acvm_cli/src/fs/mod.rs new file mode 100644 index 000000000000..b4bfa6e48b60 --- /dev/null +++ b/noir/noir-repo/tooling/acvm_cli/src/fs/mod.rs @@ -0,0 +1 @@ +pub(crate) mod witness; diff --git a/noir/noir-repo/tooling/acvm_cli/src/fs/witness.rs b/noir/noir-repo/tooling/acvm_cli/src/fs/witness.rs new file mode 100644 index 000000000000..5c2d2a9ad058 --- /dev/null +++ b/noir/noir-repo/tooling/acvm_cli/src/fs/witness.rs @@ -0,0 +1,60 @@ +//! These witness functions are only used by the ACVM CLI and we'll most likely deprecate them. +use std::{collections::BTreeMap, path::Path}; + +use acir::{native_types::Witness, FieldElement}; +use acvm::acir::{native_types::WitnessMap, AcirField}; + +use noir_artifact_cli::errors::{CliError, FilesystemError}; + +/// Returns the circuit's parameters parsed from a TOML file at the given location. +/// +/// The expected format is the witness map, not ABI inputs, for example: +/// ```toml +/// "0" = '0x0000000000000000000000000000000000000000000000000000000000100000' +/// "1" = '0x0000000000000000000000000000000000000000000000000000000000000020' +/// "2" = '0x00000000000000000000000000000000000000000000000000000000000328b1' +/// "3" = '0x0000000000000000000000000000000000000000000000000000000000000001' +/// "4" = '0x0000000000000000000000000000000000000000000000000000000000000010' +/// "5" = '0x0000000000000000000000000000000000000000000000000000000000000011' +/// ``` +pub(crate) fn read_witness_from_file( + file_path: &Path, +) -> Result, CliError> { + if !file_path.exists() { + return Err(CliError::FilesystemError(FilesystemError::MissingInputFile( + file_path.to_path_buf(), + ))); + } + + let input_string = std::fs::read_to_string(file_path) + .map_err(|e| FilesystemError::InvalidInputFile(file_path.to_path_buf(), e.to_string()))?; + + let input_map = input_string + .parse::() + .map_err(|e| FilesystemError::InvalidInputFile(file_path.to_path_buf(), e.to_string()))?; + + let mut witnesses: WitnessMap = WitnessMap::new(); + + for (key, value) in input_map.into_iter() { + let index = + Witness(key.trim().parse().map_err(|_| CliError::WitnessIndexError(key.clone()))?); + if !value.is_str() { + return Err(CliError::WitnessValueError(key.clone())); + } + let field = FieldElement::from_hex(value.as_str().unwrap()).unwrap(); + witnesses.insert(index, field); + } + + Ok(witnesses) +} + +/// Creates a TOML representation of the provided witness map +pub(crate) fn create_output_witness_string( + witnesses: &WitnessMap, +) -> Result { + let mut witness_map: BTreeMap = BTreeMap::new(); + for (key, value) in witnesses.clone().into_iter() { + witness_map.insert(key.0.to_string(), format!("0x{}", value.to_hex())); + } + toml::to_string(&witness_map).map_err(CliError::OutputWitnessSerializationFailed) +} diff --git a/noir/noir-repo/tooling/acvm_cli/src/main.rs b/noir/noir-repo/tooling/acvm_cli/src/main.rs index 33cadc73a7cf..a30360a947ce 100644 --- a/noir/noir-repo/tooling/acvm_cli/src/main.rs +++ b/noir/noir-repo/tooling/acvm_cli/src/main.rs @@ -4,13 +4,14 @@ #![cfg_attr(not(test), warn(unused_crate_dependencies, unused_extern_crates))] mod cli; -mod errors; use std::env; use tracing_appender::rolling; use tracing_subscriber::{fmt::format::FmtSpan, EnvFilter}; +mod fs; + fn main() { // Setup tracing if let Ok(log_dir) = env::var("ACVM_LOG_DIR") { diff --git a/noir/noir-repo/tooling/artifact_cli/Cargo.toml b/noir/noir-repo/tooling/artifact_cli/Cargo.toml new file mode 100644 index 000000000000..2f7ae68a4776 --- /dev/null +++ b/noir/noir-repo/tooling/artifact_cli/Cargo.toml @@ -0,0 +1,43 @@ +[package] +name = "noir_artifact_cli" +description = "Commands working on noir build artifacts" +version.workspace = true +authors.workspace = true +edition.workspace = true +license.workspace = true +rust-version.workspace = true +repository.workspace = true + +[lints] +workspace = true + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[lib] +path = "src/lib.rs" + +[[bin]] +name = "noir-execute" +path = "src/bin/execute.rs" + +[dependencies] +clap.workspace = true +color-eyre.workspace = true +const_format.workspace = true +serde_json.workspace = true +thiserror.workspace = true +toml.workspace = true +tracing-subscriber.workspace = true + +# Noir repo dependencies +acir.workspace = true +acvm.workspace = true +bn254_blackbox_solver.workspace = true +fm.workspace = true +nargo = { workspace = true, features = ["rpc"] } +noirc_abi.workspace = true +noirc_artifacts_info.workspace = true +noirc_artifacts.workspace = true +noirc_driver.workspace = true +noirc_errors.workspace = true +serde.workspace = true diff --git a/noir/noir-repo/tooling/artifact_cli/src/bin/execute.rs b/noir/noir-repo/tooling/artifact_cli/src/bin/execute.rs new file mode 100644 index 000000000000..b66177a3d572 --- /dev/null +++ b/noir/noir-repo/tooling/artifact_cli/src/bin/execute.rs @@ -0,0 +1,46 @@ +#![forbid(unsafe_code)] +#![warn(unreachable_pub)] + +use clap::{command, Parser, Subcommand}; +use color_eyre::eyre; +use const_format::formatcp; +use tracing_subscriber::{fmt::format::FmtSpan, EnvFilter}; + +use noir_artifact_cli::commands::execute_cmd; + +const PKG_VERSION: &str = env!("CARGO_PKG_VERSION"); +static VERSION_STRING: &str = formatcp!("version = {}\n", PKG_VERSION,); + +#[derive(Parser, Debug)] +#[command(name="noir-execute", author, version=VERSION_STRING, about, long_about = None)] +struct ExecutorCli { + #[command(flatten)] + command: execute_cmd::ExecuteCommand, +} + +#[non_exhaustive] +#[derive(Subcommand, Clone, Debug)] +enum ArtifactCommand { + Execute(execute_cmd::ExecuteCommand), +} + +pub fn start_cli() -> eyre::Result<()> { + let ExecutorCli { command } = ExecutorCli::parse(); + + execute_cmd::run(command)?; + + Ok(()) +} + +fn main() { + tracing_subscriber::fmt() + .with_span_events(FmtSpan::ACTIVE) + .with_ansi(true) + .with_env_filter(EnvFilter::from_env("NOIR_LOG")) + .init(); + + if let Err(e) = start_cli() { + eprintln!("{e:?}"); + std::process::exit(1); + } +} diff --git a/noir/noir-repo/tooling/artifact_cli/src/commands/execute_cmd.rs b/noir/noir-repo/tooling/artifact_cli/src/commands/execute_cmd.rs new file mode 100644 index 000000000000..9a117329c151 --- /dev/null +++ b/noir/noir-repo/tooling/artifact_cli/src/commands/execute_cmd.rs @@ -0,0 +1,209 @@ +use std::{collections::BTreeMap, path::PathBuf}; + +use acir::{circuit::Program, native_types::WitnessStack, FieldElement}; +use bn254_blackbox_solver::Bn254BlackBoxSolver; +use clap::Args; +use color_eyre::eyre::{self, bail}; + +use crate::{ + errors::CliError, + fs::{inputs::read_inputs_from_file, witness::save_witness_to_dir}, + Artifact, +}; +use nargo::{foreign_calls::DefaultForeignCallBuilder, NargoError, PrintOutput}; +use noirc_abi::{input_parser::InputValue, Abi}; +use noirc_artifacts::debug::DebugArtifact; + +use super::parse_and_normalize_path; + +/// Execute a binary program or a circuit artifact. +#[derive(Debug, Clone, Args)] +pub struct ExecuteCommand { + /// Path to the JSON build artifact (either a program or a contract). + #[clap(long, short, value_parser = parse_and_normalize_path)] + artifact_path: PathBuf, + + /// Path to the Prover.toml file which contains the inputs and the + /// optional return value in ABI format. + #[clap(long, short, value_parser = parse_and_normalize_path)] + prover_file: PathBuf, + + /// Path to the directory where the output witness should be saved. + /// If empty then the results are discarded. + #[clap(long, short, value_parser = parse_and_normalize_path)] + output_dir: Option, + + /// Write the execution witness to named file + /// + /// Defaults to the name of the circuit being executed. + #[clap(long, short)] + witness_name: Option, + + /// Name of the function to execute, if the artifact is a contract. + #[clap(long)] + contract_fn: Option, + + /// JSON RPC url to solve oracle calls. + #[clap(long)] + oracle_resolver: Option, + + /// Use pedantic ACVM solving, i.e. double-check some black-box function assumptions when solving. + #[clap(long, default_value_t = false)] + pedantic_solving: bool, +} + +pub fn run(args: ExecuteCommand) -> eyre::Result<()> { + let artifact = Artifact::read_from_file(&args.artifact_path)?; + + let circuit = match artifact { + Artifact::Program(program) => Circuit { + name: None, + abi: program.abi, + bytecode: program.bytecode, + debug_symbols: program.debug_symbols, + file_map: program.file_map, + }, + Artifact::Contract(contract) => { + let names = + contract.functions.iter().map(|f| f.name.clone()).collect::>().join(","); + + let Some(ref name) = args.contract_fn else { + bail!("--contract-fn missing; options: [{names}]"); + }; + let Some(function) = contract.functions.into_iter().find(|f| f.name == *name) else { + bail!("unknown --contract-fn '{name}'; options: [{names}]"); + }; + + Circuit { + name: Some(name.clone()), + abi: function.abi, + bytecode: function.bytecode, + debug_symbols: function.debug_symbols, + file_map: contract.file_map, + } + } + }; + + match execute(&circuit, &args) { + Ok(solved) => { + save_witness(circuit, args, solved)?; + } + Err(CliError::CircuitExecutionError(err)) => { + show_diagnostic(circuit, err); + } + Err(e) => { + bail!("failed to execute the circuit: {e}"); + } + } + Ok(()) +} + +/// Parameters necessary to execute a circuit, display execution failures, etc. +struct Circuit { + name: Option, + abi: Abi, + bytecode: Program, + debug_symbols: noirc_errors::debug_info::ProgramDebugInfo, + file_map: BTreeMap, +} + +struct SolvedWitnesses { + expected_return: Option, + actual_return: Option, + witness_stack: WitnessStack, +} + +/// Execute a circuit and return the output witnesses. +fn execute(circuit: &Circuit, args: &ExecuteCommand) -> Result { + let (input_map, expected_return) = read_inputs_from_file(&args.prover_file, &circuit.abi)?; + + let initial_witness = circuit.abi.encode(&input_map, None)?; + + // TODO: Build a custom foreign call executor that reads from the Oracle transcript, + // and use it as a base for the default executor; see `DefaultForeignCallBuilder::build_with_base` + let mut foreign_call_executor = DefaultForeignCallBuilder { + output: PrintOutput::Stdout, + enable_mocks: false, + resolver_url: args.oracle_resolver.clone(), + root_path: None, + package_name: None, + } + .build(); + + let witness_stack = nargo::ops::execute_program( + &circuit.bytecode, + initial_witness, + &Bn254BlackBoxSolver(args.pedantic_solving), + &mut foreign_call_executor, + )?; + + let main_witness = + &witness_stack.peek().expect("Should have at least one witness on the stack").witness; + + let (_, actual_return) = circuit.abi.decode(main_witness)?; + + Ok(SolvedWitnesses { expected_return, actual_return, witness_stack }) +} + +/// Print an error stack trace, if possible. +fn show_diagnostic(circuit: Circuit, err: NargoError) { + if let Some(diagnostic) = nargo::errors::try_to_diagnose_runtime_error( + &err, + &circuit.abi, + &circuit.debug_symbols.debug_infos, + ) { + let debug_artifact = DebugArtifact { + debug_symbols: circuit.debug_symbols.debug_infos, + file_map: circuit.file_map, + }; + diagnostic.report(&debug_artifact, false); + } +} + +/// Print information about the witness and compare to expectations, +/// returning errors if something isn't right. +fn save_witness( + circuit: Circuit, + args: ExecuteCommand, + solved: SolvedWitnesses, +) -> eyre::Result<()> { + let artifact = args.artifact_path.file_stem().and_then(|s| s.to_str()).unwrap_or_default(); + let name = circuit + .name + .as_ref() + .map(|name| format!("{artifact}.{name}")) + .unwrap_or_else(|| artifact.to_string()); + + println!("[{}] Circuit witness successfully solved", name); + + if let Some(ref witness_dir) = args.output_dir { + let witness_path = save_witness_to_dir( + solved.witness_stack, + &args.witness_name.unwrap_or_else(|| name.clone()), + witness_dir, + )?; + println!("[{}] Witness saved to {}", name, witness_path.display()); + } + + // Check that the circuit returned a non-empty result if the ABI expects a return value. + if let Some(ref expected) = circuit.abi.return_type { + if solved.actual_return.is_none() { + bail!("Missing return witness; expected a value of type {expected:?}"); + } + } + + // Check that if the prover file contained a `return` entry then that's what we got. + if let Some(expected) = solved.expected_return { + match solved.actual_return { + None => { + bail!("Missing return witness;\nexpected:\n{expected:?}"); + } + Some(actual) if actual != expected => { + bail!("Unexpected return witness;\nexpected:\n{expected:?}\ngot:\n{actual:?}"); + } + _ => {} + } + } + + Ok(()) +} diff --git a/noir/noir-repo/tooling/artifact_cli/src/commands/mod.rs b/noir/noir-repo/tooling/artifact_cli/src/commands/mod.rs new file mode 100644 index 000000000000..78f3b19292f2 --- /dev/null +++ b/noir/noir-repo/tooling/artifact_cli/src/commands/mod.rs @@ -0,0 +1,17 @@ +use std::path::PathBuf; + +use color_eyre::eyre; +use eyre::eyre; + +pub mod execute_cmd; + +/// Parses a path and turns it into an absolute one by joining to the current directory, +/// then normalizes it. +fn parse_and_normalize_path(path: &str) -> eyre::Result { + use fm::NormalizePath; + let mut path: PathBuf = path.parse().map_err(|e| eyre!("failed to parse path: {e}"))?; + if !path.is_absolute() { + path = std::env::current_dir().unwrap().join(path).normalize(); + } + Ok(path) +} diff --git a/noir/noir-repo/tooling/artifact_cli/src/errors.rs b/noir/noir-repo/tooling/artifact_cli/src/errors.rs new file mode 100644 index 000000000000..5f302f78695a --- /dev/null +++ b/noir/noir-repo/tooling/artifact_cli/src/errors.rs @@ -0,0 +1,64 @@ +use acir::FieldElement; +use nargo::NargoError; +use noirc_abi::errors::{AbiError, InputParserError}; +use std::path::PathBuf; +use thiserror::Error; + +#[derive(Debug, Error)] +pub enum FilesystemError { + #[error("Cannot find input file '{0}'")] + MissingInputFile(PathBuf), + + #[error("Failed to parse input file '{0}': {1}")] + InvalidInputFile(PathBuf, String), + + #[error("Cannot find bytecode file '{0}'")] + MissingBytecodeFile(PathBuf), + + #[error("Failed to read bytecode file '{0}': {1}")] + InvalidBytecodeFile(PathBuf, String), + + #[error("Failed to create output witness file '{0}': {1}")] + OutputWitnessCreationFailed(PathBuf, String), + + #[error(transparent)] + IoError(#[from] std::io::Error), +} + +#[derive(Debug, Error)] +pub enum CliError { + /// Filesystem errors + #[error(transparent)] + FilesystemError(#[from] FilesystemError), + + /// Error related to ABI input deserialization + #[error("Failed to deserialize inputs")] + InputDeserializationError(#[from] InputParserError), + + /// Error related to ABI encoding + #[error(transparent)] + AbiError(#[from] AbiError), + + /// Error related to artifact deserialization + #[error("Failed to deserialize artifact from JSON")] + ArtifactDeserializationError(#[from] serde_json::Error), + + /// Error related to circuit deserialization + #[error("Failed to deserialize circuit from bytecode")] + CircuitDeserializationError(#[from] std::io::Error), + + /// Error related to circuit execution + #[error(transparent)] + CircuitExecutionError(#[from] NargoError), + + /// Input Witness Value Error + #[error("Failed to parse witness value {0}")] + WitnessValueError(String), + + /// Input Witness Index Error + #[error("Failed to parse witness index {0}")] + WitnessIndexError(String), + + #[error("Failed to serialize output witness: {0}")] + OutputWitnessSerializationFailed(#[from] toml::ser::Error), +} diff --git a/noir/noir-repo/tooling/artifact_cli/src/fs/artifact.rs b/noir/noir-repo/tooling/artifact_cli/src/fs/artifact.rs new file mode 100644 index 000000000000..856fc472fc73 --- /dev/null +++ b/noir/noir-repo/tooling/artifact_cli/src/fs/artifact.rs @@ -0,0 +1,37 @@ +use std::path::Path; + +use crate::{ + errors::{CliError, FilesystemError}, + Artifact, +}; +use noirc_artifacts::contract::ContractArtifact; +use noirc_artifacts::program::ProgramArtifact; + +impl Artifact { + /// Try to parse an artifact as a binary program or a contract + pub fn read_from_file(path: &Path) -> Result { + let json = std::fs::read(path).map_err(FilesystemError::from)?; + + let as_program = || serde_json::from_slice::(&json).map(Artifact::Program); + let as_contract = + || serde_json::from_slice::(&json).map(Artifact::Contract); + + as_program() + .or_else(|e| as_contract().map_err(|_| e)) + .map_err(CliError::ArtifactDeserializationError) + } +} + +/// Returns the circuit's bytecode read from the file at the given location +pub fn read_bytecode_from_file( + work_dir: &Path, + file_name: &str, +) -> Result, FilesystemError> { + let file_path = work_dir.join(file_name); + if !file_path.exists() { + return Err(FilesystemError::MissingBytecodeFile(file_path.clone())); + } + let bytecode: Vec = std::fs::read(&file_path) + .map_err(|e| FilesystemError::InvalidBytecodeFile(file_path, e.to_string()))?; + Ok(bytecode) +} diff --git a/noir/noir-repo/tooling/artifact_cli/src/fs/inputs.rs b/noir/noir-repo/tooling/artifact_cli/src/fs/inputs.rs new file mode 100644 index 000000000000..f115af92041b --- /dev/null +++ b/noir/noir-repo/tooling/artifact_cli/src/fs/inputs.rs @@ -0,0 +1,56 @@ +use noirc_abi::{ + input_parser::{Format, InputValue}, + Abi, InputMap, MAIN_RETURN_NAME, +}; +use std::{collections::BTreeMap, path::Path}; + +use crate::errors::CliError; + +/// Returns the circuit's parameters and its return value, if one exists. +/// +/// The file is is expected to contain ABI encoded inputs in TOML or JSON format. +pub fn read_inputs_from_file( + file_path: &Path, + abi: &Abi, +) -> Result<(InputMap, Option), CliError> { + use crate::errors::FilesystemError::{InvalidInputFile, MissingInputFile}; + use CliError::FilesystemError; + + let has_params = !abi.parameters.is_empty(); + let has_return = abi.return_type.is_some(); + let has_file = file_path.exists(); + + if !has_params && !has_return { + return Ok((BTreeMap::new(), None)); + } + if !has_params && !has_file { + // Reading a return value from the `Prover.toml` is optional, + // so if the ABI has no parameters we can skip reading the file if it doesn't exist. + return Ok((BTreeMap::new(), None)); + } + if has_params && !has_file { + return Err(FilesystemError(MissingInputFile(file_path.to_path_buf()))); + } + + let Some(ext) = file_path.extension().and_then(|e| e.to_str()) else { + return Err(FilesystemError(InvalidInputFile( + file_path.to_path_buf(), + "cannot determine input format".to_string(), + ))); + }; + + let Some(format) = Format::from_ext(ext) else { + return Err(FilesystemError(InvalidInputFile( + file_path.to_path_buf(), + format!("unknown input format: {ext}"), + ))); + }; + + let inputs = std::fs::read_to_string(file_path) + .map_err(|e| FilesystemError(InvalidInputFile(file_path.to_path_buf(), e.to_string())))?; + + let mut inputs = format.parse(&inputs, abi)?; + let return_value = inputs.remove(MAIN_RETURN_NAME); + + Ok((inputs, return_value)) +} diff --git a/noir/noir-repo/tooling/artifact_cli/src/fs/mod.rs b/noir/noir-repo/tooling/artifact_cli/src/fs/mod.rs new file mode 100644 index 000000000000..1a7043151684 --- /dev/null +++ b/noir/noir-repo/tooling/artifact_cli/src/fs/mod.rs @@ -0,0 +1,3 @@ +pub mod artifact; +pub mod inputs; +pub mod witness; diff --git a/noir/noir-repo/tooling/artifact_cli/src/fs/witness.rs b/noir/noir-repo/tooling/artifact_cli/src/fs/witness.rs new file mode 100644 index 000000000000..f486a53f14d8 --- /dev/null +++ b/noir/noir-repo/tooling/artifact_cli/src/fs/witness.rs @@ -0,0 +1,27 @@ +use std::path::{Path, PathBuf}; + +use acir::{native_types::WitnessStackError, FieldElement}; +use acvm::acir::native_types::WitnessStack; + +use crate::errors::FilesystemError; + +/// Write `witness.gz` to the output directory. +pub fn save_witness_to_dir( + witnesses: WitnessStack, + witness_name: &str, + witness_dir: &Path, +) -> Result { + std::fs::create_dir_all(witness_dir)?; + + let witness_path = witness_dir.join(witness_name).with_extension("gz"); + + let buf: Vec = witnesses.try_into().map_err(|e: WitnessStackError| { + FilesystemError::OutputWitnessCreationFailed(witness_path.clone(), format!("{e:?}")) + })?; + + std::fs::write(&witness_path, buf.as_slice()).map_err(|e| { + FilesystemError::OutputWitnessCreationFailed(witness_path.clone(), e.to_string()) + })?; + + Ok(witness_path) +} diff --git a/noir/noir-repo/tooling/artifact_cli/src/lib.rs b/noir/noir-repo/tooling/artifact_cli/src/lib.rs new file mode 100644 index 000000000000..2cd2341b7b70 --- /dev/null +++ b/noir/noir-repo/tooling/artifact_cli/src/lib.rs @@ -0,0 +1,12 @@ +use noirc_artifacts::{contract::ContractArtifact, program::ProgramArtifact}; + +pub mod commands; +pub mod errors; +pub mod fs; + +/// A parsed JSON build artifact. +#[derive(Debug, Clone)] +pub enum Artifact { + Program(ProgramArtifact), + Contract(ContractArtifact), +} diff --git a/noir/noir-repo/tooling/noirc_abi/src/input_parser/mod.rs b/noir/noir-repo/tooling/noirc_abi/src/input_parser/mod.rs index 565870b0835e..fce627c8d313 100644 --- a/noir/noir-repo/tooling/noirc_abi/src/input_parser/mod.rs +++ b/noir/noir-repo/tooling/noirc_abi/src/input_parser/mod.rs @@ -186,6 +186,7 @@ impl InputValue { /// The different formats that are supported when parsing /// the initial witness values +#[derive(Debug, Clone, PartialEq, Eq)] #[cfg_attr(test, derive(strum_macros::EnumIter))] pub enum Format { Json, @@ -199,6 +200,14 @@ impl Format { Format::Toml => "toml", } } + + pub fn from_ext(ext: &str) -> Option { + match ext { + "json" => Some(Self::Json), + "toml" => Some(Self::Toml), + _ => None, + } + } } impl Format { @@ -443,8 +452,9 @@ fn field_to_signed_hex(f: FieldElement, bit_size: u32) -> String { mod test { use acvm::{AcirField, FieldElement}; use num_bigint::BigUint; + use strum::IntoEnumIterator; - use super::{parse_str_to_field, parse_str_to_signed}; + use super::{parse_str_to_field, parse_str_to_signed, Format}; fn big_uint_from_field(field: FieldElement) -> BigUint { BigUint::from_bytes_be(&field.to_be_bytes()) @@ -521,6 +531,14 @@ mod test { ); assert!(parse_str_to_signed("-32769", 16, "arg_name").is_err()); } + + #[test] + fn test_from_ext() { + for fmt in Format::iter() { + assert_eq!(Format::from_ext(fmt.ext()), Some(fmt)); + } + assert_eq!(Format::from_ext("invalid extension"), None); + } } #[cfg(test)] diff --git a/noir/noir-repo/tooling/noirc_artifacts/src/contract.rs b/noir/noir-repo/tooling/noirc_artifacts/src/contract.rs index 2b78b6470375..0649a34e6255 100644 --- a/noir/noir-repo/tooling/noirc_artifacts/src/contract.rs +++ b/noir/noir-repo/tooling/noirc_artifacts/src/contract.rs @@ -9,7 +9,7 @@ use std::collections::{BTreeMap, HashMap}; use fm::FileId; -#[derive(Clone, Serialize, Deserialize)] +#[derive(Clone, Serialize, Deserialize, Debug)] pub struct ContractOutputsArtifact { pub structs: HashMap>, pub globals: HashMap>, @@ -21,7 +21,7 @@ impl From for ContractOutputsArtifact { } } -#[derive(Clone, Serialize, Deserialize)] +#[derive(Clone, Serialize, Deserialize, Debug)] pub struct ContractArtifact { /// Version of noir used to compile this contract pub noir_version: String,