fix(ecash): guard Minibits claim polls against races and stop replayed claims retrying forever

The UI polls wallet.ecash-lnaddress-claim every 8s, but a single poll (auth +
/claim + relay fetch + redeem loop) can outlast that interval. Two overlapping
claim_and_redeem runs then loaded the same last_dm_seen_at, fetched/redeemed
the same claims, and last-writer-wins on save — rewinding the watermark and/or
double-redeeming. A double-redeemed or state-loss-replayed claim then failed
forever as "already spent" with no way to leave pending_claims, leaving a
permanent orange retry banner.

- STATE_LOCK (backend) + an in-flight guard (UI) serialize claim polls and
  the lnaddress registration/token-refresh path, so two callers can't race on
  minibits.json.
- pending_claims now tracks per-claim attempts (PendingClaim, migrating
  transparently from the old plain-string shape); a claim that fails
  MAX_CLAIM_ATTEMPTS times is dropped instead of retried forever.
- A redeem failure recognized as mint error 11001 (already redeemed) is
  treated as terminal and dropped immediately — the value was already swept,
  so retrying it is pointless. ClaimOutcome gains dropped_count so the two
  drop reasons (harmless vs. real loss) are visible to the caller.
- save_state now writes via temp-file + rename instead of truncating
  minibits.json in place — the exact disk-full failure mode that corrupted
  this file on archy-x250-pa3, 2026-09-08, could otherwise destroy
  pending_claims tokens that /claim had already consumed server-side
  (unrecoverable, unlike relay DMs).
- minibits_error no longer panics on a multi-byte UTF-8 boundary when
  truncating a server error body (was byte-slicing, not char-safe).
- register_profile's name-collision check now matches the structured
  error.name == ALREADY_EXISTS instead of a raw "already" substring, so an
  unrelated error message doesn't burn a retry attempt.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EZnFgeUBKY5UAfyJFsYccS
This commit is contained in:
2026-09-09 03:50:34 +00:00
co-authored by Claude Sonnet 5
parent fc5b51ab2f
commit 4e410d7c98
5 changed files with 290 additions and 39 deletions
+1
View File
@@ -442,6 +442,7 @@ impl RpcHandler {
"claimed_count": outcome.claimed_count,
"received_sats": outcome.received_sats,
"failed_count": outcome.failed_count,
"dropped_count": outcome.dropped_count,
}))
}
+215 -35
View File
@@ -138,14 +138,14 @@ pub struct MinibitsState {
pub server_nostr_pubkey: String,
#[serde(default)]
pub created_at: String,
/// Raw NIP-04-encrypted claim tokens fetched from `/claim` but not yet
/// successfully redeemed. A claim is consumed server-side the instant
/// `/claim` returns it, so it is stashed here *before* decrypt/redeem is
/// Raw NIP-04-encrypted claim tokens fetched from `/claim` (or a relay DM)
/// but not yet successfully redeemed. A claim is consumed server-side the
/// instant it's fetched, so it is stashed here *before* decrypt/redeem is
/// attempted — a local failure (mint briefly down, bad cached server key,
/// process crash mid-loop) then retries next poll instead of losing the
/// coins outright.
#[serde(default)]
pub pending_claims: Vec<String>,
pub pending_claims: Vec<PendingClaim>,
/// Unix timestamp of the newest Nostr DM we've already pulled into
/// `pending_claims` (see `fetch_relay_dms`). Nostr events never expire
/// from relays, so without this watermark every poll would re-fetch and
@@ -155,6 +155,64 @@ pub struct MinibitsState {
pub last_dm_seen_at: u64,
}
/// A claim token queued for redeem, plus how many times redeem has already
/// been tried. Deserializes from either shape: the pre-existing plain string
/// (a node's `minibits.json` written before this field existed) becomes
/// `attempts: 0`, so upgrading never drops or resets an operator's queued
/// claims (see the module's data-preservation invariant).
#[derive(Debug, Clone, Serialize)]
pub struct PendingClaim {
pub token: String,
#[serde(default)]
pub attempts: u32,
}
impl<'de> Deserialize<'de> for PendingClaim {
fn deserialize<D>(deserializer: D) -> std::result::Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
#[derive(Deserialize)]
#[serde(untagged)]
enum Repr {
Legacy(String),
Full {
token: String,
#[serde(default)]
attempts: u32,
},
}
Ok(match Repr::deserialize(deserializer)? {
Repr::Legacy(token) => PendingClaim { token, attempts: 0 },
Repr::Full { token, attempts } => PendingClaim { token, attempts },
})
}
}
/// A claim is dropped (not retried again) after this many failed decrypt/redeem
/// attempts — a backstop against a token that fails for a permanent reason
/// `is_already_redeemed` doesn't catch (e.g. a corrupt payload), which would
/// otherwise retry forever and keep `failed_count` stuck non-zero.
const MAX_CLAIM_ATTEMPTS: u32 = 20;
/// True when `ecash::receive_token` failed because the token was already
/// redeemed (mint error 11001, see `mint_client::describe_mint_error_code`) —
/// a terminal condition, not a reason to retry. Seen after a state-file
/// watermark rewind replays an already-swept relay DM, or (pre-`STATE_LOCK`)
/// after a race let two polls redeem the same claim.
fn is_already_redeemed(err: &anyhow::Error) -> bool {
err.to_string().contains(super::mint_client::ALREADY_REDEEMED_MSG)
}
/// Serializes every in-flight Minibits state read-modify-write
/// (`lnaddress`'s registration/token-refresh and `claim_and_redeem`'s fetch/
/// redeem cycle) so two callers can never race on `minibits.json`. Without
/// this, two overlapping claim polls can each load the same
/// `last_dm_seen_at`, fetch the same DMs, and last-writer-wins on save —
/// rewinding the watermark and/or duplicating a pending token into a double
/// redeem.
static STATE_LOCK: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(());
/// A fresh Nostr keypair + seedHash derived from the node's ecash phrase.
struct MinibitsIdentity {
keys: nostr_sdk::Keys,
@@ -184,19 +242,28 @@ struct ProfileRecord {
pubkey: String,
}
/// Parse a Minibits `{"error": {"name": ..., "message": ...}}` response body,
/// when present.
fn parse_minibits_error(body: &str) -> Option<(String, String)> {
let v: serde_json::Value = serde_json::from_str(body).ok()?;
let err = v.get("error")?;
let name = err.get("name").and_then(|n| n.as_str()).unwrap_or("ERROR");
let msg = err.get("message").and_then(|m| m.as_str()).unwrap_or("");
Some((name.to_string(), msg.to_string()))
}
/// Turn a non-2xx Minibits response into a readable error, surfacing the
/// server's `error.name`/`error.message` when present.
fn minibits_error(status: reqwest::StatusCode, body: &str) -> anyhow::Error {
if let Ok(v) = serde_json::from_str::<serde_json::Value>(body) {
if let Some(err) = v.get("error") {
let name = err.get("name").and_then(|n| n.as_str()).unwrap_or("ERROR");
let msg = err.get("message").and_then(|m| m.as_str()).unwrap_or("");
if let Some((name, msg)) = parse_minibits_error(body) {
return anyhow!("Minibits API error {status}: {name} {msg}");
}
}
// Truncate on a char boundary, not a byte index — `body` is
// server-controlled and a multi-byte UTF-8 character straddling byte 180
// would otherwise panic inside this RPC handler's own error path.
anyhow!(
"Minibits API error {status}: {}",
&body[..body.len().min(180)]
body.chars().take(180).collect::<String>()
)
}
@@ -234,6 +301,15 @@ async fn load_state(data_dir: &Path) -> Result<Option<MinibitsState>> {
/// Write the state file 0600 — it holds a bearer JWT. Same sensitivity class as
/// the ecash files it sits beside, so it gets the same owner-only mode.
///
/// Writes via a temp file + rename rather than truncating `minibits.json` in
/// place: a disk-full write hitting the truncate-then-write path destroyed
/// this exact file on archy-x250-pa3, 2026-09-08. `pending_claims` entries
/// sourced from `/claim` are unrecoverable once consumed server-side (unlike
/// relay DMs, which stay on the relay), so losing that field to a partial
/// write is not something the existing corrupt-file self-heal can undo — this
/// makes the write itself atomic instead. `STATE_LOCK` (held by every caller)
/// makes a fixed temp filename safe: only one writer runs at a time.
async fn save_state(data_dir: &Path, state: &MinibitsState) -> Result<()> {
let path = state_path(data_dir);
if let Some(parent) = path.parent() {
@@ -243,17 +319,22 @@ async fn save_state(data_dir: &Path, state: &MinibitsState) -> Result<()> {
}
let content = serde_json::to_string_pretty(state)
.context("Failed to serialize the Minibits profile")?;
fs::write(&path, content)
let tmp_path = path.with_extension("json.tmp");
fs::write(&tmp_path, content)
.await
.with_context(|| format!("Failed to write {}", path.display()))?;
.with_context(|| format!("Failed to write {}", tmp_path.display()))?;
#[cfg(unix)]
{
use std::os::unix::fs::PermissionsExt;
fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600))
fs::set_permissions(&tmp_path, std::fs::Permissions::from_mode(0o600))
.await
.with_context(|| format!("Failed to chmod 0600 {}", path.display()))?;
.with_context(|| format!("Failed to chmod 0600 {}", tmp_path.display()))?;
}
fs::rename(&tmp_path, &path)
.await
.with_context(|| format!("Failed to move {} into place", tmp_path.display()))?;
Ok(())
}
@@ -395,8 +476,14 @@ async fn register_profile(
.context("Minibits profile response was not the expected shape")?;
return Ok(rec);
}
// Name collision → draw another. Anything else is fatal.
let is_taken = body.contains("ALREADY_EXISTS") || body.contains("already");
// Name collision → draw another. Anything else is fatal. Match on the
// structured error name, not a raw substring: an unrelated failure
// whose free-text message happens to contain the word "already"
// (e.g. a rate-limit or session message) must not burn one of the
// 6 retry attempts here.
let is_taken = parse_minibits_error(&body)
.map(|(name, _)| name.eq_ignore_ascii_case("ALREADY_EXISTS"))
.unwrap_or(false);
if is_taken {
warn!("Minibits name '{wallet_id}' taken, retrying (attempt {attempt})");
last_err = Some(minibits_error(status, &body));
@@ -463,6 +550,11 @@ pub async fn lnaddress(data_dir: &Path) -> Result<serde_json::Value> {
let (phrase, seed) = ecash_phrase(data_dir).await?;
let identity = derive_identity(&phrase, &seed)?;
// Hold the same lock a claim poll uses: both read-modify-write
// `minibits.json`, and this call registers the profile the first time —
// it must not race a poll that's mid-save.
let _guard = STATE_LOCK.lock().await;
let mut state = match load_state(data_dir).await? {
Some(st) => st,
None => {
@@ -511,26 +603,20 @@ pub struct ClaimOutcome {
/// dropped. Non-zero here means real, unswept value the operator should
/// know about.
pub failed_count: usize,
/// Claims permanently given up on this poll: either recognized as
/// already redeemed elsewhere (harmless — the value was already swept),
/// or a decrypt/redeem failure that hit `MAX_CLAIM_ATTEMPTS`. The latter
/// case means real value that was lost; the former does not.
pub dropped_count: usize,
}
const NO_CLAIMS: ClaimOutcome = ClaimOutcome {
claimed_count: 0,
received_sats: 0,
failed_count: 0,
dropped_count: 0,
};
/// Make sure the Minibits mint is on the accepted-mints allow-list.
///
/// `ecash::receive_token` checks the raw accepted-mints file directly (not
/// the more lenient `ecash::is_mint_trusted`, which always trusts the default
/// mint) — so an operator who edited their accepted-mints list (e.g. via the
/// `streaming.configure-mints` RPC) and dropped the default mint would
/// otherwise cause every Minibits claim to fail *after* the claim was already
/// consumed server-side, permanently losing those coins with nothing but a
/// log line to show for it. The Minibits Lightning address is inherently
/// backed by this one mint — registering it already implies trusting the
/// mint — so self-heal the allow-list here rather than let that combination
/// silently strand funds.
/// Fetch NIP-04 DM (kind 4) events addressed to `our_pubkey` newer than
/// `since`, from `CLAIM_RELAY_URLS`. Returns each event's raw (still
/// encrypted) content plus its `created_at`, newest last. This — not
@@ -613,6 +699,19 @@ pub async fn claim_and_redeem(data_dir: &Path) -> Result<ClaimOutcome> {
if network == EcashNetwork::Testnet {
return Ok(NO_CLAIMS);
}
// The UI polls this every 8s, and a single poll (auth + `/claim` + relay
// fetch + redeem loop) can run well past that — so a second poll starting
// before the first finishes is expected, not exceptional. Skip it rather
// than queue: both copies would otherwise load the same
// `last_dm_seen_at`, fetch/redeem the same claims, and last-writer-wins on
// save (rewinding the watermark and/or double-redeeming). Nothing is lost
// by skipping — relay DMs persist and get picked up next tick.
let Ok(_guard) = STATE_LOCK.try_lock() else {
debug!("Minibits: a claim poll is already in flight, skipping this tick");
return Ok(NO_CLAIMS);
};
ensure_mint_accepted(data_dir, &network.default_mint()).await?;
let client = reqwest::Client::builder()
@@ -663,7 +762,10 @@ pub async fn claim_and_redeem(data_dir: &Path) -> Result<ClaimOutcome> {
Ok(claims) => {
for claim in &claims {
match claim.get("token").and_then(|t| t.as_str()) {
Some(t) => state.pending_claims.push(t.to_string()),
Some(t) => state.pending_claims.push(PendingClaim {
token: t.to_string(),
attempts: 0,
}),
None => warn!("Minibits claim had no 'token' field; skipping"),
}
}
@@ -687,7 +789,10 @@ pub async fn claim_and_redeem(data_dir: &Path) -> Result<ClaimOutcome> {
warn!("Minibits: ignoring claim DM from unexpected pubkey {author}");
continue;
}
state.pending_claims.push(content);
state.pending_claims.push(PendingClaim {
token: content,
attempts: 0,
});
state.last_dm_seen_at = state.last_dm_seen_at.max(created_at);
}
@@ -704,13 +809,23 @@ pub async fn claim_and_redeem(data_dir: &Path) -> Result<ClaimOutcome> {
let to_process = std::mem::take(&mut state.pending_claims);
let mut redeemed = 0usize;
let mut sats = 0u64;
let mut dropped = 0usize;
let mut still_pending = Vec::new();
for enc in &to_process {
let decoded = match nip04::decrypt(identity.keys.secret_key(), &server_pk, enc) {
for mut claim in to_process {
let decoded = match nip04::decrypt(identity.keys.secret_key(), &server_pk, &claim.token) {
Ok(d) => d,
Err(e) => {
claim.attempts += 1;
if claim.attempts >= MAX_CLAIM_ATTEMPTS {
warn!(
"Minibits: dropping a claim after {} failed decrypt attempts ({e})",
claim.attempts
);
dropped += 1;
} else {
warn!("Minibits claim could not be decrypted ({e}); will retry next poll");
still_pending.push(enc.clone());
still_pending.push(claim);
}
continue;
}
};
@@ -720,9 +835,26 @@ pub async fn claim_and_redeem(data_dir: &Path) -> Result<ClaimOutcome> {
sats += got;
info!("Minibits: redeemed a claimed payment ({got} sats)");
}
// Terminal: the token was already swept (a watermark-rewind
// replay of an old DM, or — pre-`STATE_LOCK` — a race's double
// redeem). Retrying changes nothing, so drop it instead of
// leaving `failed_count` stuck non-zero forever.
Err(e) if is_already_redeemed(&e) => {
info!("Minibits: a claimed token was already redeemed elsewhere; dropping it ({e})");
dropped += 1;
}
Err(e) => {
claim.attempts += 1;
if claim.attempts >= MAX_CLAIM_ATTEMPTS {
warn!(
"Minibits: dropping a claim after {} failed redeem attempts ({e})",
claim.attempts
);
dropped += 1;
} else {
warn!("Minibits claim decrypted but failed to redeem ({e}); will retry next poll");
still_pending.push(enc.clone());
still_pending.push(claim);
}
}
}
}
@@ -731,13 +863,61 @@ pub async fn claim_and_redeem(data_dir: &Path) -> Result<ClaimOutcome> {
state.pending_claims = still_pending;
save_state(data_dir, &state).await?;
Ok(ClaimOutcome { claimed_count: redeemed, received_sats: sats, failed_count })
Ok(ClaimOutcome {
claimed_count: redeemed,
received_sats: sats,
failed_count,
dropped_count: dropped,
})
}
#[cfg(test)]
mod tests {
use super::*;
#[tokio::test]
async fn state_lock_rejects_a_second_concurrent_holder() {
// Regression guard for the overlapping-claim race: `claim_and_redeem`
// must skip rather than run when another call already holds
// `STATE_LOCK`, or two polls can race on `minibits.json`.
let _guard = STATE_LOCK.lock().await;
assert!(STATE_LOCK.try_lock().is_err());
}
#[test]
fn pending_claim_migrates_from_the_legacy_plain_string_shape() {
// A node's existing `minibits.json` (written before `attempts`
// existed) has `pending_claims` as a plain string array — it must
// load as `attempts: 0`, not fail or drop the queued claim.
let legacy: PendingClaim = serde_json::from_str("\"encrypted-token\"").unwrap();
assert_eq!(legacy.token, "encrypted-token");
assert_eq!(legacy.attempts, 0);
let current: PendingClaim =
serde_json::from_str(r#"{"token":"encrypted-token","attempts":3}"#).unwrap();
assert_eq!(current.token, "encrypted-token");
assert_eq!(current.attempts, 3);
}
#[test]
fn minibits_error_does_not_panic_on_a_multibyte_boundary() {
// Reproduces the byte-slice panic: a multi-byte UTF-8 character
// straddling byte 180 of a server-controlled error body must not
// crash the RPC handler's own error path.
let body = format!("{}{}", "x".repeat(179), "€".repeat(10));
let err = minibits_error(reqwest::StatusCode::BAD_REQUEST, &body);
assert!(err.to_string().contains("Minibits API error"));
}
#[test]
fn is_already_redeemed_matches_only_the_specific_mint_error() {
let redeemed = anyhow!("Could not receive this ecash: {}", super::super::mint_client::ALREADY_REDEEMED_MSG);
assert!(is_already_redeemed(&redeemed));
let unrelated = anyhow!("Could not receive this ecash: mint unreachable");
assert!(!is_already_redeemed(&unrelated));
}
#[test]
fn derived_nostr_key_matches_the_nip06_vector() {
// The Minibits app derives its Nostr key at m/44'/1237'/0'/0/0 with an
+7 -1
View File
@@ -71,10 +71,16 @@ pub struct MintResult {
/// keyset codes shared by NUT-02/03/04/05 — the codes a swap/melt/mint call
/// can actually hit. Returns `None` for anything else (e.g. Lightning/quote
/// codes in the 20000s) so the caller falls back to the mint's own `detail`.
/// Text of the NUT error-code-11001 translation, exposed so callers that
/// received an `anyhow::Error` from a receive/redeem path (e.g. Minibits
/// claim replay) can recognize an already-spent token as terminal rather than
/// retrying it forever.
pub const ALREADY_REDEEMED_MSG: &str = "This ecash has already been redeemed — it can't be claimed twice.";
fn describe_mint_error_code(code: i64) -> Option<&'static str> {
Some(match code {
10001 => "The mint rejected these coins as invalid.",
11001 => "This ecash has already been redeemed — it can't be claimed twice.",
11001 => ALREADY_REDEEMED_MSG,
11002 => "This ecash is already being redeemed elsewhere — try again in a moment.",
11003 => "The mint already issued new coins for this exact request — there's nothing left to redeem.",
11004 => "This request is still being processed by the mint — try again in a moment.",
@@ -242,6 +242,10 @@ const lnClaimedSats = ref(0)
// operator should see it rather than have it be a silent, unbounded wait.
const lnPendingClaims = ref(0)
let lnClaimTimer: ReturnType<typeof setInterval> | null = null
// A poll can outlast the 8s interval (backend auth + relay fetch + redeem
// loop) — without this, the next tick fires on top of it and both calls hit
// the backend's `minibits.json` at once.
let lnPollInFlight = false
async function loadLnAddress() {
if (lnAddress.value || lnAddressLoading.value) return
@@ -281,6 +285,8 @@ async function pollLnClaims() {
stopLnClaimPoll()
return
}
if (lnPollInFlight) return
lnPollInFlight = true
try {
const res = await rpcClient.call<{ received_sats?: number; failed_count?: number }>({
method: 'wallet.ecash-lnaddress-claim',
@@ -292,6 +298,8 @@ async function pollLnClaims() {
lnPendingClaims.value = res?.failed_count || 0
} catch {
// Transient poll failure (offline, mint busy) — keep polling.
} finally {
lnPollInFlight = false
}
}
@@ -71,3 +71,59 @@ describe('ReceiveBitcoinModal — ecash tab click', () => {
wrapper.unmount()
})
})
// Regression guard for the overlapping-claim race: a single
// wallet.ecash-lnaddress-claim call can outlast the 8s poll interval (backend
// auth + relay fetch + redeem loop), and a second call firing on top of it
// raced on the backend's minibits.json (see minibits.rs STATE_LOCK).
describe('ReceiveBitcoinModal — ecash claim poll', () => {
it('does not start a second claim poll while one is still in flight', async () => {
vi.useFakeTimers()
let resolveClaim: (v: unknown) => void = () => {}
vi.mocked(rpcClient.call).mockImplementation((args: unknown) => {
const method = (args as { method?: string })?.method
if (method === 'wallet.ecash-lnaddress') {
return Promise.resolve({ address: 'someone@minibits.cash' } as never)
}
if (method === 'wallet.ecash-lnaddress-claim') {
return new Promise((resolve) => {
resolveClaim = resolve
}) as never
}
return Promise.resolve({} as never)
})
const wrapper = mount(ReceiveBitcoinModal, {
props: { show: true },
attachTo: document.body,
})
await flushPromises()
const tabs = Array.from(document.body.querySelectorAll('button'))
const ecashTab = tabs.find((b) => b.textContent?.toLowerCase().includes('ecash'))
ecashTab!.dispatchEvent(new Event('click', { bubbles: true }))
await flushPromises()
const claimCalls = () =>
vi
.mocked(rpcClient.call)
.mock.calls.filter(([a]) => (a as { method?: string })?.method === 'wallet.ecash-lnaddress-claim').length
await vi.advanceTimersByTimeAsync(8000)
expect(claimCalls()).toBe(1)
// Second tick fires while the first claim call is still unresolved.
await vi.advanceTimersByTimeAsync(8000)
expect(claimCalls()).toBe(1)
resolveClaim({ received_sats: 0, failed_count: 0 })
await flushPromises()
// Once the in-flight call finishes, the next tick is free to poll again.
await vi.advanceTimersByTimeAsync(8000)
expect(claimCalls()).toBe(2)
wrapper.unmount()
vi.useRealTimers()
})
})