From 7bf3b4926197d47608280dc436081d236768eb78 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Mon, 8 Jul 2024 12:37:01 +0100 Subject: [PATCH] chore: extracts refactoring of reading error payloads from #5403 (#5413) # Description ## Problem\* Resolves ## 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 --- acvm-repo/acvm/src/pwg/brillig.rs | 108 ++++++++++++++++-------------- 1 file changed, 58 insertions(+), 50 deletions(-) diff --git a/acvm-repo/acvm/src/pwg/brillig.rs b/acvm-repo/acvm/src/pwg/brillig.rs index 3a639df044a..91dedac8e35 100644 --- a/acvm-repo/acvm/src/pwg/brillig.rs +++ b/acvm-repo/acvm/src/pwg/brillig.rs @@ -162,63 +162,27 @@ impl<'b, B: BlackBoxFunctionSolver, 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 })) @@ -283,6 +247,50 @@ impl<'b, B: BlackBoxFunctionSolver, 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( + memory: &[MemoryValue], + revert_data_offset: usize, + revert_data_size: usize, +) -> Option> { + // 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. ///