From 88dd3598d22197a22565e524cecdc08107cf76ac Mon Sep 17 00:00:00 2001 From: Tom Harding Date: Tue, 1 Jul 2014 14:26:57 -0700 Subject: [PATCH 1/2] Check signatures before respend relay Check that all inputs are completely valid before actually relaying a double-spend. --- src/main.cpp | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 04d9523e26e..1294e5b2ed8 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -127,7 +127,7 @@ namespace { // 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); +static bool RelayableRespend(const COutPoint& outPoint, const CTransaction& doubleSpend, bool fInBlock, CBloomFilter& filter); ////////////////////////////////////////////////////////////////////////////// // @@ -156,7 +156,7 @@ struct CMainSignals { // 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; + boost::signals2::signal DetectedDoubleSpend; } g_signals; } // anon namespace @@ -166,7 +166,7 @@ void RegisterInternalSignals() { 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)); + g_signals.DetectedDoubleSpend.connect(boost::bind(RelayableRespend, _1, _2, _3, doubleSpendFilter)); } @@ -936,6 +936,7 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa return false; // Check for conflicts with in-memory transactions + bool relayableRespend = false; { LOCK(pool.cs); // protect pool.mapNextTx for (unsigned int i = 0; i < tx.vin.size(); i++) @@ -944,8 +945,9 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa // 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)) { - g_signals.DetectedDoubleSpend(outpoint, tx, false); - return false; + relayableRespend = g_signals.DetectedDoubleSpend(outpoint, tx, false); + if (!relayableRespend) + return false; } } } @@ -1038,16 +1040,24 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa { return error("AcceptToMemoryPool: : ConnectInputs failed %s", hash.ToString()); } - // Store transaction in memory - pool.addUnchecked(hash, entry); + + if (relayableRespend) + { + RelayTransaction(tx); + } + else + { + // Store transaction in memory + pool.addUnchecked(hash, entry); + } } g_signals.SyncTransaction(tx, NULL); - return true; + return !relayableRespend; } -static void RelayDoubleSpend(const COutPoint& outPoint, const CTransaction& doubleSpend, bool fInBlock, CBloomFilter& filter) +static bool RelayableRespend(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 @@ -1060,7 +1070,7 @@ static void RelayDoubleSpend(const COutPoint& outPoint, const CTransaction& doub // 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; + if (fInBlock || filter.contains(outPoint)) return false; // Apply an independent rate limit to double-spend relays static double dRespendCount; @@ -1071,7 +1081,7 @@ static void RelayDoubleSpend(const COutPoint& outPoint, const CTransaction& doub if (RateLimitExceeded(dRespendCount, nLastRespendTime, nRespendLimit, nSize)) { LogPrint("mempool", "Double-spend relay rejected by rate limiter\n"); - return; + return false; } LogPrint("mempool", "Rate limit dRespendCount: %g => %g\n", dRespendCount, dRespendCount+nSize); @@ -1083,10 +1093,7 @@ static void RelayDoubleSpend(const COutPoint& outPoint, const CTransaction& doub filter.insert(outPoint); - RelayTransaction(doubleSpend); - - // Share conflict with wallet - g_signals.SyncTransaction(doubleSpend, NULL); + return true; } From 0da6b3fd187da3aa810aaa584d8bd197ad4fa2b9 Mon Sep 17 00:00:00 2001 From: Tom Harding Date: Wed, 2 Jul 2014 09:27:29 -0700 Subject: [PATCH 2/2] Remove signal DoubleSpendDetected, use function Also removes the need for forward reference to RelayableRespend. --- src/init.cpp | 2 +- src/main.cpp | 103 +++++++++++++++++++++++---------------------------- src/main.h | 4 +- 3 files changed, 50 insertions(+), 59 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index da13218a940..6c0898c036d 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1176,7 +1176,7 @@ bool AppInit2(boost::thread_group& threadGroup) LogPrintf("mapAddressBook.size() = %u\n", pwalletMain ? pwalletMain->mapAddressBook.size() : 0); #endif - RegisterInternalSignals(); + InitRespendFilter(); StartNode(threadGroup); if (fServer) StartRPCThreads(); diff --git a/src/main.cpp b/src/main.cpp index 1294e5b2ed8..7baf33c39f9 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -125,9 +125,14 @@ namespace { } // anon namespace -// Forward reference functions defined here: +// Bloom filter to limit respend relays to one static const unsigned int MAX_DOUBLESPEND_BLOOM = 1000; -static bool RelayableRespend(const COutPoint& outPoint, const CTransaction& doubleSpend, bool fInBlock, CBloomFilter& filter); +static CBloomFilter doubleSpendFilter; +void InitRespendFilter() { + seed_insecure_rand(); + doubleSpendFilter = CBloomFilter(MAX_DOUBLESPEND_BLOOM, 0.01, insecure_rand(), BLOOM_UPDATE_NONE); +} + ////////////////////////////////////////////////////////////////////////////// // @@ -151,24 +156,10 @@ 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; } // anon namespace -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(RelayableRespend, _1, _2, _3, doubleSpendFilter)); -} - void RegisterWallet(CWalletInterface* pwalletIn) { g_signals.SyncTransaction.connect(boost::bind(&CWalletInterface::SyncTransaction, pwalletIn, _1, _2)); @@ -908,6 +899,45 @@ bool RateLimitExceeded(double& dCount, int64_t& nLastTime, int64_t nLimit, unsig return false; } +static bool RelayableRespend(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 false; + + // 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 false; + } + + 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); + + return true; +} + bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransaction &tx, bool fLimitFree, bool* pfMissingInputs, bool fRejectInsaneFee) { @@ -945,7 +975,7 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa // 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)) { - relayableRespend = g_signals.DetectedDoubleSpend(outpoint, tx, false); + relayableRespend = RelayableRespend(outpoint, tx, false, doubleSpendFilter); if (!relayableRespend) return false; } @@ -1057,45 +1087,6 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa return !relayableRespend; } -static bool RelayableRespend(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 false; - - // 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 false; - } - - 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); - - return true; -} - int CMerkleTx::GetDepthInMainChainINTERNAL(CBlockIndex* &pindexRet) const { diff --git a/src/main.h b/src/main.h index 19f44690080..26450bb7212 100644 --- a/src/main.h +++ b/src/main.h @@ -108,8 +108,8 @@ struct CNodeStateStats; struct CBlockTemplate; -/** Set up internal signal handlers **/ -void RegisterInternalSignals(); +/** Initialize respend bloom filter **/ +void InitRespendFilter(); /** Register a wallet to receive updates from core */ void RegisterWallet(CWalletInterface* pwalletIn);