Skip to content

Commit

Permalink
replace BrilligFunctionFailed.found_trap to .reason
Browse files Browse the repository at this point in the history 
 * Abstract `extract_failure_payload` function for improving readability
  • Loading branch information
@anaPerezGhiglia
anaPerezGhiglia committed Jun 28, 2024
1 parent 54c45a3 commit 58e8fb4
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 57 deletions.
103 changes: 53 additions & 50 deletions acvm-repo/acvm/src/pwg/brillig.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,59 +169,11 @@ impl<'b, B: BlackBoxFunctionSolver<F>, F: AcirField> BrilligSolver<'b, F, B> {
brillig_index: *brillig_index,
})
.collect();
let (payload, found_trap) = match reason {
FailureReason::RuntimeError { message } => {
(Some(ResolvedAssertionPayload::String(message)), false)
}

FailureReason::Trap { revert_data_offset, revert_data_size } => {
// Since noir can only revert with strings currently, we can parse return data as a string
let payload = if revert_data_size == 0 {
None
} else {
let memory = self.vm.get_memory();
let mut revert_values_iter = memory
[revert_data_offset..(revert_data_offset + revert_data_size)]
.iter();
let error_selector = ErrorSelector::new(
revert_values_iter
.next()
.expect("Incorrect revert data size")
.try_into()
.expect("Error selector is not u64"),
);

match error_selector {
STRING_ERROR_SELECTOR => {
// If the error selector is 0, it means the error is a string
let string = revert_values_iter
.map(|memory_value| {
let as_u8: u8 = memory_value
.try_into()
.expect("String item is not u8");
as_u8 as char
})
.collect();
Some(ResolvedAssertionPayload::String(string))
}
_ => {
// If the error selector is not 0, it means the error is a custom error
Some(ResolvedAssertionPayload::Raw(RawAssertionPayload {
selector: error_selector,
data: revert_values_iter
.map(|value| value.to_field())
.collect(),
}))
}
}
};
(payload, true)
}
};
Err(OpcodeResolutionError::BrilligFunctionFailed {
payload,
payload: self.extract_failure_payload(&reason),
call_stack,
found_trap,
reason,
})
}
VMStatus::ForeignCallWait { function, inputs } => {
Expand All @@ -230,6 +182,57 @@ impl<'b, B: BlackBoxFunctionSolver<F>, F: AcirField> BrilligSolver<'b, F, B> {
}
}

fn extract_failure_payload(
&self,
reason: &FailureReason,
) -> Option<ResolvedAssertionPayload<F>> {
match reason {
FailureReason::RuntimeError { message } => {
Some(ResolvedAssertionPayload::String(message.clone()))
}
FailureReason::Trap { revert_data_offset, revert_data_size } => {
// Since noir can only revert with strings currently, we can parse return data as a string
let payload = if *revert_data_size == 0 {
None
} else {
let memory = self.vm.get_memory();
let mut revert_values_iter = memory
[*revert_data_offset..(*revert_data_offset + *revert_data_size)]
.iter();
let error_selector = ErrorSelector::new(
revert_values_iter
.next()
.expect("Incorrect revert data size")
.try_into()
.expect("Error selector is not u64"),
);

match error_selector {
STRING_ERROR_SELECTOR => {
// If the error selector is 0, it means the error is a string
let string = revert_values_iter
.map(|memory_value| {
let as_u8: u8 =
memory_value.try_into().expect("String item is not u8");
as_u8 as char
})
.collect();
Some(ResolvedAssertionPayload::String(string))
}
_ => {
// If the error selector is not 0, it means the error is a custom error
Some(ResolvedAssertionPayload::Raw(RawAssertionPayload {
selector: error_selector,
data: revert_values_iter.map(|value| value.to_field()).collect(),
}))
}
}
};
payload
}
}
}

pub(crate) fn finalize(
self,
witness: &mut WitnessMap<F>,
Expand Down
7 changes: 4 additions & 3 deletions acvm-repo/acvm/src/pwg/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use acir::{
AcirField, BlackBoxFunc,
};
use acvm_blackbox_solver::BlackBoxResolutionError;
use brillig_vm::FailureReason;

use self::{
arithmetic::ExpressionSolver, blackbox::bigint::AcvmBigIntSolver, directives::solve_directives,
Expand Down Expand Up @@ -132,7 +133,7 @@ pub enum OpcodeResolutionError<F> {
BrilligFunctionFailed {
call_stack: Vec<OpcodeLocation>,
payload: Option<ResolvedAssertionPayload<F>>,
found_trap: bool,
reason: FailureReason,
},
AcirMainCallAttempted {
opcode_location: ErrorLocation,
Expand All @@ -151,8 +152,8 @@ impl<F: acir::AcirField> std::fmt::Display for OpcodeResolutionError<F> {
OpcodeResolutionError::UnsatisfiedConstrain { .. } => write!(f, "Cannot satisfy constraint"),
OpcodeResolutionError::IndexOutOfBounds { array_size, index, .. } => write!(f, "Index out of bounds, array has size {}, but index was {}", array_size, index),
OpcodeResolutionError::BlackBoxFunctionFailed(func, reason) => write!(f,"Failed to solve blackbox function: {}, reason: {}", func, reason),
OpcodeResolutionError::BrilligFunctionFailed{found_trap: false, .. } => write!(f, "Failed to solve brillig function"),
OpcodeResolutionError::BrilligFunctionFailed{found_trap: true, .. } => write!(f, "Cannot satisfy constraint"),
OpcodeResolutionError::BrilligFunctionFailed{reason: FailureReason::Trap {.. }, .. } => write!(f, "Cannot satisfy constraint"),
OpcodeResolutionError::BrilligFunctionFailed{ .. } => write!(f, "Failed to solve brillig function"),
OpcodeResolutionError::AcirMainCallAttempted{ .. } => write!(f, "Attempted to call `main` with a `Call` opcode"),
OpcodeResolutionError::AcirCallOutputsMismatch {results_size, outputs_size, .. } => write!(f, "{} result values were provided for {} call output witnesses, most likely due to bad ACIR codegen", results_size, outputs_size),
}
Expand Down
4 changes: 2 additions & 2 deletions acvm-repo/acvm/tests/solver.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use acir::{

use acvm::pwg::{ACVMStatus, ErrorLocation, ForeignCallWaitInfo, OpcodeResolutionError, ACVM};
use acvm_blackbox_solver::StubbedBlackBoxSolver;
use brillig_vm::brillig::HeapValueType;
use brillig_vm::{brillig::HeapValueType, FailureReason};

// Reenable these test cases once we move the brillig implementation of inversion down into the acvm stdlib.

Expand Down Expand Up @@ -673,7 +673,7 @@ fn unsatisfied_opcode_resolved_brillig() {
ACVMStatus::Failure(OpcodeResolutionError::BrilligFunctionFailed {
payload: None,
call_stack: vec![OpcodeLocation::Brillig { acir_index: 0, brillig_index: 3 }],
found_trap: true,
reason: FailureReason::Trap { revert_data_offset: 0, revert_data_size: 0 },
}),
"The first opcode is not satisfiable, expected an error indicating this"
);
Expand Down
8 changes: 6 additions & 2 deletions tooling/debugger/src/context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::foreign_calls::DebugForeignCallExecutor;
use acvm::acir::circuit::brillig::BrilligBytecode;
use acvm::acir::circuit::{Circuit, Opcode, OpcodeLocation};
use acvm::acir::native_types::{Witness, WitnessMap, WitnessStack};
use acvm::brillig_vm::MemoryValue;
use acvm::brillig_vm::{FailureReason, MemoryValue};
use acvm::pwg::{
ACVMStatus, AcirCallWaitInfo, BrilligSolver, BrilligSolverStatus, ForeignCallWaitInfo,
OpcodeNotSolvable, OpcodeResolutionError, StepResult, ACVM,
Expand Down Expand Up @@ -472,7 +472,11 @@ impl<'a, B: BlackBoxFunctionSolver<FieldElement>> DebugContext<'a, B> {
self.handle_foreign_call(foreign_call)
}
Err(err) => {
if let OpcodeResolutionError::BrilligFunctionFailed { found_trap: true, .. } = err {
if let OpcodeResolutionError::BrilligFunctionFailed {
reason: FailureReason::Trap { .. },
..
} = err
{
// return solver ownership so brillig_solver it has the right opcode location
self.brillig_solver = Some(solver);
}
Expand Down

0 comments on commit 58e8fb4

Please sign in to comment.