Skip to content

Commit

Permalink
chore: extracts refactoring of reading error payloads from #5403 (#5413)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves <!-- Link to GitHub Issue -->

## Summary\*

This PR pulls out a modified version of @anaPerezGhiglia's refactoring
to how we extract error payloads from the brillig VM's memory from #5403
to separate it from the creation of the new error variant.

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: Ana Perez Ghiglia <[email protected]>
  • Loading branch information
TomAFrench and anaPerezGhiglia authored Jul 8, 2024
1 parent 580c16d commit 7bf3b49
Showing 1 changed file with 58 additions and 50 deletions.
108 changes: 58 additions & 50 deletions acvm-repo/acvm/src/pwg/brillig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,63 +162,27 @@ impl<'b, B: BlackBoxFunctionSolver<F>, F: AcirField> BrilligSolver<'b, F, B> {
VMStatus::Finished { .. } => Ok(BrilligSolverStatus::Finished),
VMStatus::InProgress => Ok(BrilligSolverStatus::InProgress),
VMStatus::Failure { reason, call_stack } => {
let call_stack = call_stack
.iter()
.map(|brillig_index| OpcodeLocation::Brillig {
acir_index: self.acir_index,
brillig_index: *brillig_index,
})
.collect();
let payload = match reason {
FailureReason::RuntimeError { message } => {
Some(ResolvedAssertionPayload::String(message))
}
FailureReason::Trap { revert_data_offset, revert_data_size } => {
// Since noir can only revert with strings currently, we can parse return data as a string
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(),
}))
}
}
}
extract_failure_payload_from_memory(
self.vm.get_memory(),
revert_data_offset,
revert_data_size,
)
}
};
Err(OpcodeResolutionError::BrilligFunctionFailed {
payload,
call_stack: call_stack
.iter()
.map(|brillig_index| OpcodeLocation::Brillig {
acir_index: self.acir_index,
brillig_index: *brillig_index,
})
.collect(),
})

Err(OpcodeResolutionError::BrilligFunctionFailed { payload, call_stack })
}
VMStatus::ForeignCallWait { function, inputs } => {
Ok(BrilligSolverStatus::ForeignCallWait(ForeignCallWaitInfo { function, inputs }))
Expand Down Expand Up @@ -283,6 +247,50 @@ impl<'b, B: BlackBoxFunctionSolver<F>, F: AcirField> BrilligSolver<'b, F, B> {
}
}

/// Extracts a `ResolvedAssertionPayload` from a block of memory of a Brillig VM instance.
///
/// Returns `None` if the amount of memory requested is zero.
fn extract_failure_payload_from_memory<F: AcirField>(
memory: &[MemoryValue<F>],
revert_data_offset: usize,
revert_data_size: usize,
) -> Option<ResolvedAssertionPayload<F>> {
// Since noir can only revert with strings currently, we can parse return data as a string
if revert_data_size == 0 {
None
} else {
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(),
}))
}
}
}
}

/// Encapsulates a request from a Brillig VM process that encounters a [foreign call opcode][acir::brillig_vm::Opcode::ForeignCall]
/// where the result of the foreign call has not yet been provided.
///
Expand Down

0 comments on commit 7bf3b49

Please sign in to comment.