LNURL-pay resolution in max_sendable#23
Merged
Merged
Conversation
1 task
amackillop
force-pushed
the
austin_mdk-865_v1-max-withdrawable
branch
from
e9ca454 to
2165ca5
Compare
amackillop
changed the base branch from
austin_mdk-865_v1-max-withdrawable
to
master
Pull the build-route-params / find-route / fold-into-estimate tail out of compute_estimate's FromRoute branch into a private helper. No behaviour change. The LNURL-pay branch added in the follow-up commit fetches a BOLT11 invoice asynchronously and then needs the exact same routing tail to turn the resulting PaymentParameters into an estimate. Lifting it here lets that branch reuse a single tested code path instead of duplicating the find_route + estimate_from_route sequence.
Wire an HrnResolver through compute_estimate so the LNURL-pay arm
fetches a concrete BOLT11 invoice from the callback before pricing.
The estimate is driven by find_route over a real path now, not a
flat percentage buffer.
Target amount is min(balance, max_value): the fee at the largest
amount we could plausibly send. If balance falls below min_value
the destination is unreachable; surface that as
MaxSendableError::BelowLnurlMin { min_msat, balance_msat } so consumers
can render a recipient-specific message ("minimum send is N
sats") instead of conflating it with a dust balance. LNURL fetch
or validation failures surface as a new
MaxSendableError::LnurlResolutionFailed variant.
compute_estimate becomes async; MdkClient::max_sendable likewise.
The sole caller (/getbalance) is already inside an async fn, so
the change is a single .await.
The resolver is built with `HTTPHrnResolver::with_client(self.http_client.clone())`
rather than `::new()` so LNURL callbacks go through the configured
SOCKS proxy. `MdkClient` now holds the shared `reqwest::Client`
that was previously passed only to `MdkApiClient`; pay_any.rs
should be switched over in a follow-up so it stops leaking IP
on LN-Address payments too.
HRNs are already resolved by PaymentInstructions::parse. BIP 353
names land here as whatever reusable methods the recipient
publishes in their BIP 321 URI (most commonly a BOLT12 offer);
LN-Address names land as LNURL-pay. No extra branch needed.
Existing dispatch tests are converted to #[tokio::test]. The
LNURL fetch path itself is exercised manually via /getbalance
against a real LN-Address endpoint; producing a signed BOLT11 for
an arbitrary amount in a unit test would need its own keying
harness, which isn't worth the surface area.
`HTTPHrnResolver::new()` builds its own `reqwest::Client` and ignores any proxy config, so LN-Address pays via /pay were reaching the LNURL callback host directly even when `socks_proxy` was set. /getbalance's max_sendable path was fixed in the previous commit; this is the matching fix for the actual send path. Expose `MdkClient::http_client()` and pass that shared client to `HTTPHrnResolver::with_client`. No new dependency; the client was already built in `MdkClient::new()` and handed to `MdkApiClient`. Possible only because the reqwest 0.13 bump aligned mdkd with bitcoin-payment-instructions; pre-bump the two crates pinned incompatible reqwest majors and `with_client` rejected ours.
amackillop
force-pushed
the
austin_mdk-865_resolve-lnurl
branch
from
509e939 to
e5f802f
Compare
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What it does
HrnResolverthroughcompute_estimateand resolvesLNURL-pay destinations to a concrete BOLT11 invoice before pricing.
Target amount is
min(balance, max_value): the fee at the largestamount we could plausibly send.
reqwest 0.12 → 0.13 bump aligns mdkd's reqwest with the bpi rev's,
which is what makes
HTTPHrnResolver::with_client(...)acceptMdkClient's sharedreqwest::Client./payLN-Address sends.pay_anywasbuilding
HTTPHrnResolver::new(), which constructs a freshreqwest client that ignores
socks_proxy. Bothmax_sendableandpay_anynow reuse the shared client.Error domain
MaxSendableErrorgains two variants for the API layer:BelowLnurlMin { min_msat, balance_msat }— distinct from a dustbalance. The destination has a hard floor we can't meet, and the
UI should say so rather than show "0 sat sendable".
LnurlResolutionFailed(String)— transport or validation failurefrom the LNURL callback. Surfaced rather than buffer-falling-back
so intermittent issues don't masquerade as a successful estimate.
Commits
Four atomic commits, each passes
just check:estimate_via_payment_paramshelper (pure refactor)max_sendablepay_anyLNURL fetches through the shared HTTP clientOut of scope
/getbalanceaccepting adest=query param. Today the daemoncalls
max_sendable(None); the destination-aware path isreachable only from inside the library.
hits the callback. Revisit if telemetry shows it matters.
ldk-node-lsp, which still pulls the olderbpi rev transitively. Test-build dup only.