feat: BOLT12 offer payouts via LNDK and BIP-353 DNS resolution#720
feat: BOLT12 offer payouts via LNDK and BIP-353 DNS resolution#720QaidVoid wants to merge 2 commits intoMostroP2P:mainfrom
Conversation
Accept BOLT12 offers (lno1...) as buyer payout destinations by routing through an optional LNDK daemon alongside LND. LNDK implements BOLT12 (invoice_request onion messaging, LDK offer parsing); LND continues to handle routing. Opt-in via lightning.lndk_enabled (default false). When disabled, BOLT12 offers are rejected at validation time. Setup requirements are in docs/LNDK_SETUP.md. The BOLT11 path is unchanged. BOLT12 lives in do_payment_bolt12 so LND's streaming payment API and LNDK's synchronous RPC don't share plumbing. Uses two-step GetInvoice -> validate -> PayInvoice rather than PayOffer, which does not verify amount or expiry.
Resolve `user@domain.tld` payment addresses to BOLT12 offers via DNSSEC-validated DNS TXT records (BIP-353). On success the resolved offer replaces the address and the BOLT12 payout path runs unchanged. On failure the address is left untouched and the LNURL fallback handles it — Lightning Addresses keep working. DoH client reuses the existing reqwest HTTP client; no new resolver dependency. DNSSEC validated via the AD flag; bip353_skip_dnssec gates the check off for regtest. Gated on lndk_enabled since the resolved offer is unpayable without LNDK.
WalkthroughThis PR introduces BOLT12 offer support to Mostro through LNDK integration, including gRPC client implementation, BIP-353 DNS address resolution, invoice format classification and validation, configuration options, and wiring into order and payment flows. Changes
Sequence DiagramsequenceDiagram
actor User
participant Mostro
participant LNDK as LNDK<br/>(gRPC)
participant LND
User->>Mostro: Pay BOLT12 offer
Mostro->>Mostro: Classify & validate offer
Mostro->>LNDK: GetInvoice (offer)
LNDK->>LNDK: Generate BOLT12 invoice
LNDK-->>Mostro: Bolt12InvoiceContents
Mostro->>Mostro: Validate fetched invoice<br/>(amount, expiry, paths)
Mostro->>LNDK: PayInvoice (invoice)
LNDK->>LND: Pay via onion messages<br/>(blinded paths)
LND-->>LNDK: Preimage
LNDK-->>Mostro: Preimage
Mostro->>Mostro: Mark payment success
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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: 4
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)
1244-1286:⚠️ Potential issue | 🔴 CriticalStore the buyer's original address in
buyer_invoice, not the resolved BIP-353 offer; re-resolve at payout time.The code resolves BIP-353 addresses to BOLT12 offers and stores the resolved offer in
order.buyer_invoice(seeorder.rs:96-100), then uses that stored offer directly at payout time indo_payment_bolt12(release.rs:476,do_payment_bolt12atrelease.rs:579) without re-resolution. This defeats BIP-353's offer rotation: if the issuer rotates or expires the offer between order creation and payout, the stored offer becomes stale and payment fails.Instead, preserve the original
user@domainaddress inbuyer_invoiceand callresolve_bip353again at payout time (indo_paymentbefore routing todo_payment_bolt12) to fetch the current offer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/util.rs` around lines 1244 - 1286, The current logic in util.rs resolves BIP-353 addresses via crate::lightning::bip353::resolve_bip353 and then stores the resolved BOLT12 offer in order.buyer_invoice; instead, keep the original user@domain string in order.buyer_invoice (do not replace it with the resolved offer) and only use resolve_bip353 for validation (is_valid_invoice) or temporary checks here; then, remove usage of the stored offer in do_payment_bolt12 and instead call resolve_bip353 again from do_payment (before routing to do_payment_bolt12) to fetch the live BOLT12 offer at payout time so offer rotation/expiration is respected. Ensure resolve_bip353, is_valid_invoice, order.buyer_invoice, do_payment, and do_payment_bolt12 are the referenced symbols to locate changes.
🧹 Nitpick comments (15)
build.rs (1)
4-7: Optional: emitcargo:rerun-if-changedfor the proto inputs.Currently
build.rshas norerun-if-changeddirective for the.protofiles, so edits toproto/lndkrpc.proto(orproto/admin.proto) won't trigger a rebuild during local development. Adding directives keeps generated Tonic types in sync with the schema.♻️ Suggested addition
tonic_prost_build::configure() .protoc_arg("--experimental_allow_proto3_optional") .compile_protos(&["proto/admin.proto", "proto/lndkrpc.proto"], &["proto"]) .unwrap_or_else(|e| panic!("Failed to compile protos {:?}", e)); + + println!("cargo:rerun-if-changed=proto/admin.proto"); + println!("cargo:rerun-if-changed=proto/lndkrpc.proto");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build.rs` around lines 4 - 7, Add cargo rebuild triggers for the proto inputs so Cargo reruns the build script when proto files change: in the build script where tonic_prost_build::configure() and .compile_protos(...) are invoked, emit println!("cargo:rerun-if-changed=proto/lndkrpc.proto") and println!("cargo:rerun-if-changed=proto/admin.proto") (optionally also for the proto directory) before calling compile_protos to ensure generated Tonic code is regenerated on proto edits.src/util.rs (1)
1251-1251: Minor: tighten the address heuristic.
pr.contains('@') && !pr.contains(' ')will also match degenerate inputs like"@","user@", or"@domain"and forward them toresolve_bip353. Resolution will fail and the value will fall through to LNURL — functional, but it adds a wasted DNS round-trip per malformed submission. A cheapLightningAddress::from_str(&pr).is_ok()(already used elsewhere inis_valid_invoice) or a tighter regex would short-circuit clearly invalid inputs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/util.rs` at line 1251, The current heuristic `pr.contains('@') && !pr.contains(' ')` is too permissive and forwards degenerate inputs to resolve_bip353; replace it by attempting to parse as a LightningAddress first (e.g. call LightningAddress::from_str(&pr).is_ok()) or use the stricter validation used in is_valid_invoice so only well-formed addresses are sent to resolve_bip353; update the branch that sets `pr` to call LightningAddress::from_str(&pr).is_ok() (or an equivalent regex) before invoking resolve_bip353 to avoid unnecessary DNS lookups on invalid inputs.settings.tpl.toml (1)
28-30: Use neutral / non-mainnetexample paths in the template.The macaroon path defaults to
…/chain/bitcoin/mainnet/admin.macaroonwhile every other LND path in the template (lnd_cert_file,lnd_macaroon_file) uses Polar regtest paths. An operator who copies the template wholesale could end up pointing LNDK at mainnet credentials while LND itself is on regtest, mixing networks. Recommend using a clearly placeholder path or aligning with the existing Polarregtestconvention used above.♻️ Proposed alignment
-lndk_cert_file = "/home/user/.lndk/data/tls-cert.pem" +lndk_cert_file = "/home/user/.lndk/tls-cert.pem" # LND macaroon used by LNDK for payment authorization -lndk_macaroon_file = "/home/user/.lnd/data/chain/bitcoin/mainnet/admin.macaroon" +lndk_macaroon_file = "/home/user/.polar/networks/1/volumes/lnd/alice/data/chain/bitcoin/regtest/admin.macaroon"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@settings.tpl.toml` around lines 28 - 30, The template uses a mainnet macaroon path for lndk_macaroon_file which is inconsistent with the other Polar/regtest example paths and could cause accidental mainnet usage; update the lndk_macaroon_file value to a neutral placeholder or align it with the Polar regtest convention used by lnd_cert_file and lnd_macaroon_file (e.g., replace the /chain/bitcoin/mainnet/... path with a /chain/bitcoin/regtest/... or a clearly marked placeholder like /path/to/admin.macaroon) so lndk_cert_file, lndk_macaroon_file and lnd_macaroon_file consistently reference non-mainnet example locations.src/main.rs (1)
126-146: Optional: avoid the secondget_node_info()round-trip.
get_node_info()is already called at line 127 to populateLN_STATUS(whose response is moved intoLnStatus::from_get_info_response). Calling it again at line 146 just to readfeaturesis an extra gRPC round-trip on the boot path. You could either persist the originalGetInfoResponsefor one-shot reuse or extendLnStatuswith afeatures: HashMap<u32, Feature>field so the second call is unnecessary.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main.rs` around lines 126 - 146, The code makes a second gRPC round-trip by calling ln_client.get_node_info() again to check features for LNDK; reuse the initial GetInfoResponse instead: modify LnStatus::from_get_info_response or LnStatus to retain the original GetInfoResponse (or a features map) when you create LN_STATUS from the first ln_client.get_node_info() call, then replace the second ln_client.get_node_info().await.ok() usage in the lndk_client branch with a lookup from LN_STATUS (or LnStatus.features) so the extra get_node_info() call is removed; update references to LnStatus and LN_STATUS accessors accordingly (keep LndkConnector::new_from_settings, lndk_client logic unchanged).src/app/context.rs (1)
42-105: LGTM!
Option<LndkConnector>is the right shape —Nonecleanly disables BOLT12 payouts at every consumer site (do_payment_bolt12already handles the runtime-Nonecase explicitly).Optional follow-up: when BOLT12 unit tests land, consider adding a
with_lndk(...)method onTestContextBuilderso handler tests can inject a mock connector instead of going through the always-Nonepath.Also applies to: 250-250
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/context.rs` around lines 42 - 105, Add a with_lndk(...) builder method to TestContextBuilder so tests can inject a mock LndkConnector instead of relying on the always-None path; implement TestContextBuilder::with_lndk(self, lndk: Option<LndkConnector>) -> Self (or accept LndkConnector and wrap in Some) to set the builder's lndk field and return Self, and update any test setup that constructs TestContextBuilder to allow passing a mock connector into AppContext::new via the builder.src/config/types.rs (1)
205-262: Optional: alignDefaultwith#[serde(default = "...")]helpers.
LightningSettingsderivesDefault, so direct Rust construction (LightningSettings::default()— used intest_settings()and elsewhere) yieldslndk_grpc_host = "",lndk_fetch_invoice_timeout = 0, andbip353_doh_resolver = "". TOML deserialization with missing fields, on the other hand, uses thedefault_*helpers and gets the documented values. Today this is benign becauselndk_enabled/bip353_enableddefault tofalse, but anyone toggling those flags in a hand-built settings struct will get a 0-second timeout silently.Consider implementing
Defaultmanually (asMostroSettingsandRpcSettingsalready do) so both paths agree.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config/types.rs` around lines 205 - 262, LightningSettings derives Default but that Default yields empty/zero values for fields that have serde default helpers (lndk_grpc_host, lndk_fetch_invoice_timeout, bip353_doh_resolver), causing mismatch between LightningSettings::default() and TOML deserialization; implement a manual impl Default for LightningSettings that constructs the struct with the same default values used by the serde helpers (call default_lndk_grpc_host(), default_lndk_fetch_timeout(), default_bip353_doh_resolver() for the respective fields) and sensible defaults for the remaining fields (empty strings, zeros, false as appropriate) so tests like test_settings() and any hand-built settings match deserialized defaults.src/lightning/bip353.rs (2)
95-122: Minor:parse_bitcoin_uridoesn't enforce thebitcoin:scheme.The parser only looks for
?and anlno=parameter, so it would happily extractlno1…from any TXT payload that happens to embed a query string (e.g. a strayhttps://…?lno=…). For BIP-353 the lookup name is targeted, so the blast radius is small, but a cheaptxt.starts_with("bitcoin:")(after dechunking) would make the contract explicit and reject malformed records earlier.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lightning/bip353.rs` around lines 95 - 122, The parse_bitcoin_uri function currently accepts any string with a query containing lno=; after dechunking/trim you should enforce the BIP-353 scheme by verifying the dechunked txt starts with "bitcoin:" (e.g., let txt = ...; if !txt.starts_with("bitcoin:") { return None; }) before splitting on '?', so malformed or unrelated query strings are rejected; keep the rest of the logic (query extraction and lno lookup) unchanged.
27-84: Doc/return-type mismatch:Erris never returned.The doc comment says "Only returns
Errfor truly malformed input", but every failure path in the function body returnsOk(None)— there is no path that constructs anErr. Either tighten the signature toOption<String>or drop theErrclause from the docstring (and decide which malformed-input cases should genuinely error out).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lightning/bip353.rs` around lines 27 - 84, The function resolve_bip353 currently has signature pub async fn resolve_bip353(address: &str) -> Result<Option<String>, MostroError> but never returns Err, so change it to return Option<String> (pub async fn resolve_bip353(address: &str) -> Option<String>), update its body to return Some(...) / None as it already does, remove handling of MostroError, update any call sites to handle Option instead of Result, and update the doc comment to remove the "Err" clause; alternatively, if you prefer real error returns, add specific Err returns in resolve_bip353 for truly malformed input (e.g., when Settings::get_ln() or query_doh fails) and construct MostroError accordingly—pick one approach and make callers and the docstring consistent with resolve_bip353 and Settings::get_ln/query_doh usage.src/app/order.rs (1)
96-101: Minor: avoid double-clone oforder.The block clones
ordertwice — once to materialize anOrderforvalidate_invoice, and again to constructorder_with_resolved. You can fold these into a single clone by validating off the already-cloned value.♻️ Suggested tweak
- let resolved_invoice = validate_invoice(&msg, &Order::from(order.clone())).await?; - let mut order_with_resolved = order.clone(); - if resolved_invoice.is_some() { - order_with_resolved.buyer_invoice = resolved_invoice; - } - let order = &order_with_resolved; + let mut order_with_resolved = order.clone(); + let resolved_invoice = + validate_invoice(&msg, &Order::from(order_with_resolved.clone())).await?; + if resolved_invoice.is_some() { + order_with_resolved.buyer_invoice = resolved_invoice; + } + let order = &order_with_resolved;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/order.rs` around lines 96 - 101, Clone `order` only once by creating a single mutable clone (e.g., let mut order_with_resolved = order.clone()), then call validate_invoice(&msg, &Order::from(...)) using that cloned value (either by borrowing or moving into Order::from as the type permits) instead of cloning again, update order_with_resolved.buyer_invoice with the resolved invoice when present, and then use &order_with_resolved for the rest; adjust the call sites around validate_invoice, Order::from, and order_with_resolved to avoid the second clone.src/lightning/lndk.rs (3)
163-168: RedundantamountonPayInvoiceRequestfor an amount-bearing invoice.The fetched invoice already pins
amount_msats, andvalidate_fetched_invoice(lines 155–161) has just confirmed it matchesamount_msats. Sendingamount: Some(amount_msats)again is harmless today but duplicates the source of truth — if LNDK ever treats this field as an override (for amountless invoices) it could mask a desync. Setting it toNonehere would make the intent clearer and rely solely on the validated invoice contents.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lightning/lndk.rs` around lines 163 - 168, The PayInvoiceRequest currently sets amount: Some(amount_msats) even though validate_fetched_invoice already confirmed the fetched invoice (fetched.invoice_hex_str) contains the expected amount_msats; remove the redundant amount field by setting amount to None in the PayInvoiceRequest construction so the request relies solely on the validated invoice contents (update the PayInvoiceRequest used when building pay_req and ensure validate_fetched_invoice remains the source of truth).
82-87: HardcodedlocalhostSNI may fail for non-sidecar LNDK deployments.This works when LNDK runs on the same host as Mostro (the documented common case), but if an operator points
lndk_grpc_hostat a remote LNDK whose TLS cert was generated for a different SAN (e.g. its real hostname), the TLS handshake will fail with a confusing "invalid certificate: name not in cert" error. Consider deriving the SNI fromlndk_grpc_host(stripping scheme/port) and only falling back to"localhost", or exposing it as a setting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lightning/lndk.rs` around lines 82 - 87, The TLS SNI is currently hardcoded to "localhost" which breaks remote LNDK deployments; change the ClientTlsConfig::domain_name call to derive the name from the configured lndk_grpc_host (parse/strip any scheme and port to get the hostname) and pass that hostname to domain_name, falling back to "localhost" only if parsing yields nothing or the host is empty; update the code around Certificate::from_pem(&cert) / ClientTlsConfig::new() to use the derived SNI or expose it as a config value.
107-114: The field namelndk_fee_limit_percentis misleading — it stores a fraction, not a percent.The field is documented in
src/config/types.rsas "Fee limit for BOLT12 payments as percent", but the code at line 112–114 treats it as a fraction (multiplying by 100.0 and clamping). The template examplelndk_fee_limit_percent = 0.2and fallback tomostro_settings.max_routing_fee(which is also a fraction) both confirm the actual units are fractional. An operator reading the config name or the field's doc comment would reasonably setlndk_fee_limit_percent = 2expecting "2%", but this would be interpreted as 200%. Rename tolndk_fee_limit_fraction(matchingmax_routing_feenaming), or accept a true percent and divide by 100 at the config layer. Also update the doc comment insrc/config/types.rs:236to say "fraction" instead of "percent".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lightning/lndk.rs` around lines 107 - 114, The config field lndk_fee_limit_percent is misnamed — it stores a fractional value (like max_routing_fee) but is documented/ named as a percent; rename it to lndk_fee_limit_fraction in the config type and update its doc comment to say "fraction" (and update any example/template values to use fractional form), then update usages in lightning/lndk.rs (replace lndk_fee_limit_percent with lndk_fee_limit_fraction when building fee_fraction for fee_limit_percent) and any other places that reference the old symbol; to avoid breaking existing configs, add a serde alias/rename for backward compatibility (e.g. #[serde(alias = "lndk_fee_limit_percent")] or serde rename) on the new field.src/lightning/offers.rs (3)
99-132: Consider rejecting offers whose chain set excludes the active network.
validate_offerchecks denomination but notoffer.chains(). A buyer can submit a perfectly-parseable BOLT12 offer that only advertises e.g. signet while Mostro/LND/LNDK are on mainnet; that offer will pass validation here and fail later at fetch/pay time, leaving the order in a degraded state. A defensive check against the configured network's chain hash up front would surface the mismatch synchronously, with the sameInvoiceInvalidErrorsemantics already used for currency mismatches.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lightning/offers.rs` around lines 99 - 132, validate_offer currently skips checking offer.chains(), so add a defensive check after parsing the Offer (after Offer::from_str and before the amount match) that the parsed offer.chains() includes the active network's chain hash and return MostroInternalErr(ServiceError::InvoiceInvalidError) if it does not; obtain the active network chain hash from the same config/source used elsewhere in the module (or add an active_chain_hash parameter to validate_offer if no accessor exists) so a non-matching offer (e.g., signet vs mainnet) is rejected synchronously.
51-57: Redundant prefix checks:lntbsandlnbcrtare shadowed.
starts_with("lntb")already matcheslntbs…, andstarts_with("lnbc")already matcheslnbcrt…, so the explicitlntbs/lnbcrtarms are unreachable. All four currently map toBolt11so there is no functional bug, but the redundancy is misleading — readers may infer signet/regtest are being treated specially.♻️ Cleanup
- if lower.starts_with("lnbc") - || lower.starts_with("lntb") - || lower.starts_with("lntbs") - || lower.starts_with("lnbcrt") - { + if lower.starts_with("lnbc") || lower.starts_with("lntb") { return InvoiceFormat::Bolt11; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lightning/offers.rs` around lines 51 - 57, The prefix checks in the block that returns InvoiceFormat::Bolt11 are redundant: remove the explicit starts_with("lntbs") and starts_with("lnbcrt") checks and keep only starts_with("lnbc") and starts_with("lntb") (the code that builds/uses the lower variable and returns InvoiceFormat::Bolt11 should remain), so behavior is unchanged but the unreachable arms are eliminated and readability improved; update any nearby comment if it suggests special-casing signet/regtest to avoid confusion.
144-151:Quantity::Bounded(n).get() < 1is unreachable code.The
lightningcrate'sQuantity::Boundedvariant wraps aNonZeroU64, son.get()is guaranteed to be ≥ 1. The check will never trigger and can be removed. Simplify the match arm to accept any bounded quantity:Suggested cleanup
match offer.supported_quantity() { - Quantity::One | Quantity::Unbounded => {} - Quantity::Bounded(n) => { - if n.get() < 1 { - return Err(MostroInternalErr(ServiceError::InvoiceInvalidError)); - } - } + Quantity::One | Quantity::Unbounded | Quantity::Bounded(_) => {} }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lightning/offers.rs` around lines 144 - 151, The match on offer.supported_quantity() in offers.rs contains an unreachable check: inside the Quantity::Bounded(n) arm the code tests if n.get() < 1 and returns an error, but Quantity::Bounded holds a NonZeroU64 so n.get() is always ≥ 1; remove that conditional and simply accept the Bounded(n) arm (i.e., drop the n.get() < 1 check and the MostroInternalErr return) so the match becomes a no-op for One, Unbounded, and Bounded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/LNDK_SETUP.md`:
- Line 13: Add explicit language specifiers to the two fenced code blocks that
currently have none: change the block containing the ASCII diagram "Buyer ----
Nostr ----> Mostro ----gRPC----> LNDK ----gRPC----> LND …" to start with
```text and change the block containing the configuration snippet beginning with
"[protocol]" to start with ```ini so both blocks satisfy markdownlint MD040.
In `@settings.tpl.toml`:
- Around line 21-46: The template added required-looking keys that will break
existing configs; update LightningSettings in src/config/types.rs to make the
new fields optional by adding #[serde(default)] to lndk_enabled, lndk_cert_file,
lndk_macaroon_file, bip353_enabled, and bip353_skip_dnssec (or alternatively
revert these entries in settings.tpl.toml to commented form like the
[anti_abuse_bond] pattern); ensure the struct field names exactly match those
keys and add sensible Rust defaults if needed, and document the chosen approach
in the changelog.
In `@src/lightning/bip353.rs`:
- Around line 27-84: The logs in resolve_bip353 currently emit the full BIP-353
address (user@domain); change all tracing calls to avoid logging the raw
local-part by only including the domain (domain) and either a short
deterministic hash or redact of the user (e.g. hash of user or
"[redacted_user]") instead of address. Update the tracing::warn/info/debug calls
that currently reference {address} (the DoH failure, DNS status, DNSSEC warning,
resolved bolt12, and no-offer debug messages) to include domain=%domain and
user_hash=%user_hash (or user="[redacted]") so diagnostics keep domain context
but do not expose the plain user identifier; compute user_hash from the local
variable user before logging.
In `@src/util.rs`:
- Around line 1251-1265: The logs in the BIP-353 resolution branch leak the full
user@domain payout address via tracing::info! and tracing::warn!; update the
logging in the block that calls
crate::lightning::bip353::resolve_bip353(&pr).await so it does not emit the raw
pr value—either redact the local-part (log only "@domain"), or log a stable
short hash (e.g., SHA-256 first 8 bytes) of pr, or lower the level to debug;
change the tracing::info! and tracing::warn! calls that reference {pr} to use
the chosen non-identifying representation instead while leaving the rest of the
control flow and returned value (offer or pr) unchanged.
---
Outside diff comments:
In `@src/util.rs`:
- Around line 1244-1286: The current logic in util.rs resolves BIP-353 addresses
via crate::lightning::bip353::resolve_bip353 and then stores the resolved BOLT12
offer in order.buyer_invoice; instead, keep the original user@domain string in
order.buyer_invoice (do not replace it with the resolved offer) and only use
resolve_bip353 for validation (is_valid_invoice) or temporary checks here; then,
remove usage of the stored offer in do_payment_bolt12 and instead call
resolve_bip353 again from do_payment (before routing to do_payment_bolt12) to
fetch the live BOLT12 offer at payout time so offer rotation/expiration is
respected. Ensure resolve_bip353, is_valid_invoice, order.buyer_invoice,
do_payment, and do_payment_bolt12 are the referenced symbols to locate changes.
---
Nitpick comments:
In `@build.rs`:
- Around line 4-7: Add cargo rebuild triggers for the proto inputs so Cargo
reruns the build script when proto files change: in the build script where
tonic_prost_build::configure() and .compile_protos(...) are invoked, emit
println!("cargo:rerun-if-changed=proto/lndkrpc.proto") and
println!("cargo:rerun-if-changed=proto/admin.proto") (optionally also for the
proto directory) before calling compile_protos to ensure generated Tonic code is
regenerated on proto edits.
In `@settings.tpl.toml`:
- Around line 28-30: The template uses a mainnet macaroon path for
lndk_macaroon_file which is inconsistent with the other Polar/regtest example
paths and could cause accidental mainnet usage; update the lndk_macaroon_file
value to a neutral placeholder or align it with the Polar regtest convention
used by lnd_cert_file and lnd_macaroon_file (e.g., replace the
/chain/bitcoin/mainnet/... path with a /chain/bitcoin/regtest/... or a clearly
marked placeholder like /path/to/admin.macaroon) so lndk_cert_file,
lndk_macaroon_file and lnd_macaroon_file consistently reference non-mainnet
example locations.
In `@src/app/context.rs`:
- Around line 42-105: Add a with_lndk(...) builder method to TestContextBuilder
so tests can inject a mock LndkConnector instead of relying on the always-None
path; implement TestContextBuilder::with_lndk(self, lndk: Option<LndkConnector>)
-> Self (or accept LndkConnector and wrap in Some) to set the builder's lndk
field and return Self, and update any test setup that constructs
TestContextBuilder to allow passing a mock connector into AppContext::new via
the builder.
In `@src/app/order.rs`:
- Around line 96-101: Clone `order` only once by creating a single mutable clone
(e.g., let mut order_with_resolved = order.clone()), then call
validate_invoice(&msg, &Order::from(...)) using that cloned value (either by
borrowing or moving into Order::from as the type permits) instead of cloning
again, update order_with_resolved.buyer_invoice with the resolved invoice when
present, and then use &order_with_resolved for the rest; adjust the call sites
around validate_invoice, Order::from, and order_with_resolved to avoid the
second clone.
In `@src/config/types.rs`:
- Around line 205-262: LightningSettings derives Default but that Default yields
empty/zero values for fields that have serde default helpers (lndk_grpc_host,
lndk_fetch_invoice_timeout, bip353_doh_resolver), causing mismatch between
LightningSettings::default() and TOML deserialization; implement a manual impl
Default for LightningSettings that constructs the struct with the same default
values used by the serde helpers (call default_lndk_grpc_host(),
default_lndk_fetch_timeout(), default_bip353_doh_resolver() for the respective
fields) and sensible defaults for the remaining fields (empty strings, zeros,
false as appropriate) so tests like test_settings() and any hand-built settings
match deserialized defaults.
In `@src/lightning/bip353.rs`:
- Around line 95-122: The parse_bitcoin_uri function currently accepts any
string with a query containing lno=; after dechunking/trim you should enforce
the BIP-353 scheme by verifying the dechunked txt starts with "bitcoin:" (e.g.,
let txt = ...; if !txt.starts_with("bitcoin:") { return None; }) before
splitting on '?', so malformed or unrelated query strings are rejected; keep the
rest of the logic (query extraction and lno lookup) unchanged.
- Around line 27-84: The function resolve_bip353 currently has signature pub
async fn resolve_bip353(address: &str) -> Result<Option<String>, MostroError>
but never returns Err, so change it to return Option<String> (pub async fn
resolve_bip353(address: &str) -> Option<String>), update its body to return
Some(...) / None as it already does, remove handling of MostroError, update any
call sites to handle Option instead of Result, and update the doc comment to
remove the "Err" clause; alternatively, if you prefer real error returns, add
specific Err returns in resolve_bip353 for truly malformed input (e.g., when
Settings::get_ln() or query_doh fails) and construct MostroError
accordingly—pick one approach and make callers and the docstring consistent with
resolve_bip353 and Settings::get_ln/query_doh usage.
In `@src/lightning/lndk.rs`:
- Around line 163-168: The PayInvoiceRequest currently sets amount:
Some(amount_msats) even though validate_fetched_invoice already confirmed the
fetched invoice (fetched.invoice_hex_str) contains the expected amount_msats;
remove the redundant amount field by setting amount to None in the
PayInvoiceRequest construction so the request relies solely on the validated
invoice contents (update the PayInvoiceRequest used when building pay_req and
ensure validate_fetched_invoice remains the source of truth).
- Around line 82-87: The TLS SNI is currently hardcoded to "localhost" which
breaks remote LNDK deployments; change the ClientTlsConfig::domain_name call to
derive the name from the configured lndk_grpc_host (parse/strip any scheme and
port to get the hostname) and pass that hostname to domain_name, falling back to
"localhost" only if parsing yields nothing or the host is empty; update the code
around Certificate::from_pem(&cert) / ClientTlsConfig::new() to use the derived
SNI or expose it as a config value.
- Around line 107-114: The config field lndk_fee_limit_percent is misnamed — it
stores a fractional value (like max_routing_fee) but is documented/ named as a
percent; rename it to lndk_fee_limit_fraction in the config type and update its
doc comment to say "fraction" (and update any example/template values to use
fractional form), then update usages in lightning/lndk.rs (replace
lndk_fee_limit_percent with lndk_fee_limit_fraction when building fee_fraction
for fee_limit_percent) and any other places that reference the old symbol; to
avoid breaking existing configs, add a serde alias/rename for backward
compatibility (e.g. #[serde(alias = "lndk_fee_limit_percent")] or serde rename)
on the new field.
In `@src/lightning/offers.rs`:
- Around line 99-132: validate_offer currently skips checking offer.chains(), so
add a defensive check after parsing the Offer (after Offer::from_str and before
the amount match) that the parsed offer.chains() includes the active network's
chain hash and return MostroInternalErr(ServiceError::InvoiceInvalidError) if it
does not; obtain the active network chain hash from the same config/source used
elsewhere in the module (or add an active_chain_hash parameter to validate_offer
if no accessor exists) so a non-matching offer (e.g., signet vs mainnet) is
rejected synchronously.
- Around line 51-57: The prefix checks in the block that returns
InvoiceFormat::Bolt11 are redundant: remove the explicit starts_with("lntbs")
and starts_with("lnbcrt") checks and keep only starts_with("lnbc") and
starts_with("lntb") (the code that builds/uses the lower variable and returns
InvoiceFormat::Bolt11 should remain), so behavior is unchanged but the
unreachable arms are eliminated and readability improved; update any nearby
comment if it suggests special-casing signet/regtest to avoid confusion.
- Around line 144-151: The match on offer.supported_quantity() in offers.rs
contains an unreachable check: inside the Quantity::Bounded(n) arm the code
tests if n.get() < 1 and returns an error, but Quantity::Bounded holds a
NonZeroU64 so n.get() is always ≥ 1; remove that conditional and simply accept
the Bounded(n) arm (i.e., drop the n.get() < 1 check and the MostroInternalErr
return) so the match becomes a no-op for One, Unbounded, and Bounded.
In `@src/main.rs`:
- Around line 126-146: The code makes a second gRPC round-trip by calling
ln_client.get_node_info() again to check features for LNDK; reuse the initial
GetInfoResponse instead: modify LnStatus::from_get_info_response or LnStatus to
retain the original GetInfoResponse (or a features map) when you create
LN_STATUS from the first ln_client.get_node_info() call, then replace the second
ln_client.get_node_info().await.ok() usage in the lndk_client branch with a
lookup from LN_STATUS (or LnStatus.features) so the extra get_node_info() call
is removed; update references to LnStatus and LN_STATUS accessors accordingly
(keep LndkConnector::new_from_settings, lndk_client logic unchanged).
In `@src/util.rs`:
- Line 1251: The current heuristic `pr.contains('@') && !pr.contains(' ')` is
too permissive and forwards degenerate inputs to resolve_bip353; replace it by
attempting to parse as a LightningAddress first (e.g. call
LightningAddress::from_str(&pr).is_ok()) or use the stricter validation used in
is_valid_invoice so only well-formed addresses are sent to resolve_bip353;
update the branch that sets `pr` to call LightningAddress::from_str(&pr).is_ok()
(or an equivalent regex) before invoking resolve_bip353 to avoid unnecessary DNS
lookups on invalid inputs.
🪄 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
Run ID: 03f4e435-f884-4f07-bae3-2cf5d320eb9b
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
Cargo.tomlbuild.rsdocs/LNDK_SETUP.mddocs/STARTUP_AND_CONFIG.mdproto/lndkrpc.protosettings.tpl.tomlsrc/app/context.rssrc/app/order.rssrc/app/release.rssrc/config/types.rssrc/config/wizard.rssrc/lightning/bip353.rssrc/lightning/invoice.rssrc/lightning/lndk.rssrc/lightning/mod.rssrc/lightning/offers.rssrc/main.rssrc/rpc/service.rssrc/util.rs
|
|
||
| ## Architecture | ||
|
|
||
| ``` |
There was a problem hiding this comment.
Add language specifiers to fenced code blocks (MD040).
Static analysis (markdownlint MD040) flagged two fenced blocks without a language. As per coding guidelines: "Add a language specifier to every fenced code block in documentation to comply with markdownlint MD040".
📝 Proposed fix
-```
+```text
Buyer ---- Nostr ----> Mostro ----gRPC----> LNDK ----gRPC----> LND
…-```
+```ini
[protocol]
protocol.custom-message=513
protocol.custom-nodeann=39
protocol.custom-init=39Also applies to: 46-46
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 13-13: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/LNDK_SETUP.md` at line 13, Add explicit language specifiers to the two
fenced code blocks that currently have none: change the block containing the
ASCII diagram "Buyer ---- Nostr ----> Mostro ----gRPC----> LNDK
----gRPC----> LND …" to start with ```text and change the block containing the
configuration snippet beginning with "[protocol]" to start with ```ini so both
blocks satisfy markdownlint MD040.
| # --- BOLT12 via LNDK (experimental, optional) --- | ||
| # Set to true to accept BOLT12 offers (lno1...) as buyer payout destinations. | ||
| # Requires running an LNDK daemon alongside LND. See docs/LNDK_SETUP.md. | ||
| lndk_enabled = false | ||
| # LNDK gRPC endpoint (must start with https://) | ||
| lndk_grpc_host = "https://127.0.0.1:7000" | ||
| # TLS certificate file (self-signed, generated by LNDK on first run) | ||
| lndk_cert_file = "/home/user/.lndk/data/tls-cert.pem" | ||
| # LND macaroon used by LNDK for payment authorization | ||
| lndk_macaroon_file = "/home/user/.lnd/data/chain/bitcoin/mainnet/admin.macaroon" | ||
| # Timeout for BOLT12 invoice fetch from the offer issuer (seconds) | ||
| lndk_fetch_invoice_timeout = 60 | ||
| # Fee limit for BOLT12 payments as percent. If unset, falls back to mostro.max_routing_fee. | ||
| # lndk_fee_limit_percent = 0.2 | ||
|
|
||
| # --- BIP-353 DNS resolution --- | ||
| # Resolves `user@domain` to BOLT12 offers via DNSSEC-validated DNS TXT | ||
| # records. Falls back to LNURL if resolution fails. Requires LNDK. | ||
| bip353_enabled = false | ||
| # DNS-over-HTTPS resolver (must support RFC 8484 JSON API) | ||
| bip353_doh_resolver = "https://1.1.1.1/dns-query" | ||
|
|
||
| # Skip DNSSEC validation for BIP-353. DANGER: regtest only — without | ||
| # DNSSEC an attacker controlling DNS can redirect payouts. | ||
| bip353_skip_dnssec = false | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether LightningSettings applies serde defaults to the new fields.
rg -nP --type=rust -C3 '\b(lndk_enabled|lndk_grpc_host|lndk_cert_file|lndk_macaroon_file|lndk_fetch_invoice_timeout|bip353_enabled|bip353_doh_resolver|bip353_skip_dnssec)\b' src/config/Repository: MostroP2P/mostro
Length of output: 3226
Backward-compat: new uncommented keys will break existing settings.toml files on upgrade.
Per project convention, all uncommented parameters in settings.tpl.toml are required in the operator's ~/.mostro/settings.toml for the daemon to start; serde defaults are not applied during parsing for backward compatibility. Five of the new uncommented keys lack #[serde(default)] in LightningSettings (src/config/types.rs: lndk_enabled, lndk_cert_file, lndk_macaroon_file, bip353_enabled, bip353_skip_dnssec), and will cause existing deployments to fail at startup until manually added, contradicting the PR objective that "older settings.toml parse unchanged."
Either:
- Comment-out these keys in the template (mirroring the
[anti_abuse_bond]pattern) so they're advertised but optional, or - Add
#[serde(default)]to the fields lacking it and document the upgrade path in the changelog.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@settings.tpl.toml` around lines 21 - 46, The template added required-looking
keys that will break existing configs; update LightningSettings in
src/config/types.rs to make the new fields optional by adding #[serde(default)]
to lndk_enabled, lndk_cert_file, lndk_macaroon_file, bip353_enabled, and
bip353_skip_dnssec (or alternatively revert these entries in settings.tpl.toml
to commented form like the [anti_abuse_bond] pattern); ensure the struct field
names exactly match those keys and add sensible Rust defaults if needed, and
document the chosen approach in the changelog.
| pub async fn resolve_bip353(address: &str) -> Result<Option<String>, MostroError> { | ||
| let ln = Settings::get_ln(); | ||
|
|
||
| if !ln.bip353_enabled || !ln.lndk_enabled { | ||
| return Ok(None); | ||
| } | ||
|
|
||
| let (user, domain) = match address.split_once('@') { | ||
| Some(pair) => pair, | ||
| None => return Ok(None), | ||
| }; | ||
|
|
||
| if user.is_empty() || domain.is_empty() { | ||
| return Ok(None); | ||
| } | ||
|
|
||
| let dns_name = build_dns_name(user, domain); | ||
| let resolver_url = &ln.bip353_doh_resolver; | ||
|
|
||
| let response = match query_doh(resolver_url, &dns_name).await { | ||
| Ok(resp) => resp, | ||
| Err(e) => { | ||
| tracing::warn!("BIP-353 DoH query failed for {address}: {e}"); | ||
| return Ok(None); | ||
| } | ||
| }; | ||
|
|
||
| // DNS status 0 = NOERROR | ||
| if response.status != 0 { | ||
| tracing::debug!( | ||
| "BIP-353 DNS returned status {} for {address}", | ||
| response.status | ||
| ); | ||
| return Ok(None); | ||
| } | ||
|
|
||
| // DNSSEC validation via the AD (Authenticated Data) flag | ||
| if !response.ad && !ln.bip353_skip_dnssec { | ||
| tracing::warn!( | ||
| "BIP-353 DNSSEC not validated for {address} (AD=false). \ | ||
| Set bip353_skip_dnssec=true to override (regtest only)." | ||
| ); | ||
| return Ok(None); | ||
| } | ||
|
|
||
| // Extract lno offer from TXT records | ||
| for answer in &response.answer { | ||
| if let Some(offer) = parse_bitcoin_uri(&answer.data) { | ||
| if offer.starts_with("lno1") { | ||
| tracing::info!("BIP-353 resolved {address} → bolt12 offer"); | ||
| return Ok(Some(offer)); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| tracing::debug!("BIP-353 no lno offer found in TXT records for {address}"); | ||
| Ok(None) | ||
| } |
There was a problem hiding this comment.
Privacy: avoid logging the full BIP-353 address (user identifier).
The address parameter is essentially a payment-email (user@domain) — a user identifier. The function logs it verbatim at info!, warn!, and debug! levels:
- L49:
BIP-353 DoH query failed for {address}: {e} - L57:
BIP-353 DNS returned status {} for {address} - L66:
BIP-353 DNSSEC not validated for {address} (AD=false)… - L76:
BIP-353 resolved {address} → bolt12 offer - L82:
BIP-353 no lno offer found in TXT records for {address}
In production these logs end up captured by aggregators and tied to other order metadata, which is exactly the PII-retention shape we want to avoid. Consider logging only the domain (which is necessary context for diagnosing DoH issues) and either hashing the local-part or omitting it entirely.
🛡️ Suggested approach
- let (user, domain) = match address.split_once('@') {
+ let (_user, domain) = match address.split_once('@') {
Some(pair) => pair,
None => return Ok(None),
};
…
- tracing::warn!("BIP-353 DoH query failed for {address}: {e}");
+ tracing::warn!(domain = %domain, "BIP-353 DoH query failed: {e}");(Apply the same domain = %domain pattern to the other tracing calls.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lightning/bip353.rs` around lines 27 - 84, The logs in resolve_bip353
currently emit the full BIP-353 address (user@domain); change all tracing calls
to avoid logging the raw local-part by only including the domain (domain) and
either a short deterministic hash or redact of the user (e.g. hash of user or
"[redacted_user]") instead of address. Update the tracing::warn/info/debug calls
that currently reference {address} (the DoH failure, DNS status, DNSSEC warning,
resolved bolt12, and no-offer debug messages) to include domain=%domain and
user_hash=%user_hash (or user="[redacted]") so diagnostics keep domain context
but do not expose the plain user identifier; compute user_hash from the local
variable user before logging.
| let pr = if pr.contains('@') && !pr.contains(' ') { | ||
| match crate::lightning::bip353::resolve_bip353(&pr).await { | ||
| Ok(Some(offer)) => { | ||
| tracing::info!("BIP-353 DNS: {pr} → bolt12 offer"); | ||
| offer | ||
| } | ||
| Ok(None) => pr, | ||
| Err(e) => { | ||
| tracing::warn!("BIP-353 DNS error for {pr}: {e}"); | ||
| pr | ||
| } | ||
| } | ||
| } else { | ||
| pr | ||
| }; |
There was a problem hiding this comment.
Avoid logging the full user@domain payout address at info/warn level.
tracing::info!("BIP-353 DNS: {pr} → bolt12 offer") and the corresponding warn line emit the buyer's BIP-353 address (a user-identifying payment endpoint) into structured logs that may be aggregated, retained, or shipped off-host. A buyer who reports a payout problem becomes pseudonymously linkable across orders via these log lines. Since this is on the hot path of every payout submission, the volume is non-trivial.
Consider one of:
- Redact the local-part (e.g., log only
…@domain). - Log a stable hash (e.g., SHA-256 first 8 bytes) instead of the raw address.
- Drop to
debuglevel so it isn't enabled in production by default.
🛡️ Sketch
- Ok(Some(offer)) => {
- tracing::info!("BIP-353 DNS: {pr} → bolt12 offer");
+ Ok(Some(offer)) => {
+ let domain = pr.rsplit_once('@').map(|(_, d)| d).unwrap_or("?");
+ tracing::info!("BIP-353 DNS resolved address @{domain} → bolt12 offer");
offer
}
Ok(None) => pr,
Err(e) => {
- tracing::warn!("BIP-353 DNS error for {pr}: {e}");
+ let domain = pr.rsplit_once('@').map(|(_, d)| d).unwrap_or("?");
+ tracing::warn!("BIP-353 DNS error for @{domain}: {e}");
pr
}📝 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.
| let pr = if pr.contains('@') && !pr.contains(' ') { | |
| match crate::lightning::bip353::resolve_bip353(&pr).await { | |
| Ok(Some(offer)) => { | |
| tracing::info!("BIP-353 DNS: {pr} → bolt12 offer"); | |
| offer | |
| } | |
| Ok(None) => pr, | |
| Err(e) => { | |
| tracing::warn!("BIP-353 DNS error for {pr}: {e}"); | |
| pr | |
| } | |
| } | |
| } else { | |
| pr | |
| }; | |
| let pr = if pr.contains('@') && !pr.contains(' ') { | |
| match crate::lightning::bip353::resolve_bip353(&pr).await { | |
| Ok(Some(offer)) => { | |
| let domain = pr.rsplit_once('@').map(|(_, d)| d).unwrap_or("?"); | |
| tracing::info!("BIP-353 DNS resolved address @{domain} → bolt12 offer"); | |
| offer | |
| } | |
| Ok(None) => pr, | |
| Err(e) => { | |
| let domain = pr.rsplit_once('@').map(|(_, d)| d).unwrap_or("?"); | |
| tracing::warn!("BIP-353 DNS error for @{domain}: {e}"); | |
| pr | |
| } | |
| } | |
| } else { | |
| pr | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/util.rs` around lines 1251 - 1265, The logs in the BIP-353 resolution
branch leak the full user@domain payout address via tracing::info! and
tracing::warn!; update the logging in the block that calls
crate::lightning::bip353::resolve_bip353(&pr).await so it does not emit the raw
pr value—either redact the local-part (log only "@domain"), or log a stable
short hash (e.g., SHA-256 first 8 bytes) of pr, or lower the level to debug;
change the tracing::info! and tracing::warn! calls that reference {pr} to use
the chosen non-identifying representation instead while leaving the rest of the
control flow and returned value (offer or pr) unchanged.
grunch
requested review from
AndreaDiazCorreia,
BraCR10,
Catrya,
arkanoider and
grunch
1 participant
Related Issue
Summary
Two features that let Mostro accept modern Bitcoin payment identifiers
as buyer payout destinations without changing any of the existing
BOLT11 / LNURL / Lightning-Address flow:
string (
lno1...) as their payout target. Mostro routes the paymentthrough an optional LNDK daemon
running alongside LND.
user@domain.tldpayment address. Mostro resolves it viaDNSSEC-validated DNS TXT records over DNS-over-HTTPS, replaces the
address with the resolved BOLT12 offer, and the BOLT12 path takes
over. On any failure, the LNURL fallback handles it, so Lightning
Addresses keep working as before.
Both features are opt-in and disabled by default. When disabled, the
new code paths are inert and BOLT12 offers are rejected at validation
with a clear error.
Motivation
BOLT12 closes UX gaps in BOLT11: offers are reusable (no per-retry
invoice), receiver identity is hidden via blinded paths, and BIP-353
turns
alice@example.cominto a real payment identifier rather than anLNURL UI hack. Mostro's escrow flow keeps using BOLT11 hold invoices
(BOLT12 has no hold-invoice equivalent), so this PR only touches the
buyer payout step. That is the one place in the protocol where the
destination is chosen by an external party rather than the daemon.
LNDK is the standard sidecar approach for BOLT12 with LND today: LDK
implements offers, LND provides routing and onion-message forwarding,
LNDK glues them together. Going via LNDK avoids forking LND or building
a parallel routing stack.
Architecture
do_paymentbecomes a thin dispatch over a newclassify()that routesBOLT12 offers to a new
do_payment_bolt12(LNDK path) and everythingelse to a renamed
do_payment_lndwhose body is byte-for-byteidentical to the prior
do_payment. The BOLT11/LNURL/LN-Address pathis intentionally untouched.
For BIP-353,
validate_invoiceresolvesuser@domainto a BOLT12 offerbefore validation; the resolved offer is written back so the downstream
payout path sees the offer instead of the alias. The DoH client reuses
the existing
reqwestHTTP client; no new resolver dependency.For BOLT12,
pay_offer_validated()does a defensive two-stepGetInvoice → validate → PayInvoice. We don't use LNDK's one-shotPayOfferbecause it doesn't verify the fetched invoice's amount orexpiry against what the caller asked for.
Configuration
All new fields live in the existing
[lightning]block. The structgains a struct-level
#[serde(default)]so older settings.toml filesparse cleanly.
Operator setup details are in
docs/LNDK_SETUP.md(LND build tags,custom-message flags, LNDK install, macaroon baking, troubleshooting
table, BIP-353 resolver setup).
Backward compatibility
false, behavior is identical tocurrent Mostro: BOLT11/LNURL/LN-Address only, no LNDK process required.
the existing LNURL-pay path; existing Lightning Addresses keep working.
lni1…) are rejected at validationrather than failing later in payout.
Scope and limitations
Intentionally deferred to follow-ups: BOLT12 offer creation (dev-fee
receipt still uses BOLT11), BOLT12-aware retry behavior (today it shares
the BOLT11 retry loop), and any change to the seller→Mostro escrow leg
(BOLT12 has no hold-invoice equivalent).
Test plan
Automated
cargo test: 261 passed, 0 failed (rebased on top ofmain).cargo clippy --all-targets --all-features: clean.cargo fmt --check: clean.bip353(DoH JSON, DNS-name construction, URIparsing including chunked-TXT, DNSSEC AD-flag enforcement) and
offers(classifyover each request shape,validate_offerrejection cases,
validate_fetched_invoiceamount/expiry/sanitychecks).
End-to-end regtest
Topology: bitcoind regtest, two LND nodes (Mostro + seller), LNDK v0.3.0
sidecar on the Mostro LND, Core Lightning node as the BOLT12 offer
issuer peered with both LNDs, and a mock DoH server answering
regtest.user._bitcoin-payment.mostro.localwith a freshly-minted CLNoffer and
AD: true.Scenario A: BIP-353 + BOLT12 (order
ff7caa31)regtest@mostro.local.BIP-353 resolved regtest@mostro.local → bolt12 offerand persists the resolved
lno1...offer as the order'sbuyer_invoice.release, Mostrod callsLNDK::GetInvoice → validate → PayInvoice.peered with CLN, so the BOLT12 reply path through it was broken).
After peering them, the retry loop kicked in automatically and the
second attempt succeeded with
BOLT12 offer paid, preimage=0cad817d….lnd → clnshifted by exactly 100_000 sats,CLN's
our_amount_msatincreased by 100_000_000 msat.This exercised DoH resolution, DNSSEC AD check,
buyer_invoicerewrite,the BOLT12 dispatch in
do_payment, the GetInvoice→validate→PayInvoiceflow, and retry-on-timeout behavior.
Scenario B: BOLT11 regression (order
7bf442c0)lnbcrt500u…invoice on the seller LND.releasetriggersdo_payment_lnd.Order…: Invoice with hash: b21a252e… paid!andtransitions to
success. No BOLT12 path touched.This confirms the pre-existing BOLT11 / LNURL / LN-Address path is
untouched.
Summary by CodeRabbit
Release Notes
New Features
Documentation