[RFC] event: expose BOLT12 invoice in PaymentSuccessful for proof of payment #733
Conversation
|
👋 Thanks for assigning @tnull as a reviewer! |
|
@tnull IDK if this is a good design to have with the ffi, but I had to work around some unify ffi limitation with the enum type that is used inside the |
|
🔔 1st Reminder Hey @tnull! This PR has been waiting for your review. |
|
🔔 2nd Reminder Hey @tnull! This PR has been waiting for your review. |
|
🔔 3rd Reminder Hey @tnull! This PR has been waiting for your review. |
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
Exposing the BOLT12 invoice makes sense to me, though we should do it properly instead of just exposing the bytes.
vincenzopalazzo
force-pushed
the
macros/bolt12-pop
branch
from
4c6b18e to
a3a60b3
Compare
There was a problem hiding this comment.
The changes are getting closer, but please make sure to avoid unnecessary boilerplate and stick to the approach we took for the other types (like Offer, Refund, etc).
This also needs a rebase by now, sorry!
Btw, please re-request review when you made updates, otherwise I might not always see it immediately.
| "StaticInvoice", | ||
| }; | ||
|
|
||
| dictionary PaidBolt12Invoice { |
There was a problem hiding this comment.
Can we make this an
[Enum]
interface PaidBolt12Invoice {
instead?
There was a problem hiding this comment.
Mh I was hacking round this this afternoon and this is what I got
error: failed to run custom build command for `ldk-node v0.8.0+git`
Caused by:
process didn't exit successfully (exit status: 101)
--- stdout
cargo:rerun-if-changed=bindings/ldk_node.udl
cargo:rerun-if-env-changed=UNIFFI_TESTS_DISABLE_EXTENSIONS
--- stderr
thread 'main' panicked at build.rs:10:59:
called `Result::unwrap()` on an `Err` value: Objects cannot currently be used in enum variant data
The key part is:
Objects cannot currently be used in enum variant data
This occurs when trying to use:
[Enum]
interface PaidBolt12Invoice {
Bolt12Invoice(Bolt12Invoice invoice);
StaticInvoice(StaticInvoice invoice);
};It looks like that it is happening because Bolt12Invoice and StaticInvoice are defined as interface (Objects) in UniFFI, they cannot be used as parameters in enum variants.
Am I missing something?
src/types.rs
Outdated
| /// Unlike [`Bolt12Invoice`], a `StaticInvoice` does not support proof of payment | ||
| /// because the payment hash is not derived from a preimage known only to the recipient. | ||
| #[derive(Debug, Clone, PartialEq, Eq)] | ||
| pub struct StaticInvoice { |
There was a problem hiding this comment.
No need to wrap StaticInvoice in the non-uniffi case. Please follow the same approach as we did for Offer, Refund, etc. and reuse the existing maybe_wrap macros etc.
There was a problem hiding this comment.
Ok let me know if now this is solving your concern, thanks!
vincenzopalazzo
force-pushed
the
macros/bolt12-pop
branch
3 times, most recently
from
05838f1 to
22abd3b
Compare
After a successful BOLT12 payment, users need access to the paid invoice to provide proof of payment to third parties. The invoice contains the payment_hash that can be verified against sha256(preimage). This adds a `bolt12_invoice` field to `PaymentSuccessful` containing the `PaidBolt12Invoice` which wraps either a regular `Bolt12Invoice` or a `StaticInvoice` for async payments. For UniFFI bindings, `PaidBolt12Invoice` is exposed as a dictionary struct that delegates serialization to LDK's native type. For non-UniFFI builds, we simply re-export LDK's `PaidBolt12Invoice` enum directly. Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
vincenzopalazzo
force-pushed
the
macros/bolt12-pop
branch
from
22abd3b to
a8d05cb
Compare
3 participants
This patch adds the
bolt12_invoicefield to thePaymentSuccessfulevent, enabling users to obtain proof of payment for BOLT12 transactions.Problem:
Previously, after a successful BOLT12 payment, users had no way to access the paid invoice data. This made it impossible to provide proof of payment to third parties, who need both the payment preimage and the original invoice to verify that sha256(preimage) matches the invoice's payment_hash.
Solution:
Add a
bolt12_invoice: Option<Vec<u8>>field toPaymentSuccessfulthat contains the serialized BOLT12 invoice bytes. The invoice is serialized using LDK's standard encoding, which can be parsed back usingBolt12Invoice::try_from(bytes)in native Rust, or by hex-encoding the bytes and usingBolt12Invoice.from_str()in FFI bindings.Design decisions:
Vec<u8>rather than the complexPaidBolt12Invoicetype to avoid UniFFI limitations with objects in enum variantsNoneforStaticInvoice(async payments) since proof of payment is not possible for those payment types anywayThis implementation follows the maintainer guidance from PR #563 to expose the invoice via the event rather than storing it in the payment store.