Specification of redeem_script is required in PartiallySignedTransaction.sign() for all inputs about python-bitcointx HOT 14 CLOSED

The checks there are primarily for correctness, so that incorrect combinations that the program working with PSBT might create or encounter would be catched. I believe that there should be strict rules by default, and those who need these rules relaxed, should do this explicitly, with understanding of why they are doing so.

Where it makes sense to relax the checks for certain usecases, I'm open to adding special flags like allow_unknown_sighash_types argument to PSBT_Input() (that flag is propagated from deserialize()), unless the usecase is too narrow -- to avoid a situation with too many options.

But in your example, I don't see how this particular check would fail with Alice taking foreign UTXO from the blockchain. If these UTXO are p2sh-wrapped, then Alice wouldn't know if they are segwit in the first place, and will have to put non-witness inputs into PSBT she creates.

If these UTXO are supplied by Bob, Alice will need to check that Bob didn't snuck an UTXO that belongs to Alice in there, so that Alice can unknowingly sign extra input belonging to her. In this case, it is much safer for Alice to just sign the inputs she explicitly selected.

from python-bitcointx.

Alice will need to check that Bob didn't snuck an UTXO that belongs to Alice in there

I even think that there might emerge some protocols where Bob could know some future input that Alice will make, and simply checking checking her existing UTXO set would not be enough.

Iterating through inputs and signing them is very simple, by the way:

from python-bitcointx.

If these UTXO are supplied by Bob, Alice will need to check that Bob didn't snuck an UTXO that belongs to Alice in there, so that Alice can unknowingly sign extra input belonging to her.

Yes, as mentioned in OP, this is not the case in this scenario. Only Alice chooses.

But in your example, I don't see how this particular check would fail with Alice taking foreign UTXO from the blockchain. If these UTXO are p2sh-wrapped, then Alice wouldn't know if they are segwit in the first place, and will have to put non-witness inputs into PSBT she creates.

Well in that specific example I had in mind, not really: SNICKER is a speculative coinjoin proposal to an unknown counterparty. The script type of such a p2sh output could be assumed to be p2sh-p2wpkh in some specific case (let's say Alice has a high probability to believe it), and the only consequence of wrong assumptions in these cases is that coinjoins Bob receives the proposed PSBT with let's say a PSBT_IN_WITNESS_UTXO field, which is actually wrong, because his utxo is non-witness, and I guess the main consequence could be that the feerate may be calculated incorrectly as a result. But Bob has no obligation to take up the proposal so that's fine.

About this though:

then Alice wouldn't know if they are segwit in the first place, and will have to put non-witness inputs into PSBT she creates.

... I'm a bit confused; doesn't this code:

indicate that also for non-witness utxos, you need to provide the redeem script (if the scriptPubKey being spent is p2sh)? I haven't actually investigated non-witness at all yet, but I was just looking at it.

That point about feerate seems to be the main reason why not knowing the type of scriptpubkey (if scripthash; obviously if keyhash there is no confusion) could be a stumbling block. But if you are able to assume it (e.g. if you assume the p2sh is p2sh-p2wpkh), and all that's unknown is the redeemscript, it seems not necessary to prevent partial signing when that information is not known for unowned inputs. I did ask Andrew Chow about this on IRC, and he agrees that it needn't be prevented, but is logically a policy for a particular signer role implementation.

Re: doing it manually, thanks for the example :) .. as per my OP, I was a bit reluctant. I guess I'll have to do that but it feels awkward and boundary-layer violating a little bit. The PartiallySignedTransaction should be in charge of how we are able to sign inputs, it feels wrong to interact with PSBTInput objects directly. I guess it just comes down to the fact that what I am talking about seems probably like an edge-case to you, but to me it's an important application of BIP174. Is there no way this can be a "only turn this check off if you know what you're doing" flag?

from python-bitcointx.

Thinking about this again, it might be safe to return None when the redeem script is not specified. This is 'cannot sign' case rather than 'Invalid structure' case.

from python-bitcointx.

Regarding the 'check that inputs are not owned by me` -- it is prevout what is important (txid + output index), not the script, so I don't think these checks are relevant for this. And as I said, the correct way in this case will be signing individual inputs that Alice selected, as the 'not mine' detection may not be guaranteed to be robust in all cases.

from python-bitcointx.

Regarding the 'check that inputs are not owned by me` -- it is prevout what is important (txid + output index), not the script, so I don't think these checks are relevant for this

I didn't really understand this. In some scenarios a wallet may be able to check its ownership by script, but not by prevout.

And as I said, the correct way in this case will be signing individual inputs that Alice selected, as the 'not mine' detection may not be guaranteed to be robust in all cases.

Yes, I guess this makes sense. Automatic recognition is probably not desirable in a security-critical kind of protocol like this, even if it works fine in some cases. Perhaps pass indices into PartiallySignedTransaction.sign()? Or maybe not, I don't know.

from python-bitcointx.

Perhaps pass indices into PartiallySignedTransaction.sign()?

I don't think this is better than just call input.sign() on individual PSBT_Inputs -- I think it is better to have this as explicit code rather than implicit selection via arguments to PartiallySignedTransaction.sign()

from python-bitcointx.

In some scenarios a wallet may be able to check its ownership by script, but not by prevout.

You're right. But this is also an argument to not increase complexity by doing these checks and simply sign selected individual inputs instead.

from python-bitcointx.

OK :) Thanks for your time, if I hit any other stumbling blocks I'll let you know. Feel free to close this then if there isn't something actionable.

from python-bitcointx.

I'm going to add changes -- return None from PSBT_Input.sign() when there's no redeem script specified for p2sh scriptPubKey

from python-bitcointx.

96f1ea5 -- please check if this fixes your problem

from python-bitcointx.

The 'sign individual inputs' argument is for the case where there is something that you could (and able to) sign accidentally. In your case, it is impossible to sign without redeem script (which is a part of the witness/scriptSig), so skipping such inputs (return None from sign() method) is the right behavior.

from python-bitcointx.

I did a quick manual test of 96f1ea5 and it works fine as intended, thanks! (obviously I won't be using it yet as I am using the latest release as a dependency, but there is no problem just doing per-input signing now and changing it later).

from python-bitcointx.

Good! Will close this issue then.

Given that there's another fix concerning witness encoding (1 was encoded as OP_1/51 in CScriptWitness, which is incorrect, should be 01), I think two fixes and some added sanity checks are enough changes to justify new minor version, which I'm going to push now.

from python-bitcointx.