fix: recover from failed payout resolution, and bound outbound network latency - #845
fix: recover from failed payout resolution, and bound outbound network latency#845grunch wants to merge 3 commits into
Conversation
`do_payment` resolves a lightning address before paying the buyer. When
that resolution produced no invoice it returned early, which skipped all
of the failure bookkeeping that the rest of the payment path relies on.
`find_failed_payment` selects on `failed_payment == true`, so an order
that failed this way kept `failed_payment = false` and was invisible to
the retry job. The error is also discarded at both call sites
(`let _ = do_payment(...)`), and the buyer notification lives inside
`check_failure_retries`, which was never reached. The order stayed in
`SettledHoldInvoice` with the seller's funds captured, the buyer unpaid
and never told, and no path back: a buyer sending a corrected invoice
only resets `payment_attempts` and depends on that same retry job.
Route the failure through `check_failure_retries`, exactly as a failed
`send_payment` already is. An empty response is treated the same as an
error, since `resolv_ln_address` reports some server-side rejections as
`Ok("")` (#814).
Message handlers run sequentially on a single event-loop task, so an outbound request one handler makes delays every other user's messages for as long as it takes. Some of those requests had no effective upper bound, which left responsiveness dependent on hosts the node does not control. - LNURL operations get a total wall-clock budget covering every round-trip, plus a connect timeout and a finite redirect chain. A per-request timeout alone was not enough: `resolv_ln_address` makes two sequential requests, and the second host comes from the first response. This also makes the 15s timeout in `dev_fee` unreachable, so it is removed rather than left advertising a limit that is no longer the real one. - `LndConnector::new` gets a connect timeout, where it previously had none. The RPCs after it are deliberately left alone: a timeout on `settle_invoice` would report failure for work LND may have done. - `order_action` runs its local checks (fiat currency, amounts, range limits, quote) before the payment request, and validates it without network I/O via the new `InvoiceCheck::Offline`. The address is still persisted and still resolved at payout — which is now a recoverable failure — so probing liveness at creation time bought an answer that can go stale before it is used, at the cost of an HTTP request to a sender-named host on the event loop for every order created. - `add_invoice_action` rejects on order status before resolving the payment request, so an order that cannot accept an invoice no longer pays for the round-trip first. Takes and invoice submission keep resolving over the network (`InvoiceCheck::Online`), where reachability is part of the decision.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 480166a33a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let client = timeout( | ||
| LND_CONNECT_TIMEOUT, |
There was a problem hiding this comment.
When this new 10-second deadline expires during a buyer payout, do_payment propagates the error from LndConnector::new() with ? before reaching either check_failure_retries call. The release and admin-settle callers discard that error, while the scheduler only retries orders whose failed_payment flag is set, so an already-settled order can remain permanently unpaid after a slow or wedged LND handshake. Handle connection failure through the payout failure bookkeeping before returning.
Useful? React with 👍 / 👎.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughInvoice validation now supports explicit offline and online modes. LNURL and LND operations receive bounded timeouts, order flows defer or gate validation appropriately, and failed Lightning address payouts update retry bookkeeping. ChangesInvoice and payment flows
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant OrderAction
participant validate_invoice
participant InvoiceValidator
OrderAction->>validate_invoice: validate with InvoiceCheck::Offline
validate_invoice->>InvoiceValidator: perform local validation
InvoiceValidator-->>validate_invoice: validation result
validate_invoice-->>OrderAction: continue or reject order
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/order.rs`:
- Around line 132-150: Ensure the invoice validated by validate_invoice in
order_action is the exact value persisted by prepare_new_order as buyer_invoice
and published through publish_order. Propagate the validated invoice into the
relevant order/SmallOrder field before publication, rather than discarding it,
or establish and document the existing field equivalence if guaranteed.
In `@src/app/release.rs`:
- Around line 521-527: Update the failure path around check_failure_retries in
the release flow to propagate its error instead of ignoring failed retry
bookkeeping; only return LnAddressParseError after the retry update succeeds,
preserving the existing logging for successful updates.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e0d68a5f-5562-40d3-87f5-8f93bed8f6b1
📒 Files selected for processing (9)
src/app/add_invoice.rssrc/app/dev_fee.rssrc/app/order.rssrc/app/release.rssrc/app/take_sell.rssrc/lightning/invoice.rssrc/lightning/mod.rssrc/lnurl.rssrc/util.rs
| // Validate the payment request last: every check above is local and | ||
| // cheap, so a malformed order is rejected without touching it. | ||
| // | ||
| // `Offline` deliberately skips resolving a lightning address or LNURL | ||
| // over the network. Note this is not a throwaway value — the address | ||
| // is persisted as `orders.buyer_invoice` (see `prepare_new_order`) and | ||
| // is what `do_payment` pays out to. Skipping the reachability probe | ||
| // here is safe *because* a failure to resolve at payout time is now | ||
| // handled as a payment failure: the buyer is notified and the retry | ||
| // job picks the order up. Checking it here instead would put an HTTP | ||
| // request to a sender-chosen host on the event loop for every order | ||
| // created, delaying every other message behind it, in exchange for a | ||
| // liveness answer that can go stale before it is ever used. | ||
| // | ||
| // The returned value is discarded because it is the same string this | ||
| // function already holds: `validate_invoice` re-reads it from `msg`. | ||
| let _invoice = | ||
| validate_invoice(&msg, &Order::from(order.clone()), InvoiceCheck::Offline).await?; | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate prepare_new_order and confirm it reads the payment request from the
# same message field validated by validate_invoice in order_action.
ast-grep run --pattern 'fn prepare_new_order($$$) { $$$ }' --lang rust src
rg -n -B2 -A20 'fn prepare_new_order' srcRepository: MostroP2P/mostro
Length of output: 1347
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate order files =="
fd -e rs 'order.*\.rs$|util\.rs$' src
echo
echo "== relevant order.rs outline =="
ast-grep outline src/app/order.rs --view expanded || true
echo
echo "== order.rs line 110-165 =="
sed -n '110,165p' src/app/order.rs | nl -ba -v110
echo
echo "== util.rs prepare_new_order relevant section =="
sed -n '550,640p' src/util.rs | nl -ba -v550
echo
echo "== validate_invoice definitions/usages =="
rg -n -B5 -A30 'async fn validate_invoice|fn validate_invoice|validate_invoice\(' srcRepository: MostroP2P/mostro
Length of output: 2082
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== order.rs line 110-165 =="
sed -n '110,165p' src/app/order.rs
echo
echo "== util.rs prepare_new_order relevant section =="
sed -n '550,640p' src/util.rs
echo
echo "== validate_invoice definitions/usages =="
rg -n -B5 -A30 'async fn validate_invoice|fn validate_invoice|validate_invoice\(' src
echo
echo "== payer_invoice fields/parsing =="
rg -n -B3 -A3 'payer_invoice|payment_invoice|purchase\.' src/app src/util.rsRepository: MostroP2P/mostro
Length of output: 19986
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== publish_order implementation =="
rg -n -B5 -A80 'async fn publish_order|fn publish_order' src
echo
echo "== Message/new_order/get_payment_request definitions =="
rg -n -B8 -A25 'fn new_order|get_payment_request|pub enum Payload|struct Message' src
echo
echo "== buyer_invoice references =="
rg -n -B3 -A3 '\bbuyer_invoice\b' srcRepository: MostroP2P/mostro
Length of output: 40210
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== inspect Message definitions and SmallOrder conversion =="
rg -n 'struct Message|impl Message|impl From<Order> for SmallOrder|impl From<SmallOrder> for Order|fn from\(order' src/nip33.rs src protocol 2>/dev/null || true
fd -e rs 'message|protocol|order' src | sort | head -50
echo
echo "== source files likely containing protocol/message types =="
fd -e rs '' src | rg '(message|protocol|order|mostro)' | head -80
echo
echo "== grep exact symbols with file context =="
rg -n -B5 -A40 'struct Message|impl Message|fn new_order\(|fn get_payment_request\(|impl From<Order> for SmallOrder|buyer_invoice:' src >/tmp/sym_search.txt || true
sed -n '1,220p' /tmp/sym_search.txtRepository: MostroP2P/mostro
Length of output: 12937
Clarify buyer_invoice validation before publication.
prepare_new_order persists new_order.buyer_invoice.clone(), but order_action only validates msg.get_payment_request() and then ignores the result. If Message and SmallOrder use different fields for the payment request/buyer_invoice, or if publishing ignores the field validated here, an order can persist an unchecked invoice. Map the validated invoice into order/SmallOrder before calling publish_order, or explicitly document these types are guaranteed to share the same field.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/order.rs` around lines 132 - 150, Ensure the invoice validated by
validate_invoice in order_action is the exact value persisted by
prepare_new_order as buyer_invoice and published through publish_order.
Propagate the validated invoice into the relevant order/SmallOrder field before
publication, rather than discarding it, or establish and document the existing
field equivalence if guaranteed.
| if let Ok(failed_payment) = check_failure_retries(ctx, &order, request_id).await { | ||
| info!( | ||
| "Order id {} has {} failed payments retries", | ||
| failed_payment.id, failed_payment.payment_attempts | ||
| ); | ||
| } | ||
| return Err(MostroInternalErr(ServiceError::LnAddressParseError)); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Propagate retry-bookkeeping failures.
Line 521 discards every check_failure_retries error, including a failed UPDATE orders. That can return LnAddressParseError without setting failed_payment or incrementing attempts—the exact unrecoverable state this change intends to prevent. Propagate the error before returning the resolution error.
Proposed fix
- if let Ok(failed_payment) = check_failure_retries(ctx, &order, request_id).await {
- info!(
- "Order id {} has {} failed payments retries",
- failed_payment.id, failed_payment.payment_attempts
- );
- }
+ let failed_payment = check_failure_retries(ctx, &order, request_id).await?;
+ info!(
+ "Order id {} has {} failed payments retries",
+ failed_payment.id, failed_payment.payment_attempts
+ );
return Err(MostroInternalErr(ServiceError::LnAddressParseError));📝 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.
| if let Ok(failed_payment) = check_failure_retries(ctx, &order, request_id).await { | |
| info!( | |
| "Order id {} has {} failed payments retries", | |
| failed_payment.id, failed_payment.payment_attempts | |
| ); | |
| } | |
| return Err(MostroInternalErr(ServiceError::LnAddressParseError)); | |
| let failed_payment = check_failure_retries(ctx, &order, request_id).await?; | |
| info!( | |
| "Order id {} has {} failed payments retries", | |
| failed_payment.id, failed_payment.payment_attempts | |
| ); | |
| return Err(MostroInternalErr(ServiceError::LnAddressParseError)); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/release.rs` around lines 521 - 527, Update the failure path around
check_failure_retries in the release flow to propagate its error instead of
ignoring failed retry bookkeeping; only return LnAddressParseError after the
retry update succeeds, preserving the existing logging for successful updates.
Review of the previous commit surfaced two more paths out of `do_payment` that skip the failure bookkeeping, leaving the same unrecoverable state it set out to fix. `LndConnector::new()` propagated its error with `?` before either `check_failure_retries` call. The bounded connect deadline added in this branch makes that reachable on a wedged LND rather than only a misconfigured one, so a settled order could stay unpaid, unnotified and invisible to the retry job after a slow handshake. `check_failure_retries` was also called through `if let Ok(..)`, which discarded its errors — including a failed `UPDATE orders`. A failure there produces exactly the state being guarded against, so it must not pass unnoticed. Both now go through `record_payout_failure`, which logs a bookkeeping failure at `error` instead of dropping it. Propagating it would be no better: both callers of `do_payment` discard its `Result`, so the log is the only signal an operator gets. The existing `do_payment_fails_fast_when_lnd_is_unreachable` test now asserts the flag and the attempt counter are persisted; verified to fail against the previous behavior.
|
Thanks both — addressed in ac2c989. Record LND connection timeouts as payout failures (codex, `lightning/mod.rs`): correct, and it is a hole this branch widened. `LndConnector::new()` propagated with `?` ahead of both `check_failure_retries` calls, and the connect deadline added here makes it reachable on a wedged LND rather than only a misconfigured one. Fixed. Propagate retry-bookkeeping failures (coderabbit, `release.rs`): agreed that discarding the result is wrong — a failed `UPDATE orders` produces precisely the state this branch is guarding against. I did not take the `?` propagation, though, because it would not reach anyone: both callers of `do_payment` are `let _ = do_payment(...)`, so the error would be dropped one frame later. Instead both sites now go through a `record_payout_failure` helper that logs a bookkeeping failure at `error` with a note that the order may need manual recovery, which is the only signal an operator actually gets. The pre-existing `send_payment` failure path had the same pattern and was switched over too, so all three exits are consistent. Clarify `buyer_invoice` validation before publication (coderabbit, `order.rs`): checked, and the two are the same field — no fix needed, but I expanded the comment since the ask was to document it. For a `NewOrder` the payload is `Payload::Order`, and `Message::get_payment_request` resolves that to `ord.buyer_invoice`, which is exactly what `prepare_new_order` copies into the stored order. A `NewOrder` with any other payload never reaches that code at all, because `get_order` returns `None` and the block is skipped. So there is no path where one value is validated and a different one persisted. Both new assertions were verified to fail against the previous behavior before the fixes were applied. Full suite: 1053 passed, clippy clean. |
![ermeme[bot]](https://avatars.githubusercontent.com/in/3690068?s=60&v=4){kind=small}
There was a problem hiding this comment.
Strict adversarial review of ac2c989d2ba9f3f2b6cce3e2b8303eb0f04a5aba.
The two payout-recovery blockers found on the previous head are resolved: failed LNURL resolution and failed/timed-out LND connection now enter payout-failure bookkeeping, and bookkeeping failures are surfaced loudly for manual recovery. I also verified that NewOrder validation reads the same buyer_invoice value that is persisted.
Focused local checks passed with Rust 1.94.0 (do_payment, offline/online invoice validation, bounded LNURL validation, and status-before-resolution), and all required exact-head CI checks are green.
Non-blocking follow-up: the pre-existing asynchronous PaymentStatus::Failed branch still uses the older silent if let Ok(...) bookkeeping pattern, so the helper's “every path” wording is broader than the current implementation.
No blocking issues remain.
arkanoider
left a comment
arkanoider
left a comment
There was a problem hiding this comment.
I tested manually this morning, all the specified case works correctly now. I added a small follow up an non-blocking alignment with new record_payout_failure function. Take a look.
tACK for me
| ) | ||
| .await; | ||
| } | ||
| PaymentStatus::Failed => { |
There was a problem hiding this comment.
Non-blocking follow-up from the adversarial review: this async arm still uses the older silent if let Ok(check_failure_retries) pattern, so a bookkeeping failure is dropped. The sync paths already go through record_payout_failure.
| PaymentStatus::Failed => { | |
| info!( | |
| "Order Id {}: Invoice with hash: {} has failed!", | |
| order.id, msg.payment.payment_hash | |
| ); | |
| // Same bookkeeping as the sync failure paths above: | |
| // log loudly if the UPDATE fails instead of dropping it. | |
| record_payout_failure(&ctx, &order, request_id).await; | |
| } |
That keeps every settled-payout failure path consistent with the helper's contract.
arkanoider
left a comment
arkanoider
left a comment
There was a problem hiding this comment.
Review — Approve with comments
Deep pass over ac2c989d (commits 7e412251 → 480166a3 → ac2c989d). Recommendation: approve and merge, with one small consistency follow-up.
Verdict
This correctly supersedes #843. Commit order matters and is right:
- Payout recovery first — a failed LN address resolution (or empty
pr) is recorded as a payment failure so the order is no longer stuckSettledHoldInvoice, unpaid, unnotified, and invisible tofind_failed_payment. - Then Offline validation at
NewOrder+ LNURL/LND bounds — safe because of (1). #843's "discarded result ⇒ skip network" assumption was wrong for the persistedbuyer_invoice; this PR documents and fixes that.
No blocking correctness issues for the stated goals.
What works well
Stuck-payout fix (do_payment)
- Resolution errors and
Ok("")both go through bookkeeping (covers #814-style empty responses). - LND connect failures / timeouts also go through bookkeeping — important because the new connect deadline makes that path more reachable on a wedged LND.
record_payout_failurelogs bookkeeping failure aterrorinstead of?into callers that dolet _ = do_payment(...). Right call.- Regression test
do_payment_records_failure_when_payout_address_yields_no_invoice(and the tightened LND-unreachable assertions) pin the actual bug.
Latency / DoS mitigation
InvoiceCheck::Offlineonorder_actionafter local checks — removes the cheap unauthenticated NewOrder stall vector.Onlineretained ontake_sell/add_invoicewhere reachability is part of the decision.- Status check before Online resolve in
add_invoice_action(plus the blackhole test that asserts <1s rejection). LNURL_TOTAL_BUDGETacross both hops (per-request timeout alone was insufficient forresolv_ln_address).- Connect timeout + finite redirects; dead 15s wrapper removed from
dev_fee.rs. - LND connect bounded; settle RPCs correctly left unbounded.
Clarity
- Comments on Offline vs Online and on why
buyer_invoicevalidated == persisted are accurate and useful for the next reader.
Non-blocking: async PaymentStatus::Failed still silent
ac2c989d aimed to route every failed payout attempt through record_payout_failure. The sync paths do; the async arm still uses the older pattern:
PaymentStatus::Failed => {
// ...
if let Ok(failed_payment) =
check_failure_retries(&ctx, &order, request_id).await
{
info!(/* ... */);
}
}When the UPDATE (or retries-exhausted validation) fails, the error is dropped — same hole class this PR closed elsewhere, just quieter. Bookkeeping still runs when the UPDATE succeeds, so this is not a merge blocker.
Suggested change:
PaymentStatus::Failed => {
info!(
"Order Id {}: Invoice with hash: {} has failed!",
order.id, msg.payment.payment_hash
);
// Same bookkeeping as the sync failure paths above:
// log loudly if the UPDATE fails instead of dropping it.
record_payout_failure(&ctx, &order, request_id).await;
}
(Happy to land this as a one-line follow-up if you'd rather keep this PR scoped.)
Other nits (non-blocking)
| Item | Note |
|---|---|
Early do_payment exits (buyer_invoice missing / amount == 0) |
Still skip bookkeeping — pre-existing; unlikely for a settled order. |
LND outage burns payment_attempts |
Better than stuck forever; ops should know retries can exhaust during node downtime. |
pay_new_invoice resets attempts, leaves failed_payment = true |
Needed for the retry job; subtle but correct. |
| Hardcoded LNURL/LND timeouts | Fine for now; configurable later if operators hit slow LNURL hosts. |
Residual surface (acknowledged follow-ups — OK)
- #844 — per-pubkey rate limit; Online paths can still HOL-block ~5s per message.
- #837 — LNURL host / SSRF allowlisting untouched.
- Event loop remains sequential — naive
spawncorrectly deferred.
Test plan check
Covered well: LN resolve → failed_payment; LND unreachable → bookkeeping; Offline no I/O; BOLT11 parity Offline/Online; status-before-resolve; LNURL budget vs blackhole.
Nice-to-have later: dedicated Ok("") fixture; e2e Offline NewOrder → settle → unresolvable address → PaymentFailed + scheduler pickup; PaymentStatus::Failed → record_payout_failure once that arm is fixed.
Bottom line
Approve with comments. Ship the recovery + bounds. Please consider the PaymentStatus::Failed → record_payout_failure consistency fix in this PR or immediately after.
Independent deep review of
|
2 participants
Supersedes #843, which was reworked after review found that one of its changes rested on a wrong assumption. Two commits, and the order matters — the first is what makes the second safe.
1. A failed payout address is now recorded as a payment failure
do_paymentresolves a lightning address before paying the buyer (release.rs). When that resolution produced no invoice, it returned early — skipping every piece of failure bookkeeping the rest of the payment path depends on.Following it through:
find_failed_payment(db.rs) selects onfailed_payment == true. That flag was never set, so the retry job could not see the order.let _ = do_payment(...), inrelease.rsandadmin_settle.rs).check_failure_retries, which was never reached.payment_attemptsand then relies on the same retry job.Net result: the order sat in
SettledHoldInvoicewith the seller's funds captured, the buyer unpaid and never told, and no route back short of operator intervention. Related to #601 and #814.The fix routes that failure through
check_failure_retries, exactly as a failedsend_paymentalready is, so the flag is set, the buyer is notified and the retry job takes over. An empty response is treated as a failure too, sinceresolv_ln_addressreports some server-side rejections asOk("")(#814).This bug predates the branch. It's fixed first because the next commit makes the path more reachable.
2. Outbound resolution is bounded, and local checks run first
Handlers run sequentially on one event-loop task, so a request one handler makes delays every other message behind it. Several had no effective upper bound.
resolv_ln_addressmakes two sequential requests and the second host comes from the first response. This makes the 15s timeout indev_fee.rsunreachable, so it's removed rather than left advertising a limit that is no longer the real one.LndConnector::new: a connect timeout, where there was none. The RPCs after it are deliberately left alone — a timeout onsettle_invoicewould report failure for work LND may have completed, which is a worse failure mode than waiting.order_action: local checks (fiat currency, amounts, range limits, quote) run before the payment request, which is then validated without network I/O via the newInvoiceCheck::Offline. Worth being precise about why that's safe, since fix: bound outbound network latency in message handling #843 got this wrong: the address is persisted asorders.buyer_invoiceand is what gets paid out. It's safe because commit 1 makes a resolution failure at payout time recoverable. Probing liveness at creation only buys an answer that can go stale before it's used.add_invoice_action: rejects on order status before resolving the payment request. An order in a status that can't accept an invoice no longer pays for the round-trip first.take_sellandadd_invoicekeep resolving over the network (InvoiceCheck::Online), where reachability is genuinely part of the decision. Behavior there is unchanged apart from the bound.Test plan
cargo test— 1053 passed, 0 failedcargo clippy --all-targets -- -D warnings— cleancargo fmt --allNew coverage:
do_payment_records_failure_when_payout_address_yields_no_invoice— assertsfailed_paymentandpayment_attemptsare persisted. Verified to fail against the old implementation before the fix was applied, so it pins the actual behavior rather than restating it.add_invoice_action_rejects_disallowed_status_before_resolving_address— points the payment request at a host that completes the handshake and never answers, then asserts the status rejection returns in well under a second.lnurl_validation_against_unresponsive_host_is_time_bounded— the same unresponsive host cannot hold validation open past the budget. Uses an ephemeral port via LNURL rather than the fixed 8080 thatstart_test_serverowns, so it doesn't race the existing tests.offline_validation_*— the offline path returns immediately, and still enforces every local BOLT11 rule.validate_invoice_pathsnow runs its whole table under both check modes, pinning down that they agree on every BOLT11 case.Manual check: create an order with a lightning address on a slow host and confirm it's created without the daemon pausing; take a sell with an unreachable address and confirm it's still rejected there; settle an order whose payout address no longer resolves and confirm the buyer receives
PaymentFailedand the order is picked up by the retry job.Follow-ups
Summary by CodeRabbit
Bug Fixes
Reliability