From 8b4c419ed6c49859cdf4e53761532caa15815a91 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Wed, 11 Apr 2018 17:16:43 +0200 Subject: [PATCH] Revert "Merge #7542: Implement "feefilter" P2P message" (#2025) This reverts commit 11ac70af9e3e8209c8069096d281ba4b2f3d7fe3. --- doc/bips.md | 1 - qa/pull-tester/rpc-tests.py | 1 - qa/rpc-tests/p2p-feefilter.py | 115 ------------------------ qa/rpc-tests/p2p-leaktests.py | 1 - qa/rpc-tests/test_framework/mininode.py | 19 ---- src/init.cpp | 2 - src/net.cpp | 3 - src/net.h | 6 -- src/net_processing.cpp | 56 ------------ src/policy/fees.cpp | 18 ---- src/policy/fees.h | 14 --- src/protocol.cpp | 2 - src/protocol.h | 6 -- src/validation.cpp | 3 - src/validation.h | 6 -- src/version.h | 3 - 16 files changed, 256 deletions(-) delete mode 100755 qa/rpc-tests/p2p-feefilter.py diff --git a/doc/bips.md b/doc/bips.md index b01f0f3215..87a33b0f84 100644 --- a/doc/bips.md +++ b/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)). diff --git a/qa/pull-tester/rpc-tests.py b/qa/pull-tester/rpc-tests.py index e1faaca089..7690ee5511 100755 --- a/qa/pull-tester/rpc-tests.py +++ b/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', diff --git a/qa/rpc-tests/p2p-feefilter.py b/qa/rpc-tests/p2p-feefilter.py deleted file mode 100755 index 86ce0b42e6..0000000000 --- a/qa/rpc-tests/p2p-feefilter.py +++ /dev/null @@ -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() diff --git a/qa/rpc-tests/p2p-leaktests.py b/qa/rpc-tests/p2p-leaktests.py index 41ca84d779..b9d8e851a6 100755 --- a/qa/rpc-tests/p2p-leaktests.py +++ b/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) diff --git a/qa/rpc-tests/test_framework/mininode.py b/qa/rpc-tests/test_framework/mininode.py index 9e86f9aa4a..3deff983cb 100755 --- a/qa/rpc-tests/test_framework/mininode.py +++ b/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("", _("Specify data directory")); strUsage += HelpMessageOpt("-dbcache=", 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=", _("Imports blocks from external blk000??.dat file on startup")); strUsage += HelpMessageOpt("-maxorphantx=", strprintf(_("Keep at most unconnectable transactions in memory (default: %u)"), DEFAULT_MAX_ORPHAN_TRANSACTIONS)); strUsage += HelpMessageOpt("-maxmempool=", strprintf(_("Keep the transaction memory pool below megabytes (default: %u)"), DEFAULT_MAX_MEMPOOL_SIZE)); diff --git a/src/net.cpp b/src/net.cpp index 73dc6e8227..31d78519b7 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2799,9 +2799,6 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn fPingQueued = false; fMasternode = false; nMinPingUsecTime = std::numeric_limits::max(); - minFeeFilter = 0; - lastSentFeeFilter = 0; - nextSendTimeFeeFilter = 0; fPauseRecv = false; fPauseSend = false; nProcessQueueSize = 0; diff --git a/src/net.h b/src/net.h index 5741926989..f472958e8f 100644 --- a/src/net.h +++ b/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 nMinPingUsecTime; // Whether a ping is requested. std::atomic 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(); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 151e7fd581..4012b636a0 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -50,8 +50,6 @@ std::atomic nTimeBestReceived(0); // Used only to inform the wallet of when we last received a block -extern FeeFilterRounder filterRounder; - struct IteratorComparator { template @@ -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& 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& 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& interr for (std::set::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& 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& 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; } diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 34835c803a..c067277796 100644 --- a/src/policy/fees.cpp +++ b/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::iterator it = feeset.lower_bound(currentMinFee); - if ((it != feeset.begin() && insecure_rand.rand32() % 3 != 0) || it == feeset.end()) { - it--; - } - return *it; -} diff --git a/src/policy/fees.h b/src/policy/fees.h index 26cfd94659..eaae3e594f 100644 --- a/src/policy/fees.h +++ b/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 feeset; - FastRandomContext insecure_rand; -}; #endif /*BITCOIN_POLICYESTIMATOR_H */ diff --git a/src/protocol.cpp b/src/protocol.cpp index 1ac44645cb..4cd85e24de 100644 --- a/src/protocol.cpp +++ b/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, diff --git a/src/protocol.h b/src/protocol.h index 8cdc1a734e..dc7389dc88 100644 --- a/src/protocol.h +++ b/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. diff --git a/src/validation.cpp b/src/validation.cpp index 805f3ae284..ba84a93370 100644 --- a/src/validation.cpp +++ b/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 mapRejectedBlocks GUARDED_BY(cs_main); static void CheckBlockIndex(const Consensus::Params& consensusParams); diff --git a/src/validation.h b/src/validation.h index f5c57a4388..18d7b8b888 100644 --- a/src/validation.h +++ b/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; diff --git a/src/version.h b/src/version.h index 9686b88639..b1657a9168 100644 --- a/src/version.h +++ b/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;