Cache transaction validation successes #6077

1. 
   sipa commented at 5:53 pm on April 28, 2015: member

   This is an alternative to @gavinandresen’s #5835, which caches transaction validation results in an mru cache in main, rather than relying on the mempool in consensus code. The cache keeps the last 33k results, and uses around 3 MB on 64-bit systems.

   This pull request also contains a rebased version of the unit test from #5835, and the optimizations from #6064.

2. sipa renamed this:
   Cache transactionc validation successes
   Cache transaction validation successes
   on Apr 28, 2015
3. laanwj added the label Validation on Apr 29, 2015
4. sipa force-pushed on Apr 29, 2015
5. 
   sipa commented at 4:55 pm on April 29, 2015: member

   Reimplemented the mrumap using boost::multiindex, which is more compact, and offers erasing elements too. Entries are now removed from the tx validation cache when they are accepted into a block, meaning the last 33k valid unconfirmed transactions are cached, rather than just the last 33k whatsoever.
6. sipa force-pushed on Apr 29, 2015
7. sipa force-pushed on Apr 30, 2015
8. sipa force-pushed on Apr 30, 2015
9. 
   sipa commented at 3:35 pm on April 30, 2015: member

   Rebased on new #6064.
10. sipa force-pushed on May 1, 2015
11. 
    gavinandresen commented at 9:01 pm on May 15, 2015: contributor

    Concept ACK. I hope to run some benchmarks to see how much this speeds up validation with a stream of real ’tx’ and ‘block’ messages and different -dbcache settings, but that probably won’t happen until I’m back from New York late next week.

    I also want to benchmark #5967 and #6102 .

12. sipa force-pushed on Jun 14, 2015
13. 
    sipa commented at 2:12 pm on June 14, 2015: member

    Rebased.
14. sipa force-pushed on Jun 16, 2015
15. 
    sipa commented at 2:49 pm on June 16, 2015: member

    Actually rebased.
16. 
    sipa commented at 1:22 pm on June 21, 2015: member

    Been running on bitcoin.sipa.be for a week, no problems.
17. 
    sipa commented at 12:51 pm on July 27, 2015: member

    ping
18. 
    jgarzik commented at 1:19 pm on July 27, 2015: contributor

    “it works” positive & negative test + review ACK
19. Cache transaction validation successes 17b11428c1
20. sipa force-pushed on Jul 27, 2015
21. 
    sipa commented at 1:30 pm on July 27, 2015: member

    Rebased.
22. Unit test doublespends in new blocks

    As suggested by Greg Maxwell-- unit test to make sure a block
    with a double-spend in it doesn't pass validation if half of
    the double-spend is already in the memory pool (so full-blown
    transaction validation is skipped) when the block is received.

    517e6dd256
23. sipa force-pushed on Jul 27, 2015
24. 
    laanwj commented at 2:13 pm on July 27, 2015: member

    ACK
25. laanwj merged this on Jul 27, 2015
26. laanwj closed this on Jul 27, 2015

    ---

27. laanwj referenced this in commit 08e9c57ba2 on Jul 27, 2015
28. sipa referenced this in commit bc484ef8db on Jul 27, 2015
29. 
    laanwj commented at 4:39 pm on July 27, 2015: member

    Backported to 0.11 as bc484ef8db02715e283e4cddd2c972316c0688dd
30. 
    jtimon commented at 4:21 pm on July 28, 2015: contributor

    I missed this.. We want to cache unconfirmed transactions, sure. But why cache the transactions that are being confirmed in connectBlock?

    EDIT: As in, couldn’t this have been done in AcceptToMemoryPool instead of CheckScript (which, by the way, it’s called twice inside AcceptToMemoryPool)?

31. 
    sipa commented at 4:24 pm on July 28, 2015: member

    @jtimon To avoid rechecking the same transaction when it enters a block, when it has already been verified for the mempool (or whatever other reason).
32. 
    jtimon commented at 4:28 pm on July 28, 2015: contributor

    @sipa Thanks, I guess this the simplest way to do that right now.
33. 
    sdaftuar commented at 5:54 pm on July 28, 2015: member

    As mentioned on IRC, I think there’s a problem with transaction checks involving block height. Consider a coinbase spend that is valid when the transaction arrives and is accepted into the mempool, but is invalid at any earlier block height because of the coinbase maturity test. If there’s a reorg, then when validating blocks at earlier heights, we’d consider blocks valid even if they include this transaction at a too-early height, because of this validation cache. (A test that demonstrates this is here: https://gist.github.com/sdaftuar/23a3db523e68541a3195)
34. 
    jtimon commented at 6:02 pm on July 28, 2015: contributor

    @sdaftuar now that you say it, that’s why I repeated the checks even when the transaction had been fully validated here: https://github.com/jtimon/bitcoin/commit/935ee1ec875308f27339418363c787ec061d335f#diff-7ec3c68a81efff79b6ca22ac1f1eabbaR779 Maybe I was too fast closing #6445 (which should solve the problem) after all…
35. 
    sipa commented at 6:14 pm on July 28, 2015: member

    Going to revert, thanks for pointing this out, @sdaftuar.
36. 
    jtimon commented at 11:04 pm on August 21, 2015: contributor

    This wasn’t fully reverted, https://github.com/sipa/bitcoin/commit/517e6dd25618522c716e64859554b0f29c6e65d0 broke #6235 and #6382 @laanwj don’t merge #6235 because that would break the build on master. Fixing now…
37. laanwj referenced this in commit 17ea542aa6 on Oct 9, 2015
38. zkbot referenced this in commit f1aeaec471 on Mar 21, 2018
39. zkbot referenced this in commit 4fc490c430 on Dec 4, 2019
40. zkbot referenced this in commit 868c63f92d on Dec 4, 2019
41. DrahtBot locked this on Sep 8, 2021

Contributors
sipa gavinandresen jgarzik laanwj jtimon sdaftuar

Labels
Validation