utACK 24c398f

needs 0.13.1 milestone tag?

tACK 26df793 on my testnet pool

utACK

Tested ACK 26df793

utACK 26df793

Added script tests for some special S values

The newly added tests of 70fbd82 revealed some interesting properties of the current implementation of LOW_S. The LOW_S rule is enforced only if R and S are both below 0xFFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFE BAAEDCE6 AF48A03B BFD25E8C D0364141. Otherwise, the CHECKSIG will return a False to the stack, instead of fail immediately.

You could find the examples here: https://github.com/jl2012/bips/blob/bip146outofrange/bip-0146.mediawiki#LOW_S

The alternative plan is to do NULLDUMMY only : #8636

@jl2012 to clarify, the CHECKSIG operation can fail before a LOW_S signature is detected? Why is that an issue?

Is it because a LOW_S signature could get through conceivably if an alternative branch subsequently returns true?

Is that a bad thing? Or should we only allow canonical signatures to be allowed to be CHECKSIG'd? Meaning we'd need some kind of "pre-pass"...

@dcousens the problem is CHECKSIG won't fail immediately due to LOW_S if either R or S is out-of-range.

I think this is how it actually get processed:

1. secp256k1_scalar_check_overflow checks if R or S is overflow, if yes
2. ecdsa_signature_parse_der_lax transforms both R and S to 0
3. secp256k1_scalar_is_high checks if S is high. Since S has become 0 in step 2, it doesn't think it's high so the LOW_S rule is effectively bypassed
4. secp256k1_ecdsa_verify failed because R=S=0
5. CHECKSIG returns a 0 to the top stack; script evaluation continues

So we have a weird condition that "if R overflows, we bypass LOW_S check"

Before this was discovered, we thought the rule was simply 0x01 <= S <= 0x7FFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF 5D576E73 57A4501D DFE92F46 681B20A0. (as described in BIP62)

Now I have to use a page of text and examples to precisely describe it: https://github.com/jl2012/bips/blob/bip146outofrange/bip-0146.mediawiki#LOW_S
and without any real use case. It is difficult to justify the softfork in current form.

We could fix that by, for example, making a simple byte-to-byte comparison with 0x7FFFF...20A0 , or fix the test in libsecp256k1. Or we could simply require a null signature for failed CHECKSIG operation (see #8634).

Having said that, there won't be any irreversible harm if the softfork had been done without this being discovered. It was still a softfork and it would work as we thought for 99.99999% of transactions on the blockchain. However, if an alternative implementation did not replicate this precisely, they would fork away due to the 0.00001% of transactions.

Thanks for the great explanation @jl2012, excellent point about implementation replication.

The plan is to do NULLDUMMY only (#8636), and leave LOW_S later, probably at the same time banning non-zero failing signature.

Please remove 0.13.1 tag