From b4ee63998e2fd23ff57dd815e3ef605b290b50db Mon Sep 17 00:00:00 2001 From: Riccardo Casatta Date: Mon, 16 Sep 2024 14:33:28 +0200 Subject: [PATCH 1/9] add electrum raw test --- tests/electrum.rs | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests/electrum.rs b/tests/electrum.rs index b2bab95f9..9c6ce5b7c 100644 --- a/tests/electrum.rs +++ b/tests/electrum.rs @@ -1,4 +1,7 @@ pub mod common; +use std::io::{Read, Write}; +use std::net::TcpStream; + use common::Result; use bitcoind::bitcoincore_rpc::RpcApi; @@ -137,3 +140,38 @@ fn test_electrum() -> Result<()> { Ok(()) } + +/// Test the Electrum RPC server using an headless Electrum wallet +/// This only runs on Bitcoin (non-Liquid) mode. +#[cfg_attr(not(feature = "liquid"), test)] +#[cfg_attr(feature = "liquid", allow(dead_code))] +fn test_electrum_raw() { + // Spawn an Electrs Electrum RPC server + let (_electrum_server, electrum_addr, mut _tester) = common::init_electrum_tester().unwrap(); + std::thread::sleep(std::time::Duration::from_millis(1000)); + + let mut stream = TcpStream::connect(electrum_addr).unwrap(); + let write = "{\"jsonrpc\": \"2.0\", \"method\": \"server.version\", \"id\": 0}"; + + let s = write_and_read(&mut stream, write); + let expected = "{\"id\":0,\"jsonrpc\":\"2.0\",\"result\":[\"electrs-esplora 0.4.1\",\"1.4\"]}"; + assert_eq!(s, expected); +} + +fn write_and_read(stream: &mut TcpStream, write: &str) -> String { + stream.write_all(write.as_bytes()).unwrap(); + stream.write(b"\n").unwrap(); + stream.flush().unwrap(); + let mut result = vec![]; + loop { + let mut buf = [0u8]; + stream.read_exact(&mut buf).unwrap(); + + if buf[0] == b'\n' { + break; + } else { + result.push(buf[0]); + } + } + std::str::from_utf8(&result).unwrap().to_string() +} From 73d065489ee717a48087f4919898469e0f679b45 Mon Sep 17 00:00:00 2001 From: Riccardo Casatta Date: Mon, 16 Sep 2024 15:14:28 +0200 Subject: [PATCH 2/9] refactor out handling a single Value --- src/electrum/server.rs | 93 +++++++++++++++++++++++------------------- 1 file changed, 50 insertions(+), 43 deletions(-) diff --git a/src/electrum/server.rs b/src/electrum/server.rs index 2beae0855..ec491edf8 100644 --- a/src/electrum/server.rs +++ b/src/electrum/server.rs @@ -546,49 +546,7 @@ impl Connection { match msg { Message::Request(line) => { let cmd: Value = from_str(&line).chain_err(|| "invalid JSON format")?; - match ( - cmd.get("method"), - cmd.get("params").unwrap_or_else(|| &empty_params), - cmd.get("id"), - ) { - ( - Some(&Value::String(ref method)), - &Value::Array(ref params), - Some(ref id), - ) => { - conditionally_log_rpc_event!( - self, - json!({ - "event": "rpc request", - "id": id, - "method": method, - "params": if let Some(RpcLogging::Full) = self.rpc_logging { - json!(params) - } else { - Value::Null - } - }) - ); - - let reply = self.handle_command(method, params, id)?; - - conditionally_log_rpc_event!( - self, - json!({ - "event": "rpc response", - "method": method, - "payload_size": reply.to_string().as_bytes().len(), - "duration_micros": start_time.elapsed().as_micros(), - "id": id, - }) - ); - - self.send_values(&[reply])? - } - _ => { - bail!("invalid command: {}", cmd) - } - } + self.handle_value(cmd, &empty_params, start_time)?; } Message::PeriodicUpdate => { let values = self @@ -601,6 +559,55 @@ impl Connection { } } + fn handle_value( + &mut self, + cmd: Value, + empty_params: &Value, + start_time: Instant, + ) -> Result<()> { + Ok( + match ( + cmd.get("method"), + cmd.get("params").unwrap_or_else(|| empty_params), + cmd.get("id"), + ) { + (Some(&Value::String(ref method)), &Value::Array(ref params), Some(ref id)) => { + conditionally_log_rpc_event!( + self, + json!({ + "event": "rpc request", + "id": id, + "method": method, + "params": if let Some(RpcLogging::Full) = self.rpc_logging { + json!(params) + } else { + Value::Null + } + }) + ); + + let reply = self.handle_command(method, params, id)?; + + conditionally_log_rpc_event!( + self, + json!({ + "event": "rpc response", + "method": method, + "payload_size": reply.to_string().as_bytes().len(), + "duration_micros": start_time.elapsed().as_micros(), + "id": id, + }) + ); + + self.send_values(&[reply])? + } + _ => { + bail!("invalid command: {}", cmd) + } + }, + ) + } + fn parse_requests(mut reader: BufReader, tx: &SyncSender) -> Result<()> { loop { let mut line = Vec::::new(); From ebe3cebb91e98edc1061ebd7f1806f9e0d27b576 Mon Sep 17 00:00:00 2001 From: Riccardo Casatta Date: Mon, 16 Sep 2024 15:17:41 +0200 Subject: [PATCH 3/9] handle_value return result instead of sending --- src/electrum/server.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/electrum/server.rs b/src/electrum/server.rs index ec491edf8..cc8e2136b 100644 --- a/src/electrum/server.rs +++ b/src/electrum/server.rs @@ -546,7 +546,8 @@ impl Connection { match msg { Message::Request(line) => { let cmd: Value = from_str(&line).chain_err(|| "invalid JSON format")?; - self.handle_value(cmd, &empty_params, start_time)?; + let reply = self.handle_value(cmd, &empty_params, start_time)?; + self.send_values(&[reply])? } Message::PeriodicUpdate => { let values = self @@ -564,7 +565,7 @@ impl Connection { cmd: Value, empty_params: &Value, start_time: Instant, - ) -> Result<()> { + ) -> Result { Ok( match ( cmd.get("method"), @@ -599,7 +600,7 @@ impl Connection { }) ); - self.send_values(&[reply])? + reply } _ => { bail!("invalid command: {}", cmd) From 9cfaa43dc7e0228ba2ad2fda13e115b9f03a2988 Mon Sep 17 00:00:00 2001 From: Riccardo Casatta Date: Mon, 16 Sep 2024 15:26:10 +0200 Subject: [PATCH 4/9] handle batch requests as array --- src/electrum/server.rs | 13 +++++++++++-- tests/electrum.rs | 6 ++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/electrum/server.rs b/src/electrum/server.rs index cc8e2136b..8b8b2b93e 100644 --- a/src/electrum/server.rs +++ b/src/electrum/server.rs @@ -546,8 +546,17 @@ impl Connection { match msg { Message::Request(line) => { let cmd: Value = from_str(&line).chain_err(|| "invalid JSON format")?; - let reply = self.handle_value(cmd, &empty_params, start_time)?; - self.send_values(&[reply])? + if let Value::Array(arr) = cmd { + let mut result = Vec::with_capacity(arr.len()); + for el in arr { + let reply = self.handle_value(el, &empty_params, start_time)?; + result.push(reply) + } + self.send_values(&[Value::Array(result)])? + } else { + let reply = self.handle_value(cmd, &empty_params, start_time)?; + self.send_values(&[reply])? + } } Message::PeriodicUpdate => { let values = self diff --git a/tests/electrum.rs b/tests/electrum.rs index 9c6ce5b7c..3dacf74b3 100644 --- a/tests/electrum.rs +++ b/tests/electrum.rs @@ -156,6 +156,12 @@ fn test_electrum_raw() { let s = write_and_read(&mut stream, write); let expected = "{\"id\":0,\"jsonrpc\":\"2.0\",\"result\":[\"electrs-esplora 0.4.1\",\"1.4\"]}"; assert_eq!(s, expected); + + let write = "[{\"jsonrpc\": \"2.0\", \"method\": \"server.version\", \"id\": 0}]"; + let s = write_and_read(&mut stream, write); + let expected = + "[{\"id\":0,\"jsonrpc\":\"2.0\",\"result\":[\"electrs-esplora 0.4.1\",\"1.4\"]}]"; + assert_eq!(s, expected); } fn write_and_read(stream: &mut TcpStream, write: &str) -> String { From ce877afe9200d21ccdf00cb2fbb79ba5be0bc85e Mon Sep 17 00:00:00 2001 From: Riccardo Casatta Date: Tue, 15 Oct 2024 08:07:08 +0200 Subject: [PATCH 5/9] timer tick separately for each rpc req --- src/electrum/server.rs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/electrum/server.rs b/src/electrum/server.rs index 8b8b2b93e..ecd7f6f3f 100644 --- a/src/electrum/server.rs +++ b/src/electrum/server.rs @@ -541,7 +541,6 @@ impl Connection { let empty_params = json!([]); loop { let msg = receiver.recv().chain_err(|| "channel closed")?; - let start_time = Instant::now(); trace!("RPC {:?}", msg); match msg { Message::Request(line) => { @@ -549,12 +548,12 @@ impl Connection { if let Value::Array(arr) = cmd { let mut result = Vec::with_capacity(arr.len()); for el in arr { - let reply = self.handle_value(el, &empty_params, start_time)?; + let reply = self.handle_value(el, &empty_params)?; result.push(reply) } self.send_values(&[Value::Array(result)])? } else { - let reply = self.handle_value(cmd, &empty_params, start_time)?; + let reply = self.handle_value(cmd, &empty_params)?; self.send_values(&[reply])? } } @@ -569,12 +568,8 @@ impl Connection { } } - fn handle_value( - &mut self, - cmd: Value, - empty_params: &Value, - start_time: Instant, - ) -> Result { + fn handle_value(&mut self, cmd: Value, empty_params: &Value) -> Result { + let start_time = Instant::now(); Ok( match ( cmd.get("method"), From efb342fbf53781214caf40be2359667b1ca6a59e Mon Sep 17 00:00:00 2001 From: Riccardo Casatta Date: Fri, 1 Nov 2024 13:42:12 +0100 Subject: [PATCH 6/9] limit max limit of batch request via json array --- src/electrum/server.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/electrum/server.rs b/src/electrum/server.rs index ecd7f6f3f..d03b10b05 100644 --- a/src/electrum/server.rs +++ b/src/electrum/server.rs @@ -30,6 +30,7 @@ use crate::util::{create_socket, spawn_thread, BlockId, BoolThen, Channel, FullH const ELECTRS_VERSION: &str = env!("CARGO_PKG_VERSION"); const PROTOCOL_VERSION: ProtocolVersion = ProtocolVersion::new(1, 4); const MAX_HEADERS: usize = 2016; +const MAX_ARRAY_BATCH: usize = 20; #[cfg(feature = "electrum-discovery")] use crate::electrum::{DiscoveryManager, ServerFeatures}; @@ -546,6 +547,13 @@ impl Connection { Message::Request(line) => { let cmd: Value = from_str(&line).chain_err(|| "invalid JSON format")?; if let Value::Array(arr) = cmd { + if arr.len() > MAX_ARRAY_BATCH { + bail!( + "Too many elements in batch requests {} max:{}", + arr.len(), + MAX_ARRAY_BATCH + ); + } let mut result = Vec::with_capacity(arr.len()); for el in arr { let reply = self.handle_value(el, &empty_params)?; From 1bec7b4e0894ef1c76b079c9e430eebd93f5e2e7 Mon Sep 17 00:00:00 2001 From: Riccardo Casatta Date: Mon, 4 Nov 2024 13:43:12 +0100 Subject: [PATCH 7/9] ci: run on PR or push on new-index --- .github/workflows/rust.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index bd7647b04..4f51c0f16 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -1,6 +1,10 @@ name: CI -on: [push, pull_request] +on: + push: + branches: + - new-index + pull_request: {} jobs: test: From 885887acebd7449f11646193a141606dd3904c8f Mon Sep 17 00:00:00 2001 From: Riccardo Casatta Date: Mon, 4 Nov 2024 16:27:46 +0100 Subject: [PATCH 8/9] ignore test and execute singularly At the moment we cannot concurrently tests because the server will conflict with each other, this make the test_electrum_raw test ignored by default, and add a test in CI specifically for this test. --- .github/workflows/rust.yml | 4 ++++ tests/electrum.rs | 1 + 2 files changed, 5 insertions(+) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 4f51c0f16..b6b7ff5d6 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -45,9 +45,13 @@ jobs: - name: Tests (Bitcoin mode, REST+Electrum) run: RUST_LOG=debug cargo test + - name: Test test_electrum_raw + run: RUST_LOG=debug cargo test -- --include-ignored test_electrum_raw + - name: Tests (Liquid mode, REST) run: RUST_LOG=debug cargo test --features liquid + nix: runs-on: ubuntu-latest steps: diff --git a/tests/electrum.rs b/tests/electrum.rs index 3dacf74b3..bbb62325a 100644 --- a/tests/electrum.rs +++ b/tests/electrum.rs @@ -145,6 +145,7 @@ fn test_electrum() -> Result<()> { /// This only runs on Bitcoin (non-Liquid) mode. #[cfg_attr(not(feature = "liquid"), test)] #[cfg_attr(feature = "liquid", allow(dead_code))] +#[ignore = "must be launched singularly, otherwise conflict with the other server"] fn test_electrum_raw() { // Spawn an Electrs Electrum RPC server let (_electrum_server, electrum_addr, mut _tester) = common::init_electrum_tester().unwrap(); From 250fb9b33d990822f06c6b1ef552c0ebd137e93a Mon Sep 17 00:00:00 2001 From: Riccardo Casatta Date: Mon, 25 Nov 2024 11:44:55 +0100 Subject: [PATCH 9/9] fix test comment --- tests/electrum.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/electrum.rs b/tests/electrum.rs index bbb62325a..8bf148336 100644 --- a/tests/electrum.rs +++ b/tests/electrum.rs @@ -141,7 +141,7 @@ fn test_electrum() -> Result<()> { Ok(()) } -/// Test the Electrum RPC server using an headless Electrum wallet +/// Test the Electrum RPC server using a raw TCP socket /// This only runs on Bitcoin (non-Liquid) mode. #[cfg_attr(not(feature = "liquid"), test)] #[cfg_attr(feature = "liquid", allow(dead_code))]