Merge pull request #4558

8f3f94a Revert "CBloomFilter::clear() method" (Wladimir J. van der Laan)
98e84aa Revert "Relay double-spends, subject to anti-DOS" (Wladimir J. van der Laan)
3015e0b Revert "UI to alert of respend attempt affecting wallet." (Wladimir J. van der Laan)
39d3f2c Revert "Add -respendnotify option and new RPC data" (Wladimir J. van der Laan)
680f725 Revert "Add release notes entry" (Wladimir J. van der Laan)
ad26dc9 Revert "Formatting, spelling, comment fixes." (Wladimir J. van der Laan)
cd057bf Revert "Check signatures before respend relay" (Wladimir J. van der Laan)
67cc8f2 Revert "Remove signal DoubleSpendDetected, use function" (Wladimir J. van der Laan)

Author: laanwj

contrib/debian/manpages/bitcoin-qt.1

@@ -139,9 +139,6 @@ Execute command when the best block changes (%s in cmd is replaced by block hash
 \fB\-walletnotify=\fR<cmd>
 Execute command when a wallet transaction changes (%s in cmd is replaced by TxID)
 .TP
-\fB\-respendnotify=\fR<cmd>
-Execute command when a network tx respends wallet tx input (%s=respend TxID, %t=wallet TxID)
-.TP
 \fB\-alertnotify=\fR<cmd>
 Execute command when a relevant alert is received (%s in cmd is replaced by message)
 .TP

doc/release-notes.md

@@ -39,49 +39,3 @@ 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=<cmd>`.  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 confirm the first acceptable spend they see
-

src/bloom.cpp

@@ -94,13 +94,6 @@ 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;

src/bloom.h

@@ -78,8 +78,6 @@ class CBloomFilter
     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;

src/core.cpp

@@ -124,22 +124,6 @@ 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;

src/core.h

@@ -255,9 +255,6 @@ class CTransaction
         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

src/init.cpp

@@ -1229,7 +1229,6 @@ bool AppInit2(boost::thread_group& threadGroup)
     LogPrintf("mapAddressBook.size() = %u\n",  pwalletMain ? pwalletMain->mapAddressBook.size() : 0);
 #endif
 
-    InitRespendFilter();
     StartNode(threadGroup);
     if (fServer)
         StartRPCThreads();

src/main.cpp

@@ -7,7 +7,6 @@
 
 #include "addrman.h"
 #include "alert.h"
-#include "bloom.h"
 #include "chainparams.h"
 #include "checkpoints.h"
 #include "checkqueue.h"
@@ -126,15 +125,6 @@ namespace {
 
 } // anon namespace
 
-// Bloom filter to limit respend relays to one
-static const unsigned int MAX_DOUBLESPEND_BLOOM = 1000;
-static CBloomFilter doubleSpendFilter;
-void InitRespendFilter() {
-    seed_insecure_rand();
-    doubleSpendFilter = CBloomFilter(MAX_DOUBLESPEND_BLOOM, 0.01, insecure_rand(), BLOOM_UPDATE_NONE);
-}
-
-
 //////////////////////////////////////////////////////////////////////////////
 //
 // dispatching functions
@@ -161,7 +151,6 @@ struct CMainSignals {
 
 } // anon namespace
 
-
 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));
@@ -881,60 +870,6 @@ int64_t GetMinRelayFee(const CTransaction& tx, unsigned int nBytes, bool fAllowF
     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);
-
-    dCount *= pow(1.0 - 1.0/600.0, (double)(nNow - nLastTime));
-    nLastTime = nNow;
-    if (dCount >= nLimit*10*1000)
-        return true;
-    dCount += nSize;
-    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)
@@ -964,18 +899,15 @@ 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++)
     {
         COutPoint outpoint = tx.vin[i].prevout;
-        // 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))
+        if (pool.mapNextTx.count(outpoint))
         {
-            relayableRespend = RelayableRespend(outpoint, tx, false, doubleSpendFilter);
-            if (!relayableRespend)
-                return false;
+            // Disable replacement feature for now
+            return false;
         }
     }
     }
@@ -1046,15 +978,23 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa
         // be annoying or make others' transactions take longer to confirm.
         if (fLimitFree && nFees < ::minRelayTxFee.GetFee(nSize))
         {
+            static CCriticalSection csFreeLimiter;
             static double dFreeCount;
-            static int64_t nLastFreeTime;
-            static int64_t nFreeLimit = GetArg("-limitfreerelay", 15);
+            static int64_t nLastTime;
+            int64_t nNow = GetTime();
 
-            if (RateLimitExceeded(dFreeCount, nLastFreeTime, nFreeLimit, nSize))
+            LOCK(csFreeLimiter);
+
+            // 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)
                 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 > ::minRelayTxFee.GetFee(nSize) * 10000)
@@ -1068,21 +1008,13 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa
         {
             return error("AcceptToMemoryPool: : ConnectInputs failed %s", hash.ToString());
         }
-
-        if (relayableRespend)
-        {
-            RelayTransaction(tx);
-        }
-        else
-        {
-            // Store transaction in memory
-            pool.addUnchecked(hash, entry);
-        }
+        // Store transaction in memory
+        pool.addUnchecked(hash, entry);
     }
 
     g_signals.SyncTransaction(tx, NULL);
 
-    return !relayableRespend;
+    return true;
 }
 
 

src/main.h

@@ -112,9 +112,6 @@ struct CNodeStateStats;
 
 struct CBlockTemplate;
 
-/** Initialize respend bloom filter **/
-void InitRespendFilter();
-
 /** Register a wallet to receive updates from core */
 void RegisterWallet(CWalletInterface* pwalletIn);
 /** Unregister a wallet from core */

src/qt/guiconstants.h

@@ -23,10 +23,6 @@ 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 QColor(255, 255, 255)
-/* 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.