diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 35747552f..f590db5ef 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -10,6 +10,7 @@ #include "net.h" #include "net_processing.h" #include "netbase.h" +#include "policy/policy.h" #include "protocol.h" #include "sync.h" #include "timedata.h" @@ -417,6 +418,7 @@ UniValue getnetworkinfo(const JSONRPCRequest& request) " ,...\n" " ],\n" " \"relayfee\": x.xxxxxxxx, (numeric) minimum relay fee for non-free transactions in " + CURRENCY_UNIT + "/kB\n" + " \"incrementalfee\": x.xxxxxxxx, (numeric) minimum fee increment for mempool limiting or BIP 125 replacement in " + CURRENCY_UNIT + "/kB\n" " \"localaddresses\": [ (array) list of local addresses\n" " {\n" " \"address\": \"xxxx\", (string) network address\n" @@ -447,6 +449,7 @@ UniValue getnetworkinfo(const JSONRPCRequest& request) } obj.push_back(Pair("networks", GetNetworksInfo())); obj.push_back(Pair("relayfee", ValueFromAmount(::minRelayTxFee.GetFeePerK()))); + obj.push_back(Pair("incrementalfee", ValueFromAmount(::incrementalRelayFee.GetFeePerK()))); UniValue localAddresses(UniValue::VARR); { LOCK(cs_mapLocalHost); diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 2f74f57d0..f856d8a91 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -9,6 +9,7 @@ #include "consensus/validation.h" #include "validation.h" #include "miner.h" +#include "policy/policy.h" #include "pubkey.h" #include "script/standard.h" #include "txmempool.h" @@ -24,6 +25,8 @@ BOOST_FIXTURE_TEST_SUITE(miner_tests, TestingSetup) +static CFeeRate blockMinFeeRate = CFeeRate(DEFAULT_BLOCK_MIN_TX_FEE); + static struct { unsigned char extranonce; @@ -112,7 +115,7 @@ void TestPackageSelection(const CChainParams& chainparams, CScript scriptPubKey, BOOST_CHECK(pblocktemplate->block.vtx[2]->GetHash() == hashHighFeeTx); BOOST_CHECK(pblocktemplate->block.vtx[3]->GetHash() == hashMediumFeeTx); - // Test that a package below the min relay fee doesn't get included + // Test that a package below the block min tx fee doesn't get included tx.vin[0].prevout.hash = hashHighFeeTx; tx.vout[0].nValue = 5000000000LL - 1000 - 50000; // 0 fee uint256 hashFreeTx = tx.GetHash(); @@ -120,8 +123,8 @@ void TestPackageSelection(const CChainParams& chainparams, CScript scriptPubKey, size_t freeTxSize = ::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION); // Calculate a fee on child transaction that will put the package just - // below the min relay fee (assuming 1 child tx of the same size). - CAmount feeToUse = minRelayTxFee.GetFee(2*freeTxSize) - 1; + // below the block min tx fee (assuming 1 child tx of the same size). + CAmount feeToUse = blockMinFeeRate.GetFee(2*freeTxSize) - 1; tx.vin[0].prevout.hash = hashFreeTx; tx.vout[0].nValue = 5000000000LL - 1000 - 50000 - feeToUse; @@ -158,7 +161,7 @@ void TestPackageSelection(const CChainParams& chainparams, CScript scriptPubKey, // This tx can't be mined by itself tx.vin[0].prevout.hash = hashFreeTx2; tx.vout.resize(1); - feeToUse = minRelayTxFee.GetFee(freeTxSize); + feeToUse = blockMinFeeRate.GetFee(freeTxSize); tx.vout[0].nValue = 5000000000LL - 100000000 - feeToUse; uint256 hashLowFeeTx2 = tx.GetHash(); mempool.addUnchecked(hashLowFeeTx2, entry.Fee(feeToUse).SpendsCoinbase(false).FromTx(tx)); diff --git a/src/validation.cpp b/src/validation.cpp index d499d7a0d..3142b3291 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -932,14 +932,14 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState& state, const C // Finally in addition to paying more fees than the conflicts the // new transaction must pay for its own bandwidth. CAmount nDeltaFees = nModifiedFees - nConflictingFees; - if (nDeltaFees < ::minRelayTxFee.GetFee(nSize)) + if (nDeltaFees < ::incrementalRelayFee.GetFee(nSize)) { return state.DoS(0, false, REJECT_INSUFFICIENTFEE, "insufficient fee", false, strprintf("rejecting replacement %s, not enough additional fees to relay; %s < %s", hash.ToString(), FormatMoney(nDeltaFees), - FormatMoney(::minRelayTxFee.GetFee(nSize)))); + FormatMoney(::incrementalRelayFee.GetFee(nSize)))); } } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 87bf3ecbb..bfc738afa 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2682,8 +2682,8 @@ UniValue bumpfee(const JSONRPCRequest& request) "By default, the new fee will be calculated automatically using estimatefee.\n" "The user can specify a confirmation target for estimatefee.\n" "Alternatively, the user can specify totalFee, or use RPC setpaytxfee to set a higher fee rate.\n" - "At a minimum, the new fee rate must be high enough to pay a new relay fee (relay fee amount returned\n" - "by getnetworkinfo RPC) and to enter the node's mempool.\n" + "At a minimum, the new fee rate must be high enough to pay an additional new relay fee (incrementalfee\n" + "returned by getnetworkinfo) to enter the node's mempool.\n" "\nArguments:\n" "1. txid (string, required) The txid to be bumped\n" "2. options (object, optional)\n" @@ -2704,8 +2704,8 @@ UniValue bumpfee(const JSONRPCRequest& request) "\nResult:\n" "{\n" " \"txid\": \"value\", (string) The id of the new transaction\n" - " \"oldfee\": n, (numeric) Fee of the replaced transaction\n" - " \"fee\": n, (numeric) Fee of the new transaction\n" + " \"origfee\": n, (numeric) Fee of the replaced transaction\n" + " \"fee\": n, (numeric) Fee of the new transaction\n" "}\n" "\nExamples:\n" "\nBump the fee, get the new transaction\'s txid\n" + @@ -2769,6 +2769,10 @@ UniValue bumpfee(const JSONRPCRequest& request) throw JSONRPCError(RPC_MISC_ERROR, "Transaction does not have a change output"); } + // signature sizes can vary by a byte, so add 1 for each input when calculating the new fee + int64_t txSize = GetVirtualTransactionSize(*(wtx.tx)); + const int64_t maxNewTxSize = txSize + wtx.tx->vin.size(); + // optional parameters bool specifiedConfirmTarget = false; int newConfirmTarget = nTxConfirmTarget; @@ -2794,10 +2798,11 @@ UniValue bumpfee(const JSONRPCRequest& request) } } else if (options.exists("totalFee")) { totalFee = options["totalFee"].get_int64(); - if (totalFee <= 0) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid totalFee (cannot be <= 0)"); - } else if (totalFee > maxTxFee) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid totalFee (cannot be higher than maxTxFee)"); + CAmount requiredFee = CWallet::GetRequiredFee(maxNewTxSize); + if (totalFee < requiredFee ) { + throw JSONRPCError(RPC_INVALID_PARAMETER, + strprintf("Insufficient totalFee (cannot be less than required fee %s)", + FormatMoney(requiredFee))); } } @@ -2806,42 +2811,53 @@ UniValue bumpfee(const JSONRPCRequest& request) } } - // signature sizes can vary by a byte, so add 1 for each input when calculating the new fee - int64_t txSize = GetVirtualTransactionSize(*(wtx.tx)); - const int64_t maxNewTxSize = txSize + wtx.tx->vin.size(); - // calculate the old fee and fee-rate CAmount nOldFee = wtx.GetDebit(ISMINE_SPENDABLE) - wtx.tx->GetValueOut(); CFeeRate nOldFeeRate(nOldFee, txSize); CAmount nNewFee; CFeeRate nNewFeeRate; + // The wallet uses a conservative WALLET_INCREMENTAL_RELAY_FEE value to + // future proof against changes to network wide policy for incremental relay + // fee that our node may not be aware of. + CFeeRate walletIncrementalRelayFee = CFeeRate(WALLET_INCREMENTAL_RELAY_FEE); + if (::incrementalRelayFee > walletIncrementalRelayFee) { + walletIncrementalRelayFee = ::incrementalRelayFee; + } if (totalFee > 0) { - CAmount minTotalFee = nOldFeeRate.GetFee(maxNewTxSize) + minRelayTxFee.GetFee(maxNewTxSize); + CAmount minTotalFee = nOldFeeRate.GetFee(maxNewTxSize) + ::incrementalRelayFee.GetFee(maxNewTxSize); if (totalFee < minTotalFee) { - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid totalFee, must be at least %s (oldFee %s + relayFee %s)", FormatMoney(minTotalFee), nOldFeeRate.GetFee(maxNewTxSize), minRelayTxFee.GetFee(maxNewTxSize))); + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Insufficient totalFee, must be at least %s (oldFee %s + incrementalFee %s)", + FormatMoney(minTotalFee), FormatMoney(nOldFeeRate.GetFee(maxNewTxSize)), FormatMoney(::incrementalRelayFee.GetFee(maxNewTxSize)))); } nNewFee = totalFee; nNewFeeRate = CFeeRate(totalFee, maxNewTxSize); } else { - // use the user-defined payTxFee if possible, otherwise use smartfee / fallbackfee - if (!specifiedConfirmTarget && payTxFee.GetFeePerK() != 0) { - nNewFeeRate = payTxFee; - } else { - nNewFeeRate = mempool.estimateSmartFee(newConfirmTarget); + // if user specified a confirm target then don't consider any global payTxFee + if (specifiedConfirmTarget) { + nNewFee = CWallet::GetMinimumFee(maxNewTxSize, newConfirmTarget, mempool, CAmount(0)); } - if (nNewFeeRate.GetFeePerK() == 0) { - nNewFeeRate = CWallet::fallbackFee; + // otherwise use the regular wallet logic to select payTxFee or default confirm target + else { + nNewFee = CWallet::GetMinimumFee(maxNewTxSize, newConfirmTarget, mempool); } - // new fee rate must be at least old rate + minimum relay rate - if (nNewFeeRate.GetFeePerK() < nOldFeeRate.GetFeePerK() + ::minRelayTxFee.GetFeePerK()) { - nNewFeeRate = CFeeRate(nOldFeeRate.GetFeePerK() + ::minRelayTxFee.GetFeePerK()); - } + nNewFeeRate = CFeeRate(nNewFee, maxNewTxSize); - nNewFee = nNewFeeRate.GetFee(maxNewTxSize); + // New fee rate must be at least old rate + minimum incremental relay rate + if (nNewFeeRate.GetFeePerK() < nOldFeeRate.GetFeePerK() + walletIncrementalRelayFee.GetFeePerK()) { + nNewFeeRate = CFeeRate(nOldFeeRate.GetFeePerK() + walletIncrementalRelayFee.GetFeePerK()); + nNewFee = nNewFeeRate.GetFee(maxNewTxSize); + } } + // Check that in all cases the new fee doesn't violate maxTxFee + if (nNewFee > maxTxFee) { + throw JSONRPCError(RPC_MISC_ERROR, + strprintf("Specified or calculated fee %s is too high (cannot be higher than maxTxFee %s)", + FormatMoney(nNewFee), FormatMoney(maxTxFee))); + } + // check that fee rate is higher than mempool's minimum fee // (no point in bumping fee if we know that the new tx won't be accepted to the mempool) // This may occur if the user set TotalFee or paytxfee too low, if fallbackfee is too low, or, perhaps, @@ -2864,7 +2880,7 @@ UniValue bumpfee(const JSONRPCRequest& request) // If the output would become dust, discard it (converting the dust to fee) poutput->nValue -= nDelta; - if (poutput->nValue <= poutput->GetDustThreshold(::minRelayTxFee)) { + if (poutput->nValue <= poutput->GetDustThreshold(::dustRelayFee)) { LogPrint("rpc", "Bumping fee and discarding dust output\n"); nNewFee += poutput->nValue; tx.vout.erase(tx.vout.begin() + nOutput); @@ -2913,7 +2929,7 @@ UniValue bumpfee(const JSONRPCRequest& request) UniValue result(UniValue::VOBJ); result.push_back(Pair("txid", wtxBumped.GetHash().GetHex())); - result.push_back(Pair("oldfee", ValueFromAmount(nOldFee))); + result.push_back(Pair("origfee", ValueFromAmount(nOldFee))); result.push_back(Pair("fee", ValueFromAmount(nNewFee))); return result; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 517202a9b..9a5f35b6e 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2802,8 +2802,13 @@ CAmount CWallet::GetRequiredFee(unsigned int nTxBytes) CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarget, const CTxMemPool& pool) { - // payTxFee is user-set "I want to pay this much" - CAmount nFeeNeeded = payTxFee.GetFee(nTxBytes); + // payTxFee is the user-set global for desired feerate + return GetMinimumFee(nTxBytes, nConfirmTarget, pool, payTxFee.GetFee(nTxBytes)); +} + +CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarget, const CTxMemPool& pool, CAmount targetFee) +{ + CAmount nFeeNeeded = targetFee; // User didn't set: use -txconfirmtarget to estimate... if (nFeeNeeded == 0) { int estimateFoundTarget = nConfirmTarget; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index a7fc05b62..200ec0cba 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -48,6 +48,8 @@ static const CAmount DEFAULT_TRANSACTION_FEE = 0; static const CAmount DEFAULT_FALLBACK_FEE = 20000; //! -mintxfee default static const CAmount DEFAULT_TRANSACTION_MINFEE = 1000; +//! minimum recommended increment for BIP 125 replacement txs +static const CAmount WALLET_INCREMENTAL_RELAY_FEE = 5000; //! target minimum change amount static const CAmount MIN_CHANGE = CENT; //! final minimum change amount after paying for fees @@ -802,6 +804,11 @@ public: * and the required fee */ static CAmount GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarget, const CTxMemPool& pool); + /** + * Estimate the minimum fee considering required fee and targetFee or if 0 + * then fee estimation for nConfirmTarget + */ + static CAmount GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarget, const CTxMemPool& pool, CAmount targetFee); /** * Return the minimum required fee taking into account the * floating relay fee and user set minimum transaction fee