Links the real EmbeddedDaemon into the ObsidianDragonTests target (its deps were already present) and adds two POSIX integration tests that exercise the actual fork/exec/waitpid fixes headlessly: - testExecFailureReported (F2): start() against a non-executable file must fail with a precise "not executable or wrong architecture" reason. - testDaemonCrashDetected (F1): a short-lived child that exits abnormally is still detected (crash_count_ increments) while isRunning() is hammered from the test thread — a regression test for the reap race. Writing the F2 test surfaced a real bug: start()'s failure branch called setState(State::Error, "Failed to start dragonxd process"), and setState stores the Error message into last_error_ — clobbering the precise message startProcess() had just set, so getLastError()/the UI only ever saw the generic string. Fixed to pass the preserved detail to setState, so the precise reason survives and now also reaches the state callback (crash panel / status). ctest 1/1, green including the two new integration tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
34 KiB
Daemon Startup Hardening — Implementation Plan
Eight verified edge-case defects in how ObsidianDragon brings up (and watches) the
dragonxd daemon at launch. Each entry is a buildable fix: the defect (with exact
line references), the chosen approach, the call sites, a representative change, and how
to verify it.
- Scope: full-node startup path (
--liteexcludes the embedded daemon entirely). - Source: line references are exact against branch
dev@45b652f. - Provenance: findings verified by direct source read; each fix designed by an independent agent grounded in the cited files, with a sequencing pass for ordering, shared helpers, and merge conflicts.
Severity: 2 High, 6 Medium · Effort: ≈ 25–35 engineering-hours · 7 landing steps.
Status legend: ☐ not started · ◐ in progress · ☑ landed & verified
Status: all 8 landed & verified (build-clean, ctest green after each) across four commits on
dev — lifecycle cluster (F1/F2/F4), filesystem+params cluster (F7/F6/F5), F3, and F8. Six new
pure-helper unit tests added.
Wrap-up done: release notes added (CHANGELOG.md, F8 breaking change front and center); i18n
back-fill applied additively to res/lang/*.json (42 keys — all 6 for es/de/fr/pt/ru; 6 zh/ja/ko
entries whose glyphs aren't in the current NotoSansCJK-Subset.ttf were left on English fallback
rather than render as tofu).
Still owed before release: a CJK subset-font rebuild (scripts/build_cjk_subset.py, needs
the Noto CJK source font) to cover the 6 deferred zh/ja/ko strings. (F1 and F2 now have headless
integration-test coverage — see the progress log — so their GUI repros are optional, not blocking.)
Recommended rollout sequence
A real dependency order, not a checklist. The daemon-lifecycle cluster lands first
because it makes the State::Error / crash_count_ contract trustworthy — which the
connect-stall panel and the lock gate both build on. The filesystem cluster lands
around a single shared helper. The connectivity-breaking security flip lands last.
| Step | Finding(s) | Site | Why here | Status |
|---|---|---|---|---|
| 1 | F1 | embedded_daemon.cpp · isRunning() |
Smallest/highest-severity; establishes the reliable Error/crash-count transition steps 3 & 6 depend on. | ☑ |
| 2 | F2 | embedded_daemon.cpp · startProcess() |
Same file family, different function; test the F1+F2 pair together with kill -SEGV / bad-binary repros. |
☑ |
| 3 | F4 | embedded_daemon.cpp · start() |
After F1/F2 so crash-count semantics are settled; its bail deliberately stays out of the crash path. | ☑ |
| 4 | F7 | util/platform · connection.cpp |
Structural owner of the fs-error idiom + ConnectionConfig that F5/F6/F8 reuse. |
☑ |
| 5 | F6 + F5 | app.cpp · verifySaplingParams() |
Same startEmbeddedDaemon / verifySaplingParams block; land together. |
☑ |
| 6 | F3 | app.cpp · renderLoadingOverlay() |
After F1 — panel is guarded off during State::Error (owned by the crash-count hint). |
☑ |
| 7 | F8 | connection.cpp · tryConnect() |
Largest; only connectivity-breaking default flip — land last, with release notes. | ☑ |
F1 — Double-waitpid race can swallow a daemon crash
Severity: High · Effort: S (~1–2h) · Status: ☑ landed & verified
The defect
EmbeddedDaemon::isRunning() (embedded_daemon.cpp:1136, POSIX branch) calls
waitpid(WNOHANG) — from the UI thread, nearly every frame — racing
monitorProcess()'s own reap at :1244. waitpid is one-shot: if the UI thread wins,
the monitor never decodes the exit, so crash_count_ never increments, State::Error
never fires, and the 3-strike auto-restart cap (app_network.cpp:479) is defeated. The
sibling XmrigManager::isRunning() (xmrig_manager.cpp:512) already fixed exactly this
with an atomic read.
The fix
Make isRunning() read the existing std::atomic<State> state_ (member at
embedded_daemon.h:253) instead of calling waitpid, leaving monitorProcess() as the
sole reaper. Predicate is Running || Stopping — Stopping must stay "alive" because
stop()'s graceful/SIGTERM wait loops poll isRunning() before the process has exited.
Files touched
src/daemon/embedded_daemon.cpp—isRunning(), POSIX branch (~1136)
Core change
bool EmbeddedDaemon::isRunning() const // POSIX branch
{
// Read the atomic state_ instead of waitpid() — monitorProcess() is the
// sole reaper. Previously both threads reaped; if the UI thread won, the
// monitor never saw the exit (crash_count_ / exit code / Error all lost).
if (process_pid_ <= 0) return false;
State s = state_.load(std::memory_order_relaxed);
// Stopping stays "alive": stop()'s wait loops poll isRunning() while
// state_ == Stopping, before the process has actually terminated.
return (s == State::Running || s == State::Stopping);
}
Verification
- Manual:
kill -SEGVthe daemon 10–20×; the monitor must report the exit and incrementcrash_count_every time (previously intermittent). - Regression: a normal Settings-driven stop still escalates SIGTERM→SIGKILL (the
Stoppingpredicate). - Not unit-testable (real fork/exec/waitpid) — consistent with the no-process-spawn harness.
Dependencies
Mirrors XmrigManager::isRunning(). Flags a separate latent hazard (out of scope):
stop()'s final blocking waitpid (:1220) can still race a mid-sleep monitor
iteration — file as its own ticket.
F2 — exec-after-fork silent failure: "Running" for a daemon that never started
Severity: High · Effort: S (~2–3h) · Status: ☑ landed & verified
The defect
In startProcess() (embedded_daemon.cpp:957–1061, POSIX) the parent runs
process_pid_ = pid; return true; unconditionally after fork() — with no
exec-status handshake. On a non-executable / wrong-arch / corrupt binary the child's
execv fails and it _exit(127)s, but start() has already set State::Running
(:565). The real cause never reaches last_error_; it surfaces later, generically,
as "exited unexpectedly (exit code 127)".
The fix
Add a close-on-exec self-pipe handshake — pipe() + fcntl(FD_CLOEXEC), deliberately
not pipe2() (macOS lacks it; the POSIX branch is shared). The child writes errno
only on execv failure; a successful exec closes the write end for free. Parent reads:
EOF ⇒ success; 4 bytes ⇒ reap the zombie, set a precise last_error_ ("not executable
or wrong architecture"), and return false so start() never reports Running. EINTR-safe
on both ends. Also comments the unchecked parent-side setpgid at :1053.
Files touched
src/daemon/embedded_daemon.cpp—startProcess()parent read pathsrc/daemon/embedded_daemon.cpp— childexecv-failure write (~1043)src/daemon/embedded_daemon.cpp—setpgidbest-effort comment (~1053)
Core change
// Self-pipe exec handshake (pipe()+FD_CLOEXEC; NOT pipe2 — macOS lacks it).
int execpipe[2]; pipe(execpipe);
fcntl(execpipe[0], F_SETFD, FD_CLOEXEC);
fcntl(execpipe[1], F_SETFD, FD_CLOEXEC);
pid_t pid = fork();
if (pid == 0) { // child
close(execpipe[0]);
/* setpgid / chdir / dup2 / argv … */
execv(binary_path.c_str(), argv.data());
int e = errno; // execv failed
while (write(execpipe[1], &e, sizeof e) < 0 && errno == EINTR) {}
_exit(127);
}
close(execpipe[1]); // parent: must close or read() never EOFs
int child_errno = 0, total = 0;
for (;;) { // EOF ⇒ exec ok; 4 bytes ⇒ exec failed
ssize_t n = read(execpipe[0], (char*)&child_errno + total, sizeof(int) - total);
if (n == 0) break;
if (n < 0) { if (errno == EINTR) continue; break; }
if ((total += n) >= (int)sizeof(int)) break;
}
close(execpipe[0]);
if (total >= (int)sizeof(int)) { // exec never happened
waitpid(pid, nullptr, 0); // reap the zombie
last_error_ = "dragonxd could not be executed: " +
std::string(strerror(child_errno)) +
" — not executable or wrong architecture";
return false; // start() no longer reports Running
}
Verification
- Point at a
chmod -x/ wrong-arch file →start()returns false immediately, precise message, no leftover zombie. - Success path: real binary still starts with no perceptible added latency.
- Optional pure
formatExecFailureError(errno)helper for atest_phase4.cppunit test.
Dependencies
F1 (same function family; sequence F1→F2). Highest-risk mistake: forgetting
FD_CLOEXEC makes every successful start hang the parent read forever.
F4 — Stale datadir-lock start → restart storm that wedges the UI
Severity: Medium · Effort: S (~3–5h) · Status: ☑ landed & verified
The defect
start() (embedded_daemon.cpp:466) gates only on the RPC port (:482), never on
isDaemonProcessRunning() (:1292). A graceful shutdown frees the port but keeps the
datadir .lock for up to ~90s. A rapid stop→start spawns a daemon that dies "Cannot
obtain a lock on data directory" — routed to the generic crash path. With a ~4s retry
cadence, three lock races in ~12s exhaust the 3-strike budget and wedge the UI long
before the lock actually clears.
The fix
Fail-fast with a short bounded local wait (~300ms), not a 90s block. After the port
bail, consult isDaemonProcessRunning() — gated by !skip_port_check_ and exempt when
override_datadir_ is set, so the isolated migrate-to-seed daemon still works. A pure
evaluateDatadirLockGate() returns a distinct non-crash Error that never increments
crash_count_. The connect loop's own retry then absorbs the transient.
Files touched
src/daemon/embedded_daemon.h— decision struct, helper decl, poll constantssrc/daemon/embedded_daemon.cpp—start()gate +evaluateDatadirLockGate()
Core change
static StartLockGateDecision evaluateDatadirLockGate(
bool skipPortCheck, bool isolatedOverride, bool stillRunningAfterWait) {
if (skipPortCheck || isolatedOverride) return {true, ""}; // migrate-to-seed exempt
if (!stillRunningAfterWait) return {true, ""};
return {false, "A previous dragonxd is still shutting down and holding the "
"data directory lock. Retrying shortly…"};
}
// start() — after the isPortInUse() bail, before setState(Starting):
if (!skip_port_check_ && override_datadir_.empty()) {
bool stillLocked = false; // ~300ms bounded wait, NOT ~90s
for (int i = 0; i < kDatadirLockWaitMaxPolls; ++i) {
if (!isDaemonProcessRunning()) { stillLocked = false; break; }
stillLocked = true;
std::this_thread::sleep_for(std::chrono::milliseconds(kDatadirLockWaitPollMs));
}
auto gate = evaluateDatadirLockGate(false, false, stillLocked);
if (!gate.proceed) { setState(State::Error, gate.errorMessage); return false; }
}
Verification
- Unit:
evaluateDatadirLockGate()across the skip / isolated / still-running matrix. - Manual: rapid restart into a lingering lock → distinct message, no crash-cap wedge.
- Migrate-to-seed second daemon still starts (isolated exemption).
Dependencies
F1/F2 (must not touch crash_count_; wording must not collide with the monitor's
"exited unexpectedly"). Same TU, different function.
F5 — Extraction / copy write-failures never surfaced up front
Severity: Medium · Effort: S (~2–3h) · Status: ☑ landed & verified
The defect
startEmbeddedDaemon() discards extractEmbeddedResources()'s bool return
(app.cpp:4152) and the second copy-fallback loop drops copy_file's error_code
entirely (:4236). Only Sapling params existence is re-checked — never the daemon
binary/CLI/tx/asmap. A disk-full or truncated dragonxd write falls straight through to
spawn and fails opaquely. The innermost write already returns false
(embedded_resources.cpp:307) — the signal is simply thrown away.
The fix
Minimal, surgical wiring — no new abstraction. Capture the extraction return and, on
failure, set daemon_status_ = TR("sb_daemon_extract_failed") and return false before
spawning. In the second copy loop, check ec after each copy_file, track copyFailed,
and abort with a dir-parameterized sb_daemon_files_failed. An absent source stays
fine (optional files); only an actual error_code counts. Written so F6/F7 slot in later
without re-touching this control flow.
Files touched
src/app.cpp—startEmbeddedDaemon()extraction check (~4152)src/app.cpp— second copy-fallback loop (~4210–4242)src/util/i18n.cpp+res/lang/*.json— 2 additive keys
Core change
// stop discarding the extraction result (~4152)
if (!resources::extractEmbeddedResources()) {
daemon_status_ = TR("sb_daemon_extract_failed"); // disk full / permission denied
return false; // abort before spawning
}
// second copy-fallback loop — was dropping ec entirely (~4236)
bool copyFailed = false;
for (const char* name : { "asmap.dat", "dragonxd", "dragonx-cli", "dragonx-tx" }) {
fs::path dst = fs::path(daemon_dir) / name;
if (fs::exists(dst)) continue; // already present — skip
for (const auto& dir : searchDirs) {
fs::path src = fs::path(dir) / name;
if (!fs::exists(src)) continue; // absent source is OK, not a failure
fs::copy_file(src, dst, ec);
if (ec) { copyFailed = true; ec.clear(); }
break;
}
}
if (copyFailed) {
char buf[512];
snprintf(buf, sizeof buf, TR("sb_daemon_files_failed"), daemon_dir.c_str());
daemon_status_ = buf;
return false; // don't fall through to spawn
}
Verification
- Unit:
extractEmbeddedResources()returns false without embedded resources. - Extract the copy loop into a testable helper; force one dst write to fail (dst is an existing directory).
- Manual: near-full tmpfs / read-only dir → clear status, daemon controller never constructed.
Dependencies
Shares the daemon_status_ surfacing convention with F6; its early-return pattern is the
template F7 matches. Open item: remove truncated dst files so a retry re-copies.
F6 — Sapling params validated by existence/size only, never hashed
Severity: Medium · Effort: S (~3–5h) · Status: ☑ landed & verified
As-built note.
verifySaplingParams()now delegates to a public, injectableverifySaplingParamsIn(dir, digests)so the integrity + marker-cache logic is unit-testable with synthetic small files (the real 48 MB params aren't in the repo). i18n keys for F5 were added toi18n.cpp(English source of truth); theres/lang/*.jsonback-fill viascripts/add_missing_translations.pyis deferred to a single run at the end of the batch, per the cross-cutting note. Non-English locales fall back to English until then.
The defect
verifySaplingParams() (connection.cpp:123) only calls fs::exists();
resourceNeedsUpdate() (embedded_resources.cpp:250) is size-only. On Linux (no
embedded resources) a truncated-but-present param passes and is handed to the daemon,
which then fails to build shielded proofs mid-operation — far from the real cause.
The fix
Add a pinned { filename → size, sha256 } table (one source of truth, cross-referenced
to scripts/build-lite-backend-artifact.sh) and hash-check each param after the
existence check, reusing the existing util::sha256Hex (no second implementation). Since
these are ~48 MB, cache the result via a .sapling_verified marker keyed on
size:mtime — re-hash only when the stat line changes, so startup isn't slowed.
Files touched
src/rpc/connection.h—verifySaplingParamsdeclsrc/rpc/connection.cpp— digest table, marker helpers, rewrite
Core change
// connection.cpp — pinned known-good digests
// (source of truth: scripts/build-lite-backend-artifact.sh ensure_sapling_params)
constexpr SaplingParamDigest kSaplingParamDigests[] = {
{ "sapling-spend.params", 47958396, "8e48ffd2…efc13" },
{ "sapling-output.params", 3592860, "2f0ebbcb…fb0e4" },
};
bool Connection::verifySaplingParams() {
// existence check (unchanged) …
// cache: skip re-hashing a ~48 MB file unless size:mtime changed
if (readMarkerMatches(marker, statLines)) return true;
for (auto& d : kSaplingParamDigests)
if (util::sha256Hex(bytes) != d.sha256) return false; // reuse existing helper
writeMarker(marker, statLines);
return true;
}
Verification
- Unit: good params pass; truncated / wrong-bytes rejected; marker cache short-circuits re-hash unless size/mtime changed. Real temp-file fixtures (matches existing
sha256Hextests).
Dependencies
F7 (reuse fs-error idiom; shares the startEmbeddedDaemon/verifySaplingParams block).
Third caller of the existing util::sha256Hex.
F7 — Directory-create errors universally ignored on the daemon-env path
Severity: Medium · Effort: S (~3–4h) · Status: ☑ landed & verified
As-built notes. Two deviations from the original design, both confirmed against the code: (1)
embedded_resources.cpp:270already checks itserror_codeand returnsfalseon failure — it was not a bug, so it is left untouched. (2) Of the fourautoDetectConfigcallers, only the primary connect path (app_network.cpp:243) was wired to checkdir_error; the other three degrade gracefully on their own —app.cpp:4306andapp_wizard.cpp:912are stop paths that already gate on empty creds, andsettings_page.cpp:434is read-only display.dir_erroris set byautoDetectConfig, so they can be wired later if desired.
The defect
Five startup directory-create sites either drop the error_code or use the throwing
overload with no catch: main.cpp:730, connection.cpp:216 (can throw uncaught
through its callers), embedded_resources.cpp:270, app.cpp:4172/4218. A read-only
home or permission-denied yields a confusing "conf missing" / "binary not found"
downstream — or an uncaught filesystem_error — instead of a clear cause.
The fix
One shared, non-throwing Platform::ensureDirectory(dir, outError) in
util/platform.{h,cpp} that produces a single consistent message. Replace all five
sites; autoDetectConfig() moves off the throwing overload and sets a new
ConnectionConfig::dir_error that its four callers check and bail on. This is the
structural owner of the fs-error idiom that F5 and F6 reuse.
Files touched
src/util/platform.h/.cpp—ensureDirectory()src/rpc/connection.h/.cpp—dir_error+autoDetectConfigmain.cpp,app.cpp,app_network.cpp,app_wizard.cpp,settings_page.cpp,embedded_resources.cpp— 5 sites + 4 callerstests/test_phase4.cpp—TestPlatformEnsureDirectory
Core change
// util/platform.cpp — one shared, non-throwing helper
bool Platform::ensureDirectory(const std::string& dir, std::string* outError) {
std::error_code ec;
if (std::filesystem::is_directory(dir, ec)) return true;
ec.clear();
std::filesystem::create_directories(dir, ec);
if (ec) {
if (outError)
*outError = "Cannot create " + dir + ": " + ec.message() +
". Check permissions / free space.";
return false;
}
return true;
}
// Replaces 5 ad-hoc sites; autoDetectConfig() now sets ConnectionConfig::dir_error,
// and its 4 callers bail on it.
Verification
- Unit
TestPlatformEnsureDirectory: existing dir → true; fresh nested → created; POSIX unwritable → false + message. - All four
autoDetectConfigcallers toleratedir_error. Pre-App-init site (main.cpp) reports via stderr / MessageBox.
Dependencies
Owns Platform::ensureDirectory (used by F5, F6) and the ConnectionConfig
extension (coordinated with F8). Land before F5/F6/F8.
F8 — Plaintext-remote RPC credential transmission is warn-only
Severity: Medium · Effort: M (~6–9h) · Status: ☑ landed & verified
⚠️ RELEASE NOTES REQUIRED — breaking default flip. A wallet configured to talk to a remote
rpchostover plain HTTP (norpctls=1) will now be refused at connect time instead of warned. Affected users must addrpcallowplaintext=1toDRAGONX.conf(or switch torpctls=1) to reconnect. Local/embedded daemons (127.0.0.0/8,localhost,::1) are unaffected. Call this out prominently in the release notes.As-built note. Shipped the security-complete core:
isLocalHosttightened to exact loopback (isExactIPv4Loopback—127.evil.comno longer passes), refuse-by-default intryConnect, and therpcallowplaintextconf-key opt-in. The Settings toggle UI was deferred — the RPC section ofsettings_page.cppis read-only display and a security toggle there is riskier surface; the conf-key opt-in fully covers recovery, and the refusal status/notification tells the user exactly what to add. The toggle can be added later (persist aSettingsflag and OR it intoallowsPlaintextRemote).
The defect
A remote rpchost without rpctls=1 sends Basic-auth rpcuser:rpcpassword over
cleartext HTTP. tryConnect() (app_network.cpp:314) only shows a dismissible
warning then proceeds — a local-network MITM sees the credentials. Compounding it,
isLocalHost()'s naive rfind("127.",0)==0 misclassifies 127.evil.com as local,
suppressing even the warning.
The fix
Change the policy to refuse-by-default with an explicit, persisted opt-in — a
rpcallowplaintext=1 conf key (for hand-editors) and a Settings toggle. Block the
connect and show a blocking modal explaining the risk and how to enable TLS or opt
in; localhost is unaffected. Tighten isLocalHost() to exact 127.x.y.z / ::1 /
localhost via isExactIPv4Loopback(). Back-compat: default off ⇒ existing remote
users hit a hard stop until they opt in — ship with prominent release notes.
Files touched
src/rpc/connection.h/.cpp—isLocalHost,allow_plaintext_remote,parseConfFilesrc/config/settings.h/.cpp— persisted opt-insrc/app_network.cpp,src/app.h— refuse + modal dispatchsrc/ui/windows/plaintext_remote_rpc_dialog.h— new blocking modalsrc/ui/pages/settings_page.cpp— toggle UI
Core change
// Tightened loopback test — "127.evil.com" is NOT local
bool Connection::isLocalHost(const std::string& host) {
std::string h = stripBrackets(lowercase(host));
return h == "localhost" || h == "::1" || isExactIPv4Loopback(h); // exact 127.x.y.z
}
// Refuse-by-default with an explicit, persisted opt-in
const bool plaintextRemote = rpc::Connection::usesPlaintextRemote(config);
const bool plaintextAllowed = config.allow_plaintext_remote // rpcallowplaintext=1
|| settings_.getAllowPlaintextRemoteRpc(); // Settings toggle
if (plaintextRemote && !plaintextAllowed) {
connection_status_ = TR("sb_plaintext_remote_blocked");
showPlaintextRemoteRpcDialog(config.host + ":" + config.port); // blocking modal
return; // no creds sent
}
Verification
- Unit:
isLocalHost—127.evil.comfalse,127.0.0.1/::1/localhosttrue;allowsPlaintextRemotehonors conf key + settings flag. - Manual: remote plaintext blocked; modal fires; opt-in persists across restart.
Dependencies
F7 (second extender of ConnectionConfig/parseConfFile; land after so the struct grows
once). Wire renderPlaintextRemoteRpcDialog into the app modal-dispatch list.
Shared helpers & coordination points
| Helper | Purpose | Used by |
|---|---|---|
Platform::ensureDirectory() |
Single non-throwing directory-create with one consistent message; replaces five ad-hoc sites. Owned by F7. | F7, F5, F6 |
ConnectionConfig extension |
Coordination point, not a function: F7 adds dir_error, F8 adds allow_plaintext_remote. Land F7→F8 so it grows once per step. |
F7, F8 |
util::sha256Hex (existing) |
Already-compiled, curl-free SHA-256. F6 becomes its third caller — no second hash routine. | F6 |
connectHasStalled() (new, pure) |
Stall predicate split out of the ImGui/App code for unit testing, per the *_updater_core.cpp precedent. |
F3 |
evaluateDatadirLockGate() (new, pure) |
Lock-gate decision as {proceed, message} from three booleans — unit-testable without real process/fs I/O. |
F4 |
F3 — Unbounded connect spinner (deferred to step 6)
Severity: Medium · Effort: S (~3–5h) · Status: ☑ landed & verified
As-built note.
renderLoadingOverlay()is a pure draw-list overlay with no interactive widgets (the existing crash case at ~5289 already communicates via guidance text, relying on the sidebar staying reachable). So rather than injectActionButtons — which would fight the non-interactive overlay — the stall notice follows that same idiom: a "Taking longer than expected" title + a reassuring body (with elapsed seconds) + a full-node-gated hint ("Open Settings → Restart Daemon, or check the Console"). This let me drop the plannedWalletState::connect_stalledflag too: the stalled state is computed locally in the overlay fromconnect_stall_since_, so the only new member isApp::connect_stall_since_.
The connect loop retries forever while !state_.connected (app.cpp:1239);
loading_timer_ only animates the spinner. Stamp connect_stall_since_ when
"reachable but not ready" is first seen; a pure connectHasStalled() helper (new
util/connect_stall.h, default 45s from ui.toml) flips state_.connect_stalled at
threshold, and renderLoadingOverlay() shows a "Taking longer than expected" panel with
Retry / Restart daemon / Open console (full-node gated). The background retry keeps
firing — recovery clears the panel automatically. Guarded off while the daemon is in
State::Error (owned by F1's crash-count hint). Full detail lives in the sequencing/
design record; see the shared-helper table above.
Cross-cutting notes
- One TU, three functions.
embedded_daemon.cppis edited by F1 (isRunning), F2 (startProcess) and F4 (start) — no literal hunk overlap, but land in order to keep "monitorProcess is the sole reaper" coherent. - Connection struct grows twice.
connection.h/.cppis touched by F6, F7 and F8; F7 and F8 both extendConnectionConfigandparseConfFile— highest collision risk. Sequence F7→F6→F8. - Testability split. The three new pure predicates all get
tests/test_phase4.cppcoverage. F1/F2's fork/exec/waitpid changes are not unit-testable — they rely on manualkill/ non-executable-binary repros, consistent with the no-process-spawn harness. - i18n is additive-only. Add each finding's English keys to
strings_, then runscripts/add_missing_translations.pyonce at the very end (json.dump indent=4, sort_keys=True, ensure_ascii=False) — never bulk-regenerate ares/lang/*.json. - F8 is a breaking default flip. Refuse-plaintext-by-default stops existing
remote-RPC users cold until they opt in. Lands last, gated behind a persisted opt-in,
with release notes calling out the new
rpcallowplaintextkey and the Settings toggle. - Latent hazard, out of scope. F1 surfaces (but doesn't fix) a second
double-
waitpidwindow betweenstop()'s final blocking reap (:1220) and a mid-sleep monitor iteration — file it as its own ticket.
Progress log
-
F1/F2 integration tests — ☑ added
testExecFailureReported(F2) andtestDaemonCrashDetected(F1) totest_phase4.cpp, driving the realEmbeddedDaemonfork/exec/waitpid code headlessly (POSIX; required linkingembedded_daemon.cppinto the test target — its deps were already there). The F1 test hammersisRunning()from the test thread while the child exits, so it's a genuine regression test for the reap race. The F2 test caught a real bug:start()'s failure branch overwrotestartProcess()'s preciselast_error_("…not executable or wrong architecture") with a generic "Failed to start dragonxd process" (becausesetState(Error, …)stores its message intolast_error_), so the precise reason never reachedgetLastError()/the UI — fixed to preserve the detail (now also surfaced via the state callback / crash panel). Build-clean;ctest1/1. -
F1 — ☑ landed:
isRunning()(POSIX) now reads the atomicstate_(predicateRunning || Stopping) instead of callingwaitpid, leavingmonitorProcess()the sole reaper. Clean build (all targets link);ctest1/1 passing. Not unit-testable — needs the manualkill -SEGVrepro before release. -
F2 — ☑ landed:
startProcess()(POSIX) now creates aFD_CLOEXECself-pipe beforefork(); the child writeserrnoto it onexecvfailure, the parent reads EOF-vs-errno and, on failure, reaps the zombie + sets a preciselast_error_("not executable or wrong architecture") + returnsfalse(sostart()no longer reportsRunningfor a daemon that never started). Parent-sidesetpgidis now best-effort with aDEBUG_LOGFon failure. Clean build;ctest1/1 passing. Not unit-testable — needs the manual non-executable / wrong-arch-binary repro before release. -
F8 — ☑ landed:
isLocalHost()tightened to exact loopback viaisExactIPv4Loopback(a127.-prefixed hostname like127.evil.comis no longer misclassified as local).tryConnect()now refuses a plaintext connection to a remote host instead of warn-and-proceeding — a local-network MITM can no longer capturerpcuser:rpcpassword— unless the user opts in withrpcallowplaintext=1inDRAGONX.conf(newConnectionConfig::allow_plaintext_remote+allowsPlaintextRemote()policy). The refusal surfaces via status line + a one-time notification. NewtestIsLocalHost(12 assertions) +testAllowsPlaintextRemote(5). Clean build;ctest1/1 passing. Breaking — needs release notes; Settings-toggle UI deferred (see as-built note). -
F3 — ☑ landed: the connect loop now stamps
connect_stall_since_ = ImGui::GetTime()the moment the daemon first goes "reachable but not ready" (warmup branch +applyDaemonInitStatus), and clears it inonConnected/onDisconnected/ warmup-complete — all inapp_network.cpp. The pureutil::connectHasStalled(stallSince, now, threshold)helper (newutil/connect_stall.h, default 45 s fromui.toml) drives a draw-list "Taking longer than expected" notice inrenderLoadingOverlay()(title + elapsed-seconds body + full-node hint), guarded off while the daemon is inState::Error. Background retry continues, so the notice self-clears on connect. NewtestConnectHasStalledunit test (7 assertions). Clean build;ctest1/1 passing. (Draw-list text, not buttons — see as-built note above.) -
F6 — ☑ landed:
verifySaplingParams()now hash-verifies each Sapling param against its pinned canonical SHA-256 (frombuild-lite-backend-artifact.sh), replacing the existence-only check, so a truncated/corrupt-but-present param is rejected instead of failing later on a shielded op. A<params_dir>/.sapling_verifiedmarker keyed onsize:mtimeskips re-hashing ~48 MB on every startup. Logic extracted to the injectableverifySaplingParamsIn(dir, digests); newtestVerifySaplingParamsunit test (valid / marker fast-path / wrong-hash / truncated / missing). Clean build;ctest1/1 passing. -
F5 — ☑ landed:
startEmbeddedDaemon()now checksextractEmbeddedResources()'s return (abort withsb_daemon_extract_failedon failure) and the previously-droppedcopy_fileerror_codein the daemon-binary fallback loop (abort withsb_daemon_files_failedincl. the dir), so a disk-full / truncateddragonxdwrite is surfaced up front instead of failing opaquely at spawn. An absent source file stays non-fatal. Two i18n keys added toi18n.cpp. Clean build;ctest1/1 passing. -
F7 — ☑ landed: new non-throwing
Platform::ensureDirectory(dir, outError)inutil/platform.{h,cpp}with one consistent message. Replaces the unchecked/throwing directory-create sites atmain.cpp:730(pre-init: now logs +MessageBoxAon Windows +return 1),connection.cpp:216(autoDetectConfig now uses the ec overload — no more uncaughtfilesystem_error— and sets the newConnectionConfig::dir_error), and bothapp.cppdaemon-dir sites (surface viadaemon_status_+return false). Primary connect path (app_network.cpp:243) checksdir_errorand bails to the status line instead of mislabelling it "waiting for config".embedded_resources.cpp:270left as-is (already correct). NewtestPlatformEnsureDirectoryunit test (existing-dir / fresh-nested / empty / parent-is-file). Clean build;ctest1/1 passing. -
F4 — ☑ landed:
start()now gates on a lingering datadir lock after the port bail. When!skip_port_check_ && override_datadir_.empty(), it pollsisDaemonProcessRunning()with a bounded ~300 ms wait (3 × 100 ms, breaks early), then a pure header-inlineevaluateDatadirLockGate()decides: if a siblingdragonxdis still alive it bails with a distinct non-crashState::Error("…holding the data directory lock. Retrying shortly…") that never touchescrash_count_, so the 3-strike cap can't trip; the connect loop's retry resumes once the lock clears. Isolated migrate-to-seed starts are exempt. NewtestDatadirLockGateunit test (5 assertions, proceed/bail/2× exempt) added totest_phase4.cpp. Clean build;ctest1/1 passing.