[Tests] Missing test case for SIGHASH_SINGLE|SIGHASH_ANYONECANPAY #12060

issue NicolasDorier opened this issue on December 31, 2017

NicolasDorier commented at 1:18 AM on December 31, 2017: contributor

I recently found a consensus bug in NBitcoin script interpreter. (https://github.com/MetacoSA/NBitcoin/commit/7fd1c358b920fdc6c75417f9f2646f2fc0f59585)

It was caused by an out of bound signature with SIGHASH_SINGLE|SIGHASH_ANYONECANPAY recently mined on testnet.

My test cases for the script evaluator are using the same, fairly recent (maybe 1 year ago ?) json test cases of Bitcoin Core, and it failed to catch this bug. So I guess this test is missing from Core as well. (I still need to confirm that)

I wrote this test meanwhile in NBitcoin https://github.com/MetacoSA/NBitcoin/blob/7fd1c358b920fdc6c75417f9f2646f2fc0f59585/NBitcoin.Tests/script_tests.cs#L785

I plan to add test case to bitcoin core for it as well (in script_valid.json which I don't find anymore), but I just don't have time for now, so I am opening issue meanwhile.
fanquake added the label Tests on Dec 31, 2017
practicalswift commented at 4:49 PM on January 2, 2018: contributor

Nice find!
Christewart commented at 5:10 PM on January 3, 2018: member

It appears I have the same bug in bitcoin-s, so I took the time to update tx_valid.json to add the test case you have linked in NBitcoin.
NicolasDorier commented at 2:03 PM on January 4, 2018: contributor

@Christewart where did you got the bug from? I thought I got the bug from bitcoin core, but core does not have such bug since the beginning... Maybe I got this code from BitcoinJ?
Christewart commented at 5:50 PM on January 5, 2018: member

@NicolasDorier I used bitcoinj for my first pass at implementing signature serialization inside of bitcoin-s. I actually have bitcoinj comments inside the bitcoin-s code to help with my implementation. I would guess that bitcoinj probably has this error too -- but I haven't checked with them.

https://github.com/bitcoin-s/bitcoin-s-core/blob/master/src/main/scala/org/bitcoins/core/crypto/TransactionSignatureSerializer.scala#L33
NicolasDorier commented at 4:13 AM on January 9, 2018: contributor

It seems they don't have this bug now. I probably got it from BitcoinJ initially though.
MarcoFalke referenced this in commit 0910cbe4ef on Jan 11, 2018
Christewart commented at 7:35 PM on January 11, 2018: member

I believe this can be closed now?
MarcoFalke closed this on Jan 12, 2018
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#12082 Adding test case for SINGLE|ANYONECANPAY hash type in tx_valid.json