From c40eb1fd8a0ba63b2d122e42b47dfa9dca5bf7b0 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Thu, 19 Sep 2024 20:17:02 +0100 Subject: [PATCH] fix: decode databus return values (#6095) # Description ## Problem\* Resolves #5715 ## Summary\* This PR picks up from @guipublic's work on #6074 to enable decoding databus return values in `noirc_abi`. I've added a test to show this working ## Additional Context ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 7 ++- .../noir_js/scripts/compile_test_programs.sh | 1 + tooling/noir_js/test/node/execute.test.ts | 47 ++++--------------- .../noir_compiled_examples/databus/Nargo.toml | 5 ++ .../databus/src/main.nr | 12 +++++ tooling/noirc_abi/src/lib.rs | 17 +++---- 6 files changed, 40 insertions(+), 49 deletions(-) create mode 100644 tooling/noir_js/test/noir_compiled_examples/databus/Nargo.toml create mode 100644 tooling/noir_js/test/noir_compiled_examples/databus/src/main.nr diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 430801dacf6..15d72c2ae74 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -450,7 +450,12 @@ impl<'a> Context<'a> { warnings.extend(self.acir_context.warnings.clone()); // Add the warnings from the alter Ssa passes - Ok(self.acir_context.finish(input_witness, return_witnesses, warnings)) + Ok(self.acir_context.finish( + input_witness, + // Don't embed databus return witnesses into the circuit. + if self.data_bus.return_data.is_some() { Vec::new() } else { return_witnesses }, + warnings, + )) } fn initialize_databus( diff --git a/tooling/noir_js/scripts/compile_test_programs.sh b/tooling/noir_js/scripts/compile_test_programs.sh index 0642159aa69..c65aea823a4 100755 --- a/tooling/noir_js/scripts/compile_test_programs.sh +++ b/tooling/noir_js/scripts/compile_test_programs.sh @@ -5,3 +5,4 @@ nargo --program-dir ./test/noir_compiled_examples/assert_lt compile --force nargo --program-dir ./test/noir_compiled_examples/assert_msg_runtime compile --force nargo --program-dir ./test/noir_compiled_examples/fold_fibonacci compile --force nargo --program-dir ./test/noir_compiled_examples/assert_raw_payload compile --force +nargo --program-dir ./test/noir_compiled_examples/databus compile --force diff --git a/tooling/noir_js/test/node/execute.test.ts b/tooling/noir_js/test/node/execute.test.ts index 7c93a911042..a442b3c42df 100644 --- a/tooling/noir_js/test/node/execute.test.ts +++ b/tooling/noir_js/test/node/execute.test.ts @@ -2,6 +2,7 @@ import assert_lt_json from '../noir_compiled_examples/assert_lt/target/assert_lt import assert_msg_json from '../noir_compiled_examples/assert_msg_runtime/target/assert_msg_runtime.json' assert { type: 'json' }; import fold_fibonacci_json from '../noir_compiled_examples/fold_fibonacci/target/fold_fibonacci.json' assert { type: 'json' }; import assert_raw_payload_json from '../noir_compiled_examples/assert_raw_payload/target/assert_raw_payload.json' assert { type: 'json' }; +import databus_json from '../noir_compiled_examples/databus/target/databus.json' assert { type: 'json' }; import { Noir, ErrorWithPayload } from '@noir-lang/noir_js'; import { CompiledCircuit } from '@noir-lang/types'; @@ -10,6 +11,7 @@ import { expect } from 'chai'; const assert_lt_program = assert_lt_json as CompiledCircuit; const assert_msg_runtime = assert_msg_json as CompiledCircuit; const fold_fibonacci_program = fold_fibonacci_json as CompiledCircuit; +const databus_program = databus_json as CompiledCircuit; it('executes a single-ACIR program correctly', async () => { const inputs = { @@ -86,46 +88,15 @@ it('successfully executes a program with multiple acir circuits', async () => { const inputs = { x: '10', }; - try { - await new Noir(fold_fibonacci_program).execute(inputs); - } catch (error) { - const knownError = error as Error; - expect(knownError.message).to.equal('Circuit execution failed: Error: Cannot satisfy constraint'); - } -}); - -it('successfully executes a program with multiple acir circuits', async () => { - const inputs = { - x: '10', - }; - try { - await new Noir(fold_fibonacci_program).execute(inputs); - } catch (error) { - const knownError = error as Error; - expect(knownError.message).to.equal('Circuit execution failed: Error: Cannot satisfy constraint'); - } -}); - -it('successfully executes a program with multiple acir circuits', async () => { - const inputs = { - x: '10', - }; - try { - await new Noir(fold_fibonacci_program).execute(inputs); - } catch (error) { - const knownError = error as Error; - expect(knownError.message).to.equal('Circuit execution failed: Error: Cannot satisfy constraint'); - } + expect(() => new Noir(fold_fibonacci_program).execute(inputs)).to.not.throw(); }); -it('successfully executes a program with multiple acir circuits', async () => { +it('successfully decodes the return values from a program using the databus', async () => { const inputs = { - x: '10', + x: '3', + y: '4', + z: [1, 2, 3, 4], }; - try { - await new Noir(fold_fibonacci_program).execute(inputs); - } catch (error) { - const knownError = error as Error; - expect(knownError.message).to.equal('Circuit execution failed: Error: Cannot satisfy constraint'); - } + const { returnValue } = await new Noir(databus_program).execute(inputs); + expect(returnValue).to.be.eq('0x09'); }); diff --git a/tooling/noir_js/test/noir_compiled_examples/databus/Nargo.toml b/tooling/noir_js/test/noir_compiled_examples/databus/Nargo.toml new file mode 100644 index 00000000000..d484b80325e --- /dev/null +++ b/tooling/noir_js/test/noir_compiled_examples/databus/Nargo.toml @@ -0,0 +1,5 @@ +[package] +name = "databus" +type = "bin" +authors = [""] +[dependencies] diff --git a/tooling/noir_js/test/noir_compiled_examples/databus/src/main.nr b/tooling/noir_js/test/noir_compiled_examples/databus/src/main.nr new file mode 100644 index 00000000000..ecc7794cf9e --- /dev/null +++ b/tooling/noir_js/test/noir_compiled_examples/databus/src/main.nr @@ -0,0 +1,12 @@ +fn main(mut x: u32, y: call_data(0) u32, z: call_data(0) [u32; 4]) -> return_data u32 { + let a = z[x]; + unsafe { + a + foo(y) + } +} + +// Use an unconstrained function to force the compiler to avoid inlining +unconstrained fn foo(x: u32) -> u32 { + x + 1 +} + diff --git a/tooling/noirc_abi/src/lib.rs b/tooling/noirc_abi/src/lib.rs index c3e1ade04fa..39f2e935618 100644 --- a/tooling/noirc_abi/src/lib.rs +++ b/tooling/noirc_abi/src/lib.rs @@ -197,7 +197,12 @@ impl Abi { /// Returns whether any values are needed to be made public for verification. pub fn has_public_inputs(&self) -> bool { - self.return_type.is_some() || self.parameters.iter().any(|param| param.is_public()) + let has_public_args = self.parameters.iter().any(|param| param.is_public()); + let has_public_return = self + .return_type + .as_ref() + .map_or(false, |typ| matches!(typ.visibility, AbiVisibility::Public)); + has_public_args || has_public_return } /// Returns `true` if the ABI contains no parameters or return value. @@ -346,15 +351,7 @@ impl Abi { .copied() }) { - // We do not return value for the data bus. - if return_type.visibility == AbiVisibility::DataBus { - None - } else { - Some(decode_value( - &mut return_witness_values.into_iter(), - &return_type.abi_type, - )?) - } + Some(decode_value(&mut return_witness_values.into_iter(), &return_type.abi_type)?) } else { // Unlike for the circuit inputs, we tolerate not being able to find the witness values for the return value. // This is because the user may be decoding a partial witness map for which is hasn't been calculated yet.