Skip to content

Commit d6f304a

Browse files
stevenroosestevenroose
committed
Merge #684: Reduce relay min rate to 100 sat/vkB
bf2f1c6 Reduce min relay to 100 sat/vkB (Gregory Sanders) d8dc3f2 modify p2p_feefilter test to catch rounding error (Gregory Sanders) 130d190 Disallow implicit conversion for CFeeRate constructor (Gregory Sanders) 36fceda feefilter: Compute the absolute fee rather than stored rate to match mempool acceptance logic (Gregory Sanders) Pull request description: Includes an upstream bugfix for feefilter/mempool divergence when the rate is not `%1000=0`. Does not lower the minimum wallet default! This must be chosen manually for the time being(`settxfee`), at the risk of not propagating to the rest of the network if peers have not also updated. Fee estimation is also not yet updated to support lower feerates. Tree-SHA512: 561b47d6f5fd898eec56db41189a6c514e54682b817996f2e188d791a9234102df52bd191e02dced468df30da5816b9acfde53154939180db9bf23bd20256a53
2 parents 073cb3c + bf2f1c6 commit d6f304a

File tree

11 files changed

+41
-23
lines changed

11 files changed

+41
-23
lines changed

‎src/net_processing.cpp‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3694,10 +3694,10 @@ bool PeerLogicValidation::SendMessages(CNode* pto)
36943694
if (fSendTrickle && pto->fSendMempool) {
36953695
auto vtxinfo = mempool.infoAll();
36963696
pto->fSendMempool = false;
3697-
CAmount filterrate = 0;
3697+
CFeeRate filterrate;
36983698
{
36993699
LOCK(pto->cs_feeFilter);
3700-
filterrate = pto->minFeeFilter;
3700+
filterrate = CFeeRate(pto->minFeeFilter);
37013701
}
37023702

37033703
LOCK(pto->cs_filter);
@@ -3706,9 +3706,9 @@ bool PeerLogicValidation::SendMessages(CNode* pto)
37063706
const uint256& hash = txinfo.tx->GetHash();
37073707
CInv inv(MSG_TX, hash);
37083708
pto->setInventoryTxToSend.erase(hash);
3709-
if (filterrate) {
3710-
if (txinfo.feeRate.GetFeePerK() < filterrate)
3711-
continue;
3709+
// Don't send transactions that peers will not put into their mempool
3710+
if (txinfo.fee < filterrate.GetFee(txinfo.vsize)) {
3711+
continue;
37123712
}
37133713
if (pto->pfilter) {
37143714
if (!pto->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue;
@@ -3731,10 +3731,10 @@ bool PeerLogicValidation::SendMessages(CNode* pto)
37313731
for (std::set<uint256>::iterator it = pto->setInventoryTxToSend.begin(); it != pto->setInventoryTxToSend.end(); it++) {
37323732
vInvTx.push_back(it);
37333733
}
3734-
CAmount filterrate = 0;
3734+
CFeeRate filterrate;
37353735
{
37363736
LOCK(pto->cs_feeFilter);
3737-
filterrate = pto->minFeeFilter;
3737+
filterrate = CFeeRate(pto->minFeeFilter);
37383738
}
37393739
// Topologically and fee-rate sort the inventory we send for privacy and priority reasons.
37403740
// A heap is used so that not all items need sorting if only a few are being sent.
@@ -3761,7 +3761,8 @@ bool PeerLogicValidation::SendMessages(CNode* pto)
37613761
if (!txinfo.tx) {
37623762
continue;
37633763
}
3764-
if (filterrate && txinfo.feeRate.GetFeePerK() < filterrate) {
3764+
// Peer told you to not send transactions at that feerate? Don't bother sending it.
3765+
if (txinfo.fee < filterrate.GetFee(txinfo.vsize)) {
37653766
continue;
37663767
}
37673768
if (pto->pfilter && !pto->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue;

‎src/policy/feerate.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ class CFeeRate
2525
/** Fee rate of 0 satoshis per kB */
2626
CFeeRate() : nSatoshisPerK(0) { }
2727
template<typename I>
28-
CFeeRate(const I _nSatoshisPerK): nSatoshisPerK(_nSatoshisPerK) {
28+
explicit CFeeRate(const I _nSatoshisPerK): nSatoshisPerK(_nSatoshisPerK) {
2929
// We've previously had bugs creep in from silent double->int conversion...
3030
static_assert(std::is_integral<I>::value, "CFeeRate should be used without floats");
3131
}

‎src/policy/policy.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ extern CAsset policyAsset;
2222
/** Default for -blockmaxweight, which controls the range of block weights the mining code will create **/
2323
static const unsigned int DEFAULT_BLOCK_MAX_WEIGHT = MAX_BLOCK_WEIGHT - 4000;
2424
/** Default for -blockmintxfee, which sets the minimum feerate for a transaction in blocks created by mining code **/
25-
static const unsigned int DEFAULT_BLOCK_MIN_TX_FEE = 1000;
25+
static const unsigned int DEFAULT_BLOCK_MIN_TX_FEE = 100;
2626
/** The maximum weight for transactions we're willing to relay/mine */
2727
static const unsigned int MAX_STANDARD_TX_WEIGHT = 400000;
2828
/** The minimum non-witness size for transactions we're willing to relay/mine (1 segwit input + 1 P2WPKH output = 82 bytes) */
@@ -34,7 +34,7 @@ static const unsigned int MAX_STANDARD_TX_SIGOPS_COST = MAX_BLOCK_SIGOPS_COST/5;
3434
/** Default for -maxmempool, maximum megabytes of mempool memory usage */
3535
static const unsigned int DEFAULT_MAX_MEMPOOL_SIZE = 300;
3636
/** Default for -incrementalrelayfee, which sets the minimum feerate increase for mempool limiting or BIP 125 replacement **/
37-
static const unsigned int DEFAULT_INCREMENTAL_RELAY_FEE = 1000;
37+
static const unsigned int DEFAULT_INCREMENTAL_RELAY_FEE = 100;
3838
/** Default for -bytespersigop */
3939
static const unsigned int DEFAULT_BYTES_PER_SIGOP = 20;
4040
/** The maximum number of witness stack items in a standard P2WSH script */

‎src/test/test_bitcoin.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,9 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName, const std::st
6060
// Set policy asset for correct fee output generation
6161
policyAsset = CAsset();
6262

63+
// For unit tests, increase minrelay to "normal" 1000 sat/vkB
64+
incrementalRelayFee = CFeeRate(1000);
65+
6366
noui_connect();
6467
}
6568

‎src/txmempool.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -855,7 +855,7 @@ void CTxMemPool::queryHashes(std::vector<uint256>& vtxid)
855855
}
856856

857857
static TxMempoolInfo GetInfo(CTxMemPool::indexed_transaction_set::const_iterator it) {
858-
return TxMempoolInfo{it->GetSharedTx(), it->GetTime(), CFeeRate(it->GetFee(), it->GetTxSize()), it->GetModifiedFee() - it->GetFee()};
858+
return TxMempoolInfo{it->GetSharedTx(), it->GetTime(), it->GetFee(), it->GetTxSize(), it->GetModifiedFee() - it->GetFee()};
859859
}
860860

861861
std::vector<TxMempoolInfo> CTxMemPool::infoAll() const

‎src/txmempool.h‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -338,8 +338,11 @@ struct TxMempoolInfo
338338
/** Time the transaction entered the mempool. */
339339
int64_t nTime;
340340

341-
/** Feerate of the transaction. */
342-
CFeeRate feeRate;
341+
/** Fee of the transaction. */
342+
CAmount fee;
343+
344+
/** Virtual size of the transaction. */
345+
size_t vsize;
343346

344347
/** The fee delta. */
345348
int64_t nFeeDelta;

‎src/validation.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ static const bool DEFAULT_WHITELISTRELAY = true;
5555
/** Default for -whitelistforcerelay. */
5656
static const bool DEFAULT_WHITELISTFORCERELAY = false;
5757
/** Default for -minrelaytxfee, minimum relay fee for transactions */
58-
static const unsigned int DEFAULT_MIN_RELAY_TX_FEE = 1000;
58+
static const unsigned int DEFAULT_MIN_RELAY_TX_FEE = 100;
5959
//! -maxtxfee default
6060
static const CAmount DEFAULT_TRANSACTION_MAXFEE = COIN / 10;
6161
//! Discourage users to set fees higher than this amount (in satoshis) per kB

‎src/wallet/rpcwallet.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2625,7 +2625,7 @@ static UniValue settxfee(const JSONRPCRequest& request)
26252625

26262626
CAmount nAmount = AmountFromValue(request.params[0]);
26272627
CFeeRate tx_fee_rate(nAmount, 1000);
2628-
if (tx_fee_rate == 0) {
2628+
if (tx_fee_rate == CFeeRate(0)) {
26292629
// automatic selection
26302630
} else if (tx_fee_rate < ::minRelayTxFee) {
26312631
throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("txfee cannot be less than min relay tx fee (%s)", ::minRelayTxFee.ToString()));

‎test/bitcoin_functional/functional/test_framework/util.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,7 @@ def initialize_datadir(dirname, n):
323323
f.write("scriptprefix=196\n")
324324
f.write("bech32_hrp=bcrt\n")
325325
f.write("con_dyna_deploy_start="+str(2**31)+"\n") # Never starts
326+
f.write("minrelaytxfee=0.00001\n")
326327
os.makedirs(os.path.join(datadir, 'stderr'), exist_ok=True)
327328
os.makedirs(os.path.join(datadir, 'stdout'), exist_ok=True)
328329
return datadir

‎test/functional/p2p_feefilter.py‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,12 @@ def clear_invs(self):
4141
class FeeFilterTest(BitcoinTestFramework):
4242
def set_test_params(self):
4343
self.num_nodes = 2
44+
# We lower the various required feerates for this test
45+
# to catch a corner-case where feefilter used to slightly undercut
46+
# mempool and wallet feerate calculation based on GetFee
47+
# rounding down 3 places, leading to stranded transactions.
48+
# See issue #16499
49+
self.extra_args = [["-minrelaytxfee=0.00000100", "-mintxfee=0.00000100"]]*self.num_nodes
4450

4551
def skip_test_if_missing_module(self):
4652
self.skip_if_no_wallet()
@@ -54,22 +60,25 @@ def run_test(self):
5460

5561
self.nodes[0].add_p2p_connection(TestP2PConn())
5662

57-
# Test that invs are received for all txs at feerate of 20 sat/byte
58-
node1.settxfee(Decimal("0.00020000"))
63+
# Test that invs are received by test connection for all txs at
64+
# feerate of .2 sat/byte
65+
node1.settxfee(Decimal("0.00000200"))
5966
txids = [node1.sendtoaddress(node1.getnewaddress(), 1) for x in range(3)]
6067
assert(allInvsMatch(txids, self.nodes[0].p2p))
6168
self.nodes[0].p2p.clear_invs()
6269

63-
# Set a filter of 15 sat/byte
64-
self.nodes[0].p2p.send_and_ping(msg_feefilter(15000))
70+
# Set a filter of .15 sat/byte on test connection
71+
self.nodes[0].p2p.send_and_ping(msg_feefilter(150))
6572

66-
# Test that txs are still being received (paying 20 sat/byte)
73+
# Test that txs are still being received by test connection (paying .15 sat/byte)
74+
node1.settxfee(Decimal("0.00000150"))
6775
txids = [node1.sendtoaddress(node1.getnewaddress(), 1) for x in range(3)]
6876
assert(allInvsMatch(txids, self.nodes[0].p2p))
6977
self.nodes[0].p2p.clear_invs()
7078

71-
# Change tx fee rate to 10 sat/byte and test they are no longer received
72-
node1.settxfee(Decimal("0.00010000"))
79+
# Change tx fee rate to .1 sat/byte and test they are no longer received
80+
# by the test connection
81+
node1.settxfee(Decimal("0.00000100"))
7382
[node1.sendtoaddress(node1.getnewaddress(), 1) for x in range(3)]
7483
sync_mempools(self.nodes) # must be sure node 0 has received all txs
7584

0 commit comments

Comments
 (0)