Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
252 changes: 237 additions & 15 deletions rust/src/api/orders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1861,6 +1861,14 @@ async fn dispatch_mostro_message(
log::info!("[orders] gift-wrap Canceled for trade={trade_pubkey_hex}");
if let Some(order_id) = &kind.id {
let oid = order_id.to_string();
// A stale Canceled replayed over a finished trade — e.g. the
// taker-timeout cancel of an order that was later re-taken
// and completed — must not overwrite the terminal outcome.
// The wipe path below is unaffected: it starts from
// pending/waiting, which are not terminal.
if status_sync_blocked_by_terminal(&oid, &kind.action).await {
return;
}
// Deliberately NOT removed from the order book. The book is
// fed only by the daemon's Kind 38383 events, and on a
// taker-responsible timeout mostrod republishes the order as
Expand Down Expand Up @@ -1991,6 +1999,9 @@ async fn dispatch_mostro_message(
.and_then(map_core_status)
.or_else(|| status_for_action(&kind.action))
{
if status_sync_blocked_by_terminal(&order_id, &kind.action).await {
Comment thread
Catrya marked this conversation as resolved.
Outdated
return;
}
crate::api::logging::blog_info(
"orders",
format!(
Expand Down Expand Up @@ -2034,6 +2045,9 @@ async fn dispatch_mostro_message(
);
return;
};
if status_sync_blocked_by_terminal(&order_id, &kind.action).await {
return;
}
crate::api::logging::blog_info(
"orders",
format!(
Expand Down Expand Up @@ -2108,6 +2122,9 @@ async fn dispatch_mostro_message(
bolt11.len(),
amount
);
if status_sync_blocked_by_terminal(&order_id, &kind.action).await {
return;
}
// Save the hold invoice and update status to WaitingPayment.
crate::api::logging::blog_info(
"orders",
Expand Down Expand Up @@ -2168,6 +2185,9 @@ async fn dispatch_mostro_message(
// reply classification).
let new_status = status_for_action(&kind.action);
if let Some(status) = new_status {
if status_sync_blocked_by_terminal(&order_id, &kind.action).await {
return;
}
crate::api::logging::blog_info(
"orders",
format!(
Expand Down Expand Up @@ -2365,6 +2385,58 @@ fn status_for_action(action: &mostro_core::message::Action) -> Option<OrderStatu
}
}

/// Statuses no daemon message may leave: mostrod never reopens a canceled
/// or completed trade. `SettledHoldInvoice` and `Dispute` are deliberately
/// NOT here — they still progress (to `Success` / admin resolutions).
fn is_hard_terminal(status: &OrderStatus) -> bool {
matches!(
status,
OrderStatus::Canceled
| OrderStatus::CanceledByAdmin
| OrderStatus::CooperativelyCanceled
| OrderStatus::Expired
| OrderStatus::Success
| OrderStatus::SettledByAdmin
| OrderStatus::CompletedByAdmin
)
}

/// Current locally known status for a trade: the DB row when present
/// (authoritative across restarts), else the in-memory book entry.
async fn current_local_status(order_id: &str) -> Option<OrderStatus> {
if let Some(db) = crate::db::app_db::db() {
if let Ok(Some(trade)) = db.get_trade_by_order_id(order_id).await {
return Some(trade.order.status);
}
}
order_book().get_order(order_id).await.map(|o| o.status)
}

/// True when a Kind 14 status sync must be skipped: the trade already sits
/// in a hard-terminal status. Relays deliver the startup backlog
/// newest-first, so a progression message that would move a finished trade
/// is an out-of-order replay, not a real transition — applying it walks
/// the status backwards and re-emits action requests to the UI.
async fn status_sync_blocked_by_terminal(
order_id: &str,
action: &mostro_core::message::Action,
) -> bool {
let Some(local) = current_local_status(order_id).await else {
return false;
};
if is_hard_terminal(&local) {
crate::api::logging::blog_debug(
"orders",
format!(
"skip replayed {action:?} order={}: already {local:?}",
crate::api::logging::short_id(order_id),
),
);
return true;
}
false
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

/// Extracts the status and calculated sats to persist from an inbound
/// `add-invoice` payload.
///
Expand Down Expand Up @@ -3109,14 +3181,7 @@ pub(crate) async fn refresh_subscriptions_for_active_node() {
}
};

let trade_key_map = build_trade_key_map().await;
let trade_pubkeys: Vec<nostr_sdk::PublicKey> = trade_key_map
.keys()
.filter_map(|hex| nostr_sdk::PublicKey::from_hex(hex).ok())
.collect();
// Seed the refreshable coverage map: keys derived after this point join
// it (and the relay filter) via ensure_global_dm_coverage.
*global_dm_keys().write().await = trade_key_map;
let trade_pubkeys = seed_global_dm_coverage().await;

if let Err(e) = subscribe_node_filters(&client, mostro_pubkey, trade_pubkeys).await {
log::error!("[orders] node switch: re-subscribe failed: {e}");
Expand Down Expand Up @@ -3145,6 +3210,13 @@ pub(crate) async fn refresh_subscriptions_for_active_node() {
/// derived later — a new order or take — was covered only by the 30-minute
/// per-trade receiver, and a solver assignment arriving after that expired
/// was never decrypted.
///
/// Seeded in full by BOTH subscription entry points — startup and node
/// switch — via [`seed_global_dm_coverage`]; `ensure_global_dm_coverage`
/// adds keys derived mid-session. The event loop decrypts against this map
/// and `resubscribe_global_dm_filter` rebuilds the relay filter from it
/// alone, so an unseeded or shrunk map makes previous sessions' trades
/// undecryptable and silently unsubscribes them.
static GLOBAL_DM_KEYS: std::sync::OnceLock<
tokio::sync::RwLock<HashMap<String, (nostr_sdk::Keys, u32)>>,
> = std::sync::OnceLock::new();
Expand Down Expand Up @@ -3211,6 +3283,22 @@ async fn resubscribe_global_dm_filter() {
}
}

/// Derive every known trade key and merge it into the refreshable coverage
/// map, returning the full pubkey set for the relay filter.
///
/// Union, not replace: a session key inserted concurrently (create/take in
/// flight while subscriptions restart) must never be evicted.
async fn seed_global_dm_coverage() -> Vec<nostr_sdk::PublicKey> {
let derived = build_trade_key_map().await;
let mut map = global_dm_keys().write().await;
for (hex, entry) in derived {
map.entry(hex).or_insert(entry);
}
map.keys()
.filter_map(|hex| nostr_sdk::PublicKey::from_hex(hex).ok())
.collect()
}

async fn build_trade_key_map() -> HashMap<String, (nostr_sdk::Keys, u32)> {
let mut map = HashMap::new();
let max_index = match crate::api::identity::get_identity().await {
Expand Down Expand Up @@ -3464,13 +3552,12 @@ async fn _run_order_subscription() {
};
crate::api::logging::blog_info("orders", format!("subscribing to Kind 38383 from mostro={}", mostro_pubkey.to_hex()));

// Build a map of all known trade keys so we can decrypt ANY kind-14
// Mostro reply, not just those from the current session.
let trade_key_map = build_trade_key_map().await;
let trade_pubkeys: Vec<nostr_sdk::PublicKey> = trade_key_map
.keys()
.filter_map(|hex| nostr_sdk::PublicKey::from_hex(hex).ok())
.collect();
// Derive and seed the decryption coverage for ALL known trade keys —
// the event loop decrypts against global_dm_keys, not a local map, and
// resubscribe_global_dm_filter rebuilds the relay filter from it alone.
// Unseeded, every previous session's trade is undecryptable and falls
// off the filter on the session's first create or take.
let trade_pubkeys = seed_global_dm_coverage().await;
crate::api::logging::blog_info("orders", format!("trade key map: {} keys derived for gift-wrap decryption", trade_pubkeys.len()));

// Get notifications receiver before subscribing to avoid missing
Expand Down Expand Up @@ -4396,6 +4483,141 @@ mod tests {
assert_eq!(global_dm_keys().read().await.len(), before);
}

/// The hard-terminal set must match protocol finality: statuses mostrod
/// never reopens block replayed syncs, while statuses that still
/// progress (settled → success, dispute → admin resolution) must not.
#[test]
fn hard_terminal_matches_protocol_finality() {
use crate::api::types::OrderStatus as S;
for s in [
S::Canceled,
S::CanceledByAdmin,
S::CooperativelyCanceled,
S::Expired,
S::Success,
S::SettledByAdmin,
S::CompletedByAdmin,
] {
assert!(is_hard_terminal(&s), "{s:?} must be terminal");
}
for s in [
S::Pending,
S::WaitingBuyerInvoice,
S::WaitingPayment,
S::Active,
S::FiatSent,
S::SettledHoldInvoice,
S::Dispute,
S::InProgress,
] {
assert!(!is_hard_terminal(&s), "{s:?} must not be terminal");
}
}

/// Startup replays arrive newest-first: a progression message for a
/// trade already terminal is an out-of-order replay and must be
/// skipped; open trades and unknown orders must not be blocked.
#[tokio::test]
async fn terminal_trades_block_replayed_status_syncs() {
use mostro_core::message::Action;

let canceled_id = uuid::Uuid::new_v4().to_string();
let mut canceled = dummy_order_info(&canceled_id);
canceled.status = crate::api::types::OrderStatus::Canceled;
order_book().upsert_order(canceled).await;
assert!(
status_sync_blocked_by_terminal(&canceled_id, &Action::WaitingSellerToPay)
.await
);

let active_id = uuid::Uuid::new_v4().to_string();
let mut active = dummy_order_info(&active_id);
active.status = crate::api::types::OrderStatus::Active;
order_book().upsert_order(active).await;
assert!(
!status_sync_blocked_by_terminal(&active_id, &Action::FiatSentOk).await
);

// Unknown order: nothing local to protect, sync proceeds.
assert!(
!status_sync_blocked_by_terminal("no-such-order", &Action::AddInvoice).await
);
}

/// A stale Canceled replayed over a finished trade (the taker-timeout
/// cancel of an order later re-taken and completed) must be skipped
/// entirely at the handler level: no status write, no TradeUpdate.
#[tokio::test]
async fn replayed_cancel_over_terminal_trade_is_skipped() {
use mostro_core::message::{Action, Message};

let order_uuid = uuid::Uuid::new_v4();
let order_id = order_uuid.to_string();
let mut done = dummy_order_info(&order_id);
done.status = crate::api::types::OrderStatus::Success;
order_book().upsert_order(done).await;

let mut rx = trade_updates_tx().subscribe();

let sender = nostr_sdk::PublicKey::from_hex(&active_mostro_pubkey())
.expect("valid mostro pubkey");
let unwrapped = mostro_core::nip59::UnwrappedMessage {
message: Message::new_order(
Some(order_uuid),
None,
None,
Action::Canceled,
None,
),
signature: None,
sender,
identity: sender,
created_at: nostr_sdk::Timestamp::from(0u64),
};
dispatch_mostro_message(unwrapped, "test-cancel-replay", "ff00ff00", 1).await;

// The book entry keeps its terminal outcome...
let status = order_book()
.get_order(&order_id)
.await
.expect("order still cached")
.status;
assert_eq!(status, crate::api::types::OrderStatus::Success);

// ...and no TradeUpdate was emitted for this order. Drain the
// broadcast (parallel tests may emit for other orders) and filter
// by our id; the suppressed emission would already be buffered by
// the time dispatch returned.
let mut leaked = false;
while let Ok(update) = rx.try_recv() {
if update.order_id == order_id {
leaked = true;
}
}
assert!(!leaked, "stale Canceled must not emit a TradeUpdate");
}

/// #277 cause 3: the coverage seed must be a union that never evicts a
/// key already in the map. A replace (or a missing seed at startup)
/// leaves previous sessions' trades undecryptable — their kind-14s drop
/// as no-matching-p-tag — and the next relay-filter rebuild silently
/// unsubscribes them.
#[tokio::test]
async fn seeding_coverage_never_evicts_existing_keys() {
let session = nostr_sdk::Keys::generate();
ensure_global_dm_coverage(&session, 92).await;

// No identity in unit tests → the derived set is empty; the seed
// must still keep the session key and report it for the filter.
let pubkeys = seed_global_dm_coverage().await;

assert!(global_dm_keys()
.read()
.await
.contains_key(&session.public_key().to_hex()));
assert!(pubkeys.contains(&session.public_key()));
}

/// PR #252 review (ermeme P1): a create rejected for an unsupported node
/// protocol must fail BEFORE any maker-ownership record is persisted. The
/// content fingerprint is durable — were it stored, any later public order
Expand Down
45 changes: 45 additions & 0 deletions specs/004-mostro-p2p-client/contracts/orders.md
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,39 @@ call — it arrives as a Kind 14 (NIP-44) message from mostrod. This
section documents the full chain so Flutter providers and screens know
what to listen to. Reference: <https://mostro.network/protocol/seller_pay_hold_invoice.html>.

### Kind-14 delivery & decryption coverage

Receiving a daemon Kind 14 takes two independent layers, and BOTH must
cover the trade or its messages are lost (dropped as
`no-matching-p-tag`, observable in the logs with the map size):

- **Delivery** — the bulk `mostro-dm` relay subscription, author-pinned to
the active node, whose `#p` filter must include the trade key's pubkey.
- **Decryption** — the refreshable coverage map (`global_dm_keys`,
pubkey → keys+index) the event loop decrypts against.

Coverage invariants:

- **Both subscription entry points seed in full.** Startup
(`_run_order_subscription`) and node switch derive every known trade key
(indexes `1..=identity.trade_key_index`) and seed the map through the
shared `seed_global_dm_coverage()` before subscribing. A session that
does not rehydrate leaves every previous session's trade deaf: statuses
freeze at whatever the public Kind 38383 shows (masked `in-progress`),
requests like add-invoice never reach the user, and the daemon
eventually cancels by timeout (#277 cause 3).
- **Seeding is a union, never a replace** — a key derived concurrently by
a create/take in flight must survive the seed.
- **Mid-session keys join incrementally**: every derive path calls
`ensure_global_dm_coverage`, which inserts the key and re-issues the
relay filter under the same stable subscription id.
- **The relay filter is always rebuilt from the full map** — never from
session-local state. A rebuild from a subset silently unsubscribes the
missing trades at the relay.
- The temporary 30-minute per-trade receivers (see #182) are an
additional delivery path, not a substitute: they exist only for trades
touched this session and mask coverage gaps while they run.

### Inbound Kind 14 actions consumed by `dispatch_mostro_message`

| Action | Payload variant | Effect on the local trade row |
Expand All @@ -265,6 +298,18 @@ what to listen to. Reference: <https://mostro.network/protocol/seller_pay_hold_i
| `AdminSettled` / `AdminCanceled` | (status sync) | `status → SettledByAdmin` / `CanceledByAdmin` |
| `Canceled` | (none) | Never-active trade (pending/waiting): row + in-memory session **deleted**; otherwise `status → Canceled` (history kept). See below. |

A sync that would move a trade out of a **hard-terminal** status
(`Canceled` / `CanceledByAdmin` / `CooperativelyCanceled` / `Expired` /
`Success` / `SettledByAdmin` / `CompletedByAdmin`) is skipped entirely —
no book/DB write, no emission. Relays deliver the startup backlog
newest-first, so such a message is an out-of-order replay, not a real
transition; mostrod never reopens a finished trade. `SettledHoldInvoice`
and `Dispute` still progress and are deliberately not in the set.
`Canceled` applies the same guard — a stale timeout-cancel replayed over
an order that was later re-taken and completed must not overwrite the
outcome; its wipe path is unaffected, since it starts from non-terminal
waiting states.

Every arm above that syncs a status also emits a `TradeUpdate` (see
`on_trade_updated`) after the in-memory book update and the DB
persistence attempt — DB failures are logged, never suppress the
Expand Down
Loading