Combine CCoinsViewCache's HaveCoins and const GetCoins into AccessCoins.

The efficient version of CCoinsViewCache::GetCoins only works for known-to-exist
cache entries, requiring a separate HaveCoins call beforehand. This is
inefficient as both perform a hashtable lookup.

Replace the non-mutable GetCoins with AccessCoins, which returns a potentially-NULL
pointer. This also decreases the overloading of GetCoins.

Also replace some copying (inefficient) GetCoins calls with equivalent AccessCoins,
decreasing the copying.
This commit is contained in:
Pieter Wuille 2014-09-02 21:21:15 +02:00
parent b8d92236f6
commit 629d75faac
7 changed files with 54 additions and 51 deletions

View File

@ -418,12 +418,12 @@ static void MutateTxSign(CMutableTransaction& tx, const string& flagStr)
// Sign what we can: // Sign what we can:
for (unsigned int i = 0; i < mergedTx.vin.size(); i++) { for (unsigned int i = 0; i < mergedTx.vin.size(); i++) {
CTxIn& txin = mergedTx.vin[i]; CTxIn& txin = mergedTx.vin[i];
CCoins coins; const CCoins* coins = view.AccessCoins(txin.prevout.hash);
if (!view.GetCoins(txin.prevout.hash, coins) || !coins.IsAvailable(txin.prevout.n)) { if (!coins || !coins->IsAvailable(txin.prevout.n)) {
fComplete = false; fComplete = false;
continue; continue;
} }
const CScript& prevPubKey = coins.vout[txin.prevout.n].scriptPubKey; const CScript& prevPubKey = coins->vout[txin.prevout.n].scriptPubKey;
txin.scriptSig.clear(); txin.scriptSig.clear();
// Only sign SIGHASH_SINGLE if there's a corresponding output: // Only sign SIGHASH_SINGLE if there's a corresponding output:

View File

@ -110,9 +110,13 @@ CCoins &CCoinsViewCache::GetCoins(const uint256 &txid) {
return it->second; return it->second;
} }
const CCoins &CCoinsViewCache::GetCoins(const uint256 &txid) const { const CCoins* CCoinsViewCache::AccessCoins(const uint256 &txid) const {
/* Avoid redundant implementation with the const-cast. */ CCoinsMap::const_iterator it = FetchCoins(txid);
return const_cast<CCoinsViewCache*>(this)->GetCoins(txid); if (it == cacheCoins.end()) {
return NULL;
} else {
return &it->second;
}
} }
bool CCoinsViewCache::SetCoins(const uint256 &txid, const CCoins &coins) { bool CCoinsViewCache::SetCoins(const uint256 &txid, const CCoins &coins) {
@ -162,9 +166,9 @@ unsigned int CCoinsViewCache::GetCacheSize() const {
const CTxOut &CCoinsViewCache::GetOutputFor(const CTxIn& input) const const CTxOut &CCoinsViewCache::GetOutputFor(const CTxIn& input) const
{ {
const CCoins &coins = GetCoins(input.prevout.hash); const CCoins* coins = AccessCoins(input.prevout.hash);
assert(coins.IsAvailable(input.prevout.n)); assert(coins && coins->IsAvailable(input.prevout.n));
return coins.vout[input.prevout.n]; return coins->vout[input.prevout.n];
} }
int64_t CCoinsViewCache::GetValueIn(const CTransaction& tx) const int64_t CCoinsViewCache::GetValueIn(const CTransaction& tx) const
@ -182,19 +186,12 @@ int64_t CCoinsViewCache::GetValueIn(const CTransaction& tx) const
bool CCoinsViewCache::HaveInputs(const CTransaction& tx) const bool CCoinsViewCache::HaveInputs(const CTransaction& tx) const
{ {
if (!tx.IsCoinBase()) { if (!tx.IsCoinBase()) {
// first check whether information about the prevout hash is available
for (unsigned int i = 0; i < tx.vin.size(); i++) { for (unsigned int i = 0; i < tx.vin.size(); i++) {
const COutPoint &prevout = tx.vin[i].prevout; const COutPoint &prevout = tx.vin[i].prevout;
if (!HaveCoins(prevout.hash)) const CCoins* coins = AccessCoins(prevout.hash);
return false; if (!coins || !coins->IsAvailable(prevout.n)) {
}
// then check whether the actual outputs are available
for (unsigned int i = 0; i < tx.vin.size(); i++) {
const COutPoint &prevout = tx.vin[i].prevout;
const CCoins &coins = GetCoins(prevout.hash);
if (!coins.IsAvailable(prevout.n))
return false; return false;
}
} }
} }
return true; return true;
@ -207,10 +204,11 @@ double CCoinsViewCache::GetPriority(const CTransaction &tx, int nHeight) const
double dResult = 0.0; double dResult = 0.0;
BOOST_FOREACH(const CTxIn& txin, tx.vin) BOOST_FOREACH(const CTxIn& txin, tx.vin)
{ {
const CCoins &coins = GetCoins(txin.prevout.hash); const CCoins* coins = AccessCoins(txin.prevout.hash);
if (!coins.IsAvailable(txin.prevout.n)) continue; assert(coins);
if (coins.nHeight < nHeight) { if (!coins->IsAvailable(txin.prevout.n)) continue;
dResult += coins.vout[txin.prevout.n].nValue * (nHeight-coins.nHeight); if (coins->nHeight < nHeight) {
dResult += coins->vout[txin.prevout.n].nValue * (nHeight-coins->nHeight);
} }
} }
return tx.ComputePriority(dResult); return tx.ComputePriority(dResult);

View File

@ -344,11 +344,13 @@ public:
bool SetBestBlock(const uint256 &hashBlock); bool SetBestBlock(const uint256 &hashBlock);
bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock); bool BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock);
// Return a pointer to CCoins in the cache, or NULL if not found. This is
// more efficient than GetCoins. Modifications to other cache entries are
// allowed while accessing the returned pointer.
const CCoins* AccessCoins(const uint256 &txid) const;
// Return a modifiable reference to a CCoins. Check HaveCoins first. // Return a modifiable reference to a CCoins. Check HaveCoins first.
// Many methods explicitly require a CCoinsViewCache because of this method, to reduce
// copying.
CCoins &GetCoins(const uint256 &txid); CCoins &GetCoins(const uint256 &txid);
const CCoins &GetCoins(const uint256 &txid) const;
// Push the modifications applied to this cache to its base. // Push the modifications applied to this cache to its base.
// Failure to call this method before destruction will cause the changes to be forgotten. // Failure to call this method before destruction will cause the changes to be forgotten.

View File

@ -1017,9 +1017,9 @@ bool GetTransaction(const uint256 &hash, CTransaction &txOut, uint256 &hashBlock
int nHeight = -1; int nHeight = -1;
{ {
CCoinsViewCache &view = *pcoinsTip; CCoinsViewCache &view = *pcoinsTip;
CCoins coins; const CCoins* coins = view.AccessCoins(hash);
if (view.GetCoins(hash, coins)) if (coins)
nHeight = coins.nHeight; nHeight = coins->nHeight;
} }
if (nHeight > 0) if (nHeight > 0)
pindexSlow = chainActive[nHeight]; pindexSlow = chainActive[nHeight];
@ -1371,19 +1371,20 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi
for (unsigned int i = 0; i < tx.vin.size(); i++) for (unsigned int i = 0; i < tx.vin.size(); i++)
{ {
const COutPoint &prevout = tx.vin[i].prevout; const COutPoint &prevout = tx.vin[i].prevout;
const CCoins &coins = inputs.GetCoins(prevout.hash); const CCoins *coins = inputs.AccessCoins(prevout.hash);
assert(coins);
// If prev is coinbase, check that it's matured // If prev is coinbase, check that it's matured
if (coins.IsCoinBase()) { if (coins->IsCoinBase()) {
if (nSpendHeight - coins.nHeight < COINBASE_MATURITY) if (nSpendHeight - coins->nHeight < COINBASE_MATURITY)
return state.Invalid( return state.Invalid(
error("CheckInputs() : tried to spend coinbase at depth %d", nSpendHeight - coins.nHeight), error("CheckInputs() : tried to spend coinbase at depth %d", nSpendHeight - coins->nHeight),
REJECT_INVALID, "bad-txns-premature-spend-of-coinbase"); REJECT_INVALID, "bad-txns-premature-spend-of-coinbase");
} }
// Check for negative or overflow input values // Check for negative or overflow input values
nValueIn += coins.vout[prevout.n].nValue; nValueIn += coins->vout[prevout.n].nValue;
if (!MoneyRange(coins.vout[prevout.n].nValue) || !MoneyRange(nValueIn)) if (!MoneyRange(coins->vout[prevout.n].nValue) || !MoneyRange(nValueIn))
return state.DoS(100, error("CheckInputs() : txin values out of range"), return state.DoS(100, error("CheckInputs() : txin values out of range"),
REJECT_INVALID, "bad-txns-inputvalues-outofrange"); REJECT_INVALID, "bad-txns-inputvalues-outofrange");
@ -1413,10 +1414,11 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi
if (fScriptChecks) { if (fScriptChecks) {
for (unsigned int i = 0; i < tx.vin.size(); i++) { for (unsigned int i = 0; i < tx.vin.size(); i++) {
const COutPoint &prevout = tx.vin[i].prevout; const COutPoint &prevout = tx.vin[i].prevout;
const CCoins &coins = inputs.GetCoins(prevout.hash); const CCoins* coins = inputs.AccessCoins(prevout.hash);
assert(coins);
// Verify signature // Verify signature
CScriptCheck check(coins, tx, i, flags, 0); CScriptCheck check(*coins, tx, i, flags, 0);
if (pvChecks) { if (pvChecks) {
pvChecks->push_back(CScriptCheck()); pvChecks->push_back(CScriptCheck());
check.swap(pvChecks->back()); check.swap(pvChecks->back());
@ -1428,7 +1430,7 @@ bool CheckInputs(const CTransaction& tx, CValidationState &state, const CCoinsVi
// arguments; if so, don't trigger DoS protection to // arguments; if so, don't trigger DoS protection to
// avoid splitting the network between upgraded and // avoid splitting the network between upgraded and
// non-upgraded nodes. // non-upgraded nodes.
CScriptCheck check(coins, tx, i, CScriptCheck check(*coins, tx, i,
flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS, 0); flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS, 0);
if (check()) if (check())
return state.Invalid(false, REJECT_NONSTANDARD, "non-mandatory-script-verify-flag"); return state.Invalid(false, REJECT_NONSTANDARD, "non-mandatory-script-verify-flag");
@ -1614,8 +1616,8 @@ bool ConnectBlock(CBlock& block, CValidationState& state, CBlockIndex* pindex, C
(pindex->nHeight==91880 && pindex->GetBlockHash() == uint256("0x00000000000743f190a18c5577a3c2d2a1f610ae9601ac046a38084ccb7cd721"))); (pindex->nHeight==91880 && pindex->GetBlockHash() == uint256("0x00000000000743f190a18c5577a3c2d2a1f610ae9601ac046a38084ccb7cd721")));
if (fEnforceBIP30) { if (fEnforceBIP30) {
BOOST_FOREACH(const CTransaction& tx, block.vtx) { BOOST_FOREACH(const CTransaction& tx, block.vtx) {
const uint256& hash = tx.GetHash(); const CCoins* coins = view.AccessCoins(tx.GetHash());
if (view.HaveCoins(hash) && !view.GetCoins(hash).IsPruned()) if (coins && !coins->IsPruned())
return state.DoS(100, error("ConnectBlock() : tried to overwrite transaction"), return state.DoS(100, error("ConnectBlock() : tried to overwrite transaction"),
REJECT_INVALID, "bad-txns-BIP30"); REJECT_INVALID, "bad-txns-BIP30");
} }

View File

@ -167,12 +167,13 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn)
nTotalIn += mempool.mapTx[txin.prevout.hash].GetTx().vout[txin.prevout.n].nValue; nTotalIn += mempool.mapTx[txin.prevout.hash].GetTx().vout[txin.prevout.n].nValue;
continue; continue;
} }
const CCoins &coins = view.GetCoins(txin.prevout.hash); const CCoins* coins = view.AccessCoins(txin.prevout.hash);
assert(coins);
int64_t nValueIn = coins.vout[txin.prevout.n].nValue; int64_t nValueIn = coins->vout[txin.prevout.n].nValue;
nTotalIn += nValueIn; nTotalIn += nValueIn;
int nConf = pindexPrev->nHeight - coins.nHeight + 1; int nConf = pindexPrev->nHeight - coins->nHeight + 1;
dPriority += (double)nValueIn * nConf; dPriority += (double)nValueIn * nConf;
} }

View File

@ -565,7 +565,7 @@ Value signrawtransaction(const Array& params, bool fHelp)
BOOST_FOREACH(const CTxIn& txin, mergedTx.vin) { BOOST_FOREACH(const CTxIn& txin, mergedTx.vin) {
const uint256& prevHash = txin.prevout.hash; const uint256& prevHash = txin.prevout.hash;
CCoins coins; CCoins coins;
view.GetCoins(prevHash, coins); // this is certainly allowed to fail view.AccessCoins(prevHash); // this is certainly allowed to fail
} }
view.SetBackend(viewDummy); // switch back to avoid locking mempool for too long view.SetBackend(viewDummy); // switch back to avoid locking mempool for too long
@ -669,12 +669,12 @@ Value signrawtransaction(const Array& params, bool fHelp)
// Sign what we can: // Sign what we can:
for (unsigned int i = 0; i < mergedTx.vin.size(); i++) { for (unsigned int i = 0; i < mergedTx.vin.size(); i++) {
CTxIn& txin = mergedTx.vin[i]; CTxIn& txin = mergedTx.vin[i];
CCoins coins; const CCoins* coins = view.AccessCoins(txin.prevout.hash);
if (!view.GetCoins(txin.prevout.hash, coins) || !coins.IsAvailable(txin.prevout.n)) { if (coins == NULL || !coins->IsAvailable(txin.prevout.n)) {
fComplete = false; fComplete = false;
continue; continue;
} }
const CScript& prevPubKey = coins.vout[txin.prevout.n].scriptPubKey; const CScript& prevPubKey = coins->vout[txin.prevout.n].scriptPubKey;
txin.scriptSig.clear(); txin.scriptSig.clear();
// Only sign SIGHASH_SINGLE if there's a corresponding output: // Only sign SIGHASH_SINGLE if there's a corresponding output:
@ -732,9 +732,9 @@ Value sendrawtransaction(const Array& params, bool fHelp)
fOverrideFees = params[1].get_bool(); fOverrideFees = params[1].get_bool();
CCoinsViewCache &view = *pcoinsTip; CCoinsViewCache &view = *pcoinsTip;
CCoins existingCoins; const CCoins* existingCoins = view.AccessCoins(hashTx);
bool fHaveMempool = mempool.exists(hashTx); bool fHaveMempool = mempool.exists(hashTx);
bool fHaveChain = view.GetCoins(hashTx, existingCoins) && existingCoins.nHeight < 1000000000; bool fHaveChain = existingCoins && existingCoins->nHeight < 1000000000;
if (!fHaveMempool && !fHaveChain) { if (!fHaveMempool && !fHaveChain) {
// push to local node and sync with wallets // push to local node and sync with wallets
CValidationState state; CValidationState state;

View File

@ -509,8 +509,8 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const
const CTransaction& tx2 = it2->second.GetTx(); const CTransaction& tx2 = it2->second.GetTx();
assert(tx2.vout.size() > txin.prevout.n && !tx2.vout[txin.prevout.n].IsNull()); assert(tx2.vout.size() > txin.prevout.n && !tx2.vout[txin.prevout.n].IsNull());
} else { } else {
const CCoins &coins = pcoins->GetCoins(txin.prevout.hash); const CCoins* coins = pcoins->AccessCoins(txin.prevout.hash);
assert(coins.IsAvailable(txin.prevout.n)); assert(coins && coins->IsAvailable(txin.prevout.n));
} }
// Check whether its inputs are marked in mapNextTx. // Check whether its inputs are marked in mapNextTx.
std::map<COutPoint, CInPoint>::const_iterator it3 = mapNextTx.find(txin.prevout); std::map<COutPoint, CInPoint>::const_iterator it3 = mapNextTx.find(txin.prevout);