Remove unused and confusing CTransaction constructor

MarcoFalke · MarcoFalke · commit faac31521bb7 · 2020-12-07T14:59:33.000+01:00
diff --git a/src/primitives/transaction.cpp b/src/primitives/transaction.cpp
@@ -77,8 +77,6 @@ uint256 CTransaction::ComputeWitnessHash() const
     return SerializeHash(*this, SER_GETHASH, 0);
 }
 
-/* For backward compatibility, the hash is initialized to 0. TODO: remove the need for this default constructor entirely. */
-CTransaction::CTransaction() : vin(), vout(), nVersion(CTransaction::CURRENT_VERSION), nLockTime(0), hash{}, m_witness_hash{} {}
 CTransaction::CTransaction(const CMutableTransaction& tx) : vin(tx.vin), vout(tx.vout), nVersion(tx.nVersion), nLockTime(tx.nLockTime), hash{ComputeHash()}, m_witness_hash{ComputeWitnessHash()} {}
 CTransaction::CTransaction(CMutableTransaction&& tx) : vin(std::move(tx.vin)), vout(std::move(tx.vout)), nVersion(tx.nVersion), nLockTime(tx.nLockTime), hash{ComputeHash()}, m_witness_hash{ComputeWitnessHash()} {}
 
diff --git a/src/primitives/transaction.h b/src/primitives/transaction.h
@@ -287,12 +287,9 @@ class CTransaction
     uint256 ComputeWitnessHash() const;
 
 public:
-    /** Construct a CTransaction that qualifies as IsNull() */
-    CTransaction();
-
     /** Convert a CMutableTransaction into a CTransaction. */
-    explicit CTransaction(const CMutableTransaction &tx);
-    CTransaction(CMutableTransaction &&tx);
+    explicit CTransaction(const CMutableTransaction& tx);
+    CTransaction(CMutableTransaction&& tx);
 
     template <typename Stream>
     inline void Serialize(Stream& s) const {
@@ -393,7 +390,6 @@ struct CMutableTransaction
 };
 
 typedef std::shared_ptr<const CTransaction> CTransactionRef;
-static inline CTransactionRef MakeTransactionRef() { return std::make_shared<const CTransaction>(); }
 template <typename Tx> static inline CTransactionRef MakeTransactionRef(Tx&& txIn) { return std::make_shared<const CTransaction>(std::forward<Tx>(txIn)); }
 
 /** A generic txid reference (txid or wtxid). */
diff --git a/src/test/fuzz/script_sigcache.cpp b/src/test/fuzz/script_sigcache.cpp
@@ -29,7 +29,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)
     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
 
     const std::optional<CMutableTransaction> mutable_transaction = ConsumeDeserializable<CMutableTransaction>(fuzzed_data_provider);
-    const CTransaction tx = mutable_transaction ? CTransaction{*mutable_transaction} : CTransaction{};
+    const CTransaction tx{mutable_transaction ? *mutable_transaction : CMutableTransaction{}};
     const unsigned int n_in = fuzzed_data_provider.ConsumeIntegral<unsigned int>();
     const CAmount amount = ConsumeMoney(fuzzed_data_provider);
     const bool store = fuzzed_data_provider.ConsumeBool();
diff --git a/src/test/fuzz/transaction.cpp b/src/test/fuzz/transaction.cpp
@@ -42,7 +42,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)
             return CTransaction(deserialize, ds);
         } catch (const std::ios_base::failure&) {
             valid_tx = false;
-            return CTransaction();
+            return CTransaction{CMutableTransaction{}};
         }
     }();
     bool valid_mutable_tx = true;
diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp
@@ -215,7 +215,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
     // We cannot source new unconfirmed inputs(bip125 rule 2)
     new_coin_control.m_min_depth = 1;
 
-    CTransactionRef tx_new = MakeTransactionRef();
+    CTransactionRef tx_new;
     CAmount fee_ret;
     int change_pos_in_out = -1; // No requested location for change
     bilingual_str fail_reason;

Original file line number	Diff line number	Diff line change
`@@ -77,8 +77,6 @@ uint256 CTransaction::ComputeWitnessHash() const`
`77`	`77`	`return SerializeHash(*this, SER_GETHASH, 0);`
`78`	`78`	`}`
`79`	`79`
`80`		`-/* For backward compatibility, the hash is initialized to 0. TODO: remove the need for this default constructor entirely. */`
`81`		`-CTransaction::CTransaction() : vin(), vout(), nVersion(CTransaction::CURRENT_VERSION), nLockTime(0), hash{}, m_witness_hash{} {}`
`82`	`80`	`CTransaction::CTransaction(const CMutableTransaction& tx) : vin(tx.vin), vout(tx.vout), nVersion(tx.nVersion), nLockTime(tx.nLockTime), hash{ComputeHash()}, m_witness_hash{ComputeWitnessHash()} {}`
`83`	`81`	`CTransaction::CTransaction(CMutableTransaction&& tx) : vin(std::move(tx.vin)), vout(std::move(tx.vout)), nVersion(tx.nVersion), nLockTime(tx.nLockTime), hash{ComputeHash()}, m_witness_hash{ComputeWitnessHash()} {}`
`84`	`82`
Original file line number	Diff line number	Diff line change
`@@ -42,7 +42,7 @@ void test_one_input(const std::vector<uint8_t>& buffer)`
`42`	`42`	`return CTransaction(deserialize, ds);`
`43`	`43`	`} catch (const std::ios_base::failure&) {`
`44`	`44`	`valid_tx = false;`
`45`		`- return CTransaction();`
	`45`	`+ return CTransaction{CMutableTransaction{}};`
`46`	`46`	`}`
`47`	`47`	`}();`
`48`	`48`	`bool valid_mutable_tx = true;`