Revert "Merge #7542: Implement "feefilter" P2P message" (#2025)

This reverts commit 11ac70af9e.

doc/bips.md

@@ -26,6 +26,5 @@ BIPs that are implemented by Bitcoin Core (up-to-date up to **v0.13.0**):
 * [`BIP 113`](https://github.com/bitcoin/bips/blob/master/bip-0113.mediawiki): Median time past lock-time calculations have been implemented since **v0.12.1** ([PR #6566](https://github.com/bitcoin/bitcoin/pull/6566)) and have been activated since *block 419328*.
 * [`BIP 125`](https://github.com/bitcoin/bips/blob/master/bip-0125.mediawiki): Opt-in full replace-by-fee signaling honoured in mempool and mining as of **v0.12.0** ([PR 6871](https://github.com/bitcoin/bitcoin/pull/6871)).
 * [`BIP 130`](https://github.com/bitcoin/bips/blob/master/bip-0130.mediawiki): direct headers announcement is negotiated with peer versions `>=70012` as of **v0.12.0** ([PR 6494](https://github.com/bitcoin/bitcoin/pull/6494)).
-* [`BIP 133`](https://github.com/bitcoin/bips/blob/master/bip-0133.mediawiki): feefilter messages are respected and sent for peer versions `>=70013` as of **v0.13.0** ([PR 7542](https://github.com/bitcoin/bitcoin/pull/7542)).
 * [`BIP 147`](https://github.com/bitcoin/bips/blob/master/bip-0147.mediawiki): NULLDUMMY softfork as of **v0.13.1** ([PR 8636](https://github.com/bitcoin/bitcoin/pull/8636) and [PR 8937](https://github.com/bitcoin/bitcoin/pull/8937)).
 * [`BIP 152`](https://github.com/bitcoin/bips/blob/master/bip-0152.mediawiki): Compact block transfer and related optimizations are used as of **v0.13.0** ([PR 8068](https://github.com/bitcoin/bitcoin/pull/8068)).

qa/pull-tester/rpc-tests.py

@@ -183,7 +183,6 @@ testScriptsExt = [
     'p2p-timeouts.py',
     # vv Tests less than 60s vv
     'bip9-softforks.py',
-    'p2p-feefilter.py',
     'rpcbind_test.py',
     # vv Tests less than 30s vv
     'bip65-cltv.py',

qa/rpc-tests/p2p-feefilter.py

@@ -1,115 +0,0 @@
-#!/usr/bin/env python3
-# Copyright (c) 2016 The Bitcoin Core developers
-# Distributed under the MIT software license, see the accompanying
-# file COPYING or http://www.opensource.org/licenses/mit-license.php.
-#
-
-from test_framework.mininode import *
-from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import *
-import time
-
-'''
-FeeFilterTest -- test processing of feefilter messages
-'''
-
-def hashToHex(hash):
-    return format(hash, '064x')
-
-# Wait up to 60 secs to see if the testnode has received all the expected invs
-def allInvsMatch(invsExpected, testnode):
-    for x in range(60):
-        with mininode_lock:
-            if (sorted(invsExpected) == sorted(testnode.txinvs)):
-                return True
-        time.sleep(1)
-    return False
-
-# TestNode: bare-bones "peer".  Used to track which invs are received from a node
-# and to send the node feefilter messages.
-class TestNode(SingleNodeConnCB):
-    def __init__(self):
-        SingleNodeConnCB.__init__(self)
-        self.txinvs = []
-
-    def on_inv(self, conn, message):
-        for i in message.inv:
-            if (i.type == 1):
-                self.txinvs.append(hashToHex(i.hash))
-
-    def clear_invs(self):
-        with mininode_lock:
-            self.txinvs = []
-
-    def send_filter(self, feerate):
-        self.send_message(msg_feefilter(feerate))
-        self.sync_with_ping()
-
-class FeeFilterTest(BitcoinTestFramework):
-
-    def __init__(self):
-        super().__init__()
-        self.num_nodes = 2
-        self.setup_clean_chain = False
-
-    def setup_network(self):
-        # Node1 will be used to generate txs which should be relayed from Node0
-        # to our test node
-        self.nodes = []
-        self.nodes.append(start_node(0, self.options.tmpdir, ["-debug", "-logtimemicros"]))
-        self.nodes.append(start_node(1, self.options.tmpdir, ["-debug", "-logtimemicros"]))
-        connect_nodes(self.nodes[0], 1)
-
-    def run_test(self):
-        node1 = self.nodes[1]
-        node0 = self.nodes[0]
-        # Get out of IBD
-        node1.generate(1)
-        sync_blocks(self.nodes)
-
-        # Setup the p2p connections and start up the network thread.
-        test_node = TestNode()
-        connection = NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], test_node)
-        test_node.add_connection(connection)
-        NetworkThread().start()
-        test_node.wait_for_verack()
-
-        # Test that invs are received for all txs at feerate of 20 sat/byte
-        node1.settxfee(Decimal("0.00020000"))
-        txids = [node1.sendtoaddress(node1.getnewaddress(), 1) for x in range(3)]
-        assert(allInvsMatch(txids, test_node))
-        test_node.clear_invs()
-
-        # Set a filter of 15 sat/byte
-        test_node.send_filter(15000)
-
-        # Test that txs are still being received (paying 20 sat/byte)
-        txids = [node1.sendtoaddress(node1.getnewaddress(), 1) for x in range(3)]
-        assert(allInvsMatch(txids, test_node))
-        test_node.clear_invs()
-
-        # Change tx fee rate to 10 sat/byte and test they are no longer received
-        node1.settxfee(Decimal("0.00010000"))
-        [node1.sendtoaddress(node1.getnewaddress(), 1) for x in range(3)]
-        sync_mempools(self.nodes) # must be sure node 0 has received all txs 
-
-        # Send one transaction from node0 that should be received, so that we
-        # we can sync the test on receipt (if node1's txs were relayed, they'd
-        # be received by the time this node0 tx is received). This is
-        # unfortunately reliant on the current relay behavior where we batch up
-        # to 35 entries in an inv, which means that when this next transaction
-        # is eligible for relay, the prior transactions from node1 are eligible
-        # as well.
-        node0.settxfee(Decimal("0.00020000"))
-        txids = [node0.sendtoaddress(node0.getnewaddress(), 1)]
-        assert(allInvsMatch(txids, test_node))
-        test_node.clear_invs()
-
-        # Remove fee filter and check that txs are received again
-        test_node.send_filter(0)
-        txids = [node1.sendtoaddress(node1.getnewaddress(), 1) for x in range(3)]
-        assert(allInvsMatch(txids, test_node))
-        test_node.clear_invs()
-
-if __name__ == '__main__':
-    FeeFilterTest().main()

qa/rpc-tests/p2p-leaktests.py

@@ -55,7 +55,6 @@ class CLazyNode(NodeConnCB):
     def on_ping(self, conn, message): self.bad_message(message)
     def on_mempool(self, conn): self.bad_message(message)
     def on_pong(self, conn, message): self.bad_message(message)
-    def on_feefilter(self, conn, message): self.bad_message(message)
     def on_sendheaders(self, conn, message): self.bad_message(message)
     def on_sendcmpct(self, conn, message): self.bad_message(message)
     def on_cmpctblock(self, conn, message): self.bad_message(message)

qa/rpc-tests/test_framework/mininode.py

@@ -1201,23 +1201,6 @@ def wait_until(predicate, *, attempts=float('inf'), timeout=float('inf')):
 
     return False
 
-class msg_feefilter(object):
-    command = b"feefilter"
-
-    def __init__(self, feerate=0):
-        self.feerate = feerate
-
-    def deserialize(self, f):
-        self.feerate = struct.unpack("<Q", f.read(8))[0]
-
-    def serialize(self):
-        r = b""
-        r += struct.pack("<Q", self.feerate)
-        return r
-
-    def __repr__(self):
-        return "msg_feefilter(feerate=%08x)" % self.feerate
-
 class msg_sendcmpct(object):
     command = b"sendcmpct"
 
@@ -1370,7 +1353,6 @@ class NodeConnCB(object):
     def on_close(self, conn): pass
     def on_mempool(self, conn): pass
     def on_pong(self, conn, message): pass
-    def on_feefilter(self, conn, message): pass
     def on_sendheaders(self, conn, message): pass
     def on_sendcmpct(self, conn, message): pass
     def on_cmpctblock(self, conn, message): pass
@@ -1428,7 +1410,6 @@ class NodeConn(asyncore.dispatcher):
         b"getheaders": msg_getheaders,
         b"reject": msg_reject,
         b"mempool": msg_mempool,
-        b"feefilter": msg_feefilter,
         b"sendheaders": msg_sendheaders,
         b"sendcmpct": msg_sendcmpct,
         b"cmpctblock": msg_cmpctblock,

src/init.cpp

@@ -407,8 +407,6 @@ std::string HelpMessage(HelpMessageMode mode)
     }
     strUsage += HelpMessageOpt("-datadir=<dir>", _("Specify data directory"));
     strUsage += HelpMessageOpt("-dbcache=<n>", strprintf(_("Set database cache size in megabytes (%d to %d, default: %d)"), nMinDbCache, nMaxDbCache, nDefaultDbCache));
-    if (showDebug)
-        strUsage += HelpMessageOpt("-feefilter", strprintf("Tell other nodes to filter invs to us by our mempool min fee (default: %u)", DEFAULT_FEEFILTER));
     strUsage += HelpMessageOpt("-loadblock=<file>", _("Imports blocks from external blk000??.dat file on startup"));
     strUsage += HelpMessageOpt("-maxorphantx=<n>", strprintf(_("Keep at most <n> unconnectable transactions in memory (default: %u)"), DEFAULT_MAX_ORPHAN_TRANSACTIONS));
     strUsage += HelpMessageOpt("-maxmempool=<n>", strprintf(_("Keep the transaction memory pool below <n> megabytes (default: %u)"), DEFAULT_MAX_MEMPOOL_SIZE));

src/net.cpp

@@ -2799,9 +2799,6 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn
     fPingQueued = false;
     fMasternode = false;
     nMinPingUsecTime = std::numeric_limits<int64_t>::max();
-    minFeeFilter = 0;
-    lastSentFeeFilter = 0;
-    nextSendTimeFeeFilter = 0;
     fPauseRecv = false;
     fPauseSend = false;
     nProcessQueueSize = 0;

src/net.h

@@ -8,7 +8,6 @@
 
 #include "addrdb.h"
 #include "addrman.h"
-#include "amount.h"
 #include "bloom.h"
 #include "compat.h"
 #include "hash.h"
@@ -798,11 +797,6 @@ public:
     std::atomic<int64_t> nMinPingUsecTime;
     // Whether a ping is requested.
     std::atomic<bool> fPingQueued;
-    // Minimum fee rate with which to filter inv's to this node
-    CAmount minFeeFilter;
-    CCriticalSection cs_feeFilter;
-    CAmount lastSentFeeFilter;
-    int64_t nextSendTimeFeeFilter;
 
     CNode(NodeId id, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn, SOCKET hSocketIn, const CAddress &addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const std::string &addrNameIn = "", bool fInboundIn = false);
     ~CNode();

src/net_processing.cpp

@@ -50,8 +50,6 @@
 
 std::atomic<int64_t> nTimeBestReceived(0); // Used only to inform the wallet of when we last received a block
 
-extern FeeFilterRounder filterRounder;
-
 struct IteratorComparator
 {
     template<typename I>
@@ -2843,17 +2841,6 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
         pfrom->fRelayTxes = true;
     }
 
-    else if (strCommand == NetMsgType::FEEFILTER) {
-        CAmount newFeeFilter = 0;
-        vRecv >> newFeeFilter;
-        if (MoneyRange(newFeeFilter)) {
-            {
-                LOCK(pfrom->cs_feeFilter);
-                pfrom->minFeeFilter = newFeeFilter;
-            }
-            LogPrint("net", "received: feefilter of %s from peer=%d\n", CFeeRate(newFeeFilter).ToString(), pfrom->id);
-        }
-    }
 
     else if (strCommand == NetMsgType::NOTFOUND) {
         // We do not care about the NOTFOUND message, but logging an Unknown Command
@@ -3361,11 +3348,6 @@ bool SendMessages(CNode* pto, CConnman& connman, const std::atomic<bool>& interr
             if (fSendTrickle && pto->fSendMempool) {
                 auto vtxinfo = mempool.infoAll();
                 pto->fSendMempool = false;
-                CAmount filterrate = 0;
-                {
-                    LOCK(pto->cs_feeFilter);
-                    filterrate = pto->minFeeFilter;
-                }
 
                 LOCK(pto->cs_filter);
 
@@ -3373,10 +3355,6 @@ bool SendMessages(CNode* pto, CConnman& connman, const std::atomic<bool>& interr
                     const uint256& hash = txinfo.tx->GetHash();
                     CInv inv(MSG_TX, hash);
                     pto->setInventoryTxToSend.erase(hash);
-                    if (filterrate) {
-                        if (txinfo.feeRate.GetFeePerK() < filterrate)
-                            continue;
-                    }
                     if (pto->pfilter) {
                         if (!pto->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue;
                     }
@@ -3401,11 +3379,6 @@ bool SendMessages(CNode* pto, CConnman& connman, const std::atomic<bool>& interr
                 for (std::set<uint256>::iterator it = pto->setInventoryTxToSend.begin(); it != pto->setInventoryTxToSend.end(); it++) {
                     vInvTx.push_back(it);
                 }
-                CAmount filterrate = 0;
-                {
-                    LOCK(pto->cs_feeFilter);
-                    filterrate = pto->minFeeFilter;
-                }
                 // Topologically and fee-rate sort the inventory we send for privacy and priority reasons.
                 // A heap is used so that not all items need sorting if only a few are being sent.
                 CompareInvMempoolOrder compareInvMempoolOrder(&mempool);
@@ -3431,9 +3404,6 @@ bool SendMessages(CNode* pto, CConnman& connman, const std::atomic<bool>& interr
                     if (!txinfo.tx) {
                         continue;
                     }
-                    if (filterrate && txinfo.feeRate.GetFeePerK() < filterrate) {
-                        continue;
-                    }
                     if (pto->pfilter && !pto->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue;
                     // Send
                     vInv.push_back(CInv(MSG_TX, hash));
@@ -3580,32 +3550,6 @@ bool SendMessages(CNode* pto, CConnman& connman, const std::atomic<bool>& interr
             LogPrint("net", "SendMessages -- GETDATA -- pushed size = %lu peer=%d\n", vGetData.size(), pto->id);
         }
 
-        //
-        // Message: feefilter
-        //
-        // We don't want white listed peers to filter txs to us if we have -whitelistforcerelay
-        if (pto->nVersion >= FEEFILTER_VERSION && GetBoolArg("-feefilter", DEFAULT_FEEFILTER) &&
-            !(pto->fWhitelisted && GetBoolArg("-whitelistforcerelay", DEFAULT_WHITELISTFORCERELAY))) {
-            CAmount currentFilter = mempool.GetMinFee(GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFeePerK();
-            int64_t timeNow = GetTimeMicros();
-            if (timeNow > pto->nextSendTimeFeeFilter) {
-                CAmount filterToSend = filterRounder.round(currentFilter);
-                // If we don't allow free transactions, then we always have a fee filter of at least minRelayTxFee
-                if (GetArg("-limitfreerelay", DEFAULT_LIMITFREERELAY) <= 0)
-                    filterToSend = std::max(filterToSend, ::minRelayTxFee.GetFeePerK());
-                if (filterToSend != pto->lastSentFeeFilter) {
-                    connman.PushMessage(pto, msgMaker.Make(NetMsgType::FEEFILTER, filterToSend));
-                    pto->lastSentFeeFilter = filterToSend;
-                }
-                pto->nextSendTimeFeeFilter = PoissonNextSend(timeNow, AVG_FEEFILTER_BROADCAST_INTERVAL);
-            }
-            // If the fee filter has changed substantially and it's still more than MAX_FEEFILTER_CHANGE_DELAY
-            // until scheduled broadcast, then move the broadcast to within MAX_FEEFILTER_CHANGE_DELAY.
-            else if (timeNow + MAX_FEEFILTER_CHANGE_DELAY * 1000000 < pto->nextSendTimeFeeFilter &&
-                     (currentFilter < 3 * pto->lastSentFeeFilter / 4 || currentFilter > 4 * pto->lastSentFeeFilter / 3)) {
-                pto->nextSendTimeFeeFilter = timeNow + GetRandInt(MAX_FEEFILTER_CHANGE_DELAY) * 1000000;
-            }
-        }
     }
     return true;
 }

src/policy/fees.cpp

@@ -8,7 +8,6 @@
 
 #include "amount.h"
 #include "primitives/transaction.h"
-#include "random.h"
 #include "streams.h"
 #include "txmempool.h"
 #include "util.h"
@@ -487,20 +486,3 @@ void CBlockPolicyEstimator::Read(CAutoFile& filein, int nFileVersion)
     }
 }
 
-FeeFilterRounder::FeeFilterRounder(const CFeeRate& minIncrementalFee)
-{
-    CAmount minFeeLimit = std::max(CAmount(1), minIncrementalFee.GetFeePerK() / 2);
-    feeset.insert(0);
-    for (double bucketBoundary = minFeeLimit; bucketBoundary <= MAX_FEERATE; bucketBoundary *= FEE_SPACING) {
-        feeset.insert(bucketBoundary);
-    }
-}
-
-CAmount FeeFilterRounder::round(CAmount currentMinFee)
-{
-    std::set<double>::iterator it = feeset.lower_bound(currentMinFee);
-    if ((it != feeset.begin() && insecure_rand.rand32() % 3 != 0) || it == feeset.end()) {
-        it--;
-    }
-    return *it;
-}

src/policy/fees.h

@@ -262,18 +262,4 @@ private:
     unsigned int trackedTxs;
     unsigned int untrackedTxs;
 };
-
-class FeeFilterRounder
-{
-public:
-    /** Create new FeeFilterRounder */
-    FeeFilterRounder(const CFeeRate& minIncrementalFee);
-
-    /** Quantize a minimum fee for privacy purpose before broadcast **/
-    CAmount round(CAmount currentMinFee);
-
-private:
-    std::set<double> feeset;
-    FastRandomContext insecure_rand;
-};
 #endif /*BITCOIN_POLICYESTIMATOR_H */

src/protocol.cpp

@@ -35,7 +35,6 @@ const char *FILTERADD="filteradd";
 const char *FILTERCLEAR="filterclear";
 const char *REJECT="reject";
 const char *SENDHEADERS="sendheaders";
-const char *FEEFILTER="feefilter";
 const char *SENDCMPCT="sendcmpct";
 const char *CMPCTBLOCK="cmpctblock";
 const char *GETBLOCKTXN="getblocktxn";
@@ -125,7 +124,6 @@ const static std::string allNetMessageTypes[] = {
     NetMsgType::FILTERCLEAR,
     NetMsgType::REJECT,
     NetMsgType::SENDHEADERS,
-    NetMsgType::FEEFILTER,
     NetMsgType::SENDCMPCT,
     NetMsgType::CMPCTBLOCK,
     NetMsgType::GETBLOCKTXN,

src/protocol.h

@@ -215,12 +215,6 @@ extern const char *REJECT;
  * @see https://bitcoin.org/en/developer-reference#sendheaders
  */
 extern const char *SENDHEADERS;
-/**
- * The feefilter message tells the receiving peer not to inv us any txs
- * which do not meet the specified min fee rate.
- * @since protocol version 70013 as described by BIP133
- */
-extern const char *FEEFILTER;
 
 /**
  * Contains a 1-byte bool and 8-byte LE version number.

src/validation.cpp

@@ -17,12 +17,10 @@
 #include "consensus/validation.h"
 #include "hash.h"
 #include "init.h"
-#include "policy/fees.h"
 #include "policy/policy.h"
 #include "pow.h"
 #include "primitives/block.h"
 #include "primitives/transaction.h"
-#include "random.h"
 #include "script/script.h"
 #include "script/sigcache.h"
 #include "script/standard.h"
@@ -98,7 +96,6 @@ CFeeRate minRelayTxFee = CFeeRate(DEFAULT_MIN_RELAY_TX_FEE);
 CAmount maxTxFee = DEFAULT_TRANSACTION_MAXFEE;
 
 CTxMemPool mempool(::minRelayTxFee);
-FeeFilterRounder filterRounder(::minRelayTxFee);
 std::map<uint256, int64_t> mapRejectedBlocks GUARDED_BY(cs_main);
 
 static void CheckBlockIndex(const Consensus::Params& consensusParams);

src/validation.h

@@ -117,10 +117,6 @@ static const unsigned int INVENTORY_BROADCAST_INTERVAL = 5;
 /** Maximum number of inventory items to send per transmission.
  *  Limits the impact of low-fee transaction floods. */
 static const unsigned int INVENTORY_BROADCAST_MAX = 7 * INVENTORY_BROADCAST_INTERVAL;
-/** Average delay between feefilter broadcasts in seconds. */
-static const unsigned int AVG_FEEFILTER_BROADCAST_INTERVAL = 10 * 60;
-/** Maximum feefilter broadcast delay after significant change. */
-static const unsigned int MAX_FEEFILTER_CHANGE_DELAY = 5 * 60;
 /** Block download timeout base, expressed in millionths of the block interval (i.e. 2.5 min) */
 static const int64_t BLOCK_DOWNLOAD_TIMEOUT_BASE = 1000000;
 /** Additional block download timeout per parallel downloading peer (i.e. 1.25 min) */
@@ -144,8 +140,6 @@ static const unsigned int DEFAULT_BANSCORE_THRESHOLD = 100;
 
 /** Default for -mempoolreplacement */
 static const bool DEFAULT_ENABLE_REPLACEMENT = false;
-/** Default for using fee filter */
-static const bool DEFAULT_FEEFILTER = true;
 
 /** Maximum number of headers to announce when relaying blocks with headers message.*/
 static const unsigned int MAX_BLOCKS_TO_ANNOUNCE = 8;

src/version.h

@@ -38,9 +38,6 @@ static const int NO_BLOOM_VERSION = 70201;
 //! "sendheaders" command and announcing blocks with headers starts with this version
 static const int SENDHEADERS_VERSION = 70201;
 
-//! "feefilter" tells peers to filter invs to you by fee starts with this version
-static const int FEEFILTER_VERSION = 99999; // disable for now (clarify deployment later)
-
 //! DIP0001 was activated in this version
 static const int DIP0001_PROTOCOL_VERSION = 70208;