Add BOLT12 swap support via Boltz API#197
Conversation
Add support for BOLT12 submarine swaps (paying offers) and the building blocks for BOLT12 reverse swaps (offer registration). This includes: - LnInvoice enum supporting both BOLT11 and BOLT12 invoices with backward-compatible serde for existing stored swap data - fetch_bolt12_invoice: resolve BOLT12 offers into invoices via Boltz - prepare_bolt12_offer_payment / pay_bolt12_offer: submarine swaps using BOLT12 offers - register_bolt12_offer / update_bolt12_webhook: register offers with Boltz for reverse swap invoice request delivery - Minimal BOLT12 invoice TLV parser to extract payment hashes without requiring heavy LDK dependencies - Bolt12InvoiceRequestPayload type for webhook/WebSocket integration Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds BOLT12 invoice support: a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant BoltzAPI as BoltzAPI
participant Parser as Parser
participant Storage as Storage
Client->>BoltzAPI: fetch_bolt12_invoice / prepare_bolt12_offer_payment / pay_bolt12_offer / register_bolt12_offer
BoltzAPI-->>Client: invoice string / payment responses
Client->>Parser: extract_bolt12_payment_hash(invoice)
Parser-->>Client: payment_hash (32 bytes)
Client->>Storage: persist SubmarineSwapData(invoice: LnInvoice::Bolt12 | LnInvoice::Bolt11)
Storage-->>Client: ack
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
🔍 Arkana PR Review — BOLT12 Swap SupportOverviewThis PR adds BOLT12 offer payment support (submarine swaps) and BOLT12 reverse swap offer registration to the Rust SDK, interfacing with Boltz's BOLT12 API endpoints. It introduces an Good: well-documented, solid test coverage for the TLV parser and serde logic, and the backward-compatibility approach via Security Findings1. No invoice-to-offer binding validation in
If the Boltz API is compromised or MITM'd (even with TLS, consider supply-chain scenarios), a rogue invoice could redirect the payment. Consider adding at minimum a warning in the 2. The 3. BOLT12 TLV streams are required to have monotonically increasing type numbers. The parser doesn't enforce this, which is fine for extraction purposes but means it would accept malformed invoices. Not a security issue for payment hash extraction specifically, just noting it. Code Quality4. Significant code duplication between These two methods share ~80 lines of identical logic (invoice fetch, swap creation, storage insert). Suggested refactor: pub async fn pay_bolt12_offer(&self, offer: &str, amount_sats: Option<u64>) -> Result<Bolt12SubmarineSwapResult, Error> {
let swap_data = self.prepare_bolt12_offer_payment(offer, amount_sats).await?;
let txid = self.send_vtxo(swap_data.vhtlc_address, swap_data.amount).await?;
// ...
}5. No Each BOLT12 method creates a new 6. The only difference is Protocol Correctness
Cross-Repo Impact
SummarySolid foundational work for BOLT12 support. The main actionable item is the code duplication between |
- Fix dprint formatting (doc comment line wrapping) - Fix sqlite test to use LnInvoice::Bolt11 wrapper for invoice field Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ark-client/src/boltz.rs`:
- Around line 1898-1902: The pay_bolt12_offer flow currently commits funds
immediately after bolt12_fetch, which allows a malicious fetch endpoint to
substitute an invoice it controls; update pay_bolt12_offer to first verify the
fetched BOLT12 invoice is bound to the original Offer by checking the invoice's
signing key against the Offer's signing key (or against the public key of the
final hop from the Offer's message paths) and only commit/lock funds after that
verification succeeds; likewise apply the same validation to the related
functions/flows in the 2058-2157 range to ensure no other one-shot APIs fund
before signature/offer-binding is validated, or remove the one-shot commit
behavior until this check is implemented.
- Around line 1925-1932: The BOLT12 HTTP paths are creating an unconfigured
reqwest client (reqwest::Client::new()) which ignores the timeout in
self.inner.timeout; replace those calls by using the same timeout-configured
client used by get_fees/get_limits (or introduce a small helper like
make_timeout_client(&self.inner.timeout)) and use that client in
fetch_bolt12_invoice, prepare_bolt12_offer_payment, pay_bolt12_offer,
register_bolt12_offer, and update_bolt12_webhook so requests respect the
configured timeout and reuse shared setup.
- Around line 3090-3109: The TLV length is currently truncated with `as` and
then added to `cursor` unsafely; replace the unsafe cast and unchecked addition
by first converting `tlv_len` (u64) to usize via `try_into()` and returning an
error if it fails, and compute the end index using
`cursor.checked_add(tlv_len_usize)` to detect overflow before slicing `data`;
apply these checks in the parsing block that handles
`BOLT12_PAYMENT_HASH_TLV_TYPE` (and the surrounding TLV loop) so you only slice
`data[cursor..end]` after a successful `try_into()` and `checked_add()`,
returning the existing ad_hoc error on failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7c731fdf-2ca5-49b3-8b35-4630977a9297
📒 Files selected for processing (2)
ark-client/src/boltz.rsark-client/src/lib.rs
🔍 Arkana PR Review — Follow-up (CI fixes)New commit: Changes Since Last ReviewFormatting-only changes (doc comment line wrapping for dprint) and a test fix wrapping a bare Assessment✅ No functional changes — purely mechanical fixes to pass CI. The sqlite test update is correct and consistent with the type change in the main commit. No new security concerns. |
- Document trust model for BOLT12 offer payment methods (invoice is fetched via Boltz and not locally verified against offer signing key, matching existing BOLT11 trust model) - Apply configured timeout to all BOLT12 HTTP clients via reqwest::Client::builder().timeout(self.inner.timeout) - Use try_into/checked_add for TLV length parsing to prevent truncation on 32-bit platforms and overflow in cursor arithmetic Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
🔍 Follow-up Review (commit
|
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ark-client/src/boltz.rs`:
- Around line 79-85: The current #[serde(untagged)] LnInvoice enum lets any
string become LnInvoice::Bolt12, so change to a custom Deserialize impl for
LnInvoice that first attempts to parse the input string as a Bolt11Invoice
(using s.parse::<Bolt11Invoice>() or equivalent) and returns LnInvoice::Bolt11
on success; if that fails, validate the string is a well-formed BOLT12 by
calling the existing extract_bolt12_payment_hash(&s) (or the project’s BOLT12
validation function) and return LnInvoice::Bolt12(s) only if validation
succeeds, otherwise return a serde::de::Error::custom to preserve the previous
fail-fast behavior; remove or replace the #[serde(untagged)] annotation and
ensure payment_hash()/other callers still work with the validated enum.
- Around line 133-140: The Bolt12InvoiceRequestPayload struct is missing the
WebSocket request identifier required for responding to `invoice.request`;
update the exported type Bolt12InvoiceRequestPayload to include a public id
field (e.g., pub id: String) alongside the existing offer and invoice_request
fields so the WebSocket `id` is deserialized/serialized with the struct (serde
rename_all = "camelCase" will handle the field name).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 38864820-fdac-4e55-a84e-446248c98789
📒 Files selected for processing (1)
ark-client/src/boltz.rs
…t id - Replace serde(untagged) Deserialize with custom impl that validates Bolt12 invoice strings by checking payment hash extractability, rejecting garbage strings at deser time instead of later at use - Add optional `id` field to Bolt12InvoiceRequestPayload for WebSocket invoice.request messages (required when replying to Boltz via WS) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
🔍 Arkana PR Review —
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ark-client/src/boltz.rs`:
- Around line 3188-3218: The parser currently accepts non-canonical BigSize
encodings; update read_bigsize to reject oversized encodings by validating
minimality: after decoding val in the 0xfd branch (bytes with tag 0xfd) ensure
val >= 0xfd (i.e. > 0xfc) otherwise return an error; in the 0xfe branch ensure
val >= 0x10000 (i.e. > 0xffff) otherwise return an error; in the 0xff branch
ensure val >= 0x1_0000_0000 (i.e. > 0xffffffff) otherwise return an error; keep
the same error style and reference the same variables (data, val) and match arms
(0xfd, 0xfe, 0xff) in read_bigsize.
- Around line 3140-3178: The helper currently returns immediately on the first
TLV type 168, allowing payloads with valid-looking payment_hash followed by
truncated/garbage TLVs to be accepted; change it to fully validate the entire
TLV stream and only accept the invoice if all TLVs parse correctly and a
payment_hash was seen. Concretely, in the loop in boltz.rs (the TLV-parsing
helper used by LnInvoice::deserialize) do not return inside the tlv_type ==
BOLT12_PAYMENT_HASH_TLV_TYPE branch; instead, store the parsed 32-byte
payment_hash (e.g., into an Option<[u8;32]> or sha256::Hash variable) and
continue parsing the remaining TLVs with the same length/overflow checks; after
the loop, if the stored payment_hash exists return it, otherwise return an error
about missing payment_hash. This also ensures later payment_hash entries
override earlier ones if desired and that the entire TLV stream is validated
before accepting the invoice.
- Around line 1981-1989: fetch_bolt12_invoice currently returns whatever string
comes back from the bolt12_fetch response without validating it; update
fetch_bolt12_invoice to validate the returned invoice string before returning
(similar to the checks done in prepare_bolt12_offer_payment/pay_bolt12_offer) by
parsing/decoding the invoice (use the existing bolt12 invoice parsing/validation
routine or call the same validation helper those functions use) and return an
error (wrapped with context) if parsing/validation fails so callers cannot
persist or use an invalid invoice.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ba8cd0a0-21b6-480e-a09a-9ceb9d253967
📒 Files selected for processing (1)
ark-client/src/boltz.rs
- Extract `create_bolt12_submarine_swap` private method to deduplicate code between `prepare_bolt12_offer_payment` and `pay_bolt12_offer` - Add breaking-change marker on `LnInvoice::Bolt12(String)` variant Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Addressed the arkana review feedback in f457840:
|
🔍 Iterative Review — Refactor + Breaking Change Marker (
|
There was a problem hiding this comment.
♻️ Duplicate comments (3)
ark-client/src/boltz.rs (3)
3086-3128:⚠️ Potential issue | 🟠 MajorParse the full TLV stream before accepting this invoice.
LnInvoice::deserializenow relies on this helper as its BOLT12 validation boundary. Returning on the first type168lets payloads with a valid-lookingpayment_hashand trailing truncated or garbage TLVs deserialize successfully.Suggested fix
- let mut cursor = 0; + let mut cursor = 0; + let mut payment_hash = None; while cursor < data.len() { let (tlv_type, consumed) = read_bigsize(&data[cursor..]).map_err(|e| { Error::ad_hoc(format!("failed to read TLV type in bolt12 invoice: {e}")) })?; cursor += consumed; @@ if tlv_type == BOLT12_PAYMENT_HASH_TLV_TYPE { if tlv_len != 32 { return Err(Error::ad_hoc(format!( "unexpected bolt12 payment_hash length: expected 32, got {tlv_len}" ))); } let hash_bytes: [u8; 32] = data[cursor..end] .try_into() .map_err(|_| Error::ad_hoc("failed to convert payment_hash bytes"))?; - return Ok(sha256::Hash::from_byte_array(hash_bytes)); + if payment_hash + .replace(sha256::Hash::from_byte_array(hash_bytes)) + .is_some() + { + return Err(Error::ad_hoc("duplicate payment_hash TLV in bolt12 invoice")); + } } cursor = end; } - Err(Error::ad_hoc( - "payment_hash (TLV type 168) not found in bolt12 invoice", - )) + payment_hash.ok_or_else(|| { + Error::ad_hoc("payment_hash (TLV type 168) not found in bolt12 invoice") + }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ark-client/src/boltz.rs` around lines 3086 - 3128, The code currently returns immediately when encountering BOLT12_PAYMENT_HASH_TLV_TYPE inside this TLV parsing loop, which allows invoices with a valid-looking payment_hash followed by truncated or garbage TLVs to pass; change the logic in this parsing block (used by LnInvoice::deserialize) to parse the entire TLV stream before accepting the invoice: instead of returning on finding type 168, record the 32-byte payment_hash (e.g. set a local Option or store bytes) and continue parsing until cursor reaches the end (ensuring all TLV lengths/consumed checks via read_bigsize and boundary checks still apply); after the loop, if the payment_hash was found and the whole stream parsed successfully, construct and return the sha256::Hash, otherwise return the existing "payment_hash ... not found" error.
1982-1989:⚠️ Potential issue | 🟠 MajorValidate the fetched invoice before returning it.
This public helper still treats any
invoicestring frombolt12_fetchas success.create_bolt12_submarine_swapre-checks it later, but direct callers cannot access the private validator and can persist garbage that only fails much later.Suggested fix
let response: Bolt12FetchInvoiceResponse = response .json() .await .map_err(|e| Error::ad_hoc(e.to_string())) .context("failed to deserialize bolt12 fetch response")?; + extract_bolt12_payment_hash(&response.invoice) + .context("bolt12_fetch returned invalid BOLT12 invoice")?; tracing::info!("Fetched BOLT12 invoice from offer"); Ok(response.invoice)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ark-client/src/boltz.rs` around lines 1982 - 1989, The code returns response.invoice unchecked; ensure the fetched invoice string is validated before returning by reusing or extracting the same validation logic used in create_bolt12_submarine_swap: call the existing private validator (or extract it as a public/accessible helper, e.g., validate_bolt12_invoice) to parse/verify the Bolt12 string from Bolt12FetchInvoiceResponse and return an error if parsing/validation fails (non-empty, correct format, required fields present) instead of returning raw response.invoice.
3134-3164:⚠️ Potential issue | 🟡 MinorReject non-canonical BigSize encodings here.
Inputs like
0xfd 00 a8are malformed BigSize values, butread_bigsizecurrently accepts them. Since this parser now backs deserialization-time validation, those malformed TLVs are still treated as valid invoices.Suggested fix
0xfd => { if data.len() < 3 { return Err("unexpected end of data for 2-byte BigSize"); } let val = u16::from_be_bytes([data[1], data[2]]) as u64; + if val < 0xfd { + return Err("non-canonical 2-byte BigSize"); + } Ok((val, 3)) } 0xfe => { if data.len() < 5 { return Err("unexpected end of data for 4-byte BigSize"); } let val = u32::from_be_bytes([data[1], data[2], data[3], data[4]]) as u64; + if val < 0x1_0000 { + return Err("non-canonical 4-byte BigSize"); + } Ok((val, 5)) } 0xff => { if data.len() < 9 { return Err("unexpected end of data for 8-byte BigSize"); } let val = u64::from_be_bytes([ data[1], data[2], data[3], data[4], data[5], data[6], data[7], data[8], ]); + if val < 0x1_0000_0000 { + return Err("non-canonical 8-byte BigSize"); + } Ok((val, 9)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ark-client/src/boltz.rs` around lines 3134 - 3164, read_bigsize currently accepts non-canonical encodings (e.g., 0xfd 00 a8); update read_bigsize to reject these by validating canonical ranges: in the 0xfd arm return Err when val < 0xfd, in the 0xfe arm return Err when val < 0x10000, and in the 0xff arm return Err when val < 0x1_0000_0000; keep the existing length checks and return types but replace acceptance of too-small values with a clear Err (e.g., "non-canonical BigSize encoding") so malformed TLVs are rejected during deserialization.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@ark-client/src/boltz.rs`:
- Around line 3086-3128: The code currently returns immediately when
encountering BOLT12_PAYMENT_HASH_TLV_TYPE inside this TLV parsing loop, which
allows invoices with a valid-looking payment_hash followed by truncated or
garbage TLVs to pass; change the logic in this parsing block (used by
LnInvoice::deserialize) to parse the entire TLV stream before accepting the
invoice: instead of returning on finding type 168, record the 32-byte
payment_hash (e.g. set a local Option or store bytes) and continue parsing until
cursor reaches the end (ensuring all TLV lengths/consumed checks via
read_bigsize and boundary checks still apply); after the loop, if the
payment_hash was found and the whole stream parsed successfully, construct and
return the sha256::Hash, otherwise return the existing "payment_hash ... not
found" error.
- Around line 1982-1989: The code returns response.invoice unchecked; ensure the
fetched invoice string is validated before returning by reusing or extracting
the same validation logic used in create_bolt12_submarine_swap: call the
existing private validator (or extract it as a public/accessible helper, e.g.,
validate_bolt12_invoice) to parse/verify the Bolt12 string from
Bolt12FetchInvoiceResponse and return an error if parsing/validation fails
(non-empty, correct format, required fields present) instead of returning raw
response.invoice.
- Around line 3134-3164: read_bigsize currently accepts non-canonical encodings
(e.g., 0xfd 00 a8); update read_bigsize to reject these by validating canonical
ranges: in the 0xfd arm return Err when val < 0xfd, in the 0xfe arm return Err
when val < 0x10000, and in the 0xff arm return Err when val < 0x1_0000_0000;
keep the existing length checks and return types but replace acceptance of
too-small values with a clear Err (e.g., "non-canonical BigSize encoding") so
malformed TLVs are rejected during deserialization.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b9a607d0-4c2d-42aa-9ce2-eb64970c6fcf
📒 Files selected for processing (1)
ark-client/src/boltz.rs
There was a problem hiding this comment.
Blocking issues found.
pay_bolt12_offerstill funds a server-fetched invoice without proving that it belongs to the original offer. Boltz's own BOLT12 docs require verifying that binding before using the invoice.fetch_bolt12_invoicereturns arbitrary server output without even local BOLT12 validation.extract_bolt12_payment_hashstops at the firstpayment_hashTLV, so malformed trailing TLVs still passLnInvoicedeserialization.
cargo test -p ark-client passes locally on the PR head.
🔍 Arkana Review — rust-sdk#197 (updated)Add BOLT12 swap support via Boltz API by @vincenzopalazzo SummaryIntroduces BOLT12 invoice and offer support for submarine swaps alongside existing BOLT11 in the Rust SDK. This is a significant feature addition (~780 lines) touching the core swap data model. Key Changes
Security Analysis✅ Payment hash validation at deserialization — ✅ TLV parser safety — Bounds checking on all reads, non-canonical BigSize rejection, overflow protection via
✅ Backward compatibility — ✅ Timeout hardening — The Test Coverage
Cross-Repo Impact
SuggestionThe Overall: Well-structured addition with good documentation of trust boundaries and clear TODO for future LDK integration. The minimal TLV parser is a pragmatic choice that avoids pulling in heavy dependencies. |
1 participant
Summary
LnInvoiceenum (Bolt11/Bolt12) with backward-compatible serde, replacing theBolt11Invoicetype onSubmarineSwapData.invoicefetch_bolt12_invoiceto resolve BOLT12 offers into invoices viaPOST /lightning/BTC/bolt12_fetchprepare_bolt12_offer_paymentandpay_bolt12_offerfor submarine swaps paying BOLT12 offersregister_bolt12_offerandupdate_bolt12_webhookfor BOLT12 reverse swap offer registrationextract_bolt12_payment_hash) to extract payment hashes without heavy LDK dependenciesBolt12InvoiceRequestPayloadandBolt12InvoiceParamstypes for webhook/WebSocket integrationNotes
SubmarineSwapData(with bare Bolt11 invoice strings) deserializes correctly viaserde(untagged)-- verified by testsfetch_bolt12_invoicedirectlyTest plan
test_read_bigsize)LnInvoiceserde roundtrip (Bolt11, Bolt12, backward compat)cargo checkclean,cargo test -p ark-clientall 12 tests pass🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests