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. 
   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

5. 
   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)?
6. 
   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

7. 
   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?

8. 
   gmaxwell commented at 7:08 pm on November 3, 2015: contributor

   utACK (will test)
9. 
   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.
10. gmaxwell added this to the milestone 0.12.0 on Nov 5, 2015
11. 
    TheBlueMatt commented at 11:33 pm on November 6, 2015: member

    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]

12. 
    TheBlueMatt commented at 11:34 pm on November 6, 2015: member

    In other words, of some random set of blocks (~356331-357552), this pull took the ConnectBlock time from 230s to 160s!
13. 
    sipa commented at 0:32 am on November 7, 2015: member

    Concept & code review ACK.
14. laanwj added the label Validation on Nov 10, 2015
15. 
    petertodd commented at 9:06 pm on November 10, 2015: contributor

    utACK modulo duplicate testnet coinbases issue.
16. 
    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

17. 
    sipa commented at 7:21 am on November 11, 2015: member

    Untested ACK
18. 
    morcos commented at 12:08 pm on November 12, 2015: member

    Also confirmed unique coinbases on tesnet3.
19. sipa merged this on Nov 12, 2015
20. sipa closed this on Nov 12, 2015

    ---

21. sipa referenced this in commit 54e8bfec83 on Nov 12, 2015
22. TheExiledMonk referenced this in commit dbea57a93f on Oct 11, 2016
23. cerebrus29301 referenced this in commit 04aaeee24f on Feb 4, 2017
24. cerebrus29301 referenced this in commit 97d36d950f on Feb 4, 2017
25. laanwj referenced this in commit 3fa24bb217 on Mar 7, 2018
26. deadalnix referenced this in commit 1e1a278bde on Feb 3, 2020
27. furszy referenced this in commit 85b5f2eb83 on Aug 2, 2020
28. random-zebra referenced this in commit 3c767c46b5 on Aug 8, 2020
29. Stamek referenced this in commit 87ced407c7 on Aug 22, 2020
30. PastaPastaPasta referenced this in commit 5d90db11a0 on Jun 27, 2021
31. PastaPastaPasta referenced this in commit 99db01e5cd on Jun 28, 2021
32. PastaPastaPasta referenced this in commit 1026aba5f4 on Jun 28, 2021
33. PastaPastaPasta referenced this in commit bd203ede2e on Jun 28, 2021
34. PastaPastaPasta referenced this in commit 22c1d77c23 on Jun 29, 2021
35. DrahtBot locked this on Sep 8, 2021

Contributors
morcos instagibbs gavinandresen sdaftuar gmaxwell sipa TheBlueMatt petertodd

Labels
Validation

Milestone
0.12.0