tests(rpc): Add some RPC acceptance tests (#3641)

* tests(rpc): add an rpc endpoint test

* tests(rpc): add an rpc port conflict test

* tests(rpc): refactor some imports

* tests(rpc): fix failures, make test more complete

* tests(rpc): parse json response for better coverage

* tests(rpc): change request

* tests(rpc): wait until port is open in rpc_endpoint test

* tests(rpc): add a delay between launching 2 nodes

* tests(rpc): try 5 seconds

* refactor(rpc): open rpc server faster

* tests(rpc): extend `LAUNCH_DELAY` to 15 seconds

* fix(rpc): disable rpc_conflict test for windows

* fix(ci): skip the RPC tests if the network is disabled

* rustfmt

* fix(zebrad/test): test function return type

* tests(rpc): print server output in assert

* fix(rpc): fix acceptance test looking for string in `build` field

* fix(rpc): reduce the number of acceptable characters in version output

Co-authored-by: teor <teor@riseup.net>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

Cargo.lock

@@ -5915,6 +5915,7 @@ dependencies = [
  "sentry",
  "sentry-tracing",
  "serde",
+ "serde_json",
  "tempfile",
  "thiserror",
  "tokio",

zebrad/Cargo.toml

@@ -61,6 +61,7 @@ abscissa_core = { version = "0.5", features = ["testing"] }
 once_cell = "1.9"
 regex = "1.5.4"
 semver = "1.0.6"
+serde_json = "1.0"
 tempfile = "3.3.0"
 tokio = { version = "1.16.1", features = ["full", "test-util"] }
 

zebrad/src/commands/start.rs

@@ -154,6 +154,9 @@ impl StartCmd {
             .buffer(mempool::downloads::MAX_INBOUND_CONCURRENCY)
             .service(mempool);
 
+        // Launch RPC server
+        let rpc_task_handle = RpcServer::spawn(config.rpc, app_version().to_string());
+
         let setup_data = InboundSetupData {
             address_book,
             block_download_peer_set: peer_set.clone(),
@@ -197,19 +200,17 @@ impl StartCmd {
                 .in_current_span(),
         );
 
-        let rpc_task_handle = RpcServer::spawn(config.rpc, app_version().to_string());
-
         info!("spawned initial Zebra tasks");
 
         // TODO: put tasks into an ongoing FuturesUnordered and a startup FuturesUnordered?
 
         // ongoing tasks
+        pin!(rpc_task_handle);
         pin!(syncer_task_handle);
         pin!(mempool_crawler_task_handle);
         pin!(mempool_queue_checker_task_handle);
         pin!(tx_gossip_task_handle);
         pin!(progress_task_handle);
-        pin!(rpc_task_handle);
 
         // startup tasks
         let groth16_download_handle_fused = (&mut groth16_download_handle).fuse();
@@ -220,6 +221,13 @@ impl StartCmd {
             let mut exit_when_task_finishes = true;
 
             let result = select! {
+                rpc_result = &mut rpc_task_handle => {
+                    rpc_result
+                        .expect("unexpected panic in the rpc task");
+                    info!("rpc task exited");
+                    Ok(())
+                }
+
                 sync_result = &mut syncer_task_handle => sync_result
                     .expect("unexpected panic in the syncer task")
                     .map(|_| info!("syncer task exited")),
@@ -251,13 +259,6 @@ impl StartCmd {
                     Ok(())
                 }
 
-                rpc_result = &mut rpc_task_handle => {
-                    rpc_result
-                        .expect("unexpected panic in the rpc task");
-                    info!("rpc task exited");
-                    Ok(())
-                }
-
                 // Unlike other tasks, we expect the download task to finish while Zebra is running.
                 groth16_download_result = &mut groth16_download_handle_fused => {
                     groth16_download_result

zebrad/tests/acceptance.rs

@@ -52,7 +52,11 @@ use zebrad::{
 ///
 /// Previously, this value was 3 seconds, which caused rare
 /// metrics or tracing test failures in Windows CI.
-const LAUNCH_DELAY: Duration = Duration::from_secs(10);
+const LAUNCH_DELAY: Duration = Duration::from_secs(15);
+
+/// The amount of time we wait between launching two
+/// conflicting nodes.
+const BETWEEN_NODES_DELAY: Duration = Duration::from_secs(2);
 
 /// Returns a config with:
 /// - a Zcash listener on an unused port on IPv4 localhost, and
@@ -1540,7 +1544,74 @@ async fn tracing_endpoint() -> Result<()> {
     Ok(())
 }
 
-// TODO: RPC endpoint and port conflict tests (#3165)
+#[tokio::test]
+async fn rpc_endpoint() -> Result<()> {
+    use hyper::{body::to_bytes, Body, Client, Method, Request};
+    use serde_json::Value;
+
+    zebra_test::init();
+    if zebra_test::net::zebra_skip_network_tests() {
+        return Ok(());
+    }
+
+    // [Note on port conflict](#Note on port conflict)
+    let port = random_known_port();
+    let endpoint = format!("127.0.0.1:{}", port);
+    let url = format!("http://{}", endpoint);
+
+    // Write a configuration that has RPC listen_addr set
+    let mut config = default_test_config()?;
+    config.rpc.listen_addr = Some(endpoint.parse().unwrap());
+
+    let dir = testdir()?.with_config(&mut config)?;
+    let mut child = dir.spawn_child(&["start"])?;
+
+    // Wait until port is open.
+    child.expect_stdout_line_matches(format!("Opened RPC endpoint at {}", endpoint).as_str())?;
+
+    // Create an http client
+    let client = Client::new();
+
+    // Create a request to call `getinfo` RPC method
+    let req = Request::builder()
+        .method(Method::POST)
+        .uri(url)
+        .header("content-type", "application/json")
+        .body(Body::from(
+            r#"{"jsonrpc":"1.0","method":"getinfo","params":[],"id":123}"#,
+        ))?;
+
+    // Make the call to the RPC endpoint
+    let res = client.request(req).await?;
+
+    // Test rpc endpoint response
+    assert!(res.status().is_success());
+
+    let body = to_bytes(res).await;
+    let (body, mut child) = child.kill_on_error(body)?;
+
+    let parsed: Value = serde_json::from_slice(&body)?;
+
+    // Check that we have at least 4 characters in the `build` field.
+    let build = parsed["result"]["build"].as_str().unwrap();
+    assert!(build.len() > 4, "Got {}", build);
+
+    // Check that the `subversion` field has "Zebra" in it.
+    let subversion = parsed["result"]["subversion"].as_str().unwrap();
+    assert!(subversion.contains("Zebra"), "Got {}", subversion);
+
+    child.kill()?;
+
+    let output = child.wait_with_output()?;
+    let output = output.assert_failure()?;
+
+    // [Note on port conflict](#Note on port conflict)
+    output
+        .assert_was_killed()
+        .wrap_err("Possible port conflict. Are there other acceptance tests running?")?;
+
+    Ok(())
+}
 
 /// Launch `zebrad` with an RPC port, and make sure `lightwalletd` works with Zebra.
 ///
@@ -1736,6 +1807,34 @@ fn zebra_tracing_conflict() -> Result<()> {
     Ok(())
 }
 
+/// Start 2 zebrad nodes using the same RPC listener port, but different
+/// state directories and Zcash listener ports. The first node should get
+/// exclusive use of the port. The second node will panic.
+#[test]
+#[cfg(not(target_os = "windows"))]
+fn zebra_rpc_conflict() -> Result<()> {
+    zebra_test::init();
+
+    // [Note on port conflict](#Note on port conflict)
+    let port = random_known_port();
+    let listen_addr = format!("127.0.0.1:{}", port);
+
+    // Write a configuration that has our created RPC listen_addr
+    let mut config = default_test_config()?;
+    config.rpc.listen_addr = Some(listen_addr.parse().unwrap());
+    let dir1 = testdir()?.with_config(&mut config)?;
+    let regex1 = regex::escape(&format!(r"Opened RPC endpoint at {}", listen_addr));
+
+    // From another folder create a configuration with the same endpoint.
+    // `rpc.listen_addr` will be the same in the 2 nodes.
+    // But they will have different Zcash listeners (auto port) and states (ephemeral)
+    let dir2 = testdir()?.with_config(&mut config)?;
+
+    check_config_conflict(dir1, regex1.as_str(), dir2, "Unable to start RPC server")?;
+
+    Ok(())
+}
+
 /// Start 2 zebrad nodes using the same state directory, but different Zcash
 /// listener ports. The first node should get exclusive access to the database.
 /// The second node will panic with the Zcash state conflict hint added in #1535.
@@ -1798,6 +1897,9 @@ where
     // Wait until node1 has used the conflicting resource.
     node1.expect_stdout_line_matches(first_stdout_regex)?;
 
+    // Wait a bit before launching the second node.
+    std::thread::sleep(BETWEEN_NODES_DELAY);
+
     // Spawn the second node
     let node2 = second_dir.spawn_child(&["start"]);
     let (node2, mut node1) = node1.kill_on_error(node2)?;