Feat/persistence db init - #76
Merged
Merged
Conversation
…overy - Add `trade_keys` table to SQLite schema (schema v2) to persist the BIP-32 key index used per order across restarts - Extend `Storage` trait with `save_trade_key`, `get_trade_key`, and `get_trade_by_order_id`; implement in SqliteStorage, stub in IndexedDbStorage - Add `db/app_db.rs`: global `OnceCell<AppStorage>` singleton with `init_db(path)` / `db()` accessors - Expose `init_db(path: String)` to Flutter via flutter_rust_bridge - Make `store_trade_key_index` / `get_trade_key_index` async; write-through to DB on store, DB fallback with cache-warm on miss - Save `TradeInfo` to DB after successful publish in `take_order` - Add `get_trade_role(order_id)` public API that queries the persisted trade - Dart: add `tradeRoleFromDbProvider` (FutureProvider.family) that calls `getTradeRole` and maps `TradeRole` → bool - `TradeDetailScreen` falls back to `tradeRoleFromDbProvider` when the in-memory `tradeRoleProvider` map has no entry for the order, so reopened trades after an app restart show the correct buyer/seller actions
- Move `init_db` from `lib.rs` to `crate::api` so flutter_rust_bridge picks it up (frb.yaml scans `crate::api` only) - Re-run `flutter_rust_bridge_codegen generate` so `init_db` and `get_trade_role` appear in the generated bindings - Call `initDb` in `main()` right after `RustLib.init()`, passing the app documents directory path; skipped on web (IndexedDB stub is not yet implemented) Without this call, `app_db::db()` always returned `None` and all trade-key / trade-role persistence was silently a no-op.
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 (5)
🚧 Files skipped from review as they are similar to previous changes (5)
WalkthroughAdds persistent storage for trade data (SQLite/IndexedDB), exposes a Rust async DB initializer, extends storage APIs for trade keys and trade lookup, updates Flutter to initialize the DB at startup, and adds a DB-backed Riverpod provider to fall back from in-memory trade role state. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Flutter App
participant UI as TradeDetailScreen
participant Provider as Riverpod Provider
participant Rust as Rust API
participant DB as SQLite / IndexedDB
App->>Rust: init_db(path)
Rust->>DB: open storage backend
DB-->>Rust: ready
Rust-->>App: init complete
UI->>Provider: read roleMap[orderId]
alt In-memory hit
Provider-->>UI: return in-memory role
else Cache miss
Provider->>Rust: get_trade_role(orderId)
Rust->>DB: query trade by order_id
DB-->>Rust: TradeRole or None
Rust-->>Provider: Option<TradeRole>
Provider-->>UI: role (or null → default true while loading)
end
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: 4
🧹 Nitpick comments (1)
rust/src/db/app_db.rs (1)
25-34: Consider usingget_or_try_initto avoid redundant initialization attempts.The current pattern has a race window where concurrent callers both pass the
is_none()check and open separate storage instances (first wins, second is dropped). While correct,OnceCell::get_or_try_inithandles this more elegantly.♻️ Suggested simplification
pub async fn init_db(path: &str) -> Result<()> { - if APP_DB.get().is_none() { - let storage = AppStorage::open(path).await?; - // A concurrent init racing here is harmless — OnceCell guarantees - // only the first value is stored. - let _ = APP_DB.set(storage); - log::info!("[db] persistent store initialised (path={})", path); - } + APP_DB + .get_or_try_init(|| async { + let storage = AppStorage::open(path).await?; + log::info!("[db] persistent store initialised (path={})", path); + Ok(storage) + }) + .await?; Ok(()) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/src/db/app_db.rs` around lines 25 - 34, Replace the current check-then-set pattern in init_db with OnceCell's get_or_try_init to avoid redundant concurrent opens: call APP_DB.get_or_try_init with an async closure that calls AppStorage::open(path).await, propagate the Result so callers receive any error, and move the log::info! after successful initialization (i.e., after get_or_try_init returns Ok) so you only log when the cell was initialized or already contains the storage; keep the function signature and Err handling unchanged.
🤖 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/main.dart`:
- Around line 20-25: Wrap the database initialization that runs when kIsWeb is
false (the block using getApplicationDocumentsDirectory(), p.join(...,
'mostro.db') and rust_api.initDb) in a try-catch so initDb failures don’t crash
startup; on catch, log the error (include the caught exception and stacktrace)
and set a clear degraded-state or fallback path (e.g., a boolean flag or
in-memory-only mode) so the rest of the app can continue to run gracefully
instead of crashing.
In `@rust/src/api/orders.rs`:
- Around line 53-77: Change get_trade_key_index to return Option<u32> (or
Result<Option<u32>, _> if you prefer) instead of silently returning 0: update
the function signature for get_trade_key_index(order_id: &str) to return
Option<u32>, return None when the key is neither in trade_key_map() nor found in
db::app_db::db().get_trade_key, and on DB errors propagate or log and return
None; then update all callers to explicitly handle the None case (e.g.,
early-return an error or prompt the user) rather than assuming index 0 so
signature verification won't silently fail. Ensure you update any code that
writes to the cache (trade_key_map().write()) to still insert the found index
and adjust await/unwrap handling to the new return type.
In `@rust/src/db/indexeddb.rs`:
- Around line 83-97: The IndexedDB stubs currently return Err(...) which causes
callers to silently fall back to incorrect behavior on WASM; change the
implementations to no-op success semantics: in save_trade_key return Ok(())
instead of Err(...), in get_trade_key return Ok(None) (not an Err) to let
callers handle "no persisted key" correctly, and in get_trade_by_order_id return
Ok(None) so role lookups remain None without an internal error; update the
function bodies for save_trade_key, get_trade_key, and get_trade_by_order_id to
return these values (or alternatively gate with a compile-time error for web
builds if you prefer to block) so behavior matches the intended "no persistence"
fallback.
In `@rust/src/db/schema.rs`:
- Line 2: Update the comment in schema.rs to accurately state that
SQLITE_INIT_SQL is executed unconditionally on every SqliteStorage::open() call
(SqliteStorage::open) rather than only when SCHEMA_VERSION changes; mention that
the DDL uses idempotent CREATE TABLE IF NOT EXISTS so this is safe, and note
that SCHEMA_VERSION is currently defined but unused—either document intended
future use for version checks/optimizations or remove it if unnecessary.
---
Nitpick comments:
In `@rust/src/db/app_db.rs`:
- Around line 25-34: Replace the current check-then-set pattern in init_db with
OnceCell's get_or_try_init to avoid redundant concurrent opens: call
APP_DB.get_or_try_init with an async closure that calls
AppStorage::open(path).await, propagate the Result so callers receive any error,
and move the log::info! after successful initialization (i.e., after
get_or_try_init returns Ok) so you only log when the cell was initialized or
already contains the storage; keep the function signature and Err handling
unchanged.
🪄 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: 3fbd9fdc-a73c-4560-87e1-e6aa4f8a9c3a
📒 Files selected for processing (16)
lib/features/order/providers/trade_state_provider.dartlib/features/trades/screens/trade_detail_screen.dartlib/l10n/app_localizations_de.dartlib/l10n/app_localizations_es.dartlib/l10n/app_localizations_fr.dartlib/l10n/app_localizations_it.dartlib/main.dartrust/src/api/mod.rsrust/src/api/orders.rsrust/src/db/app_db.rsrust/src/db/indexeddb.rsrust/src/db/mod.rsrust/src/db/schema.rsrust/src/db/sqlite.rsrust/src/frb_generated.rsrust/src/lib.rs
- Wrap initDb() call in main() with try/catch so a DB failure degrades to memory-only mode instead of crashing startup - Change get_trade_key_index() to return Option<u32>; all four callers now propagate an explicit error when no key is found rather than silently falling back to index 0 (which would cause signature verification failures on the daemon side) - Fix IndexedDB stubs for save_trade_key, get_trade_key, and get_trade_by_order_id to return no-op Ok / Ok(None) instead of Err, so callers behave correctly on WASM without spurious error logs - Replace check-then-set pattern in init_db() with get_or_try_init() to avoid redundant concurrent opens - Update schema.rs comments to accurately reflect that SQLITE_INIT_SQL runs unconditionally (safe due to IF NOT EXISTS) and that SCHEMA_VERSION is currently unused at runtime
This was referenced Apr 4, 2026
This was referenced Jul 8, 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.
trade_keystable to SQLite schema (schema v2) to persist theBIP-32 key index used per order across restarts
Storagetrait withsave_trade_key,get_trade_key, andget_trade_by_order_id; implement in SqliteStorage, stub in IndexedDbStoragedb/app_db.rs: globalOnceCell<AppStorage>singleton withinit_db(path)/db()accessorsinit_db(path: String)to Flutter via flutter_rust_bridgestore_trade_key_index/get_trade_key_indexasync; write-throughto DB on store, DB fallback with cache-warm on miss
TradeInfoto DB after successful publish intake_orderget_trade_role(order_id)public API that queries the persisted tradetradeRoleFromDbProvider(FutureProvider.family) that callsgetTradeRoleand mapsTradeRole→ boolTradeDetailScreenfalls back totradeRoleFromDbProviderwhen thein-memory
tradeRoleProvidermap has no entry for the order, soreopened trades after an app restart show the correct buyer/seller actions
Summary by CodeRabbit
New Features
Chores