Brainstorming: CTransaction vs CMutableTransaction #15525

If I was to build bitcoin from scratch, I would probably define CBlock and CTransaction, and not CBlock, CTransaction, and CMutableTransaction; CMutableTransaction seems to me to be what should be CTransaction and CTransaction what should be const CTransaction just like there's CBlock and const CBlock and then there is CTransactionRef which could easily be made into std::shared_ptr<const CTransaction> (wait, I think it already is).

There's a bunch of code changes which concern themselves with that distinction, some rejected/closed, some merged:

• #14337
• #14906
• #14908
• #14156
• #13359
• #13309

This seems to be a constant source of confusion and for unnecessary code churn. Would it not maybe be better to get rid of CMutableTransaction once and for all in favor of a clear distinction as in CTransaction and const CTransaction?

@scravy Perhaps it's not clear why the distinction was introduced at first.

CTransaction has a cached txid in it, that is computer at construction time, CMutableTransaction does (obviously) not. This is a useful feature to have, as txids are used very frequently.

There are of course alternatives, such as computing the txid only on first use, or simply leaving the field mutable and making sure it's recomputed anytime before it's used. However, the current solution is safe, hard to screw up, and has no performance impact due to locking or atomics.

Furthermore, in almost all performance critical parts of the code, transactions are immutable. Only in wallet signing related things they aren't.

There are also some vague future advancements like using blob allocation for transactions (all arrays/scripts allocated in a single allocated area) to reduce memory usage, which wouldn't apply to mutable transactions.

@sipa Thank you for the insight.