Skip to content

Commit 2b72d46

Browse files
gavinandresengavinandresen
committed
Handle "conflicted" transactions properly
Extend CMerkleTx::GetDepthInMainChain with the concept of a "conflicted" transaction-- a transaction generated by the wallet that is not in the main chain or in the mempool, and, therefore, will likely never be confirmed. GetDepthInMainChain() now returns -1 for conflicted transactions (0 for unconfirmed-but-in-the-mempool, and >1 for confirmed). This makes getbalance, getbalance '*', and listunspent all agree when there are mutated transactions in the wallet. Before: listunspent: one 49BTC output getbalance: 96 BTC (change counted twice) getbalance '*': 46 BTC (spends counted twice) After: all agree, 49 BTC available to spend.
1 parent f582eda commit 2b72d46

File tree

8 files changed

+194
-11
lines changed

8 files changed

+194
-11
lines changed

‎qa/rpc-tests/txnmall.sh

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
#!/usr/bin/env bash
2+
3+
# Test block generation and basic wallet sending
4+
5+
if [ $# -lt 1 ]; then
6+
echo "Usage: $0 path_to_binaries"
7+
echo "e.g. $0 ../../src"
8+
exit 1
9+
fi
10+
11+
BITCOIND=${1}/bitcoind
12+
CLI=${1}/bitcoin-cli
13+
14+
DIR="${BASH_SOURCE%/*}"
15+
SENDANDWAIT="${DIR}/send.sh"
16+
if [[ ! -d "$DIR" ]]; then DIR="$PWD"; fi
17+
. "$DIR/util.sh"
18+
19+
D=$(mktemp -d test.XXXXX)
20+
21+
# Two nodes; one will play the part of merchant, the
22+
# other an evil transaction-mutating miner.
23+
24+
D1=${D}/node1
25+
CreateDataDir $D1 port=11000 rpcport=11001
26+
B1ARGS="-datadir=$D1 -debug"
27+
$BITCOIND $B1ARGS &
28+
B1PID=$!
29+
30+
D2=${D}/node2
31+
CreateDataDir $D2 port=11010 rpcport=11011
32+
B2ARGS="-datadir=$D2 -debug"
33+
$BITCOIND $B2ARGS &
34+
B2PID=$!
35+
36+
trap "kill -9 $B1PID $B2PID; rm -rf $D" EXIT
37+
38+
# Wait until all four nodes are at the same block number
39+
function WaitBlocks {
40+
while :
41+
do
42+
sleep 1
43+
BLOCKS1=$( GetBlocks $B1ARGS )
44+
BLOCKS2=$( GetBlocks $B2ARGS )
45+
if (( $BLOCKS1 == $BLOCKS2 ))
46+
then
47+
break
48+
fi
49+
done
50+
}
51+
52+
# Wait until node has $N peers
53+
function WaitPeers {
54+
while :
55+
do
56+
PEERS=$( $CLI $1 getconnectioncount )
57+
if (( "$PEERS" == $2 ))
58+
then
59+
break
60+
fi
61+
sleep 1
62+
done
63+
}
64+
65+
# Start with B2 connected to B1:
66+
$CLI $B2ARGS addnode 127.0.0.1:11000 onetry
67+
WaitPeers "$B1ARGS" 1
68+
69+
# 1 block, 50 XBT each == 50 XBT
70+
$CLI $B1ARGS setgenerate true 1
71+
72+
WaitBlocks
73+
# 100 blocks, 0 mature == 0 XBT
74+
$CLI $B2ARGS setgenerate true 100
75+
WaitBlocks
76+
77+
CheckBalance $B1ARGS 50
78+
CheckBalance $B2ARGS 0
79+
80+
# restart B2 with no connection
81+
$CLI $B2ARGS stop > /dev/null 2>&1
82+
wait $B2PID
83+
$BITCOIND $B2ARGS &
84+
B2PID=$!
85+
86+
B2ADDRESS=$( $CLI $B2ARGS getnewaddress )
87+
88+
# Have B1 create two transactions; second will
89+
# spend change from first, since B1 starts with only a single
90+
# 50 bitcoin output:
91+
TXID1=$( $CLI $B1ARGS sendtoaddress $B2ADDRESS 1.0 )
92+
TXID2=$( $CLI $B1ARGS sendtoaddress $B2ADDRESS 2.0 )
93+
94+
# Mutate TXID1 and add it to B2's memory pool:
95+
RAWTX1=$( $CLI $B1ARGS getrawtransaction $TXID1 )
96+
RAWTX2=$( $CLI $B1ARGS getrawtransaction $TXID2 )
97+
# ... mutate RAWTX1:
98+
# RAWTX1 is hex-encoded, serialized transaction. So each
99+
# byte is two characters; we'll prepend the first
100+
# "push" in the scriptsig with OP_PUSHDATA1 (0x4c),
101+
# and add one to the length of the signature.
102+
# Fields are fixed; from the beginning:
103+
# 4-byte version
104+
# 1-byte varint number-of inputs (one in this case)
105+
# 32-byte previous txid
106+
# 4-byte previous output
107+
# 1-byte varint length-of-scriptsig
108+
# 1-byte PUSH this many bytes onto stack
109+
# ... etc
110+
# So: to mutate, we want to get byte 41 (hex characters 82-83),
111+
# increment it, and insert 0x4c after it.
112+
L=${RAWTX1:82:2}
113+
NEWLEN=$( printf "%x" $(( 16#$L + 1 )) )
114+
MUTATEDTX1=${RAWTX1:0:82}${NEWLEN}4c${RAWTX1:84}
115+
# ... give mutated tx1 to B2:
116+
MUTATEDTXID=$( $CLI $B2ARGS sendrawtransaction $MUTATEDTX1 )
117+
118+
echo "TXID1: " $TXID1
119+
echo "Mutated: " $MUTATEDTXID
120+
121+
# Re-connect nodes, and have B2 mine a block
122+
$CLI $B2ARGS addnode 127.0.0.1:11000 onetry
123+
WaitPeers "$B1ARGS" 1
124+
125+
$CLI $B2ARGS setgenerate true 1
126+
WaitBlocks
127+
128+
$CLI $B2ARGS stop > /dev/null 2>&1
129+
wait $B2PID
130+
$CLI $B1ARGS stop > /dev/null 2>&1
131+
wait $B1PID
132+
133+
trap "" EXIT
134+
135+
echo "Done, bitcoind's shut down. To rerun/poke around:"
136+
echo "${1}/bitcoind -datadir=$D1 -daemon"
137+
echo "${1}/bitcoind -datadir=$D2 -daemon -connect=127.0.0.1:11000"
138+
echo "To cleanup:"
139+
echo "killall bitcoind; rm -rf test.*"
140+
exit 0
141+
142+
echo "Tests successful, cleaning up"
143+
rm -rf $D
144+
exit 0

‎src/main.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -872,7 +872,7 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa
872872
}
873873

874874

875-
int CMerkleTx::GetDepthInMainChain(CBlockIndex* &pindexRet) const
875+
int CMerkleTx::GetDepthInMainChainINTERNAL(CBlockIndex* &pindexRet) const
876876
{
877877
if (hashBlock == 0 || nIndex == -1)
878878
return 0;
@@ -897,6 +897,14 @@ int CMerkleTx::GetDepthInMainChain(CBlockIndex* &pindexRet) const
897897
return chainActive.Height() - pindex->nHeight + 1;
898898
}
899899

900+
int CMerkleTx::GetDepthInMainChain(CBlockIndex* &pindexRet) const
901+
{
902+
int nResult = GetDepthInMainChainINTERNAL(pindexRet);
903+
if (nResult == 0 && !mempool.exists(GetHash()))
904+
return -1; // Not in chain, not in mempool
905+
906+
return nResult;
907+
}
900908

901909
int CMerkleTx::GetBlocksToMaturity() const
902910
{

‎src/main.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,8 @@ class CScriptCheck
423423
/** A transaction with a merkle branch linking it to the block chain. */
424424
class CMerkleTx : public CTransaction
425425
{
426+
private:
427+
int GetDepthInMainChainINTERNAL(CBlockIndex* &pindexRet) const;
426428
public:
427429
uint256 hashBlock;
428430
std::vector<uint256> vMerkleBranch;
@@ -461,9 +463,14 @@ class CMerkleTx : public CTransaction
461463

462464

463465
int SetMerkleBranch(const CBlock* pblock=NULL);
466+
467+
// Return depth of transaction in blockchain:
468+
// -1 : not in blockchain, and not in memory pool (conflicted transaction)
469+
// 0 : in memory pool, waiting to be included in a block
470+
// >=1 : this many blocks deep in the main chain
464471
int GetDepthInMainChain(CBlockIndex* &pindexRet) const;
465472
int GetDepthInMainChain() const { CBlockIndex *pindexRet; return GetDepthInMainChain(pindexRet); }
466-
bool IsInMainChain() const { return GetDepthInMainChain() > 0; }
473+
bool IsInMainChain() const { CBlockIndex *pindexRet; return GetDepthInMainChainINTERNAL(pindexRet) > 0; }
467474
int GetBlocksToMaturity() const;
468475
bool AcceptToMemoryPool(bool fLimitFree=true);
469476
};

‎src/qt/transactiondesc.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@ QString TransactionDesc::FormatTxStatus(const CWalletTx& wtx)
3030
else
3131
{
3232
int nDepth = wtx.GetDepthInMainChain();
33-
if (GetAdjustedTime() - wtx.nTimeReceived > 2 * 60 && wtx.GetRequestCount() == 0)
33+
if (nDepth < 0)
34+
return tr("conflicted");
35+
else if (GetAdjustedTime() - wtx.nTimeReceived > 2 * 60 && wtx.GetRequestCount() == 0)
3436
return tr("%1/offline").arg(nDepth);
3537
else if (nDepth < 6)
3638
return tr("%1/unconfirmed").arg(nDepth);

‎src/qt/walletmodel.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,9 @@ void WalletModel::getOutputs(const std::vector<COutPoint>& vOutpoints, std::vect
494494
BOOST_FOREACH(const COutPoint& outpoint, vOutpoints)
495495
{
496496
if (!wallet->mapWallet.count(outpoint.hash)) continue;
497-
COutput out(&wallet->mapWallet[outpoint.hash], outpoint.n, wallet->mapWallet[outpoint.hash].GetDepthInMainChain());
497+
int nDepth = wallet->mapWallet[outpoint.hash].GetDepthInMainChain();
498+
if (nDepth < 0) continue;
499+
COutput out(&wallet->mapWallet[outpoint.hash], outpoint.n, nDepth);
498500
vOutputs.push_back(out);
499501
}
500502
}
@@ -513,7 +515,9 @@ void WalletModel::listCoins(std::map<QString, std::vector<COutput> >& mapCoins)
513515
BOOST_FOREACH(const COutPoint& outpoint, vLockedCoins)
514516
{
515517
if (!wallet->mapWallet.count(outpoint.hash)) continue;
516-
COutput out(&wallet->mapWallet[outpoint.hash], outpoint.n, wallet->mapWallet[outpoint.hash].GetDepthInMainChain());
518+
int nDepth = wallet->mapWallet[outpoint.hash].GetDepthInMainChain();
519+
if (nDepth < 0) continue;
520+
COutput out(&wallet->mapWallet[outpoint.hash], outpoint.n, nDepth);
517521
vCoins.push_back(out);
518522
}
519523

‎src/rpcwallet.cpp

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ void WalletTxToJSON(const CWalletTx& wtx, Object& entry)
4545
entry.push_back(Pair("confirmations", confirms));
4646
if (wtx.IsCoinBase())
4747
entry.push_back(Pair("generated", true));
48-
if (confirms)
48+
if (confirms > 0)
4949
{
5050
entry.push_back(Pair("blockhash", wtx.hashBlock.GetHex()));
5151
entry.push_back(Pair("blockindex", wtx.nIndex));
@@ -1109,7 +1109,10 @@ void ListTransactions(const CWalletTx& wtx, const string& strAccount, int nMinDe
11091109
Object entry;
11101110
entry.push_back(Pair("account", strSentAccount));
11111111
MaybePushAddress(entry, s.first);
1112-
entry.push_back(Pair("category", "send"));
1112+
if (wtx.GetDepthInMainChain() < 0)
1113+
entry.push_back(Pair("category", "conflicted"));
1114+
else
1115+
entry.push_back(Pair("category", "send"));
11131116
entry.push_back(Pair("amount", ValueFromAmount(-s.second)));
11141117
entry.push_back(Pair("fee", ValueFromAmount(-nFee)));
11151118
if (fLong)
@@ -1141,7 +1144,12 @@ void ListTransactions(const CWalletTx& wtx, const string& strAccount, int nMinDe
11411144
entry.push_back(Pair("category", "generate"));
11421145
}
11431146
else
1144-
entry.push_back(Pair("category", "receive"));
1147+
{
1148+
if (wtx.GetDepthInMainChain() < 0)
1149+
entry.push_back(Pair("category", "conflicted"));
1150+
else
1151+
entry.push_back(Pair("category", "receive"));
1152+
}
11451153
entry.push_back(Pair("amount", ValueFromAmount(r.second)));
11461154
if (fLong)
11471155
WalletTxToJSON(wtx, entry);

‎src/wallet.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1021,11 +1021,15 @@ void CWallet::AvailableCoins(vector<COutput>& vCoins, bool fOnlyConfirmed, const
10211021
if (pcoin->IsCoinBase() && pcoin->GetBlocksToMaturity() > 0)
10221022
continue;
10231023

1024+
int nDepth = pcoin->GetDepthInMainChain();
1025+
if (nDepth < 0)
1026+
continue;
1027+
10241028
for (unsigned int i = 0; i < pcoin->vout.size(); i++) {
10251029
if (!(pcoin->IsSpent(i)) && IsMine(pcoin->vout[i]) &&
10261030
!IsLockedCoin((*it).first, i) && pcoin->vout[i].nValue > 0 &&
10271031
(!coinControl || !coinControl->HasSelected() || coinControl->IsSelected((*it).first, i)))
1028-
vCoins.push_back(COutput(pcoin, i, pcoin->GetDepthInMainChain()));
1032+
vCoins.push_back(COutput(pcoin, i, nDepth));
10291033
}
10301034
}
10311035
}

‎src/wallet.h

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -700,8 +700,11 @@ class CWalletTx : public CMerkleTx
700700
// Quick answer in most cases
701701
if (!IsFinalTx(*this))
702702
return false;
703-
if (GetDepthInMainChain() >= 1)
703+
int nDepth = GetDepthInMainChain();
704+
if (nDepth >= 1)
704705
return true;
706+
if (nDepth < 0)
707+
return false;
705708
if (!bSpendZeroConfChange || !IsFromMe()) // using wtx's cached debit
706709
return false;
707710

@@ -717,8 +720,11 @@ class CWalletTx : public CMerkleTx
717720

718721
if (!IsFinalTx(*ptx))
719722
return false;
720-
if (ptx->GetDepthInMainChain() >= 1)
723+
int nPDepth = ptx->GetDepthInMainChain();
724+
if (nPDepth >= 1)
721725
continue;
726+
if (nPDepth < 0)
727+
return false;
722728
if (!pwallet->IsFromMe(*ptx))
723729
return false;
724730

0 commit comments

Comments
 (0)