Add BOLT12 support

[tnull]

Based on #243.
Based on #244.
Based on #266.
Based on #270.

WIP, hence draft.

Ready for review.

[tnull]

Rebased on #266.

[tnull]

Addressed the open feedback, @jkczyz let me know if I can squash.

[jkczyz]

Addressed the open feedback, @jkczyz let me know if I can squash.

Yes, please squash

[tnull]

Addressed the open feedback, @jkczyz let me know if I can squash.

Yes, please squash

Squashed fixups without further changes.

[jkczyz]

Wondering if there's a better way of writing these docs. For Bolt12Offer, it's really a payment for an offer (in response to receiving a requested invoice for the offer). For Bolt12Refund, while there isn't a prior offer, there is a refund that initiated the process. Saying "without a prior offer" makes it sound almost like a spontaneous payment.

[tnull]

Yeah, I wondered the same too. I previously avoided 'for an offer' as it's really 'for the offered goods'/for an offering'. Still not sure how write about this best. For now I added some changes rather linking to Offer/Refund, but this is a bit of a cop-out tbh. We should revisit this further in the future, in particular if/when we need to make changes toward manual InvoiceRequest` handling, since then it might become apparent to which degree we have to expose LDK Node users to the intricacies of the BOLT12 flow.

Btw now also renamed offer_refund as it might be even more confusing, although I technically like it better than create_refund...

[jkczyz]

How about initiate_refund for creation and possibly request_payment for requesting? Using request_refund makes it seem like it comes before create_refund, even though the order is reversed.

[tnull]

Hmm, I actually considered initiate_refund before, but then again initiate_refund doesn't actually.. initiate the refund (i.e., we actually don't send it until the counterparty requests it). I also avoided adding payment as we omitted it everywhere else. But maybe you're right this might be slightly preferable... I'll update the API to initiate_refund/request_refund_payment. It might be slightly awkward as users would call node.bolt12_payment().request_refund_payment(), but probably doesn't matter all too much.

As mentioned elsewhere, I think it might be worth considering to find a better name for Refund, which might help to improve these intricacies.

[jkczyz]

Seems if this errors we'll still have sent out the invoice request and presumably make the payment. I noticed we often panic in the event handler when failing to update a payment. Would failing to insert here cause a panic when processing PaymentSent and updating the payment store? On restart would we continuously panic when re-processing the event?

Reading the PaymentSent handling code, it looks like update will not fail if it can't find the payment (nor panic for that matter). And so the handling code will still generate an event but just won't log. Mostly trying to understand if we can ever get into a situation where failing a payment store write causes issues later. And also when we prefer to panic over simply logging.

[tnull]

Seems if this errors we'll still have sent out the invoice request and presumably make the payment. I noticed we often panic in the event handler when failing to update a payment. Would failing to insert here cause a panic when processing PaymentSent and updating the payment store? On restart would we continuously panic when re-processing the event?

No, I don't think so as it doesn't panic, see below.

Reading the PaymentSent handling code, it looks like update will not fail if it can't find the payment (nor panic for that matter). And so the handling code will still generate an event but just won't log. Mostly trying to understand if we can ever get into a situation where failing a payment store write causes issues later. And also when we prefer to panic over simply logging.

Right, PaymentStore::update doesn't error when the payment can't be found but will return Ok(false). We only ever panic on persistence failure (I think currently also in PaymentSent though?). However, we'll need to remove all the panics soon for VSS (as persistence failures should be expected regularly then). As a first step, we will require lightningdevkit/rust-lightning#2995 to let us gracefully exit event handling and have LDK replay events .. eventually. So yeah, it will be pretty tricky to get this right, but IMO it's out-of-scope of this PR.

[tnull]

Force-pushed with the following changes:

> git diff-tree -U2 da32263 7ceaf5d
diff --git a/bindings/ldk_node.udl b/bindings/ldk_node.udl
index 9380d23..c92e165 100644
--- a/bindings/ldk_node.udl
+++ b/bindings/ldk_node.udl
@@ -113,5 +113,5 @@ interface Bolt12Payment {
        Bolt12Invoice request_refund([ByRef]Refund refund);
        [Throws=NodeError]
-       Refund offer_refund(u64 amount_msat, u32 expiry_secs);
+       Refund create_refund(u64 amount_msat, u32 expiry_secs);
 };

diff --git a/src/payment/bolt12.rs b/src/payment/bolt12.rs
index d4cc04a..a34099f 100644
--- a/src/payment/bolt12.rs
+++ b/src/payment/bolt12.rs
@@ -299,6 +299,6 @@ impl Bolt12Payment {
        }

-       /// Returns a [`Refund`] that can be used to offer a refund payment of the amount given.
-       pub fn offer_refund(&self, amount_msat: u64, expiry_secs: u32) -> Result<Refund, Error> {
+       /// Returns a [`Refund`] object that can be used to offer a refund payment of the amount given.
+       pub fn create_refund(&self, amount_msat: u64, expiry_secs: u32) -> Result<Refund, Error> {
                let mut random_bytes = [0u8; 32];
                rand::thread_rng().fill_bytes(&mut random_bytes);
diff --git a/src/payment/store.rs b/src/payment/store.rs
index 03a42a8..f7f4942 100644
--- a/src/payment/store.rs
+++ b/src/payment/store.rs
@@ -176,7 +176,8 @@ pub enum PaymentKind {
                lsp_fee_limits: LSPFeeLimits,
        },
-       /// A [BOLT 12] 'offer' payment, i.e., a payment in response to an offer.
+       /// A [BOLT 12] 'offer' payment, i.e., a payment for an [`Offer`].
        ///
        /// [BOLT 12]: https://github.com/lightning/bolts/blob/master/12-offer-encoding.md
+       /// [`Offer`]: crate::lightning::offers::offer::Offer
        Bolt12Offer {
                /// The payment hash, i.e., the hash of the `preimage`.
@@ -189,7 +190,8 @@ pub enum PaymentKind {
                offer_id: OfferId,
        },
-       /// A [BOLT 12] 'refund' payment, i.e., a payment without a prior offer.
+       /// A [BOLT 12] 'refund' payment, i.e., a payment for a [`Refund`].
        ///
        /// [BOLT 12]: https://github.com/lightning/bolts/blob/master/12-offer-encoding.md
+       /// [`Refund`]: lightning::offers::refund::Refund
        Bolt12Refund {
                /// The payment hash, i.e., the hash of the `preimage`.
@@ -356,5 +358,6 @@ where
                                        },
                                        _ => {
-                                               // We can omit updating the hash for BOLT11 payments as the payment has will always be known from the beginning.
+                                               // We can omit updating the hash for BOLT11 payments as the payment hash
+                                               // will always be known from the beginning.
                                        },
                                }
diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs
index 0dc32f5..5f3242f 100644
--- a/tests/integration_tests_rust.rs
+++ b/tests/integration_tests_rust.rs
@@ -491,5 +491,5 @@ fn simple_bolt12_send_receive() {
        // Now node_b refunds the amount node_a just overpaid.
        let overpaid_amount = expected_amount_msat - offer_amount_msat;
-       let refund = node_b.bolt12_payment().offer_refund(overpaid_amount, 3600).unwrap();
+       let refund = node_b.bolt12_payment().create_refund(overpaid_amount, 3600).unwrap();
        let invoice = node_a.bolt12_payment().request_refund(&refund).unwrap();
        expect_payment_received_event!(node_a, overpaid_amount);

[tnull]

Kicked CI once, but the flaky python CI is unrelated.

[tnull]

Now changed the Refund API to initiate_refund/request_refund_payment and tacked on a commit checking PaymentKind in non-BOLT12 integration tests.

[jkczyz]

Now changed the Refund API to initiate_refund/request_refund_payment and tacked on a commit checking PaymentKind in non-BOLT12 integration tests.

FWIW, I'm somewhat supportive of calling the method offer_refund instead of initiate_refund where "offer" is a verb, but I can see how there might be some confusion.