PSBT_Input.sign() can return None or PSBT_InpuSignInfo depending on input script type about python-bitcointx HOT 7 CLOSED

This is intended behavior.

None is returned when there's no chance we could sign the input, because something is incomplete in the information available from the PSBT, or it cannot be handled by the code, like a non-standard script (but no rules are breached, so no ValueError)

The "empty" PSBT_InutSignInfo is returned when everything is OK with PSBT, and we could sign it, but the key store does not contain the needed key.

I think that these are two different situations, and the caller should be able to distinguish them.

Maybe you can elaborate on what difficulty/problem you face because of this behavior of the library ? It is not clear to me after I skimmed through the code referenced. Is it possible that both incomplete input (without redeem script) and complete, but 'foreign' input (that you don't have keys for) bear the same meaning in your protocol ?

from python-bitcointx.

incomplete in the information available from the PSBT, or it cannot be handled by the code, like a non-standard script

Well here is another two different situations that the caller might be interested to distinguish (not provided vs not supported), but in both situations None is returned.

At the moment I am not sure if I should separate these two cases. I believe using PSBT_InputSignInfo for any of that two None-returning cases is not appropriate, because the num_sigs_missing field will be not determined.

from python-bitcointx.

Also, returning something other than None in the case when None was returned previously is quite a major breakage of the API, all the code that checked with is None will be wrong. The code that checked with if info: could be made to work by redefining __bool__() for the result class, but this would be quite ugly of a solution.

from python-bitcointx.

I think PSBT_Input.sign() could return None instead of empty PSBT_InputSignInfo in case when keystore is not supplied (will need to make keystore arg optional).

For distinguishing the "not supported script" / "missing info" cases, I believe an extra kwarg flag would be needed, to not break the None-returning API (those who need to distinguish, would supply this flag and handle results differently)

from python-bitcointx.

This is intended behavior.

Yes, I suspected as much and your explanation makes it clear. So, this is not really a problem, it's just that the "call sign() with empty keystore to inspect the signed-ness of the inputs" that I used in the above linked code, happens to get a different result returned for a p2sh case compared to a witness v0 case. It's easy for me to handle that, so I'll just do it.

(I doubt that adding more flags is needed just for this. And understood on API break.)

from python-bitcointx.

It is great that this behavior does not pose a significant problem.

This did reveal a small UX problem of the API, though (or should it be "DX" for "developer experience ? ;-))

I'll leave the issue open, because I think that the API can be improved by the changes that I described in my previous message, in a backwards-compatible way. I might return to this later with more thought.

from python-bitcointx.

For distinguishing the "not supported script" / "missing info" cases, I believe an extra kwarg flag would be needed, to not break the None-returning API (those who need to distinguish, would supply this flag and handle results differently)

I don't think this is needed, because if someone wants to detect the case when an unsupported script is encountered, they can just supply their own complex_script_helper_factory (possibly just wrap StandardMultisigSignatureHelper to check the result and throw exception).

Closing the issue.

from python-bitcointx.