Development fund implementation third phase #558

grunch · 2025-12-24T20:57:12Z

Summary by CodeRabbit

New Features
- Automated background dev-fee payer: scheduled retries, validation, LNURL resolution, payment tracking and recovery.
Behavior Change
- Dev-fee calculated when an order is taken (market & fixed), cleared on order abandonment, and seller’s share reflected in hold invoices.
Documentation
- Expanded docs on dev-fee lifecycle, timing, retries and observability; Phase 3 marked complete.
Fix
- Small-amount fee calculation enforces a 10-sat minimum.
Observability & Tests
- Improved logging and integration tests around dev-fee flows.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Enhanced the development fee technical specification with comprehensive documentation about database field management throughout the order lifecycle. Changes: - Updated Phase 3 Implementation Status to list database field update requirements alongside other required components - Added detailed "Database Field Updates" section covering: * dev_fee field: When set during order creation, special handling for market price orders, and persistence behavior * dev_fee_paid field: Initial value, trigger sequence for updates, timing of changes, and retry conditions * dev_fee_payment_hash field: Source of payment hash, format, purpose, and update conditions - Included complete payment flow timeline showing database field states at each stage from order creation through payment success/failure - Provided implementation code example for scheduler-based payment processing with atomic database updates and retry logic This documentation clarifies the complete lifecycle of dev fee database fields for Phase 3 implementation, ensuring developers understand exactly when and how each field should be modified.

Implemented Phase 3 payment execution for the development fee feature, enabling automated Lightning payments to the development fund. Changes: Database Layer (src/db.rs): - Added find_unpaid_dev_fees() query function to retrieve orders with unpaid development fees - Query filters: status='success', dev_fee>0, dev_fee_paid=0 Payment Layer (src/app/release.rs): - Implemented send_dev_fee_payment() function with multi-step payment flow: * LNURL resolution with 15-second timeout * LND connector initialization * Payment send with 5-second timeout (prevents self-payment hanging) * Payment result wait with 25-second timeout * Total timeout budget: 45 seconds - Added comprehensive error handling for each payment stage - Logging for payment initiation, success, and all failure scenarios - Returns payment hash on successful payment Scheduler Layer (src/scheduler.rs): - Added job_process_dev_fee_payment() background job - Runs every 60 seconds to process unpaid development fees - Per-payment timeout: 50 seconds (fits within 60s cycle) - On success: Updates dev_fee_paid=true and dev_fee_payment_hash - On failure: Leaves fields unchanged for automatic retry - Comprehensive logging with order_id, amount, hash, and errors Implementation Features: - Non-blocking: Order completion never blocked by dev fee payments - Automatic retry: Failed payments retry every 60 seconds - Atomic updates: Database fields updated together via Crud trait - Full audit trail: All payment attempts logged with context - Timeout protection: Prevents hanging on network/routing issues This completes the first three tasks of Phase 3, establishing the core payment infrastructure for sustainable development funding.

coderabbitai · 2025-12-24T20:57:21Z

Walkthrough

Implements scheduler-driven dev-fee payout: dev_fee set at take time, stored on orders, scheduler polls unpaid dev fees, pre-marks and attempts LNURL/LND payouts via send_dev_fee_payment, and updates DB with payment_hash/status using timeouts, retries, and expanded logging.

Changes

Cohort / File(s)	Change Summary
Docs `docs/DEV_FEE.md`	Documented Phase 3 dev-fee workflow, DB lifecycle (`dev_fee`, `dev_fee_paid`, `dev_fee_payment_hash`), timeline, SQL snippets, and observability notes.
Payment API `src/app/release.rs`	Added `pub async fn send_dev_fee_payment(order: &Order) -> Result<String, MostroError>`: validates dev_fee, resolves LNURL (15s), sends LND payment (5s), awaits result (25s), returns `payment_hash` or error; imports `DEV_FEE_LIGHTNING_ADDRESS`.
Scheduler & Orchestration `src/scheduler.rs`	Added periodic `job_process_dev_fee_payment()` and wired it into `start_scheduler()`; job finds unpaid dev fees, pre-marks PENDING, calls `send_dev_fee_payment`, updates `dev_fee_paid`/`dev_fee_payment_hash`, and handles failures/timeouts/recovery; cancellation path resets `dev_fee` when republishing.
Database & Queries `src/db.rs`, `sqlx-data.json`	Added `pub async fn find_unpaid_dev_fees(pool: &SqlitePool) -> Result<Vec<Order>, MostroError>`; changed several timestamp bindings to integer epoch; updated `update_order_to_initial_state(..., dev_fee)` signature and UPDATE query to include `dev_fee` (parameter reordering reflected in `sqlx-data.json`).
Order lifecycle (dev_fee calc) `src/app/take_buy.rs`, `src/app/take_sell.rs`, `src/util.rs`	Deferred dev_fee computation to take actions: compute `total_mostro_fee = fee * 2` and set `order.dev_fee = get_dev_fee(total_mostro_fee)` in take paths; `prepare_new_order` now initializes `dev_fee = 0`.
Cancel flow `src/app/cancel.rs`	`update_order_to_initial_state` calls now pass `dev_fee` when resetting/republishing orders.
Lightning fee calc `src/lightning/mod.rs`	Enforced minimum max-fee for tiny amounts: use `max(calculated_1pct, 10.0)` and added clarifying comment.

Sequence Diagram(s)

sequenceDiagram
    participant Scheduler as Scheduler
    participant DB as Database
    participant PaymentSvc as send_dev_fee_payment
    participant LNResolver as LNURL Resolver
    participant LND as LND Client
    participant Channel as Result Channel

    Scheduler->>DB: find_unpaid_dev_fees()
    DB-->>Scheduler: unpaid orders list

    alt for each unpaid order
        Scheduler->>DB: UPDATE dev_fee_payment_hash = "PENDING-<ts>"
        DB-->>Scheduler: OK

        Scheduler->>PaymentSvc: send_dev_fee_payment(order)

        rect rgb(245,250,255)
          Note over PaymentSvc: Resolve LNURL (≤15s)
          PaymentSvc->>LNResolver: resolve lightning address
          LNResolver-->>PaymentSvc: LNURL / error
        end

        rect rgb(245,250,255)
          Note over PaymentSvc: Initiate payment (≤5s)
          PaymentSvc->>LND: create client & send payment
          LND-->>Channel: submission ack
        end

        rect rgb(245,250,255)
          Note over PaymentSvc: Await result (≤25s)
          Channel-->>PaymentSvc: Succeeded / Failed / Timeout
        end

        alt Success
            PaymentSvc-->>Scheduler: payment_hash
            Scheduler->>DB: UPDATE dev_fee_paid=1, dev_fee_payment_hash=<hash>
            DB-->>Scheduler: OK
        else Failure / Timeout
            PaymentSvc-->>Scheduler: error
            Scheduler->>DB: clear/reset PENDING marker (retry later)
            DB-->>Scheduler: OK
        end
    end

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

Sustainable Development Fee Implementation for Mostro #549: Implements automated dev-fee calculation and payout flows that align with dev_fee fields, scheduler and payment handling added here.

Possibly related PRs

feat: add development fund configuration infrastructure #555: Provides constants/DB schema (DEV_FEE_LIGHTNING_ADDRESS, dev_fee columns) that this PR relies on.
Document development fund payment phases and architecture #556: Modifies dev_fee lifecycle and the same take/cancel paths; this PR implements the Phase 3 payment execution described there.
Added lnurl management for buy orders #499: Related LNURL resolution and client utilities used by send_dev_fee_payment.

Suggested reviewers

Catrya
arkanoider

Poem

🐇 I hop to fetch a tiny fee,
LNURLs glow and guide me free,
Scheduler ticks, I brave the race,
Hash in paw — a tiny grace,
Retry I will with eager pace.

Pre-merge checks

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: implementation of Phase 3 of the development fund system, which includes payment execution with scheduler jobs, database queries, and payment flow logic.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (2)

src/db.rs (1)

895-908: LGTM! Consider adding a database index for performance.

The function correctly queries unpaid dev fees with appropriate filtering. The implementation follows the codebase patterns for error handling and database access.

For performance optimization with large order tables, consider adding a composite index in a future migration:
CREATE INDEX idx_orders_dev_fee_unpaid 
ON orders(status, dev_fee, dev_fee_paid) 
WHERE status = 'success' AND dev_fee > 0 AND dev_fee_paid = 0;
This would significantly speed up the scheduler's periodic queries once you have many completed orders.

src/scheduler.rs (1)

511-568: LGTM! Consider concurrent processing for better scalability.

The scheduler job correctly implements the dev fee payment flow with appropriate timeouts and error handling. The 50-second timeout per payment fits well within the 60-second cycle, and the retry-on-failure logic is sound.

Optional: Process multiple payments concurrently

The current implementation processes orders sequentially. With many unpaid orders, this could cause later orders to wait until the next 60-second cycle. Consider spawning concurrent tasks with a semaphore to limit parallelism:

 async fn job_process_dev_fee_payment() {
     let pool = get_db_pool();
     let interval = 60u64; // Every 60 seconds
+    let max_concurrent = 5; // Limit concurrent payments
 
     tokio::spawn(async move {
         loop {
             info!("Checking for unpaid development fees");
 
             // Query unpaid orders
             if let Ok(unpaid_orders) = find_unpaid_dev_fees(&pool).await {
                 info!("Found {} orders with unpaid dev fees", unpaid_orders.len());
+                
+                let mut tasks = vec![];
 
                 for mut order in unpaid_orders {
-                    // 50 second timeout per payment (under 60s cycle)
-                    match tokio::time::timeout(
-                        std::time::Duration::from_secs(50),
-                        send_dev_fee_payment(&order),
-                    )
-                    .await
-                    {
-                        Ok(Ok(payment_hash)) => {
-                            // SUCCESS: Update both fields atomically
-                            let order_id = order.id;
-                            let dev_fee_amount = order.dev_fee;
-                            order.dev_fee_paid = true;
-                            order.dev_fee_payment_hash = Some(payment_hash.clone());
-
-                            if let Err(e) = order.update(&pool).await {
-                                error!("Failed to update database for order {}: {}", order_id, e);
-                            } else {
-                                info!(
-                                    "Dev fee payment succeeded for order {} - amount: {} sats, hash: {}",
-                                    order_id, dev_fee_amount, payment_hash
-                                );
-                            }
-                        }
-                        Ok(Err(e)) => {
-                            // FAILURE: Leave fields unchanged for retry
-                            error!(
-                                "Dev fee payment failed for order {} ({} sats) - error: {:?}, will retry",
-                                order.id, order.dev_fee, e
-                            );
-                        }
-                        Err(_) => {
-                            // TIMEOUT: Leave fields unchanged for retry
-                            error!(
-                                "Dev fee payment timeout (50s) for order {} ({} sats) - will retry",
-                                order.id, order.dev_fee
-                            );
-                        }
-                    }
+                    let pool_clone = pool.clone();
+                    let task = tokio::spawn(async move {
+                        // Process payment with timeout...
+                        // (same logic as before)
+                    });
+                    tasks.push(task);
+                    
+                    // Limit concurrent tasks
+                    if tasks.len() >= max_concurrent {
+                        let _ = futures::future::join_all(tasks.drain(..)).await;
+                    }
                 }
+                
+                // Wait for remaining tasks
+                let _ = futures::future::join_all(tasks).await;
             }
 
             tokio::time::sleep(tokio::time::Duration::from_secs(interval)).await;
         }
     });
 }

This allows up to 5 payments to process simultaneously while maintaining the 50-second timeout per payment.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fbbad40 and f508669.

📒 Files selected for processing (4)

docs/DEV_FEE.md
src/app/release.rs
src/db.rs
src/scheduler.rs

🧰 Additional context used

📓 Path-based instructions (3)

src/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Runtime code lives in src/ directory with subdirectories: src/app/ for order flows and business logic, src/lightning/ for LND bindings and Lightning helpers, src/rpc/ for gRPC service and types, and src/config/ for settings and loaders

Files:

src/db.rs
src/app/release.rs
src/scheduler.rs

**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Use 4-space indent, snake_case for functions, PascalCase for types, and SCREAMING_SNAKE_CASE for constants in Rust code (Rust 2021 edition)
Document non-obvious public APIs with triple-slash /// documentation comments
Prefer tracing spans over ad-hoc logging in Rust code
Co-locate tests in their modules under mod tests blocks and name tests descriptively

Files:

src/db.rs
src/app/release.rs
src/scheduler.rs

src/app/**

📄 CodeRabbit inference engine (AGENTS.md)

Mirror test fixtures under src/app/ directory where applicable

Files:

src/app/release.rs

🧠 Learnings (7)

📓 Common learnings

Learnt from: grunch
Repo: MostroP2P/mostro PR: 554
File: docs/DEV_FEE_TECHNICAL_SPEC.md:1-434
Timestamp: 2025-12-17T13:04:13.036Z
Learning: In the Mostro development fee implementation, the Lightning address `mostro_p2psats.mobi` in src/config/constants.rs is temporary for testing purposes. The production address will be `developmentmostro.network` as documented in docs/DEV_FEE_TECHNICAL_SPEC.md. This intentional mismatch is planned to be resolved before production deployment.

📚 Learning: 2025-03-11T23:26:33.763Z

Learnt from: bilthon
Repo: MostroP2P/mostro PR: 464
File: src/app/add_invoice.rs:73-82
Timestamp: 2025-03-11T23:26:33.763Z
Learning: In the Mostro codebase, the `update_order_event` function does not perform database operations - it only updates an order in memory and sends a Nostr event. The actual database update happens separately when calling `.update(pool)` afterward.

Applied to files:

src/db.rs
docs/DEV_FEE.md

📚 Learning: 2025-06-25T22:48:44.741Z

Learnt from: Catrya
Repo: MostroP2P/mostro PR: 502
File: src/app/release.rs:52-55
Timestamp: 2025-06-25T22:48:44.741Z
Learning: The PaymentFailedInfo struct used in src/app/release.rs is defined in mostro-core PR #111, creating a cross-repository dependency that causes compilation errors until the dependency is resolved.

Applied to files:

src/db.rs
src/app/release.rs
src/scheduler.rs

📚 Learning: 2025-04-27T20:07:24.558Z

Learnt from: arkanoider
Repo: MostroP2P/mostro PR: 475
File: src/db.rs:792-802
Timestamp: 2025-04-27T20:07:24.558Z
Learning: The decrypt_data function from mostro_core::order returns a type that needs to be wrapped with MostroInternalErr to convert it to MostroError, so calls should keep the .map_err(MostroInternalErr) conversion.

Applied to files:

src/db.rs

📚 Learning: 2025-06-16T23:17:54.795Z

Learnt from: Catrya
Repo: MostroP2P/mostro PR: 501
File: src/util.rs:0-0
Timestamp: 2025-06-16T23:17:54.795Z
Learning: The `get_db_pool()` function in the codebase returns `Arc<SqlitePool>`, not `&SqlitePool`. When calling functions that expect `&SqlitePool`, you need to use `&pool` to convert `Arc<SqlitePool>` to `&SqlitePool` through automatic dereferencing.

Applied to files:

src/db.rs

📚 Learning: 2025-12-17T13:04:13.036Z

Learnt from: grunch
Repo: MostroP2P/mostro PR: 554
File: docs/DEV_FEE_TECHNICAL_SPEC.md:1-434
Timestamp: 2025-12-17T13:04:13.036Z
Learning: In the Mostro development fee implementation, the Lightning address `mostro_p2psats.mobi` in src/config/constants.rs is temporary for testing purposes. The production address will be `developmentmostro.network` as documented in docs/DEV_FEE_TECHNICAL_SPEC.md. This intentional mismatch is planned to be resolved before production deployment.

Applied to files:

src/app/release.rs
docs/DEV_FEE.md

📚 Learning: 2025-06-25T22:48:44.741Z

Learnt from: Catrya
Repo: MostroP2P/mostro PR: 502
File: src/app/release.rs:52-55
Timestamp: 2025-06-25T22:48:44.741Z
Learning: PaymentFailedInfo struct and Payload::PaymentFailed variant are defined in mostro-core PR #111, creating a cross-repository dependency with the Mostro repository that requires careful coordination of merging and version updates.

Applied to files:

src/app/release.rs

🧬 Code graph analysis (3)

src/db.rs (2)

src/app/orders.rs (1)

orders (36-39)

src/app.rs (1)

e (391-391)

src/app/release.rs (3)

src/lightning/invoice.rs (1)

payment_request (131-132)

src/lnurl.rs (1)

resolv_ln_address (72-124)

src/lightning/mod.rs (1)

new (38-52)

src/scheduler.rs (3)

src/app/release.rs (1)

send_dev_fee_payment (578-688)

src/config/settings.rs (1)

get_db_pool (31-33)

src/db.rs (1)

find_unpaid_dev_fees (895-908)

🪛 markdownlint-cli2 (0.18.1)

docs/DEV_FEE.md

545-545: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: test

🔇 Additional comments (3)

src/app/release.rs (2)

564-688: LGTM! Excellent timeout strategy and error handling.

The implementation correctly handles the three-step payment flow with well-designed timeouts:

LNURL resolution: 15s (handles DNS/network issues)

send_payment: 5s (prevents hanging on self-payments or connection issues)

Payment result: 25s (allows time for routing)

Total: 45s (fits within the 50s scheduler timeout)

The comprehensive error logging at each step will greatly aid debugging and monitoring.

Based on learnings, the Lightning address used in DEV_FEE_LIGHTNING_ADDRESS is temporary for testing purposes. Please verify that the production Lightning address (development@mostro.network) will be updated before production deployment, as documented in docs/DEV_FEE_TECHNICAL_SPEC.md.

610-630: Verify the send_payment timeout prevents self-payment hangs.

The 5-second timeout on send_payment is designed to prevent indefinite blocking when attempting self-payments (where the dev fee destination uses the same Lightning node as Mostro). This is a good safeguard mentioned in the documentation.

Consider testing this specific scenario to ensure the timeout triggers correctly:

Configure DEV_FEE_LIGHTNING_ADDRESS to point to an address on the same LND node

Complete an order and observe the scheduler behavior

Verify the 5-second timeout fires and the error is logged

Confirm the order remains in retry queue for the next cycle

This will validate that the self-payment detection works as documented (docs/DEV_FEE.md lines 668-669).

docs/DEV_FEE.md (1)

49-653: Excellent documentation! Comprehensive and accurate.

The documentation thoroughly explains the database field lifecycle, payment flow timeline, and implementation details. The detailed explanations of when each field is updated, combined with code examples, will be invaluable for maintainers and future developers.

The documentation accurately reflects the implementation in src/scheduler.rs and src/app/release.rs, including:

Correct timeout values (15s, 5s, 25s)

Accurate field update logic (atomic updates on success, unchanged on failure)

Proper retry semantics (60-second scheduler cycles)

Complete error handling scenarios

docs/DEV_FEE.md

This commit addresses dev fee payment failures that were preventing successful payments to the development fund Lightning address. Fix 1: Handle multiple LND payment status messages - Changed send_dev_fee_payment() to loop over all status messages - LND sends multiple updates (Unknown -> InFlight -> Succeeded/Failed) - Previous code only read first message, treating intermediate status as error - Now ignores intermediate statuses and waits for terminal status - Matches proven pattern used in do_payment() for buyer payments Fix 2: Ensure minimum routing fee for small payments - Fee limit calculation for small amounts was rounding to 0 sats - Example: 30 sats * 0.01 = 0.3 -> truncated to 0 in integer conversion - LND cannot find routes with zero fee allowance - Added minimum fee limit of 10 sats for payments <= 1000 sats - Allows successful routing for dev fee payments (typically 30-100 sats) Additional improvement: - Enhanced error logging to include LND failure_reason code - Helps diagnose routing and payment issues Resolves "unknown status" and "no route" errors in dev fee payments.

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/app/release.rs (1)
564-684: Prefer tracing spans over ad-hoc logging for long-running operations.

This function performs multiple network operations over up to 45 seconds. Wrapping it in a tracing span would provide better observability by automatically tracking duration, allowing correlation of related log messages, and enabling distributed tracing.

As per coding guidelines: "Prefer tracing spans over ad-hoc logging in Rust code."
🔎 Example refactor using tracing spans
+use tracing::{error, info, instrument};
+
+#[instrument(skip(order), fields(order_id = %order.id, dev_fee = order.dev_fee))]
 pub async fn send_dev_fee_payment(order: &Order) -> Result<String, MostroError> {
     info!(
-        "Initiating dev fee payment for order {} - amount: {} sats to {}",
-        order.id, order.dev_fee, DEV_FEE_LIGHTNING_ADDRESS
+        "Initiating dev fee payment to {}",
+        DEV_FEE_LIGHTNING_ADDRESS
     );
 
     // Step 1: LNURL resolution (15s timeout)
This approach automatically includes order_id and dev_fee in all log messages within the span.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f508669 and 680d515.

📒 Files selected for processing (2)

src/app/release.rs
src/lightning/mod.rs

🧰 Additional context used

📓 Path-based instructions (3)

src/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Runtime code lives in src/ directory with subdirectories: src/app/ for order flows and business logic, src/lightning/ for LND bindings and Lightning helpers, src/rpc/ for gRPC service and types, and src/config/ for settings and loaders

Files:

src/lightning/mod.rs
src/app/release.rs

**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Use 4-space indent, snake_case for functions, PascalCase for types, and SCREAMING_SNAKE_CASE for constants in Rust code (Rust 2021 edition)
Document non-obvious public APIs with triple-slash /// documentation comments
Prefer tracing spans over ad-hoc logging in Rust code
Co-locate tests in their modules under mod tests blocks and name tests descriptively

Files:

src/lightning/mod.rs
src/app/release.rs

src/app/**

📄 CodeRabbit inference engine (AGENTS.md)

Mirror test fixtures under src/app/ directory where applicable

Files:

src/app/release.rs

🧠 Learnings (5)

📓 Common learnings

Learnt from: grunch
Repo: MostroP2P/mostro PR: 554
File: docs/DEV_FEE_TECHNICAL_SPEC.md:1-434
Timestamp: 2025-12-17T13:04:13.036Z
Learning: In the Mostro development fee implementation, the Lightning address `mostro_p2psats.mobi` in src/config/constants.rs is temporary for testing purposes. The production address will be `developmentmostro.network` as documented in docs/DEV_FEE_TECHNICAL_SPEC.md. This intentional mismatch is planned to be resolved before production deployment.

📚 Learning: 2025-07-17T13:51:17.338Z

Learnt from: arkanoider
Repo: MostroP2P/mostro PR: 499
File: src/lightning/invoice.rs:108-114
Timestamp: 2025-07-17T13:51:17.338Z
Learning: In Lightning invoice validation for Mostro buy orders (src/lightning/invoice.rs), zero-amount invoices (amount_sat == 0) should always pass validation even when an expected amount is provided. This is because users can submit invoices at order creation time before knowing the exact satoshi amount they'll receive. The condition `if amount_sat != res && amount_sat != 0` correctly implements this by allowing zero-amount invoices to pass while enforcing exact amount matching for non-zero invoices.

Applied to files:

src/lightning/mod.rs

📚 Learning: 2025-12-17T13:04:13.036Z

Learnt from: grunch
Repo: MostroP2P/mostro PR: 554
File: docs/DEV_FEE_TECHNICAL_SPEC.md:1-434
Timestamp: 2025-12-17T13:04:13.036Z
Learning: In the Mostro development fee implementation, the Lightning address `mostro_p2psats.mobi` in src/config/constants.rs is temporary for testing purposes. The production address will be `developmentmostro.network` as documented in docs/DEV_FEE_TECHNICAL_SPEC.md. This intentional mismatch is planned to be resolved before production deployment.

Applied to files:

src/app/release.rs

📚 Learning: 2025-06-25T22:48:44.741Z

Learnt from: Catrya
Repo: MostroP2P/mostro PR: 502
File: src/app/release.rs:52-55
Timestamp: 2025-06-25T22:48:44.741Z
Learning: The PaymentFailedInfo struct used in src/app/release.rs is defined in mostro-core PR #111, creating a cross-repository dependency that causes compilation errors until the dependency is resolved.

Applied to files:

src/app/release.rs

📚 Learning: 2025-06-25T22:48:44.741Z

Learnt from: Catrya
Repo: MostroP2P/mostro PR: 502
File: src/app/release.rs:52-55
Timestamp: 2025-06-25T22:48:44.741Z
Learning: PaymentFailedInfo struct and Payload::PaymentFailed variant are defined in mostro-core PR #111, creating a cross-repository dependency with the Mostro repository that requires careful coordination of merging and version updates.

Applied to files:

src/app/release.rs

🔇 Additional comments (2)

src/app/release.rs (2)

2-2: LGTM: Import additions support the new dev fee payment function.

The import additions are necessary and correctly scoped for the new send_dev_fee_payment function.

Also applies to: 27-27

564-684: Confirm test coverage for this public function per coding guidelines.

The coding guidelines require co-located tests under mod tests blocks for public APIs. Please ensure send_dev_fee_payment has corresponding test coverage in the same module.

Note: The Lightning address assumption is intentional and documented—this function correctly uses LNURL resolution for the configured development fee address, which differs from do_payment's dual support for both Lightning addresses and bolt11 invoices.

src/app/release.rs

Catrya

En la base de datos me sale todo el tiempo dev_fee en 0 aunque se esté haciendo bien el cálculo.
No me sale ningún log de que falla el pago del dev_fee aunque estoy usando regtest y nunca lo va a pagar, no sale ningun log de dev_fee en ningún momento.

Me ocurre tanto al crear una orden de venta como una de compra. Probé usando la db que tenía antes de probar este PR, y también la borré y reinicié y siguió pasando lo mismo

Update documentation

This commit addresses three critical issues with the development fee payment system: 1. Payment Timing and Race Condition Protection: - Dev fee payment now triggers when order reaches 'settled-hold-invoice' status - Query updated to check both 'settled-hold-invoice' AND 'success' statuses - Handles edge case where buyer payment succeeds while dev fee payment is in progress 2. Market Price Orders Dev Fee Calculation: - Fixed bug where market price orders (amount=0) had dev_fee=0 permanently - Dev fee now calculated in take_buy and take_sell actions when amount is determined - Formula: dev_fee = get_dev_fee(fee * 2) 3. Enhanced Logging and Diagnostics: - Added detailed logging for dev fee payment process - Logs show before/after state and verification of database updates - Helps track payment execution and database persistence Documentation updated with market price order flow and troubleshooting guide.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

docs/DEV_FEE.md (1)
300-300: Fix missing language specification on fenced code blocks.

The fenced code blocks at lines 300 and 322 lack language identifiers, which markdownlint flags. Add text or plaintext to improve rendering:
🔎 Proposed fix
-```
+```text
Initial Order: 100,000 sats at $50,000/BTC (market price)
-```
+```text
Market Price Order Created:
Also applies to: 322-322

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 680d515 and 2655943.

📒 Files selected for processing (6)

docs/DEV_FEE.md
src/app/release.rs
src/app/take_buy.rs
src/app/take_sell.rs
src/db.rs
src/scheduler.rs

🚧 Files skipped from review as they are similar to previous changes (2)

src/app/release.rs
src/db.rs

🧰 Additional context used

📓 Path-based instructions (3)

src/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Runtime code lives in src/ directory with subdirectories: src/app/ for order flows and business logic, src/lightning/ for LND bindings and Lightning helpers, src/rpc/ for gRPC service and types, and src/config/ for settings and loaders

Files:

src/app/take_sell.rs
src/scheduler.rs
src/app/take_buy.rs

**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Use 4-space indent, snake_case for functions, PascalCase for types, and SCREAMING_SNAKE_CASE for constants in Rust code (Rust 2021 edition)
Document non-obvious public APIs with triple-slash /// documentation comments
Prefer tracing spans over ad-hoc logging in Rust code
Co-locate tests in their modules under mod tests blocks and name tests descriptively

Files:

src/app/take_sell.rs
src/scheduler.rs
src/app/take_buy.rs

src/app/**

📄 CodeRabbit inference engine (AGENTS.md)

Mirror test fixtures under src/app/ directory where applicable

Files:

src/app/take_sell.rs
src/app/take_buy.rs

🧠 Learnings (10)

📓 Common learnings

Learnt from: grunch
Repo: MostroP2P/mostro PR: 554
File: docs/DEV_FEE_TECHNICAL_SPEC.md:1-434
Timestamp: 2025-12-17T13:04:13.036Z
Learning: In the Mostro development fee implementation, the Lightning address `mostro_p2psats.mobi` in src/config/constants.rs is temporary for testing purposes. The production address will be `developmentmostro.network` as documented in docs/DEV_FEE_TECHNICAL_SPEC.md. This intentional mismatch is planned to be resolved before production deployment.

📚 Learning: 2025-04-27T20:07:24.558Z

Learnt from: arkanoider
Repo: MostroP2P/mostro PR: 475
File: src/db.rs:792-802
Timestamp: 2025-04-27T20:07:24.558Z
Learning: The decrypt_data function from mostro_core::order returns a type that needs to be wrapped with MostroInternalErr to convert it to MostroError, so calls should keep the .map_err(MostroInternalErr) conversion.

Applied to files:

src/app/take_sell.rs
src/app/take_buy.rs

📚 Learning: 2025-03-11T23:26:33.763Z

Learnt from: bilthon
Repo: MostroP2P/mostro PR: 464
File: src/app/add_invoice.rs:73-82
Timestamp: 2025-03-11T23:26:33.763Z
Learning: In the Mostro codebase, the `update_order_event` function does not perform database operations - it only updates an order in memory and sends a Nostr event. The actual database update happens separately when calling `.update(pool)` afterward.

Applied to files:

src/app/take_sell.rs

📚 Learning: 2025-11-06T22:54:07.055Z

Learnt from: arkanoider
Repo: MostroP2P/mostro PR: 545
File: src/app.rs:182-186
Timestamp: 2025-11-06T22:54:07.055Z
Learning: In the Mostro codebase, as of PR #545, the code in check_trade_index() prevents new regular users from being created with last_trade_index = 0. Users must provide a trade_index >= 1 when creating or taking their first order. The guard at lines 184-186 in src/app.rs rejects trade_index == 0 before user creation.

Applied to files:

src/app/take_sell.rs

📚 Learning: 2025-11-06T23:01:06.496Z

Learnt from: arkanoider
Repo: MostroP2P/mostro PR: 545
File: src/app/last_trade_index.rs:67-70
Timestamp: 2025-11-06T23:01:06.496Z
Learning: In the Mostro codebase, users are only created through the check_trade_index() function when they create or take their first order. Since check_trade_index() validates and rejects trade_index = 0 (using pattern matching at line 134 in src/app.rs with `index @ 1..`), no user can ever be created with last_trade_index = 0, even though the database migration shows DEFAULT 0. Therefore, the guard in last_trade_index.rs that returns InvalidTradeIndex when last_trade_index == 0 is safe and will only catch invalid database states.

Applied to files:

src/app/take_sell.rs

📚 Learning: 2025-10-09T14:06:50.067Z

Learnt from: arkanoider
Repo: MostroP2P/mostro PR: 530
File: src/app/order.rs:278-279
Timestamp: 2025-10-09T14:06:50.067Z
Learning: In the Mostro project, tests in files like `src/app/order.rs` may use structural checks to ensure functions like `order_action` do not panic during message construction, rather than validating specific error variants.

Applied to files:

src/app/take_sell.rs

📚 Learning: 2025-06-25T22:48:44.741Z

Learnt from: Catrya
Repo: MostroP2P/mostro PR: 502
File: src/app/release.rs:52-55
Timestamp: 2025-06-25T22:48:44.741Z
Learning: PaymentFailedInfo struct and Payload::PaymentFailed variant are defined in mostro-core PR #111, creating a cross-repository dependency with the Mostro repository that requires careful coordination of merging and version updates.

Applied to files:

src/app/take_sell.rs

📚 Learning: 2025-07-17T13:51:17.338Z

Learnt from: arkanoider
Repo: MostroP2P/mostro PR: 499
File: src/lightning/invoice.rs:108-114
Timestamp: 2025-07-17T13:51:17.338Z
Learning: In Lightning invoice validation for Mostro buy orders (src/lightning/invoice.rs), zero-amount invoices (amount_sat == 0) should always pass validation even when an expected amount is provided. This is because users can submit invoices at order creation time before knowing the exact satoshi amount they'll receive. The condition `if amount_sat != res && amount_sat != 0` correctly implements this by allowing zero-amount invoices to pass while enforcing exact amount matching for non-zero invoices.

Applied to files:

src/app/take_sell.rs
docs/DEV_FEE.md
src/app/take_buy.rs

📚 Learning: 2025-12-17T13:04:13.036Z

Learnt from: grunch
Repo: MostroP2P/mostro PR: 554
File: docs/DEV_FEE_TECHNICAL_SPEC.md:1-434
Timestamp: 2025-12-17T13:04:13.036Z
Learning: In the Mostro development fee implementation, the Lightning address `mostro_p2psats.mobi` in src/config/constants.rs is temporary for testing purposes. The production address will be `developmentmostro.network` as documented in docs/DEV_FEE_TECHNICAL_SPEC.md. This intentional mismatch is planned to be resolved before production deployment.

Applied to files:

src/app/take_sell.rs
docs/DEV_FEE.md
src/app/take_buy.rs

📚 Learning: 2025-06-25T22:48:44.741Z

Learnt from: Catrya
Repo: MostroP2P/mostro PR: 502
File: src/app/release.rs:52-55
Timestamp: 2025-06-25T22:48:44.741Z
Learning: The PaymentFailedInfo struct used in src/app/release.rs is defined in mostro-core PR #111, creating a cross-repository dependency that causes compilation errors until the dependency is resolved.

Applied to files:

src/scheduler.rs

🧬 Code graph analysis (2)

src/app/take_sell.rs (1)

src/util.rs (8)

get_dev_fee (176-179)

get_fiat_amount_requested (981-998)

get_market_amount_and_fee (795-805)

get_order (1035-1048)

set_waiting_invoice_status (808-867)

show_hold_invoice (660-743)

update_order_event (594-631)

validate_invoice (1110-1138)

src/app/take_buy.rs (1)

src/util.rs (3)

get_dev_fee (176-179)

get_market_amount_and_fee (795-805)

show_hold_invoice (660-743)

🪛 GitHub Actions: CI

src/scheduler.rs

[error] 548-548: Code formatting changes detected in a multi-line log macro. Please run 'cargo fmt' to fix.

[error] 555-555: Code formatting changes detected in an info! macro block. Please run 'cargo fmt' to fix.

[error] 560-560: Code formatting changes detected in SQL query string formatting. Please run 'cargo fmt' to fix.

src/app/take_buy.rs

[error] 1-1: Code formatting issues detected by cargo fmt. Please run 'cargo fmt' to apply formatting.

🪛 markdownlint-cli2 (0.18.1)

docs/DEV_FEE.md

300-300: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

322-322: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

624-624: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

999-999: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

🔇 Additional comments (7)

src/app/take_sell.rs (1)

4-5: LGTM! Dev fee calculation correctly implemented for market price orders.

The implementation correctly:

Calculates dev_fee only when the order amount is determined from market price

Uses total_mostro_fee = order.fee * 2 to represent both parties' fees before applying the dev_fee percentage

Mirrors the identical pattern in src/app/take_buy.rs, ensuring consistency across buy and sell flows

This aligns with the broader dev fee workflow documented in docs/DEV_FEE.md and ensures market price orders properly populate dev_fee for subsequent payment processing.

Also applies to: 108-111
src/scheduler.rs (2)
1-1: LGTM! Scheduler integration follows existing pattern.

The import of send_dev_fee_payment and integration of job_process_dev_fee_payment() into the scheduler startup sequence are correctly implemented and follow the established pattern of other scheduler jobs.

Also applies to: 29-29

511-602: Query coverage is correctly implemented—no changes needed.

The implementation is well-structured with proper timeout handling and atomic updates. The find_unpaid_dev_fees() function in src/db.rs already implements the dual-status query as documented:
WHERE (status = 'settled-hold-invoice' OR status = 'success') AND dev_fee > 0 AND dev_fee_paid = 0
This correctly handles race conditions where buyer payment completes during dev fee payment failure, as specified in docs/DEV_FEE.md.

Logging verbosity can be reduced to essential messages after the feature stabilizes, though the current level is reasonable for active deployment and debugging.
docs/DEV_FEE.md (3)

186-332: Excellent documentation of market price order dev fee handling.

The new sections clearly explain critical implementation details:

Dev Fee Reset (lines 186-261): Properly documents why dev_fee must be reset to 0 when market price orders are abandoned, preventing incorrect charges on re-take at different prices.

Atomic Update Pattern (lines 262-332): Accurately reflects the implementation in src/app/take_buy.rs and src/app/take_sell.rs, emphasizing the dependency chain: amount → fee → dev_fee.

The concrete examples and SQL verification queries make this documentation immediately actionable for developers and operators.

536-692: Comprehensive database field lifecycle documentation.

The detailed breakdown of when and how each field (dev_fee, dev_fee_paid, dev_fee_payment_hash) is modified provides excellent reference material. The payment flow timeline with exact state transitions at each stage is particularly valuable for:

Understanding the asynchronous nature of dev fee payments

Debugging race conditions between buyer payment and dev fee payment

Verifying correct scheduler behavior

The documentation correctly reflects the dual-status query pattern (settled-hold-invoice OR success) to handle edge cases where order status transitions during payment failures.

999-1028: Proactive troubleshooting section addresses potential revenue loss.

The new troubleshooting section for market price orders with zero dev_fee provides:

Clear SQL queries to detect the issue

Explanation of root cause (missing calculation during take)

Verification query to measure impact

Reference to the fix in the implementation sections

This demonstrates forward-thinking documentation that anticipates operational concerns.

src/app/take_buy.rs (1)

2-3: Code logic is correct and properly mirrors take_sell.rs.

The import and dev fee calculation are accurate. The code correctly:

Adds get_dev_fee to imports (lines 2-3)

Computes total_mostro_fee as double the single-party fee

Applies the configured dev_fee percentage via get_dev_fee()

The formatting concern could not be verified in the current environment, but the code changes themselves are sound.

Catrya · 2025-12-26T20:13:30Z

Cuando la orden es tomada, si regresa a status Pending, en l base de datos el dev_fee no se restablece a 0 sino que se queda con el valor de cuando la orden fue tomada antes

When a taker abandons a market price order (doesn't proceed after taking it), the order returns to pending status after the configured timeout. Previously, amount and fee were reset to 0, but dev_fee was not reset, causing incorrect dev_fee charges when the order was re-taken at a different market price. Changes: - Reset dev_fee to 0 in scheduler timeout handler (src/scheduler.rs:357) - Add "Taker Abandonment and Order Reset" documentation section - Document both reset paths: explicit cancellation and automatic timeout - Add example flow showing timeout scenario with price change - Include troubleshooting query for detecting stale dev_fee values This fix ensures consistency between the explicit cancellation path (cancel.rs) and the automatic timeout path (scheduler.rs), both now properly reset all three price-dependent fields: amount, fee, and dev_fee.

Add troubleshooting section for detecting and understanding market price orders with stale dev_fee values after taker timeout. Includes: - Symptom detection query - Root cause explanation - Impact assessment - Fix references to both code paths - Verification query to confirm fix is working This helps operators identify and troubleshoot any remaining instances of the bug that was fixed in the previous commit.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (9)

src/scheduler.rs (2)
512-609: Consider adding function documentation.

While the implementation is correct, adding a documentation comment would help future maintainers understand the design decisions:
/// Processes unpaid development fees for completed orders
/// 
/// Runs every 60 seconds and attempts to pay dev fees for orders that have:
/// - status = 'settled-hold-invoice' OR 'success'
/// - dev_fee > 0
/// - dev_fee_paid = false
///
/// Design decisions:
/// - 50-second timeout per payment (10s buffer before next cycle)
/// - Sequential processing (one order at a time) to avoid overwhelming scheduler
/// - Automatic retry on next cycle for failed payments
/// - Enhanced logging (BEFORE/AFTER/VERIFY) for troubleshooting database persistence
async fn job_process_dev_fee_payment() {
524-603: Sequential processing may create backlog with multiple unpaid orders.

The current implementation processes orders sequentially with a 50-second timeout per payment. If multiple orders have unpaid dev fees, only one will be processed per 60-second cycle, potentially creating a backlog.

However, this appears intentional to:

Avoid overwhelming the scheduler with concurrent payment attempts

Prevent resource exhaustion if many payments fail simultaneously

Ensure each payment has adequate timeout budget

If backlog becomes a concern in production, consider tracking metrics:
-- Monitor unpaid dev fee queue depth
SELECT COUNT(*) as unpaid_count,
       MAX(strftime('%s', 'now') - created_at) as oldest_unpaid_seconds
FROM orders
WHERE (status = 'settled-hold-invoice' OR status = 'success')
  AND dev_fee > 0
  AND dev_fee_paid = 0;
docs/DEV_FEE.md (7)
106-111: Add language specifier to fenced code block.

The fenced code block is missing a language identifier, which causes markdown linting warnings. Since this appears to be example log output, specify the language:
🔎 Proposed fix
-    ```
+    ```text
     BEFORE UPDATE: order_id=abc123, dev_fee_paid=false, dev_fee_payment_hash=None
     AFTER MODIFY: order_id=abc123, dev_fee_paid=true, dev_fee_payment_hash=Some("a1b2c3...")
     ✅ DATABASE UPDATE SUCCEEDED for order abc123
     VERIFICATION: order_id=abc123, dev_fee_paid=true, dev_fee_payment_hash=Some("a1b2c3...")
-    ```
+    ```
140-148: Add language specifier to flow diagram code block.

The fenced code block for the flow diagram is missing a language identifier. Since this is a text-based diagram, specify it as text or plaintext:
🔎 Proposed fix
-```
+```text
 Order Creation → Fee Calculation → Hold Invoice → Seller Release → Dev Payment → Buyer Payment
      ↓                 ↓                ↓              ↓                ↓             ↓
387-418: Add language specifier to example scenario code block.
🔎 Proposed fix
-```
+```text
 Market Price Order Created:
 - Fiat: $100 USD
409-418: Add language specifier to bug example code block.
🔎 Proposed fix
-```
+```text
 Order Taken:
 - amount: 200,000 sats  ✓
445-461: Add language specifier to flow example code block.
🔎 Proposed fix
-```
+```text
 Order Created (market price):
 - amount: 0, fee: 0, dev_fee: 0, status: pending
1247-1276: Use heading instead of bold emphasis for issue title.

Markdown linting recommends using proper heading syntax instead of bold text for section titles:
🔎 Proposed fix
-**4. Market Price Orders With Zero Dev Fee**
+#### 4. Market Price Orders With Zero Dev Fee
 
-**Symptom:**
+##### Symptom
1277-1312: Use heading instead of bold emphasis for issue title.
🔎 Proposed fix
-**5. Market Price Orders with Stale dev_fee After Timeout**
+#### 5. Market Price Orders with Stale dev_fee After Timeout
 
-**Symptom:**
+##### Symptom
Apply similar changes to other bold subsection titles (Cause, Impact, Fix, Prevention, Verification).

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5c1cdb5 and 723e0f1.

📒 Files selected for processing (2)

docs/DEV_FEE.md
src/scheduler.rs

🧰 Additional context used

📓 Path-based instructions (2)

src/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Runtime code lives in src/ directory with subdirectories: src/app/ for order flows and business logic, src/lightning/ for LND bindings and Lightning helpers, src/rpc/ for gRPC service and types, and src/config/ for settings and loaders

Files:

src/scheduler.rs

**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Use 4-space indent, snake_case for functions, PascalCase for types, and SCREAMING_SNAKE_CASE for constants in Rust code (Rust 2021 edition)
Document non-obvious public APIs with triple-slash /// documentation comments
Prefer tracing spans over ad-hoc logging in Rust code
Co-locate tests in their modules under mod tests blocks and name tests descriptively

Files:

src/scheduler.rs

🧠 Learnings (6)

📓 Common learnings

Learnt from: grunch
Repo: MostroP2P/mostro PR: 554
File: docs/DEV_FEE_TECHNICAL_SPEC.md:1-434
Timestamp: 2025-12-17T13:04:13.036Z
Learning: In the Mostro development fee implementation, the Lightning address `mostro_p2psats.mobi` in src/config/constants.rs is temporary for testing purposes. The production address will be `developmentmostro.network` as documented in docs/DEV_FEE_TECHNICAL_SPEC.md. This intentional mismatch is planned to be resolved before production deployment.

📚 Learning: 2025-06-25T22:48:44.741Z

Learnt from: Catrya
Repo: MostroP2P/mostro PR: 502
File: src/app/release.rs:52-55
Timestamp: 2025-06-25T22:48:44.741Z
Learning: The PaymentFailedInfo struct used in src/app/release.rs is defined in mostro-core PR #111, creating a cross-repository dependency that causes compilation errors until the dependency is resolved.

Applied to files:

src/scheduler.rs

📚 Learning: 2025-12-17T13:04:13.036Z

Learnt from: grunch
Repo: MostroP2P/mostro PR: 554
File: docs/DEV_FEE_TECHNICAL_SPEC.md:1-434
Timestamp: 2025-12-17T13:04:13.036Z
Learning: In the Mostro development fee implementation, the Lightning address `mostro_p2psats.mobi` in src/config/constants.rs is temporary for testing purposes. The production address will be `developmentmostro.network` as documented in docs/DEV_FEE_TECHNICAL_SPEC.md. This intentional mismatch is planned to be resolved before production deployment.

Applied to files:

docs/DEV_FEE.md

📚 Learning: 2025-07-17T13:51:17.338Z

Learnt from: arkanoider
Repo: MostroP2P/mostro PR: 499
File: src/lightning/invoice.rs:108-114
Timestamp: 2025-07-17T13:51:17.338Z
Learning: In Lightning invoice validation for Mostro buy orders (src/lightning/invoice.rs), zero-amount invoices (amount_sat == 0) should always pass validation even when an expected amount is provided. This is because users can submit invoices at order creation time before knowing the exact satoshi amount they'll receive. The condition `if amount_sat != res && amount_sat != 0` correctly implements this by allowing zero-amount invoices to pass while enforcing exact amount matching for non-zero invoices.

Applied to files:

docs/DEV_FEE.md

📚 Learning: 2025-03-11T23:26:33.763Z

Learnt from: bilthon
Repo: MostroP2P/mostro PR: 464
File: src/app/add_invoice.rs:73-82
Timestamp: 2025-03-11T23:26:33.763Z
Learning: In the Mostro codebase, the `update_order_event` function does not perform database operations - it only updates an order in memory and sends a Nostr event. The actual database update happens separately when calling `.update(pool)` afterward.

Applied to files:

docs/DEV_FEE.md

📚 Learning: 2025-06-16T15:25:19.185Z

Learnt from: arkanoider
Repo: MostroP2P/mostro PR: 499
File: src/lightning/invoice.rs:130-145
Timestamp: 2025-06-16T15:25:19.185Z
Learning: In Lightning invoice validation (src/lightning/invoice.rs), the expiration window check `if expires_at < latest_date` is correct business logic. It rejects invoices that expire too soon (within the configured window, typically 1 hour) to ensure sufficient time for payment processing. The window represents a minimum acceptable invoice lifetime, not a maximum.

Applied to files:

docs/DEV_FEE.md

🧬 Code graph analysis (1)

src/scheduler.rs (2)

src/app/release.rs (2)

do_payment (438-521)

send_dev_fee_payment (578-688)

src/db.rs (1)

find_unpaid_dev_fees (895-908)

🪛 markdownlint-cli2 (0.18.1)

docs/DEV_FEE.md

106-106: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

140-140: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

387-387: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

409-409: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

445-445: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

782-782: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

1247-1247: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

1277-1277: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: test

🔇 Additional comments (2)

src/scheduler.rs (2)

354-358: Excellent fix for stale dev_fee bug.

This correctly resets all three price-dependent fields (amount, fee, dev_fee) when a market price order times out and returns to pending status. This ensures the next taker will have fees recalculated at the current market price rather than inheriting stale values.

566-579: Verification query pattern is excellent for debugging.

The VERIFY logging pattern that re-queries the database after update is a solid approach for troubleshooting the persistence issues mentioned in the PR. This will help confirm whether database updates are actually being persisted or if there's a deeper issue with the ORM or connection pooling.

The pattern is:

Log BEFORE: Current in-memory state

Modify: Update in-memory fields

Log AFTER: Modified in-memory state

Update DB: Persist to database

Verify: Re-query to confirm persistence

This provides a complete audit trail for debugging.

coderabbitai

Actionable comments posted: 2

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 723e0f1 and c3d3ffb.

📒 Files selected for processing (1)

src/scheduler.rs

🧰 Additional context used

📓 Path-based instructions (2)

src/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Runtime code lives in src/ directory with subdirectories: src/app/ for order flows and business logic, src/lightning/ for LND bindings and Lightning helpers, src/rpc/ for gRPC service and types, and src/config/ for settings and loaders

Files:

src/scheduler.rs

**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Use 4-space indent, snake_case for functions, PascalCase for types, and SCREAMING_SNAKE_CASE for constants in Rust code (Rust 2021 edition)
Document non-obvious public APIs with triple-slash /// documentation comments
Prefer tracing spans over ad-hoc logging in Rust code
Co-locate tests in their modules under mod tests blocks and name tests descriptively

Files:

src/scheduler.rs

🧠 Learnings (2)

📓 Common learnings

Learnt from: grunch
Repo: MostroP2P/mostro PR: 554
File: docs/DEV_FEE_TECHNICAL_SPEC.md:1-434
Timestamp: 2025-12-17T13:04:13.036Z
Learning: In the Mostro development fee implementation, the Lightning address `mostro_p2psats.mobi` in src/config/constants.rs is temporary for testing purposes. The production address will be `developmentmostro.network` as documented in docs/DEV_FEE_TECHNICAL_SPEC.md. This intentional mismatch is planned to be resolved before production deployment.

📚 Learning: 2025-06-25T22:48:44.741Z

Learnt from: Catrya
Repo: MostroP2P/mostro PR: 502
File: src/app/release.rs:52-55
Timestamp: 2025-06-25T22:48:44.741Z
Learning: The PaymentFailedInfo struct used in src/app/release.rs is defined in mostro-core PR #111, creating a cross-repository dependency that causes compilation errors until the dependency is resolved.

Applied to files:

src/scheduler.rs

🧬 Code graph analysis (1)

src/scheduler.rs (2)

src/app/release.rs (2)

do_payment (438-521)

send_dev_fee_payment (578-688)

src/db.rs (1)

find_unpaid_dev_fees (895-908)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: test

🔇 Additional comments (3)

src/scheduler.rs (3)

1-1: LGTM: Import addition is correct.

The send_dev_fee_payment import is properly added and used at line 540 in the new scheduler job.

29-29: LGTM: Scheduler wiring follows established pattern.

The call to job_process_dev_fee_payment() is correctly integrated with other scheduler jobs.

549-591: Strong observability: Comprehensive logging and verification.

The BEFORE/AFTER/VERIFY logging pattern (lines 549-560, 577-591) provides excellent visibility into database state changes, which will be invaluable for troubleshooting the duplicate payment issue flagged in my previous comment. The verification query at lines 578-583 is particularly useful for detecting silent update failures.

src/scheduler.rs

Critical Issue: If send_dev_fee_payment() succeeds but order.update() fails, the payment is made but not recorded in the database. On the next scheduler cycle, LNURL generates a new invoice (cannot reuse invoices by design), leading to a duplicate payment that cannot be reversed. Root Cause: - No atomicity between Lightning payment and database update - LNURL always generates fresh invoices on each call - Scheduler relies solely on dev_fee_paid flag to determine if payment needed - LND idempotency only works for same payment_hash (doesn't help with new invoices) Solution - Three-Phase Mitigation: Phase 1 - Recovery Guard Clause: Detects orders with payment_hash but dev_fee_paid=false (partial success). Automatically recovers by marking as paid without attempting new payment. Phase 2 - Pre-Payment Marking: Before attempting payment, marks order with sentinel "PENDING-{order_id}" in dev_fee_payment_hash field and sets dev_fee_paid=true. This prevents duplicate attempts even if payment succeeds but hash update fails. Payment flow now: 1. Pre-mark: Set dev_fee_paid=true, hash="PENDING-{id}" → UPDATE database 2. Attempt payment (protected from retry since dev_fee_paid=true) 3. On success: Update hash with actual payment_hash 4. On failure: Reset dev_fee_paid=false, hash=NULL for retry 5. On timeout: Reset for retry Phase 3 - Query Exclusion: Updated find_unpaid_dev_fees() to exclude orders with PENDING sentinel, preventing race conditions if multiple scheduler instances run. Benefits: - Prevents duplicate payments (order marked before payment attempt) - Automatic recovery from partial failures - Enhanced logging for manual reconciliation if needed - No schema changes required Trade-offs: - 2-3 database updates per payment instead of 1 - Uses sentinel value "PENDING-{order_id}" as temporary marker - Orders stuck in paid state on double failure require manual intervention (logged as CRITICAL with ACTION REQUIRED)

coderabbitai

Actionable comments posted: 0

♻️ Duplicate comments (1)

src/scheduler.rs (1)

354-358: dev_fee should be reset unconditionally when order returns to Pending.

This issue was flagged in a previous review. The dev_fee reset is currently inside the if order.price_from_api block, but per Catrya's PR comment, all orders returning to Pending status should have dev_fee cleared—not just market-price orders.

🧹 Nitpick comments (3)

src/scheduler.rs (3)
536-572: Recovery guard implementation looks solid.

The recovery logic correctly handles the partial success scenario where a payment succeeded but the DB update failed. By checking for an existing dev_fee_payment_hash and marking the order as paid without re-attempting payment, this prevents duplicate payments for recovered orders.

Minor style suggestion: lines 538-541 could use if let for more idiomatic Rust:
🔎 Optional refactor
-                   if order.dev_fee_payment_hash.is_some() {
-                       let order_id = order.id;
-                       let payment_hash = order.dev_fee_payment_hash.as_ref().unwrap().clone();
+                   if let Some(payment_hash) = order.dev_fee_payment_hash.clone() {
+                       let order_id = order.id;
556-567: Verification queries add DB overhead in production.

The verification queries after DB updates are valuable for troubleshooting during development, but each successful payment incurs two extra SELECT queries (recovery verification + success verification). Consider gating these behind a debug/verbose configuration flag or removing them once the implementation is proven stable.

Also applies to: 625-639

672-698: Timeout handling: potential edge case with in-flight payments.

When the 50-second timeout triggers, the code resets dev_fee_paid = false and dev_fee_payment_hash = None. If the payment is still in-flight in LND and eventually succeeds, the next scheduler cycle could initiate a duplicate payment since:

The order will be picked up by find_unpaid_dev_fees() again

A new invoice will be generated via resolv_ln_address

A second payment could be sent

This is mitigated by:

The 50s outer timeout exceeding internal timeouts (15s + 5s + 25s = 45s)

Sequential processing reducing concurrency risks

For additional safety, consider checking LND payment history before attempting payment, or adding a retry counter to prevent infinite retries on persistently failing orders.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c3d3ffb and fa0759f.

📒 Files selected for processing (2)

src/db.rs
src/scheduler.rs

🚧 Files skipped from review as they are similar to previous changes (1)

src/db.rs

🧰 Additional context used

📓 Path-based instructions (2)

src/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Runtime code lives in src/ directory with subdirectories: src/app/ for order flows and business logic, src/lightning/ for LND bindings and Lightning helpers, src/rpc/ for gRPC service and types, and src/config/ for settings and loaders

Files:

src/scheduler.rs

**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Use 4-space indent, snake_case for functions, PascalCase for types, and SCREAMING_SNAKE_CASE for constants in Rust code (Rust 2021 edition)
Document non-obvious public APIs with triple-slash /// documentation comments
Prefer tracing spans over ad-hoc logging in Rust code
Co-locate tests in their modules under mod tests blocks and name tests descriptively

Files:

src/scheduler.rs

🧠 Learnings (3)

📓 Common learnings

Learnt from: grunch
Repo: MostroP2P/mostro PR: 554
File: docs/DEV_FEE_TECHNICAL_SPEC.md:1-434
Timestamp: 2025-12-17T13:04:13.036Z
Learning: In the Mostro development fee implementation, the Lightning address `mostro_p2psats.mobi` in src/config/constants.rs is temporary for testing purposes. The production address will be `developmentmostro.network` as documented in docs/DEV_FEE_TECHNICAL_SPEC.md. This intentional mismatch is planned to be resolved before production deployment.

📚 Learning: 2025-06-25T22:48:44.741Z

Learnt from: Catrya
Repo: MostroP2P/mostro PR: 502
File: src/app/release.rs:52-55
Timestamp: 2025-06-25T22:48:44.741Z
Learning: The PaymentFailedInfo struct used in src/app/release.rs is defined in mostro-core PR #111, creating a cross-repository dependency that causes compilation errors until the dependency is resolved.

Applied to files:

src/scheduler.rs

📚 Learning: 2025-07-17T13:51:17.338Z

Learnt from: arkanoider
Repo: MostroP2P/mostro PR: 499
File: src/lightning/invoice.rs:108-114
Timestamp: 2025-07-17T13:51:17.338Z
Learning: In Lightning invoice validation for Mostro buy orders (src/lightning/invoice.rs), zero-amount invoices (amount_sat == 0) should always pass validation even when an expected amount is provided. This is because users can submit invoices at order creation time before knowing the exact satoshi amount they'll receive. The condition `if amount_sat != res && amount_sat != 0` correctly implements this by allowing zero-amount invoices to pass while enforcing exact amount matching for non-zero invoices.

Applied to files:

src/scheduler.rs

🧬 Code graph analysis (1)

src/scheduler.rs (3)

src/app/release.rs (1)

send_dev_fee_payment (578-688)

src/config/settings.rs (1)

get_db_pool (31-33)

src/db.rs (1)

find_unpaid_dev_fees (895-911)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: build

🔇 Additional comments (5)

src/scheduler.rs (5)

1-1: LGTM!

The new imports for send_dev_fee_payment and do_payment are correctly scoped and both are used in the file (do_payment at line 191, send_dev_fee_payment in the new dev fee job).

29-29: LGTM!

The new dev fee processing job is correctly wired into the scheduler, following the established pattern of other background jobs.

512-524: Good documentation of design decisions.

The triple-slash documentation clearly explains the scheduling interval, timeout rationale (50s with 10s buffer), and processing strategy. This aids maintainability and aligns with the coding guidelines for documenting non-obvious functionality.

574-589: Pre-payment marking strategy is well-designed.

Setting dev_fee_paid = true with a PENDING-{order_id} sentinel before attempting payment, combined with the query exclusion in find_unpaid_dev_fees(), effectively prevents race conditions across scheduler instances. The approach correctly balances atomicity constraints with the need to prevent duplicate payments.

598-641: Payment success path handles edge cases appropriately.

The critical error logging when DB update fails after successful payment (lines 609-616) provides actionable information for manual reconciliation. Since dev_fee_paid remains true from the pre-marking step, duplicate payments are prevented even if the hash update fails.

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/scheduler.rs (1)

513-527: Good documentation and design rationale.

The doc comment clearly explains the function's purpose, timing, and design decisions. The 50-second timeout with 10-second buffer is well-reasoned for sequential processing.

Consider making the interval configurable.

The 60-second interval is hardcoded at line 527, while other scheduler jobs use settings from configuration (e.g., ln_settings.payment_retries_interval). Consider adding this to the settings for operational flexibility.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fa0759f and 1d091c1.

📒 Files selected for processing (1)

src/scheduler.rs

🧰 Additional context used

📓 Path-based instructions (2)

src/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Runtime code lives in src/ directory with subdirectories: src/app/ for order flows and business logic, src/lightning/ for LND bindings and Lightning helpers, src/rpc/ for gRPC service and types, and src/config/ for settings and loaders

Files:

src/scheduler.rs

**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Use 4-space indent, snake_case for functions, PascalCase for types, and SCREAMING_SNAKE_CASE for constants in Rust code (Rust 2021 edition)
Document non-obvious public APIs with triple-slash /// documentation comments
Prefer tracing spans over ad-hoc logging in Rust code
Co-locate tests in their modules under mod tests blocks and name tests descriptively

Files:

src/scheduler.rs

🧠 Learnings (3)

📓 Common learnings

Learnt from: grunch
Repo: MostroP2P/mostro PR: 554
File: docs/DEV_FEE_TECHNICAL_SPEC.md:1-434
Timestamp: 2025-12-17T13:04:13.036Z
Learning: In the Mostro development fee implementation, the Lightning address `mostro_p2psats.mobi` in src/config/constants.rs is temporary for testing purposes. The production address will be `developmentmostro.network` as documented in docs/DEV_FEE_TECHNICAL_SPEC.md. This intentional mismatch is planned to be resolved before production deployment.

📚 Learning: 2025-06-25T22:48:44.741Z

Learnt from: Catrya
Repo: MostroP2P/mostro PR: 502
File: src/app/release.rs:52-55
Timestamp: 2025-06-25T22:48:44.741Z
Learning: The PaymentFailedInfo struct used in src/app/release.rs is defined in mostro-core PR #111, creating a cross-repository dependency that causes compilation errors until the dependency is resolved.

Applied to files:

src/scheduler.rs

📚 Learning: 2025-07-17T13:51:17.338Z

Learnt from: arkanoider
Repo: MostroP2P/mostro PR: 499
File: src/lightning/invoice.rs:108-114
Timestamp: 2025-07-17T13:51:17.338Z
Learning: In Lightning invoice validation for Mostro buy orders (src/lightning/invoice.rs), zero-amount invoices (amount_sat == 0) should always pass validation even when an expected amount is provided. This is because users can submit invoices at order creation time before knowing the exact satoshi amount they'll receive. The condition `if amount_sat != res && amount_sat != 0` correctly implements this by allowing zero-amount invoices to pass while enforcing exact amount matching for non-zero invoices.

Applied to files:

src/scheduler.rs

🧬 Code graph analysis (1)

src/scheduler.rs (3)

src/app/release.rs (2)

do_payment (438-521)

send_dev_fee_payment (578-688)

src/config/settings.rs (1)

get_db_pool (31-33)

src/db.rs (1)

find_unpaid_dev_fees (895-911)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: test

🔇 Additional comments (7)

src/scheduler.rs (7)

1-1: LGTM!

Import correctly brings in do_payment (used in job_retry_failed_payments) and send_dev_fee_payment (used in the new dev fee payment job).

19-19: LGTM!

The warn macro is appropriately added and used for the recovery scenario logging.

29-29: LGTM!

The dev fee payment job is appropriately registered in the scheduler startup sequence.

352-359: Good fix: dev_fee now reset unconditionally.

This correctly addresses the bug reported by Catrya. Moving order.dev_fee = 0 outside the price_from_api conditional ensures dev_fee is cleared for all orders returning to Pending status, not just market-price orders.

538-573: Recovery logic correctly handles partial success scenarios.

The guard at line 539 properly detects orders where payment succeeded but the DB update failed (order has a real payment hash but dev_fee_paid=false). Since find_unpaid_dev_fees excludes PENDING- prefixed hashes, this recovery path only triggers for genuinely completed payments.

592-642: Payment flow with timeout and hash update looks correct.

The 50-second timeout wrapping send_dev_fee_payment is appropriate. On success, the actual payment hash replaces the PENDING- placeholder, and the verification query provides good observability for troubleshooting.

Note: The verification queries (lines 627-639) use raw SQL instead of the ORM, which is acceptable for read-only logging purposes.

644-700: Failure and timeout handling correctly resets state for retry.

Both error paths (payment failure at lines 644-671 and timeout at lines 673-698) properly reset dev_fee_paid=false and dev_fee_payment_hash=None, allowing the order to be picked up in the next cycle.

The nested error handling for DB update failures provides good visibility into critical edge cases requiring manual intervention.

src/scheduler.rs

Problem: Orders marked with dev_fee_payment_hash="PENDING-{id}" can become stuck if the server crashes after pre-marking but before completing payment. These orders are excluded from normal processing and remain unpaid indefinitely. Solution: Add inline cleanup logic in job_process_dev_fee_payment() that runs before processing unpaid orders. The cleanup: - Finds orders with PENDING markers older than 5 minutes (300s) - Uses taken_at timestamp to calculate age - Resets dev_fee_paid=false and dev_fee_payment_hash=NULL - Logs recovery actions for monitoring Implementation: - Runs on same 60s cycle as dev fee payment job - 5-minute TTL is conservative (normal payment max is 50s) - Inline approach is simpler than separate scheduler job - Comprehensive logging for debugging and monitoring Benefits: - Automatic crash recovery - No manual intervention needed - Minimal performance overhead - Simple implementation following existing patterns

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/scheduler.rs (1)

584-753: Well-structured payment processing with comprehensive error handling.

The main dev fee payment logic is well-implemented with:

Recovery guard for partial success scenarios (lines 588-624)

Pre-marking to prevent duplicates (lines 626-641)

Proper timeout handling (50s with 60s cycle)

Comprehensive logging at critical points

Reset-on-failure for automatic retry

The sequential processing design is appropriate for preventing scheduler overload.

Optional: Consider adding observability metrics.

For production monitoring, consider adding:

Counter for successful/failed payments

Histogram for payment duration

Counter for stale cleanups

Alert threshold for repeated failures

This would help detect issues proactively without relying solely on logs.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1d091c1 and f82630b.

📒 Files selected for processing (1)

src/scheduler.rs

🧰 Additional context used

📓 Path-based instructions (2)

src/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Runtime code lives in src/ directory with subdirectories: src/app/ for order flows and business logic, src/lightning/ for LND bindings and Lightning helpers, src/rpc/ for gRPC service and types, and src/config/ for settings and loaders

Files:

src/scheduler.rs

**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Use 4-space indent, snake_case for functions, PascalCase for types, and SCREAMING_SNAKE_CASE for constants in Rust code (Rust 2021 edition)
Document non-obvious public APIs with triple-slash /// documentation comments
Prefer tracing spans over ad-hoc logging in Rust code
Co-locate tests in their modules under mod tests blocks and name tests descriptively

Files:

src/scheduler.rs

🧠 Learnings (3)

📓 Common learnings

Learnt from: grunch
Repo: MostroP2P/mostro PR: 554
File: docs/DEV_FEE_TECHNICAL_SPEC.md:1-434
Timestamp: 2025-12-17T13:04:13.036Z
Learning: In the Mostro development fee implementation, the Lightning address `mostro_p2psats.mobi` in src/config/constants.rs is temporary for testing purposes. The production address will be `developmentmostro.network` as documented in docs/DEV_FEE_TECHNICAL_SPEC.md. This intentional mismatch is planned to be resolved before production deployment.

📚 Learning: 2025-06-25T22:48:44.741Z

Learnt from: Catrya
Repo: MostroP2P/mostro PR: 502
File: src/app/release.rs:52-55
Timestamp: 2025-06-25T22:48:44.741Z
Learning: The PaymentFailedInfo struct used in src/app/release.rs is defined in mostro-core PR #111, creating a cross-repository dependency that causes compilation errors until the dependency is resolved.

Applied to files:

src/scheduler.rs

📚 Learning: 2025-07-17T13:51:17.338Z

Learnt from: arkanoider
Repo: MostroP2P/mostro PR: 499
File: src/lightning/invoice.rs:108-114
Timestamp: 2025-07-17T13:51:17.338Z
Learning: In Lightning invoice validation for Mostro buy orders (src/lightning/invoice.rs), zero-amount invoices (amount_sat == 0) should always pass validation even when an expected amount is provided. This is because users can submit invoices at order creation time before knowing the exact satoshi amount they'll receive. The condition `if amount_sat != res && amount_sat != 0` correctly implements this by allowing zero-amount invoices to pass while enforcing exact amount matching for non-zero invoices.

Applied to files:

src/scheduler.rs

🧬 Code graph analysis (1)

src/scheduler.rs (3)

src/app/release.rs (1)

do_payment (438-521)

src/config/settings.rs (1)

get_db_pool (31-33)

src/db.rs (1)

find_unpaid_dev_fees (895-911)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: build

🔇 Additional comments (3)

src/scheduler.rs (3)

1-1: LGTM: Import additions support new dev fee payment job.

The new imports for send_dev_fee_payment, do_payment, and warn are properly used in the job_process_dev_fee_payment() function.

Also applies to: 19-19

29-29: LGTM: Dev fee payment job properly wired into scheduler.

The new job is correctly integrated into the scheduler initialization sequence.

352-353: LGTM: dev_fee now reset unconditionally.

This correctly addresses the bug reported by Catrya where dev_fee was not cleared when orders returned to Pending status. The unconditional reset ensures all order types are handled consistently.

src/scheduler.rs

1. Implementation Status section (lines 69-77): Brief overview with status ⚠️ TO IMPLEMENT 2. Full Phase 4 section (lines 1112-1247): Complete specification with event structure, rationale, query examples, and implementation roadmap The Implementation Status section now shows all four phases: - ✅ Phase 1: Infrastructure (COMPLETE) - ✅ Phase 2: Fee Calculation (COMPLETE) - ✅ Phase 3: Payment Execution (COMPLETE) - ⚠️ Phase 4: Audit Events via Nostr (TO IMPLEMENT)

Fixed incorrect timestamp parameter binding in SQL queries where timestamps were being bound as strings instead of integers. This caused SQLite to perform lexicographic (alphabetical) comparison instead of proper numeric comparison on INTEGER columns. Changes: - src/scheduler.rs: Fixed cutoff_time binding in stale PENDING cleanup query (line 543) - src/db.rs: Fixed expire_time binding in find_order_by_date() function (line 730) - src/db.rs: Fixed expire_time binding in find_order_by_seconds() function (line 749) - src/db.rs: Fixed created_at binding in add_new_user() function (line 992) All timestamp bindings now use .as_u64() as i64 to match the INTEGER column types in the database schema, ensuring correct timestamp comparisons. Impact: - Proper cleanup of stale dev fee payment entries - Accurate order expiration detection - Correct user creation timestamps - Consistent timestamp handling across the codebase

Catrya · 2025-12-29T16:32:42Z

Aunque regrese a status Pending, sigue sin restablecerse en 0 el dev_fee en 0. He probado con ordenes a precio de mercado

The update_order_to_initial_state() function was not including dev_fee in its SQL UPDATE statement, causing stale dev_fee values to remain in the database after order timeouts or cancellations. This resulted in incorrect dev_fee charges when market price orders were re-taken, as the database would return the old dev_fee value instead of 0. Changes: - Add dev_fee parameter to update_order_to_initial_state() - Include dev_fee in SQL UPDATE statement - Update all callers in cancel.rs and scheduler.rs - Document the fix in DEV_FEE.md

Previously, dev_fee was calculated at different times depending on order type: - Fixed price orders: calculated at creation time in prepare_new_order() - Market price orders: calculated at take time in take_buy/take_sell This inconsistency made the code harder to maintain and reason about. Changes: - Set dev_fee = 0 for ALL orders at creation time (util.rs) - Calculate dev_fee when order is taken for both fixed and market price orders - Add dev_fee calculation in else block for fixed price orders (take_buy.rs, take_sell.rs) - Update documentation to reflect unified behavior Benefits: - Consistent behavior across all order types - Single point of calculation makes code easier to understand - All pending orders have dev_fee = 0, all taken orders have dev_fee > 0 - Simplifies logic and reduces code duplication Also fixes syntax error in db.rs (extra comma on line 813)

coderabbitai

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/util.rs (1)

385-399: Fix formatting in src/util.rs to satisfy cargo fmt.

CI is currently failing with cargo fmt --all -- --check on this file. Please run cargo fmt locally so the updated comments around prepare_new_order and the dev-fee tests are formatted according to the project’s Rust style, then push the formatted version.

Also applies to: 1453-1485

♻️ Duplicate comments (2)

src/app/cancel.rs (1)

18-25: Market-price dev_fee reset on taker cancel is fixed; consider extending to all resets.

reset_api_quotes now clears dev_fee (along with amount and fee) for price_from_api orders, and cancel_order_by_taker passes the updated order.dev_fee into update_order_to_initial_state, so the “stale dev_fee on republished market order” bug is addressed.

If you want to maintain the invariant described in DEV_FEE.md that Pending orders always have dev_fee = 0, you may also want to clear dev_fee for fixed-price orders when they’re reset to the initial Pending state (either by moving order.dev_fee = 0 outside the if order.price_from_api or by passing a literal 0 for dev_fee when calling update_order_to_initial_state in this path).

Also applies to: 193-199

docs/DEV_FEE.md (1)

265-279: Update docs to match current dev_fee lifecycle and fix fenced-code lint warnings.

Two things worth tightening up here:

The “dev_fee Field Lifecycle” snippet still shows prepare_new_order() computing dev_fee at creation time, but the actual implementation now initializes dev_fee = 0 and defers calculation to take_buy/take_sell. The surrounding text earlier in the doc reflects the new behavior; this section should be updated to avoid conflicting guidance.

Several fenced blocks (e.g., the architecture timeline around line 150, the market-order examples around 398/417/462, and the long lifecycle timeline around 819) are missing a language identifier and are flagged by markdownlint. Add an explicit language such as rust, sql, or text/plaintext as appropriate to satisfy MD040.

Adjusting those will keep the spec aligned with the code and keep the docs pipeline clean.

Also applies to: 735-749, 150-158, 398-417, 462-475, 819-887

🧹 Nitpick comments (1)

src/scheduler.rs (1)

514-759: Dev-fee payment scheduler is robust; keep an eye on throughput as volume grows.

The new job_process_dev_fee_payment job:

Cleans up stale PENDING-* markers,

Uses find_unpaid_dev_fees to avoid double processing,

Pre-marks rows before attempting payment,

Wraps send_dev_fee_payment in a 50s timeout, and

Logs/updates/verification-queries around each DB write.

This looks solid for current expected volumes. If dev-fee traffic increases substantially in the future, consider adding a concurrency limit (e.g. small buffered channel + select!) or batching to avoid one slow payment stalling the entire loop for up to 50 seconds per order.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c803471 and 2349669.

📒 Files selected for processing (8)

docs/DEV_FEE.md
sqlx-data.json
src/app/cancel.rs
src/app/take_buy.rs
src/app/take_sell.rs
src/db.rs
src/scheduler.rs
src/util.rs

🚧 Files skipped from review as they are similar to previous changes (1)

src/app/take_buy.rs

🧰 Additional context used

📓 Path-based instructions (4)

src/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

Runtime code lives in src/ directory with subdirectories: src/app/ for order flows and business logic, src/lightning/ for LND bindings and Lightning helpers, src/rpc/ for gRPC service and types, and src/config/ for settings and loaders

Files:

src/util.rs
src/app/cancel.rs
src/scheduler.rs
src/app/take_sell.rs
src/db.rs

**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Use 4-space indent, snake_case for functions, PascalCase for types, and SCREAMING_SNAKE_CASE for constants in Rust code (Rust 2021 edition)
Document non-obvious public APIs with triple-slash /// documentation comments
Prefer tracing spans over ad-hoc logging in Rust code
Co-locate tests in their modules under mod tests blocks and name tests descriptively

Files:

src/util.rs
src/app/cancel.rs
src/scheduler.rs
src/app/take_sell.rs
src/db.rs

src/app/**

📄 CodeRabbit inference engine (AGENTS.md)

Mirror test fixtures under src/app/ directory where applicable

Files:

src/app/cancel.rs
src/app/take_sell.rs

sqlx-data.json

📄 CodeRabbit inference engine (AGENTS.md)

SQLx offline metadata must be maintained in sqlx-data.json file

Files:

sqlx-data.json

🧠 Learnings (12)

📓 Common learnings

Learnt from: grunch
Repo: MostroP2P/mostro PR: 554
File: docs/DEV_FEE_TECHNICAL_SPEC.md:1-434
Timestamp: 2025-12-17T13:04:13.036Z
Learning: In the Mostro development fee implementation, the Lightning address `mostro_p2psats.mobi` in src/config/constants.rs is temporary for testing purposes. The production address will be `developmentmostro.network` as documented in docs/DEV_FEE_TECHNICAL_SPEC.md. This intentional mismatch is planned to be resolved before production deployment.

📚 Learning: 2025-07-17T13:51:17.338Z

Learnt from: arkanoider
Repo: MostroP2P/mostro PR: 499
File: src/lightning/invoice.rs:108-114
Timestamp: 2025-07-17T13:51:17.338Z
Learning: In Lightning invoice validation for Mostro buy orders (src/lightning/invoice.rs), zero-amount invoices (amount_sat == 0) should always pass validation even when an expected amount is provided. This is because users can submit invoices at order creation time before knowing the exact satoshi amount they'll receive. The condition `if amount_sat != res && amount_sat != 0` correctly implements this by allowing zero-amount invoices to pass while enforcing exact amount matching for non-zero invoices.

Applied to files:

src/util.rs
src/scheduler.rs
src/app/take_sell.rs
src/db.rs
docs/DEV_FEE.md

📚 Learning: 2025-12-17T13:04:13.036Z

Learnt from: grunch
Repo: MostroP2P/mostro PR: 554
File: docs/DEV_FEE_TECHNICAL_SPEC.md:1-434
Timestamp: 2025-12-17T13:04:13.036Z
Learning: In the Mostro development fee implementation, the Lightning address `mostro_p2psats.mobi` in src/config/constants.rs is temporary for testing purposes. The production address will be `developmentmostro.network` as documented in docs/DEV_FEE_TECHNICAL_SPEC.md. This intentional mismatch is planned to be resolved before production deployment.

Applied to files:

src/util.rs
src/app/take_sell.rs
src/db.rs
docs/DEV_FEE.md

📚 Learning: 2025-04-27T20:07:24.558Z

Learnt from: arkanoider
Repo: MostroP2P/mostro PR: 475
File: src/db.rs:792-802
Timestamp: 2025-04-27T20:07:24.558Z
Learning: The decrypt_data function from mostro_core::order returns a type that needs to be wrapped with MostroInternalErr to convert it to MostroError, so calls should keep the .map_err(MostroInternalErr) conversion.

Applied to files:

src/util.rs
src/app/cancel.rs
src/app/take_sell.rs
src/db.rs

📚 Learning: 2025-03-11T23:26:33.763Z

Learnt from: bilthon
Repo: MostroP2P/mostro PR: 464
File: src/app/add_invoice.rs:73-82
Timestamp: 2025-03-11T23:26:33.763Z
Learning: In the Mostro codebase, the `update_order_event` function does not perform database operations - it only updates an order in memory and sends a Nostr event. The actual database update happens separately when calling `.update(pool)` afterward.

Applied to files:

src/app/cancel.rs
src/app/take_sell.rs
src/db.rs

📚 Learning: 2025-06-25T22:48:44.741Z

Learnt from: Catrya
Repo: MostroP2P/mostro PR: 502
File: src/app/release.rs:52-55
Timestamp: 2025-06-25T22:48:44.741Z
Learning: The PaymentFailedInfo struct used in src/app/release.rs is defined in mostro-core PR #111, creating a cross-repository dependency that causes compilation errors until the dependency is resolved.

Applied to files:

src/scheduler.rs

📚 Learning: 2025-11-06T22:54:07.055Z

Learnt from: arkanoider
Repo: MostroP2P/mostro PR: 545
File: src/app.rs:182-186
Timestamp: 2025-11-06T22:54:07.055Z
Learning: In the Mostro codebase, as of PR #545, the code in check_trade_index() prevents new regular users from being created with last_trade_index = 0. Users must provide a trade_index >= 1 when creating or taking their first order. The guard at lines 184-186 in src/app.rs rejects trade_index == 0 before user creation.

Applied to files:

src/app/take_sell.rs

📚 Learning: 2025-11-06T23:01:06.496Z

Learnt from: arkanoider
Repo: MostroP2P/mostro PR: 545
File: src/app/last_trade_index.rs:67-70
Timestamp: 2025-11-06T23:01:06.496Z
Learning: In the Mostro codebase, users are only created through the check_trade_index() function when they create or take their first order. Since check_trade_index() validates and rejects trade_index = 0 (using pattern matching at line 134 in src/app.rs with `index @ 1..`), no user can ever be created with last_trade_index = 0, even though the database migration shows DEFAULT 0. Therefore, the guard in last_trade_index.rs that returns InvalidTradeIndex when last_trade_index == 0 is safe and will only catch invalid database states.

Applied to files:

src/app/take_sell.rs

📚 Learning: 2025-10-09T14:06:50.067Z

Learnt from: arkanoider
Repo: MostroP2P/mostro PR: 530
File: src/app/order.rs:278-279
Timestamp: 2025-10-09T14:06:50.067Z
Learning: In the Mostro project, tests in files like `src/app/order.rs` may use structural checks to ensure functions like `order_action` do not panic during message construction, rather than validating specific error variants.

Applied to files:

src/app/take_sell.rs

📚 Learning: 2025-06-25T22:48:44.741Z

Learnt from: Catrya
Repo: MostroP2P/mostro PR: 502
File: src/app/release.rs:52-55
Timestamp: 2025-06-25T22:48:44.741Z
Learning: PaymentFailedInfo struct and Payload::PaymentFailed variant are defined in mostro-core PR #111, creating a cross-repository dependency with the Mostro repository that requires careful coordination of merging and version updates.

Applied to files:

src/app/take_sell.rs
src/db.rs

📚 Learning: 2025-06-16T23:17:54.795Z

Learnt from: Catrya
Repo: MostroP2P/mostro PR: 501
File: src/util.rs:0-0
Timestamp: 2025-06-16T23:17:54.795Z
Learning: The `get_db_pool()` function in the codebase returns `Arc<SqlitePool>`, not `&SqlitePool`. When calling functions that expect `&SqlitePool`, you need to use `&pool` to convert `Arc<SqlitePool>` to `&SqlitePool` through automatic dereferencing.

Applied to files:

src/db.rs

📚 Learning: 2025-06-16T15:25:19.185Z

Learnt from: arkanoider
Repo: MostroP2P/mostro PR: 499
File: src/lightning/invoice.rs:130-145
Timestamp: 2025-06-16T15:25:19.185Z
Learning: In Lightning invoice validation (src/lightning/invoice.rs), the expiration window check `if expires_at < latest_date` is correct business logic. It rejects invoices that expire too soon (within the configured window, typically 1 hour) to ensure sufficient time for payment processing. The window represents a minimum acceptable invoice lifetime, not a maximum.

Applied to files:

docs/DEV_FEE.md

🧬 Code graph analysis (3)

src/app/cancel.rs (1)

src/db.rs (1)

update_order_to_initial_state (776-820)

src/scheduler.rs (3)

src/app/release.rs (2)

do_payment (438-521)

send_dev_fee_payment (578-688)

src/config/settings.rs (1)

get_db_pool (31-33)

src/db.rs (1)

find_unpaid_dev_fees (898-914)

src/app/take_sell.rs (1)

src/util.rs (8)

get_dev_fee (176-179)

get_fiat_amount_requested (982-999)

get_market_amount_and_fee (796-806)

get_order (1036-1049)

set_waiting_invoice_status (809-868)

show_hold_invoice (661-744)

update_order_event (595-632)

validate_invoice (1111-1139)

🪛 GitHub Actions: CI

src/util.rs

[error] 395-395: cargo fmt --all -- --check failed: formatting mismatch detected in src/util.rs. Run 'cargo fmt' to fix formatting.

🪛 LanguageTool

docs/DEV_FEE.md

[grammar] ~265-~265: Use a hyphen to join words.
Context: ...en, unifying the behavior for both fixed price and market price orders. Database ...

(QB_NEW_EN_HYPHEN)

[grammar] ~265-~265: Use a hyphen to join words.
Context: ...behavior for both fixed price and market price orders. Database fields `dev_fee_p...

(QB_NEW_EN_HYPHEN)

[grammar] ~270-~270: Use a hyphen to join words.
Context: ... fee: fee = get_fee(amount) (for fixed price orders where amount > 0) 2. Set `d...

(QB_NEW_EN_HYPHEN)

[grammar] ~271-~271: Use a hyphen to join words.
Context: ...dev_fee = 0 for ALL orders (both fixed price and market price) 3. Store in Orde...

(QB_NEW_EN_HYPHEN)

[grammar] ~271-~271: Use a hyphen to join words.
Context: ... ALL orders (both fixed price and market price) 3. Store in Order struct with `de...

(QB_NEW_EN_HYPHEN)

[style] ~494-~494: ‘Prior to’ might be wordy. Consider a shorter alternative.
Context: ... values Database Persistence Fix: Prior to commit c803471, the `update_order_to_...

(EN_WORDINESS_PREMIUM_PRIOR_TO)

🪛 markdownlint-cli2 (0.18.1)

docs/DEV_FEE.md

116-116: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

150-150: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

398-398: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

417-417: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

462-462: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

819-819: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

1401-1401: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

🔇 Additional comments (5)

src/db.rs (2)

721-755: Switching to numeric timestamp storage/binding looks correct.

Using expire_time.as_u64() as i64 in find_order_by_date/find_order_by_seconds and created_at.as_u64() as i64 in add_new_user aligns the bindings with integer created_at/expires_at/taken_at columns and avoids implicit SQLite string→integer casts. No functional issues spotted here.

Also applies to: 975-999

776-820: Dev-fee persistence query updates are consistent with the new workflow.

Extending update_order_to_initial_state with a dev_fee: i64 parameter and wiring it into the UPDATE orders statement, plus adding find_unpaid_dev_fees with the status IN ('settled-hold-invoice','success') AND dev_fee > 0 AND dev_fee_paid = 0 AND (dev_fee_payment_hash IS NULL OR dev_fee_payment_hash NOT LIKE 'PENDING-%') filter, matches the new scheduler flow and fixes the stale dev_fee-on-reset bug. Callers now pass the intended dev_fee value, and sqlx-data.json has been updated accordingly.

Also applies to: 898-914

sqlx-data.json (1)

13-22: sqlx metadata now matches the updated orders UPDATE statement.

The updated entry reflects 10 bind parameters and includes dev_fee in the SET list, matching update_order_to_initial_state in src/db.rs. This keeps SQLx offline validation in sync with the runtime query.

src/util.rs (1)

156-179: Dev-fee helpers and split logic are consistent across the flow.

calculate_dev_fee/get_dev_fee provide a clean separation between pure calculation and settings-based lookup; prepare_new_order keeping dev_fee = 0 and deferring calculation to take_* actions matches the documented lifecycle; and the updated show_hold_invoice, set_waiting_invoice_status, and validate_invoice all use the same seller/buyer split (seller_dev_fee = dev_fee / 2, buyer_dev_fee = dev_fee - seller_dev_fee) with saturating arithmetic. This looks internally consistent and matches the design described in DEV_FEE.md.

Also applies to: 385-414, 661-744, 808-868, 1111-1139

src/scheduler.rs (1)

352-385: Timeout republish now correctly clears and persists dev_fee.

Unconditionally setting order.dev_fee = 0 before the timeout handling and then passing order.dev_fee into update_order_to_initial_state ensures that timed-out orders reset both in-memory and in the database, fixing the previously reported stale dev_fee when orders return to Pending. The find_unpaid_dev_fees filter also prevents these reset orders from entering the dev-fee payment loop.

src/app/take_sell.rs

Invoice validation in take_sell_action was using dev_fee = 0 because dev_fee calculation happened AFTER validate_invoice was called. This caused incorrect validation of buyer invoices - the validation only checked for Mostro fee but not dev_fee, allowing buyers to submit invoices with amounts that were too high. The validate_invoice function calculates: buyer_dev_fee = order.dev_fee - (order.dev_fee / 2) total_buyer_fees = order.fee + buyer_dev_fee With dev_fee = 0, this resulted in: buyer_dev_fee = 0 total_buyer_fees = order.fee (missing dev_fee!) Changes: - Move fiat_amount_requested calculation before dev_fee (needed for market orders) - Move dev_fee calculation block before validate_invoice call - Now validate_invoice receives order with correct dev_fee value - Invoice validation properly checks: amount - (fee + buyer_dev_fee) This ensures buyer invoices are validated with the correct total fees including both Mostro fee and development fee. Note: take_buy.rs is not affected as buy orders don't call validate_invoice (seller pays hold invoice which is generated after dev_fee calculation)

Updated documentation to reflect: Phase 3 Status: - Marked as fully complete with unified calculation logic - Updated database field documentation (dev_fee always 0 at creation) - Added two new implementation commits to the list New Implementation Notes: - Commit 2349669: Unified dev_fee calculation - Documents the problem, solution, and benefits - Explains how all orders now calculate dev_fee at take time - Lists all modified files - Commit 7e9b0a0: Invoice validation fix - Documents the critical bug and its impact - Shows before/after validation amounts - Explains the reordering of operations in take_sell.rs - Notes that take_buy.rs was not affected These changes ensure the documentation accurately reflects: - Unified dev_fee calculation behavior - Invoice validation correctness - Complete implementation timeline of Phase 3

coderabbitai

Actionable comments posted: 8

🧹 Nitpick comments (1)

docs/DEV_FEE.md (1)
535-535: Use concise phrasing instead of "Prior to."

The phrase "Prior to commit" is wordy; consider "Before commit" or restructure the sentence.
🔎 Proposed fix
-Prior to commit `c803471`, the `update_order_to_initial_state()` function in `src/db.rs`
+Before commit `c803471`, the `update_order_to_initial_state()` function in `src/db.rs`

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7e9b0a0 and e5c6222.

📒 Files selected for processing (1)

docs/DEV_FEE.md

🧰 Additional context used

🧠 Learnings (4)

📓 Common learnings

Learnt from: grunch
Repo: MostroP2P/mostro PR: 554
File: docs/DEV_FEE_TECHNICAL_SPEC.md:1-434
Timestamp: 2025-12-17T13:04:13.036Z
Learning: In the Mostro development fee implementation, the Lightning address `mostro_p2psats.mobi` in src/config/constants.rs is temporary for testing purposes. The production address will be `developmentmostro.network` as documented in docs/DEV_FEE_TECHNICAL_SPEC.md. This intentional mismatch is planned to be resolved before production deployment.

📚 Learning: 2025-12-17T13:04:13.036Z

Learnt from: grunch
Repo: MostroP2P/mostro PR: 554
File: docs/DEV_FEE_TECHNICAL_SPEC.md:1-434
Timestamp: 2025-12-17T13:04:13.036Z
Learning: In the Mostro development fee implementation, the Lightning address `mostro_p2psats.mobi` in src/config/constants.rs is temporary for testing purposes. The production address will be `developmentmostro.network` as documented in docs/DEV_FEE_TECHNICAL_SPEC.md. This intentional mismatch is planned to be resolved before production deployment.

Applied to files:

docs/DEV_FEE.md

📚 Learning: 2025-07-17T13:51:17.338Z

Learnt from: arkanoider
Repo: MostroP2P/mostro PR: 499
File: src/lightning/invoice.rs:108-114
Timestamp: 2025-07-17T13:51:17.338Z
Learning: In Lightning invoice validation for Mostro buy orders (src/lightning/invoice.rs), zero-amount invoices (amount_sat == 0) should always pass validation even when an expected amount is provided. This is because users can submit invoices at order creation time before knowing the exact satoshi amount they'll receive. The condition `if amount_sat != res && amount_sat != 0` correctly implements this by allowing zero-amount invoices to pass while enforcing exact amount matching for non-zero invoices.

Applied to files:

docs/DEV_FEE.md

📚 Learning: 2025-06-16T15:25:19.185Z

Learnt from: arkanoider
Repo: MostroP2P/mostro PR: 499
File: src/lightning/invoice.rs:130-145
Timestamp: 2025-06-16T15:25:19.185Z
Learning: In Lightning invoice validation (src/lightning/invoice.rs), the expiration window check `if expires_at < latest_date` is correct business logic. It rejects invoices that expire too soon (within the configured window, typically 1 hour) to ensure sufficient time for payment processing. The window represents a minimum acceptable invoice lifetime, not a maximum.

Applied to files:

docs/DEV_FEE.md

🪛 LanguageTool

docs/DEV_FEE.md

[grammar] ~306-~306: Use a hyphen to join words.
Context: ...en, unifying the behavior for both fixed price and market price orders. Database ...

(QB_NEW_EN_HYPHEN)

[grammar] ~306-~306: Use a hyphen to join words.
Context: ...behavior for both fixed price and market price orders. Database fields `dev_fee_p...

(QB_NEW_EN_HYPHEN)

[grammar] ~311-~311: Use a hyphen to join words.
Context: ... fee: fee = get_fee(amount) (for fixed price orders where amount > 0) 2. Set `d...

(QB_NEW_EN_HYPHEN)

[grammar] ~312-~312: Use a hyphen to join words.
Context: ...dev_fee = 0 for ALL orders (both fixed price and market price) 3. Store in Orde...

(QB_NEW_EN_HYPHEN)

[grammar] ~312-~312: Use a hyphen to join words.
Context: ... ALL orders (both fixed price and market price) 3. Store in Order struct with `de...

(QB_NEW_EN_HYPHEN)

[style] ~535-~535: ‘Prior to’ might be wordy. Consider a shorter alternative.
Context: ... values Database Persistence Fix: Prior to commit c803471, the `update_order_to_...

(EN_WORDINESS_PREMIUM_PRIOR_TO)

🪛 markdownlint-cli2 (0.18.1)

docs/DEV_FEE.md

119-119: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

191-191: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

439-439: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

458-458: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

503-503: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

860-860: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

1442-1442: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: test

🔇 Additional comments (1)

docs/DEV_FEE.md (1)

43-71: Phase 3 completion documentation is thorough and aligns with reported bug fix.

The detailed explanation of the dev_fee reset bug (lines 1472–1516) and the fix applied in commit c803471 clearly addresses the issue reported by reviewer Catrya. The documentation correctly explains:

The bug: stale dev_fee values remaining in the database when orders return to pending

The fix: update_order_to_initial_state() now persists dev_fee = 0

Verification: SQL query to confirm no pending market orders have dev_fee != 0

The document provides sufficient operational context for verifying the fix in the codebase.

coderabbitai · 2025-12-30T15:51:06Z

docs/DEV_FEE.md

+    ```
+    BEFORE UPDATE: order_id=abc123, dev_fee_paid=false, dev_fee_payment_hash=None
+    AFTER MODIFY: order_id=abc123, dev_fee_paid=true, dev_fee_payment_hash=Some("a1b2c3...")
+    ✅ DATABASE UPDATE SUCCEEDED for order abc123
+    VERIFICATION: order_id=abc123, dev_fee_paid=true, dev_fee_payment_hash=Some("a1b2c3...")
+    ```
+- **Result:** Complete diagnostic trail for debugging payment issues and database persistence


⚠️ Potential issue | 🟡 Minor

Add language identifier to fenced code block (markdownlint MD040).

The example output block lacks a language specifier.

🔎 Proposed fix

- ``` + ```text BEFORE UPDATE: order_id=abc123, dev_fee_paid=false, dev_fee_payment_hash=None AFTER MODIFY: order_id=abc123, dev_fee_paid=true, dev_fee_payment_hash=Some("a1b2c3...") ✅ DATABASE UPDATE SUCCEEDED for order abc123 VERIFICATION: order_id=abc123, dev_fee_paid=true, dev_fee_payment_hash=Some("a1b2c3...") - ``` + ```

📝 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.

Suggested change

```

BEFORE UPDATE: order_id=abc123, dev_fee_paid=false, dev_fee_payment_hash=None

AFTER MODIFY: order_id=abc123, dev_fee_paid=true, dev_fee_payment_hash=Some("a1b2c3...")

✅ DATABASE UPDATE SUCCEEDED for order abc123

VERIFICATION: order_id=abc123, dev_fee_paid=true, dev_fee_payment_hash=Some("a1b2c3...")

```

- **Result:** Complete diagnostic trail for debugging payment issues and database persistence

🧰 Tools

🪛 markdownlint-cli2 (0.18.1)

119-119: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents

docs/DEV_FEE.md around lines 119 to 125: the fenced code block example is missing a language identifier which triggers markdownlint MD040; add a language tag (e.g., "text") after the opening backticks so the block starts with ```text, leaving the contents unchanged and keeping the closing ``` as-is.

coderabbitai · 2025-12-30T15:51:06Z

docs/DEV_FEE.md

 ```
-Order Creation → Fee Calculation → Hold Invoice → Order Success → Dev Payment
-     ↓                 ↓                ↓              ↓              ↓
-  amount         mostro_fee        seller pays    settle hold    LNURL resolve
-                     ↓             amount + fees      ↓              ↓
-                 dev_fee                          buyer paid    send payment
-                                                                     ↓
-                                                              update db fields
+Order Creation → Fee Calculation → Hold Invoice → Seller Release → Dev Payment → Buyer Payment
+     ↓                 ↓                ↓              ↓                ↓             ↓
+  amount         mostro_fee        seller pays    settle hold     LNURL resolve   buyer paid
+                     ↓             amount + fees      ↓                ↓             ↓
+                 dev_fee                          status=settled   send payment   status=success
+                                                                       ↓
+                                                                update db fields
 ```


⚠️ Potential issue | 🟡 Minor

Add language identifier to flow diagram block (markdownlint MD040).

The diagram block is missing a language specification.

🔎 Proposed fix

-``` +```text Order Creation → Fee Calculation → Hold Invoice → Seller Release → Dev Payment → Buyer Payment -``` +```

📝 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.

Suggested change

```

Order Creation → Fee Calculation → Hold Invoice → Order Success → Dev Payment

↓ ↓ ↓ ↓ ↓

amount mostro_fee seller pays settle hold LNURL resolve

↓ amount + fees ↓ ↓

dev_fee buyer paid send payment

↓

update db fields

Order Creation → Fee Calculation → Hold Invoice → Seller Release → Dev Payment → Buyer Payment

↓ ↓ ↓ ↓ ↓ ↓

amount mostro_fee seller pays settle hold LNURL resolve buyer paid

↓ amount + fees ↓ ↓ ↓

dev_fee status=settled send payment status=success

↓

update db fields

```

🧰 Tools

🪛 markdownlint-cli2 (0.18.1)

191-191: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents

In docs/DEV_FEE.md around lines 191 to 199 the ASCII flow diagram fenced code block is missing a language identifier (markdownlint MD040); update the opening fence from ``` to ```text so the block is ```text ... ``` and leave the closing fence as ``` to satisfy the linter.

coderabbitai · 2025-12-30T15:51:06Z

docs/DEV_FEE.md

+**Current State:** ✅ IMPLEMENTED - ALL orders are created with `dev_fee = 0`. The `dev_fee` is calculated when the order is taken, unifying the behavior for both fixed price and market price orders. Database fields `dev_fee_paid` and `dev_fee_payment_hash` are initialized to `false` and `None` respectively.

-**Required Implementation:** Update `src/util.rs::prepare_new_order()`
+**Implementation:** `src/util.rs::prepare_new_order()`

 When creating a new order:
-1. Calculate Mostro fee: `fee = get_fee(amount)`
-2. Calculate dev fee: `dev_fee = get_dev_fee(fee)`
-3. Store in Order struct with `dev_fee_paid = false`
+1. Calculate Mostro fee: `fee = get_fee(amount)` (for fixed price orders where amount > 0)
+2. Set `dev_fee = 0` for ALL orders (both fixed price and market price)


⚠️ Potential issue | 🟡 Minor

Hyphenate compound adjectives.

Use hyphens for compound adjectives modifying nouns (e.g., "fixed-price order," "market-price orders").

🔎 Proposed fix

-**Current State:** ✅ IMPLEMENTED - ALL orders are created with `dev_fee = 0`. The `dev_fee` is calculated when the order is taken, unifying the behavior for both fixed price and market price orders. Database fields `dev_fee_paid` and `dev_fee_payment_hash` are initialized to `false` and `None` respectively. +**Current State:** ✅ IMPLEMENTED - ALL orders are created with `dev_fee = 0`. The `dev_fee` is calculated when the order is taken, unifying the behavior for both fixed-price and market-price orders. Database fields `dev_fee_paid` and `dev_fee_payment_hash` are initialized to `false` and `None` respectively.

Also apply to lines 311–312 and elsewhere where this pattern appears.

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools

🪛 LanguageTool

[grammar] ~306-~306: Use a hyphen to join words.
Context: ...en, unifying the behavior for both fixed price and market price orders. Database ...

(QB_NEW_EN_HYPHEN)

[grammar] ~306-~306: Use a hyphen to join words.
Context: ...behavior for both fixed price and market price orders. Database fields `dev_fee_p...

(QB_NEW_EN_HYPHEN)

[grammar] ~311-~311: Use a hyphen to join words.
Context: ... fee: fee = get_fee(amount) (for fixed price orders where amount > 0) 2. Set `d...

(QB_NEW_EN_HYPHEN)

[grammar] ~312-~312: Use a hyphen to join words.
Context: ...dev_fee = 0 for ALL orders (both fixed price and market price) 3. Store in Orde...

(QB_NEW_EN_HYPHEN)

[grammar] ~312-~312: Use a hyphen to join words.
Context: ... ALL orders (both fixed price and market price) 3. Store in Order struct with `de...

(QB_NEW_EN_HYPHEN)

🤖 Prompt for AI Agents

In docs/DEV_FEE.md around lines 306 to 312, compound adjectives like "fixed price orders" and "market price orders" should be hyphenated when modifying nouns; update phrases to "fixed-price order" and "market-price orders" (and similar occurrences on lines 311–312) throughout the file to consistently use hyphenation for compound adjectives.

coderabbitai · 2025-12-30T15:51:06Z

docs/DEV_FEE.md

+```
+Order Created:
+- amount: 100,000 sats (known at creation)
+- fee: 1,000 sats (calculated at creation)
+- dev_fee: 0 (NOT calculated at creation anymore)
+- Status: pending
+
+Order Taken:
+- amount: 100,000 sats (unchanged)
+- fee: 1,000 sats (unchanged)
+- total_mostro_fee: 2,000 sats (both parties)
+- dev_fee: 600 sats (30%)        ← Calculated when taken
+- Status: waiting-buyer-invoice
+
+Order Completes:
+- Dev fee payment triggered: 600 sats sent to dev fund ✓
+```
+


⚠️ Potential issue | 🟡 Minor

Add language identifier to code block example (markdownlint MD040).

Fixed price order example lacks language specification.

🔎 Proposed fix

-``` +```text Order Created: - amount: 100,000 sats (known at creation) -``` +```

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools

🪛 markdownlint-cli2 (0.18.1)

439-439: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents

In docs/DEV_FEE.md around lines 439 to 456 the fenced code block example lacks a language identifier (markdownlint MD040); update the opening fence from ``` to ```text so the block is explicitly marked as plain text (keep the closing ``` unchanged) to satisfy the linter.

coderabbitai · 2025-12-30T15:51:06Z

docs/DEV_FEE.md

+```
+Order Created:
+- Fiat: $100 USD
+- amount: 0 (unknown until taken)
+- fee: 0
+- dev_fee: 0
+- Status: pending
+
+Order Taken:
+- Market Price Lookup: $100 @ $50,000 = 200,000 sats
+- amount: 200,000 sats           ← Calculated
+- fee: 2,000 sats (1%)           ← Calculated
+- total_mostro_fee: 4,000 sats (both parties)
+- dev_fee: 1,200 sats (30%)      ← Calculated
+- Status: waiting-buyer-invoice
+
+Order Completes:
+- Dev fee payment triggered: 1,200 sats sent to dev fund ✓
 ```


⚠️ Potential issue | 🟡 Minor

Add language identifier to code block example (markdownlint MD040).

Market price order example lacks language specification.

🔎 Proposed fix

-``` +```text Order Created: - Fiat: $100 USD -``` +```

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools

🪛 markdownlint-cli2 (0.18.1)

458-458: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents

In docs/DEV_FEE.md around lines 458 to 476 the fenced code block showing the market price order example has no language identifier; add a language tag (e.g., ```text) to the opening fence so the block begins with ```text and keep the closing fence unchanged to satisfy markdownlint MD040 and improve rendering.

coderabbitai · 2025-12-30T15:51:06Z

docs/DEV_FEE.md

+```
+Order Created (market price):
+- amount: 0, fee: 0, dev_fee: 0, status: pending
+
+Order Taken at BTC=$50,000:
+- amount: 200,000 sats, fee: 2,000 sats, dev_fee: 600 sats
+- status: waiting-buyer-invoice
+
+Taker Abandons (timeout after expiration_seconds):
+- Scheduler detects timeout (taken_at > expiration_seconds)
+- Resets: amount: 0, fee: 0, dev_fee: 0
+- status: pending (ready for new taker)
+
+Order Re-taken at BTC=$52,000 (price increased):
+- amount: 192,308 sats, fee: 1,923 sats, dev_fee: 577 sats
+- Correct dev_fee for new market price ✓
+```


⚠️ Potential issue | 🟡 Minor

Add language identifier to code block example (markdownlint MD040).

Order creation example flow lacks language specification.

🔎 Proposed fix

-``` +```text Order Created (market price): - amount: 0, fee: 0, dev_fee: 0, status: pending -``` +```

Committable suggestion skipped: line range outside the PR's diff.

🧰 Tools

🪛 markdownlint-cli2 (0.18.1)

503-503: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents

In docs/DEV_FEE.md around lines 503 to 519, the markdown code block example is missing a language identifier which triggers markdownlint MD040; update the opening fence from ``` to ```text (or another appropriate language like ```md) and ensure the closing fence remains ``` so the block is correctly fenced with a language tag; keep the block content unchanged.

coderabbitai · 2025-12-30T15:51:06Z

docs/DEV_FEE.md

+```
+Order Creation (t=0):
+  └─> dev_fee = calculated_value (e.g., 300 sats)
+  └─> dev_fee_paid = 0
+  └─> dev_fee_payment_hash = NULL
+  └─> Database: INSERT INTO orders (dev_fee, dev_fee_paid, dev_fee_payment_hash) VALUES (300, 0, NULL)
+
+Order Processing (t=minutes):
+  └─> Buyer and seller complete trade
+  └─> Dev fee fields remain unchanged
+  └─> Database: No updates to dev_fee fields
+
+Order Release (t=seller_release):
+  └─> Seller's hold invoice settled
+  └─> status = 'settled-hold-invoice'
+  └─> Dev fee fields unchanged (dev_fee_paid = 0)
+  └─> Order enters scheduler queue
+  └─> Buyer payment initiated (asynchronous)
+
+Scheduler Cycle (t=next_60s_cycle, typically within 60s of seller release):
+  └─> Scheduler wakes up every 60 seconds
+  └─> Query: SELECT * FROM orders WHERE (status = 'settled-hold-invoice' OR status = 'success') AND dev_fee > 0 AND dev_fee_paid = 0
+  └─> Order found in unpaid queue
+  └─> Call: send_dev_fee_payment(order)
+
+Payment Attempt (t=payment_start):
+  └─> Step 1/3: LNURL resolution (timeout: 15s)
+  └─> Step 2/3: LND send_payment call (timeout: 5s)
+  └─> Step 3/3: Wait for payment result (timeout: 25s)
+
+Dev Fee Payment Success (t=payment_complete, ~3-8 seconds typical):
+  └─> Payment hash received: "a1b2c3d4e5f6..."
+  └─> dev_fee_paid = 1
+  └─> dev_fee_payment_hash = "a1b2c3d4e5f6..."
+  └─> Database: UPDATE orders SET dev_fee_paid = 1, dev_fee_payment_hash = 'a1b2c3d4e5f6...' WHERE id = ?
+  └─> Order removed from retry queue
+  └─> DONE ✓
+
+Order Success (t=order_complete, after dev fee payment):
+  └─> status = 'success'
+  └─> Buyer receives satoshis
+  └─> Dev fee already paid (dev_fee_paid = 1)
+  └─> Database: UPDATE orders SET status = 'success' WHERE id = ?
+
+Dev Fee Payment Failure (t=payment_timeout, could be 15s, 25s, or 50s timeout):
+  └─> Error logged (LNURL failure, routing failure, timeout, etc.)
+  └─> dev_fee_paid = 0 (unchanged)
+  └─> dev_fee_payment_hash = NULL (unchanged)
+  └─> Database: No update (fields remain unchanged)
+  └─> Order remains in retry queue
+  └─> Retry on next scheduler cycle (60 seconds later)
+  └─> Will retry indefinitely until payment succeeds
+
+Edge Case - Buyer Payment Fails:
+  └─> Order remains in status = 'settled-hold-invoice'
+  └─> failed_payment = true
+  └─> Retry scheduler (job_retry_failed_payments) attempts buyer payment again
+  └─> Dev fee already paid regardless (dev_fee_paid = 1)
+  └─> This is correct: seller released, fee was earned
+
+Edge Case - Dev Fee Payment Fails, Buyer Payment Succeeds (Race Condition):
+  └─> Seller releases → status = 'settled-hold-invoice'
+  └─> Scheduler attempts dev fee payment → FAILS (dev_fee_paid = 0)
+  └─> Simultaneously, buyer payment → SUCCEEDS → status = 'success'
+  └─> Order now in 'success' status with dev_fee_paid = 0
+  └─> Query includes both statuses, so order is still picked up on next cycle
+  └─> Dev fee payment eventually succeeds and updates dev_fee_paid = 1
+```


⚠️ Potential issue | 🟡 Minor

Add language identifier to timeline diagram (markdownlint MD040).

Payment flow timeline diagram is missing language specification.

🔎 Proposed fix

-``` +```text Order Creation (t=0): └─> dev_fee = calculated_value (e.g., 300 sats) -``` +```

🧰 Tools

🪛 markdownlint-cli2 (0.18.1)

860-860: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents

In docs/DEV_FEE.md around lines 860 to 927, the timeline diagram fenced code blocks lack a language identifier which triggers markdownlint MD040; update each triple-backtick fence that wraps the diagram to include the text language identifier (change ``` to ```text at the opening fence) so the diagram is explicitly marked as plain text and the lint rule is satisfied.

coderabbitai · 2025-12-30T15:51:06Z

docs/DEV_FEE.md

 - Orders still complete successfully regardless of dev fee payment failures
 - Failed payments automatically retry every 60 seconds via scheduler

+**4. Market Price Orders With Zero Dev Fee**


⚠️ Potential issue | 🟡 Minor

Use heading instead of emphasis.

Numbered item "4. Market Price Orders With Zero Dev Fee" should be a heading, not emphasized text (markdownlint MD036).

🔎 Proposed fix

-**4. Market Price Orders With Zero Dev Fee** +#### 4. Market Price Orders With Zero Dev Fee

📝 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.

Suggested change

**4. Market Price Orders With Zero Dev Fee**

#### 4. Market Price Orders With Zero Dev Fee

🧰 Tools

🪛 markdownlint-cli2 (0.18.1)

1442-1442: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

🤖 Prompt for AI Agents

In docs/DEV_FEE.md around line 1442, the numbered item "4. Market Price Orders With Zero Dev Fee" is formatted as emphasized text but should be a heading; change the markdown from emphasis (e.g., *...* or _..._) to an appropriate heading level (e.g., "#### 4. Market Price Orders With Zero Dev Fee" or "### 4. Market Price Orders With Zero Dev Fee") so it complies with markdownlint MD036 and renders as a section heading.

grunch added 2 commits December 24, 2025 17:30

grunch changed the title ~~Development fund 03~~ Development fund implementation phase 03 Dec 24, 2025

grunch changed the title ~~Development fund implementation phase 03~~ Development fund implementation third phase Dec 24, 2025

coderabbitai bot reviewed Dec 24, 2025

View reviewed changes

docs/DEV_FEE.md Show resolved Hide resolved

grunch added 2 commits December 25, 2025 14:12

Fix linting errors

680d515

coderabbitai bot reviewed Dec 25, 2025

View reviewed changes

src/app/release.rs Show resolved Hide resolved

Catrya reviewed Dec 25, 2025

View reviewed changes

grunch added 4 commits December 26, 2025 09:34

Validate dev_fee amount before attempting payment.

eaf3319

Fix edge case on dev fund payment

42253f1

Update documentation

Fix linting errors

5c1cdb5

coderabbitai bot reviewed Dec 26, 2025

View reviewed changes

grunch added 2 commits December 27, 2025 15:29

coderabbitai bot reviewed Dec 27, 2025

View reviewed changes

Add function documentation

c3d3ffb

coderabbitai bot reviewed Dec 27, 2025

View reviewed changes

src/scheduler.rs Show resolved Hide resolved

src/scheduler.rs Show resolved Hide resolved

grunch added 2 commits December 27, 2025 16:41

Fix lint errors

fa0759f

coderabbitai bot reviewed Dec 27, 2025

View reviewed changes

dev_fee should be reset unconditionally

1d091c1

coderabbitai bot reviewed Dec 27, 2025

View reviewed changes

src/scheduler.rs Show resolved Hide resolved

grunch requested a review from Catrya December 27, 2025 20:36

grunch added 2 commits December 27, 2025 17:51

Fix lint errors

f82630b

coderabbitai bot reviewed Dec 27, 2025

View reviewed changes

src/scheduler.rs Show resolved Hide resolved

grunch added 2 commits December 27, 2025 18:17

grunch added 2 commits December 30, 2025 11:11

coderabbitai bot reviewed Dec 30, 2025

View reviewed changes

src/app/take_sell.rs Show resolved Hide resolved

grunch added 3 commits December 30, 2025 12:13

Fix linting error

1d9aa22

grunch merged commit a2ec45e into main Dec 30, 2025
5 of 6 checks passed

grunch deleted the development-fund-03 branch December 30, 2025 15:50

coderabbitai bot reviewed Dec 30, 2025

View reviewed changes

coderabbitai bot mentioned this pull request Dec 30, 2025

feat: implement Phase 4 dev fee audit events via Nostr #559

Merged

grunch mentioned this pull request Jan 2, 2026

Sustainable Development Fee Implementation for Mostro #549

Closed

	4. Market Price Orders With Zero Dev Fee
	#### 4. Market Price Orders With Zero Dev Fee

Development fund implementation third phase #558

Development fund implementation third phase #558

Uh oh!

Conversation

grunch commented Dec 24, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Dec 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related issues

Possibly related PRs

Suggested reviewers

Poem

Pre-merge checks

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Catrya left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Catrya commented Dec 26, 2025

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Catrya commented Dec 29, 2025

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Dec 30, 2025

Choose a reason for hiding this comment

Uh oh!

grunch commented Dec 24, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 24, 2025 •

edited

Loading