Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add script verification flags for DER format #1090

Closed
wants to merge 1 commit into from

Conversation

rnicoll
Copy link
Contributor

@rnicoll rnicoll commented Oct 1, 2015

Add script verification flags for DER format encoding, low-S signatures and strict encoding.
Pass script verification flags through to signature parser.
Convert block verification flags to a sub-enum of Block.
Remove DER signature format verification flag from block flags.
Add test transaction with a non-standard DER signature, with the verify flag set/unset accordingly, to tx_invalid.json and tx_valid.json

Note this changes behaviour of Script.correctlySpends(Transaction, long, Script) as the method applies all flags, which now includes the new DER flag. The method version which takes in flags as an argument is unaffected.

@rnicoll rnicoll force-pushed the bip66 branch 2 times, most recently from 9066084 to 26d1825 Compare October 18, 2015 10:12
@rnicoll
Copy link
Contributor Author

rnicoll commented Oct 18, 2015

Have now fixed the excessive log spam that was upsetting Travis.

@@ -109,5 +109,9 @@
[[["ad503f72c18df5801ee64d76090afe4c607fb2b822e9b7b63c5826c50e22fc3b", 0, "0x21 0x027c3a97665bf283a102a587a62a30a0c102d4d3b141015e2cae6f64e2543113e5 CHECKSIG NOT"]],
"01000000013bfc220ec526583cb6b7e922b8b27f604cfe0a09764de61e80f58dc1723f50ad0000000000ffffffff0101000000000000002321027c3a97665bf283a102a587a62a30a0c102d4d3b141015e2cae6f64e2543113e5ac00000000", "P2SH"],

["A transaction with a non-standard DER signature."],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I take it you created these test cases yourself? That's great!

There is only one issue with that: we sync these .json files from Bitcoin Core on a regular basis. So maybe you can submit your testcases to Core too?

Add script verification flags for DER format encoding, low-S signatures
and strict encoding.
Pass script verification flags through to signature parser.
Convert block verification flags to a sub-enum of Block.
Remove DER signature format verification flag from block flags.
Add test transaction with a non-standard DER signature, with the verify
flag set/unset accordingly, to tx_invalid.json and tx_valid.json
@rnicoll
Copy link
Contributor Author

rnicoll commented Oct 18, 2015

Okay, just cutting & pasting in the Bitcoin script tests isn't as trivial as I thought, I'll look at syncing both test data sets later on. Bitcoin doesn't have a transaction test (my mistake), so I have submitted the two test cases as bitcoin/bitcoin#6848.

Unused "{}" removed as well.

@schildbach
Copy link
Member

Merged. Thanks!

@schildbach schildbach closed this Oct 20, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants