fix(#273): clear trades, messages and sessions on identity deletion - #298
fix(#273): clear trades, messages and sessions on identity deletion#298codaMW wants to merge 2 commits into
Conversation
…eletion Generating a new user rotated the identity but left the previous user's data behind: delete_identity() cleared the identity row and trade-key mappings but not the trades table, the messages table, or the in-memory sessions, so the new identity inherited the old one's My Trades list and chats — a privacy issue, and dead state (the trade keys were already cleared). Add clear_trades / clear_messages to the DB trait (SQLite implemented; IndexedDB stubbed alongside the existing clear_trade_keys pending MostroP2P#233) and a SessionManager::clear_all(). Call them from delete_identity() — messages before trades for the FK — and empty the in-memory sessions. On the Dart side, invalidate rawTradesProvider after regenerate() so My Trades (and the chat rooms derived from it) reflect the clean slate immediately. Verified on a physical device (Nokia C31): after Generate New User, My Trades and chats are empty. Adds a SQLite test that the clears empty both tables.
|
Warning Review limit reached
Next review available in: 27 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
WalkthroughIdentity cleanup now removes persisted trades, messages, and in-memory sessions. Identity regeneration also invalidates the cached trades provider. SQLite supports the cleanup operations, while IndexedDB uses successful no-op implementations. ChangesIdentity cleanup
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@lib/features/account/screens/account_screen.dart`:
- Around line 390-395: Add a focused provider test for the identity-regeneration
cache reset around rawTradesProvider: override it with a mutable fetcher, verify
an initial non-empty result, change the fetcher to return an empty list,
invalidate rawTradesProvider, and assert the subsequent read is empty without
invoking native identity APIs.
In `@rust/src/api/identity.rs`:
- Around line 307-317: The delete_identity cleanup path must not report success
when clear_messages or clear_trades fails. Update delete_identity to propagate
either cleanup error and stop before identity replacement, ensuring
importAndStore and regenerate cannot proceed; alternatively, make both cleanup
operations atomic in one transaction.
In `@rust/src/db/indexeddb.rs`:
- Around line 243-249: Implement clear_messages to open a read-write IndexedDB
transaction for MESSAGES_STORE and clear its object store, awaiting the
operation and propagating any errors through Result. Leave clear_trades
unchanged as a no-op.
🪄 Autofix
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 Plus
Run ID: 4af5cd31-2de4-4bfc-b866-8b97070cbe53
📒 Files selected for processing (6)
lib/features/account/screens/account_screen.dartrust/src/api/identity.rsrust/src/db/indexeddb.rsrust/src/db/mod.rsrust/src/db/sqlite.rsrust/src/mostro/session.rs
…web messages, test invalidation - delete_identity() now propagates clear_messages/clear_trades errors instead of logging and returning Ok. These tables are not identity-scoped and have no reconcile fallback, so a silent failure would leak the previous identity's history; propagating aborts regenerate/importAndStore before the replacement identity is created (deleteIdentity runs before the new identity exists). - IndexedDB clear_messages now clears MESSAGES_STORE in a read-write transaction rather than no-op'ing: messages are persisted on web (save_message), so identity deletion must actually wipe them. clear_trades stays a no-op (no web trades store yet, MostroP2P#233). - Add a provider test: invalidating rawTradesProvider after the DB is cleared yields an empty list (the cache reset the account screen relies on).
1 participant
Problem
"Generate new user" rotated the identity but never deleted the data derived from the old one.
delete_identity()cleared the in-memory identity, the persisted identity row, the trade-key mappings and the logs but thetradestable (My Trades history), themessagestable (chat history), and the in-memory sessions survived. The new identity started with fresh keys yet inherited the previous user's entire trade list and conversations a privacy issue, and dead state (those trade keys were already cleared, so nothing could operate on the orders).Fix
clear_trades/clear_messagesto the DB trait, mirroringclear_trade_keys. SQLite implements both; IndexedDB stubs them alongside the existingclear_trade_keysstub, pending IndexedDB persistence (Web: IndexedDB storage backend is a stub — nothing persists across a reload #233).SessionManager::clear_all()to drop every in-memory session.delete_identity()clear_messagesbeforeclear_tradesfor themessages.trade_id -> trades(id)FK and empty the in-memory sessions.rawTradesProviderafterIdentityService.regenerate()so My Trades reflects the clean slate immediately. The chat rooms list (chatRoomsFromTradesProvider) derives fromrawTradesProvider, so it clears in the same pass.Testing
clear_messages+clear_trades, both tables are empty; clearing again on empty tables is a no-op.cargo test --lib(255) /clippy -D warnings/ wasm check all green;flutter analyzeclean.Closes #273.
Summary by CodeRabbit
New Features
Bug Fixes