Merge #13011: Cache witness hash in CTransaction

fac1223a568fa1ad6dd602350598eed278d115e8 Cache witness hash in CTransaction (MarcoFalke)
faab55fbb17f2ea5080bf02bc59eeef5ca746f07 Make CMutableTransaction constructor explicit (MarcoFalke)

Pull request description:

  This speeds up:
  * compactblocks (v2)
  * ATMP
  * validation and miner (via `BlockWitnessMerkleRoot`)
  * sigcache (see also unrelated #13204)
  * rpc and rest (nice, but irrelevant)

  This presumably slows down rescan, which uses a `CTransaction` and its `GetHash`, but never uses the `GetWitnessHash`. The slow down is proportional to the number of witness transactions in the rescan window. I.e. early in the chain there should be no measurable slow down. Later in the chain, there should be a slow down, but acceptable given the speedups in the modules mentioned above.

Tree-SHA512: 443e86acfcceb5af2163e68840c581d44159af3fd1fce266cab3504b29fcd74c50812b69a00d41582e7e1c5ea292f420ce5e892cdfab691da9c24ed1c44536c7
This commit is contained in:
Wladimir J. van der Laan 2018-05-23 19:04:25 +02:00 committed by UdjinM6
parent 21bb444ad3
commit c8ee5e1665
No known key found for this signature in database
GPG Key ID: 83592BD1400D58D9
10 changed files with 23 additions and 25 deletions

View File

@ -557,7 +557,7 @@ bool CCoinJoinClientSession::SignFinalTransaction(const CTransaction& finalTrans
if (fMasternodeMode || pnode == nullptr) return false; if (fMasternodeMode || pnode == nullptr) return false;
if (!mixingMasternode) return false; if (!mixingMasternode) return false;
finalMutableTransaction = finalTransactionNew; finalMutableTransaction = CMutableTransaction{finalTransactionNew};
LogPrint(BCLog::COINJOIN, "CCoinJoinClientSession::%s -- finalMutableTransaction=%s", __func__, finalMutableTransaction.ToString()); /* Continued */ LogPrint(BCLog::COINJOIN, "CCoinJoinClientSession::%s -- finalMutableTransaction=%s", __func__, finalMutableTransaction.ToString()); /* Continued */
// STEP 1: check final transaction general rules // STEP 1: check final transaction general rules

View File

@ -501,7 +501,7 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr)
// mergedTx will end up with all the signatures; it // mergedTx will end up with all the signatures; it
// starts as a clone of the raw tx: // starts as a clone of the raw tx:
CMutableTransaction mergedTx{tx}; CMutableTransaction mergedTx{tx};
const CTransaction txv{tx}; const CMutableTransaction txv{tx};
CCoinsView viewDummy; CCoinsView viewDummy;
CCoinsViewCache view(&viewDummy); CCoinsViewCache view(&viewDummy);

View File

@ -91,9 +91,9 @@ uint256 CTransaction::ComputeHash() const
} }
/* For backward compatibility, the hash is initialized to 0. TODO: remove the need for this default constructor entirely. */ /* 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), nType(TRANSACTION_NORMAL), nLockTime(0), hash() {} CTransaction::CTransaction() : vin(), vout(), nVersion(CTransaction::CURRENT_VERSION), nType(TRANSACTION_NORMAL), nLockTime(0), hash{} {}
CTransaction::CTransaction(const CMutableTransaction &tx) : vin(tx.vin), vout(tx.vout), nVersion(tx.nVersion), nType(tx.nType), nLockTime(tx.nLockTime), vExtraPayload(tx.vExtraPayload), hash(ComputeHash()) {} CTransaction::CTransaction(const CMutableTransaction& tx) : vin(tx.vin), vout(tx.vout), nVersion(tx.nVersion), nType(tx.nType), nLockTime(tx.nLockTime), vExtraPayload(tx.vExtraPayload), hash{ComputeHash()} {}
CTransaction::CTransaction(CMutableTransaction &&tx) : vin(std::move(tx.vin)), vout(std::move(tx.vout)), nVersion(tx.nVersion), nType(tx.nType), nLockTime(tx.nLockTime), vExtraPayload(tx.vExtraPayload), hash(ComputeHash()) {} CTransaction::CTransaction(CMutableTransaction&& tx) : vin(std::move(tx.vin)), vout(std::move(tx.vout)), nVersion(tx.nVersion), nType(tx.nType), nLockTime(tx.nLockTime), vExtraPayload(tx.vExtraPayload), hash{ComputeHash()} {}
CAmount CTransaction::GetValueOut() const CAmount CTransaction::GetValueOut() const
{ {

View File

@ -232,9 +232,7 @@ public:
return vin.empty() && vout.empty(); return vin.empty() && vout.empty();
} }
const uint256& GetHash() const { const uint256& GetHash() const { return hash; }
return hash;
}
// Return sum of txouts. // Return sum of txouts.
CAmount GetValueOut() const; CAmount GetValueOut() const;
@ -277,7 +275,7 @@ struct CMutableTransaction
std::vector<uint8_t> vExtraPayload; // only available for special transaction types std::vector<uint8_t> vExtraPayload; // only available for special transaction types
CMutableTransaction(); CMutableTransaction();
CMutableTransaction(const CTransaction& tx); explicit CMutableTransaction(const CTransaction& tx);
SERIALIZE_METHODS(CMutableTransaction, obj) SERIALIZE_METHODS(CMutableTransaction, obj)
{ {

View File

@ -312,7 +312,7 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test)
if (InsecureRandRange(10) == 0 && coinbase_coins.size()) { if (InsecureRandRange(10) == 0 && coinbase_coins.size()) {
auto utxod = FindRandomFrom(coinbase_coins); auto utxod = FindRandomFrom(coinbase_coins);
// Reuse the exact same coinbase // Reuse the exact same coinbase
tx = std::get<0>(utxod->second); tx = CMutableTransaction{std::get<0>(utxod->second)};
// shouldn't be available for reconnection if it's been duplicated // shouldn't be available for reconnection if it's been duplicated
disconnected_coins.erase(utxod->first); disconnected_coins.erase(utxod->first);
@ -331,7 +331,7 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test)
// 1/20 times reconnect a previously disconnected tx // 1/20 times reconnect a previously disconnected tx
if (randiter % 20 == 2 && disconnected_coins.size()) { if (randiter % 20 == 2 && disconnected_coins.size()) {
auto utxod = FindRandomFrom(disconnected_coins); auto utxod = FindRandomFrom(disconnected_coins);
tx = std::get<0>(utxod->second); tx = CMutableTransaction{std::get<0>(utxod->second)};
prevout = tx.vin[0].prevout; prevout = tx.vin[0].prevout;
if (!CTransaction(tx).IsCoinBase() && !utxoset.count(prevout)) { if (!CTransaction(tx).IsCoinBase() && !utxoset.count(prevout)) {
disconnected_coins.erase(utxod->first); disconnected_coins.erase(utxod->first);

View File

@ -605,7 +605,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTests)
// [tx1] // [tx1]
// //
CTransactionRef tx1 = make_tx(MK_OUTPUTS(10 * COIN)); CTransactionRef tx1 = make_tx(MK_OUTPUTS(10 * COIN));
pool.addUnchecked(tx1->GetHash(), entry.Fee(10000LL).FromTx(*tx1)); pool.addUnchecked(tx1->GetHash(), entry.Fee(10000LL).FromTx(tx1));
// Ancestors / descendants should be 1 / 1 (itself / itself) // Ancestors / descendants should be 1 / 1 (itself / itself)
pool.GetTransactionAncestry(tx1->GetHash(), ancestors, descendants); pool.GetTransactionAncestry(tx1->GetHash(), ancestors, descendants);
@ -617,7 +617,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTests)
// [tx1].0 <- [tx2] // [tx1].0 <- [tx2]
// //
CTransactionRef tx2 = make_tx(MK_OUTPUTS(495 * CENT, 5 * COIN), MK_INPUTS(tx1)); CTransactionRef tx2 = make_tx(MK_OUTPUTS(495 * CENT, 5 * COIN), MK_INPUTS(tx1));
pool.addUnchecked(tx2->GetHash(), entry.Fee(10000LL).FromTx(*tx2)); pool.addUnchecked(tx2->GetHash(), entry.Fee(10000LL).FromTx(tx2));
// Ancestors / descendants should be: // Ancestors / descendants should be:
// transaction ancestors descendants // transaction ancestors descendants
@ -636,7 +636,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTests)
// [tx1].0 <- [tx2].0 <- [tx3] // [tx1].0 <- [tx2].0 <- [tx3]
// //
CTransactionRef tx3 = make_tx(MK_OUTPUTS(290 * CENT, 200 * CENT), MK_INPUTS(tx2)); CTransactionRef tx3 = make_tx(MK_OUTPUTS(290 * CENT, 200 * CENT), MK_INPUTS(tx2));
pool.addUnchecked(tx3->GetHash(), entry.Fee(10000LL).FromTx(*tx3)); pool.addUnchecked(tx3->GetHash(), entry.Fee(10000LL).FromTx(tx3));
// Ancestors / descendants should be: // Ancestors / descendants should be:
// transaction ancestors descendants // transaction ancestors descendants
@ -661,7 +661,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTests)
// \---1 <- [tx4] // \---1 <- [tx4]
// //
CTransactionRef tx4 = make_tx(MK_OUTPUTS(290 * CENT, 250 * CENT), MK_INPUTS(tx2), MK_INPUT_IDX(1)); CTransactionRef tx4 = make_tx(MK_OUTPUTS(290 * CENT, 250 * CENT), MK_INPUTS(tx2), MK_INPUT_IDX(1));
pool.addUnchecked(tx4->GetHash(), entry.Fee(10000LL).FromTx(*tx4)); pool.addUnchecked(tx4->GetHash(), entry.Fee(10000LL).FromTx(tx4));
// Ancestors / descendants should be: // Ancestors / descendants should be:
// transaction ancestors descendants // transaction ancestors descendants
@ -698,13 +698,13 @@ BOOST_AUTO_TEST_CASE(MempoolAncestryTests)
CTransactionRef& tyi = *ty[i]; CTransactionRef& tyi = *ty[i];
tyi = make_tx(MK_OUTPUTS(v), i > 0 ? MK_INPUTS(*ty[i-1]) : std::vector<CTransactionRef>()); tyi = make_tx(MK_OUTPUTS(v), i > 0 ? MK_INPUTS(*ty[i-1]) : std::vector<CTransactionRef>());
v -= 50 * CENT; v -= 50 * CENT;
pool.addUnchecked(tyi->GetHash(), entry.Fee(10000LL).FromTx(*tyi)); pool.addUnchecked(tyi->GetHash(), entry.Fee(10000LL).FromTx(tyi));
pool.GetTransactionAncestry(tyi->GetHash(), ancestors, descendants); pool.GetTransactionAncestry(tyi->GetHash(), ancestors, descendants);
BOOST_CHECK_EQUAL(ancestors, i+1); BOOST_CHECK_EQUAL(ancestors, i+1);
BOOST_CHECK_EQUAL(descendants, i+1); BOOST_CHECK_EQUAL(descendants, i+1);
} }
CTransactionRef ty6 = make_tx(MK_OUTPUTS(5 * COIN), MK_INPUTS(tx3, ty5)); CTransactionRef ty6 = make_tx(MK_OUTPUTS(5 * COIN), MK_INPUTS(tx3, ty5));
pool.addUnchecked(ty6->GetHash(), entry.Fee(10000LL).FromTx(*ty6)); pool.addUnchecked(ty6->GetHash(), entry.Fee(10000LL).FromTx(ty6));
// Ancestors / descendants should be: // Ancestors / descendants should be:
// transaction ancestors descendants // transaction ancestors descendants

View File

@ -133,7 +133,7 @@ CMutableTransaction BuildCreditingTransaction(const CScript& scriptPubKey)
return txCredit; return txCredit;
} }
CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CMutableTransaction& txCredit) CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CTransaction& txCredit)
{ {
CMutableTransaction txSpend; CMutableTransaction txSpend;
txSpend.nVersion = 1; txSpend.nVersion = 1;
@ -155,7 +155,7 @@ void DoTest(const CScript& scriptPubKey, const CScript& scriptSig, int flags, co
bool expect = (scriptError == SCRIPT_ERR_OK); bool expect = (scriptError == SCRIPT_ERR_OK);
bool fEnableDIP0020Opcodes = (SCRIPT_ENABLE_DIP0020_OPCODES & flags) != 0; bool fEnableDIP0020Opcodes = (SCRIPT_ENABLE_DIP0020_OPCODES & flags) != 0;
ScriptError err; ScriptError err;
CMutableTransaction txCredit = BuildCreditingTransaction(scriptPubKey); const CTransaction txCredit{BuildCreditingTransaction(scriptPubKey)};
CMutableTransaction tx = BuildSpendingTransaction(scriptSig, txCredit); CMutableTransaction tx = BuildSpendingTransaction(scriptSig, txCredit);
CMutableTransaction tx2 = tx; CMutableTransaction tx2 = tx;
BOOST_CHECK_MESSAGE(VerifyScript(scriptSig, scriptPubKey, flags, MutableTransactionSignatureChecker(&tx, 0, txCredit.vout[0].nValue), &err) == expect, message); BOOST_CHECK_MESSAGE(VerifyScript(scriptSig, scriptPubKey, flags, MutableTransactionSignatureChecker(&tx, 0, txCredit.vout[0].nValue), &err) == expect, message);
@ -1018,7 +1018,7 @@ BOOST_AUTO_TEST_CASE(script_CHECKMULTISIG12)
CScript scriptPubKey12; CScript scriptPubKey12;
scriptPubKey12 << OP_1 << ToByteVector(key1.GetPubKey()) << ToByteVector(key2.GetPubKey()) << OP_2 << OP_CHECKMULTISIG; scriptPubKey12 << OP_1 << ToByteVector(key1.GetPubKey()) << ToByteVector(key2.GetPubKey()) << OP_2 << OP_CHECKMULTISIG;
CMutableTransaction txFrom12 = BuildCreditingTransaction(scriptPubKey12); const CTransaction txFrom12{BuildCreditingTransaction(scriptPubKey12)};
CMutableTransaction txTo12 = BuildSpendingTransaction(CScript(), txFrom12); CMutableTransaction txTo12 = BuildSpendingTransaction(CScript(), txFrom12);
CScript goodsig1 = sign_multisig(scriptPubKey12, key1, txTo12); CScript goodsig1 = sign_multisig(scriptPubKey12, key1, txTo12);
@ -1049,7 +1049,7 @@ BOOST_AUTO_TEST_CASE(script_CHECKMULTISIG23)
CScript scriptPubKey23; CScript scriptPubKey23;
scriptPubKey23 << OP_2 << ToByteVector(key1.GetPubKey()) << ToByteVector(key2.GetPubKey()) << ToByteVector(key3.GetPubKey()) << OP_3 << OP_CHECKMULTISIG; scriptPubKey23 << OP_2 << ToByteVector(key1.GetPubKey()) << ToByteVector(key2.GetPubKey()) << ToByteVector(key3.GetPubKey()) << OP_3 << OP_CHECKMULTISIG;
CMutableTransaction txFrom23 = BuildCreditingTransaction(scriptPubKey23); const CTransaction txFrom23{BuildCreditingTransaction(scriptPubKey23)};
CMutableTransaction txTo23 = BuildSpendingTransaction(CScript(), txFrom23); CMutableTransaction txTo23 = BuildSpendingTransaction(CScript(), txFrom23);
std::vector<CKey> keys; std::vector<CKey> keys;

View File

@ -232,7 +232,7 @@ CBlock TestChainSetup::CreateBlock(const std::vector<CMutableTransaction>& txns,
if (!CalcCbTxMerkleRootQuorums(block, chainActive.Tip(), cbTx.merkleRootQuorums, state)) { if (!CalcCbTxMerkleRootQuorums(block, chainActive.Tip(), cbTx.merkleRootQuorums, state)) {
BOOST_ASSERT(false); BOOST_ASSERT(false);
} }
CMutableTransaction tmpTx = *block.vtx[0]; CMutableTransaction tmpTx{*block.vtx[0]};
SetTxPayload(tmpTx, cbTx); SetTxPayload(tmpTx, cbTx);
block.vtx[0] = MakeTransactionRef(tmpTx); block.vtx[0] = MakeTransactionRef(tmpTx);
} }

View File

@ -24,7 +24,7 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi
BOOST_AUTO_TEST_SUITE(tx_validationcache_tests) BOOST_AUTO_TEST_SUITE(tx_validationcache_tests)
static bool static bool
ToMemPool(CMutableTransaction& tx) ToMemPool(const CMutableTransaction& tx)
{ {
LOCK(cs_main); LOCK(cs_main);

View File

@ -2584,8 +2584,8 @@ bool CWalletTx::IsTrusted() const
bool CWalletTx::IsEquivalentTo(const CWalletTx& _tx) const bool CWalletTx::IsEquivalentTo(const CWalletTx& _tx) const
{ {
CMutableTransaction tx1 = *this->tx; CMutableTransaction tx1 {*this->tx};
CMutableTransaction tx2 = *_tx.tx; CMutableTransaction tx2 {*_tx.tx};
for (auto& txin : tx1.vin) txin.scriptSig = CScript(); for (auto& txin : tx1.vin) txin.scriptSig = CScript();
for (auto& txin : tx2.vin) txin.scriptSig = CScript(); for (auto& txin : tx2.vin) txin.scriptSig = CScript();
return CTransaction(tx1) == CTransaction(tx2); return CTransaction(tx1) == CTransaction(tx2);