Skip to content

Commit

Permalink
feat: handle empty response foreign calls without an external resolver (
Browse files Browse the repository at this point in the history
#4959)

# Description

## Problem\*

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

## Summary\*

I've modified the `DefaultForeignCallExecutor` so that rather than
panicking on an unrecognised foreign call we return an empty response.

This solves an issue experienced by @spalladino where `nargo test` was
failing due to the use of a custom debugging oracle which doesn't return
any values to the circuit. The only situation where these oracle calls
not being handled would change the outcome of the test would be if they
modify state in an external resolver such as to modify the response of a
different oracle call (and so we'd still fail on those anyway).

## Additional Context

I've cleared up some empty `Prover.toml` files as well.

## 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: jfecher <jake@aztecprotocol.com>
  • Loading branch information
TomAFrench and jfecher committed May 2, 2024
1 parent 2e085b9 commit 0154bde
Show file tree
Hide file tree
Showing 9 changed files with 67 additions and 31 deletions.
Empty file.
Empty file.
Empty file.
7 changes: 7 additions & 0 deletions test_programs/noir_test_success/ignored_oracle/Nargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "ignored_oracle"
type = "bin"
authors = [""]
compiler_version = ">=0.23.0"

[dependencies]
23 changes: 23 additions & 0 deletions test_programs/noir_test_success/ignored_oracle/src/main.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// In `nargo test` we want to avoid the need for an external oracle resolver service to be required in the situation
// where its existence doesn't affect whether the tests will pass or fail. We then want to be able to handle any
// oracles which return zero field elements.

// Note that this custom oracle doesn't return any new values into the program.
// We can then safely continue execution even in the case where there is no oracle resolver to handle it.
#[oracle(custom_debug)]
unconstrained fn custom_debug() {}

// However this oracle call should return a field element. We expect the ACVM to raise an error when it
// doesn't receive this value.
#[oracle(custom_getter)]
unconstrained fn custom_getter() -> Field {}

#[test]
unconstrained fn unit_return_oracle_ignored() {
custom_debug();
}

#[test(should_fail_with = "0 output values were provided as a foreign call result for 1 destination slots")]
unconstrained fn field_return_oracle_fails() {
let _ = custom_getter();
}
Empty file.
Empty file.
Empty file.
68 changes: 37 additions & 31 deletions tooling/nargo/src/ops/foreign_calls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,43 +315,49 @@ impl ForeignCallExecutor for DefaultForeignCallExecutor {
.iter()
.position(|response| response.matches(foreign_call_name, &foreign_call.inputs));

match (mock_response_position, &self.external_resolver) {
(Some(response_position), _) => {
let mock = self
.mocked_responses
.get_mut(response_position)
.expect("Invalid position of mocked response");

mock.last_called_params = Some(foreign_call.inputs.clone());

let result = mock.result.values.clone();

if let Some(times_left) = &mut mock.times_left {
*times_left -= 1;
if *times_left == 0 {
self.mocked_responses.remove(response_position);
}
}
if let Some(response_position) = mock_response_position {
// If the program has registered a mocked response to this oracle call then we prefer responding
// with that.

let mock = self
.mocked_responses
.get_mut(response_position)
.expect("Invalid position of mocked response");

mock.last_called_params = Some(foreign_call.inputs.clone());

let result = mock.result.values.clone();

Ok(result.into())
if let Some(times_left) = &mut mock.times_left {
*times_left -= 1;
if *times_left == 0 {
self.mocked_responses.remove(response_position);
}
}
(None, Some(external_resolver)) => {
let encoded_params: Vec<_> =
foreign_call.inputs.iter().map(build_json_rpc_arg).collect();

let req =
external_resolver.build_request(foreign_call_name, &encoded_params);
Ok(result.into())
} else if let Some(external_resolver) = &self.external_resolver {
// If the user has registered an external resolver then we forward any remaining oracle calls there.

let response = external_resolver.send_request(req)?;
let encoded_params: Vec<_> =
foreign_call.inputs.iter().map(build_json_rpc_arg).collect();

let parsed_response: ForeignCallResult = response.result()?;
let req = external_resolver.build_request(foreign_call_name, &encoded_params);

Ok(parsed_response.into())
}
(None, None) => panic!(
"No mock for foreign call {}({:?})",
foreign_call_name, &foreign_call.inputs
),
let response = external_resolver.send_request(req)?;

let parsed_response: ForeignCallResult = response.result()?;

Ok(parsed_response.into())
} else {
// If there's no registered mock oracle response and no registered resolver then we cannot
// return a correct response to the ACVM. The best we can do is to return an empty response,
// this allows us to ignore any foreign calls which exist solely to pass information from inside
// the circuit to the environment (e.g. custom logging) as the execution will still be able to progress.
//
// We optimistically return an empty response for all oracle calls as the ACVM will error
// should a response have been required.
Ok(ForeignCallResult::default().into())
}
}
}
Expand Down

0 comments on commit 0154bde

Please sign in to comment.