Skip to content

Commit

Permalink
feat!: add session id to foreign call RPC requests (#5205)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

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

## Summary\*

This PR adds an `id` field to RPC foreign call requests so that the
TriXE is able to maintain separate internal state for different test
functions so that Aztec integration tests can be run in parallel. This
has necessitated some changes to the request format so this is a
breaking change.


The RPC request now looks like

```
'{
    "jsonrpc":"2.0",
    "method":"resolve_foreign_call",
    "params":[{
        "session_id":3789997497881369652,
        "function": "foo",
        "inputs": [
            "0000000000000000000000000000000000000000000000000000000000000001", 
["0000000000000000000000000000000000000000000000000000000000000001","0000000000000000000000000000000000000000000000000000000000000002"
    ]]
}]}
```

## Additional Context



## Documentation\*

Check one:
- [] No documentation needed.
- [x] 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.
  • Loading branch information
TomAFrench authored Jun 10, 2024
1 parent 17c859d commit 14adafc
Show file tree
Hide file tree
Showing 9 changed files with 101 additions and 38 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ similar-asserts = "1.5.0"
tempfile = "3.6.0"
jsonrpc = { version = "0.16.0", features = ["minreq_http"] }
flate2 = "1.0.24"
rand = "0.8.5"

im = { version = "15.1", features = ["serde"] }
tracing = "0.1.40"
Expand Down
3 changes: 2 additions & 1 deletion acvm-repo/acvm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ repository.workspace = true
num-bigint.workspace = true
thiserror.workspace = true
tracing.workspace = true
serde.workspace = true

acir.workspace = true
brillig_vm.workspace = true
Expand All @@ -36,7 +37,7 @@ bls12_381 = [
]

[dev-dependencies]
rand = "0.8.5"
rand.workspace = true
proptest = "1.2.0"
paste = "1.0.14"
ark-bls12-381 = { version = "^0.4.0", default-features = false, features = ["curve"] }
3 changes: 2 additions & 1 deletion acvm-repo/acvm/src/pwg/brillig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use acir::{
};
use acvm_blackbox_solver::BlackBoxFunctionSolver;
use brillig_vm::{FailureReason, MemoryValue, VMStatus, VM};
use serde::{Deserialize, Serialize};

use crate::{pwg::OpcodeNotSolvable, OpcodeResolutionError};

Expand Down Expand Up @@ -286,7 +287,7 @@ impl<'b, B: BlackBoxFunctionSolver<F>, F: AcirField> BrilligSolver<'b, F, B> {
/// where the result of the foreign call has not yet been provided.
///
/// The caller must resolve this opcode externally based upon the information in the request.
#[derive(Debug, PartialEq, Clone)]
#[derive(Debug, PartialEq, Clone, Serialize, Deserialize)]
pub struct ForeignCallWaitInfo<F> {
/// An identifier interpreted by the caller process
pub function: String,
Expand Down
1 change: 1 addition & 0 deletions acvm-repo/brillig/src/foreign_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use serde::{Deserialize, Serialize};

/// Single output of a [foreign call][crate::Opcode::ForeignCall].
#[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Clone)]
#[serde(untagged)]
pub enum ForeignCallParam<F> {
Single(F),
Array(Vec<F>),
Expand Down
17 changes: 5 additions & 12 deletions docs/docs/how_to/how-to-oracles.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,24 +137,17 @@ app.listen(5555);
Now, we will add our `getSqrt` method, as expected by the `#[oracle(getSqrt)]` decorator in our Noir code. It maps through the params array and returns their square roots:

```js
server.addMethod("getSqrt", async (params) => {
const values = params[0].Array.map((field) => {
server.addMethod("resolve_function_call", async (params) => {
if params.function !== "getSqrt" {
throw Error("Unexpected foreign call")
};
const values = params.inputs[0].Array.map((field) => {
return `${Math.sqrt(parseInt(field, 16))}`;
});
return { values: [{ Array: values }] };
});
```

:::tip

Brillig expects an object with an array of values. Each value is an object declaring to be `Single` or `Array` and returning a field element *as a string*. For example:

```json
{ "values": [{ "Array": ["1", "2"] }]}
{ "values": [{ "Single": "1" }]}
{ "values": [{ "Single": "1" }, { "Array": ["1", "2"] }]}
```

If you're using Typescript, the following types may be helpful in understanding the expected return value and making sure they're easy to follow:

```js
Expand Down
2 changes: 1 addition & 1 deletion tooling/acvm_cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,6 @@ tracing-subscriber.workspace = true
tracing-appender = "0.2.3"

[dev-dependencies]
rand = "0.8.5"
rand.workspace = true
proptest = "1.2.0"
paste = "1.0.14"
1 change: 1 addition & 0 deletions tooling/nargo/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ codespan-reporting.workspace = true
tracing.workspace = true
rayon = "1.8.0"
jsonrpc.workspace = true
rand.workspace = true

[dev-dependencies]
# TODO: This dependency is used to generate unit tests for `get_all_paths_in_dir`
Expand Down
109 changes: 86 additions & 23 deletions tooling/nargo/src/ops/foreign_calls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ use acvm::{
};
use jsonrpc::{arg as build_json_rpc_arg, minreq_http::Builder, Client};
use noirc_printable_type::{decode_string_value, ForeignCallError, PrintableValueDisplay};
use rand::Rng;
use serde::{Deserialize, Serialize};

pub trait ForeignCallExecutor {
fn execute(
Expand Down Expand Up @@ -96,6 +98,12 @@ impl MockedCall {

#[derive(Debug, Default)]
pub struct DefaultForeignCallExecutor {
/// A randomly generated id for this `DefaultForeignCallExecutor`.
///
/// This is used so that a single `external_resolver` can distinguish between requests from multiple
/// instantiations of `DefaultForeignCallExecutor`.
id: u64,

/// Mocks have unique ids used to identify them in Noir, allowing to update or remove them.
last_mock_id: usize,
/// The registered mocks
Expand All @@ -106,6 +114,20 @@ pub struct DefaultForeignCallExecutor {
external_resolver: Option<Client>,
}

#[derive(Debug, Serialize, Deserialize)]
struct ResolveForeignCallRequest<F> {
/// A session ID which allows the external RPC server to link this foreign call request to other foreign calls
/// for the same program execution.
///
/// This is intended to allow a single RPC server to maintain state related to multiple program executions being
/// performed in parallel.
session_id: u64,

#[serde(flatten)]
/// The foreign call which the external RPC server is to provide a response for.
function_call: ForeignCallWaitInfo<F>,
}

impl DefaultForeignCallExecutor {
pub fn new(show_output: bool, resolver_url: Option<&str>) -> Self {
let oracle_resolver = resolver_url.map(|resolver_url| {
Expand All @@ -123,6 +145,7 @@ impl DefaultForeignCallExecutor {
DefaultForeignCallExecutor {
show_output,
external_resolver: oracle_resolver,
id: rand::thread_rng().gen(),
..DefaultForeignCallExecutor::default()
}
}
Expand Down Expand Up @@ -275,10 +298,13 @@ impl ForeignCallExecutor for DefaultForeignCallExecutor {
} 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 encoded_params: Vec<_> =
foreign_call.inputs.iter().map(build_json_rpc_arg).collect();
let encoded_params = vec![build_json_rpc_arg(ResolveForeignCallRequest {
session_id: self.id,
function_call: foreign_call.clone(),
})];

let req = external_resolver.build_request(foreign_call_name, &encoded_params);
let req =
external_resolver.build_request("resolve_foreign_call", &encoded_params);

let response = external_resolver.send_request(req)?;

Expand Down Expand Up @@ -312,43 +338,49 @@ mod tests {

use crate::ops::{DefaultForeignCallExecutor, ForeignCallExecutor};

use super::ResolveForeignCallRequest;

#[allow(unreachable_pub)]
#[rpc]
pub trait OracleResolver {
#[rpc(name = "echo")]
fn echo(
&self,
param: ForeignCallParam<FieldElement>,
) -> RpcResult<ForeignCallResult<FieldElement>>;

#[rpc(name = "sum")]
fn sum(
#[rpc(name = "resolve_foreign_call")]
fn resolve_foreign_call(
&self,
array: ForeignCallParam<FieldElement>,
req: ResolveForeignCallRequest<FieldElement>,
) -> RpcResult<ForeignCallResult<FieldElement>>;
}

struct OracleResolverImpl;

impl OracleResolver for OracleResolverImpl {
fn echo(
&self,
param: ForeignCallParam<FieldElement>,
) -> RpcResult<ForeignCallResult<FieldElement>> {
Ok(vec![param].into())
impl OracleResolverImpl {
fn echo(&self, param: ForeignCallParam<FieldElement>) -> ForeignCallResult<FieldElement> {
vec![param].into()
}

fn sum(
&self,
array: ForeignCallParam<FieldElement>,
) -> RpcResult<ForeignCallResult<FieldElement>> {
fn sum(&self, array: ForeignCallParam<FieldElement>) -> ForeignCallResult<FieldElement> {
let mut res: FieldElement = 0_usize.into();

for value in array.fields() {
res += value;
}

Ok(res.into())
res.into()
}
}

impl OracleResolver for OracleResolverImpl {
fn resolve_foreign_call(
&self,
req: ResolveForeignCallRequest<FieldElement>,
) -> RpcResult<ForeignCallResult<FieldElement>> {
let response = match req.function_call.function.as_str() {
"sum" => self.sum(req.function_call.inputs[0].clone()),
"echo" => self.echo(req.function_call.inputs[0].clone()),
"id" => FieldElement::from(req.session_id as u128).into(),

_ => panic!("unexpected foreign call"),
};
Ok(response)
}
}

Expand Down Expand Up @@ -398,4 +430,35 @@ mod tests {

server.close();
}

#[test]
fn foreign_call_executor_id_is_persistent() {
let (server, url) = build_oracle_server();

let mut executor = DefaultForeignCallExecutor::new(false, Some(&url));

let foreign_call = ForeignCallWaitInfo { function: "id".to_string(), inputs: Vec::new() };

let result_1 = executor.execute(&foreign_call).unwrap();
let result_2 = executor.execute(&foreign_call).unwrap();
assert_eq!(result_1, result_2);

server.close();
}

#[test]
fn oracle_resolver_rpc_can_distinguish_executors() {
let (server, url) = build_oracle_server();

let mut executor_1 = DefaultForeignCallExecutor::new(false, Some(&url));
let mut executor_2 = DefaultForeignCallExecutor::new(false, Some(&url));

let foreign_call = ForeignCallWaitInfo { function: "id".to_string(), inputs: Vec::new() };

let result_1 = executor_1.execute(&foreign_call).unwrap();
let result_2 = executor_2.execute(&foreign_call).unwrap();
assert_ne!(result_1, result_2);

server.close();
}
}

0 comments on commit 14adafc

Please sign in to comment.