Remove P2SH transition code: P2SH violations may cause DoS trigger #1710

1. 
   sipa commented at 12:37 PM on August 23, 2012: member

   We had code to check for P2SH violations specifically, in order to not cause DoS triggers from them during the transition period. This is no longer necessary.

   It also improves performance for detecting invalid scripts a bit.

2. 
   gmaxwell commented at 2:26 PM on August 23, 2012: contributor

   I don't disagree with doing this, but we should keep in mind that a slightly unfortunate result of this is that an attacker could network isolate old pre-bip16 nodes by constantly flooding invalid transactions. If there are any pre-bip16 miners remaining, the attacker will be able to force the creation of a forked network.

3. 
   sipa commented at 4:38 PM on August 23, 2012: member

   I was about to say: if there are any non-BIP16 miners left, I pity them. However, http://blockchain.info/p2sh says 37% doesn't include the marker string?

4. 
   gmaxwell commented at 4:40 PM on August 23, 2012: contributor

   http://blockchain.info/tx-index/3618498/4005d6bea3a93fb72f006d23e2685b85069d270cb57d15f0c057ef2d5e3f78d2 doesn't suggest that it's anywhere near that many miners. I think those are mostly just ones not including it. It's probably only a very small amount without it.

5. 
   luke-jr commented at 5:48 PM on August 23, 2012: member

   19% don't enforce BIP16

6. 
   gmaxwell commented at 5:52 PM on August 23, 2012: contributor

   Probably should defer to 0.8.

7. 
   sipa commented at 5:55 PM on August 23, 2012: member

   Agree, in that case.

8. 
   BitcoinPullTester commented at 1:22 AM on August 24, 2012: none

   Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/59ddfe1caecc827aa76ee5bb2c783c92d049301a for binaries and test log.

9. 
   Diapolo commented at 9:18 AM on September 9, 2012: none

   I saw another P2SH code part which could perhaps be removed, just want to mention, it's in init.cpp:

   <pre> // Continue to put "/P2SH/" in the coinbase to monitor // BIP16 support. // This can be removed eventually... const char* pszP2SH = "/P2SH/"; COINBASE_FLAGS << std::vector<unsigned char>(pszP2SH, pszP2SH+strlen(pszP2SH)); </pre>

10. 
    gavinandresen commented at 8:22 PM on September 25, 2012: contributor

    ACK

11. 
    jgarzik commented at 8:25 PM on September 25, 2012: contributor

    ACK

12. 
    sipa commented at 5:35 PM on October 7, 2012: member

    ACK for 0.7.1 or 0.8?

13. 
    gmaxwell commented at 6:53 PM on October 7, 2012: contributor

    Luke's version tracker (http://luke.dashjr.org/programs/bitcoin/files/charts/BIP-0016.html) says 13.4% are non-P2SH. Thats now comparable to non-BIP30 (13%) so it's probably not going to get much better count-wise by waiting.

14. Remove P2SH transition code: P2SH violations may cause DoS trigger 28982cc9dc
15. 
    sipa commented at 11:42 PM on October 28, 2012: member

    Rebased.

16. 
    Diapolo commented at 6:46 AM on October 29, 2012: none

    As the discussion is over I would suggest to merge such a code-wise small pull. We have quite some pulls open so let's reduce their numbers, if they are non-controversial?

17. sipa referenced this in commit c13f5dbecf on Oct 29, 2012
18. sipa merged this on Oct 29, 2012
19. sipa closed this on Oct 29, 2012

    ---

20. laudney referenced this in commit f7e9a5faa8 on Mar 19, 2014
21. DrahtBot locked this on Sep 8, 2021

Milestone
0.8.0