Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add assertions for ACVM FunctionInput bit_size #5864

Merged
merged 31 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
4c7cf89
feat: wip ensuring values and function types match ACVM FunctionInput…
michaeljklein Aug 29, 2024
fba8b77
Merge branch 'master' into michaeljklein/acvm-check-bitsize
michaeljklein Sep 3, 2024
575c2f6
wip debugging regression_4080, add error for InvalidInputBitSize, thr…
michaeljklein Sep 4, 2024
0021a68
ensure that 'should_fail_with' in noir tests matches against the erro…
michaeljklein Sep 4, 2024
252600f
Merge branch 'master' into michaeljklein/acvm-check-bitsize
michaeljklein Sep 4, 2024
d5ebc49
cargo clippy/fmt
michaeljklein Sep 4, 2024
c38bdf4
Merge branch 'master' into michaeljklein/acvm-check-bitsize
michaeljklein Sep 4, 2024
07f3e69
nargo fmt
michaeljklein Sep 4, 2024
adfe490
patch acvm_js test to use inputs < 2^128 since the FunctionInputs use…
michaeljklein Sep 4, 2024
6606cf0
another attempt to get the bit size within the expected range
michaeljklein Sep 4, 2024
30c7a70
Merge branch 'master' into michaeljklein/acvm-check-bitsize
michaeljklein Sep 4, 2024
dd645b0
update expected bit size for multi-scalar-mul TS tests to maximum
michaeljklein Sep 5, 2024
390c0fa
Merge branch 'master' into michaeljklein/acvm-check-bitsize
michaeljklein Sep 5, 2024
2c18940
cargo fmt / yarn lint
michaeljklein Sep 5, 2024
d6ba7c3
using only the value + AcirField for InvalidInputBitSize
michaeljklein Sep 5, 2024
3ae49c4
propagate RuntimeError <F> parameter
michaeljklein Sep 5, 2024
9dea841
responding to review comments
michaeljklein Sep 5, 2024
7a13c56
Merge branch 'master' into michaeljklein/acvm-check-bitsize
michaeljklein Sep 6, 2024
59a0e13
refactoring matches and updating test for overflow
michaeljklein Sep 6, 2024
c046b73
Merge branch 'master' into michaeljklein/acvm-check-bitsize
michaeljklein Sep 9, 2024
92f936a
removed <F:AcirField> parameter from invalid bit size error, disable …
michaeljklein Sep 9, 2024
7285ef3
Merge branch 'master' into michaeljklein/acvm-check-bitsize
michaeljklein Sep 9, 2024
821b2d4
add 'bitsize' to cspell.json, replace usage of InvalidInputBitSize wi…
michaeljklein Sep 9, 2024
52eabc5
split UnsupportedIntegerSize into self and InvalidBlackBoxInputBitSize
michaeljklein Sep 10, 2024
d9c1c7f
Merge branch 'master' into michaeljklein/acvm-check-bitsize
michaeljklein Sep 10, 2024
010df44
chore: remove unnecessary trait constraints
TomAFrench Sep 11, 2024
b9d52c7
.
TomAFrench Sep 11, 2024
404c838
.
TomAFrench Sep 11, 2024
e92a509
Update acvm-repo/acvm/src/pwg/mod.rs
TomAFrench Sep 11, 2024
136b48c
chore: remove extra `From` impls
TomAFrench Sep 11, 2024
876d543
.
TomAFrench Sep 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions acvm-repo/acir/src/circuit/opcodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
mod black_box_function_call;
mod memory_operation;

pub use black_box_function_call::{BlackBoxFuncCall, ConstantOrWitnessEnum, FunctionInput};
pub use black_box_function_call::{
BlackBoxFuncCall, ConstantOrWitnessEnum, FunctionInput, InvalidInputBitSize,
};
pub use memory_operation::{BlockId, MemOp};

#[derive(Clone, PartialEq, Eq, Serialize, Deserialize)]
Expand All @@ -40,7 +42,7 @@
/// values which define the opcode.
///
/// A general expression of assert-zero opcode is the following:
/// ```
/// ```text
/// \sum_{i,j} {q_M}_{i,j}w_iw_j + \sum_i q_iw_i +q_c = 0
/// ```
///
Expand Down Expand Up @@ -157,7 +159,7 @@

Opcode::BlackBoxFuncCall(g) => write!(f, "{g}"),
Opcode::Directive(Directive::ToLeRadix { a, b, radix: _ }) => {
write!(f, "DIR::TORADIX ")?;

Check warning on line 162 in acvm-repo/acir/src/circuit/opcodes.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (TORADIX)
write!(
f,
// TODO (Note): this assumes that the decomposed bits have contiguous witness indices
Expand Down Expand Up @@ -188,7 +190,7 @@
match databus {
BlockType::Memory => write!(f, "INIT ")?,
BlockType::CallData(id) => write!(f, "INIT CALLDATA {} ", id)?,
BlockType::ReturnData => write!(f, "INIT RETURNDATA ")?,

Check warning on line 193 in acvm-repo/acir/src/circuit/opcodes.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (RETURNDATA)
}
write!(f, "(id: {}, len: {}) ", block_id.0, init.len())
}
Expand Down
36 changes: 31 additions & 5 deletions acvm-repo/acir/src/circuit/opcodes/black_box_function_call.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use crate::native_types::Witness;
use crate::BlackBoxFunc;
use crate::{AcirField, BlackBoxFunc};

use serde::{Deserialize, Deserializer, Serialize, Serializer};
use thiserror::Error;

// Note: Some functions will not use all of the witness
// So we need to supply how many bits of the witness is needed
Expand All @@ -13,8 +15,8 @@ pub enum ConstantOrWitnessEnum<F> {

#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)]
pub struct FunctionInput<F> {
pub input: ConstantOrWitnessEnum<F>,
pub num_bits: u32,
input: ConstantOrWitnessEnum<F>,
num_bits: u32,
}

impl<F> FunctionInput<F> {
Expand All @@ -25,16 +27,40 @@ impl<F> FunctionInput<F> {
}
}

pub fn input(self) -> ConstantOrWitnessEnum<F> {
self.input
}

pub fn input_ref(&self) -> &ConstantOrWitnessEnum<F> {
&self.input
}

pub fn num_bits(&self) -> u32 {
self.num_bits
}

pub fn witness(witness: Witness, num_bits: u32) -> FunctionInput<F> {
FunctionInput { input: ConstantOrWitnessEnum::Witness(witness), num_bits }
}
}

#[derive(Clone, PartialEq, Eq, Debug, Error)]
#[error("FunctionInput value has too many bits: value: {value}, {} >= {max_bits}", value.num_bits())]
pub struct InvalidInputBitSize<F: AcirField> {
pub value: F,
pub max_bits: u32,
}

pub fn constant(value: F, num_bits: u32) -> FunctionInput<F> {
FunctionInput { input: ConstantOrWitnessEnum::Constant(value), num_bits }
impl<F: AcirField> FunctionInput<F> {
pub fn constant(value: F, max_bits: u32) -> Result<FunctionInput<F>, InvalidInputBitSize<F>> {
if value.num_bits() <= max_bits {
Ok(FunctionInput { input: ConstantOrWitnessEnum::Constant(value), num_bits: max_bits })
} else {
Err(InvalidInputBitSize {
value,
max_bits,
})
}
}
}

Expand Down
1 change: 1 addition & 0 deletions acvm-repo/acir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,13 @@
pub use acir_field::{AcirField, FieldElement};
pub use brillig;
pub use circuit::black_box_functions::BlackBoxFunc;
pub use circuit::opcodes::InvalidInputBitSize;

#[cfg(test)]
mod reflection {
//! Getting test failures? You've probably changed the ACIR serialization format.
//!
//! These tests generate C++ deserializers for [`ACIR bytecode`][super::circuit::Circuit]

Check warning on line 21 in acvm-repo/acir/src/lib.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (deserializers)
//! and the [`WitnessMap`] structs. These get checked against the C++ files committed to the `codegen` folder
//! to see if changes have been to the serialization format. These are almost always a breaking change!
//!
Expand Down
16 changes: 8 additions & 8 deletions acvm-repo/acir/tests/test_program_serialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,13 @@ fn multi_scalar_mul_circuit() {
let multi_scalar_mul: Opcode<FieldElement> =
Opcode::BlackBoxFuncCall(BlackBoxFuncCall::MultiScalarMul {
points: vec![
FunctionInput::witness(Witness(1), 128),
FunctionInput::witness(Witness(2), 128),
FunctionInput::witness(Witness(1), FieldElement::max_num_bits()),
FunctionInput::witness(Witness(2), FieldElement::max_num_bits()),
FunctionInput::witness(Witness(3), 1),
],
scalars: vec![
FunctionInput::witness(Witness(4), 128),
FunctionInput::witness(Witness(5), 128),
FunctionInput::witness(Witness(4), FieldElement::max_num_bits()),
FunctionInput::witness(Witness(5), FieldElement::max_num_bits()),
],
outputs: (Witness(6), Witness(7), Witness(8)),
});
Expand All @@ -91,10 +91,10 @@ fn multi_scalar_mul_circuit() {
let bytes = Program::serialize_program(&program);

let expected_serialization: Vec<u8> = vec![
31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 93, 141, 11, 10, 0, 32, 8, 67, 43, 181, 15, 116, 232,
142, 158, 210, 130, 149, 240, 112, 234, 212, 156, 78, 12, 39, 67, 71, 158, 142, 80, 29, 44,
228, 66, 90, 168, 119, 189, 74, 115, 131, 174, 78, 115, 58, 124, 70, 254, 130, 59, 74, 253,
68, 255, 255, 221, 39, 54, 221, 93, 91, 132, 193, 0, 0, 0,
31, 139, 8, 0, 0, 0, 0, 0, 0, 255, 93, 141, 11, 10, 0, 32, 8, 67, 43, 181, 15, 116, 255,
227, 70, 74, 11, 86, 194, 195, 169, 83, 115, 58, 49, 156, 12, 29, 121, 58, 66, 117, 176,
144, 11, 105, 161, 222, 245, 42, 205, 13, 186, 58, 205, 233, 240, 25, 249, 11, 238, 40,
245, 19, 253, 255, 119, 159, 216, 103, 157, 249, 169, 193, 0, 0, 0,
];

assert_eq!(bytes, expected_serialization)
Expand Down
12 changes: 12 additions & 0 deletions acvm-repo/acir_field/src/field_element.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,18 @@ impl<'de, T: PrimeField> Deserialize<'de> for FieldElement<T> {
}
}

impl<F: PrimeField> From<BigUint> for FieldElement<F> {
fn from(a: BigUint) -> FieldElement<F> {
FieldElement(F::from(a))
}
}

impl<F: PrimeField> From<FieldElement<F>> for BigUint {
fn from(a: FieldElement<F>) -> BigUint {
F::into(a.0)
}
}

impl<F: PrimeField> From<u128> for FieldElement<F> {
fn from(a: u128) -> FieldElement<F> {
FieldElement(F::from(a))
Expand Down
26 changes: 17 additions & 9 deletions acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use acir::{
circuit::{
opcodes::{BlackBoxFuncCall, ConstantOrWitnessEnum, FunctionInput},
opcodes::{BlackBoxFuncCall, ConstantOrWitnessEnum},
Circuit, Opcode,
},
native_types::Witness,
Expand Down Expand Up @@ -73,10 +73,13 @@ impl<F: AcirField> RangeOptimizer<F> {
}
}

Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE {
input:
FunctionInput { input: ConstantOrWitnessEnum::Witness(witness), num_bits },
}) => Some((*witness, *num_bits)),
Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE { input }) => {
if let ConstantOrWitnessEnum::Witness(witness) = input.input() {
Some((witness, input.num_bits()))
} else {
None
}
}

_ => None,
}) else {
Expand Down Expand Up @@ -107,10 +110,15 @@ impl<F: AcirField> RangeOptimizer<F> {
let mut optimized_opcodes = Vec::with_capacity(self.circuit.opcodes.len());
for (idx, opcode) in self.circuit.opcodes.into_iter().enumerate() {
let (witness, num_bits) = match opcode {
Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE {
input:
FunctionInput { input: ConstantOrWitnessEnum::Witness(w), num_bits: bits },
}) => (w, bits),
Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE { input })
if matches!(input.input_ref(), ConstantOrWitnessEnum::Witness(..)) =>
{
if let ConstantOrWitnessEnum::Witness(witness) = input.input() {
(witness, input.num_bits())
} else {
unreachable!("The matches! above ensures only a witness is possible")
}
}
_ => {
// If its not the range opcode, add it to the opcode
// list and continue;
Expand Down
2 changes: 1 addition & 1 deletion acvm-repo/acvm/src/pwg/blackbox/bigint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ impl AcvmBigIntSolver {
Ok(())
}

pub(crate) fn bigint_op<F>(
pub(crate) fn bigint_op<F: AcirField>(
&mut self,
lhs: u32,
rhs: u32,
Expand Down
6 changes: 3 additions & 3 deletions acvm-repo/acvm/src/pwg/blackbox/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
use pedersen::{pedersen, pedersen_hash};
pub(crate) use range::solve_range_opcode;
use signature::{
ecdsa::{secp256k1_prehashed, secp256r1_prehashed},

Check warning on line 33 in acvm-repo/acvm/src/pwg/blackbox/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (prehashed)

Check warning on line 33 in acvm-repo/acvm/src/pwg/blackbox/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (prehashed)
schnorr::schnorr_verify,
};

Expand All @@ -42,11 +42,11 @@
inputs: &[FunctionInput<F>],
) -> Option<Witness> {
inputs.iter().find_map(|input| {
if let ConstantOrWitnessEnum::Witness(witness) = input.input {
if witness_assignments.contains_key(&witness) {
if let ConstantOrWitnessEnum::Witness(ref witness) = input.input_ref() {
if witness_assignments.contains_key(witness) {
None
} else {
Some(witness)
Some(*witness)
}
} else {
None
Expand Down Expand Up @@ -145,7 +145,7 @@
signature,
hashed_message: message,
output,
} => secp256k1_prehashed(

Check warning on line 148 in acvm-repo/acvm/src/pwg/blackbox/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (prehashed)
initial_witness,
public_key_x,
public_key_y,
Expand All @@ -159,7 +159,7 @@
signature,
hashed_message: message,
output,
} => secp256r1_prehashed(

Check warning on line 162 in acvm-repo/acvm/src/pwg/blackbox/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (prehashed)
initial_witness,
public_key_x,
public_key_y,
Expand Down
57 changes: 47 additions & 10 deletions acvm-repo/acvm/src/pwg/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
brillig::ForeignCallResult,
circuit::{
brillig::{BrilligBytecode, BrilligFunctionId},
opcodes::{AcirFunctionId, BlockId, ConstantOrWitnessEnum, FunctionInput},
opcodes::{
AcirFunctionId, BlockId, ConstantOrWitnessEnum, FunctionInput, InvalidInputBitSize,
},
AssertionPayload, ErrorSelector, ExpressionOrMemory, Opcode, OpcodeLocation,
RawAssertionPayload, ResolvedAssertionPayload, STRING_ERROR_SELECTOR,
},
Expand Down Expand Up @@ -37,7 +39,7 @@
pub use brillig::ForeignCallWaitInfo;

#[derive(Debug, Clone, PartialEq)]
pub enum ACVMStatus<F> {
pub enum ACVMStatus<F: AcirField> {
/// All opcodes have been solved.
Solved,

Expand All @@ -56,13 +58,13 @@
RequiresForeignCall(ForeignCallWaitInfo<F>),

/// The ACVM has encountered a request for an ACIR [call][acir::circuit::Opcode]
/// to execute a separate ACVM instance. The result of the ACIR call must be passd back to the ACVM.

Check warning on line 61 in acvm-repo/acvm/src/pwg/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (passd)
///
/// Once this is done, the ACVM can be restarted to solve the remaining opcodes.
RequiresAcirCall(AcirCallWaitInfo<F>),
}

impl<F> std::fmt::Display for ACVMStatus<F> {
impl<F: AcirField> std::fmt::Display for ACVMStatus<F> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
ACVMStatus::Solved => write!(f, "Solved"),
Expand All @@ -74,7 +76,7 @@
}
}

pub enum StepResult<'a, F, B: BlackBoxFunctionSolver<F>> {
pub enum StepResult<'a, F: AcirField, B: BlackBoxFunctionSolver<F>> {
Status(ACVMStatus<F>),
IntoBrillig(BrilligSolver<'a, F, B>),
}
Expand Down Expand Up @@ -118,7 +120,7 @@
}

#[derive(Clone, PartialEq, Eq, Debug, Error)]
pub enum OpcodeResolutionError<F> {
pub enum OpcodeResolutionError<F: AcirField> {
#[error("Cannot solve opcode: {0}")]
OpcodeNotSolvable(#[from] OpcodeNotSolvable<F>),
#[error("Cannot satisfy constraint")]
Expand All @@ -128,6 +130,11 @@
},
#[error("Index out of bounds, array has size {array_size:?}, but index was {index:?}")]
IndexOutOfBounds { opcode_location: ErrorLocation, index: u32, array_size: u32 },
#[error("Cannot solve opcode: {invalid_input_bit_size}")]
InvalidInputBitSize {
opcode_location: ErrorLocation,
invalid_input_bit_size: InvalidInputBitSize<F>,
},
#[error("Failed to solve blackbox function: {0}, reason: {1}")]
BlackBoxFunctionFailed(BlackBoxFunc, String),
#[error("Failed to solve brillig function")]
Expand All @@ -142,7 +149,7 @@
AcirCallOutputsMismatch { opcode_location: ErrorLocation, results_size: u32, outputs_size: u32 },
}

impl<F> From<BlackBoxResolutionError> for OpcodeResolutionError<F> {
impl<F: AcirField> From<BlackBoxResolutionError> for OpcodeResolutionError<F> {
fn from(value: BlackBoxResolutionError) -> Self {
match value {
BlackBoxResolutionError::Failed(func, reason) => {
Expand All @@ -152,7 +159,16 @@
}
}

pub struct ACVM<'a, F, B: BlackBoxFunctionSolver<F>> {
impl<F: AcirField> From<InvalidInputBitSize<F>> for OpcodeResolutionError<F> {
fn from(invalid_input_bit_size: InvalidInputBitSize<F>) -> Self {
Self::InvalidInputBitSize {
opcode_location: ErrorLocation::Unresolved,
invalid_input_bit_size,
}
}
}

pub struct ACVM<'a, F: AcirField, B: BlackBoxFunctionSolver<F>> {
status: ACVMStatus<F>,

backend: &'a B,
Expand Down Expand Up @@ -387,6 +403,13 @@
*opcode_index = ErrorLocation::Resolved(location);
*assertion_payload = self.extract_assertion_payload(location);
}
OpcodeResolutionError::InvalidInputBitSize {
opcode_location: opcode_index,
..
} => {
let location = OpcodeLocation::Acir(self.instruction_pointer());
*opcode_index = ErrorLocation::Resolved(location);
}
// All other errors are thrown normally.
_ => (),
};
Expand Down Expand Up @@ -623,7 +646,7 @@
// Returns the concrete value for a particular witness
// If the witness has no assignment, then
// an error is returned
pub fn witness_to_value<F>(
pub fn witness_to_value<F: AcirField>(
initial_witness: &WitnessMap<F>,
witness: Witness,
) -> Result<&F, OpcodeResolutionError<F>> {
Expand All @@ -637,8 +660,22 @@
initial_witness: &WitnessMap<F>,
input: FunctionInput<F>,
) -> Result<F, OpcodeResolutionError<F>> {
match input.input {
ConstantOrWitnessEnum::Witness(witness) => Ok(*witness_to_value(initial_witness, witness)?),
match input.input() {
ConstantOrWitnessEnum::Witness(witness) => {
let initial_value = *witness_to_value(initial_witness, witness)?;
if initial_value.num_bits() <= input.num_bits() {
Ok(initial_value)
} else {
Err(OpcodeResolutionError::InvalidInputBitSize {
opcode_location: ErrorLocation::Unresolved,

invalid_input_bit_size: InvalidInputBitSize {
value: initial_value,
max_bits: input.num_bits(),
},
})
}
}
ConstantOrWitnessEnum::Constant(value) => Ok(value),
}
}
Expand Down
Loading
Loading