Allow node wallet to receive payjoin transactions

[jbesraa]

fully addresses #177 blocked by https://github.com/lightningdevkit/rust-lightning/pull/3024

[jbesraa]

Currently blocked by the failing CI https://github.com/lightningdevkit/ldk-node/actions/runs/9287478277/job/25556603253?pr=301, a tracking issue on rust bitcoin is already open here https://github.com/rust-bitcoin/rust-bitcoinconsensus/issues/32. @tnull I wonder if this is a deal breaker or we can introduce the bitcoinconsensus feature optionally for non-uniffi which mean the payjoin receiver wouldnt be available for bindings

[tnull]

@tnull I wonder if this is a deal breaker or we can introduce the bitcoinconsensus feature optionally for non-uniffi which mean the payjoin receiver wouldnt be available for bindings

Hmm, unfortunately I think it's a bit of a deal breaker as we really want to keep the bindings and Rust as congruent as possible. Especially so as, in contrast to 'receive-and-open-channel', payjoin send/receive would be expected to be a client-side feature that mobile users might benefit from?

I mean if it really blocks your progress we could consider landing the code behind a cfg(payjoin) flag so it's already in the codebase and can be used, e.g., in tests. But I'd be hesitant to fully expose it in Rust and not expose it in the bindings at all.

Two potential ways to go about it:

a) Push for a fix in bitcoinconsensus. b) Manually implement the necessary consensus checks, if they are not too many?

[jbesraa]

@tnull I wonder if this is a deal breaker or we can introduce the bitcoinconsensus feature optionally for non-uniffi which mean the payjoin receiver wouldnt be available for bindings

Hmm, unfortunately I think it's a bit of a deal breaker as we really want to keep the bindings and Rust as congruent as possible. Especially so as, in contrast to 'receive-and-open-channel', payjoin send/receive would be expected to be a client-side feature that mobile users might benefit from?

I mean if it really blocks your progress we could consider landing the code behind a cfg(payjoin) flag so it's already in the codebase and can be used, e.g., in tests. But I'd be hesitant to fully expose it in Rust and not expose it in the bindings at all.

Two potential ways to go about it:

a) Push for a fix in bitcoinconsensus. b) Manually implement the necessary consensus checks, if they are not too many?

yup, working on option a. Got in touch with BitcoinZavior to get some input from him about bindings, otherwise I am researching/experimenting with solutions.

Now that https://github.com/lightningdevkit/rust-lightning/pull/3024 is ready for reivew, this bitcoinconsensus binding issue is the only blocker for both regular payjoin receiver and receive-open-channel flows.

[jbesraa]

@tnull Apparently I dont need the bitcoinconsensus feature after all.

Previously I was checking if the transaction sent by the sender is broadcast-able, but the payjoin protocol requires that only if payjoin used in a non-interactive receiver, as mentioned here https://github.com/bitcoin/bips/blob/master/bip-0078.mediawiki#receivers-original-psbt-checklist. So the binding issue is longer blocking this.

[tnull]

@tnull Apparently I dont need the bitcoinconsensus feature after all.

Previously I was checking if the transaction sent by the sender is broadcast-able, but the payjoin protocol requires that only if payjoin used in a non-interactive receiver, as mentioned here https://github.com/bitcoin/bips/blob/master/bip-0078.mediawiki#receivers-original-psbt-checklist. So the binding issue is longer blocking this.

Cool, let me know when this is ready for review.

What is the idea for testing this? Could we add some integration tests testing sending/receiving? To this end, since both PRs only hold a single commit and this one is based on #295, would it make sense to close #295, and add a third commit adding integration tests?

[jbesraa]

integration test already added here https://github.com/lightningdevkit/ldk-node/pull/301/files#diff-ebba69f49eedcd3dd6a656eff964c6184b6d29c02fbad6a49d3b41c87cc3de79

I am very close to open the third PR (payjoin->channel opening), and based on the work I have done there I expect a couple of minor changes here. Will let you know when its RFR.

[jbesraa]

@tnull I think this is ready for review. This adds the payjoin receiver part and also opening channels from payjoin request (and the sender commit is CP here).

There are a couple of things that needs discussion, I added a comment about them in the code: https://github.com/lightningdevkit/ldk-node/pull/301/files#diff-4db9c4cca2ff80c2ad80cbf7111959e148b31ee24cb354f8620c417047fe7b46R132

https://github.com/lightningdevkit/ldk-node/pull/301/files#diff-03a28135b3f471a98f1c40eb8a4dcc7c4759f4bf9aabc83c12c71f14d68e00c1R518

let me know if this PR is small enough for review and I can close https://github.com/lightningdevkit/ldk-node/pull/295.

[tnull]

@tnull I think this is ready for review. This adds the payjoin receiver part and also opening channels from payjoin request (and the sender commit is CP here).

There are a couple of things that needs discussion, I added a comment about them in the code: https://github.com/lightningdevkit/ldk-node/pull/301/files#diff-4db9c4cca2ff80c2ad80cbf7111959e148b31ee24cb354f8620c417047fe7b46R132

https://github.com/lightningdevkit/ldk-node/pull/301/files#diff-03a28135b3f471a98f1c40eb8a4dcc7c4759f4bf9aabc83c12c71f14d68e00c1R518

Ah, unfortunately these links don't resolve for me by now. Could you describe the issues here.

let me know if this PR is small enough for review and I can close #295.

Hmm, you're right, ~2k lines changes is a mouth full, so let start with #295, land it behind a cfg-gate, and then remove the cfg flag here once we're positive it's ready and tested, i.e., also after the FundingSigned PR landed and we upgraded to the corresponding LDK release.