Cache segwit signature hash components inside CTransaction to optimize validation performance

[sdaftuar]

Now that CTransactions are typically created once and passed around as shared pointers, and blocks are frequently constructed from the transactions in our mempool (thanks to BIP 152 compact blocks), we can calculate the PrecomputedTransactionData once and save it in the CTransaction, to avoid doing extra work in ConnectBlock.

This PR adds a new deserialization flag to allow for deserializing a CTransaction without calculating these hashes, which is used in ProcessGetData.

Finally, this PR includes a commit which caches the witness hash inside CTransaction.

@jl2012 This was inspired by #9572, although with the approach I've taken here, I don't think it's necessary to additionally skip the hashing for non-segwit transactions, as very few hashes are typically calculated during block validation now (just the ones that are requested via getblocktxn).

Thanks to @TheBlueMatt and @ryanofsky for reviewing several iterations of this.

[sipa]

I strongly dislike moving validation-specific logic into CTransaction (and into primitives/* in general). Wouldn't it be possible to creating a validation-specific wrapper around CTransaction, and then perhaps change CTransactionRef to point to that wrapper?

[TheBlueMatt]

@sipa I tend to disagree pretty strongly. I know you prefer primitives/* to be very dumb-data-storage objects, but since CTransaction is const, it would be quite a waste to not use that to store any precomputed thing we can in it. @sdaftuar had a previous version of this patch that made CTransactionRef a wrapper around a shared_ptr to the CTransaction as well as the precomputed data, but that, too, goes in primitives and ended up just being more complication for no reason.

Anyway, we're already halfway between "dumb storage" and "precalculated" anyway, since we precalculate the hash and store that. The distinction of not precalculating the witness hashes seems arbitrary to me.

[sipa]

Ideally, there should be a BlockValidationContext class or something that can cache all things necessary for validation. I understand that's a non-trivial refactor now, but I do think it should be the goal to even have the hash out of CTransaction...

[TheBlueMatt]

I think that would generate massive layer violations - you would need to use that to pre-compute data in wallet, net, mempool-acceptance, etc.

[sipa]

At least let's not make the situation worse with data that is completely specific to validation.

[TheBlueMatt]

I think you missed my point - I dont believe there is another route to go down (besides, possibly, creating a wrapper around CTransaction[Ref] which is what is actually used everywhere) to get this kind of win.

We can create a src/validation_primitives/transaction.h, but then literally nothing would use the src/primitives/transaction.h version.

[sipa]

Of course there are alternatives. They may need a lot of refactoring to do cleanly, but it feels so incredibly wrong to burden the data structure with things that only validation cares about. NACK.

[TheBlueMatt]

@sipa An alternative like creating a validation context is probably more layer-violating than this...you need to calculate this data in wallet, and net (ie everywhere), so putting it anywhere else means layer violations like crazy.

[sipa]

Net doesn't need the signature validation data. The wallet can use it, but it hardly matters.

[dcousens]

concept NACK for all the reasons @sipa pointed out.

I dont believe there is another route to go down (besides, possibly, creating a wrapper around CTransaction[Ref] which is what is actually used everywhere) to get this kind of win.

Out of interest, what is the magnitude of this win?

[sdaftuar]

@sipa Sorry I missed the discussion yesterday on this; I do have an alternate approach that changes CTransactionRef to point to a wrapper around CTransaction, which I'll go ahead and PR for discussion. I ran into some issues with that approach that might also make it objectionable, but maybe there are ways to better solve those problems that would make it workable.

@dcousens Regarding the performance gain: I've been running some benchmarks on my workstation, and over the last 100 blocks, it looks like this speeds up validation by about 7%.