From cfd8cbf58307511ac0cc9106c299695c2ca779de Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Wed, 30 Aug 2023 22:53:37 +0100 Subject: [PATCH] chore(acir)!: Remove unused `Directive` opcodes (#510) --- acir/src/circuit/directives.rs | 19 ----- acir/src/circuit/mod.rs | 8 +-- acir/src/circuit/opcodes.rs | 16 +---- acir/tests/test_program_serialization.rs | 88 +++++------------------- acvm/src/compiler/mod.rs | 4 -- acvm/src/pwg/directives/mod.rs | 57 +-------------- acvm/tests/solver.rs | 40 +++-------- acvm_js/test/shared/foreign_call.ts | 28 +++----- 8 files changed, 42 insertions(+), 218 deletions(-) diff --git a/acir/src/circuit/directives.rs b/acir/src/circuit/directives.rs index 2f2dac415..32c0bd633 100644 --- a/acir/src/circuit/directives.rs +++ b/acir/src/circuit/directives.rs @@ -15,12 +15,6 @@ pub struct QuotientDirective { /// You can think of them as opcodes that allow one to use non-determinism /// In the future, this can be replaced with asm non-determinism blocks pub enum Directive { - //Inverts the value of x and stores it in the result variable - Invert { - x: Witness, - result: Witness, - }, - //Performs euclidian division of a / b (as integers) and stores the quotient in q and the rest in r Quotient(QuotientDirective), @@ -39,27 +33,14 @@ pub enum Directive { bits: Vec, // control bits of the network which permutes the inputs into its sorted version sort_by: Vec, // specify primary index to sort by, then the secondary,... For instance, if tuple is 2 and sort_by is [1,0], then a=[(a0,b0),..] is sorted by bi and then ai. }, - Log(LogInfo), } impl Directive { pub fn name(&self) -> &str { match self { - Directive::Invert { .. } => "invert", Directive::Quotient(_) => "quotient", Directive::ToLeRadix { .. } => "to_le_radix", Directive::PermutationSort { .. } => "permutation_sort", - Directive::Log { .. } => "log", } } } - -#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] -// If values are compile time and/or known during -// evaluation, we can form an output string during ACIR generation. -// Otherwise, we must store witnesses whose values will -// be fetched during the PWG stage. -pub enum LogInfo { - FinalizedOutput(String), - WitnessOutput(Vec), -} diff --git a/acir/src/circuit/mod.rs b/acir/src/circuit/mod.rs index d1833527b..c4e3ac0fc 100644 --- a/acir/src/circuit/mod.rs +++ b/acir/src/circuit/mod.rs @@ -208,12 +208,6 @@ mod tests { use crate::native_types::Witness; use acir_field::FieldElement; - fn directive_opcode() -> Opcode { - Opcode::Directive(super::directives::Directive::Invert { - x: Witness(0), - result: Witness(1), - }) - } fn and_opcode() -> Opcode { Opcode::BlackBoxFuncCall(BlackBoxFuncCall::AND { lhs: FunctionInput { witness: Witness(1), num_bits: 4 }, @@ -231,7 +225,7 @@ mod tests { fn serialization_roundtrip() { let circuit = Circuit { current_witness_index: 5, - opcodes: vec![and_opcode(), range_opcode(), directive_opcode()], + opcodes: vec![and_opcode(), range_opcode()], private_parameters: BTreeSet::new(), public_parameters: PublicInputs(BTreeSet::from_iter(vec![Witness(2), Witness(12)])), return_values: PublicInputs(BTreeSet::from_iter(vec![Witness(4), Witness(12)])), diff --git a/acir/src/circuit/opcodes.rs b/acir/src/circuit/opcodes.rs index d422ffb17..dc7f73b47 100644 --- a/acir/src/circuit/opcodes.rs +++ b/acir/src/circuit/opcodes.rs @@ -1,6 +1,6 @@ use super::{ brillig::Brillig, - directives::{Directive, LogInfo, QuotientDirective}, + directives::{Directive, QuotientDirective}, }; use crate::native_types::{Expression, Witness}; use serde::{Deserialize, Serialize}; @@ -100,10 +100,6 @@ impl std::fmt::Display for Opcode { write!(f, " ]") } - Opcode::Directive(Directive::Invert { x, result: r }) => { - write!(f, "DIR::INVERT ")?; - write!(f, "(_{}, out: _{}) ", x.witness_index(), r.witness_index()) - } Opcode::Directive(Directive::Quotient(QuotientDirective { a, b, q, r, predicate })) => { write!(f, "DIR::QUOTIENT ")?; if let Some(pred) = predicate { @@ -145,15 +141,7 @@ impl std::fmt::Display for Opcode { bits.last().unwrap().witness_index(), ) } - Opcode::Directive(Directive::Log(info)) => match info { - LogInfo::FinalizedOutput(output_string) => write!(f, "Log: {output_string}"), - LogInfo::WitnessOutput(witnesses) => write!( - f, - "Log: _{}..._{}", - witnesses.first().unwrap().witness_index(), - witnesses.last().unwrap().witness_index() - ), - }, + Opcode::Brillig(brillig) => { write!(f, "BRILLIG: ")?; writeln!(f, "inputs: {:?}", brillig.inputs)?; diff --git a/acir/tests/test_program_serialization.rs b/acir/tests/test_program_serialization.rs index 6a0c3c290..6d69a1392 100644 --- a/acir/tests/test_program_serialization.rs +++ b/acir/tests/test_program_serialization.rs @@ -14,14 +14,13 @@ use std::collections::BTreeSet; use acir::{ circuit::{ brillig::{Brillig, BrilligInputs, BrilligOutputs}, - directives::Directive, opcodes::{BlackBoxFuncCall, BlockId, FunctionInput, MemOp}, Circuit, Opcode, PublicInputs, }, native_types::{Expression, Witness}, }; use acir_field::FieldElement; -use brillig::{BinaryFieldOp, HeapArray, RegisterIndex, RegisterOrMemory}; +use brillig::{HeapArray, RegisterIndex, RegisterOrMemory}; #[test] fn addition_circuit() { @@ -170,72 +169,28 @@ fn schnorr_verify_circuit() { #[test] fn simple_brillig_foreign_call() { - let fe_0 = FieldElement::zero(); - let fe_1 = FieldElement::one(); - let w_x = Witness(1); - let w_y = Witness(2); - let w_oracle = Witness(3); - let w_z = Witness(4); - let w_z_inverse = Witness(5); - let w_x_plus_y = Witness(6); - let w_equal_res = Witness(7); - - let equal_opcode = brillig::Opcode::BinaryFieldOp { - op: BinaryFieldOp::Equals, - lhs: RegisterIndex::from(0), - rhs: RegisterIndex::from(1), - destination: RegisterIndex::from(2), - }; + let w_input = Witness(1); + let w_inverted = Witness(2); let brillig_data = Brillig { inputs: vec![ - BrilligInputs::Single(Expression { - // Input Register 0 - mul_terms: vec![], - linear_combinations: vec![(fe_1, w_x), (fe_1, w_y)], - q_c: fe_0, - }), - BrilligInputs::Single(Expression::default()), // Input Register 1 + BrilligInputs::Single(w_input.into()), // Input Register 0, ], // This tells the BrilligSolver which witnesses its output registers correspond to outputs: vec![ - BrilligOutputs::Simple(w_x_plus_y), // Output Register 0 - from input - BrilligOutputs::Simple(w_oracle), // Output Register 1 - BrilligOutputs::Simple(w_equal_res), // Output Register 2 + BrilligOutputs::Simple(w_inverted), // Output Register 1 ], // stack of foreign call/oracle resolutions, starts empty foreign_call_results: vec![], - bytecode: vec![ - equal_opcode, - // Oracles are named 'foreign calls' in brillig - brillig::Opcode::ForeignCall { - function: "invert".into(), - destinations: vec![RegisterOrMemory::RegisterIndex(RegisterIndex::from(1))], - inputs: vec![RegisterOrMemory::RegisterIndex(RegisterIndex::from(0))], - }, - ], + bytecode: vec![brillig::Opcode::ForeignCall { + function: "invert".into(), + destinations: vec![RegisterOrMemory::RegisterIndex(RegisterIndex::from(0))], + inputs: vec![RegisterOrMemory::RegisterIndex(RegisterIndex::from(0))], + }], predicate: None, }; - let opcodes = vec![ - Opcode::Brillig(brillig_data), - Opcode::Arithmetic(Expression { - mul_terms: vec![], - linear_combinations: vec![(fe_1, w_x), (fe_1, w_y), (-fe_1, w_z)], - q_c: fe_0, - }), - Opcode::Directive(Directive::Invert { x: w_z, result: w_z_inverse }), - Opcode::Arithmetic(Expression { - mul_terms: vec![(fe_1, w_z, w_z_inverse)], - linear_combinations: vec![], - q_c: -fe_1, - }), - Opcode::Arithmetic(Expression { - mul_terms: vec![], - linear_combinations: vec![(-fe_1, w_oracle), (fe_1, w_z_inverse)], - q_c: fe_0, - }), - ]; + let opcodes = vec![Opcode::Brillig(brillig_data)]; let circuit = Circuit { current_witness_index: 8, opcodes, @@ -247,16 +202,11 @@ fn simple_brillig_foreign_call() { circuit.write(&mut bytes).unwrap(); let expected_serialization: Vec = vec![ - 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 181, 148, 209, 10, 195, 32, 12, 69, 99, 109, 183, 126, - 78, 82, 181, 141, 111, 251, 149, 201, 44, 236, 101, 15, 99, 236, 251, 183, 49, 11, 161, - 245, 173, 233, 5, 137, 4, 57, 120, 111, 208, 30, 0, 58, 248, 203, 126, 87, 3, 91, 45, 189, - 75, 169, 184, 79, 100, 20, 89, 141, 30, 11, 43, 214, 213, 216, 86, 48, 79, 34, 239, 159, - 206, 149, 172, 229, 190, 21, 61, 83, 106, 47, 56, 247, 199, 59, 63, 95, 166, 114, 74, 246, - 170, 178, 186, 54, 15, 27, 173, 195, 209, 251, 60, 13, 153, 28, 93, 113, 136, 137, 3, 250, - 144, 70, 38, 166, 192, 225, 54, 176, 115, 153, 61, 79, 49, 197, 9, 35, 121, 151, 105, 14, - 209, 205, 5, 214, 234, 221, 11, 229, 88, 186, 85, 208, 90, 222, 37, 27, 20, 115, 16, 200, - 205, 179, 222, 203, 182, 138, 254, 187, 3, 230, 101, 160, 254, 189, 45, 250, 0, 87, 206, - 28, 176, 11, 5, 0, 0, + 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 173, 143, 81, 10, 0, 16, 16, 68, 199, 42, 57, 14, 55, + 112, 25, 31, 126, 124, 72, 206, 79, 161, 86, 225, 135, 87, 219, 78, 187, 53, 205, 104, 0, + 2, 29, 201, 52, 103, 222, 220, 216, 230, 13, 43, 254, 121, 25, 158, 151, 54, 153, 117, 27, + 53, 116, 136, 197, 167, 124, 107, 184, 64, 236, 73, 56, 83, 1, 18, 139, 122, 157, 67, 1, 0, + 0, ]; assert_eq!(bytes, expected_serialization) @@ -280,9 +230,9 @@ fn complex_brillig_foreign_call() { inputs: vec![ // Input Register 0 BrilligInputs::Array(vec![ - Expression { mul_terms: vec![], linear_combinations: vec![(fe_1, a)], q_c: fe_0 }, - Expression { mul_terms: vec![], linear_combinations: vec![(fe_1, b)], q_c: fe_0 }, - Expression { mul_terms: vec![], linear_combinations: vec![(fe_1, c)], q_c: fe_0 }, + Expression::from(a), + Expression::from(b), + Expression::from(c), ]), // Input Register 1 BrilligInputs::Single(Expression { diff --git a/acvm/src/compiler/mod.rs b/acvm/src/compiler/mod.rs index dbb731554..001b4904e 100644 --- a/acvm/src/compiler/mod.rs +++ b/acvm/src/compiler/mod.rs @@ -212,9 +212,6 @@ pub fn compile( } Opcode::Directive(directive) => { match directive { - Directive::Invert { result, .. } => { - transformer.mark_solvable(*result); - } Directive::Quotient(quotient_directive) => { transformer.mark_solvable(quotient_directive.q); transformer.mark_solvable(quotient_directive.r); @@ -229,7 +226,6 @@ pub fn compile( transformer.mark_solvable(*witness); } } - Directive::Log(_) => (), } new_acir_opcode_positions.push(acir_opcode_positions[index]); transformed_opcodes.push(opcode.clone()); diff --git a/acvm/src/pwg/directives/mod.rs b/acvm/src/pwg/directives/mod.rs index e0fc46e11..d7dbb3eda 100644 --- a/acvm/src/pwg/directives/mod.rs +++ b/acvm/src/pwg/directives/mod.rs @@ -1,7 +1,7 @@ use std::cmp::Ordering; use acir::{ - circuit::directives::{Directive, LogInfo, QuotientDirective}, + circuit::directives::{Directive, QuotientDirective}, native_types::WitnessMap, FieldElement, }; @@ -10,7 +10,7 @@ use num_traits::Zero; use crate::OpcodeResolutionError; -use super::{get_value, insert_value, witness_to_value, ErrorLocation}; +use super::{get_value, insert_value, ErrorLocation}; mod sorting; @@ -25,11 +25,6 @@ pub(super) fn solve_directives( directive: &Directive, ) -> Result<(), OpcodeResolutionError> { match directive { - Directive::Invert { x, result } => { - let val = witness_to_value(initial_witness, *x)?; - insert_value(result, val.inverse(), initial_witness)?; - Ok(()) - } Directive::Quotient(QuotientDirective { a, b, q, r, predicate }) => { let val_a = get_value(a, initial_witness)?; let val_b = get_value(b, initial_witness)?; @@ -123,55 +118,7 @@ pub(super) fn solve_directives( } Ok(()) } - Directive::Log(info) => { - let witnesses = match info { - LogInfo::FinalizedOutput(output_string) => { - println!("{output_string}"); - return Ok(()); - } - LogInfo::WitnessOutput(witnesses) => witnesses, - }; - - if witnesses.len() == 1 { - let witness = &witnesses[0]; - let log_value = witness_to_value(initial_witness, *witness)?; - println!("{}", format_field_string(*log_value)); - return Ok(()); - } - - // If multiple witnesses are to be fetched for a log directive, - // it assumed that an array is meant to be printed to standard output - // - // Collect all field element values corresponding to the given witness indices - // and convert them to hex strings. - let mut elements_as_hex = Vec::with_capacity(witnesses.len()); - for witness in witnesses { - let element = witness_to_value(initial_witness, *witness)?; - elements_as_hex.push(format_field_string(*element)); - } - - // Join all of the hex strings using a comma - let comma_separated_elements = elements_as_hex.join(", "); - - let output_witnesses_string = "[".to_owned() + &comma_separated_elements + "]"; - - println!("{output_witnesses_string}"); - - Ok(()) - } - } -} - -/// This trims any leading zeroes. -/// A singular '0' will be prepended as well if the trimmed string has an odd length. -/// A hex string's length needs to be even to decode into bytes, as two digits correspond to -/// one byte. -fn format_field_string(field: FieldElement) -> String { - let mut trimmed_field = field.to_hex().trim_start_matches('0').to_owned(); - if trimmed_field.len() % 2 != 0 { - trimmed_field = "0".to_owned() + &trimmed_field } - "0x".to_owned() + &trimmed_field } #[cfg(test)] diff --git a/acvm/tests/solver.rs b/acvm/tests/solver.rs index 38bd51701..a888a062d 100644 --- a/acvm/tests/solver.rs +++ b/acvm/tests/solver.rs @@ -45,7 +45,10 @@ impl BlackBoxFunctionSolver for StubbedBackend { } } +// Reenable these test cases once we move the brillig implementation of inversion down into the acvm stdlib. + #[test] +#[ignore] fn inversion_brillig_oracle_equivalence() { // Opcodes below describe the following: // fn main(x : Field, y : pub Field) { @@ -108,7 +111,7 @@ fn inversion_brillig_oracle_equivalence() { linear_combinations: vec![(fe_1, w_x), (fe_1, w_y), (-fe_1, w_z)], q_c: fe_0, }), - Opcode::Directive(Directive::Invert { x: w_z, result: w_z_inverse }), + // Opcode::Directive(Directive::Invert { x: w_z, result: w_z_inverse }), Opcode::Arithmetic(Expression { mul_terms: vec![(fe_1, w_z, w_z_inverse)], linear_combinations: vec![], @@ -155,6 +158,7 @@ fn inversion_brillig_oracle_equivalence() { } #[test] +#[ignore] fn double_inversion_brillig_oracle() { // Opcodes below describe the following: // fn main(x : Field, y : pub Field) { @@ -235,7 +239,7 @@ fn double_inversion_brillig_oracle() { linear_combinations: vec![(fe_1, w_x), (fe_1, w_y), (-fe_1, w_z)], q_c: fe_0, }), - Opcode::Directive(Directive::Invert { x: w_z, result: w_z_inverse }), + // Opcode::Directive(Directive::Invert { x: w_z, result: w_z_inverse }), Opcode::Arithmetic(Expression { mul_terms: vec![(fe_1, w_z, w_z_inverse)], linear_combinations: vec![], @@ -423,24 +427,14 @@ fn oracle_dependent_execution() { #[test] fn brillig_oracle_predicate() { - // Opcodes below describe the following: - // fn main(x : Field, y : pub Field, cond: bool) { - // let z = x + y; - // let z_inverse = 1/z - // if cond { - // assert( z_inverse == Oracle("inverse", x + y) ); - // } - // } let fe_0 = FieldElement::zero(); let fe_1 = FieldElement::one(); let w_x = Witness(1); let w_y = Witness(2); let w_oracle = Witness(3); - let w_z = Witness(4); - let w_z_inverse = Witness(5); - let w_x_plus_y = Witness(6); - let w_equal_res = Witness(7); - let w_lt_res = Witness(8); + let w_x_plus_y = Witness(4); + let w_equal_res = Witness(5); + let w_lt_res = Witness(6); let equal_opcode = BrilligOpcode::BinaryFieldOp { op: BinaryFieldOp::Equals, @@ -478,20 +472,7 @@ fn brillig_oracle_predicate() { foreign_call_results: vec![], }); - let opcodes = vec![ - brillig_opcode, - Opcode::Arithmetic(Expression { - mul_terms: vec![], - linear_combinations: vec![(fe_1, w_x), (fe_1, w_y), (-fe_1, w_z)], - q_c: fe_0, - }), - Opcode::Directive(Directive::Invert { x: w_z, result: w_z_inverse }), - Opcode::Arithmetic(Expression { - mul_terms: vec![(fe_1, w_z, w_z_inverse)], - linear_combinations: vec![], - q_c: -fe_1, - }), - ]; + let opcodes = vec![brillig_opcode]; let witness_assignments = BTreeMap::from([ (Witness(1), FieldElement::from(2u128)), @@ -506,7 +487,6 @@ fn brillig_oracle_predicate() { // ACVM should be able to be finalized in `Solved` state. acvm.finalize(); } - #[test] fn unsatisfied_opcode_resolved() { let a = Witness(0); diff --git a/acvm_js/test/shared/foreign_call.ts b/acvm_js/test/shared/foreign_call.ts index 7224002ce..038b64a50 100644 --- a/acvm_js/test/shared/foreign_call.ts +++ b/acvm_js/test/shared/foreign_call.ts @@ -2,21 +2,14 @@ import { WitnessMap } from "../../../result/"; // See `simple_brillig_foreign_call` integration test in `acir/tests/test_program_serialization.rs`. export const bytecode = Uint8Array.from([ - 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 181, 148, 209, 10, 195, 32, 12, 69, 99, - 109, 183, 126, 78, 82, 181, 141, 111, 251, 149, 201, 44, 236, 101, 15, 99, - 236, 251, 183, 49, 11, 161, 245, 173, 233, 5, 137, 4, 57, 120, 111, 208, 30, - 0, 58, 248, 203, 126, 87, 3, 91, 45, 189, 75, 169, 184, 79, 100, 20, 89, 141, - 30, 11, 43, 214, 213, 216, 86, 48, 79, 34, 239, 159, 206, 149, 172, 229, 190, - 21, 61, 83, 106, 47, 56, 247, 199, 59, 63, 95, 166, 114, 74, 246, 170, 178, - 186, 54, 15, 27, 173, 195, 209, 251, 60, 13, 153, 28, 93, 113, 136, 137, 3, - 250, 144, 70, 38, 166, 192, 225, 54, 176, 115, 153, 61, 79, 49, 197, 9, 35, - 121, 151, 105, 14, 209, 205, 5, 214, 234, 221, 11, 229, 88, 186, 85, 208, 90, - 222, 37, 27, 20, 115, 16, 200, 205, 179, 222, 203, 182, 138, 254, 187, 3, 230, - 101, 160, 254, 189, 45, 250, 0, 87, 206, 28, 176, 11, 5, 0, 0, + 31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 173, 143, 81, 10, 0, 16, 16, 68, 199, 42, + 57, 14, 55, 112, 25, 31, 126, 124, 72, 206, 79, 161, 86, 225, 135, 87, 219, + 78, 187, 53, 205, 104, 0, 2, 29, 201, 52, 103, 222, 220, 216, 230, 13, 43, + 254, 121, 25, 158, 151, 54, 153, 117, 27, 53, 116, 136, 197, 167, 124, 107, + 184, 64, 236, 73, 56, 83, 1, 18, 139, 122, 157, 67, 1, 0, 0, ]); export const initialWitnessMap: WitnessMap = new Map([ - [1, "0x0000000000000000000000000000000000000000000000000000000000000002"], - [2, "0x0000000000000000000000000000000000000000000000000000000000000003"], + [1, "0x0000000000000000000000000000000000000000000000000000000000000005"], ]); export const oracleCallName = "invert"; @@ -29,11 +22,6 @@ export const oracleResponse = [ ]; export const expectedWitnessMap = new Map([ - [1, "0x0000000000000000000000000000000000000000000000000000000000000002"], - [2, "0x0000000000000000000000000000000000000000000000000000000000000003"], - [3, "0x135b52945a13d9aa49b9b57c33cd568ba9ae5ce9ca4a2d06e7f3fbd4c6666667"], - [4, "0x0000000000000000000000000000000000000000000000000000000000000005"], - [5, "0x135b52945a13d9aa49b9b57c33cd568ba9ae5ce9ca4a2d06e7f3fbd4c6666667"], - [6, "0x0000000000000000000000000000000000000000000000000000000000000005"], - [7, "0x0000000000000000000000000000000000000000000000000000000000000000"], + [1, "0x0000000000000000000000000000000000000000000000000000000000000005"], + [2, "0x135b52945a13d9aa49b9b57c33cd568ba9ae5ce9ca4a2d06e7f3fbd4c6666667"], ]);