fix: wallet receive reliability, bitcoin install self-heal, ElectrumX app tile

Fixes three Bitcoin/wallet failures observed across the fleet on v1.7.90-alpha
(all nodes were already on the latest build — these were live bugs, not stale
builds), plus the missing ElectrumX tile, and adds automated coverage so each
can't regress silently.

Receive address (".116 receive fails", ".228 false 'wallet is locked'"):
- LND publishes its REST API on a host port that can drift from the manifest
  (a container created when the mapping was 8080 kept publishing 8080 after the
  manifest moved to 18080). The in-process client connects to the manifest port,
  gets connection-refused, and wallet init fails forever while the container
  looks "Up". Add published-port drift detection to the reconciler
  (container_ports_drifted / host_port_bindings_drifted) that recreates a
  drifted backend even for restart-sensitive apps — a drifted container is
  already broken, so leaving it "untouched" only perpetuates the failure.
- Receive errors now carry a stable [CODE] token (REST_UNREACHABLE, WALLET_LOCKED,
  WALLET_UNINITIALIZED, SYNCING) and always start with "Bitcoin address" so they
  survive the RPC error sanitizer instead of collapsing to the generic
  "Operation failed". The UI maps the code instead of guessing wallet state from
  substrings — so an unreachable REST endpoint is no longer mislabelled "locked".

Bitcoin install (".198 bitcoin gone / reinstall just stops"):
- bitcoin-knots requires the secret bitcoin-rpc-txrelay-rpcauth, which was only
  generated by the tx-relay flow. Nodes that never used tx-relay lacked it, so
  secret resolution hard-failed and the whole Bitcoin stack cascaded. Generate
  it idempotently before bitcoin starts (ensure_app_secrets, reusing
  ensure_txrelay_credentials), and name the missing secret in the error so a
  genuine gap is actionable instead of a bare "IO error".

ElectrumX app tile missing on every node with it installed:
- The catalog generator dropped electrumx because the manifest had no
  interfaces.main block, so the tile had no launch URL and was hidden. Declare
  the companion UI port (50002) in the manifest, regenerate the catalog, and let
  an app with a known launch URL stay launchable while its backend is still
  "starting" (ElectrumX indexes for 10m+).

Test harness:
- New lifecycle bats suites: bitcoin-receive, port-drift, secret-completeness
  (validated live; port-drift catches the real .116 drift).
- Rust unit tests for drift detection, the receive reason-code classifier, and
  the named-missing-secret error; vitest for the UI code mapping.
- create-release.sh now runs tests/release/run.sh and aborts the release on
  failure — previously it ran no tests at all.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
archipelago
2026-06-14 03:12:56 -04:00
co-authored by Claude Opus 4.8
parent bb808df89a
commit 0ed892a412
15 changed files with 733 additions and 22 deletions
+1 -1
View File
@@ -1,4 +1,4 @@
mod handler;
mod rpc;
pub(crate) mod rpc;
pub use handler::ApiHandler;
@@ -107,7 +107,7 @@ struct TrustedRelayPeer {
}
#[derive(Debug, Clone)]
struct TxRelayCredentials {
pub(crate) struct TxRelayCredentials {
username: String,
password: String,
}
@@ -648,7 +648,13 @@ async fn txrelay_credentials_available(data_dir: &Path) -> bool {
&& fs::metadata(&client_env_path).await.is_ok()
}
async fn ensure_txrelay_credentials(data_dir: &Path) -> Result<TxRelayCredentials> {
/// Idempotently ensure the tx-relay credential trio exists in the secrets dir:
/// the random password, its derived `rpcauth` line, and the client env file.
/// Bitcoin backend manifests reference `bitcoin-rpc-txrelay-rpcauth` as a
/// required `secret_env`, so this must run before bitcoind starts — otherwise
/// secret resolution hard-fails and the whole Bitcoin stack cascades (the .198
/// failure). Safe to call repeatedly; it only writes what's missing or stale.
pub(crate) async fn ensure_txrelay_credentials(data_dir: &Path) -> Result<TxRelayCredentials> {
let (password_path, rpcauth_path, client_env_path) = txrelay_secret_paths(data_dir);
let password = match read_trimmed(&password_path).await {
Some(value) => value,
+144 -12
View File
@@ -9,9 +9,15 @@ use super::LND_REST_BASE_URL;
impl RpcHandler {
/// Generate a new on-chain Bitcoin address.
pub(in crate::api::rpc) async fn handle_lnd_newaddress(&self) -> Result<serde_json::Value> {
let (client, macaroon_hex) = self.lnd_client().await?;
let (client, macaroon_hex) = self.lnd_client().await.map_err(|e| {
tracing::warn!(error = %format!("{e:#}"), "LND newaddress: client/macaroon unavailable");
receive_error(
RECEIVE_WALLET_UNINITIALIZED,
"The Lightning wallet isn't set up on this node yet. Finish wallet setup, then try again.",
)
})?;
let resp = client
let resp = match client
.get(format!("{LND_REST_BASE_URL}/v1/newaddress"))
// LND's REST gateway parses `type` as the AddressType enum by its
// proto name (or integer), NOT the lncli aliases. "p2wkh" is not a
@@ -21,13 +27,26 @@ impl RpcHandler {
.header("Grpc-Metadata-macaroon", &macaroon_hex)
.send()
.await
.context("LND REST connection failed")?;
{
Ok(resp) => resp,
Err(e) => {
// The .116 case: LND container is up but its REST endpoint isn't
// reachable on the expected port (e.g. published-port drift), so
// the connection is refused. This is NOT a locked wallet — emit a
// distinct code so the UI stops mislabelling it.
tracing::warn!(error = %format!("{e:#}"), "LND newaddress: REST connection failed");
return Err(receive_error(
RECEIVE_REST_UNREACHABLE,
"The Lightning wallet service isn't reachable yet. It may be starting up or recovering — please try again in a moment.",
));
}
};
let status = resp.status();
let raw_body = resp
.text()
.await
.context("LND address response could not be read")?;
.context("Bitcoin address response could not be read")?;
let body: serde_json::Value = serde_json::from_str(&raw_body).unwrap_or_else(|_| {
serde_json::json!({
"raw": raw_body,
@@ -36,11 +55,9 @@ impl RpcHandler {
if !status.is_success() {
let message = lnd_error_message(&body);
anyhow::bail!(
"Bitcoin address generation failed ({}): {}",
status,
message
);
let code = classify_lnd_address_error(&message);
tracing::warn!(%status, lnd_message = %message, code, "LND newaddress returned an error");
return Err(receive_error(code, default_receive_detail(code)));
}
if let Some(error) = body
@@ -48,14 +65,21 @@ impl RpcHandler {
.or_else(|| body.get("message"))
.and_then(|v| v.as_str())
{
anyhow::bail!("Bitcoin address generation failed: {}", error);
let code = classify_lnd_address_error(error);
tracing::warn!(lnd_message = %error, code, "LND newaddress returned an error body");
return Err(receive_error(code, default_receive_detail(code)));
}
let address = body
.get("address")
.and_then(|v| v.as_str())
.filter(|addr| !addr.trim().is_empty())
.ok_or_else(|| anyhow::anyhow!("Bitcoin address generation failed: LND did not return a Bitcoin address. The wallet may still be locked, uninitialized, or waiting for Bitcoin to sync."))?
.ok_or_else(|| {
receive_error(
RECEIVE_WALLET_UNINITIALIZED,
"The wallet didn't return an address yet. It may still be unlocking or waiting for Bitcoin to sync — please try again shortly.",
)
})?
.to_string();
Ok(serde_json::json!({ "address": address }))
@@ -583,9 +607,75 @@ fn lnd_error_message(body: &serde_json::Value) -> String {
.to_string()
}
// Stable, machine-readable reason codes for receive-address failures. They are
// embedded in the error message as a `[CODE]` token so the frontend
// (neode-ui/src/utils/bitcoinReceive.ts) can show an accurate explanation
// instead of guessing wallet state by substring-matching — which is what made
// .228 report "wallet is locked" when LND's REST was merely unreachable.
//
// Every receive error string starts with "Bitcoin address" so it survives the
// RPC error sanitizer (api/rpc/middleware.rs) unchanged rather than being
// flattened to the generic "Operation failed" message (the .116 symptom).
pub(crate) const RECEIVE_REST_UNREACHABLE: &str = "LND_REST_UNREACHABLE";
pub(crate) const RECEIVE_WALLET_LOCKED: &str = "LND_WALLET_LOCKED";
pub(crate) const RECEIVE_WALLET_UNINITIALIZED: &str = "LND_WALLET_UNINITIALIZED";
pub(crate) const RECEIVE_SYNCING: &str = "LND_SYNCING";
pub(crate) const RECEIVE_LND_ERROR: &str = "LND_ERROR";
/// Build a receive-address error carrying a reason code the UI can map.
fn receive_error(code: &str, detail: &str) -> anyhow::Error {
anyhow::anyhow!("Bitcoin address unavailable [{code}]: {detail}")
}
/// A sensible default human message per code (used for non-UI callers and logs;
/// the frontend renders its own copy from the code).
fn default_receive_detail(code: &str) -> &'static str {
match code {
RECEIVE_REST_UNREACHABLE => {
"The Lightning wallet service isn't reachable yet. It may be starting up or recovering — please try again in a moment."
}
RECEIVE_WALLET_LOCKED => {
"The Lightning wallet is locked. Unlock it (or finish wallet setup), then try again."
}
RECEIVE_WALLET_UNINITIALIZED => {
"The Lightning wallet isn't set up yet. Finish wallet setup, then try again."
}
RECEIVE_SYNCING => {
"The wallet is still syncing with the Bitcoin network. Please try again once it has caught up."
}
_ => "Couldn't generate a Bitcoin address right now. Please try again shortly.",
}
}
/// Classify a non-2xx LND error body/message into a reason code. The wording of
/// LND's REST errors is stable enough to bucket: a locked wallet, an
/// uninitialized wallet, a syncing chain, or some other failure.
fn classify_lnd_address_error(message: &str) -> &'static str {
let m = message.to_lowercase();
if m.contains("locked") || m.contains("unlock") {
RECEIVE_WALLET_LOCKED
} else if m.contains("synchroniz")
|| m.contains("syncing")
|| m.contains("not yet ready")
|| m.contains("in the process of starting")
{
RECEIVE_SYNCING
} else if m.contains("wallet not found")
|| m.contains("not exist")
|| m.contains("uninitialized")
|| m.contains("not initialized")
|| m.contains("create a wallet")
|| m.contains("no wallet")
{
RECEIVE_WALLET_UNINITIALIZED
} else {
RECEIVE_LND_ERROR
}
}
#[cfg(test)]
mod tests {
use super::lnd_error_message;
use super::*;
#[test]
fn lnd_error_message_prefers_message_field() {
@@ -604,4 +694,46 @@ mod tests {
"unknown LND error"
);
}
#[test]
fn classify_locked_wallet() {
assert_eq!(
classify_lnd_address_error("wallet locked, please unlock"),
RECEIVE_WALLET_LOCKED
);
}
#[test]
fn classify_uninitialized_wallet() {
assert_eq!(
classify_lnd_address_error("wallet not found, create a wallet first"),
RECEIVE_WALLET_UNINITIALIZED
);
}
#[test]
fn classify_syncing() {
assert_eq!(
classify_lnd_address_error("server is still in the process of starting"),
RECEIVE_SYNCING
);
}
#[test]
fn classify_unknown_is_generic_error() {
assert_eq!(
classify_lnd_address_error("some other failure"),
RECEIVE_LND_ERROR
);
}
#[test]
fn receive_error_starts_with_sanitizer_safe_prefix_and_embeds_code() {
// Must start with "Bitcoin address" (survives the RPC error sanitizer)
// and carry the [CODE] token the frontend parses.
let err = receive_error(RECEIVE_REST_UNREACHABLE, "unreachable");
let s = format!("{err}");
assert!(s.starts_with("Bitcoin address"), "got: {s}");
assert!(s.contains("[LND_REST_UNREACHABLE]"), "got: {s}");
}
}
@@ -297,6 +297,53 @@ async fn wait_for_manifest_host_ports(manifest: &AppManifest, timeout_secs: u64)
Ok(())
}
/// Pure published-port drift check. `port_bindings_json` is the JSON that
/// `podman inspect --format '{{json .HostConfig.PortBindings}}'` emits, e.g.
/// `{"8080/tcp":[{"HostIp":"","HostPort":"18080"}]}`. Returns true only when a
/// manifest container-port is positively published to a *different* host port
/// than the manifest now asks for. Absence of a binding is deliberately NOT
/// treated as drift here — that case is handled by the host-port repair/restart
/// path and by host-networked apps that publish nothing — so we never trigger a
/// destructive recreate on a false positive.
fn host_port_bindings_drifted(
port_bindings_json: &str,
manifest_ports: &[archipelago_container::manifest::PortMapping],
) -> bool {
let parsed: serde_json::Value = match serde_json::from_str(port_bindings_json) {
Ok(v) => v,
Err(_) => return false,
};
let Some(map) = parsed.as_object() else {
return false;
};
for port in manifest_ports {
let proto = if port.protocol.is_empty() {
"tcp"
} else {
port.protocol.as_str()
};
let key = format!("{}/{}", port.container, proto);
let Some(bindings) = map.get(&key).and_then(|b| b.as_array()) else {
// Container-port not currently published — not our case.
continue;
};
if bindings.is_empty() {
continue;
}
let expected = port.host.to_string();
let matches_expected = bindings.iter().any(|b| {
b.get("HostPort")
.and_then(|h| h.as_str())
.map(|h| h == expected)
.unwrap_or(false)
});
if !matches_expected {
return true;
}
}
false
}
async fn ensure_user_podman_socket() -> Result<()> {
let socket_path = "/run/user/1000/podman/podman.sock";
if podman_socket_accepts_connections(socket_path).await {
@@ -734,8 +781,19 @@ struct FileSecretsProvider {
impl SecretsProvider for FileSecretsProvider {
fn read(&self, name: &str) -> std::result::Result<String, ManifestError> {
let path = self.root.join(name);
let data = std::fs::read_to_string(&path).map_err(ManifestError::Io)?;
Ok(data.trim().to_string())
match std::fs::read_to_string(&path) {
Ok(data) => Ok(data.trim().to_string()),
// Name the missing secret explicitly so the failure is actionable
// instead of a bare "IO error: No such file or directory" that hides
// which secret (and which app) is blocked.
Err(e) if e.kind() == std::io::ErrorKind::NotFound => {
Err(ManifestError::Invalid(format!(
"required secret '{name}' is missing (expected at {}); the app cannot start until it is generated",
path.display()
)))
}
Err(e) => Err(ManifestError::Io(e)),
}
}
}
@@ -1054,6 +1112,7 @@ impl ProdContainerOrchestrator {
let lock = self.app_lock(&app_id).await;
let _guard = lock.lock().await;
self.ensure_app_secrets(&app_id).await?;
let mut resolved_manifest = lm.manifest.clone();
self.resolve_dynamic_env(&mut resolved_manifest)?;
let name = compute_container_name(&lm.manifest);
@@ -1094,6 +1153,22 @@ impl ProdContainerOrchestrator {
self.run_post_start_hooks(&app_id).await?;
return Ok(ReconcileAction::Started);
}
// Published-port drift means the container exists but maps
// its ports to the wrong host ports (e.g. lnd REST stuck on
// host 8080 while the manifest/clients expect 18080, as seen
// on .116). The container is already non-functional, so
// recreate it even for restart-sensitive apps during boot —
// leaving it "untouched" would perpetuate the breakage.
if self
.container_ports_drifted(&name, &resolved_manifest)
.await
{
tracing::info!(app_id = %app_id, container = %name, "container published-port drift detected — recreating");
let _ = self.runtime.stop_container(&name).await;
let _ = self.runtime.remove_container(&name).await;
self.install_fresh(lm).await?;
return Ok(ReconcileAction::Installed);
}
if self.container_env_drifted(&name, &resolved_manifest).await {
if mode == ReconcileMode::ExistingOnly
&& is_restart_sensitive_app(&app_id)
@@ -1154,8 +1229,12 @@ impl ProdContainerOrchestrator {
// reading it. A Rewritten outcome is fine here — we're
// about to start from a stopped state anyway.
self.prepare_for_start(&resolved_manifest).await?;
if self.container_env_drifted(&name, &resolved_manifest).await {
tracing::info!(app_id = %app_id, container = %name, "stopped container env drift detected — recreating");
if self.container_env_drifted(&name, &resolved_manifest).await
|| self
.container_ports_drifted(&name, &resolved_manifest)
.await
{
tracing::info!(app_id = %app_id, container = %name, "stopped container env/port drift detected — recreating");
let _ = self.runtime.remove_container(&name).await;
self.install_fresh(lm).await?;
return Ok(ReconcileAction::Installed);
@@ -1297,6 +1376,7 @@ impl ProdContainerOrchestrator {
/// Build-or-pull, create, start. Assumes the per-app mutex is already held.
async fn install_fresh(&self, lm: &LoadedManifest) -> Result<()> {
self.ensure_app_secrets(&lm.manifest.app.id).await?;
let mut resolved_manifest = lm.manifest.clone();
self.resolve_dynamic_env(&mut resolved_manifest)?;
@@ -2300,6 +2380,25 @@ impl ProdContainerOrchestrator {
Self::detect_disk_gb()
}
/// Ensure app-specific secrets exist *before* env resolution. The Bitcoin
/// backends reference `bitcoin-rpc-txrelay-rpcauth` as a required
/// `secret_env`; it is normally created by the tx-relay flow, so nodes that
/// never used tx-relay lack it and `resolve_secret_env` hard-fails — taking
/// bitcoind (and everything that depends on it) down. Generating it here,
/// idempotently, lets reconcile/install self-heal that state (the .198 case)
/// instead of cascading. Must be called before every `resolve_dynamic_env`.
async fn ensure_app_secrets(&self, app_id: &str) -> Result<()> {
if cfg!(test) {
return Ok(());
}
if matches!(app_id, "bitcoin-knots" | "bitcoin-core" | "bitcoin") {
crate::api::rpc::bitcoin_relay::ensure_txrelay_credentials(&self.data_dir)
.await
.context("ensuring bitcoin tx-relay credentials")?;
}
Ok(())
}
fn resolve_dynamic_env(&self, manifest: &mut AppManifest) -> Result<()> {
let facts = self.detect_host_facts();
let mut env = manifest.app.environment.clone();
@@ -2443,6 +2542,42 @@ impl ProdContainerOrchestrator {
false
}
/// True when a *running* container publishes a manifest container-port to a
/// different host port than the manifest now asks for (published-port
/// drift). This catches the class of failure seen on .116, where `lnd` was
/// created mapping host 8080 -> container 8080 but the current manifest maps
/// host 18080 -> container 8080, so every in-process REST client (which
/// connects to the manifest port) gets connection-refused forever while the
/// container looks "Up". `container_env_drifted` never inspects ports, and
/// `wait_for_manifest_host_ports` only restarts the stale container (which
/// republishes the wrong mapping), so without this check the drift is
/// self-perpetuating.
async fn container_ports_drifted(&self, name: &str, manifest: &AppManifest) -> bool {
if cfg!(test) {
return false;
}
if manifest.app.ports.is_empty() {
return false;
}
let inspect = tokio::process::Command::new("podman")
.args([
"inspect",
name,
"--format",
"{{json .HostConfig.PortBindings}}",
])
.output()
.await;
let Ok(output) = inspect else {
return false;
};
if !output.status.success() {
return false;
}
let bindings = String::from_utf8_lossy(&output.stdout);
host_port_bindings_drifted(&bindings, &manifest.app.ports)
}
async fn apply_data_uid(&self, manifest: &AppManifest) -> Result<()> {
let Some(uid_gid) = manifest.app.container.data_uid.as_ref() else {
return Ok(());
@@ -2752,6 +2887,7 @@ impl ContainerOrchestrator for ProdContainerOrchestrator {
let lm = self.loaded(app_id).await?;
let lock = self.app_lock(app_id).await;
let _guard = lock.lock().await;
self.ensure_app_secrets(app_id).await?;
let name = compute_container_name(&lm.manifest);
let mut resolved_manifest = lm.manifest.clone();
self.resolve_dynamic_env(&mut resolved_manifest)?;
@@ -2913,6 +3049,61 @@ mod tests {
use async_trait::async_trait;
use std::sync::Mutex as StdMutex;
fn port(host: u16, container: u16) -> archipelago_container::manifest::PortMapping {
archipelago_container::manifest::PortMapping {
host,
container,
protocol: "tcp".to_string(),
}
}
#[test]
fn port_drift_detected_when_host_port_differs() {
// The .116 case: container publishes container-port 8080 on host 8080,
// but the manifest now asks for host 18080.
let bindings = r#"{"8080/tcp":[{"HostIp":"","HostPort":"8080"}]}"#;
assert!(host_port_bindings_drifted(bindings, &[port(18080, 8080)]));
}
#[test]
fn no_drift_when_host_port_matches() {
let bindings = r#"{"8080/tcp":[{"HostIp":"0.0.0.0","HostPort":"18080"}]}"#;
assert!(!host_port_bindings_drifted(bindings, &[port(18080, 8080)]));
}
#[test]
fn no_drift_when_binding_absent() {
// Absence is handled elsewhere (host-port repair / host-networked apps);
// never treat it as drift to avoid a destructive recreate on a false
// positive.
assert!(!host_port_bindings_drifted("{}", &[port(18080, 8080)]));
assert!(!host_port_bindings_drifted("null", &[port(18080, 8080)]));
}
#[test]
fn no_drift_on_unparseable_bindings() {
assert!(!host_port_bindings_drifted(
"not json",
&[port(18080, 8080)]
));
}
#[test]
fn missing_secret_error_names_the_secret() {
use archipelago_container::manifest::SecretsProvider;
let provider = FileSecretsProvider {
root: PathBuf::from("/nonexistent-secrets-dir-xyz"),
};
let err = provider
.read("bitcoin-rpc-txrelay-rpcauth")
.expect_err("missing secret must error");
let msg = err.to_string();
assert!(
msg.contains("bitcoin-rpc-txrelay-rpcauth"),
"error should name the missing secret, got: {msg}"
);
}
/// Instrumented in-memory runtime. Every call is recorded so tests can assert
/// the exact sequence of side effects.
#[derive(Default)]