server.features should not be tied to peer discovery (returns -32603 when --electrum-public-hosts unset)

[EddieHouston]

Summary

server.features is currently wired through the optional peer-discovery subsystem, so it returns a JSON-RPC -32603 ("discovery is disabled") internal error whenever --electrum-public-hosts is not configured. It would be more protocol-correct (and consistent with upstream romanz/electrs) to serve server.features independently of discovery.

Current behavior

server.features is gated behind the electrum-discovery feature and reads its payload from the DiscoveryManager:

#[cfg(feature = "electrum-discovery")]
fn server_features(&self) -> Result<Value> {
    let discovery = self
        .discovery
        .as_ref()
        .chain_err(|| "discovery is disabled")?;
    Ok(json!(discovery.our_features()))
}

When electrum-discovery is built but --electrum-public-hosts is unset, self.discovery is None, so a normal client calling server.features (e.g. for a genesis-hash / protocol-version compatibility check) receives:

-32603  "discovery is disabled"

-32603 is InternalError, which is misleading for what is a valid, standard request.

Why this is a problem

server.features is general server-info metadata, not a discovery operation — clients call it to check chain/protocol compatibility before anything else. Of the fields in ServerFeatures, only hosts depends on discovery config; everything else is always available:

Field	Source	Needs discovery config?
genesis_hash	genesis_hash(network_type)	no
server_version	constant	no
protocol_min / protocol_max	PROTOCOL_VERSION	no
hash_function	"sha256"	no
pruning	None	no
hosts	--electrum-public-hosts	yes

The Electrum protocol explicitly allows hosts to be an empty object, so server.features can return everything else with empty hosts when discovery is unconfigured. This is what upstream romanz/electrs does — it serves server.features unconditionally.

Proposed change

1. Construct the ServerFeatures payload from config (currently built inline inside the discovery .map()), defaulting hosts to empty when --electrum-public-hosts is unset.
2. Have server.features return that payload directly instead of going through self.discovery, and ungate it from #[cfg(feature = "electrum-discovery")] so it works in plain Bitcoin builds.
3. Pass the same payload into DiscoveryManager::new() so there is a single source of truth.

server.add_peer should stay tied to discovery (it genuinely needs the DiscoveryManager). As a minor follow-up, when discovery is disabled it could return -32601 (method not found) rather than -32603, since "this server does not accept peers" is closer to method-not-found than an internal error.

[EddieHouston]

For reference: mempool/electrs (the fork this repo's JSON-RPC 2.0 error objects were ported from, in mempool/electrs#103) has already implemented exactly this decoupling, so there's a proven reference rather than a novel design.

In their src/electrum/server.rs, ServerFeatures is stored as a field on the connection (built once at startup from config, with hosts taken from electrum_public_hosts), and server.features just returns it unconditionally:

fn server_features(&self) -> Result<Value> {
    Ok(json!(self.server_features.as_ref()))
}

It is always present in the dispatch match and never returns an error — so there's no -32603 "discovery is disabled" for server.features.

server.add_peer stays gated on the electrum-discovery feature / DiscoveryManager (as proposed here):

• built without electrum-discovery → falls through to the &_ arm → -32601 method not found
• built with discovery but electrum_public_hosts unset → -32603 "discovery is disabled"

Cross-fork comparison:

Method	romanz/electrs	mempool/electrs	Blockstream/electrs (current)
server.features (no hosts)	success	success	-32603 "discovery is disabled"
server.add_peer (no discovery)	n/a (no such method)	-32601 / -32603	-32603

So the fix is essentially porting mempool's server_features field pattern — mirroring how mempool/electrs#103 was ported.

[EddieHouston]

Direct reference implementation: mempool/electrs has since landed this exact change in mempool/electrs#150 ("feat[electrum]: Support server.features on all feature combinations", closes mempool/electrs#149). It's a small diff (+22/-19) across src/config.rs and src/electrum/server.rs and matches the proposal above:

1. config.rs — move electrum_public_hosts out of #[cfg(feature = "electrum-discovery")] so the field always exists (defaults to None when the feature is off).
2. server.rs — build a server_features: Arc<ServerFeatures> once at RPC startup with hosts = electrum_public_hosts.unwrap_or_default() (empty map when unconfigured), and thread it into each Connection.
3. server_features() becomes Ok(json!(self.server_features.as_ref())), and the dispatch arm is ungated so it works in all build configurations.
4. Discovery still receives the same struct (server_features.as_ref().clone()); server.add_peer stays gated on the discovery feature.

So this is a ready-to-port reference, same as how the JSON-RPC 2.0 error objects were ported from mempool/electrs#103.

One nuance when porting: mempool/electrs#150 sets server_version from VERSION_STRING, whereas this repo currently constructs it inline as format!("electrs-esplora {}", ELECTRS_VERSION) — that existing string should be preserved rather than adopting mempool's value verbatim.