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 more script edge condition tests. #6112

Merged
merged 1 commit into from May 7, 2015

Conversation

davecgh
Copy link
Contributor

@davecgh davecgh commented May 6, 2015

This pull request adds some tests to the script_valid.json and tx_invalid.json data which exercise more edge conditions that are not currently being tested.

This commit adds some tests to the script_valid.json and tx_invalid.json
data which exercise more edge conditions that are not currently being
tested.
@gavinandresen
Copy link
Contributor

Untested ACK-- I trust Travis exercised these new tests.
Should be merged quickly in my opinion, testing edge cases is a very good thing.

@sipa
Copy link
Member

sipa commented May 6, 2015 via email

["See FindAndDelete, which will only remove if the signature, including the hash type, matches"],
[[["0000000000000000000000000000000000000000000000000000000000000100", 0, "DUP HASH160 0x14 0x5b6462475454710f3c22f5fdf0b40704c92f25c3 EQUALVERIFY CHECKSIGVERIFY 1 0x47 0x3044022067288ea50aa799543a536ff9306f8e1cba05b9c6b10951175b924f96732555ed022026d7b5265f38d21541519e4a1e55044d5b9e17e15cdbaf29ae3792e99e883e7a81"]],
"01000000010001000000000000000000000000000000000000000000000000000000000000000000006a473044022067288ea50aa799543a536ff9306f8e1cba05b9c6b10951175b924f96732555ed022026d7b5265f38d21541519e4a1e55044d5b9e17e15cdbaf29ae3792e99e883e7a012103ba8c8b86dea131c22ab967e6dd99bdae8eff7a1f75a2c35f1f944109e3fe5e22ffffffff010000000000000000015100000000", "P2SH"],

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

Damn that's subtle, and also a malleability source come to think of it.

Copy link
Contributor

Choose a reason for hiding this comment

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

To be exact, any bits not covered by the 0x1f == SIGHASH_SINGLE mask can be changed at will w/o invalidating the signature. Not a major issue as the signer has to allow it to happen, but it's an interesting edge case that could come up in certain protocols if you can fool someone into somehow signing the hash 0x1 - conceivable in a multisig payment channel for instance.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, actually I see what you're doing - this isn't related to the SIGHASH_SINGLE bug like I thought. You do successfully test what you think you're testing, but not in a realistic way. More realistic would be to create a test with a CHECKSIG in the scriptSig triggering this condition - that could be an actual tx with otherwise valid hashes.

What I thought you'd noticed was that the SIGHASH_SINGLE bug works regardless of the state of the other SIGHASH flags, something which I don't think anyone's noticed before.

@petertodd
Copy link
Contributor

utACK

@laanwj laanwj merged commit 1c54757 into bitcoin:master May 7, 2015
laanwj added a commit that referenced this pull request May 7, 2015
1c54757 Add more script edge condition tests. (Dave Collins)
dexX7 added a commit to dexX7/bitcoinj that referenced this pull request Sep 29, 2015
Original submission:
```
commit 1c54757f8678c9131ec55c128b8f1d5b6c73c2f9
Author: Dave Collins <davec@conformal.com>
Date:   Wed May 6 10:16:18 2015 -0500

    Add more script edge condition tests.

    This commit adds some tests to the script_valid.json and tx_invalid.json
    data which exercise more edge conditions that are not currently being
    tested.
```

Via:
bitcoin/bitcoin#6112
dexX7 added a commit to dexX7/bitcoinj that referenced this pull request Sep 29, 2015
Original submission:
```
commit 1c54757f8678c9131ec55c128b8f1d5b6c73c2f9
Author: Dave Collins <davec@conformal.com>
Date:   Wed May 6 10:16:18 2015 -0500

    Add more script edge condition tests.

    This commit adds some tests to the script_valid.json and tx_invalid.json
    data which exercise more edge conditions that are not currently being
    tested.
```

Via:
bitcoin/bitcoin#6112
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants