diff --git a/qa/rpc-tests/README.md b/qa/rpc-tests/README.md index ee9e8b35..15aede6c 100644 --- a/qa/rpc-tests/README.md +++ b/qa/rpc-tests/README.md @@ -1,9 +1,10 @@ Regression tests of RPC interface ================================= -wallet.sh : Exercise wallet send/receive code. +Bash scripts that use the RPC interface and command-line bitcoin-cli to test +full functionality in -regtest mode. -walletbackup.sh : Exercise wallet backup / dump / import +wallet.sh : Exercise wallet send/receive code. txnmall.sh : Test proper accounting of malleable transactions diff --git a/qa/rpc-tests/conflictedbalance.sh b/qa/rpc-tests/conflictedbalance.sh new file mode 100755 index 00000000..9d854d2d --- /dev/null +++ b/qa/rpc-tests/conflictedbalance.sh @@ -0,0 +1,143 @@ +#!/usr/bin/env bash + +# Test marking of spent outputs + +# Create a transaction graph with four transactions, +# A/B/C/D +# C spends A +# D spends B and C + +# Then simulate C being mutated, to create C' +# that is mined. +# A is still (correctly) considered spent. +# B should be treated as unspent + +if [ $# -lt 1 ]; then + echo "Usage: $0 path_to_binaries" + echo "e.g. $0 ../../src" + exit 1 +fi + +set -f + +BITCOIND=${1}/bitcoind +CLI=${1}/bitcoin-cli + +DIR="${BASH_SOURCE%/*}" +SENDANDWAIT="${DIR}/send.sh" +if [[ ! -d "$DIR" ]]; then DIR="$PWD"; fi +. "$DIR/util.sh" + +D=$(mktemp -d test.XXXXX) + +# Two nodes; one will play the part of merchant, the +# other an evil transaction-mutating miner. + +D1=${D}/node1 +CreateDataDir $D1 port=11000 rpcport=11001 +B1ARGS="-datadir=$D1 -debug=mempool" +$BITCOIND $B1ARGS & +B1PID=$! + +D2=${D}/node2 +CreateDataDir $D2 port=11010 rpcport=11011 +B2ARGS="-datadir=$D2 -debug=mempool" +$BITCOIND $B2ARGS & +B2PID=$! + +# Wait until all four nodes are at the same block number +function WaitBlocks { + while : + do + sleep 1 + declare -i BLOCKS1=$( GetBlocks $B1ARGS ) + declare -i BLOCKS2=$( GetBlocks $B2ARGS ) + if (( BLOCKS1 == BLOCKS2 )) + then + break + fi + done +} + +# Wait until node has $N peers +function WaitPeers { + while : + do + declare -i PEERS=$( $CLI $1 getconnectioncount ) + if (( PEERS == "$2" )) + then + break + fi + sleep 1 + done +} + +echo "Generating test blockchain..." + +# Start with B2 connected to B1: +$CLI $B2ARGS addnode 127.0.0.1:11000 onetry +WaitPeers "$B1ARGS" 1 + +# 2 block, 50 XBT each == 100 XBT +# These will be transactions "A" and "B" +$CLI $B1ARGS setgenerate true 2 + +WaitBlocks +# 100 blocks, 0 mature == 0 XBT +$CLI $B2ARGS setgenerate true 100 +WaitBlocks + +CheckBalance "$B1ARGS" 100 +CheckBalance "$B2ARGS" 0 + +# restart B2 with no connection +$CLI $B2ARGS stop > /dev/null 2>&1 +wait $B2PID +$BITCOIND $B2ARGS & +B2PID=$! + +B1ADDRESS=$( $CLI $B1ARGS getnewaddress ) +B2ADDRESS=$( $CLI $B2ARGS getnewaddress ) + +# Transaction C: send-to-self, spend A +TXID_C=$( $CLI $B1ARGS sendtoaddress $B1ADDRESS 50.0) + +# Transaction D: spends B and C +TXID_D=$( $CLI $B1ARGS sendtoaddress $B2ADDRESS 100.0) + +CheckBalance "$B1ARGS" 0 + +# Mutate TXID_C and add it to B2's memory pool: +RAWTX_C=$( $CLI $B1ARGS getrawtransaction $TXID_C ) + +# ... mutate C to create C' +L=${RAWTX_C:82:2} +NEWLEN=$( printf "%x" $(( 16#$L + 1 )) ) +MUTATEDTX_C=${RAWTX_C:0:82}${NEWLEN}4c${RAWTX_C:84} +# ... give mutated tx1 to B2: +MUTATEDTXID=$( $CLI $B2ARGS sendrawtransaction $MUTATEDTX_C ) + +echo "TXID_C: " $TXID_C +echo "Mutated: " $MUTATEDTXID + +# Re-connect nodes, and have both nodes mine some blocks: +$CLI $B2ARGS addnode 127.0.0.1:11000 onetry +WaitPeers "$B1ARGS" 1 + +# Having B2 mine the next block puts the mutated +# transaction C in the chain: +$CLI $B2ARGS setgenerate true 1 +WaitBlocks + +# B1 should still be able to spend 100, because D is conflicted +# so does not count as a spend of B +CheckBalance "$B1ARGS" 100 + +$CLI $B2ARGS stop > /dev/null 2>&1 +wait $B2PID +$CLI $B1ARGS stop > /dev/null 2>&1 +wait $B1PID + +echo "Tests successful, cleaning up" +rm -rf $D +exit 0 diff --git a/qa/rpc-tests/txnmall.sh b/qa/rpc-tests/txnmall.sh index 06e4f710..11e02764 100755 --- a/qa/rpc-tests/txnmall.sh +++ b/qa/rpc-tests/txnmall.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash -# Test block generation and basic wallet sending +# Test proper accounting with malleable transactions if [ $# -lt 1 ]; then echo "Usage: $0 path_to_binaries" @@ -35,16 +35,14 @@ B2ARGS="-datadir=$D2" $BITCOIND $B2ARGS & B2PID=$! -trap "kill -9 $B1PID $B2PID; rm -rf $D" EXIT - -# Wait until all four nodes are at the same block number +# Wait until both nodes are at the same block number function WaitBlocks { while : do sleep 1 - BLOCKS1=$( GetBlocks $B1ARGS ) - BLOCKS2=$( GetBlocks $B2ARGS ) - if (( $BLOCKS1 == $BLOCKS2 )) + declare -i BLOCKS1=$( GetBlocks $B1ARGS ) + declare -i BLOCKS2=$( GetBlocks $B2ARGS ) + if (( BLOCKS1 == BLOCKS2 )) then break fi @@ -55,8 +53,8 @@ function WaitBlocks { function WaitPeers { while : do - PEERS=$( $CLI $1 getconnectioncount ) - if (( "$PEERS" == $2 )) + declare -i PEERS=$( $CLI $1 getconnectioncount ) + if (( PEERS == "$2" )) then break fi @@ -64,6 +62,8 @@ function WaitPeers { done } +echo "Generating test blockchain..." + # Start with B2 connected to B1: $CLI $B2ARGS addnode 127.0.0.1:11000 onetry WaitPeers "$B1ARGS" 1 @@ -76,8 +76,8 @@ WaitBlocks $CLI $B2ARGS setgenerate true 100 WaitBlocks -CheckBalance $B1ARGS 50 -CheckBalance $B2ARGS 0 +CheckBalance "$B1ARGS" 50 +CheckBalance "$B2ARGS" 0 # restart B2 with no connection $CLI $B2ARGS stop > /dev/null 2>&1 @@ -85,20 +85,18 @@ wait $B2PID $BITCOIND $B2ARGS & B2PID=$! -B2ADDRESS=$( $CLI $B2ARGS getnewaddress ) +B2ADDRESS=$( $CLI $B2ARGS getaccountaddress "from1" ) # Have B1 create two transactions; second will # spend change from first, since B1 starts with only a single # 50 bitcoin output: -$CLI $B1ARGS move "" "foo" 10.0 -$CLI $B1ARGS move "" "bar" 10.0 +$CLI $B1ARGS move "" "foo" 10.0 > /dev/null +$CLI $B1ARGS move "" "bar" 10.0 > /dev/null TXID1=$( $CLI $B1ARGS sendfrom foo $B2ADDRESS 1.0 0) TXID2=$( $CLI $B1ARGS sendfrom bar $B2ADDRESS 2.0 0) # Mutate TXID1 and add it to B2's memory pool: RAWTX1=$( $CLI $B1ARGS getrawtransaction $TXID1 ) -RAWTX2=$( $CLI $B1ARGS getrawtransaction $TXID2 ) -# ... mutate RAWTX1: # RAWTX1 is hex-encoded, serialized transaction. So each # byte is two characters; we'll prepend the first # "push" in the scriptsig with OP_PUSHDATA1 (0x4c), @@ -123,28 +121,28 @@ echo "TXID1: " $TXID1 echo "Mutated: " $MUTATEDTXID # Re-connect nodes, and have B2 mine a block +# containing the mutant: $CLI $B2ARGS addnode 127.0.0.1:11000 onetry -WaitPeers "$B1ARGS" 1 +$CLI $B2ARGS setgenerate true 1 +WaitBlocks -$CLI $B2ARGS setgenerate true 3 -WaitBlocks -$CLI $B1ARGS setgenerate true 3 -WaitBlocks +# B1 should have 49 BTC; the 2 BTC send is +# conflicted, and should not count in +# balances. +CheckBalance "$B1ARGS" 49 +CheckBalance "$B1ARGS" 49 "*" +CheckBalance "$B1ARGS" 9 "foo" +CheckBalance "$B1ARGS" 10 "bar" + +# B2 should have 51 BTC +CheckBalance "$B2ARGS" 51 +CheckBalance "$B2ARGS" 1 "from1" $CLI $B2ARGS stop > /dev/null 2>&1 wait $B2PID $CLI $B1ARGS stop > /dev/null 2>&1 wait $B1PID -trap "" EXIT - -echo "Done, bitcoind's shut down. To rerun/poke around:" -echo "${1}/bitcoind -datadir=$D1 -daemon" -echo "${1}/bitcoind -datadir=$D2 -daemon -connect=127.0.0.1:11000" -echo "To cleanup:" -echo "killall bitcoind; rm -rf test.*" -exit 0 - echo "Tests successful, cleaning up" rm -rf $D exit 0 diff --git a/qa/rpc-tests/util.sh b/qa/rpc-tests/util.sh index d1e4c941..9001c42f 100644 --- a/qa/rpc-tests/util.sh +++ b/qa/rpc-tests/util.sh @@ -41,8 +41,9 @@ function AssertEqual { # CheckBalance -datadir=... amount account minconf function CheckBalance { + declare -i EXPECT="$2" B=$( $CLI $1 getbalance $3 $4 ) - if (( $( echo "$B == $2" | bc ) == 0 )) + if (( $( echo "$B == $EXPECT" | bc ) == 0 )) then echoerr "bad balance: $B (expected $2)" exit 1 @@ -87,5 +88,5 @@ function SendRawTxn { # Use: GetBlocks # returns number of blocks from getinfo function GetBlocks { - ExtractKey blocks "$( $CLI $1 getinfo )" + $CLI $1 getblockcount } diff --git a/qa/rpc-tests/wallet.sh b/qa/rpc-tests/wallet.sh index 8d5a6cdc..2940566a 100755 --- a/qa/rpc-tests/wallet.sh +++ b/qa/rpc-tests/wallet.sh @@ -8,6 +8,8 @@ if [ $# -lt 1 ]; then exit 1 fi +set -f + BITCOIND=${1}/bitcoind CLI=${1}/bitcoin-cli @@ -19,40 +21,40 @@ if [[ ! -d "$DIR" ]]; then DIR="$PWD"; fi D=$(mktemp -d test.XXXXX) D1=${D}/node1 -CreateDataDir $D1 port=11000 rpcport=11001 +CreateDataDir "$D1" port=11000 rpcport=11001 B1ARGS="-datadir=$D1" $BITCOIND $B1ARGS & B1PID=$! D2=${D}/node2 -CreateDataDir $D2 port=11010 rpcport=11011 connect=127.0.0.1:11000 +CreateDataDir "$D2" port=11010 rpcport=11011 connect=127.0.0.1:11000 B2ARGS="-datadir=$D2" $BITCOIND $B2ARGS & B2PID=$! D3=${D}/node3 -CreateDataDir $D3 port=11020 rpcport=11021 connect=127.0.0.1:11000 +CreateDataDir "$D3" port=11020 rpcport=11021 connect=127.0.0.1:11000 B3ARGS="-datadir=$D3" $BITCOIND $BITCOINDARGS $B3ARGS & B3PID=$! -trap "kill -9 $B1PID $B2PID $B3PID; rm -rf $D" EXIT - # Wait until all three nodes are at the same block number function WaitBlocks { while : do sleep 1 - BLOCKS1=$( GetBlocks $B1ARGS ) - BLOCKS2=$( GetBlocks $B2ARGS ) - BLOCKS3=$( GetBlocks $B3ARGS ) - if (( $BLOCKS1 == $BLOCKS2 && $BLOCKS2 == $BLOCKS3 )) + declare -i BLOCKS1=$( GetBlocks $B1ARGS ) + declare -i BLOCKS2=$( GetBlocks $B2ARGS ) + declare -i BLOCKS3=$( GetBlocks $B3ARGS ) + if (( BLOCKS1 == BLOCKS2 && BLOCKS2 == BLOCKS3 )) then break fi done } +echo "Generating test blockchain..." + # 1 block, 50 XBT each == 50 XBT $CLI $B1ARGS setgenerate true 1 WaitBlocks @@ -60,8 +62,8 @@ WaitBlocks $CLI $B2ARGS setgenerate true 101 WaitBlocks -CheckBalance $B1ARGS 50 -CheckBalance $B2ARGS 50 +CheckBalance "$B1ARGS" 50 +CheckBalance "$B2ARGS" 50 # Send 21 XBT from 1 to 3. Second # transaction will be child of first, and @@ -80,25 +82,25 @@ WaitBlocks # B1 should end up with 100 XBT in block rewards plus fees, # minus the 21 XBT sent to B3: -CheckBalance $B1ARGS "100-21" -CheckBalance $B3ARGS "21" +CheckBalance "$B1ARGS" "100-21" +CheckBalance "$B3ARGS" "21" # B1 should have two unspent outputs; create a couple # of raw transactions to send them to B3, submit them through # B2, and make sure both B1 and B3 pick them up properly: RAW1=$(CreateTxn1 $B1ARGS 1 $(Address $B3ARGS "from1" ) ) RAW2=$(CreateTxn1 $B1ARGS 2 $(Address $B3ARGS "from1" ) ) -RAWTXID1=$(SendRawTxn $B2ARGS $RAW1) -RAWTXID2=$(SendRawTxn $B2ARGS $RAW2) +RAWTXID1=$(SendRawTxn "$B2ARGS" $RAW1) +RAWTXID2=$(SendRawTxn "$B2ARGS" $RAW2) # Have B2 mine a block to confirm transactions: $CLI $B2ARGS setgenerate true 1 WaitBlocks # Check balances after confirmation -CheckBalance $B1ARGS 0 -CheckBalance $B3ARGS 100 -CheckBalance $B3ARGS "100-21" "from1" +CheckBalance "$B1ARGS" 0 +CheckBalance "$B3ARGS" 100 +CheckBalance "$B3ARGS" "100-21" "from1" $CLI $B3ARGS stop > /dev/null 2>&1 wait $B3PID @@ -108,6 +110,5 @@ $CLI $B1ARGS stop > /dev/null 2>&1 wait $B1PID echo "Tests successful, cleaning up" -trap "" EXIT rm -rf $D exit 0 diff --git a/src/main.cpp b/src/main.cpp index 8a5b659e..7afaa9e7 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1866,17 +1866,23 @@ bool static DisconnectTip(CValidationState &state) { // Write the chain state to disk, if necessary. if (!WriteChainState(state)) return false; - // Ressurect mempool transactions from the disconnected block. + // Resurrect mempool transactions from the disconnected block. BOOST_FOREACH(const CTransaction &tx, block.vtx) { // ignore validation errors in resurrected transactions + list removed; CValidationState stateDummy; if (!tx.IsCoinBase()) if (!AcceptToMemoryPool(mempool, stateDummy, tx, false, NULL)) - mempool.remove(tx, true); + mempool.remove(tx, removed, true); } mempool.check(pcoinsTip); // Update chainActive and related variables. UpdateTip(pindexDelete->pprev); + // Let wallets know transactions went from 1-confirmed to + // 0-confirmed or conflicted: + BOOST_FOREACH(const CTransaction &tx, block.vtx) { + SyncWithWallets(tx.GetHash(), tx, NULL); + } return true; } @@ -1907,13 +1913,24 @@ bool static ConnectTip(CValidationState &state, CBlockIndex *pindexNew) { if (!WriteChainState(state)) return false; // Remove conflicting transactions from the mempool. + list txConflicted; BOOST_FOREACH(const CTransaction &tx, block.vtx) { - mempool.remove(tx); - mempool.removeConflicts(tx); + list unused; + mempool.remove(tx, unused); + mempool.removeConflicts(tx, txConflicted); } mempool.check(pcoinsTip); // Update chainActive & related variables. UpdateTip(pindexNew); + // Tell wallet about transactions that went from mempool + // to conflicted: + BOOST_FOREACH(const CTransaction &tx, txConflicted) { + SyncWithWallets(tx.GetHash(), tx, NULL); + } + // ... and about transactions that got confirmed: + BOOST_FOREACH(const CTransaction &tx, block.vtx) { + SyncWithWallets(tx.GetHash(), tx, &block); + } return true; } diff --git a/src/qt/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp index e1a9140f..2d8fcd7a 100644 --- a/src/qt/coincontroldialog.cpp +++ b/src/qt/coincontroldialog.cpp @@ -468,11 +468,12 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog) BOOST_FOREACH(const COutput& out, vOutputs) { - // unselect already spent, very unlikely scenario, this could happen when selected are spent elsewhere, like rpc or another computer - if (out.tx->IsSpent(out.i)) + // unselect already spent, very unlikely scenario, this could happen + // when selected are spent elsewhere, like rpc or another computer + uint256 txhash = out.tx->GetHash(); + COutPoint outpt(txhash, out.i); + if (model->isSpent(outpt)) { - uint256 txhash = out.tx->GetHash(); - COutPoint outpt(txhash, out.i); coinControl->UnSelect(outpt); continue; } diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 3549cd49..eae448fe 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -501,6 +501,12 @@ void WalletModel::getOutputs(const std::vector& vOutpoints, std::vect } } +bool WalletModel::isSpent(const COutPoint& outpoint) const +{ + LOCK(wallet->cs_wallet); + return wallet->IsSpent(outpoint.hash, outpoint.n); +} + // AvailableCoins + LockedCoins grouped by wallet address (put change in one group with wallet address) void WalletModel::listCoins(std::map >& mapCoins) const { diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h index 91a6fba2..28a9169e 100644 --- a/src/qt/walletmodel.h +++ b/src/qt/walletmodel.h @@ -180,6 +180,7 @@ public: bool getPubKey(const CKeyID &address, CPubKey& vchPubKeyOut) const; void getOutputs(const std::vector& vOutpoints, std::vector& vOutputs); + bool isSpent(const COutPoint& outpoint) const; void listCoins(std::map >& mapCoins) const; bool isLockedCoin(uint256 hash, unsigned int n) const; diff --git a/src/rpcdump.cpp b/src/rpcdump.cpp index 9e1d4784..635d4ac1 100644 --- a/src/rpcdump.cpp +++ b/src/rpcdump.cpp @@ -128,7 +128,6 @@ Value importprivkey(const Array& params, bool fHelp) if (fRescan) { pwalletMain->ScanForWalletTransactions(chainActive.Genesis(), true); - pwalletMain->ReacceptWalletTransactions(); } } @@ -216,7 +215,6 @@ Value importwallet(const Array& params, bool fHelp) LogPrintf("Rescanning last %i blocks\n", chainActive.Height() - pindex->nHeight + 1); pwalletMain->ScanForWalletTransactions(pindex); - pwalletMain->ReacceptWalletTransactions(); pwalletMain->MarkDirty(); if (!fGood) diff --git a/src/rpcwallet.cpp b/src/rpcwallet.cpp index 7b605af5..d3b6c349 100644 --- a/src/rpcwallet.cpp +++ b/src/rpcwallet.cpp @@ -560,7 +560,7 @@ int64_t GetAccountBalance(CWalletDB& walletdb, const string& strAccount, int nMi for (map::iterator it = pwalletMain->mapWallet.begin(); it != pwalletMain->mapWallet.end(); ++it) { const CWalletTx& wtx = (*it).second; - if (!IsFinalTx(wtx)) + if (!IsFinalTx(wtx) || wtx.GetBlocksToMaturity() > 0 || wtx.GetDepthInMainChain() < 0) continue; int64_t nReceived, nSent, nFee; @@ -1324,13 +1324,14 @@ Value listaccounts(const Array& params, bool fHelp) string strSentAccount; list > listReceived; list > listSent; - if (wtx.GetBlocksToMaturity() > 0) + int nDepth = wtx.GetDepthInMainChain(); + if (wtx.GetBlocksToMaturity() > 0 || nDepth < 0) continue; wtx.GetAmounts(listReceived, listSent, nFee, strSentAccount); mapAccountBalances[strSentAccount] -= nFee; BOOST_FOREACH(const PAIRTYPE(CTxDestination, int64_t)& s, listSent) mapAccountBalances[strSentAccount] -= s.second; - if (wtx.GetDepthInMainChain() >= nMinDepth) + if (nDepth >= nMinDepth) { BOOST_FOREACH(const PAIRTYPE(CTxDestination, int64_t)& r, listReceived) if (pwalletMain->mapAddressBook.count(r.first)) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index be251d1d..64c9eac7 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -86,7 +86,7 @@ bool CTxMemPool::addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry) } -bool CTxMemPool::remove(const CTransaction &tx, bool fRecursive) +void CTxMemPool::remove(const CTransaction &tx, std::list& removed, bool fRecursive) { // Remove transaction from memory pool { @@ -95,34 +95,37 @@ bool CTxMemPool::remove(const CTransaction &tx, bool fRecursive) if (fRecursive) { for (unsigned int i = 0; i < tx.vout.size(); i++) { std::map::iterator it = mapNextTx.find(COutPoint(hash, i)); - if (it != mapNextTx.end()) - remove(*it->second.ptx, true); + if (it == mapNextTx.end()) + continue; + remove(*it->second.ptx, removed, true); } } if (mapTx.count(hash)) { + removed.push_front(tx); BOOST_FOREACH(const CTxIn& txin, tx.vin) mapNextTx.erase(txin.prevout); mapTx.erase(hash); nTransactionsUpdated++; } } - return true; } -bool CTxMemPool::removeConflicts(const CTransaction &tx) +void CTxMemPool::removeConflicts(const CTransaction &tx, std::list& removed) { // Remove transactions which depend on inputs of tx, recursively + list result; LOCK(cs); BOOST_FOREACH(const CTxIn &txin, tx.vin) { std::map::iterator it = mapNextTx.find(txin.prevout); if (it != mapNextTx.end()) { const CTransaction &txConflict = *it->second.ptx; if (txConflict != tx) - remove(txConflict, true); + { + remove(txConflict, removed, true); + } } } - return true; } void CTxMemPool::clear() diff --git a/src/txmempool.h b/src/txmempool.h index a652c424..4509e957 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -5,6 +5,8 @@ #ifndef BITCOIN_TXMEMPOOL_H #define BITCOIN_TXMEMPOOL_H +#include + #include "coins.h" #include "core.h" #include "sync.h" @@ -72,8 +74,8 @@ public: void setSanityCheck(bool _fSanityCheck) { fSanityCheck = _fSanityCheck; } bool addUnchecked(const uint256& hash, const CTxMemPoolEntry &entry); - bool remove(const CTransaction &tx, bool fRecursive = false); - bool removeConflicts(const CTransaction &tx); + void remove(const CTransaction &tx, std::list& removed, bool fRecursive = false); + void removeConflicts(const CTransaction &tx, std::list& removed); void clear(); void queryHashes(std::vector& vtxid); void pruneSpent(const uint256& hash, CCoins &coins); diff --git a/src/wallet.cpp b/src/wallet.cpp index eaf0b984..3ecd994e 100644 --- a/src/wallet.cpp +++ b/src/wallet.cpp @@ -32,6 +32,15 @@ struct CompareValueOnly } }; +const CWalletTx* CWallet::GetWalletTx(const uint256& hash) const +{ + LOCK(cs_wallet); + std::map::const_iterator it = mapWallet.find(hash); + if (it == mapWallet.end()) + return NULL; + return &(it->second); +} + CPubKey CWallet::GenerateNewKey() { AssertLockHeld(cs_wallet); // mapKeyMetadata @@ -239,18 +248,20 @@ set CWallet::GetConflicts(const uint256& txid) const return result; const CWalletTx& wtx = it->second; - std::pair range; + std::pair range; BOOST_FOREACH(const CTxIn& txin, wtx.vin) { - range = mapTxConflicts.equal_range(txin.prevout); - for (TxConflicts::const_iterator it = range.first; it != range.second; ++it) + if (mapTxSpends.count(txin.prevout) <= 1) + continue; // No conflict if zero or one spends + range = mapTxSpends.equal_range(txin.prevout); + for (TxSpends::const_iterator it = range.first; it != range.second; ++it) result.insert(it->second); } return result; } -void CWallet::SyncMetaData(pair range) +void CWallet::SyncMetaData(pair range) { // We want all the wallet transactions in range to have the same metadata as // the oldest (smallest nOrderPos). @@ -258,7 +269,7 @@ void CWallet::SyncMetaData(pair ra int nMinOrderPos = std::numeric_limits::max(); const CWalletTx* copyFrom = NULL; - for (TxConflicts::iterator it = range.first; it != range.second; ++it) + for (TxSpends::iterator it = range.first; it != range.second; ++it) { const uint256& hash = it->second; int n = mapWallet[hash].nOrderPos; @@ -269,7 +280,7 @@ void CWallet::SyncMetaData(pair ra } } // Now copy data from copyFrom to rest: - for (TxConflicts::iterator it = range.first; it != range.second; ++it) + for (TxSpends::iterator it = range.first; it != range.second; ++it) { const uint256& hash = it->second; CWalletTx* copyTo = &mapWallet[hash]; @@ -281,28 +292,48 @@ void CWallet::SyncMetaData(pair ra copyTo->nTimeSmart = copyFrom->nTimeSmart; copyTo->fFromMe = copyFrom->fFromMe; copyTo->strFromAccount = copyFrom->strFromAccount; - // vfSpent not copied on purpose // nOrderPos not copied on purpose // cached members not copied on purpose } } -void CWallet::AddToConflicts(const uint256& wtxhash) +// Outpoint is spent if any non-conflicted transaction +// spends it: +bool CWallet::IsSpent(const uint256& hash, unsigned int n) const { - assert(mapWallet.count(wtxhash)); - CWalletTx& thisTx = mapWallet[wtxhash]; - if (thisTx.IsCoinBase()) + const COutPoint outpoint(hash, n); + pair range; + range = mapTxSpends.equal_range(outpoint); + + for (TxSpends::const_iterator it = range.first; it != range.second; ++it) + { + const uint256& wtxid = it->second; + std::map::const_iterator mit = mapWallet.find(wtxid); + if (mit != mapWallet.end() && mit->second.GetDepthInMainChain() >= 0) + return true; // Spent + } + return false; +} + +void CWallet::AddToSpends(const COutPoint& outpoint, const uint256& wtxid) +{ + mapTxSpends.insert(make_pair(outpoint, wtxid)); + + pair range; + range = mapTxSpends.equal_range(outpoint); + SyncMetaData(range); +} + + +void CWallet::AddToSpends(const uint256& wtxid) +{ + assert(mapWallet.count(wtxid)); + CWalletTx& thisTx = mapWallet[wtxid]; + if (thisTx.IsCoinBase()) // Coinbases don't spend anything! return; BOOST_FOREACH(const CTxIn& txin, thisTx.vin) - { - mapTxConflicts.insert(make_pair(txin.prevout, wtxhash)); - - pair range; - range = mapTxConflicts.equal_range(txin.prevout); - if (range.first != range.second) - SyncMetaData(range); - } + AddToSpends(txin.prevout, wtxid); } bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) @@ -423,33 +454,6 @@ CWallet::TxItems CWallet::OrderedTxItems(std::list& acentries, return txOrdered; } -void CWallet::WalletUpdateSpent(const CTransaction &tx) -{ - // Anytime a signature is successfully verified, it's proof the outpoint is spent. - // Update the wallet spent flag if it doesn't know due to wallet.dat being - // restored from backup or the user making copies of wallet.dat. - { - LOCK(cs_wallet); - BOOST_FOREACH(const CTxIn& txin, tx.vin) - { - map::iterator mi = mapWallet.find(txin.prevout.hash); - if (mi != mapWallet.end()) - { - CWalletTx& wtx = (*mi).second; - if (txin.prevout.n >= wtx.vout.size()) - LogPrintf("WalletUpdateSpent: bad wtx %s\n", wtx.GetHash().ToString()); - else if (!wtx.IsSpent(txin.prevout.n) && IsMine(wtx.vout[txin.prevout.n])) - { - LogPrintf("WalletUpdateSpent found spent coin %sbc %s\n", FormatMoney(wtx.GetCredit()), wtx.GetHash().ToString()); - wtx.MarkSpent(txin.prevout.n); - wtx.WriteToDisk(); - NotifyTransactionChanged(this, txin.prevout.hash, CT_UPDATED); - } - } - } - } -} - void CWallet::MarkDirty() { { @@ -466,7 +470,7 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet) if (fFromLoadWallet) { mapWallet[hash] = wtxIn; - AddToConflicts(hash); + AddToSpends(hash); } else { @@ -526,7 +530,7 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet) wtxIn.GetHash().ToString(), wtxIn.hashBlock.ToString()); } - AddToConflicts(hash); + AddToSpends(hash); } bool fUpdated = false; @@ -549,7 +553,6 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet) wtx.fFromMe = wtxIn.fFromMe; fUpdated = true; } - fUpdated |= wtx.UpdateSpent(wtxIn.vfSpent); } //// debug print @@ -560,8 +563,8 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet) if (!wtx.WriteToDisk()) return false; - // since AddToWallet is called directly for self-originating transactions, check for consumption of own coins - WalletUpdateSpent(wtx); + // Break debit/credit balance caches: + wtx.MarkDirty(); // Notify UI of new or updated transaction NotifyTransactionChanged(this, hash, fInsertedNew ? CT_NEW : CT_UPDATED); @@ -596,14 +599,25 @@ bool CWallet::AddToWalletIfInvolvingMe(const uint256 &hash, const CTransaction& wtx.SetMerkleBranch(pblock); return AddToWallet(wtx); } - else - WalletUpdateSpent(tx); } return false; } -void CWallet::SyncTransaction(const uint256 &hash, const CTransaction& tx, const CBlock* pblock) { +void CWallet::SyncTransaction(const uint256 &hash, const CTransaction& tx, const CBlock* pblock) +{ AddToWalletIfInvolvingMe(hash, tx, pblock, true); + + if (mapWallet.count(hash) == 0) + return; // Not one of ours + + // If a transaction changes 'conflicted' state, that changes the balance + // available of the outputs it spends. So force those to be + // recomputed, also: + BOOST_FOREACH(const CTxIn& txin, tx.vin) + { + if (mapWallet.count(txin.prevout.hash)) + mapWallet[txin.prevout.hash].MarkDirty(); + } } void CWallet::EraseFromWallet(const uint256 &hash) @@ -804,78 +818,6 @@ void CWalletTx::GetAccountAmounts(const string& strAccount, int64_t& nReceived, } } -void CWalletTx::AddSupportingTransactions() -{ - vtxPrev.clear(); - - const int COPY_DEPTH = 3; - if (SetMerkleBranch() < COPY_DEPTH) - { - vector vWorkQueue; - BOOST_FOREACH(const CTxIn& txin, vin) - vWorkQueue.push_back(txin.prevout.hash); - - { - LOCK(pwallet->cs_wallet); - map mapWalletPrev; - set setAlreadyDone; - for (unsigned int i = 0; i < vWorkQueue.size(); i++) - { - uint256 hash = vWorkQueue[i]; - if (setAlreadyDone.count(hash)) - continue; - setAlreadyDone.insert(hash); - - CMerkleTx tx; - map::const_iterator mi = pwallet->mapWallet.find(hash); - if (mi != pwallet->mapWallet.end()) - { - tx = (*mi).second; - BOOST_FOREACH(const CMerkleTx& txWalletPrev, (*mi).second.vtxPrev) - mapWalletPrev[txWalletPrev.GetHash()] = &txWalletPrev; - } - else if (mapWalletPrev.count(hash)) - { - tx = *mapWalletPrev[hash]; - } - else - { - continue; - } - - int nDepth = tx.SetMerkleBranch(); - vtxPrev.push_back(tx); - - if (nDepth < COPY_DEPTH) - { - BOOST_FOREACH(const CTxIn& txin, tx.vin) - vWorkQueue.push_back(txin.prevout.hash); - } - } - } - } - - reverse(vtxPrev.begin(), vtxPrev.end()); -} - -bool CWalletTx::AcceptWalletTransaction() -{ - { - LOCK(mempool.cs); - // Add previous supporting transactions first - BOOST_FOREACH(CMerkleTx& tx, vtxPrev) - { - if (!tx.IsCoinBase()) - { - uint256 hash = tx.GetHash(); - if (!mempool.exists(hash) && pcoinsTip->HaveCoins(hash)) - tx.AcceptToMemoryPool(false); - } - } - return AcceptToMemoryPool(false); - } - return false; -} bool CWalletTx::WriteToDisk() { @@ -916,69 +858,26 @@ int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate) void CWallet::ReacceptWalletTransactions() { - bool fRepeat = true; - while (fRepeat) + LOCK(cs_wallet); + BOOST_FOREACH(PAIRTYPE(const uint256, CWalletTx)& item, mapWallet) { - LOCK(cs_wallet); - fRepeat = false; - bool fMissing = false; - BOOST_FOREACH(PAIRTYPE(const uint256, CWalletTx)& item, mapWallet) - { - CWalletTx& wtx = item.second; - if (wtx.IsCoinBase() && wtx.IsSpent(0)) - continue; + const uint256& wtxid = item.first; + CWalletTx& wtx = item.second; + assert(wtx.GetHash() == wtxid); - CCoins coins; - bool fUpdated = false; - bool fFound = pcoinsTip->GetCoins(wtx.GetHash(), coins); - if (fFound || wtx.GetDepthInMainChain() > 0) - { - // Update fSpent if a tx got spent somewhere else by a copy of wallet.dat - for (unsigned int i = 0; i < wtx.vout.size(); i++) - { - if (wtx.IsSpent(i)) - continue; - if ((i >= coins.vout.size() || coins.vout[i].IsNull()) && IsMine(wtx.vout[i])) - { - wtx.MarkSpent(i); - fUpdated = true; - fMissing = true; - } - } - if (fUpdated) - { - LogPrintf("ReacceptWalletTransactions found spent coin %sbc %s\n", FormatMoney(wtx.GetCredit()), wtx.GetHash().ToString()); - wtx.MarkDirty(); - wtx.WriteToDisk(); - } - } - else - { - // Re-accept any txes of ours that aren't already in a block - if (!wtx.IsCoinBase()) - wtx.AcceptWalletTransaction(); - } - } - if (fMissing) + int nDepth = wtx.GetDepthInMainChain(); + + if (!wtx.IsCoinBase() && nDepth < 0) { - // TODO: optimize this to scan just part of the block chain? - if (ScanForWalletTransactions(chainActive.Genesis())) - fRepeat = true; // Found missing transactions: re-do re-accept. + // Try to add to memory pool + LOCK(mempool.cs); + wtx.AcceptToMemoryPool(false); } } } void CWalletTx::RelayWalletTransaction() { - BOOST_FOREACH(const CMerkleTx& tx, vtxPrev) - { - // Important: versions of bitcoin before 0.8.6 had a bug that inserted - // empty transactions into the vtxPrev, which will cause the node to be - // banned when retransmitted, hence the check for !tx.vin.empty() - if (!tx.IsCoinBase() && !tx.vin.empty()) - if (tx.GetDepthInMainChain() == 0) - RelayTransaction((CTransaction)tx, tx.GetHash()); - } if (!IsCoinBase()) { if (GetDepthInMainChain() == 0) { @@ -1104,6 +1003,7 @@ void CWallet::AvailableCoins(vector& vCoins, bool fOnlyConfirmed, const LOCK(cs_wallet); for (map::const_iterator it = mapWallet.begin(); it != mapWallet.end(); ++it) { + const uint256& wtxid = it->first; const CWalletTx* pcoin = &(*it).second; if (!IsFinalTx(*pcoin)) @@ -1120,7 +1020,7 @@ void CWallet::AvailableCoins(vector& vCoins, bool fOnlyConfirmed, const continue; for (unsigned int i = 0; i < pcoin->vout.size(); i++) { - if (!(pcoin->IsSpent(i)) && IsMine(pcoin->vout[i]) && + if (!(IsSpent(wtxid, i)) && IsMine(pcoin->vout[i]) && !IsLockedCoin((*it).first, i) && pcoin->vout[i].nValue > 0 && (!coinControl || !coinControl->HasSelected() || coinControl->IsSelected((*it).first, i))) vCoins.push_back(COutput(pcoin, i, nDepth)); @@ -1452,8 +1352,6 @@ bool CWallet::CreateTransaction(const vector >& vecSend, continue; } - // Fill vtxPrev by copying from previous transactions vtxPrev - wtxNew.AddSupportingTransactions(); wtxNew.fTimeReceivedIsTxTime = true; break; @@ -1490,14 +1388,12 @@ bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey) // otherwise just for transaction history. AddToWallet(wtxNew); - // Mark old coins as spent + // Notify that old coins are spent set setCoins; BOOST_FOREACH(const CTxIn& txin, wtxNew.vin) { CWalletTx &coin = mapWallet[txin.prevout.hash]; coin.BindWallet(this); - coin.MarkSpent(txin.prevout.n); - coin.WriteToDisk(); NotifyTransactionChanged(this, coin.GetHash(), CT_UPDATED); } @@ -1855,7 +1751,7 @@ std::map CWallet::GetAddressBalances() if(!ExtractDestination(pcoin->vout[i].scriptPubKey, addr)) continue; - int64_t n = pcoin->IsSpent(i) ? 0 : pcoin->vout[i].nValue; + int64_t n = IsSpent(walletEntry.first, i) ? 0 : pcoin->vout[i].nValue; if (!balances.count(addr)) balances[addr] = 0; diff --git a/src/wallet.h b/src/wallet.h index eb192f1c..7feb86d2 100644 --- a/src/wallet.h +++ b/src/wallet.h @@ -108,11 +108,15 @@ private: int64_t nNextResend; int64_t nLastResend; - // Used to detect and report conflicted transactions: - typedef std::multimap TxConflicts; - TxConflicts mapTxConflicts; - void AddToConflicts(const uint256& wtxhash); - void SyncMetaData(std::pair); + // Used to keep track of spent outpoints, and + // detect and report conflicts (double-spends or + // mutated transactions where the mutant gets mined). + typedef std::multimap TxSpends; + TxSpends mapTxSpends; + void AddToSpends(const COutPoint& outpoint, const uint256& wtxid); + void AddToSpends(const uint256& wtxid); + + void SyncMetaData(std::pair); public: /// Main wallet lock. @@ -169,12 +173,16 @@ public: int64_t nTimeFirstKey; + const CWalletTx* GetWalletTx(const uint256& hash) const; + // check whether we are allowed to upgrade (or already support) to the named feature bool CanSupportFeature(enum WalletFeature wf) { AssertLockHeld(cs_wallet); return nWalletMaxVersion >= wf; } void AvailableCoins(std::vector& vCoins, bool fOnlyConfirmed=true, const CCoinControl *coinControl = NULL) const; bool SelectCoinsMinConf(int64_t nTargetValue, int nConfMine, int nConfTheirs, std::vector vCoins, std::set >& setCoinsRet, int64_t& nValueRet) const; + bool IsSpent(const uint256& hash, unsigned int n) const; + bool IsLockedCoin(uint256 hash, unsigned int n) const; void LockCoin(COutPoint& output); void UnlockCoin(COutPoint& output); @@ -234,7 +242,6 @@ public: void SyncTransaction(const uint256 &hash, const CTransaction& tx, const CBlock* pblock); bool AddToWalletIfInvolvingMe(const uint256 &hash, const CTransaction& tx, const CBlock* pblock, bool fUpdate); void EraseFromWallet(const uint256 &hash); - void WalletUpdateSpent(const CTransaction& prevout); int ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate = false); void ReacceptWalletTransactions(); void ResendWalletTransactions(); @@ -439,7 +446,6 @@ private: const CWallet* pwallet; public: - std::vector vtxPrev; mapValue_t mapValue; std::vector > vOrderForm; unsigned int fTimeReceivedIsTxTime; @@ -447,7 +453,6 @@ public: unsigned int nTimeSmart; char fFromMe; std::string strFromAccount; - std::vector vfSpent; // which outputs are already spent int64_t nOrderPos; // position in ordered transaction list // memory only @@ -485,7 +490,6 @@ public: void Init(const CWallet* pwalletIn) { pwallet = pwalletIn; - vtxPrev.clear(); mapValue.clear(); vOrderForm.clear(); fTimeReceivedIsTxTime = false; @@ -493,7 +497,6 @@ public: nTimeSmart = 0; fFromMe = false; strFromAccount.clear(); - vfSpent.clear(); fDebitCached = false; fCreditCached = false; fImmatureCreditCached = false; @@ -518,15 +521,6 @@ public: { pthis->mapValue["fromaccount"] = pthis->strFromAccount; - std::string str; - BOOST_FOREACH(char f, vfSpent) - { - str += (f ? '1' : '0'); - if (f) - fSpent = true; - } - pthis->mapValue["spent"] = str; - WriteOrderPos(pthis->nOrderPos, pthis->mapValue); if (nTimeSmart) @@ -534,7 +528,8 @@ public: } nSerSize += SerReadWrite(s, *(CMerkleTx*)this, nType, nVersion,ser_action); - READWRITE(vtxPrev); + std::vector vUnused; // Used to be vtxPrev + READWRITE(vUnused); READWRITE(mapValue); READWRITE(vOrderForm); READWRITE(fTimeReceivedIsTxTime); @@ -546,12 +541,6 @@ public: { pthis->strFromAccount = pthis->mapValue["fromaccount"]; - if (mapValue.count("spent")) - BOOST_FOREACH(char c, pthis->mapValue["spent"]) - pthis->vfSpent.push_back(c != '0'); - else - pthis->vfSpent.assign(vout.size(), fSpent); - ReadOrderPos(pthis->nOrderPos, pthis->mapValue); pthis->nTimeSmart = mapValue.count("timesmart") ? (unsigned int)atoi64(pthis->mapValue["timesmart"]) : 0; @@ -564,26 +553,6 @@ public: pthis->mapValue.erase("timesmart"); ) - // marks certain txout's as spent - // returns true if any update took place - bool UpdateSpent(const std::vector& vfNewSpent) - { - bool fReturn = false; - for (unsigned int i = 0; i < vfNewSpent.size(); i++) - { - if (i == vfSpent.size()) - break; - - if (vfNewSpent[i] && !vfSpent[i]) - { - vfSpent[i] = true; - fReturn = true; - fAvailableCreditCached = false; - } - } - return fReturn; - } - // make sure balances are recalculated void MarkDirty() { @@ -599,27 +568,6 @@ public: MarkDirty(); } - void MarkSpent(unsigned int nOut) - { - if (nOut >= vout.size()) - throw std::runtime_error("CWalletTx::MarkSpent() : nOut out of range"); - vfSpent.resize(vout.size()); - if (!vfSpent[nOut]) - { - vfSpent[nOut] = true; - fAvailableCreditCached = false; - } - } - - bool IsSpent(unsigned int nOut) const - { - if (nOut >= vout.size()) - throw std::runtime_error("CWalletTx::IsSpent() : nOut out of range"); - if (nOut >= vfSpent.size()) - return false; - return (!!vfSpent[nOut]); - } - int64_t GetDebit() const { if (vin.empty()) @@ -661,6 +609,9 @@ public: int64_t GetAvailableCredit(bool fUseCache=true) const { + if (pwallet == 0) + return 0; + // Must wait until coinbase is safely deep enough in the chain before valuing it if (IsCoinBase() && GetBlocksToMaturity() > 0) return 0; @@ -671,7 +622,7 @@ public: int64_t nCredit = 0; for (unsigned int i = 0; i < vout.size(); i++) { - if (!IsSpent(i)) + if (!pwallet->IsSpent(GetHash(), i)) { const CTxOut &txout = vout[i]; nCredit += pwallet->GetCredit(txout); @@ -719,38 +670,14 @@ public: if (!bSpendZeroConfChange || !IsFromMe()) // using wtx's cached debit return false; - // If no confirmations but it's from us, we can still - // consider it confirmed if all dependencies are confirmed - std::map mapPrev; - std::vector vWorkQueue; - vWorkQueue.reserve(vtxPrev.size()+1); - vWorkQueue.push_back(this); - for (unsigned int i = 0; i < vWorkQueue.size(); i++) + // Trusted if all inputs are from us and are in the mempool: + BOOST_FOREACH(const CTxIn& txin, vin) { - const CMerkleTx* ptx = vWorkQueue[i]; - - if (!IsFinalTx(*ptx)) + // Transactions not sent by us: not trusted + const CWalletTx* parent = pwallet->GetWalletTx(txin.prevout.hash); + const CTxOut& parentOut = parent->vout[txin.prevout.n]; + if (parent == NULL || !pwallet->IsMine(parentOut)) return false; - int nPDepth = ptx->GetDepthInMainChain(); - if (nPDepth >= 1) - continue; - if (nPDepth < 0) - return false; - if (!pwallet->IsFromMe(*ptx)) - return false; - - if (mapPrev.empty()) - { - BOOST_FOREACH(const CMerkleTx& tx, vtxPrev) - mapPrev[tx.GetHash()] = &tx; - } - - BOOST_FOREACH(const CTxIn& txin, ptx->vin) - { - if (!mapPrev.count(txin.prevout.hash)) - return false; - vWorkQueue.push_back(mapPrev[txin.prevout.hash]); - } } return true; } @@ -760,8 +687,6 @@ public: int64_t GetTxTime() const; int GetRequestCount() const; - void AddSupportingTransactions(); - bool AcceptWalletTransaction(); void RelayWalletTransaction(); std::set GetConflicts() const;