Cargo.toml (1)

32-32: Consider bumping reqwest minimum version spec to "0.13.2".

The latest published reqwest version is 0.13.2. The current "0.13.1" spec resolves to ^0.13.1 so Cargo.lock will already pick up 0.13.2, but aligning the spec with the actual minimum tested version avoids regressing to a known-older patch in edge-case resolution scenarios.

The native-tls + http2 feature combination is valid: reqwest 0.13 made native-tls imply ALPN automatically, so most users will now get HTTP/2 upgrades. The http2 feature is a default feature in reqwest 0.13, so it must be listed explicitly when default-features = false, which this declaration correctly does.

✏️ Optional: bump minimum version

-reqwest = { version = "0.13.1", default-features = false, features = ["native-tls", "json", "http2"] }
+reqwest = { version = "0.13.2", default-features = false, features = ["native-tls", "json", "http2"] }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Cargo.toml` at line 32, Update the reqwest dependency spec in Cargo.toml from
"0.13.1" to "0.13.2" by changing the line defining reqwest = { version =
"0.13.1", default-features = false, features = ["native-tls", "json", "http2"] }
so the minimum required reqwest version matches the latest published 0.13.2.

src/util/chat_utils.rs (1)

229-231: _admin_keys is a dead parameter in the public API surface.

The parameter is never read in the function body. Since this is a pub async fn, the dead parameter pollutes the call-site signature and every caller must pass a reference they know is ignored. Consider removing it or replacing it with _: &Keys if it must be kept for interface stability.

♻️ Proposed removal

 pub async fn fetch_admin_chat_updates(
     client: &Client,
-    _admin_keys: &Keys,
     disputes: &[AdminDispute],
     admin_chat_last_seen: &HashMap<(String, ChatParty), AdminChatLastSeen>,
 ) -> Result<Vec<AdminChatUpdate>, anyhow::Error> {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/util/chat_utils.rs` around lines 229 - 231, The public async function
fetch_admin_chat_updates currently accepts an unused parameter _admin_keys;
either remove this parameter from the function signature (and update all callers
to stop passing it) or explicitly mark it as ignored by renaming to _: &Keys to
preserve the API while signaling it's unused; update the function declaration
for fetch_admin_chat_updates and any call sites accordingly, and run a build to
ensure no remaining references or warnings.

src/ui/key_handler/mod.rs (1)

168-211: Ctrl+S save-attachment handler looks correct; consider logging when derivation is skipped.

The nested guard structure is clear. The ECDH fallback derivation for missing decryption keys (lines 182–203) correctly maps ChatSender::Buyer → dispute.buyer_pubkey and ChatSender::Seller → dispute.seller_pubkey, skipping Admin senders.

One minor observation: if the pubkey string is present but fails to parse (line 195), or if derive_shared_key fails (line 196), the attachment is still sent to the channel without a decryption key, resulting in an .enc file being saved without decryption. This is the intended fallback per the PR description, but there's no user-visible indication that key derivation failed — only the blossom.rs save path will name the file .enc. Consider adding a log::warn! in the error branches so the admin can diagnose why a file wasn't decrypted.

🔧 Optional: add logging for derivation failures

                                     if let Ok(sender_pk) = PublicKey::parse(pk_str) {
                                         if let Ok(shared) = crate::util::blossom::derive_shared_key(
                                             admin_keys, &sender_pk,
                                         ) {
                                             attachment.decryption_key = Some(shared.to_vec());
+                                        } else {
+                                            log::warn!("Failed to derive shared key for attachment in dispute {}", dispute_id_key);
                                         }
+                                    } else {
+                                        log::warn!("Failed to parse sender pubkey for attachment decryption: {}", pk_str);
                                     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/key_handler/mod.rs` around lines 168 - 211, The handler sends
attachments even when pubkey parsing (PublicKey::parse for pk_str) or ECDH key
derivation (crate::util::blossom::derive_shared_key using admin_keys) fails; add
explicit logging in those failure branches so admins see why decryption_key
remained None. Specifically, inside the block handling
attachment.decryption_key.is_none() (the match on msg.sender and the nested
if-let Ok(sender_pk) / if-let Ok(shared) branches), add log::warn! messages when
PublicKey::parse(pk_str) returns Err and when derive_shared_key returns Err
(include dispute.dispute_id and msg.sender/context), then proceed to send via
tx.send((dispute_id_key, attachment)) as before.

src/ui/tabs/disputes_in_progress_tab.rs (1)

461-468: Side effect in render function — acceptable but worth noting.

Clearing attachment_toast inside render_disputes_in_progress is a side effect in what is conceptually a render pass. This works fine here since app is &mut, but if you ever refactor toward a stricter model/view separation, this expiry logic would belong in the main loop or a dedicated tick handler.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/tabs/disputes_in_progress_tab.rs` around lines 461 - 468, The
render_disputes_in_progress function currently mutates app by clearing
app.attachment_toast when the toast has expired, which is a side effect that
belongs in your main update/tick path; move the expiry logic out of
render_disputes_in_progress and into a centralized timer/tick handler (e.g., the
main event loop or a new handle_tick/update_timers function) that runs before
rendering, and have render_disputes_in_progress only read app.attachment_toast;
locate the code that checks app.attachment_toast and ATTACHMENT_TOAST_DURATION
and extract it into that tick/update function (or call a small helper like
expire_attachment_toast(&mut app)) so behavior is preserved without mutating
state during render.

src/ui/helpers.rs (1)

271-316: Consider logging when attachment JSON is partially valid but unparseable.

If the JSON has a recognized "type" (image_encrypted / file_encrypted) but is missing a required field (e.g., blossom_url, nonce, filename), the function silently returns None and the raw JSON blob is shown to the user as a plain chat message. A log::warn before returning None after matching the type would help diagnose relay-side or sender-side issues.

Sketch

     let (file_type, icon) = match msg_type {
         "image_encrypted" => (ChatAttachmentType::Image, "🖼"),
         "file_encrypted" => (ChatAttachmentType::File, "📎"),
         _ => return None,
     };
-    let blossom_url = obj.get("blossom_url")?.as_str()?.to_string();
-    let nonce_str = obj.get("nonce")?.as_str()?;
-    let nonce = decode_nonce_12(nonce_str)?;
-    let filename = obj.get("filename")?.as_str()?.to_string();
+    let blossom_url = match obj.get("blossom_url").and_then(|v| v.as_str()) {
+        Some(u) => u.to_string(),
+        None => { log::warn!("Attachment JSON missing blossom_url"); return None; }
+    };
+    let nonce_str = match obj.get("nonce").and_then(|v| v.as_str()) {
+        Some(n) => n,
+        None => { log::warn!("Attachment JSON missing nonce"); return None; }
+    };
+    let nonce = match decode_nonce_12(nonce_str) {
+        Some(n) => n,
+        None => { log::warn!("Attachment JSON has invalid nonce: {}", nonce_str); return None; }
+    };
+    let filename = match obj.get("filename").and_then(|v| v.as_str()) {
+        Some(f) => f.to_string(),
+        None => { log::warn!("Attachment JSON missing filename"); return None; }
+    };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/helpers.rs` around lines 271 - 316, In try_parse_attachment_message,
when msg_type matches "image_encrypted" or "file_encrypted" but subsequent
required fields (blossom_url, nonce, filename, or key decoding) are missing
causing the function to return None, add a log::warn that includes the msg_type
and the original content (or which key was missing) so partial/invalid
attachment JSON is visible in logs; place this warning immediately before
returning None in those failure paths after extracting msg_type (reference
symbols: try_parse_attachment_message, msg_type, blossom_url, nonce, filename,
decryption_key).

src/util/chat_utils.rs (1)

56-95: Custom wrap event bypasses the NIP-59 seal layer — document the deviation.

build_custom_wrap_event skips the standard NIP-59 three-layer structure (rumor → seal → gift wrap) and instead directly encrypts a signed text_note with an ephemeral key using NIP-44. The function name makes the custom nature explicit, and the comment references the Mostro protocol chat spec, but consider adding a brief doc comment explaining why the seal layer is omitted (e.g., "Mostro protocol chat uses a simplified two-layer wrap: signed inner event encrypted with ephemeral-to-shared-key NIP-44, then wrapped as Kind::GiftWrap"). This will help future maintainers understand the intentional deviation.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/util/chat_utils.rs` around lines 56 - 95, Add a short doc comment to
build_custom_wrap_event explaining the intentional deviation from NIP-59: state
that instead of the standard three-layer rumor→seal→gift-wrap flow the function
creates a signed inner Event (via EventBuilder::text_note), encrypts it directly
with an ephemeral key using nip44::encrypt, and wraps it as Kind::GiftWrap;
mention why (e.g., Mostro uses a simplified two-layer approach) and reference
the relevant symbols (build_custom_wrap_event, EventBuilder::text_note,
nip44::encrypt, Kind::GiftWrap, SETTINGS) so maintainers know this is deliberate
and not a bug.

src/ui/helpers.rs (1)

264-306: Nonce field from attachment JSON is silently ignored.

try_parse_attachment_message extracts blossom_url, filename, key, and mime_type from the JSON but does not extract the nonce field. The decryption path in blossom::decrypt_blob reads the nonce from the first 12 bytes of the downloaded blob, which is correct for the stated blob layout [nonce:12][ciphertext][tag:16].

However, if the JSON nonce and the blob-embedded nonce ever diverge (e.g., a sender includes a separate nonce in JSON but does not prepend it to the blob), decryption will silently fail. Consider logging a warning if the JSON nonce is present but doesn't match the blob prefix, or at least documenting the assumption.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/helpers.rs` around lines 264 - 306, The function
try_parse_attachment_message currently ignores a JSON "nonce" field which can
cause silent decryption failures; update try_parse_attachment_message to parse
the "nonce" field (base64-decode it, verify length == 12) and store it on the
ChatAttachment (add an Option<[u8;12]> or Vec<u8> field if missing) instead of
dropping it, and then ensure callers of ChatAttachment (e.g., wherever
blossom::decrypt_blob is invoked) compare the JSON nonce to the blob-prefixed
nonce and log a warning if both are present and differ; additionally
validate/handle invalid base64 or wrong-length nonces by treating them as None
and logging a debug/warn message.

src/util/blossom.rs (1)

148-175: A new reqwest::Client is created per save — consider reusing.

save_attachment_to_disk constructs a fresh reqwest::Client on line 157 for each invocation. Client::new() allocates a connection pool and TLS state. Since saves are infrequent this is not a blocker, but if the caller already holds a client (e.g., in spawn_save_attachment), passing it in would avoid repeated setup.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/util/blossom.rs` around lines 148 - 175, The function
save_attachment_to_disk creates a new reqwest::Client per call which is
expensive; change its signature to accept a shared Client (e.g., client:
&reqwest::Client or client: Arc<Client>) instead of constructing one inside,
update call sites such as spawn_save_attachment to pass the existing client
through, and remove Client::new() from save_attachment_to_disk so
fetch_blob(&client, ...) uses the provided client; ensure lifetimes/ownership
are adjusted (or wrap in Arc) and update imports/signatures accordingly.

src/ui/key_handler/mod.rs (1)

167-233: Two shared-key derivation paths are intentionally used for different cryptographic purposes—document the distinction to prevent accidental swapping.

blossom::derive_shared_key (line 197) derives keys via nostr_sdk::secp256k1::ecdh::shared_secret_point for ChaCha20-Poly1305 file decryption, while chat_utils::derive_shared_keys uses nostr_sdk::util::generate_shared_key for NIP-59 message wrapping. Both approaches are documented at their respective module levels (blossom.rs module doc and chat_utils.rs function doc), but there is no cross-reference explaining why both coexist. Adding a comment in key_handler (near line 197) clarifying that blossom::derive_shared_key is specifically for file decryption would prevent future maintainers from accidentally substituting chat_utils::derive_shared_keys or consolidating the two paths.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/key_handler/mod.rs` around lines 167 - 233, Add a short clarifying
comment immediately above the blossom::derive_shared_key usage in
key_handler::handle_key_event (where the attachment decryption_key is derived)
stating that blossom::derive_shared_key derives a ChaCha20-Poly1305
file-decryption key via nostr_sdk::secp256k1::ecdh::shared_secret_point and must
not be swapped with chat_utils::derive_shared_keys, which uses
nostr_sdk::util::generate_shared_key for NIP-59 message wrapping; mention both
functions by name and their distinct cryptographic purpose to prevent future
accidental consolidation or substitution.