-
Notifications
You must be signed in to change notification settings - Fork 1
feat(disputes): make the dispute chat survive a restart #256
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
703370b
3e21328
6e4d079
37b1964
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,7 +14,8 @@ use std::sync::OnceLock; | |
| use tokio::sync::{broadcast, RwLock}; | ||
| use tokio::sync::broadcast::error::RecvError; | ||
|
|
||
| use crate::api::types::{Dispute, DisputeResolution, DisputeStatus}; | ||
| use crate::api::types::{Dispute, DisputeResolution, DisputeStatus, OrderStatus}; | ||
| use crate::db::Storage; | ||
|
|
||
| // ── Dispute store ───────────────────────────────────────────────────────────── | ||
|
|
||
|
|
@@ -296,6 +297,7 @@ pub async fn handle_admin_took_dispute(trade_id: String, admin_pubkey: String) - | |
| }) | ||
| .await; | ||
| log::info!("[disputes] created record for peer-opened dispute trade={trade_id}"); | ||
| persist_admin_pubkey(&trade_id, &admin_pubkey_for_key).await; | ||
| return derive_admin_shared_key(&trade_id, &admin_pubkey_for_key).await; | ||
| } | ||
|
|
||
|
|
@@ -325,9 +327,78 @@ pub async fn handle_admin_took_dispute(trade_id: String, admin_pubkey: String) - | |
| }) | ||
| .await?; | ||
|
|
||
| persist_admin_pubkey(&trade_id, &admin_pubkey_for_key).await; | ||
| derive_admin_shared_key(&trade_id, &admin_pubkey_for_key).await | ||
| } | ||
|
|
||
| /// Persist the solver pubkey so the dispute chat survives a restart. | ||
| /// | ||
| /// Deliberately narrow: the dispute record stays in memory (its status and | ||
| /// resolution come back from daemon events), but this pubkey arrives exactly | ||
| /// once and cannot be re-derived. Without it, a restart mid-dispute leaves the | ||
| /// party unable to reach the solver at all. | ||
| /// | ||
| /// Best-effort — a storage failure must not undo an already-applied dispute | ||
| /// update, and the live listener keeps working for this session. | ||
| async fn persist_admin_pubkey(order_id: &str, admin_pubkey_hex: &str) { | ||
| let Some(db) = crate::db::app_db::db() else { | ||
| log::warn!("[disputes] no store — solver pubkey will not survive a restart"); | ||
| return; | ||
| }; | ||
| if let Err(e) = db | ||
| .set_setting( | ||
| &crate::db::settings_keys::dispute_admin(order_id), | ||
| admin_pubkey_hex, | ||
| ) | ||
| .await | ||
| { | ||
| log::warn!("[disputes] could not persist solver pubkey for {order_id}: {e}"); | ||
| } | ||
| } | ||
|
|
||
| /// Drop the persisted solver pubkey for `order_id`. | ||
| /// | ||
| /// The stored key is what rehydration reads as "this order has a live | ||
| /// dispute", so it must not outlive the dispute: left behind, every restart | ||
| /// would resurrect a finished dispute as `InReview`, arm a listener for it and | ||
| /// keep accepting evidence. Called both when a resolution reaches the store and | ||
| /// when rehydration meets an already-finished trade. | ||
| /// | ||
| /// Best-effort like the write: a storage failure only means the stale key is | ||
| /// seen again — and cleared again — on the next pass. | ||
| async fn clear_admin_pubkey(order_id: &str) { | ||
| let Some(db) = crate::db::app_db::db() else { | ||
| return; | ||
| }; | ||
| if let Err(e) = db | ||
| .delete_setting(&crate::db::settings_keys::dispute_admin(order_id)) | ||
| .await | ||
| { | ||
| log::warn!("[disputes] could not clear solver pubkey for {order_id}: {e}"); | ||
| } | ||
| } | ||
|
|
||
| /// `true` when the order reached a state in which no dispute can still be live. | ||
| /// | ||
| /// This is what keeps rehydration from resurrecting finished disputes, and it | ||
| /// deliberately reads the *trade* status rather than the dispute record: the | ||
| /// daemon's `admin-settled` / `admin-canceled` are persisted by the status-sync | ||
| /// arm in `orders.rs`, which does not route them into the dispute store, so the | ||
| /// trade row is the durable evidence that the dispute is over. | ||
| fn is_order_finished(status: &OrderStatus) -> bool { | ||
| matches!( | ||
| status, | ||
| OrderStatus::SettledByAdmin | ||
| | OrderStatus::CanceledByAdmin | ||
| | OrderStatus::CompletedByAdmin | ||
| | OrderStatus::Success | ||
| | OrderStatus::Canceled | ||
| | OrderStatus::CooperativelyCanceled | ||
| | OrderStatus::Expired | ||
| ) | ||
| } | ||
|
|
||
|
|
||
| /// Derive the dispute-chat keys for `trade_id` and start listening. | ||
| /// | ||
| /// Both sides ECDH their trade key against the solver's pubkey and split the | ||
|
|
@@ -382,7 +453,91 @@ async fn derive_admin_shared_key(trade_id: &str, admin_pubkey_hex: &str) -> Resu | |
| /// would stay invisible for the rest of the process. Idempotent — the | ||
| /// per-channel single-owner guard makes a spawn for an already-listening | ||
| /// dispute a no-op. | ||
| /// Rebuild dispute records for orders with a persisted solver pubkey. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This introduces externally observable persistence and restart-rehydration behavior for AGENTS.md reference: AGENTS.md:L75-L75 Useful? React with 👍 / 👎. |
||
| /// | ||
| /// Trades are the enumeration source, so no key-prefix scan is needed: each | ||
| /// persisted trade is asked whether it has a stored solver. Records already in | ||
| /// memory win — they are at least as fresh as storage. | ||
| /// | ||
| /// Status is `InReview`: a stored solver means one took the dispute, and any | ||
| /// later resolution arrives as a daemon event. Restoring the record is what | ||
| /// lets `submit_evidence` work again after a restart, since it refuses without | ||
| /// one. Only *unfinished* trades are restored — see [`is_order_finished`]. | ||
| /// | ||
| /// **Web has no rehydration.** `lib/main.dart` skips `initDb` off native, and | ||
| /// the IndexedDB store's `list_trades` is still the empty stub of #233, so both | ||
| /// the store lookup and the enumeration source are missing there. A browser | ||
| /// reload therefore still loses the solver pubkey; the persistence path lights | ||
| /// up on web once #233 lands trade persistence, with no change needed here. | ||
| async fn rehydrate_disputes_from_storage() { | ||
| let Some(db) = crate::db::app_db::db() else { | ||
| return; | ||
| }; | ||
| let trades = match db.list_trades().await { | ||
| Ok(t) => t, | ||
| Err(e) => { | ||
| log::warn!("[disputes] rehydrate: list_trades failed: {e}"); | ||
| return; | ||
| } | ||
| }; | ||
|
|
||
| for trade in trades { | ||
| let order_id = trade.order.id.clone(); | ||
| if dispute_store().get(&order_id).await.is_some() { | ||
| continue; | ||
| } | ||
| let admin_hex = match db | ||
| .get_setting(&crate::db::settings_keys::dispute_admin(&order_id)) | ||
| .await | ||
| { | ||
| Ok(Some(hex)) => hex, | ||
| Ok(None) => continue, | ||
| Err(e) => { | ||
| log::warn!("[disputes] rehydrate: reading solver for {order_id}: {e}"); | ||
| continue; | ||
| } | ||
| }; | ||
|
|
||
| // The trade already ended — the solver key is stale. Restoring it here | ||
| // would recreate the dispute as `InReview` on every single restart, | ||
| // arm a listener nobody is on the other end of, and keep letting | ||
| // evidence be submitted against a closed case. Clear it instead. | ||
| if is_order_finished(&trade.order.status) { | ||
| log::info!( | ||
| "[disputes] rehydrate: dropping stale solver for finished order={order_id} status={:?}", | ||
| trade.order.status | ||
| ); | ||
| clear_admin_pubkey(&order_id).await; | ||
| continue; | ||
| } | ||
|
|
||
| dispute_store() | ||
| .upsert(Dispute { | ||
| id: uuid::Uuid::new_v4().to_string(), | ||
| trade_id: order_id.clone(), | ||
| status: DisputeStatus::InReview, | ||
|
Comment on lines
+622
to
+626
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a stored trade has already reached Useful? React with 👍 / 👎. |
||
| initiated_by_me: false, | ||
| reason: None, | ||
| admin_pubkey: Some(admin_hex), | ||
| resolution: None, | ||
| opened_at: unix_now(), | ||
| resolved_at: None, | ||
| // The pre-restart read state is not recoverable, and this is | ||
| // an active dispute waiting on the user — default to unread so | ||
| // it surfaces rather than being silently marked as seen. | ||
| is_read: false, | ||
| }) | ||
| .await; | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| log::info!("[disputes] rehydrated dispute record order={order_id}"); | ||
| } | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| pub(crate) async fn resubscribe_active_dispute_chats() { | ||
| // A restart leaves the in-memory store empty, so the loop below would find | ||
| // nothing to re-arm. Refill it from the persisted solver pubkeys first — | ||
| // that is the one piece of a dispute that cannot be re-derived. | ||
| rehydrate_disputes_from_storage().await; | ||
|
|
||
| for dispute in dispute_store().all().await { | ||
| if dispute.status != DisputeStatus::InReview { | ||
| continue; | ||
|
|
@@ -416,7 +571,14 @@ async fn resolve_dispute(trade_id: String, resolution: DisputeResolution) -> Res | |
| dispute.is_read = false; | ||
| Ok(()) | ||
| }) | ||
| .await | ||
| .await?; | ||
|
|
||
| // The dispute is over, so the solver pubkey has nothing left to unlock — | ||
| // and leaving it stored would have the next restart rehydrate this exact | ||
| // dispute back to `InReview`. Only on success: a rejected resolution left | ||
| // the dispute live. | ||
| clear_admin_pubkey(&trade_id).await; | ||
| Ok(()) | ||
| } | ||
|
|
||
| // ── Stream ──────────────────────────────────────────────────────────────────── | ||
|
|
@@ -562,6 +724,113 @@ mod tests { | |
| assert!(err.to_string().contains("EvidenceEmpty")); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn the_solver_pubkey_outlives_the_in_memory_record() { | ||
| // The dispute record is in-memory by design, so a restart drops it. | ||
| // The solver pubkey must not go with it: it arrives once, in | ||
| // admin-took-dispute, and without it the chat keys cannot be derived | ||
| // again — the party would be left unable to reach the solver. | ||
| let path = std::env::temp_dir() | ||
| .join(format!("mostro_dispute_kv_{}.db", std::process::id())); | ||
| let _ = std::fs::remove_file(&path); | ||
| let db = crate::db::sqlite::SqliteStorage::open(path.to_str().unwrap()) | ||
| .await | ||
| .unwrap(); | ||
|
|
||
| let order_id = "order-dispute-1"; | ||
| let admin_pk = "0000000000000000000000000000000000000000000000000000000000000003"; | ||
| let key = crate::db::settings_keys::dispute_admin(order_id); | ||
|
|
||
| assert_eq!(db.get_setting(&key).await.unwrap(), None); | ||
|
|
||
| db.set_setting(&key, admin_pk).await.unwrap(); | ||
|
|
||
| // Reopen: this is the restart the persistence exists for. | ||
| drop(db); | ||
| let db = crate::db::sqlite::SqliteStorage::open(path.to_str().unwrap()) | ||
| .await | ||
| .unwrap(); | ||
| assert_eq!(db.get_setting(&key).await.unwrap().as_deref(), Some(admin_pk)); | ||
|
|
||
| drop(db); | ||
| let _ = std::fs::remove_file(&path); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn clearing_the_solver_key_stops_the_dispute_coming_back() { | ||
| // Rehydration reads the stored solver as "this order has a live | ||
| // dispute". Once the dispute ends the key must be gone, or every | ||
| // restart resurrects it as InReview forever. | ||
| let path = std::env::temp_dir().join(format!( | ||
| "mostro_dispute_kv_clear_{}.db", | ||
| std::process::id() | ||
| )); | ||
| let _ = std::fs::remove_file(&path); | ||
| let db = crate::db::sqlite::SqliteStorage::open(path.to_str().unwrap()) | ||
| .await | ||
| .unwrap(); | ||
|
|
||
| let key = crate::db::settings_keys::dispute_admin("order-resolved-1"); | ||
| db.set_setting(&key, "00000000000000000000000000000000000000000000000000000000000000aa") | ||
| .await | ||
| .unwrap(); | ||
| db.delete_setting(&key).await.unwrap(); | ||
|
|
||
| // Reopen: the restart that used to bring the dispute back. | ||
| drop(db); | ||
| let db = crate::db::sqlite::SqliteStorage::open(path.to_str().unwrap()) | ||
| .await | ||
| .unwrap(); | ||
| assert_eq!(db.get_setting(&key).await.unwrap(), None); | ||
|
|
||
| drop(db); | ||
| let _ = std::fs::remove_file(&path); | ||
| } | ||
|
|
||
| #[test] | ||
| fn finished_orders_are_not_rehydrated() { | ||
| // The admin verdicts are the ones that end a dispute, and `orders.rs` | ||
| // persists them on the trade even though nothing routes them into the | ||
| // dispute store — so they are what rehydration has to check. | ||
| for status in [ | ||
| OrderStatus::SettledByAdmin, | ||
| OrderStatus::CanceledByAdmin, | ||
| OrderStatus::CompletedByAdmin, | ||
| OrderStatus::Success, | ||
| OrderStatus::Canceled, | ||
| OrderStatus::CooperativelyCanceled, | ||
| OrderStatus::Expired, | ||
| ] { | ||
| assert!(is_order_finished(&status), "{status:?} should be finished"); | ||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn a_live_dispute_is_still_rehydrated() { | ||
| // The whole point of the feature: an order still under dispute (or | ||
| // otherwise mid-flight) must come back after a restart. | ||
| for status in [ | ||
| OrderStatus::Dispute, | ||
| OrderStatus::InProgress, | ||
| OrderStatus::Active, | ||
| OrderStatus::FiatSent, | ||
| OrderStatus::SettledHoldInvoice, | ||
| ] { | ||
| assert!(!is_order_finished(&status), "{status:?} should stay live"); | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| } | ||
|
|
||
| #[test] | ||
| fn each_order_gets_its_own_solver_key() { | ||
| // One party can have several disputed orders, each with its own solver. | ||
| assert_ne!( | ||
| crate::db::settings_keys::dispute_admin("order-a"), | ||
| crate::db::settings_keys::dispute_admin("order-b") | ||
| ); | ||
| assert!(crate::db::settings_keys::dispute_admin("order-a") | ||
| .starts_with(crate::db::settings_keys::DISPUTE_ADMIN_PREFIX)); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn a_peer_opened_dispute_still_records_the_solver() { | ||
| // The party that did not open the dispute has no local record when | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Flutter web,
lib/main.dartexplicitly skipsinitDb, soapp_db::db()is alwaysNoneand every new persistence attempt returns here without writing. Even if the IndexedDB settings store were initialized, itslist_trades()currently always returns an empty list, leaving rehydration with no enumeration source. Consequently, browser reloads still lose the solver pubkey and dispute chat despite this feature; web needs an initialized store plus an enumeration strategy that does not depend on unimplemented trade persistence.Useful? React with 👍 / 👎.