From 2d1fdf892114e571a39713be172977dfadbeca22 Mon Sep 17 00:00:00 2001 From: ethan-000 Date: Thu, 19 Jan 2023 23:08:19 +0000 Subject: [PATCH 01/19] refactor ToRadix to ToRadixLe and ToRadixBe --- acir/src/circuit/directives.rs | 53 +++++++++++++++++++++++++++++----- acir/src/circuit/opcodes.rs | 16 ++++++++-- acvm/src/pwg/directives.rs | 30 ++++++++++++++++++- stdlib/src/fallback.rs | 2 +- 4 files changed, 89 insertions(+), 12 deletions(-) diff --git a/acir/src/circuit/directives.rs b/acir/src/circuit/directives.rs index c3d8e2cd0..558bfb6f4 100644 --- a/acir/src/circuit/directives.rs +++ b/acir/src/circuit/directives.rs @@ -42,8 +42,15 @@ pub enum Directive { bit_size: u32, }, - //decomposition of a: a=\sum b[i]*radix^i where b is an array of witnesses < radix - ToRadix { + //decomposition of a: a=\sum b[i]*radix^i where b is an array of witnesses < radix in little endian form + ToRadixLe { + a: Expression, + b: Vec, + radix: u32, + }, + + //decomposition of a: a=\sum b[i]*radix^i where b is an array of witnesses < radix in big endian form + ToRadixBe { a: Expression, b: Vec, radix: u32, @@ -57,7 +64,8 @@ impl Directive { Directive::Quotient { .. } => "quotient", Directive::Truncate { .. } => "truncate", Directive::OddRange { .. } => "odd_range", - Directive::ToRadix { .. } => "to_radix", + Directive::ToRadixLe { .. } => "to_radix_le", + Directive::ToRadixBe { .. } => "to_radix_be", } } fn to_u16(&self) -> u16 { @@ -66,7 +74,8 @@ impl Directive { Directive::Quotient { .. } => 1, Directive::Truncate { .. } => 2, Directive::OddRange { .. } => 3, - Directive::ToRadix { .. } => 4, + Directive::ToRadixLe { .. } => 4, + Directive::ToRadixBe { .. } => 5, } } @@ -108,7 +117,15 @@ impl Directive { write_u32(&mut writer, r.witness_index())?; write_u32(&mut writer, *bit_size)?; } - Directive::ToRadix { a, b, radix } => { + Directive::ToRadixLe { a, b, radix } => { + a.write(&mut writer)?; + write_u32(&mut writer, b.len() as u32)?; + for bit in b { + write_u32(&mut writer, bit.witness_index())?; + } + write_u32(&mut writer, *radix)?; + } + Directive::ToRadixBe { a, b, radix } => { a.write(&mut writer)?; write_u32(&mut writer, b.len() as u32)?; for bit in b { @@ -176,7 +193,20 @@ impl Directive { let radix = read_u32(&mut reader)?; - Ok(Directive::ToRadix { a, b, radix }) + Ok(Directive::ToRadixLe { a, b, radix }) + } + 5 => { + let a = Expression::read(&mut reader)?; + let b_len = read_u32(&mut reader)?; + let mut b = Vec::with_capacity(b_len as usize); + for _ in 0..b_len { + let witness = Witness(read_u32(&mut reader)?); + b.push(witness) + } + + let radix = read_u32(&mut reader)?; + + Ok(Directive::ToRadixBe { a, b, radix }) } _ => Err(std::io::ErrorKind::InvalidData.into()), @@ -228,7 +258,13 @@ fn serialisation_roundtrip() { bit_size: 32, }; - let to_radix = Directive::ToRadix { + let to_radix_le = Directive::ToRadixLe { + a: Expression::default(), + b: vec![Witness(1u32), Witness(2u32), Witness(3u32), Witness(4u32)], + radix: 4, + }; + + let to_radix_be = Directive::ToRadixBe { a: Expression::default(), b: vec![Witness(1u32), Witness(2u32), Witness(3u32), Witness(4u32)], radix: 4, @@ -240,7 +276,8 @@ fn serialisation_roundtrip() { quotient_predicate, truncate, odd_range, - to_radix, + to_radix_le, + to_radix_be, ]; for directive in directives { diff --git a/acir/src/circuit/opcodes.rs b/acir/src/circuit/opcodes.rs index 37017adde..5aa4053bc 100644 --- a/acir/src/circuit/opcodes.rs +++ b/acir/src/circuit/opcodes.rs @@ -152,8 +152,20 @@ impl std::fmt::Display for Opcode { ) } Opcode::BlackBoxFuncCall(g) => write!(f, "{g}"), - Opcode::Directive(Directive::ToRadix { a, b, radix: _ }) => { - write!(f, "DIR::TORADIX ")?; + Opcode::Directive(Directive::ToRadixLe { a, b, radix: _ }) => { + write!(f, "DIR::TORADIXLE ")?; + write!( + f, + // TODO (Note): this assumes that the decomposed bits have contiguous witness indices + // This should be the case, however, we can also have a function which checks this + "(_{}, [_{}..._{}])", + a, + b.first().unwrap().witness_index(), + b.last().unwrap().witness_index(), + ) + } + Opcode::Directive(Directive::ToRadixBe { a, b, radix: _ }) => { + write!(f, "DIR::TORADIXLE ")?; write!( f, // TODO (Note): this assumes that the decomposed bits have contiguous witness indices diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index 94bc2d60b..296824793 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -64,7 +64,7 @@ pub fn solve_directives( Ok(()) } - Directive::ToRadix { a, b, radix } => { + Directive::ToRadixLe { a, b, radix } => { let val_a = get_value(a, initial_witness)?; let a_big = BigUint::from_bytes_be(&val_a.to_be_bytes()); @@ -92,6 +92,34 @@ pub fn solve_directives( Ok(()) } + Directive::ToRadixBe { a, b, radix } => { + let val_a = get_value(a, initial_witness)?; + + let a_big = BigUint::from_bytes_be(&val_a.to_be_bytes()); + let a_dec = a_big.to_radix_be(*radix); + if b.len() < a_dec.len() { + return Err(OpcodeResolutionError::UnsatisfiedConstrain); + } + for i in 0..b.len() { + let v = if i < a_dec.len() { + FieldElement::from_be_bytes_reduce(&[a_dec[i]]) + } else { + FieldElement::zero() + }; + match initial_witness.entry(b[i]) { + std::collections::btree_map::Entry::Vacant(e) => { + e.insert(v); + } + std::collections::btree_map::Entry::Occupied(e) => { + if e.get() != &v { + return Err(OpcodeResolutionError::UnsatisfiedConstrain); + } + } + } + } + + Ok(()) + } Directive::OddRange { a, b, r, bit_size } => { let val_a = witness_to_value(initial_witness, *a)?; diff --git a/stdlib/src/fallback.rs b/stdlib/src/fallback.rs index 818c8b874..70447db36 100644 --- a/stdlib/src/fallback.rs +++ b/stdlib/src/fallback.rs @@ -38,7 +38,7 @@ pub(crate) fn bit_decomposition( } // Next create a directive which computes those bits. - new_gates.push(Opcode::Directive(Directive::ToRadix { + new_gates.push(Opcode::Directive(Directive::ToRadixLe { a: gate.clone(), b: bit_vector.clone(), radix: 2, From ab1a4c4e9b11fcfd376588b20df65c7133b3286a Mon Sep 17 00:00:00 2001 From: ethan-000 Date: Thu, 26 Jan 2023 23:11:00 +0000 Subject: [PATCH 02/19] change same code to one function --- acvm/src/pwg/directives.rs | 79 ++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 42 deletions(-) diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index 296824793..dffe0f4e9 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -69,56 +69,22 @@ pub fn solve_directives( let a_big = BigUint::from_bytes_be(&val_a.to_be_bytes()); let a_dec = a_big.to_radix_le(*radix); - if b.len() < a_dec.len() { - return Err(OpcodeResolutionError::UnsatisfiedConstrain); - } - for i in 0..b.len() { - let v = if i < a_dec.len() { - FieldElement::from_be_bytes_reduce(&[a_dec[i]]) - } else { - FieldElement::zero() - }; - match initial_witness.entry(b[i]) { - std::collections::btree_map::Entry::Vacant(e) => { - e.insert(v); - } - std::collections::btree_map::Entry::Occupied(e) => { - if e.get() != &v { - return Err(OpcodeResolutionError::UnsatisfiedConstrain); - } - } - } + let insertion_result = insert_to_radix(b, &a_dec, initial_witness); + match insertion_result { + Ok(()) => Ok(()), + Err(e) => Err(e), } - - Ok(()) } Directive::ToRadixBe { a, b, radix } => { let val_a = get_value(a, initial_witness)?; let a_big = BigUint::from_bytes_be(&val_a.to_be_bytes()); let a_dec = a_big.to_radix_be(*radix); - if b.len() < a_dec.len() { - return Err(OpcodeResolutionError::UnsatisfiedConstrain); + let insertion_result = insert_to_radix(b, &a_dec, initial_witness); + match insertion_result { + Ok(()) => Ok(()), + Err(e) => Err(e), } - for i in 0..b.len() { - let v = if i < a_dec.len() { - FieldElement::from_be_bytes_reduce(&[a_dec[i]]) - } else { - FieldElement::zero() - }; - match initial_witness.entry(b[i]) { - std::collections::btree_map::Entry::Vacant(e) => { - e.insert(v); - } - std::collections::btree_map::Entry::Occupied(e) => { - if e.get() != &v { - return Err(OpcodeResolutionError::UnsatisfiedConstrain); - } - } - } - } - - Ok(()) } Directive::OddRange { a, b, r, bit_size } => { let val_a = witness_to_value(initial_witness, *a)?; @@ -140,3 +106,32 @@ pub fn solve_directives( } } } + +fn insert_to_radix( + b: &Vec, + a_dec: &Vec, + initial_witness: &mut BTreeMap, +) -> Result<(), OpcodeResolutionError> { + if b.len() < a_dec.len() { + return Err(OpcodeResolutionError::UnsatisfiedConstrain); + } + for i in 0..b.len() { + let v = if i < a_dec.len() { + FieldElement::from_be_bytes_reduce(&[a_dec[i]]) + } else { + FieldElement::zero() + }; + match initial_witness.entry(b[i]) { + std::collections::btree_map::Entry::Vacant(e) => { + e.insert(v); + } + std::collections::btree_map::Entry::Occupied(e) => { + if e.get() != &v { + return Err(OpcodeResolutionError::UnsatisfiedConstrain); + } + } + } + } + + Ok(()) +} From 779dabdbdd45557c94643cc73c7adbc31e1c22e7 Mon Sep 17 00:00:00 2001 From: ethan-000 Date: Thu, 26 Jan 2023 23:17:11 +0000 Subject: [PATCH 03/19] could be cleaner --- acvm/src/pwg/directives.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index dffe0f4e9..203460359 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -69,8 +69,7 @@ pub fn solve_directives( let a_big = BigUint::from_bytes_be(&val_a.to_be_bytes()); let a_dec = a_big.to_radix_le(*radix); - let insertion_result = insert_to_radix(b, &a_dec, initial_witness); - match insertion_result { + match insert_to_radix(b, &a_dec, initial_witness) { Ok(()) => Ok(()), Err(e) => Err(e), } @@ -80,8 +79,7 @@ pub fn solve_directives( let a_big = BigUint::from_bytes_be(&val_a.to_be_bytes()); let a_dec = a_big.to_radix_be(*radix); - let insertion_result = insert_to_radix(b, &a_dec, initial_witness); - match insertion_result { + match insert_to_radix(b, &a_dec, initial_witness) { Ok(()) => Ok(()), Err(e) => Err(e), } From b879e743e666a756d0ff78eb18ec07f0ba4ca485 Mon Sep 17 00:00:00 2001 From: ethan-000 Date: Thu, 26 Jan 2023 23:31:17 +0000 Subject: [PATCH 04/19] change function name sightly --- acvm/src/pwg/directives.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index 203460359..86a65aa90 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -69,7 +69,7 @@ pub fn solve_directives( let a_big = BigUint::from_bytes_be(&val_a.to_be_bytes()); let a_dec = a_big.to_radix_le(*radix); - match insert_to_radix(b, &a_dec, initial_witness) { + match to_radix_outcome(b, &a_dec, initial_witness) { Ok(()) => Ok(()), Err(e) => Err(e), } @@ -79,7 +79,7 @@ pub fn solve_directives( let a_big = BigUint::from_bytes_be(&val_a.to_be_bytes()); let a_dec = a_big.to_radix_be(*radix); - match insert_to_radix(b, &a_dec, initial_witness) { + match to_radix_outcome(b, &a_dec, initial_witness) { Ok(()) => Ok(()), Err(e) => Err(e), } @@ -105,7 +105,7 @@ pub fn solve_directives( } } -fn insert_to_radix( +fn to_radix_outcome( b: &Vec, a_dec: &Vec, initial_witness: &mut BTreeMap, From ca41764020a6736d5377deca0e617d0955ad653a Mon Sep 17 00:00:00 2001 From: ethan-000 Date: Mon, 30 Jan 2023 19:57:16 +0000 Subject: [PATCH 05/19] change to one opcode --- acir/src/circuit/directives.rs | 63 ++++++++++++++-------------------- acir/src/circuit/opcodes.rs | 22 +++++------- acvm/src/pwg/directives.rs | 20 +++++------ stdlib/src/fallback.rs | 3 +- 4 files changed, 45 insertions(+), 63 deletions(-) diff --git a/acir/src/circuit/directives.rs b/acir/src/circuit/directives.rs index 558bfb6f4..a03fab08e 100644 --- a/acir/src/circuit/directives.rs +++ b/acir/src/circuit/directives.rs @@ -42,18 +42,12 @@ pub enum Directive { bit_size: u32, }, - //decomposition of a: a=\sum b[i]*radix^i where b is an array of witnesses < radix in little endian form - ToRadixLe { - a: Expression, - b: Vec, - radix: u32, - }, - - //decomposition of a: a=\sum b[i]*radix^i where b is an array of witnesses < radix in big endian form - ToRadixBe { + //decomposition of a: a=\sum b[i]*radix^i where b is an array of witnesses < radix in either little endian or big endian form + ToRadix { a: Expression, b: Vec, radix: u32, + is_little_endian: bool, }, } @@ -64,8 +58,7 @@ impl Directive { Directive::Quotient { .. } => "quotient", Directive::Truncate { .. } => "truncate", Directive::OddRange { .. } => "odd_range", - Directive::ToRadixLe { .. } => "to_radix_le", - Directive::ToRadixBe { .. } => "to_radix_be", + Directive::ToRadix { .. } => "to_radix", } } fn to_u16(&self) -> u16 { @@ -74,8 +67,7 @@ impl Directive { Directive::Quotient { .. } => 1, Directive::Truncate { .. } => 2, Directive::OddRange { .. } => 3, - Directive::ToRadixLe { .. } => 4, - Directive::ToRadixBe { .. } => 5, + Directive::ToRadix { .. } => 4, } } @@ -117,21 +109,19 @@ impl Directive { write_u32(&mut writer, r.witness_index())?; write_u32(&mut writer, *bit_size)?; } - Directive::ToRadixLe { a, b, radix } => { - a.write(&mut writer)?; - write_u32(&mut writer, b.len() as u32)?; - for bit in b { - write_u32(&mut writer, bit.witness_index())?; - } - write_u32(&mut writer, *radix)?; - } - Directive::ToRadixBe { a, b, radix } => { + Directive::ToRadix { + a, + b, + radix, + is_little_endian, + } => { a.write(&mut writer)?; write_u32(&mut writer, b.len() as u32)?; for bit in b { write_u32(&mut writer, bit.witness_index())?; } write_u32(&mut writer, *radix)?; + write_u32(&mut writer, *is_little_endian as u32)?; } }; @@ -192,21 +182,18 @@ impl Directive { } let radix = read_u32(&mut reader)?; - - Ok(Directive::ToRadixLe { a, b, radix }) - } - 5 => { - let a = Expression::read(&mut reader)?; - let b_len = read_u32(&mut reader)?; - let mut b = Vec::with_capacity(b_len as usize); - for _ in 0..b_len { - let witness = Witness(read_u32(&mut reader)?); - b.push(witness) + let endianess = read_u32(&mut reader)?; + let mut is_little_endian = true; + if endianess == 0 { + is_little_endian = false; } - let radix = read_u32(&mut reader)?; - - Ok(Directive::ToRadixBe { a, b, radix }) + Ok(Directive::ToRadix { + a, + b, + radix, + is_little_endian, + }) } _ => Err(std::io::ErrorKind::InvalidData.into()), @@ -258,16 +245,18 @@ fn serialisation_roundtrip() { bit_size: 32, }; - let to_radix_le = Directive::ToRadixLe { + let to_radix_le = Directive::ToRadix { a: Expression::default(), b: vec![Witness(1u32), Witness(2u32), Witness(3u32), Witness(4u32)], radix: 4, + is_little_endian: true, }; - let to_radix_be = Directive::ToRadixBe { + let to_radix_be = Directive::ToRadix { a: Expression::default(), b: vec![Witness(1u32), Witness(2u32), Witness(3u32), Witness(4u32)], radix: 4, + is_little_endian: false, }; let directives = vec![ diff --git a/acir/src/circuit/opcodes.rs b/acir/src/circuit/opcodes.rs index 5aa4053bc..4bc11dca0 100644 --- a/acir/src/circuit/opcodes.rs +++ b/acir/src/circuit/opcodes.rs @@ -152,28 +152,22 @@ impl std::fmt::Display for Opcode { ) } Opcode::BlackBoxFuncCall(g) => write!(f, "{g}"), - Opcode::Directive(Directive::ToRadixLe { a, b, radix: _ }) => { - write!(f, "DIR::TORADIXLE ")?; - write!( - f, - // TODO (Note): this assumes that the decomposed bits have contiguous witness indices - // This should be the case, however, we can also have a function which checks this - "(_{}, [_{}..._{}])", - a, - b.first().unwrap().witness_index(), - b.last().unwrap().witness_index(), - ) - } - Opcode::Directive(Directive::ToRadixBe { a, b, radix: _ }) => { + Opcode::Directive(Directive::ToRadix { + a, + b, + radix: _, + is_little_endian, + }) => { write!(f, "DIR::TORADIXLE ")?; write!( f, // TODO (Note): this assumes that the decomposed bits have contiguous witness indices // This should be the case, however, we can also have a function which checks this - "(_{}, [_{}..._{}])", + "(_{}, [_{}..._{}], endianess: {})", a, b.first().unwrap().witness_index(), b.last().unwrap().witness_index(), + if *is_little_endian { "little" } else { "big" } ) } } diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index 86a65aa90..fe1b0bd41 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -64,21 +64,19 @@ pub fn solve_directives( Ok(()) } - Directive::ToRadixLe { a, b, radix } => { + Directive::ToRadix { + a, + b, + radix, + is_little_endian, + } => { let val_a = get_value(a, initial_witness)?; let a_big = BigUint::from_bytes_be(&val_a.to_be_bytes()); - let a_dec = a_big.to_radix_le(*radix); - match to_radix_outcome(b, &a_dec, initial_witness) { - Ok(()) => Ok(()), - Err(e) => Err(e), + let mut a_dec = a_big.to_radix_be(*radix); + if *is_little_endian { + a_dec = a_big.to_radix_le(*radix); } - } - Directive::ToRadixBe { a, b, radix } => { - let val_a = get_value(a, initial_witness)?; - - let a_big = BigUint::from_bytes_be(&val_a.to_be_bytes()); - let a_dec = a_big.to_radix_be(*radix); match to_radix_outcome(b, &a_dec, initial_witness) { Ok(()) => Ok(()), Err(e) => Err(e), diff --git a/stdlib/src/fallback.rs b/stdlib/src/fallback.rs index 70447db36..5591ec736 100644 --- a/stdlib/src/fallback.rs +++ b/stdlib/src/fallback.rs @@ -38,10 +38,11 @@ pub(crate) fn bit_decomposition( } // Next create a directive which computes those bits. - new_gates.push(Opcode::Directive(Directive::ToRadixLe { + new_gates.push(Opcode::Directive(Directive::ToRadix { a: gate.clone(), b: bit_vector.clone(), radix: 2, + is_little_endian: true, })); // Now apply constraints to the bits such that they are the bit decomposition From 397eede64950abc64352accbda799ab49deb4d7c Mon Sep 17 00:00:00 2001 From: ethan-000 Date: Mon, 30 Jan 2023 19:58:23 +0000 Subject: [PATCH 06/19] change let a_dec --- acvm/src/pwg/directives.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index fe1b0bd41..33a4d967b 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -73,10 +73,11 @@ pub fn solve_directives( let val_a = get_value(a, initial_witness)?; let a_big = BigUint::from_bytes_be(&val_a.to_be_bytes()); - let mut a_dec = a_big.to_radix_be(*radix); - if *is_little_endian { - a_dec = a_big.to_radix_le(*radix); - } + let a_dec = if *is_little_endian { + a_big.to_radix_le(*radix) + } else { + a_big.to_radix_be(*radix) + }; match to_radix_outcome(b, &a_dec, initial_witness) { Ok(()) => Ok(()), Err(e) => Err(e), From ffe3f0e9e4ec10638df89b5703ca379c3e9279d7 Mon Sep 17 00:00:00 2001 From: ethan-000 Date: Mon, 30 Jan 2023 20:09:14 +0000 Subject: [PATCH 07/19] small refactor --- acir/src/circuit/directives.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/acir/src/circuit/directives.rs b/acir/src/circuit/directives.rs index a03fab08e..1b794030c 100644 --- a/acir/src/circuit/directives.rs +++ b/acir/src/circuit/directives.rs @@ -182,17 +182,13 @@ impl Directive { } let radix = read_u32(&mut reader)?; - let endianess = read_u32(&mut reader)?; - let mut is_little_endian = true; - if endianess == 0 { - is_little_endian = false; - } + let is_little_endian = read_u32(&mut reader)?; Ok(Directive::ToRadix { a, b, radix, - is_little_endian, + is_little_endian: if is_little_endian == 1 { true } else { false }, }) } From b4c848bca0245c81c421839c32489b378d6d475b Mon Sep 17 00:00:00 2001 From: Kevaundray Wedderburn Date: Sat, 4 Feb 2023 00:12:13 +0000 Subject: [PATCH 08/19] remove redundant if --- acir/src/circuit/directives.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acir/src/circuit/directives.rs b/acir/src/circuit/directives.rs index 1b794030c..d0138738c 100644 --- a/acir/src/circuit/directives.rs +++ b/acir/src/circuit/directives.rs @@ -188,7 +188,7 @@ impl Directive { a, b, radix, - is_little_endian: if is_little_endian == 1 { true } else { false }, + is_little_endian: is_little_endian == 1, }) } From 058077753ea2d9b088f5f83ca8d80df05a6aa76d Mon Sep 17 00:00:00 2001 From: Kevaundray Wedderburn Date: Sat, 4 Feb 2023 00:14:21 +0000 Subject: [PATCH 09/19] remove LE postfix --- acir/src/circuit/opcodes.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acir/src/circuit/opcodes.rs b/acir/src/circuit/opcodes.rs index 4bc11dca0..3176dcd59 100644 --- a/acir/src/circuit/opcodes.rs +++ b/acir/src/circuit/opcodes.rs @@ -158,7 +158,7 @@ impl std::fmt::Display for Opcode { radix: _, is_little_endian, }) => { - write!(f, "DIR::TORADIXLE ")?; + write!(f, "DIR::TORADIX ")?; write!( f, // TODO (Note): this assumes that the decomposed bits have contiguous witness indices From 057491f6cc57cf4d4c99f35084556e15f8cda24d Mon Sep 17 00:00:00 2001 From: Kevaundray Wedderburn Date: Sat, 4 Feb 2023 00:41:42 +0000 Subject: [PATCH 10/19] add `insert_value` method --- acvm/src/pwg.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/acvm/src/pwg.rs b/acvm/src/pwg.rs index 1be89ef19..83a161069 100644 --- a/acvm/src/pwg.rs +++ b/acvm/src/pwg.rs @@ -63,3 +63,27 @@ pub fn get_value( Ok(result) } + +// Inserts `value` into the initial witness map +// under the key of `witness`. +// Returns an error, if there was already a value in the map +// which does not match the value that one is about to insert +fn insert_value( + witness: &Witness, + value: FieldElement, + initial_witness: &mut BTreeMap, +) -> Result<(), OpcodeResolutionError> { + // First check if the value is present in the map + match initial_witness.entry(*witness) { + std::collections::btree_map::Entry::Vacant(e) => { + e.insert(value); + } + std::collections::btree_map::Entry::Occupied(e) => { + if e.get() != &value { + return Err(OpcodeResolutionError::UnsatisfiedConstrain); + } + } + } + + Ok(()) +} From fd8ef4da4dc8980b892edb3bd7a3431856b0db9a Mon Sep 17 00:00:00 2001 From: Kevaundray Wedderburn Date: Sat, 4 Feb 2023 00:44:59 +0000 Subject: [PATCH 11/19] refactor `insert_value` to usse `.insert` method --- acvm/src/pwg.rs | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/acvm/src/pwg.rs b/acvm/src/pwg.rs index 83a161069..9781b5fd0 100644 --- a/acvm/src/pwg.rs +++ b/acvm/src/pwg.rs @@ -70,19 +70,18 @@ pub fn get_value( // which does not match the value that one is about to insert fn insert_value( witness: &Witness, - value: FieldElement, + value_to_insert: FieldElement, initial_witness: &mut BTreeMap, ) -> Result<(), OpcodeResolutionError> { - // First check if the value is present in the map - match initial_witness.entry(*witness) { - std::collections::btree_map::Entry::Vacant(e) => { - e.insert(value); - } - std::collections::btree_map::Entry::Occupied(e) => { - if e.get() != &value { - return Err(OpcodeResolutionError::UnsatisfiedConstrain); - } - } + let optional_old_value = initial_witness.insert(*witness, value_to_insert); + + let old_value = match optional_old_value { + Some(old_value) => old_value, + None => return Ok(()), + }; + + if old_value != value_to_insert { + return Err(OpcodeResolutionError::UnsatisfiedConstrain); } Ok(()) From f21983c6e6b502aa70aec39fc58432e192a7087a Mon Sep 17 00:00:00 2001 From: Kevaundray Wedderburn Date: Sat, 4 Feb 2023 00:45:56 +0000 Subject: [PATCH 12/19] refactor `to_radix_outcome` --- acvm/src/pwg/directives.rs | 52 ++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 28 deletions(-) diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index 33a4d967b..60d95fe21 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -6,7 +6,7 @@ use num_traits::{One, Zero}; use crate::OpcodeResolutionError; -use super::{get_value, witness_to_value}; +use super::{get_value, insert_value, witness_to_value}; pub fn solve_directives( initial_witness: &mut BTreeMap, @@ -70,18 +70,18 @@ pub fn solve_directives( radix, is_little_endian, } => { - let val_a = get_value(a, initial_witness)?; + let value_a = get_value(a, initial_witness)?; + + let big_integer = BigUint::from_bytes_be(&value_a.to_be_bytes()); - let a_big = BigUint::from_bytes_be(&val_a.to_be_bytes()); - let a_dec = if *is_little_endian { - a_big.to_radix_le(*radix) + // Decompose the integer into its radix digits + let decomposed_integer = if *is_little_endian { + big_integer.to_radix_le(*radix) } else { - a_big.to_radix_be(*radix) + big_integer.to_radix_be(*radix) }; - match to_radix_outcome(b, &a_dec, initial_witness) { - Ok(()) => Ok(()), - Err(e) => Err(e), - } + + to_radix_outcome(b, decomposed_integer, initial_witness) } Directive::OddRange { a, b, r, bit_size } => { let val_a = witness_to_value(initial_witness, *a)?; @@ -105,29 +105,25 @@ pub fn solve_directives( } fn to_radix_outcome( - b: &Vec, - a_dec: &Vec, + witnesses: &Vec, + decomposed_integer: Vec, initial_witness: &mut BTreeMap, ) -> Result<(), OpcodeResolutionError> { - if b.len() < a_dec.len() { + if witnesses.len() < decomposed_integer.len() { return Err(OpcodeResolutionError::UnsatisfiedConstrain); } - for i in 0..b.len() { - let v = if i < a_dec.len() { - FieldElement::from_be_bytes_reduce(&[a_dec[i]]) - } else { - FieldElement::zero() + + for (i, witness) in witnesses.into_iter().enumerate() { + // Fetch the `i'th` digit from the decomposed integer list + // and convert it to a field element. + // If it is not available, which can happen when the decomposed integer + // list is shorter than the witness list, we return 0. + let value = match decomposed_integer.get(i) { + Some(digit) => FieldElement::from_be_bytes_reduce(&[*digit]), + None => FieldElement::zero(), }; - match initial_witness.entry(b[i]) { - std::collections::btree_map::Entry::Vacant(e) => { - e.insert(v); - } - std::collections::btree_map::Entry::Occupied(e) => { - if e.get() != &v { - return Err(OpcodeResolutionError::UnsatisfiedConstrain); - } - } - } + + insert_value(witness, value, initial_witness)? } Ok(()) From a8a5960c8ad83831dc489a888613bb5dea3e3ce2 Mon Sep 17 00:00:00 2001 From: Kevaundray Wedderburn Date: Sat, 4 Feb 2023 00:49:28 +0000 Subject: [PATCH 13/19] endianess -> endianness --- acir/src/circuit/opcodes.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acir/src/circuit/opcodes.rs b/acir/src/circuit/opcodes.rs index 3176dcd59..538fa1f4c 100644 --- a/acir/src/circuit/opcodes.rs +++ b/acir/src/circuit/opcodes.rs @@ -163,7 +163,7 @@ impl std::fmt::Display for Opcode { f, // TODO (Note): this assumes that the decomposed bits have contiguous witness indices // This should be the case, however, we can also have a function which checks this - "(_{}, [_{}..._{}], endianess: {})", + "(_{}, [_{}..._{}], endianness: {})", a, b.first().unwrap().witness_index(), b.last().unwrap().witness_index(), From 6d0732539e1fafa4972ad3d1dd16809375111f2d Mon Sep 17 00:00:00 2001 From: ethan-000 Date: Wed, 8 Feb 2023 14:12:01 +0000 Subject: [PATCH 14/19] fix big endian padding --- acvm/src/pwg/directives.rs | 44 +++++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index 60d95fe21..4a445b0eb 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -81,7 +81,7 @@ pub fn solve_directives( big_integer.to_radix_be(*radix) }; - to_radix_outcome(b, decomposed_integer, initial_witness) + to_radix_outcome(b, decomposed_integer, initial_witness, is_little_endian) } Directive::OddRange { a, b, r, bit_size } => { let val_a = witness_to_value(initial_witness, *a)?; @@ -108,22 +108,42 @@ fn to_radix_outcome( witnesses: &Vec, decomposed_integer: Vec, initial_witness: &mut BTreeMap, + is_little_endian: &bool, ) -> Result<(), OpcodeResolutionError> { if witnesses.len() < decomposed_integer.len() { return Err(OpcodeResolutionError::UnsatisfiedConstrain); } - for (i, witness) in witnesses.into_iter().enumerate() { - // Fetch the `i'th` digit from the decomposed integer list - // and convert it to a field element. - // If it is not available, which can happen when the decomposed integer - // list is shorter than the witness list, we return 0. - let value = match decomposed_integer.get(i) { - Some(digit) => FieldElement::from_be_bytes_reduce(&[*digit]), - None => FieldElement::zero(), - }; - - insert_value(witness, value, initial_witness)? + if *is_little_endian { + for (i, witness) in witnesses.into_iter().enumerate() { + // Fetch the `i'th` digit from the decomposed integer list + // and convert it to a field element. + // If it is not available, which can happen when the decomposed integer + // list is shorter than the witness list, we return 0. + let value = match decomposed_integer.get(i) { + Some(digit) => FieldElement::from_be_bytes_reduce(&[*digit]), + None => FieldElement::zero(), + }; + + insert_value(witness, value, initial_witness)? + } + } else { + // if it is big endian and the decompoased integer list is shorter + // than the witness list, pad the extra part with 0 first then + // add the decompsed interger list to the witness list. + let padding_len = witnesses.len() - decomposed_integer.len(); + for (i, witness) in witnesses.into_iter().enumerate() { + if i < padding_len { + insert_value(witness, FieldElement::zero(), initial_witness)? + } else { + let value = match decomposed_integer.get(i) { + Some(digit) => FieldElement::from_be_bytes_reduce(&[*digit]), + None => FieldElement::zero(), + }; + + insert_value(witness, value, initial_witness)? + } + } } Ok(()) From 876f0661dd4548ffaaf86ece6344c912003c1e5f Mon Sep 17 00:00:00 2001 From: ethan-000 Date: Wed, 8 Feb 2023 14:46:02 +0000 Subject: [PATCH 15/19] small fix --- acvm/src/pwg/directives.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index 4a445b0eb..1426f0cb4 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -136,7 +136,7 @@ fn to_radix_outcome( if i < padding_len { insert_value(witness, FieldElement::zero(), initial_witness)? } else { - let value = match decomposed_integer.get(i) { + let value = match decomposed_integer.get(i - padding_len) { Some(digit) => FieldElement::from_be_bytes_reduce(&[*digit]), None => FieldElement::zero(), }; From 2094bccfaffc9135565f485d241e87eef687c6f3 Mon Sep 17 00:00:00 2001 From: ethan-000 Date: Thu, 16 Feb 2023 13:40:30 +0000 Subject: [PATCH 16/19] delete function and add an error --- acvm/src/lib.rs | 2 + acvm/src/pwg/directives.rs | 94 +++++++++++++++++--------------------- 2 files changed, 45 insertions(+), 51 deletions(-) diff --git a/acvm/src/lib.rs b/acvm/src/lib.rs index c44fd9632..bed9c3e03 100644 --- a/acvm/src/lib.rs +++ b/acvm/src/lib.rs @@ -31,6 +31,8 @@ pub enum OpcodeNotSolvable { MissingAssignment(u32), #[error("expression has too many unknowns {0}")] ExpressionHasTooManyUnknowns(Expression), + #[error("compiler error: unreachable code")] + UnreachableCode, } #[derive(PartialEq, Eq, Debug, Error)] diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index 1426f0cb4..5778501cb 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -4,7 +4,7 @@ use acir::{circuit::directives::Directive, native_types::Witness, FieldElement}; use num_bigint::BigUint; use num_traits::{One, Zero}; -use crate::OpcodeResolutionError; +use crate::{OpcodeNotSolvable, OpcodeResolutionError}; use super::{get_value, insert_value, witness_to_value}; @@ -74,14 +74,51 @@ pub fn solve_directives( let big_integer = BigUint::from_bytes_be(&value_a.to_be_bytes()); - // Decompose the integer into its radix digits - let decomposed_integer = if *is_little_endian { - big_integer.to_radix_le(*radix) + if *is_little_endian { + // Decompose the integer into its radix digits in little endian form. + let decomposed_integer = big_integer.to_radix_le(*radix); + + if b.len() < decomposed_integer.len() { + return Err(OpcodeResolutionError::UnsatisfiedConstrain); + } + + for (i, witness) in b.into_iter().enumerate() { + // Fetch the `i'th` digit from the decomposed integer list + // and convert it to a field element. + // If it is not available, which can happen when the decomposed integer + // list is shorter than the witness list, we return 0. + let value = match decomposed_integer.get(i) { + Some(digit) => FieldElement::from_be_bytes_reduce(&[*digit]), + None => FieldElement::zero(), + }; + + insert_value(witness, value, initial_witness)? + } } else { - big_integer.to_radix_be(*radix) + // Decompose the integer into its radix digits in big endian form. + let decomposed_integer = big_integer.to_radix_be(*radix); + + // if it is big endian and the decompoased integer list is shorter + // than the witness list, pad the extra part with 0 first then + // add the decompsed interger list to the witness list. + let padding_len = b.len() - decomposed_integer.len(); + let mut value = FieldElement::zero(); + for (i, witness) in b.into_iter().enumerate() { + if i >= padding_len { + value = match decomposed_integer.get(i - padding_len) { + Some(digit) => FieldElement::from_be_bytes_reduce(&[*digit]), + None => { + return Err(OpcodeResolutionError::OpcodeNotSolvable( + OpcodeNotSolvable::UnreachableCode, + )) + } + }; + }; + insert_value(witness, value, initial_witness)? + } }; - to_radix_outcome(b, decomposed_integer, initial_witness, is_little_endian) + Ok(()) } Directive::OddRange { a, b, r, bit_size } => { let val_a = witness_to_value(initial_witness, *a)?; @@ -103,48 +140,3 @@ pub fn solve_directives( } } } - -fn to_radix_outcome( - witnesses: &Vec, - decomposed_integer: Vec, - initial_witness: &mut BTreeMap, - is_little_endian: &bool, -) -> Result<(), OpcodeResolutionError> { - if witnesses.len() < decomposed_integer.len() { - return Err(OpcodeResolutionError::UnsatisfiedConstrain); - } - - if *is_little_endian { - for (i, witness) in witnesses.into_iter().enumerate() { - // Fetch the `i'th` digit from the decomposed integer list - // and convert it to a field element. - // If it is not available, which can happen when the decomposed integer - // list is shorter than the witness list, we return 0. - let value = match decomposed_integer.get(i) { - Some(digit) => FieldElement::from_be_bytes_reduce(&[*digit]), - None => FieldElement::zero(), - }; - - insert_value(witness, value, initial_witness)? - } - } else { - // if it is big endian and the decompoased integer list is shorter - // than the witness list, pad the extra part with 0 first then - // add the decompsed interger list to the witness list. - let padding_len = witnesses.len() - decomposed_integer.len(); - for (i, witness) in witnesses.into_iter().enumerate() { - if i < padding_len { - insert_value(witness, FieldElement::zero(), initial_witness)? - } else { - let value = match decomposed_integer.get(i - padding_len) { - Some(digit) => FieldElement::from_be_bytes_reduce(&[*digit]), - None => FieldElement::zero(), - }; - - insert_value(witness, value, initial_witness)? - } - } - } - - Ok(()) -} From b49db0c69404cce02ebc7b5a7065e55474105077 Mon Sep 17 00:00:00 2001 From: ethan-000 Date: Thu, 16 Feb 2023 17:17:46 +0000 Subject: [PATCH 17/19] remove semicolons --- acvm/src/pwg/directives.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index 5778501cb..176148be9 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -113,10 +113,10 @@ pub fn solve_directives( )) } }; - }; + } insert_value(witness, value, initial_witness)? } - }; + } Ok(()) } From feebc5aeec0b93aacc3143ebda228bfe158fd132 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Thu, 16 Feb 2023 21:08:57 +0000 Subject: [PATCH 18/19] Merge conflict --- acvm/src/pwg/directives.rs | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index 17ef16b22..123b7e0d4 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -10,8 +10,7 @@ use num_traits::{One, Zero}; use crate::{OpcodeNotSolvable, OpcodeResolutionError}; -use super::{get_value, sorting::route, insert_value,witness_to_value}; - +use super::{get_value, insert_value, sorting::route, witness_to_value}; pub fn solve_directives( initial_witness: &mut BTreeMap, @@ -137,18 +136,6 @@ pub fn solve_directives( } insert_value(witness, value, initial_witness)? } - let a_big = BigUint::from_bytes_be(&val_a.to_be_bytes()); - let a_dec = a_big.to_radix_le(*radix); - if b.len() < a_dec.len() { - return Err(OpcodeResolutionError::UnsatisfiedConstrain); - } - for i in 0..b.len() { - let v = if i < a_dec.len() { - FieldElement::from_be_bytes_reduce(&[a_dec[i]]) - } else { - FieldElement::zero() - }; - insert_witness(b[i], v, initial_witness)?; } Ok(()) From f8e3f865e964638b6eccd25dcb02cb534461133d Mon Sep 17 00:00:00 2001 From: kevaundray Date: Thu, 16 Feb 2023 21:13:04 +0000 Subject: [PATCH 19/19] fix clippy --- acvm/src/pwg/directives.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/acvm/src/pwg/directives.rs b/acvm/src/pwg/directives.rs index 123b7e0d4..d2b9a33e9 100644 --- a/acvm/src/pwg/directives.rs +++ b/acvm/src/pwg/directives.rs @@ -102,7 +102,7 @@ pub fn solve_directives( return Err(OpcodeResolutionError::UnsatisfiedConstrain); } - for (i, witness) in b.into_iter().enumerate() { + for (i, witness) in b.iter().enumerate() { // Fetch the `i'th` digit from the decomposed integer list // and convert it to a field element. // If it is not available, which can happen when the decomposed integer @@ -123,7 +123,7 @@ pub fn solve_directives( // add the decompsed interger list to the witness list. let padding_len = b.len() - decomposed_integer.len(); let mut value = FieldElement::zero(); - for (i, witness) in b.into_iter().enumerate() { + for (i, witness) in b.iter().enumerate() { if i >= padding_len { value = match decomposed_integer.get(i - padding_len) { Some(digit) => FieldElement::from_be_bytes_reduce(&[*digit]),