Correctly report high-S violations #7500

pull sipa wants to merge 1 commits into bitcoin:master from sipa:reporthighs changing 1 files +4 −1

sipa commented at 1:21 PM on February 10, 2016: member

Reported in #6862 (comment)
Correctly report high-S violations 9d95187d5d
laanwj added the label Needs backport on Feb 10, 2016
laanwj added the label Validation on Feb 10, 2016
laanwj commented at 1:29 PM on February 10, 2016: member

utACK https://github.com/sipa/bitcoin/commit/9d95187d5ddee56b6dfb55985008bdf70aed31f2
paveljanik commented at 5:48 PM on February 10, 2016: contributor

utACK https://github.com/sipa/bitcoin/commit/9d95187d5ddee56b6dfb55985008bdf70aed31f2 ... passively waiting for some High S tx to compare log output ;-)
laanwj commented at 6:00 PM on February 10, 2016: member

@paveljanik wrong commit hash :)
paveljanik commented at 6:07 PM on February 10, 2016: contributor

Yes, testing two PRs at one time 8)

paveljanik commented at 7:17 PM on February 10, 2016: contributor

ACK

Thx for random volunteer who sends a lot of High S now...

Before the change:

2016-02-10 17:28:54 ERROR: AcceptToMemoryPoolWorker: CheckInputs: 9f8604786542fc4decdd8c5e739eb2c57ba9a7bd67b2c21ecdb99abffb8f643c, non-mandatory-script-verify-flag (unknown error) (code 64)

after:

2016-02-10 19:06:42 ERROR: AcceptToMemoryPoolWorker: CheckInputs: 896a30d13dfd36612e7a77de8e085653d904dea9ecf8af0451f0ed6a78f8dd83, non-mandatory-script-verify-flag (Non-canonical signature: S value is unnecessarily high) (code 64)

laanwj commented at 7:18 PM on February 10, 2016: member

Tested ACK 9d95187 Without patch:

["0x48 0x304502203e4516da7253cf068effec6b95c41221c0cf3a8e6ccb8cbf1725b562e9afde2c022100ab1e3da73d67e32045a20e0b999e049978ea8d6ee5480d485fcf2ce0d03b2ef001","0x21 0x03363d90d447b00c9c99ceac05b6262ee053441c7e55552ffe526bad8f83ff4640 CHECKSIG","LOW_S","P2PK with high S"] 
-> unknown error

With patch:

["0x48 0x304502203e4516da7253cf068effec6b95c41221c0cf3a8e6ccb8cbf1725b562e9afde2c022100ab1e3da73d67e32045a20e0b999e049978ea8d6ee5480d485fcf2ce0d03b2ef001","0x21 0x03363d90d447b00c9c99ceac05b6262ee053441c7e55552ffe526bad8f83ff4640 CHECKSIG","LOW_S","P2PK with high S"] 
-> Non-canonical signature: S value is unnecessarily high

BTW: would it be useful to add a field for the expected script_error to the invalid tx tests, so that this is verified are part of the unit tests? (not in this pull)

laanwj merged this on Feb 10, 2016
laanwj closed this on Feb 10, 2016
laanwj referenced this in commit 2f3f4af4cc on Feb 10, 2016
laanwj referenced this in commit 889e5b3050 on Feb 10, 2016
laanwj commented at 7:35 PM on February 10, 2016: member

Cherry-picked to 0.12 as 889e5b3050e78614acb45ea0845dc8fd33b157bf
laanwj removed the label Needs backport on Feb 10, 2016
dcousens commented at 2:08 AM on February 11, 2016: contributor

utACK
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#6862 Unclear rejection messages in master