Skip to content

Commit 51a4d5d

Browse files
aakoshhTomAFrench
andauthored
feat!: Switch to using jsonrpsee for foreign calls; refactor run_test; foreign call layering (noir-lang#6849)
Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> Co-authored-by: Tom French <tom@tomfren.ch>
1 parent 6bd47be commit 51a4d5d

25 files changed

+1135
-770
lines changed

Cargo.lock

+541-447
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

+2-2
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ num-traits = "0.2"
148148
similar-asserts = "1.5.0"
149149
tempfile = "3.6.0"
150150
test-case = "3.3.1"
151-
jsonrpc = { version = "0.16.0", features = ["minreq_http"] }
151+
jsonrpsee = { version = "0.24.7", features = ["client-core"] }
152152
flate2 = "1.0.24"
153153
color-eyre = "0.6.2"
154154
rand = "0.8.5"
@@ -159,7 +159,7 @@ sha2 = { version = "0.10.6", features = ["compress"] }
159159
sha3 = "0.10.6"
160160
strum = "0.24"
161161
strum_macros = "0.24"
162-
162+
tokio = "1.42"
163163
im = { version = "15.1", features = ["serde"] }
164164
tracing = "0.1.40"
165165
tracing-web = "0.1.3"

compiler/wasm/Cargo.toml

+5-1
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@ workspace = true
1717
crate-type = ["cdylib"]
1818

1919
[dependencies]
20+
2021
acvm = { workspace = true, features = ["bn254"] }
2122
fm.workspace = true
22-
nargo.workspace = true
2323
noirc_driver.workspace = true
2424
noirc_frontend = { workspace = true, features = ["bn254"] }
2525
noirc_errors.workspace = true
@@ -33,6 +33,10 @@ gloo-utils.workspace = true
3333
tracing-subscriber.workspace = true
3434
tracing-web.workspace = true
3535

36+
# Cannot use the `rpc` feature because the HTTP dependency wouldn't compile to Wasm.
37+
# We could use `path` if `rpc` was a default feature, but we made it opt-in so we don't get any problems when publishing the workspace.
38+
nargo.workspace = true
39+
3640
# This is an unused dependency, we are adding it
3741
# so that we can enable the js feature in getrandom.
3842
getrandom = { workspace = true, features = ["js"] }

cspell.json

+2
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@
126126
"jmpifs",
127127
"jmps",
128128
"jsdoc",
129+
"jsonrpsee",
129130
"Jubjub",
130131
"keccak",
131132
"keccakf",
@@ -166,6 +167,7 @@
166167
"nomicfoundation",
167168
"noncanonical",
168169
"nouner",
170+
"oneshot",
169171
"overflowing",
170172
"pedersen",
171173
"peekable",

deny.toml

+6-1
Original file line numberDiff line numberDiff line change
@@ -66,14 +66,19 @@ exceptions = [
6666
# so we prefer to not have dependencies using it
6767
# https://tldrlegal.com/license/creative-commons-cc0-1.0-universal
6868
{ allow = ["CC0-1.0"], name = "more-asserts" },
69-
{ allow = ["CC0-1.0"], name = "jsonrpc" },
7069
{ allow = ["CC0-1.0"], name = "notify" },
7170
{ allow = ["CC0-1.0"], name = "tiny-keccak" },
7271
{ allow = ["MPL-2.0"], name = "sized-chunks" },
7372
{ allow = ["MPL-2.0"], name = "webpki-roots" },
7473
{ allow = ["CDDL-1.0"], name = "inferno" },
74+
{ allow = ["OpenSSL"], name = "ring" },
7575
]
7676

77+
[[licenses.clarify]]
78+
crate = "ring"
79+
expression = "ISC"
80+
license-files = [{ path = "LICENSE", hash = 0xbd0eed23 }]
81+
7782
# This section is considered when running `cargo deny check sources`.
7883
# More documentation about the 'sources' section can be found here:
7984
# https://embarkstudios.github.io/cargo-deny/checks/sources/cfg.html

tooling/acvm_cli/src/cli/execute_cmd.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use nargo::PrintOutput;
99

1010
use crate::cli::fs::inputs::{read_bytecode_from_file, read_inputs_from_file};
1111
use crate::errors::CliError;
12-
use nargo::{foreign_calls::DefaultForeignCallExecutor, ops::execute_program};
12+
use nargo::{foreign_calls::DefaultForeignCallBuilder, ops::execute_program};
1313

1414
use super::fs::witness::{create_output_witness_string, save_witness_to_dir};
1515

@@ -74,7 +74,8 @@ pub(crate) fn execute_program_from_witness(
7474
&program,
7575
inputs_map,
7676
&Bn254BlackBoxSolver,
77-
&mut DefaultForeignCallExecutor::new(PrintOutput::Stdout, None, None, None),
77+
&mut DefaultForeignCallBuilder { output: PrintOutput::Stdout, ..Default::default() }
78+
.build(),
7879
)
7980
.map_err(CliError::CircuitExecutionError)
8081
}

tooling/debugger/src/foreign_calls.rs

+39-15
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@ use acvm::{
44
AcirField, FieldElement,
55
};
66
use nargo::{
7-
foreign_calls::{DefaultForeignCallExecutor, ForeignCallError, ForeignCallExecutor},
7+
foreign_calls::{
8+
layers::Layer, DefaultForeignCallBuilder, ForeignCallError, ForeignCallExecutor,
9+
},
810
PrintOutput,
911
};
1012
use noirc_artifacts::debug::{DebugArtifact, DebugVars, StackFrame};
@@ -43,23 +45,31 @@ pub trait DebugForeignCallExecutor: ForeignCallExecutor<FieldElement> {
4345
fn current_stack_frame(&self) -> Option<StackFrame<FieldElement>>;
4446
}
4547

46-
pub struct DefaultDebugForeignCallExecutor<'a> {
47-
executor: DefaultForeignCallExecutor<'a, FieldElement>,
48+
#[derive(Default)]
49+
pub struct DefaultDebugForeignCallExecutor {
4850
pub debug_vars: DebugVars<FieldElement>,
4951
}
5052

51-
impl<'a> DefaultDebugForeignCallExecutor<'a> {
52-
pub fn new(output: PrintOutput<'a>) -> Self {
53-
Self {
54-
executor: DefaultForeignCallExecutor::new(output, None, None, None),
55-
debug_vars: DebugVars::default(),
56-
}
53+
impl DefaultDebugForeignCallExecutor {
54+
fn make(
55+
output: PrintOutput<'_>,
56+
ex: DefaultDebugForeignCallExecutor,
57+
) -> impl DebugForeignCallExecutor + '_ {
58+
DefaultForeignCallBuilder::default().with_output(output).build().add_layer(ex)
59+
}
60+
61+
#[allow(clippy::new_ret_no_self, dead_code)]
62+
pub fn new(output: PrintOutput<'_>) -> impl DebugForeignCallExecutor + '_ {
63+
Self::make(output, Self::default())
5764
}
5865

59-
pub fn from_artifact(output: PrintOutput<'a>, artifact: &DebugArtifact) -> Self {
60-
let mut ex = Self::new(output);
66+
pub fn from_artifact<'a>(
67+
output: PrintOutput<'a>,
68+
artifact: &DebugArtifact,
69+
) -> impl DebugForeignCallExecutor + 'a {
70+
let mut ex = Self::default();
6171
ex.load_artifact(artifact);
62-
ex
72+
Self::make(output, ex)
6373
}
6474

6575
pub fn load_artifact(&mut self, artifact: &DebugArtifact) {
@@ -72,7 +82,7 @@ impl<'a> DefaultDebugForeignCallExecutor<'a> {
7282
}
7383
}
7484

75-
impl DebugForeignCallExecutor for DefaultDebugForeignCallExecutor<'_> {
85+
impl DebugForeignCallExecutor for DefaultDebugForeignCallExecutor {
7686
fn get_variables(&self) -> Vec<StackFrame<FieldElement>> {
7787
self.debug_vars.get_variables()
7888
}
@@ -90,7 +100,7 @@ fn debug_fn_id(value: &FieldElement) -> DebugFnId {
90100
DebugFnId(value.to_u128() as u32)
91101
}
92102

93-
impl ForeignCallExecutor<FieldElement> for DefaultDebugForeignCallExecutor<'_> {
103+
impl ForeignCallExecutor<FieldElement> for DefaultDebugForeignCallExecutor {
94104
fn execute(
95105
&mut self,
96106
foreign_call: &ForeignCallWaitInfo<FieldElement>,
@@ -165,7 +175,21 @@ impl ForeignCallExecutor<FieldElement> for DefaultDebugForeignCallExecutor<'_> {
165175
self.debug_vars.pop_fn();
166176
Ok(ForeignCallResult::default())
167177
}
168-
None => self.executor.execute(foreign_call),
178+
None => Err(ForeignCallError::NoHandler(foreign_call_name.to_string())),
169179
}
170180
}
171181
}
182+
183+
impl<H, I> DebugForeignCallExecutor for Layer<H, I>
184+
where
185+
H: DebugForeignCallExecutor,
186+
I: ForeignCallExecutor<FieldElement>,
187+
{
188+
fn get_variables(&self) -> Vec<StackFrame<FieldElement>> {
189+
self.handler().get_variables()
190+
}
191+
192+
fn current_stack_frame(&self) -> Option<StackFrame<FieldElement>> {
193+
self.handler().current_stack_frame()
194+
}
195+
}

tooling/lsp/src/requests/test_run.rs

+10-3
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use std::future::{self, Future};
33
use crate::insert_all_files_for_workspace_into_file_manager;
44
use async_lsp::{ErrorCode, ResponseError};
55
use nargo::{
6+
foreign_calls::DefaultForeignCallBuilder,
67
ops::{run_test, TestStatus},
78
PrintOutput,
89
};
@@ -88,10 +89,16 @@ fn on_test_run_request_inner(
8889
&mut context,
8990
&test_function,
9091
PrintOutput::Stdout,
91-
None,
92-
Some(workspace.root_dir.clone()),
93-
Some(package.name.to_string()),
9492
&CompileOptions::default(),
93+
|output, base| {
94+
DefaultForeignCallBuilder {
95+
output,
96+
resolver_url: None, // NB without this the root and package don't do anything.
97+
root_path: Some(workspace.root_dir.clone()),
98+
package_name: Some(package.name.to_string()),
99+
}
100+
.build_with_base(base)
101+
},
95102
);
96103
let result = match test_result {
97104
TestStatus::Pass => NargoTestRunResult {

tooling/nargo/Cargo.toml

+15-9
Original file line numberDiff line numberDiff line change
@@ -21,21 +21,27 @@ noirc_errors.workspace = true
2121
noirc_frontend.workspace = true
2222
noirc_printable_type.workspace = true
2323
iter-extended.workspace = true
24+
jsonrpsee.workspace = true
25+
rayon.workspace = true
2426
thiserror.workspace = true
2527
tracing.workspace = true
26-
rayon.workspace = true
27-
jsonrpc.workspace = true
28-
rand.workspace = true
2928
serde.workspace = true
3029
serde_json.workspace = true
3130
walkdir = "2.5.0"
3231

32+
# Some dependencies are optional so we can compile to Wasm.
33+
tokio = { workspace = true, optional = true }
34+
rand = { workspace = true, optional = true }
35+
3336
[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
34-
noir_fuzzer.workspace = true
35-
proptest.workspace = true
37+
noir_fuzzer = { workspace = true }
38+
proptest = { workspace = true }
3639

3740
[dev-dependencies]
38-
jsonrpc-http-server = "18.0"
39-
jsonrpc-core-client = "18.0"
40-
jsonrpc-derive = "18.0"
41-
jsonrpc-core = "18.0"
41+
jsonrpsee = { workspace = true, features = ["server"] }
42+
43+
[features]
44+
default = []
45+
46+
# Execution currently uses HTTP based Oracle resolvers; does not compile to Wasm.
47+
rpc = ["jsonrpsee/http-client", "jsonrpsee/macros", "tokio/rt", "rand"]
+115
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
use acvm::AcirField;
2+
use serde::{Deserialize, Serialize};
3+
4+
use crate::PrintOutput;
5+
6+
use super::{
7+
layers::{self, Layer, Layering},
8+
mocker::MockForeignCallExecutor,
9+
print::PrintForeignCallExecutor,
10+
ForeignCallExecutor,
11+
};
12+
13+
#[cfg(feature = "rpc")]
14+
use super::rpc::RPCForeignCallExecutor;
15+
16+
/// A builder for [DefaultForeignCallLayers] where we can enable fields based on feature flags,
17+
/// which is easier than providing different overrides for a `new` method.
18+
#[derive(Default)]
19+
pub struct DefaultForeignCallBuilder<'a> {
20+
pub output: PrintOutput<'a>,
21+
#[cfg(feature = "rpc")]
22+
pub resolver_url: Option<String>,
23+
#[cfg(feature = "rpc")]
24+
pub root_path: Option<std::path::PathBuf>,
25+
#[cfg(feature = "rpc")]
26+
pub package_name: Option<String>,
27+
}
28+
29+
impl<'a> DefaultForeignCallBuilder<'a> {
30+
/// Override the output.
31+
pub fn with_output(mut self, output: PrintOutput<'a>) -> Self {
32+
self.output = output;
33+
self
34+
}
35+
36+
/// Compose the executor layers with [layers::Empty] as the default handler.
37+
pub fn build<F>(self) -> DefaultForeignCallLayers<'a, layers::Empty, F>
38+
where
39+
F: AcirField + Serialize + for<'de> Deserialize<'de> + 'a,
40+
{
41+
self.build_with_base(layers::Empty)
42+
}
43+
44+
/// Compose the executor layers with `base` as the default handler.
45+
pub fn build_with_base<B, F>(self, base: B) -> DefaultForeignCallLayers<'a, B, F>
46+
where
47+
F: AcirField + Serialize + for<'de> Deserialize<'de> + 'a,
48+
B: ForeignCallExecutor<F> + 'a,
49+
{
50+
let executor = {
51+
#[cfg(feature = "rpc")]
52+
{
53+
use rand::Rng;
54+
55+
base.add_layer(self.resolver_url.map(|resolver_url| {
56+
let id = rand::thread_rng().gen();
57+
RPCForeignCallExecutor::new(
58+
&resolver_url,
59+
id,
60+
self.root_path,
61+
self.package_name,
62+
)
63+
}))
64+
}
65+
#[cfg(not(feature = "rpc"))]
66+
{
67+
base
68+
}
69+
};
70+
71+
executor
72+
.add_layer(MockForeignCallExecutor::default())
73+
.add_layer(PrintForeignCallExecutor::new(self.output))
74+
}
75+
}
76+
77+
/// Facilitate static typing of layers on a base layer, so inner layers can be accessed.
78+
#[cfg(feature = "rpc")]
79+
pub type DefaultForeignCallLayers<'a, B, F> = Layer<
80+
PrintForeignCallExecutor<'a>,
81+
Layer<MockForeignCallExecutor<F>, Layer<Option<RPCForeignCallExecutor>, B>>,
82+
>;
83+
#[cfg(not(feature = "rpc"))]
84+
pub type DefaultForeignCallLayers<'a, B, F> =
85+
Layer<PrintForeignCallExecutor<'a>, Layer<MockForeignCallExecutor<F>, B>>;
86+
87+
/// Convenience constructor for code that used to create the executor this way.
88+
#[cfg(feature = "rpc")]
89+
pub struct DefaultForeignCallExecutor;
90+
91+
/// Convenience constructors for the RPC case. Non-RPC versions are not provided
92+
/// because once a crate opts into this within the workspace, everyone gets it
93+
/// even if they don't want to. For the non-RPC case we can nudge people to
94+
/// use `DefaultForeignCallBuilder` which is easier to keep flexible.
95+
#[cfg(feature = "rpc")]
96+
impl DefaultForeignCallExecutor {
97+
#[allow(clippy::new_ret_no_self)]
98+
pub fn new<'a, F>(
99+
output: PrintOutput<'a>,
100+
resolver_url: Option<&str>,
101+
root_path: Option<std::path::PathBuf>,
102+
package_name: Option<String>,
103+
) -> impl ForeignCallExecutor<F> + 'a
104+
where
105+
F: AcirField + Serialize + for<'de> Deserialize<'de> + 'a,
106+
{
107+
DefaultForeignCallBuilder {
108+
output,
109+
resolver_url: resolver_url.map(|s| s.to_string()),
110+
root_path,
111+
package_name,
112+
}
113+
.build()
114+
}
115+
}

0 commit comments

Comments
 (0)