LtbLightning / bdk-flutter

Bitcoin Development Kit - Flutter Package
MIT License
60 stars 27 forks source link

Add add_foreign_utxo #63

Closed awaik closed 1 year ago

awaik commented 1 year ago

Can you please expose the function add_foreign_utxo from bdk add_foreign_utxo

It is connected with the issue https://github.com/LtbLightning/bdk-flutter/issues/60

BitcoinZavior commented 1 year ago

This can be done as part of https://github.com/LtbLightning/bdk-flutter/issues/60

thunderbiscuit commented 1 year ago

@awaik can you expand on your use case for the method?

I've had another (maybe independent, maybe related? not sure if you're on the same team) request for this method on the server side so that chains of transactions that are yet-to-be-broadcast can all be signed by the wallet. I'm curious to hear how you plan on using the method on your app as we start exposing more and more of rust-bitcoin into the bindings.

awaik commented 1 year ago

@thunderbiscuit Hi! Exactly what we need is two independent transactions. With the second transaction spending the outputs of the first transaction.

BitcoinZavior commented 1 year ago

Assuming this scenario:

Transaction TXB is from Alice to Bob Transaction TXA is one of Alice’s existing unused UTXO We need to create a new transaction TXC which will have TXB and TXA as inputs. We need to sign this so TXB will be signed by Bob and TXA will be signed by Alice. And then we need to broadcast it.

In BDK not bdk-futter, it can be done as follows:

let addr = bdk::bitcoin::Address::from_str("2N1Ffz3WaNzbeLFBb51xyFMHYSEUXcbiSoX").unwrap();
    let foreign_utxo_satisfaction = bob
        .get_descriptor_for_keychain(bdk::KeychainKind::External)
        .max_satisfaction_weight()
        .unwrap();
    let utxo = bob.list_unspent().unwrap().remove(0);
    let psbt_input = bob.get_psbt_input(utxo.clone(), None, false).unwrap();
    let mut builder = alice.build_tx();
    builder
        .add_recipient(addr.script_pubkey(), 60_000)
        .only_witness_utxo()
        .add_foreign_utxo(utxo.outpoint, psbt_input, foreign_utxo_satisfaction)
        .unwrap();
    let (mut psbt, _) = builder.finish().unwrap();
    let finished = alice
        .sign(
            &mut psbt,
            bdk::SignOptions {
                trust_witness_utxo: true,
                ..Default::default()
            },
        )
        .unwrap();
    let finished = bob
        .sign(
            &mut psbt,
            bdk::SignOptions {
                trust_witness_utxo: true,
                ..Default::default()
            },
        )
        .unwrap();

bdk-flutterdoesn't have some of the classes and methods used in the above code.

BitcoinZavior commented 1 year ago

I understand this is using bdk rust, I will try and add this in a temporary branch for you to try.

StaxoLotl commented 1 year ago

The branch foreign-utxo has the add foreign utxo method from bdk rust implemented and exposed in flutter This is how it works in flutter: https://github.com/LtbLightning/bdk-flutter/blob/foreign-utxo/example/lib/main.dart#L236-L261

thunderbiscuit commented 1 year ago

We're building this in bdk-ffi on bitcoindevkit/bdk-ffi#358. One of the questions we have left to answer is how this foreign input is shared between Bob and Alice. We're thinking of adding methods encode/decode to base64 to the Input type.

Would that cover your use case @awaik @StaxoLotl? Should it be url-safe base64? Or would you prefer just a byte array?

BitcoinZavior commented 1 year ago

@thunderbiscuit thanks, byte array would suffice for this purpose. Encode/decode can possibly be useful for some other use cases, but not a must.

thunderbiscuit commented 1 year ago

See bitcoindevkit/bdk-ffi#362 for some discussion on this. The Input type is rust-bitcoin type, and is not consensus-encoded in any way. That means you would only be able to pass Inputs between rust-bitcoin wallets. Someone can't really send you a single input across the wire in a way that you can safely decode it unless you know it was built using one of bdk's bindings and whatnot. Not sure how good of a solution that is in the long run.

One construct that has consensus for encoding is the PSBT itself. We're thinking, why not send the whole PSBT, and then Alice can simply use the "combine" method on the PSBT? Does that solve the use case as stated above?

Bob makes a PSBT, sends it to Alice. Alice also makes a PSBT, and then combines both. She can sign it, and then send that combined PSBT to Bob who can sign it too.

@BitcoinZavior @StaxoLotl @awaik does that cover the use case? Just to be clear I'm absolutely on board with adding the add_foreign_utxo method, but I want to make sure I build something that scales and makes sense for all. Looking for confirmation on the use case and exact structure of what's required.

BitcoinZavior commented 1 year ago

@thunderbiscuit Thanks for the analysis. The PSBT approach will work. As long as Alice can send it signed( or unsigned) to Bob. Bob can later combine and either (1) sign it and then return it back to Alice so that Alice can sign. After this, Alice can broadcast. Or choose to send it to Bob to broadcast. (2) If Alice had already signed it before sending it to Bob then Bob can sign and broadcast it.

I am not familiar with @awaik actual use case but from the previous example we had done the PSBT implementation will work.

thunderbiscuit commented 1 year ago

Ok sounds good. Happy to hear it might enable the use case above.

Since it's still a method on the Wallet type in Rust, I'm still very interested in exposing it in the bindings, but I will make sure we fully understand how users would leverage it before pushing a particular API. @StaxoLotl let me know if you'd still have a use for it past the PSBT.combine() method mentioned above.

thunderbiscuit commented 1 year ago

After more discussions with Lloyd, I realized that this Psbt.combine() approach will only work if Bob in your example above can produce a PSBT in the first place. He might not be able to (missing information about other participants' inputs and required outputs? depends on the use case) and simply want to provide an input, in which case the add_foreign_utxo is still needed for Alice to add the input and build the PSBT. The question of whether that's required for the two folks above (are you guys working on the same wallet or different ones?) of if they can use the PSBT.combine approach is not clear to me yet, feel free to let me know.

Note that this method will not be required under the new BDK 1.0 API because of a change in how the Wallet and transactions are being built interact.

If someone identifies a clear need for the add_foreign_utxo method here or in any other issue on our repos I'm happy to merge the PR; if not I'll leave it aside for now at it will be superseded by the new APIs.

StaxoLotl commented 1 year ago

Thanks for investigating this scenario @thunderbiscuit

To clarify, the scenario in brief is as follows:

  1. Alice will select one UTXO from her wallet using AliceWallet.listUnspent
  2. Alice will send serialise and send the UTXO to Bob via their own internal communication channel.
  3. Bob will add Alice's UTXO as input using addForeignUxto as shown below:

https://github.com/LtbLightning/bdk-flutter/blob/18fe41fbc2fb7a084c005ff48494615f5e4c7c12/example/lib/main.dart#LL254C25-L254C25

This is the use case, and this has been tested and it works. The psbt combine is an alternate solution and thats not required for this use case.

thunderbiscuit commented 1 year ago

Ok cool thanks for letting us know.

In this case however the "alternate solution" (creating 2 PSBTs and combining them) might be what you should pick as your primary solution if that works for you, because the add_foreign_utxo will likely disappear in the 1.0 release of BDK. This is why I wanted to make sure the use case could be done in other ways. Because it can, and add_foreign_utxo is not required, I will not merge it in the bdk-ffi codebase for the moment, particularly because as of now your use case also requires Alice and Bob to have a special, non-standard and not "compatible with other libraries" workflow (you need them to share this Input type that is not consensus-encoded, meaning it will only be valid under two libraries using rust-bitcoin). I'm not opposed to merging it, but I'd need a strong use case for it that cannot be accomplished in other ways.

This means that to continue using it on your end you'll have to maintain a fork of bdk-flutter. Might be more work than is worth it. But thanks for confirming with us your use of the API! Always happy to see people building stuff with our libraries!