fix(#203): stop the public order status from driving trade actions - #271
Merged
Conversation
…s to Active status Extend status_for_action to recognize BuyerTookOrder and HoldInvoicePaymentAccepted as Active status indicators. Use or_else fallback when syncing order status from peer pubkey messages to handle payloads without explicit status field. Add test coverage for escrow-locked action mappings.
…rained trade status Add wire_status_applies guard to block public NIP-69 status updates (pending/in-progress/success/canceled) from replacing daemon-sourced states like Active or FiatSent. Public events stop updating once trades turn private, so in-progress only means "taken" and must not drag Active trades back.
Add TradeNotDisputable error handling to prevent opening disputes before escrow is locked. Check local trade status against Active/FiatSent/InProgress states before dispatching to daemon. Add localized error messages across all supported languages (en/de/es/fr/it) and test coverage for status validation.
…ase actions Introduce TradeStatus.inProgress enum value to represent NIP-69's coarse "in-progress" bucket when the order left the public book but escrow state is unknown. Map OrderStatus.inProgress separately from Active instead of conflating them. Display waiting state UI with localized "Setting up the trade" messages across all languages.
…e status Document that NIP-69 `s` tag values (pending/in-progress/success/canceled) are coarse public view only. Daemon stops publishing after trades turn private, so InProgress means "taken, state unknown" not "escrow locked". Add wire_status_applies guard requirement to prevent public events from overwriting daemon-sourced states.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (21)
WalkthroughThe client now distinguishes public ChangesTrade state reconciliation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PublicOrderEvent
participant OrderIngestion
participant LocalTradeStatus
participant TradeDetailScreen
PublicOrderEvent->>OrderIngestion: provide coarse Kind 38383 status
OrderIngestion->>LocalTradeStatus: compare persisted daemon-derived status
LocalTradeStatus-->>OrderIngestion: retain finer status when applicable
OrderIngestion->>TradeDetailScreen: expose reconciled order status
TradeDetailScreen-->>TradeDetailScreen: render InProgress setup state
Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
This was referenced Aug 5, 2026
![ermeme[bot]](https://avatars.githubusercontent.com/in/3690068?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approved — reviewed at head 6772aa63.
Verification
- CI green on this head: Rust (build/test/clippy/wasm), Flutter (analyze/test), Web (wasm build).
- Local: full
cargo test --lib— 238 passed, 0 failed, including the three new tests (escrow_locked_actions_imply_active,the_public_status_never_replaces_a_finer_local_one,only_a_funded_trade_is_disputable). git diff --checkclean.
Review
- The root-cause determination answers #203's
needs verificationcorrectly: thestag is NIP-69's coarse bucket and mostrod never publishesactive/fiat-sent/dispute, so this was status desync, not a gating mismatch. wire_status_appliesis the right invariant: wire may fillNone/Pendingor announce a terminal status, but never overwrite a finer daemon-learned state. Both ingest paths (ingest_order_event,subscribe_single_order) apply it consistently, and in both the book is repaired to the local status when the wire loses (order.status = localbefore upsert).BuyerTookOrder/HoldInvoicePaymentAcceptedimplyingActivematches the daemon semantics (escrow locked is exactly what those two messages mean for each side).- The taker-timeout republish case stays safe:
ingest_order_eventskips the gate entirely for apendingwire status, so a republished order returns to the book normally. - Defense-in-depth in
open_dispute(fail before publish with theTradeNotDisputablemarker, surfaced vialocalizedDaemonError) plus the newTradeStatus.inProgressUI state (waiting presentation, cancel kept, no dispute/fiat-sent) match the daemon's preconditions. Widget tests cover both buyer and seller. - Docs (contracts, data-model, CLAUDE.md) and l10n (en/de/es/fr/it) are in sync.
Non-blocking notes
- In
subscribe_single_order, a republishedpendingwire event while the local trade is in a waiting state is masked in the book (order.status = local). Harmless today — the public list filters non-pending and theCanceledgift wrap reconciles the trade — just noting the small window. status_allows_disputedeliberately letsInProgressthrough to the daemon. Reasonable as a backstop since the real state is unknown; the UI already hides the action there.- The stated overlap with #213 in
trade_detail_screen.dartis real — whichever merges second will need a small rebase in the status maps.
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
2 participants
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.
Closes #203.
Root cause
The
stag of a Kind 38383 event is NIP-69's four-bucket public view (pending,in-progress,success,canceled), and mostrod stops publishing once a trade turns private —active,fiat-sentanddisputenever reach the wire. Both ingest paths wrote that bucket straight into the trade row and the order book, dragging a trade that daemon messages had already advanced toActiveback toInProgress, which the UI presented as an active trade.That is what offered Open dispute (and Mark fiat sent) on a trade the daemon rejects with
CantDo: it only accepts either on anActiveorFiatSentorder. So the gating rule was right; the status feeding it was not — status desync, not a gating mismatch (answers the issue'sneeds verification).What changed
BuyerTookOrder/HoldInvoicePaymentAcceptednow implyActiveinstatus_for_action; a wire status only applies when no finer local one is known or when it is terminal (wire_status_applies);open_disputechecks the status it already holds before publishing and fails with theTradeNotDisputablemarker.in-progressgets its ownTradeStatus: waiting presentation, no dispute and no fiat-sent, cancel still available sincethe daemon accepts it there.
OrderStatus, the state machine, theopen_disputeprecondition and CLAUDE.md.Verification
cargo testandcargo clippy(24 warnings, same as main),flutter analyzeclean,flutter test197 passed. Three Rust tests and two widget tests added.Notes
trade_detail_screen.dartand the status maps; branched offmain, so whichever merges second rebases.CantDoreply is still not reconciled locally. This PR removes the path that triggered it.Summary by CodeRabbit
New Features
Bug Fixes
Localization
Documentation & Tests