Expose PSBT AddInput/AddOutput via RPC #19608

Is your feature request related to a problem? Please describe.

Alice creates a PSBT via the RPC API, and signs all inputs with SIGHASH_SINGLE | SIGHASH_ANYONECANPAY. She then sends the signed PSBT to Bob, who wants to add some more inputs and outputs via the RPC API without losing Alice's signatures, so that he can broadcast the resulting transaction without asking Alice to re-sign. Unfortunately, the PSBT RPC API doesn't seem to provide any way of appending new inputs/outputs to an existing PSBT; it only allows creating a new PSBT (via createpsbt) with the full set of inputs/outputs, which has the side effect of losing the existing signatures.

Describe the solution you'd like

It would be useful for the PSBT RPC API to expose the AddInput and AddOutput methods of PartiallySignedTransaction. These would allow adding new inputs/outputs without affecting whatever signature data might already be present in the existing inputs. Something like this:

bitcoin-cli addoutputpsbt 'existingpsbt' '{"data":"00010203"}'

Describe alternatives you've considered

I considered adapting combinepsbt to copy signatures between two transactions with different TXID's if the signatures are valid in both transactions given the SIGHASH flags, but this seems rather unwieldy.

It might be desirable to support adding multiple outputs (or inputs) in one RPC call, by making the 2nd argument an array of objects instead of an object. Or this might just complicate things unnecessarily. I don't feel particularly strongly about this.

Additional context

N/A.

+1 I'd need this feature too. I'll give this a shot soon.

@darosior How is it going? I would be available to work on this myself (for the OPs original usecase / project) if you didn't find time for it yet.

Tend to NACK. The determining of inputs and outputs to a PSBT is supposed to be done before you get to the PSBT part. By the time you have a PSBT, you shouldn't be changing the inputs and outputs. It is also generally unsafe to do so if there are already signatures in the PSBT given that most signatures are SIGHASH_ALL. Even with SIGHASH_SINGLE | SIGHASH_ANYONECANPAY, the order of outputs matters.

Even with SIGHASH_SINGLE | SIGHASH_ANYONECANPAY, the order of outputs matters.

Not with Segwit transactions iirc ? As input/output pairs can be permuted.

Edit: also, why is it generally unsafe ? Adding inputs to a transaction is often required by contracts using pre-signed transactions. As an example my use-case is to append inputs to a single-input transaction signed with ALL | ANYONECANPAY (Revault), but there are also Lightning's HTLC transactions which first input will be signed with SINGLE | ANYONECANPY to allow the claiming party to append an input/output pair if the "anchor outputs" proposal is negotiated (currently implemented by all implementations and about to be merged to the specs).

Edit2: @domob1812 will get to it when / if there is conceptual ACK from achow101 :)

Not with Segwit transactions iirc ? As input/output pairs can be permuted.

Yes, but the order still matters. If you insert an input or output at the beginning of the list instead of at the end, then that breaks the signature. Inputs and outputs might be inserted in positions other than the end in order to preserve privacy.

Edit: also, why is it generally unsafe ? Adding inputs to a transaction is often required by contracts using pre-signed transactions

Unsafe is perhaps not the correct word. Maybe undesirable or headache inducing is better.

As currently proposed, adding inputs and outputs affects PSBTs that are not part of contracts using pre-signed transactions. In general, PSBTs are not fully valid as they are missing signatures. They may contain some signatures but the transaction itself is not valid. Adding inputs and outputs without regard for signatures in existing inputs will, most likely, create invalid PSBTs.

In the case of pre-signed transactions, why is PSBT being used anyways? It isn't really an appropriate vehicle, after all those transactions are completely valid and could be broadcast to the network. It is more efficient to just use the network transaction format. A PSBT doesn't contain any data for the other party as he has to add his own inputs and outputs.

Yes, but the order still matters. If you insert an input or output at the beginning of the list instead of at the end, then that breaks the signature. Inputs and outputs might be inserted in positions other than the end in order to preserve privacy. @achow101 Does the AddInput/AddOutput API that currently exists insert them in the middle? I assumed they would append; if I'm wrong on that, then I think I agree that a straightforward plumbing of those methods to the RPC API wouldn't be sufficient.

In the case of pre-signed transactions, why is PSBT being used anyways? It isn't really an appropriate vehicle, after all those transactions are completely valid and could be broadcast to the network. It is more efficient to just use the network transaction format. A PSBT doesn't contain any data for the other party as he has to add his own inputs and outputs.

Fair question. The intermediate transactions in my use case aren't actually valid, because they contain imbalanced inputs/outputs (e.g. the outputs might sum to more than the inputs), so they can't just be broadcasted to the network. But, if I understand you correctly, you're saying that in terms of serialization/deserialization, they look exactly like valid transactions, thus we can treat them as complete, valid transactions for the purpose of transferring them between the users, and there's no need to use the PSBT serialization?

ISTM that although these methods could perhaps be added in some way (so long as they do some verification on whether there were any signatures which had signed with SIGHASH_ALL already in the PSBT before proceeding?) there is not a lot of appetite to add them as they are very likely to cause issues. @achow101 are we leaving this issue open as there's still a possibility of these RPCs being added?

For the record, i don't remember exactly why i advocated for this back when it was opened. I don't see why an application would need to use the bitcoind RPC to add inputs / outputs to a PSBT.

They could be added with after PSBTv2 (once #21283 is merged) since PSBTv2 actually supports this operation.

Oh good to know. (Also, wow that's like, the main point of PSBTv2 -- I really need to read the BIP!)