Skip to content

Made the `CTransaction` class no longer default constructible

Summary

This MR builds on top of the commit in !758 (closed).

This is in a series of 2 for code cleanup/better enforcement of design for the CTransaction class. The second MR that builds on top of this is !760 (merged).

I created this series of 3 2 to fix up some type safety issues with respect to txmempool.cpp and the CTransaction class. We want to be enforcing tight constraints on this class and not allowing possibly inefficient or buggy usage (as we saw before !761 (merged) was merged).

Previously, the default construction of CTransaction was used sparsely in this codebase -- mainly to capture the concept of a "null" or empty transaction.

The code comments themselves warned that the default constructible CTransaction class is not desired and indicated in a "TODO" that the default construction should be removed.

I am in agreement with this -- for type safety and to prevent people working with CTransaction vectors directly (really everything should be using CTransactionRef).

So I made the default constructor private (and I also made the CTransaction class final).

Instead we expose a global static member null and sharedNull to capture the concept of "nullness" where it is needed (mainly in test code).

The changes to the code were surprisingly minimal to accomplish this.

Test Plan

This commit introduces no semantic or behavioral changes to the code, this is purely a code quality commit.

  • ninja all check bench_bitcoin check-functional to ensure everything compiles and the tests pass.
Edited by Calin Culianu

Merge request reports