[wallet] Get rid of CWalletTx default constructor

No change in behavior in the normal case. But buggy mapWallet lookups with
invalid txids will now throw exceptions instead of inserting dummy entries into
the map, and potentially causing segfaults and other failures.

This also makes it a compiler error to use the mapWallet[hash] syntax which
could create dummy entries.
This commit is contained in:
Russell Yanofsky 2017-01-19 16:08:03 -05:00
parent a128bdc9e1
commit b4bc32a451
4 changed files with 12 additions and 17 deletions

View File

@ -29,7 +29,7 @@ GetResults(CWallet& wallet, std::map<CAmount, CAccountingEntry>& results)
BOOST_AUTO_TEST_CASE(acc_orderupgrade)
{
std::vector<CWalletTx*> vpwtx;
CWalletTx wtx;
CWalletTx wtx(nullptr /* pwallet */, MakeTransactionRef());
CAccountingEntry ae;
std::map<CAmount, CAccountingEntry> results;
@ -44,7 +44,7 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade)
wtx.mapValue["comment"] = "z";
m_wallet.AddToWallet(wtx);
vpwtx.push_back(&m_wallet.mapWallet[wtx.GetHash()]);
vpwtx.push_back(&m_wallet.mapWallet.at(wtx.GetHash()));
vpwtx[0]->nTimeReceived = (unsigned int)1333333335;
vpwtx[0]->nOrderPos = -1;
@ -86,7 +86,7 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade)
wtx.SetTx(MakeTransactionRef(std::move(tx)));
}
m_wallet.AddToWallet(wtx);
vpwtx.push_back(&m_wallet.mapWallet[wtx.GetHash()]);
vpwtx.push_back(&m_wallet.mapWallet.at(wtx.GetHash()));
vpwtx[1]->nTimeReceived = (unsigned int)1333333336;
wtx.mapValue["comment"] = "x";
@ -96,7 +96,7 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade)
wtx.SetTx(MakeTransactionRef(std::move(tx)));
}
m_wallet.AddToWallet(wtx);
vpwtx.push_back(&m_wallet.mapWallet[wtx.GetHash()]);
vpwtx.push_back(&m_wallet.mapWallet.at(wtx.GetHash()));
vpwtx[2]->nTimeReceived = (unsigned int)1333333329;
vpwtx[2]->nOrderPos = -1;

View File

@ -531,7 +531,7 @@ void CWallet::SyncMetaData(std::pair<TxSpends::iterator, TxSpends::iterator> ran
int nMinOrderPos = std::numeric_limits<int>::max();
const CWalletTx* copyFrom = nullptr;
for (TxSpends::iterator it = range.first; it != range.second; ++it) {
const CWalletTx* wtx = &mapWallet[it->second];
const CWalletTx* wtx = &mapWallet.at(it->second);
if (wtx->nOrderPos < nMinOrderPos) {
nMinOrderPos = wtx->nOrderPos;;
copyFrom = wtx;
@ -544,7 +544,7 @@ void CWallet::SyncMetaData(std::pair<TxSpends::iterator, TxSpends::iterator> ran
for (TxSpends::iterator it = range.first; it != range.second; ++it)
{
const uint256& hash = it->second;
CWalletTx* copyTo = &mapWallet[hash];
CWalletTx* copyTo = &mapWallet.at(hash);
if (copyFrom == copyTo) continue;
assert(copyFrom && "Oldest wallet transaction in range assumed to have been found.");
if (!copyFrom->IsEquivalentTo(*copyTo)) continue;
@ -3081,7 +3081,7 @@ bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve
// Notify that old coins are spent
for (const CTxIn& txin : wtxNew.tx->vin)
{
CWalletTx &coin = mapWallet[txin.prevout.hash];
CWalletTx &coin = mapWallet.at(txin.prevout.hash);
coin.BindWallet(this);
NotifyTransactionChanged(this, coin.GetHash(), CT_UPDATED);
}
@ -3092,7 +3092,7 @@ bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve
// Get the inserted-CWalletTx from mapWallet so that the
// fInMempool flag is cached properly
CWalletTx& wtx = mapWallet[wtxNew.GetHash()];
CWalletTx& wtx = mapWallet.at(wtxNew.GetHash());
if (fBroadcastTransactions)
{
@ -3548,7 +3548,7 @@ std::set< std::set<CTxDestination> > CWallet::GetAddressGroupings()
CTxDestination address;
if(!IsMine(txin)) /* If this input isn't mine, ignore it */
continue;
if(!ExtractDestination(mapWallet[txin.prevout.hash].tx->vout[txin.prevout.n].scriptPubKey, address))
if(!ExtractDestination(mapWallet.at(txin.prevout.hash).tx->vout[txin.prevout.n].scriptPubKey, address))
continue;
grouping.insert(address);
any_mine = true;

View File

@ -348,11 +348,6 @@ public:
mutable CAmount nAvailableWatchCreditCached;
mutable CAmount nChangeCached;
CWalletTx()
{
Init(nullptr);
}
CWalletTx(const CWallet* pwalletIn, CTransactionRef arg) : CMerkleTx(std::move(arg))
{
Init(pwalletIn);

View File

@ -265,7 +265,7 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
{
uint256 hash;
ssKey >> hash;
CWalletTx wtx;
CWalletTx wtx(nullptr /* pwallet */, MakeTransactionRef());
ssValue >> wtx;
CValidationState state;
if (!(CheckTransaction(*wtx.tx, state) && (wtx.GetHash() == hash) && state.IsValid()))
@ -603,7 +603,7 @@ DBErrors CWalletDB::LoadWallet(CWallet* pwallet)
pwallet->UpdateTimeFirstKey(1);
for (uint256 hash : wss.vWalletUpgrade)
WriteTx(pwallet->mapWallet[hash]);
WriteTx(pwallet->mapWallet.at(hash));
// Rewrite encrypted wallets of versions 0.4.0 and 0.5.0rc:
if (wss.fIsEncrypted && (wss.nFileVersion == 40000 || wss.nFileVersion == 50000))
@ -664,7 +664,7 @@ DBErrors CWalletDB::FindWalletTx(std::vector<uint256>& vTxHash, std::vector<CWal
uint256 hash;
ssKey >> hash;
CWalletTx wtx;
CWalletTx wtx(nullptr /* pwallet */, MakeTransactionRef());
ssValue >> wtx;
vTxHash.push_back(hash);