fix: phantom orders in order book and My Trades on daemon timeout - #159
Merged
Conversation
- create_order no longer inserts the optimistic order into the public book store. - This prevents a phantom with no Kind 38383 from appearing after a daemon timeout or late rejection. - The maker still sees it in My Trades; the book is sourced only from Kind 38383 events.
- New Storage::delete_trade(id) deletes the trade row by its order ID. - SQLite runs DELETE FROM trades WHERE id = ?; IndexedDB stays a stub. - Shared infra used by the late-CantDo and orphan-expiry cleanups.
- On no daemon response, create_order returns an error and persists nothing, matching v1. - The timeout is raised from 5s to 10s to tolerate a merely-slow node. - The add-order screen shows a localized no response message; no fake pending order is created.
- A one-shot startup sweep removes pending trades absent from the book and past a grace age. - It self-heals phantoms persisted by older app versions now that create_order never persists them. - A status-and-book guard ensures real pending or taken orders are never deleted.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
WalkthroughThe order submit flow now uses a localized timeout message when the daemon does not respond. The Rust order API returns ChangesOrder timeout handling
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rust/src/api/orders.rs (1)
541-564: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRollback the fingerprint bookkeeping on rejection and timeout.
Unlike the publish-failure branch, these early returns leave
trade_key_map,pending_maker_keys, andpending_local_idspopulated._run_order_subscription()later treats that content fingerprint as proof of ownership, so a later unrelated order with the sameckcan be markedis_mineand reconciled against the abandoned local UUID.Suggested fix
+ let rollback_pending_create = || { + if let Ok(mut m) = trade_key_map().write() { + m.remove(&order.id); + m.remove(&ck); + } + if let Ok(mut m) = pending_maker_keys().write() { + m.remove(&trade_pk_hex); + } + if let Ok(mut m) = pending_local_ids().write() { + m.remove(&ck); + } + }; + let daemon_id = match confirmation { Ok(Ok(DaemonConfirmation::Confirmed { daemon_id })) => { crate::api::logging::blog_info("orders", format!( "create_order confirmed by daemon: {daemon_id}" )); daemon_id } Ok(Ok(DaemonConfirmation::Rejected { reason, message })) => { + rollback_pending_create(); crate::api::logging::blog_warn("orders", format!( "create_order rejected: {reason} — {message}" )); return Err(anyhow::anyhow!("{message}")); } _ => { + rollback_pending_create(); crate::api::logging::blog_warn("orders", format!( "create_order: no daemon response within 10s for id={}", order.id )); return Err(anyhow::anyhow!("NoDaemonResponse")); } };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rust/src/api/orders.rs` around lines 541 - 564, In the create_order confirmation handling inside orders::create_order, the rejection and timeout early returns are leaving fingerprint bookkeeping behind; make sure the same cleanup used in the publish-failure path also removes the entry from trade_key_map and clears the matching values from pending_maker_keys and pending_local_ids before returning on DaemonConfirmation::Rejected and the no-response branch. Locate the logic around the confirmation match in create_order and apply the cleanup consistently for all non-confirmed outcomes so _run_order_subscription() does not later misidentify abandoned state as owned.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rust/src/api/orders.rs`:
- Around line 1583-1609: The startup phantom sweep in the orders cleanup path is
using “not seen in the in-memory book after a fixed delay” as the only signal to
delete pending trades, which can remove real orders. Update the sweep logic
around the startup-sleep/list_trades loop and delete_trade_if_phantom call to
require stronger evidence before deletion, such as checking for the old
optimistic/local-ID pattern or a daemon-provided terminal state instead of
relying on absence after 20s. Keep the Pending and MIN_TRADE_AGE_SECS guards,
but make the final deletion decision in this startup sweep more conservative.
In `@rust/src/db/indexeddb.rs`:
- Around line 43-45: The IndexedDB backend still leaves phantom-trade cleanup
unimplemented, so the web path never deletes stale trades. Update
IndexedDbStorage in indexeddb.rs to implement delete_trade using
indexed_db_futures for web persistence, and make sure the related trade lookup
flow used by orders::sweep_phantom_trades() and delete_trade_if_phantom() is
fully supported by the IndexedDB methods list_trades(), get_trade_by_order_id(),
and delete_trade() rather than returning a not-implemented error.
---
Outside diff comments:
In `@rust/src/api/orders.rs`:
- Around line 541-564: In the create_order confirmation handling inside
orders::create_order, the rejection and timeout early returns are leaving
fingerprint bookkeeping behind; make sure the same cleanup used in the
publish-failure path also removes the entry from trade_key_map and clears the
matching values from pending_maker_keys and pending_local_ids before returning
on DaemonConfirmation::Rejected and the no-response branch. Locate the logic
around the confirmation match in create_order and apply the cleanup consistently
for all non-confirmed outcomes so _run_order_subscription() does not later
misidentify abandoned state as owned.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0a72a886-2c42-48d5-b7a3-28f21ce0a884
📒 Files selected for processing (16)
lib/features/order/screens/add_order_screen.dartlib/l10n/app_de.arblib/l10n/app_en.arblib/l10n/app_es.arblib/l10n/app_fr.arblib/l10n/app_it.arblib/l10n/app_localizations.dartlib/l10n/app_localizations_de.dartlib/l10n/app_localizations_en.dartlib/l10n/app_localizations_es.dartlib/l10n/app_localizations_fr.dartlib/l10n/app_localizations_it.dartrust/src/api/orders.rsrust/src/db/indexeddb.rsrust/src/db/mod.rsrust/src/db/sqlite.rs
- The sweep treated absent from the book after 20s as proof of a phantom, risking deletion of a real pending order on slow or offline relay sync. - create_order no longer persists unconfirmed orders, so no new phantoms appear; the sweep's only value (legacy cleanup) didn't justify the data-loss risk. - Drops the now-unused delete_trade DB method that was added solely for the sweep.
![ermeme[bot]](https://avatars.githubusercontent.com/in/3690068?s=60&v=4){kind=small}
This was referenced Jul 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix #157
Problem
When creating an order, if the Mostro node rejected late or never responded, the order still showed up — in the public order book and in My Trades, as if it had been created, with a cancel option, even though no Kind 38383 event was ever published. It only disappeared on restart (order book) or lingered forever (My Trades).
Root cause:
create_orderwaited a few seconds for the daemon confirmation and, on timeout, took an optimistic path, inserting the order into the same in-memory store that feeds the public book and persisting aTradeInfoto the DB.Fix
Aligned the create flow with the v1 mobile until the node acks it):
connection and try again later") and createed from 5s to 10s to tolerate a merely-slow node.
are never deleted.
The success path is unchanged: it goes throck (
DaemonConfirmation::Confirmed), not the timeout branch.Commits
fix(orders): fail order creation on dae itTesting
the book or My Trades.
Notes
PayBondInvoice, which doesn'tresolve the create_order confirmation, thiesponse" until the anti-abuse bond epic isimplemented. Documented so it isn't a regreSummary by CodeRabbit