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

fix: add new BrilligFunctionUnsatisfiedConstrain resolution error variant #5403

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
26 changes: 16 additions & 10 deletions acvm-repo/acvm/src/pwg/brillig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,20 +169,26 @@ impl<'b, B: BlackBoxFunctionSolver<F>, F: AcirField> BrilligSolver<'b, F, B> {
brillig_index: *brillig_index,
})
.collect();
let payload = match reason {

Err(match reason {
FailureReason::RuntimeError { message } => {
Some(ResolvedAssertionPayload::String(message))
OpcodeResolutionError::BrilligFunctionFailed {
call_stack,
payload: Some(ResolvedAssertionPayload::String(message)),
}
}

FailureReason::Trap { revert_data_offset, revert_data_size } => {
extract_failure_payload_from_memory(
self.vm.get_memory(),
revert_data_offset,
revert_data_size,
)
OpcodeResolutionError::BrilligFunctionUnsatisfiedConstrain {
call_stack,
payload: extract_failure_payload_from_memory(
self.vm.get_memory(),
revert_data_offset,
revert_data_size,
),
}
}
};

Err(OpcodeResolutionError::BrilligFunctionFailed { payload, call_stack })
})
}
VMStatus::ForeignCallWait { function, inputs } => {
Ok(BrilligSolverStatus::ForeignCallWait(ForeignCallWaitInfo { function, inputs }))
Expand Down
7 changes: 6 additions & 1 deletion acvm-repo/acvm/src/pwg/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,11 @@ pub enum OpcodeResolutionError<F> {
call_stack: Vec<OpcodeLocation>,
payload: Option<ResolvedAssertionPayload<F>>,
},
#[error("Cannot satisfy constraint")]
BrilligFunctionUnsatisfiedConstrain {
call_stack: Vec<OpcodeLocation>,
payload: Option<ResolvedAssertionPayload<F>>,
},
#[error("Attempted to call `main` with a `Call` opcode")]
AcirMainCallAttempted { opcode_location: ErrorLocation },
#[error("{results_size:?} result values were provided for {outputs_size:?} call output witnesses, most likely due to bad ACIR codegen")]
Expand Down Expand Up @@ -502,7 +507,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver<F>> ACVM<'a, F, B> {

fn map_brillig_error(&self, mut err: OpcodeResolutionError<F>) -> OpcodeResolutionError<F> {
match &mut err {
OpcodeResolutionError::BrilligFunctionFailed { call_stack, payload } => {
OpcodeResolutionError::BrilligFunctionUnsatisfiedConstrain { call_stack, payload } => {
// Some brillig errors have static strings as payloads, we can resolve them here
let last_location =
call_stack.last().expect("Call stacks should have at least one item");
Expand Down
2 changes: 1 addition & 1 deletion acvm-repo/acvm/tests/solver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,7 @@ fn unsatisfied_opcode_resolved_brillig() {
let solver_status = acvm.solve();
assert_eq!(
solver_status,
ACVMStatus::Failure(OpcodeResolutionError::BrilligFunctionFailed {
ACVMStatus::Failure(OpcodeResolutionError::BrilligFunctionUnsatisfiedConstrain {
payload: None,
call_stack: vec![OpcodeLocation::Brillig { acir_index: 0, brillig_index: 3 }]
}),
Expand Down
12 changes: 9 additions & 3 deletions acvm-repo/acvm_js/src/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,9 +200,11 @@ impl<'a, B: BlackBoxFunctionSolver<FieldElement>> ProgramExecutor<'a, B> {
opcode_location: ErrorLocation::Resolved(opcode_location),
..
} => Some(vec![*opcode_location]),
OpcodeResolutionError::BrilligFunctionFailed { call_stack, .. } => {
Some(call_stack.clone())
}
OpcodeResolutionError::BrilligFunctionFailed { call_stack, .. }
| OpcodeResolutionError::BrilligFunctionUnsatisfiedConstrain {
call_stack,
..
} => Some(call_stack.clone()),
_ => None,
};
// If the failed opcode has an assertion message, integrate it into the error message for backwards compatibility.
Expand All @@ -215,6 +217,10 @@ impl<'a, B: BlackBoxFunctionSolver<FieldElement>> ProgramExecutor<'a, B> {
| OpcodeResolutionError::BrilligFunctionFailed {
payload: Some(payload),
..
}
| OpcodeResolutionError::BrilligFunctionUnsatisfiedConstrain {
payload: Some(payload),
..
} => match payload {
ResolvedAssertionPayload::Raw(raw_payload) => {
("Assertion failed".to_string(), Some(raw_payload))
Expand Down
11 changes: 8 additions & 3 deletions tooling/debugger/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -471,9 +471,14 @@ impl<'a, B: BlackBoxFunctionSolver<FieldElement>> DebugContext<'a, B> {
self.brillig_solver = Some(solver);
self.handle_foreign_call(foreign_call)
}
Err(err) => DebugCommandResult::Error(NargoError::ExecutionError(
ExecutionError::SolvingError(err, None),
)),
Err(err) => {
// return solver ownership so context has the right opcode location when reporting error
self.brillig_solver = Some(solver);

DebugCommandResult::Error(NargoError::ExecutionError(ExecutionError::SolvingError(
err, None,
)))
}
}
}

Expand Down
5 changes: 5 additions & 0 deletions tooling/nargo/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ impl<F: AcirField> NargoError<F> {
| OpcodeResolutionError::UnsatisfiedConstrain { .. }
| OpcodeResolutionError::AcirMainCallAttempted { .. }
| OpcodeResolutionError::BrilligFunctionFailed { .. }
| OpcodeResolutionError::BrilligFunctionUnsatisfiedConstrain { .. }
| OpcodeResolutionError::AcirCallOutputsMismatch { .. } => None,
OpcodeResolutionError::BlackBoxFunctionFailed(_, reason) => {
Some(reason.to_string())
Expand Down Expand Up @@ -111,6 +112,10 @@ fn extract_locations_from_error<F: AcirField>(
ExecutionError::SolvingError(
OpcodeResolutionError::BrilligFunctionFailed { .. },
acir_call_stack,
)
| ExecutionError::SolvingError(
OpcodeResolutionError::BrilligFunctionUnsatisfiedConstrain { .. },
acir_call_stack,
) => acir_call_stack.clone(),
ExecutionError::AssertionFailed(_, call_stack) => Some(call_stack.clone()),
ExecutionError::SolvingError(
Expand Down
10 changes: 9 additions & 1 deletion tooling/nargo/src/ops/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,11 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver<F>, E: ForeignCallExecutor<F>>
self.call_stack.push(resolved_location);
Some(self.call_stack.clone())
}
OpcodeResolutionError::BrilligFunctionFailed { call_stack, .. } => {
OpcodeResolutionError::BrilligFunctionFailed { call_stack, .. }
| OpcodeResolutionError::BrilligFunctionUnsatisfiedConstrain {
call_stack,
..
} => {
let brillig_call_stack =
call_stack.iter().map(|location| ResolvedOpcodeLocation {
acir_function_index: self.current_function_index,
Expand All @@ -111,6 +115,10 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver<F>, E: ForeignCallExecutor<F>>

let assertion_payload: Option<ResolvedAssertionPayload<F>> = match &error {
OpcodeResolutionError::BrilligFunctionFailed { payload, .. }
| OpcodeResolutionError::BrilligFunctionUnsatisfiedConstrain {
payload,
..
}
| OpcodeResolutionError::UnsatisfiedConstrain { payload, .. } => {
payload.clone()
}
Expand Down
Loading