Consider returning TransactionDetails from TxBuilder finish() method

[thunderbiscuit]

The finish() method on the `TxBuilder on the Rust side of BDK has the following signature:

pub fn finish(self) -> Result<(Psbt, TransactionDetails), Error> {

Whereas the finish() method in the bindings only returns a PartiallySignedBitcoinTransaction.

This, among other things, makes it impossible to get the total fees that the transaction will require and display them to the user before broadcasting (the broadcast() method returns the TransactionDetails we want).

cc @ismyhc

[ismyhc]

I would concur with this. Im working on a Swift wallet prototype and absolutely need this functionality to make the user aware of how much they are transferring with.

[allisonmoyer]

We definitely need this functionality in our app as well. Any timeline on adding the details to finish?

[thunderbiscuit]

I'm looking into it now, and I think the first step of this is to decide what we need to expose at the bindings level.

For example our current TransactionDetails object omits the transaction field present in the Rust library:

// bindings
pub struct TransactionDetails {
    pub fee: Option<u64>,
    pub received: u64,
    pub sent: u64,
    pub txid: String,
}

// Rust BDK
pub struct TransactionDetails {
    pub transaction: Option<Transaction>,
    pub txid: Txid,
    pub received: u64,
    pub sent: u64,
    pub fee: Option<u64>,
    pub confirmation_time: Option<BlockTime>,
}

This transaction field is an Option on the Transaction type, which has the whole nine yards of what's in the tx (version, locktime, list of inputs and list of outputs). I don't mind digging this deep and exposing all of that if we need to, but the exact shape of what we should expose might depend on what people need; for example, @allisonmoyer do you need this TransactionDetails for the same reason me and @ismyhc need it, i.e. for calculating fees and weight/size (which I could add in a separate object without exposing the entirety of the tx data), or do you need the internals of the transaction itself?

These are good API discussions, so I just want to bring @notmandatory into this.

[notmandatory]

@thunderbiscuit you are asking the right question, it's a bit tedious but not hard to expose the whole Transaction structure so it can be returned but before we do that it would be best to understand what it's needed for. At a minimum we should add the missing confirmation_time field (with None meaning that the tx is unconfirmed).

[notmandatory]

Some more details on why Transaction details are needed in https://github.com/bitcoindevkit/bdk-swift/issues/27, which is to show received to / sent to information for transactions inputs/outputs.

[ismyhc]

Some more details on why Transaction details are needed in bitcoindevkit/bdk-swift#27, which is to show received to / sent to information for transactions inputs/outputs.

Yup, I posted that a bit back. I ended up closing it today because I felt like maybe it should be in this repo, but I also noticed that there was a deprecation to vout's address field in bitcoin core recently. It looks like it's being replaced by an address field though?

https://github.com/bitcoin/bitcoin/pull/20286/files#diff-a58e7bb9d9a8a0287c0b7281d99da4e79b6f8c2a5780c24c6d76c14212c48640

At any rate, with that issue I was trying to get at the send/receive address's from the already broadcasted transaction. I understand that sometimes It can be hard to reason about those, but it just seems like fundamental data to be included in the transaction data. This would at least allow the consumer to parse those and display.

[allisonmoyer]

@thunderbiscuit yep, we need it primarily for calculating fees prior to broadcasting

[thunderbiscuit]

@ismyhc I added this as a discussion point for the next bindings dev call. Looking for the addresses in this case will be more about exposing the underlying rust-bitcoin library. I'm looking into it right now and here is how I think one would go about it:

1. Open up the Option<Transaction>

pub struct Transaction {
    pub version: i32,
    pub lock_time: u32,
    pub input: Vec<TxIn>,
    pub output: Vec<TxOut>,
}

2. Parse the Vec<TxOut> and pull the script_pubkeys

pub struct TxOut {
    pub value: u64,
    pub script_pubkey: Script,
}

3. Call the from_script() associated function on the Address type for each script.

@allisonmoyer I'll open a draft PR adding just the feerate to the TransactionDetails as a starting point, and we can see if this does what you and I need.

[thunderbiscuit]

As per bitcoindevkit/bdk#524, the fee rate cannot currently be calculated before the transaction is signed (because the tx doesn't have witness data at this point).

This means we won't be able to return it on the TxBuilder.finish(), but only once the Wallet.sign(psbt) -> PSBT has been called on it.

[allisonmoyer]

Returning it from sign works for us as well. Will the transaction have an ID at that point? Because would also be interested in that data if it's available

[thunderbiscuit]

Yep we'll be able to have a txid with it for sure. In fact it will probably be just an extra method on the PartiallySignedBitcoinTransaction struct. My only question at the moment is how to handle the situation where the psbt is not yet signed.

[notmandatory]

I created a draft PR to add some new functions to PartiallySignedBitcoinTransaction to get it's current fee_amount and fee_rate. If we get these into the rust bdk and expose in bdk-ffi this will allow a wallet to show the accurate fee rate after finalizing/signing a PSBT before broadcasting it. How does the approach look?

[ismyhc]

I created a draft PR to add some new functions to PartiallySignedBitcoinTransaction to get it's current fee_amount and fee_rate. If we get these into the rust bdk and expose in bdk-ffi this will allow a wallet to show the accurate fee rate after finalizing/signing a PSBT before broadcasting it. How does the approach look?

The draft looks great to me!

[notmandatory]

I screwed up and didn't make the new psbt module in bdk public , so we'll need these three PRs before we can get transaction details and fee rates on finalized PSBTs:

• Make psbt mod public and add required docs bdk#782
• Add FeeRate struct and PSBT fee_amount and fee_rate functions #208
• Change TxBuilder.finish() to return new TxBuilderResult #209

I've moved this to release 0.11.0 so we don't hold up (late) release 0.10.0.

[thunderbiscuit]

In case anyone is finding this issue through search, as of #307 you'll be able to get addresses for all inputs and outputs that were used in a transaction through pulling the list of transactions the wallet knows about using Wallet.list_transactions(), which returns a TransactionDetails object. From there you can use the transaction field and pull all the inputs and output scripts, and simply call the Address.from_script() on them.