docs: add Core Services specs (MostroService, ExchangeService, EncryptionService)#35
docs: add Core Services specs (MostroService, ExchangeService, EncryptionService)#35
Conversation
…tionService) New specs: **MOSTRO_SERVICE.md** - Central Mostro protocol communication service - Gift wrap event handling (NIP-59 decryption) - Message reception pipeline (deduplication, session matching, restore filtering) - Child order linking for range orders - Message publishing with PoW - Integration with SubscriptionManager, SessionNotifier, OrderNotifier **EXCHANGE_SERVICE.md** - Abstract base class for exchange rate providers - HTTP utilities (timeout, error handling, validation) - YadioExchangeService implementation - Riverpod integration (ExchangeRateNotifier) - Currency code discovery - Performance considerations (caching, rate limiting) **ENCRYPTION_SERVICE.md** - ChaCha20-Poly1305 AEAD encryption - Secure random generation (Fortuna CSPRNG) - Blob serialization (nonce + ciphertext + auth tag) - EncryptionResult model - Convenience methods (encryptToBlob, decryptFromBlob) - Security properties and nonce uniqueness rules - Usage examples for encrypted image uploads Updated README.md with all three specs and migration notes.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdded comprehensive reference documentation for three core services: Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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 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 |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
.specify/v1-reference/README.md (1)
77-79: Consider making the bridge version explicit in these migration notes.Since these rows are now key migration pointers, adding “flutter_rust_bridge 2.x” in the V2 phrasing would reduce ambiguity during implementation planning.
Based on learnings: Use flutter_rust_bridge 2.x for Rust-Flutter interop.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.specify/v1-reference/README.md around lines 77 - 79, Update the three migration-note rows (MOSTRO_SERVICE.md, EXCHANGE_SERVICE.md, ENCRYPTION_SERVICE.md) to explicitly mention the bridge version by appending "flutter_rust_bridge 2.x" in the V2 phrasing for each entry; specifically edit the V2 migration text for MOSTRO_SERVICE.md, EXCHANGE_SERVICE.md, and ENCRYPTION_SERVICE.md so they read something like "Move core logic to Rust (flutter_rust_bridge 2.x; Flutter handles UI integration)", "Keep in Flutter (HTTP API integration; use flutter_rust_bridge 2.x where applicable)" and "Move to Rust (crypto should be platform-agnostic; flutter_rust_bridge 2.x)" to remove ambiguity during implementation planning.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.specify/v1-reference/ENCRYPTION_SERVICE.md:
- Around line 39-41: Update the EncryptionResult.fromBlob spec to explicitly
validate and parse the blob: require blob.length >= 28 bytes (12-byte nonce + at
least 0-byte ciphertext + 16-byte authTag) and throw a clear error if shorter;
extract nonce as the first 12 bytes, authTag as the last 16 bytes, and
ciphertext as the bytes in between, then construct the EncryptionResult from
those three components; also state exact error behavior for malformed input
(e.g., raise/return a parse/validation error).
In @.specify/v1-reference/EXCHANGE_SERVICE.md:
- Around line 184-190: The UI example shows "1 BTC = ... USD" but
exchangeRateProvider('USD') (which uses getExchangeRate(currency, 'BTC'))
returns a USD→BTC rate, so the direction is reversed; fix by making the example
consistent: either call exchangeRateProvider('BTC') to obtain a BTC→USD rate
when displaying "1 BTC = ... USD", or keep exchangeRateProvider('USD') and
change the text to "1 USD = ${value.toStringAsFixed(2)} BTC"; update the snippet
where exchangeRateProvider('USD') is referenced to use the chosen correction
(exchangeRateProvider('BTC') or adjusted display string) so the provider and
displayed direction match.
- Around line 72-79: The getRequest() error handling currently re-wraps
HttpException in the generic catch block, losing its specificity; add an
explicit on HttpException { rethrow; } clause before the final generic catch to
let HttpException propagate unchanged (retain existing TimeoutException and
FormatException handlers and the thrown HttpException instances for non-200
responses).
In @.specify/v1-reference/MOSTRO_SERVICE.md:
- Around line 98-109: In _isRestorePayload, make the payload null- and
type-safe: first ensure payload is non-null and a Map (e.g., check payload !=
null && payload is Map) before calling containsKey; when testing 'orders' verify
payload['orders'] is a List and isNotEmpty before accessing index 0, and guard
the buyer_trade_pubkey check with a null-safe lookup (e.g.,
payload['orders'][0]?['buyer_trade_pubkey'] != null); also ensure the initial
payload assignment from json['restore']['payload'] ?? json['order']['payload']
handles missing keys safely (avoid direct indexing on null). Ensure all
containsKey and list-index checks use these guards in _isRestorePayload.
- Around line 168-178: publishOrder() currently assumes
sessions.firstWhereOrNull(...) returns a non-null session and directly accesses
session.tradeKey.private when calling MostroMessage.wrap, which can crash;
update the code to guard against a missing session by checking if session ==
null after the firstWhereOrNull call and handle it (log/error/return or throw a
clear exception) before using session.tradeKey.private, then pass the private
key to MostroMessage.wrap only when the session exists and has a tradeKey.
---
Nitpick comments:
In @.specify/v1-reference/README.md:
- Around line 77-79: Update the three migration-note rows (MOSTRO_SERVICE.md,
EXCHANGE_SERVICE.md, ENCRYPTION_SERVICE.md) to explicitly mention the bridge
version by appending "flutter_rust_bridge 2.x" in the V2 phrasing for each
entry; specifically edit the V2 migration text for MOSTRO_SERVICE.md,
EXCHANGE_SERVICE.md, and ENCRYPTION_SERVICE.md so they read something like "Move
core logic to Rust (flutter_rust_bridge 2.x; Flutter handles UI integration)",
"Keep in Flutter (HTTP API integration; use flutter_rust_bridge 2.x where
applicable)" and "Move to Rust (crypto should be platform-agnostic;
flutter_rust_bridge 2.x)" to remove ambiguity during implementation planning.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 937b0a06-ce61-47fb-b3a6-3458b0237d8f
📒 Files selected for processing (4)
.specify/v1-reference/ENCRYPTION_SERVICE.md.specify/v1-reference/EXCHANGE_SERVICE.md.specify/v1-reference/MOSTRO_SERVICE.md.specify/v1-reference/README.md
| static EncryptionResult fromBlob(Uint8List blob) { | ||
| // Extract: nonce (first 12), authTag (last 16), data (middle) | ||
| } |
There was a problem hiding this comment.
fromBlob() is underspecified for a reference spec.
This section currently leaves core parsing behavior implicit. Please document explicit validation and extraction rules (min length 28 bytes, nonce first 12, tag last 16, middle ciphertext) to make this implementable without guesswork.
Suggested doc snippet completion
static EncryptionResult fromBlob(Uint8List blob) {
- // Extract: nonce (first 12), authTag (last 16), data (middle)
+ if (blob.length < 28) {
+ throw ArgumentError('Blob too short: must include nonce (12) + authTag (16)');
+ }
+ final nonce = blob.sublist(0, 12);
+ final authTag = blob.sublist(blob.length - 16);
+ final encryptedData = blob.sublist(12, blob.length - 16);
+ return EncryptionResult(
+ encryptedData: encryptedData,
+ nonce: nonce,
+ authTag: authTag,
+ );
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| static EncryptionResult fromBlob(Uint8List blob) { | |
| // Extract: nonce (first 12), authTag (last 16), data (middle) | |
| } | |
| static EncryptionResult fromBlob(Uint8List blob) { | |
| if (blob.length < 28) { | |
| throw ArgumentError('Blob too short: must include nonce (12) + authTag (16)'); | |
| } | |
| final nonce = blob.sublist(0, 12); | |
| final authTag = blob.sublist(blob.length - 16); | |
| final encryptedData = blob.sublist(12, blob.length - 16); | |
| return EncryptionResult( | |
| encryptedData: encryptedData, | |
| nonce: nonce, | |
| authTag: authTag, | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.specify/v1-reference/ENCRYPTION_SERVICE.md around lines 39 - 41, Update the
EncryptionResult.fromBlob spec to explicitly validate and parse the blob:
require blob.length >= 28 bytes (12-byte nonce + at least 0-byte ciphertext +
16-byte authTag) and throw a clear error if shorter; extract nonce as the first
12 bytes, authTag as the last 16 bytes, and ciphertext as the bytes in between,
then construct the EncryptionResult from those three components; also state
exact error behavior for malformed input (e.g., raise/return a parse/validation
error).
| throw HttpException('Failed to load data: ${response.statusCode}', uri: url); | ||
| } on TimeoutException { | ||
| throw HttpException('Request timed out', uri: url); | ||
| } on FormatException catch (e) { | ||
| throw HttpException('Invalid response format: ${e.message}', uri: url); | ||
| } catch (e) { | ||
| throw HttpException('Request failed: $e', uri: url); | ||
| } |
There was a problem hiding this comment.
Avoid re-wrapping your own HttpException in getRequest().
The generic catch (e) will also catch the HttpException you throw for non-200 responses, which loses error specificity and contradicts the documented error mapping table. Add a dedicated on HttpException { rethrow; } before the generic catch.
Suggested doc snippet fix
if (response.statusCode == 200) {
return json.decode(response.body) as Map<String, dynamic>;
}
throw HttpException('Failed to load data: ${response.statusCode}', uri: url);
+} on HttpException {
+ rethrow;
} on TimeoutException {
throw HttpException('Request timed out', uri: url);
} on FormatException catch (e) {
throw HttpException('Invalid response format: ${e.message}', uri: url);
} catch (e) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| throw HttpException('Failed to load data: ${response.statusCode}', uri: url); | |
| } on TimeoutException { | |
| throw HttpException('Request timed out', uri: url); | |
| } on FormatException catch (e) { | |
| throw HttpException('Invalid response format: ${e.message}', uri: url); | |
| } catch (e) { | |
| throw HttpException('Request failed: $e', uri: url); | |
| } | |
| throw HttpException('Failed to load data: ${response.statusCode}', uri: url); | |
| } on HttpException { | |
| rethrow; | |
| } on TimeoutException { | |
| throw HttpException('Request timed out', uri: url); | |
| } on FormatException catch (e) { | |
| throw HttpException('Invalid response format: ${e.message}', uri: url); | |
| } catch (e) { | |
| throw HttpException('Request failed: $e', uri: url); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.specify/v1-reference/EXCHANGE_SERVICE.md around lines 72 - 79, The
getRequest() error handling currently re-wraps HttpException in the generic
catch block, losing its specificity; add an explicit on HttpException { rethrow;
} clause before the final generic catch to let HttpException propagate unchanged
(retain existing TimeoutException and FormatException handlers and the thrown
HttpException instances for non-200 responses).
| final rate = ref.watch(exchangeRateProvider('USD')); | ||
|
|
||
| rate.when( | ||
| data: (value) => Text('1 BTC = ${value.toStringAsFixed(2)} USD'), | ||
| loading: () => CircularProgressIndicator(), | ||
| error: (err, stack) => Text('Error: $err'), | ||
| ); |
There was a problem hiding this comment.
Rate direction in UI example is reversed.
exchangeRateProvider('USD') (with getExchangeRate(currency, 'BTC')) yields a USD→BTC rate, but the UI text says 1 BTC = ... USD. This is misleading for integrators.
Suggested doc snippet fix
-rate.when(
- data: (value) => Text('1 BTC = ${value.toStringAsFixed(2)} USD'),
+rate.when(
+ data: (value) => Text('1 USD = ${value.toStringAsFixed(8)} BTC'),
loading: () => CircularProgressIndicator(),
error: (err, stack) => Text('Error: $err'),
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| final rate = ref.watch(exchangeRateProvider('USD')); | |
| rate.when( | |
| data: (value) => Text('1 BTC = ${value.toStringAsFixed(2)} USD'), | |
| loading: () => CircularProgressIndicator(), | |
| error: (err, stack) => Text('Error: $err'), | |
| ); | |
| final rate = ref.watch(exchangeRateProvider('USD')); | |
| rate.when( | |
| data: (value) => Text('1 USD = ${value.toStringAsFixed(8)} BTC'), | |
| loading: () => CircularProgressIndicator(), | |
| error: (err, stack) => Text('Error: $err'), | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.specify/v1-reference/EXCHANGE_SERVICE.md around lines 184 - 190, The UI
example shows "1 BTC = ... USD" but exchangeRateProvider('USD') (which uses
getExchangeRate(currency, 'BTC')) returns a USD→BTC rate, so the direction is
reversed; fix by making the example consistent: either call
exchangeRateProvider('BTC') to obtain a BTC→USD rate when displaying "1 BTC =
... USD", or keep exchangeRateProvider('USD') and change the text to "1 USD =
${value.toStringAsFixed(2)} BTC"; update the snippet where
exchangeRateProvider('USD') is referenced to use the chosen correction
(exchangeRateProvider('BTC') or adjusted display string) so the provider and
displayed direction match.
| final session = sessions.firstWhereOrNull( | ||
| (s) => s.orderId == message.id || s.requestId == message.requestId, | ||
| ); | ||
|
|
||
| final pow = ref.read(mostroInstanceProvider)?.pow ?? 0; | ||
|
|
||
| final wrappedEvent = await MostroMessage.wrap( | ||
| message, | ||
| mostroPubkey, | ||
| session.tradeKey.private, | ||
| pow: pow, |
There was a problem hiding this comment.
Guard for missing session before accessing trade key in publishOrder().
firstWhereOrNull can return null; dereferencing session.tradeKey.private without a check turns this into a crash path.
Suggested doc snippet fix
final session = sessions.firstWhereOrNull(
(s) => s.orderId == message.id || s.requestId == message.requestId,
);
+if (session == null) {
+ throw StateError(
+ 'No session found for orderId=${message.id} requestId=${message.requestId}',
+ );
+}
final pow = ref.read(mostroInstanceProvider)?.pow ?? 0;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| final session = sessions.firstWhereOrNull( | |
| (s) => s.orderId == message.id || s.requestId == message.requestId, | |
| ); | |
| final pow = ref.read(mostroInstanceProvider)?.pow ?? 0; | |
| final wrappedEvent = await MostroMessage.wrap( | |
| message, | |
| mostroPubkey, | |
| session.tradeKey.private, | |
| pow: pow, | |
| final session = sessions.firstWhereOrNull( | |
| (s) => s.orderId == message.id || s.requestId == message.requestId, | |
| ); | |
| if (session == null) { | |
| throw StateError( | |
| 'No session found for orderId=${message.id} requestId=${message.requestId}', | |
| ); | |
| } | |
| final pow = ref.read(mostroInstanceProvider)?.pow ?? 0; | |
| final wrappedEvent = await MostroMessage.wrap( | |
| message, | |
| mostroPubkey, | |
| session.tradeKey.private, | |
| pow: pow, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.specify/v1-reference/MOSTRO_SERVICE.md around lines 168 - 178,
publishOrder() currently assumes sessions.firstWhereOrNull(...) returns a
non-null session and directly accesses session.tradeKey.private when calling
MostroMessage.wrap, which can crash; update the code to guard against a missing
session by checking if session == null after the firstWhereOrNull call and
handle it (log/error/return or throw a clear exception) before using
session.tradeKey.private, then pass the private key to MostroMessage.wrap only
when the session exists and has a tradeKey.
✅ Actions performedReview triggered.
|
- ENCRYPTION_SERVICE.md: Added explicit validation in fromBlob (requires >=28 bytes, extracts nonce/authTag/ciphertext with clear error on malformed input)
- EXCHANGE_SERVICE.md: Added 'on HttpException { rethrow; }' to preserve specificity in error handling
- EXCHANGE_SERVICE.md: Fixed UI example to show correct rate direction (1 USD = X BTC, not 1 BTC = X USD)
- MOSTRO_SERVICE.md: Made _isRestorePayload null- and type-safe (guards all containsKey and list accesses)
- MOSTRO_SERVICE.md: Added session null-check in publishOrder before accessing tradeKey.private
- README.md: Added flutter_rust_bridge 2.x mention to all three Core Services migration notes
1 participant
Comprehensive documentation for Core Services layer (point 14).
New Specs
MOSTRO_SERVICE.md
EXCHANGE_SERVICE.md
ENCRYPTION_SERVICE.md
Updated Files
README.md
All specs include comprehensive cross-references to related documentation.
Summary by CodeRabbit
Documentation