Repository: gakonst/ethers-rs
Pull request #1265 (2022-05-14 21:31:05)

fix: all request ids start at 1
## Motivation

I came across an issue in foundry trying to deploy a contract to fantom using the official RPC `https://rpc.ftm.tools/`. A call to `eth_chainId` fails to deserialize, because the sent out request has the ID `zero`, to which the response is `{"jsonrpc":"2.0","id":null,"result":"0xfa"}`. For some reason the server fails to unmarshall the request ID and responds with a `null` ID. When sending a request with a non-zero ID, everything works fine. Strictly speaking, this is an error on their side, since nothing in the JSON-RPC forbids a zero ID and I am not entirely sure why this occurs, since go-opera uses the `rpc` package straight from the go-ethereum repo and I don't think geth exhibits this behaviour. Anyways, the fix is simple and the alternative behaviour is consistent with other eth libraries like ethers-js.

## Solution

All request IDs for all transports start at 1.

## PR Checklist

- [ ] Added Tests
- [ ] Added Documentation
- [ ] Updated the changelog



Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 8fdbc201..458ac4c7 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1401,6 +1401,7 @@ dependencies = [
 name = "ethers-solc"
 version = "0.3.0"
 dependencies = [
+ "cfg-if 1.0.0",
  "colored",
  "criterion",
  "dunce",
diff --git a/ethers-providers/src/transports/common.rs b/ethers-providers/src/transports/common.rs
index 483114cd..ad8fa665 100644
--- a/ethers-providers/src/transports/common.rs
+++ b/ethers-providers/src/transports/common.rs
@@ -212,6 +212,8 @@ impl fmt::Display for Authorization {
 
 #[cfg(test)]
 mod tests {
+    use ethers_core::types::U64;
+
     use super::*;
 
     #[test]
@@ -247,10 +249,28 @@ mod tests {
             }
             _ => panic!("expected `Error` response"),
         }
+
+        let response: Response<'_> =
+            serde_json::from_str(r#"{"jsonrpc":"2.0","result":"0xfa","id":0}"#).unwrap();
+
+        match response {
+            Response::Success { id, result } => {
+                assert_eq!(id, 0);
+                let result: U64 = serde_json::from_str(result.get()).unwrap();
+                assert_eq!(result.as_u64(), 250);
+            }
+            _ => panic!("expected `Success` response"),
+        }
     }
 
     #[test]
     fn ser_request() {
+        let request: Request<()> = Request::new(0, "eth_chainId", ());
+        assert_eq!(
+            &serde_json::to_string(&request).unwrap(),
+            r#"{"id":0,"jsonrpc":"2.0","method":"eth_chainId"}"#
+        );
+
         let request: Request<()> = Request::new(300, "method_name", ());
         assert_eq!(
             &serde_json::to_string(&request).unwrap(),
diff --git a/ethers-providers/src/transports/http.rs b/ethers-providers/src/transports/http.rs
index 56bdf851..242126d8 100644
--- a/ethers-providers/src/transports/http.rs
+++ b/ethers-providers/src/transports/http.rs
@@ -149,7 +149,7 @@ impl Provider {
     /// let provider = Http::new_with_client(url, client);
     /// ```
     pub fn new_with_client(url: impl Into<Url>, client: reqwest::Client) -> Self {
-        Self { id: AtomicU64::new(0), client, url: url.into() }
+        Self { id: AtomicU64::new(1), client, url: url.into() }
     }
 }
 
@@ -164,7 +164,7 @@ impl FromStr for Provider {
 
 impl Clone for Provider {
     fn clone(&self) -> Self {
-        Self { id: AtomicU64::new(0), client: self.client.clone(), url: self.url.clone() }
+        Self { id: AtomicU64::new(1), client: self.client.clone(), url: self.url.clone() }
     }
 }
 
diff --git a/ethers-providers/src/transports/ws.rs b/ethers-providers/src/transports/ws.rs
index 53039b6a..99e4a406 100644
--- a/ethers-providers/src/transports/ws.rs
+++ b/ethers-providers/src/transports/ws.rs
@@ -114,7 +114,7 @@ impl Ws {
         // Spawn the server
         WsServer::new(ws, stream).spawn();
 
-        Self { id: Arc::new(AtomicU64::new(0)), instructions: sink }
+        Self { id: Arc::new(AtomicU64::new(1)), instructions: sink }
     }
 
     /// Returns true if the WS connection is active, false otherwise
diff --git a/ethers-solc/Cargo.toml b/ethers-solc/Cargo.toml
index a184ea32..c0acee2a 100644
--- a/ethers-solc/Cargo.toml
+++ b/ethers-solc/Cargo.toml
@@ -19,7 +19,7 @@ serde_json = "1.0.68"
 serde = { version = "1.0.130", features = ["derive"] }
 semver = { version = "1.0.9", features = ["serde"] }
 walkdir = "2.3.2"
-tokio = { version = "1.15.0", default-features = false, features = ["process", "io-util", "fs", "time"], optional = true }
+tokio = { version = "1.15.0", default-features = false, features = ["rt"] }
 futures-util = { version = "^0.3", optional = true }
 once_cell = "1.10.0"
 regex = "1.5.5"
@@ -39,6 +39,7 @@ solang-parser = { default-features = false, version = "=0.1.13" }
 rayon = "1.5.2"
 rand = { version = "0.8.5", optional = true }
 path-slash = "0.1.4"
+cfg-if = "1.0.0"
 
 [target.'cfg(not(target_arch = "wasm32"))'.dependencies]
 home = "0.5.3"
@@ -80,7 +81,7 @@ required-features = ["full", "project-util"]
 
 [features]
 default = ["rustls"]
-async = ["tokio", "futures-util"]
+async = ["tokio/process", "tokio/io-util", "tokio/fs", "tokio/time", "futures-util"]
 full = ["async", "svm-solc"]
 svm-solc = ["svm/blocking", "svm-builds", "sha2"]
 # Utilities for creating and testing project workspaces
diff --git a/ethers-solc/src/compile/mod.rs b/ethers-solc/src/compile/mod.rs
index 384f9c75..92e3b9a9 100644
--- a/ethers-solc/src/compile/mod.rs
+++ b/ethers-solc/src/compile/mod.rs
@@ -371,9 +371,21 @@ impl Solc {
     /// Blocking version of `Self::install`
     #[cfg(all(feature = "svm-solc"))]
     pub fn blocking_install(version: &Version) -> std::result::Result<Self, svm::SolcVmError> {
+        use crate::utils::RuntimeOrHandle;
+
         tracing::trace!("blocking installing solc version \"{}\"", version);
         crate::report::solc_installation_start(version);
-        match svm::blocking_install(version) {
+        // the async version `svm::install` is used instead of `svm::blocking_intsall`
+        // because the underlying `reqwest::blocking::Client` does not behave well
+        // in tokio rt. see https://github.com/seanmonstar/reqwest/issues/1017
+        cfg_if::cfg_if! {
+            if #[cfg(target_arch = "wasm32")] {
+                let installation = svm::blocking_install(version);
+            } else {
+                let installation = RuntimeOrHandle::new().block_on(svm::install(version));
+            }
+        };
+        match installation {
             Ok(path) => {
                 crate::report::solc_installation_success(version);
                 Ok(Solc::new(path))
@@ -723,6 +735,7 @@ mod tests {
         let other = solc().async_compile(&serde_json::json!(input)).await.unwrap();
         assert_eq!(out, other);
     }
+
     #[cfg(feature = "async")]
     #[tokio::test]
     async fn async_solc_compile_works2() {
@@ -799,6 +812,15 @@ mod tests {
         assert_eq!(res.solc, expected);
     }
 
+    #[test]
+    #[cfg(feature = "svm-solc")]
+    fn can_install_solc_in_tokio_rt() {
+        let version = Version::from_str("0.8.6").unwrap();
+        let rt = tokio::runtime::Runtime::new().unwrap();
+        let result = rt.block_on(async { Solc::blocking_install(&version) });
+        assert!(result.is_ok());
+    }
+
     #[test]
     fn does_not_find_not_installed_version() {
         let ver = "1.1.1";
diff --git a/ethers-solc/src/utils.rs b/ethers-solc/src/utils.rs
index 825107ea..a5bd3e11 100644
--- a/ethers-solc/src/utils.rs
+++ b/ethers-solc/src/utils.rs
@@ -330,6 +330,40 @@ pub(crate) fn find_fave_or_alt_path(root: impl AsRef<Path>, fave: &str, alt: &st
     p
 }
 
+#[cfg(not(target_arch = "wasm32"))]
+use tokio::runtime::{Handle, Runtime};
+
+#[cfg(not(target_arch = "wasm32"))]
+#[derive(Debug)]
+pub enum RuntimeOrHandle {
+    Runtime(Runtime),
+    Handle(Handle),
+}
+
+#[cfg(not(target_arch = "wasm32"))]
+impl Default for RuntimeOrHandle {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
+#[cfg(not(target_arch = "wasm32"))]
+impl RuntimeOrHandle {
+    pub fn new() -> RuntimeOrHandle {
+        match Handle::try_current() {
+            Ok(handle) => RuntimeOrHandle::Handle(handle),
+            Err(_) => RuntimeOrHandle::Runtime(Runtime::new().expect("Failed to start runtime")),
+        }
+    }
+
+    pub fn block_on<F: std::future::Future>(&self, f: F) -> F::Output {
+        match &self {
+            RuntimeOrHandle::Runtime(runtime) => runtime.block_on(f),
+            RuntimeOrHandle::Handle(handle) => tokio::task::block_in_place(|| handle.block_on(f)),
+        }
+    }
+}
+
 /// Creates a new named tempdir
 #[cfg(any(test, feature = "project-util"))]
 pub(crate) fn tempdir(name: &str) -> Result<tempfile::TempDir, SolcIoError> {


Test patch:
diff --git a/ethers-solc/tests/project.rs b/ethers-solc/tests/project.rs
index 5bc568e7..21698fd7 100644
--- a/ethers-solc/tests/project.rs
+++ b/ethers-solc/tests/project.rs
@@ -17,6 +17,7 @@ use ethers_solc::{
     ProjectPathsConfig, Solc, TestFileFilter,
 };
 use pretty_assertions::assert_eq;
+use semver::Version;
 
 #[allow(unused)]
 fn init_tracing() {
@@ -1329,3 +1330,53 @@ fn can_compile_model_checker_sample() {
     assert!(!compiled.has_compiler_errors());
     assert!(compiled.has_compiler_warnings());
 }
+
+fn remove_solc_if_exists(version: &Version) {
+    match Solc::find_svm_installed_version(version.to_string()).unwrap() {
+        Some(_) => svm::remove_version(&version).expect("failed to remove version"),
+        None => {}
+    };
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn can_install_solc_and_compile_version() {
+    let project = TempProject::dapptools().unwrap();
+    let version = Version::new(0, 8, 10);
+
+    project
+        .add_source(
+            "Contract",
+            format!(
+                r#"
+pragma solidity {};
+contract Contract {{ }}
+"#,
+                version.to_string()
+            ),
+        )
+        .unwrap();
+
+    remove_solc_if_exists(&version);
+
+    let compiled = project.compile().unwrap();
+    assert!(!compiled.has_compiler_errors());
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn can_install_solc_and_compile_std_json_input_async() {
+    let tmp = TempProject::dapptools_init().unwrap();
+    tmp.assert_no_errors();
+    let source = tmp.list_source_files().into_iter().find(|p| p.ends_with("Dapp.t.sol")).unwrap();
+    let input = tmp.project().standard_json_input(source).unwrap();
+    let solc = &tmp.project().solc;
+
+    assert!(input.settings.remappings.contains(&"ds-test/=lib/ds-test/src/".parse().unwrap()));
+    let input: CompilerInput = input.into();
+    assert!(input.sources.contains_key(Path::new("lib/ds-test/src/test.sol")));
+
+    remove_solc_if_exists(&solc.version().expect("failed to get version"));
+
+    let out = solc.async_compile(&input).await.unwrap();
+    assert!(!out.has_error());
+    assert!(out.sources.contains_key("lib/ds-test/src/test.sol"));
+}