Merge #14908: test: Removed implicit CTransaction constructor calls from tests and benchmarks.

8db0c3d42b Removed implicit CTransaction conversion from benchmaks (lucash-dev)
ed61abedb2 Removed implicit CTransaction constructor from tests (lucash-dev)

Pull request description:

  This PR was split from #14906 and is a prerequisite for it.
  It updates tests and benchmarks, removing all implicit calls to `CTransaction(CMutableTransaction&)` constructors. This will make possible making the constructor explicit in the next PR.
  The original rationale for making the constructor explicit:

   - Conversion constructors should not be explicit unless there's a strong reason for it (in the opinion of, for example, https://google.github.io/styleguide/cppguide.html, and https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Ro-conversion. Let me know your take on this).
   - This particular conversion is very costly -- it implies a serialization plus hash of the transaction.
   - Even though `CTransaction` and `CMutableTransaction` represent the same data, they have very different use cases and performance properties.
   - Making it explicit allows for easier reasoning of performance trade-offs.
   - There has been previous performance issues caused by unneeded use of this implicit conversion.
   - This PR creates a map for places to look for possible refactoring and performance gains (this benefit still holds if the PR is not merged).

Tree-SHA512: de8073aa6ff8a3153bcbe10818616677ecf9598e4978d8a0b4c39a262e71c36be5679cec08554c760d1f011ba6d37350318248eef15f6d9b86f9e4462b2de0d2

2 files changed, 6 insertions, 5 deletions

diff --git a/src/bench/ccoins_caching.cpp b/src/bench/ccoins_caching.cpp
index b8d82c0a8..9cfd5d23e 100644
--- a/src/bench/ccoins_caching.cpp
+++ b/src/bench/ccoins_caching.cpp
@@ -35,14 +35,14 @@ SetupDummyInputs(CBasicKeyStore& keystoreRet, CCoinsViewCache& coinsRet)
     dummyTransactions[0].vout[0].scriptPubKey << ToByteVector(key[0].GetPubKey()) << OP_CHECKSIG;
     dummyTransactions[0].vout[1].nValue = 50 * COIN;
     dummyTransactions[0].vout[1].scriptPubKey << ToByteVector(key[1].GetPubKey()) << OP_CHECKSIG;
-    AddCoins(coinsRet, dummyTransactions[0], 0);
+    AddCoins(coinsRet, CTransaction(dummyTransactions[0]), 0);
 
     dummyTransactions[1].vout.resize(2);
     dummyTransactions[1].vout[0].nValue = 21 * COIN;
     dummyTransactions[1].vout[0].scriptPubKey = GetScriptForDestination(key[2].GetPubKey().GetID());
     dummyTransactions[1].vout[1].nValue = 22 * COIN;
     dummyTransactions[1].vout[1].scriptPubKey = GetScriptForDestination(key[3].GetPubKey().GetID());
-    AddCoins(coinsRet, dummyTransactions[1], 0);
+    AddCoins(coinsRet, CTransaction(dummyTransactions[1]), 0);
 
     return dummyTransactions;
 }
@@ -76,10 +76,11 @@ static void CCoinsCaching(benchmark::State& state)
     t1.vout[0].scriptPubKey << OP_1;
 
     // Benchmark.
+    const CTransaction tx_1(t1);
     while (state.KeepRunning()) {
-        bool success = AreInputsStandard(t1, coins);
+        bool success = AreInputsStandard(tx_1, coins);
         assert(success);
-        CAmount value = coins.GetValueIn(t1);
+        CAmount value = coins.GetValueIn(tx_1);
         assert(value == (50 + 21 + 22) * COIN);
     }
 }
diff --git a/src/bench/mempool_eviction.cpp b/src/bench/mempool_eviction.cpp
index 3908a7d23..49ea6e88b 100644
--- a/src/bench/mempool_eviction.cpp
+++ b/src/bench/mempool_eviction.cpp
@@ -127,7 +127,7 @@ static void MempoolEviction(benchmark::State& state)
         AddTx(tx6_r, 1100LL, pool);
         AddTx(tx7_r, 9000LL, pool);
         pool.TrimToSize(pool.DynamicMemoryUsage() * 3 / 4);
-        pool.TrimToSize(GetVirtualTransactionSize(tx1));
+        pool.TrimToSize(GetVirtualTransactionSize(*tx1_r));
     }
 }