Stop ecash recovery failures from silently reusing or abandoning backup state

2026-10-06 14:34:25 -04:00
parent a7cc7084f2
commit 12e2a82b28
5 changed files with 245 additions and 33 deletions
@@ -426,7 +426,7 @@ pub async fn save_accepted_mints(data_dir: &Path, mints: &AcceptedMints) -> Resu
 /// through here. On a node with no phrase yet the source is absent and the
 /// behaviour is exactly as it was before: valid proofs, no backup.
 async fn mint_client(data_dir: &Path, mint_url: &str) -> Result<MintClient> {
-    Ok(MintClient::new(mint_url)?.with_recovery(RecoverySource::load(data_dir).await))
+    Ok(MintClient::new(mint_url)?.with_recovery(RecoverySource::load(data_dir).await?))
 }

 /// Request a mint quote — returns a Lightning invoice to pay.
@@ -1424,7 +1424,7 @@ pub struct RestoreOutcome {
 /// coins or resurrecting spent ones, which matters because the most likely
 /// time to press this button is when something already looks wrong.
 pub async fn restore_from_seed(data_dir: &Path, mint_url: &str) -> Result<RestoreOutcome> {
-    let recovery = RecoverySource::load(data_dir).await.ok_or_else(|| {
+    let recovery = RecoverySource::load(data_dir).await?.ok_or_else(|| {
        anyhow::anyhow!(
            "This wallet has no backup phrase yet, so there is nothing to restore from. \
             Set one up in Settings → Ecash backup phrase."
@@ -221,11 +221,10 @@ impl MintClient {
    /// the `(secret, blinding factor, amount)` needed to unblind the mint's
    /// signatures afterwards.
    ///
-    /// Prefers NUT-13 derivation so the resulting proofs are restorable. Falls
-    /// back to random secrets when this wallet has no phrase yet, or when the
-    /// keyset id is one NUT-13 cannot address — a random secret still mints a
-    /// perfectly valid, spendable proof, so refusing here would break the
-    /// wallet to protect a backup that does not exist.
+    /// Uses NUT-13 when the wallet has a recovery source. Derivation or durable
+    /// counter failures stop before sending a mint request; they must not
+    /// silently turn a backed-up wallet into one with unrecoverable outputs.
+    /// Legacy wallets with no seed retain their explicit random-output path.
    async fn blinded_outputs(
        &self,
        keyset_id: &str,
@@ -235,13 +234,14 @@ impl MintClient {
        Vec<(Vec<u8>, secp256k1::SecretKey, u64)>,
    )> {
        let derived = match &self.recovery {
-            Some(source) => match source.next_outputs(keyset_id, amounts.len()).await {
-                Ok(pairs) => Some(pairs),
-                Err(e) => {
-                    warn!("Minting unrecoverable proofs — NUT-13 derivation failed: {e:#}");
-                    None
-                }
-            },
+            Some(source) => Some(
+                source
+                    .next_outputs(keyset_id, amounts.len())
+                    .await
+                    .context(
+                        "Could not prepare recoverable ecash outputs; no mint request was sent",
+                    )?,
+            ),
            None => None,
        };

@@ -908,4 +908,57 @@ mod tests {
        let client = MintClient::new("http://mint.example.com").unwrap();
        assert_eq!(client.url(), "http://mint.example.com");
    }
+
+    #[tokio::test]
+    async fn backed_outputs_fail_closed_when_counter_storage_is_damaged() {
+        let directory = tempfile::tempdir().unwrap();
+        let (_, master) = crate::seed::MasterSeed::generate().unwrap();
+        super::super::nut13::establish_from_master(directory.path(), &master)
+            .await
+            .unwrap();
+        let recovery = RecoverySource::load(directory.path())
+            .await
+            .unwrap()
+            .unwrap();
+        let client = MintClient::new("http://127.0.0.1:1")
+            .unwrap()
+            .with_recovery(Some(recovery.clone()));
+        let counter = directory.path().join("wallet/cashu_counters.json");
+        tokio::fs::write(&counter, "").await.unwrap();
+        assert!(client
+            .blinded_outputs("009a1f293253e41e", &[1, 2])
+            .await
+            .is_err());
+        assert!(tokio::fs::read(&counter).await.unwrap().is_empty());
+        tokio::fs::remove_file(&counter).await.unwrap();
+        let (messages, blinding) = client
+            .blinded_outputs("009a1f293253e41e", &[1, 2])
+            .await
+            .unwrap();
+        assert_eq!(messages.len(), 2);
+        for (index, (secret, _, _)) in blinding.iter().enumerate() {
+            assert!(
+                *secret
+                    == recovery
+                        .derive_at("009a1f293253e41e", index as u32)
+                        .unwrap()
+                        .0
+            );
+        }
+        assert!(client
+            .blinded_outputs("01fc0ec0e59cd6fa", &[1])
+            .await
+            .is_err());
+    }
+
+    #[tokio::test]
+    async fn an_explicit_legacy_wallet_without_a_seed_still_prepares_outputs() {
+        let client = MintClient::new("http://127.0.0.1:1").unwrap();
+        let (messages, blinding) = client
+            .blinded_outputs("009a1f293253e41e", &[1, 2])
+            .await
+            .unwrap();
+        assert_eq!(messages.len(), 2);
+        assert_eq!(blinding.len(), 2);
+    }
 }
@@ -197,8 +197,10 @@ pub fn seed_exists(data_dir: &Path) -> bool {
 /// telling the operator their backup was fine.
 pub async fn load_seed(data_dir: &Path) -> Result<Option<EcashSeed>> {
    let path = seed_path(data_dir);
-    let Ok(content) = fs::read_to_string(&path).await else {
-        return Ok(None);
+    let content = match fs::read_to_string(&path).await {
+        Ok(content) => content,
+        Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(None),
+        Err(error) => return Err(error).context("Could not read the existing ecash backup seed"),
    };
    let stored: StoredSeed = serde_json::from_str(&content)
        .with_context(|| format!("The ecash seed file is damaged: {}", path.display()))?;
@@ -400,9 +402,10 @@ pub async fn reserve_counters(data_dir: &Path, keyset_id: &str, count: usize) ->
    let path = data_dir.join(COUNTER_FILE);

    let mut state: StoredCounters = match fs::read_to_string(&path).await {
-        Ok(content) if !content.trim().is_empty() => serde_json::from_str(&content)
+        Ok(content) => serde_json::from_str(&content)
            .with_context(|| format!("The ecash counter file is damaged: {}", path.display()))?,
-        _ => StoredCounters::default(),
+        Err(error) if error.kind() == std::io::ErrorKind::NotFound => StoredCounters::default(),
+        Err(error) => return Err(error).context("Could not read the ecash counter file"),
    };

    let start = *state.counters.get(keyset_id).unwrap_or(&0);
@@ -418,13 +421,49 @@ pub async fn reserve_counters(data_dir: &Path, keyset_id: &str, count: usize) ->
    }
    let content =
        serde_json::to_string_pretty(&state).context("Failed to serialize ecash counters")?;
-    fs::write(&path, content)
-        .await
-        .context("Failed to persist ecash counters")?;
+    persist_counters(&path, content.as_bytes()).await?;

    Ok(start)
 }

+/// Never truncate the active reservation file. A reservation is not usable
+/// until both its replacement file and directory entry have reached storage.
+async fn persist_counters(path: &Path, content: &[u8]) -> Result<()> {
+    use tokio::io::AsyncWriteExt;
+    struct PendingCounterFile(PathBuf);
+    impl Drop for PendingCounterFile {
+        fn drop(&mut self) {
+            let _ = std::fs::remove_file(&self.0);
+        }
+    }
+    let parent = path.parent().context("Counter file has no directory")?;
+    let temporary =
+        PendingCounterFile(parent.join(format!(".cashu-counters-{}.tmp", uuid::Uuid::new_v4())));
+    let mut file = fs::OpenOptions::new()
+        .write(true)
+        .create_new(true)
+        .mode(0o600)
+        .open(&temporary.0)
+        .await
+        .context("Could not create the ecash counter reservation")?;
+    file.write_all(content)
+        .await
+        .context("Could not write the ecash counter reservation")?;
+    file.sync_all()
+        .await
+        .context("Could not flush the ecash counter reservation")?;
+    drop(file);
+    fs::rename(&temporary.0, path)
+        .await
+        .context("Could not replace the ecash counter reservation")?;
+    fs::File::open(parent)
+        .await?
+        .sync_all()
+        .await
+        .context("Could not flush the ecash counter directory")?;
+    Ok(())
+}
+
 /// Read the next-unused counter for a keyset without reserving anything.
 pub async fn counter_for(data_dir: &Path, keyset_id: &str) -> u32 {
    let path = data_dir.join(COUNTER_FILE);
@@ -463,17 +502,13 @@ impl RecoverySource {
    /// Build a recovery source for this node, or `None` when the wallet has no
    /// seed yet. Callers fall back to random secrets in that case, which is
    /// exactly the pre-NUT-13 behaviour — correct, just not restorable.
-    pub async fn load(data_dir: &Path) -> Option<Self> {
-        match load_seed(data_dir).await {
-            Ok(Some(seed)) => Some(Self {
+    pub async fn load(data_dir: &Path) -> Result<Option<Self>> {
+        match load_seed(data_dir).await? {
+            Some(seed) => Ok(Some(Self {
                seed,
                data_dir: data_dir.to_path_buf(),
-            }),
-            Ok(None) => None,
-            Err(e) => {
-                warn!("Ecash wallet seed unusable, minting unrecoverable proofs: {e:#}");
-                None
-            }
+            })),
+            None => Ok(None),
        }
    }

@@ -485,6 +520,7 @@ impl RecoverySource {
    ) -> Result<Vec<(Vec<u8>, SecretKey)>> {
        // Fail the derivation *before* burning counters if this keyset id is
        // one NUT-13 cannot address.
+        self.seed.derive_output(keyset_id, 0)?;
        let start = reserve_counters(&self.data_dir, keyset_id, count).await?;
        (0..count)
            .map(|i| self.seed.derive_output(keyset_id, start + i as u32))
@@ -763,8 +799,8 @@ mod tests {

        assert!(load_seed(d).await.is_err());
        assert!(
-            RecoverySource::load(d).await.is_none(),
-            "an unusable seed must not be presented as a working one"
+            RecoverySource::load(d).await.is_err(),
+            "an unusable seed must not downgrade to an unbacked wallet"
        );
    }

@@ -775,7 +811,10 @@ mod tests {
        let (_, master) = MasterSeed::from_mnemonic_words(TEST_MNEMONIC).unwrap();
        establish_from_master(d, &master).await.unwrap();

-        let source = RecoverySource::load(d).await.expect("seed was established");
+        let source = RecoverySource::load(d)
+            .await
+            .unwrap()
+            .expect("seed was established");
        let first = source.next_outputs(V1_KEYSET, 2).await.unwrap();
        let second = source.next_outputs(V1_KEYSET, 2).await.unwrap();

@@ -794,4 +833,81 @@ mod tests {
            assert_eq!(secret, &expected);
        }
    }
+
+    #[tokio::test]
+    async fn missing_seed_is_distinct_from_a_seed_read_failure() {
+        let dir = tempfile::tempdir().unwrap();
+        assert!(RecoverySource::load(dir.path()).await.unwrap().is_none());
+        fs::create_dir_all(seed_path(dir.path())).await.unwrap();
+        assert!(load_seed(dir.path()).await.is_err());
+        assert!(RecoverySource::load(dir.path()).await.is_err());
+        assert!(seed_path(dir.path()).is_dir());
+    }
+
+    #[tokio::test]
+    async fn empty_or_corrupt_counters_never_reset_a_reservation() {
+        let dir = tempfile::tempdir().unwrap();
+        fs::create_dir_all(dir.path().join("wallet")).await.unwrap();
+        let path = dir.path().join(COUNTER_FILE);
+        for damaged in ["", "   ", "{ truncated"] {
+            fs::write(&path, damaged).await.unwrap();
+            assert!(reserve_counters(dir.path(), V1_KEYSET, 1).await.is_err());
+            assert_eq!(fs::read_to_string(&path).await.unwrap(), damaged);
+        }
+        fs::remove_file(&path).await.unwrap();
+        fs::create_dir(&path).await.unwrap();
+        assert!(reserve_counters(dir.path(), V1_KEYSET, 1).await.is_err());
+        assert!(path.is_dir());
+    }
+
+    #[tokio::test]
+    async fn concurrent_counter_reservations_survive_reload_and_leave_no_temporary_files() {
+        let dir = tempfile::tempdir().unwrap();
+        let mut tasks = Vec::new();
+        for _ in 0..24 {
+            let path = dir.path().to_path_buf();
+            tasks.push(tokio::spawn(async move {
+                reserve_counters(&path, V1_KEYSET, 2).await.unwrap()
+            }));
+        }
+        let mut starts = std::collections::HashSet::new();
+        for task in tasks {
+            assert!(starts.insert(task.await.unwrap()));
+        }
+        assert_eq!(counter_for(dir.path(), V1_KEYSET).await, 48);
+        assert_eq!(
+            reserve_counters(dir.path(), V1_KEYSET, 1).await.unwrap(),
+            48
+        );
+        let entries = std::fs::read_dir(dir.path().join("wallet"))
+            .unwrap()
+            .map(|entry| entry.unwrap().file_name())
+            .collect::<Vec<_>>();
+        assert_eq!(
+            entries,
+            vec![std::ffi::OsString::from("cashu_counters.json")]
+        );
+        #[cfg(unix)]
+        {
+            use std::os::unix::fs::PermissionsExt;
+            assert_eq!(
+                std::fs::metadata(dir.path().join(COUNTER_FILE))
+                    .unwrap()
+                    .permissions()
+                    .mode()
+                    & 0o777,
+                0o600
+            );
+        }
+    }
+
+    #[tokio::test]
+    async fn invalid_derivation_does_not_reserve_counters() {
+        let dir = tempfile::tempdir().unwrap();
+        let (_, master) = MasterSeed::from_mnemonic_words(TEST_MNEMONIC).unwrap();
+        establish_from_master(dir.path(), &master).await.unwrap();
+        let source = RecoverySource::load(dir.path()).await.unwrap().unwrap();
+        assert!(source.next_outputs("01fc0ec0e59cd6fa", 1).await.is_err());
+        assert!(!dir.path().join(COUNTER_FILE).exists());
+    }
 }