Skip to content

Conversation

@pinheadmz
Copy link

Addresses bitcoin#17977 (comment)

Tests the actual previous transaction output scriptPubKey for P2SH before applying taproot standardness checks.

Also adds test for version-1-witness-in-P2SH which is NOT taproot and should not be subject to these standardness checks.

This is a hard case to test, because P2SH is correctly checked in interpreter VerifyWitnessProgram() and so the test case will be thrown out as non-standard for spending an unknown witness version.

To test the test, I wrote a commit pinheadmz@ba1ac81 which bypasses the check in VerifyWitnessProgram(), allowing the v1-in-P2SH to get caught in policy IsWitnessStandard() which is patched by this PR.

I'm not sure if there's a better way to cover this patch because there's (at least) two standardness checks applied.

@jnewbery
Copy link

jnewbery commented Mar 9, 2020

Yes, I think you're right that checking for !prevScript.IsPayToScriptHash() is a bug, and the change in IsWitnessStandard() seems correct to me. I haven't reviewed the tests yet.

@sipa sipa force-pushed the taproot branch 9 times, most recently from 4f72ac9 to 7641c52 Compare March 21, 2020 18:55
@jnewbery
Copy link

Is this fixed by bitcoin#17977 (comment) ?

@pinheadmz
Copy link
Author

@jnewbery looks like it -- if you're interested in the test I can rebase so the diff is just that? Or just close.

@sipa
Copy link
Owner

sipa commented Mar 21, 2020

Oh, I missed this PR, sorry about that.

I think this is fixed, but I'll cherry-pick the test.

@sipa
Copy link
Owner

sipa commented Mar 21, 2020

Done.

@sipa sipa closed this Mar 21, 2020
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.

3 participants