From 200d4b4440c547b43039891bd11915f6774b11d7 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Thu, 19 Sep 2024 01:02:54 +0000 Subject: [PATCH 1/5] fix: decode databus return values --- .../noir_js/scripts/compile_test_programs.sh | 1 + tooling/noir_js/test/node/execute.test.ts | 49 +++++-------------- .../noir_compiled_examples/databus/Nargo.toml | 5 ++ .../databus/src/main.nr | 12 +++++ tooling/noirc_abi/src/lib.rs | 10 +--- 5 files changed, 30 insertions(+), 47 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/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..f979e72db6b 100644 --- a/tooling/noir_js/test/node/execute.test.ts +++ b/tooling/noir_js/test/node/execute.test.ts @@ -2,6 +2,8 @@ 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 +12,8 @@ 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 +90,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..3c63fcad43e 100644 --- a/tooling/noirc_abi/src/lib.rs +++ b/tooling/noirc_abi/src/lib.rs @@ -346,15 +346,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. From 2d33abf5a181e438e401fa0be64241c9d6205145 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Thu, 19 Sep 2024 01:08:06 +0000 Subject: [PATCH 2/5] . --- tooling/noir_js/test/node/execute.test.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tooling/noir_js/test/node/execute.test.ts b/tooling/noir_js/test/node/execute.test.ts index f979e72db6b..a442b3c42df 100644 --- a/tooling/noir_js/test/node/execute.test.ts +++ b/tooling/noir_js/test/node/execute.test.ts @@ -4,7 +4,6 @@ import fold_fibonacci_json from '../noir_compiled_examples/fold_fibonacci/target 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'; import { expect } from 'chai'; @@ -14,7 +13,6 @@ 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 = { x: '2', @@ -95,10 +93,10 @@ 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: "3", - y: "4", - z: [1,2,3,4], + x: '3', + y: '4', + z: [1, 2, 3, 4], }; const { returnValue } = await new Noir(databus_program).execute(inputs); - expect(returnValue).to.be.eq("0x09") + expect(returnValue).to.be.eq('0x09'); }); From b78806e1d2cc21188a46857890fc1533966c4b90 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Thu, 19 Sep 2024 14:23:37 +0000 Subject: [PATCH 3/5] fix: update `has_public_inputs` to account for databus --- tooling/noirc_abi/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tooling/noirc_abi/src/lib.rs b/tooling/noirc_abi/src/lib.rs index 3c63fcad43e..781a522737d 100644 --- a/tooling/noirc_abi/src/lib.rs +++ b/tooling/noirc_abi/src/lib.rs @@ -197,7 +197,9 @@ 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. From f739f3d02bd8c315108d471eed60b809ee0f982b Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Thu, 19 Sep 2024 14:30:21 +0000 Subject: [PATCH 4/5] . --- tooling/noirc_abi/src/lib.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tooling/noirc_abi/src/lib.rs b/tooling/noirc_abi/src/lib.rs index 781a522737d..39f2e935618 100644 --- a/tooling/noirc_abi/src/lib.rs +++ b/tooling/noirc_abi/src/lib.rs @@ -198,8 +198,11 @@ impl Abi { /// Returns whether any values are needed to be made public for verification. pub fn has_public_inputs(&self) -> bool { 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 + 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. From 3763be3d0e49ed2b50ca2c6cff7318876a9eaa29 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Thu, 19 Sep 2024 14:58:08 +0000 Subject: [PATCH 5/5] fix: don't embed databus return witnesses into the circuit --- compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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(