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

Signing a PSBT with some inputs not having utxo throws ValueError #52

Closed
AdamISZ opened this issue Oct 17, 2020 · 3 comments
Closed

Signing a PSBT with some inputs not having utxo throws ValueError #52

AdamISZ opened this issue Oct 17, 2020 · 3 comments

Comments

@AdamISZ
Copy link

AdamISZ commented Oct 17, 2020

I was previously using sign() method of psbt.PartiallySignedTransaction with a keystore containing keys for only some inputs, and it was working fine, but I now have a use case where some of the other inputs (which I'm not signing and don't have keys for) are required (see BIP78) to not have the utxo field filled in. When I call sign() in this case, those inputs throw the above mentioned ValueError (which btw has a trivial typo: 'utxo is not set for of PSBT_Input at index {self.index}').

Currently I only have the ugly option of signing first, then deleting the utxo field. But unless I missed something, it should be possible to call sign() on the overall PSBT without it throwing an Exception (though I suppose, it could certainly be an option).

@dgpv
Copy link

dgpv commented Oct 17, 2020

Yes, it looks like we should just return None if utxo is not set, now it always raises an exception

utxo = self.witness_utxo or self.utxo
if utxo is None:
raise ValueError(
f'utxo is not set for of PSBT_Input at index {self.index}')

a few lines above there's also a check if the index is set on UTXO:

if self.index is None:
raise ValueError(
'index is not set for PSBT_Input')

index on the input may be unset if someone created PSBT_Input directly and did not supply or did not set the index.

It seems to me that conceptually, in both of this cases we can just return "None" and not throw execption.

The only situation where throwing an exception seems to be of value is when someone needs to make sure that all inputs are signable. But "unsignable" inputs return None in other cases such as unrecognized script, and the check can be accomplished just by iterating over psbt.inputs doing if inp.sign(...) is None:

I think I will just replace raise ValueError(...) with return None in both cases: #53

Let me know if you think that there's a better way to handle this

@AdamISZ
Copy link
Author

AdamISZ commented Oct 17, 2020

From my view, I think returning None is right. A caller can always inspect the SignResult object to check (I think?) that the result is as intended. I believe I agree also about index; I don't have such a use case right now, but I could maybe imagine it.

@dgpv
Copy link

dgpv commented Oct 17, 2020

I released v1.1.1.post2 with the fix

@dgpv dgpv closed this as completed Oct 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants