Cache hashes in CTransaction and CBlock #4292

This is an alternative to #4283, which caches computed hashes inside CTransaction and CBlock.

It should cover more cases, but relies on the assumption that the hash does not change once computed (except for a few cases, where MarkDirty() is called), and that there are no two threads asking for a hash simultaneously for the same time (or otherwise don't destructively overwrite each other's data).

I also don't think these actually belong inside core data structures, but should be part of a "validation context" object for blocks or transactions instead. Those don't exist, however, and this simplifies the code in many places (by not requiring the hash to be passed separately to avoid recalculation).

and that there are no two threads asking for a hash simultaneously for the same time (or otherwise don't destructively overwrite each other's data)

I'm not sure that this is a safe assumption, the GetHash() is called all over the place. That's hard to control.

Thinking about it, as you have an explicit MarkDirty() (in contrast to one that is called with high frequency on every change), we could make that compute the hash. That'd make race issues more controllable -- you'd just have to make sure no two threads are updating a transaction (and calling subsequent MarkDirty) at the same time. (i also don't think it would give extra overhead - one of the things that gets called for sure on a transaction is GetHash, if only to put it in a map of some kind)

Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/d78428a9c1ae848eab1c193f6ef3ce3edbd76c3f for binaries and test log.

This could happen for one of several reasons:

1. It chanages paths in makefile.linux-mingw or otherwise changes build scripts in a way that made them incompatible with the automated testing scripts (please tweak those patches in qa/pull-tester)
2. It adds/modifies tests which test network rules (thanks for doing that), which conflicts with a patch applied at test time
3. It does not build on either Linux i386 or Win32 (via MinGW cross compile)
4. The test suite fails on either Linux i386 or Win32
5. The block test-cases failed (lookup the first bNN identifier which failed in https://github.com/TheBlueMatt/test-scripts/blob/master/FullBlockTestGenerator.java)

If you believe this to be in error, please ping BlueMatt on freenode or TheBlueMatt here.

This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

@sipa FWIW, this style of caching is what I did in python and C libs, and others have copied also. It is useful.

Another, and much cleaner, solution is adding a CMutableTransaction and CMutableBlock, which have the same fields, but mutable, and do not provide a GetHash. You can convert them to their immutable versions (which can be very efficient by just swapping the vin/vout/vtx vectors) once they are finished.

@sipa I like that solution a lot. It has self-enforcing correctness. No chance to forget MarkDirty, and thread problems are avoided by never sharing the mutable structure between threads.

@sipa +1

I was thinking of something like a CMutableTransaction myself for my python-bitcoinlib for use in conjunction with Cython support. I've already made CScript's immutable (they're bytes subclasses) and would have made CMutableTxIn and CMutableTxOut as well. COutPoint can probably be conveniently made immutable always - not often you want to change one of its two members without changing the other.

Closing in favor of #4309.