incorrect verify_tx_signature (would result in confused blame)

Line 153 in 5283814

    
           def verify_tx_signature(self, signature, transaction, verification_key, tx_hash):

This function needs to not only check the signature, but also first it must enforce STRICTENC and LOWS rules.

For STRICTENC you just need to copy-paste this logic: https://github.com/Bitcoin-ABC/bitcoin-abc/blob/master/src/script/sigencoding.cpp#L27

For LOWS you just need to check that the s integer (available in that function) is between 0x1 and 0x7FFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF 5D576E73 57A4501D DFE92F46 681B20A0 (inclusive). This value can be calculated as order//2.

There is also a restriction that r must be between 1 and ~~order~~ field size-1 (inclusive)... not sure where this is checked right now (if at all) but it's a hard constraint to test/violate if I remember right.

Thanks for this. I'm in the middle of coding a bugfix for #70 now -- but I'll check this out ASAP after that's done. I really appreciate your looking over this and suggesting this change!

ah, small note: the linked c++ function is for checking a signature minus the final byte (hashtype)

The full signature check would presumably involve enforcing (maybe implicitly?) that the final hashtype byte is 0x41? I'm guessing that sighash ALL|FORKID is the mandatory signature hashtype for shuffles.

made a mistake in first comment where I said 1 <= r <= order-1... actually it's 1 <= r <= (field size)-1. I wouldn't worry about this check though, since from what I understand it's basically impossible to find a violation.

Dummy question: Doesn't electron cash already sign this way?

@markblundeberg Can you take a look at this PR? I basically translated that function from Bitcoin ABC. Is this sufficient?

#110

Or do I need to do more?

Thanks!

@markblundeberg .. Thanks for offering to take this further. Much appreciated.

Fixed in #111