fix(lnd): hold LND's lifecycle lock across a rotation; mock the rotation RPCs
Demo images / Build & push demo images (push) Successful in 3m55s
Demo images / Build & push demo images (push) Successful in 3m55s
Reviewing the rotation against what this dev node actually did to LND today —
25 restarts, most of them automatic — surfaced a race the code did not defend
against. Between "stop LND" and "start LND" the rotation owns a stopped
container whose credential material is being deleted, and two background actors
step in there unasked: the health monitor restarts any container it finds
stopped, and the reconciler starts one whose unit is enabled.
Either brings LND back up mid-deletion. LND re-mints macaroons.db on unlock, so
the deletion loop would race a live process writing that file, or "succeed"
against material that had already been regenerated — and the operator would be
told they had rotated while the old root key was still in service. That is the
one outcome this feature exists to make impossible.
It now holds `app_ops::op_lock("lnd")` for the whole rotation. That is the lock
both actors already consult (`lifecycle_op_in_flight`; the health monitor
reaches it through `lifecycle_op_covers_container`), and it additionally
serialises against the package.start/stop/restart workers, so "Restart" on
Lightning mid-rotation queues instead of interleaving. A rotation requested
while one of those is in flight fails fast with a short explanation rather than
waiting silently behind an operation that may itself take minutes.
Deliberately NOT the `user-stopped` marker `recreate_wallet_destructively` uses
for its own window. That marker is a file on disk: a rotation that died between
marking and clearing would leave Lightning suppressed permanently, fixable only
by finding and editing JSON on the node. A lock guard releases when it drops, on
every path including a panic.
Also mocks the three RPCs in mock-backend.js, so the Settings section can be
driven end-to-end without a node — the dev preview otherwise shows only a load
error. The mock advances one step per poll rather than on a timer, which is
deterministic and makes every intermediate state observable.
Verified: cargo check + fmt clean, 6/6 rotation tests, 12/12 component tests,
mock-rpc-parity unchanged (its 2 failures are the in-flight Reticulum panel, not
this), and the three RPCs driven against the live mock through the full arc —
idle → started → 7 steps → ok with the channel count preserved, plus both
password-rejection paths.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
1a98b2d0e7
commit
cfa6c6cb0d
@@ -518,8 +518,46 @@ impl RpcHandler {
|
||||
|
||||
// ── The rotation itself ──────────────────────────────────────────────────────
|
||||
|
||||
/// Hold LND's lifecycle lock for the whole rotation, then do the work.
|
||||
///
|
||||
/// Between "stop LND" and "start LND" this owns a stopped container with its
|
||||
/// credential material deleted — the single worst moment for another actor to
|
||||
/// step in. Two would, unasked: the health monitor restarts any container it
|
||||
/// finds stopped, and the reconciler starts one whose unit is enabled. Either
|
||||
/// brings LND back up mid-deletion, and LND re-mints `macaroons.db` on unlock —
|
||||
/// so the deletion loop would race a live process writing that file, or
|
||||
/// "succeed" against material that had already been regenerated, leaving the
|
||||
/// operator told they had rotated while the old root key was still in service.
|
||||
///
|
||||
/// `app_ops::op_lock` is the mechanism both of those actors already consult
|
||||
/// (`lifecycle_op_in_flight`, via `lifecycle_op_covers_container` in the health
|
||||
/// monitor), and it also serialises against the package.start/stop/restart
|
||||
/// workers, so an operator hitting "Restart" on Lightning mid-rotation queues
|
||||
/// instead of interleaving.
|
||||
///
|
||||
/// Chosen over the `user-stopped` marker that `recreate_wallet_destructively`
|
||||
/// uses for its own window: that marker is a file on disk, so a rotation that
|
||||
/// died between marking and clearing would leave Lightning suppressed
|
||||
/// *permanently*, fixable only by finding and editing JSON on the node. This
|
||||
/// guard releases when it drops, on every path including a panic.
|
||||
async fn run_rotation(
|
||||
orchestrator: Option<Arc<dyn crate::container::ContainerOrchestrator>>,
|
||||
) -> Result<()> {
|
||||
let lock = crate::app_ops::op_lock(LND_CONTAINER);
|
||||
// Fail fast rather than queue. This is a button someone just pressed: a
|
||||
// silent wait behind a start/stop/restart that may itself take minutes reads
|
||||
// as "nothing happened", and the honest answer is short.
|
||||
let _guard = lock.try_lock().map_err(|_| {
|
||||
anyhow::anyhow!(
|
||||
"another Lightning start/stop/restart is in progress on this node — \
|
||||
wait for it to finish and try again"
|
||||
)
|
||||
})?;
|
||||
rotate_with_lnd_pinned(orchestrator).await
|
||||
}
|
||||
|
||||
async fn rotate_with_lnd_pinned(
|
||||
orchestrator: Option<Arc<dyn crate::container::ContainerOrchestrator>>,
|
||||
) -> Result<()> {
|
||||
// 1. Preflight — establish what must survive, while LND can still be asked.
|
||||
with_progress(|p| p.set("preflight", StepState::Running, None));
|
||||
@@ -584,7 +622,8 @@ async fn run_rotation(
|
||||
);
|
||||
});
|
||||
|
||||
// 3. Stop.
|
||||
// 3. Stop. Nothing may restart LND from here until step 5 — see the lock
|
||||
// `run_rotation` holds around this whole function.
|
||||
with_progress(|p| p.set("stop", StepState::Running, None));
|
||||
stop_lnd().await.context("stopping LND")?;
|
||||
with_progress(|p| p.set("stop", StepState::Done, None));
|
||||
|
||||
Reference in New Issue
Block a user