Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

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

33 changes: 33 additions & 0 deletions bin/bitcoin-rs/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,39 @@ mod tests {
);
}

#[test]
fn public_listener_cli_overrides_environment_and_file() -> anyhow::Result<()> {
let config = load_file(
"--config",
"public_bind = '127.0.0.1:8080'\nrpc_max_connections = 4\npublic_max_connections = 32\n",
&[
"--public-bind",
"0.0.0.0:9090",
"--public-max-connections",
"16",
],
[
(
OsString::from("BITCOIN_RS_PUBLIC_BIND"),
OsString::from("127.0.0.1:8081"),
),
(
OsString::from("BITCOIN_RS_RPC_MAX_CONNECTIONS"),
OsString::from("8"),
),
(
OsString::from("BITCOIN_RS_PUBLIC_MAX_CONNECTIONS"),
OsString::from("24"),
),
]
.into_iter(),
)?;
assert_eq!(config.rpc.public_bind, Some("0.0.0.0:9090".parse()?));
assert_eq!(config.rpc.max_connections, 8);
assert_eq!(config.rpc.public_max_connections, 16);
Ok(())
}

#[test]
fn environment_parses_script_index() {
let config = resolved(&["bitcoin-rs"], &[("BITCOIN_RS_SCRIPTINDEX", "full")]);
Expand Down
29 changes: 29 additions & 0 deletions crates/node/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,12 @@ pub struct P2pConfig {
#[derive(Clone, Debug)]
pub struct RpcConfig {
pub bind: SocketAddr,
/// Opt-in separate public REST/Esplora listener.
pub public_bind: Option<SocketAddr>,
/// Connection budget of the shared or management listener.
pub max_connections: usize,
/// Independent connection budget of the public listener.
pub public_max_connections: usize,
pub rest: bool,
/// Optional experimental keyless BIP352 read service, off by default.
pub silent_payments: bool,
Expand Down Expand Up @@ -288,6 +294,9 @@ impl NodeConfig {
},
rpc: RpcConfig {
bind: settings.rpc.bind.unwrap_or(profile.rpc_bind),
public_bind: settings.rpc.public_bind,
max_connections: settings.rpc.max_connections.unwrap_or(128),
public_max_connections: settings.rpc.public_max_connections.unwrap_or(128),
rest: settings.rpc.rest.unwrap_or(false),
silent_payments: settings.rpc.silent_payments.unwrap_or(false),
auth: rpc_auth(settings),
Expand Down Expand Up @@ -366,6 +375,26 @@ impl NodeConfig {
self.rpc.bind.ip().is_loopback() || self.rpc.auth != Auth::GeneratedCookie,
"non-loopback RPC bind requires explicit credentials (--rpc-password or --rpc-cookie)"
);
anyhow::ensure!(
self.rpc.max_connections > 0 && self.rpc.public_max_connections > 0,
"RPC and public connection limits must be positive"
);
anyhow::ensure!(
self.rpc.public_bind.is_none_or(|address| {
let management = self.rpc.bind;
// Port zero asks the OS for an unused port. Distinct
// concrete IPs may share a port; a same-family wildcard
// overlaps every concrete address on that port.
// Cross-family dual-stack conflicts remain OS-checked at bind.
address.port() == 0
|| address.port() != management.port()
|| (address.ip() != management.ip()
&& (address.is_ipv4() != management.is_ipv4()
|| (!address.ip().is_unspecified()
&& !management.ip().is_unspecified())))
Comment on lines +391 to +394

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This rejects distinct IPv6 link-local listeners that use the same address on different interfaces because ip() drops the scope ID. Account for distinct nonzero scope IDs when checking link-local addresses.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At crates/node/src/config.rs, line 391:

<comment>This rejects distinct IPv6 link-local listeners that use the same address on different interfaces because `ip()` drops the scope ID. Account for distinct nonzero scope IDs when checking link-local addresses.</comment>

<file context>
@@ -380,10 +380,20 @@ impl NodeConfig {
+                // Cross-family dual-stack conflicts remain OS-checked at bind.
+                address.port() == 0
+                    || address.port() != management.port()
+                    || (address.ip() != management.ip()
+                        && (address.is_ipv4() != management.is_ipv4()
+                            || (!address.ip().is_unspecified()
</file context>
Suggested change
|| (address.ip() != management.ip()
&& (address.is_ipv4() != management.is_ipv4()
|| (!address.ip().is_unspecified()
&& !management.ip().is_unspecified())))
|| ((address.ip() != management.ip()
|| matches!(
(address, management),
(SocketAddr::V6(public), SocketAddr::V6(management))
if public.ip().is_unicast_link_local()
&& public.scope_id() != 0
&& management.scope_id() != 0
&& public.scope_id() != management.scope_id()
))
&& (address.is_ipv4() != management.is_ipv4()
|| (!address.ip().is_unspecified()
&& !management.ip().is_unspecified())))

}),
"public-bind and rpc-bind must not overlap"
);
Comment thread
gosunuts marked this conversation as resolved.
anyhow::ensure!(
self.storage.backend.is_compiled_in(),
"unsupported storage backend {}",
Expand Down
72 changes: 50 additions & 22 deletions crates/node/src/lifecycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@ use core::num::NonZeroU32;
use std::sync::Arc;
use std::time::Duration;

use anyhow::Result;
use anyhow::{Context as _, Result};

use bitcoin_rs_chain::{BlockBodySource, LatchReader};
use bitcoin_rs_chainstate::verify::{
CheckLevel, VerifyError, VerifyOutcome, VerifyRequest, verify_recent_blocks,
};
use bitcoin_rs_mining::MiningControl;
use bitcoin_rs_rpc::{
RpcServer,
RpcServer, RpcSurface,
context::{
ChainControl, ChainControlError, ChainHandles, Context, ContextHandles, IndexHandles,
MempoolHandles, MiningHandles, NetworkHandles,
Expand Down Expand Up @@ -59,7 +59,6 @@ mod shutdown {
/// poisons the index namespace.
pub(crate) const DRAIN_DEADLINE: Duration = Duration::from_secs(5);

const RPC_MAX_CONNECTIONS: usize = 128;
const RPC_IDLE_TIMEOUT: Duration = Duration::from_secs(30);

#[derive(Clone)]
Expand Down Expand Up @@ -159,13 +158,18 @@ fn verify_request(check_level: u8, nblocks: u32) -> VerifyRequest {
}
}

/// Binds the RPC listener without spawning a worker. Startup records the
struct RpcBindings {
context: Arc<Context>,
listeners: Vec<(RpcServer, RpcSurface)>,
}

/// Binds every RPC/HTTP listener before spawning a worker. Startup records each
/// worker immediately after spawning, so a later failure cannot detach it.
fn bind_rpc(
state: &NodeState,
mining_control: &Arc<dyn MiningControl>,
block_body_source: Arc<dyn BlockBodySource>,
) -> Result<(Arc<Context>, RpcServer)> {
) -> Result<RpcBindings> {
let config = state.config();
let rpc_auth = Arc::new(config.rpc.auth.to_rpc_auth(&config.data_dir)?);
let chainstate = state.chainstate();
Expand Down Expand Up @@ -216,13 +220,36 @@ fn bind_rpc(
let handler = Arc::new(bitcoin_rs_rpc::Handler::new(Arc::clone(&context)));
let server = RpcServer::bind(
state.config().rpc.bind,
rpc_auth,
handler,
RPC_MAX_CONNECTIONS,
Arc::clone(&rpc_auth),
Comment thread
gosunuts marked this conversation as resolved.
Arc::clone(&handler),
config.rpc.max_connections,
RPC_IDLE_TIMEOUT,
state.config().rpc.rest,
)?;
Ok((context, server))
)
.with_context(|| format!("failed to bind RPC listener at {}", config.rpc.bind))?;
let mut servers = Vec::with_capacity(2);
if let Some(address) = config.rpc.public_bind {
// RpcServer's existing binding API requires Auth. Public mode shares
// this object only for API compatibility: its closed route policy
// rejects JSON-RPC before consulting credentials, even valid ones.
let public = RpcServer::bind(
address,
rpc_auth,
handler,
config.rpc.public_max_connections,
RPC_IDLE_TIMEOUT,
config.rpc.rest,
)
.with_context(|| format!("failed to bind public HTTP listener at {address}"))?;
servers.push((server, RpcSurface::Management));
servers.push((public, RpcSurface::Public));
Comment thread
gosunuts marked this conversation as resolved.
} else {
servers.push((server, RpcSurface::Shared));
}
Ok(RpcBindings {
context,
listeners: servers,
})
}

// Records that teardown reached the bootstrap join on the current thread.
Expand Down Expand Up @@ -287,7 +314,7 @@ pub(crate) struct NodeServices {
event_loop_signal: Option<crossbeam_channel::Sender<()>>,
/// Stops and joins its listener in Drop.
metrics: Option<crate::metrics::MetricsServer>,
rpc_thread: Option<std::thread::JoinHandle<std::io::Result<()>>>,
rpc_threads: Vec<std::thread::JoinHandle<std::io::Result<()>>>,
/// Chainstate idle maintenance (journal flush, retention); joined
/// before the final clean checkpoint publication.
maintenance_worker: Option<std::thread::JoinHandle<()>>,
Expand Down Expand Up @@ -367,7 +394,7 @@ impl NodeServices {
set_first_error(first_error, anyhow::anyhow!("event loop thread panicked"));
}
}
if let Some(handle) = self.rpc_thread.take() {
for handle in self.rpc_threads.drain(..) {
match handle.join() {
Ok(Ok(())) => tracing::info!("rpc listener exited cleanly"),
// RPC listener returned an I/O error.
Expand Down Expand Up @@ -773,21 +800,22 @@ pub(crate) fn start_node(
// state.
crate::chain_effects::bootstrap_genesis(&chainstate, &state.chain_followers())
.map_err(|error| anyhow::Error::new(error).context("failed to bootstrap genesis"))?;
let (context, rpc_server) = bind_rpc(state, &mining_control, block_body_source)?;
let rpc_local_addr = rpc_server.local_addr()?;
tracing::info!(addr = %rpc_local_addr, "rpc listener bound");
let rpc_shutdown = state.shutdown_reader();
guard.services.rpc_thread = Some(
std::thread::Builder::new()
let RpcBindings { context, listeners } = bind_rpc(state, &mining_control, block_body_source)?;
for (rpc_server, surface) in listeners {
let rpc_local_addr = rpc_server.local_addr()?;
tracing::info!(addr = %rpc_local_addr, ?surface, "rpc listener bound");
let rpc_shutdown = state.shutdown_reader();
let worker = std::thread::Builder::new()
.name("bitcoin-rs-rpc".into())
.spawn(move || {
let result = rpc_server.serve_with_shutdown(rpc_shutdown.clone());
let result = rpc_server.serve_surface_with_shutdown(rpc_shutdown.clone(), surface);
Comment thread
gosunuts marked this conversation as resolved.
if !rpc_shutdown.is_triggered() {
tracing::error!(error = ?result.as_ref().err(), "rpc listener exited before shutdown");
tracing::error!(?surface, error = ?result.as_ref().err(), "rpc listener exited before shutdown");
}
result
})?,
);
})?;
guard.services.rpc_threads.push(worker);
}
let peer_ready: Arc<dyn Fn(bitcoin_rs_p2p::PeerSource) + Send + Sync> =
Arc::new(move |source: bitcoin_rs_p2p::PeerSource| {
peer_ready_sync.on_peer_ready(source);
Expand Down
12 changes: 12 additions & 0 deletions crates/node/src/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,18 @@ fn describe_node_metrics() {
TXINDEX_READINESS_GAUGE,
"txindex capability readiness from the live getcapabilities source; 1 for the active state label, 0 for the others"
);
metrics::describe_gauge!(
"rpc.connections.active",
"admitted live HTTP connections, by listener surface"
);
metrics::describe_counter!(
"rpc.connections.rejected_total",
"HTTP connection budget refusals, by listener surface"
);
metrics::describe_histogram!(
"rpc.http.request_seconds",
"HTTP time from complete request head through response, by listener surface and route family"
);
}

static PROMETHEUS_HANDLE: Mutex<Option<(EvidenceIdentity, PrometheusHandle)>> = Mutex::new(None);
Expand Down
18 changes: 18 additions & 0 deletions crates/node/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,24 @@ macro_rules! option_rows {
env["BITCOIN_RS_RPC_BIND", str::parse]
toml native("rpc_bind")
}
/// Separate public HTTP bind; when set, rpc-bind serves JSON-RPC only.
public_bind as public_bind: Option<SocketAddr> {
cli[#[arg(long = "public-bind")]]
env["BITCOIN_RS_PUBLIC_BIND", str::parse]
toml native("public_bind")
}
/// Maximum live connections on the RPC listener (default 128).
max_connections as rpc_max_connections: Option<usize> {
cli[#[arg(long = "rpc-max-connections")]]
env["BITCOIN_RS_RPC_MAX_CONNECTIONS", str::parse]
toml native("rpc_max_connections")
}
/// Maximum live connections on the optional public listener (default 128).
public_max_connections as public_max_connections: Option<usize> {
cli[#[arg(long = "public-max-connections")]]
env["BITCOIN_RS_PUBLIC_MAX_CONNECTIONS", str::parse]
toml native("public_max_connections")
}
/// Whether the REST gateway is enabled.
rest as rest: Option<bool> {
cli[#[arg(long = "rest")]]
Expand Down
65 changes: 65 additions & 0 deletions crates/node/tests/config_layered.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,71 @@ use bitcoin_rs_node::{
use bitcoin_rs_primitives::Network;
use bitcoin_rs_rpc::zmq::ZmqTopic;

#[test]
fn public_listener_is_opt_in_and_budgets_resolve_independently() -> Result<()> {
let defaults = NodeConfig::default_for_network(Network::Regtest);
assert_eq!(defaults.rpc.public_bind, None);
assert_eq!(defaults.rpc.max_connections, 128);
assert_eq!(defaults.rpc.public_max_connections, 128);
let file: UserConfig = toml::from_str(
"public_bind = '0.0.0.0:8080'\nrpc_max_connections = 8\npublic_max_connections = 32",
)?;
let mut environment = UserConfig::default();
environment.apply_env(
"BITCOIN_RS_PUBLIC_BIND",
std::ffi::OsStr::new("127.0.0.1:9090"),
)?;
environment.apply_env(
"BITCOIN_RS_PUBLIC_MAX_CONNECTIONS",
std::ffi::OsStr::new("16"),
)?;
let config = resolve(&[&file, &environment])?;
assert_eq!(config.rpc.public_bind, Some("127.0.0.1:9090".parse()?));
assert_eq!(config.rpc.max_connections, 8);
assert_eq!(config.rpc.public_max_connections, 16);
for invalid in [
"rpc_max_connections = 0",
"public_max_connections = 0",
"rpc_bind = '127.0.0.1:8332'\npublic_bind = '127.0.0.1:8332'",
] {
let layer: UserConfig = toml::from_str(invalid)?;
assert!(resolve(&[&layer]).is_err(), "{invalid}");
}
Ok(())
}

#[test]
fn public_listener_bind_validation_handles_wildcards_and_ephemeral_ports() -> Result<()> {
for (management, public, valid) in [
("127.0.0.1:8332", "127.0.0.1:8332", false),
("0.0.0.0:8332", "127.0.0.1:8332", false),
("127.0.0.1:8332", "0.0.0.0:8332", false),
("[::]:8332", "[::1]:8332", false),
("[::1]:8332", "[::]:8332", false),
("127.0.0.1:8332", "127.0.0.2:8332", true),
("127.0.0.1:8332", "127.0.0.1:8080", true),
("0.0.0.0:0", "0.0.0.0:0", true),
("127.0.0.1:0", "127.0.0.1:8332", true),
("127.0.0.1:8332", "127.0.0.1:0", true),
] {
let mut config = NodeConfig::default_for_network(Network::Regtest);
config.rpc.bind = management.parse()?;
config.rpc.public_bind = Some(public.parse()?);
config.rpc.auth = bitcoin_rs_node::Auth::basic("operator", "secret");
let result = config.validate();
assert_eq!(result.is_ok(), valid, "{management}, {public}: {result:?}");
if let Err(error) = result {
assert!(error.to_string().contains("must not overlap"), "{error}");
}
}
// Both sockets stay bound: the second port-zero bind must select a
// different available port rather than reuse the first listener's port.
let management = std::net::TcpListener::bind("0.0.0.0:0")?;
let public = std::net::TcpListener::bind("0.0.0.0:0")?;
assert_ne!(management.local_addr()?.port(), public.local_addr()?.port());
Ok(())
}

#[test]
fn silent_payments_toml_overlay_preserves_explicit_disable() -> Result<()> {
assert!(
Expand Down
Loading
Loading