Heh, sure, ACK this, though I seem to recall arguments about it every time somebody tries to do this.

Don't mind removing this commit although I don't see why this is an issue.

there is a tab or something on this line that should be deleted

same on this line, trailing whitespace

How does one figure out whether there is an empty line or whether it has spaces/tabs?

More arguments for rustfmt :P . But yeah, I will take care of these things in future.

On my system git diff highlights trailing whitespace in red. I don't know what setting causes it to do this.

I assume the line should read "for non-segwit and non-P2SH wrapped segwit outputs" b/c in the current reading it seems that all P2SH (non-wrapped) should fall into this first options as well

would

match (inp.non_witness_utxo, inp.witness_utxo) {
    (Some(non_witness_utxo), None) => { ... }
    (None, _) => { ... }
    (Some(_), Some(_)) => { /* error 2 */ }
    _ => { /* error 2 */ }
}

work better since it will cover currently uncovered situation if both non-witness and witness UTXO are present for the same output?

Agreed, it would be good to catch this case and error out.

@apoelstra @dr-orlovsky , I tried to follow the specification line by line write https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki#simple-signer-algorithm. I think the specification does not error if both are present and this is the expected behaviour

In fact, from the BIP. An input can have both PSBT_IN_NON_WITNESS_UTXO and PSBT_IN_WITNESS_UTXO.

Ok, but which of them has priority over the other? Here, if non-witness is present, witness is just simply ignored...

Yes, I followed the algorithm listed https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki#simple-signer-algorithm.

Ok, at least this is according to the spec... However it's wired, will look into spec to see if there could be an issue with it...

single space is missing before = if

This can be simplified once #387 will be merged - just another reason to have that PR, @apoelstra

Merged now :)

trailing whitespace here

This comment should be changed to indicate that it looks at the PSBT data to determine the appropriate sighash mode as well as whether to sign using segwit or legacy. As written it sounds like its primary job is to decide between segwit/non-segwit which isn't true.

You can use .to_bytes() instead of .clone().into_bytes()

Same down here, use .to_bytes

Yeah, clone().into_bytes() looks silly. Idk why I did this..