From 95d4450a41e9b3f7a739eeefec322bf9366ce824 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Thu, 14 Dec 2017 03:17:58 +0000 Subject: [PATCH 1/2] [wallet] Tidy up CWallet::FundTransaction --- src/wallet/wallet.cpp | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index cb81ec37f5350..4777641e729c0 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2592,9 +2592,8 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC { std::vector vecSend; - // Turn the txout set into a CRecipient vector - for (size_t idx = 0; idx < tx.vout.size(); idx++) - { + // Turn the txout set into a CRecipient vector. + for (size_t idx = 0; idx < tx.vout.size(); idx++) { const CTxOut& txOut = tx.vout[idx]; CRecipient recipient = {txOut.scriptPubKey, txOut.nValue, setSubtractFeeFromOutputs.count(idx) == 1}; vecSend.push_back(recipient); @@ -2602,8 +2601,9 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC coinControl.fAllowOtherInputs = true; - for (const CTxIn& txin : tx.vin) + for (const CTxIn& txin : tx.vin) { coinControl.Select(txin.prevout); + } CReserveKey reservekey(this); CWalletTx wtx; @@ -2613,31 +2613,29 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC if (nChangePosInOut != -1) { tx.vout.insert(tx.vout.begin() + nChangePosInOut, wtx.tx->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. + // 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(); } - // 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++) + // 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; + } - // Add new txins (keeping original txin scriptSig/order) - for (const CTxIn& txin : wtx.tx->vin) - { - if (!coinControl.IsSelected(txin.prevout)) - { + // Add new txins while keeping original txin scriptSig/order. + for (const CTxIn& txin : wtx.tx->vin) { + if (!coinControl.IsSelected(txin.prevout)) { tx.vin.push_back(txin); - if (lockUnspents) - { - LOCK2(cs_main, cs_wallet); - LockCoin(txin.prevout); + if (lockUnspents) { + LOCK2(cs_main, cs_wallet); + LockCoin(txin.prevout); } } } - return true; } From 03a5dc9c3c522c500c77fdecd52d091db048d1b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Thu, 14 Dec 2017 03:18:29 +0000 Subject: [PATCH 2/2] [wallet] Make CWallet::FundTransaction atomic --- src/wallet/wallet.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 4777641e729c0..106c910258367 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2605,6 +2605,10 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC coinControl.Select(txin.prevout); } + // Acquire the locks to prevent races to the new locked unspents between the + // CreateTransaction call and LockCoin calls (when lockUnspents is true). + LOCK2(cs_main, cs_wallet); + CReserveKey reservekey(this); CWalletTx wtx; if (!CreateTransaction(vecSend, wtx, reservekey, nFeeRet, nChangePosInOut, strFailReason, coinControl, false)) { @@ -2630,7 +2634,6 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC tx.vin.push_back(txin); if (lockUnspents) { - LOCK2(cs_main, cs_wallet); LockCoin(txin.prevout); } }