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

TxBuilder::add_psbt #153

Open
LLFourn opened this issue Oct 30, 2020 · 4 comments
Open

TxBuilder::add_psbt #153

LLFourn opened this issue Oct 30, 2020 · 4 comments
Labels
module-wallet new feature New feature or request

Comments

@LLFourn
Copy link
Contributor

LLFourn commented Oct 30, 2020

The idea would be that instead of calling add_recipients and add_utxo and eventually add_foreign_utxo, you could just add a PSBT and the wallet would then figure out which of those it owns and which it doesn't and then fill in all the details it can.

Does this make sense?

@afilini
Copy link
Member

afilini commented Oct 30, 2020

I guess you are referring to the Updater role as described by BIP174, basically an entity that can add the previous utxos, witness script, derivation paths, etc to a PSBT.

We currently have a method that does that (Wallet::complete_transaction), but it's not public yet. I guess we could update the interface a little bit (for instance, it takes the full TxBuilder just to read a couple of parameters that we could just pass as arguments individually) and then we could make it public. Or we could keep that one private and add a new public method that wraps it and does some pre-processing before calling complete_transaction (mainly the creation of prev_script_pubkeys), which is probably what I would do.

I don't know if the best way is to go through TxBuilder, I would just make a method takes a PSBT and returns a PSBT since you wouldn't need pretty much any of the other options exposed by TxBuilder.

@afilini afilini added new feature New feature or request module-wallet labels Oct 30, 2020
@LLFourn
Copy link
Contributor Author

LLFourn commented Oct 31, 2020

I don't know if the best way is to go through TxBuilder, I would just make a method takes a PSBT and returns a PSBT since you wouldn't need pretty much any of the other options exposed by TxBuilder.

I think concretely what I am aiming at is being able to do the naive coinjoin mentioned in BIP174. i.e. I generate a PSBT with my inputs and some outputs (where the coinjoin output is fixed to 0.1 BTC). I pass it onto you can then you add your inputs and your coinjoin output and so on. Once everyone has finished adding stuff we pass it back (with randomly re-ordered coinjoin outputs) and everyone adds their signatures.

I don't know if the best way is to go through TxBuilder, I would just make a method takes a PSBT and returns a PSBT since you wouldn't need pretty much any of the other options exposed by TxBuilder.

For this it looks like I do need TxBuilder options like add_recipient, add_utxo, coin selection filters and so on. In short you want create_tx where you pass in a PSBT as the starting point. I don't really see a straightforward way of turning Wallet::complete_transaction into what I want.

@afilini
Copy link
Member

afilini commented Nov 2, 2020

Right, I got it now. Appending inputs/outputs to existing PSBTs, not just metadata.

I'm actually thinking this could be part of a larger refactoring that also improves the current bump_fee: for instance, currently we don't support adding extra recipients when bumping fees, or even merging multiple unconfirmed transactions, which should be allowed by BIP125.

I guess in a way the logic to do that would be very similar, with the only difference that "bump_fee" takes the transaction from the wallet's database, while you are proposing to take a PSBT from the outside. I think we could have a common method that contains all the logic, and bump_fee could call that method after having done its few checks on the new fee rate and stuff like that.

@murchandamus
Copy link
Contributor

Maybe something called amend_transaction or extend_transaction that would take parameters for recipient_outputs, feerate and optionally required_utxos and then essentially treats the existing transaction stub as an additional source of output and input definitions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module-wallet new feature New feature or request
Projects
Status: No status
Development

No branches or pull requests

3 participants