feat(boltz): pass invoice description through to reverse swaps#226
Conversation
WalkthroughThis PR extends reverse submarine swap invoice creation with optional BOLT11 description support. The change introduces a 639-byte UTF-8 validation constraint, updates both invoice generation methods to accept and forward descriptions to Boltz, and adapts sample and test code to the expanded API. ChangesInvoice Description Support for Reverse Swaps
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ark-client/src/boltz.rs (1)
4470-4510: ⚡ Quick winAdd explicit JSON assertions for the new
descriptionwire field.The reverse request tests now populate
description: None, but they still only assert referral behavior. Please also assertdescriptionomission/presence so schema regressions are caught.🧪 Suggested test additions
#[test] fn reverse_swap_request_omits_referral_id_when_none() { let request = CreateReverseSwapRequest { from: Asset::Btc, to: Asset::Ark, invoice_amount: Some(Amount::from_sat(1000)), onchain_amount: None, claim_public_key: PublicKey::from_str( "02c6047f9441ed7d6d3045406e95c07cd85c778e4b8cef3ca7abac09b95c709ee5", ) .unwrap(), preimage_hash: sha256::Hash::from_byte_array([1u8; 32]), invoice_expiry: Some(3600), referral_id: None, description: None, }; let json: serde_json::Value = serde_json::to_value(&request).unwrap(); assert!(json.get("referralId").is_none()); assert!(json.get("referral_id").is_none()); + assert!(json.get("description").is_none()); } + +#[test] +fn reverse_swap_request_serializes_description_when_set() { + let request = CreateReverseSwapRequest { + from: Asset::Btc, + to: Asset::Ark, + invoice_amount: Some(Amount::from_sat(1000)), + onchain_amount: None, + claim_public_key: PublicKey::from_str( + "02c6047f9441ed7d6d3045406e95c07cd85c778e4b8cef3ca7abac09b95c709ee5", + ) + .unwrap(), + preimage_hash: sha256::Hash::from_byte_array([1u8; 32]), + invoice_expiry: Some(3600), + referral_id: None, + description: Some("memo".to_string()), + }; + + let json: serde_json::Value = serde_json::to_value(&request).unwrap(); + assert_eq!(json["description"], "memo"); +}🤖 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 `@ark-client/src/boltz.rs` around lines 4470 - 4510, The tests for CreateReverseSwapRequest currently set description: None but never assert its JSON presence; update reverse_swap_request_omits_referral_id_when_none to also assert json.get("description").is_none() (and json.get("description") vs any underscore variant if applicable), and update or add a test (e.g., extend reverse_swap_request_serializes_referral_id_when_set or add reverse_swap_request_serializes_description_when_set) that constructs CreateReverseSwapRequest with description: Some("...".to_string()) and asserts serde_json::to_value(&request)["description"] equals that string; reference CreateReverseSwapRequest, reverse_swap_request_serializes_referral_id_when_set, and reverse_swap_request_omits_referral_id_when_none to locate where to add these assertions.
🤖 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.
Nitpick comments:
In `@ark-client/src/boltz.rs`:
- Around line 4470-4510: The tests for CreateReverseSwapRequest currently set
description: None but never assert its JSON presence; update
reverse_swap_request_omits_referral_id_when_none to also assert
json.get("description").is_none() (and json.get("description") vs any underscore
variant if applicable), and update or add a test (e.g., extend
reverse_swap_request_serializes_referral_id_when_set or add
reverse_swap_request_serializes_description_when_set) that constructs
CreateReverseSwapRequest with description: Some("...".to_string()) and asserts
serde_json::to_value(&request)["description"] equals that string; reference
CreateReverseSwapRequest, reverse_swap_request_serializes_referral_id_when_set,
and reverse_swap_request_omits_referral_id_when_none to locate where to add
these assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2d3363e8-0626-4a1e-baa4-3399bdb8e0ed
📒 Files selected for processing (3)
ark-client-sample/src/main.rsark-client/src/boltz.rse2e-tests/tests/boltz_reverse.rs
There was a problem hiding this comment.
✅ Approved — clean, well-bounded feature addition
Reviewed: full diff + full file context for ark-client/src/boltz.rs, ark-client-sample/src/main.rs, e2e-tests/tests/boltz_reverse.rs. Cross-repo impact checked.
Summary
Adds an optional description: Option<String> parameter to get_ln_invoice and get_ln_invoice_from_hash, threaded through to the Boltz /v2/swap/reverse API as a JSON field. The CreateReverseSwapRequest struct gains the field with skip_serializing_if = "Option::is_none" so existing callers passing None produce identical wire format.
Findings
No protocol-critical concerns. This change does not touch VTXOs, transaction signing, forfeit paths, round lifecycle, connector trees, or exit paths. It only adds an optional metadata field to the Boltz reverse swap HTTP request.
Correctness: ✅
- Validation constant
MAX_BOLT11_DESCRIPTION_BYTES = 639is correctly derived from BOLT11 spec (10-bit length in 5-bit groups →floor(1023 * 5 / 8) = 639). validate_invoice_descriptioncorrectly uses.len()(byte length) which matches the BOLT11 byte limit.- Validation runs before any network call — fail-fast, good.
serde(rename_all = "camelCase")onCreateReverseSwapRequestwon't renamedescription(single word) — wire format is correct, confirmed againstboltz-swapTS package (types.ts:73).skip_serializing_if = "Option::is_none"ensures backward-compatible JSON whenNone.
Test coverage: ✅
- Unit tests cover
None, empty string, at-limit, and over-limit cases forvalidate_invoice_description. - Existing serialization tests updated with
description: None. - E2E test updated to compile with new signature.
One minor note (non-blocking): The validate_invoice_description uses .len() which counts UTF-8 bytes — this is correct for the BOLT11 byte limit, but worth noting that a multi-byte character could be split at the boundary. Since the BOLT11 spec operates on bytes and the string is passed to Boltz (which generates the invoice), Boltz will ultimately enforce encoding. No action needed.
⚠️ Cross-repo breakage
ark-flutter-example (rust/src/ark/client.rs:178) calls client.get_ln_invoice(SwapAmount::..., None) with 2 args. After this PR merges, that call will fail to compile. The fix is trivial (add , None as third arg), but someone should open a follow-up PR or coordinate the update.
No other external consumers found. three-three-slot-machine and ark-rs-flutter-example depend on ark-client but don't call these methods.
No merge conflicts with open PRs (#224 BOLT12, #210 version header, #166 wallet traits, #161 boarding balance).
🤖 Reviewed by Arkana
Summary by CodeRabbit