highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-11-11 14:38:29 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge #9680: Unify CWalletTx construction

Browse Source 
b4bc32a451 [wallet] Get rid of CWalletTx default constructor (Russell Yanofsky)
a128bdc9e1 [wallet] Construct CWalletTx objects in CommitTransaction (Russell Yanofsky)

Pull request description:

  Two commits:

  - `Construct CWalletTx objects in CommitTransaction` moves a bunch of CWalletTx initialization into CWallet::CommitTransaction to dedup some code and avoid future inconsistencies in how wallet transactions are created.
  - `Get rid of CWalletTx default constructor` does what is described and eliminates the possibility of empty transaction entries being inadvertently created by mapWallet[hash] accesses.

  Both of these changes were originally part of #9381

Tree-SHA512: af3841c4f0539e0662d81b33c5369fc70aa06ddde1c59cb00fb21c9e4c7d9ff47f1edc5040cb463af1333838802c56b3ef875b939e2b804ee45b8e0294a4371c
This commit is contained in:
Pieter Wuille
2018-03-13 19:07:49 -07:00
parent 18462960c0 b4bc32a451
commit 6acd8700bc
10 changed files with 80 additions and 91 deletions
Download Patch File Download Diff File Expand all files Collapse all files

43
src/wallet/wallet.cpp
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;
@@ -2629,13 +2629,13 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC
LOCK2(cs_main, cs_wallet);
CReserveKey reservekey(this);
CWalletTx wtx;
if (!CreateTransaction(vecSend, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, coinControl, false)) {
CTransactionRef tx_new;
if (!CreateTransaction(vecSend, tx_new, reservekey, nFeeRet, nChangePosInOut, strFailReason, coinControl, false)) {
return false;
}
if (nChangePosInOut != -1) {
tx.vout.insert(tx.vout.begin() + nChangePosInOut, wtx.tx->vout[nChangePosInOut]);
tx.vout.insert(tx.vout.begin() + nChangePosInOut, tx_new->vout[nChangePosInOut]);
// We don't have the normal Create/Commit cycle, and don't want to risk
// reusing change, so just remove the key from the keypool here.
reservekey.KeepKey();
@@ -2644,11 +2644,11 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC
// Copy output sizes from new transaction; they may have had the fee
// subtracted from them.
for (unsigned int idx = 0; idx < tx.vout.size(); idx++) {
tx.vout[idx].nValue = wtx.tx->vout[idx].nValue;
tx.vout[idx].nValue = tx_new->vout[idx].nValue;
}
// Add new txins while keeping original txin scriptSig/order.
for (const CTxIn& txin : wtx.tx->vin) {
for (const CTxIn& txin : tx_new->vin) {
if (!coinControl.IsSelected(txin.prevout)) {
tx.vin.push_back(txin);
@@ -2689,7 +2689,7 @@ OutputType CWallet::TransactionChangeType(OutputType change_type, const std::vec
return g_address_type;
}
bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, CAmount& nFeeRet,
bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CTransactionRef& tx, CReserveKey& reservekey, CAmount& nFeeRet,
int& nChangePosInOut, std::string& strFailReason, const CCoinControl& coin_control, bool sign)
{
CAmount nValue = 0;
@@ -2713,8 +2713,6 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
return false;
}
wtxNew.fTimeReceivedIsTxTime = true;
wtxNew.BindWallet(this);
CMutableTransaction txNew;
// Discourage fee sniping.
@@ -2802,7 +2800,6 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
nChangePosInOut = nChangePosRequest;
txNew.vin.clear();
txNew.vout.clear();
wtxNew.fFromMe = true;
bool fFirst = true;
CAmount nValueToSelect = nValue;
@@ -3017,11 +3014,11 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
}
}
// Embed the constructed transaction data in wtxNew.
wtxNew.SetTx(MakeTransactionRef(std::move(txNew)));
// Return the constructed transaction data.
tx = MakeTransactionRef(std::move(txNew));
// Limit size
if (GetTransactionWeight(*wtxNew.tx) >= MAX_STANDARD_TX_WEIGHT)
if (GetTransactionWeight(*tx) >= MAX_STANDARD_TX_WEIGHT)
{
strFailReason = _("Transaction too large");
return false;
@@ -3031,7 +3028,7 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
if (gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS)) {
// Lastly, ensure this tx will pass the mempool's chain limits
LockPoints lp;
CTxMemPoolEntry entry(wtxNew.tx, 0, 0, 0, false, 0, lp);
CTxMemPoolEntry entry(tx, 0, 0, 0, false, 0, lp);
CTxMemPool::setEntries setAncestors;
size_t nLimitAncestors = gArgs.GetArg("-limitancestorcount", DEFAULT_ANCESTOR_LIMIT);
size_t nLimitAncestorSize = gArgs.GetArg("-limitancestorsize", DEFAULT_ANCESTOR_SIZE_LIMIT)*1000;
@@ -3058,10 +3055,18 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
/**
* Call after CreateTransaction unless you want to abort
*/
bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey, CConnman* connman, CValidationState& state)
bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm, std::string fromAccount, CReserveKey& reservekey, CConnman* connman, CValidationState& state)
{
{
LOCK2(cs_main, cs_wallet);
CWalletTx wtxNew(this, std::move(tx));
wtxNew.mapValue = std::move(mapValue);
wtxNew.vOrderForm = std::move(orderForm);
wtxNew.strFromAccount = std::move(fromAccount);
wtxNew.fTimeReceivedIsTxTime = true;
wtxNew.fFromMe = true;
LogPrintf("CommitTransaction:\n%s", wtxNew.tx->ToString());
{
// Take key pair from key pool so it won't be used again
@@ -3074,7 +3079,7 @@ bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey, CCon
// 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);
}
@@ -3085,7 +3090,7 @@ bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey, CCon
// 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)
{
@@ -3541,7 +3546,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;