Skip BIP 30 verification where not necessary #6931

1. 
   morcos commented at 2:20 AM on November 3, 2015: member

   Since BIP 34 became active, it has been impossible to generate duplicate coinbases, and therefore not possible to generate any other duplicate transaction ID's.

   This pull will allow us to skip BIP 30 verification if we know we are on a chain after the point BIP 34 was activated and there are no latent duplicate transaction chains.
   On mainnet there were 2 cases of duplicate coinbases, and in both cases the duplicate overwrote the original before there was an opportunity to spend it. I have not actually verified that there are no duplicate coinbases on testnet3, and we need to verify that or turn off the skipping on testnet before merging this pull. (EDIT: verified)

   In conjunction with #6932 this will help ConnectBlock be much more efficient with caching access to the database.

2. Skip BIP30 check after BIP34 activation 06d81ad516
3. Make skipping BIP30 check chain agnostic 33c90cf197
4. morcos cross-referenced this on Nov 3, 2015 from issue ModifyNewCoins saves database lookups by morcos
5. 
   instagibbs commented at 4:07 PM on November 3, 2015: member

   Do you have a benchmark with both PRs together?

   utACK, aside from the testnet3 coinbase issue

6. 
   gavinandresen commented at 5:11 PM on November 3, 2015: contributor

   Concept ACK, but the logic seems more complicated than necessary. Couldn't you just remove the BIP30 checking code entirely now that BIP34 has activated? Why do the work in the old chain, when we know the old chain doesn't violate the rules (and have headers-first to prevent somebody from feeding us a low-work bogus chain)?

7. 
   sdaftuar commented at 5:22 PM on November 3, 2015: member

   @gavinandresen Headers-first doesn't actually prevent someone from doing the attack -- for example the single peer a node chooses to download headers from could feed a bogus chain before a node ever learns about the real chain.

   Concept ACK

8. 
   morcos commented at 5:42 PM on November 3, 2015: member

   @instagibbs I'm working on putting together some benchmarks. There are a set of pulls that I've been working on to speed up ConnectBlock and CreateNewBlock. I'll post an issue identifying them all and with some benchmarks. But roughly speaking we're talking about these times on average over some recent data for CreateNewBlock (with 1GB dbcache and 1M sigcachesize)

   Code version	time in ms
   Master	550
   libsecp256k merged	340
   libsecp and these 2 pulls*	160

   * I actually didn't do the coinbase lookup required in 6932 for this test, so that can be the difference between 0 and 1 db lookup. @gavinandresen I'd be interested if you have any thoughts on how best to benchmark this as well?

9. 
   gmaxwell commented at 7:08 PM on November 3, 2015: contributor

   utACK (will test)

10. 
    sipa commented at 6:04 AM on November 4, 2015: member

    @gavinandresen I wanted to do that first too when this brought up, see #6916.

11. gmaxwell added this to the milestone 0.12.0 on Nov 5, 2015
12. TheBlueMatt cross-referenced this on Nov 6, 2015 from issue Benchmark sanity checks and fork checks in ConnectBlock by TheBlueMatt
13. 
    TheBlueMatt commented at 11:33 PM on November 6, 2015: contributor

    Some random stats (generated using #6965):

    2015-11-06 23:22:03 UpdateTip: new best=00000000000000000e168ce1bbcdab4551cc35e9d5128b75e5d603143cdc970a height=357552 log2_work=82.822239 tx=69465120 date=2015-05-22 12:42:52 progress=0.799217 cache=1120.9MiB(601684tx) 2015-11-06 23:22:03 - Connect postprocess: 3.40ms [2.65s] 2015-11-06 23:22:03 - Connect block: 328.16ms [230.41s] 2015-11-06 23:22:03 - Load block from disk: 18.28ms [21.80s] 2015-11-06 23:22:03 - Sanity checks: 2.83ms [1.73s] 2015-11-06 23:22:03 - Fork checks: 68.39ms [38.97s] 2015-11-06 23:22:03 - Connect 1501 transactions: 178.28ms (0.119ms/tx, 0.043ms/txin) [92.67s] 2015-11-06 23:22:03 - Verify 4109 txins: 245.83ms (0.060ms/txin) [150.33s] 2015-11-06 23:22:03 - Index writing: 0.04ms [0.06s] 2015-11-06 23:22:03 - Callbacks: 0.03ms [0.03s] 2015-11-06 23:22:03 - Connect total: 318.09ms [191.65s] 2015-11-06 23:22:03 - Flush: 26.23ms [14.01s] 2015-11-06 23:22:03 - Writing chainstate: 2.05ms [0.67s]

    2015-11-06 23:32:37 UpdateTip: new best=00000000000000000e168ce1bbcdab4551cc35e9d5128b75e5d603143cdc970a height=357552 log2_work=82.822239 tx=69465120 date=2015-05-22 12:42:52 progress=0.799210 cache=1120.9MiB(601684tx) 2015-11-06 23:32:37 - Connect postprocess: 5.06ms [2.85s] 2015-11-06 23:32:37 - Connect block: 198.12ms [160.29s] 2015-11-06 23:32:37 - Load block from disk: 14.21ms [7.67s] 2015-11-06 23:32:37 - Sanity checks: 2.79ms [1.81s] 2015-11-06 23:32:37 - Fork checks: 0.04ms [0.14s] 2015-11-06 23:32:37 - Connect 1501 transactions: 173.07ms (0.115ms/tx, 0.042ms/txin) [96.87s] 2015-11-06 23:32:37 - Verify 4109 txins: 196.50ms (0.048ms/txin) [131.39s] 2015-11-06 23:32:37 - Index writing: 0.08ms [0.07s] 2015-11-06 23:32:37 - Callbacks: 0.07ms [0.03s] 2015-11-06 23:32:37 - Connect total: 200.48ms [134.03s] 2015-11-06 23:32:37 - Flush: 27.12ms [15.16s] 2015-11-06 23:32:37 - Writing chainstate: 2.27ms [0.83s]

14. 
    TheBlueMatt commented at 11:34 PM on November 6, 2015: contributor

    In other words, of some random set of blocks (~356331-357552), this pull took the ConnectBlock time from 230s to 160s!

15. 
    sipa commented at 12:32 AM on November 7, 2015: member

    Concept & code review ACK.

16. morcos cross-referenced this on Nov 9, 2015 from issue Summary of potential performance improvements by morcos
17. laanwj added the label Validation on Nov 10, 2015
18. 
    petertodd commented at 9:06 PM on November 10, 2015: contributor

    utACK modulo duplicate testnet coinbases issue.

19. 
    petertodd commented at 10:35 PM on November 10, 2015: contributor

    Just confirmed that all coinbases from block #0 to block #21112 on testnet3 are unique.

    Also successfully did a sync from scratch on testnet3.

    ACK

20. 
    sipa commented at 7:21 AM on November 11, 2015: member

    Untested ACK

21. 
    morcos commented at 12:08 PM on November 12, 2015: member

    Also confirmed unique coinbases on tesnet3.

22. sipa merged this on Nov 12, 2015
23. sipa closed this on Nov 12, 2015

    ---

24. sipa referenced this in commit 54e8bfec83 on Nov 12, 2015
25. Infernoman cross-referenced this on Sep 25, 2016 from issue Skip BIP 30 verification where not necessary #6931 by Infernoman
26. TheExiledMonk referenced this in commit dbea57a93f on Oct 11, 2016
27. cerebrus29301 referenced this in commit 04aaeee24f on Feb 4, 2017
28. cerebrus29301 referenced this in commit 97d36d950f on Feb 4, 2017
29. dagurval cross-referenced this on Mar 14, 2017 from issue Chainparams backports by dagurval
30. morcos cross-referenced this on Jan 16, 2018 from issue Fix overly eager BIP30 bypass by morcos
31. laanwj referenced this in commit 3fa24bb217 on Mar 7, 2018
32. deadalnix referenced this in commit 1e1a278bde on Feb 3, 2020
33. random-zebra cross-referenced this on Jul 31, 2020 from issue [Core] Remove BIP30 check by random-zebra
34. random-zebra cross-referenced this on Jul 31, 2020 from issue [Core] ModifyNewCoins saves database lookups by random-zebra
35. furszy referenced this in commit 85b5f2eb83 on Aug 2, 2020
36. random-zebra referenced this in commit 3c767c46b5 on Aug 8, 2020
37. Stamek referenced this in commit 87ced407c7 on Aug 22, 2020
38. nuttycom cross-referenced this on Feb 11, 2021 from issue Backport of bitcoin/bitcoin#10195 by nuttycom
39. PastaPastaPasta referenced this in commit 5d90db11a0 on Jun 27, 2021
40. PastaPastaPasta referenced this in commit 99db01e5cd on Jun 28, 2021
41. PastaPastaPasta referenced this in commit 1026aba5f4 on Jun 28, 2021
42. PastaPastaPasta referenced this in commit bd203ede2e on Jun 28, 2021
43. PastaPastaPasta referenced this in commit 22c1d77c23 on Jun 29, 2021
44. bitcoin locked this on Sep 8, 2021

Contributors

morcosinstagibbsgavinandresensdaftuargmaxwellsipaTheBlueMattpetertodd

Milestone
0.12.0