feat(us10): phase 13 — post-trade rating system - #64
Merged
Conversation
T094 — rust/src/api/reputation.rs: submit_rating (validates 1–5, AlreadyRated, PrivacyModeEnabled), get/set_privacy_mode, get_rating_for_trade, handle_rating_received, RatingStream. Add RatingInfo + RatingReceivedEvent to types.rs. 7 tests pass. T095 — lib/features/rate/screens/rate_counterpart_screen.dart: "RATE" header, double-bolt success indicator, StarRating widget, "X/5" display, green filled SUBMIT (disabled until rating > 0), green outline CLOSE (skips rating). Route /rate_user/:orderId wired. T096 — lib/features/rate/widgets/star_rating.dart: 5 tappable stars, filled = mostroGreen #8CC63F, empty = dark-gray outline, tap sets 1-based rating, read-only when onChanged is null. T097 — trade_detail_screen.dart: add pendingRating + rated to TradeStatus enum; action buttons for each state (RATE→/rate_user, CLOSE); instruction text for both states. Route no longer stub.
…tion - Replace separate contains()/insert() calls with try_insert_if_absent() that holds the write lock for the full check-and-insert, preventing TOCTOU races on concurrent submit_rating() calls - Add privacy_lock() test Mutex to serialize tests that mutate the global privacy_mode flag, preventing flaky parallel test failures
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 (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdded a post-trade rating feature: a Flutter rating screen and star widget, route wiring and trade-status UI changes, plus a new Rust in-memory reputation API (ratings storage, privacy toggle, event broadcast) and corresponding types and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Flutter UI
participant Router as AppRoute
participant Backend as Rust reputation API
participant Store as In-Memory Store
participant Events as Broadcast Channel
UI->>Router: Navigate to /rate_user/:orderId
Router->>UI: Instantiates RateCounterpartScreen(orderId)
UI->>UI: User selects rating via StarRating
UI->>Backend: submit_rating(trade_id, score)
Backend->>Store: validate & write local rating (if allowed)
Store-->>Backend: ack
Backend->>Events: publish RatingReceivedEvent
Events-->>Backend: subscribers notified (on_rating_received)
Backend-->>UI: submission result (success/failure)
UI->>UI: pop / show snackbar
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/features/rate/screens/rate_counterpart_screen.dart`:
- Around line 36-43: In _submit(), replace the placeholder delay with a call to
the generated Rust bridge function reputation.submit_rating(widget.orderId,
_rating) (or the equivalent FFI wrapper) and await it; only call context.pop()
after that call completes successfully; catch bridge errors and surface them to
the user (e.g., show a snackbar/dialog) so privacy/AlreadyRated errors are
visible; ensure _isSubmitting is cleared in a finally block (setState(() =>
_isSubmitting = false)) and keep the mounted checks before calling context.pop()
or any UI updates.
In `@lib/features/rate/widgets/star_rating.dart`:
- Around line 41-50: Replace the pointer-only GestureDetector with an
interactive IconButton so each star is keyboard-focusable and exposed to
assistive tech: in the widget that builds each star (currently using
GestureDetector with onTap => onChanged!(index + 1)), swap to IconButton, wire
its onPressed to call onChanged!(index + 1) when onChanged is not null, preserve
the Icon parameters (use isFilled to pick Icons.star_rounded or
Icons.star_outline_rounded, color from filledColor/emptyColor, and size from
starSize), and add a semantic label (e.g., "Rate X out of Y" or "Select
{index+1} stars") via the IconButton's tooltip or by wrapping with Semantics so
screen readers get per-star descriptions.
In `@rust/src/api/reputation.rs`:
- Around line 95-117: submit_rating currently accepts any trade_id and only
writes local state; change it to first validate the trade is complete by looking
up the session/trade (use your session/trade lookup helper to obtain the trade
key and counterparty Mostro pubkey), ensure privacy_mode is still false, then
publish a RateUser MostroMessage wrapped via NIP-59 Gift Wrap to the
counterparty and await confirmation/success; only after the publish succeeds
should you call rating_store() and invoke try_insert_if_absent with the
RatingInfo to persist locally, and on publish failure return an error without
persisting. Ensure errors from the publish path are propagated and that the
sequence prevents TOCTOU races by re-checking privacy_mode/trade completion
immediately before persistence.
- Around line 22-63: The current RatingStore uses ratings: HashMap<String,
RatingInfo> which only allows one RatingInfo per trade_id and causes
races/overwrites between the two parties; change the storage to represent both
sides (e.g. replace RatingInfo value with a small struct TradeRatings { mine:
Option<RatingInfo>, peer: Option<RatingInfo> } or switch keys to (trade_id,
is_mine)) and update RatingStore methods accordingly: modify get to return the
appropriate side(s), update try_insert_if_absent and insert to insert into the
correct slot (mine vs peer) instead of rejecting/overwriting, and adjust any
callsites such as submit_rating and handle_rating_received to indicate which
side the rating belongs to so the store can populate the correct field. Ensure
event_tx semantics remain unchanged and that atomic locking still protects the
composite TradeRatings entry.
- Around line 150-177: handle_rating_received currently records incoming ratings
and emits RatingReceivedEvent regardless of privacy mode; update it to check the
privacy flag (use whatever module-level function/variable that indicates privacy
mode, e.g., privacy_mode_enabled() or is_privacy_mode()) at the start and return
early when privacy is enabled. Specifically, in handle_rating_received, if
privacy mode is active skip the store.insert(...) call and do not send
store.event_tx.send(event) — simply return Ok(()). Keep the rest of validation
(score range) intact and reference RatingReceivedEvent, RatingInfo,
rating_store(), and unix_now() when applying the guard.
🪄 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: 668acb6e-10a4-4a2f-9328-8af7029faf55
📒 Files selected for processing (8)
lib/core/app_routes.dartlib/features/rate/screens/rate_counterpart_screen.dartlib/features/rate/widgets/star_rating.dartlib/features/trades/screens/trade_detail_screen.dartrust/src/api/mod.rsrust/src/api/reputation.rsrust/src/api/types.rsspecs/004-mostro-p2p-client/tasks.md
Comment on lines
+36
to
+43
| Future<void> _submit() async { | ||
| if (_rating == 0) return; | ||
| setState(() => _isSubmitting = true); | ||
| try { | ||
| // TODO(bridge): Call reputation.submit_rating(widget.orderId, _rating) | ||
| // via Rust bridge once FFI bindings are generated. | ||
| await Future.delayed(const Duration(milliseconds: 300)); | ||
| if (mounted) context.pop(); |
Contributor
There was a problem hiding this comment.
SUBMIT currently reports success without saving anything.
This handler only waits 300 ms and pops the route; it never calls rust/src/api/reputation.rs::submit_rating(). The UI therefore shows a successful flow while nothing is persisted, and backend errors like PrivacyModeEnabled / AlreadyRated can never reach the user. Wire the generated bridge call here and dismiss only after it succeeds.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/features/rate/screens/rate_counterpart_screen.dart` around lines 36 - 43,
In _submit(), replace the placeholder delay with a call to the generated Rust
bridge function reputation.submit_rating(widget.orderId, _rating) (or the
equivalent FFI wrapper) and await it; only call context.pop() after that call
completes successfully; catch bridge errors and surface them to the user (e.g.,
show a snackbar/dialog) so privacy/AlreadyRated errors are visible; ensure
_isSubmitting is cleared in a finally block (setState(() => _isSubmitting =
false)) and keep the mounted checks before calling context.pop() or any UI
updates.
Comment on lines
+95
to
+117
| pub async fn submit_rating(trade_id: String, score: u8) -> Result<()> { | ||
| if score < 1 || score > 5 { | ||
| bail!("InvalidScore: score must be between 1 and 5, got {score}"); | ||
| } | ||
|
|
||
| let store = rating_store(); | ||
|
|
||
| if store.privacy_mode.load(Ordering::SeqCst) { | ||
| bail!("PrivacyModeEnabled: cannot submit rating while privacy mode is active"); | ||
| } | ||
|
|
||
| // TODO(Phase 14+): Look up the session to get trade key + mostro pubkey, | ||
| // then send a RateUser MostroMessage via NIP-59 Gift Wrap. | ||
|
|
||
| // Atomic check-and-insert under write lock — prevents TOCTOU races. | ||
| store | ||
| .try_insert_if_absent(RatingInfo { | ||
| trade_id, | ||
| score, | ||
| is_mine: true, | ||
| created_at: unix_now(), | ||
| }) | ||
| .await?; |
Contributor
There was a problem hiding this comment.
submit_rating accepts arbitrary trades and never publishes the rating.
The only enforced preconditions here are score range and privacy mode. That lets callers rate unfinished/nonexistent trade_ids, and the success path only writes to local in-memory state—no RateUser message is sent, so counterpart reputation cannot update outside this process or survive restart. Validate that the trade is complete, publish the protocol message, and only persist the rating after that succeeds.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rust/src/api/reputation.rs` around lines 95 - 117, submit_rating currently
accepts any trade_id and only writes local state; change it to first validate
the trade is complete by looking up the session/trade (use your session/trade
lookup helper to obtain the trade key and counterparty Mostro pubkey), ensure
privacy_mode is still false, then publish a RateUser MostroMessage wrapped via
NIP-59 Gift Wrap to the counterparty and await confirmation/success; only after
the publish succeeds should you call rating_store() and invoke
try_insert_if_absent with the RatingInfo to persist locally, and on publish
failure return an error without persisting. Ensure errors from the publish path
are propagated and that the sequence prevents TOCTOU races by re-checking
privacy_mode/trade completion immediately before persistence.
…ibility
rust/src/api/reputation.rs:
- Replace HashMap<String,RatingInfo> with HashMap<String,TradeRatings>
so mine/peer ratings for the same trade coexist without overwriting;
try_insert_mine() guards only the local-user slot, insert_peer() upserts
the peer slot independently
- Add privacy mode guard to handle_rating_received: incoming ratings are
silently discarded when privacy mode is active
- Add 2 new tests: privacy discard and mine+peer coexistence
lib/features/rate/widgets/star_rating.dart:
- Replace GestureDetector with IconButton so each star is keyboard-
focusable and exposed to assistive tech; tooltip provides per-star
semantic label ("Select N stars")
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.
T094 — rust/src/api/reputation.rs: submit_rating (validates 1–5,
AlreadyRated, PrivacyModeEnabled), get/set_privacy_mode,
get_rating_for_trade, handle_rating_received, RatingStream.
Add RatingInfo + RatingReceivedEvent to types.rs. 7 tests pass.
T095 — lib/features/rate/screens/rate_counterpart_screen.dart:
"RATE" header, double-bolt success indicator, StarRating widget,
"X/5" display, green filled SUBMIT (disabled until rating > 0),
green outline CLOSE (skips rating). Route /rate_user/:orderId wired.
T096 — lib/features/rate/widgets/star_rating.dart:
5 tappable stars, filled = mostroGreen #8CC63F, empty = dark-gray
outline, tap sets 1-based rating, read-only when onChanged is null.
T097 — trade_detail_screen.dart: add pendingRating + rated to
TradeStatus enum; action buttons for each state (RATE→/rate_user,
CLOSE); instruction text for both states. Route no longer stub.
Summary by CodeRabbit