Add LUD-12 comment to dev fee LNURL-pay calls

Dev fee payments carried no identifying information, making it
impossible to trace which order or Mostro node originated a given
payment on the receiving end. Add an optional LUD-12 comment
(order id + node pubkey) to the LNURL-pay callback, gated by the
server-advertised commentAllowed limit.

While rewriting the callback-building code to attach the comment,
found that amount was concatenated onto the raw callback string
before parsing it as a URL. Real LNURL servers can return a callback
that already carries its own query string (e.g. ?id=...), so the
second ? silently mangled the query and amount never reached the
server as its own parameter. Extract build_callback_url, a pure
helper that adds both amount and comment via query_pairs_mut, with
a regression test for a callback with a pre-existing query string.

Also thread the already-cached AppContext::keys() through the
scheduler -> dev fee cycle instead of calling get_keys() (which
re-parses the nsec on every call), matching the pattern the
codebase already documents as preferred.

Closes #694

Author: ToRyVand

src/app/dev_fee.rs

@@ -71,6 +71,7 @@ use mostro_core::error::MostroError;
 use mostro_core::error::MostroError::MostroInternalErr;
 use mostro_core::error::ServiceError;
 use mostro_core::order::Order;
+use nostr_sdk::Keys;
 use sqlx::SqlitePool;
 use std::collections::HashSet;
 use tokio::sync::mpsc::channel;
@@ -87,13 +88,14 @@ pub async fn run_dev_fee_cycle(
     pool: &SqlitePool,
     ln_client: &mut LndConnector,
     confirmed: &mut HashSet<uuid::Uuid>,
+    keys: &Keys,
 ) {
     info!("Checking for unpaid development fees");
 
     cleanup_stale_pending_markers(pool).await;
     verify_confirmed_orders(pool, ln_client, confirmed).await;
     recover_partial_payments(pool, ln_client, confirmed).await;
-    process_new_dev_fee_payments(pool, ln_client, confirmed).await;
+    process_new_dev_fee_payments(pool, ln_client, confirmed, keys).await;
 }
 
 // ── Phase 1: Stale PENDING cleanup ──────────────────────────────────────
@@ -392,6 +394,7 @@ async fn process_new_dev_fee_payments(
     pool: &SqlitePool,
     ln_client: &mut LndConnector,
     confirmed: &mut HashSet<uuid::Uuid>,
+    keys: &Keys,
 ) {
     let unpaid_orders = match find_unpaid_dev_fees(pool).await {
         Ok(orders) => orders,
@@ -434,7 +437,7 @@ async fn process_new_dev_fee_payments(
 
         let (payment_request, payment_hash_hex) = match tokio::time::timeout(
             std::time::Duration::from_secs(20),
-            resolve_dev_fee_invoice(&order),
+            resolve_dev_fee_invoice(&order, keys),
         )
         .await
         {
@@ -883,7 +886,10 @@ fn parse_pending_timestamp(marker: &str) -> Option<u64> {
 ///
 /// # Timeouts
 /// - LNURL resolution: 15 seconds
-pub async fn resolve_dev_fee_invoice(order: &Order) -> Result<(String, String), MostroError> {
+pub async fn resolve_dev_fee_invoice(
+    order: &Order,
+    keys: &Keys,
+) -> Result<(String, String), MostroError> {
     info!(
         "Resolving dev fee invoice for order {} - amount: {} sats to {}",
         order.id, order.dev_fee, DEV_FEE_LIGHTNING_ADDRESS
@@ -893,9 +899,21 @@ pub async fn resolve_dev_fee_invoice(order: &Order) -> Result<(String, String),
         return Err(MostroInternalErr(ServiceError::WrongAmountError));
     }
 
+    // LUD-12 comment so the receiving end can trace which order/node a dev
+    // fee payment came from.
+    let comment = format!(
+        "mostro-dev-fee order={} node={}",
+        order.id,
+        keys.public_key()
+    );
+
     let payment_request = tokio::time::timeout(
         std::time::Duration::from_secs(15),
-        resolv_ln_address(DEV_FEE_LIGHTNING_ADDRESS, order.dev_fee as u64),
+        resolv_ln_address(
+            DEV_FEE_LIGHTNING_ADDRESS,
+            order.dev_fee as u64,
+            Some(comment.as_str()),
+        ),
     )
     .await
     .map_err(|_| {

src/app/release.rs

@@ -496,7 +496,7 @@ pub async fn do_payment(
         return Err(MostroInternalErr(ServiceError::InvoiceInvalidError));
     }
     let payment_request = if let Ok(addr) = ln_addr {
-        resolv_ln_address(&addr.to_string(), amount)
+        resolv_ln_address(&addr.to_string(), amount, None)
             .await
             .map_err(|_| MostroInternalErr(ServiceError::LnAddressParseError))?
     } else {

src/lnurl.rs

@@ -69,7 +69,40 @@ pub async fn ln_exists(address: &str) -> Result<(), MostroError> {
     }
 }
 
-pub async fn resolv_ln_address(address: &str, amount: u64) -> Result<String, MostroError> {
+/// LUD-12: returns the comment truncated to `max_len` chars, or `None` if
+/// there's no comment to send or the server advertises no support for one
+/// (`max_len == 0`).
+fn fit_comment(comment: Option<&str>, max_len: usize) -> Option<String> {
+    let comment = comment.filter(|_| max_len > 0)?;
+    Some(comment.chars().take(max_len).collect())
+}
+
+/// Builds the LNURL-pay callback URL, adding `amount` (and `comment`, per
+/// LUD-12, when the server allows it) as proper query parameters via
+/// `query_pairs_mut` — never by string-concatenating onto `callback`, which
+/// silently mangles the query if `callback` already carries one (a real LNURL
+/// server behavior, e.g. `https://host/cb?id=abc`).
+fn build_callback_url(
+    callback: &str,
+    amount_msat: u64,
+    comment: Option<&str>,
+    comment_allowed: usize,
+) -> Result<reqwest::Url, MostroError> {
+    let mut url = reqwest::Url::parse(callback)
+        .map_err(|_| MostroInternalErr(ServiceError::LnAddressParseError))?;
+    url.query_pairs_mut()
+        .append_pair("amount", &amount_msat.to_string());
+    if let Some(value) = fit_comment(comment, comment_allowed) {
+        url.query_pairs_mut().append_pair("comment", &value);
+    }
+    Ok(url)
+}
+
+pub async fn resolv_ln_address(
+    address: &str,
+    amount: u64,
+    comment: Option<&str>,
+) -> Result<String, MostroError> {
     // Get the url from the str - could be a LNURL or a Lightning Address
     let url = extract_lnurl(address).await?;
     // Convert the amount to msat
@@ -99,7 +132,9 @@ pub async fn resolv_ln_address(address: &str, amount: u64) -> Result<String, Mos
             return Ok("".to_string());
         }
         let callback = body["callback"].as_str().unwrap_or("");
-        let callback = format!("{callback}?amount={amount_msat}");
+        let comment_allowed = body["commentAllowed"].as_u64().unwrap_or(0) as usize;
+        let callback =
+            build_callback_url(callback, amount_msat, comment, comment_allowed)?.to_string();
         let res = HTTP_CLIENT
             .get(callback)
             .send()
@@ -122,3 +157,81 @@ pub async fn resolv_ln_address(address: &str, amount: u64) -> Result<String, Mos
         Ok("".to_string())
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::{build_callback_url, fit_comment};
+
+    #[test]
+    fn build_callback_url_adds_amount_as_its_own_param() {
+        let url = build_callback_url("https://pay.example.com/cb", 100_000, None, 0).unwrap();
+        assert_eq!(
+            url.query_pairs().collect::<Vec<_>>(),
+            vec![("amount".into(), "100000".into())]
+        );
+    }
+
+    #[test]
+    fn build_callback_url_preserves_existing_query_params() {
+        // Regression test: callback already carries its own query string
+        // (common in the wild, e.g. LNbits-style `?id=...`). The old
+        // `format!("{callback}?amount={amount_msat}")` approach produced a
+        // second `?`, which is not a delimiter, so `amount` got swallowed
+        // into the `id` value instead of becoming its own parameter.
+        let url =
+            build_callback_url("https://pay.example.com/cb?id=abc", 100_000, None, 0).unwrap();
+        let pairs = url.query_pairs().collect::<Vec<_>>();
+        assert_eq!(
+            pairs,
+            vec![
+                ("id".into(), "abc".into()),
+                ("amount".into(), "100000".into())
+            ]
+        );
+    }
+
+    #[test]
+    fn build_callback_url_adds_comment_when_allowed() {
+        let url =
+            build_callback_url("https://pay.example.com/cb", 100_000, Some("order=1"), 50).unwrap();
+        let pairs = url.query_pairs().collect::<Vec<_>>();
+        assert_eq!(pairs[0], ("amount".into(), "100000".into()));
+        assert_eq!(pairs[1], ("comment".into(), "order=1".into()));
+    }
+
+    #[test]
+    fn build_callback_url_omits_comment_when_not_allowed() {
+        let url =
+            build_callback_url("https://pay.example.com/cb", 100_000, Some("order=1"), 0).unwrap();
+        assert_eq!(
+            url.query_pairs().collect::<Vec<_>>(),
+            vec![("amount".into(), "100000".into())]
+        );
+    }
+
+    #[test]
+    fn fit_comment_none_when_not_allowed() {
+        assert_eq!(fit_comment(Some("order=1"), 0), None);
+    }
+
+    #[test]
+    fn fit_comment_none_when_no_comment() {
+        assert_eq!(fit_comment(None, 50), None);
+    }
+
+    #[test]
+    fn fit_comment_passes_through_when_short_enough() {
+        assert_eq!(
+            fit_comment(Some("order=1 node=abc"), 50),
+            Some("order=1 node=abc".to_string())
+        );
+    }
+
+    #[test]
+    fn fit_comment_truncates_to_server_limit() {
+        assert_eq!(
+            fit_comment(Some("order=1 node=abc"), 7),
+            Some("order=1".to_string())
+        );
+    }
+}

src/scheduler.rs

@@ -784,8 +784,9 @@ async fn job_process_dev_fee_payment(ctx: AppContext) {
 
     tokio::spawn(async move {
         let pool = ctx.pool();
+        let keys = ctx.keys();
         loop {
-            run_dev_fee_cycle(pool, &mut ln_client, &mut confirmed).await;
+            run_dev_fee_cycle(pool, &mut ln_client, &mut confirmed, keys).await;
             tokio::time::sleep(tokio::time::Duration::from_secs(interval)).await;
         }
     });