From 8fbf03995df9a2003be603be1a930bc3373d56e0 Mon Sep 17 00:00:00 2001 From: Tom Harding Date: Wed, 25 Jun 2014 16:21:29 -0700 Subject: [PATCH 1/6] CBloomFilter::clear() method --- src/bloom.cpp | 7 +++++++ src/bloom.h | 2 ++ src/test/bloom_tests.cpp | 4 ++++ 3 files changed, 13 insertions(+) diff --git a/src/bloom.cpp b/src/bloom.cpp index 26e366179..85a2ddc18 100644 --- a/src/bloom.cpp +++ b/src/bloom.cpp @@ -94,6 +94,13 @@ bool CBloomFilter::contains(const uint256& hash) const return contains(data); } +void CBloomFilter::clear() +{ + vData.assign(vData.size(),0); + isFull = false; + isEmpty = true; +} + bool CBloomFilter::IsWithinSizeConstraints() const { return vData.size() <= MAX_BLOOM_FILTER_SIZE && nHashFuncs <= MAX_HASH_FUNCS; diff --git a/src/bloom.h b/src/bloom.h index 956bead87..d0caf9e9f 100644 --- a/src/bloom.h +++ b/src/bloom.h @@ -78,6 +78,8 @@ public: bool contains(const COutPoint& outpoint) const; bool contains(const uint256& hash) const; + void clear(); + // True if the size is <= MAX_BLOOM_FILTER_SIZE and the number of hash functions is <= MAX_HASH_FUNCS // (catch a filter which was just deserialized which was too big) bool IsWithinSizeConstraints() const; diff --git a/src/test/bloom_tests.cpp b/src/test/bloom_tests.cpp index b56d998be..5c6c7d858 100644 --- a/src/test/bloom_tests.cpp +++ b/src/test/bloom_tests.cpp @@ -45,6 +45,10 @@ BOOST_AUTO_TEST_CASE(bloom_create_insert_serialize) expected[i] = (char)vch[i]; BOOST_CHECK_EQUAL_COLLECTIONS(stream.begin(), stream.end(), expected.begin(), expected.end()); + + BOOST_CHECK_MESSAGE( filter.contains(ParseHex("99108ad8ed9bb6274d3980bab5a85c048f0950c8")), "BloomFilter doesn't contain just-inserted object!"); + filter.clear(); + BOOST_CHECK_MESSAGE( !filter.contains(ParseHex("99108ad8ed9bb6274d3980bab5a85c048f0950c8")), "BloomFilter should be empty!"); } BOOST_AUTO_TEST_CASE(bloom_create_insert_serialize_with_tweak) From d640a3ceab4f4372c2a0f738c1286cfde4b41b50 Mon Sep 17 00:00:00 2001 From: Tom Harding Date: Wed, 25 Jun 2014 23:41:44 -0700 Subject: [PATCH 2/6] Relay double-spends, subject to anti-DOS Allows network wallets and other clients to see transactions that respend a prevout already spent in an unconfirmed transaction in this node's mempool. Knowledge of an attempted double-spend is of interest to recipients of the first spend. In some cases, it will allow these recipients to withhold goods or services upon being alerted of a double-spend that deprives them of payment. As before, respends are not added to the mempool. Anti-Denial-of-Service-Attack provisions: - Use a bloom filter to relay only one respend per mempool prevout - Rate-limit respend relays to a default of 100 thousand bytes/minute - Define tx2.IsEquivalentTo(tx1): equality when scriptSigs are not considered - Do not relay these equivalent transactions Remove an unused variable declaration in txmempool.cpp. --- src/core.cpp | 16 ++++++++ src/core.h | 3 ++ src/init.cpp | 1 + src/main.cpp | 100 +++++++++++++++++++++++++++++++++++++++------- src/main.h | 3 ++ src/txmempool.cpp | 1 - 6 files changed, 109 insertions(+), 15 deletions(-) diff --git a/src/core.cpp b/src/core.cpp index 6c5ee1c0f..ca2862452 100644 --- a/src/core.cpp +++ b/src/core.cpp @@ -119,6 +119,22 @@ CTransaction& CTransaction::operator=(const CTransaction &tx) { return *this; } +bool CTransaction::IsEquivalentTo(const CTransaction& tx) const +{ + if (nVersion != tx.nVersion || + nLockTime != tx.nLockTime || + vin.size() != tx.vin.size() || + vout != tx.vout) + return false; + for (unsigned int i = 0; i < vin.size(); i++) + { + if (vin[i].nSequence != tx.vin[i].nSequence || + vin[i].prevout != tx.vin[i].prevout) + return false; + } + return true; +} + int64_t CTransaction::GetValueOut() const { int64_t nValueOut = 0; diff --git a/src/core.h b/src/core.h index 27fda9555..860683157 100644 --- a/src/core.h +++ b/src/core.h @@ -256,6 +256,9 @@ public: return hash; } + // True if only scriptSigs are different + bool IsEquivalentTo(const CTransaction& tx) const; + // Return sum of txouts. int64_t GetValueOut() const; // GetValueIn() is a method on CCoinsViewCache, because diff --git a/src/init.cpp b/src/init.cpp index c7170b0f2..bd732753e 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1175,6 +1175,7 @@ bool AppInit2(boost::thread_group& threadGroup) LogPrintf("mapAddressBook.size() = %u\n", pwalletMain ? pwalletMain->mapAddressBook.size() : 0); #endif + RegisterInternalSignals(); StartNode(threadGroup); if (fServer) StartRPCThreads(); diff --git a/src/main.cpp b/src/main.cpp index d86fd3a24..48237c1b5 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -7,6 +7,7 @@ #include "addrman.h" #include "alert.h" +#include "bloom.h" #include "chainparams.h" #include "checkpoints.h" #include "checkqueue.h" @@ -122,6 +123,10 @@ namespace { map::iterator> > mapBlocksToDownload; } +// Forward reference functions defined here: +static const unsigned int MAX_DOUBLESPEND_BLOOM = 1000; +static void RelayDoubleSpend(const COutPoint& outPoint, const CTransaction& doubleSpend, bool fInBlock, CBloomFilter& filter); + ////////////////////////////////////////////////////////////////////////////// // // dispatching functions @@ -143,9 +148,24 @@ struct CMainSignals { boost::signals2::signal Inventory; // Tells listeners to broadcast their data. boost::signals2::signal Broadcast; + // Notifies listeners of detection of a double-spent transaction. Arguments are outpoint that is + // double-spent, first transaction seen, double-spend transaction, and whether the second double-spend + // transaction was first seen in a block. + // Note: only notifies if the previous transaction is in the memory pool; if previous transction was in a block, + // then the double-spend simply fails when we try to lookup the inputs in the current UTXO set. + boost::signals2::signal DetectedDoubleSpend; } g_signals; } +void RegisterInternalSignals() { + static CBloomFilter doubleSpendFilter; + seed_insecure_rand(); + doubleSpendFilter = CBloomFilter(MAX_DOUBLESPEND_BLOOM, 0.01, insecure_rand(), BLOOM_UPDATE_NONE); + + g_signals.DetectedDoubleSpend.connect(boost::bind(RelayDoubleSpend, _1, _2, _3, doubleSpendFilter)); +} + + void RegisterWallet(CWalletInterface* pwalletIn) { g_signals.SyncTransaction.connect(boost::bind(&CWalletInterface::SyncTransaction, pwalletIn, _1, _2)); g_signals.EraseTransaction.connect(boost::bind(&CWalletInterface::EraseFromWallet, pwalletIn, _1)); @@ -824,6 +844,22 @@ int64_t GetMinFee(const CTransaction& tx, unsigned int nBytes, bool fAllowFree, return nMinFee; } +// Exponentially limit the rate of nSize flow to nLimit. nLimit unit is thousands-per-minute. +bool RateLimitExceeded(double& dCount, int64_t& nLastTime, int64_t nLimit, unsigned int nSize) +{ + static CCriticalSection csLimiter; + int64_t nNow = GetTime(); + + LOCK(csLimiter); + + // Use an exponentially decaying ~10-minute window: + dCount *= pow(1.0 - 1.0/600.0, (double)(nNow - nLastTime)); + nLastTime = nNow; + if (dCount >= nLimit*10*1000) + return true; + dCount += nSize; + return false; +} bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransaction &tx, bool fLimitFree, bool* pfMissingInputs, bool fRejectInsaneFee) @@ -858,9 +894,10 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa for (unsigned int i = 0; i < tx.vin.size(); i++) { COutPoint outpoint = tx.vin[i].prevout; - if (pool.mapNextTx.count(outpoint)) + // Does tx conflict with a member of the pool, and is it not equivalent to that member? + if (pool.mapNextTx.count(outpoint) && !tx.IsEquivalentTo(*pool.mapNextTx[outpoint].ptx)) { - // Disable replacement feature for now + g_signals.DetectedDoubleSpend(outpoint, tx, false); return false; } } @@ -932,23 +969,15 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa // be annoying or make others' transactions take longer to confirm. if (fLimitFree && nFees < CTransaction::minRelayTxFee.GetFee(nSize)) { - static CCriticalSection csFreeLimiter; static double dFreeCount; - static int64_t nLastTime; - int64_t nNow = GetTime(); - - LOCK(csFreeLimiter); + static int64_t nLastFreeTime; + static int64_t nFreeLimit = GetArg("-limitfreerelay", 15); - // Use an exponentially decaying ~10-minute window: - dFreeCount *= pow(1.0 - 1.0/600.0, (double)(nNow - nLastTime)); - nLastTime = nNow; - // -limitfreerelay unit is thousand-bytes-per-minute - // At default rate it would take over a month to fill 1GB - if (dFreeCount >= GetArg("-limitfreerelay", 15)*10*1000) + if (RateLimitExceeded(dFreeCount, nLastFreeTime, nFreeLimit, nSize)) return state.DoS(0, error("AcceptToMemoryPool : free transaction rejected by rate limiter"), REJECT_INSUFFICIENTFEE, "insufficient priority"); + LogPrint("mempool", "Rate limit dFreeCount: %g => %g\n", dFreeCount, dFreeCount+nSize); - dFreeCount += nSize; } if (fRejectInsaneFee && nFees > CTransaction::minRelayTxFee.GetFee(nSize) * 10000) @@ -971,6 +1000,49 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa return true; } +static void +RelayDoubleSpend(const COutPoint& outPoint, const CTransaction& doubleSpend, bool fInBlock, CBloomFilter& filter) +{ + // Relaying double-spend attempts to our peers lets them detect when + // somebody might be trying to cheat them. However, blindly relaying + // every double-spend across the entire network gives attackers + // a denial-of-service attack: just generate a stream of double-spends + // re-spending the same (limited) set of outpoints owned by the attacker. + // So, we use a bloom filter and only relay (at most) the first double + // spend for each outpoint. False-positives ("we have already relayed") + // are OK, because if the peer doesn't hear about the double-spend + // from us they are very likely to hear about it from another peer, since + // each peer uses a different, randomized bloom filter. + + if (fInBlock || filter.contains(outPoint)) return; + + // Apply an independent rate limit to double-spend relays + static double dRespendCount; + static int64_t nLastRespendTime; + static int64_t nRespendLimit = GetArg("-limitrespendrelay", 100); + unsigned int nSize = ::GetSerializeSize(doubleSpend, SER_NETWORK, PROTOCOL_VERSION); + + if (RateLimitExceeded(dRespendCount, nLastRespendTime, nRespendLimit, nSize)) + { + LogPrint("mempool", "Double-spend relay rejected by rate limiter\n"); + return; + } + + LogPrint("mempool", "Rate limit dRespendCount: %g => %g\n", dRespendCount, dRespendCount+nSize); + + // Clear the filter on average every MAX_DOUBLE_SPEND_BLOOM + // insertions + if (insecure_rand()%MAX_DOUBLESPEND_BLOOM == 0) + filter.clear(); + + filter.insert(outPoint); + + RelayTransaction(doubleSpend); + + // Share conflict with wallet + g_signals.SyncTransaction(doubleSpend, NULL); +} + int CMerkleTx::GetDepthInMainChainINTERNAL(CBlockIndex* &pindexRet) const { diff --git a/src/main.h b/src/main.h index 0332db216..c7f3dc438 100644 --- a/src/main.h +++ b/src/main.h @@ -108,6 +108,9 @@ struct CNodeStateStats; struct CBlockTemplate; +/** Set up internal signal handlers **/ +void RegisterInternalSignals(); + /** Register a wallet to receive updates from core */ void RegisterWallet(CWalletInterface* pwalletIn); /** Unregister a wallet from core */ diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 52f82ef04..97a426dd3 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -415,7 +415,6 @@ void CTxMemPool::remove(const CTransaction &tx, std::list& removed 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); From ada5a067c75f19a724cc054286ecf2254e5dbe8f Mon Sep 17 00:00:00 2001 From: Tom Harding Date: Thu, 26 Jun 2014 18:31:05 -0700 Subject: [PATCH 3/6] UI to alert of respend attempt affecting wallet. Respend transactions that conflict with transactions already in the wallet are added to it. They are not displayed unless they also involve the wallet, or get into a block. If they do not involve the wallet, they continue not to affect balance. Transactions that involve the wallet, and have conflicting non-equivalent transactions, are highlighted in red. When the conflict first occurs, a modal dialog is thrown. CWallet::SyncMetaData is changed to sync only to equivalent transactions. When a conflict is added to the wallet, counter nConflictsReceived is incremented. This acts like a change in active block height for the purpose of triggering UI updates. --- src/qt/guiconstants.h | 4 ++++ src/qt/transactionfilterproxy.cpp | 4 ++-- src/qt/transactionrecord.cpp | 10 +++++++--- src/qt/transactionrecord.h | 13 ++++++++++-- src/qt/transactiontablemodel.cpp | 18 ++++++++++++----- src/qt/walletmodel.cpp | 8 ++++++++ src/ui_interface.h | 3 ++- src/wallet.cpp | 33 +++++++++++++++++++++++++------ src/wallet.h | 15 ++++++++++++-- 9 files changed, 87 insertions(+), 21 deletions(-) diff --git a/src/qt/guiconstants.h b/src/qt/guiconstants.h index 5ae4bc833..44e361e1e 100644 --- a/src/qt/guiconstants.h +++ b/src/qt/guiconstants.h @@ -23,6 +23,10 @@ static const int STATUSBAR_ICONSIZE = 16; #define COLOR_NEGATIVE QColor(255, 0, 0) /* Transaction list -- bare address (without label) */ #define COLOR_BAREADDRESS QColor(140, 140, 140) +/* Transaction list -- has conflicting transactions */ +#define COLOR_HASCONFLICTING Qt::white; +/* Transaction list -- has conflicting transactions - background */ +#define COLOR_HASCONFLICTING_BG QColor(192, 0, 0) /* Tooltips longer than this (in characters) are converted into rich text, so that they can be word-wrapped. diff --git a/src/qt/transactionfilterproxy.cpp b/src/qt/transactionfilterproxy.cpp index f9546fddb..729302978 100644 --- a/src/qt/transactionfilterproxy.cpp +++ b/src/qt/transactionfilterproxy.cpp @@ -24,7 +24,7 @@ TransactionFilterProxy::TransactionFilterProxy(QObject *parent) : typeFilter(ALL_TYPES), minAmount(0), limitRows(-1), - showInactive(true) + showInactive(false) { } @@ -39,7 +39,7 @@ bool TransactionFilterProxy::filterAcceptsRow(int sourceRow, const QModelIndex & qint64 amount = llabs(index.data(TransactionTableModel::AmountRole).toLongLong()); int status = index.data(TransactionTableModel::StatusRole).toInt(); - if(!showInactive && status == TransactionStatus::Conflicted) + if(!showInactive && status == TransactionStatus::Conflicted && type == TransactionRecord::Other) return false; if(!(TYPE(type) & typeFilter)) return false; diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp index eec2b57e8..21f1b7356 100644 --- a/src/qt/transactionrecord.cpp +++ b/src/qt/transactionrecord.cpp @@ -170,6 +170,8 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx) status.depth = wtx.GetDepthInMainChain(); status.cur_num_blocks = chainActive.Height(); + status.hasConflicting = false; + if (!IsFinalTx(wtx, chainActive.Height() + 1)) { if (wtx.nLockTime < LOCKTIME_THRESHOLD) @@ -213,6 +215,7 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx) if (status.depth < 0) { status.status = TransactionStatus::Conflicted; + status.hasConflicting = !(wtx.GetConflicts(false).empty()); } else if (GetAdjustedTime() - wtx.nTimeReceived > 2 * 60 && wtx.GetRequestCount() == 0) { @@ -221,6 +224,7 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx) else if (status.depth == 0) { status.status = TransactionStatus::Unconfirmed; + status.hasConflicting = !(wtx.GetConflicts(false).empty()); } else if (status.depth < RecommendedNumConfirmations) { @@ -231,13 +235,13 @@ void TransactionRecord::updateStatus(const CWalletTx &wtx) status.status = TransactionStatus::Confirmed; } } - } -bool TransactionRecord::statusUpdateNeeded() +bool TransactionRecord::statusUpdateNeeded(int64_t nConflictsReceived) { AssertLockHeld(cs_main); - return status.cur_num_blocks != chainActive.Height(); + return (status.cur_num_blocks != chainActive.Height() || + status.cur_num_conflicts != nConflictsReceived); } QString TransactionRecord::getTxID() const diff --git a/src/qt/transactionrecord.h b/src/qt/transactionrecord.h index af6fd403b..4c2847144 100644 --- a/src/qt/transactionrecord.h +++ b/src/qt/transactionrecord.h @@ -20,7 +20,8 @@ class TransactionStatus public: TransactionStatus(): countsForBalance(false), sortKey(""), - matures_in(0), status(Offline), depth(0), open_for(0), cur_num_blocks(-1) + matures_in(0), status(Offline), hasConflicting(false), depth(0), open_for(0), cur_num_blocks(-1), + cur_num_conflicts(-1) { } enum Status { @@ -51,6 +52,10 @@ public: /** @name Reported status @{*/ Status status; + + // Has conflicting transactions spending same prevout + bool hasConflicting; + qint64 depth; qint64 open_for; /**< Timestamp if status==OpenUntilDate, otherwise number of additional blocks that need to be mined before @@ -59,6 +64,10 @@ public: /** Current number of blocks (to know whether cached status is still valid) */ int cur_num_blocks; + + /** Number of conflicts received into wallet as of last status update */ + int64_t cur_num_conflicts; + }; /** UI model for a transaction. A core transaction can be represented by multiple UI transactions if it has @@ -133,7 +142,7 @@ public: /** Return whether a status update is needed. */ - bool statusUpdateNeeded(); + bool statusUpdateNeeded(int64_t nConflictsReceived); }; #endif // TRANSACTIONRECORD_H diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index b9fcd0d6b..eb5c3abdb 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -168,8 +168,7 @@ public: parent->endRemoveRows(); break; case CT_UPDATED: - // Miscellaneous updates -- nothing to do, status update will take care of this, and is only computed for - // visible transactions. + emit parent->dataChanged(parent->index(lowerIndex, parent->Status), parent->index(upperIndex-1, parent->Amount)); break; } } @@ -190,20 +189,21 @@ public: // stuck if the core is holding the locks for a longer time - for // example, during a wallet rescan. // - // If a status update is needed (blocks came in since last check), - // update the status of this transaction from the wallet. Otherwise, + // If a status update is needed (blocks or conflicts came in since last check), + // update the status of this transaction from the wallet. Otherwise, // simply re-use the cached status. TRY_LOCK(cs_main, lockMain); if(lockMain) { TRY_LOCK(wallet->cs_wallet, lockWallet); - if(lockWallet && rec->statusUpdateNeeded()) + if(lockWallet && rec->statusUpdateNeeded(wallet->nConflictsReceived)) { std::map::iterator mi = wallet->mapWallet.find(rec->hash); if(mi != wallet->mapWallet.end()) { rec->updateStatus(mi->second); + rec->status.cur_num_conflicts = wallet->nConflictsReceived; } } } @@ -363,6 +363,8 @@ QString TransactionTableModel::formatTxType(const TransactionRecord *wtx) const return tr("Payment to yourself"); case TransactionRecord::Generated: return tr("Mined"); + case TransactionRecord::Other: + return tr("Other"); default: return QString(); } @@ -535,7 +537,13 @@ QVariant TransactionTableModel::data(const QModelIndex &index, int role) const return formatTooltip(rec); case Qt::TextAlignmentRole: return column_alignments[index.column()]; + case Qt::BackgroundColorRole: + if (rec->status.hasConflicting) + return COLOR_HASCONFLICTING_BG; + break; case Qt::ForegroundRole: + if (rec->status.hasConflicting) + return COLOR_HASCONFLICTING; // Non-confirmed (but not immature) as transactions are grey if(!rec->status.countsForBalance && rec->status.status != TransactionStatus::Immature) { diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 2f633a26c..3d6f2ab9b 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -138,6 +138,14 @@ void WalletModel::checkBalanceChanged() void WalletModel::updateTransaction(const QString &hash, int status) { + if (status == CT_GOT_CONFLICT) + { + emit message(tr("Conflict Received"), + tr("WARNING: Transaction may never be confirmed. Its input was seen being spent by another transaction on the network. Wait for confirmation!"), + CClientUIInterface::MSG_WARNING); + return; + } + if(transactionTableModel) transactionTableModel->updateTransaction(hash, status); diff --git a/src/ui_interface.h b/src/ui_interface.h index e1a3e04e1..e9fcd91d4 100644 --- a/src/ui_interface.h +++ b/src/ui_interface.h @@ -21,7 +21,8 @@ enum ChangeType { CT_NEW, CT_UPDATED, - CT_DELETED + CT_DELETED, + CT_GOT_CONFLICT }; /** Signals for UI communication. */ diff --git a/src/wallet.cpp b/src/wallet.cpp index f6fd6e958..79cc5ba3c 100644 --- a/src/wallet.cpp +++ b/src/wallet.cpp @@ -256,7 +256,7 @@ bool CWallet::SetMaxVersion(int nVersion) return true; } -set CWallet::GetConflicts(const uint256& txid) const +set CWallet::GetConflicts(const uint256& txid, bool includeEquivalent) const { set result; AssertLockHeld(cs_wallet); @@ -274,7 +274,8 @@ set CWallet::GetConflicts(const uint256& txid) const 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); + if (includeEquivalent || !wtx.IsEquivalentTo(mapWallet.at(it->second))) + result.insert(it->second); } return result; } @@ -303,6 +304,7 @@ void CWallet::SyncMetaData(pair range) const uint256& hash = it->second; CWalletTx* copyTo = &mapWallet[hash]; if (copyFrom == copyTo) continue; + if (!copyFrom->IsEquivalentTo(*copyTo)) continue; copyTo->mapValue = copyFrom->mapValue; copyTo->vOrderForm = copyFrom->vOrderForm; // fTimeReceivedIsTxTime not copied on purpose @@ -588,6 +590,20 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet) // Notify UI of new or updated transaction NotifyTransactionChanged(this, hash, fInsertedNew ? CT_NEW : CT_UPDATED); + // Notifications for existing transactions that now have conflicts with this one + if (fInsertedNew) + { + BOOST_FOREACH(const uint256& conflictHash, wtxIn.GetConflicts(false)) + { + CWalletTx& txConflict = mapWallet[conflictHash]; + NotifyTransactionChanged(this, conflictHash, CT_UPDATED); //Updates UI table + if (IsFromMe(txConflict) || IsMine(txConflict)) + { + NotifyTransactionChanged(this, conflictHash, CT_GOT_CONFLICT); //Throws dialog + } + } + } + // notify an external script when a wallet transaction comes in or is updated std::string strCmd = GetArg("-walletnotify", ""); @@ -610,7 +626,12 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pbl AssertLockHeld(cs_wallet); bool fExisted = mapWallet.count(tx.GetHash()); if (fExisted && !fUpdate) return false; - if (fExisted || IsMine(tx) || IsFromMe(tx)) + + bool fIsConflicting = IsConflicting(tx); + if (fIsConflicting) + nConflictsReceived++; + + if (fExisted || IsMine(tx) || IsFromMe(tx) || fIsConflicting) { CWalletTx wtx(this,tx); // Get merkle branch if transaction was found in a block @@ -896,7 +917,7 @@ void CWallet::ReacceptWalletTransactions() int nDepth = wtx.GetDepthInMainChain(); - if (!wtx.IsCoinBase() && nDepth < 0) + if (!wtx.IsCoinBase() && nDepth < 0 && (IsMine(wtx) || IsFromMe(wtx))) { // Try to add to memory pool LOCK(mempool.cs); @@ -916,13 +937,13 @@ void CWalletTx::RelayWalletTransaction() } } -set CWalletTx::GetConflicts() const +set CWalletTx::GetConflicts(bool includeEquivalent) const { set result; if (pwallet != NULL) { uint256 myHash = GetHash(); - result = pwallet->GetConflicts(myHash); + result = pwallet->GetConflicts(myHash, includeEquivalent); result.erase(myHash); } return result; diff --git a/src/wallet.h b/src/wallet.h index 8494ce9a3..1c2512d67 100644 --- a/src/wallet.h +++ b/src/wallet.h @@ -141,6 +141,9 @@ public: MasterKeyMap mapMasterKeys; unsigned int nMasterKeyMaxID; + // Increment to cause UI refresh, similar to new block + int64_t nConflictsReceived; + CWallet() { SetNull(); @@ -163,6 +166,7 @@ public: nNextResend = 0; nLastResend = 0; nTimeFirstKey = 0; + nConflictsReceived = 0; } std::map mapWallet; @@ -305,6 +309,13 @@ public: { return (GetDebit(tx) > 0); } + bool IsConflicting(const CTransaction& tx) const + { + BOOST_FOREACH(const CTxIn& txin, tx.vin) + if (mapTxSpends.count(txin.prevout)) + return true; + return false; + } int64_t GetDebit(const CTransaction& tx) const { int64_t nDebit = 0; @@ -377,7 +388,7 @@ public: int GetVersion() { LOCK(cs_wallet); return nWalletVersion; } // Get wallet transactions that conflict with given transaction (spend same outputs) - std::set GetConflicts(const uint256& txid) const; + std::set GetConflicts(const uint256& txid, bool includeEquivalent) const; /** Address book entry changed. * @note called with lock cs_wallet held. @@ -699,7 +710,7 @@ public: void RelayWalletTransaction(); - std::set GetConflicts() const; + std::set GetConflicts(bool includeEquivalent=true) const; }; From 9004798e62e987ddf50030b17fa1881b63dd5e45 Mon Sep 17 00:00:00 2001 From: Tom Harding Date: Thu, 26 Jun 2014 18:31:40 -0700 Subject: [PATCH 4/6] Add -respendnotify option and new RPC data -respendnotify= Execute command when a network tx respends wallet tx input (%s=respend TxID, %t=wallet TxID) Add respendsobserved array to gettransaction, listtransactions, and listsinceblock RPCs. This omits the malleated clones that are included in the walletconflicts array. Add RPC help for respendsobserved and walletconflicts (help was missing for the latter). --- contrib/debian/manpages/bitcoin-qt.1 | 3 +++ src/init.cpp | 1 + src/rpcwallet.cpp | 22 ++++++++++++++++++++++ src/wallet.cpp | 8 ++++++++ 4 files changed, 34 insertions(+) diff --git a/contrib/debian/manpages/bitcoin-qt.1 b/contrib/debian/manpages/bitcoin-qt.1 index cd478b187..25a423f9c 100644 --- a/contrib/debian/manpages/bitcoin-qt.1 +++ b/contrib/debian/manpages/bitcoin-qt.1 @@ -139,6 +139,9 @@ Execute command when the best block changes (%s in cmd is replaced by block hash \fB\-walletnotify=\fR Execute command when a wallet transaction changes (%s in cmd is replaced by TxID) .TP +\fB\-respendnotify=\fR +Execute command when a network tx respends wallet tx input (%s=respend TxID, %t=wallet TxID) +.TP \fB\-alertnotify=\fR Execute command when a relevant alert is received (%s in cmd is replaced by message) .TP diff --git a/src/init.cpp b/src/init.cpp index bd732753e..3d0c03328 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -260,6 +260,7 @@ std::string HelpMessage(HelpMessageMode mode) strUsage += " -upgradewallet " + _("Upgrade wallet to latest format") + " " + _("on startup") + "\n"; strUsage += " -wallet= " + _("Specify wallet file (within data directory)") + " " + _("(default: wallet.dat)") + "\n"; strUsage += " -walletnotify= " + _("Execute command when a wallet transaction changes (%s in cmd is replaced by TxID)") + "\n"; + strUsage += " -respendnotify= " + _("Execute command when a network tx respends wallet tx input (%s=respend TxID, %t=wallet TxID)") + "\n"; strUsage += " -zapwallettxes= " + _("Delete all wallet transactions and only recover those part of the blockchain through -rescan on startup") + "\n"; strUsage += " " + _("(default: 1, 1 = keep tx meta data e.g. account owner and payment request information, 2 = drop tx meta data)") + "\n"; #endif diff --git a/src/rpcwallet.cpp b/src/rpcwallet.cpp index 4f27cef08..38e96133b 100644 --- a/src/rpcwallet.cpp +++ b/src/rpcwallet.cpp @@ -58,6 +58,10 @@ void WalletTxToJSON(const CWalletTx& wtx, Object& entry) BOOST_FOREACH(const uint256& conflict, wtx.GetConflicts()) conflicts.push_back(conflict.GetHex()); entry.push_back(Pair("walletconflicts", conflicts)); + Array respends; + BOOST_FOREACH(const uint256& respend, wtx.GetConflicts(false)) + respends.push_back(respend.GetHex()); + entry.push_back(Pair("respendsobserved", respends)); entry.push_back(Pair("time", wtx.GetTxTime())); entry.push_back(Pair("timereceived", (int64_t)wtx.nTimeReceived)); BOOST_FOREACH(const PAIRTYPE(string,string)& item, wtx.mapValue) @@ -1211,6 +1215,12 @@ Value listtransactions(const Array& params, bool fHelp) " \"blockindex\": n, (numeric) The block index containing the transaction. Available for 'send' and 'receive'\n" " category of transactions.\n" " \"txid\": \"transactionid\", (string) The transaction id. Available for 'send' and 'receive' category of transactions.\n" + " \"walletconflicts\" : [\n" + " \"conflictid\", (string) Ids of transactions, including equivalent clones, that re-spend a txid input.\n" + " ],\n" + " \"respendsobserved\" : [\n" + " \"respendid\", (string) Ids of transactions, NOT equivalent clones, that re-spend a txid input. \"Double-spends.\"\n" + " ],\n" " \"time\": xxx, (numeric) The transaction time in seconds since epoch (midnight Jan 1 1970 GMT).\n" " \"timereceived\": xxx, (numeric) The time received in seconds since epoch (midnight Jan 1 1970 GMT). Available \n" " for 'send' and 'receive' category of transactions.\n" @@ -1376,6 +1386,12 @@ Value listsinceblock(const Array& params, bool fHelp) " \"blockindex\": n, (numeric) The block index containing the transaction. Available for 'send' and 'receive' category of transactions.\n" " \"blocktime\": xxx, (numeric) The block time in seconds since epoch (1 Jan 1970 GMT).\n" " \"txid\": \"transactionid\", (string) The transaction id. Available for 'send' and 'receive' category of transactions.\n" + " \"walletconflicts\" : [\n" + " \"conflictid\", (string) Ids of transactions, including equivalent clones, that re-spend a txid input.\n" + " ],\n" + " \"respendsobserved\" : [\n" + " \"respendid\", (string) Ids of transactions, NOT equivalent clones, that re-spend a txid input. \"Double-spends.\"\n" + " ],\n" " \"time\": xxx, (numeric) The transaction time in seconds since epoch (Jan 1 1970 GMT).\n" " \"timereceived\": xxx, (numeric) The time received in seconds since epoch (Jan 1 1970 GMT). Available for 'send' and 'receive' category of transactions.\n" " \"comment\": \"...\", (string) If a comment is associated with the transaction.\n" @@ -1448,6 +1464,12 @@ Value gettransaction(const Array& params, bool fHelp) " \"blockindex\" : xx, (numeric) The block index\n" " \"blocktime\" : ttt, (numeric) The time in seconds since epoch (1 Jan 1970 GMT)\n" " \"txid\" : \"transactionid\", (string) The transaction id.\n" + " \"walletconflicts\" : [\n" + " \"conflictid\", (string) Ids of transactions, including equivalent clones, that re-spend a txid input.\n" + " ],\n" + " \"respendsobserved\" : [\n" + " \"respendid\", (string) Ids of transactions, NOT equivalent clones, that re-spend a txid input. \"Double-spends.\"\n" + " ],\n" " \"time\" : ttt, (numeric) The transaction time in seconds since epoch (1 Jan 1970 GMT)\n" " \"timereceived\" : ttt, (numeric) The time received in seconds since epoch (1 Jan 1970 GMT)\n" " \"details\" : [\n" diff --git a/src/wallet.cpp b/src/wallet.cpp index 79cc5ba3c..91910c6ea 100644 --- a/src/wallet.cpp +++ b/src/wallet.cpp @@ -600,6 +600,14 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet) if (IsFromMe(txConflict) || IsMine(txConflict)) { NotifyTransactionChanged(this, conflictHash, CT_GOT_CONFLICT); //Throws dialog + // external respend notify + std::string strCmd = GetArg("-respendnotify", ""); + if (!strCmd.empty()) + { + boost::replace_all(strCmd, "%s", wtxIn.GetHash().GetHex()); + boost::replace_all(strCmd, "%t", conflictHash.GetHex()); + boost::thread t(runCommand, strCmd); // thread runs free + } } } } From 9fa53dd3bdc6f62b16a7c2b970449c8c35f4c41b Mon Sep 17 00:00:00 2001 From: Tom Harding Date: Fri, 27 Jun 2014 07:49:27 -0700 Subject: [PATCH 5/6] Add release notes entry --- doc/release-notes.md | 46 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/doc/release-notes.md b/doc/release-notes.md index 9272d427c..e68e12310 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -19,3 +19,49 @@ estimate. Statistics used to estimate fees and priorities are saved in the data directory in the 'fee_estimates.dat' file just before program shutdown, and are read in at startup. + +Double-Spend Relay and Alerts +============================= +VERY IMPORTANT: *It has never been safe, and remains unsafe, to rely* +*on unconfirmed transactions.* + +Relay +----- +When an attempt is seen on the network to spend the same unspent funds +more than once, it is no longer ignored. Instead, it is broadcast, to +serve as an alert. This broadcast is subject to protections against +denial-of-service attacks. + +Wallets and other bitcoin services should alert their users to +double-spends that affect them. Merchants and other users may have +enough time to withhold goods or services when payment becomes +uncertain, until confirmation. + +Bitcoin Core Wallet Alerts +-------------------------- +The Bitcoin Core wallet now makes respend attempts visible in several +ways. + +If you are online, and a respend affecting one of your wallet +transactions is seen, a notification is immediately issued to the +command registered with `-respendnotify=`. Additionally, if +using the GUI: + - An alert box is immediately displayed. + - The affected wallet transaction is highlighted in red until it is + confirmed (and it may never be confirmed). + +A `respendsobserved` array is added to `gettransaction`, `listtransactions`, +and `listsinceblock` RPC results. + +Warning +------- +*If you rely on an unconfirmed transaction, these change do VERY* +*LITTLE to protect you from a malicious double-spend, because:* + + - You may learn about the respend too late to avoid doing whatever + you were being paid for + - Using other relay rules, a double-spender can craft his crime to + resist broadcast + - Miners can choose which conflicting spend to confirm, and some + miners may not confirmg the first acceptable spend they see + From 7a19efe04069d9a1e251cdc94b25184f76d9d901 Mon Sep 17 00:00:00 2001 From: Tom Harding Date: Fri, 27 Jun 2014 16:47:33 -0700 Subject: [PATCH 6/6] Formatting, spelling, comment fixes. --- doc/release-notes.md | 2 +- src/main.cpp | 6 ++---- src/qt/transactionrecord.h | 13 ++++++++++--- src/qt/transactiontablemodel.cpp | 4 ++-- src/qt/walletmodel.cpp | 14 +++++++------- src/wallet.cpp | 2 +- 6 files changed, 23 insertions(+), 18 deletions(-) diff --git a/doc/release-notes.md b/doc/release-notes.md index e68e12310..3a4079e43 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -63,5 +63,5 @@ Warning - Using other relay rules, a double-spender can craft his crime to resist broadcast - Miners can choose which conflicting spend to confirm, and some - miners may not confirmg the first acceptable spend they see + miners may not confirm the first acceptable spend they see diff --git a/src/main.cpp b/src/main.cpp index 48237c1b5..df810f575 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -852,7 +852,6 @@ bool RateLimitExceeded(double& dCount, int64_t& nLastTime, int64_t nLimit, unsig LOCK(csLimiter); - // Use an exponentially decaying ~10-minute window: dCount *= pow(1.0 - 1.0/600.0, (double)(nNow - nLastTime)); nLastTime = nNow; if (dCount >= nLimit*10*1000) @@ -973,7 +972,7 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa static int64_t nLastFreeTime; static int64_t nFreeLimit = GetArg("-limitfreerelay", 15); - if (RateLimitExceeded(dFreeCount, nLastFreeTime, nFreeLimit, nSize)) + if (RateLimitExceeded(dFreeCount, nLastFreeTime, nFreeLimit, nSize)) return state.DoS(0, error("AcceptToMemoryPool : free transaction rejected by rate limiter"), REJECT_INSUFFICIENTFEE, "insufficient priority"); @@ -1000,8 +999,7 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa return true; } -static void -RelayDoubleSpend(const COutPoint& outPoint, const CTransaction& doubleSpend, bool fInBlock, CBloomFilter& filter) +static void RelayDoubleSpend(const COutPoint& outPoint, const CTransaction& doubleSpend, bool fInBlock, CBloomFilter& filter) { // Relaying double-spend attempts to our peers lets them detect when // somebody might be trying to cheat them. However, blindly relaying diff --git a/src/qt/transactionrecord.h b/src/qt/transactionrecord.h index 4c2847144..37679cebf 100644 --- a/src/qt/transactionrecord.h +++ b/src/qt/transactionrecord.h @@ -19,10 +19,17 @@ class TransactionStatus { public: TransactionStatus(): - countsForBalance(false), sortKey(""), - matures_in(0), status(Offline), hasConflicting(false), depth(0), open_for(0), cur_num_blocks(-1), + countsForBalance(false), + sortKey(""), + matures_in(0), + status(Offline), + hasConflicting(false), + depth(0), + open_for(0), + cur_num_blocks(-1), cur_num_conflicts(-1) - { } + { + } enum Status { Confirmed, /**< Have 6 or more confirmations (normal tx) or fully mature (mined tx) **/ diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index eb5c3abdb..d7f4c043c 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -538,9 +538,9 @@ QVariant TransactionTableModel::data(const QModelIndex &index, int role) const case Qt::TextAlignmentRole: return column_alignments[index.column()]; case Qt::BackgroundColorRole: - if (rec->status.hasConflicting) + if (rec->status.hasConflicting) return COLOR_HASCONFLICTING_BG; - break; + break; case Qt::ForegroundRole: if (rec->status.hasConflicting) return COLOR_HASCONFLICTING; diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 3d6f2ab9b..defc815de 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -138,13 +138,13 @@ void WalletModel::checkBalanceChanged() void WalletModel::updateTransaction(const QString &hash, int status) { - if (status == CT_GOT_CONFLICT) - { - emit message(tr("Conflict Received"), - tr("WARNING: Transaction may never be confirmed. Its input was seen being spent by another transaction on the network. Wait for confirmation!"), - CClientUIInterface::MSG_WARNING); - return; - } + if (status == CT_GOT_CONFLICT) + { + emit message(tr("Conflict Received"), + tr("WARNING: Transaction may never be confirmed. Its input was seen being spent by another transaction on the network. Wait for confirmation!"), + CClientUIInterface::MSG_WARNING); + return; + } if(transactionTableModel) transactionTableModel->updateTransaction(hash, status); diff --git a/src/wallet.cpp b/src/wallet.cpp index 91910c6ea..daca7ac04 100644 --- a/src/wallet.cpp +++ b/src/wallet.cpp @@ -637,7 +637,7 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pbl bool fIsConflicting = IsConflicting(tx); if (fIsConflicting) - nConflictsReceived++; + nConflictsReceived++; if (fExisted || IsMine(tx) || IsFromMe(tx) || fIsConflicting) {