Fix for nostr sdk 40 issue on incoming message#465
Conversation
WalkthroughThis pull request updates the dependency version in Changes
Sequence Diagram(s)sequenceDiagram
participant E as Event Source
participant A as App.run
participant P as Parser
participant S as Serializer
participant C as Client
E->>A: Receive event with rumor content
A->>P: Parse message & signature
P-->>A: Return (Message, Option<Signature>)
A->>S: Serialize Message using as_json()
S-->>A: JSON-formatted Message
A->>C: Send event (&event)
C-->>A: Acknowledge event sent
sequenceDiagram
participant M as Main
participant C as Client
M->>C: subscribe(subscription, None)
C-->>M: Confirm subscription
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/scheduler.rs (1)
214-214: Changed to pass event by reference but still cloningUpdated to use
client.send_event(&ev.clone())to align with the nostr-sdk 0.40.0 API changes, but it's still unnecessarily cloning the event before taking a reference.Consider removing the redundant clone:
-let _ = client.send_event(&ev.clone()).await; +let _ = client.send_event(ev).await;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
Cargo.toml(2 hunks)src/app.rs(2 hunks)src/app/admin_cancel.rs(2 hunks)src/app/admin_settle.rs(2 hunks)src/app/admin_take_dispute.rs(2 hunks)src/app/dispute.rs(2 hunks)src/app/release.rs(1 hunks)src/main.rs(1 hunks)src/nip33.rs(3 hunks)src/scheduler.rs(3 hunks)src/util.rs(4 hunks)
🔇 Additional comments (24)
Cargo.toml (1)
19-19:Details
✅ Verification successful
Updated nostr-sdk dependency to version 0.40.0
The version bump of nostr-sdk from 0.38.0 to 0.40.0 is directly related to fixing the issue with parsing messages from mostro-cli as mentioned in the PR objectives.
🏁 Script executed:
#!/bin/bash # Verify if the nostr-sdk API changes between versions 0.38.0 and 0.40.0 affect message parsing rg -A 3 -B 3 "client\.send_event|client\.subscribe" --type rustLength of output: 3482
Verified Dependency Update: nostr-sdk v0.40.0 Consistently Fixes Message Parsing
The targeted API methods (client.send_eventandclient.subscribe) are used consistently across multiple modules, and there’s no evidence of regression or misalignment with the new version. This bump correctly addresses the message parsing issues observed with mostro-cli.src/app/release.rs (1)
116-116: Changed to pass event by referenceThe modification changes
client.send_event(event)toclient.send_event(&event)to match the updated API in nostr-sdk 0.40.0. This passes a reference instead of transferring ownership of the event.src/scheduler.rs (2)
126-126: Changed to pass relay_ev by referenceUpdated to use
client.send_event(&relay_ev)instead of moving ownership, aligning with the nostr-sdk 0.40.0 API changes.
157-157: Changed to pass info_ev by referenceUpdated to use
client.send_event(&info_ev)instead of moving ownership, aligning with the nostr-sdk 0.40.0 API changes.src/main.rs (1)
93-93: Updated subscribe call to match new APIChanged from
client.subscribe(vec![subscription], None)toclient.subscribe(subscription, None)to match the updated API in nostr-sdk 0.40.0, which now expects a single Filter object rather than a vector.src/app/dispute.rs (2)
31-47: Consistent tag creation approach appliedThe change from
Tags::newtoTags::from_listaligns with similar changes throughout the codebase, creating a more consistent approach to tag creation.
58-58: Improved memory handling by passing event referencePassing the event by reference (
&event) instead of by value prevents unnecessary ownership transfer, which is more efficient and follows Rust best practices.src/app/admin_take_dispute.rs (2)
251-264: Consistent tag creation approach appliedThe change from
Tags::newtoTags::from_listmaintains consistency with similar changes across the codebase, improving code standardization.
286-293: Improved memory handling by passing event referencePassing the event by reference (
&event) instead of by value prevents unnecessary cloning and follows Rust ownership best practices.src/app/admin_cancel.rs (2)
86-99: Consistent tag creation approach appliedThe change from
Tags::newtoTags::from_listcontinues the pattern seen in other files, creating a more uniform API usage throughout the codebase.
106-108: Improved memory handling with reference passingChanged to pass the event as a reference (
&event) instead of by value, which is more efficient and consistent with other send_event calls in the PR.src/app/admin_settle.rs (3)
86-99: Consistent tag creation approach appliedThe change from
Tags::newtoTags::from_listmaintains the consistent pattern applied across all modified files in this PR.
107-109: Improved memory handling by passing event referenceThe event is now passed by reference (
&event) instead of by value, preventing unnecessary ownership transfer and keeping consistent with other similar calls.
1-157: Overall implementation successfully addresses the nostr sdk issueThe changes consistently update event handling across all dispute management functions, effectively addressing the issue with parsing messages from mostro-cli. The approach of passing events by reference and standardizing tag creation is applied uniformly across all affected files.
src/nip33.rs (3)
34-34: Method update:Tags::newtoTags::from_listThe change from
Tags::newtoTags::from_listappears to be part of a consistent update across the codebase to fix message parsing issues betweenmostro-cliandmostro.
145-145: Method update:Tags::newtoTags::from_listThis change is consistent with the same pattern applied throughout the codebase.
157-157: Method update:Tags::newtoTags::from_listThis change is consistent with the same pattern applied throughout the codebase.
src/app.rs (3)
298-306: Fixed message parsing to properly handlemostro-climessagesThis change improves type safety by directly parsing the message into a structured
(Message, Option<Signature>)tuple rather than using a more generic approach. This appears to be the core fix for the issue with parsing messages frommostro-cli.
308-315: Added proper message serialization for signature verificationThe message is now properly serialized to JSON format before signature verification, which ensures consistent handling of messages regardless of their source.
321-321: Updated signature verification to use the properly serialized messageNow using the correctly serialized message JSON for signature verification, which ensures compatibility with messages from
mostro-cli.src/util.rs (4)
345-345: Memory optimization: Passing event reference instead of moving ownershipChanged to pass a reference to the event (
&event) rather than moving ownership, which improves memory efficiency. This change is consistent with similar changes throughout the codebase.
434-435: Method update:Tags::newtoTags::from_listThis change is consistent with the pattern applied throughout the codebase, likely to fix message parsing issues.
446-446: Memory optimization: Passing event reference instead of moving ownershipChanged to pass a reference to the event (
&event) rather than moving ownership, which improves memory efficiency. This change is consistent with similar changes throughout the codebase.
527-527: Memory optimization: Passing event reference instead of moving ownershipChanged to pass a reference to the event (
&event) rather than moving ownership, which improves memory efficiency. This change is consistent with similar changes throughout the codebase.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
* 📝 Add docstrings to `feature-full-privacy-mode-checks` (#455) * add feature to check correctly if order is full privacy or normal * 📝 Add docstrings to `feature-full-privacy-mode-checks` Docstrings generation was requested by @arkanoider. * #454 (comment) The following files were modified: * `src/app/order.rs` * `src/util.rs` --------- Co-authored-by: arkanoider <github.913zc@simplelogin.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: arkanoider <113362043+arkanoider@users.noreply.github.com> * Fix network in order event (#452) Now in orers event 38383 the network that comes out is the correct one * Feature anyhow removal (#459) * remove of anyhow dependency start * removed anyhow dependecy * remove patch from cargo.toml * Update mostro-core version --------- Co-authored-by: Francisco Calderón <fjcalderon@gmail.com> * Feature disputes (#463) * Feature: dispute fix for new logic * fix for coop cancel case * Add logic for info message about dispute for solver * first working dispute info message for admin after taking disputes * added fields to dispute info message for solver * removed some probably useless and refactored a bit admin-take-dispute function * clippy suggestion * other cosmetics to admin take file * some other cosmetics on admin-take-dispute file * fix on admin-take * Update admin_take_dispute.rs * fixed a wrong search of order id in admin take file@ * Fix: added correct check to add solver from admin cli * fix: wrong check on admin cancel * Fix initiator pubkey: now is the trade key of initiator not identity * quick fix for full privacy orders * new logic with mixed full privacy - regular reputation mode * Update cargo toml to compile with version 0.6.32 of mostro-core - this is in preparation for migrating to nostr-sdk 0.40 * rollback sdk to 0.38 version to merge disputes - we will come back to 0.40 when we will fix signature issue * Update mostro-core dependency --------- Co-authored-by: Francisco Calderón <fjcalderon@gmail.com> * added correction for the case of buyer adding back a new invoice after payment failure (#462) * Fix for nostr sdk 40 issue on incoming message (#465) * testing sdk 40 * fix for sdk 40 * cleaned cargo.toml * Bump mostro core version --------- Co-authored-by: Francisco Calderón <fjcalderon@gmail.com> * fix: sends order with an updated 'status' field as the reply to add-invoice when there is a preimage (#464) * Feature-nip69-order-status (#467) * feature: align to nip69 order status in nostr event * Improved in progress nip69 logic * refined event states of the order with nip69 request * add another check to avoid multiple events with in-progress state * Privacy range order fix (#468) * add optional field for users in full privacy inside solver message * To be tested - privacy range child order fix * fix cargo.toml * bumped mostro-core version to 6.35 * New user or privacy order send same user info with zeros (#471) * new user or privacy order send same user info with zeros * fix days field and removed some println! macro with nicer tracing messages * bumped mostro-core version to 6.36 and added requested fix to compile * Add previous order state in database for solver message (#472) * add previous state in database for solver message * bumped mostro-core version to 6.36 * Taker info message to maker. I think that could be modifies in a more generic UserInfo. (#473) * added logic to send infos to maker when an order is taken * Add takers info message for maker * fix cargo.toml * set peer pubkey to empty string when notifying taker reputation to maker * moved user info message to the right point of the order flow * fix cargo.toml * changed message struct from UserDisputeInfo to UserInfo * fix: get master keys for user infos * Bumped mostro-core to version 6.38 * fix for buy order flow with message to maker (#479) * Rabbit fixes --------- Co-authored-by: Francisco Calderón <fjcalderon@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Catrya <140891948+Catrya@users.noreply.github.com> Co-authored-by: Bilthon <bilthon@gmail.com>
* testing sdk 40 * fix for sdk 40 * cleaned cargo.toml * Bump mostro core version --------- Co-authored-by: Francisco Calderón <fjcalderon@gmail.com>
* 📝 Add docstrings to `feature-full-privacy-mode-checks` (#455) * add feature to check correctly if order is full privacy or normal * 📝 Add docstrings to `feature-full-privacy-mode-checks` Docstrings generation was requested by @arkanoider. * #454 (comment) The following files were modified: * `src/app/order.rs` * `src/util.rs` --------- Co-authored-by: arkanoider <github.913zc@simplelogin.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: arkanoider <113362043+arkanoider@users.noreply.github.com> * Feature disputes (#463) * Feature: dispute fix for new logic * fix for coop cancel case * Add logic for info message about dispute for solver * first working dispute info message for admin after taking disputes * added fields to dispute info message for solver * removed some probably useless and refactored a bit admin-take-dispute function * clippy suggestion * other cosmetics to admin take file * some other cosmetics on admin-take-dispute file * fix on admin-take * Update admin_take_dispute.rs * fixed a wrong search of order id in admin take file@ * Fix: added correct check to add solver from admin cli * fix: wrong check on admin cancel * Fix initiator pubkey: now is the trade key of initiator not identity * quick fix for full privacy orders * new logic with mixed full privacy - regular reputation mode * Update cargo toml to compile with version 0.6.32 of mostro-core - this is in preparation for migrating to nostr-sdk 0.40 * rollback sdk to 0.38 version to merge disputes - we will come back to 0.40 when we will fix signature issue * Update mostro-core dependency --------- Co-authored-by: Francisco Calderón <fjcalderon@gmail.com> * Fix for nostr sdk 40 issue on incoming message (#465) * testing sdk 40 * fix for sdk 40 * cleaned cargo.toml * Bump mostro core version --------- Co-authored-by: Francisco Calderón <fjcalderon@gmail.com> * Feature-nip69-order-status (#467) * feature: align to nip69 order status in nostr event * Improved in progress nip69 logic * refined event states of the order with nip69 request * add another check to avoid multiple events with in-progress state * Taker info message to maker. I think that could be modifies in a more generic UserInfo. (#473) * added logic to send infos to maker when an order is taken * Add takers info message for maker * fix cargo.toml * set peer pubkey to empty string when notifying taker reputation to maker * moved user info message to the right point of the order flow * fix cargo.toml * changed message struct from UserDisputeInfo to UserInfo * fix: get master keys for user infos * Bumped mostro-core to version 6.38 * first commit for database encryption * Moving on with encryption work * Implemented base64 on cipher * Takebuy and takesell pending orders check now working with encryption * Removed unwraps * Finishing code: changed password management and completing checks * Finishing code: used hashset for search * added unit test for evaluate timings * first order cycle completed - to be fixed empty password case * Added logic for testing a caching idea using hashmap to speed up search of identity keys when encrypted * ready to start review process * Cleaned test * removed patch from cargo.toml * Rabbit fixes * some improvements to have also rating working with encryption * Removed useless call in db.rs * fix for tests * Cosmetics on release.rs - improved a bit a function in more ribust way * fix: wrong check on full privacy orders in disputes * fix: more meaningful variable name in dispute setup * fix: wrong commented lines reactivated * fix: some fixes in privacy orders with encryption * fix: some fixes following coderabbit advices Introduced a random delay on password entering to avoid timing attacks. Removed awaiting from store_encrypted function that does not require async. * refactor: removed many useless connection opening to database with direct use of borrowed pool variable * chore: more meaningful text for user in db.rs * chore: improved encryption unit test now fix salt for testing works - fix salt and password are used to test timings with decryption using key caching, to test it do: cargo test test_fetch_string_column_scalar -- --nocapture * refactor: some improvements in db.rs improved password error management reduce code duplication for pending orders checks * chore(rebase-on-main): commit cargo.lock for rebasing on top of main encrytpion * feat: completing refactor and rebase of encryption pr * refactor: introduced new refactoring of encryption-decryption function - cargo compiles now, we will have to create the new mostro-core after some testings. - bumped nostr-sdk to 0.41 release, no breaking changes * chore: temporary added git branch of mostro-core to compile * chore(test): fix tests * chore: bumped mostro-core version in cargo.toml * fix: wrong database path string creation * chore: fix tests in db.rs * fix: child order after a trade with a full privacy user have wrong master keys * fix: typo on check password hash function * fix: child order event after a complete order does not send user information also if order is normal (not full privacy) * fix: if a child order come from a privacy order user rating are sent in the event wit all zeroes * fix: fixed bug found by Catrya - when I recreate the child order master keys were saved without encryption so when a takesell or takebuy comes in the search for pending user orders generate a decrytpion error. --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Francisco Calderón <fjcalderon@gmail.com>
@grunch @Catrya ,
this is the fix that makes finally messages from
mostro-cliparsed again frommostro.This is the pull request of mostro-core to use for testing.
The only file involved in the fix is
app.rs, the other files are just to align withnostr-sdkbumped version.I tested message incoming from cli and now it's working, @Catrya when you will make some test chat me as usual for any doutbs.
Summary by CodeRabbit
Summary by CodeRabbit