Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DO NOT MERGE: Send & Receive payjoin v2 working reference #344

Closed
wants to merge 28 commits into from

Conversation

DanGould
Copy link
Contributor

@DanGould DanGould commented Dec 16, 2024

This draft is the basis for e2e Payjoin V2 support.

The Sender contains all logic to create PSBTs to fulfil a given bitcoin URI and respond to updates.

The Receiver creates bitcoin URIs and listens for messages to receive sessions it spawns.

A Payjoin Manager depends on NetworkCubit and spawns isolates to run payjoin sessions concurrently. This should spawn persisted sessions on startup, pause them when the app goes to the background, and resume them when it comes to the foreground. (Eventually background processes can be run but that's for later).

This is DRAFT. I plan to rebase into a nice reviewable history.

There are 1 or 2 bugs remaining in the way of an end to end demo

1

The receiver won't finalize and sign the proposal PSBT. I believe that's because the PSBT produced by the payjoin receiver state machine doesn't have the non-witness UTXO details (redeem script or witness script) and I'm not sure yet how to get them out of bdk. One hacky way to do so is to build a faux transaction with the TxBuilder if this method were exposed. Another way would be to derive it from the appropriate descriptor.

EDIT Mon Dec 16 21:15:43 2024 UTC

The reason it's not signing is because I'm not supplying witness_script NOR bip32_derivation information with the PSBTin to the finalizer, and bdk.Wallet.sign() requires one or the other to be present if trustWitnessUtxo: true.

With trustWitnessUtxo: false, the non_witness_utxo data is required.

I beleive the way around this is to expose get_psbt_input(LocalUtxo: utxo) -> Input from bdk-flutter such that we have access to at least the witness_script for a given LocalUtxo. @BitcoinZavior I'm going to work on a fork of bdk-flutter to get this in ASAP. I think this function get_psbt_input is exposed but the psbt::Input type doesn't expose sufficient information about its contents. Input details aren't exposed in bdk-ffi either, so this is going to require a bit of digging but is definitely possible to pull out

2

I believe we may run into another issue on the sender side at signing where our app will need to reintroduce UTXO data the receiver removes. Bitcoin Core does this on its own, but BDK, at least this old version, does not

Finally, the current implementation fully triggers payjoin sends on the first send screen rather than building the psbt and only launching the payjoin on the Confirm Transaction screen, and then does not differentiate on the Success screen even though the transaction is only communicated to the receiver and not broadcast. We must differentiate for clarity.

My plan is to solve the signing bugs tomorrow. Any help getting the UI and flow into place would be great. Once we get these, we should be able to move on to ensuring that history is displayed properly, showing net flows for Payjoins.

@DanGould
Copy link
Contributor Author

Rebased on main but still incomplete. All commits up to 3bc6064 build and run

@DanGould
Copy link
Contributor Author

I figured out how to send the specific data back and forth across the isolate boundary. It's not super ugly but there's a lot of boilerplate serialization code. I got most of the checks to pass this way, and I'm sure signing would work this way, based on how the two regions of memory and threads are interacting, but I need getCandidateInputs List data to somehow be serialized across this boundary. I think this is the last hurdle to get receiving actually working

@i5hi i5hi self-assigned this Dec 19, 2024
@i5hi i5hi added the urgent requires immediate attention label Dec 19, 2024
@DanGould DanGould changed the title Send & Receive payjoin v2 DO NOT MERGE: Send & Receive payjoin v2 working reference Dec 19, 2024
@DanGould DanGould force-pushed the send-payjoin branch 2 times, most recently from 3bba3cc to d8f42c7 Compare December 19, 2024 22:15
DanGould and others added 5 commits December 20, 2024 10:47
InputPairs seem not to be able to send across the boundary. But this separates
sending secrets from crossing into a payjoin isolate. Sending happens on main.
With main / isolate separation so that signing can happen in the main thread
@DanGould DanGould force-pushed the send-payjoin branch 3 times, most recently from 20371d0 to f54bde7 Compare December 20, 2024 17:37
✅ passes send-receive smoke test
@DanGould
Copy link
Contributor Author

Closed because this functionality has been merged elsewhere

@DanGould DanGould closed this Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
urgent requires immediate attention
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants