Make tx_valid.json/tx_invalid.json validation flags configurable #4876

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

sipa commented at 7:38 PM on September 8, 2014: member

As discussed with @schildbach and @petertodd on IRC (who were willing to add the same to BitcoinJ and python-bitcoin)

This adds P2SH+STRICTENC to every test case for now. Way overkill, but makes this patch have no semantic impact.
schildbach commented at 8:00 PM on September 8, 2014: contributor

Thanks sipa!
schildbach commented at 8:20 PM on September 8, 2014: contributor

Misleading title: Actually it's the script_*.json validation flags.
Make script_{valid,invalid}.json validation flags configurable 6022b5dc6b
sipa force-pushed on Sep 8, 2014
sipa commented at 8:39 PM on September 8, 2014: member

@schildbach Thanks, fixed.
BitcoinPullTester commented at 9:07 PM on September 8, 2014: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4876_6022b5dc6ba930797db46e00426923b53d6594b3/ for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
gavinandresen commented at 9:12 PM on September 8, 2014: contributor

ACK.
petertodd commented at 3:30 PM on September 9, 2014: contributor

ut ACK.
schildbach commented at 8:04 PM on September 10, 2014: contributor

I have prepared the code for bitcoinj. I'm now waiting for this PR to be merged.
jgarzik commented at 3:09 PM on September 12, 2014: contributor

Concept ACK. I did not verify that any tests were added or removed.
sipa merged this on Sep 12, 2014
sipa closed this on Sep 12, 2014
sipa referenced this in commit b9307d2f55 on Sep 12, 2014
schildbach commented at 7:51 AM on September 19, 2014: contributor
Current state of progress on bitcoinj master:
- proper JSON parser used (it was just doing some String.split() and regexp operations)
- all verifyFlags are parsed
- P2SH and NULLDUMMY are implemented
- STRICTENC is not implemented (the tests pass anyway -- is this expected?)
petertodd commented at 9:22 AM on September 19, 2014: contributor

@schildbach STRICTENC is not implemented (the tests pass anyway -- is this expected?)

That's expected - the STRICTENC tests are in a separate file right now and need to be put in the .json testcases.
schildbach commented at 9:41 AM on September 19, 2014: contributor

Thanks, What's that file called, and (when) will they be put into the .json testcases?
petertodd commented at 11:00 AM on September 19, 2014: contributor

They're in canonical_tests.cpp

When? When someone does the work. :) I'm in no rush myself, because I've been told python-bitcoinlib doesn't even reliably spit out canonical signatures w/ the EVEN/ODD or whatever it is rule; I'm in no rush to delve into OpenSSL's docs and figure out how to fix that.
sipa commented at 4:35 PM on September 19, 2014: member

@schildbach I'll work on that. @petertodd Don't confuse the strict DER ("canonical encoding") requirement with the low-S rule (which is just there to break malleability). The latter (the S value in the signature needs to be less than half of the curve's order; the exact semantics are in BIP62) is Bitcoin specific, and not something you can ask OpenSSL to do, unfortunately. It's not hard to fix manually though. There is no even/odd rule anymore - that used to be in the code for a short time, but low-s instead of even-s is not harder and results in on average half a byte smaller sigs.
MarcoFalke locked this on Sep 8, 2021

Contributors