Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

readme: take orders #34

Closed
wants to merge 1 commit into from
Closed

readme: take orders #34

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ $ cargo run
## Progress Overview
- [x] Displays order list
- [x] Settings tab
- [ ] Take orders (Buy & Sell)
- [x] Take orders (Buy & Sell)
Copy link

@coderabbitai coderabbitai bot Dec 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Based on the code analysis so far, I can generate the final response:

Marking "Take orders" as complete appears premature

While basic order-taking functionality exists (take_order method), the implementation appears incomplete:

  • No proper handling of Nostr event kind 4 (encrypted DMs) was found
  • Order status handling is minimal, only checking for "Pending" status
  • The implementation relies on features marked as pending in README (e.g., direct messaging)
  • Critical order flow components like hold invoice handling were not found in the codebase

Please consider:

  • Keeping this item unchecked until the complete order flow is implemented
  • Adding the dependent features (DMs, hold invoices, status updates) first
  • Implementing proper error handling and status transitions
🔗 Analysis chain

Verify the completeness of the "Take orders" implementation

While marking the "Take orders" feature as complete, several dependent features are still pending implementation:

  • Direct messaging with peers (NIP-17)
  • Fiat sent confirmation
  • Release functionality
  • Dispute handling

Please verify that the current implementation:

  1. Properly handles the complete order flow
  2. Functions correctly without these pending features
  3. Has adequate fallback mechanisms until these features are implemented

Let's verify the implementation completeness:

Let's gather more specific information about the implementation.

Let's gather more specific information about the buy/sell implementation.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for the presence of essential order-taking functionality

# Look for Nostr event kind 4 handling
echo "Checking for Nostr event kind 4 handling..."
rg -l "kind:\s*4" --type rust

# Look for hold invoice implementation
echo "Checking for hold invoice handling..."
rg -l "hold.?invoice" --type rust

# Look for order status management
echo "Checking for order status updates..."
rg -l "waiting-payment|waiting-buyer-invoice" --type rust

# Look for range order handling
echo "Checking for range order implementation..."
rg -l "range.?order" --type rust

Length of output: 534

Script:

#!/bin/bash
# Check for order-related functionality in main.rs
echo "=== Order implementation in main.rs ==="
rg -A 10 "range.?order" src/main.rs

# Look for order-related types and structures
echo -e "\n=== Order-related types and enums ==="
ast-grep --pattern 'enum $_ {
  $$$
}'

# Search for order status handling
echo -e "\n=== Order status handling ==="
rg -A 5 "status|state" --type rust

# Check for buy/sell command implementations
echo -e "\n=== Buy/Sell command implementations ==="
rg -A 10 "(buy|sell).*command" --type rust

Length of output: 10967

Script:

#!/bin/bash
# Search for order kind definitions and handling
echo "=== Order Kind Implementation ==="
ast-grep --pattern 'enum OrderKind {
  $$$
}'

# Look for order taking functionality
echo -e "\n=== Order Taking Implementation ==="
rg -A 15 "take.*order|handle.*order" --type rust

# Check for Nostr event handling related to orders
echo -e "\n=== Nostr Event Handling ==="
rg -A 10 "Kind::(4|9735)" --type rust

# Look for order status enum definition
echo -e "\n=== Order Status Definition ==="
ast-grep --pattern 'enum Status {
  $$$
}'

Length of output: 4370

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with the rabbit

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@grunch, glad we agree. If there's anything I can assist with in implementing the remaining aspects of the "Take orders (Buy & Sell)" feature—such as handling encrypted DMs (Nostr event kind 4), hold invoice processing, or order status updates—please feel free to reach out.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't tested by with this you confirm that someone can take a buy order, mostro send you a lightining invoice and mostrui shows the QR code to pay it, and mostrui can handle the invoice payment by the seller, is this the behavior we have right now on take buy ?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@grunch, based on the current codebase analysis, it appears that the functionality you described—where someone can take a buy order, Mostro sends a Lightning invoice, mostrui displays the QR code for payment, and mostrui handles the invoice payment by the seller—is not fully implemented yet.

Specifically:

  • Lightning Invoice Handling: There's no implementation found for receiving or processing Lightning invoices from Mostro.
  • QR Code Display: The codebase does not contain functionality to display QR codes for payment.
  • Invoice Payment Handling: There isn't code to handle the seller's payment of the invoice within mostrui.

Given that these critical components are missing, it might be premature to mark the "Take orders (Buy & Sell)" feature as complete in the README. It may be advisable to keep this item unchecked until the full functionality is implemented and tested.

If you'd like, I can assist with outlining the steps needed to implement these features.

🔗 Analysis chain
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for implementation of "Take Buy Order" functionality

echo "=== Checking for 'Take Buy Order' functionality ==="
rg -A 5 "fn.*take_buy_order" --type rust

echo -e "\n=== Checking for Lightning invoice handling ==="
rg -A 10 "(lightning_invoice|create_invoice|hold_invoice|payment_request)" --type rust

echo -e "\n=== Checking for QR code display functionality ==="
rg -A 5 "(qr_code|QRCode|render_qr)" --type rust

echo -e "\n=== Checking for invoice payment handling by the seller ==="
rg -A 10 "(handle_invoice_payment|process_payment|payment_successful)" --type rust

Length of output: 749

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't tested by with this you confirm that someone can take a buy order, mostro send you a lightining invoice and mostrui shows the QR code to pay it, and mostrui can handle the invoice payment by the seller, is this the behavior we have right now on take buy ?

oh, you're right, it just send take-buy action to mostro. But mostrui doesn't show the QR code to pay for it nor can it handle the payment of the invoice by the seller.
I will close this pr until the whole process is completed.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

- [ ] Posts Orders (Buy & Sell)
- [ ] Direct message with peers (use nip-17)
- [ ] Fiat sent
Expand Down